Re: [PATCH 17/20] hw/ide/pci: Unexport bmdma_active_if()

[Bernhard Beschow]

Am 15. Februar 2023 11:27:09 UTC schrieb "Philippe Mathieu-Daudé" 
<philmd@linaro.org>:
>From: Bernhard Beschow <shentey@gmail.com>
>
>The function is only used inside ide/pci.c, so doesn't need to be exported.
>
>Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/ide/pci.c         | 6 ++++++
> include/hw/ide/pci.h | 6 ------
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
>diff --git a/hw/ide/pci.c b/hw/ide/pci.c
>index 2ddcb49b27..fc9224bbc9 100644
>--- a/hw/ide/pci.c
>+++ b/hw/ide/pci.c
>@@ -104,6 +104,12 @@ const MemoryRegionOps pci_ide_data_le_ops = {
>     .endianness = DEVICE_LITTLE_ENDIAN,
> };
> 
>+static IDEState *bmdma_active_if(BMDMAState *bmdma)
>+{
>+    assert(bmdma->bus->retry_unit != (uint8_t)-1);
>+    return bmdma->bus->ifs + bmdma->bus->retry_unit;
>+}
>+
> static void bmdma_start_dma(const IDEDMA *dma, IDEState *s,
>                             BlockCompletionFunc *dma_cb)
> {
>diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
>index 2a6284acac..7b5e3f6e1c 100644
>--- a/include/hw/ide/pci.h
>+++ b/include/hw/ide/pci.h
>@@ -55,12 +55,6 @@ struct PCIIDEState {
>     MemoryRegion data_bar[2];
> };
> 
>-static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
>-{
>-    assert(bmdma->bus->retry_unit != (uint8_t)-1);
>-    return bmdma->bus->ifs + bmdma->bus->retry_unit;
>-}
>-
> void bmdma_init(IDEBus *bus, BMDMAState *bm, PCIIDEState *d);
> void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val);
> extern MemoryRegionOps bmdma_addr_ioport_ops;

Cool, where did you find this? ;)

This patch, your other patches doing s/ide/ide_bus/, and the fact that IDEBus 
instantiates two IDE devices itself make me wonder whether the IDE devices 
should really be instantiated in the usual QOM way. Then perhaps it could turn 
out that all the s/ide/ide_bus/ patches aren't really needed since the 
functions could operate on the IDE device directly. Not sure though...

This might all be a rabbit hole, but since you already started looking into 
it... ;)

Best regards,
Bernhard