diff mbox series

[4/9] dt-bindings: phy: qcom,uniphy: Add ipq5332 USB3 SS UNIPHY

Message ID 20230829135818.2219438-5-quic_ipkumar@quicinc.com
State Changes Requested
Headers show
Series Enable USB3 for IPQ5332 | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dt-meta-schema fail build log

Commit Message

Praveenkumar I Aug. 29, 2023, 1:58 p.m. UTC
Add ipq5332 USB3 SS UNIPHY support.

Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
---
 .../devicetree/bindings/phy/qcom,uniphy.yaml  | 117 +++++++++++++++++-
 1 file changed, 114 insertions(+), 3 deletions(-)

Comments

Dmitry Baryshkov Aug. 29, 2023, 2:16 p.m. UTC | #1
On Tue, 29 Aug 2023 at 17:00, Praveenkumar I <quic_ipkumar@quicinc.com> wrote:
>
> Add ipq5332 USB3 SS UNIPHY support.
>
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
>  .../devicetree/bindings/phy/qcom,uniphy.yaml  | 117 +++++++++++++++++-
>  1 file changed, 114 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> index cbe2cc820009..17ba661b3d9b 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> @@ -19,21 +19,53 @@ properties:
>      enum:
>        - qcom,usb-ss-ipq4019-phy
>        - qcom,usb-hs-ipq4019-phy
> +      - qcom,ipq5332-usb-ssphy
>
>    reg:
>      maxItems: 1
>
> +  reg-names:
> +    items:
> +      - const: phy_base
> +
> +  clocks:
> +    maxItems: 3
> +
> +  clock-names:
> +    maxItems: 3
> +
> +  "#clock-cells":
> +    const: 0
> +
>    resets:
> +    minItems: 1
>      maxItems: 2
>
>    reset-names:
> -    items:
> -      - const: por_rst
> -      - const: srif_rst
> +    minItems: 1
> +    maxItems: 2
> +
> +  clock-output-names:
> +    maxItems: 1
>
>    "#phy-cells":
>      const: 0
>
> +  qcom,phy-mux-sel:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description:
> +      PHY Mux Selection for used to select which interface is going to use the
> +      combo PHY.
> +    items:
> +      - items:
> +          - description: phandle to TCSR syscon region
> +          - description: offset to the PHY Mux selection register
> +          - description: value to write on the PHY Mux selection register

Generally these values should be a part of the driver, since they are
specific to the particular SoC, rather than being different from
device to device.

> +
> +  vdd-supply:
> +    description:
> +      Phandle to 5V regulator supply to PHY digital circuit.
> +
>  required:
>    - compatible
>    - reg
> @@ -41,6 +73,68 @@ required:
>    - reset-names
>    - "#phy-cells"
>
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,ipq5332-usb-ssphy
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 3
> +        clock-names:
> +          items:
> +            - const: pipe
> +            - const: phy_cfg_ahb
> +            - const: phy_ahb
> +
> +        "#clock-cells":
> +          const: 0
> +
> +        clock-output-names:
> +          maxItems: 1
> +
> +        resets:
> +          maxItems: 1
> +        reset-names:
> +          items:
> +            - const: por_rst
> +
> +        vdda-supply:
> +          description:
> +            Phandle to 5V regulator supply to PHY digital circuit.
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,usb-ss-ipq4019-phy
> +    then:
> +      properties:
> +        resets:
> +          maxItems: 1
> +        reset-names:
> +          items:
> +            - const: por_rst
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,usb-hs-ipq4019-phy
> +    then:
> +      properties:
> +        resets:
> +          maxItems: 2
> +        reset-names:
> +          items:
> +            - const: por_rst
> +            - const: srif_rst
> +
>  additionalProperties: false
>
>  examples:
> @@ -55,3 +149,20 @@ examples:
>                 <&gcc USB2_HSPHY_S_ARES>;
>        reset-names = "por_rst", "srif_rst";
>      };
> +
> +  - |
> +    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> +
> +    ssuniphy@4b0000 {
> +      #phy-cells = <0>;
> +      #clock-cells = <0>;
> +      compatible = "qcom,ipq5332-usb-ssphy";
> +      reg = <0x4b0000 0x800>;
> +      clocks = <&gcc GCC_USB0_PIPE_CLK>,
> +               <&gcc GCC_USB0_PHY_CFG_AHB_CLK>,
> +               <&gcc GCC_PCIE3X1_PHY_AHB_CLK>;
> +      clock-names = "pipe", "phy_cfg_ahb", "phy_ahb";
> +
> +      resets = <&gcc GCC_USB0_PHY_BCR>;
> +      reset-names = "por_rst";
> +    };
> --
> 2.34.1
>
Rob Herring Aug. 29, 2023, 2:35 p.m. UTC | #2
On Tue, 29 Aug 2023 19:28:13 +0530, Praveenkumar I wrote:
> Add ipq5332 USB3 SS UNIPHY support.
> 
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
>  .../devicetree/bindings/phy/qcom,uniphy.yaml  | 117 +++++++++++++++++-
>  1 file changed, 114 insertions(+), 3 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:
In file included from Documentation/devicetree/bindings/phy/qcom,uniphy.example.dts:45:
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:19: warning: "GCC_BLSP1_AHB_CLK" redefined
   19 | #define GCC_BLSP1_AHB_CLK                               10
      | 
