qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/1] hw/ide: share bmdma read and write functions between pii


From: Liav Albani
Subject: Re: [PATCH 1/1] hw/ide: share bmdma read and write functions between piix.c and via.c
Date: Sat, 19 Feb 2022 15:05:10 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.1


On 2/19/22 13:19, BALATON Zoltan wrote:
On Sat, 19 Feb 2022, Liav Albani wrote:
Instead of letting each implementation to duplicate this code, we can
share these functions between IDE PIIX3/4 and VIA implementations.

OK but there's a way to take this even further as cmd646 also uses similar functions just with more cases so you could remove the cases handled by these functions and only leave the difference and call the default function from the default case. E.g. (untested, just to show the idea):

hw/ide/cmd646.c:
static uint64_t bmdma_read(void *opaque, hwaddr addr,
                           unsigned size)
{
    BMDMAState *bm = opaque;
    PCIDevice *pci_dev = PCI_DEVICE(bm->pci_dev);
    uint32_t val;

    if (size != 1) {
        return ((uint64_t)1 << (size * 8)) - 1;
    }

    switch(addr & 3) {
    case 1:
        val = pci_dev->config[MRDMODE];
        break;
    case 3:
        if (bm == &bm->pci_dev->bmdma[0]) {
            val = pci_dev->config[UDIDETCR0];
        } else {
            val = pci_dev->config[UDIDETCR1];
        }
        break;
    default:
        val = bmdma_default_read(opaque, addr, size);
        break;
    }

    trace_bmdma_read_cmd646(addr, val);
    return val;
}

Yeah, I see how I can do that for both bmdma write and read of cmd646. I'll send a v2 right away with a fix.
Signed-off-by: Liav Albani <liavalb@gmail.com>
---
hw/ide/pci.c         | 47 ++++++++++++++++++++++++++++++++++++++++
hw/ide/piix.c        | 50 ++-----------------------------------------
hw/ide/via.c         | 51 ++------------------------------------------
include/hw/ide/pci.h |  4 ++++
4 files changed, 55 insertions(+), 97 deletions(-)

diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 84ba733548..c8b867659a 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -502,6 +502,53 @@ static const struct IDEDMAOps bmdma_ops = {
    .reset = bmdma_reset,
};

+uint64_t bmdma_default_read(void *opaque, hwaddr addr,
+                           unsigned size)

Indentation off? Also everywhere else, usually we indent not with the parenthesis but with the list within. (Auto indent in most editors does that probably.)

I guess you mean that it should be:

+uint64_t bmdma_default_read(void *opaque, hwaddr addr,
+                                                unsigned size)

Like this?

I'm using Visual Studio Code, so I might not have the correct settings for this editor with QEMU. The checkpatch script doesn't complain on style issues, so what can I do to make this correct?

Best regards,
Liav

Regards,
BALATON Zoltan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]