| 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
| ||
|
Message-ID: <20250212042504.GA66848@bhelgaas>
Date: Tue, 11 Feb 2025 22:25:04 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Chen Wang <unicorn_wang@...look.com>
Cc: Chen Wang <unicornxw@...il.com>, kw@...ux.com,
u.kleine-koenig@...libre.com, aou@...s.berkeley.edu, arnd@...db.de,
bhelgaas@...gle.com, conor+dt@...nel.org, guoren@...nel.org,
inochiama@...look.com, krzk+dt@...nel.org, lee@...nel.org,
lpieralisi@...nel.org, manivannan.sadhasivam@...aro.org,
palmer@...belt.com, paul.walmsley@...ive.com, pbrobinson@...il.com,
robh@...nel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
linux-riscv@...ts.infradead.org, chao.wei@...hgo.com,
xiaoguang.xing@...hgo.com, fengchun.li@...hgo.com
Subject: Re: [PATCH v3 1/5] dt-bindings: pci: Add Sophgo SG2042 PCIe host
On Wed, Feb 12, 2025 at 09:50:11AM +0800, Chen Wang wrote:
> On 2025/2/12 7:34, Bjorn Helgaas wrote:
> > On Sun, Jan 26, 2025 at 10:27:27AM +0800, Chen Wang wrote:
> > > On 2025/1/23 6:21, Bjorn Helgaas wrote:
> > > > On Wed, Jan 15, 2025 at 03:06:37PM +0800, Chen Wang wrote:
> > > > > From: Chen Wang <unicorn_wang@...look.com>
> > > > >
> > > > > Add binding for Sophgo SG2042 PCIe host controller.
> ...
> > > > "sophgo,link-id" corresponds to Cadence documentation, but I
> > > > think it is somewhat misleading in the binding because a PCIe
> > > > "Link" refers to the downstream side of a Root Port. If we
> > > > use "link-id" to identify either Core0 or Core1 of a Cadence
> > > > IP, it sort of bakes in the idea that there can never be more
> > > > than one Root Port per Core.
> > >
> > > The fact is that for the cadence IP used by sg2042, only one
> > > root port is supported per core.
> >
> > 1) That's true today but may not be true forever.
> >
> > 2) Even if there's only one root port forever, "link" already
> > means something specific in PCIe, and this usage means something
> > different, so it's a little confusing. Maybe a comment to say
> > that this refers to a "Core", not a PCIe link, is the best we can
> > do.
>
> How about using "sophgo,core-id", as I said in the binding
> description, "every IP is composed of 2 cores (called link0 & link1
> as Cadence's term).". This avoids the conflict with the concept
> "link " in the PCIe specification, what do you think?
I think that would be great.
> > > Based on the above analysis, I think the introduction of a
> > > three-layer structure (pcie-core-port) looks a bit too
> > > complicated for candence IP. In fact, the source of the
> > > discussion at the beginning of this issue was whether some
> > > attributes should be placed under the host bridge or the root
> > > port. I suggest that adding the root port layer on the basis of
> > > the existing patch may be enough. What do you think?
> > >
> > > e.g.,
> > >
> > > pcie_rc0: pcie@...0000000 {
> > > compatible = "sophgo,sg2042-pcie-host";
> > > ...... // host bride level properties
> > > sophgo,link-id = <0>;
> > > port {
> > > // port level properties
> > > vendor-id = <0x1f1c>;
> > > device-id = <0x2042>;
> > > num-lanes = <4>;
> > > }
> > > };
> > >
> > > pcie_rc1: pcie@...2000000 {
> > > compatible = "sophgo,sg2042-pcie-host";
> > > ...... // host bride level properties
> > > sophgo,link-id = <0>;
> > > port {
> > > // port level properties
> > > vendor-id = <0x1f1c>;
> > > device-id = <0x2042>;
> > > num-lanes = <2>;
> > > };
> > > };
> > >
> > > pcie_rc2: pcie@...2800000 {
> > > compatible = "sophgo,sg2042-pcie-host";
> > > ...... // host bride level properties
> > > sophgo,link-id = <0>;
> > > port {
> > > // port level properties
> > > vendor-id = <0x1f1c>;
> > > device-id = <0x2042>;
> > > num-lanes = <2>;
> > > }
> > > };
> >
> > Where does linux,pci-domain go?
> >
> > Can you show how link-id 0 and link-id 1 would both be used? I
> > assume they need to be connected somehow, since IIUC there's some
> > register shared between them?
>
> Oh, sorry, I made a typo when I was giving the example. I wrote all
> the link-id values as 0. I rewrote it as follows. I
> changed "sophgo,link-id" to "sophgo,core-id", and added
> "linux,pci-domain".
>
> e.g.,
>
> pcie_rc0: pcie@...0000000 {
>
> compatible = "sophgo,sg2042-pcie-host";
> ...... // host bride level properties
> linux,pci-domain = <0>;
> sophgo,core-id = <0>;
> port {
> // port level properties
> vendor-id = <0x1f1c>;
> device-id = <0x2042>;
> num-lanes = <4>;
> }
> };
>
> pcie_rc1: pcie@...2000000 {
> compatible = "sophgo,sg2042-pcie-host";
> ...... // host bride level properties
> linux,pci-domain = <1>;
> sophgo,core-id = <0>;
> port {
> // port level properties
> vendor-id = <0x1f1c>;
> device-id = <0x2042>;
> num-lanes = <2>;
> };
> };
>
> pcie_rc2: pcie@...2800000 {
> compatible = "sophgo,sg2042-pcie-host";
> ...... // host bride level properties
> linux,pci-domain = <2>;
> sophgo,core-id = <1>;
> port {
> // port level properties
> vendor-id = <0x1f1c>;
> device-id = <0x2042>;
> num-lanes = <2>;
> }
>
> };
>
> pcie_rc1 and pcie_rc2 share registers in cdns_pcie1_ctrl. By using
> different "sophgo,core-id" values, they can distinguish and access
> the registers they need in cdns_pcie1_ctrl.
Where does cdns_pcie1_ctrl fit in this example? Does that enclose
both pcie_rc1 and pcie_rc2?
Powered by blists - more mailing lists