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: > > > > > > > > > > > + 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. It might be a totally wrong concept, but it is what the binding said, so they are within their rights to use that name. > > > > > > 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. 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 from the text binding (and why). Thanks, Conor.
Attachment:
signature.asc
Description: PGP signature
