Re: [Qemu-devel] [PATCH v2 3/3] bcm2835: add sdhost and gpio controllers

2017-02-22 Thread Clement Deschamps 
Hello,


On 02/22/2017 07:27 PM, Andrew Baumann wrote:
> Hi,
>
>> From: Clement Deschamps [mailto:clement.descha...@antfield.fr]
>> Sent: Wednesday, 22 February 2017 3:24
>> Subject: [PATCH v2 3/3] bcm2835: add sdhost and gpio controllers
>>
>> This adds the bcm2835_sdhost and bcm2835_gpio to the BCM2835 platform.
>>
>> The bcm2835_gpio has a link to both the sdhci and sdhost controllers for
>> supporting the alternate function of GPIOs 48-53 (SD controller selection)
>>
>> Signed-off-by: Clement Deschamps 
> [...]
>> +/* SDHOST */
>> +object_property_set_bool(OBJECT(>sdhost), true, "realized", );
>> +if (err) {
>> +error_propagate(errp, err);
>> +return;
>> +}
>> +
>> +memory_region_add_subregion(>peri_mr, MMCI0_OFFSET,
>> +sysbus_mmio_get_region(SYS_BUS_DEVICE(>sdhost), 0));
>> +sysbus_connect_irq(SYS_BUS_DEVICE(>sdhost), 0,
>> +qdev_get_gpio_in_named(DEVICE(>ic), BCM2835_IC_GPU_IRQ,
>> +   INTERRUPT_SDIO));
>> +object_property_add_alias(OBJECT(s), "sd-bus-2", OBJECT(>sdhost),
>> +  "sd-bus", );
> Is this alias still meaningful / needed, or is it a relic from the previous 
> version? Right now it doesn't appear to be used, and I'm thinking that if 
> someone did try to use it (e.g. at the board level by connecting an SD card) 
> then the new GPIO logic would allow swapping the two SD cards between the two 
> controllers, which doesn't sound like a faithful recreation of the hardware.
>
> Otherwise,
> Reviewed-by: Andrew Baumann 
>
> Cheers,
> Andrew

Thank you Andrew for your review.

You're right, we don't need to expose anymore the sdbus of the sdhost.
We can remove this alias.


Peter, do you agree with the sdcard reparenting function I implemented
in the GPIO controller (following your advices) ?


Best,
Clément

Re: [Qemu-devel] [PATCH v2 3/3] bcm2835: add sdhost and gpio controllers

2017-02-22 Thread Andrew Baumann 
Hi,

> From: Clement Deschamps [mailto:clement.descha...@antfield.fr]
> Sent: Wednesday, 22 February 2017 3:24
> Subject: [PATCH v2 3/3] bcm2835: add sdhost and gpio controllers
> 
> This adds the bcm2835_sdhost and bcm2835_gpio to the BCM2835 platform.
> 
> The bcm2835_gpio has a link to both the sdhci and sdhost controllers for
> supporting the alternate function of GPIOs 48-53 (SD controller selection)
> 
> Signed-off-by: Clement Deschamps 
[...]
> +/* SDHOST */
> +object_property_set_bool(OBJECT(>sdhost), true, "realized", );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
> +
> +memory_region_add_subregion(>peri_mr, MMCI0_OFFSET,
> +sysbus_mmio_get_region(SYS_BUS_DEVICE(>sdhost), 0));
> +sysbus_connect_irq(SYS_BUS_DEVICE(>sdhost), 0,
> +qdev_get_gpio_in_named(DEVICE(>ic), BCM2835_IC_GPU_IRQ,
> +   INTERRUPT_SDIO));
> +object_property_add_alias(OBJECT(s), "sd-bus-2", OBJECT(>sdhost),
> +  "sd-bus", );

Is this alias still meaningful / needed, or is it a relic from the previous 
version? Right now it doesn't appear to be used, and I'm thinking that if 
someone did try to use it (e.g. at the board level by connecting an SD card) 
then the new GPIO logic would allow swapping the two SD cards between the two 
controllers, which doesn't sound like a faithful recreation of the hardware.

Otherwise,
Reviewed-by: Andrew Baumann 

Cheers,
Andrew

[Qemu-devel] [PATCH v2 3/3] bcm2835: add sdhost and gpio controllers

2017-02-22 Thread Clement Deschamps 
This adds the bcm2835_sdhost and bcm2835_gpio to the BCM2835 platform.