In file included from Documentation/devicetree/bindings/phy/qcom,uniphy.example.dts:18:
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:40: note: this is the location of the previous definition
   40 | #define GCC_BLSP1_AHB_CLK                               21
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:20: warning: "GCC_BLSP1_QUP1_I2C_APPS_CLK" redefined
   20 | #define GCC_BLSP1_QUP1_I2C_APPS_CLK                     11
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:41: note: this is the location of the previous definition
   41 | #define GCC_BLSP1_QUP1_I2C_APPS_CLK                     22
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:21: warning: "GCC_BLSP1_QUP1_SPI_APPS_CLK" redefined
   21 | #define GCC_BLSP1_QUP1_SPI_APPS_CLK                     12
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:42: note: this is the location of the previous definition
   42 | #define GCC_BLSP1_QUP1_SPI_APPS_CLK                     23
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:23: warning: "GCC_BLSP1_QUP2_I2C_APPS_CLK" redefined
   23 | #define GCC_BLSP1_QUP2_I2C_APPS_CLK                     14
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:43: note: this is the location of the previous definition
   43 | #define GCC_BLSP1_QUP2_I2C_APPS_CLK                     24
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:24: warning: "GCC_BLSP1_QUP2_SPI_APPS_CLK" redefined
   24 | #define GCC_BLSP1_QUP2_SPI_APPS_CLK                     15
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:44: note: this is the location of the previous definition
   44 | #define GCC_BLSP1_QUP2_SPI_APPS_CLK                     25
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:30: warning: "GCC_BLSP1_UART1_APPS_CLK" redefined
   30 | #define GCC_BLSP1_UART1_APPS_CLK                        21
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:45: note: this is the location of the previous definition
   45 | #define GCC_BLSP1_UART1_APPS_CLK                        26
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:32: warning: "GCC_BLSP1_UART2_APPS_CLK" redefined
   32 | #define GCC_BLSP1_UART2_APPS_CLK                        23
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:46: note: this is the location of the previous definition
   46 | #define GCC_BLSP1_UART2_APPS_CLK                        27
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:42: warning: "GCC_GP1_CLK" redefined
   42 | #define GCC_GP1_CLK                                     33
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:48: note: this is the location of the previous definition
   48 | #define GCC_GP1_CLK                                     29
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:44: warning: "GCC_GP2_CLK" redefined
   44 | #define GCC_GP2_CLK                                     35
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:49: note: this is the location of the previous definition
   49 | #define GCC_GP2_CLK                                     30
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:98: warning: "GCC_PRNG_AHB_CLK" redefined
   98 | #define GCC_PRNG_AHB_CLK                                89
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:62: note: this is the location of the previous definition
   62 | #define GCC_PRNG_AHB_CLK                                43
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:117: warning: "GCC_QPIC_AHB_CLK" redefined
  117 | #define GCC_QPIC_AHB_CLK                                108
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:63: note: this is the location of the previous definition
   63 | #define GCC_QPIC_AHB_CLK                                44
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:118: warning: "GCC_QPIC_CLK" redefined
  118 | #define GCC_QPIC_CLK                                    109
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:64: note: this is the location of the previous definition
   64 | #define GCC_QPIC_CLK                                    45
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:122: warning: "GCC_SDCC1_AHB_CLK" redefined
  122 | #define GCC_SDCC1_AHB_CLK                               113
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:65: note: this is the location of the previous definition
   65 | #define GCC_SDCC1_AHB_CLK                               46
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:123: warning: "GCC_SDCC1_APPS_CLK" redefined
  123 | #define GCC_SDCC1_APPS_CLK                              114
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:66: note: this is the location of the previous definition
   66 | #define GCC_SDCC1_APPS_CLK                              47
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:191: warning: "GCC_BLSP1_BCR" redefined
  191 | #define GCC_BLSP1_BCR                                   8
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:126: note: this is the location of the previous definition
  126 | #define GCC_BLSP1_BCR                                   30
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:192: warning: "GCC_BLSP1_QUP1_BCR" redefined
  192 | #define GCC_BLSP1_QUP1_BCR                              9
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:127: note: this is the location of the previous definition
  127 | #define GCC_BLSP1_QUP1_BCR                              31
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:195: warning: "GCC_BLSP1_QUP2_BCR" redefined
  195 | #define GCC_BLSP1_QUP2_BCR                              12
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:129: note: this is the location of the previous definition
  129 | #define GCC_BLSP1_QUP2_BCR                              33
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:203: warning: "GCC_BLSP1_UART1_BCR" redefined
  203 | #define GCC_BLSP1_UART1_BCR                             20
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:128: note: this is the location of the previous definition
  128 | #define GCC_BLSP1_UART1_BCR                             32
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:205: warning: "GCC_BLSP1_UART2_BCR" redefined
  205 | #define GCC_BLSP1_UART2_BCR                             22
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:130: note: this is the location of the previous definition
  130 | #define GCC_BLSP1_UART2_BCR                             34
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:260: warning: "GCC_PCNOC_BCR" redefined
  260 | #define GCC_PCNOC_BCR                                   77
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:148: note: this is the location of the previous definition
  148 | #define GCC_PCNOC_BCR                                   52
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:263: warning: "GCC_PRNG_BCR" redefined
  263 | #define GCC_PRNG_BCR                                    80
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:135: note: this is the location of the previous definition
  135 | #define GCC_PRNG_BCR                                    39
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:274: warning: "GCC_QDSS_BCR" redefined
  274 | #define GCC_QDSS_BCR                                    91
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:165: note: this is the location of the previous definition
  165 | #define GCC_QDSS_BCR                                    69
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:290: warning: "GCC_QPIC_BCR" redefined
  290 | #define GCC_QPIC_BCR                                    107
      | 
