On Tue, 5 May 2015 22:55:00 +1000
David Gibson <da...@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 <nik...@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 <th...@redhat.com>
