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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240208-revoke-doorman-5ba34f39c743@spud>
Date: Thu, 8 Feb 2024 19:44:59 +0000
From: Conor Dooley <conor@...nel.org>
To: Frank Li <Frank.li@....com>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>,
	Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczyński <kw@...ux.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>,
	"open list:PCI SUBSYSTEM" <linux-pci@...r.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@...r.kernel.org>,
	open list <linux-kernel@...r.kernel.org>, imx@...ts.linux.dev
Subject: Re: [PATCH 1/1] dt-bindings: pci: layerscape-pci: Convert to yaml
 file

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.

> 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.

> > > > > +  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.

Thanks,
Conor.


Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)