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

[Fabrizio Castro]

Hi Geert,

> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: 10 June 2026 08:53
> To: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx>
> Cc: Felix Gu <ustc.gu@xxxxxxxxx>; Mark Brown <broonie@xxxxxxxxxx>; linux-spi@xxxxxxxxxxxxxxx; linux-
> renesas-soc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] spi: rzv2h-rspi: fix incorrect readl() accessor for 8-bit RX path
> 
> 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.

Indeed.

Kind regards,
Fab

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