Re: [PATCH 1/3] mfd: max8997: use regmap to access registers

[Krzysztof Kozlowski <k.kozlowski@samsung.com>]

On Tue, 2014-03-11 at 15:59 +0100, Robert Baldyga wrote:
> >> @@ -180,12 +167,12 @@ static struct irq_chip max8997_irq_chip = { 
> >> static irqreturn_t max8997_irq_thread(int irq, void *data) {
> >> struct max8997_dev *max8997 = data; -	u8
> >> irq_reg[MAX8997_IRQ_GROUP_NR] = {}; -	u8 irq_src; +	unsigned int
> >> irq_reg[MAX8997_IRQ_GROUP_NR] = {}; +	unsigned int irq_src; int
> >> ret; int i, cur_irq;
> >> 
> >> -	ret = max8997_read_reg(max8997->i2c, MAX8997_REG_INTSRC, 
> >> &irq_src); +	ret = regmap_read(max8997->regmap,
> >> MAX8997_REG_INTSRC, &irq_src); if (ret < 0) { dev_err(max8997->dev,
> >> "Failed to read interrupt source: %d\n", ret); @@ -194,8 +181,9 @@
> >> static irqreturn_t max8997_irq_thread(int irq, void *data)
> >> 
> >> if (irq_src & MAX8997_IRQSRC_PMIC) { /* PMIC INT1 ~ INT4 */ - 
> >> max8997_bulk_read(max8997->i2c, MAX8997_REG_INT1, 4, - 
> >> &irq_reg[PMIC_INT1]); +		for (i = 0; i < 4; ++i) + 
> >> regmap_read(max8997->regmap, +				MAX8997_REG_INT1+i, 
> >> &irq_reg[PMIC_INT1+i]);
> > 
> > Can't you use here one bulk read instead of 4xregmap_read()?
> 
> Mixing regmap_read and regmap_bulk_read is not good idea, because the
> first function returns register value as unsigned int, but the second
> returns reg value to each single byte. So it would need to do some
> additional operations, and makes things more complicated.

The only mixing I see would be in reading FLASHSTATUS register which
could be resolved with:

if (irq_src & MAX8997_IRQSRC_FLASH) {
       /* Flash Status Interrupt */
       unsigned int data;
       ret = regmap_read(max8997->regmap, MAX8997_REG_FLASHSTATUS,
		       &data);
       irq_reg[FLASH_STATUS] = data;
}

This isn't more complicated than replacing bulk read with a loop :).

Best regards,
Krzysztof