[PATCH 0/3] Raspi sd-bus cleanup and multiphase reset

[PATCH 0/3] Raspi sd-bus cleanup and multiphase reset

[Damien Hedde]

Hi All,

This series is a follow-up of my reset series,
https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg04664.html

I've extracted the raspberry-pi related patches. As suggested by Peter
in the previous version of the reset serie, these patches do a cleanup
of the raspberry-pi sd-bus then does the multiphase reset switch of the
gpio soc part.

Patch 1 remove the bcm2835_gpio sd-bus which is only used to host the
sd-card before the machine initial reset. As the soc exhibits the "default"
sd-bus to the machine using an alias, we can simply exhibit the sdhci instead.
Patch 2 prepare the multiphase reset switch by isolating the sd-card parent
change. Patch 3 finally does the multiphase transition of the bcm2835_gpio.

Thanks for your feedback,
Damien

Based-on: <20191220115035.709876-1-damien.hedde@greensocs.com>

Damien Hedde (3):
  hw/arm/bcm2835: remove gpio/sd-bus
  hw/gpio/bcm2835_gpio: Isolate sdbus reparenting
  hw/gpio/bcm2835_gpio: Update to resettable

 include/hw/gpio/bcm2835_gpio.h |  1 -
 hw/arm/bcm2835_peripherals.c   |  2 +-
 hw/gpio/bcm2835_gpio.c         | 35 +++++++++++++++++++++++-----------
 3 files changed, 25 insertions(+), 13 deletions(-)

-- 
2.24.0

[PATCH 1/3] hw/arm/bcm2835: remove gpio/sd-bus

[Damien Hedde]

Remove gpio/sdbus which is only used to host the sd card before reset.

Instead directly we exhibit the sdhci bus to the soc and machine. Thus
the sd card is created on the sdhci bus and do not need to be moved
during first reset.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/gpio/bcm2835_gpio.h | 1 -
 hw/arm/bcm2835_peripherals.c   | 2 +-
 hw/gpio/bcm2835_gpio.c         | 6 ------
 3 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/include/hw/gpio/bcm2835_gpio.h b/include/hw/gpio/bcm2835_gpio.h
index b0de0a3c74..f8416d43d3 100644
--- a/include/hw/gpio/bcm2835_gpio.h
+++ b/include/hw/gpio/bcm2835_gpio.h
@@ -23,7 +23,6 @@ typedef struct BCM2835GpioState {
     MemoryRegion iomem;
 
     /* SDBus selector */
-    SDBus sdbus;
     SDBus *sdbus_sdhci;
     SDBus *sdbus_sdhost;
 
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 17207ae07e..93477c5b2f 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -360,7 +360,7 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(&s->peri_mr, GPIO_OFFSET,
                 sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->gpio), 0));
 
-    object_property_add_alias(OBJECT(s), "sd-bus", OBJECT(&s->gpio), "sd-bus",
+    object_property_add_alias(OBJECT(s), "sd-bus", OBJECT(&s->sdhci), "sd-bus",
                               &err);
     if (err) {
         error_propagate(errp, err);
diff --git a/hw/gpio/bcm2835_gpio.c b/hw/gpio/bcm2835_gpio.c
index 91ce3d10cc..25c180423f 100644
--- a/hw/gpio/bcm2835_gpio.c
+++ b/hw/gpio/bcm2835_gpio.c
@@ -267,9 +267,6 @@ static void bcm2835_gpio_reset(DeviceState *dev)
 
     s->sd_fsel = 0;
 
-    /* SDHCI is selected by default */
-    sdbus_reparent_card(&s->sdbus, s->sdbus_sdhci);
-
     s->lev0 = 0;
     s->lev1 = 0;
 }
@@ -299,9 +296,6 @@ static void bcm2835_gpio_init(Object *obj)
     DeviceState *dev = DEVICE(obj);
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 
-    qbus_create_inplace(&s->sdbus, sizeof(s->sdbus),
-                        TYPE_SD_BUS, DEVICE(s), "sd-bus");
-
     memory_region_init_io(&s->iomem, obj,
             &bcm2835_gpio_ops, s, "bcm2835_gpio", 0x1000);
     sysbus_init_mmio(sbd, &s->iomem);
-- 
2.24.0

[PATCH 2/3] hw/gpio/bcm2835_gpio: Isolate sdbus reparenting

[Damien Hedde]

Split gpfsel_set() in 2 so that the sdbus reparenting is done
in a dedicated function: gpfsel_update_sdbus().

Update call sites of gpfsel_set() to also call gpfsel_update_sdbus().

This commit is a preparation to switch to multiphase reset.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 hw/gpio/bcm2835_gpio.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/gpio/bcm2835_gpio.c b/hw/gpio/bcm2835_gpio.c
index 25c180423f..88dc652018 100644
--- a/hw/gpio/bcm2835_gpio.c
+++ b/hw/gpio/bcm2835_gpio.c
@@ -75,7 +75,10 @@ static void gpfsel_set(BCM2835GpioState *s, uint8_t reg, uint32_t value)
             s->fsel[index] = fsel;
         }
     }
