Message ID | 1430816036-26408-4-git-send-email-nikunj@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
On Tue, May 05, 2015 at 02:23:53PM +0530, Nikunj A Dadhania wrote: > Current code missed the Prog IF register. All Class Code, Subclass, > and Prog IF registers are needed to identify the accurate device type. > > For example: USB controllers use the PROG IF for denoting: USB > FullSpeed, HighSpeed or SuperSpeed. > > Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> Um.. I'm guessing the CLASS_PROG register essentially includes the CLASS_DEVICE value? Otherwise it looks like you're losing the CLASS_DEVICE value. For the benefit of those who don't remember the PCI spec from memory, can you explain in more detail what the situation is with the several class registers and how they overlap / interact. > --- > hw/ppc/spapr_pci.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index ea1a092..8b02a3e 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -899,8 +899,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, > _FDT(fdt_setprop_cell(fdt, offset, "revision-id", > pci_default_read_config(dev, PCI_REVISION_ID, 1))); > _FDT(fdt_setprop_cell(fdt, offset, "class-code", > - pci_default_read_config(dev, PCI_CLASS_DEVICE, 2) > - << 8)); > + pci_default_read_config(dev, PCI_CLASS_PROG, 3))); > if (pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1)) { > _FDT(fdt_setprop_cell(fdt, offset, "interrupts", > pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1)));
On Tue, 5 May 2015 22:55:00 +1000 David Gibson <david@gibson.dropbear.id.au> wrote: > On Tue, May 05, 2015 at 02:23:53PM +0530, Nikunj A Dadhania wrote: > > Current code missed the Prog IF register. All Class Code, Subclass, > > and Prog IF registers are needed to identify the accurate device type. > > > > For example: USB controllers use the PROG IF for denoting: USB > > FullSpeed, HighSpeed or SuperSpeed. > > > > Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> > > Um.. I'm guessing the CLASS_PROG register essentially includes the > CLASS_DEVICE value? Otherwise it looks like you're losing the > CLASS_DEVICE value. > > For the benefit of those who don't remember the PCI spec from memory, > can you explain in more detail what the situation is with the several > class registers and how they overlap / interact. In the PCI local bus spec, the "Class Code" is a 3-bytes field starting at offset 9 in the config space. The QEMU defines seem to split this up into PCI_CLASS_PROG (9) and PCI_CLASS_DEVICE (10), but originally they belong together. > > --- > > hw/ppc/spapr_pci.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > > index ea1a092..8b02a3e 100644 > > --- a/hw/ppc/spapr_pci.c > > +++ b/hw/ppc/spapr_pci.c > > @@ -899,8 +899,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, > > _FDT(fdt_setprop_cell(fdt, offset, "revision-id", > > pci_default_read_config(dev, PCI_REVISION_ID, 1))); > > _FDT(fdt_setprop_cell(fdt, offset, "class-code", > > - pci_default_read_config(dev, PCI_CLASS_DEVICE, 2) > > - << 8)); > > + pci_default_read_config(dev, PCI_CLASS_PROG, 3))); > > if (pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1)) { > > _FDT(fdt_setprop_cell(fdt, offset, "interrupts", > > pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1))); > Patch looks fine to me. Reviewed-by: Thomas Huth <thuth@redhat.com>
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index ea1a092..8b02a3e 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -899,8 +899,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, _FDT(fdt_setprop_cell(fdt, offset, "revision-id", pci_default_read_config(dev, PCI_REVISION_ID, 1))); _FDT(fdt_setprop_cell(fdt, offset, "class-code", - pci_default_read_config(dev, PCI_CLASS_DEVICE, 2) - << 8)); + pci_default_read_config(dev, PCI_CLASS_PROG, 3))); if (pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1)) { _FDT(fdt_setprop_cell(fdt, offset, "interrupts", pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1)));
Current code missed the Prog IF register. All Class Code, Subclass, and Prog IF registers are needed to identify the accurate device type. For example: USB controllers use the PROG IF for denoting: USB FullSpeed, HighSpeed or SuperSpeed. Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> --- hw/ppc/spapr_pci.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)