mbox series

[00/41] phy: qcom-qmp: convert to newer style of bindings

Message ID 20230324022514.1800382-1-dmitry.baryshkov@linaro.org
Headers show
Series phy: qcom-qmp: convert to newer style of bindings | expand

Message

Dmitry Baryshkov March 24, 2023, 2:24 a.m. UTC
Reviewing several patchsets for newer platforms made me understand that
having two styles of QMP PHY bindings causes confusion. Despite binding
documents having notes telling that old bindings should be used for
older platforms, it is too easy to attempt adding new platform with
older QMP PHY binding. Thus let's have just a single documented style of
bindings.

To facilitate this, migrate all the bindings, extend QMP PHY drivers
with offset tables and update DTS files.

Dependencies: [1], [2], [3]:

[1] https://lore.kernel.org/linux-arm-msm/20230323144726.1614344-1-dmitry.baryshkov@linaro.org
[2] https://lore.kernel.org/linux-arm-msm/20230324021651.1799969-1-dmitry.baryshkov@linaro.org
[3] https://lore.kernel.org/linux-arm-msm/20230324001752.1768505-1-dmitry.baryshkov@linaro.org


Dmitry Baryshkov (41):
  dt-bindings: phy: migrate QMP USB PHY bindings to
    qcom,sc8280xp-qmp-usb3-uni-phy.yaml
  dt-bindings: phy: migrate combo QMP PHY bindings to
    qcom,sc8280xp-qmp-usb43dp-phy.yaml
  dt-bindings: phy: migrate QMP UFS PHY bindings to
    qcom,sc8280xp-qmp-ufs-phy.yaml
  dt-bindings: phy: migrate QMP PCIe PHY bindings to
    qcom,sc8280xp-qmp-pcie-phy.yaml
  phy: qcom-qmp-usb: make QPHY_PCS_MISC_CLAMP_ENABLE access conditional
  phy: qcom-qmp: move PCS MISC V4 registers to separate header
  phy: qcom-qmp-usb: populate offsets configuration
  phy: qcom-qmp-ufs: populate offsets configuration
  phy: qcom-qmp-pcie: populate offsets configuration
  arm64: dts: qcom: ipq6018: switch USB QMP PHY to new style of bindings
  arm64: dts: qcom: ipq8074: switch USB QMP PHY to new style of bindings
  arm64: dts: qcom: msm8996: switch USB QMP PHY to new style of bindings
  arm64: dts: qcom: msm8998: switch USB QMP PHY to new style of bindings
  arm64: dts: qcom: sdm845: switch USB QMP PHY to new style of bindings
  arm64: dts: qcom: sm8150: switch USB QMP PHY to new style of bindings
  arm64: dts: qcom: sm8250: switch USB QMP PHY to new style of bindings
  arm64: dts: qcom: sm8350: switch USB QMP PHY to new style of bindings
  arm64: dts: qcom: sc7180: switch USB+DP QMP PHY to new style of
    bindings
  arm64: dts: qcom: sc7280: switch USB+DP QMP PHY to new style of
    bindings
  arm64: dts: qcom: sdm845: switch USB+DP QMP PHY to new style of
    bindings
  arm64: dts: qcom: sm8250: switch USB+DP QMP PHY to new style of
    bindings
  arm64: dts: qcom: msm8996: switch UFS QMP PHY to new style of bindings
  arm64: dts: qcom: msm8998: switch UFS QMP PHY to new style of bindings
  arm64: dts: qcom: sdm845: switch UFS QMP PHY to new style of bindings
  arm64: dts: qcom: sm6115: switch UFS QMP PHY to new style of bindings
  arm64: dts: qcom: sm6350: switch UFS QMP PHY to new style of bindings
  arm64: dts: qcom: sm8150: switch UFS QMP PHY to new style of bindings
  arm64: dts: qcom: sm8250: switch UFS QMP PHY to new style of bindings
  arm64: dts: qcom: sm8350: switch UFS QMP PHY to new style of bindings
  arm64: dts: qcom: sm8450: switch UFS QMP PHY to new style of bindings
  arm64: dts: qcom: ipq6018: switch PCIe QMP PHY to new style of
    bindings
  arm64: dts: qcom: ipq8074: switch PCIe QMP PHY to new style of
    bindings
  arm64: dts: qcom: msm8998: switch PCIe QMP PHY to new style of
    bindings
  arm64: dts: qcom: sc7280: switch PCIe QMP PHY to new style of bindings
  arm64: dts: qcom: sdm845: switch PCIe QMP PHY to new style of bindings
  arm64: dts: qcom: sm8150: switch PCIe QMP PHY to new style of bindings
  arm64: dts: qcom: sm8250: switch PCIe QMP PHY to new style of bindings
  arm64: dts: qcom: sm8450: switch PCIe QMP PHY to new style of bindings
  ARM: dts: qcom-sdx55: switch USB QMP PHY to new style of bindings
  ARM: dts: qcom-sdx65: switch USB QMP PHY to new style of bindings
  ARM: dts: qcom-sdx55: switch PCIe QMP PHY to new style of bindings

 .../phy/qcom,ipq8074-qmp-pcie-phy.yaml        | 299 -------------
 .../phy/qcom,msm8996-qmp-ufs-phy.yaml         | 244 -----------
 .../phy/qcom,msm8996-qmp-usb3-phy.yaml        | 394 ------------------
 .../phy/qcom,sc7180-qmp-usb3-dp-phy.yaml      | 276 ------------
 .../phy/qcom,sc8280xp-qmp-pcie-phy.yaml       | 213 ++++++++--
 .../phy/qcom,sc8280xp-qmp-ufs-phy.yaml        |  94 ++++-
 .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml   | 236 ++++++++++-
 .../phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml    | 124 +++++-
 arch/arm/boot/dts/qcom-sdx55.dtsi             |  57 +--
 arch/arm/boot/dts/qcom-sdx65.dtsi             |  29 +-
 arch/arm64/boot/dts/qcom/ipq6018.dtsi         |  63 ++-
 arch/arm64/boot/dts/qcom/ipq8074.dtsi         | 123 +++---
 arch/arm64/boot/dts/qcom/msm8996.dtsi         |  57 +--
 arch/arm64/boot/dts/qcom/msm8998.dtsi         |  77 ++--
 arch/arm64/boot/dts/qcom/sc7180.dtsi          |  55 +--
 arch/arm64/boot/dts/qcom/sc7280.dtsi          |  90 ++--
 arch/arm64/boot/dts/qcom/sdm845.dtsi          | 174 +++-----
 arch/arm64/boot/dts/qcom/sm6115.dtsi          |  17 +-
 arch/arm64/boot/dts/qcom/sm6350.dtsi          |  18 +-
 arch/arm64/boot/dts/qcom/sm8150.dtsi          | 153 +++----
 arch/arm64/boot/dts/qcom/sm8250.dtsi          | 211 ++++------
 arch/arm64/boot/dts/qcom/sm8350.dtsi          |  60 +--
 arch/arm64/boot/dts/qcom/sm8450.dtsi          | 110 ++---
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c      |  84 ++++
 .../phy/qualcomm/phy-qcom-qmp-pcs-misc-v4.h   |  17 +
 drivers/phy/qualcomm/phy-qcom-qmp-ufs.c       |  10 +
 drivers/phy/qualcomm/phy-qcom-qmp-usb.c       | 122 +++++-
 drivers/phy/qualcomm/phy-qcom-qmp.h           |   8 -
 28 files changed, 1316 insertions(+), 2099 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml
 delete mode 100644 Documentation/devicetree/bindings/phy/qcom,msm8996-qmp-ufs-phy.yaml
 delete mode 100644 Documentation/devicetree/bindings/phy/qcom,msm8996-qmp-usb3-phy.yaml
 delete mode 100644 Documentation/devicetree/bindings/phy/qcom,sc7180-qmp-usb3-dp-phy.yaml
 create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-misc-v4.h