./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,gcc-ipq4019.h:141: note: this is the location of the previous definition
  141 | #define GCC_QPIC_BCR                                    45
      | 

doc reference errors (make refcheckdocs):
Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml
MAINTAINERS: Documentation/devicetree/bindings/phy/qcom-usb-ipq4019-phy.yaml

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230829135818.2219438-5-quic_ipkumar@quicinc.com

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.
Rob Herring Aug. 29, 2023, 4:16 p.m. UTC | #3
On Tue, Aug 29, 2023 at 07:28:13PM +0530, Praveenkumar I wrote:
> Add ipq5332 USB3 SS UNIPHY support.
> 
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
>  .../devicetree/bindings/phy/qcom,uniphy.yaml  | 117 +++++++++++++++++-
>  1 file changed, 114 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> index cbe2cc820009..17ba661b3d9b 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> @@ -19,21 +19,53 @@ properties:
>      enum:
>        - qcom,usb-ss-ipq4019-phy
>        - qcom,usb-hs-ipq4019-phy
> +      - qcom,ipq5332-usb-ssphy
>  
>    reg:
>      maxItems: 1
>  
> +  reg-names:
> +    items:
> +      - const: phy_base
> +
> +  clocks:
> +    maxItems: 3
> +
> +  clock-names:
> +    maxItems: 3
> +
> +  "#clock-cells":
> +    const: 0
> +
>    resets:
> +    minItems: 1
>      maxItems: 2
>  
>    reset-names:
> -    items:
> -      - const: por_rst
> -      - const: srif_rst

No need to remove this and duplicate the names multiple times. Just add 
'minItems: 1' here and then the if/then schemas only need either 
minItems or maxItems.

> +    minItems: 1
> +    maxItems: 2
> +
> +  clock-output-names:
> +    maxItems: 1
>  
>    "#phy-cells":
>      const: 0
>  
> +  qcom,phy-mux-sel:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description:
> +      PHY Mux Selection for used to select which interface is going to use the
> +      combo PHY.
> +    items:
> +      - items:
> +          - description: phandle to TCSR syscon region
> +          - description: offset to the PHY Mux selection register
> +          - description: value to write on the PHY Mux selection register
> +
> +  vdd-supply:
> +    description:
> +      Phandle to 5V regulator supply to PHY digital circuit.
> +
>  required:
>    - compatible
>    - reg
> @@ -41,6 +73,68 @@ required:
>    - reset-names
>    - "#phy-cells"
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,ipq5332-usb-ssphy
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 3
> +        clock-names:
> +          items:
> +            - const: pipe
> +            - const: phy_cfg_ahb
> +            - const: phy_ahb

