On Fri, Dec 29, 2017 at 9:21 AM, Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
> Hi Alistair,
>
> On 12/18/2017 10:13 PM, Alistair Francis wrote:
>> On Thu, Dec 14, 2017 at 7:15 PM, Philippe Mathieu-Daudé <f4...@amsat.org>
>> wrote:
>>> add sysbus/pci/sdbus separator comments to keep it clearer
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
>>
>> I'm still unsure about this. Won't this leave us with properties that
>> have no impact on the device? That seems very confusing to me.
>
> (from previous series)
>> [...] but aren't we now going to
>> have device properties that aren't actually connected to anything?
>
> I'm also confused :)
>
> My understanding is the "pending-insert-quirk" worries you.
Exactly, my worry is having properties that aren't connected. As a
user I expect that if I set a property that will have some effect on
how QEMU runs. A un-connected property is just confusing.
> This property is dependent of the HCI IP, so regardless the HCI is
> accessed through a MMIO sysbus or a PCI bus the quirk might exists (for
> this property, only the BCM implementation).
So it has always been configuration dependent?
Alistair
>
> With v3 series applied, the monitor 'qtree' output is:
>
> $ qemu-system-aarch64 -M xlnx-zcu102 -monitor stdio -S
> (qemu) info qtree
> dev: generic-sdhci, id ""
> gpio-out "sysbus-irq" 1
> sd-spec-version = 3 (0x3)
> timeout-freq = 0 (0x0)
> freq-in-mhz = true
> clock-freq = 0 (0x0)
> max-block-length = 512 (0x200)
> dma = true
> sdma = true
> adma1 = false
> adma2 = true
> suspend = true
> high-speed = true
> 3v3 = true
> 3v0 = false
> 1v8 = true
> 64bit = true
> slot-type = 0 (0x0)
> bus-speed = 7 (0x7)
> driver-strength = 7 (0x7)
> maxcurr = 0 (0x0)
> pending-insert-quirk = false
> dma-memory = ""
> mmio 00000000ff170000/0000000000000100
> bus: sd-bus
> type sdhci-bus
> dev: sd-card, id ""
> drive = ""
> spi = false
>
> Are you worried about this output?
>
>>> ---
>>> hw/sd/sdhci.c | 21 ++++++++++-----------
>>> 1 file changed, 10 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
>>> index 2823da00da..dbdfd54350 100644
>>> --- a/hw/sd/sdhci.c
>>> +++ b/hw/sd/sdhci.c
>>> @@ -1265,13 +1265,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);
>>> @@ -1304,7 +1308,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;
>>> }
>>>
>>> @@ -1319,14 +1323,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)
>>> {
>>> @@ -1359,7 +1356,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;
>>> }
>>> @@ -1373,6 +1370,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
>>>
>>>
>