Comments

Johan Hovold March 24, 2023, 7:48 a.m. UTC | #1
On Fri, Mar 24, 2023 at 05:24:34AM +0300, Dmitry Baryshkov wrote:
> Migrate legacy bindings (described in qcom,msm8996-qmp-usb3-phy.yaml)
> to qcom,sc8280xp-qmp-usb3-uni-phy.yaml. This removes a need to declare
> the child PHY node or split resource regions.

This needs to be done more care, rather than just dumping the old mess
we have in the new schema file.

Same comment for the other conversions.

NAK for the whole series for now.

> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../phy/qcom,msm8996-qmp-usb3-phy.yaml        | 394 ------------------
>  .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml   | 236 ++++++++++-
>  2 files changed, 226 insertions(+), 404 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/phy/qcom,msm8996-qmp-usb3-phy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,msm8996-qmp-usb3-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,msm8996-qmp-usb3-phy.yaml
> deleted file mode 100644
> index e81a38281f8c..000000000000
> --- a/Documentation/devicetree/bindings/phy/qcom,msm8996-qmp-usb3-phy.yaml
> +++ /dev/null
> @@ -1,394 +0,0 @@
> -# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> -%YAML 1.2
> ----
> -$id: http://devicetree.org/schemas/phy/qcom,msm8996-qmp-usb3-phy.yaml#
> -$schema: http://devicetree.org/meta-schemas/core.yaml#
> -
> -title: Qualcomm QMP PHY controller (USB, MSM8996)
> -
> -maintainers:
> -  - Vinod Koul <vkoul@kernel.org>
> -
> -description:
> -  QMP PHY controller supports physical layer functionality for a number of
> -  controllers on Qualcomm chipsets, such as, PCIe, UFS, and USB.
> -
> -  Note that these bindings are for SoCs up to SC8180X. For newer SoCs, see
> -  qcom,sc8280xp-qmp-usb3-uni-phy.yaml.
> -
> -properties:
> -  compatible:
> -    enum:
> -      - qcom,ipq6018-qmp-usb3-phy
> -      - qcom,ipq8074-qmp-usb3-phy
> -      - qcom,msm8996-qmp-usb3-phy
> -      - qcom,msm8998-qmp-usb3-phy
> -      - qcom,qcm2290-qmp-usb3-phy
> -      - qcom,sc7180-qmp-usb3-phy
> -      - qcom,sc8180x-qmp-usb3-phy
> -      - qcom,sdm845-qmp-usb3-phy
> -      - qcom,sdm845-qmp-usb3-uni-phy
> -      - qcom,sdx55-qmp-usb3-uni-phy
> -      - qcom,sdx65-qmp-usb3-uni-phy
> -      - qcom,sm6115-qmp-usb3-phy
> -      - qcom,sm8150-qmp-usb3-phy
> -      - qcom,sm8150-qmp-usb3-uni-phy
> -      - qcom,sm8250-qmp-usb3-phy
> -      - qcom,sm8250-qmp-usb3-uni-phy
> -      - qcom,sm8350-qmp-usb3-phy
> -      - qcom,sm8350-qmp-usb3-uni-phy
> -      - qcom,sm8450-qmp-usb3-phy
> -
> -  reg:
> -    minItems: 1
> -    items:
> -      - description: serdes
> -      - description: DP_COM
> -
> -  "#address-cells":
> -    enum: [ 1, 2 ]
> -
> -  "#size-cells":
> -    enum: [ 1, 2 ]
> -
> -  ranges: true
> -
> -  clocks:
> -    minItems: 3
> -    maxItems: 4
> -
> -  clock-names:
> -    minItems: 3
> -    maxItems: 4
> -
> -  power-domains:
> -    maxItems: 1
> -
> -  resets:
> -    maxItems: 2
> -
> -  reset-names:
> -    maxItems: 2
> -
> -  vdda-phy-supply: true
> -
> -  vdda-pll-supply: true
> -
> -  vddp-ref-clk-supply: true
> -
> -patternProperties:
> -  "^phy@[0-9a-f]+$":
> -    type: object
> -    description: single PHY-provider child node
> -    properties:
> -      reg:
> -        minItems: 3
> -        maxItems: 6
> -
> -      clocks:
> -        items:
> -          - description: PIPE clock
> -
> -      clock-names:
> -        deprecated: true
> -        items:
> -          - const: pipe0
> -
> -      "#clock-cells":
> -        const: 0
> -
> -      clock-output-names:
> -        maxItems: 1
> -
> -      "#phy-cells":
> -        const: 0
> -
> -    required:
> -      - reg
> -      - clocks
> -      - "#clock-cells"
> -      - clock-output-names
> -      - "#phy-cells"
> -
> -    additionalProperties: false
> -
> -required:
> -  - compatible
> -  - reg
> -  - "#address-cells"
> -  - "#size-cells"
> -  - ranges
> -  - clocks
> -  - clock-names
> -  - resets
> -  - reset-names
> -  - vdda-phy-supply
> -  - vdda-pll-supply
> -
> -additionalProperties: false
> -
> -allOf:
> -  - if:
> -      properties:
> -        compatible:
> -          contains:
> -            enum:
> -              - qcom,sc7180-qmp-usb3-phy
> -    then:
> -      properties:
> -        clocks:
> -          maxItems: 4
> -        clock-names:
> -          items:
> -            - const: aux
> -            - const: cfg_ahb
> -            - const: ref
> -            - const: com_aux
> -        resets:
> -          maxItems: 1
> -        reset-names:
> -          items:
> -            - const: phy
> -
> -  - if:
> -      properties:
> -        compatible:
> -          contains:
> -            enum:
> -              - qcom,sdm845-qmp-usb3-uni-phy
> -    then:
> -      properties:
> -        clocks:
> -          maxItems: 4
> -        clock-names:
> -          items:
> -            - const: aux
> -            - const: cfg_ahb
> -            - const: ref
> -            - const: com_aux
> -        resets:
> -          maxItems: 2
> -        reset-names:
> -          items:
> -            - const: phy
> -            - const: common
> -
> -  - if:
> -      properties:
> -        compatible:
> -          contains:
> -            enum:
> -              - qcom,ipq8074-qmp-usb3-phy
> -              - qcom,msm8996-qmp-usb3-phy
> -              - qcom,msm8998-qmp-usb3-phy
> -              - qcom,sdx55-qmp-usb3-uni-phy
> -              - qcom,sdx65-qmp-usb3-uni-phy
> -    then:
> -      properties:
> -        clocks:
> -          maxItems: 3
> -        clock-names:
> -          items:
> -            - const: aux
> -            - const: cfg_ahb
> -            - const: ref
> -        resets:
> -          maxItems: 2
> -        reset-names:
> -          items:
> -            - const: phy
> -            - const: common
> -
> -  - if:
> -      properties:
> -        compatible:
> -          contains:
> -            enum:
> -              - qcom,sm8150-qmp-usb3-phy
> -              - qcom,sm8150-qmp-usb3-uni-phy
> -              - qcom,sm8250-qmp-usb3-uni-phy
> -              - qcom,sm8350-qmp-usb3-uni-phy
> -    then:
> -      properties:
> -        clocks:
> -          maxItems: 4
> -        clock-names:
> -          items:
> -            - const: aux
> -            - const: ref_clk_src
> -            - const: ref
> -            - const: com_aux
> -        resets:
> -          maxItems: 2
> -        reset-names:
> -          items:
> -            - const: phy
> -            - const: common
> -
> -  - if:
> -      properties:
> -        compatible:
> -          contains:
> -            enum:
> -              - qcom,sm8250-qmp-usb3-phy
> -              - qcom,sm8350-qmp-usb3-phy
> -    then:
> -      properties:
> -        clocks:
> -          maxItems: 3
> -        clock-names:
> -          items:
> -            - const: aux
> -            - const: ref_clk_src
> -            - const: com_aux
> -        resets:
> -          maxItems: 2
> -        reset-names:
> -          items:
> -            - const: phy
> -            - const: common
> -
> -  - if:
> -      properties:
> -        compatible:
> -          contains:
> -            enum:
> -              - qcom,qcm2290-qmp-usb3-phy
> -              - qcom,sm6115-qmp-usb3-phy
> -    then:
> -      properties:
> -        clocks:
> -          maxItems: 3
> -        clock-names:
> -          items:
> -            - const: cfg_ahb
> -            - const: ref
> -            - const: com_aux
> -        resets:
> -          maxItems: 2
> -        reset-names:
> -          items:
> -            - const: phy_phy
> -            - const: phy
> -
> -  - if:
> -      properties:
> -        compatible:
> -          contains:
> -            enum:
> -              - qcom,sdm845-qmp-usb3-phy
> -              - qcom,sm8150-qmp-usb3-phy
> -              - qcom,sm8350-qmp-usb3-phy
> -              - qcom,sm8450-qmp-usb3-phy
> -    then:
> -      patternProperties:
> -        "^phy@[0-9a-f]+$":
> -          properties:
> -            reg:
> -              items:
> -                - description: TX lane 1
> -                - description: RX lane 1
> -                - description: PCS
> -                - description: TX lane 2
> -                - description: RX lane 2
> -                - description: PCS_MISC
> -
> -  - if:
> -      properties:
> -        compatible:
> -          contains:
> -            enum:
> -              - qcom,msm8998-qmp-usb3-phy
> -    then:
> -      patternProperties:
> -        "^phy@[0-9a-f]+$":
> -          properties:
> -            reg:
> -              items:
> -                - description: TX lane 1
> -                - description: RX lane 1
> -                - description: PCS
> -                - description: TX lane 2
> -                - description: RX lane 2
> -
> -  - if:
> -      properties:
> -        compatible:
> -          contains:
> -            enum:
> -              - qcom,ipq6018-qmp-usb3-phy
> -              - qcom,ipq8074-qmp-usb3-phy
> -              - qcom,qcm2290-qmp-usb3-phy
> -              - qcom,sc7180-qmp-usb3-phy
> -              - qcom,sc8180x-qmp-usb3-phy
> -              - qcom,sdx55-qmp-usb3-uni-phy
> -              - qcom,sdx65-qmp-usb3-uni-phy
> -              - qcom,sm6115-qmp-usb3-phy
> -              - qcom,sm8150-qmp-usb3-uni-phy
> -              - qcom,sm8250-qmp-usb3-phy
> -    then:
> -      patternProperties:
> -        "^phy@[0-9a-f]+$":
> -          properties:
> -            reg:
> -              items:
> -                - description: TX
> -                - description: RX
> -                - description: PCS
> -                - description: PCS_MISC
> -
> -  - if:
> -      properties:
> -        compatible:
> -          contains:
> -            enum:
> -              - qcom,msm8996-qmp-usb3-phy
> -              - qcom,sm8250-qmp-usb3-uni-phy
> -              - qcom,sm8350-qmp-usb3-uni-phy
> -    then:
> -      patternProperties:
> -        "^phy@[0-9a-f]+$":
> -          properties:
> -            reg:
> -              items:
> -                - description: TX
> -                - description: RX
> -                - description: PCS
> -
> -examples:
> -  - |
> -    #include <dt-bindings/clock/qcom,gcc-sdm845.h>
> -    usb_2_qmpphy: phy-wrapper@88eb000 {
> -        compatible = "qcom,sdm845-qmp-usb3-uni-phy";
> -        reg = <0x088eb000 0x18c>;
> -        #address-cells = <1>;
> -        #size-cells = <1>;
> -        ranges = <0x0 0x088eb000 0x2000>;
> -
> -        clocks = <&gcc GCC_USB3_SEC_PHY_AUX_CLK >,
> -                 <&gcc GCC_USB_PHY_CFG_AHB2PHY_CLK>,
> -                 <&gcc GCC_USB3_SEC_CLKREF_CLK>,
> -                 <&gcc GCC_USB3_SEC_PHY_COM_AUX_CLK>;
> -        clock-names = "aux", "cfg_ahb", "ref", "com_aux";
> -
> -        resets = <&gcc GCC_USB3PHY_PHY_SEC_BCR>,
> -                 <&gcc GCC_USB3_PHY_SEC_BCR>;
> -        reset-names = "phy", "common";
> -
> -        vdda-phy-supply = <&vdda_usb2_ss_1p2>;
> -        vdda-pll-supply = <&vdda_usb2_ss_core>;
> -
> -        usb_2_ssphy: phy@200 {
> -                reg = <0x200 0x128>,
> -                      <0x400 0x1fc>,
> -                      <0x800 0x218>,
> -                      <0x600 0x70>;
> -
> -                clocks = <&gcc GCC_USB3_SEC_PHY_PIPE_CLK>;
> -
> -                #clock-cells = <0>;
> -                clock-output-names = "usb3_uni_phy_pipe_clk_src";
> -
> -                #phy-cells = <0>;
> -            };
> -        };
> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
> index 16fce1038285..29a417fb7af1 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
> @@ -16,20 +16,37 @@ description:
>  properties:
>    compatible:
>      enum:
> +      - qcom,ipq6018-qmp-usb3-phy
> +      - qcom,ipq8074-qmp-usb3-phy
> +      - qcom,msm8996-qmp-usb3-phy
> +      - qcom,msm8998-qmp-usb3-phy
> +      - qcom,qcm2290-qmp-usb3-phy
> +      - qcom,sc7180-qmp-usb3-phy
> +      - qcom,sc8180x-qmp-usb3-phy
>        - qcom,sc8280xp-qmp-usb3-uni-phy
> +      - qcom,sdm845-qmp-usb3-phy
> +      - qcom,sdm845-qmp-usb3-uni-phy
> +      - qcom,sdx55-qmp-usb3-uni-phy
> +      - qcom,sdx65-qmp-usb3-uni-phy
> +      - qcom,sm6115-qmp-usb3-phy
> +      - qcom,sm8150-qmp-usb3-phy
> +      - qcom,sm8150-qmp-usb3-uni-phy
> +      - qcom,sm8250-qmp-usb3-phy
> +      - qcom,sm8250-qmp-usb3-uni-phy
> +      - qcom,sm8350-qmp-usb3-phy
> +      - qcom,sm8350-qmp-usb3-uni-phy
> +      - qcom,sm8450-qmp-usb3-phy
>  
>    reg:
>      maxItems: 1
>  
>    clocks:
> -    maxItems: 4
> +    minItems: 4
> +    maxItems: 5
>  
>    clock-names:
> -    items:
> -      - const: aux
> -      - const: ref
> -      - const: com_aux
> -      - const: pipe
> +    minItems: 4
> +    maxItems: 5
>  
>    power-domains:
>      maxItems: 1
> @@ -38,9 +55,7 @@ properties:
>      maxItems: 2
>  
>    reset-names:
> -    items:
> -      - const: phy
> -      - const: phy_phy
> +    maxItems: 2
>  
>    vdda-phy-supply: true
>  
> @@ -60,7 +75,6 @@ required:
>    - reg
>    - clocks
>    - clock-names
> -  - power-domains
>    - resets
>    - reset-names
>    - vdda-phy-supply
> @@ -71,6 +85,179 @@ required:
>  
>  additionalProperties: false
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sc7180-qmp-usb3-phy
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 5
> +        clock-names:
> +          items:
> +            - const: aux
> +            - const: cfg_ahb
> +            - const: ref
> +            - const: com_aux
> +            - const: pipe
> +        resets:
> +          maxItems: 1
> +        reset-names:
> +          items:
> +            - const: phy