The bcm2835_gpio has a link to both the sdhci and sdhost controllers for
supporting the alternate function of GPIOs 48-53 (SD controller selection)

Signed-off-by: Clement Deschamps 
---
 hw/arm/bcm2835_peripherals.c | 44 
 include/hw/arm/bcm2835_peripherals.h |  4 
 2 files changed, 48 insertions(+)

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 2e641a3989..adc419dfcf 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -91,6 +91,11 @@ static void bcm2835_peripherals_init(Object *obj)
 object_property_add_child(obj, "sdhci", OBJECT(>sdhci), NULL);
 qdev_set_parent_bus(DEVICE(>sdhci), sysbus_get_default());
 
+/* SDHOST */
+object_initialize(>sdhost, sizeof(s->sdhost), TYPE_BCM2835_SDHOST);
+object_property_add_child(obj, "sdhost", OBJECT(>sdhost), NULL);
+qdev_set_parent_bus(DEVICE(>sdhost), sysbus_get_default());
+
 /* DMA Channels */
 object_initialize(>dma, sizeof(s->dma), TYPE_BCM2835_DMA);
 object_property_add_child(obj, "dma", OBJECT(>dma), NULL);
@@ -98,6 +103,16 @@ static void bcm2835_peripherals_init(Object *obj)
 
 object_property_add_const_link(OBJECT(>dma), "dma-mr",
OBJECT(>gpu_bus_mr), _abort);
+
+/* GPIO */
+object_initialize(>gpio, sizeof(s->gpio), TYPE_BCM2835_GPIO);
+object_property_add_child(obj, "gpio", OBJECT(>gpio), NULL);
+qdev_set_parent_bus(DEVICE(>gpio), sysbus_get_default());
+
+object_property_add_const_link(OBJECT(>gpio), "sdhci",
+   OBJECT(>sdhci), _abort);
+object_property_add_const_link(OBJECT(>gpio), "sdhost",
+   OBJECT(>sdhost), _abort);
 }
 
 static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
@@ -259,6 +274,25 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
 return;
 }
 
+/* SDHOST */
+object_property_set_bool(OBJECT(>sdhost), true, "realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+memory_region_add_subregion(>peri_mr, MMCI0_OFFSET,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(>sdhost), 0));
+sysbus_connect_irq(SYS_BUS_DEVICE(>sdhost), 0,
+qdev_get_gpio_in_named(DEVICE(>ic), BCM2835_IC_GPU_IRQ,
+   INTERRUPT_SDIO));
+object_property_add_alias(OBJECT(s), "sd-bus-2", OBJECT(>sdhost),
+  "sd-bus", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
 /* DMA Channels */
 object_property_set_bool(OBJECT(>dma), true, "realized", );
 if (err) {
@@ -277,6 +311,16 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
   BCM2835_IC_GPU_IRQ,
   INTERRUPT_DMA0 + n));
 }
+
+/* GPIO */
+object_property_set_bool(OBJECT(>gpio), true, "realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+memory_region_add_subregion(>peri_mr, GPIO_OFFSET,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(>gpio), 0));
 }
 
 static void bcm2835_peripherals_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/arm/bcm2835_peripherals.h 
b/include/hw/arm/bcm2835_peripherals.h
index e12ae3721a..4c87859e31 100644
--- a/include/hw/arm/bcm2835_peripherals.h
+++ b/include/hw/arm/bcm2835_peripherals.h
@@ -21,6 +21,8 @@
 #include "hw/misc/bcm2835_property.h"
 #include "hw/misc/bcm2835_mbox.h"
 #include "hw/sd/sdhci.h"
+#include "hw/sd/bcm2835_sdhost.h"
+#include "hw/gpio/bcm2835_gpio.h"
 
 #define TYPE_BCM2835_PERIPHERALS "bcm2835-peripherals"
 #define BCM2835_PERIPHERALS(obj) \
@@ -43,6 +45,8 @@ typedef struct BCM2835PeripheralState {
 BCM2835PropertyState property;
 BCM2835MboxState mboxes;
 SDHCIState sdhci;
+BCM2835SDHostState sdhost;
+BCM2835GpioState gpio;
 } BCM2835PeripheralState;
 
 #endif /* BCM2835_PERIPHERALS_H */
-- 
2.11.1