1. Patchew
  2. QEMU
  3. [PATCH 0/6] VIA and general PCI IDE cleanup
  4. View patch

[PATCH 5/6] hw/ide: Extract bmdma_status_writeb()

Bernhard Beschow posted 6 patches 2 years, 8 months ago
Maintainers: John Snow <jsnow@redhat.com>, BALATON Zoltan <balaton@eik.bme.hu>, Huacai Chen <chenhuacai@kernel.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>
There is a newer version of this series
[PATCH 5/6] hw/ide: Extract bmdma_status_writeb()
Posted by Bernhard Beschow 2 years, 8 months ago 
Every TYPE_PCI_IDE device performs the same not-so-trivial bit manipulation by
copy'n'paste code. Extract this into bmdma_status_writeb(), mirroring
bmdma_cmd_writeb().

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 include/hw/ide/pci.h | 1 +
 hw/ide/cmd646.c      | 2 +-
 hw/ide/pci.c         | 5 +++++
 hw/ide/piix.c        | 2 +-
 hw/ide/sii3112.c     | 6 ++----
 hw/ide/via.c         | 2 +-
 6 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index 74c127e32f..1ff469de87 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -58,6 +58,7 @@ struct PCIIDEState {
 
 void bmdma_init(IDEBus *bus, BMDMAState *bm, PCIIDEState *d);
 void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val);
+void bmdma_status_writeb(BMDMAState *bm, uint32_t val);
 extern MemoryRegionOps bmdma_addr_ioport_ops;
 void pci_ide_create_devs(PCIDevice *dev);
 
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index a094a6e12a..cabe9048b1 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -144,7 +144,7 @@ static void bmdma_write(void *opaque, hwaddr addr,
         cmd646_update_irq(pci_dev);
         break;
     case 2:
-        bm->status = (val & 0x60) | (bm->status & 1) | (bm->status & ~val & 0x06);
+        bmdma_status_writeb(bm, val);
         break;
     case 3:
         if (bm == &bm->pci_dev->bmdma[0]) {
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 4cf1e9c679..b258fd2f50 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -318,6 +318,11 @@ void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val)
     bm->cmd = val & 0x09;
 }
 
+void bmdma_status_writeb(BMDMAState *bm, uint32_t val)
+{
+    bm->status = (val & 0x60) | (bm->status & BM_STATUS_DMAING) | (bm->status & ~val & 0x06);
+}
+
 static uint64_t bmdma_addr_read(void *opaque, hwaddr addr,
                                 unsigned width)
 {
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index a32f7ccece..47e0b474c3 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -76,7 +76,7 @@ static void bmdma_write(void *opaque, hwaddr addr,
         bmdma_cmd_writeb(bm, val);
         break;
     case 2:
-        bm->status = (val & 0x60) | (bm->status & 1) | (bm->status & ~val & 0x06);
+        bmdma_status_writeb(bm, val);
         break;
     }
 }
diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
index 5dd3d03c29..63dc4a0494 100644
--- a/hw/ide/sii3112.c
+++ b/hw/ide/sii3112.c
@@ -149,8 +149,7 @@ static void sii3112_reg_write(void *opaque, hwaddr addr,
         break;
     case 0x02:
     case 0x12:
-        d->i.bmdma[0].status = (val & 0x60) | (d->i.bmdma[0].status & 1) |
-                               (d->i.bmdma[0].status & ~val & 6);
+        bmdma_status_writeb(&d->i.bmdma[0], val);
         break;
     case 0x04 ... 0x07:
         bmdma_addr_ioport_ops.write(&d->i.bmdma[0], addr - 4, val, size);
@@ -165,8 +164,7 @@ static void sii3112_reg_write(void *opaque, hwaddr addr,
         break;
     case 0x0a:
     case 0x1a:
-        d->i.bmdma[1].status = (val & 0x60) | (d->i.bmdma[1].status & 1) |
-                               (d->i.bmdma[1].status & ~val & 6);
+        bmdma_status_writeb(&d->i.bmdma[1], val);
         break;
     case 0x0c ... 0x0f:
         bmdma_addr_ioport_ops.write(&d->i.bmdma[1], addr - 12, val, size);
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 91253fa4ef..fff23803a6 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -75,7 +75,7 @@ static void bmdma_write(void *opaque, hwaddr addr,
         bmdma_cmd_writeb(bm, val);
         break;
     case 2:
-        bm->status = (val & 0x60) | (bm->status & 1) | (bm->status & ~val & 0x06);
+        bmdma_status_writeb(bm, val);
         break;
     default:;
     }
-- 
2.40.1
Re: [PATCH 5/6] hw/ide: Extract bmdma_status_writeb()
Posted by Mark Cave-Ayland 2 years, 8 months ago 
On 21/05/2023 12:15, Bernhard Beschow wrote:
> Every TYPE_PCI_IDE device performs the same not-so-trivial bit manipulation by
> copy'n'paste code. Extract this into bmdma_status_writeb(), mirroring
> bmdma_cmd_writeb().
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   include/hw/ide/pci.h | 1 +
>   hw/ide/cmd646.c      | 2 +-
>   hw/ide/pci.c         | 5 +++++
>   hw/ide/piix.c        | 2 +-
>   hw/ide/sii3112.c     | 6 ++----
>   hw/ide/via.c         | 2 +-
>   6 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
> index 74c127e32f..1ff469de87 100644
> --- a/include/hw/ide/pci.h
> +++ b/include/hw/ide/pci.h
> @@ -58,6 +58,7 @@ struct PCIIDEState {
>   
>   void bmdma_init(IDEBus *bus, BMDMAState *bm, PCIIDEState *d);
>   void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val);
> +void bmdma_status_writeb(BMDMAState *bm, uint32_t val);
>   extern MemoryRegionOps bmdma_addr_ioport_ops;
>   void pci_ide_create_devs(PCIDevice *dev);
>   
> diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
> index a094a6e12a..cabe9048b1 100644
> --- a/hw/ide/cmd646.c
> +++ b/hw/ide/cmd646.c
> @@ -144,7 +144,7 @@ static void bmdma_write(void *opaque, hwaddr addr,
>           cmd646_update_irq(pci_dev);
>           break;
>       case 2:
> -        bm->status = (val & 0x60) | (bm->status & 1) | (bm->status & ~val & 0x06);
> +        bmdma_status_writeb(bm, val);
>           break;
>       case 3:
>           if (bm == &bm->pci_dev->bmdma[0]) {
> diff --git a/hw/ide/pci.c b/hw/ide/pci.c
> index 4cf1e9c679..b258fd2f50 100644
> --- a/hw/ide/pci.c
> +++ b/hw/ide/pci.c
> @@ -318,6 +318,11 @@ void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val)
>       bm->cmd = val & 0x09;
>   }
>   
> +void bmdma_status_writeb(BMDMAState *bm, uint32_t val)
> +{
> +    bm->status = (val & 0x60) | (bm->status & BM_STATUS_DMAING) | (bm->status & ~val & 0x06);

Does this fit the 80 char limit if you run the series through scripts/checkpatch.pl?

> +}
> +
>   static uint64_t bmdma_addr_read(void *opaque, hwaddr addr,
>                                   unsigned width)
>   {
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index a32f7ccece..47e0b474c3 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -76,7 +76,7 @@ static void bmdma_write(void *opaque, hwaddr addr,
>           bmdma_cmd_writeb(bm, val);
>           break;
>       case 2:
> -        bm->status = (val & 0x60) | (bm->status & 1) | (bm->status & ~val & 0x06);
> +        bmdma_status_writeb(bm, val);
>           break;
>       }
>   }
> diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
> index 5dd3d03c29..63dc4a0494 100644
> --- a/hw/ide/sii3112.c
> +++ b/hw/ide/sii3112.c
> @@ -149,8 +149,7 @@ static void sii3112_reg_write(void *opaque, hwaddr addr,
>           break;
>       case 0x02:
>       case 0x12:
> -        d->i.bmdma[0].status = (val & 0x60) | (d->i.bmdma[0].status & 1) |
> -                               (d->i.bmdma[0].status & ~val & 6);
> +        bmdma_status_writeb(&d->i.bmdma[0], val);
>           break;
>       case 0x04 ... 0x07:
>           bmdma_addr_ioport_ops.write(&d->i.bmdma[0], addr - 4, val, size);
> @@ -165,8 +164,7 @@ static void sii3112_reg_write(void *opaque, hwaddr addr,
>           break;
>       case 0x0a:
>       case 0x1a:
> -        d->i.bmdma[1].status = (val & 0x60) | (d->i.bmdma[1].status & 1) |
> -                               (d->i.bmdma[1].status & ~val & 6);
> +        bmdma_status_writeb(&d->i.bmdma[1], val);
>           break;
>       case 0x0c ... 0x0f:
>           bmdma_addr_ioport_ops.write(&d->i.bmdma[1], addr - 12, val, size);
> diff --git a/hw/ide/via.c b/hw/ide/via.c
> index 91253fa4ef..fff23803a6 100644
> --- a/hw/ide/via.c
> +++ b/hw/ide/via.c
> @@ -75,7 +75,7 @@ static void bmdma_write(void *opaque, hwaddr addr,
>           bmdma_cmd_writeb(bm, val);
>           break;
>       case 2:
> -        bm->status = (val & 0x60) | (bm->status & 1) | (bm->status & ~val & 0x06);
> +        bmdma_status_writeb(bm, val);
>           break;
>       default:;
>       }

Otherwise looks good to me:

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.
Re: [PATCH 5/6] hw/ide: Extract bmdma_status_writeb()
Posted by BALATON Zoltan 2 years, 8 months ago 
On Sun, 21 May 2023, Bernhard Beschow wrote:
> Every TYPE_PCI_IDE device performs the same not-so-trivial bit manipulation by
> copy'n'paste code. Extract this into bmdma_status_writeb(), mirroring
> bmdma_cmd_writeb().
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.