Received-SPF: pass (google.com: domain of linux-kernel+bounces-58725-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Thu, 8 Feb 2024 16:34:19 -0500
From: Frank Li <Frank.li@nxp.com>
To: Conor Dooley <conor@kernel.org>
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: <ZcVI23X4T04cFyJL@lizhi-Precision-Tower-5810>
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>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240208-outing-nature-74b6fab0cdea@spud>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?GrKkqjwKCMieODccLpwiRMuoDU4zQ6zVFFuCQhmX0q0GKaVFO69ZvWO9FQmP?=
 =?us-ascii?Q?BDD96ZLsK8fePuWk+LYBHBp7Ihx8HXqfIRo9/bzVynKlc1R/Kt4bcMyMiv/W?=
 =?us-ascii?Q?sxIP1Eu311UP8xYELcccKj2eyRS02aifnON6xwtisNIghI557SAXnP07TWLr?=
 =?us-ascii?Q?T2Duq2YWlOE9xCPHu7zM0QQSU+nVDRoG150dn8R/K+0CO2TpCBE8SkT2qWza?=
 =?us-ascii?Q?6QkKbzxCbm2pf5IoA6F1PRQ0/PdQJqJuQKQWgNG+2hX3Zq+ubW44AqiOiFwx?=
 =?us-ascii?Q?A4Ay4vhyP7OuOODsekXW62tst6SRNlOowB80Zgnui5syOpARcqpFYMHGJkUb?=
 =?us-ascii?Q?8oDZ8SAqnN/xYYAIpIduIqKIABSr8spcuVYXHU/k4X5dat2OJ+vLfNGtiZN6?=
 =?us-ascii?Q?BFihLs2FqCpc5w+0+DTBlezrF5OGhUyGBgrdIGxsnlS4RtyR40MhCLKmpXhT?=
 =?us-ascii?Q?aOpD4Pk1dl8N35ig/KmN+1JgzCjCJDqUDZiVjbN1su1Djhp+IuDdXGITzIDf?=
 =?us-ascii?Q?B3XTFwx3dVeKml1L097hefLb6RrZZSJbg9twUt0+2REXi+SAFyAbPqAqrFPx?=
 =?us-ascii?Q?id52RcW2ytM5DgD9p9egT7wstuZ/BO1MLf4XH1JuTdPUOi1dz697PkGVqHo8?=
 =?us-ascii?Q?j3l8UQboEzuhZMWKLZsLCEi06hRnGWf4M8HUujT4h/rWIIc2ealIvwdzaAYp?=
 =?us-ascii?Q?1m4gybCd8+mNz5XtdRfPHt2P0pAntOyytEGt1HApiJeVrX2XJJ3HICAIDJB/?=
 =?us-ascii?Q?SoIyMxKAFnlrs1jDt4RLIpyW8ldEhjbJ40/7CVw08t2Hv1ujdDKO1eiG8z87?=
 =?us-ascii?Q?Vxqtw5rWJcVX0zUgCsptsAnaqmSUeTgKltrJsCO/HK2yUOXcCYGtcppGx1KS?=
 =?us-ascii?Q?zu0CN0yPX1KLsjs0rWIgX/QZKmyUzRKWiRuSQOtNtj9CO4Yv9ZBXSnoZ7Dw1?=
 =?us-ascii?Q?VoWQfAyum16At8UYmk1DAL3TMnKaKHKt2t3kRuHigOCAU5d5QZop55WrmkPn?=
 =?us-ascii?Q?ON4Jw5xckGd9BTAqRpaXSvtkNTburSNrnOwP8I1Y3D99VYC0xE7BuTqr8fr7?=
 =?us-ascii?Q?TyA6R+w32DKuRpXyWEoR/DOxqBKmx9vae6eD0bPyBJO0gKqByKdLKo+z77Kk?=
 =?us-ascii?Q?MjKb1Wn3hCZOuudQgr6KzYITFLQ3O2iJUbjD4lALYSXbkkWdkz27QcbAzSWh?=
 =?us-ascii?Q?n5CVZTlxfahTGZkI4GWtDVZw1YZDGzE+cWR9+is1AIxp8G/B2Z+Zk2+2HVpQ?=
 =?us-ascii?Q?bIcD4jPw3RGzjEWzH//5+U9obIz7/BkglzvipkGdefWsjI3UpneFFIhOUBlw?=
 =?us-ascii?Q?vo3V/8RA5eIMdAde+E87gMmXpN3mVy1WAbX8IVQl29UokLAbN9xa+s9MzgyK?=
 =?us-ascii?Q?EW+oSLbnFeMy6ys/kE1qGRJmi9qDkxjQkyaqZKsO9cOXwIJ/DE2mMHiLpbJu?=
 =?us-ascii?Q?Og+rdIs6LFLopIXEck6uLzsqvt/SNsTsBJtn8Ri4aG5AjbzJqOJeXsSgFvst?=
 =?us-ascii?Q?mDuGR7QocLs4iRryjMl6cSyllefSbmViEsiEqhJCpOv+aUum5vI9fxXnlOAc?=
 =?us-ascii?Q?ukyp/79IFWg/sm3tWFylVVymKzU7FSZbBq+9vLki?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 618fbd39-fcbe-4126-812c-08dc28edb9ac
X-MS-Exchange-CrossTenant-AuthSource: PAXPR04MB9642.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Feb 2024 21:34:26.8105
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 1HvZGQrHhIsDNgCQe3Pgw75dInETlShDjDe59ZNrU2c2sf0ytynandrKwf+MgJISX+wTpWSalSt5/eDHK4sWOg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR04MB6787

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

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.

> 
> > I check old txt file, which have not mention it. I can remove it.
> 
> 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.

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.

If want to create a basic work version like this, which included some minus
fixes.  

The both method is fine for me. Second method just need more efforts.

Frank
> 
> Thanks,
> Conor.