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.

* 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:

-- >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.

Reply via email to