How do the other variants work without any clocks? Magic?

Define the names in the top level and then just set the number of items 
or disallow the property in the if/then schemas.

> +
> +        "#clock-cells":
> +          const: 0
> +
> +        clock-output-names:
> +          maxItems: 1
> +
> +        resets:
> +          maxItems: 1
> +        reset-names:
> +          items:
> +            - const: por_rst
> +
> +        vdda-supply:
> +          description:
> +            Phandle to 5V regulator supply to PHY digital circuit.
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,usb-ss-ipq4019-phy
> +    then:
> +      properties:
> +        resets:
> +          maxItems: 1
> +        reset-names:
> +          items:
> +            - const: por_rst
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,usb-hs-ipq4019-phy
> +    then:
> +      properties:
> +        resets:
> +          maxItems: 2
> +        reset-names:
> +          items:
> +            - const: por_rst
> +            - const: srif_rst
> +
>  additionalProperties: false
>  
>  examples:
> @@ -55,3 +149,20 @@ examples:
>                 <&gcc USB2_HSPHY_S_ARES>;
>        reset-names = "por_rst", "srif_rst";
>      };
> +
> +  - |
> +    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> +
> +    ssuniphy@4b0000 {
> +      #phy-cells = <0>;
> +      #clock-cells = <0>;
> +      compatible = "qcom,ipq5332-usb-ssphy";
> +      reg = <0x4b0000 0x800>;
> +      clocks = <&gcc GCC_USB0_PIPE_CLK>,
> +               <&gcc GCC_USB0_PHY_CFG_AHB_CLK>,
> +               <&gcc GCC_PCIE3X1_PHY_AHB_CLK>;
> +      clock-names = "pipe", "phy_cfg_ahb", "phy_ahb";
> +
> +      resets = <&gcc GCC_USB0_PHY_BCR>;
> +      reset-names = "por_rst";
> +    };
> -- 
> 2.34.1
>
Krzysztof Kozlowski Aug. 29, 2023, 4:59 p.m. UTC | #4
On 29/08/2023 15:58, Praveenkumar I wrote:
> Add ipq5332 USB3 SS UNIPHY support.
> 
> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> ---
>  .../devicetree/bindings/phy/qcom,uniphy.yaml  | 117 +++++++++++++++++-
>  1 file changed, 114 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> index cbe2cc820009..17ba661b3d9b 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
> @@ -19,21 +19,53 @@ properties:
>      enum:
>        - qcom,usb-ss-ipq4019-phy
>        - qcom,usb-hs-ipq4019-phy
> +      - qcom,ipq5332-usb-ssphy
>  
>    reg:
>      maxItems: 1
>  
> +  reg-names:
> +    items:
> +      - const: phy_base

Why do you need it?

> +
> +  clocks:
> +    maxItems: 3
> +
> +  clock-names:
> +    maxItems: 3
> +
> +  "#clock-cells":
> +    const: 0
> +
...
>  examples:
> @@ -55,3 +149,20 @@ examples:
>                 <&gcc USB2_HSPHY_S_ARES>;
>        reset-names = "por_rst", "srif_rst";
>      };
> +
> +  - |
> +    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> +
> +    ssuniphy@4b0000 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation



Best regards,
Krzysztof
Krzysztof Kozlowski Aug. 29, 2023, 5:08 p.m. UTC | #5
On 29/08/2023 16:35, Rob Herring wrote:
> 
> On Tue, 29 Aug 2023 19:28:13 +0530, Praveenkumar I wrote:
>> Add ipq5332 USB3 SS UNIPHY support.
>>
>> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
>> ---
>>  .../devicetree/bindings/phy/qcom,uniphy.yaml  | 117 +++++++++++++++++-
>>  1 file changed, 114 insertions(+), 3 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:
> In file included from Documentation/devicetree/bindings/phy/qcom,uniphy.example.dts:45:
> ./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:19: warning: "GCC_BLSP1_AHB_CLK" redefined
>    19 | #define GCC_BLSP1_AHB_CLK                               10
>       | 

So the only patch which actually needed dependency information did not
have it. All other patches have something, even defconfig (!). Confusing.

