Received-SPF: pass (google.com: domain of linux-kernel+bounces-58766-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 8 Feb 2024 22:11:20 +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-nanometer-coke-07811889bafc@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>
 <20240208-revoke-doorman-5ba34f39c743@spud>
 <ZcU3ohEg5Z1ky+/W@lizhi-Precision-Tower-5810>
 <20240208-outing-nature-74b6fab0cdea@spud>
 <ZcVI23X4T04cFyJL@lizhi-Precision-Tower-5810>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="1wFBWAeU2OQ4hQEl"
Content-Disposition: inline
In-Reply-To: <ZcVI23X4T04cFyJL@lizhi-Precision-Tower-5810>


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

On Thu, Feb 08, 2024 at 04:34:19PM -0500, Frank Li wrote:
> On Thu, Feb 08, 2024 at 09:20:08PM +0000, Conor Dooley wrote:
> > On Thu, Feb 08, 2024 at 03:20:50PM -0500, Frank Li wrote:
> >=20
> > > > > > > > > +  reg:
> > > > > > > > > +    maxItems: 2
> > > > > > > > > +
> > > > > > > > > +  reg-names:
> > > > > > > > > +    items:
> > > > > > > > > +      - const: regs
> > > > > > > > > +      - const: addr_space
> > > > > > > >=20
> > > > > > > > The example uses "regs" and "config". Where did addr_space =
come from?
> > > > > > >=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
> > > >=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_spa=
ce".
> > > > Checking that there's not a u-boot or *bsd that uses "config" would=
 also
> > > > be very helpful.
> > >=20
> > > I confused. Actually this two part PCIE-RC and PCIE-EP.
> > > PCIE-RC using 'config'
> > > PCIE-EP using 'addr_spcae'
> >=20
> > Yeah, I get this. The text binding makes it seem like "config" should be
> > used for both RC and EP, so I am just asking you to check that there are
> > no drivers in other kernels or bootloaders that use "config" for EP
> > mode.
>=20
> There are not 'config' concept for EP mode. Only RC mode have 'config'
> space concept to get PCIe device's config space. EP mode only have
> "add_space" for outbound windows. If other place using "config" for EP, it
> is totally wrong, they should fix it.

It might be a totally wrong concept, but it is what the binding said, so
they are within their rights to use that name.

>=20
> >=20
> > > I check old txt file, which have not mention it. I can remove it.
> >=20
> > if you drop "addr_space", you'll need to update the endpoint driver so
> > that it supports both "addr_space" and "config". If there are no
> > endpoint drivers using "config" in other operating systems, and all the
> > dts files use "addr_space", documenting "reg" and "addr_space" for
> > endpoint mode seems fair to me.
>=20
> It is up to how to create patches. "addr_space" needs. If you want me to
> create one version, which 100% match original txt. I can do that. Then
> create increment patch to fix the problem.
>=20
> If want to create a basic work version like this, which included some min=
us
> fixes. =20
>=20
> The both method is fine for me. Second method just need more efforts.

I don't have a problem with you making it "addr_space" as long as you
check to make sure that there are no users of "config" outside the
kernel and you mention in the commit message that this is a difference
=66rom the text binding (and why).

Thanks,
Conor.

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

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

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZcVRiAAKCRB4tDGHoIJi
0jBnAQCTnRYkic8ytH+lBjI2iV6es96Hip0STSW/bDe2HdeSmAEA9DDxK8QheU2Q
BzbwvLnS5EhLH7ofoK7nlfCGHTwkHw4=
=lYmd
-----END PGP SIGNATURE-----

--1wFBWAeU2OQ4hQEl--