| Message ID | 20201012124506.3406909-2-philmd@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw: Use PCI macros from 'hw/pci/pci.h' | expand |
On Mon, 12 Oct 2020, Philippe Mathieu-Daudé wrote: > From: Philippe Mathieu-Daudé <f4bug@amsat.org> > > The PCI_ADDR() macro use generic PCI fields shifted by 8-bit. > Rewrite it extracting the shift operation one layer. > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/pci-host/bonito.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c > index a99eced0657..abb3ee86769 100644 > --- a/hw/pci-host/bonito.c > +++ b/hw/pci-host/bonito.c > @@ -196,8 +196,8 @@ FIELD(BONGENCFG, PCIQUEUE, 12, 1) > #define PCI_IDSEL_VIA686B (1 << PCI_IDSEL_VIA686B_BIT) > > #define PCI_ADDR(busno , devno , funno , regno) \ > - ((((busno) << 16) & 0xff0000) + (((devno) << 11) & 0xf800) + \ > - (((funno) << 8) & 0x700) + (regno)) > + ((((busno) << 8) & 0xff00) + (((devno) << 3) & 0xf8) + \ > + (((funno) & 0x7) << 8) + (regno)) Are you missing a << 8 somewhere before + (regno) or both of these are equally unreadable and I've missed something? This seems to be completely replaced by next patch so what's the point of this change? Regards, BALATON Zoltan > > typedef struct BonitoState BonitoState; > >
On 10/12/20 3:55 PM, BALATON Zoltan wrote: > On Mon, 12 Oct 2020, Philippe Mathieu-Daudé wrote: >> From: Philippe Mathieu-Daudé <f4bug@amsat.org> >> >> The PCI_ADDR() macro use generic PCI fields shifted by 8-bit. >> Rewrite it extracting the shift operation one layer. >> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> hw/pci-host/bonito.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c >> index a99eced0657..abb3ee86769 100644 >> --- a/hw/pci-host/bonito.c >> +++ b/hw/pci-host/bonito.c >> @@ -196,8 +196,8 @@ FIELD(BONGENCFG, PCIQUEUE, 12, 1) >> #define PCI_IDSEL_VIA686B (1 << PCI_IDSEL_VIA686B_BIT) >> >> #define PCI_ADDR(busno , devno , funno , regno) \ >> - ((((busno) << 16) & 0xff0000) + (((devno) << 11) & 0xf800) + \ >> - (((funno) << 8) & 0x700) + (regno)) >> + ((((busno) << 8) & 0xff00) + (((devno) << 3) & 0xf8) + \ >> + (((funno) & 0x7) << 8) + (regno)) > > Are you missing a << 8 somewhere before + (regno) or both of these are > equally unreadable and I've missed something? This seems to be > completely replaced by next patch so what's the point of this change? I might have missed a parenthesis somewhere indeed =) I'm happy to merge it in the next patch, I thought it would be easier to review but it isn't. Thanks for reviewing! > > Regards, > BALATON Zoltan > >> >> typedef struct BonitoState BonitoState; >> >>
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c index a99eced0657..abb3ee86769 100644 --- a/hw/pci-host/bonito.c +++ b/hw/pci-host/bonito.c @@ -196,8 +196,8 @@ FIELD(BONGENCFG, PCIQUEUE, 12, 1) #define PCI_IDSEL_VIA686B (1 << PCI_IDSEL_VIA686B_BIT) #define PCI_ADDR(busno , devno , funno , regno) \ - ((((busno) << 16) & 0xff0000) + (((devno) << 11) & 0xf800) + \ - (((funno) << 8) & 0x700) + (regno)) + ((((busno) << 8) & 0xff00) + (((devno) << 3) & 0xf8) + \ + (((funno) & 0x7) << 8) + (regno)) typedef struct BonitoState BonitoState;