On Mon, Dec 03, 2018 at 04:41:50PM +0000, Lorenzo Pieralisi wrote:
> On Thu, Nov 22, 2018 at 04:53:54PM +0800, Hanjie Lin wrote:
>
> [...]
>
> > +static int meson_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
> > + u32 *val)
> > +{
> > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > +
> > + /*
> > + * there is a bug of MESON AXG pcie controller that software can not
> > + * programe PCI_CLASS_DEVICE register, so we must return a fake right
> > + * value to ensure driver could probe successfully.
> > + */
> > + if (where == PCI_CLASS_REVISION) {
> > + *val = readl(pci->dbi_base + PCI_CLASS_REVISION);
> > + /* keep revision id */
> > + *val &= PCI_CLASS_REVISION_MASK;
> > + *val |= PCI_CLASS_BRIDGE_PCI << 16;
> > + return PCIBIOS_SUCCESSFUL;
> > + }
>
> As I said before, this looks broken. If this code (or other drivers with
> the same broken assumptions, eg dwc/pcie-qcom.c) carries out a, say,
> byte sized config access of eg PCI_CLASS_DEVICE you will get junk out of
> it according to your comment above.
>
> I would like to pick Bjorn's brain on this to see what we can really do
> to fix this (and other) drivers.
- Check to see whether you're reading anything in the 32-bit dword at
offset 0x08.
- Do the 32-bit readl().
- Insert the correct Sub-Class and Base Class code (you also throw
away the Programming Interface; not sure why that is)
- If you're reading something smaller than 32 bits, mask & shift as
needed. pci_bridge_emul_conf_read() does something similar that
you might be able to copy.
Out of curiosity, what code depends on PCI_CLASS_BRIDGE_PCI? There
are several places in the kernel that currently depend on it, but I
think several of them *should* be checking dev->hdr_type to identify a
type 1 header instead.
Bjorn
