Re: [PATCH v2 1/5] Refactor i.MX6UL processor code

[Philippe Mathieu-Daudé]

Hi Jean-Christophe,

On 29/7/23 23:17, Jean-Christophe Dubois wrote:

* Add Addr and size definition for all i.MX6UL devices in i.MX6UL header file.
* 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 
---
  hw/arm/fsl-imx6ul.c | 149 
  include/hw/arm/fsl-imx6ul.h | 149 +---
  2 files changed, 239 insertions(+), 59 deletions(-)




  enum FslIMX6ULMemoryMap {
  FSL_IMX6UL_MMDC_ADDR= 0x8000,
-FSL_IMX6UL_MMDC_SIZE= 2 * 1024 * 1024 * 1024UL,
+FSL_IMX6UL_MMDC_SIZE= (2 * 1024 * 1024 * 1024UL),
  
  FSL_IMX6UL_QSPI1_MEM_ADDR   = 0x6000,

+FSL_IMX6UL_QSPI1_MEM_SIZE   = (256 * 1024 * 1024UL),
+
  FSL_IMX6UL_EIM_ALIAS_ADDR   = 0x5800,
+FSL_IMX6UL_EIM_ALIAS_SIZE   = (128 * 1024 * 1024UL),
+
  FSL_IMX6UL_EIM_CS_ADDR  = 0x5000,
+FSL_IMX6UL_EIM_CS_SIZE  = (128 * 1024 * 1024UL),
+
  FSL_IMX6UL_AES_ENCRYPT_ADDR = 0x1000,
+FSL_IMX6UL_AES_ENCRYPT_SIZE = (1024 * 1024UL),
+
  FSL_IMX6UL_QSPI1_RX_ADDR= 0x0C00,
+FSL_IMX6UL_QSPI1_RX_SIZE= (32 * 1024 * 1024UL),


Please use the KiB / MiB definitions from "qemu/units.h" (the
resulting code is easier to read. No need for parenthesis.

[PATCH v2 1/5] Refactor i.MX6UL processor code

[Jean-Christophe Dubois]

* Add Addr and size definition for all i.MX6UL devices in i.MX6UL header file.
* 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 
---
 hw/arm/fsl-imx6ul.c | 149 
 include/hw/arm/fsl-imx6ul.h | 149 +---
 2 files changed, 239 insertions(+), 59 deletions(-)

diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
index 2189dcbb72..910316b628 100644
--- a/hw/arm/fsl-imx6ul.c
+++ b/hw/arm/fsl-imx6ul.c
@@ -69,7 +69,7 @@ static void fsl_imx6ul_init(Object *obj)
 object_initialize_child(obj, "gpr", >gpr, TYPE_IMX7_GPR);
 
 /*
- * GPIOs 1 to 5
+ * GPIOs
  */
 for (i = 0; i < FSL_IMX6UL_NUM_GPIOS; i++) {
 snprintf(name, NAME_SIZE, "gpio%d", i);
@@ -77,7 +77,7 @@ static void fsl_imx6ul_init(Object *obj)
 }
 
 /*
- * GPT 1, 2
+ * GPTs
  */
 for (i = 0; i < FSL_IMX6UL_NUM_GPTS; i++) {
 snprintf(name, NAME_SIZE, "gpt%d", i);
@@ -85,7 +85,7 @@ static void fsl_imx6ul_init(Object *obj)
 }
 
 /*
- * EPIT 1, 2
+ * EPITs
  */
 for (i = 0; i < FSL_IMX6UL_NUM_EPITS; i++) {
 snprintf(name, NAME_SIZE, "epit%d", i + 1);
@@ -93,7 +93,7 @@ static void fsl_imx6ul_init(Object *obj)
 }
 
 /*
- * eCSPI
+ * eCSPIs
  */
 for (i = 0; i < FSL_IMX6UL_NUM_ECSPIS; i++) {
 snprintf(name, NAME_SIZE, "spi%d", i + 1);
@@ -101,7 +101,7 @@ static void fsl_imx6ul_init(Object *obj)
 }
 
 /*
- * I2C
+ * I2Cs
  */
 for (i = 0; i < FSL_IMX6UL_NUM_I2CS; i++) {
 snprintf(name, NAME_SIZE, "i2c%d", i + 1);
@@ -109,7 +109,7 @@ static void fsl_imx6ul_init(Object *obj)
 }
 
 /*
- * UART
+ * UARTs
  */
 for (i = 0; i < FSL_IMX6UL_NUM_UARTS; i++) {
 snprintf(name, NAME_SIZE, "uart%d", i);
@@ -117,25 +117,31 @@ static void fsl_imx6ul_init(Object *obj)
 }
 
 /*
- * Ethernet
+ * Ethernets
  */
 for (i = 0; i < FSL_IMX6UL_NUM_ETHS; i++) {
 snprintf(name, NAME_SIZE, "eth%d", i);
 object_initialize_child(obj, name, >eth[i], TYPE_IMX_ENET);
 }
 
-/* USB */
+/*
+ * USB PHYs
+ */
 for (i = 0; i < FSL_IMX6UL_NUM_USB_PHYS; i++) {
 snprintf(name, NAME_SIZE, "usbphy%d", i);
 object_initialize_child(obj, name, >usbphy[i], TYPE_IMX_USBPHY);
 }
+
+/*
+ * USBs
+ */
 for (i = 0; i < FSL_IMX6UL_NUM_USBS; i++) {
 snprintf(name, NAME_SIZE, "usb%d", i);
 object_initialize_child(obj, name, >usb[i], TYPE_CHIPIDEA);
 }
 
 /*
- * SDHCI
+ * SDHCIs
  */
 for (i = 0; i < FSL_IMX6UL_NUM_USDHCS; i++) {
 snprintf(name, NAME_SIZE, "usdhc%d", i);
@@ -143,7 +149,7 @@ static void fsl_imx6ul_init(Object *obj)
 }
 
 /*
- * Watchdog
+ * Watchdogs
  */
 for (i = 0; i < FSL_IMX6UL_NUM_WDTS; i++) {
 snprintf(name, NAME_SIZE, "wdt%d", i);
@@ -189,10 +195,10 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error 
**errp)
  * A7MPCORE DAP
  */
 create_unimplemented_device("a7mpcore-dap", FSL_IMX6UL_A7MPCORE_DAP_ADDR,
-0x10);
+FSL_IMX6UL_A7MPCORE_DAP_SIZE);
 
 /*
- * GPT 1, 2
+ * GPTs
  */
 for (i = 0; i < FSL_IMX6UL_NUM_GPTS; i++) {
 static const hwaddr FSL_IMX6UL_GPTn_ADDR[FSL_IMX6UL_NUM_GPTS] = {
@@ -217,7 +223,7 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error 
**errp)
 }
 
 /*
- * EPIT 1, 2
+ * EPITs
  */
 for (i = 0; i < FSL_IMX6UL_NUM_EPITS; i++) {
 static const hwaddr FSL_IMX6UL_EPITn_ADDR[FSL_IMX6UL_NUM_EPITS] = {
@@ -242,7 +248,7 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error 
**errp)
 }
 
 /*
- * GPIO
+ * GPIOs
  */
 for (i = 0; i < FSL_IMX6UL_NUM_GPIOS; i++) {
 static const hwaddr FSL_IMX6UL_GPIOn_ADDR[FSL_IMX6UL_NUM_GPIOS] = {
@@ -286,15 +292,10 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error 
**errp)
 /*
  * IOMUXC and IOMUXC_GPR
  */
-for (i = 0; i < 1; i++) {
-static const hwaddr FSL_IMX6UL_IOMUXCn_ADDR[FSL_IMX6UL_NUM_IOMUXCS] = {
-FSL_IMX6UL_IOMUXC_ADDR,
-FSL_IMX6UL_IOMUXC_GPR_ADDR,
-};
-
-snprintf(name, NAME_SIZE, "iomuxc%d", i);
-create_unimplemented_device(name, FSL_IMX6UL_IOMUXCn_ADDR[i], 0x4000);
-}
+create_unimplemented_device("iomuxc", FSL_IMX6UL_IOMUXC_ADDR,
+FSL_IMX6UL_IOMUXC_SIZE);
+create_unimplemented_device("iomuxc_gpr", FSL_IMX6UL_IOMUXC_GPR_ADDR,
+FSL_IMX6UL_IOMUXC_GPR_SIZE);
 
 /*
  * CCM
@@ -314,7 +315,9 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error 
**errp)