On 29/02/2020 23:02, BALATON Zoltan wrote:
> We'll need a flag for implementing some device specific behaviour in
> via-ide but we already have a currently CMD646 specific field that can
> be repurposed for this and leave room for furhter flags if needed in
> the future. This patch changes the "secondary" field to "flags" and
> define the flags for CMD646 and via-ide and change CMD646 and its
> users accordingly.
>
> Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu>
> ---
> hw/alpha/dp264.c | 2 +-
> hw/ide/cmd646.c | 12 ++++++------
> hw/sparc64/sun4u.c | 9 ++-------
> include/hw/ide.h | 4 ++--
> include/hw/ide/pci.h | 7 ++++++-
> 5 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
> index 8d71a30617..e4075feaaf 100644
> --- a/hw/alpha/dp264.c
> +++ b/hw/alpha/dp264.c
> @@ -102,7 +102,7 @@ static void clipper_init(MachineState *machine)
> DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> ide_drive_get(hd, ARRAY_SIZE(hd));
>
> - pci_cmd646_ide_init(pci_bus, hd, 0);
> + pci_cmd646_ide_init(pci_bus, hd, -1, false);
> }
>
> /* Load PALcode. Given that this is not "real" cpu palcode,
> diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
> index 335c060673..0be650791f 100644
> --- a/hw/ide/cmd646.c
> +++ b/hw/ide/cmd646.c
> @@ -256,7 +256,7 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error
> **errp)
> pci_conf[PCI_CLASS_PROG] = 0x8f;
>
> pci_conf[CNTRL] = CNTRL_EN_CH0; // enable IDE0
> - if (d->secondary) {
> + if (d->flags & BIT(PCI_IDE_SECONDARY)) {
> /* XXX: if not enabled, really disable the seconday IDE controller */
> pci_conf[CNTRL] |= CNTRL_EN_CH1; /* enable IDE1 */
> }
> @@ -317,20 +317,20 @@ static void pci_cmd646_ide_exitfn(PCIDevice *dev)
> }
> }
>
> -void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
> - int secondary_ide_enabled)
> +void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn,
> + bool secondary_ide_enabled)
> {
> PCIDevice *dev;
>
> - dev = pci_create(bus, -1, "cmd646-ide");
> - qdev_prop_set_uint32(&dev->qdev, "secondary", secondary_ide_enabled);
> + dev = pci_create(bus, devfn, "cmd646-ide");
> + qdev_prop_set_bit(&dev->qdev, "secondary", secondary_ide_enabled);
> qdev_init_nofail(&dev->qdev);
>
> pci_ide_create_devs(dev, hd_table);
> }
Note that legacy init functions such as pci_cmd646_ide_init() should be removed
where
possible, and in fact I posted a patch last week to completely remove it. This
is
because using qdev directly allows each board to wire up the device as required,
rather than pushing it down into a set of init functions with different
defaults.
Given that you're working in this area, I'd highly recommend doing the same for
via_ide_init() too.
> static Property cmd646_ide_properties[] = {
> - DEFINE_PROP_UINT32("secondary", PCIIDEState, secondary, 0),
> + DEFINE_PROP_BIT("secondary", PCIIDEState, flags, PCI_IDE_SECONDARY,
> false),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index b7ac42f7a5..b64899300c 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -50,8 +50,7 @@
> #include "hw/sparc/sparc64.h"
> #include "hw/nvram/fw_cfg.h"
> #include "hw/sysbus.h"
> -#include "hw/ide.h"
> -#include "hw/ide/pci.h"
> +#include "hw/ide/internal.h"
> #include "hw/loader.h"
> #include "hw/fw-path-provider.h"
> #include "elf.h"
> @@ -664,11 +663,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
> }
>
> ide_drive_get(hd, ARRAY_SIZE(hd));
> -
> - pci_dev = pci_create(pci_busA, PCI_DEVFN(3, 0), "cmd646-ide");
> - qdev_prop_set_uint32(&pci_dev->qdev, "secondary", 1);
> - qdev_init_nofail(&pci_dev->qdev);
> - pci_ide_create_devs(pci_dev, hd);
> + pci_cmd646_ide_init(pci_busA, hd, PCI_DEVFN(3, 0), true);
>
> /* Map NVRAM into I/O (ebus) space */
> nvram = m48t59_init(NULL, 0, 0, NVRAM_SIZE, 1968, 59);
> diff --git a/include/hw/ide.h b/include/hw/ide.h
> index 28d8a06439..d88c5ee695 100644
> --- a/include/hw/ide.h
> +++ b/include/hw/ide.h
> @@ -12,8 +12,8 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int
> iobase2, int isairq,
> DriveInfo *hd0, DriveInfo *hd1);
>
> /* ide-pci.c */
> -void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
> - int secondary_ide_enabled);
> +void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn,
> + bool secondary_ide_enabled);
> PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int
> devfn);
> PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
> PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
> diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
> index a9f2c33e68..21075edf16 100644
> --- a/include/hw/ide/pci.h
> +++ b/include/hw/ide/pci.h
> @@ -40,6 +40,11 @@ typedef struct BMDMAState {
> #define TYPE_PCI_IDE "pci-ide"
> #define PCI_IDE(obj) OBJECT_CHECK(PCIIDEState, (obj), TYPE_PCI_IDE)
>
> +enum {
> + PCI_IDE_SECONDARY, /* used only for cmd646 */
> + PCI_IDE_LEGACY_IRQ
> +};
> +
> typedef struct PCIIDEState {
> /*< private >*/
> PCIDevice parent_obj;
> @@ -47,7 +52,7 @@ typedef struct PCIIDEState {
>
> IDEBus bus[2];
> BMDMAState bmdma[2];
> - uint32_t secondary; /* used only for cmd646 */
> + uint32_t flags;
> MemoryRegion bmdma_bar;
> MemoryRegion cmd_bar[2];
> MemoryRegion data_bar[2];
ATB,
Mark.
