[PATCH 00/10] PCI: qcom: Add support for SC8280XP and SA8540P

[PATCH 00/10] PCI: qcom: Add support for SC8280XP and SA8540P

[Johan Hovold]

This series adds support for the PCIe controllers found on SC8280XP and
SA8540P.

Included are also three patches that clean up the way the driver handles
different IP revisions (e.g. by modelling optional clocks as being truly
optional).

These patches depend on the recently merged (but currently held off?)
PIPE clock series:

	https://lore.kernel.org/all/20220608105238.2973600-1-dmitry.baryshkov@linaro.org/

as well as the about-to-be-merged MSI series:

	https://lore.kernel.org/all/20220620112015.1600380-1-dmitry.baryshkov@linaro.org/

While preparing the dt-binding update I found a bug in the dt-schema
introduced by the latter series and which is fixed by the second patch
below. Not sure it's worth respinning a v16 of the MSI series just for
that but if so then the fix could be folded into:

	https://lore.kernel.org/all/20220620112015.1600380-6-dmitry.baryshkov@linaro.org/

Johan


Johan Hovold (10):
  dt-bindings: PCI: qcom: Fix reset conditional
  dt-bindings: PCI: qcom: Fix msi-interrupt conditional
  dt-bindings: PCI: qcom: Enumerate platforms with single msi interrupt
  dt-bindings: PCI: qcom: Add SC8280XP to binding
  dt-bindings: PCI: qcom: Add SA8540P to binding
  PCI: qcom: Add support for SC8280XP
  PCI: qcom: Add support for SA8540P
  PCI: qcom: Make all optional clocks optional
  PCI: qcom: Clean up IP configurations
  PCI: qcom: Sort device-id table

 .../devicetree/bindings/pci/qcom,pcie.yaml    |  74 ++++++++++-
 drivers/pci/controller/dwc/pcie-qcom.c        | 117 +++++++-----------
 2 files changed, 114 insertions(+), 77 deletions(-)

-- 
2.35.1

[PATCH 01/10] dt-bindings: PCI: qcom: Fix reset conditional

[Johan Hovold]

Fix the reset conditional which always evaluated to true due to a
misspelled property name ("compatibles" in plural).

Fixes: 6700a9b00f0a ("dt-bindings: PCI: qcom: Do not require resets on msm8996 platforms")
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 7e84063afe25..ed9f9462a758 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -615,7 +615,7 @@ allOf:
   - if:
       not:
         properties:
-          compatibles:
+          compatible:
             contains:
               enum:
                 - qcom,pcie-msm8996
-- 
2.35.1

Re: [PATCH 01/10] dt-bindings: PCI: qcom: Fix reset conditional

[Dmitry Baryshkov]

On 29 June 2022 17:09:51 GMT+03:00, Johan Hovold <johan+linaro@kernel.org> wrote:
>Fix the reset conditional which always evaluated to true due to a
>misspelled property name ("compatibles" in plural).
>
>Fixes: 6700a9b00f0a ("dt-bindings: PCI: qcom: Do not require resets on msm8996 platforms")
>Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>---
> Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>index 7e84063afe25..ed9f9462a758 100644
>--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>@@ -615,7 +615,7 @@ allOf:
>   - if:
>       not:
>         properties:
>-          compatibles:
>+          compatible:


Argh. Thanks for noticing and fixing the typo.
If necessary I can respin MSI series in a few days.

Anyway, for this patch:
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

>             contains:
>               enum:
>                 - qcom,pcie-msm8996

-- 
With best wishes
Dmitry

Re: [PATCH 01/10] dt-bindings: PCI: qcom: Fix reset conditional

[Krzysztof Kozlowski]

On 29/06/2022 16:09, Johan Hovold wrote:
> Fix the reset conditional which always evaluated to true due to a
> misspelled property name ("compatibles" in plural).
> 
> Fixes: 6700a9b00f0a ("dt-bindings: PCI: qcom: Do not require resets on msm8996 platforms")
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH 01/10] dt-bindings: PCI: qcom: Fix reset conditional

[Manivannan Sadhasivam]

On Wed, Jun 29, 2022 at 04:09:51PM +0200, Johan Hovold wrote:
> Fix the reset conditional which always evaluated to true due to a
> misspelled property name ("compatibles" in plural).
> 
> Fixes: 6700a9b00f0a ("dt-bindings: PCI: qcom: Do not require resets on msm8996 platforms")
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index 7e84063afe25..ed9f9462a758 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -615,7 +615,7 @@ allOf:
>    - if:
>        not:
>          properties:
> -          compatibles:
> +          compatible:
>              contains:
>                enum:
>                  - qcom,pcie-msm8996
> -- 
> 2.35.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 01/10] dt-bindings: PCI: qcom: Fix reset conditional

[Bjorn Helgaas]

On Wed, Jun 29, 2022 at 04:09:51PM +0200, Johan Hovold wrote:
> Fix the reset conditional which always evaluated to true due to a
> misspelled property name ("compatibles" in plural).
> 
> Fixes: 6700a9b00f0a ("dt-bindings: PCI: qcom: Do not require resets on msm8996 platforms")
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

I applied this patch to pci/ctrl/qcom-pending for v5.20, thanks!

I'd like to see an ack from Stanimir (per get_maintainers.pl) for the
rest.

> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index 7e84063afe25..ed9f9462a758 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -615,7 +615,7 @@ allOf:
>    - if:
>        not:
>          properties:
> -          compatibles:
> +          compatible:
>              contains:
>                enum:
>                  - qcom,pcie-msm8996
> -- 
> 2.35.1
> 

[PATCH 02/10] dt-bindings: PCI: qcom: Fix msi-interrupt conditional

[Johan Hovold]

Fix the msi-interrupt conditional which always evaluated to false due to
a misspelled property name ("compatibles" in plural).

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index ed9f9462a758..a1b4fc70e162 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -628,7 +628,7 @@ allOf:
     # On older chipsets it's always 1 msi interrupt
   - if:
       properties:
-        compatibles:
+        compatible:
           contains:
             enum:
               - qcom,pcie-msm8996
-- 
2.35.1

Re: [PATCH 02/10] dt-bindings: PCI: qcom: Fix msi-interrupt conditional

[Dmitry Baryshkov]

On 29 June 2022 17:09:52 GMT+03:00, Johan Hovold <johan+linaro@kernel.org> wrote:
>Fix the msi-interrupt conditional which always evaluated to false due to
>a misspelled property name ("compatibles" in plural).
>
>Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>---
> Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


>
>diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>index ed9f9462a758..a1b4fc70e162 100644
>--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>@@ -628,7 +628,7 @@ allOf:
>     # On older chipsets it's always 1 msi interrupt
>   - if:
>       properties:
>-        compatibles:
>+        compatible:
>           contains:
>             enum:
>               - qcom,pcie-msm8996

-- 
With best wishes
Dmitry

Re: [PATCH 02/10] dt-bindings: PCI: qcom: Fix msi-interrupt conditional

[Krzysztof Kozlowski]

On 29/06/2022 16:09, Johan Hovold wrote:
> Fix the msi-interrupt conditional which always evaluated to false due to
> a misspelled property name ("compatibles" in plural).
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH 02/10] dt-bindings: PCI: qcom: Fix msi-interrupt conditional

[Dmitry Baryshkov]

