On Thu, Aug 03, 2017 at 03:35:46AM +0000, Z.q. Hou wrote: > Hi Bjorn, > > Thanks a lot for your comments! > > > -----Original Message----- > > From: Bjorn Helgaas [mailto:helgaas@xxxxxxxxxx] > > Sent: 2017年8月3日 5:30 > > To: Z.q. Hou <zhiqiang.hou@xxxxxxx> > > Cc: linux-pci@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > > bhelgaas@xxxxxxxxxx; robh+dt@xxxxxxxxxx; Minghuan Lian > > <minghuan.Lian@xxxxxxxxxxxxx>; Mingkai Hu <mingkai.hu@xxxxxxxxxxxxx>; > > Roy Zang <tie-fei.zang@xxxxxxxxxxxxx> > > Subject: Re: [PATCH] PCI: layerscape: Add support for ls2088a and ls1088a > > > > [+cc Minghuan, Mingkai, Roy] > > > > On Thu, Jul 06, 2017 at 05:39:44PM +0800, Zhiqiang Hou wrote: > > > From: Hou Zhiqiang <Zhiqiang.Hou@xxxxxxx> > > > > > > The ls1088a and ls2088a has the same PCIe controller, so the ls1088a > > > will reuse the ls2088a's pcie compatible. > > > > > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@xxxxxxx> > > > --- > > > Documentation/devicetree/bindings/pci/layerscape-pci.txt | 1 + > > > drivers/pci/dwc/pci-layerscape.c | 9 > > +++++++++ > > > 2 files changed, 10 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt > > > b/Documentation/devicetree/bindings/pci/layerscape-pci.txt > > > index ee1c72d5..cb735e1 100644 > > > --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt > > > +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt > > > @@ -15,6 +15,7 @@ Required properties: > > > - compatible: should contain the platform identifier such as: > > > "fsl,ls1021a-pcie", "snps,dw-pcie" > > > "fsl,ls2080a-pcie", "fsl,ls2085a-pcie", "snps,dw-pcie" > > > + "fsl,ls2088a-pcie", "fsl,ls1088a-pcie" > > > > You add "fsl,ls1088a-pcie" here, but not in the ls_pcie_of_match[] > > table in the driver, so I don't think this will actually make the > > driver claim ls1088a devices, will it? > > I mean the ls1088a pcie DT nodes will use the "fsl,ls2088a-pcie" > too, so I didn't add an entry for ls1088a pcie to > ls_pcie_of_match[]. Is it reasonable? Or it's better to add > "fsl,ls1088a-pcie" to ls_pcie_of_match[], then ls1088a pcie DT nodes > use the compatible string "fsl,ls1088a-pcie"? Your binding says "fsl,ls1088a-pcie" is a valid value for "compatible". But if a DT actually *uses* that, I don't think the driver will claim it. If you want to describe ls1088a devices with "fsl,ls2088a-pcie", I think you could do that, but then why would you even mention "fsl,ls1088a-pcie" in the binding? Personally, if ls1088a and ls2088a are different in any way, I would think you should use different identifiers for them (and add them both to the binding and to the driver). That way if a future feature depends on the difference, you'll have a way to test for it. But I'm not a DT guy. Maybe Rob has a better recommendation? Bjorn
