Now both sysbus/pci classes inherit of the 'pending-insert-quirk' property,
which is a HCI dependent property (regardless if accessed through a MMIO
sysbus or a PCI bus).
So far only the BCM implementation has to use it.
Add sysbus/pci/sdbus comments to have clearer code blocks separation.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
---
include/hw/sd/sdhci.h | 4 +++-
hw/sd/sdhci.c | 21 ++++++++++-----------
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index dacd726537..8041c9629e 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -79,13 +79,15 @@ typedef struct SDHCIState {
uint32_t buf_maxsz;
uint16_t data_count; /* current element in FIFO buffer */
uint8_t stopped_state;/* Current SDHC state */
- bool pending_insert_quirk;/* Quirk for Raspberry Pi card insert int */
bool pending_insert_state;
/* Buffer Data Port Register - virtual access point to R and W buffers */
/* Software Reset Register - always reads as 0 */
/* Force Event Auto CMD12 Error Interrupt Reg - write only */
/* Force Event Error Interrupt Register- write only */
/* RO Host Controller Version Register always reads as 0x2401 */
+
+ /* Configurable properties */
+ bool pending_insert_quirk; /* Quirk for Raspberry Pi card insert int */
} SDHCIState;
#define TYPE_PCI_SDHCI "sdhci-pci"
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 365bc80009..a11469fbca 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1266,13 +1266,17 @@ const VMStateDescription sdhci_vmstate = {
/* Capabilities registers provide information on supported features of this
* specific host controller implementation */
-static Property sdhci_pci_properties[] = {
+static Property sdhci_properties[] = {
DEFINE_PROP_UINT32("capareg", SDHCIState, capareg,
SDHC_CAPAB_REG_DEFAULT),
DEFINE_PROP_UINT32("maxcurr", SDHCIState, maxcurr, 0),
+ DEFINE_PROP_BOOL("pending-insert-quirk", SDHCIState, pending_insert_quirk,
+ false),
DEFINE_PROP_END_OF_LIST(),
};
+/* --- qdev PCI --- */
+
static void sdhci_pci_realize(PCIDevice *dev, Error **errp)
{
SDHCIState *s = PCI_SDHCI(dev);
@@ -1305,7 +1309,7 @@ static void sdhci_pci_class_init(ObjectClass *klass, void *data)
k->class_id = PCI_CLASS_SYSTEM_SDHCI;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
dc->vmsd = &sdhci_vmstate;
- dc->props = sdhci_pci_properties;
+ dc->props = sdhci_properties;
dc->reset = sdhci_poweron_reset;
}
@@ -1320,14 +1324,7 @@ static const TypeInfo sdhci_pci_info = {
},
};
-static Property sdhci_sysbus_properties[] = {
- DEFINE_PROP_UINT32("capareg", SDHCIState, capareg,
- SDHC_CAPAB_REG_DEFAULT),
- DEFINE_PROP_UINT32("maxcurr", SDHCIState, maxcurr, 0),
- DEFINE_PROP_BOOL("pending-insert-quirk", SDHCIState, pending_insert_quirk,
- false),
- DEFINE_PROP_END_OF_LIST(),
-};
+/* --- qdev SysBus --- */
static void sdhci_sysbus_init(Object *obj)
{
@@ -1360,7 +1357,7 @@ static void sdhci_sysbus_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
dc->vmsd = &sdhci_vmstate;
- dc->props = sdhci_sysbus_properties;
+ dc->props = sdhci_properties;
dc->realize = sdhci_sysbus_realize;
dc->reset = sdhci_poweron_reset;
}
@@ -1374,6 +1371,8 @@ static const TypeInfo sdhci_sysbus_info = {
.class_init = sdhci_sysbus_class_init,
};
+/* --- qdev bus master --- */
+
static void sdhci_bus_class_init(ObjectClass *klass, void *data)
{
SDBusClass *sbc = SD_BUS_CLASS(klass);
--
2.15.1
On 11 January 2018 at 19:30, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > Now both sysbus/pci classes inherit of the 'pending-insert-quirk' property, > which is a HCI dependent property (regardless if accessed through a MMIO > sysbus or a PCI bus). > So far only the BCM implementation has to use it. > > Add sysbus/pci/sdbus comments to have clearer code blocks separation. > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > Reviewed-by: Alistair Francis <alistair.francis@xilinx.com> I'm not sure about this patch, because it means that we now have no mechanism for having a property which exists on the sysbus device but not on the PCI device (and as per my comments on the last patch in this set, we do need to be able to do that). thanks -- PMM
On Fri, Jan 12, 2018 at 2:05 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 11 January 2018 at 19:30, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> Now both sysbus/pci classes inherit of the 'pending-insert-quirk' property, >> which is a HCI dependent property (regardless if accessed through a MMIO >> sysbus or a PCI bus). >> So far only the BCM implementation has to use it. >> >> Add sysbus/pci/sdbus comments to have clearer code blocks separation. >> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com> > > I'm not sure about this patch, because it means that we now have > no mechanism for having a property which exists on the sysbus > device but not on the PCI device (and as per my comments on the > last patch in this set, we do need to be able to do that). This might be what Alistair tried to explain me in his previous reviews... I'll respin, thanks! Phil.
Hi Peter,
On 01/12/2018 02:20 PM, Philippe Mathieu-Daudé wrote:
> On Fri, Jan 12, 2018 at 2:05 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 11 January 2018 at 19:30, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>> Now both sysbus/pci classes inherit of the 'pending-insert-quirk' property,
>>> which is a HCI dependent property (regardless if accessed through a MMIO
>>> sysbus or a PCI bus).
>>> So far only the BCM implementation has to use it.
>>>
>>> Add sysbus/pci/sdbus comments to have clearer code blocks separation.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
>>
>> I'm not sure about this patch, because it means that we now have
>> no mechanism for having a property which exists on the sysbus
>> device but not on the PCI device (and as per my comments on the
>> last patch in this set, we do need to be able to do that).
Does this look OK to you?
static Property sdhci_sysbus_pending_insert_quirk_property =
DEFINE_PROP_BOOL("pending-insert-quirk",
SDHCIState, pending_insert_quirk, false);
static Property sdhci_sysbus_dma_mr_property =
DEFINE_PROP_LINK("dma",
SDHCIState, dma_mr, TYPE_MEMORY_REGION, MemoryRegion *);
static void sdhci_sysbus_post_init(Object *obj)
{
SDHCIState *s = SYSBUS_SDHCI(obj);
sdhci_common_post_init(obj); /* add common properties */
qdev_property_add_static(DEVICE(obj),
&sdhci_sysbus_pending_insert_quirk_property, &error_abort);
qdev_property_add_static(DEVICE(obj),
&sdhci_sysbus_dma_mr_property, &error_abort);
}
static const TypeInfo sdhci_sysbus_info = {
.name = TYPE_SYSBUS_SDHCI,
.parent = TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(SDHCIState),
.instance_init = sdhci_sysbus_init,
.instance_post_init = sdhci_sysbus_post_init,
...
};
This way we can have a property which exists on the sysbus
device but not on the PCI device, and keep the common one in the same
piece of code (I later add a few common properties).
Regards,
Phil.
> On 01/12/2018 02:20 PM, Philippe Mathieu-Daudé wrote:
>> On Fri, Jan 12, 2018 at 2:05 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 11 January 2018 at 19:30, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>>> Now both sysbus/pci classes inherit of the 'pending-insert-quirk' property,
>>>> which is a HCI dependent property (regardless if accessed through a MMIO
>>>> sysbus or a PCI bus).
>>>> So far only the BCM implementation has to use it.
>>>>
>>>> Add sysbus/pci/sdbus comments to have clearer code blocks separation.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
>>>
>>> I'm not sure about this patch, because it means that we now have
>>> no mechanism for having a property which exists on the sysbus
>>> device but not on the PCI device (and as per my comments on the
>>> last patch in this set, we do need to be able to do that).
>
> Does this look OK to you?
>
> static Property sdhci_sysbus_pending_insert_quirk_property =
> DEFINE_PROP_BOOL("pending-insert-quirk",
> SDHCIState, pending_insert_quirk, false);
>
> static Property sdhci_sysbus_dma_mr_property =
> DEFINE_PROP_LINK("dma",
> SDHCIState, dma_mr, TYPE_MEMORY_REGION, MemoryRegion *);
>
> static void sdhci_sysbus_post_init(Object *obj)
No need to use the post_init(), this can go in the sdhci_initfn().
> {
> SDHCIState *s = SYSBUS_SDHCI(obj);
>
> sdhci_common_post_init(obj); /* add common properties */
> qdev_property_add_static(DEVICE(obj),
> &sdhci_sysbus_pending_insert_quirk_property, &error_abort);
> qdev_property_add_static(DEVICE(obj),
> &sdhci_sysbus_dma_mr_property, &error_abort);
> }
>
> static const TypeInfo sdhci_sysbus_info = {
> .name = TYPE_SYSBUS_SDHCI,
> .parent = TYPE_SYS_BUS_DEVICE,
> .instance_size = sizeof(SDHCIState),
> .instance_init = sdhci_sysbus_init,
> .instance_post_init = sdhci_sysbus_post_init,
Not needed.
> ...
> };
>
> This way we can have a property which exists on the sysbus
> device but not on the PCI device, and keep the common one in the same
> piece of code (I later add a few common properties).
© 2016 - 2026 Red Hat, Inc.