[PATCH v2 00/13] i.MX and SDHCI improvements

[PATCH v2 00/13] i.MX and SDHCI improvements

[Bernhard Beschow]

This series fixes some details in i.MX platform devices, improves SDHCI
compatibility with U-Boot and modernizes some code.

The first 5 patches are bugfixes 1/ resolving infinite loop in U-Boot esdhc
driver, 2/ fixing a character echoing issue in imx-serial, 3/ fixing IRQ sharing
issue in Designware PCIe emulation, and 4/ fixing GPIO level preservation across
resets in imx-gpio.

Patches 6 and 7 modernize SD card emulation by turning presence and
write-protect GPIOs into qdev GPIOs and then further allowing the GPIOs to be
inverted, just like device tree allows.

The rest of the series is cosmetics including turning DPRINTF() into trace
events which eases debugging.

v2:
* Drop redundant implementation of TYPE_OR_IRQ (David, Zoltan)
* Use absolute QOM paths when tracing in imx_gpio and imx_i2c (Phil)
* Trace hexadecimal values in imx_serial (Phil)
* Do NOT move inversion of presence and write-protect GPIOs since that changes
the internal logic of the device

Bernhard Beschow (13):
  hw/sd/sdhci: Set SDHC_NIS_DMA bit when appropriate
  hw/char/imx_serial: Fix reset value of UFCR register
  hw/char/imx_serial: Update all state before restarting ageing timer
  hw/pci-host/designware: Expose MSI IRQ
  hw/gpio/imx_gpio: Don't clear input GPIO values upon reset
  hw/sd/sd: Remove legacy sd_set_cb() in favor of GPIOs
  hw/sd/sd: Allow for inverting polarities of presence and write-protect
    GPIOs
  hw/char/imx_serial: Turn some DPRINTF() statements into trace events
  hw/timer/imx_gpt: Remove unused define
  tests/qtest/libqos: Reuse TYPE_IMX_I2C define
  hw/i2c/imx_i2c: Convert DPRINTF() to trace events
  hw/misc/imx6_src: Convert DPRINTF() to trace events
  hw/gpio/imx_gpio: Turn DPRINTF() into trace events

 include/hw/arm/fsl-imx6.h                  |  4 +-
 include/hw/arm/fsl-imx7.h                  |  4 +-
 include/hw/char/imx_serial.h               |  2 +-
 include/hw/pci-host/designware.h           |  1 +
 include/hw/sd/sdcard_legacy.h              |  1 -
 hw/arm/fsl-imx6.c                          | 13 ++++-
 hw/arm/fsl-imx7.c                          | 13 ++++-
 hw/char/imx_serial.c                       | 65 ++++++++++++++--------
 hw/gpio/imx_gpio.c                         | 19 +++----
 hw/i2c/imx_i2c.c                           | 21 ++-----
 hw/misc/imx6_src.c                         | 23 ++------
 hw/pci-host/designware.c                   |  7 +--
 hw/sd/sd.c                                 | 39 ++++++-------
 hw/sd/sdhci.c                              | 11 +++-
 hw/timer/imx_gpt.c                         |  4 --
 tests/qtest/libqos/arm-imx25-pdk-machine.c |  5 +-
 tests/qtest/libqos/i2c-imx.c               |  4 +-
 hw/arm/Kconfig                             |  2 +
 hw/char/trace-events                       |  5 ++
 hw/gpio/trace-events                       |  5 ++
 hw/i2c/trace-events                        |  5 ++
 hw/misc/trace-events                       |  6 ++
 22 files changed, 151 insertions(+), 108 deletions(-)

-- 
2.48.0

[PATCH v2 01/13] hw/sd/sdhci: Set SDHC_NIS_DMA bit when appropriate

[Bernhard Beschow]

In U-Boot, the fsl_esdhc[_imx] driver waits for both "transmit completed" and
"DMA" bits in esdhc_send_cmd_common() by means of DATA_COMPLETE constant. QEMU
currently misses to set the DMA bit which causes the driver to loop forever. Fix
that by setting the DMA bit if enabled when doing DMA block transfers.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/sd/sdhci.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 299cd4bc1b..a958c11497 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -665,12 +665,13 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
         }
     }
 
+    if (s->norintstsen & SDHC_NISEN_DMA) {
+        s->norintsts |= SDHC_NIS_DMA;
+    }
+
     if (s->blkcnt == 0) {
         sdhci_end_transfer(s);
     } else {
-        if (s->norintstsen & SDHC_NISEN_DMA) {
-            s->norintsts |= SDHC_NIS_DMA;
-        }
         sdhci_update_irq(s);
     }
 }
@@ -691,6 +692,10 @@ static void sdhci_sdma_transfer_single_block(SDHCIState *s)
     }
     s->blkcnt--;
 
+    if (s->norintstsen & SDHC_NISEN_DMA) {
+        s->norintsts |= SDHC_NIS_DMA;
+    }
+
     sdhci_end_transfer(s);
 }
 
-- 
2.48.0

Re: [PATCH v2 01/13] hw/sd/sdhci: Set SDHC_NIS_DMA bit when appropriate

[Michael Tokarev]

11.01.2025 21:36, Bernhard Beschow wrote:
> In U-Boot, the fsl_esdhc[_imx] driver waits for both "transmit completed" and
> "DMA" bits in esdhc_send_cmd_common() by means of DATA_COMPLETE constant. QEMU
> currently misses to set the DMA bit which causes the driver to loop forever. Fix
> that by setting the DMA bit if enabled when doing DMA block transfers.
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>

Is this a qemu-stable material?

Thanks,

/mjt

Re: [PATCH v2 01/13] hw/sd/sdhci: Set SDHC_NIS_DMA bit when appropriate

[Bernhard Beschow]

Am 15. Januar 2025 12:55:29 UTC schrieb Michael Tokarev <mjt@tls.msk.ru>:
>11.01.2025 21:36, Bernhard Beschow wrote:
>> In U-Boot, the fsl_esdhc[_imx] driver waits for both "transmit completed" and
>> "DMA" bits in esdhc_send_cmd_common() by means of DATA_COMPLETE constant. QEMU
>> currently misses to set the DMA bit which causes the driver to loop forever. Fix
>> that by setting the DMA bit if enabled when doing DMA block transfers.
>> 
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>
>Is this a qemu-stable material?

Good question. Given that this part of the code has some further issues [1] I'd rather not alter stable behavior because we might just trade one bug for another.

Best regards,
Bernhard

[1] <https://lore.kernel.org/qemu-devel/4b846383-83bf-4252-a172-95604f2f585b@linaro.org/>


>
>Thanks,
>
>/mjt

Re: [PATCH v2 01/13] hw/sd/sdhci: Set SDHC_NIS_DMA bit when appropriate

[Michael Tokarev]

17.01.2025 02:39, Bernhard Beschow wrote:
> Am 15. Januar 2025 12:55:29 UTC schrieb Michael Tokarev <mjt@tls.msk.ru>:

>> Is this a qemu-stable material?
> 
> Good question. Given that this part of the code has some further issues [1] I'd rather not alter stable behavior because we might just trade one bug for another.

And it's a good answer!  Thank you for sharing your thoughts.
I agree, I dropped this (simple) change from the stable series.

/mjt

[PATCH v2 02/13] hw/char/imx_serial: Fix reset value of UFCR register

[Bernhard Beschow]

The value of the UCFR register is respected when echoing characters to the
terminal, but its reset value is reserved. Fix the reset value to the one
documented in the datasheet.

While at it move the related attribute out of the section of unimplemented
registers since its value is actually respected.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 include/hw/char/imx_serial.h | 2 +-
 hw/char/imx_serial.c         | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/char/imx_serial.h b/include/hw/char/imx_serial.h
