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

From: Krzysztof Kozlowski
Date: Tue Mar 11 2014 - 11:20:30 EST


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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/