[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [Qemu-devel] [PATCH v2 07/14] sm501: Fix device endia

From: BALATON Zoltan
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH v2 07/14] sm501: Fix device endianness
Date: Fri, 3 Mar 2017 21:11:05 +0100 (CET)
User-agent: Alpine 2.20 (BSF 67 2015-01-07)

On Fri, 3 Mar 2017, Peter Maydell wrote:
On 3 March 2017 at 02:15, BALATON Zoltan <address@hidden> wrote:
Maybe it's not correct but works for everything I could test better than the
original code (which was broken even for the SH images one can find) so I
think we could just go with this until someone complains and provides a test
case. I've given up on trying to understand it because I don't really know
this device and SH so I could only go by the images I could find and they
seem to like it this way.

So what cases have you tested? The Linux kernel seems to support:
* sh embedded device, little endian
* PCI card, little endian host
* PCI card, big endian host
and there are also
* 16 bit pixel format
* 32 bit pixel format

which makes 6 different combinations.
(It's a shame there's no sh4 BE code to run on this.)

As mentioned before I've used this image to test SH:


with video=800x600-16 kernel parameter where changing -16 to different bit depths (8, 32) reproduces the problem. On ppc I've tested with MorphOS which uses 16 bpp, Linux and U-boot which I think uses 8 bit. These run on real hardware so I think if they work the emulation should be OK.

You've said before that PCI is likely always little endian and SH embedded is LE too so unless something uses the device in BE mode (which we don't emulate) setting it to little endian should be correct. For frame buffer endianness I can only go with what I've seen clients doing and my patch does what worked for the above on SH and PPC. I can't prove it's correct but works for the systems I've targeted and also fixes the SH case which seemed to be broken.

Looking at how the SWAP_FB_ENDIAN flag gets set:
* for the r2d board it is set always
* for PCI devices, it is set only if big-endian

I suspect that what we actually have here is something
* for the PCI device it's always little endian (and the
  code ought not to need to depend on TARGET_WORDS_BIGENDIAN)
* sysbus device may be more complicated

Also I note there's an SM501_ENDIAN_CONTROL register on the
device, which presumably if written changes the behaviour.

I've seen this but it's not emulated so it can be ignored for now. The spec
also says that default is little endian so for regs at least

Yes, that makes sense. So we should be modelling this as an "always
little endian device".

The changes you have to the memory region ops achieve this for
the registers implemented in this device itself. It looks like
SYSBUS_OHCI is already always little endian. You also need to
change the argument to the serial_mm_init() call to pass

OK, will do that.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]