linux-kernel - Re: [PATCH] regmap: Allow read_reg

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: <542C2C34.6080408@metafoo.de>
Date:	Wed, 01 Oct 2014 18:30:44 +0200
From:	Lars-Peter Clausen <lars@...afoo.de>
To:	Dan Murphy <dmurphy@...com>, broonie@...nel.org,
	gregkh@...uxfoundation.org
CC:	linux-kernel@...r.kernel.org, linux-omap@...r.kernel.org
Subject: Re: [PATCH] regmap: Allow read_reg_mask to be 0

On 10/01/2014 01:39 PM, Dan Murphy wrote:
> Lars
>
> On 09/30/2014 04:29 PM, Lars-Peter Clausen wrote:
>> On 09/30/2014 11:18 PM, Dan Murphy wrote:
>>> Lars
>>>
>>> On 09/30/2014 04:03 PM, Lars-Peter Clausen wrote:
>>>> On 09/30/2014 06:07 PM, Dan Murphy wrote:
>>>>> There may be spi devices that do not require a
>>>>> register read mask to read the registers.
>>>>>
>>>>> Currently the code sets the read mask based on
>>>>> a non-zero value passed in from the driver or if that
>>>>> value is 0 sets the read mask to 0x80.
>>>>
>>>> It only sets it to the bus default if both read_flag_mask and write_flag_mask are 0. The assumption is that both of them being zero is a invalid configuration and either of them (or both) have to be non-zero for proper operation, since otherwise the device can't tell the difference between a read and a write.
>>>>
>>>> Do you have a device where both the read and the write mask is 0?
>>>>
>>>> - Lars
>>>
>>> Yes I do have a device that the read/write mask are both 0.
>>>
>>> The device, which is already in production, has a specific control register that sets either the reading or writing of the rest of the registers.
>>>
>>> Here is the data sheet
>>>
>>> http://www.ti.com/lit/ds/symlink/afe4403.pdf
>>>
>>> See page 61 control0.
>>>
>>> Driver is written for this part just want to get this lead patch in or maybe an alternate solution.
>>
>> Looking at this the generic SPI regmap implementation might not necessarily be the best thing to use here and you are probably better of implementing either your own regmap bus or reg_read/reg_write callbacks that automatically set/clear the SPI_READ bit in the control register depending on the operation.
>>
>> - Lars
>
> I am not sure implementing a different SPI regmap implementation is really the best thing.
> I am handling this control bit toggling in the peripheral driver.

This is the very thing that regmap is supposed to hide, the specifics of how 
the read or write access happens. regmap_read() is supposed to do a read, 
regmap_write() is supposed to do a write. If regmap_read() only does a write 
if you previously did a regmap_write(regmap, CTRL0, SPI_READ); then the 
semantics of the interface are broken. This means you can't use generic 
infrastructure and it will confuse people who read and review your code.

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