RE: [PATCH v9 4/6] iio: adc: ad4691: add SPI offload support

[Sabau, Radu bogdan]

> -----Original Message-----
> From: Jonathan Cameron <jic23@xxxxxxxxxx>
> Sent: Thursday, May 7, 2026 6:11 PM
> To: Sabau, Radu bogdan <Radu.Sabau@xxxxxxxxxx>
> Cc: Lars-Peter Clausen <lars@xxxxxxxxxx>; Hennerich, Michael
> <Michael.Hennerich@xxxxxxxxxx>; David Lechner <dlechner@xxxxxxxxxxxx>;
> Sa, Nuno <Nuno.Sa@xxxxxxxxxx>; Andy Shevchenko <andy@xxxxxxxxxx>;
> Rob Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>;
> Conor Dooley <conor+dt@xxxxxxxxxx>; Uwe Kleine-König
> <ukleinek@xxxxxxxxxx>; Liam Girdwood <lgirdwood@xxxxxxxxx>; Mark Brown
> <broonie@xxxxxxxxxx>; Linus Walleij <linusw@xxxxxxxxxx>; Bartosz
> Golaszewski <brgl@xxxxxxxxxx>; Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>;
> Jonathan Corbet <corbet@xxxxxxx>; Shuah Khan
> <skhan@xxxxxxxxxxxxxxxxxxx>; linux-iio@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-
> pwm@xxxxxxxxxxxxxxx; linux-gpio@xxxxxxxxxxxxxxx; linux-doc@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v9 4/6] iio: adc: ad4691: add SPI offload support
> 
> 
> > > +		/* TX: address phase, CS stays asserted into data phase */
> > > +		st->scan_xfers[2 * k].tx_buf = offload->tx_cmd[k];
> > > +		st->scan_xfers[2 * k].len = sizeof(offload->tx_cmd[k]);
> > > +		st->scan_xfers[2 * k].bits_per_word = bpw;
> >
> > "When bits_per_word is greater than 8 (like bpw = 16 here), the SPI
> framework
> > treats tx_buf as an array of native 16-bit words.
> > On little-endian architectures, the controller will byte-swap the data before
> > transmitting it. Will using a u8 array and put_unaligned_be16() result in the
> > command bytes being reversed on the wire?"
> >
> > Switched to cpu_to_be16() assigned directly into __be16 scan_tx[],
> > matching the non-offload path. This makes the intended wire format
> > self-evident and sidesteps the byte-ordering question entirely.
> 
> This confuses me a bit because the SPI controller should work with
> native endian and from that generate the expected big endian on the wire.
> 
> So on a little endian host byte order in address space is LH but it will
> write top bit of H first thus the ADC channel address needs to be in the
> second byte.
> On a big endian host despite the ordering in memory being HL, the top
> bit of H is still written first thus in needs to be in the first byte.
> 
> 
> If you using cpu_to_be16() to assign a 16 bit value swapping only on little
> endian
> and start with the cmd in L on little endian you'll end up with LH swapped to
> HL and on big endian HL but the little endian SPI controller should then swap
> it again sending what it thinks is the high byte first (L) whereas the big endian
> system will send H.
> 
> Upshot. I think the field should be native endian. If a byte swap is needed
> it should be unconditional and not rely on endianness of the host.
> 

Correct. With bits_per_word=16 the SPI controller reads tx_buf as a native
16-bit word. cpu_to_be16() stores BE bytes, but on an LE host those bytes are
read back as a different native integer, sending the wrong byte first.

scan_tx is declared as u16. Both offload paths store the exact wire value as a
plain native u16 — no endianness macro. Native storage is self-consistent:
store X, read back X, SPI shifts X out MSB-first → correct wire bytes on any
host. Trace for AVG_IN(0) = 0x8201, expected wire [0x82, 0x01]:

  cpu_to_be16(0x8201), LE host → native 0x0182 → wire [0x01, 0x82]  (incorrect)
  (u16)0x8201,         LE host → native 0x8201 → wire [0x82, 0x01]  (correct)
  (u16)0x8201,         BE host → native 0x8201 → wire [0x82, 0x01]  (correct)

Manual offload: TX and RX are full-duplex (same clock cycles, shared xfer):

    st->scan_tx[k] = (u16)(AD4691_ADC_CHAN(bit) << 8);
    /* e.g. ch=0: 0x8000 → wire [0x80, 0x00] */

CNV burst offload: TX and RX are separate xfers, both use bits_per_word=bpw:

    st->scan_tx[k] = 0x8000 | AD4691_AVG_IN(bit);
    /* e.g. AVG_IN(0): 0x8201 → wire [0x82, 0x01] */

The state-reset transfer uses a u8 buffer with bits_per_word=8 (default);
put_unaligned_be16() writes bytes in memory order, which SPI sends as-is.