| Message ID | CH2PPF4D26F8E1C1CBD2A866C59AA55CD7AA2A12@CH2PPF4D26F8E1C.namprd07.prod.outlook.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | Enhance the PCIe controller driver | expand |
On 27/03/2025 12:19, Manikandan Karunakaran Pillai wrote: > Document the compatible property for the newly added values for PCIe EP and > RP configurations. Fix the compilation issues that came up for the existing > Cadence bindings These are two different commits. > > Signed-off-by: Manikandan K Pillai <mpillai@cadence.com> > --- > .../bindings/pci/cdns,cdns-pcie-ep.yaml | 12 +- > .../bindings/pci/cdns,cdns-pcie-host.yaml | 119 +++++++++++++++--- > 2 files changed, 110 insertions(+), 21 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml > index 98651ab22103..aa4ad69a9b71 100644 > --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml > +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml > @@ -7,14 +7,22 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# > title: Cadence PCIe EP Controller > > maintainers: > - - Tom Joseph <tjoseph@cadence.com> > + - Manikandan K Pillai <mpillai@cadence.com> > > allOf: > - $ref: cdns-pcie-ep.yaml# > > properties: > compatible: > - const: cdns,cdns-pcie-ep > + oneOf: > + - const: cdns,cdns-pcie-ep > + - const: cdns,cdns-pcie-hpa-ep What is hpa? Which soc is that? I don't think this should keep growing, but instead use SoC based compatibles. Anyway, that's enum. > + - const: cdns,cdns-cix-pcie-hpa-ep What is cix? If you want to stuff here soc in the middle, then no, no no. Please read devicetree spec and writing bindings how the compatibles are created. > + - description: PCIe EP controller from cadence > + items: > + - const: cdns,cdns-pcie-ep > + - const: cdns,cdns-pcie-hpa-ep > + - const: cdns,cdns-cix-pcie-hpa-ep This makes no sense. > > reg: > maxItems: 2 > diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml > index a8190d9b100f..bb7ffb9ddaf9 100644 > --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml > +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml > @@ -7,16 +7,30 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# > title: Cadence PCIe host controller > > maintainers: > - - Tom Joseph <tjoseph@cadence.com> > + - Manikandan K Pillai <mpillai@cadence.com> > > allOf: > - - $ref: cdns-pcie-host.yaml# > + - $ref: cdns-pcie.yaml# Why? > > properties: > + "#size-cells": > + const: 2 > + "#address-cells": > + const: 3 Huh? Why? Nothing here makes sense. Best regards, Krzysztof
>EXTERNAL MAIL > > >On 27/03/2025 12:19, Manikandan Karunakaran Pillai wrote: >> Document the compatible property for the newly added values for PCIe EP >and >> RP configurations. Fix the compilation issues that came up for the existing >> Cadence bindings > >These are two different commits. Ok > >> >> Signed-off-by: Manikandan K Pillai <mpillai@cadence.com> >> --- >> .../bindings/pci/cdns,cdns-pcie-ep.yaml | 12 +- >> .../bindings/pci/cdns,cdns-pcie-host.yaml | 119 +++++++++++++++--- >> 2 files changed, 110 insertions(+), 21 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml >b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml >> index 98651ab22103..aa4ad69a9b71 100644 >> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml >> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml >> @@ -7,14 +7,22 @@ $schema: >https://urldefense.com/v3/__http://devicetree.org/meta- >schemas/core.yaml*__;Iw!!EHscmS1ygiU1lA!CB5lvkvRUKSEDPSjpW7GJoPNyXZ >xMge5SyndD4Z-VVLCZvzLIPDP-BMRjhKZ2UTxi6a18vaodaU$ >> title: Cadence PCIe EP Controller >> >> maintainers: >> - - Tom Joseph <tjoseph@cadence.com> >> + - Manikandan K Pillai <mpillai@cadence.com> >> >> allOf: >> - $ref: cdns-pcie-ep.yaml# >> >> properties: >> compatible: >> - const: cdns,cdns-pcie-ep >> + oneOf: >> + - const: cdns,cdns-pcie-ep >> + - const: cdns,cdns-pcie-hpa-ep > >What is hpa? Which soc is that? > >I don't think this should keep growing, but instead use SoC based >compatibles. > >Anyway, that's enum. > HPA is high performance architecture based controllers. The major difference here in PCIe controllers is that the address map changes. Each of the compatibles defined here have different address maps that allow the driver to support them from the driver using compable property that provides the info from related data "struct of_device_id" in the driver. >> + - const: cdns,cdns-cix-pcie-hpa-ep > >What is cix? If you want to stuff here soc in the middle, then no, no >no. Please read devicetree spec and writing bindings how the compatibles >are created. > As mentioned in the earlier sections, cix is another implementation of the PCIe controller where the address map is changed by our customer >> + - description: PCIe EP controller from cadence >> + items: >> + - const: cdns,cdns-pcie-ep >> + - const: cdns,cdns-pcie-hpa-ep >> + - const: cdns,cdns-cix-pcie-hpa-ep > >This makes no sense. > Only one of the above compatible is valid for PCIe controllers, which will be defined in the SoC related binding. >> >> reg: >> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml >b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml >> index a8190d9b100f..bb7ffb9ddaf9 100644 >> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml >> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml >> >> maintainers: >> - - Tom Joseph <tjoseph@cadence.com> >> + - Manikandan K Pillai <mpillai@cadence.com> >> >> allOf: >> - - $ref: cdns-pcie-host.yaml# >> + - $ref: cdns-pcie.yaml# > >Why? > The existing yaml files were throwing out errors and the changes in these files are for fixing them. >> >> properties: >> + "#size-cells": >> + const: 2 >> + "#address-cells": >> + const: 3 > >Huh? Why? Nothing here makes sense. > > Compilation error related fixes. >Best regards, >Krzysztof
On 28/03/2025 06:07, Manikandan Karunakaran Pillai wrote: >> EXTERNAL MAIL >> >> >> On 27/03/2025 12:19, Manikandan Karunakaran Pillai wrote: >>> Document the compatible property for the newly added values for PCIe EP >> and >>> RP configurations. Fix the compilation issues that came up for the existing >>> Cadence bindings >> >> These are two different commits. > > Ok > >> >>> >>> Signed-off-by: Manikandan K Pillai <mpillai@cadence.com> >>> --- >>> .../bindings/pci/cdns,cdns-pcie-ep.yaml | 12 +- >>> .../bindings/pci/cdns,cdns-pcie-host.yaml | 119 +++++++++++++++--- >>> 2 files changed, 110 insertions(+), 21 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml >> b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml >>> index 98651ab22103..aa4ad69a9b71 100644 >>> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml >>> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml >>> @@ -7,14 +7,22 @@ $schema: >> https://urldefense.com/v3/__http://devicetree.org/meta- >> schemas/core.yaml*__;Iw!!EHscmS1ygiU1lA!CB5lvkvRUKSEDPSjpW7GJoPNyXZ >> xMge5SyndD4Z-VVLCZvzLIPDP-BMRjhKZ2UTxi6a18vaodaU$ >>> title: Cadence PCIe EP Controller >>> >>> maintainers: >>> - - Tom Joseph <tjoseph@cadence.com> >>> + - Manikandan K Pillai <mpillai@cadence.com> >>> >>> allOf: >>> - $ref: cdns-pcie-ep.yaml# >>> >>> properties: >>> compatible: >>> - const: cdns,cdns-pcie-ep >>> + oneOf: >>> + - const: cdns,cdns-pcie-ep >>> + - const: cdns,cdns-pcie-hpa-ep >> >> What is hpa? Which soc is that? >> >> I don't think this should keep growing, but instead use SoC based >> compatibles. >> >> Anyway, that's enum. >> > > HPA is high performance architecture based controllers. The major difference here in PCIe controllers is that > the address map changes. Each of the compatibles defined here have different address maps that allow the driver > to support them from the driver using compable property that provides the info from related data "struct of_device_id" in the driver. Just switch to SoC specific compatibles. > >>> + - const: cdns,cdns-cix-pcie-hpa-ep >> >> What is cix? If you want to stuff here soc in the middle, then no, no >> no. Please read devicetree spec and writing bindings how the compatibles >> are created. >> > > As mentioned in the earlier sections, cix is another implementation of the PCIe controller where > the address map is changed by our customer So a SoC. Use SoC compatibles and follow every other recent binding. > >>> + - description: PCIe EP controller from cadence >>> + items: >>> + - const: cdns,cdns-pcie-ep >>> + - const: cdns,cdns-pcie-hpa-ep >>> + - const: cdns,cdns-cix-pcie-hpa-ep >> >> This makes no sense. >> > Only one of the above compatible is valid for PCIe controllers, which will be defined in the SoC related binding. That's not how lists are working. Don't explain me what it does, because I know that it does nothing good: it's broken code. You can explain me what you wanted to achieve, but still this part is just wrong and makes no sense. Drop. > >>> >>> reg: >>> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml >> b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml >>> index a8190d9b100f..bb7ffb9ddaf9 100644 >>> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml >>> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml >>> >>> maintainers: >>> - - Tom Joseph <tjoseph@cadence.com> >>> + - Manikandan K Pillai <mpillai@cadence.com> >>> >>> allOf: >>> - - $ref: cdns-pcie-host.yaml# >>> + - $ref: cdns-pcie.yaml# >> >> Why? >> > > The existing yaml files were throwing out errors and the changes in these files are for fixing them. Then rather investigate the errors instead of doing random changes. > >>> >>> properties: >>> + "#size-cells": >>> + const: 2 >>> + "#address-cells": >>> + const: 3 >> >> Huh? Why? Nothing here makes sense. >> >> > Compilation error related fixes. NAK, no point at all. Best regards, Krzysztof
On Thu, Mar 27, 2025 at 11:19:47AM +0000, Manikandan Karunakaran Pillai wrote: > Document the compatible property for the newly added values for PCIe EP and > RP configurations. Fix the compilation issues that came up for the existing > Cadence bindings > > Signed-off-by: Manikandan K Pillai <mpillai@cadence.com> > --- > .../bindings/pci/cdns,cdns-pcie-ep.yaml | 12 +- > .../bindings/pci/cdns,cdns-pcie-host.yaml | 119 +++++++++++++++--- > 2 files changed, 110 insertions(+), 21 deletions(-) One more thing: SoB mismatch. Maybe got corrupted by Microsoft (it is known), so you really need to fix your mailing setup or use b4 relay. Best regards, Krzysztof
On 2025/3/28 16:22, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL > > On Thu, Mar 27, 2025 at 11:19:47AM +0000, Manikandan Karunakaran Pillai wrote: >> Document the compatible property for the newly added values for PCIe EP and >> RP configurations. Fix the compilation issues that came up for the existing >> Cadence bindings >> >> Signed-off-by: Manikandan K Pillai <mpillai@cadence.com> >> --- >> .../bindings/pci/cdns,cdns-pcie-ep.yaml | 12 +- >> .../bindings/pci/cdns,cdns-pcie-host.yaml | 119 +++++++++++++++--- >> 2 files changed, 110 insertions(+), 21 deletions(-) > > One more thing: SoB mismatch. Maybe got corrupted by Microsoft (it is > known), so you really need to fix your mailing setup or use b4 relay. > Hi Krzysztof, I have obtained Manikandan's consent and we will collaborate to submit the series patch. Our Cixtech P1 (internal name sky1) is currently upstream. Because I need upstream Cadence root port driver, However, the Cadence common code of the current linux master does not support HPA[High Performance Architecture IP] is the second generation of cadence PCIe IP. Subsequently, I will send git send-email to pci mail list. Peter Chen patchs: https://patchwork.kernel.org/project/linux-arm-kernel/cover/20250324062420.360289-1-peter.chen@cixtech.com/ Best regards, Hans
On 28/03/2025 09:48, Hans Zhang wrote: > > > On 2025/3/28 16:22, Krzysztof Kozlowski wrote: >> EXTERNAL EMAIL >> >> On Thu, Mar 27, 2025 at 11:19:47AM +0000, Manikandan Karunakaran Pillai wrote: >>> Document the compatible property for the newly added values for PCIe EP and >>> RP configurations. Fix the compilation issues that came up for the existing >>> Cadence bindings >>> >>> Signed-off-by: Manikandan K Pillai <mpillai@cadence.com> >>> --- >>> .../bindings/pci/cdns,cdns-pcie-ep.yaml | 12 +- >>> .../bindings/pci/cdns,cdns-pcie-host.yaml | 119 +++++++++++++++--- >>> 2 files changed, 110 insertions(+), 21 deletions(-) >> >> One more thing: SoB mismatch. Maybe got corrupted by Microsoft (it is >> known), so you really need to fix your mailing setup or use b4 relay. >> > > Hi Krzysztof, > > I have obtained Manikandan's consent and we will collaborate to submit It does not matter. You still need proper SoB / DCO chain. Please follow submitting patches. Best regards, Krzysztof
On 2025/3/28 17:17, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL > > On 28/03/2025 09:48, Hans Zhang wrote: >> >> >> On 2025/3/28 16:22, Krzysztof Kozlowski wrote: >>> EXTERNAL EMAIL >>> >>> On Thu, Mar 27, 2025 at 11:19:47AM +0000, Manikandan Karunakaran Pillai wrote: >>>> Document the compatible property for the newly added values for PCIe EP and >>>> RP configurations. Fix the compilation issues that came up for the existing >>>> Cadence bindings >>>> >>>> Signed-off-by: Manikandan K Pillai <mpillai@cadence.com> >>>> --- >>>> .../bindings/pci/cdns,cdns-pcie-ep.yaml | 12 +- >>>> .../bindings/pci/cdns,cdns-pcie-host.yaml | 119 +++++++++++++++--- >>>> 2 files changed, 110 insertions(+), 21 deletions(-) >>> >>> One more thing: SoB mismatch. Maybe got corrupted by Microsoft (it is >>> known), so you really need to fix your mailing setup or use b4 relay. >>> >> >> Hi Krzysztof, >> >> I have obtained Manikandan's consent and we will collaborate to submit > > It does not matter. You still need proper SoB / DCO chain. Please follow > submitting patches. > Hi Krzysztof, Thank you very much for reminding me. I will pay attention to it. Thanks Hans
diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml index 98651ab22103..aa4ad69a9b71 100644 --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml @@ -7,14 +7,22 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# title: Cadence PCIe EP Controller maintainers: - - Tom Joseph <tjoseph@cadence.com> + - Manikandan K Pillai <mpillai@cadence.com> allOf: - $ref: cdns-pcie-ep.yaml# properties: compatible: - const: cdns,cdns-pcie-ep + oneOf: + - const: cdns,cdns-pcie-ep + - const: cdns,cdns-pcie-hpa-ep + - const: cdns,cdns-cix-pcie-hpa-ep + - description: PCIe EP controller from cadence + items: + - const: cdns,cdns-pcie-ep + - const: cdns,cdns-pcie-hpa-ep + - const: cdns,cdns-cix-pcie-hpa-ep reg: maxItems: 2 diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml index a8190d9b100f..bb7ffb9ddaf9 100644 --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml @@ -7,16 +7,30 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# title: Cadence PCIe host controller maintainers: - - Tom Joseph <tjoseph@cadence.com> + - Manikandan K Pillai <mpillai@cadence.com> allOf: - - $ref: cdns-pcie-host.yaml# + - $ref: cdns-pcie.yaml# properties: + "#size-cells": + const: 2 + "#address-cells": + const: 3 + compatible: - const: cdns,cdns-pcie-host + oneOf: + - const: cdns,cdns-pcie-host + - const: cdns,cdns-pcie-hpa-host + - const: cdns,cdns-cix-pcie-hpa-host + - description: PCIe RP controller from cadence + items: + - const: cdns,cdns-pcie-host + - const: cdns,cdns-pcie-hpa-host + - const: cdns,cdns-cix-pcie-hpa-host reg: + minItems: 1 maxItems: 2 reg-names: @@ -24,6 +38,74 @@ properties: - const: reg - const: cfg + device_type: + const: pci + + vendor-id: + const: 0x17cd + + device-id: + enum: + - 0x0200 + + "#interrupt-cells": true + + interrupt-map: + minItems: 1 + maxItems: 8 + + interrupt-map-mask: + items: + - const: 0 + - const: 0 + - const: 0 + - const: 7 + + interrupts: + minItems: 1 + maxItems: 8 + + interrupt-names: + items: + - const: msi1 + - const: msi0 + + linux,pci-domain: + description: + If present this property assigns a fixed PCI domain number to a PCI + Endpoint Controller, otherwise an unstable (across boots) unique number + will be assigned. It is required to either not set this property at all + or set it for all PCI endpoint controllers in the system, otherwise + potentially conflicting domain numbers may be assigned to endpoint + controllers. The domain number for each endpoint controller in the system + must be unique. + $ref: /schemas/types.yaml#/definitions/uint32 + + ranges: + minItems: 1 + maxItems: 8 + + bus-range: + description: | + The PCI bus number range; as this is a single bus, the range + should be specified as the same value twice. + + dma-ranges: + description: | + A single range for the inbound memory region. If not supplied, + defaults to 1GiB at 0x40000000. Note there are hardware restrictions on + the allowed combinations of address and size. + maxItems: 1 + + phys: + maxItems: 1 + + phy-names: + items: + - const: pcie-phy + + msi-parent: true + required: - reg - reg-names @@ -33,37 +115,36 @@ unevaluatedProperties: false examples: - | bus { - #address-cells = <2>; - #size-cells = <2>; + #address-cells = <2>; + #size-cells = <2>; pcie@fb000000 { compatible = "cdns,cdns-pcie-host"; - device_type = "pci"; #address-cells = <3>; #size-cells = <2>; + device_type = "pci"; bus-range = <0x0 0xff>; linux,pci-domain = <0>; vendor-id = <0x17cd>; device-id = <0x0200>; - reg = <0x0 0xfb000000 0x0 0x01000000>, - <0x0 0x41000000 0x0 0x00001000>; + reg = <0xfb000000 0x01000000>,<0x41000000 0x00001000>; reg-names = "reg", "cfg"; - ranges = <0x02000000 0x0 0x42000000 0x0 0x42000000 0x0 0x1000000>, - <0x01000000 0x0 0x43000000 0x0 0x43000000 0x0 0x0010000>; - dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x1 0x00000000>; - - #interrupt-cells = <0x1>; + ranges = <0x02000000 0x0 0x42000000 0x42000000 0x0 0x1000000 0x0>; - interrupt-map = <0x0 0x0 0x0 0x1 &gic 0x0 0x0 0x0 14 0x1>, - <0x0 0x0 0x0 0x2 &gic 0x0 0x0 0x0 15 0x1>, - <0x0 0x0 0x0 0x3 &gic 0x0 0x0 0x0 16 0x1>, - <0x0 0x0 0x0 0x4 &gic 0x0 0x0 0x0 17 0x1>; + dma-ranges = <0x02000000 0x0 0x0 0x0 0x1 0x00000000 0x0>; - interrupt-map-mask = <0x0 0x0 0x0 0x7>; + #interrupt-cells = <1>; - msi-parent = <&its_pci>; + interrupt-parent = <&gic>; + interrupts = <0 118 4>, <0 116 1>; + interrupt-names = "msi1", "msi0"; + interrupt-map-mask = <0x0 0x0 0x0 0x7>; + interrupt-map = <0x0 0x0 0x0 0x1 &pcie_intc 0x1>, + <0x0 0x0 0x0 0x2 &pcie_intc 0x2>, + <0x0 0x0 0x0 0x3 &pcie_intc 0x3>, + <0x0 0x0 0x0 0x4 &pcie_intc 0x4>; phys = <&pcie_phy0>; phy-names = "pcie-phy";
Document the compatible property for the newly added values for PCIe EP and RP configurations. Fix the compilation issues that came up for the existing Cadence bindings Signed-off-by: Manikandan K Pillai <mpillai@cadence.com> --- .../bindings/pci/cdns,cdns-pcie-ep.yaml | 12 +- .../bindings/pci/cdns,cdns-pcie-host.yaml | 119 +++++++++++++++--- 2 files changed, 110 insertions(+), 21 deletions(-)