Re: [PATCH v8 3/4] clocksource/drivers: Add a goldfish-timer clocksource

[Laurent Vivier]

Le 16/01/2022 à 11:44, Arnd Bergmann a écrit :
> On Sat, Jan 15, 2022 at 8:32 PM Laurent Vivier <laurent@xxxxxxxxx> wrote:
>
> > +
> > +/* goldfish endianness depends on CPU endianness */
> > +#ifdef CONFIG_CPU_BIG_ENDIAN
> > +#define goldfish_ioread32 ioread32be
> > +#define goldfish_iowrite32 iowrite32be
> > +#else
> > +#define goldfish_ioread32 ioread32
> > +#define goldfish_iowrite32 iowrite32
> > +#endif
>
> This is not what I meant here, as you are breaking big-endian support
> for all other
> architectures in the process.
>
> On architectures that support both big-endian and little-endian kernels, devices
> (including emulated ones) can't know which type of kernel you are running, so
> this has to be fixed by architecture. Ideally this macro should be in
> an architecture
> specific header file, but you can also just make this a check for m68k and  hope
> that qemu doesn't duplicate this bug on architectures that gain support for this
> driver in the future.

Virtio has exactly the same problem in the past (prio to v1.0), and it uses __LITTLE_ENDIAN in 
virtio_legacy_is_little_endian() to know the endianness of the device.

So the code can become:

#ifdef __LITTLE_ENDIAN
#define goldfish_ioread32 ioread32
#define goldfish_iowrite32 iowrite32
#else
#define goldfish_ioread32 ioread32be
#define goldfish_iowrite32 iowrite32be
#endif

What do you prefer: CONFIG_M68K or __LITTLE_ENDIAN?

Thanks,
Laurent