index 65f0e97c76..90ba3ff18c 100644
--- a/include/hw/char/imx_serial.h
+++ b/include/hw/char/imx_serial.h
@@ -109,13 +109,13 @@ struct IMXSerialState {
     uint32_t ucr1;
     uint32_t ucr2;
     uint32_t uts1;
+    uint32_t ufcr;
 
     /*
      * The registers below are implemented just so that the
      * guest OS sees what it has written
      */
     uint32_t onems;
-    uint32_t ufcr;
     uint32_t ubmr;
     uint32_t ubrc;
     uint32_t ucr3;
diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
index 12705a1337..f805da23ff 100644
--- a/hw/char/imx_serial.c
+++ b/hw/char/imx_serial.c
@@ -159,6 +159,7 @@ static void imx_serial_reset(IMXSerialState *s)
     s->ucr3 = 0x700;
     s->ubmr = 0;
     s->ubrc = 4;
+    s->ufcr = BIT(11) | BIT(0);
 
     fifo32_reset(&s->rx_fifo);
     timer_del(&s->ageing_timer);
-- 
2.48.0

Re: [PATCH v2 02/13] hw/char/imx_serial: Fix reset value of UFCR register

[Peter Maydell]

On Sat, 11 Jan 2025 at 18:37, Bernhard Beschow <shentey@gmail.com> wrote:
>
> The value of the UCFR register is respected when echoing characters to the
> terminal, but its reset value is reserved. Fix the reset value to the one
> documented in the datasheet.
>
> While at it move the related attribute out of the section of unimplemented
> registers since its value is actually respected.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[PATCH v2 03/13] hw/char/imx_serial: Update all state before restarting ageing timer

[Bernhard Beschow]

Fixes characters to be "echoed" after each keystroke rather than after every
other since imx_serial_rx_fifo_ageing_timer_restart() would see ~UTS1_RXEMPTY
only after every other keystroke.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/char/imx_serial.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
index f805da23ff..be06f39a4d 100644
--- a/hw/char/imx_serial.c
+++ b/hw/char/imx_serial.c
@@ -381,14 +381,14 @@ static void imx_put_data(void *opaque, uint32_t value)
     if (fifo32_num_used(&s->rx_fifo) >= rxtl) {
         s->usr1 |= USR1_RRDY;
     }
-
-    imx_serial_rx_fifo_ageing_timer_restart(s);
-
     s->usr2 |= USR2_RDR;
     s->uts1 &= ~UTS1_RXEMPTY;
     if (value & URXD_BRK) {
         s->usr2 |= USR2_BRCD;
     }
+
+    imx_serial_rx_fifo_ageing_timer_restart(s);
+
     imx_update(s);
 }
 
-- 
2.48.0

Re: [PATCH v2 03/13] hw/char/imx_serial: Update all state before restarting ageing timer

[Peter Maydell]

On Sat, 11 Jan 2025 at 18:37, Bernhard Beschow <shentey@gmail.com> wrote:
>
> Fixes characters to be "echoed" after each keystroke rather than after every
> other since imx_serial_rx_fifo_ageing_timer_restart() would see ~UTS1_RXEMPTY
> only after every other keystroke.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[PATCH v2 04/13] hw/pci-host/designware: Expose MSI IRQ

[Bernhard Beschow]

Fixes INTD and MSI interrupts poking the same IRQ line without keeping track of
each other's IRQ level. Furthermore, SoCs such as the i.MX 8M Plus don't share
the MSI IRQ with the INTx lines, so expose it as a dedicated pin.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 include/hw/arm/fsl-imx6.h        |  4 +++-
 include/hw/arm/fsl-imx7.h        |  4 +++-
 include/hw/pci-host/designware.h |  1 +
 hw/arm/fsl-imx6.c                | 13 ++++++++++++-
 hw/arm/fsl-imx7.c                | 13 ++++++++++++-
 hw/pci-host/designware.c         |  7 +++----
 hw/arm/Kconfig                   |  2 ++
 7 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/include/hw/arm/fsl-imx6.h b/include/hw/arm/fsl-imx6.h
index 61c593ffd2..9da32fc189 100644
--- a/include/hw/arm/fsl-imx6.h
+++ b/include/hw/arm/fsl-imx6.h
@@ -33,6 +33,7 @@
 #include "hw/usb/chipidea.h"
 #include "hw/usb/imx-usb-phy.h"
 #include "hw/pci-host/designware.h"
+#include "hw/or-irq.h"
 #include "exec/memory.h"
 #include "cpu.h"
 #include "qom/object.h"
@@ -73,6 +74,7 @@ struct FslIMX6State {
     ChipideaState      usb[FSL_IMX6_NUM_USBS];
     IMXFECState        eth;
     DesignwarePCIEHost pcie;
+    OrIRQState         pcie4_msi_irq;
     MemoryRegion       rom;
     MemoryRegion       caam;
     MemoryRegion       ocram;
@@ -457,7 +459,7 @@ struct FslIMX6State {
 #define FSL_IMX6_PCIE1_IRQ 120
 #define FSL_IMX6_PCIE2_IRQ 121
 #define FSL_IMX6_PCIE3_IRQ 122
-#define FSL_IMX6_PCIE4_IRQ 123
+#define FSL_IMX6_PCIE4_MSI_IRQ 123
 #define FSL_IMX6_DCIC1_IRQ 124
 #define FSL_IMX6_DCIC2_IRQ 125
 #define FSL_IMX6_MLB150_HIGH_IRQ 126
diff --git a/include/hw/arm/fsl-imx7.h b/include/hw/arm/fsl-imx7.h
index 411fa1c2e3..aa7818c499 100644
--- a/include/hw/arm/fsl-imx7.h
+++ b/include/hw/arm/fsl-imx7.h
@@ -36,6 +36,7 @@
 #include "hw/net/imx_fec.h"
 #include "hw/pci-host/designware.h"
 #include "hw/usb/chipidea.h"
+#include "hw/or-irq.h"
 #include "cpu.h"
 #include "qom/object.h"
 #include "qemu/units.h"
@@ -85,6 +86,7 @@ struct FslIMX7State {
     IMX7GPRState       gpr;
     ChipideaState      usb[FSL_IMX7_NUM_USBS];
     DesignwarePCIEHost pcie;
+    OrIRQState         pcie4_msi_irq;
     MemoryRegion       rom;
     MemoryRegion       caam;
     MemoryRegion       ocram;
@@ -428,7 +430,7 @@ enum FslIMX7IRQs {
     FSL_IMX7_PCI_INTA_IRQ = 125,
     FSL_IMX7_PCI_INTB_IRQ = 124,
     FSL_IMX7_PCI_INTC_IRQ = 123,
-    FSL_IMX7_PCI_INTD_IRQ = 122,
+    FSL_IMX7_PCI_INTD_MSI_IRQ = 122,
 
     FSL_IMX7_UART7_IRQ    = 126,
 
diff --git a/include/hw/pci-host/designware.h b/include/hw/pci-host/designware.h
index c484e377a8..bf8b278978 100644
--- a/include/hw/pci-host/designware.h
+++ b/include/hw/pci-host/designware.h
@@ -86,6 +86,7 @@ struct DesignwarePCIEHost {
         MemoryRegion io;
 
         qemu_irq     irqs[4];
+        qemu_irq     msi;
     } pci;
 
     MemoryRegion mmio;
diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
index ac8c66e242..88b9ccff49 100644
--- a/hw/arm/fsl-imx6.c
+++ b/hw/arm/fsl-imx6.c
@@ -106,6 +106,8 @@ static void fsl_imx6_init(Object *obj)
     object_initialize_child(obj, "eth", &s->eth, TYPE_IMX_ENET);
 
     object_initialize_child(obj, "pcie", &s->pcie, TYPE_DESIGNWARE_PCIE_HOST);
+    object_initialize_child(obj, "pcie4-msi-irq", &s->pcie4_msi_irq,
+                            TYPE_OR_IRQ);
 }
 
 static void fsl_imx6_realize(DeviceState *dev, Error **errp)
@@ -435,14 +437,23 @@ static void fsl_imx6_realize(DeviceState *dev, Error **errp)
     sysbus_realize(SYS_BUS_DEVICE(&s->pcie), &error_abort);
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->pcie), 0, FSL_IMX6_PCIe_REG_ADDR);
 
+    object_property_set_int(OBJECT(&s->pcie4_msi_irq), "num-lines", 2,
+                            &error_abort);
+    qdev_realize(DEVICE(&s->pcie4_msi_irq), NULL, &error_abort);
+
+    irq = qdev_get_gpio_in(DEVICE(&s->a9mpcore), FSL_IMX6_PCIE4_MSI_IRQ);
+    qdev_connect_gpio_out(DEVICE(&s->pcie4_msi_irq), 0, irq);
+
     irq = qdev_get_gpio_in(DEVICE(&s->a9mpcore), FSL_IMX6_PCIE1_IRQ);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->pcie), 0, irq);
     irq = qdev_get_gpio_in(DEVICE(&s->a9mpcore), FSL_IMX6_PCIE2_IRQ);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->pcie), 1, irq);
     irq = qdev_get_gpio_in(DEVICE(&s->a9mpcore), FSL_IMX6_PCIE3_IRQ);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->pcie), 2, irq);
