On Mon, Jul 04, 2022 at 12:23:23PM +0200, Laurent Vivier wrote:
> On 04/07/2022 12:21, Richard Henderson wrote:
> > On 7/4/22 15:46, Laurent Vivier wrote:
> > > On 04/07/2022 11:59, Richard Henderson wrote:
> > > > On 7/4/22 02:58, Stafford Horne wrote:
> > > > > -static const MemoryRegionOps goldfish_rtc_ops = {
> > > > > - .read = goldfish_rtc_read,
> > > > > - .write = goldfish_rtc_write,
> > > > > - .endianness = DEVICE_NATIVE_ENDIAN,
> > > > > - .valid = {
> > > > > - .min_access_size = 4,
> > > > > - .max_access_size = 4
> > > > > - }
> > > > > +static const MemoryRegionOps goldfish_rtc_ops[3] = {
> > > > > + [DEVICE_NATIVE_ENDIAN] = {
> > > > > + .read = goldfish_rtc_read,
> > > > > + .write = goldfish_rtc_write,
> > > > > + .endianness = DEVICE_NATIVE_ENDIAN,
> > > > > + .valid = {
> > > > > + .min_access_size = 4,
> > > > > + .max_access_size = 4
> > > > > + }
> > > > > + },
> > > > > + [DEVICE_LITTLE_ENDIAN] = {
> > > > > + .read = goldfish_rtc_read,
> > > > > + .write = goldfish_rtc_write,
> > > > > + .endianness = DEVICE_LITTLE_ENDIAN,
> > > > > + .valid = {
> > > > > + .min_access_size = 4,
> > > > > + .max_access_size = 4
> > > > > + }
> > > > > + },
> > > > > + [DEVICE_BIG_ENDIAN] = {
> > > > > + .read = goldfish_rtc_read,
> > > > > + .write = goldfish_rtc_write,
> > > > > + .endianness = DEVICE_BIG_ENDIAN,
> > > > > + .valid = {
> > > > > + .min_access_size = 4,
> > > > > + .max_access_size = 4
> > > > > + }
> > > > > + },
> > > > > };
> > > >
> > > > You don't need 3 copies, only big and little.
> > > >
> > > > > +static Property goldfish_rtc_properties[] = {
> > > > > + DEFINE_PROP_UINT8("endianness", GoldfishRTCState, endianness,
> > > > > + DEVICE_NATIVE_ENDIAN),
> > > > > + DEFINE_PROP_END_OF_LIST(),
> > > > > +};
> > > >
> > > > ... and I think the clear desire for default is little-endian.
> > > > I would make the property be bool, and add a comment that this
> > > > is only for m68k compatibility, so don't use it in new code.
> > >
> > > m68k doesn't really need this.
> > >
> > > kernel with the m68k virt machine and goldfish device supports
> > > "native" mode so I think it's not needed to add another layer of
> > > complexity for it.
> >
> > "Another level"? I'm talking about removing "native", and only having
> > "big" and "little", which is less complexity.
>
> "Less complexity" is to keep only native. I'm not against the change, I'm
> just saying it's not needed by m68k.
Hi Laurent,
I would agree if we only had m68k. But I am making this change so that OpenRISC
(another big-endian architecture) could use this. In the OpenRISC case we want
to use this as little-endian so no kernel updates would be needed.
So in the end we will have the following qemu platforms:
riscv{LE}--------------->goldfish_rtc{LE}
mips-longsoon3{LE}------>goldfish_rtc{LE}
openrisc{BE}------------>goldfish_rtc{LE} (LE to BE conversion done in driver)
m68k{BE}---------------->goldfish_rtc{BE} (only big-endian user)
-Stafford