This is just a subset of the next entrie's resets and could possibly be
merged.

> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sc8280xp-qmp-usb3-phy
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 4
> +        clock-names:
> +          items:
> +            - const: aux
> +            - const: ref
> +            - const: com_aux
> +            - const: pipe
> +        resets:
> +          maxItems: 1
> +        reset-names:
> +          items:
> +            - const: phy
> +            - const: phy_phy
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sdm845-qmp-usb3-uni-phy
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 5
> +        clock-names:
> +          items:
> +            - const: aux
> +            - const: cfg_ahb
> +            - const: ref
> +            - const: com_aux
> +            - const: pipe
> +        resets:
> +          maxItems: 2
> +        reset-names:
> +          items:
> +            - const: phy
> +            - const: common

Is this really a DP-USB phy? Then it does not belong in this schema,
otherwise the phy name looks wrong.

> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,ipq8074-qmp-usb3-phy
> +              - qcom,msm8996-qmp-usb3-phy
> +              - qcom,msm8998-qmp-usb3-phy
> +              - qcom,sdx55-qmp-usb3-uni-phy
> +              - qcom,sdx65-qmp-usb3-uni-phy
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 4
> +        clock-names:
> +          items:
> +            - const: aux
> +            - const: cfg_ahb
> +            - const: ref
> +            - const: pipe
> +        resets:
> +          maxItems: 2
> +        reset-names:
> +          items:
> +            - const: phy
> +            - const: common