-    irq = qdev_get_gpio_in(DEVICE(&s->a9mpcore), FSL_IMX6_PCIE4_IRQ);
+    irq = qdev_get_gpio_in(DEVICE(&s->pcie4_msi_irq), 0);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->pcie), 3, irq);
+    irq = qdev_get_gpio_in(DEVICE(&s->pcie4_msi_irq), 1);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->pcie), 4, irq);
 
     /*
      * PCIe PHY
diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
index 05e3389fbe..004bf49937 100644
--- a/hw/arm/fsl-imx7.c
+++ b/hw/arm/fsl-imx7.c
@@ -150,6 +150,8 @@ static void fsl_imx7_init(Object *obj)
      * PCIE
      */
     object_initialize_child(obj, "pcie", &s->pcie, TYPE_DESIGNWARE_PCIE_HOST);
+    object_initialize_child(obj, "pcie4-msi-irq", &s->pcie4_msi_irq,
+                            TYPE_OR_IRQ);
 
     /*
      * USBs
@@ -597,14 +599,23 @@ static void fsl_imx7_realize(DeviceState *dev, Error **errp)
     sysbus_realize(SYS_BUS_DEVICE(&s->pcie), &error_abort);
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->pcie), 0, FSL_IMX7_PCIE_REG_ADDR);
 
+    object_property_set_int(OBJECT(&s->pcie4_msi_irq), "num-lines", 2,
+                            &error_abort);
+    qdev_realize(DEVICE(&s->pcie4_msi_irq), NULL, &error_abort);
+
+    irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore), FSL_IMX7_PCI_INTD_MSI_IRQ);
+    qdev_connect_gpio_out(DEVICE(&s->pcie4_msi_irq), 0, irq);
+
     irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore), FSL_IMX7_PCI_INTA_IRQ);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->pcie), 0, irq);
     irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore), FSL_IMX7_PCI_INTB_IRQ);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->pcie), 1, irq);
     irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore), FSL_IMX7_PCI_INTC_IRQ);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->pcie), 2, irq);
-    irq = qdev_get_gpio_in(DEVICE(&s->a7mpcore), FSL_IMX7_PCI_INTD_IRQ);
+    irq = qdev_get_gpio_in(DEVICE(&s->pcie4_msi_irq), 0);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->pcie), 3, irq);
+    irq = qdev_get_gpio_in(DEVICE(&s->pcie4_msi_irq), 1);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->pcie), 4, irq);
 
     /*
      * USBs
diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index c3fc37b904..3e8c36e6a7 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -55,8 +55,6 @@
 #define DESIGNWARE_PCIE_ATU_DEVFN(x)               (((x) >> 16) & 0xff)
 #define DESIGNWARE_PCIE_ATU_UPPER_TARGET           0x91C
 
-#define DESIGNWARE_PCIE_IRQ_MSI                    3
-
 static DesignwarePCIEHost *
 designware_pcie_root_to_host(DesignwarePCIERoot *root)
 {
@@ -90,7 +88,7 @@ static void designware_pcie_root_msi_write(void *opaque, hwaddr addr,
     root->msi.intr[0].status |= BIT(val) & root->msi.intr[0].enable;
 
     if (root->msi.intr[0].status & ~root->msi.intr[0].mask) {
-        qemu_set_irq(host->pci.irqs[DESIGNWARE_PCIE_IRQ_MSI], 1);
+        qemu_set_irq(host->pci.msi, 1);
     }
 }
 
@@ -335,7 +333,7 @@ static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address,
     case DESIGNWARE_PCIE_MSI_INTR0_STATUS:
         root->msi.intr[0].status ^= val;
         if (!root->msi.intr[0].status) {
-            qemu_set_irq(host->pci.irqs[DESIGNWARE_PCIE_IRQ_MSI], 0);
+            qemu_set_irq(host->pci.msi, 0);
         }
         break;
 
@@ -680,6 +678,7 @@ static void designware_pcie_host_realize(DeviceState *dev, Error **errp)
     for (i = 0; i < ARRAY_SIZE(s->pci.irqs); i++) {
         sysbus_init_irq(sbd, &s->pci.irqs[i]);
     }
+    sysbus_init_irq(sbd, &s->pci.msi);
 
     memory_region_init_io(&s->mmio,
                           OBJECT(s),
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index e779b5af95..256013ca80 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -516,6 +516,7 @@ config FSL_IMX6
     select PL310  # cache controller
     select PCI_EXPRESS_DESIGNWARE
     select SDHCI
+    select OR_IRQ
 
 config ASPEED_SOC
     bool
@@ -573,6 +574,7 @@ config FSL_IMX7
     select WDT_IMX2
     select PCI_EXPRESS_DESIGNWARE
     select SDHCI
+    select OR_IRQ
     select UNIMP
 
 config ARM_SMMUV3
-- 
2.48.0

Re: [PATCH v2 04/13] hw/pci-host/designware: Expose MSI IRQ

[Peter Maydell]

On Sat, 11 Jan 2025 at 18:37, Bernhard Beschow <shentey@gmail.com> wrote:
>
> Fixes INTD and MSI interrupts poking the same IRQ line without keeping track of
> each other's IRQ level. Furthermore, SoCs such as the i.MX 8M Plus don't share
> the MSI IRQ with the INTx lines, so expose it as a dedicated pin.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[PATCH v2 05/13] hw/gpio/imx_gpio: Don't clear input GPIO values upon reset

[Bernhard Beschow]

Input GPIO values such as a present SD card may get notified before the GPIO
controller itself gets reset. Claring the input values thus loses data. Assuming
that input GPIO events are only fired when the state changes, the input values
shouldn't be reset.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/gpio/imx_gpio.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/gpio/imx_gpio.c b/hw/gpio/imx_gpio.c
index 898f80f8c8..67c47a7280 100644
--- a/hw/gpio/imx_gpio.c
+++ b/hw/gpio/imx_gpio.c
@@ -302,7 +302,6 @@ static void imx_gpio_reset(DeviceState *dev)
 
     s->dr       = 0;
     s->gdir     = 0;
-    s->psr      = 0;
     s->icr      = 0;
     s->imr      = 0;
     s->isr      = 0;
-- 
2.48.0

Re: [PATCH v2 05/13] hw/gpio/imx_gpio: Don't clear input GPIO values upon reset

[Peter Maydell]

On Sat, 11 Jan 2025 at 18:37, Bernhard Beschow <shentey@gmail.com> wrote:
>
> Input GPIO values such as a present SD card may get notified before the GPIO
> controller itself gets reset. Claring the input values thus loses data. Assuming
> that input GPIO events are only fired when the state changes, the input values
> shouldn't be reset.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  hw/gpio/imx_gpio.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/hw/gpio/imx_gpio.c b/hw/gpio/imx_gpio.c
> index 898f80f8c8..67c47a7280 100644
> --- a/hw/gpio/imx_gpio.c
> +++ b/hw/gpio/imx_gpio.c
> @@ -302,7 +302,6 @@ static void imx_gpio_reset(DeviceState *dev)
>
>      s->dr       = 0;
>      s->gdir     = 0;
> -    s->psr      = 0;
>      s->icr      = 0;
>      s->imr      = 0;
>      s->isr      = 0;

I guess so; we really don't do a good job of consistency about
how to handle GPIO lines that might be high on reset.

The other approach to this would be:
 * devices on both ends implement three-phase reset
 * device that has the GPIO line high on reset should ensure
   it does the qemu_set_irq() for that in the reset "exit" phase
 * device on the input end should:
   - clear the input-data state to 0 in the "hold" phase
   - ensure that its qemu_irq callback function does the
     right thing if it's called during an "exit" phase
     (probably needs no code changes)

That is more "what my mental model of the right thing is"
rather than something we actually do in many (any?) devices
today, though.

thanks
-- PMM

[PATCH v2 06/13] hw/sd/sd: Remove legacy sd_set_cb() in favor of GPIOs

[Bernhard Beschow]

Commit ce5dd27534b0 "hw/sd: Remove omap2_mmc device" removed the last user of
sd_set_cb(). Rework this functionality into GPIOs.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 include/hw/sd/sdcard_legacy.h |  1 -
 hw/sd/sd.c                    | 31 ++++++++++++++-----------------
 2 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/include/hw/sd/sdcard_legacy.h b/include/hw/sd/sdcard_legacy.h
index 0dc3889555..a121232560 100644
--- a/include/hw/sd/sdcard_legacy.h
+++ b/include/hw/sd/sdcard_legacy.h
@@ -36,7 +36,6 @@ SDState *sd_init(BlockBackend *blk, bool is_spi);
 int sd_do_command(SDState *card, SDRequest *request, uint8_t *response);
 void sd_write_byte(SDState *card, uint8_t value);
 uint8_t sd_read_byte(SDState *card);
-void sd_set_cb(SDState *card, qemu_irq readonly, qemu_irq insert);
 
 /* sd_enable should not be used -- it is only used on the nseries boards,
  * where it is part of a broken implementation of the MMC card slot switch
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 0330d432fd..aa8d86e1af 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -820,6 +820,16 @@ static inline uint64_t sd_addr_to_wpnum(uint64_t addr)
     return addr >> (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT);
 }
 
+static bool sd_get_inserted(SDState *sd)
+{
+    return sd->blk && blk_is_inserted(sd->blk);
+}
+
+static bool sd_get_readonly(SDState *sd)
+{
+    return sd->wp_switch;
+}
+
 static void sd_reset(DeviceState *dev)
 {
     SDState *sd = SDMMC_COMMON(dev);
@@ -865,16 +875,9 @@ static void sd_reset(DeviceState *dev)
     sd->dat_lines = 0xf;
     sd->cmd_line = true;
     sd->multi_blk_cnt = 0;
-}
 
-static bool sd_get_inserted(SDState *sd)
-{
-    return sd->blk && blk_is_inserted(sd->blk);
-}
-
-static bool sd_get_readonly(SDState *sd)
-{
-    return sd->wp_switch;
+    qemu_set_irq(sd->readonly_cb, sd_get_readonly(sd));
+    qemu_set_irq(sd->inserted_cb, sd_get_inserted(sd));
 }
 
 static void sd_cardchange(void *opaque, bool load, Error **errp)
@@ -1034,14 +1037,6 @@ SDState *sd_init(BlockBackend *blk, bool is_spi)
     return sd;
 }
 
-void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert)
-{
-    sd->readonly_cb = readonly;
-    sd->inserted_cb = insert;
-    qemu_set_irq(readonly, sd->blk ? !blk_is_writable(sd->blk) : 0);
-    qemu_set_irq(insert, sd->blk ? blk_is_inserted(sd->blk) : 0);
-}
-
 static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len)
 {
     trace_sdcard_read_block(addr, len);
@@ -2727,6 +2722,8 @@ static void sd_instance_init(Object *obj)
     sd->last_cmd_name = "UNSET";
     sd->enable = true;
     sd->ocr_power_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sd_ocr_powerup, sd);
+    qdev_init_gpio_out_named(DEVICE(sd), &sd->inserted_cb, "cd", 1);
+    qdev_init_gpio_out_named(DEVICE(sd), &sd->readonly_cb, "wp", 1);
 }
 
 static void sd_instance_finalize(Object *obj)
-- 
2.48.0

Re: [PATCH v2 06/13] hw/sd/sd: Remove legacy sd_set_cb() in favor of GPIOs

[Peter Maydell]

On Sat, 11 Jan 2025 at 18:37, Bernhard Beschow <shentey@gmail.com> wrote:
>
> Commit ce5dd27534b0 "hw/sd: Remove omap2_mmc device" removed the last user of
> sd_set_cb(). Rework this functionality into GPIOs.
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>

What is this for? We have a non-legacy API for "the SD controller
needs to know when the SD card is inserted or the readonly
status changes", which is that the controller implements the
SDBasClass set_inserted and set_readonly methods. (See the pl011
for an example.)

I would prefer it if we used that consistently, rather than having
two mechanisms, one using GPIO lines and one using class methods.
I think we should delete the sd_set_cb() API and handling code
entirely.

thanks
-- PMM

Re: [PATCH v2 06/13] hw/sd/sd: Remove legacy sd_set_cb() in favor of GPIOs

[Bernhard Beschow]

Am 27. Januar 2025 13:24:46 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
>On Sat, 11 Jan 2025 at 18:37, Bernhard Beschow <shentey@gmail.com> wrote:
>>
>> Commit ce5dd27534b0 "hw/sd: Remove omap2_mmc device" removed the last user of
>> sd_set_cb(). Rework this functionality into GPIOs.
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>
>What is this for? We have a non-legacy API for "the SD controller
>needs to know when the SD card is inserted or the readonly
>status changes", which is that the controller implements the
>SDBasClass set_inserted and set_readonly methods. (See the pl011
>for an example.)
>
>I would prefer it if we used that consistently, rather than having
>two mechanisms, one using GPIO lines and one using class methods.
>I think we should delete the sd_set_cb() API and handling code
>entirely.

According to the Linux MMC controller DT schema, there are actually two ways to implement cd and wp lines [1]. When implementing the imx8mp-evk board, I thought I would need to model the GPIO style [2], hence I implemented it plus the active low part on the SD card. Later I noticed that the card gets detected anyway without the GPIO wiring, so I'm fine if the code gets removed instead.

Best regards,
Bernhard

[1] <https://github.com/torvalds/linux/blob/v6.13/Documentation/devicetree/bindings/mmc/mmc-controller.yaml#L60>
[2] <https://github.com/torvalds/linux/blob/v6.13/arch/arm64/boot/dts/freescale/imx8mp-evk.dts#L776>

>
>thanks
>-- PMM

Re: [PATCH v2 06/13] hw/sd/sd: Remove legacy sd_set_cb() in favor of GPIOs

[Peter Maydell]

On Mon, 27 Jan 2025 at 23:11, Bernhard Beschow <shentey@gmail.com> wrote:
>
>
>
> Am 27. Januar 2025 13:24:46 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
> >On Sat, 11 Jan 2025 at 18:37, Bernhard Beschow <shentey@gmail.com> wrote:
> >>
> >> Commit ce5dd27534b0 "hw/sd: Remove omap2_mmc device" removed the last user of
> >> sd_set_cb(). Rework this functionality into GPIOs.
> >>
> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> >
> >What is this for? We have a non-legacy API for "the SD controller
> >needs to know when the SD card is inserted or the readonly
> >status changes", which is that the controller implements the
> >SDBasClass set_inserted and set_readonly methods. (See the pl011
> >for an example.)
> >
> >I would prefer it if we used that consistently, rather than having
> >two mechanisms, one using GPIO lines and one using class methods.
> >I think we should delete the sd_set_cb() API and handling code
> >entirely.
>
> According to the Linux MMC controller DT schema, there are actually two ways to implement cd and wp lines [1]. When implementing the imx8mp-evk board, I thought I would need to model the GPIO style [2], hence I implemented it plus the active low part on the SD card. Later I noticed that the card gets detected anyway without the GPIO wiring, so I'm fine if the code gets removed instead.

Even if you did need to implement that GPIO wiring, I think the
right way to do it is for the SD controller to implement a
subclass of SDBusClass so it can have its own implementations
of the set_inserted and set_readonly methods, and then it
is the SD controller object that has the GPIO lines, not
the SD card itself.

(I have a series almost ready to send out which QOMifies
the omap_mmc device and then deletes the "legacy" SD API,
including sd_set_cb().)

thanks
-- PMM

Re: [PATCH v2 06/13] hw/sd/sd: Remove legacy sd_set_cb() in favor of GPIOs

[Bernhard Beschow]

Am 28. Januar 2025 10:34:23 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
>On Mon, 27 Jan 2025 at 23:11, Bernhard Beschow <shentey@gmail.com> wrote:
>>
>>
>>
>> Am 27. Januar 2025 13:24:46 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
>> >On Sat, 11 Jan 2025 at 18:37, Bernhard Beschow <shentey@gmail.com> wrote:
>> >>
>> >> Commit ce5dd27534b0 "hw/sd: Remove omap2_mmc device" removed the last user of
>> >> sd_set_cb(). Rework this functionality into GPIOs.
>> >>
>> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> >> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> >
>> >What is this for? We have a non-legacy API for "the SD controller
>> >needs to know when the SD card is inserted or the readonly
>> >status changes", which is that the controller implements the
>> >SDBasClass set_inserted and set_readonly methods. (See the pl011
>> >for an example.)
>> >
>> >I would prefer it if we used that consistently, rather than having
>> >two mechanisms, one using GPIO lines and one using class methods.
>> >I think we should delete the sd_set_cb() API and handling code
>> >entirely.
>>
>> According to the Linux MMC controller DT schema, there are actually two ways to implement cd and wp lines [1]. When implementing the imx8mp-evk board, I thought I would need to model the GPIO style [2], hence I implemented it plus the active low part on the SD card. Later I noticed that the card gets detected anyway without the GPIO wiring, so I'm fine if the code gets removed instead.
>
>Even if you did need to implement that GPIO wiring, I think the
>right way to do it is for the SD controller to implement a
>subclass of SDBusClass so it can have its own implementations
>of the set_inserted and set_readonly methods, and then it
>is the SD controller object that has the GPIO lines, not
>the SD card itself.

I agree.

>
>(I have a series almost ready to send out which QOMifies
>the omap_mmc device and then deletes the "legacy" SD API,
>including sd_set_cb().)

Okay, let's just remove this legacy API.

Best regards,
Bernhard

>
>thanks
>-- PMM

[PATCH v2 07/13] hw/sd/sd: Allow for inverting polarities of presence and write-protect GPIOs

[Bernhard Beschow]

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/sd/sd.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index aa8d86e1af..a50e5c20c8 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -181,6 +181,8 @@ struct SDState {
     qemu_irq inserted_cb;
     QEMUTimer *ocr_power_timer;
     bool enable;
+    bool readonly_active_low;
+    bool inserted_active_low;
     uint8_t dat_lines;
     bool cmd_line;
 };
@@ -876,8 +878,8 @@ static void sd_reset(DeviceState *dev)
     sd->cmd_line = true;
     sd->multi_blk_cnt = 0;
 
-    qemu_set_irq(sd->readonly_cb, sd_get_readonly(sd));
-    qemu_set_irq(sd->inserted_cb, sd_get_inserted(sd));
+    qemu_set_irq(sd->readonly_cb, sd_get_readonly(sd) ^ sd->readonly_active_low);
+    qemu_set_irq(sd->inserted_cb, sd_get_inserted(sd) ^ sd->inserted_active_low);
 }
 
 static void sd_cardchange(void *opaque, bool load, Error **errp)
@@ -896,9 +898,9 @@ static void sd_cardchange(void *opaque, bool load, Error **errp)
     }
 
     if (sd->me_no_qdev_me_kill_mammoth_with_rocks) {
-        qemu_set_irq(sd->inserted_cb, inserted);
+        qemu_set_irq(sd->inserted_cb, inserted ^ sd->inserted_active_low);
         if (inserted) {
-            qemu_set_irq(sd->readonly_cb, readonly);
+            qemu_set_irq(sd->readonly_cb, readonly ^ sd->readonly_active_low);
         }
     } else {
         sdbus = SD_BUS(qdev_get_parent_bus(dev));
@@ -2797,6 +2799,8 @@ static void emmc_realize(DeviceState *dev, Error **errp)
 
 static const Property sdmmc_common_properties[] = {
     DEFINE_PROP_DRIVE("drive", SDState, blk),
+    DEFINE_PROP_BOOL("cd-active-low", SDState, inserted_active_low, false),
+    DEFINE_PROP_BOOL("wp-active-low", SDState, readonly_active_low, false),
 };
 
 static const Property sd_properties[] = {
-- 
2.48.0

Re: [PATCH v2 07/13] hw/sd/sd: Allow for inverting polarities of presence and write-protect GPIOs

[Philippe Mathieu-Daudé]

On 11/1/25 19:37, Bernhard Beschow wrote:
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/sd/sd.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)

See question on v1:
https://lore.kernel.org/qemu-devel/e202623e-eac7-4ed3-87fb-002491ddf745@linaro.org/

Re: [PATCH v2 07/13] hw/sd/sd: Allow for inverting polarities of presence and write-protect GPIOs

[Bernhard Beschow]

Am 15. Januar 2025 17:32:13 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 11/1/25 19:37, Bernhard Beschow wrote:
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>   hw/sd/sd.c | 12 ++++++++----
>>   1 file changed, 8 insertions(+), 4 deletions(-)
>
>See question on v1:
>https://lore.kernel.org/qemu-devel/e202623e-eac7-4ed3-87fb-002491ddf745@linaro.org/

Oh, I missed that. I answered there to keep the context.

[PATCH v2 08/13] hw/char/imx_serial: Turn some DPRINTF() statements into trace events

[Bernhard Beschow]

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/char/imx_serial.c | 58 +++++++++++++++++++++++++++++---------------
 hw/char/trace-events |  5 ++++
 2 files changed, 44 insertions(+), 19 deletions(-)

diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
index be06f39a4d..38b4865157 100644
--- a/hw/char/imx_serial.c
+++ b/hw/char/imx_serial.c
@@ -27,6 +27,7 @@
 #include "qemu/log.h"
 #include "qemu/module.h"
 #include "qemu/fifo32.h"
+#include "trace.h"
 
 #ifndef DEBUG_IMX_UART
 #define DEBUG_IMX_UART 0
@@ -185,10 +186,10 @@ static uint64_t imx_serial_read(void *opaque, hwaddr offset,
                                 unsigned size)
 {
     IMXSerialState *s = (IMXSerialState *)opaque;
+    Chardev *chr = qemu_chr_fe_get_driver(&s->chr);
     uint32_t c, rx_used;
     uint8_t rxtl = s->ufcr & TL_MASK;
-
-    DPRINTF("read(offset=0x%" HWADDR_PRIx ")\n", offset);
+    uint64_t value;
 
     switch (offset >> 2) {
     case 0x0: /* URXD */