On 29/06/2022 17:09, Johan Hovold wrote:
> Fix the msi-interrupt conditional which always evaluated to false due to
> a misspelled property name ("compatibles" in plural).
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Fixes: 6700a9b00f0a ("dt-bindings: PCI: qcom: Do not require resets on 
msm8996 platforms")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


> ---
>   Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index ed9f9462a758..a1b4fc70e162 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -628,7 +628,7 @@ allOf:
>       # On older chipsets it's always 1 msi interrupt
>     - if:
>         properties:
> -        compatibles:
> +        compatible:
>             contains:
>               enum:
>                 - qcom,pcie-msm8996


-- 
With best wishes
Dmitry

Re: [PATCH 02/10] dt-bindings: PCI: qcom: Fix msi-interrupt conditional

[Dmitry Baryshkov]

On 07/07/2022 16:34, Dmitry Baryshkov wrote:
> On 29/06/2022 17:09, Johan Hovold wrote:
>> Fix the msi-interrupt conditional which always evaluated to false due to
>> a misspelled property name ("compatibles" in plural).
>>
>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> 
> Fixes: 6700a9b00f0a ("dt-bindings: PCI: qcom: Do not require resets on 
> msm8996 platforms")

Ugh, I was thinking about a different chunk (which also has this issue), 
so please ignore the Fixes.

> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 
> 
>> ---
>>   Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml 
>> b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> index ed9f9462a758..a1b4fc70e162 100644
>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> @@ -628,7 +628,7 @@ allOf:
>>       # On older chipsets it's always 1 msi interrupt
>>     - if:
>>         properties:
>> -        compatibles:
>> +        compatible:
>>             contains:
>>               enum:
>>                 - qcom,pcie-msm8996
> 
> 


-- 
With best wishes
Dmitry

Re: [PATCH 02/10] dt-bindings: PCI: qcom: Fix msi-interrupt conditional

[Johan Hovold]

On Thu, Jul 07, 2022 at 04:41:02PM +0300, Dmitry Baryshkov wrote:
> On 07/07/2022 16:34, Dmitry Baryshkov wrote:
> > On 29/06/2022 17:09, Johan Hovold wrote:
> >> Fix the msi-interrupt conditional which always evaluated to false due to
> >> a misspelled property name ("compatibles" in plural).
> >>
> >> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > 
> > Fixes: 6700a9b00f0a ("dt-bindings: PCI: qcom: Do not require resets on 
> > msm8996 platforms")
> 
> Ugh, I was thinking about a different chunk (which also has this issue), 
> so please ignore the Fixes.

Heh, yeah, I was pretty I had already added it for the issue in mainline.

Johan

Re: [PATCH 02/10] dt-bindings: PCI: qcom: Fix msi-interrupt conditional

[Manivannan Sadhasivam]

On Wed, Jun 29, 2022 at 04:09:52PM +0200, Johan Hovold wrote:
> Fix the msi-interrupt conditional which always evaluated to false due to
> a misspelled property name ("compatibles" in plural).
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index ed9f9462a758..a1b4fc70e162 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -628,7 +628,7 @@ allOf:
>      # On older chipsets it's always 1 msi interrupt
>    - if:
>        properties:
> -        compatibles:
> +        compatible:
>            contains:
>              enum:
>                - qcom,pcie-msm8996
> -- 
> 2.35.1
> 

-- 
மணிவண்ணன் சதாசிவம்

[PATCH 03/10] dt-bindings: PCI: qcom: Enumerate platforms with single msi interrupt

[Johan Hovold]

Explicitly enumerate the older platforms that have a single msi host
interrupt. This allows for adding further platforms without resorting
to nested conditionals.

Drop the redundant comment about older chipsets instead of moving it.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 .../devicetree/bindings/pci/qcom,pcie.yaml      | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index a1b4fc70e162..8560c65e6f0b 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -625,7 +625,6 @@ allOf:
         - reset-names
 
     # On newer chipsets support either 1 or 8 msi interrupts
-    # On older chipsets it's always 1 msi interrupt
   - if:
       properties:
         compatible:
@@ -660,7 +659,21 @@ allOf:
                 - const: msi5
                 - const: msi6
                 - const: msi7
-    else:
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-apq8064
+              - qcom,pcie-apq8084
+              - qcom,pcie-ipq4019
+              - qcom,pcie-ipq6018
+              - qcom,pcie-ipq8064
+              - qcom,pcie-ipq8064-v2
+              - qcom,pcie-ipq8074
+              - qcom,pcie-qcs404
+    then:
       properties:
         interrupts:
           maxItems: 1
-- 
2.35.1

Re: [PATCH 03/10] dt-bindings: PCI: qcom: Enumerate platforms with single msi interrupt

[Krzysztof Kozlowski]

On 29/06/2022 16:09, Johan Hovold wrote:
> Explicitly enumerate the older platforms that have a single msi host
> interrupt. This allows for adding further platforms without resorting
> to nested conditionals.
> 
> Drop the redundant comment about older chipsets instead of moving it.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

This does not exist in linux-next, so it should be squashed it with the
previous series.

> ---
>  .../devicetree/bindings/pci/qcom,pcie.yaml      | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index a1b4fc70e162..8560c65e6f0b 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -625,7 +625,6 @@ allOf:
>          - reset-names
>  
>      # On newer chipsets support either 1 or 8 msi interrupts
> -    # On older chipsets it's always 1 msi interrupt
>    - if:
>        properties:
>          compatible:
> @@ -660,7 +659,21 @@ allOf:
>                  - const: msi5
>                  - const: msi6
>                  - const: msi7
> -    else:
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,pcie-apq8064
> +              - qcom,pcie-apq8084
> +              - qcom,pcie-ipq4019
> +              - qcom,pcie-ipq6018
> +              - qcom,pcie-ipq8064
> +              - qcom,pcie-ipq8064-v2
> +              - qcom,pcie-ipq8074
> +              - qcom,pcie-qcs404

Otherwise I cannot even check the context...

> +    then:
>        properties:
>          interrupts:
>            maxItems: 1


Best regards,
Krzysztof

Re: [PATCH 03/10] dt-bindings: PCI: qcom: Enumerate platforms with single msi interrupt

[Johan Hovold]

On Fri, Jul 01, 2022 at 10:33:35AM +0200, Krzysztof Kozlowski wrote:
> On 29/06/2022 16:09, Johan Hovold wrote:
> > Explicitly enumerate the older platforms that have a single msi host
> > interrupt. This allows for adding further platforms without resorting
> > to nested conditionals.
> > 
> > Drop the redundant comment about older chipsets instead of moving it.
> > 
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> 
> This does not exist in linux-next, so it should be squashed it with the
> previous series.

As mentioned in the cover letter this depends on the MSI series that has
unfortunately not yet been merged.

That series is self-contained and ready to be merged, so this follow-up
does not need to be squashed in.

> > ---
> >  .../devicetree/bindings/pci/qcom,pcie.yaml      | 17 +++++++++++++++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > index a1b4fc70e162..8560c65e6f0b 100644
> > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > @@ -625,7 +625,6 @@ allOf:
> >          - reset-names
> >  
> >      # On newer chipsets support either 1 or 8 msi interrupts
> > -    # On older chipsets it's always 1 msi interrupt
> >    - if:
> >        properties:
> >          compatible:
> > @@ -660,7 +659,21 @@ allOf:
> >                  - const: msi5
> >                  - const: msi6
> >                  - const: msi7
> > -    else:
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - qcom,pcie-apq8064
> > +              - qcom,pcie-apq8084
> > +              - qcom,pcie-ipq4019
> > +              - qcom,pcie-ipq6018
> > +              - qcom,pcie-ipq8064
> > +              - qcom,pcie-ipq8064-v2
> > +              - qcom,pcie-ipq8074
> > +              - qcom,pcie-qcs404
> 
> Otherwise I cannot even check the context...

Yeah, I realise that makes reviewing a bit harder, but hopefully the
maintainer will pick up the MSI series soon.

> 
> > +    then:
> >        properties:
> >          interrupts:
> >            maxItems: 1

Johan

Re: [PATCH 03/10] dt-bindings: PCI: qcom: Enumerate platforms with single msi interrupt

[Rob Herring]

On Fri, Jul 01, 2022 at 10:38:42AM +0200, Johan Hovold wrote:
> On Fri, Jul 01, 2022 at 10:33:35AM +0200, Krzysztof Kozlowski wrote:
> > On 29/06/2022 16:09, Johan Hovold wrote:
> > > Explicitly enumerate the older platforms that have a single msi host
> > > interrupt. This allows for adding further platforms without resorting
> > > to nested conditionals.
> > > 
> > > Drop the redundant comment about older chipsets instead of moving it.
> > > 
> > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > 
> > This does not exist in linux-next, so it should be squashed it with the
> > previous series.
> 
> As mentioned in the cover letter this depends on the MSI series that has
> unfortunately not yet been merged.
> 
> That series is self-contained and ready to be merged, so this follow-up
> does not need to be squashed in.

I suspect that Bjorn would rather squash these in.

Rob

Re: [PATCH 03/10] dt-bindings: PCI: qcom: Enumerate platforms with single msi interrupt

[Johan Hovold]

On Fri, Jul 01, 2022 at 12:38:19PM -0600, Rob Herring wrote:
> On Fri, Jul 01, 2022 at 10:38:42AM +0200, Johan Hovold wrote:
> > On Fri, Jul 01, 2022 at 10:33:35AM +0200, Krzysztof Kozlowski wrote:
> > > On 29/06/2022 16:09, Johan Hovold wrote:
> > > > Explicitly enumerate the older platforms that have a single msi host
> > > > interrupt. This allows for adding further platforms without resorting
> > > > to nested conditionals.
> > > > 
> > > > Drop the redundant comment about older chipsets instead of moving it.
> > > > 
> > > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > > 
> > > This does not exist in linux-next, so it should be squashed it with the
> > > previous series.
> > 
> > As mentioned in the cover letter this depends on the MSI series that has
> > unfortunately not yet been merged.
> > 
> > That series is self-contained and ready to be merged, so this follow-up
> > does not need to be squashed in.
> 
> I suspect that Bjorn would rather squash these in.

Sure. Squashing in the compatible-conditional fix makes sense, but the
motivation for this one is the SoC added by this series so I'd argue
that it belongs here. But either way is fine with me.

Johan

Re: [PATCH 03/10] dt-bindings: PCI: qcom: Enumerate platforms with single msi interrupt

[Krzysztof Kozlowski]

On 29/06/2022 16:09, Johan Hovold wrote:
> Explicitly enumerate the older platforms that have a single msi host
> interrupt. This allows for adding further platforms without resorting
> to nested conditionals.

How does it allow it? New platform if not explicitly added to first
"if:" will fall into the "else:", so will be handled and there is no
need for nested if.

Best regards,
Krzysztof

Re: [PATCH 03/10] dt-bindings: PCI: qcom: Enumerate platforms with single msi interrupt

[Manivannan Sadhasivam]

On Wed, Jun 29, 2022 at 04:09:53PM +0200, Johan Hovold wrote:
> Explicitly enumerate the older platforms that have a single msi host
> interrupt. This allows for adding further platforms without resorting
> to nested conditionals.
> 
> Drop the redundant comment about older chipsets instead of moving it.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

With the comment from Krzysztof on wording mentioned in patch 4/10
addressed,

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani


> ---
>  .../devicetree/bindings/pci/qcom,pcie.yaml      | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index a1b4fc70e162..8560c65e6f0b 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -625,7 +625,6 @@ allOf:
>          - reset-names
>  
>      # On newer chipsets support either 1 or 8 msi interrupts
> -    # On older chipsets it's always 1 msi interrupt
>    - if:
>        properties:
>          compatible:
> @@ -660,7 +659,21 @@ allOf:
>                  - const: msi5
>                  - const: msi6
>                  - const: msi7
> -    else:
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,pcie-apq8064
> +              - qcom,pcie-apq8084
> +              - qcom,pcie-ipq4019
> +              - qcom,pcie-ipq6018
> +              - qcom,pcie-ipq8064
> +              - qcom,pcie-ipq8064-v2
> +              - qcom,pcie-ipq8074
> +              - qcom,pcie-qcs404
> +    then:
>        properties:
>          interrupts:
>            maxItems: 1
> -- 
> 2.35.1
> 

-- 
மணிவண்ணன் சதாசிவம்

[PATCH 04/10] dt-bindings: PCI: qcom: Add SC8280XP to binding

[Johan Hovold]

Add the SC8280XP platform to the binding.

SC8280XP use four host interrupts for MSI routing so remove the obsolete
comment referring to newer chipsets supporting one or eight interrupts
(e.g. for backwards compatibility).

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 .../devicetree/bindings/pci/qcom,pcie.yaml    | 50 ++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 8560c65e6f0b..a039f6110322 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -27,6 +27,7 @@ properties:
       - qcom,pcie-qcs404
       - qcom,pcie-sc7280
       - qcom,pcie-sc8180x
+      - qcom,pcie-sc8280xp
       - qcom,pcie-sdm845
       - qcom,pcie-sm8150
       - qcom,pcie-sm8250
@@ -181,6 +182,7 @@ allOf:
             enum:
               - qcom,pcie-sc7280
               - qcom,pcie-sc8180x
+              - qcom,pcie-sc8280xp
               - qcom,pcie-sm8250
               - qcom,pcie-sm8450-pcie0
               - qcom,pcie-sm8450-pcie1
@@ -596,6 +598,35 @@ allOf:
           items:
             - const: pci # PCIe core reset
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-sc8280xp
+    then:
+      properties:
+        clocks:
+          minItems: 8
+          maxItems: 9
+        clock-names:
+          minItems: 8
+          items:
+            - const: aux # Auxiliary clock
+            - const: cfg # Configuration clock
+            - const: bus_master # Master AXI clock
+            - const: bus_slave # Slave AXI clock
+            - const: slave_q2a # Slave Q2A clock
+            - const: ddrss_sf_tbu # PCIe SF TBU clock
+            - const: noc_aggr_4 # NoC aggregate 4 clock
+            - const: noc_aggr_south_sf # NoC aggregate South SF clock
+            - const: cnoc_qx # Configuration NoC QX clock
+        resets:
+          maxItems: 1
+        reset-names:
+          items:
+            - const: pci # PCIe core reset
+
   - if:
       not:
         properties:
@@ -624,7 +655,6 @@ allOf:
         - resets
         - reset-names
 
-    # On newer chipsets support either 1 or 8 msi interrupts
   - if:
       properties:
         compatible:
@@ -660,6 +690,24 @@ allOf:
                 - const: msi6
                 - const: msi7
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-sc8280xp
+    then:
+      properties:
+        interrupts:
+          minItems: 4
+          maxItems: 4
+        interrupt-names:
+          items:
+            - const: msi0
+            - const: msi1
+            - const: msi2
+            - const: msi3
+
   - if:
       properties:
         compatible:
-- 
2.35.1

Re: [PATCH 04/10] dt-bindings: PCI: qcom: Add SC8280XP to binding

[Krzysztof Kozlowski]

On 29/06/2022 16:09, Johan Hovold wrote:
> Add the SC8280XP platform to the binding.
> 
> SC8280XP use four host interrupts for MSI routing so remove the obsolete
> comment referring to newer chipsets supporting one or eight interrupts
> (e.g. for backwards compatibility).
> 

(...)

>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,pcie-sc8280xp
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 4
> +          maxItems: 4
> +        interrupt-names:
> +          items:
> +            - const: msi0
> +            - const: msi1
> +            - const: msi2
> +            - const: msi3

What the previous #3 commit is missing is:
"This allows for adding further platforms with for example four MSI
interrupts, without resorting to nested conditionals."

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 04/10] dt-bindings: PCI: qcom: Add SC8280XP to binding

[Johan Hovold]

On Fri, Jul 01, 2022 at 10:37:55AM +0200, Krzysztof Kozlowski wrote:
> On 29/06/2022 16:09, Johan Hovold wrote:
> > Add the SC8280XP platform to the binding.
> > 
> > SC8280XP use four host interrupts for MSI routing so remove the obsolete
> > comment referring to newer chipsets supporting one or eight interrupts
> > (e.g. for backwards compatibility).
> > 
> 
> (...)
> 
> >  
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - qcom,pcie-sc8280xp
> > +    then:
> > +      properties:
> > +        interrupts:
> > +          minItems: 4
> > +          maxItems: 4
> > +        interrupt-names:
> > +          items:
> > +            - const: msi0
> > +            - const: msi1
> > +            - const: msi2
> > +            - const: msi3
> 
> What the previous #3 commit is missing is:
> "This allows for adding further platforms with for example four MSI
> interrupts, without resorting to nested conditionals."

Yeah, that would make the comment more self-contained. I'll amend it if
I need to respin.

Johan

Re: [PATCH 04/10] dt-bindings: PCI: qcom: Add SC8280XP to binding

[Manivannan Sadhasivam]

On Wed, Jun 29, 2022 at 04:09:54PM +0200, Johan Hovold wrote:
> Add the SC8280XP platform to the binding.
> 
> SC8280XP use four host interrupts for MSI routing so remove the obsolete
> comment referring to newer chipsets supporting one or eight interrupts
> (e.g. for backwards compatibility).
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Wondering why 4 on this SoC. Is this what added in downstream or you also
verified it with IP documentation.

Thanks,
Mani

> ---
>  .../devicetree/bindings/pci/qcom,pcie.yaml    | 50 ++++++++++++++++++-
>  1 file changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index 8560c65e6f0b..a039f6110322 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -27,6 +27,7 @@ properties:
>        - qcom,pcie-qcs404
>        - qcom,pcie-sc7280
>        - qcom,pcie-sc8180x
> +      - qcom,pcie-sc8280xp
>        - qcom,pcie-sdm845
>        - qcom,pcie-sm8150
>        - qcom,pcie-sm8250
> @@ -181,6 +182,7 @@ allOf:
>              enum:
>                - qcom,pcie-sc7280
>                - qcom,pcie-sc8180x
> +              - qcom,pcie-sc8280xp
>                - qcom,pcie-sm8250
>                - qcom,pcie-sm8450-pcie0
>                - qcom,pcie-sm8450-pcie1
> @@ -596,6 +598,35 @@ allOf:
>            items:
>              - const: pci # PCIe core reset
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,pcie-sc8280xp
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 8
> +          maxItems: 9
> +        clock-names:
> +          minItems: 8
> +          items:
> +            - const: aux # Auxiliary clock
> +            - const: cfg # Configuration clock
> +            - const: bus_master # Master AXI clock
> +            - const: bus_slave # Slave AXI clock
> +            - const: slave_q2a # Slave Q2A clock
> +            - const: ddrss_sf_tbu # PCIe SF TBU clock
> +            - const: noc_aggr_4 # NoC aggregate 4 clock
> +            - const: noc_aggr_south_sf # NoC aggregate South SF clock
> +            - const: cnoc_qx # Configuration NoC QX clock
> +        resets:
> +          maxItems: 1
> +        reset-names:
> +          items:
> +            - const: pci # PCIe core reset
> +
>    - if:
>        not:
>          properties:
> @@ -624,7 +655,6 @@ allOf:
>          - resets
>          - reset-names
>  
> -    # On newer chipsets support either 1 or 8 msi interrupts
>    - if:
>        properties:
>          compatible:
> @@ -660,6 +690,24 @@ allOf:
>                  - const: msi6
>                  - const: msi7
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,pcie-sc8280xp
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 4
> +          maxItems: 4
> +        interrupt-names:
> +          items:
> +            - const: msi0
> +            - const: msi1
> +            - const: msi2
> +            - const: msi3
> +
>    - if:
>        properties:
>          compatible:
> -- 
> 2.35.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 04/10] dt-bindings: PCI: qcom: Add SC8280XP to binding

[Johan Hovold]

On Sat, Jul 09, 2022 at 01:30:53PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Jun 29, 2022 at 04:09:54PM +0200, Johan Hovold wrote:
> > Add the SC8280XP platform to the binding.
> > 
> > SC8280XP use four host interrupts for MSI routing so remove the obsolete
> > comment referring to newer chipsets supporting one or eight interrupts
> > (e.g. for backwards compatibility).
> > 
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> 
> Wondering why 4 on this SoC. Is this what added in downstream or you also
> verified it with IP documentation.

I don't have direct access to the IP docs, but I've verified it with
people who do.

Johan

[PATCH 05/10] dt-bindings: PCI: qcom: Add SA8540P to binding

[Johan Hovold]

SA8540P is a new platform related to SC8280XP but which uses a single
host interrupt for MSI routing.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index a039f6110322..e9a7c8c783e7 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -25,6 +25,7 @@ properties:
       - qcom,pcie-ipq4019
       - qcom,pcie-ipq8074
       - qcom,pcie-qcs404
+      - qcom,pcie-sa8540p
       - qcom,pcie-sc7280
       - qcom,pcie-sc8180x
       - qcom,pcie-sc8280xp
@@ -603,6 +604,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - qcom,pcie-sa8540p
               - qcom,pcie-sc8280xp
     then:
       properties:
@@ -720,6 +722,7 @@ allOf:
               - qcom,pcie-ipq8064
               - qcom,pcie-ipq8064-v2
               - qcom,pcie-ipq8074
+              - qcom,pcie-sa8540p
               - qcom,pcie-qcs404
     then:
       properties:
-- 
2.35.1

Re: [PATCH 05/10] dt-bindings: PCI: qcom: Add SA8540P to binding

[Krzysztof Kozlowski]

On 29/06/2022 16:09, Johan Hovold wrote:
> SA8540P is a new platform related to SC8280XP but which uses a single
> host interrupt for MSI routing.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index a039f6110322..e9a7c8c783e7 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -25,6 +25,7 @@ properties:
>        - qcom,pcie-ipq4019
>        - qcom,pcie-ipq8074
>        - qcom,pcie-qcs404
> +      - qcom,pcie-sa8540p
>        - qcom,pcie-sc7280
>        - qcom,pcie-sc8180x
>        - qcom,pcie-sc8280xp
> @@ -603,6 +604,7 @@ allOf:
>          compatible:
>            contains:
>              enum:
> +              - qcom,pcie-sa8540p
>                - qcom,pcie-sc8280xp
>      then:
>        properties:
> @@ -720,6 +722,7 @@ allOf:
>                - qcom,pcie-ipq8064
>                - qcom,pcie-ipq8064-v2
>                - qcom,pcie-ipq8074
> +              - qcom,pcie-sa8540p

Alphabetical order please.

>                - qcom,pcie-qcs404
>      then:
>        properties:


Best regards,
Krzysztof

Re: [PATCH 05/10] dt-bindings: PCI: qcom: Add SA8540P to binding

[Johan Hovold]

On Fri, Jul 01, 2022 at 10:38:24AM +0200, Krzysztof Kozlowski wrote:
> On 29/06/2022 16:09, Johan Hovold wrote:
> > SA8540P is a new platform related to SC8280XP but which uses a single
> > host interrupt for MSI routing.
> > 
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > ---
> >  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > index a039f6110322..e9a7c8c783e7 100644
> > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > @@ -25,6 +25,7 @@ properties:
> >        - qcom,pcie-ipq4019
> >        - qcom,pcie-ipq8074
> >        - qcom,pcie-qcs404
> > +      - qcom,pcie-sa8540p
> >        - qcom,pcie-sc7280
> >        - qcom,pcie-sc8180x
> >        - qcom,pcie-sc8280xp
> > @@ -603,6 +604,7 @@ allOf:
> >          compatible:
> >            contains:
> >              enum:
> > +              - qcom,pcie-sa8540p
> >                - qcom,pcie-sc8280xp
> >      then:
> >        properties:
> > @@ -720,6 +722,7 @@ allOf:
> >                - qcom,pcie-ipq8064
> >                - qcom,pcie-ipq8064-v2
> >                - qcom,pcie-ipq8074
> > +              - qcom,pcie-sa8540p
> 
> Alphabetical order please.

Oops. Will fix.

> >                - qcom,pcie-qcs404

Johan

Re: [PATCH 05/10] dt-bindings: PCI: qcom: Add SA8540P to binding

[Manivannan Sadhasivam]

On Wed, Jun 29, 2022 at 04:09:55PM +0200, Johan Hovold wrote:
> SA8540P is a new platform related to SC8280XP but which uses a single
> host interrupt for MSI routing.
> 

The newer chipsets are supposed to use 8 MSI's. How come this one uses only 1?

Thanks,
Mani

> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index a039f6110322..e9a7c8c783e7 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -25,6 +25,7 @@ properties:
>        - qcom,pcie-ipq4019
>        - qcom,pcie-ipq8074
>        - qcom,pcie-qcs404
> +      - qcom,pcie-sa8540p
>        - qcom,pcie-sc7280
>        - qcom,pcie-sc8180x
>        - qcom,pcie-sc8280xp
> @@ -603,6 +604,7 @@ allOf:
>          compatible:
>            contains:
>              enum:
> +              - qcom,pcie-sa8540p
>                - qcom,pcie-sc8280xp
>      then:
>        properties:
> @@ -720,6 +722,7 @@ allOf:
>                - qcom,pcie-ipq8064
>                - qcom,pcie-ipq8064-v2
>                - qcom,pcie-ipq8074
> +              - qcom,pcie-sa8540p
>                - qcom,pcie-qcs404
>      then:
>        properties:
> -- 
> 2.35.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 05/10] dt-bindings: PCI: qcom: Add SA8540P to binding