Same here.

> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sm8150-qmp-usb3-phy
> +              - qcom,sm8150-qmp-usb3-uni-phy
> +              - qcom,sm8250-qmp-usb3-uni-phy
> +              - qcom,sm8350-qmp-usb3-uni-phy
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 5
> +        clock-names:
> +          items:
> +            - const: aux
> +            - const: ref_clk_src

As we've discussed before, this clock does not belong in the binding and
this should definitely not be reproduced in the new one.

> +            - const: ref
> +            - const: com_aux
> +            - const: pipe
> +        resets:
> +          maxItems: 2
> +        reset-names:
> +          items:
> +            - const: phy
> +            - const: common
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sm8250-qmp-usb3-phy
> +              - qcom,sm8350-qmp-usb3-phy
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 4
> +        clock-names:
> +          items:
> +            - const: aux
> +            - const: ref_clk_src

Same here, was this supposed to be ref?

> +            - const: com_aux
> +            - const: pipe
> +        resets:
> +          maxItems: 2
> +        reset-names:
> +          items:
> +            - const: phy
> +            - const: common

Another combo PHY?

> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,qcm2290-qmp-usb3-phy
> +              - qcom,sm6115-qmp-usb3-phy
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 4
> +        clock-names:
> +          items:
> +            - const: cfg_ahb
> +            - const: ref
> +            - const: com_aux
> +            - const: pipe
> +        resets:
> +          maxItems: 2
> +        reset-names:
> +          items:
> +            - const: phy_phy
> +            - const: phy

