Re: [PATCH 1/1] dt-bindings: pci: layerscape-pci: Convert to yaml file

[Frank Li]

On Thu, Feb 08, 2024 at 07:44:59PM +0000, Conor Dooley wrote:
> On Thu, Feb 08, 2024 at 02:34:47PM -0500, Frank Li wrote:
> > On Thu, Feb 08, 2024 at 07:12:47PM +0000, Conor Dooley wrote:
> > > On Wed, Feb 07, 2024 at 12:49:19PM -0500, Frank Li wrote:
> > > > On Wed, Feb 07, 2024 at 05:17:55PM +0000, Conor Dooley wrote:
> > > > > On Wed, Feb 07, 2024 at 01:24:02AM -0500, Frank Li wrote:
> 
> > > > > > +
> > > > > > +  This controller derives its clocks from the Reset Configuration Word (RCW)
> > > > > > +  which is used to describe the PLL settings at the time of chip-reset.
> > > > > > +
> > > > > > +  Also as per the available Reference Manuals, there is no specific 'version'
> > > > > > +  register available in the Freescale PCIe controller register set,
> > > > > > +  which can allow determining the underlying DesignWare PCIe controller version
> > > > > > +  information.
> > > > > > +
> > > > > > +properties:
> > > > > > +  compatible:
> > > > > > +    enum:
> > > > > > +      - fsl,ls2088a-pcie-ep
> > > > > > +      - fsl,ls1088a-pcie-ep
> > > > > > +      - fsl,ls1046a-pcie-ep
> > > > > > +      - fsl,ls1028a-pcie-ep
> > > > > > +      - fsl,lx2160ar2-pcie-ep
> > > > > 
> > > > > Where did the fallback compatible go?
> > > > 
> > > > So far, no fallback compatible needed now. each devices already have its
> > > > compatible string.
> > > 
> > > It used to exist though, have you checked that u-boot or *bsd etc do not
> > > use the fallback compatible? You also need to mention your justification
> > > for removing it in the commit message.
> > 
> > This commit just convert binding doc from txt to yaml. I just make sure
> > which equal to what descript in txt.
> 
> The text binding does have a fallback compatible though:
>   EP mode:
> 	"fsl,ls1028a-pcie-ep", "fsl,ls-pcie-ep"
> So this is a change compared to the text binding, without any
> justification for it being okay to do.

Okay, I see what your concern. I think old txt is wrong. Or try to work
as it, but actually not. 

> 
> > If there are someting wrong in "uboot"
> > or "bsd", we can fixed it later.
> 
> If other bits of software are using the fallback, you cannot remove it.
> 
> > I checked driver code. exited dts tree
> > under kernel, which use unexited fallback compatible string
> > "fsl, lx-pcie-ep", which should be removed at dts file.
> 
> What do you mean by "unexisted"? It was in the text binding, so it is
> perfectly fine to have it in the dts. Given it has users, I don't think
> you should be removing the fallback without a very good justification.
> 

No driver parse "fsl,lx-pcie-ep". I can keep it as equal as old txt file
and remove later if need.

> > > > > > +  reg:
> > > > > > +    maxItems: 2
> > > > > > +
> > > > > > +  reg-names:
> > > > > > +    items:
> > > > > > +      - const: regs
> > > > > > +      - const: addr_space
> > > > > 
> > > > > The example uses "regs" and "config". Where did addr_space come from?
> > > > 
> > > > Example just show pcie-host part. Not show pcie-ep part.
> > > > pcie-ep part need 'addr_space'.
> > > 
> > > Okay. Again, please mention where this is coming from.
> > 
> > Ideally it comes from snsp,dwc-pcie-ep.yaml. but it is use 'dbi' instead
> > of 'regs'. It needs extra effort to make driver code algin common
> > snps,dwc-pcie-ep.yaml, and update exist all dts files.
> > 
> > I think it will be deleted soon. 
> 
> What I am looking for here is you to explain in the commit message that
> the endpoint driver in linux and the dts have always used "addr_space".
> Checking that there's not a u-boot or *bsd that uses "config" would also
> be very helpful.

I confused. Actually this two part PCIE-RC and PCIE-EP.
PCIE-RC using 'config'
PCIE-EP using 'addr_spcae'

I check old txt file, which have not mention it. I can remove it.

Frank
> 
> Thanks,
> Conor.
>