Le 02/08/2023 à 23:32, Philippe Mathieu-Daudé a écrit :
Hi Jean-Christophe,
On 2/8/23 23:08, Jean-Christophe Dubois wrote:
* Add Addr and size definition for all i.MX6UL devices in i.MX6UL
header file.
I'm OK with your patch, but some addr/size are added, while other
are changed. It is hard to review. Having one patch for changes
and another for additions would help review.
I tried to set addresses and sizes following the order set for devices
in the i.MX6UL reference manual. I found it easier to follow then (and
make reasonably sure I didn't miss some).
I certainly understand that the reorder is annoying for the review. I
can try to do as you intended but I am not sure the reorder review would
be really easier.
* Use those newly defined named constants whenever possible.
* Standardize the way we init a familly of unimplemented devices
- SAI
- PWM (add missing PWM instances)
- CAN
* Add/rework few comments
Signed-off-by: Jean-Christophe Dubois <j...@tribudubois.net>
---
hw/arm/fsl-imx6ul.c | 149 +++++++++++++++++++++++------------
include/hw/arm/fsl-imx6ul.h | 150 +++++++++++++++++++++++++++++++++---
2 files changed, 240 insertions(+), 59 deletions(-)
diff --git a/include/hw/arm/fsl-imx6ul.h b/include/hw/arm/fsl-imx6ul.h
index 9ee15ae38d..5d381740ef 100644
--- a/include/hw/arm/fsl-imx6ul.h
+++ b/include/hw/arm/fsl-imx6ul.h
For example here:
+ FSL_IMX6UL_SNVS_HP_SIZE = (4 * KiB),
+
FSL_IMX6UL_USBPHY2_ADDR = 0x020CA000,
- FSL_IMX6UL_USBPHY2_SIZE = (4 * 1024),
- FSL_IMX6UL_USBPHY1_SIZE = (4 * 1024),
+ FSL_IMX6UL_USBPHYn_SIZE = 0x100,
Don't we also need:
Well, I did not modify the i.MX USB PHY file by itself. It is a fact
that the last i.MX USB PHY register is at 0x80 offset and a 0x1000
memory region for the device is certainly oversized even if the
processor memory map is actually provisioning a 0x1000 address space
between distinct USB PHY devices. My intent in lowering the device
register region size as close to the real size as possible was that in
case a device was not "implemented" in Qemu we could just map it as an
unimplemented device (allowing dummy access to the register range) but
get some kind of platform "bus error" if the software was trying to
access some "registers" in the upper part of the memory region (as you
would on the real hardware?).
So 0x1000 is not wrong per se as the USB phy device implementation code
is logging the illegal access when software is doing access over 0x80
offset. This would just not trigger a processor hardware access fault
(when it could/should?).
-- >8 --
--- a/hw/usb/imx-usb-phy.c
+++ b/hw/usb/imx-usb-phy.c
@@ -210,7 +210,7 @@ static void imx_usbphy_realize(DeviceState *dev,
Error **errp)
IMXUSBPHYState *s = IMX_USBPHY(dev);
memory_region_init_io(&s->iomem, OBJECT(s), &imx_usbphy_ops, s,
- "imx-usbphy", 0x1000);
+ "imx-usbphy", 0x100);
sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem);
}
---
?
Thanks,
Phil.