Hi Arnd, On Thu, Nov 19, 2020 at 4:54 PM Arnd Bergmann <arnd@xxxxxxxxxx> wrote: > On Thu, Nov 19, 2020 at 1:53 PM Geert Uytterhoeven > <geert+renesas@xxxxxxxxx> wrote: > > There is no reason to keep on using the __raw_{read,write}l() I/O > > accessors in Renesas ARM or SuperH driver code. Switch to using the > > plain {read,write}l() I/O accessors, to have a chance that this works on > > big-endian. > > > > Suggested-by: Arnd Bergmann <arnd@xxxxxxxxxx> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> > > I don't think this one is correct, as based on > > static void fsi_pio_push16(struct fsi_priv *fsi, u8 *_buf, int samples) > { > int i; > > if (fsi_is_enable_stream(fsi)) { > /* > * stream mode > * see > * fsi_pio_push_init() > */ > u32 *buf = (u32 *)_buf; > > for (i = 0; i < samples / 2; i++) > fsi_reg_write(fsi, DODT, buf[i]); > } else { > /* normal mode */ > u16 *buf = (u16 *)_buf; > > for (i = 0; i < samples; i++) > fsi_reg_write(fsi, DODT, ((u32)*(buf + i) << 8)); > } > } > > the same helper is used for both accessing endianness-sensitive > register values (which need the converrsion in writel()) and > possibly in-memory byte streams that need the __raw_writel() > behavior of copying the input bytes in sequence rather than sets of > native-endian 16-bit or 32-bit samples. Thanks, good catch! > > --- > > Assembler output difference on SuperH checked. > > I'm also not sure whether changing this breaks big-endian SuperH, > which defines the accessors differently from Arm and most other > architectures. On SH, this driver is only used on SH7724 systems. Compiling an ecovec24_defconfig kernel with CONFIG_CPU_BIG_ENDIAN=y shows that the same code (native 32-bit access) is generated for big-endian as for little-endian, which means that it indeed must be broken for one of them. But this is not changed by my patch. > Maybe better just mark the driver as 'depends on SH || !CPU_BIG_ENDIAN' > as it is clearly broken on big-endian Arm. "depends on !CPU_BIG_ENDIAN"? 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