@@ -209,49 +210,67 @@ static uint64_t imx_serial_read(void *opaque, hwaddr offset,
             imx_serial_rx_fifo_ageing_timer_restart(s);
             qemu_chr_fe_accept_input(&s->chr);
         }
-        return c;
+        value = c;
+        break;
 
     case 0x20: /* UCR1 */
-        return s->ucr1;
+        value = s->ucr1;
+        break;
 
     case 0x21: /* UCR2 */
-        return s->ucr2;
+        value = s->ucr2;
+        break;
 
     case 0x25: /* USR1 */
-        return s->usr1;
+        value = s->usr1;
+        break;
 
     case 0x26: /* USR2 */
-        return s->usr2;
+        value = s->usr2;
+        break;
 
     case 0x2A: /* BRM Modulator */
-        return s->ubmr;
+        value = s->ubmr;
+        break;
 
     case 0x2B: /* Baud Rate Count */
-        return s->ubrc;
+        value = s->ubrc;
+        break;
 
     case 0x2d: /* Test register */
-        return s->uts1;
+        value = s->uts1;
+        break;
 
     case 0x24: /* UFCR */
-        return s->ufcr;
+        value = s->ufcr;
+        break;
 
     case 0x2c:
-        return s->onems;
+        value = s->onems;
+        break;
 
     case 0x22: /* UCR3 */
