| Message ID | 20190719093821.14278-1-oohall@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] pci-quirk: Re-order struct members | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (3a6fdede6ce117facec0108afe716cf5d0472c3f) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | fail | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
On Fri, 2019-07-19 at 19:38 +1000, Oliver O'Halloran wrote: > Signed-off-by: Oliver O'Halloran <oohall@gmail.com> LGTM Acked-by: Michael Neuling <mikey@neuling.org> oohal: Will you release my family now? :-P Mikey > --- > core/pci-quirk.c | 4 ++-- > include/pci-quirk.h | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/core/pci-quirk.c b/core/pci-quirk.c > index 1007e9621d71..0862e8dc4072 100644 > --- a/core/pci-quirk.c > +++ b/core/pci-quirk.c > @@ -52,8 +52,8 @@ static void quirk_astbmc_vga(struct phb *phb __unused, > /* Quirks are: {fixup function, vendor ID, (device ID or PCI_ANY_ID)} */ > static const struct pci_quirk quirk_table[] = { > /* ASPEED 2400 VGA device */ > - { &quirk_astbmc_vga, 0x1a03, 0x2000 }, > - { NULL, 0, 0 } > + { 0x1a03, 0x2000, &quirk_astbmc_vga }, > + { 0, 0, NULL } > }; > > static void __pci_handle_quirk(struct phb *phb, struct pci_device *pd, > diff --git a/include/pci-quirk.h b/include/pci-quirk.h > index 1883248af548..bea0c2f88601 100644 > --- a/include/pci-quirk.h > +++ b/include/pci-quirk.h > @@ -22,9 +22,9 @@ > #define PCI_ANY_ID 0xFFFF > > struct pci_quirk { > - void (*fixup)(struct phb *, struct pci_device *); > uint16_t vendor_id; > uint16_t device_id; > + void (*fixup)(struct phb *, struct pci_device *); > }; > > void pci_handle_quirk(struct phb *phb, struct pci_device *pd);
I'm curious, why are you re-ordering the struct members? Also you need to update the quirk table in core/test/run-pci-quirk.c. Other than that this looks ok though so once that's fixed you can add: Reviewed-by: Alistair Popple <alistair@popple.id.au> On Friday, 19 July 2019 7:38:19 PM AEST Oliver O'Halloran wrote: > Signed-off-by: Oliver O'Halloran <oohall@gmail.com> > --- > core/pci-quirk.c | 4 ++-- > include/pci-quirk.h | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/core/pci-quirk.c b/core/pci-quirk.c > index 1007e9621d71..0862e8dc4072 100644 > --- a/core/pci-quirk.c > +++ b/core/pci-quirk.c > @@ -52,8 +52,8 @@ static void quirk_astbmc_vga(struct phb *phb __unused, > /* Quirks are: {fixup function, vendor ID, (device ID or PCI_ANY_ID)} */ > static const struct pci_quirk quirk_table[] = { > /* ASPEED 2400 VGA device */ > - { &quirk_astbmc_vga, 0x1a03, 0x2000 }, > - { NULL, 0, 0 } > + { 0x1a03, 0x2000, &quirk_astbmc_vga }, > + { 0, 0, NULL } > }; > > static void __pci_handle_quirk(struct phb *phb, struct pci_device *pd, > diff --git a/include/pci-quirk.h b/include/pci-quirk.h > index 1883248af548..bea0c2f88601 100644 > --- a/include/pci-quirk.h > +++ b/include/pci-quirk.h > @@ -22,9 +22,9 @@ > #define PCI_ANY_ID 0xFFFF > > struct pci_quirk { > - void (*fixup)(struct phb *, struct pci_device *); > uint16_t vendor_id; > uint16_t device_id; > + void (*fixup)(struct phb *, struct pci_device *); > }; > > void pci_handle_quirk(struct phb *phb, struct pci_device *pd);
On Wed, Jul 24, 2019 at 11:27 AM Alistair Popple <alistair@popple.id.au> wrote: > > I'm curious, why are you re-ordering the struct members? Oh whoops, the commit message went AWOL. Having the function first means the vendor and device IDs will be unaligned unless the function names are the same length (or padded out) and that just won't do. > Also you need to update the quirk table in core/test/run-pci-quirk.c. Other > than that this looks ok though so once that's fixed you can add: ah yeah, good point. > > Reviewed-by: Alistair Popple <alistair@popple.id.au>
diff --git a/core/pci-quirk.c b/core/pci-quirk.c index 1007e9621d71..0862e8dc4072 100644 --- a/core/pci-quirk.c +++ b/core/pci-quirk.c @@ -52,8 +52,8 @@ static void quirk_astbmc_vga(struct phb *phb __unused, /* Quirks are: {fixup function, vendor ID, (device ID or PCI_ANY_ID)} */ static const struct pci_quirk quirk_table[] = { /* ASPEED 2400 VGA device */ - { &quirk_astbmc_vga, 0x1a03, 0x2000 }, - { NULL, 0, 0 } + { 0x1a03, 0x2000, &quirk_astbmc_vga }, + { 0, 0, NULL } }; static void __pci_handle_quirk(struct phb *phb, struct pci_device *pd, diff --git a/include/pci-quirk.h b/include/pci-quirk.h index 1883248af548..bea0c2f88601 100644 --- a/include/pci-quirk.h +++ b/include/pci-quirk.h @@ -22,9 +22,9 @@ #define PCI_ANY_ID 0xFFFF struct pci_quirk { - void (*fixup)(struct phb *, struct pci_device *); uint16_t vendor_id; uint16_t device_id; + void (*fixup)(struct phb *, struct pci_device *); }; void pci_handle_quirk(struct phb *phb, struct pci_device *pd);
Signed-off-by: Oliver O'Halloran <oohall@gmail.com> --- core/pci-quirk.c | 4 ++-- include/pci-quirk.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)