Best regards,
Krzysztof
Dmitry Baryshkov Aug. 29, 2023, 6:46 p.m. UTC | #6
On Tue, 29 Aug 2023 at 20:09, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 29/08/2023 16:35, Rob Herring wrote:
> >
> > On Tue, 29 Aug 2023 19:28:13 +0530, Praveenkumar I wrote:
> >> Add ipq5332 USB3 SS UNIPHY support.
> >>
> >> Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> >> ---
> >>  .../devicetree/bindings/phy/qcom,uniphy.yaml  | 117 +++++++++++++++++-
> >>  1 file changed, 114 insertions(+), 3 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:
> > In file included from Documentation/devicetree/bindings/phy/qcom,uniphy.example.dts:45:
> > ./scripts/dtc/include-prefixes/dt-bindings/clock/qcom,ipq5332-gcc.h:19: warning: "GCC_BLSP1_AHB_CLK" redefined
> >    19 | #define GCC_BLSP1_AHB_CLK                               10
> >       |
>
> So the only patch which actually needed dependency information did not
> have it. All other patches have something, even defconfig (!). Confusing.

Much simpler. This patch adds a second example to the schema. Both
examples include something-gcc.h. As both examples end up in the same
example.dts file, second include conflicts with the first one.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
index cbe2cc820009..17ba661b3d9b 100644
--- a/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,uniphy.yaml
@@ -19,21 +19,53 @@  properties:
     enum:
       - qcom,usb-ss-ipq4019-phy
       - qcom,usb-hs-ipq4019-phy
+      - qcom,ipq5332-usb-ssphy
 
   reg:
     maxItems: 1
 
+  reg-names:
+    items:
+      - const: phy_base
+
+  clocks:
+    maxItems: 3
+
+  clock-names:
+    maxItems: 3
+
+  "#clock-cells":
+    const: 0
+
   resets:
+    minItems: 1
     maxItems: 2
 
   reset-names:
-    items:
-      - const: por_rst
-      - const: srif_rst
+    minItems: 1
+    maxItems: 2
+
+  clock-output-names:
+    maxItems: 1
 
   "#phy-cells":
     const: 0
 
+  qcom,phy-mux-sel:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      PHY Mux Selection for used to select which interface is going to use the
+      combo PHY.
+    items:
+      - items:
+          - description: phandle to TCSR syscon region
+          - description: offset to the PHY Mux selection register
+          - description: value to write on the PHY Mux selection register
+
+  vdd-supply:
+    description:
+      Phandle to 5V regulator supply to PHY digital circuit.
+
 required:
   - compatible
   - reg
@@ -41,6 +73,68 @@  required:
   - reset-names
   - "#phy-cells"
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,ipq5332-usb-ssphy
+    then:
+      properties:
+        clocks:
+          maxItems: 3
+        clock-names:
+          items:
+            - const: pipe
+            - const: phy_cfg_ahb
+            - const: phy_ahb
+
+        "#clock-cells":
+          const: 0
+
+        clock-output-names:
+          maxItems: 1
+
+        resets:
+          maxItems: 1
+        reset-names:
+          items:
+            - const: por_rst
+
+        vdda-supply:
+          description:
+            Phandle to 5V regulator supply to PHY digital circuit.
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,usb-ss-ipq4019-phy
+    then:
+      properties:
+        resets:
+          maxItems: 1
+        reset-names:
+          items:
+            - const: por_rst
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,usb-hs-ipq4019-phy
+    then:
+      properties:
+        resets:
+          maxItems: 2
+        reset-names:
+          items:
+            - const: por_rst
+            - const: srif_rst
+
 additionalProperties: false
 
 examples:
@@ -55,3 +149,20 @@  examples:
                <&gcc USB2_HSPHY_S_ARES>;
       reset-names = "por_rst", "srif_rst";
     };
+
+  - |
+    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
+
+    ssuniphy@4b0000 {
+      #phy-cells = <0>;
+      #clock-cells = <0>;
+      compatible = "qcom,ipq5332-usb-ssphy";
+      reg = <0x4b0000 0x800>;
+      clocks = <&gcc GCC_USB0_PIPE_CLK>,
+               <&gcc GCC_USB0_PHY_CFG_AHB_CLK>,
+               <&gcc GCC_PCIE3X1_PHY_AHB_CLK>;
+      clock-names = "pipe", "phy_cfg_ahb", "phy_ahb";
+
+      resets = <&gcc GCC_USB0_PHY_BCR>;
+      reset-names = "por_rst";
+    };