-        return s->ucr3;
+        value = s->ucr3;
+        break;
 
     case 0x23: /* UCR4 */
-        return s->ucr4;
+        value = s->ucr4;
+        break;
 
     case 0x29: /* BRM Incremental */
-        return 0x0; /* TODO */
+        value = 0x0; /* TODO */
+        break;
 
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Bad register at offset 0x%"
                       HWADDR_PRIx "\n", TYPE_IMX_SERIAL, __func__, offset);
-        return 0;
+        value = 0;
+        break;
     }
+
+    trace_imx_serial_read(chr ? chr->label : "NODEV", offset, value);
+
+    return value;
 }
 
 static void imx_serial_write(void *opaque, hwaddr offset,
@@ -261,8 +280,7 @@ static void imx_serial_write(void *opaque, hwaddr offset,
     Chardev *chr = qemu_chr_fe_get_driver(&s->chr);
     unsigned char ch;
 
-    DPRINTF("write(offset=0x%" HWADDR_PRIx ", value = 0x%x) to %s\n",
-            offset, (unsigned int)value, chr ? chr->label : "NODEV");
+    trace_imx_serial_write(chr ? chr->label : "NODEV", offset, value);
 
     switch (offset >> 2) {
     case 0x10: /* UTXD */
@@ -374,9 +392,11 @@ static int imx_can_receive(void *opaque)
 static void imx_put_data(void *opaque, uint32_t value)
 {
     IMXSerialState *s = (IMXSerialState *)opaque;
+    Chardev *chr = qemu_chr_fe_get_driver(&s->chr);
     uint8_t rxtl = s->ufcr & TL_MASK;
 
-    DPRINTF("received char\n");
+    trace_imx_serial_put_data(chr ? chr->label : "NODEV", value);
+
     imx_serial_rx_fifo_push(s, value);
     if (fifo32_num_used(&s->rx_fifo) >= rxtl) {
         s->usr1 |= USR1_RRDY;
diff --git a/hw/char/trace-events b/hw/char/trace-events
index 59e1f734a7..92e002da7a 100644
--- a/hw/char/trace-events
+++ b/hw/char/trace-events
@@ -52,6 +52,11 @@ escc_sunkbd_event_out(int ch) "Translated keycode 0x%2.2x"
 escc_kbd_command(int val) "Command %d"
 escc_sunmouse_event(int dx, int dy, int buttons_state) "dx=%d dy=%d buttons=0x%01x"
 
+# imx_serial.c
+imx_serial_read(const char *chrname, uint64_t addr, uint64_t value) "%s:[0x%03" PRIu64 "] -> 0x%08" PRIx64
+imx_serial_write(const char *chrname, uint64_t addr, uint64_t value) "%s:[0x%03" PRIu64 "] <- 0x%08" PRIx64
+imx_serial_put_data(const char *chrname, uint32_t value) "%s: 0x%" PRIx32
+
 # pl011.c
 pl011_irq_state(int level) "irq state %d"
 pl011_read(uint32_t addr, uint32_t value, const char *regname) "addr 0x%03x value 0x%08x reg %s"
-- 
2.48.0

[PATCH v2 09/13] hw/timer/imx_gpt: Remove unused define

[Bernhard Beschow]

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/timer/imx_gpt.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/hw/timer/imx_gpt.c b/hw/timer/imx_gpt.c
index 2663a9d9ef..11eca9fa4d 100644
--- a/hw/timer/imx_gpt.c
+++ b/hw/timer/imx_gpt.c
@@ -20,10 +20,6 @@
 #include "qemu/log.h"
 #include "trace.h"
 
-#ifndef DEBUG_IMX_GPT
-#define DEBUG_IMX_GPT 0
-#endif
-
 static const char *imx_gpt_reg_name(uint32_t reg)
 {
     switch (reg) {
-- 
2.48.0

[PATCH v2 10/13] tests/qtest/libqos: Reuse TYPE_IMX_I2C define

[Bernhard Beschow]

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 tests/qtest/libqos/arm-imx25-pdk-machine.c | 5 +++--
 tests/qtest/libqos/i2c-imx.c               | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/libqos/arm-imx25-pdk-machine.c b/tests/qtest/libqos/arm-imx25-pdk-machine.c
index 8fe128fae8..2d8b754343 100644
--- a/tests/qtest/libqos/arm-imx25-pdk-machine.c
+++ b/tests/qtest/libqos/arm-imx25-pdk-machine.c
@@ -23,6 +23,7 @@
 #include "libqos-malloc.h"
 #include "qgraph.h"
 #include "i2c.h"
+#include "hw/i2c/imx_i2c.h"
 
 #define ARM_PAGE_SIZE            4096
 #define IMX25_PDK_RAM_START      0x80000000
@@ -50,7 +51,7 @@ static void *imx25_pdk_get_driver(void *object, const char *interface)
 static QOSGraphObject *imx25_pdk_get_device(void *obj, const char *device)
 {
     QIMX25PDKMachine *machine = obj;
-    if (!g_strcmp0(device, "imx.i2c")) {
+    if (!g_strcmp0(device, TYPE_IMX_I2C)) {
         return &machine->i2c_1.obj;
     }
 
@@ -86,7 +87,7 @@ static void imx25_pdk_register_nodes(void)
         .extra_device_opts = "bus=i2c-bus.0"
     };
     qos_node_create_machine("arm/imx25-pdk", qos_create_machine_arm_imx25_pdk);
-    qos_node_contains("arm/imx25-pdk", "imx.i2c", &edge, NULL);
+    qos_node_contains("arm/imx25-pdk", TYPE_IMX_I2C, &edge, NULL);
 }
 
 libqos_init(imx25_pdk_register_nodes);
diff --git a/tests/qtest/libqos/i2c-imx.c b/tests/qtest/libqos/i2c-imx.c
index 710cb926d6..6d868e4cc4 100644
--- a/tests/qtest/libqos/i2c-imx.c
+++ b/tests/qtest/libqos/i2c-imx.c
@@ -209,8 +209,8 @@ void imx_i2c_init(IMXI2C *s, QTestState *qts, uint64_t addr)
 
 static void imx_i2c_register_nodes(void)
 {
-    qos_node_create_driver("imx.i2c", NULL);
-    qos_node_produces("imx.i2c", "i2c-bus");
+    qos_node_create_driver(TYPE_IMX_I2C, NULL);
+    qos_node_produces(TYPE_IMX_I2C, "i2c-bus");
 }
 
 libqos_init(imx_i2c_register_nodes);
-- 
2.48.0

[PATCH v2 11/13] hw/i2c/imx_i2c: Convert DPRINTF() to trace events

[Bernhard Beschow]

Also print the QOM canonical path when tracing which allows for distinguishing
the many instances a typical i.MX SoC has.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i2c/imx_i2c.c    | 21 +++++----------------
 hw/i2c/trace-events |  5 +++++
 2 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/hw/i2c/imx_i2c.c b/hw/i2c/imx_i2c.c
index c565fd5b8a..d62213b9e0 100644
--- a/hw/i2c/imx_i2c.c
+++ b/hw/i2c/imx_i2c.c
@@ -25,18 +25,7 @@
 #include "hw/i2c/i2c.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
-
-#ifndef DEBUG_IMX_I2C
-#define DEBUG_IMX_I2C 0
-#endif
-
-#define DPRINTF(fmt, args...) \
-    do { \
-        if (DEBUG_IMX_I2C) { \
-            fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_I2C, \
-                                             __func__, ##args); \
-        } \
-    } while (0)
+#include "trace.h"
 
 static const char *imx_i2c_get_regname(unsigned offset)
 {
@@ -152,8 +141,8 @@ static uint64_t imx_i2c_read(void *opaque, hwaddr offset,
         break;
     }
 
-    DPRINTF("read %s [0x%" HWADDR_PRIx "] -> 0x%02x\n",
-            imx_i2c_get_regname(offset), offset, value);
+    trace_imx_i2c_read(DEVICE(s)->canonical_path, imx_i2c_get_regname(offset),
+                       offset, value);
 
     return (uint64_t)value;
 }
@@ -163,8 +152,8 @@ static void imx_i2c_write(void *opaque, hwaddr offset,
 {
     IMXI2CState *s = IMX_I2C(opaque);
 
-    DPRINTF("write %s [0x%" HWADDR_PRIx "] <- 0x%02x\n",
-            imx_i2c_get_regname(offset), offset, (int)value);
+    trace_imx_i2c_read(DEVICE(s)->canonical_path, imx_i2c_get_regname(offset),
+                       offset, value);
 
     value &= 0xff;
 
diff --git a/hw/i2c/trace-events b/hw/i2c/trace-events
index f708a7ace1..1ad0e95c0e 100644
--- a/hw/i2c/trace-events
+++ b/hw/i2c/trace-events
@@ -56,3 +56,8 @@ npcm7xx_smbus_recv_fifo(const char *id, uint8_t received, uint8_t expected) "%s
 
 pca954x_write_bytes(uint8_t value) "PCA954X write data: 0x%02x"
 pca954x_read_data(uint8_t value) "PCA954X read data: 0x%02x"
+
+# imx_i2c.c
+
+imx_i2c_read(const char *id, const char *reg, uint64_t ofs, uint64_t value) "%s:[%s (0x%" PRIx64 ")] -> 0x%02" PRIx64
+imx_i2c_write(const char *id, const char *reg, uint64_t ofs, uint64_t value) "%s:[%s (0x%" PRIx64 ")] <- 0x%02" PRIx64
-- 
2.48.0

Re: [PATCH v2 11/13] hw/i2c/imx_i2c: Convert DPRINTF() to trace events

[Corey Minyard]

On Sat, Jan 11, 2025 at 07:37:09PM +0100, Bernhard Beschow wrote:
> Also print the QOM canonical path when tracing which allows for distinguishing
> the many instances a typical i.MX SoC has.
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>

This seems reasonable.

Acked-by: Corey Minyard <cminyard@mvista.com>

> ---
>  hw/i2c/imx_i2c.c    | 21 +++++----------------
>  hw/i2c/trace-events |  5 +++++
>  2 files changed, 10 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/i2c/imx_i2c.c b/hw/i2c/imx_i2c.c
> index c565fd5b8a..d62213b9e0 100644
> --- a/hw/i2c/imx_i2c.c
> +++ b/hw/i2c/imx_i2c.c
> @@ -25,18 +25,7 @@
>  #include "hw/i2c/i2c.h"
>  #include "qemu/log.h"
>  #include "qemu/module.h"
> -
> -#ifndef DEBUG_IMX_I2C
> -#define DEBUG_IMX_I2C 0
> -#endif
> -
> -#define DPRINTF(fmt, args...) \
> -    do { \
> -        if (DEBUG_IMX_I2C) { \
> -            fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_I2C, \
> -                                             __func__, ##args); \
> -        } \
> -    } while (0)
> +#include "trace.h"
>  
>  static const char *imx_i2c_get_regname(unsigned offset)
>  {
> @@ -152,8 +141,8 @@ static uint64_t imx_i2c_read(void *opaque, hwaddr offset,
>          break;
>      }
>  
> -    DPRINTF("read %s [0x%" HWADDR_PRIx "] -> 0x%02x\n",
> -            imx_i2c_get_regname(offset), offset, value);
> +    trace_imx_i2c_read(DEVICE(s)->canonical_path, imx_i2c_get_regname(offset),
> +                       offset, value);
>  
>      return (uint64_t)value;
>  }
> @@ -163,8 +152,8 @@ static void imx_i2c_write(void *opaque, hwaddr offset,
>  {
>      IMXI2CState *s = IMX_I2C(opaque);
>  
> -    DPRINTF("write %s [0x%" HWADDR_PRIx "] <- 0x%02x\n",
> -            imx_i2c_get_regname(offset), offset, (int)value);
> +    trace_imx_i2c_read(DEVICE(s)->canonical_path, imx_i2c_get_regname(offset),
> +                       offset, value);
>  
>      value &= 0xff;
>  
> diff --git a/hw/i2c/trace-events b/hw/i2c/trace-events
> index f708a7ace1..1ad0e95c0e 100644
> --- a/hw/i2c/trace-events
> +++ b/hw/i2c/trace-events
> @@ -56,3 +56,8 @@ npcm7xx_smbus_recv_fifo(const char *id, uint8_t received, uint8_t expected) "%s
>  
>  pca954x_write_bytes(uint8_t value) "PCA954X write data: 0x%02x"
>  pca954x_read_data(uint8_t value) "PCA954X read data: 0x%02x"
> +
> +# imx_i2c.c
> +
> +imx_i2c_read(const char *id, const char *reg, uint64_t ofs, uint64_t value) "%s:[%s (0x%" PRIx64 ")] -> 0x%02" PRIx64
> +imx_i2c_write(const char *id, const char *reg, uint64_t ofs, uint64_t value) "%s:[%s (0x%" PRIx64 ")] <- 0x%02" PRIx64
> -- 
> 2.48.0
> 
> 

[PATCH v2 12/13] hw/misc/imx6_src: Convert DPRINTF() to trace events

[Bernhard Beschow]

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/misc/imx6_src.c   | 23 +++++------------------
 hw/misc/trace-events |  6 ++++++
 2 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/hw/misc/imx6_src.c b/hw/misc/imx6_src.c
index dc6a2b92ba..06cc46292e 100644
--- a/hw/misc/imx6_src.c
+++ b/hw/misc/imx6_src.c
@@ -17,18 +17,7 @@
 #include "qemu/module.h"
 #include "target/arm/arm-powerctl.h"
 #include "hw/core/cpu.h"
-
-#ifndef DEBUG_IMX6_SRC
-#define DEBUG_IMX6_SRC 0
-#endif
-
-#define DPRINTF(fmt, args...) \
-    do { \
-        if (DEBUG_IMX6_SRC) { \
-            fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX6_SRC, \
-                                             __func__, ##args); \
-        } \
-    } while (0)
+#include "trace.h"
 
 static const char *imx6_src_reg_name(uint32_t reg)
 {
@@ -87,7 +76,7 @@ static void imx6_src_reset(DeviceState *dev)
 {
     IMX6SRCState *s = IMX6_SRC(dev);
 
-    DPRINTF("\n");
+    trace_imx6_src_reset();
 
     memset(s->regs, 0, sizeof(s->regs));
 
@@ -111,7 +100,7 @@ static uint64_t imx6_src_read(void *opaque, hwaddr offset, unsigned size)
 
     }
 
-    DPRINTF("reg[%s] => 0x%" PRIx32 "\n", imx6_src_reg_name(index), value);
+    trace_imx6_src_read(imx6_src_reg_name(index), value);
 
     return value;
 }
@@ -134,8 +123,7 @@ static void imx6_clear_reset_bit(CPUState *cpu, run_on_cpu_data data)
     assert(bql_locked());
 
     s->regs[SRC_SCR] = deposit32(s->regs[SRC_SCR], ri->reset_bit, 1, 0);
-    DPRINTF("reg[%s] <= 0x%" PRIx32 "\n",
-            imx6_src_reg_name(SRC_SCR), s->regs[SRC_SCR]);
+    trace_imx6_clear_reset_bit(imx6_src_reg_name(SRC_SCR), s->regs[SRC_SCR]);
 
     g_free(ri);
 }
@@ -173,8 +161,7 @@ static void imx6_src_write(void *opaque, hwaddr offset, uint64_t value,
         return;
     }
 
-    DPRINTF("reg[%s] <= 0x%" PRIx32 "\n", imx6_src_reg_name(index),
-            (uint32_t)current_value);
+    trace_imx6_src_write(imx6_src_reg_name(index), value);
 
     change_mask = s->regs[index] ^ (uint32_t)current_value;
 
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 0f5d2b5666..cf1abe6928 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -253,6 +253,12 @@ ccm_clock_freq(uint32_t clock, uint32_t freq) "(Clock = %d) = %d"
 ccm_read_reg(const char *reg_name, uint32_t value) "reg[%s] <= 0x%" PRIx32
 ccm_write_reg(const char *reg_name, uint32_t value) "reg[%s] => 0x%" PRIx32
 
+# imx6_src.c
+imx6_src_read(const char *reg_name, uint32_t value) "reg[%s] => 0x%" PRIx32
+imx6_src_write(const char *reg_name, uint64_t value) "reg[%s] <= 0x%" PRIx64
+imx6_clear_reset_bit(const char *reg_name, uint32_t value) "reg[%s] <= 0x%" PRIx32
+imx6_src_reset(void) ""
+
 # imx7_src.c
 imx7_src_read(const char *reg_name, uint32_t value) "reg[%s] => 0x%" PRIx32
 imx7_src_write(const char *reg_name, uint32_t value) "reg[%s] <= 0x%" PRIx32
-- 
2.48.0

[PATCH v2 13/13] hw/gpio/imx_gpio: Turn DPRINTF() into trace events

[Bernhard Beschow]

While at it add a trace event for input GPIO events.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/gpio/imx_gpio.c   | 18 +++++++-----------
 hw/gpio/trace-events |  5 +++++
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/hw/gpio/imx_gpio.c b/hw/gpio/imx_gpio.c
index 67c47a7280..25546221e0 100644
--- a/hw/gpio/imx_gpio.c
+++ b/hw/gpio/imx_gpio.c
@@ -24,6 +24,7 @@
 #include "migration/vmstate.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
+#include "trace.h"
 
 #ifndef DEBUG_IMX_GPIO
 #define DEBUG_IMX_GPIO 0
@@ -34,14 +35,6 @@ typedef enum IMXGPIOLevel {
     IMX_GPIO_LEVEL_HIGH = 1,
 } IMXGPIOLevel;
 
-#define DPRINTF(fmt, args...) \
-    do { \
-        if (DEBUG_IMX_GPIO) { \
-            fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX_GPIO, \
-                                             __func__, ##args); \
-        } \
-    } while (0)
-
 static const char *imx_gpio_reg_name(uint32_t reg)
 {
     switch (reg) {
@@ -111,6 +104,8 @@ static void imx_gpio_set(void *opaque, int line, int level)
     IMXGPIOState *s = IMX_GPIO(opaque);
     IMXGPIOLevel imx_level = level ? IMX_GPIO_LEVEL_HIGH : IMX_GPIO_LEVEL_LOW;
 
+    trace_imx_gpio_set(DEVICE(s)->canonical_path, line, imx_level);
+
     imx_gpio_set_int_line(s, line, imx_level);
 
     /* this is an input signal, so set PSR */
@@ -200,7 +195,8 @@ static uint64_t imx_gpio_read(void *opaque, hwaddr offset, unsigned size)
         break;
     }
 
-    DPRINTF("(%s) = 0x%" PRIx32 "\n", imx_gpio_reg_name(offset), reg_value);
+    trace_imx_gpio_read(DEVICE(s)->canonical_path, imx_gpio_reg_name(offset),
+                        reg_value);
 
     return reg_value;
 }
@@ -210,8 +206,8 @@ static void imx_gpio_write(void *opaque, hwaddr offset, uint64_t value,
 {
     IMXGPIOState *s = IMX_GPIO(opaque);
 
-    DPRINTF("(%s, value = 0x%" PRIx32 ")\n", imx_gpio_reg_name(offset),
-            (uint32_t)value);
+    trace_imx_gpio_write(DEVICE(s)->canonical_path, imx_gpio_reg_name(offset),
+                         value);
 
     switch (offset) {
     case DR_ADDR:
diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
index b91cc7e9a4..cea896b28f 100644
--- a/hw/gpio/trace-events
+++ b/hw/gpio/trace-events
@@ -1,5 +1,10 @@
 # See docs/devel/tracing.rst for syntax documentation.
 
+# imx_gpio.c
+imx_gpio_read(const char *id, const char *reg, uint32_t value) "%s:[%s] -> 0x%" PRIx32
+imx_gpio_write(const char *id, const char *reg, uint32_t value) "%s:[%s] <- 0x%" PRIx32
+imx_gpio_set(const char *id, int line, int level) "%s:[%d] <- %d"
+
 # npcm7xx_gpio.c
 npcm7xx_gpio_read(const char *id, uint64_t offset, uint64_t value) " %s offset: 0x%04" PRIx64 " value 0x%08" PRIx64
 npcm7xx_gpio_write(const char *id, uint64_t offset, uint64_t value) "%s offset: 0x%04" PRIx64 " value 0x%08" PRIx64
-- 
2.48.0

Re: [PATCH v2 00/13] i.MX and SDHCI improvements

[Bernhard Beschow]

Am 11. Januar 2025 18:36:58 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>This series fixes some details in i.MX platform devices, improves SDHCI
>
>compatibility with U-Boot and modernizes some code.
>
>
>
>The first 5 patches are bugfixes 1/ resolving infinite loop in U-Boot esdhc
>
>driver, 2/ fixing a character echoing issue in imx-serial, 3/ fixing IRQ sharing
>
>issue in Designware PCIe emulation, and 4/ fixing GPIO level preservation across
>
>resets in imx-gpio.
>
>
>
>Patches 6 and 7 modernize SD card emulation by turning presence and
>
>write-protect GPIOs into qdev GPIOs and then further allowing the GPIOs to be
>
>inverted, just like device tree allows.
>
>
>
>The rest of the series is cosmetics including turning DPRINTF() into trace
>
>events which eases debugging.
>
>
>
>v2:
>
>* Drop redundant implementation of TYPE_OR_IRQ (David, Zoltan)
>
>* Use absolute QOM paths when tracing in imx_gpio and imx_i2c (Phil)
>
>* Trace hexadecimal values in imx_serial (Phil)
>
>* Do NOT move inversion of presence and write-protect GPIOs since that changes
>
>the internal logic of the device
>
>
>
>Bernhard Beschow (13):
>
>  hw/sd/sdhci: Set SDHC_NIS_DMA bit when appropriate
>

>  hw/char/imx_serial: Fix reset value of UFCR register
>
>  hw/char/imx_serial: Update all state before restarting ageing timer
>
>  hw/pci-host/designware: Expose MSI IRQ
>
>  hw/gpio/imx_gpio: Don't clear input GPIO values upon reset
>

Does anybody feel comfortable enough reviewing the above four patches? I hit those issues while working on i.MX machines.

The below two patches are less critical for my work but there were patches with discussions floating around recently, e.g. [1].

Thanks,
Bernhard

[1] <https://patchew.org/QEMU/20250108100240.960593-1-clg@redhat.com/>

>  hw/sd/sd: Remove legacy sd_set_cb() in favor of GPIOs
>
>  hw/sd/sd: Allow for inverting polarities of presence and write-protect
>
>    GPIOs
>

>  hw/char/imx_serial: Turn some DPRINTF() statements into trace events
>
>  hw/timer/imx_gpt: Remove unused define
>
>  tests/qtest/libqos: Reuse TYPE_IMX_I2C define
>
>  hw/i2c/imx_i2c: Convert DPRINTF() to trace events
>
>  hw/misc/imx6_src: Convert DPRINTF() to trace events
>
>  hw/gpio/imx_gpio: Turn DPRINTF() into trace events
>
>
>
> include/hw/arm/fsl-imx6.h                  |  4 +-
>
> include/hw/arm/fsl-imx7.h                  |  4 +-
>
> include/hw/char/imx_serial.h               |  2 +-
>
> include/hw/pci-host/designware.h           |  1 +
>
> include/hw/sd/sdcard_legacy.h              |  1 -
>
> hw/arm/fsl-imx6.c                          | 13 ++++-
>
> hw/arm/fsl-imx7.c                          | 13 ++++-
>
> hw/char/imx_serial.c                       | 65 ++++++++++++++--------
>
> hw/gpio/imx_gpio.c                         | 19 +++----
>
> hw/i2c/imx_i2c.c                           | 21 ++-----
>
> hw/misc/imx6_src.c                         | 23 ++------
>
> hw/pci-host/designware.c                   |  7 +--
>
> hw/sd/sd.c                                 | 39 ++++++-------
>
> hw/sd/sdhci.c                              | 11 +++-
>
> hw/timer/imx_gpt.c                         |  4 --
>
> tests/qtest/libqos/arm-imx25-pdk-machine.c |  5 +-
>
> tests/qtest/libqos/i2c-imx.c               |  4 +-
>
> hw/arm/Kconfig                             |  2 +
>
> hw/char/trace-events                       |  5 ++
>
> hw/gpio/trace-events                       |  5 ++
>
> hw/i2c/trace-events                        |  5 ++
>
> hw/misc/trace-events                       |  6 ++
>
> 22 files changed, 151 insertions(+), 108 deletions(-)
>
>
>
>-- >
>2.48.0
>
>
>

Re: [PATCH v2 00/13] i.MX and SDHCI improvements

[Peter Maydell]

On Sat, 11 Jan 2025 at 18:37, Bernhard Beschow <shentey@gmail.com> wrote:
>
> This series fixes some details in i.MX platform devices, improves SDHCI
> compatibility with U-Boot and modernizes some code.
>
> The first 5 patches are bugfixes 1/ resolving infinite loop in U-Boot esdhc
> driver, 2/ fixing a character echoing issue in imx-serial, 3/ fixing IRQ sharing
> issue in Designware PCIe emulation, and 4/ fixing GPIO level preservation across
> resets in imx-gpio.
>
> Patches 6 and 7 modernize SD card emulation by turning presence and
> write-protect GPIOs into qdev GPIOs and then further allowing the GPIOs to be
> inverted, just like device tree allows.
>
> The rest of the series is cosmetics including turning DPRINTF() into trace
> events which eases debugging.
>
> v2:
> * Drop redundant implementation of TYPE_OR_IRQ (David, Zoltan)
> * Use absolute QOM paths when tracing in imx_gpio and imx_i2c (Phil)
> * Trace hexadecimal values in imx_serial (Phil)
> * Do NOT move inversion of presence and write-protect GPIOs since that changes
> the internal logic of the device
>
> Bernhard Beschow (13):
>   hw/char/imx_serial: Fix reset value of UFCR register
>   hw/char/imx_serial: Update all state before restarting ageing timer
>   hw/pci-host/designware: Expose MSI IRQ

I've taken these three into target-arm.next; I see Philippe has
taken most of the rest; and the hw/sd patches I've left some
review comments on.

thanks
-- PMM