linux-kernel - Re: [PATCH v3 2/2] staging: iio: light: isl29018: use regmap for register access

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <4F906729.9030700@nvidia.com>
Date:	Fri, 20 Apr 2012 00:57:37 +0530
From:	Laxman Dewangan <ldewangan@...dia.com>
To:	Grant Grundler <grundler@...omium.org>
CC:	"jic23@....ac.uk" <jic23@....ac.uk>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"max@...o.at" <max@...o.at>,
	"jbrenner@...sinc.com" <jbrenner@...sinc.com>,
	"bfreed@...omium.org" <bfreed@...omium.org>,
	"lars@...afoo.de" <lars@...afoo.de>,
	"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
	"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/2] staging: iio: light: isl29018: use regmap for
 register access

On Thursday 19 April 2012 11:22 PM, Grant Grundler wrote:
> On Thu, Apr 19, 2012 at 4:15 AM, Laxman Dewangan<ldewangan@...dia.com>  wrote:
>> Using regmap for accessing register through i2c bus. This will
>> remove the code for caching registers, read-modify-write logics.
>> Also it will provide the debugfs feature to dump register
>> through regmap debugfs.
>>
>> Signed-off-by: Laxman Dewangan<ldewangan@...dia.com>
> Reviewed-by: Grant Grundler<grundler@...omium.org>
>
> Laxman,
> Thanks for reposting this patch. I was talking with Bryan Freed and it
> looks like the caching of registers will change the usage of
> ADD_COMMAND1. More details below.
>
Thanks for review.  ADD_COMMAND1 have the intrrupt flag bit. More 
details below.

>> +static bool is_volatile_reg(struct device *dev, unsigned int reg)
>> +{
>> +       switch (reg) {
>> +       case ISL29018_REG_ADD_DATA_LSB:
>> +       case ISL29018_REG_ADD_DATA_MSB:
>> +       case ISL29018_REG_ADD_COMMAND1:
>> +       case ISL29018_REG_TEST:
> Of these four, I think only ADD_COMMAND1 wasn't treated as volatile in
> the old code. Am I overlooking something?
>
> My concern is only about the additional I2C read traffic this patch
> might generate. It's possible *some* bits in that register are
> volatile and we could previously ignore them.
>

Register ADD_COMMAND1, bit 2 is interrupt flag bit which shows the 
interrupt status and hence we can not cache it.
The ISL29018 datasheet says:
Interrupt flag; Bit 2. This is the status bit of the interrupt.
The bit is set to logic high when the interrupt thresholds
have been triggered, and logic low when not yet triggered.
Once triggered, INT pin stays low and the status bit stays
high. Both interrupt pin and the status bit are automatically
cleared at the end of Command Register I transfer.


--
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/