Message ID | 20230521111534.207973-6-shentey@gmail.com |
---|---|
State | New |
Headers | show |
Series | VIA and general PCI IDE cleanup | expand |
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>
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.
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:; }
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(-)