mbox series

[00/19] Qcom PCIe cleanups and improvements

Message ID 20230306153222.157667-1-manivannan.sadhasivam@linaro.org
Headers show
Series Qcom PCIe cleanups and improvements | expand

Message

Manivannan Sadhasivam March 6, 2023, 3:32 p.m. UTC 
Hi,

This series brings in several code cleanups and improvements to the
Qualcomm PCIe controller drivers (RC and EP). The cleanup part mostly
cleans up the bitfield definitions and transitions to bulk APIs for clocks,
and resets. The improvement part renames the "mmio" region to "mhi" as per
Qualcomm's internal documentation EP driver and also adds the debugfs entry
to track link transition count in RC driver.

Testing
-------

I have tested this series on SDM845, SM8250 and SC8280XP based platforms.
However, I'm counting on Qualcomm folks CCed to do testing on older IPQ/APQ
platforms.

Merging Strategy
----------------

Binding and driver patches through PCI tree and DTS patches through Qcom
tree.

NOTE: For the sake of maintaining dependency, I've clubbed both cleanup and
improvement patches in the same series. If any of the maintainers prefer to
have them splitted, please let me know.

Thanks,
Mani


Manivannan Sadhasivam (19):
  PCI: qcom: Remove PCIE20_ prefix from register definitions
  PCI: qcom: Sort and group registers and bitfield definitions
  PCI: qcom: Use bitfield definitions for register fields
  PCI: qcom: Add missing macros for register fields
  PCI: qcom: Use lower case for hex
  PCI: qcom: Use bulk reset APIs for handling resets for IP rev 2.1.0
  PCI: qcom: Use bulk clock APIs for handling clocks for IP rev 1.0.0
  PCI: qcom: Use bulk clock APIs for handling clocks for IP rev 2.3.2
  PCI: qcom: Use bulk clock APIs for handling clocks for IP rev 2.3.3
  PCI: qcom: Use bulk reset APIs for handling resets for IP rev 2.3.3
  PCI: qcom: Use bulk reset APIs for handling resets for IP rev 2.4.0
  PCI: qcom: Use macros for defining total no. of clocks & supplies
  dt-bindings: PCI: qcom-ep: Rename "mmio" region to "mhi"
  PCI: qcom-ep: Rename "mmio" region to "mhi"
  dt-bindings: PCI: qcom: Add "mhi" register region to supported SoCs
  arm64: dts: qcom: sdm845: Add "mhi" region to the PCIe nodes
  arm64: dts: qcom: sm8250: Add "mhi" region to the PCIe nodes
  arm64: dts: qcom: sc8280xp: Add "mhi" region to the PCIe nodes
  PCI: qcom: Expose link transition counts via debugfs

 .../devicetree/bindings/pci/qcom,pcie-ep.yaml |    6 +-
 .../devicetree/bindings/pci/qcom,pcie.yaml    |   12 +-
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi        |   15 +-
 arch/arm64/boot/dts/qcom/sdm845.dtsi          |    6 +-
 arch/arm64/boot/dts/qcom/sm8250.dtsi          |    9 +-
 drivers/pci/controller/dwc/pcie-qcom-ep.c     |   38 +-
 drivers/pci/controller/dwc/pcie-qcom.c        | 1009 ++++++-----------
 7 files changed, 414 insertions(+), 681 deletions(-)

Comments

Johan Hovold March 6, 2023, 4:49 p.m. UTC | #1 
On Mon, Mar 06, 2023 at 09:02:18PM +0530, Manivannan Sadhasivam wrote:
> "mhi" register region contains the MHI registers that could be used by
> the PCIe controller drivers to get debug information like PCIe link
> transition counts on newer SoCs.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index fb32c43dd12d..2de6e7154025 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -44,11 +44,11 @@ properties:
>  
>    reg:
>      minItems: 4
> -    maxItems: 5
> +    maxItems: 6
>  
>    reg-names:
>      minItems: 4
> -    maxItems: 5
> +    maxItems: 6
>  
>    interrupts:
>      minItems: 1
> @@ -185,10 +185,12 @@ allOf:
>        properties:
>          reg:
>            minItems: 4
> -          maxItems: 4
> +          maxItems: 5
>          reg-names:
> +          minItems: 4
>            items:
>              - const: parf # Qualcomm specific registers
> +            - const: mhi # MHI registers

You need to add the new (optional) registers at the end.

>              - const: dbi # DesignWare PCIe registers
>              - const: elbi # External local bus interface registers
>              - const: config # PCIe configuration space

Johan
Rob Herring (Arm) March 6, 2023, 5:13 p.m. UTC | #2 
On Mon, 06 Mar 2023 21:02:18 +0530, Manivannan Sadhasivam wrote:
> "mhi" register region contains the MHI registers that could be used by
> the PCIe controller drivers to get debug information like PCIe link
> transition counts on newer SoCs.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/qcom,pcie.example.dtb: pcie@fc520000: reg-names:1: 'mhi' was expected
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/qcom,pcie.example.dtb: pcie@fc520000: reg-names:2: 'dbi' was expected
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/qcom,pcie.example.dtb: pcie@fc520000: reg-names:3: 'elbi' was expected
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/qcom,pcie.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230306153222.157667-16-manivannan.sadhasivam@linaro.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Krzysztof Kozlowski March 7, 2023, 8:18 a.m. UTC | #3 
On 06/03/2023 16:32, Manivannan Sadhasivam wrote:
> As per Qualcomm's internal documentation, the name of the region is "mhi"
> and not "mmio". So let's rename it to follow the convention.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
> index 89cfdee4b89f..c2d50f42cb4c 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
> @@ -22,7 +22,7 @@ properties:
>        - 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: MHI register region used as BAR
>  
>    reg-names:
>      items:
> @@ -31,7 +31,7 @@ properties:
>        - const: elbi
>        - const: atu
>        - const: addr_space
> -      - const: mmio
> +      - const: mhi

That literally breaks ABI just for convention. I don't think it's right
approach.

Best regards,
Krzysztof
Manivannan Sadhasivam March 8, 2023, 8:32 a.m. UTC | #4 
On Mon, Mar 06, 2023 at 05:49:44PM +0100, Johan Hovold wrote:
> On Mon, Mar 06, 2023 at 09:02:18PM +0530, Manivannan Sadhasivam wrote:
> > "mhi" register region contains the MHI registers that could be used by
> > the PCIe controller drivers to get debug information like PCIe link
> > transition counts on newer SoCs.
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > index fb32c43dd12d..2de6e7154025 100644
> > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > @@ -44,11 +44,11 @@ properties:
> >  
> >    reg:
> >      minItems: 4
> > -    maxItems: 5
> > +    maxItems: 6
> >  
> >    reg-names:
> >      minItems: 4
> > -    maxItems: 5
> > +    maxItems: 6
> >  
> >    interrupts:
> >      minItems: 1
> > @@ -185,10 +185,12 @@ allOf:
> >        properties:
> >          reg:
> >            minItems: 4
> > -          maxItems: 4
> > +          maxItems: 5
> >          reg-names:
> > +          minItems: 4
> >            items:
> >              - const: parf # Qualcomm specific registers
> > +            - const: mhi # MHI registers
> 
> You need to add the new (optional) registers at the end.
> 

Will do it in next revision.

Thanks,
Mani

> >              - const: dbi # DesignWare PCIe registers
> >              - const: elbi # External local bus interface registers
> >              - const: config # PCIe configuration space
> 
> Johan