linux-kernel - Re: [PATCH v11 2/3] iio: adc: max14001: New driver

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: <1b269e4f-5a2d-4de9-8757-cd4218d36bac@gmail.com>
Date: Tue, 23 Sep 2025 23:40:55 -0300
From: Marilene Andrade Garcia <marilene.agarcia@...il.com>
To: David Lechner <dlechner@...libre.com>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Cc: Kim Seer Paller <kimseer.paller@...log.com>,
 Lars-Peter Clausen <lars@...afoo.de>,
 Michael Hennerich <Michael.Hennerich@...log.com>,
 Jonathan Cameron <jic23@...nel.org>, Nuno Sá
 <nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Marcelo Schmitt <marcelo.schmitt1@...il.com>,
 Marcelo Schmitt <Marcelo.Schmitt@...log.com>,
 Ceclan Dumitru <dumitru.ceclan@...log.com>,
 Jonathan Santos <Jonathan.Santos@...log.com>,
 Dragos Bogdan <dragos.bogdan@...log.com>
Subject: Re: [PATCH v11 2/3] iio: adc: max14001: New driver

On 23/09/2025 11:27, David Lechner wrote:
> On 9/22/25 7:56 PM, Marilene Andrade Garcia wrote:
>> On 16/09/2025 15:04, David Lechner wrote:
>>> On 9/15/25 5:16 PM, Marilene Andrade Garcia wrote:
> 
> ...
> 
> 
> In general, please trim out extra stuff like I've done here when you
> reply. It makes it easier to find the important parts. I hope I didn't
> miss any of your questions.
> 
>>>> +    /*
>>>> +     * The following buffers will be bit-reversed during device
>>>> +     * communication, because the device transmits and receives data
>>>> +     * LSB-first.
>>>> +     * DMA (thus cache coherency maintenance) requires the transfer
>>>> +     * buffers to live in their own cache lines.
>>>> +     */
>>>> +    __be16 spi_tx_buffer __aligned(IIO_DMA_MINALIGN);
>>>> +    __be16 spi_rx_buffer;
>>>
>>> These would no longer be strictly big-endian, so we could
>>> just make them u16 with a note in the comments.
>>
>> Hello David, I have some doubts that I would like to clarify before sending v12. Since I am not able to test the case using SPI_LSB_FIRST, I noticed that you suggest saving the data as __le in this case. Just out of curiosity, if I use SPI_LSB_FIRST, would saving the data as __be lead to errors?
> 
> My thinking is that since we are sending things out 1 byte at a time, in order
> for the least significant bit of 16 bits to be sent first, the least significant
> byte has to be sent first. So will little-endian, the byte containing the least
> significant bit of the 16 bits will be first in memory.
> 
> __be is just a naming convention and doesn't actually cause any bytes to
> be swapped in memory. It just lets readers of the code know that the
> value stored there has to be handled carefully because it may not be
> cpu-endian. It would be confusing to readers to store a little-endian
> value in a __be16 variable, but technically, no, it would not cause any
> errors.
> 
> This is why I suggested to make it u16. It is still wrong but it is
> equally wrong in both cases. If you still want to use __be16 though,
> you could make a union instead.
> 
> union {
> 	__be16 be;
> 	__le16 le;
> } spi_tx_buffer;
> union {
> 	__be16 be;
> 	__le16 le;
> } spi_rx_buffer;
> 
>>>
>>> The scoped_guard() looks a bit odd with the extra indent. I would write
>>> it like this instead:
>>>
>>>
>>>
>>>      case IIO_CHAN_INFO_RAW: {
>>>          guard(mutex)(&st->lock);
>>>
>>>          ret = regmap_read(st->regmap, MAX14001_REG_ADC, val);
>>>          if (ret)
>>>              return ret;
>>>
>>>          return IIO_VAL_INT;
>>>      }
>>>
>>
>> Ok, thank you. But since I removed the mutex, as it was mentioned in the first comments, I should not use the guard, right? At least not for now.
>>
> 
> Correct. The regmap_read() has something similar internally already.
> 
Ok, Thank you for the answers.

Best Regards,
Marilene