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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-id: <1394551211.24263.34.camel@AMDC1943>
Date:	Tue, 11 Mar 2014 16:20:11 +0100
From:	Krzysztof Kozlowski <k.kozlowski@...sung.com>
To:	Robert Baldyga <r.baldyga@...sung.com>
Cc:	sameo@...ux.intel.com, lee.jones@...aro.org,
	myungjoo.ham@...sung.com, cw00.choi@...sung.com,
	dmitry.torokhov@...il.com, cooloney@...il.com, rpurdie@...ys.net,
	dbaryshkov@...il.com, dwmw2@...radead.org, lgirdwood@...il.com,
	broonie@...nel.org, a.zummo@...ertech.it,
	paul.gortmaker@...driver.com, linux-kernel@...r.kernel.org,
	linux-input@...r.kernel.org, linux-leds@...r.kernel.org,
	rtc-linux@...glegroups.com, m.szyprowski@...sung.com
Subject: Re: [PATCH 1/3] mfd: max8997: use regmap to access registers

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/