You should be able to get rid of most of the above by looking at the
various platforms and recognising that there are just two sets of
clocks, and probably just two sets of resets where one is a subset of
the other.

As you're introducing a new binding this should all be fixed here and
now rather than do another quick hack.

And if you don't have the time and motivation to fix this up now, then
it's better to leave the old half-broken bindings where they are for
now.

> +
>  examples:
>    - |
>      #include <dt-bindings/clock/qcom,gcc-sc8280xp.h>
> @@ -100,3 +287,32 @@ examples:
>  
>        #phy-cells = <0>;
>      };
> +  - |
> +    #define GCC_USB3_SEC_CLKREF_CLK       156
> +    #define GCC_USB_PHY_CFG_AHB2PHY_CLK   161
> +
> +    phy@88eb000 {
> +        compatible = "qcom,sdm845-qmp-usb3-uni-phy";
> +        reg = <0x088eb000 0x18c>;
> +
> +        clocks = <&gcc GCC_USB3_SEC_PHY_AUX_CLK >,
> +                 <&gcc GCC_USB_PHY_CFG_AHB2PHY_CLK>,
> +                 <&gcc GCC_USB3_SEC_CLKREF_CLK>,
> +                 <&gcc GCC_USB3_SEC_PHY_COM_AUX_CLK>,
> +                 <&gcc GCC_USB3_SEC_PHY_PIPE_CLK>;
> +        clock-names = "aux", "cfg_ahb", "ref", "com_aux", "pipe";
> +
> +        resets = <&gcc GCC_USB3PHY_PHY_SEC_BCR>,
> +                 <&gcc GCC_USB3_PHY_SEC_BCR>;
> +        reset-names = "phy", "common";

It looks like these resets should have been named 'phy_phy' and 'phy'
(and order reversed).

> +
> +        vdda-phy-supply = <&vdda_usb2_ss_1p2>;
> +        vdda-pll-supply = <&vdda_usb2_ss_core>;
> +
> +

Stray newline.

> +        #clock-cells = <0>;
> +        clock-output-names = "usb3_uni_phy_pipe_clk_src";
> +
> +        #phy-cells = <0>;
> +    };

