linux-kernel - Re: [PATCH 2/2] regmap: sdw: Remove 8-bit value size restriction

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: <487b7647-8304-8e36-00f2-c9ba3aa50004@linux.intel.com>
Date:   Fri, 13 Jan 2023 10:57:39 -0600
From:   Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To:     Charles Keepax <ckeepax@...nsource.cirrus.com>
Cc:     Mark Brown <broonie@...nel.org>, vkoul@...nel.org,
        yung-chuan.liao@...ux.intel.com, sanyog.r.kale@...el.com,
        linux-kernel@...r.kernel.org, patches@...nsource.cirrus.com
Subject: Re: [PATCH 2/2] regmap: sdw: Remove 8-bit value size restriction



On 1/13/23 05:02, Charles Keepax wrote:
> On Thu, Jan 12, 2023 at 02:19:29PM -0600, Pierre-Louis Bossart wrote:
>> On 1/12/23 13:50, Mark Brown wrote:
>>> On Thu, Jan 12, 2023 at 12:43:46PM -0600, Pierre-Louis Bossart wrote:
>>>> On 1/12/23 12:14, Mark Brown wrote:
>>>
>>>>> The regmap gather_write() operation allows the bus to take two buffers,
>>>>> one for the register and one for the value, rather than requiring the
>>>>> core combine everything into a single buffer (mainly useful for large
>>>>> transfers like firmware downloads).
>>>
>>>> Right, but that's not supported in SoundWire. sdw_nwrite() will only
>>>> work with consecutive addresses - and the auto-increment is handled in
>>>> software, not hardware.
>>>
>>> No, that's exactly what this is for.  It's for the *register address*
>>> being in a separate buffer, the data is then a sequence of consecutive
>>> register values.>
>>>> What's suggested here is to use the first element of reg_buf, which begs
>>>> the question how different this is from a regular write. If there was a
>>>> discontinuity in reg_buf then this wouldn't work at all.
>>>
>>> reg_buf contains the address of exactly one register.
>>
>> So what's the difference with a plain write() of N data?
> 
> There are two back end interfaces in regmap, the reg_write/read
> and the plain write/read. Both have currently have some
> limitations when dealing with SoundWire.
> 
> The reg_write/reg_read can only deal with a single register
> at a time, which is really far from ideal, since it means
> all transactions will be broken up into individual registers
> at the regmap level, mostly depriving the SoundWire side
> of the opportunity to do things like a BRA transfer if it
> deems that suitable. And denying users the ability to use the
> regmap_raw_read/write API at all.
> 
> The write/read interface allows us to pass the full transaction
> through, but does have the downside it copies the address around
> a bit more and does some pointless endian swaps on big endian
> systems. This interface is generally used by buses like I2C/SPI
> where there is no actual concept of a register address only a
> buffer of bytes to be sent/read, thus prefers to pass a single
> working buffer if it sensibly can. I went with this solution
> because it enables all the functionality and the downside is
> fairly minimal, apart from looking a little clunky as you note.

The change from reg_write/read_reg to write/read seems ok, what I was
asking about was the gather_write.

+	.write = regmap_sdw_write,
+	.gather_write = regmap_sdw_gather_write,
+	.read = regmap_sdw_read,

what happens if you only have .write and .read? What does the
.gather_write help with if you only use only address?