| Message ID | 20240213081201.78951-5-philmd@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/ide/ahci: Housekeeping | expand |
On 2/12/24 22:11, Philippe Mathieu-Daudé wrote: > Introduce the 'ich9' variable and inline ahci_get_num_ports(). > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > include/hw/ide/ahci.h | 1 - > hw/i386/pc_q35.c | 6 ++++-- > hw/ide/ahci.c | 8 -------- > hw/mips/boston.c | 6 ++++-- > 4 files changed, 8 insertions(+), 13 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> As far as it goes. But it certainly highlights that > + g_assert(MAX_SATA_PORTS == ich9->ahci.ports); > + ide_drive_get(hd, ich9->ahci.ports); .... > + g_assert(ARRAY_SIZE(hd) == ich9->ahci.ports); > + ide_drive_get(hd, ich9->ahci.ports); ports is always a constant. Or perhaps that's only from this PCI usage? r~
On 13/2/24 17:43, Richard Henderson wrote: > On 2/12/24 22:11, Philippe Mathieu-Daudé wrote: >> Introduce the 'ich9' variable and inline ahci_get_num_ports(). >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> include/hw/ide/ahci.h | 1 - >> hw/i386/pc_q35.c | 6 ++++-- >> hw/ide/ahci.c | 8 -------- >> hw/mips/boston.c | 6 ++++-- >> 4 files changed, 8 insertions(+), 13 deletions(-) > > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > > As far as it goes. But it certainly highlights that > >> + g_assert(MAX_SATA_PORTS == ich9->ahci.ports); >> + ide_drive_get(hd, ich9->ahci.ports); > .... >> + g_assert(ARRAY_SIZE(hd) == ich9->ahci.ports); >> + ide_drive_get(hd, ich9->ahci.ports); > > ports is always a constant. Or perhaps that's only from this PCI usage? I'm just moving code around :) I'm not sure about this assert, but TBH I consider ide_drive_get() as legacy API (pre -blockdev, only 7 uses in the tree, half from old boards). I'll let that constant cleanup for later. Regards, Phil.
diff --git a/include/hw/ide/ahci.h b/include/hw/ide/ahci.h index 6818d02063..dbef377f3d 100644 --- a/include/hw/ide/ahci.h +++ b/include/hw/ide/ahci.h @@ -52,7 +52,6 @@ typedef struct AHCIState { } AHCIState; -int32_t ahci_get_num_ports(PCIDevice *dev); void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **hd); #define TYPE_SYSBUS_AHCI "sysbus-ahci" diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index e298f4ff32..c50e3bfc42 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -304,16 +304,18 @@ static void pc_q35_init(MachineState *machine) if (pcms->sata_enabled) { PCIDevice *pdev; + AHCIPCIState *ich9; /* ahci and SATA device, for q35 1 ahci controller is built-in */ pdev = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_SATA1_DEV, ICH9_SATA1_FUNC), "ich9-ahci"); + ich9 = ICH9_AHCI(pdev); idebus[0] = qdev_get_child_bus(DEVICE(pdev), "ide.0"); idebus[1] = qdev_get_child_bus(DEVICE(pdev), "ide.1"); - g_assert(MAX_SATA_PORTS == ahci_get_num_ports(pdev)); - ide_drive_get(hd, ahci_get_num_ports(pdev)); + g_assert(MAX_SATA_PORTS == ich9->ahci.ports); + ide_drive_get(hd, ich9->ahci.ports); ahci_ide_create_devs(pdev, hd); } else { idebus[0] = idebus[1] = NULL; diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index aa9381a7b2..8b97c6b0e7 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1896,14 +1896,6 @@ static void sysbus_ahci_register_types(void) type_init(sysbus_ahci_register_types) -int32_t ahci_get_num_ports(PCIDevice *dev) -{ - AHCIPCIState *d = ICH9_AHCI(dev); - AHCIState *ahci = &d->ahci; - - return ahci->ports; -} - void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **hd) { AHCIPCIState *d = ICH9_AHCI(dev); diff --git a/hw/mips/boston.c b/hw/mips/boston.c index 0ec0b98066..a6c7bc18ff 100644 --- a/hw/mips/boston.c +++ b/hw/mips/boston.c @@ -678,6 +678,7 @@ static void boston_mach_init(MachineState *machine) MemoryRegion *sys_mem = get_system_memory(); XilinxPCIEHost *pcie2; PCIDevice *pdev; + AHCIPCIState *ich9; DriveInfo *hd[6]; Chardev *chr; int fw_size, fit_err; @@ -771,8 +772,9 @@ static void boston_mach_init(MachineState *machine) pdev = pci_create_simple_multifunction(&PCI_BRIDGE(&pcie2->root)->sec_bus, PCI_DEVFN(0, 0), TYPE_ICH9_AHCI); - g_assert(ARRAY_SIZE(hd) == ahci_get_num_ports(pdev)); - ide_drive_get(hd, ahci_get_num_ports(pdev)); + ich9 = ICH9_AHCI(pdev); + g_assert(ARRAY_SIZE(hd) == ich9->ahci.ports); + ide_drive_get(hd, ich9->ahci.ports); ahci_ide_create_devs(pdev, hd); if (machine->firmware) {
Introduce the 'ich9' variable and inline ahci_get_num_ports(). Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- include/hw/ide/ahci.h | 1 - hw/i386/pc_q35.c | 6 ++++-- hw/ide/ahci.c | 8 -------- hw/mips/boston.c | 6 ++++-- 4 files changed, 8 insertions(+), 13 deletions(-)