But what is the purpose of adding this example? It looks essentially the
same as the current one and is thus redundant.

> +...

Johan
Johan Hovold March 24, 2023, 7:56 a.m. UTC | #2
On Fri, Mar 24, 2023 at 05:24:36AM +0300, Dmitry Baryshkov wrote:
> Migrate legacy bindings (described in qcom,msm8996-qmp-ufs-phy.yaml)
> to qcom,sc8280xp-qmp-ufs-phy.yaml. This removes a need to declare
> the child PHY node or split resource regions.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../phy/qcom,msm8996-qmp-ufs-phy.yaml         | 244 ------------------
>  .../phy/qcom,sc8280xp-qmp-ufs-phy.yaml        |  94 ++++++-
>  2 files changed, 89 insertions(+), 249 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/phy/qcom,msm8996-qmp-ufs-phy.yaml
 
>  examples:
> @@ -84,5 +150,23 @@ examples:
>          vdda-phy-supply = <&vreg_l6b>;
>          vdda-pll-supply = <&vreg_l3b>;
>  
> +        #phy-cells = <0>;
> +    };
> +  - |
> +    #include <dt-bindings/clock/qcom,rpmh.h>
> +
> +    phy@1d87000 {
> +        compatible = "qcom,sm8250-qmp-ufs-phy";
> +        reg = <0x01d87000 0x1c0>;
> +
> +        clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
> +        clock-names = "ref", "ref_aux";
> +
> +        resets = <&ufs_mem_hc 0>;
> +        reset-names = "ufsphy";
> +
> +        vdda-phy-supply = <&vreg_l6b>;
> +        vdda-pll-supply = <&vreg_l3b>;
> +
>          #phy-cells = <0>;
>      };

This example also looks unnecessary.

Johan
Krzysztof Kozlowski March 24, 2023, 9:43 a.m. UTC | #3
On 24/03/2023 03:24, Dmitry Baryshkov wrote:
> Migrate legacy bindings (described in qcom,msm8996-qmp-usb3-phy.yaml)
> to qcom,sc8280xp-qmp-usb3-uni-phy.yaml. This removes a need to declare
> the child PHY node or split resource regions.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../phy/qcom,msm8996-qmp-usb3-phy.yaml        | 394 ------------------
>  .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml   | 236 ++++++++++-
>  2 files changed, 226 insertions(+), 404 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/phy/qcom,msm8996-qmp-usb3-phy.yaml
> 

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC.  It might happen, that command when run on an older
kernel, gives you outdated entries.  Therefore please be sure you base
your patches on recent Linux kernel.

Since you skipped DT list, there will be no tests run, thus this is
unfortunately a NAK.