[Johan Hovold]

On Sat, Jul 09, 2022 at 01:32:03PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Jun 29, 2022 at 04:09:55PM +0200, Johan Hovold wrote:
> > SA8540P is a new platform related to SC8280XP but which uses a single
> > host interrupt for MSI routing.
> > 
> 
> The newer chipsets are supposed to use 8 MSI's. How come this one uses only 1?

No idea, but the people with access to the documentation could not find
more than one interrupt for SA8540P (unlike SC8280XP).

Johan

[PATCH 06/10] PCI: qcom: Add support for SC8280XP

[Johan Hovold]

The SC8280XP platform has seven PCIe controllers: two used with USB4,
two 4-lane, two 2-lane and one 1-lane.

Add a new "qcom,pcie-sc8280xp" compatible string and reuse the 1.9.0
ops.

Note that the SC8280XP controllers need two or three interconnect
clocks to be enabled. Model these as optional clocks to avoid encoding
devicetree data in the PCIe driver.

Note that the same could be done for the SM8450 interconnect clocks and
possibly also for the TBU clocks.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index ff1b40f213c1..da3f1cdc4ba6 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -160,7 +160,7 @@ struct qcom_pcie_resources_2_3_3 {
 
 /* 6 clocks typically, 7 for sm8250 */
 struct qcom_pcie_resources_2_7_0 {
-	struct clk_bulk_data clks[9];
+	struct clk_bulk_data clks[12];
 	int num_clks;
 	struct regulator_bulk_data supplies[2];
 	struct reset_control *pci_reset;
@@ -1119,6 +1119,7 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
 	struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
 	struct dw_pcie *pci = pcie->pci;
 	struct device *dev = pci->dev;
+	unsigned int num_clks, num_opt_clks;
 	unsigned int idx;
 	int ret;
 
@@ -1148,9 +1149,20 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
 	if (pcie->cfg->has_aggre1_clk)
 		res->clks[idx++].id = "aggre1";
 
+	num_clks = idx;
+
+	ret = devm_clk_bulk_get(dev, num_clks, res->clks);
+	if (ret < 0)
+		return ret;
+
+	res->clks[idx++].id = "noc_aggr_4";
+	res->clks[idx++].id = "noc_aggr_south_sf";
+	res->clks[idx++].id = "cnoc_qx";
+
+	num_opt_clks = idx - num_clks;
 	res->num_clks = idx;
 
-	ret = devm_clk_bulk_get(dev, res->num_clks, res->clks);
+	ret = devm_clk_bulk_get_optional(dev, num_opt_clks, res->clks + num_clks);
 	if (ret < 0)
 		return ret;
 
@@ -1449,6 +1461,11 @@ static const struct qcom_pcie_cfg ipq4019_cfg = {
 	.ops = &ops_2_4_0,
 };
 
+static const struct qcom_pcie_cfg sc8280xp_cfg = {
+	.ops = &ops_1_9_0,
+	.has_ddrss_sf_tbu_clk = true,
+};
+
 static const struct qcom_pcie_cfg sdm845_cfg = {
 	.ops = &ops_2_7_0,
 	.has_tbu_clk = true,
@@ -1613,6 +1630,7 @@ static const struct of_device_id qcom_pcie_match[] = {
 	{ .compatible = "qcom,pcie-sm8150", .data = &sm8150_cfg },
 	{ .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg },
 	{ .compatible = "qcom,pcie-sc8180x", .data = &sc8180x_cfg },
+	{ .compatible = "qcom,pcie-sc8280xp", .data = &sc8280xp_cfg },
 	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &sm8450_pcie0_cfg },
 	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &sm8450_pcie1_cfg },
 	{ .compatible = "qcom,pcie-sc7280", .data = &sc7280_cfg },
-- 
2.35.1

Re: [PATCH 06/10] PCI: qcom: Add support for SC8280XP

[Rob Herring]

On Wed, Jun 29, 2022 at 04:09:56PM +0200, Johan Hovold wrote:
> The SC8280XP platform has seven PCIe controllers: two used with USB4,
> two 4-lane, two 2-lane and one 1-lane.
> 
> Add a new "qcom,pcie-sc8280xp" compatible string and reuse the 1.9.0
> ops.
> 
> Note that the SC8280XP controllers need two or three interconnect
> clocks to be enabled. Model these as optional clocks to avoid encoding
> devicetree data in the PCIe driver.

Shouldn't the interconnect binding handle these? Probably, bus clocks 
have to be the biggest single reason why clocks are such a mess in terms 
of 'the same' block having different clocks.

> 
> Note that the same could be done for the SM8450 interconnect clocks and
> possibly also for the TBU clocks.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index ff1b40f213c1..da3f1cdc4ba6 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -160,7 +160,7 @@ struct qcom_pcie_resources_2_3_3 {
>  
>  /* 6 clocks typically, 7 for sm8250 */
>  struct qcom_pcie_resources_2_7_0 {
> -	struct clk_bulk_data clks[9];
> +	struct clk_bulk_data clks[12];
>  	int num_clks;
>  	struct regulator_bulk_data supplies[2];
>  	struct reset_control *pci_reset;
> @@ -1119,6 +1119,7 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
>  	struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
>  	struct dw_pcie *pci = pcie->pci;
>  	struct device *dev = pci->dev;
> +	unsigned int num_clks, num_opt_clks;
>  	unsigned int idx;
>  	int ret;
>  
> @@ -1148,9 +1149,20 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
>  	if (pcie->cfg->has_aggre1_clk)
>  		res->clks[idx++].id = "aggre1";
>  
> +	num_clks = idx;
> +
> +	ret = devm_clk_bulk_get(dev, num_clks, res->clks);
> +	if (ret < 0)
> +		return ret;
> +
> +	res->clks[idx++].id = "noc_aggr_4";
> +	res->clks[idx++].id = "noc_aggr_south_sf";
> +	res->clks[idx++].id = "cnoc_qx";
> +
> +	num_opt_clks = idx - num_clks;
>  	res->num_clks = idx;
>  
> -	ret = devm_clk_bulk_get(dev, res->num_clks, res->clks);
> +	ret = devm_clk_bulk_get_optional(dev, num_opt_clks, res->clks + num_clks);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -1449,6 +1461,11 @@ static const struct qcom_pcie_cfg ipq4019_cfg = {
>  	.ops = &ops_2_4_0,
>  };
>  
> +static const struct qcom_pcie_cfg sc8280xp_cfg = {
> +	.ops = &ops_1_9_0,
> +	.has_ddrss_sf_tbu_clk = true,
> +};
> +
>  static const struct qcom_pcie_cfg sdm845_cfg = {
>  	.ops = &ops_2_7_0,
>  	.has_tbu_clk = true,
> @@ -1613,6 +1630,7 @@ static const struct of_device_id qcom_pcie_match[] = {
>  	{ .compatible = "qcom,pcie-sm8150", .data = &sm8150_cfg },
>  	{ .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg },
>  	{ .compatible = "qcom,pcie-sc8180x", .data = &sc8180x_cfg },
> +	{ .compatible = "qcom,pcie-sc8280xp", .data = &sc8280xp_cfg },
>  	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &sm8450_pcie0_cfg },
>  	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &sm8450_pcie1_cfg },
>  	{ .compatible = "qcom,pcie-sc7280", .data = &sc7280_cfg },
> -- 
> 2.35.1
> 
> 

Re: [PATCH 06/10] PCI: qcom: Add support for SC8280XP

[Johan Hovold]

On Fri, Jul 01, 2022 at 12:29:09PM -0600, Rob Herring wrote:
> On Wed, Jun 29, 2022 at 04:09:56PM +0200, Johan Hovold wrote:
> > The SC8280XP platform has seven PCIe controllers: two used with USB4,
> > two 4-lane, two 2-lane and one 1-lane.
> > 
> > Add a new "qcom,pcie-sc8280xp" compatible string and reuse the 1.9.0
> > ops.
> > 
> > Note that the SC8280XP controllers need two or three interconnect
> > clocks to be enabled. Model these as optional clocks to avoid encoding
> > devicetree data in the PCIe driver.
> 
> Shouldn't the interconnect binding handle these? Probably, bus clocks 
> have to be the biggest single reason why clocks are such a mess in terms 
> of 'the same' block having different clocks.

I fully agree, but I ended up following the current scheme of letting
the driver manage these, partly as I lack documentation for the
interconnect. 

Note that two interconnect clocks were recently included in the SM8450
binding:

	https://lore.kernel.org/all/YcIwcUzYCq1v4Kfs@robh.at.kernel.org/

and that the generically sounding clock names chosen make it tempting to
repurpose them:

	https://lore.kernel.org/all/1656691899-21315-4-git-send-email-quic_krichai@quicinc.com/

which seems like another step backwards.

For MSM8996, it looks like this was instead handled by never disabling
the interconnect clocks: 

	https://lore.kernel.org/all/20171207105922.31986-1-srinivas.kandagatla@linaro.org/

Johan

Re: [PATCH 06/10] PCI: qcom: Add support for SC8280XP

[Manivannan Sadhasivam]

On Wed, Jun 29, 2022 at 04:09:56PM +0200, Johan Hovold wrote:
> The SC8280XP platform has seven PCIe controllers: two used with USB4,
> two 4-lane, two 2-lane and one 1-lane.
> 
> Add a new "qcom,pcie-sc8280xp" compatible string and reuse the 1.9.0
> ops.
> 
> Note that the SC8280XP controllers need two or three interconnect
> clocks to be enabled. Model these as optional clocks to avoid encoding
> devicetree data in the PCIe driver.
> 

There isn't much info available for these clocks. Since some of these types of
clocks are already present in bindings, I think this patch is fine by itself.

I will also try to find how these clocks are laid out. But that shouldn't stop
this patch IMO.

> Note that the same could be done for the SM8450 interconnect clocks and
> possibly also for the TBU clocks.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index ff1b40f213c1..da3f1cdc4ba6 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -160,7 +160,7 @@ struct qcom_pcie_resources_2_3_3 {
>  
>  /* 6 clocks typically, 7 for sm8250 */
>  struct qcom_pcie_resources_2_7_0 {
> -	struct clk_bulk_data clks[9];
> +	struct clk_bulk_data clks[12];
>  	int num_clks;
>  	struct regulator_bulk_data supplies[2];
>  	struct reset_control *pci_reset;
> @@ -1119,6 +1119,7 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
>  	struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
>  	struct dw_pcie *pci = pcie->pci;
>  	struct device *dev = pci->dev;
> +	unsigned int num_clks, num_opt_clks;
>  	unsigned int idx;
>  	int ret;
>  
> @@ -1148,9 +1149,20 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
>  	if (pcie->cfg->has_aggre1_clk)
>  		res->clks[idx++].id = "aggre1";
>  
> +	num_clks = idx;
> +
> +	ret = devm_clk_bulk_get(dev, num_clks, res->clks);
> +	if (ret < 0)
> +		return ret;
> +
> +	res->clks[idx++].id = "noc_aggr_4";
> +	res->clks[idx++].id = "noc_aggr_south_sf";
> +	res->clks[idx++].id = "cnoc_qx";
> +
> +	num_opt_clks = idx - num_clks;
>  	res->num_clks = idx;
>  
> -	ret = devm_clk_bulk_get(dev, res->num_clks, res->clks);
> +	ret = devm_clk_bulk_get_optional(dev, num_opt_clks, res->clks + num_clks);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -1449,6 +1461,11 @@ static const struct qcom_pcie_cfg ipq4019_cfg = {
>  	.ops = &ops_2_4_0,
>  };
>  
> +static const struct qcom_pcie_cfg sc8280xp_cfg = {
> +	.ops = &ops_1_9_0,
> +	.has_ddrss_sf_tbu_clk = true,
> +};
> +
>  static const struct qcom_pcie_cfg sdm845_cfg = {
>  	.ops = &ops_2_7_0,
>  	.has_tbu_clk = true,
> @@ -1613,6 +1630,7 @@ static const struct of_device_id qcom_pcie_match[] = {
>  	{ .compatible = "qcom,pcie-sm8150", .data = &sm8150_cfg },
>  	{ .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg },
>  	{ .compatible = "qcom,pcie-sc8180x", .data = &sc8180x_cfg },
> +	{ .compatible = "qcom,pcie-sc8280xp", .data = &sc8280xp_cfg },
>  	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &sm8450_pcie0_cfg },
>  	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &sm8450_pcie1_cfg },
>  	{ .compatible = "qcom,pcie-sc7280", .data = &sc7280_cfg },
> -- 
> 2.35.1
> 

-- 
மணிவண்ணன் சதாசிவம்

[PATCH 07/10] PCI: qcom: Add support for SA8540P

[Johan Hovold]

The SA8540P platform has five PCIe controllers: two 4-lane, two 2-lane
and one 1-lane.

Add a new "qcom,pcie-sa8540p" compatible string and reuse the 1.9.0 ops.

Note that like for SC8280XP, the SA8540 controllers need two or three
interconnect clocks to be enabled.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index da3f1cdc4ba6..8ab88e5743da 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1461,6 +1461,11 @@ static const struct qcom_pcie_cfg ipq4019_cfg = {
 	.ops = &ops_2_4_0,
 };
 
+static const struct qcom_pcie_cfg sa8540p_cfg = {
+	.ops = &ops_1_9_0,
+	.has_ddrss_sf_tbu_clk = true,
+};
+
 static const struct qcom_pcie_cfg sc8280xp_cfg = {
 	.ops = &ops_1_9_0,
 	.has_ddrss_sf_tbu_clk = true,
@@ -1626,6 +1631,7 @@ static const struct of_device_id qcom_pcie_match[] = {
 	{ .compatible = "qcom,pcie-ipq8074", .data = &ipq8074_cfg },
 	{ .compatible = "qcom,pcie-ipq4019", .data = &ipq4019_cfg },
 	{ .compatible = "qcom,pcie-qcs404", .data = &ipq4019_cfg },
+	{ .compatible = "qcom,pcie-sa8540p", .data = &sa8540p_cfg },
 	{ .compatible = "qcom,pcie-sdm845", .data = &sdm845_cfg },
 	{ .compatible = "qcom,pcie-sm8150", .data = &sm8150_cfg },
 	{ .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg },
-- 
2.35.1

Re: [PATCH 07/10] PCI: qcom: Add support for SA8540P

[Rob Herring]

On Wed, Jun 29, 2022 at 04:09:57PM +0200, Johan Hovold wrote:
> The SA8540P platform has five PCIe controllers: two 4-lane, two 2-lane
> and one 1-lane.
> 
> Add a new "qcom,pcie-sa8540p" compatible string and reuse the 1.9.0 ops.
> 
> Note that like for SC8280XP, the SA8540 controllers need two or three
> interconnect clocks to be enabled.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 07/10] PCI: qcom: Add support for SA8540P

[Manivannan Sadhasivam]

On Wed, Jun 29, 2022 at 04:09:57PM +0200, Johan Hovold wrote:
> The SA8540P platform has five PCIe controllers: two 4-lane, two 2-lane
> and one 1-lane.
> 
> Add a new "qcom,pcie-sa8540p" compatible string and reuse the 1.9.0 ops.
> 
> Note that like for SC8280XP, the SA8540 controllers need two or three
> interconnect clocks to be enabled.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index da3f1cdc4ba6..8ab88e5743da 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1461,6 +1461,11 @@ static const struct qcom_pcie_cfg ipq4019_cfg = {
>  	.ops = &ops_2_4_0,
>  };
>  
> +static const struct qcom_pcie_cfg sa8540p_cfg = {
> +	.ops = &ops_1_9_0,
> +	.has_ddrss_sf_tbu_clk = true,
> +};
> +
>  static const struct qcom_pcie_cfg sc8280xp_cfg = {
>  	.ops = &ops_1_9_0,
>  	.has_ddrss_sf_tbu_clk = true,
> @@ -1626,6 +1631,7 @@ static const struct of_device_id qcom_pcie_match[] = {
>  	{ .compatible = "qcom,pcie-ipq8074", .data = &ipq8074_cfg },
>  	{ .compatible = "qcom,pcie-ipq4019", .data = &ipq4019_cfg },
>  	{ .compatible = "qcom,pcie-qcs404", .data = &ipq4019_cfg },
> +	{ .compatible = "qcom,pcie-sa8540p", .data = &sa8540p_cfg },
>  	{ .compatible = "qcom,pcie-sdm845", .data = &sdm845_cfg },
>  	{ .compatible = "qcom,pcie-sm8150", .data = &sm8150_cfg },
>  	{ .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg },
> -- 
> 2.35.1
> 

-- 
மணிவண்ணன் சதாசிவம்

[PATCH 08/10] PCI: qcom: Make all optional clocks optional

[Johan Hovold]

The kernel is not a devicetree validator and does not need to re-encode
information which is already available in the devicetree.

This is specifically true for the optional PCIe clocks, some of which
are really interconnect clocks.

Treat also the 2.7.0 optional clocks as truly optional instead of
maintaining a list of clocks per compatible (including two compatible
strings for the two identical controllers on sm8450) just to validate
the devicetree.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 28 ++++----------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 8ab88e5743da..1a564f624bb1 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -189,10 +189,6 @@ struct qcom_pcie_ops {
 
 struct qcom_pcie_cfg {
 	const struct qcom_pcie_ops *ops;
-	unsigned int has_tbu_clk:1;
-	unsigned int has_ddrss_sf_tbu_clk:1;
-	unsigned int has_aggre0_clk:1;
-	unsigned int has_aggre1_clk:1;
 };
 
 struct qcom_pcie {
@@ -1140,14 +1136,6 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
 	res->clks[idx++].id = "bus_master";
 	res->clks[idx++].id = "bus_slave";
 	res->clks[idx++].id = "slave_q2a";
-	if (pcie->cfg->has_tbu_clk)
-		res->clks[idx++].id = "tbu";
-	if (pcie->cfg->has_ddrss_sf_tbu_clk)
-		res->clks[idx++].id = "ddrss_sf_tbu";
-	if (pcie->cfg->has_aggre0_clk)
-		res->clks[idx++].id = "aggre0";
-	if (pcie->cfg->has_aggre1_clk)
-		res->clks[idx++].id = "aggre1";
 
 	num_clks = idx;
 
@@ -1155,6 +1143,10 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
 	if (ret < 0)
 		return ret;
 
+	res->clks[idx++].id = "tbu";
+	res->clks[idx++].id = "ddrss_sf_tbu";
+	res->clks[idx++].id = "aggre0";
+	res->clks[idx++].id = "aggre1";
 	res->clks[idx++].id = "noc_aggr_4";
 	res->clks[idx++].id = "noc_aggr_south_sf";
 	res->clks[idx++].id = "cnoc_qx";
@@ -1463,17 +1455,14 @@ static const struct qcom_pcie_cfg ipq4019_cfg = {
 
 static const struct qcom_pcie_cfg sa8540p_cfg = {
 	.ops = &ops_1_9_0,
-	.has_ddrss_sf_tbu_clk = true,
 };
 
 static const struct qcom_pcie_cfg sc8280xp_cfg = {
 	.ops = &ops_1_9_0,
-	.has_ddrss_sf_tbu_clk = true,
 };
 
 static const struct qcom_pcie_cfg sdm845_cfg = {
 	.ops = &ops_2_7_0,
-	.has_tbu_clk = true,
 };
 
 static const struct qcom_pcie_cfg sm8150_cfg = {
@@ -1485,31 +1474,22 @@ static const struct qcom_pcie_cfg sm8150_cfg = {
 
 static const struct qcom_pcie_cfg sm8250_cfg = {
 	.ops = &ops_1_9_0,
-	.has_tbu_clk = true,
-	.has_ddrss_sf_tbu_clk = true,
 };
 
 static const struct qcom_pcie_cfg sm8450_pcie0_cfg = {
 	.ops = &ops_1_9_0,
-	.has_ddrss_sf_tbu_clk = true,
-	.has_aggre0_clk = true,
-	.has_aggre1_clk = true,
 };
 
 static const struct qcom_pcie_cfg sm8450_pcie1_cfg = {
 	.ops = &ops_1_9_0,
-	.has_ddrss_sf_tbu_clk = true,
-	.has_aggre1_clk = true,
 };
 
 static const struct qcom_pcie_cfg sc7280_cfg = {
 	.ops = &ops_1_9_0,
-	.has_tbu_clk = true,
 };
 
 static const struct qcom_pcie_cfg sc8180x_cfg = {
 	.ops = &ops_1_9_0,
-	.has_tbu_clk = true,
 };
 
 static const struct dw_pcie_ops dw_pcie_ops = {
-- 
2.35.1

Re: [PATCH 08/10] PCI: qcom: Make all optional clocks optional

[Rob Herring]

On Wed, Jun 29, 2022 at 04:09:58PM +0200, Johan Hovold wrote:
> The kernel is not a devicetree validator and does not need to re-encode
> information which is already available in the devicetree.
> 
> This is specifically true for the optional PCIe clocks, some of which
> are really interconnect clocks.
> 
> Treat also the 2.7.0 optional clocks as truly optional instead of
> maintaining a list of clocks per compatible (including two compatible
> strings for the two identical controllers on sm8450) just to validate
> the devicetree.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 28 ++++----------------------
>  1 file changed, 4 insertions(+), 24 deletions(-)

I guess this depends on patch 6, but looks like a good cleanup.

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 08/10] PCI: qcom: Make all optional clocks optional

[Manivannan Sadhasivam]

On Wed, Jun 29, 2022 at 04:09:58PM +0200, Johan Hovold wrote:
> The kernel is not a devicetree validator and does not need to re-encode
> information which is already available in the devicetree.
> 
> This is specifically true for the optional PCIe clocks, some of which
> are really interconnect clocks.
> 
> Treat also the 2.7.0 optional clocks as truly optional instead of
> maintaining a list of clocks per compatible (including two compatible
> strings for the two identical controllers on sm8450) just to validate
> the devicetree.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 28 ++++----------------------
>  1 file changed, 4 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 8ab88e5743da..1a564f624bb1 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -189,10 +189,6 @@ struct qcom_pcie_ops {
>  
>  struct qcom_pcie_cfg {
>  	const struct qcom_pcie_ops *ops;
> -	unsigned int has_tbu_clk:1;
> -	unsigned int has_ddrss_sf_tbu_clk:1;
> -	unsigned int has_aggre0_clk:1;
> -	unsigned int has_aggre1_clk:1;
>  };
>  
>  struct qcom_pcie {
> @@ -1140,14 +1136,6 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
>  	res->clks[idx++].id = "bus_master";
>  	res->clks[idx++].id = "bus_slave";
>  	res->clks[idx++].id = "slave_q2a";
> -	if (pcie->cfg->has_tbu_clk)
> -		res->clks[idx++].id = "tbu";
> -	if (pcie->cfg->has_ddrss_sf_tbu_clk)
> -		res->clks[idx++].id = "ddrss_sf_tbu";
> -	if (pcie->cfg->has_aggre0_clk)
> -		res->clks[idx++].id = "aggre0";
> -	if (pcie->cfg->has_aggre1_clk)
> -		res->clks[idx++].id = "aggre1";
>  
>  	num_clks = idx;
>  
> @@ -1155,6 +1143,10 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
>  	if (ret < 0)
>  		return ret;
>  
> +	res->clks[idx++].id = "tbu";
> +	res->clks[idx++].id = "ddrss_sf_tbu";
> +	res->clks[idx++].id = "aggre0";
> +	res->clks[idx++].id = "aggre1";
>  	res->clks[idx++].id = "noc_aggr_4";
>  	res->clks[idx++].id = "noc_aggr_south_sf";
>  	res->clks[idx++].id = "cnoc_qx";
> @@ -1463,17 +1455,14 @@ static const struct qcom_pcie_cfg ipq4019_cfg = {
>  
>  static const struct qcom_pcie_cfg sa8540p_cfg = {
>  	.ops = &ops_1_9_0,
> -	.has_ddrss_sf_tbu_clk = true,
>  };
>  
>  static const struct qcom_pcie_cfg sc8280xp_cfg = {
>  	.ops = &ops_1_9_0,
> -	.has_ddrss_sf_tbu_clk = true,
>  };
>  
>  static const struct qcom_pcie_cfg sdm845_cfg = {
>  	.ops = &ops_2_7_0,
> -	.has_tbu_clk = true,
>  };
>  
>  static const struct qcom_pcie_cfg sm8150_cfg = {
> @@ -1485,31 +1474,22 @@ static const struct qcom_pcie_cfg sm8150_cfg = {
>  
>  static const struct qcom_pcie_cfg sm8250_cfg = {
>  	.ops = &ops_1_9_0,
> -	.has_tbu_clk = true,
> -	.has_ddrss_sf_tbu_clk = true,
>  };
>  
>  static const struct qcom_pcie_cfg sm8450_pcie0_cfg = {
>  	.ops = &ops_1_9_0,
> -	.has_ddrss_sf_tbu_clk = true,
> -	.has_aggre0_clk = true,
> -	.has_aggre1_clk = true,
>  };
>  
>  static const struct qcom_pcie_cfg sm8450_pcie1_cfg = {
>  	.ops = &ops_1_9_0,
> -	.has_ddrss_sf_tbu_clk = true,
> -	.has_aggre1_clk = true,
>  };
>  
>  static const struct qcom_pcie_cfg sc7280_cfg = {
>  	.ops = &ops_1_9_0,
> -	.has_tbu_clk = true,
>  };
>  
>  static const struct qcom_pcie_cfg sc8180x_cfg = {
>  	.ops = &ops_1_9_0,
> -	.has_tbu_clk = true,
>  };
>  
>  static const struct dw_pcie_ops dw_pcie_ops = {
> -- 
> 2.35.1
> 

-- 
மணிவண்ணன் சதாசிவம்

[PATCH 09/10] PCI: qcom: Clean up IP configurations

[Johan Hovold]

The various IP versions have different configurations that are encoded
in separate sets of operation callbacks. Currently, there is no need for
also maintaining corresponding sets of data parameters, but it is
conceivable that these may again be found useful (e.g. to implement
minor variations of the operation callbacks).

Rename the default configuration structures after the IP version they
apply to so that they can more easily be reused by different SoCs.

Note that SoC specific configurations can be added later if need arises
(e.g. cfg_sc8280xp).

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 85 ++++++++------------------
 1 file changed, 27 insertions(+), 58 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 1a564f624bb1..567601679465 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1433,65 +1433,34 @@ static const struct qcom_pcie_ops ops_1_9_0 = {
 	.config_sid = qcom_pcie_config_sid_sm8250,
 };
 
-static const struct qcom_pcie_cfg apq8084_cfg = {
+static const struct qcom_pcie_cfg cfg_1_0_0 = {
 	.ops = &ops_1_0_0,
 };
 
-static const struct qcom_pcie_cfg ipq8064_cfg = {
+static const struct qcom_pcie_cfg cfg_1_9_0 = {
+	.ops = &ops_1_9_0,
+};
+
+static const struct qcom_pcie_cfg cfg_2_1_0 = {
 	.ops = &ops_2_1_0,
 };
 
-static const struct qcom_pcie_cfg msm8996_cfg = {
+static const struct qcom_pcie_cfg cfg_2_3_2 = {
 	.ops = &ops_2_3_2,
 };
 
-static const struct qcom_pcie_cfg ipq8074_cfg = {
+static const struct qcom_pcie_cfg cfg_2_3_3 = {
 	.ops = &ops_2_3_3,
 };
 
-static const struct qcom_pcie_cfg ipq4019_cfg = {
+static const struct qcom_pcie_cfg cfg_2_4_0 = {
 	.ops = &ops_2_4_0,
 };
 
-static const struct qcom_pcie_cfg sa8540p_cfg = {
-	.ops = &ops_1_9_0,
-};
-
-static const struct qcom_pcie_cfg sc8280xp_cfg = {
-	.ops = &ops_1_9_0,
-};
-
-static const struct qcom_pcie_cfg sdm845_cfg = {
+static const struct qcom_pcie_cfg cfg_2_7_0 = {
 	.ops = &ops_2_7_0,
 };
 
-static const struct qcom_pcie_cfg sm8150_cfg = {
-	/* sm8150 has qcom IP rev 1.5.0. However 1.5.0 ops are same as
-	 * 1.9.0, so reuse the same.
-	 */
-	.ops = &ops_1_9_0,
-};
-
-static const struct qcom_pcie_cfg sm8250_cfg = {
-	.ops = &ops_1_9_0,
-};
-
-static const struct qcom_pcie_cfg sm8450_pcie0_cfg = {
-	.ops = &ops_1_9_0,
-};
-
-static const struct qcom_pcie_cfg sm8450_pcie1_cfg = {
-	.ops = &ops_1_9_0,
-};
-
-static const struct qcom_pcie_cfg sc7280_cfg = {
-	.ops = &ops_1_9_0,
-};
-
-static const struct qcom_pcie_cfg sc8180x_cfg = {
-	.ops = &ops_1_9_0,
-};
-
 static const struct dw_pcie_ops dw_pcie_ops = {
 	.link_up = qcom_pcie_link_up,
 	.start_link = qcom_pcie_start_link,
@@ -1603,23 +1572,23 @@ static int qcom_pcie_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id qcom_pcie_match[] = {
-	{ .compatible = "qcom,pcie-apq8084", .data = &apq8084_cfg },
-	{ .compatible = "qcom,pcie-ipq8064", .data = &ipq8064_cfg },
-	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &ipq8064_cfg },
-	{ .compatible = "qcom,pcie-apq8064", .data = &ipq8064_cfg },
-	{ .compatible = "qcom,pcie-msm8996", .data = &msm8996_cfg },
-	{ .compatible = "qcom,pcie-ipq8074", .data = &ipq8074_cfg },
-	{ .compatible = "qcom,pcie-ipq4019", .data = &ipq4019_cfg },
-	{ .compatible = "qcom,pcie-qcs404", .data = &ipq4019_cfg },
-	{ .compatible = "qcom,pcie-sa8540p", .data = &sa8540p_cfg },
-	{ .compatible = "qcom,pcie-sdm845", .data = &sdm845_cfg },
-	{ .compatible = "qcom,pcie-sm8150", .data = &sm8150_cfg },
-	{ .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg },
-	{ .compatible = "qcom,pcie-sc8180x", .data = &sc8180x_cfg },
-	{ .compatible = "qcom,pcie-sc8280xp", .data = &sc8280xp_cfg },
-	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &sm8450_pcie0_cfg },
-	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &sm8450_pcie1_cfg },
-	{ .compatible = "qcom,pcie-sc7280", .data = &sc7280_cfg },
+	{ .compatible = "qcom,pcie-apq8084", .data = &cfg_1_0_0 },
+	{ .compatible = "qcom,pcie-ipq8064", .data = &cfg_2_1_0 },
+	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
+	{ .compatible = "qcom,pcie-apq8064", .data = &cfg_2_1_0 },
+	{ .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
+	{ .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
+	{ .compatible = "qcom,pcie-ipq4019", .data = &cfg_2_4_0 },
+	{ .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
+	{ .compatible = "qcom,pcie-sa8540p", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-sdm845", .data = &cfg_2_7_0 },
+	{ .compatible = "qcom,pcie-sm8150", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-sm8250", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-sc8180x", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-sc8280xp", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-sc7280", .data = &cfg_1_9_0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, qcom_pcie_match);
-- 
2.35.1

Re: [PATCH 09/10] PCI: qcom: Clean up IP configurations

[Rob Herring]

On Wed, Jun 29, 2022 at 04:09:59PM +0200, Johan Hovold wrote:
> The various IP versions have different configurations that are encoded
> in separate sets of operation callbacks. Currently, there is no need for
> also maintaining corresponding sets of data parameters, but it is
> conceivable that these may again be found useful (e.g. to implement
> minor variations of the operation callbacks).
> 
> Rename the default configuration structures after the IP version they
> apply to so that they can more easily be reused by different SoCs.
> 
> Note that SoC specific configurations can be added later if need arises
> (e.g. cfg_sc8280xp).
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 85 ++++++++------------------
>  1 file changed, 27 insertions(+), 58 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 09/10] PCI: qcom: Clean up IP configurations

[Manivannan Sadhasivam]

On Wed, Jun 29, 2022 at 04:09:59PM +0200, Johan Hovold wrote:
> The various IP versions have different configurations that are encoded
> in separate sets of operation callbacks. Currently, there is no need for
> also maintaining corresponding sets of data parameters, but it is
> conceivable that these may again be found useful (e.g. to implement
> minor variations of the operation callbacks).
> 
> Rename the default configuration structures after the IP version they
> apply to so that they can more easily be reused by different SoCs.
> 
> Note that SoC specific configurations can be added later if need arises
> (e.g. cfg_sc8280xp).
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 85 ++++++++------------------
>  1 file changed, 27 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 1a564f624bb1..567601679465 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1433,65 +1433,34 @@ static const struct qcom_pcie_ops ops_1_9_0 = {
>  	.config_sid = qcom_pcie_config_sid_sm8250,
>  };
>  
> -static const struct qcom_pcie_cfg apq8084_cfg = {
> +static const struct qcom_pcie_cfg cfg_1_0_0 = {
>  	.ops = &ops_1_0_0,
>  };
>  
> -static const struct qcom_pcie_cfg ipq8064_cfg = {
> +static const struct qcom_pcie_cfg cfg_1_9_0 = {
> +	.ops = &ops_1_9_0,
> +};
> +
> +static const struct qcom_pcie_cfg cfg_2_1_0 = {
>  	.ops = &ops_2_1_0,
>  };
>  
> -static const struct qcom_pcie_cfg msm8996_cfg = {
> +static const struct qcom_pcie_cfg cfg_2_3_2 = {
>  	.ops = &ops_2_3_2,
>  };
>  
> -static const struct qcom_pcie_cfg ipq8074_cfg = {
> +static const struct qcom_pcie_cfg cfg_2_3_3 = {
>  	.ops = &ops_2_3_3,
>  };
>  
> -static const struct qcom_pcie_cfg ipq4019_cfg = {
> +static const struct qcom_pcie_cfg cfg_2_4_0 = {
>  	.ops = &ops_2_4_0,
>  };
>  
> -static const struct qcom_pcie_cfg sa8540p_cfg = {
> -	.ops = &ops_1_9_0,
> -};
> -
> -static const struct qcom_pcie_cfg sc8280xp_cfg = {
> -	.ops = &ops_1_9_0,
> -};
> -
> -static const struct qcom_pcie_cfg sdm845_cfg = {
> +static const struct qcom_pcie_cfg cfg_2_7_0 = {
>  	.ops = &ops_2_7_0,
>  };
>  
> -static const struct qcom_pcie_cfg sm8150_cfg = {
> -	/* sm8150 has qcom IP rev 1.5.0. However 1.5.0 ops are same as
> -	 * 1.9.0, so reuse the same.
> -	 */
> -	.ops = &ops_1_9_0,
> -};
> -
> -static const struct qcom_pcie_cfg sm8250_cfg = {
> -	.ops = &ops_1_9_0,
> -};
> -
> -static const struct qcom_pcie_cfg sm8450_pcie0_cfg = {
> -	.ops = &ops_1_9_0,
> -};
> -
> -static const struct qcom_pcie_cfg sm8450_pcie1_cfg = {
> -	.ops = &ops_1_9_0,
> -};
> -
> -static const struct qcom_pcie_cfg sc7280_cfg = {
> -	.ops = &ops_1_9_0,
> -};
> -
> -static const struct qcom_pcie_cfg sc8180x_cfg = {
> -	.ops = &ops_1_9_0,
> -};
> -
>  static const struct dw_pcie_ops dw_pcie_ops = {
>  	.link_up = qcom_pcie_link_up,
>  	.start_link = qcom_pcie_start_link,
> @@ -1603,23 +1572,23 @@ static int qcom_pcie_remove(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id qcom_pcie_match[] = {
> -	{ .compatible = "qcom,pcie-apq8084", .data = &apq8084_cfg },
> -	{ .compatible = "qcom,pcie-ipq8064", .data = &ipq8064_cfg },
> -	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &ipq8064_cfg },
> -	{ .compatible = "qcom,pcie-apq8064", .data = &ipq8064_cfg },
> -	{ .compatible = "qcom,pcie-msm8996", .data = &msm8996_cfg },
> -	{ .compatible = "qcom,pcie-ipq8074", .data = &ipq8074_cfg },
> -	{ .compatible = "qcom,pcie-ipq4019", .data = &ipq4019_cfg },
> -	{ .compatible = "qcom,pcie-qcs404", .data = &ipq4019_cfg },
> -	{ .compatible = "qcom,pcie-sa8540p", .data = &sa8540p_cfg },
> -	{ .compatible = "qcom,pcie-sdm845", .data = &sdm845_cfg },
> -	{ .compatible = "qcom,pcie-sm8150", .data = &sm8150_cfg },
> -	{ .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg },
> -	{ .compatible = "qcom,pcie-sc8180x", .data = &sc8180x_cfg },
> -	{ .compatible = "qcom,pcie-sc8280xp", .data = &sc8280xp_cfg },
> -	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &sm8450_pcie0_cfg },
> -	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &sm8450_pcie1_cfg },
> -	{ .compatible = "qcom,pcie-sc7280", .data = &sc7280_cfg },
> +	{ .compatible = "qcom,pcie-apq8084", .data = &cfg_1_0_0 },
> +	{ .compatible = "qcom,pcie-ipq8064", .data = &cfg_2_1_0 },
> +	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
> +	{ .compatible = "qcom,pcie-apq8064", .data = &cfg_2_1_0 },
> +	{ .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
> +	{ .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
> +	{ .compatible = "qcom,pcie-ipq4019", .data = &cfg_2_4_0 },
> +	{ .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
> +	{ .compatible = "qcom,pcie-sa8540p", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-sdm845", .data = &cfg_2_7_0 },
> +	{ .compatible = "qcom,pcie-sm8150", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-sm8250", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-sc8180x", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-sc8280xp", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-sc7280", .data = &cfg_1_9_0 },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, qcom_pcie_match);
> -- 
> 2.35.1
> 

-- 
மணிவண்ணன் சதாசிவம்

[PATCH 10/10] PCI: qcom: Sort device-id table

[Johan Hovold]

Sort the device-id table entries alphabetically by compatible string to
make it easier to find entries and add new ones.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 567601679465..093f4d4bc15d 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1572,23 +1572,23 @@ static int qcom_pcie_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id qcom_pcie_match[] = {
+	{ .compatible = "qcom,pcie-apq8064", .data = &cfg_2_1_0 },
 	{ .compatible = "qcom,pcie-apq8084", .data = &cfg_1_0_0 },
 	{ .compatible = "qcom,pcie-ipq8064", .data = &cfg_2_1_0 },
 	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
-	{ .compatible = "qcom,pcie-apq8064", .data = &cfg_2_1_0 },
-	{ .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
 	{ .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
 	{ .compatible = "qcom,pcie-ipq4019", .data = &cfg_2_4_0 },
-	{ .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
+	{ .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
 	{ .compatible = "qcom,pcie-sa8540p", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-sc7280", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-sc8180x", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-sc8280xp", .data = &cfg_1_9_0 },
 	{ .compatible = "qcom,pcie-sdm845", .data = &cfg_2_7_0 },
 	{ .compatible = "qcom,pcie-sm8150", .data = &cfg_1_9_0 },
 	{ .compatible = "qcom,pcie-sm8250", .data = &cfg_1_9_0 },
-	{ .compatible = "qcom,pcie-sc8180x", .data = &cfg_1_9_0 },
-	{ .compatible = "qcom,pcie-sc8280xp", .data = &cfg_1_9_0 },
 	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
 	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
-	{ .compatible = "qcom,pcie-sc7280", .data = &cfg_1_9_0 },
+	{ .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, qcom_pcie_match);
-- 
2.35.1

Re: [PATCH 10/10] PCI: qcom: Sort device-id table

[Krzysztof Kozlowski]

On 29/06/2022 16:10, Johan Hovold wrote:
> Sort the device-id table entries alphabetically by compatible string to
> make it easier to find entries and add new ones.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 567601679465..093f4d4bc15d 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1572,23 +1572,23 @@ static int qcom_pcie_remove(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id qcom_pcie_match[] = {
> +	{ .compatible = "qcom,pcie-apq8064", .data = &cfg_2_1_0 },
>  	{ .compatible = "qcom,pcie-apq8084", .data = &cfg_1_0_0 },
>  	{ .compatible = "qcom,pcie-ipq8064", .data = &cfg_2_1_0 },
>  	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
> -	{ .compatible = "qcom,pcie-apq8064", .data = &cfg_2_1_0 },
> -	{ .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
>  	{ .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
>  	{ .compatible = "qcom,pcie-ipq4019", .data = &cfg_2_4_0 },
> -	{ .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
> +	{ .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
>  	{ .compatible = "qcom,pcie-sa8540p", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-sc7280", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-sc8180x", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-sc8280xp", .data = &cfg_1_9_0 },
>  	{ .compatible = "qcom,pcie-sdm845", .data = &cfg_2_7_0 },
>  	{ .compatible = "qcom,pcie-sm8150", .data = &cfg_1_9_0 },
>  	{ .compatible = "qcom,pcie-sm8250", .data = &cfg_1_9_0 },
> -	{ .compatible = "qcom,pcie-sc8180x", .data = &cfg_1_9_0 },
> -	{ .compatible = "qcom,pcie-sc8280xp", .data = &cfg_1_9_0 },
>  	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
>  	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
> -	{ .compatible = "qcom,pcie-sc7280", .data = &cfg_1_9_0 },
> +	{ .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },

Idea is good but it is not sorted alphabetically (not entirely). Q goes
before S.


Best regards,
Krzysztof

Re: [PATCH 10/10] PCI: qcom: Sort device-id table

[Johan Hovold]

On Fri, Jul 01, 2022 at 10:40:52AM +0200, Krzysztof Kozlowski wrote:
> On 29/06/2022 16:10, Johan Hovold wrote:
> > Sort the device-id table entries alphabetically by compatible string to
> > make it easier to find entries and add new ones.
> > 
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > ---
> >  drivers/pci/controller/dwc/pcie-qcom.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 567601679465..093f4d4bc15d 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -1572,23 +1572,23 @@ static int qcom_pcie_remove(struct platform_device *pdev)
> >  }
> >  
> >  static const struct of_device_id qcom_pcie_match[] = {
> > +	{ .compatible = "qcom,pcie-apq8064", .data = &cfg_2_1_0 },
> >  	{ .compatible = "qcom,pcie-apq8084", .data = &cfg_1_0_0 },
> >  	{ .compatible = "qcom,pcie-ipq8064", .data = &cfg_2_1_0 },
> >  	{ .compatible = "qcom,pcie-ipq8064-v2", .data = &cfg_2_1_0 },
> > -	{ .compatible = "qcom,pcie-apq8064", .data = &cfg_2_1_0 },
> > -	{ .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
> >  	{ .compatible = "qcom,pcie-ipq8074", .data = &cfg_2_3_3 },
> >  	{ .compatible = "qcom,pcie-ipq4019", .data = &cfg_2_4_0 },
> > -	{ .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
> > +	{ .compatible = "qcom,pcie-msm8996", .data = &cfg_2_3_2 },
> >  	{ .compatible = "qcom,pcie-sa8540p", .data = &cfg_1_9_0 },
> > +	{ .compatible = "qcom,pcie-sc7280", .data = &cfg_1_9_0 },
> > +	{ .compatible = "qcom,pcie-sc8180x", .data = &cfg_1_9_0 },
> > +	{ .compatible = "qcom,pcie-sc8280xp", .data = &cfg_1_9_0 },
> >  	{ .compatible = "qcom,pcie-sdm845", .data = &cfg_2_7_0 },
> >  	{ .compatible = "qcom,pcie-sm8150", .data = &cfg_1_9_0 },
> >  	{ .compatible = "qcom,pcie-sm8250", .data = &cfg_1_9_0 },
> > -	{ .compatible = "qcom,pcie-sc8180x", .data = &cfg_1_9_0 },
> > -	{ .compatible = "qcom,pcie-sc8280xp", .data = &cfg_1_9_0 },
> >  	{ .compatible = "qcom,pcie-sm8450-pcie0", .data = &cfg_1_9_0 },
> >  	{ .compatible = "qcom,pcie-sm8450-pcie1", .data = &cfg_1_9_0 },
> > -	{ .compatible = "qcom,pcie-sc7280", .data = &cfg_1_9_0 },
> > +	{ .compatible = "qcom,pcie-qcs404", .data = &cfg_2_4_0 },
> 
> Idea is good but it is not sorted alphabetically (not entirely). Q goes
> before S.

Heh. I blame the heat.

Johan