| Message ID | 1695218113-31198-2-git-send-email-quic_msarkar@quicinc.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | arm64: qcom: sa8775p: add support for EP PCIe | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Wed, Sep 20, 2023 at 07:25:08PM +0530, Mrinmay Sarkar wrote: > Add devicetree bindings support for SA8775P SoC. > Define reg and interrupt per platform. > > Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> > --- > .../devicetree/bindings/pci/qcom,pcie-ep.yaml | 130 +++++++++++++++++---- > 1 file changed, 108 insertions(+), 22 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > index a223ce0..e860e8f 100644 > --- a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > @@ -13,6 +13,7 @@ properties: > compatible: > oneOf: > - enum: > + - qcom,sa8775p-pcie-ep > - qcom,sdx55-pcie-ep > - qcom,sm8450-pcie-ep > - items: > @@ -20,29 +21,19 @@ properties: > - const: qcom,sdx55-pcie-ep > > reg: > - items: > - - description: Qualcomm-specific PARF configuration registers > - - description: DesignWare PCIe registers > - - description: External local bus interface registers > - - description: Address Translation Unit (ATU) registers > - - description: Memory region used to map remote RC address space > - - description: BAR memory region > + minItems: 6 > + maxItems: 7 > > reg-names: > - items: > - - const: parf > - - const: dbi > - - const: elbi > - - const: atu > - - const: addr_space > - - const: mmio > + minItems: 6 > + maxItems: 7 > > clocks: > - minItems: 7 > + minItems: 5 > maxItems: 8 > > clock-names: > - minItems: 7 > + minItems: 5 > maxItems: 8 > > qcom,perst-regs: > @@ -57,14 +48,12 @@ properties: > - description: Perst separation enable offset > > interrupts: > - items: > - - description: PCIe Global interrupt > - - description: PCIe Doorbell interrupt > + minItems: 2 > + maxItems: 3 > > interrupt-names: > - items: > - - const: global > - - const: doorbell > + minItems: 2 > + maxItems: 3 > > reset-gpios: > description: GPIO used as PERST# input signal > @@ -122,6 +111,51 @@ allOf: > compatible: > contains: > enum: > + - qcom,sa8775p-pcie-ep > + then: > + properties: > + reg: > + items: > + - description: Qualcomm-specific PARF configuration registers > + - description: DesignWare PCIe registers > + - description: External local bus interface registers > + - description: Address Translation Unit (ATU) registers > + - description: Memory region used to map remote RC address space > + - description: BAR memory region > + - description: DMA memory region It should be described as "DMA register space" or something, because this could be misinterpreted as memory region for doing DMA. - Mani
On Wed, Sep 20, 2023 at 07:25:08PM +0530, Mrinmay Sarkar wrote: > Add devicetree bindings support for SA8775P SoC. > Define reg and interrupt per platform. > > Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> > --- > .../devicetree/bindings/pci/qcom,pcie-ep.yaml | 130 +++++++++++++++++---- > 1 file changed, 108 insertions(+), 22 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > index a223ce0..e860e8f 100644 > --- a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > @@ -13,6 +13,7 @@ properties: > compatible: > oneOf: > - enum: > + - qcom,sa8775p-pcie-ep > - qcom,sdx55-pcie-ep > - qcom,sm8450-pcie-ep > - items: > @@ -20,29 +21,19 @@ properties: > - const: qcom,sdx55-pcie-ep > > reg: > - items: > - - description: Qualcomm-specific PARF configuration registers > - - description: DesignWare PCIe registers > - - description: External local bus interface registers > - - description: Address Translation Unit (ATU) registers > - - description: Memory region used to map remote RC address space > - - description: BAR memory region > + minItems: 6 > + maxItems: 7 > > reg-names: > - items: > - - const: parf > - - const: dbi > - - const: elbi > - - const: atu > - - const: addr_space > - - const: mmio > + minItems: 6 > + maxItems: 7 Don't move these into if/then schemas. Then we are duplicating the names, and there is no reason to keep them aligned for new compatibles. Rob
On 9/22/2023 12:08 AM, Rob Herring wrote: > On Wed, Sep 20, 2023 at 07:25:08PM +0530, Mrinmay Sarkar wrote: >> Add devicetree bindings support for SA8775P SoC. >> Define reg and interrupt per platform. >> >> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> >> --- >> .../devicetree/bindings/pci/qcom,pcie-ep.yaml | 130 +++++++++++++++++---- >> 1 file changed, 108 insertions(+), 22 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml >> index a223ce0..e860e8f 100644 >> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml >> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml >> @@ -13,6 +13,7 @@ properties: >> compatible: >> oneOf: >> - enum: >> + - qcom,sa8775p-pcie-ep >> - qcom,sdx55-pcie-ep >> - qcom,sm8450-pcie-ep >> - items: >> @@ -20,29 +21,19 @@ properties: >> - const: qcom,sdx55-pcie-ep >> >> reg: >> - items: >> - - description: Qualcomm-specific PARF configuration registers >> - - description: DesignWare PCIe registers >> - - description: External local bus interface registers >> - - description: Address Translation Unit (ATU) registers >> - - description: Memory region used to map remote RC address space >> - - description: BAR memory region >> + minItems: 6 >> + maxItems: 7 >> >> reg-names: >> - items: >> - - const: parf >> - - const: dbi >> - - const: elbi >> - - const: atu >> - - const: addr_space >> - - const: mmio >> + minItems: 6 >> + maxItems: 7 > > Don't move these into if/then schemas. Then we are duplicating the > names, and there is no reason to keep them aligned for new compatibles. > > Rob Hi Rob, As we have one extra reg property (dma) required for sa8775p-pcie-ep, isn't it expected to be moved in if/then as per number of regs required. Anyways we would have duplication of some properties for new compatibles where the member numbers differs for a property. Are you suggesting to add the extra reg property (dma) in the existing reg and reg-names list, and add minItems/maxItems for all compatibles present in this file ? -Shazad
On 10/6/2023 4:24 PM, Shazad Hussain wrote: > > > On 9/22/2023 12:08 AM, Rob Herring wrote: >> On Wed, Sep 20, 2023 at 07:25:08PM +0530, Mrinmay Sarkar wrote: >>> Add devicetree bindings support for SA8775P SoC. >>> Define reg and interrupt per platform. >>> >>> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> >>> --- >>> .../devicetree/bindings/pci/qcom,pcie-ep.yaml | 130 >>> +++++++++++++++++---- >>> 1 file changed, 108 insertions(+), 22 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml >>> b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml >>> index a223ce0..e860e8f 100644 >>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml >>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml >>> @@ -13,6 +13,7 @@ properties: >>> compatible: >>> oneOf: >>> - enum: >>> + - qcom,sa8775p-pcie-ep >>> - qcom,sdx55-pcie-ep >>> - qcom,sm8450-pcie-ep >>> - items: >>> @@ -20,29 +21,19 @@ properties: >>> - const: qcom,sdx55-pcie-ep >>> reg: >>> - items: >>> - - description: Qualcomm-specific PARF configuration registers >>> - - description: DesignWare PCIe registers >>> - - description: External local bus interface registers >>> - - description: Address Translation Unit (ATU) registers >>> - - description: Memory region used to map remote RC address space >>> - - description: BAR memory region >>> + minItems: 6 >>> + maxItems: 7 >>> reg-names: >>> - items: >>> - - const: parf >>> - - const: dbi >>> - - const: elbi >>> - - const: atu >>> - - const: addr_space >>> - - const: mmio >>> + minItems: 6 >>> + maxItems: 7 >> >> Don't move these into if/then schemas. Then we are duplicating the >> names, and there is no reason to keep them aligned for new compatibles. >> >> Rob > > Hi Rob, > As we have one extra reg property (dma) required for sa8775p-pcie-ep, > isn't it expected to be moved in if/then as per number of regs > required. Anyways we would have duplication of some properties for new > compatibles where the member numbers differs for a property. > > Are you suggesting to add the extra reg property (dma) in the existing > reg and reg-names list, and add minItems/maxItems for all compatibles > present in this file ? > > -Shazad Here we have defined reg and interrupt per platform as clocks is defined. -Mrinmay
On Wed, 11 Oct 2023 at 14:14, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote: > > > On 10/6/2023 4:24 PM, Shazad Hussain wrote: > > > > > > On 9/22/2023 12:08 AM, Rob Herring wrote: > >> On Wed, Sep 20, 2023 at 07:25:08PM +0530, Mrinmay Sarkar wrote: > >>> Add devicetree bindings support for SA8775P SoC. > >>> Define reg and interrupt per platform. > >>> > >>> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> > >>> --- > >>> .../devicetree/bindings/pci/qcom,pcie-ep.yaml | 130 > >>> +++++++++++++++++---- > >>> 1 file changed, 108 insertions(+), 22 deletions(-) > >>> > >>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > >>> b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > >>> index a223ce0..e860e8f 100644 > >>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > >>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > >>> @@ -13,6 +13,7 @@ properties: > >>> compatible: > >>> oneOf: > >>> - enum: > >>> + - qcom,sa8775p-pcie-ep > >>> - qcom,sdx55-pcie-ep > >>> - qcom,sm8450-pcie-ep > >>> - items: > >>> @@ -20,29 +21,19 @@ properties: > >>> - const: qcom,sdx55-pcie-ep > >>> reg: > >>> - items: > >>> - - description: Qualcomm-specific PARF configuration registers > >>> - - description: DesignWare PCIe registers > >>> - - description: External local bus interface registers > >>> - - description: Address Translation Unit (ATU) registers > >>> - - description: Memory region used to map remote RC address space > >>> - - description: BAR memory region > >>> + minItems: 6 > >>> + maxItems: 7 > >>> reg-names: > >>> - items: > >>> - - const: parf > >>> - - const: dbi > >>> - - const: elbi > >>> - - const: atu > >>> - - const: addr_space > >>> - - const: mmio > >>> + minItems: 6 > >>> + maxItems: 7 > >> > >> Don't move these into if/then schemas. Then we are duplicating the > >> names, and there is no reason to keep them aligned for new compatibles. > >> > >> Rob > > > > Hi Rob, > > As we have one extra reg property (dma) required for sa8775p-pcie-ep, > > isn't it expected to be moved in if/then as per number of regs > > required. Anyways we would have duplication of some properties for new > > compatibles where the member numbers differs for a property. > > > > Are you suggesting to add the extra reg property (dma) in the existing > > reg and reg-names list, and add minItems/maxItems for all compatibles > > present in this file ? This is what we have been doing in other cases: if the list is an extension of the current list, there is no need to duplicate it. One can use min/maxItems instead. > > > > -Shazad > > Here we have defined reg and interrupt per platform as clocks is defined. > > -Mrinmay >
On 10/11/2023 5:13 PM, Dmitry Baryshkov wrote: > On Wed, 11 Oct 2023 at 14:14, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote: >> >> On 10/6/2023 4:24 PM, Shazad Hussain wrote: >>> >>> On 9/22/2023 12:08 AM, Rob Herring wrote: >>>> On Wed, Sep 20, 2023 at 07:25:08PM +0530, Mrinmay Sarkar wrote: >>>>> Add devicetree bindings support for SA8775P SoC. >>>>> Define reg and interrupt per platform. >>>>> >>>>> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> >>>>> --- >>>>> .../devicetree/bindings/pci/qcom,pcie-ep.yaml | 130 >>>>> +++++++++++++++++---- >>>>> 1 file changed, 108 insertions(+), 22 deletions(-) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml >>>>> b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml >>>>> index a223ce0..e860e8f 100644 >>>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml >>>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml >>>>> @@ -13,6 +13,7 @@ properties: >>>>> compatible: >>>>> oneOf: >>>>> - enum: >>>>> + - qcom,sa8775p-pcie-ep >>>>> - qcom,sdx55-pcie-ep >>>>> - qcom,sm8450-pcie-ep >>>>> - items: >>>>> @@ -20,29 +21,19 @@ properties: >>>>> - const: qcom,sdx55-pcie-ep >>>>> reg: >>>>> - items: >>>>> - - description: Qualcomm-specific PARF configuration registers >>>>> - - description: DesignWare PCIe registers >>>>> - - description: External local bus interface registers >>>>> - - description: Address Translation Unit (ATU) registers >>>>> - - description: Memory region used to map remote RC address space >>>>> - - description: BAR memory region >>>>> + minItems: 6 >>>>> + maxItems: 7 >>>>> reg-names: >>>>> - items: >>>>> - - const: parf >>>>> - - const: dbi >>>>> - - const: elbi >>>>> - - const: atu >>>>> - - const: addr_space >>>>> - - const: mmio >>>>> + minItems: 6 >>>>> + maxItems: 7 >>>> Don't move these into if/then schemas. Then we are duplicating the >>>> names, and there is no reason to keep them aligned for new compatibles. >>>> >>>> Rob >>> Hi Rob, >>> As we have one extra reg property (dma) required for sa8775p-pcie-ep, >>> isn't it expected to be moved in if/then as per number of regs >>> required. Anyways we would have duplication of some properties for new >>> compatibles where the member numbers differs for a property. >>> >>> Are you suggesting to add the extra reg property (dma) in the existing >>> reg and reg-names list, and add minItems/maxItems for all compatibles >>> present in this file ? > This is what we have been doing in other cases: if the list is an > extension of the current list, there is no need to duplicate it. One > can use min/maxItems instead. Hi Dmitry we have tried using min/maxItems rather than duplicating but somehow catch up with some warnings in dt_bindings check //local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: pcie-ep@1c00000: reg: [[29360128, 12288], [1073741824, 3869], [1073745696, 200], [1073745920, 4096], [1073750016, 4096], [29372416, 12288]] is too short// // from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#// ///local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: pcie-ep@1c00000: reg-names: ['parf', 'dbi', 'elbi', 'atu', 'addr_space', 'mmio'] is too short// // from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#// ///local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: pcie-ep@1c00000: interrupts: [[0, 140, 4], [0, 145, 4]] is too short// // from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#// ///local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: pcie-ep@1c00000: interrupt-names: ['global', 'doorbell'] is too short// // from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#// / //local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: pcie-ep@1c00000: interrupt-names: ['global', 'doorbell'] is too short/ added the patch in attachment. --Mrinmay >>> -Shazad >> Here we have defined reg and interrupt per platform as clocks is defined. >> >> -Mrinmay >> > From 520653ae6996366942f21a8942b5d8ac33e30ee3 Mon Sep 17 00:00:00 2001 From: Mrinmay Sarkar <quic_msarkar@quicinc.com> Date: Fri, 13 Oct 2023 18:09:56 +0530 Subject: [PATCH] dt-bindings: PCI: qcom-ep: Add support for SA8775P SoC Add devicetree bindings support for SA8775P SoC. Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> --- .../devicetree/bindings/pci/qcom,pcie-ep.yaml | 73 ++++++++++++++++++- 1 file changed, 70 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml index a223ce029cab..00eef92685a2 100644 --- a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml @@ -13,9 +13,11 @@ properties: compatible: oneOf: - enum: + - qcom,sa8775p-pcie-ep - qcom,sdx55-pcie-ep - qcom,sm8450-pcie-ep - items: + - const: qcom,sa8775p-pcie-ep - const: qcom,sdx65-pcie-ep - const: qcom,sdx55-pcie-ep @@ -27,6 +29,7 @@ properties: - description: Address Translation Unit (ATU) registers - description: Memory region used to map remote RC address space - description: BAR memory region + - description: DMA register space reg-names: items: @@ -36,13 +39,14 @@ properties: - const: atu - const: addr_space - const: mmio + - const: dma clocks: - minItems: 7 + minItems: 5 maxItems: 8 clock-names: - minItems: 7 + minItems: 5 maxItems: 8 qcom,perst-regs: @@ -60,11 +64,13 @@ properties: items: - description: PCIe Global interrupt - description: PCIe Doorbell interrupt + - description: DMA interrupt interrupt-names: items: - const: global - const: doorbell + - const: dma reset-gpios: description: GPIO used as PERST# input signal @@ -125,7 +131,13 @@ allOf: - qcom,sdx55-pcie-ep then: properties: - clocks: + reg: + maxItems: 6 + minItems: 6 + reg-names: + maxItems: 6 + minItems: 6 + clocks: items: - description: PCIe Auxiliary clock - description: PCIe CFG AHB clock @@ -143,6 +155,12 @@ allOf: - const: slave_q2a - const: sleep - const: ref + interrupts: + maxItems: 2 + minItems: 2 + interrupt-names: + maxItems: 3 + minItems: 3 - if: properties: @@ -152,6 +170,13 @@ allOf: - qcom,sm8450-pcie-ep then: properties: + properties: + reg: + maxItems: 6 + minItems: 6 + reg-names: + maxItems: 6 + minItems: 6 clocks: items: - description: PCIe Auxiliary clock @@ -172,6 +197,48 @@ allOf: - const: ref - const: ddrss_sf_tbu - const: aggre_noc_axi + interrupts: + maxItems: 2 + minItems: 2 + interrupt-names: + maxItems: 3 + minItems: 3 + + - if: + properties: + compatible: + contains: + enum: + - qcom,sa8775p-pcie-ep + then: + properties: + properties: + reg: + maxItems: 7 + minItems: 7 + reg-names: + maxItems: 7 + minItems: 7 + clocks: + items: + - description: PCIe Auxiliary clock + - description: PCIe CFG AHB clock + - description: PCIe Master AXI clock + - description: PCIe Slave AXI clock + - description: PCIe Slave Q2A AXI clock + clock-names: + items: + - const: aux + - const: cfg + - const: bus_master + - const: bus_slave + - const: slave_q2a + interrupts: + maxItems: 3 + minItems: 3 + interrupt-names: + maxItems: 3 + minItems: 3 unevaluatedProperties: false
On Fri, 13 Oct 2023 at 15:55, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote: > > > On 10/11/2023 5:13 PM, Dmitry Baryshkov wrote: > > On Wed, 11 Oct 2023 at 14:14, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote: > >> > >> On 10/6/2023 4:24 PM, Shazad Hussain wrote: > >>> > >>> On 9/22/2023 12:08 AM, Rob Herring wrote: > >>>> On Wed, Sep 20, 2023 at 07:25:08PM +0530, Mrinmay Sarkar wrote: > >>>>> Add devicetree bindings support for SA8775P SoC. > >>>>> Define reg and interrupt per platform. > >>>>> > >>>>> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> > >>>>> --- > >>>>> .../devicetree/bindings/pci/qcom,pcie-ep.yaml | 130 > >>>>> +++++++++++++++++---- > >>>>> 1 file changed, 108 insertions(+), 22 deletions(-) > >>>>> > >>>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > >>>>> b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > >>>>> index a223ce0..e860e8f 100644 > >>>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > >>>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > >>>>> @@ -13,6 +13,7 @@ properties: > >>>>> compatible: > >>>>> oneOf: > >>>>> - enum: > >>>>> + - qcom,sa8775p-pcie-ep > >>>>> - qcom,sdx55-pcie-ep > >>>>> - qcom,sm8450-pcie-ep > >>>>> - items: > >>>>> @@ -20,29 +21,19 @@ properties: > >>>>> - const: qcom,sdx55-pcie-ep > >>>>> reg: > >>>>> - items: > >>>>> - - description: Qualcomm-specific PARF configuration registers > >>>>> - - description: DesignWare PCIe registers > >>>>> - - description: External local bus interface registers > >>>>> - - description: Address Translation Unit (ATU) registers > >>>>> - - description: Memory region used to map remote RC address space > >>>>> - - description: BAR memory region > >>>>> + minItems: 6 > >>>>> + maxItems: 7 > >>>>> reg-names: > >>>>> - items: > >>>>> - - const: parf > >>>>> - - const: dbi > >>>>> - - const: elbi > >>>>> - - const: atu > >>>>> - - const: addr_space > >>>>> - - const: mmio > >>>>> + minItems: 6 > >>>>> + maxItems: 7 > >>>> Don't move these into if/then schemas. Then we are duplicating the > >>>> names, and there is no reason to keep them aligned for new compatibles. > >>>> > >>>> Rob > >>> Hi Rob, > >>> As we have one extra reg property (dma) required for sa8775p-pcie-ep, > >>> isn't it expected to be moved in if/then as per number of regs > >>> required. Anyways we would have duplication of some properties for new > >>> compatibles where the member numbers differs for a property. > >>> > >>> Are you suggesting to add the extra reg property (dma) in the existing > >>> reg and reg-names list, and add minItems/maxItems for all compatibles > >>> present in this file ? > > This is what we have been doing in other cases: if the list is an > > extension of the current list, there is no need to duplicate it. One > > can use min/maxItems instead. > Hi Dmitry > > we have tried using min/maxItems rather than duplicating but somehow > catch up with some warnings in dt_bindings check > > //local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: > pcie-ep@1c00000: reg: [[29360128, 12288], [1073741824, 3869], > [1073745696, 200], [1073745920, 4096], [1073750016, 4096], [29372416, > 12288]] is too short// > // from schema $id: > http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#// > ///local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: > pcie-ep@1c00000: reg-names: ['parf', 'dbi', 'elbi', 'atu', 'addr_space', > 'mmio'] is too short// > // from schema $id: missing min/maxItems for reg and reg-names > http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#// > ///local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: > pcie-ep@1c00000: interrupts: [[0, 140, 4], [0, 145, 4]] is too short// > // from schema $id: > http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#// > ///local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: > pcie-ep@1c00000: interrupt-names: ['global', 'doorbell'] is too short// > // from schema $id: > http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#// incorrect min/maxItems for interrupts. > //local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: > pcie-ep@1c00000: interrupt-names: ['global', 'doorbell'] is too short/ > > added the patch in attachment. > > --Mrinmay > > >>> -Shazad > >> Here we have defined reg and interrupt per platform as clocks is defined. > >> > >> -Mrinmay > >> > >
On 10/13/2023 10:08 PM, Dmitry Baryshkov wrote: > On Fri, 13 Oct 2023 at 15:55, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote: >> >> On 10/11/2023 5:13 PM, Dmitry Baryshkov wrote: >>> On Wed, 11 Oct 2023 at 14:14, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote: >>>> On 10/6/2023 4:24 PM, Shazad Hussain wrote: >>>>> On 9/22/2023 12:08 AM, Rob Herring wrote: >>>>>> On Wed, Sep 20, 2023 at 07:25:08PM +0530, Mrinmay Sarkar wrote: >>>>>>> Add devicetree bindings support for SA8775P SoC. >>>>>>> Define reg and interrupt per platform. >>>>>>> >>>>>>> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> >>>>>>> --- >>>>>>> .../devicetree/bindings/pci/qcom,pcie-ep.yaml | 130 >>>>>>> +++++++++++++++++---- >>>>>>> 1 file changed, 108 insertions(+), 22 deletions(-) >>>>>>> >>>>>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml >>>>>>> b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml >>>>>>> index a223ce0..e860e8f 100644 >>>>>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml >>>>>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml >>>>>>> @@ -13,6 +13,7 @@ properties: >>>>>>> compatible: >>>>>>> oneOf: >>>>>>> - enum: >>>>>>> + - qcom,sa8775p-pcie-ep >>>>>>> - qcom,sdx55-pcie-ep >>>>>>> - qcom,sm8450-pcie-ep >>>>>>> - items: >>>>>>> @@ -20,29 +21,19 @@ properties: >>>>>>> - const: qcom,sdx55-pcie-ep >>>>>>> reg: >>>>>>> - items: >>>>>>> - - description: Qualcomm-specific PARF configuration registers >>>>>>> - - description: DesignWare PCIe registers >>>>>>> - - description: External local bus interface registers >>>>>>> - - description: Address Translation Unit (ATU) registers >>>>>>> - - description: Memory region used to map remote RC address space >>>>>>> - - description: BAR memory region >>>>>>> + minItems: 6 >>>>>>> + maxItems: 7 >>>>>>> reg-names: >>>>>>> - items: >>>>>>> - - const: parf >>>>>>> - - const: dbi >>>>>>> - - const: elbi >>>>>>> - - const: atu >>>>>>> - - const: addr_space >>>>>>> - - const: mmio >>>>>>> + minItems: 6 >>>>>>> + maxItems: 7 >>>>>> Don't move these into if/then schemas. Then we are duplicating the >>>>>> names, and there is no reason to keep them aligned for new compatibles. >>>>>> >>>>>> Rob >>>>> Hi Rob, >>>>> As we have one extra reg property (dma) required for sa8775p-pcie-ep, >>>>> isn't it expected to be moved in if/then as per number of regs >>>>> required. Anyways we would have duplication of some properties for new >>>>> compatibles where the member numbers differs for a property. >>>>> >>>>> Are you suggesting to add the extra reg property (dma) in the existing >>>>> reg and reg-names list, and add minItems/maxItems for all compatibles >>>>> present in this file ? >>> This is what we have been doing in other cases: if the list is an >>> extension of the current list, there is no need to duplicate it. One >>> can use min/maxItems instead. >> Hi Dmitry >> >> we have tried using min/maxItems rather than duplicating but somehow >> catch up with some warnings in dt_bindings check >> >> //local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: >> pcie-ep@1c00000: reg: [[29360128, 12288], [1073741824, 3869], >> [1073745696, 200], [1073745920, 4096], [1073750016, 4096], [29372416, >> 12288]] is too short// >> // from schema $id: >> http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#// >> ///local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: >> pcie-ep@1c00000: reg-names: ['parf', 'dbi', 'elbi', 'atu', 'addr_space', >> 'mmio'] is too short// >> // from schema $id: > missing min/maxItems for reg and reg-names > >> http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#// >> ///local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: >> pcie-ep@1c00000: interrupts: [[0, 140, 4], [0, 145, 4]] is too short// >> // from schema $id: >> http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#// >> ///local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: >> pcie-ep@1c00000: interrupt-names: ['global', 'doorbell'] is too short// >> // from schema $id: >> http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#// > incorrect min/maxItems for interrupts. I am getting the same warnings even after correcting the min/maxItems for interrupt. > -Mrinmay >> //local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: >> pcie-ep@1c00000: interrupt-names: ['global', 'doorbell'] is too short/ >> >> added the patch in attachment. >> >> --Mrinmay >> >>>>> -Shazad >>>> Here we have defined reg and interrupt per platform as clocks is defined. >>>> >>>> -Mrinmay >>>> > >
On Fri, 13 Oct 2023 at 15:55, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote: > > > On 10/11/2023 5:13 PM, Dmitry Baryshkov wrote: > > On Wed, 11 Oct 2023 at 14:14, Mrinmay Sarkar <quic_msarkar@quicinc.com> wrote: > >> > >> On 10/6/2023 4:24 PM, Shazad Hussain wrote: > >>> > >>> On 9/22/2023 12:08 AM, Rob Herring wrote: > >>>> On Wed, Sep 20, 2023 at 07:25:08PM +0530, Mrinmay Sarkar wrote: > >>>>> Add devicetree bindings support for SA8775P SoC. > >>>>> Define reg and interrupt per platform. > >>>>> > >>>>> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> > >>>>> --- > >>>>> .../devicetree/bindings/pci/qcom,pcie-ep.yaml | 130 > >>>>> +++++++++++++++++---- > >>>>> 1 file changed, 108 insertions(+), 22 deletions(-) > >>>>> > >>>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > >>>>> b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > >>>>> index a223ce0..e860e8f 100644 > >>>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > >>>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml > >>>>> @@ -13,6 +13,7 @@ properties: > >>>>> compatible: > >>>>> oneOf: > >>>>> - enum: > >>>>> + - qcom,sa8775p-pcie-ep > >>>>> - qcom,sdx55-pcie-ep > >>>>> - qcom,sm8450-pcie-ep > >>>>> - items: > >>>>> @@ -20,29 +21,19 @@ properties: > >>>>> - const: qcom,sdx55-pcie-ep > >>>>> reg: > >>>>> - items: > >>>>> - - description: Qualcomm-specific PARF configuration registers > >>>>> - - description: DesignWare PCIe registers > >>>>> - - description: External local bus interface registers > >>>>> - - description: Address Translation Unit (ATU) registers > >>>>> - - description: Memory region used to map remote RC address space > >>>>> - - description: BAR memory region > >>>>> + minItems: 6 > >>>>> + maxItems: 7 > >>>>> reg-names: > >>>>> - items: > >>>>> - - const: parf > >>>>> - - const: dbi > >>>>> - - const: elbi > >>>>> - - const: atu > >>>>> - - const: addr_space > >>>>> - - const: mmio > >>>>> + minItems: 6 > >>>>> + maxItems: 7 > >>>> Don't move these into if/then schemas. Then we are duplicating the > >>>> names, and there is no reason to keep them aligned for new compatibles. > >>>> > >>>> Rob > >>> Hi Rob, > >>> As we have one extra reg property (dma) required for sa8775p-pcie-ep, > >>> isn't it expected to be moved in if/then as per number of regs > >>> required. Anyways we would have duplication of some properties for new > >>> compatibles where the member numbers differs for a property. > >>> > >>> Are you suggesting to add the extra reg property (dma) in the existing > >>> reg and reg-names list, and add minItems/maxItems for all compatibles > >>> present in this file ? > > This is what we have been doing in other cases: if the list is an > > extension of the current list, there is no need to duplicate it. One > > can use min/maxItems instead. > Hi Dmitry > > we have tried using min/maxItems rather than duplicating but somehow > catch up with some warnings in dt_bindings check > > //local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: > pcie-ep@1c00000: reg: [[29360128, 12288], [1073741824, 3869], > [1073745696, 200], [1073745920, 4096], [1073750016, 4096], [29372416, > 12288]] is too short// > // from schema $id: > http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#// > ///local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: > pcie-ep@1c00000: reg-names: ['parf', 'dbi', 'elbi', 'atu', 'addr_space', > 'mmio'] is too short// > // from schema $id: > http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#// > ///local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: > pcie-ep@1c00000: interrupts: [[0, 140, 4], [0, 145, 4]] is too short// > // from schema $id: > http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#// > ///local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: > pcie-ep@1c00000: interrupt-names: ['global', 'doorbell'] is too short// > // from schema $id: > http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#// > / > > //local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: > pcie-ep@1c00000: interrupt-names: ['global', 'doorbell'] is too short/ > > added the patch in attachment. Please, don't send patches as attachments. It is impossible to comment on it. So, few points I had to fix to make your patch to work: - Please, understand the difference between enum and items. You'd need to add your compat string to only one of them. Or to a new entry. But adding it to both entries is a definite mistake. - You have extended items for existing platforms (reg, reg-names, interrupts, interrupt-names). However you failed to add corresponding minItems, allowing existing platforms to use the list with less items in it. - You do not need to have maxItems:N, minItems:N with the same value. Please drop these minItems, it is the default. - You haven't reviewed the patch on your own. You have erroneously nested 'properties' clauses in two places. $ git diff --stat Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-) Hope this helps.
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml index a223ce0..e860e8f 100644 --- a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml @@ -13,6 +13,7 @@ properties: compatible: oneOf: - enum: + - qcom,sa8775p-pcie-ep - qcom,sdx55-pcie-ep - qcom,sm8450-pcie-ep - items: @@ -20,29 +21,19 @@ properties: - const: qcom,sdx55-pcie-ep reg: - items: - - description: Qualcomm-specific PARF configuration registers - - description: DesignWare PCIe registers - - description: External local bus interface registers - - description: Address Translation Unit (ATU) registers - - description: Memory region used to map remote RC address space - - description: BAR memory region + minItems: 6 + maxItems: 7 reg-names: - items: - - const: parf - - const: dbi - - const: elbi - - const: atu - - const: addr_space - - const: mmio + minItems: 6 + maxItems: 7 clocks: - minItems: 7 + minItems: 5 maxItems: 8 clock-names: - minItems: 7 + minItems: 5 maxItems: 8 qcom,perst-regs: @@ -57,14 +48,12 @@ properties: - description: Perst separation enable offset interrupts: - items: - - description: PCIe Global interrupt - - description: PCIe Doorbell interrupt + minItems: 2 + maxItems: 3 interrupt-names: - items: - - const: global - - const: doorbell + minItems: 2 + maxItems: 3 reset-gpios: description: GPIO used as PERST# input signal @@ -122,6 +111,51 @@ allOf: compatible: contains: enum: + - qcom,sa8775p-pcie-ep + then: + properties: + reg: + items: + - description: Qualcomm-specific PARF configuration registers + - description: DesignWare PCIe registers + - description: External local bus interface registers + - description: Address Translation Unit (ATU) registers + - description: Memory region used to map remote RC address space + - description: BAR memory region + - description: DMA memory region + reg-names: + items: + - const: parf + - const: dbi + - const: elbi + - const: atu + - const: addr_space + - const: mmio + - const: dma + else: + properties: + reg: + items: + - description: Qualcomm-specific PARF configuration registers + - description: DesignWare PCIe registers + - description: External local bus interface registers + - description: Address Translation Unit (ATU) registers + - description: Memory region used to map remote RC address space + - description: BAR memory region + reg-names: + items: + - const: parf + - const: dbi + - const: elbi + - const: atu + - const: addr_space + - const: mmio + + - if: + properties: + compatible: + contains: + enum: - qcom,sdx55-pcie-ep then: properties: @@ -173,6 +207,58 @@ allOf: - const: ddrss_sf_tbu - const: aggre_noc_axi + - if: + properties: + compatible: + contains: + enum: + - qcom,sa8775-pcie-ep + then: + properties: + clocks: + items: + - description: PCIe Auxiliary clock + - description: PCIe CFG AHB clock + - description: PCIe Master AXI clock + - description: PCIe Slave AXI clock + - description: PCIe Slave Q2A AXI clock + clock-names: + items: + - const: aux + - const: cfg + - const: bus_master + - const: bus_slave + - const: slave_q2a + + - if: + properties: + compatible: + contains: + enum: + - qcom,sa8775p-pcie-ep + then: + properties: + interrupts: + items: + - description: PCIe Global interrupt + - description: PCIe Doorbell interrupt + - description: DMA interrupt + interrupt-names: + items: + - const: global + - const: doorbell + - const: dma + else: + properties: + interrupts: + items: + - description: PCIe Global interrupt + - description: PCIe Doorbell interrupt + interrupt-names: + items: + - const: global + - const: doorbell + unevaluatedProperties: false examples:
Add devicetree bindings support for SA8775P SoC. Define reg and interrupt per platform. Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com> --- .../devicetree/bindings/pci/qcom,pcie-ep.yaml | 130 +++++++++++++++++---- 1 file changed, 108 insertions(+), 22 deletions(-)