Best regards,
Krzysztof
Dmitry Baryshkov March 24, 2023, 12:16 p.m. UTC | #4
On Fri, 24 Mar 2023 at 10:04, Johan Hovold <johan@kernel.org> wrote:
>
> On Fri, Mar 24, 2023 at 05:24:37AM +0300, Dmitry Baryshkov wrote:
> > Migrate legacy bindings (described in qcom,ipq8074-qmp-pcie-phy.yaml)
> > to qcom,sc8280xp-qmp-pcie-phy.yaml. This removes a need to declare
> > the child PHY node or split resource regions.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  .../phy/qcom,ipq8074-qmp-pcie-phy.yaml        | 299 ------------------
> >  .../phy/qcom,sc8280xp-qmp-pcie-phy.yaml       | 213 +++++++++++--
> >  2 files changed, 187 insertions(+), 325 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/phy/qcom,ipq8074-qmp-pcie-phy.yaml
>
> > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
> > index ef49efbd0a20..328588448c6b 100644
> > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
> > @@ -16,10 +16,23 @@ description:
> >  properties:
> >    compatible:
> >      enum:
> > +      - qcom,ipq6018-qmp-pcie-phy
> > +      - qcom,ipq8074-qmp-gen3-pcie-phy
> > +      - qcom,ipq8074-qmp-pcie-phy
> > +      - qcom,msm8998-qmp-pcie-phy
> > +      - qcom,sc8180x-qmp-pcie-phy
> >        - qcom,sc8280xp-qmp-gen3x1-pcie-phy
> >        - qcom,sc8280xp-qmp-gen3x2-pcie-phy
> >        - qcom,sc8280xp-qmp-gen3x4-pcie-phy
> > +      - qcom,sdm845-qhp-pcie-phy
> > +      - qcom,sdm845-qmp-pcie-phy
> > +      - qcom,sdx55-qmp-pcie-phy
> > +      - qcom,sm8250-qmp-gen3x1-pcie-phy
> > +      - qcom,sm8250-qmp-gen3x2-pcie-phy
> > +      - qcom,sm8250-qmp-modem-pcie-phy
> >        - qcom,sm8350-qmp-gen3x1-pcie-phy
> > +      - qcom,sm8450-qmp-gen3x1-pcie-phy
> > +      - qcom,sm8450-qmp-gen4x2-pcie-phy
> >        - qcom,sm8550-qmp-gen3x2-pcie-phy
> >        - qcom,sm8550-qmp-gen4x2-pcie-phy
> >
> > @@ -28,18 +41,12 @@ properties:
> >      maxItems: 2
> >
> >    clocks:
> > -    minItems: 5
> > +    minItems: 3
> >      maxItems: 6
> >
> >    clock-names:
> > -    minItems: 5
> > -    items:
> > -      - const: aux
> > -      - const: cfg_ahb
> > -      - const: ref
> > -      - const: rchng
> > -      - const: pipe
> > -      - const: pipediv2
> > +    minItems: 3
> > +    maxItems: 6
> >
> >    power-domains:
> >      maxItems: 1
> > @@ -50,9 +57,7 @@ properties:
> >
> >    reset-names:
> >      minItems: 1
> > -    items:
> > -      - const: phy
> > -      - const: phy_nocsr
> > +    maxItems: 2
> >
> >    vdda-phy-supply: true
> >
> > @@ -83,11 +88,8 @@ required:
> >    - reg
> >    - clocks
> >    - clock-names
> > -  - power-domains
> >    - resets
> >    - reset-names
> > -  - vdda-phy-supply
> > -  - vdda-pll-supply
> >    - "#clock-cells"
> >    - clock-output-names
> >    - "#phy-cells"
> > @@ -119,21 +121,116 @@ allOf:
> >          compatible:
> >            contains:
> >              enum:
> > -              - qcom,sm8350-qmp-gen3x1-pcie-phy
> > -              - qcom,sm8550-qmp-gen3x2-pcie-phy
> > -              - qcom,sm8550-qmp-gen4x2-pcie-phy
> > +              - qcom,msm8998-qmp-pcie-phy
> >      then:
> >        properties:
> >          clocks:
> > -          maxItems: 5
> > +          maxItems: 4
> >          clock-names:
> > +          items:
> > +            - const: aux
> > +            - const: cfg_ahb
> > +            - const: ref
> > +            - const: pipe
> > +        resets:
> > +          maxItems: 2
> > +        reset-names:
> > +          items:
> > +            - const: phy
> > +            - const: common
>
> Reset name looks wrong here too.
>
> > +      required:
> > +        - vdda-phy-supply
> > +        - vdda-pll-supply
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - qcom,ipq6018-qmp-pcie-phy
> > +              - qcom,ipq8074-qmp-gen3-pcie-phy
> > +              - qcom,ipq8074-qmp-pcie-phy
> > +    then:
> > +      properties:
> > +        clocks:
> > +          maxItems: 3
> > +        clock-names:
> > +          items:
> > +            - const: aux
> > +            - const: cfg_ahb
> > +            - const: pipe
> > +        resets:
> > +          maxItems: 2
> > +        reset-names:
> > +          items:
> > +            - const: phy
> > +            - const: common
>
> Same here.
>
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - qcom,sc8180x-qmp-pcie-phy
> > +              - qcom,sdm845-qhp-pcie-phy
> > +              - qcom,sdm845-qmp-pcie-phy
> > +              - qcom,sdx55-qmp-pcie-phy
> > +              - qcom,sm8250-qmp-gen3x1-pcie-phy
> > +              - qcom,sm8250-qmp-gen3x2-pcie-phy
> > +              - qcom,sm8250-qmp-modem-pcie-phy
> > +              - qcom,sm8450-qmp-gen3x1-pcie-phy
> > +              - qcom,sm8450-qmp-gen4x2-pcie-phy
> > +    then:
> > +      properties:
> > +        clocks:
> >            maxItems: 5
> > -    else:
> > +        clock-names:
> > +          items:
> > +            - const: aux
> > +            - const: cfg_ahb
> > +            - const: ref
> > +            - const: refgen
>
> This one should be named 'rchng' and this set a strict subset of the
> sc8280xp clocks.