+}
 
+static void gpfsel_update_sdbus(BCM2835GpioState *s)
+{
     /* SD controller selection (48-53) */
     if (s->sd_fsel != 0
             && (s->fsel[48] == 0) /* SD_CLK_R */
@@ -210,6 +213,7 @@ static void bcm2835_gpio_write(void *opaque, hwaddr offset,
     case GPFSEL4:
     case GPFSEL5:
         gpfsel_set(s, offset / 4, value);
+        gpfsel_update_sdbus(s);
         break;
     case GPSET0:
         gpset(s, value, 0, 32, &s->lev0);
@@ -261,11 +265,15 @@ static void bcm2835_gpio_reset(DeviceState *dev)
     BCM2835GpioState *s = BCM2835_GPIO(dev);
 
     int i;
+    /*
+     * Initialize the gpfsel registers. In particular, it selects the SDHCI bus
+     * for the sd card.
+     */
     for (i = 0; i < 6; i++) {
         gpfsel_set(s, i, 0);
     }
-
-    s->sd_fsel = 0;
+    /* Update s->sd_fsel and move the sd card */
+    gpfsel_update_sdbus(s);
 
     s->lev0 = 0;
     s->lev1 = 0;
-- 
2.24.0

[PATCH 3/3] hw/gpio/bcm2835_gpio: Update to resettable

[Damien Hedde]

Transition the bcm2835_gpio device class to Resettable.
The sdbus reparenting is delayed in hold phase to respect
resettable side-effect rules.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/gpio/bcm2835_gpio.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/hw/gpio/bcm2835_gpio.c b/hw/gpio/bcm2835_gpio.c
index 88dc652018..4d393c6a47 100644
--- a/hw/gpio/bcm2835_gpio.c
+++ b/hw/gpio/bcm2835_gpio.c
@@ -260,9 +260,9 @@ err_out:
             __func__, offset);
 }
 
-static void bcm2835_gpio_reset(DeviceState *dev)
+static void bcm2835_gpio_reset_enter(Object *obj, ResetType type)
 {
-    BCM2835GpioState *s = BCM2835_GPIO(dev);
+    BCM2835GpioState *s = BCM2835_GPIO(obj);
 
     int i;
     /*
@@ -272,13 +272,22 @@ static void bcm2835_gpio_reset(DeviceState *dev)
     for (i = 0; i < 6; i++) {
         gpfsel_set(s, i, 0);
     }
-    /* Update s->sd_fsel and move the sd card */
-    gpfsel_update_sdbus(s);
 
     s->lev0 = 0;
     s->lev1 = 0;
 }
 
+static void bcm2835_gpio_reset_hold(Object *obj)
+{
+    BCM2835GpioState *s = BCM2835_GPIO(obj);
+
+    /*
+     * Update s->sd_fsel and move the sd card according to the config set in
+     * bcm2835_gpio_reset_enter().
+     */
+    gpfsel_update_sdbus(s);
+}
+
 static const MemoryRegionOps bcm2835_gpio_ops = {
     .read = bcm2835_gpio_read,
     .write = bcm2835_gpio_write,
@@ -336,10 +345,12 @@ static void bcm2835_gpio_realize(DeviceState *dev, Error **errp)
 static void bcm2835_gpio_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
 
     dc->vmsd = &vmstate_bcm2835_gpio;
     dc->realize = &bcm2835_gpio_realize;
-    dc->reset = &bcm2835_gpio_reset;
+    rc->phases.enter = &bcm2835_gpio_reset_enter;
+    rc->phases.hold  = &bcm2835_gpio_reset_hold;
 }
 
 static const TypeInfo bcm2835_gpio_info = {
-- 
2.24.0

Re: [PATCH 0/3] Raspi sd-bus cleanup and multiphase reset

[Richard Henderson]

On 12/21/19 12:46 AM, Damien Hedde wrote:
> Hi All,
> 
> This series is a follow-up of my reset series,
> https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg04664.html
> 
> I've extracted the raspberry-pi related patches. As suggested by Peter
> in the previous version of the reset serie, these patches do a cleanup
> of the raspberry-pi sd-bus then does the multiphase reset switch of the
> gpio soc part.
> 
> Patch 1 remove the bcm2835_gpio sd-bus which is only used to host the
> sd-card before the machine initial reset. As the soc exhibits the "default"
> sd-bus to the machine using an alias, we can simply exhibit the sdhci instead.
> Patch 2 prepare the multiphase reset switch by isolating the sd-card parent
> change. Patch 3 finally does the multiphase transition of the bcm2835_gpio.
> 
> Thanks for your feedback,
> Damien
> 
> Based-on: <20191220115035.709876-1-damien.hedde@greensocs.com>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~