Re: [PATCH] spi: rzv2h-rspi: fix incorrect readl() accessor for 8-bit RX path

[Geert Uytterhoeven]

Hi Fabrizio,

On Tue, 9 Jun 2026 at 23:04, Fabrizio Castro
<fabrizio.castro.jz@xxxxxxxxxxx> wrote:
> > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> > On Tue, 9 Jun 2026 at 16:14, Felix Gu <ustc.gu@xxxxxxxxx> wrote:
> >  On Mon, Jun 8, 2026 at 3:55 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> > > > On Fri, 5 Jun 2026 at 17:26, Felix Gu <ustc.gu@xxxxxxxxx> wrote:
> > > > >
> > > > > Fixes: 8b61c8919dff ("spi: Add driver for the RZ/V2H(P) RSPI IP")
> > > > > Signed-off-by: Felix Gu <ustc.gu@xxxxxxxxx>
> > > >
> > > > According to Chapter 7.5 Serial Peripheral Interface (RSPI) Subsection
> > > > 7.5.2.1 List of Registers, the SPI Data Register supports access sizes
> > > > of 8, 16, and 32 bits.
> > > >
> > > > However, the "Access Size [bits]*1" column header has a foot note:
> > > >
> > > >     "Note 1.  The read access size is fixed at 32 bits."
> > > >
> > > > Hence that means the rzv2h_rspi_rx_u8() function as generated by the
> > > > RZV2H_RSPI_RX() macro is correct, but rzv2h_rspi_rx_u16() is not?
>
> True, it looks like rzv2h_rspi_rx_u8() is correct but rzv2h_rspi_rx_u16()
> is not, and could use improving.
>
> > > >
> > > > Also, readw() in rzv2h_rx_irq_handler() is wrong, too?
>
> That looks correct to me. The access size for SPSR is 8 or 16 bit
> (as specified in the list of registers from section 7.5.2.1, and also
> repeated in section 7.5.2.2.18), and we are reading it with readw?

The confusion part is that the footnote in the column header of section
7.5.2.1 appears to apply to all registers, while apparently it does
not apply to e.g. SPSR (and e.g. SPDCR?).

BTW, e.g. SPDCR and SPDCR2 also support\ only access sizes of 8 and 16
bits, but 7.5.2.2.16 still states that read access size of SPDCR2 is
fixed at 32 bits. while 7.5.2.2.15 does not have such a note for SPDCR.
Those registers are aligned to 4-byte boundaries, though, unlike SPSR.

So there is room for improvement in the documentation.

> > > Thanks for the detailed explanation.
> > > I didn't check the reference, just thought it's a little weird here.
> > >
> > > Although the column header has a footnote to indicate the access
> > > size is fixed at 32bits.
> > > But in 7.5.2.2 register description, it specifies that for registers like
> > > SPDR, SPCR, SPSCR, SPCMD, SPDCR2, SPSSR, their access
> > > size are fixed at 32bits, but for SPSR, there is no footnote to specify
> > > the access size.
> >
> > True.
> >
> > > And SPSR is 0x52, readl on the register cause alignment issue?
> >
> > Good point. Let's wait for Fabrizio's response...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds