Received-SPF: pass (google.com: domain of linux-kernel+bounces-58634-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Thu, 8 Feb 2024 19:44:59 +0000
From: Conor Dooley <conor@kernel.org>
To: Frank Li <Frank.li@nxp.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Lorenzo Pieralisi <lpieralisi@kernel.org>,
	Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kw@linux.com>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	"open list:PCI SUBSYSTEM" <linux-pci@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>, imx@lists.linux.dev
Subject: Re: [PATCH 1/1] dt-bindings: pci: layerscape-pci: Convert to yaml
 file
Message-ID: <20240208-revoke-doorman-5ba34f39c743@spud>
References: <20240207062403.304367-1-Frank.Li@nxp.com>
 <20240207-yoga-mobility-90a728f6342c@spud>
 <ZcPCn8q7viB/qcOH@lizhi-Precision-Tower-5810>
 <20240208-jarring-frolic-8d4c9b409127@spud>
 <ZcUs16+Z+I4m4q00@lizhi-Precision-Tower-5810>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="rMxUxceKvHhVm2vW"
Content-Disposition: inline
In-Reply-To: <ZcUs16+Z+I4m4q00@lizhi-Precision-Tower-5810>


--rMxUxceKvHhVm2vW
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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 Configuratio=
n 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 speci=
fic 'version'
> > > > > +  register available in the Freescale PCIe controller register s=
et,
> > > > > +  which can allow determining the underlying DesignWare PCIe con=
troller 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
> > > >=20
> > > > Where did the fallback compatible go?
> > >=20
> > > So far, no fallback compatible needed now. each devices already have =
its
> > > compatible string.
> >=20
> > 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.
>=20
> 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
> > > >=20
> > > > The example uses "regs" and "config". Where did addr_space come fro=
m?
> > >=20
> > > Example just show pcie-host part. Not show pcie-ep part.
> > > pcie-ep part need 'addr_space'.
> >=20
> > Okay. Again, please mention where this is coming from.
>=20
> 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.
>=20
> I think it will be deleted soon.=20

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.


--rMxUxceKvHhVm2vW
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZcUvOwAKCRB4tDGHoIJi
0pEOAQC+6pG9TSecqqT+uAlpAWFYvajdjJeRy9uODgGeXd24MgEA2eU3EIx1Ow4G
bun7vzHAbXh2WeyPo16JHqUZjTFBRAQ=
=Du8B
-----END PGP SIGNATURE-----

--rMxUxceKvHhVm2vW--