Ack. Same story as the resets. Let's stop my grumbling and move
clock/reset parsing to legacy vs non-legacy code.

>
> > +            - const: pipe
> > +        resets:
> > +          maxItems: 1
> > +        reset-names:
> > +          items:
> > +            - const: phy
> > +      required:
> > +        - vdda-phy-supply
> > +        - vdda-pll-supply
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - qcom,sm8350-qmp-gen3x1-pcie-phy
> > +              - qcom,sm8550-qmp-gen3x2-pcie-phy
> > +        resets:
> > +          minItems: 1
> > +        reset-names:
> > +          items:
> > +            - const: phy
> > +    then:
> >        properties:
> >          clocks:
> > -          minItems: 6
> > +          maxItems: 5
> >          clock-names:
> > -          minItems: 6
> > +          items:
> > +            - const: aux
> > +            - const: cfg_ahb
> > +            - const: ref
> > +            - const: rchng
> > +            - const: pipe
> > +        resets:
> > +          maxItems: 1
> > +        reset-names:
> > +          items:
> > +            - const: phy
> > +      required:
> > +        - vdda-phy-supply
> > +        - vdda-pll-supply
> >
> >    - if:
> >        properties:
> > @@ -143,16 +240,53 @@ allOf:
> >                - qcom,sm8550-qmp-gen4x2-pcie-phy
> >      then:
> >        properties:
> > +        clocks:
> > +          maxItems: 5
> > +        clock-names:
> > +          items:
> > +            - const: aux
> > +            - const: cfg_ahb
> > +            - const: ref
> > +            - const: rchng
> > +            - const: pipe
> >          resets:
> >            minItems: 2
> >          reset-names:
> > -          minItems: 2
> > -    else:
> > +          items:
> > +            - const: phy
> > +            - const: phy_nocsr
> > +      required:
> > +        - vdda-phy-supply
> > +        - vdda-pll-supply
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - qcom,sc8280xp-qmp-gen3x1-pcie-phy
> > +              - qcom,sc8280xp-qmp-gen3x2-pcie-phy
> > +              - qcom,sc8280xp-qmp-gen3x4-pcie-phy
> > +    then:
> >        properties:
> > +        clocks:
> > +          minItems: 6
> > +        clock-names:
> > +          items:
> > +            - const: aux
> > +            - const: cfg_ahb
> > +            - const: ref
> > +            - const: rchng
> > +            - const: pipe
> > +            - const: pipediv2
> >          resets:
> > -          maxItems: 1
> > +          minItems: 1
> >          reset-names:
> > -          maxItems: 1
> > +          items:
> > +            - const: phy
> > +      required:
> > +        - vdda-phy-supply
> > +        - vdda-pll-supply
> >
> >  examples:
> >    - |
> > @@ -213,3 +347,30 @@ examples:
> >
> >        #phy-cells = <0>;
> >      };
> > +  - |
> > +    #define GCC_PCIE1_PHY_REFGEN_CLK   47
> > +    #define GCC_PCIE_PHY_AUX_CLK       71
> > +    #define GCC_PCIE_WIGIG_CLKREF_EN   74
> > +
> > +    phy@1c0e000 {
> > +        compatible = "qcom,sm8250-qmp-gen3x2-pcie-phy";
> > +        reg = <0x01c0e000 0x1c0>;
> > +
> > +        clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
> > +                 <&gcc GCC_PCIE_1_CFG_AHB_CLK>,
> > +                 <&gcc GCC_PCIE_WIGIG_CLKREF_EN>,
> > +                 <&gcc GCC_PCIE1_PHY_REFGEN_CLK>,
> > +                 <&gcc GCC_PCIE_1_PIPE_CLK>;
> > +        clock-names = "aux", "cfg_ahb", "ref", "refgen", "pipe";
> > +
> > +        resets = <&gcc GCC_PCIE_1_PHY_BCR>;
> > +        reset-names = "phy";
> > +
> > +        vdda-phy-supply = <&vreg_l10c_0p88>;
> > +        vdda-pll-supply = <&vreg_l6b_1p2>;
> > +
> > +        #clock-cells = <0>;
> > +        clock-output-names = "pcie_1_pipe_clk";
> > +
> > +        #phy-cells = <0>;
> > +    };
>
> This example also looks redundant.
>
> Johan