[PATCH v5 0/6] Enable IPQ5332 USB2

[PATCH v5 0/6] Enable IPQ5332 USB2

[Varadarajan Narayanan]

This patch series adds the relevant phy and controller
configurations for enabling USB2 on IPQ5332

v5:
	Binding and dts:-
		Fix email id
		Removed 'Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>'
		as had to change bindings file to be able to use generic phy instead of
		usb-phy

	Driver:-
		Remove unused definition
		Use generic phy instead of usb-phy
v4:
	Binding and dts:-
		Change node name (bindings & dts)
	Driver:-
		Remove unused enum
		static const for '.data'
		Error handling for devm_clk_get
v3:
	Fix bindings file based on review comments

v1:
	Cleanup DTS
	Combine driver, kconfig and makefile patches
	Remove unused functions from M31 driver
	Drop the clock driver changes

Varadarajan Narayanan (6):
  dt-bindings: usb: dwc3: Add IPQ5332 compatible
  dt-bindings: phy: qcom,m31: Document qcom,m31 USB phy
  phy: qcom-m31: Introduce qcom,m31 USB phy driver
  arm64: dts: qcom: ipq5332: Add USB related nodes
  arm64: dts: qcom: ipq5332: Enable USB
  arm64: defconfig: Enable M31 USB phy driver

 .../bindings/phy/qcom,ipq5332-usb-hsphy.yaml       |  51 ++++
 .../devicetree/bindings/usb/qcom,dwc3.yaml         |   3 +
 arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts        |  12 +
 arch/arm64/boot/dts/qcom/ipq5332.dtsi              |  56 +++++
 arch/arm64/configs/defconfig                       |   1 +
 drivers/phy/qualcomm/Kconfig                       |  11 +
 drivers/phy/qualcomm/Makefile                      |   1 +
 drivers/phy/qualcomm/phy-qcom-m31.c                | 264 +++++++++++++++++++++
 8 files changed, 399 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
 create mode 100644 drivers/phy/qualcomm/phy-qcom-m31.c

-- 
2.7.4

[PATCH v5 0/6] Enable IPQ5332 USB2

[Varadarajan Narayanan]

This patch series adds the relevant phy and controller
configurations for enabling USB2 on IPQ5332

v5:
	Binding and dts:-
		Fix email id
		Removed 'Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>'
		as had to change bindings file to be able to use generic phy instead of
		usb-phy

	Driver:-
		Remove unused definition
		Use generic phy instead of usb-phy
v4:
	Binding and dts:-
		Change node name (bindings & dts)
	Driver:-
		Remove unused enum
		static const for '.data'
		Error handling for devm_clk_get
v3:
	Fix bindings file based on review comments

v1:
	Cleanup DTS
	Combine driver, kconfig and makefile patches
	Remove unused functions from M31 driver
	Drop the clock driver changes

Varadarajan Narayanan (6):
  dt-bindings: usb: dwc3: Add IPQ5332 compatible
  dt-bindings: phy: qcom,m31: Document qcom,m31 USB phy
  phy: qcom-m31: Introduce qcom,m31 USB phy driver
  arm64: dts: qcom: ipq5332: Add USB related nodes
  arm64: dts: qcom: ipq5332: Enable USB
  arm64: defconfig: Enable M31 USB phy driver

 .../bindings/phy/qcom,ipq5332-usb-hsphy.yaml       |  51 ++++
 .../devicetree/bindings/usb/qcom,dwc3.yaml         |   3 +
 arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts        |  12 +
 arch/arm64/boot/dts/qcom/ipq5332.dtsi              |  56 +++++
 arch/arm64/configs/defconfig                       |   1 +
 drivers/phy/qualcomm/Kconfig                       |  11 +
 drivers/phy/qualcomm/Makefile                      |   1 +
 drivers/phy/qualcomm/phy-qcom-m31.c                | 264 +++++++++++++++++++++
 8 files changed, 399 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
 create mode 100644 drivers/phy/qualcomm/phy-qcom-m31.c

-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 0/6] Enable IPQ5332 USB2

[Varadarajan Narayanan]

This patch series adds the relevant phy and controller
configurations for enabling USB2 on IPQ5332

v5:
	Binding and dts:-
		Fix email id
		Removed 'Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>'
		as had to change bindings file to be able to use generic phy instead of
		usb-phy

	Driver:-
		Remove unused definition
		Use generic phy instead of usb-phy
v4:
	Binding and dts:-
		Change node name (bindings & dts)
	Driver:-
		Remove unused enum
		static const for '.data'
		Error handling for devm_clk_get
v3:
	Fix bindings file based on review comments

v1:
	Cleanup DTS
	Combine driver, kconfig and makefile patches
	Remove unused functions from M31 driver
	Drop the clock driver changes

Varadarajan Narayanan (6):
  dt-bindings: usb: dwc3: Add IPQ5332 compatible
  dt-bindings: phy: qcom,m31: Document qcom,m31 USB phy
  phy: qcom-m31: Introduce qcom,m31 USB phy driver
  arm64: dts: qcom: ipq5332: Add USB related nodes
  arm64: dts: qcom: ipq5332: Enable USB
  arm64: defconfig: Enable M31 USB phy driver

 .../bindings/phy/qcom,ipq5332-usb-hsphy.yaml       |  51 ++++
 .../devicetree/bindings/usb/qcom,dwc3.yaml         |   3 +
 arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts        |  12 +
 arch/arm64/boot/dts/qcom/ipq5332.dtsi              |  56 +++++
 arch/arm64/configs/defconfig                       |   1 +
 drivers/phy/qualcomm/Kconfig                       |  11 +
 drivers/phy/qualcomm/Makefile                      |   1 +
 drivers/phy/qualcomm/phy-qcom-m31.c                | 264 +++++++++++++++++++++
 8 files changed, 399 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
 create mode 100644 drivers/phy/qualcomm/phy-qcom-m31.c

-- 
2.7.4


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v5 1/6] dt-bindings: usb: dwc3: Add IPQ5332 compatible

[Varadarajan Narayanan]

Document the IPQ5332 dwc3 compatible.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v1:
	Add ipq5332 to interrupts sections
---
 Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
index ae24dac..9447b54 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -14,6 +14,7 @@ properties:
     items:
       - enum:
           - qcom,ipq4019-dwc3
+          - qcom,ipq5332-dwc3
           - qcom,ipq6018-dwc3
           - qcom,ipq8064-dwc3
           - qcom,ipq8074-dwc3
@@ -246,6 +247,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - qcom,ipq5332-dwc3
               - qcom,msm8994-dwc3
               - qcom,qcs404-dwc3
     then:
@@ -410,6 +412,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - qcom,ipq5332-dwc3
               - qcom,sdm660-dwc3
     then:
       properties:
-- 
2.7.4

[PATCH v5 1/6] dt-bindings: usb: dwc3: Add IPQ5332 compatible

[Varadarajan Narayanan]

Document the IPQ5332 dwc3 compatible.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v1:
	Add ipq5332 to interrupts sections
---
 Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
index ae24dac..9447b54 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -14,6 +14,7 @@ properties:
     items:
       - enum:
           - qcom,ipq4019-dwc3
+          - qcom,ipq5332-dwc3
           - qcom,ipq6018-dwc3
           - qcom,ipq8064-dwc3
           - qcom,ipq8074-dwc3
@@ -246,6 +247,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - qcom,ipq5332-dwc3
               - qcom,msm8994-dwc3
               - qcom,qcs404-dwc3
     then:
@@ -410,6 +412,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - qcom,ipq5332-dwc3
               - qcom,sdm660-dwc3
     then:
       properties:
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 1/6] dt-bindings: usb: dwc3: Add IPQ5332 compatible

[Varadarajan Narayanan]

Document the IPQ5332 dwc3 compatible.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v1:
	Add ipq5332 to interrupts sections
---
 Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
index ae24dac..9447b54 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -14,6 +14,7 @@ properties:
     items:
       - enum:
           - qcom,ipq4019-dwc3
+          - qcom,ipq5332-dwc3
           - qcom,ipq6018-dwc3
           - qcom,ipq8064-dwc3
           - qcom,ipq8074-dwc3
@@ -246,6 +247,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - qcom,ipq5332-dwc3
               - qcom,msm8994-dwc3
               - qcom,qcs404-dwc3
     then:
@@ -410,6 +412,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - qcom,ipq5332-dwc3
               - qcom,sdm660-dwc3
     then:
       properties:
-- 
2.7.4


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v5 2/6] dt-bindings: phy: qcom,m31: Document qcom,m31 USB phy

[Varadarajan Narayanan]

Document the M31 USB2 phy present in IPQ5332.

Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v5:
	Add '#phy-cells', to be able to use generic phy
	Remove 'Reviewed-by: Krzysztof Kozlowski' due to above change
v4:
	Move M31 URL to description
	Remove maxItems and relevant content from clock-names
	Change node name to generic name
	'make dt_binding_check DT_SCHEMA_FILES=qcom' passed
v3:
	Incorporate review comments. Will bring in ipq5018 compatible
	string while posting ipq5018 usb patchset.

v1:
	Rename qcom,m31.yaml -> qcom,ipq5332-usb-hsphy.yaml
	Drop default binding "m31,usb-hsphy"
	Add clock
	Remove 'oneOf' from compatible
	Remove 'qscratch' region from register space as it is not needed
	Remove reset-names
	Fix the example definition
---
 .../bindings/phy/qcom,ipq5332-usb-hsphy.yaml       | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml

diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
new file mode 100644
index 0000000..e0b282b
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/qcom,ipq5332-usb-hsphy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: M31 USB PHY
+
+maintainers:
+  - Sricharan Ramabadhran <quic_srichara@quicinc.com>
+  - Varadarajan Narayanan <quic_varada@quicinc.com>
+
+description:
+  USB M31 PHY (https://www.m31tech.com) found in Qualcomm
+  IPQ5018, IPQ5332 SoCs.
+
+properties:
+  "#phy-cells":
+    const: 0
+
+  compatible:
+    enum:
+      - qcom,ipq5332-usb-hsphy
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: cfg_ahb
+
+  resets:
+    maxItems: 1
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
+    usbphy0: usb-phy@7b000 {
+        compatible = "qcom,ipq5332-usb-hsphy";
+        reg = <0x0007b000 0x12c>;
+
+        clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>;
+        clock-names = "cfg_ahb";
+
+        resets = <&gcc GCC_QUSB2_0_PHY_BCR>;
+    };
-- 
2.7.4

[PATCH v5 2/6] dt-bindings: phy: qcom,m31: Document qcom,m31 USB phy

[Varadarajan Narayanan]

Document the M31 USB2 phy present in IPQ5332.

Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v5:
	Add '#phy-cells', to be able to use generic phy
	Remove 'Reviewed-by: Krzysztof Kozlowski' due to above change
v4:
	Move M31 URL to description
	Remove maxItems and relevant content from clock-names
	Change node name to generic name
	'make dt_binding_check DT_SCHEMA_FILES=qcom' passed
v3:
	Incorporate review comments. Will bring in ipq5018 compatible
	string while posting ipq5018 usb patchset.

v1:
	Rename qcom,m31.yaml -> qcom,ipq5332-usb-hsphy.yaml
	Drop default binding "m31,usb-hsphy"
	Add clock
	Remove 'oneOf' from compatible
	Remove 'qscratch' region from register space as it is not needed
	Remove reset-names
	Fix the example definition
---
 .../bindings/phy/qcom,ipq5332-usb-hsphy.yaml       | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml

diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
new file mode 100644
index 0000000..e0b282b
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/qcom,ipq5332-usb-hsphy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: M31 USB PHY
+
+maintainers:
+  - Sricharan Ramabadhran <quic_srichara@quicinc.com>
+  - Varadarajan Narayanan <quic_varada@quicinc.com>
+
+description:
+  USB M31 PHY (https://www.m31tech.com) found in Qualcomm
+  IPQ5018, IPQ5332 SoCs.
+
+properties:
+  "#phy-cells":
+    const: 0
+
+  compatible:
+    enum:
+      - qcom,ipq5332-usb-hsphy
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: cfg_ahb
+
+  resets:
+    maxItems: 1
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
+    usbphy0: usb-phy@7b000 {
+        compatible = "qcom,ipq5332-usb-hsphy";
+        reg = <0x0007b000 0x12c>;
+
+        clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>;
+        clock-names = "cfg_ahb";
+
+        resets = <&gcc GCC_QUSB2_0_PHY_BCR>;
+    };
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 2/6] dt-bindings: phy: qcom,m31: Document qcom,m31 USB phy

[Varadarajan Narayanan]

Document the M31 USB2 phy present in IPQ5332.

Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v5:
	Add '#phy-cells', to be able to use generic phy
	Remove 'Reviewed-by: Krzysztof Kozlowski' due to above change
v4:
	Move M31 URL to description
	Remove maxItems and relevant content from clock-names
	Change node name to generic name
	'make dt_binding_check DT_SCHEMA_FILES=qcom' passed
v3:
	Incorporate review comments. Will bring in ipq5018 compatible
	string while posting ipq5018 usb patchset.

v1:
	Rename qcom,m31.yaml -> qcom,ipq5332-usb-hsphy.yaml
	Drop default binding "m31,usb-hsphy"
	Add clock
	Remove 'oneOf' from compatible
	Remove 'qscratch' region from register space as it is not needed
	Remove reset-names
	Fix the example definition
---
 .../bindings/phy/qcom,ipq5332-usb-hsphy.yaml       | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml

diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
new file mode 100644
index 0000000..e0b282b
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/qcom,ipq5332-usb-hsphy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: M31 USB PHY
+
+maintainers:
+  - Sricharan Ramabadhran <quic_srichara@quicinc.com>
+  - Varadarajan Narayanan <quic_varada@quicinc.com>
+
+description:
+  USB M31 PHY (https://www.m31tech.com) found in Qualcomm
+  IPQ5018, IPQ5332 SoCs.
+
+properties:
+  "#phy-cells":
+    const: 0
+
+  compatible:
+    enum:
+      - qcom,ipq5332-usb-hsphy
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: cfg_ahb
+
+  resets:
+    maxItems: 1
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
+    usbphy0: usb-phy@7b000 {
+        compatible = "qcom,ipq5332-usb-hsphy";
+        reg = <0x0007b000 0x12c>;
+
+        clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>;
+        clock-names = "cfg_ahb";
+
+        resets = <&gcc GCC_QUSB2_0_PHY_BCR>;
+    };
-- 
2.7.4


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 2/6] dt-bindings: phy: qcom,m31: Document qcom,m31 USB phy

[Bjorn Andersson]

On Fri, Jul 21, 2023 at 10:05:27AM +0530, Varadarajan Narayanan wrote:
> Document the M31 USB2 phy present in IPQ5332.
> 
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>

As Sricharan is the first one to certify the patch's origin, it seems
likely that he's the author. Please add a Co-developed-by: Sricharan.

[..]
> diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> new file mode 100644
> index 0000000..e0b282b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/qcom,ipq5332-usb-hsphy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: M31 USB PHY
> +
> +maintainers:
> +  - Sricharan Ramabadhran <quic_srichara@quicinc.com>
> +  - Varadarajan Narayanan <quic_varada@quicinc.com>
> +
> +description:
> +  USB M31 PHY (https://www.m31tech.com) found in Qualcomm
> +  IPQ5018, IPQ5332 SoCs.
> +
> +properties:
> +  "#phy-cells":
> +    const: 0
> +
> +  compatible:
> +    enum:

const?

> +      - qcom,ipq5332-usb-hsphy
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: cfg_ahb
> +
> +  resets:
> +    maxItems: 1
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> +    usbphy0: usb-phy@7b000 {

usb@

And you don't need to give the node a label in the example.

> +        compatible = "qcom,ipq5332-usb-hsphy";
> +        reg = <0x0007b000 0x12c>;
> +
> +        clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>;
> +        clock-names = "cfg_ahb";
> +

No #phy-cells?

Regards,
Bjorn

> +        resets = <&gcc GCC_QUSB2_0_PHY_BCR>;
> +    };
> -- 
> 2.7.4
> 

Re: [PATCH v5 2/6] dt-bindings: phy: qcom,m31: Document qcom,m31 USB phy

[Bjorn Andersson]

On Fri, Jul 21, 2023 at 10:05:27AM +0530, Varadarajan Narayanan wrote:
> Document the M31 USB2 phy present in IPQ5332.
> 
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>

As Sricharan is the first one to certify the patch's origin, it seems
likely that he's the author. Please add a Co-developed-by: Sricharan.

[..]
> diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> new file mode 100644
> index 0000000..e0b282b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/qcom,ipq5332-usb-hsphy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: M31 USB PHY
> +
> +maintainers:
> +  - Sricharan Ramabadhran <quic_srichara@quicinc.com>
> +  - Varadarajan Narayanan <quic_varada@quicinc.com>
> +
> +description:
> +  USB M31 PHY (https://www.m31tech.com) found in Qualcomm
> +  IPQ5018, IPQ5332 SoCs.
> +
> +properties:
> +  "#phy-cells":
> +    const: 0
> +
> +  compatible:
> +    enum:

const?

> +      - qcom,ipq5332-usb-hsphy
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: cfg_ahb
> +
> +  resets:
> +    maxItems: 1
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> +    usbphy0: usb-phy@7b000 {

usb@

And you don't need to give the node a label in the example.

> +        compatible = "qcom,ipq5332-usb-hsphy";
> +        reg = <0x0007b000 0x12c>;
> +
> +        clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>;
> +        clock-names = "cfg_ahb";
> +

No #phy-cells?

Regards,
Bjorn

> +        resets = <&gcc GCC_QUSB2_0_PHY_BCR>;
> +    };
> -- 
> 2.7.4
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 2/6] dt-bindings: phy: qcom,m31: Document qcom,m31 USB phy

[Bjorn Andersson]

On Fri, Jul 21, 2023 at 10:05:27AM +0530, Varadarajan Narayanan wrote:
> Document the M31 USB2 phy present in IPQ5332.
> 
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>

As Sricharan is the first one to certify the patch's origin, it seems
likely that he's the author. Please add a Co-developed-by: Sricharan.

[..]
> diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> new file mode 100644
> index 0000000..e0b282b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/qcom,ipq5332-usb-hsphy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: M31 USB PHY
> +
> +maintainers:
> +  - Sricharan Ramabadhran <quic_srichara@quicinc.com>
> +  - Varadarajan Narayanan <quic_varada@quicinc.com>
> +
> +description:
> +  USB M31 PHY (https://www.m31tech.com) found in Qualcomm
> +  IPQ5018, IPQ5332 SoCs.
> +
> +properties:
> +  "#phy-cells":
> +    const: 0
> +
> +  compatible:
> +    enum:

const?

> +      - qcom,ipq5332-usb-hsphy
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: cfg_ahb
> +
> +  resets:
> +    maxItems: 1
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> +    usbphy0: usb-phy@7b000 {

usb@

And you don't need to give the node a label in the example.

> +        compatible = "qcom,ipq5332-usb-hsphy";
> +        reg = <0x0007b000 0x12c>;
> +
> +        clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>;
> +        clock-names = "cfg_ahb";
> +

No #phy-cells?

Regards,
Bjorn

> +        resets = <&gcc GCC_QUSB2_0_PHY_BCR>;
> +    };
> -- 
> 2.7.4
> 

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 2/6] dt-bindings: phy: qcom,m31: Document qcom,m31 USB phy

[Rob Herring]

On Fri, Jul 21, 2023 at 10:10:57PM -0700, Bjorn Andersson wrote:
> On Fri, Jul 21, 2023 at 10:05:27AM +0530, Varadarajan Narayanan wrote:
> > Document the M31 USB2 phy present in IPQ5332.
> > 
> > Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> 
> As Sricharan is the first one to certify the patch's origin, it seems
> likely that he's the author. Please add a Co-developed-by: Sricharan.
> 
> [..]
> > diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> > new file mode 100644
> > index 0000000..e0b282b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> > @@ -0,0 +1,51 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/phy/qcom,ipq5332-usb-hsphy.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: M31 USB PHY
> > +
> > +maintainers:
> > +  - Sricharan Ramabadhran <quic_srichara@quicinc.com>
> > +  - Varadarajan Narayanan <quic_varada@quicinc.com>
> > +
> > +description:
> > +  USB M31 PHY (https://www.m31tech.com) found in Qualcomm
> > +  IPQ5018, IPQ5332 SoCs.
> > +
> > +properties:
> > +  "#phy-cells":
> > +    const: 0
> > +
> > +  compatible:
> > +    enum:
> 
> const?
> 
> > +      - qcom,ipq5332-usb-hsphy
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    items:
> > +      - const: cfg_ahb
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> > +    usbphy0: usb-phy@7b000 {
> 
> usb@

You mean phy@? But 'usb2-phy' is accepted too.

> 
> And you don't need to give the node a label in the example.
> 
> > +        compatible = "qcom,ipq5332-usb-hsphy";
> > +        reg = <0x0007b000 0x12c>;
> > +
> > +        clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>;
> > +        clock-names = "cfg_ahb";
> > +
> 
> No #phy-cells?
> 
> Regards,
> Bjorn
> 
> > +        resets = <&gcc GCC_QUSB2_0_PHY_BCR>;
> > +    };
> > -- 
> > 2.7.4
> > 

Re: [PATCH v5 2/6] dt-bindings: phy: qcom,m31: Document qcom,m31 USB phy

[Rob Herring]

On Fri, Jul 21, 2023 at 10:10:57PM -0700, Bjorn Andersson wrote:
> On Fri, Jul 21, 2023 at 10:05:27AM +0530, Varadarajan Narayanan wrote:
> > Document the M31 USB2 phy present in IPQ5332.
> > 
> > Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> 
> As Sricharan is the first one to certify the patch's origin, it seems
> likely that he's the author. Please add a Co-developed-by: Sricharan.
> 
> [..]
> > diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> > new file mode 100644
> > index 0000000..e0b282b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> > @@ -0,0 +1,51 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/phy/qcom,ipq5332-usb-hsphy.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: M31 USB PHY
> > +
> > +maintainers:
> > +  - Sricharan Ramabadhran <quic_srichara@quicinc.com>
> > +  - Varadarajan Narayanan <quic_varada@quicinc.com>
> > +
> > +description:
> > +  USB M31 PHY (https://www.m31tech.com) found in Qualcomm
> > +  IPQ5018, IPQ5332 SoCs.
> > +
> > +properties:
> > +  "#phy-cells":
> > +    const: 0
> > +
> > +  compatible:
> > +    enum:
> 
> const?
> 
> > +      - qcom,ipq5332-usb-hsphy
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    items:
> > +      - const: cfg_ahb
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> > +    usbphy0: usb-phy@7b000 {
> 
> usb@

You mean phy@? But 'usb2-phy' is accepted too.

> 
> And you don't need to give the node a label in the example.
> 
> > +        compatible = "qcom,ipq5332-usb-hsphy";
> > +        reg = <0x0007b000 0x12c>;
> > +
> > +        clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>;
> > +        clock-names = "cfg_ahb";
> > +
> 
> No #phy-cells?
> 
> Regards,
> Bjorn
> 
> > +        resets = <&gcc GCC_QUSB2_0_PHY_BCR>;
> > +    };
> > -- 
> > 2.7.4
> > 

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 2/6] dt-bindings: phy: qcom,m31: Document qcom,m31 USB phy

[Bjorn Andersson]

On Mon, Jul 24, 2023 at 10:01:36AM -0600, Rob Herring wrote:
> On Fri, Jul 21, 2023 at 10:10:57PM -0700, Bjorn Andersson wrote:
> > On Fri, Jul 21, 2023 at 10:05:27AM +0530, Varadarajan Narayanan wrote:
> > [..]
> > > diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
[..]
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> > > +    usbphy0: usb-phy@7b000 {
> > 
> > usb@
> 
> You mean phy@? But 'usb2-phy' is accepted too.
> 

Yes, had the controller in mind, sorry about that.

Thanks,
Bjorn

Re: [PATCH v5 2/6] dt-bindings: phy: qcom,m31: Document qcom,m31 USB phy

[Bjorn Andersson]

On Mon, Jul 24, 2023 at 10:01:36AM -0600, Rob Herring wrote:
> On Fri, Jul 21, 2023 at 10:10:57PM -0700, Bjorn Andersson wrote:
> > On Fri, Jul 21, 2023 at 10:05:27AM +0530, Varadarajan Narayanan wrote:
> > [..]
> > > diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
[..]
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> > > +    usbphy0: usb-phy@7b000 {
> > 
> > usb@
> 
> You mean phy@? But 'usb2-phy' is accepted too.
> 

Yes, had the controller in mind, sorry about that.

Thanks,
Bjorn

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 2/6] dt-bindings: phy: qcom,m31: Document qcom,m31 USB phy

[Varadarajan Narayanan]

Bjorn & Rob,

On Tue, Jul 25, 2023 at 10:24:15PM -0700, Bjorn Andersson wrote:
> On Mon, Jul 24, 2023 at 10:01:36AM -0600, Rob Herring wrote:
> > On Fri, Jul 21, 2023 at 10:10:57PM -0700, Bjorn Andersson wrote:
> > > On Fri, Jul 21, 2023 at 10:05:27AM +0530, Varadarajan Narayanan wrote:
> > > [..]
> > > > diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> [..]
> > > > +examples:
> > > > +  - |
> > > > +    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> > > > +    usbphy0: usb-phy@7b000 {
> > >
> > > usb@
> >
> > You mean phy@? But 'usb2-phy' is accepted too.
> >
>
> Yes, had the controller in mind, sorry about that.

Have posted a new patchset addressing the comments.
Please provide your feedback.

Thanks
Varada

Re: [PATCH v5 2/6] dt-bindings: phy: qcom,m31: Document qcom,m31 USB phy

[Varadarajan Narayanan]

Bjorn & Rob,

On Tue, Jul 25, 2023 at 10:24:15PM -0700, Bjorn Andersson wrote:
> On Mon, Jul 24, 2023 at 10:01:36AM -0600, Rob Herring wrote:
> > On Fri, Jul 21, 2023 at 10:10:57PM -0700, Bjorn Andersson wrote:
> > > On Fri, Jul 21, 2023 at 10:05:27AM +0530, Varadarajan Narayanan wrote:
> > > [..]
> > > > diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> [..]
> > > > +examples:
> > > > +  - |
> > > > +    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> > > > +    usbphy0: usb-phy@7b000 {
> > >
> > > usb@
> >
> > You mean phy@? But 'usb2-phy' is accepted too.
> >
>
> Yes, had the controller in mind, sorry about that.

Have posted a new patchset addressing the comments.
Please provide your feedback.

Thanks
Varada

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 2/6] dt-bindings: phy: qcom,m31: Document qcom,m31 USB phy

[Varadarajan Narayanan]

Bjorn & Rob,

On Tue, Jul 25, 2023 at 10:24:15PM -0700, Bjorn Andersson wrote:
> On Mon, Jul 24, 2023 at 10:01:36AM -0600, Rob Herring wrote:
> > On Fri, Jul 21, 2023 at 10:10:57PM -0700, Bjorn Andersson wrote:
> > > On Fri, Jul 21, 2023 at 10:05:27AM +0530, Varadarajan Narayanan wrote:
> > > [..]
> > > > diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq5332-usb-hsphy.yaml
> [..]
> > > > +examples:
> > > > +  - |
> > > > +    #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> > > > +    usbphy0: usb-phy@7b000 {
> > >
> > > usb@
> >
> > You mean phy@? But 'usb2-phy' is accepted too.
> >
>
> Yes, had the controller in mind, sorry about that.

Have posted a new patchset addressing the comments.
Please provide your feedback.

Thanks
Varada

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v5 3/6] phy: qcom-m31: Introduce qcom,m31 USB phy driver

[Varadarajan Narayanan]

Add the M31 USB2 phy driver.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v5:
	Kconfig and Makefile:- place snippet according to sorted order
	Use generic phy instead of usb-phy
	Use ARRAY_SIZE for reg init instead of blank last entry
	Fix copyright year

v4:
	Remove unused enum
	Error handling for devm_clk_get
v1:
	Combine driver, makefile and kconfig into 1 patch
	Remove 'qscratch' region and its usage. The controller driver takes care
	of those settings
	Use compatible/data to handle ipq5332 init
	Drop the default case
	Get resources by index instead of name as there is only one resource
	Add clock
	Fix review comments in the driver
---
 drivers/phy/qualcomm/Kconfig        |  11 ++
 drivers/phy/qualcomm/Makefile       |   1 +
 drivers/phy/qualcomm/phy-qcom-m31.c | 264 ++++++++++++++++++++++++++++++++++++
 3 files changed, 276 insertions(+)
 create mode 100644 drivers/phy/qualcomm/phy-qcom-m31.c

diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
index ced6038..d638906 100644
--- a/drivers/phy/qualcomm/Kconfig
+++ b/drivers/phy/qualcomm/Kconfig
@@ -143,6 +143,17 @@ config PHY_QCOM_EUSB2_REPEATER
 	  PMICs. The repeater is paired with a Synopsys eUSB2 Phy
 	  on Qualcomm SOCs.
 
+config PHY_QCOM_M31_USB
+	tristate "Qualcomm M31 HS PHY driver support"
+	depends on (USB || USB_GADGET) && ARCH_QCOM
+	select USB_PHY
+	help
+	  Enable this to support M31 HS PHY transceivers on Qualcomm chips
+	  with DWC3 USB core. It handles PHY initialization, clock
+	  management required after resetting the hardware and power
+	  management. This driver is required even for peripheral only or
+	  host only mode configurations.
+
 config PHY_QCOM_USB_HS
 	tristate "Qualcomm USB HS PHY module"
 	depends on USB_ULPI_BUS
diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
index df94581..ffd609a 100644
--- a/drivers/phy/qualcomm/Makefile
+++ b/drivers/phy/qualcomm/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)	+= phy-qcom-apq8064-sata.o
 obj-$(CONFIG_PHY_QCOM_EDP)		+= phy-qcom-edp.o
 obj-$(CONFIG_PHY_QCOM_IPQ4019_USB)	+= phy-qcom-ipq4019-usb.o
 obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)	+= phy-qcom-ipq806x-sata.o
+obj-$(CONFIG_PHY_QCOM_M31_USB)		+= phy-qcom-m31.o
 obj-$(CONFIG_PHY_QCOM_PCIE2)		+= phy-qcom-pcie2.o
 
 obj-$(CONFIG_PHY_QCOM_QMP_COMBO)	+= phy-qcom-qmp-combo.o
diff --git a/drivers/phy/qualcomm/phy-qcom-m31.c b/drivers/phy/qualcomm/phy-qcom-m31.c
new file mode 100644
index 0000000..4627425
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-m31.c
@@ -0,0 +1,264 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2014-2016, 2020-2023, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+#include <linux/usb/of.h>
+#include <linux/usb/phy.h>
+
+#define USB2PHY_PORT_UTMI_CTRL1		0x40
+
+#define USB2PHY_PORT_UTMI_CTRL2		0x44
+ #define UTMI_ULPI_SEL			BIT(7)
+ #define UTMI_TEST_MUX_SEL		BIT(6)
+
+#define HS_PHY_CTRL_REG			0x10
+ #define UTMI_OTG_VBUS_VALID		BIT(20)
+ #define SW_SESSVLD_SEL			BIT(28)
+
+#define USB_PHY_UTMI_CTRL0		0x3c
+
+#define USB_PHY_UTMI_CTRL5		0x50
+ #define POR_EN				BIT(1)
+
+#define USB_PHY_HS_PHY_CTRL_COMMON0	0x54
+ #define COMMONONN			BIT(7)
+ #define FSEL				BIT(4)
+ #define RETENABLEN			BIT(3)
+ #define FREQ_24MHZ			(GENMASK(6, 6) | GENMASK(4, 4))
+
+#define USB_PHY_HS_PHY_CTRL2		0x64
+ #define USB2_SUSPEND_N_SEL		BIT(3)
+ #define USB2_SUSPEND_N			BIT(2)
+ #define USB2_UTMI_CLK_EN		BIT(1)
+
+#define USB_PHY_CFG0			0x94
+ #define UTMI_PHY_OVERRIDE_EN		BIT(1)
+
+#define USB_PHY_REFCLK_CTRL		0xa0
+ #define CLKCORE			BIT(1)
+
+#define USB2PHY_PORT_POWERDOWN		0xa4
+ #define POWER_UP			BIT(0)
+ #define POWER_DOWN			0
+
+#define USB_PHY_FSEL_SEL		0xb8
+ #define FREQ_SEL			BIT(0)
+
+#define USB2PHY_USB_PHY_M31_XCFGI_1	0xbc
+ #define USB2_0_TX_ENABLE		BIT(2)
+
+#define USB2PHY_USB_PHY_M31_XCFGI_4	0xc8
+ #define HSTX_SLEW_RATE_565PS		GENMASK(1, 0)
+ #define PLL_CHARGING_PUMP_CURRENT_35UA	GENMASK(4, 3)
+ #define ODT_VALUE_38_02_OHM		GENMASK(7, 6)
+
+#define USB2PHY_USB_PHY_M31_XCFGI_5	0xcc
+ #define ODT_VALUE_45_02_OHM		BIT(2)
+ #define HSTX_PRE_EMPHASIS_LEVEL_0_55MA	BIT(0)
+
+#define USB2PHY_USB_PHY_M31_XCFGI_11	0xe4
+ #define XCFG_COARSE_TUNE_NUM		BIT(1)
+ #define XCFG_FINE_TUNE_NUM		BIT(3)
+
+struct m31_phy_regs {
+	u32 off;
+	u32 val;
+	u32 delay;
+};
+
+struct m31_priv_data {
+	bool				ulpi_mode;
+	const struct m31_phy_regs	*regs;
+	const int			nregs;
+};
+
+#define M31_REG(__o, __v, __d)	\
+	{ .off = __o, .val = __v, .delay = __d }
+
+struct m31_phy_regs m31_ipq5332_regs[] = {
+	M31_REG(USB_PHY_CFG0, UTMI_PHY_OVERRIDE_EN, 0),
+	M31_REG(USB_PHY_UTMI_CTRL5, POR_EN, 15),
+	M31_REG(USB_PHY_FSEL_SEL, FREQ_SEL, 0),
+	M31_REG(USB_PHY_HS_PHY_CTRL_COMMON0,
+		COMMONONN | FREQ_24MHZ | RETENABLEN, 0),
+	M31_REG(USB_PHY_UTMI_CTRL5, POR_EN, 0),
+	M31_REG(USB_PHY_HS_PHY_CTRL2,
+		USB2_SUSPEND_N_SEL | USB2_SUSPEND_N | USB2_UTMI_CLK_EN, 0),
+	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_11,
+		XCFG_COARSE_TUNE_NUM  | XCFG_FINE_TUNE_NUM, 0),
+	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_4,
+		HSTX_SLEW_RATE_565PS | PLL_CHARGING_PUMP_CURRENT_35UA |
+		ODT_VALUE_38_02_OHM, 0),
+	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_1, USB2_0_TX_ENABLE, 0),
+	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_5,
+		ODT_VALUE_45_02_OHM | HSTX_PRE_EMPHASIS_LEVEL_0_55MA, 4),
+	M31_REG(USB_PHY_UTMI_CTRL5, 0x0, 0),
+	M31_REG(USB_PHY_HS_PHY_CTRL2, USB2_SUSPEND_N | USB2_UTMI_CLK_EN, 0),
+};
+
+struct m31usb_phy {
+	struct phy			*phy;
+	void __iomem			*base;
+	const struct m31_phy_regs	*regs;
+	int				nregs;
+
+	struct clk			*cfg_ahb_clk;
+	struct reset_control		*phy_reset;
+
+	bool				cable_connected;
+	bool				suspended;
+	bool				ulpi_mode;
+};
+
+static void m31usb_phy_enable_clock(struct m31usb_phy *qphy)
+{
+	const struct m31_phy_regs *regs = qphy->regs;
+	int i;
+
+	for (i = 0; i < qphy->nregs; i++) {
+		writel(regs[i].val, qphy->base + regs[i].off);
+		if (regs[i].delay)
+			udelay(regs[i].delay);
+	}
+}
+
+static int m31usb_phy_init(struct phy *phy)
+{
+	int ret;
+	struct m31usb_phy *qphy = phy_get_drvdata(phy);
+
+	ret = clk_prepare_enable(qphy->cfg_ahb_clk);
+	if (ret) {
+		dev_err(&phy->dev, "failed to enable cfg ahb clock, %d\n", ret);
+		return ret;
+	}
+
+	/* Perform phy reset */
+	reset_control_assert(qphy->phy_reset);
+	udelay(5);
+	reset_control_deassert(qphy->phy_reset);
+
+	/* configure for ULPI mode if requested */
+	if (qphy->ulpi_mode)
+		writel(0x0, qphy->base + USB2PHY_PORT_UTMI_CTRL2);
+
+	/* Enable the PHY */
+	writel(POWER_UP, qphy->base + USB2PHY_PORT_POWERDOWN);
+
+	/* Make sure above write completed */
+	wmb();
+
+	/* Turn on phy ref clock */
+	m31usb_phy_enable_clock(qphy);
+
+	return 0;
+}
+
+static int m31usb_phy_shutdown(struct phy *phy)
+{
+	struct m31usb_phy *qphy = phy_get_drvdata(phy);
+
+	/* Disable the PHY */
+	writel_relaxed(POWER_DOWN, qphy->base + USB2PHY_PORT_POWERDOWN);
+	/* Make sure above write completed */
+	wmb();
+
+	clk_disable_unprepare(qphy->cfg_ahb_clk);
+
+	return 0;
+}
+
+static const struct phy_ops m31usb_phy_gen_ops = {
+	.power_on	= m31usb_phy_init,
+	.power_off	= m31usb_phy_shutdown,
+	.owner		= THIS_MODULE,
+};
+
+static int m31usb_phy_probe(struct platform_device *pdev)
+{
+	const struct m31_priv_data *data;
+	struct device *dev = &pdev->dev;
+	struct m31usb_phy *qphy;
+	struct phy_provider *phy_provider;
+	int ret;
+
+	qphy = devm_kzalloc(dev, sizeof(*qphy), GFP_KERNEL);
+	if (!qphy)
+		return -ENOMEM;
+
+	qphy->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(qphy->base))
+		return PTR_ERR(qphy->base);
+
+	qphy->phy_reset = devm_reset_control_get_exclusive_by_index(dev, 0);
+	if (IS_ERR(qphy->phy_reset))
+		return PTR_ERR(qphy->phy_reset);
+
+	qphy->cfg_ahb_clk = devm_clk_get(dev, "cfg_ahb");
+	if (IS_ERR(qphy->cfg_ahb_clk)) {
+		ret = PTR_ERR(qphy->cfg_ahb_clk);
+		dev_err(dev, "failed to get cfg_ahb clock, %d\n", ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, qphy);
+
+	data = of_device_get_match_data(dev);
+	qphy->regs		= data->regs;
+	qphy->nregs		= data->nregs;
+	qphy->ulpi_mode		= data->ulpi_mode;
+
+	qphy->phy = devm_phy_create(dev, NULL, &m31usb_phy_gen_ops);
+	if (IS_ERR(qphy->phy)) {
+		ret = PTR_ERR(qphy->phy);
+		dev_err(dev, "failed to create phy, %d\n", ret);
+		qphy->phy = NULL;
+		return ret;
+	}
+
+	dev_set_drvdata(dev, qphy);
+	phy_set_drvdata(qphy->phy, qphy);
+
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (!IS_ERR(phy_provider))
+		dev_info(dev, "Registered M31 USB phy\n");
+
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct m31_priv_data m31_ipq5332_data = {
+	.ulpi_mode = false,
+	.regs = m31_ipq5332_regs,
+	.nregs = ARRAY_SIZE(m31_ipq5332_regs),
+};
+
+static const struct of_device_id m31usb_phy_id_table[] = {
+	{ .compatible = "qcom,ipq5332-usb-hsphy", .data = &m31_ipq5332_data },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, m31usb_phy_id_table);
+
+static struct platform_driver m31usb_phy_driver = {
+	.probe		= m31usb_phy_probe,
+	.driver = {
+		.name	= "qcom-m31usb-phy",
+		.of_match_table = m31usb_phy_id_table,
+	},
+};
+
+module_platform_driver(m31usb_phy_driver);
+
+MODULE_DESCRIPTION("USB2 Qualcomm M31 HSPHY driver");
+MODULE_LICENSE("GPL");
-- 
2.7.4

[PATCH v5 3/6] phy: qcom-m31: Introduce qcom,m31 USB phy driver

[Varadarajan Narayanan]

Add the M31 USB2 phy driver.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v5:
	Kconfig and Makefile:- place snippet according to sorted order
	Use generic phy instead of usb-phy
	Use ARRAY_SIZE for reg init instead of blank last entry
	Fix copyright year

v4:
	Remove unused enum
	Error handling for devm_clk_get
v1:
	Combine driver, makefile and kconfig into 1 patch
	Remove 'qscratch' region and its usage. The controller driver takes care
	of those settings
	Use compatible/data to handle ipq5332 init
	Drop the default case
	Get resources by index instead of name as there is only one resource
	Add clock
	Fix review comments in the driver
---
 drivers/phy/qualcomm/Kconfig        |  11 ++
 drivers/phy/qualcomm/Makefile       |   1 +
 drivers/phy/qualcomm/phy-qcom-m31.c | 264 ++++++++++++++++++++++++++++++++++++
 3 files changed, 276 insertions(+)
 create mode 100644 drivers/phy/qualcomm/phy-qcom-m31.c

diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
index ced6038..d638906 100644
--- a/drivers/phy/qualcomm/Kconfig
+++ b/drivers/phy/qualcomm/Kconfig
@@ -143,6 +143,17 @@ config PHY_QCOM_EUSB2_REPEATER
 	  PMICs. The repeater is paired with a Synopsys eUSB2 Phy
 	  on Qualcomm SOCs.
 
+config PHY_QCOM_M31_USB
+	tristate "Qualcomm M31 HS PHY driver support"
+	depends on (USB || USB_GADGET) && ARCH_QCOM
+	select USB_PHY
+	help
+	  Enable this to support M31 HS PHY transceivers on Qualcomm chips
+	  with DWC3 USB core. It handles PHY initialization, clock
+	  management required after resetting the hardware and power
+	  management. This driver is required even for peripheral only or
+	  host only mode configurations.
+
 config PHY_QCOM_USB_HS
 	tristate "Qualcomm USB HS PHY module"
 	depends on USB_ULPI_BUS
diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
index df94581..ffd609a 100644
--- a/drivers/phy/qualcomm/Makefile
+++ b/drivers/phy/qualcomm/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)	+= phy-qcom-apq8064-sata.o
 obj-$(CONFIG_PHY_QCOM_EDP)		+= phy-qcom-edp.o
 obj-$(CONFIG_PHY_QCOM_IPQ4019_USB)	+= phy-qcom-ipq4019-usb.o
 obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)	+= phy-qcom-ipq806x-sata.o
+obj-$(CONFIG_PHY_QCOM_M31_USB)		+= phy-qcom-m31.o
 obj-$(CONFIG_PHY_QCOM_PCIE2)		+= phy-qcom-pcie2.o
 
 obj-$(CONFIG_PHY_QCOM_QMP_COMBO)	+= phy-qcom-qmp-combo.o
diff --git a/drivers/phy/qualcomm/phy-qcom-m31.c b/drivers/phy/qualcomm/phy-qcom-m31.c
new file mode 100644
index 0000000..4627425
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-m31.c
@@ -0,0 +1,264 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2014-2016, 2020-2023, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+#include <linux/usb/of.h>
+#include <linux/usb/phy.h>
+
+#define USB2PHY_PORT_UTMI_CTRL1		0x40
+
+#define USB2PHY_PORT_UTMI_CTRL2		0x44
+ #define UTMI_ULPI_SEL			BIT(7)
+ #define UTMI_TEST_MUX_SEL		BIT(6)
+
+#define HS_PHY_CTRL_REG			0x10
+ #define UTMI_OTG_VBUS_VALID		BIT(20)
+ #define SW_SESSVLD_SEL			BIT(28)
+
+#define USB_PHY_UTMI_CTRL0		0x3c
+
+#define USB_PHY_UTMI_CTRL5		0x50
+ #define POR_EN				BIT(1)
+
+#define USB_PHY_HS_PHY_CTRL_COMMON0	0x54
+ #define COMMONONN			BIT(7)
+ #define FSEL				BIT(4)
+ #define RETENABLEN			BIT(3)
+ #define FREQ_24MHZ			(GENMASK(6, 6) | GENMASK(4, 4))
+
+#define USB_PHY_HS_PHY_CTRL2		0x64
+ #define USB2_SUSPEND_N_SEL		BIT(3)
+ #define USB2_SUSPEND_N			BIT(2)
+ #define USB2_UTMI_CLK_EN		BIT(1)
+
+#define USB_PHY_CFG0			0x94
+ #define UTMI_PHY_OVERRIDE_EN		BIT(1)
+
+#define USB_PHY_REFCLK_CTRL		0xa0
+ #define CLKCORE			BIT(1)
+
+#define USB2PHY_PORT_POWERDOWN		0xa4
+ #define POWER_UP			BIT(0)
+ #define POWER_DOWN			0
+
+#define USB_PHY_FSEL_SEL		0xb8
+ #define FREQ_SEL			BIT(0)
+
+#define USB2PHY_USB_PHY_M31_XCFGI_1	0xbc
+ #define USB2_0_TX_ENABLE		BIT(2)
+
+#define USB2PHY_USB_PHY_M31_XCFGI_4	0xc8
+ #define HSTX_SLEW_RATE_565PS		GENMASK(1, 0)
+ #define PLL_CHARGING_PUMP_CURRENT_35UA	GENMASK(4, 3)
+ #define ODT_VALUE_38_02_OHM		GENMASK(7, 6)
+
+#define USB2PHY_USB_PHY_M31_XCFGI_5	0xcc
+ #define ODT_VALUE_45_02_OHM		BIT(2)
+ #define HSTX_PRE_EMPHASIS_LEVEL_0_55MA	BIT(0)
+
+#define USB2PHY_USB_PHY_M31_XCFGI_11	0xe4
+ #define XCFG_COARSE_TUNE_NUM		BIT(1)
+ #define XCFG_FINE_TUNE_NUM		BIT(3)
+
+struct m31_phy_regs {
+	u32 off;
+	u32 val;
+	u32 delay;
+};
+
+struct m31_priv_data {
+	bool				ulpi_mode;
+	const struct m31_phy_regs	*regs;
+	const int			nregs;
+};
+
+#define M31_REG(__o, __v, __d)	\
+	{ .off = __o, .val = __v, .delay = __d }
+
+struct m31_phy_regs m31_ipq5332_regs[] = {
+	M31_REG(USB_PHY_CFG0, UTMI_PHY_OVERRIDE_EN, 0),
+	M31_REG(USB_PHY_UTMI_CTRL5, POR_EN, 15),
+	M31_REG(USB_PHY_FSEL_SEL, FREQ_SEL, 0),
+	M31_REG(USB_PHY_HS_PHY_CTRL_COMMON0,
+		COMMONONN | FREQ_24MHZ | RETENABLEN, 0),
+	M31_REG(USB_PHY_UTMI_CTRL5, POR_EN, 0),
+	M31_REG(USB_PHY_HS_PHY_CTRL2,
+		USB2_SUSPEND_N_SEL | USB2_SUSPEND_N | USB2_UTMI_CLK_EN, 0),
+	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_11,
+		XCFG_COARSE_TUNE_NUM  | XCFG_FINE_TUNE_NUM, 0),
+	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_4,
+		HSTX_SLEW_RATE_565PS | PLL_CHARGING_PUMP_CURRENT_35UA |
+		ODT_VALUE_38_02_OHM, 0),
+	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_1, USB2_0_TX_ENABLE, 0),
+	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_5,
+		ODT_VALUE_45_02_OHM | HSTX_PRE_EMPHASIS_LEVEL_0_55MA, 4),
+	M31_REG(USB_PHY_UTMI_CTRL5, 0x0, 0),
+	M31_REG(USB_PHY_HS_PHY_CTRL2, USB2_SUSPEND_N | USB2_UTMI_CLK_EN, 0),
+};
+
+struct m31usb_phy {
+	struct phy			*phy;
+	void __iomem			*base;
+	const struct m31_phy_regs	*regs;
+	int				nregs;
+
+	struct clk			*cfg_ahb_clk;
+	struct reset_control		*phy_reset;
+
+	bool				cable_connected;
+	bool				suspended;
+	bool				ulpi_mode;
+};
+
+static void m31usb_phy_enable_clock(struct m31usb_phy *qphy)
+{
+	const struct m31_phy_regs *regs = qphy->regs;
+	int i;
+
+	for (i = 0; i < qphy->nregs; i++) {
+		writel(regs[i].val, qphy->base + regs[i].off);
+		if (regs[i].delay)
+			udelay(regs[i].delay);
+	}
+}
+
+static int m31usb_phy_init(struct phy *phy)
+{
+	int ret;
+	struct m31usb_phy *qphy = phy_get_drvdata(phy);
+
+	ret = clk_prepare_enable(qphy->cfg_ahb_clk);
+	if (ret) {
+		dev_err(&phy->dev, "failed to enable cfg ahb clock, %d\n", ret);
+		return ret;
+	}
+
+	/* Perform phy reset */
+	reset_control_assert(qphy->phy_reset);
+	udelay(5);
+	reset_control_deassert(qphy->phy_reset);
+
+	/* configure for ULPI mode if requested */
+	if (qphy->ulpi_mode)
+		writel(0x0, qphy->base + USB2PHY_PORT_UTMI_CTRL2);
+
+	/* Enable the PHY */
+	writel(POWER_UP, qphy->base + USB2PHY_PORT_POWERDOWN);
+
+	/* Make sure above write completed */
+	wmb();
+
+	/* Turn on phy ref clock */
+	m31usb_phy_enable_clock(qphy);
+
+	return 0;
+}
+
+static int m31usb_phy_shutdown(struct phy *phy)
+{
+	struct m31usb_phy *qphy = phy_get_drvdata(phy);
+
+	/* Disable the PHY */
+	writel_relaxed(POWER_DOWN, qphy->base + USB2PHY_PORT_POWERDOWN);
+	/* Make sure above write completed */
+	wmb();
+
+	clk_disable_unprepare(qphy->cfg_ahb_clk);
+
+	return 0;
+}
+
+static const struct phy_ops m31usb_phy_gen_ops = {
+	.power_on	= m31usb_phy_init,
+	.power_off	= m31usb_phy_shutdown,
+	.owner		= THIS_MODULE,
+};
+
+static int m31usb_phy_probe(struct platform_device *pdev)
+{
+	const struct m31_priv_data *data;
+	struct device *dev = &pdev->dev;
+	struct m31usb_phy *qphy;
+	struct phy_provider *phy_provider;
+	int ret;
+
+	qphy = devm_kzalloc(dev, sizeof(*qphy), GFP_KERNEL);
+	if (!qphy)
+		return -ENOMEM;
+
+	qphy->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(qphy->base))
+		return PTR_ERR(qphy->base);
+
+	qphy->phy_reset = devm_reset_control_get_exclusive_by_index(dev, 0);
+	if (IS_ERR(qphy->phy_reset))
+		return PTR_ERR(qphy->phy_reset);
+
+	qphy->cfg_ahb_clk = devm_clk_get(dev, "cfg_ahb");
+	if (IS_ERR(qphy->cfg_ahb_clk)) {
+		ret = PTR_ERR(qphy->cfg_ahb_clk);
+		dev_err(dev, "failed to get cfg_ahb clock, %d\n", ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, qphy);
+
+	data = of_device_get_match_data(dev);
+	qphy->regs		= data->regs;
+	qphy->nregs		= data->nregs;
+	qphy->ulpi_mode		= data->ulpi_mode;
+
+	qphy->phy = devm_phy_create(dev, NULL, &m31usb_phy_gen_ops);
+	if (IS_ERR(qphy->phy)) {
+		ret = PTR_ERR(qphy->phy);
+		dev_err(dev, "failed to create phy, %d\n", ret);
+		qphy->phy = NULL;
+		return ret;
+	}
+
+	dev_set_drvdata(dev, qphy);
+	phy_set_drvdata(qphy->phy, qphy);
+
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (!IS_ERR(phy_provider))
+		dev_info(dev, "Registered M31 USB phy\n");
+
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct m31_priv_data m31_ipq5332_data = {
+	.ulpi_mode = false,
+	.regs = m31_ipq5332_regs,
+	.nregs = ARRAY_SIZE(m31_ipq5332_regs),
+};
+
+static const struct of_device_id m31usb_phy_id_table[] = {
+	{ .compatible = "qcom,ipq5332-usb-hsphy", .data = &m31_ipq5332_data },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, m31usb_phy_id_table);
+
+static struct platform_driver m31usb_phy_driver = {
+	.probe		= m31usb_phy_probe,
+	.driver = {
+		.name	= "qcom-m31usb-phy",
+		.of_match_table = m31usb_phy_id_table,
+	},
+};
+
+module_platform_driver(m31usb_phy_driver);
+
+MODULE_DESCRIPTION("USB2 Qualcomm M31 HSPHY driver");
+MODULE_LICENSE("GPL");
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 3/6] phy: qcom-m31: Introduce qcom,m31 USB phy driver

[Varadarajan Narayanan]

Add the M31 USB2 phy driver.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v5:
	Kconfig and Makefile:- place snippet according to sorted order
	Use generic phy instead of usb-phy
	Use ARRAY_SIZE for reg init instead of blank last entry
	Fix copyright year

v4:
	Remove unused enum
	Error handling for devm_clk_get
v1:
	Combine driver, makefile and kconfig into 1 patch
	Remove 'qscratch' region and its usage. The controller driver takes care
	of those settings
	Use compatible/data to handle ipq5332 init
	Drop the default case
	Get resources by index instead of name as there is only one resource
	Add clock
	Fix review comments in the driver
---
 drivers/phy/qualcomm/Kconfig        |  11 ++
 drivers/phy/qualcomm/Makefile       |   1 +
 drivers/phy/qualcomm/phy-qcom-m31.c | 264 ++++++++++++++++++++++++++++++++++++
 3 files changed, 276 insertions(+)
 create mode 100644 drivers/phy/qualcomm/phy-qcom-m31.c

diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
index ced6038..d638906 100644
--- a/drivers/phy/qualcomm/Kconfig
+++ b/drivers/phy/qualcomm/Kconfig
@@ -143,6 +143,17 @@ config PHY_QCOM_EUSB2_REPEATER
 	  PMICs. The repeater is paired with a Synopsys eUSB2 Phy
 	  on Qualcomm SOCs.
 
+config PHY_QCOM_M31_USB
+	tristate "Qualcomm M31 HS PHY driver support"
+	depends on (USB || USB_GADGET) && ARCH_QCOM
+	select USB_PHY
+	help
+	  Enable this to support M31 HS PHY transceivers on Qualcomm chips
+	  with DWC3 USB core. It handles PHY initialization, clock
+	  management required after resetting the hardware and power
+	  management. This driver is required even for peripheral only or
+	  host only mode configurations.
+
 config PHY_QCOM_USB_HS
 	tristate "Qualcomm USB HS PHY module"
 	depends on USB_ULPI_BUS
diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
index df94581..ffd609a 100644
--- a/drivers/phy/qualcomm/Makefile
+++ b/drivers/phy/qualcomm/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)	+= phy-qcom-apq8064-sata.o
 obj-$(CONFIG_PHY_QCOM_EDP)		+= phy-qcom-edp.o
 obj-$(CONFIG_PHY_QCOM_IPQ4019_USB)	+= phy-qcom-ipq4019-usb.o
 obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)	+= phy-qcom-ipq806x-sata.o
+obj-$(CONFIG_PHY_QCOM_M31_USB)		+= phy-qcom-m31.o
 obj-$(CONFIG_PHY_QCOM_PCIE2)		+= phy-qcom-pcie2.o
 
 obj-$(CONFIG_PHY_QCOM_QMP_COMBO)	+= phy-qcom-qmp-combo.o
diff --git a/drivers/phy/qualcomm/phy-qcom-m31.c b/drivers/phy/qualcomm/phy-qcom-m31.c
new file mode 100644
index 0000000..4627425
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-m31.c
@@ -0,0 +1,264 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2014-2016, 2020-2023, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+#include <linux/usb/of.h>
+#include <linux/usb/phy.h>
+
+#define USB2PHY_PORT_UTMI_CTRL1		0x40
+
+#define USB2PHY_PORT_UTMI_CTRL2		0x44
+ #define UTMI_ULPI_SEL			BIT(7)
+ #define UTMI_TEST_MUX_SEL		BIT(6)
+
+#define HS_PHY_CTRL_REG			0x10
+ #define UTMI_OTG_VBUS_VALID		BIT(20)
+ #define SW_SESSVLD_SEL			BIT(28)
+
+#define USB_PHY_UTMI_CTRL0		0x3c
+
+#define USB_PHY_UTMI_CTRL5		0x50
+ #define POR_EN				BIT(1)
+
+#define USB_PHY_HS_PHY_CTRL_COMMON0	0x54
+ #define COMMONONN			BIT(7)
+ #define FSEL				BIT(4)
+ #define RETENABLEN			BIT(3)
+ #define FREQ_24MHZ			(GENMASK(6, 6) | GENMASK(4, 4))
+
+#define USB_PHY_HS_PHY_CTRL2		0x64
+ #define USB2_SUSPEND_N_SEL		BIT(3)
+ #define USB2_SUSPEND_N			BIT(2)
+ #define USB2_UTMI_CLK_EN		BIT(1)
+
+#define USB_PHY_CFG0			0x94
+ #define UTMI_PHY_OVERRIDE_EN		BIT(1)
+
+#define USB_PHY_REFCLK_CTRL		0xa0
+ #define CLKCORE			BIT(1)
+
+#define USB2PHY_PORT_POWERDOWN		0xa4
+ #define POWER_UP			BIT(0)
+ #define POWER_DOWN			0
+
+#define USB_PHY_FSEL_SEL		0xb8
+ #define FREQ_SEL			BIT(0)
+
+#define USB2PHY_USB_PHY_M31_XCFGI_1	0xbc
+ #define USB2_0_TX_ENABLE		BIT(2)
+
+#define USB2PHY_USB_PHY_M31_XCFGI_4	0xc8
+ #define HSTX_SLEW_RATE_565PS		GENMASK(1, 0)
+ #define PLL_CHARGING_PUMP_CURRENT_35UA	GENMASK(4, 3)
+ #define ODT_VALUE_38_02_OHM		GENMASK(7, 6)
+
+#define USB2PHY_USB_PHY_M31_XCFGI_5	0xcc
+ #define ODT_VALUE_45_02_OHM		BIT(2)
+ #define HSTX_PRE_EMPHASIS_LEVEL_0_55MA	BIT(0)
+
+#define USB2PHY_USB_PHY_M31_XCFGI_11	0xe4
+ #define XCFG_COARSE_TUNE_NUM		BIT(1)
+ #define XCFG_FINE_TUNE_NUM		BIT(3)
+
+struct m31_phy_regs {
+	u32 off;
+	u32 val;
+	u32 delay;
+};
+
+struct m31_priv_data {
+	bool				ulpi_mode;
+	const struct m31_phy_regs	*regs;
+	const int			nregs;
+};
+
+#define M31_REG(__o, __v, __d)	\
+	{ .off = __o, .val = __v, .delay = __d }
+
+struct m31_phy_regs m31_ipq5332_regs[] = {
+	M31_REG(USB_PHY_CFG0, UTMI_PHY_OVERRIDE_EN, 0),
+	M31_REG(USB_PHY_UTMI_CTRL5, POR_EN, 15),
+	M31_REG(USB_PHY_FSEL_SEL, FREQ_SEL, 0),
+	M31_REG(USB_PHY_HS_PHY_CTRL_COMMON0,
+		COMMONONN | FREQ_24MHZ | RETENABLEN, 0),
+	M31_REG(USB_PHY_UTMI_CTRL5, POR_EN, 0),
+	M31_REG(USB_PHY_HS_PHY_CTRL2,
+		USB2_SUSPEND_N_SEL | USB2_SUSPEND_N | USB2_UTMI_CLK_EN, 0),
+	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_11,
+		XCFG_COARSE_TUNE_NUM  | XCFG_FINE_TUNE_NUM, 0),
+	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_4,
+		HSTX_SLEW_RATE_565PS | PLL_CHARGING_PUMP_CURRENT_35UA |
+		ODT_VALUE_38_02_OHM, 0),
+	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_1, USB2_0_TX_ENABLE, 0),
+	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_5,
+		ODT_VALUE_45_02_OHM | HSTX_PRE_EMPHASIS_LEVEL_0_55MA, 4),
+	M31_REG(USB_PHY_UTMI_CTRL5, 0x0, 0),
+	M31_REG(USB_PHY_HS_PHY_CTRL2, USB2_SUSPEND_N | USB2_UTMI_CLK_EN, 0),
+};
+
+struct m31usb_phy {
+	struct phy			*phy;
+	void __iomem			*base;
+	const struct m31_phy_regs	*regs;
+	int				nregs;
+
+	struct clk			*cfg_ahb_clk;
+	struct reset_control		*phy_reset;
+
+	bool				cable_connected;
+	bool				suspended;
+	bool				ulpi_mode;
+};
+
+static void m31usb_phy_enable_clock(struct m31usb_phy *qphy)
+{
+	const struct m31_phy_regs *regs = qphy->regs;
+	int i;
+
+	for (i = 0; i < qphy->nregs; i++) {
+		writel(regs[i].val, qphy->base + regs[i].off);
+		if (regs[i].delay)
+			udelay(regs[i].delay);
+	}
+}
+
+static int m31usb_phy_init(struct phy *phy)
+{
+	int ret;
+	struct m31usb_phy *qphy = phy_get_drvdata(phy);
+
+	ret = clk_prepare_enable(qphy->cfg_ahb_clk);
+	if (ret) {
+		dev_err(&phy->dev, "failed to enable cfg ahb clock, %d\n", ret);
+		return ret;
+	}
+
+	/* Perform phy reset */
+	reset_control_assert(qphy->phy_reset);
+	udelay(5);
+	reset_control_deassert(qphy->phy_reset);
+
+	/* configure for ULPI mode if requested */
+	if (qphy->ulpi_mode)
+		writel(0x0, qphy->base + USB2PHY_PORT_UTMI_CTRL2);
+
+	/* Enable the PHY */
+	writel(POWER_UP, qphy->base + USB2PHY_PORT_POWERDOWN);
+
+	/* Make sure above write completed */
+	wmb();
+
+	/* Turn on phy ref clock */
+	m31usb_phy_enable_clock(qphy);
+
+	return 0;
+}
+
+static int m31usb_phy_shutdown(struct phy *phy)
+{
+	struct m31usb_phy *qphy = phy_get_drvdata(phy);
+
+	/* Disable the PHY */
+	writel_relaxed(POWER_DOWN, qphy->base + USB2PHY_PORT_POWERDOWN);
+	/* Make sure above write completed */
+	wmb();
+
+	clk_disable_unprepare(qphy->cfg_ahb_clk);
+
+	return 0;
+}
+
+static const struct phy_ops m31usb_phy_gen_ops = {
+	.power_on	= m31usb_phy_init,
+	.power_off	= m31usb_phy_shutdown,
+	.owner		= THIS_MODULE,
+};
+
+static int m31usb_phy_probe(struct platform_device *pdev)
+{
+	const struct m31_priv_data *data;
+	struct device *dev = &pdev->dev;
+	struct m31usb_phy *qphy;
+	struct phy_provider *phy_provider;
+	int ret;
+
+	qphy = devm_kzalloc(dev, sizeof(*qphy), GFP_KERNEL);
+	if (!qphy)
+		return -ENOMEM;
+
+	qphy->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(qphy->base))
+		return PTR_ERR(qphy->base);
+
+	qphy->phy_reset = devm_reset_control_get_exclusive_by_index(dev, 0);
+	if (IS_ERR(qphy->phy_reset))
+		return PTR_ERR(qphy->phy_reset);
+
+	qphy->cfg_ahb_clk = devm_clk_get(dev, "cfg_ahb");
+	if (IS_ERR(qphy->cfg_ahb_clk)) {
+		ret = PTR_ERR(qphy->cfg_ahb_clk);
+		dev_err(dev, "failed to get cfg_ahb clock, %d\n", ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, qphy);
+
+	data = of_device_get_match_data(dev);
+	qphy->regs		= data->regs;
+	qphy->nregs		= data->nregs;
+	qphy->ulpi_mode		= data->ulpi_mode;
+
+	qphy->phy = devm_phy_create(dev, NULL, &m31usb_phy_gen_ops);
+	if (IS_ERR(qphy->phy)) {
+		ret = PTR_ERR(qphy->phy);
+		dev_err(dev, "failed to create phy, %d\n", ret);
+		qphy->phy = NULL;
+		return ret;
+	}
+
+	dev_set_drvdata(dev, qphy);
+	phy_set_drvdata(qphy->phy, qphy);
+
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (!IS_ERR(phy_provider))
+		dev_info(dev, "Registered M31 USB phy\n");
+
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct m31_priv_data m31_ipq5332_data = {
+	.ulpi_mode = false,
+	.regs = m31_ipq5332_regs,
+	.nregs = ARRAY_SIZE(m31_ipq5332_regs),
+};
+
+static const struct of_device_id m31usb_phy_id_table[] = {
+	{ .compatible = "qcom,ipq5332-usb-hsphy", .data = &m31_ipq5332_data },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, m31usb_phy_id_table);
+
+static struct platform_driver m31usb_phy_driver = {
+	.probe		= m31usb_phy_probe,
+	.driver = {
+		.name	= "qcom-m31usb-phy",
+		.of_match_table = m31usb_phy_id_table,
+	},
+};
+
+module_platform_driver(m31usb_phy_driver);
+
+MODULE_DESCRIPTION("USB2 Qualcomm M31 HSPHY driver");
+MODULE_LICENSE("GPL");
-- 
2.7.4


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 3/6] phy: qcom-m31: Introduce qcom,m31 USB phy driver

[Bjorn Andersson]

On Fri, Jul 21, 2023 at 10:05:28AM +0530, Varadarajan Narayanan wrote:
> Add the M31 USB2 phy driver.

Is it possible to extend this description a little bit? E.g. mention
where this PHY is used.

> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v5:
> 	Kconfig and Makefile:- place snippet according to sorted order
> 	Use generic phy instead of usb-phy
> 	Use ARRAY_SIZE for reg init instead of blank last entry
> 	Fix copyright year
> 
> v4:
> 	Remove unused enum
> 	Error handling for devm_clk_get
> v1:
> 	Combine driver, makefile and kconfig into 1 patch
> 	Remove 'qscratch' region and its usage. The controller driver takes care
> 	of those settings
> 	Use compatible/data to handle ipq5332 init
> 	Drop the default case
> 	Get resources by index instead of name as there is only one resource
> 	Add clock
> 	Fix review comments in the driver
> ---
>  drivers/phy/qualcomm/Kconfig        |  11 ++
>  drivers/phy/qualcomm/Makefile       |   1 +
>  drivers/phy/qualcomm/phy-qcom-m31.c | 264 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 276 insertions(+)
>  create mode 100644 drivers/phy/qualcomm/phy-qcom-m31.c
> 
> diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
> index ced6038..d638906 100644
> --- a/drivers/phy/qualcomm/Kconfig
> +++ b/drivers/phy/qualcomm/Kconfig
> @@ -143,6 +143,17 @@ config PHY_QCOM_EUSB2_REPEATER
>  	  PMICs. The repeater is paired with a Synopsys eUSB2 Phy
>  	  on Qualcomm SOCs.
>  
> +config PHY_QCOM_M31_USB
> +	tristate "Qualcomm M31 HS PHY driver support"
> +	depends on (USB || USB_GADGET) && ARCH_QCOM

The USB part seems reasonable, but it's not clear to me why you depend
on USB_GADGET. And this is the only option in this directory that
depends on it's consumer subsystem...

It is reasonable help the user not choose the driver if ARCH_QCOM is
disabled, but most other options in this file allow compilation during
COMPILE_TEST. This provides additional build coverage, so perhaps you
could adopt this as well?

> +	select USB_PHY

USB_PHY is for drivers in drivers/usb/phy, but you're using the generic
PHY framework. Is this really needed, or just a remnant from downstream?

You should however select GENERIC_PHY, like all other options in this
file.

> +	help
> +	  Enable this to support M31 HS PHY transceivers on Qualcomm chips
> +	  with DWC3 USB core. It handles PHY initialization, clock
> +	  management required after resetting the hardware and power
> +	  management. This driver is required even for peripheral only or
> +	  host only mode configurations.
> +
>  config PHY_QCOM_USB_HS
>  	tristate "Qualcomm USB HS PHY module"
>  	depends on USB_ULPI_BUS
> diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
> index df94581..ffd609a 100644
> --- a/drivers/phy/qualcomm/Makefile
> +++ b/drivers/phy/qualcomm/Makefile
> @@ -4,6 +4,7 @@ obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)	+= phy-qcom-apq8064-sata.o
>  obj-$(CONFIG_PHY_QCOM_EDP)		+= phy-qcom-edp.o
>  obj-$(CONFIG_PHY_QCOM_IPQ4019_USB)	+= phy-qcom-ipq4019-usb.o
>  obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)	+= phy-qcom-ipq806x-sata.o
> +obj-$(CONFIG_PHY_QCOM_M31_USB)		+= phy-qcom-m31.o
>  obj-$(CONFIG_PHY_QCOM_PCIE2)		+= phy-qcom-pcie2.o
>  
>  obj-$(CONFIG_PHY_QCOM_QMP_COMBO)	+= phy-qcom-qmp-combo.o
> diff --git a/drivers/phy/qualcomm/phy-qcom-m31.c b/drivers/phy/qualcomm/phy-qcom-m31.c
> new file mode 100644
> index 0000000..4627425
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-m31.c
> @@ -0,0 +1,264 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2014-2016, 2020-2023, The Linux Foundation. All rights reserved.

Please fix this.

> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>

Rob Herring has posted a bunch of patches lately removing uses of
of_device.h, so please don't introduce a new one.

of.h and of_platform.h are to be used instead.

> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +#include <linux/usb/of.h>
> +#include <linux/usb/phy.h>

I don't think you need the last two, in a generic phy driver.

> +
> +#define USB2PHY_PORT_UTMI_CTRL1		0x40
> +
> +#define USB2PHY_PORT_UTMI_CTRL2		0x44
> + #define UTMI_ULPI_SEL			BIT(7)
> + #define UTMI_TEST_MUX_SEL		BIT(6)
> +
> +#define HS_PHY_CTRL_REG			0x10
> + #define UTMI_OTG_VBUS_VALID		BIT(20)
> + #define SW_SESSVLD_SEL			BIT(28)
> +
> +#define USB_PHY_UTMI_CTRL0		0x3c
> +
> +#define USB_PHY_UTMI_CTRL5		0x50
> + #define POR_EN				BIT(1)
> +
> +#define USB_PHY_HS_PHY_CTRL_COMMON0	0x54
> + #define COMMONONN			BIT(7)
> + #define FSEL				BIT(4)
> + #define RETENABLEN			BIT(3)
> + #define FREQ_24MHZ			(GENMASK(6, 6) | GENMASK(4, 4))
> +
> +#define USB_PHY_HS_PHY_CTRL2		0x64
> + #define USB2_SUSPEND_N_SEL		BIT(3)
> + #define USB2_SUSPEND_N			BIT(2)
> + #define USB2_UTMI_CLK_EN		BIT(1)
> +
> +#define USB_PHY_CFG0			0x94
> + #define UTMI_PHY_OVERRIDE_EN		BIT(1)
> +
> +#define USB_PHY_REFCLK_CTRL		0xa0
> + #define CLKCORE			BIT(1)
> +
> +#define USB2PHY_PORT_POWERDOWN		0xa4
> + #define POWER_UP			BIT(0)
> + #define POWER_DOWN			0
> +
> +#define USB_PHY_FSEL_SEL		0xb8
> + #define FREQ_SEL			BIT(0)
> +
> +#define USB2PHY_USB_PHY_M31_XCFGI_1	0xbc
> + #define USB2_0_TX_ENABLE		BIT(2)
> +
> +#define USB2PHY_USB_PHY_M31_XCFGI_4	0xc8
> + #define HSTX_SLEW_RATE_565PS		GENMASK(1, 0)
> + #define PLL_CHARGING_PUMP_CURRENT_35UA	GENMASK(4, 3)
> + #define ODT_VALUE_38_02_OHM		GENMASK(7, 6)
> +
> +#define USB2PHY_USB_PHY_M31_XCFGI_5	0xcc
> + #define ODT_VALUE_45_02_OHM		BIT(2)
> + #define HSTX_PRE_EMPHASIS_LEVEL_0_55MA	BIT(0)
> +
> +#define USB2PHY_USB_PHY_M31_XCFGI_11	0xe4
> + #define XCFG_COARSE_TUNE_NUM		BIT(1)
> + #define XCFG_FINE_TUNE_NUM		BIT(3)
> +
> +struct m31_phy_regs {
> +	u32 off;
> +	u32 val;
> +	u32 delay;
> +};
> +
> +struct m31_priv_data {
> +	bool				ulpi_mode;
> +	const struct m31_phy_regs	*regs;
> +	const int			nregs;

Doesn't hurt to spell out that this is an unsigned int, and you don't
need the const on the nregs.

> +};
> +
> +#define M31_REG(__o, __v, __d)	\
> +	{ .off = __o, .val = __v, .delay = __d }

This macro doesn't add any value.

Compare:
	M31_REG(USB_PHY_CFG0, UTMI_PHY_OVERRIDE_EN, 0),
with:
	{ USB_PHY_CFG0, UTMI_PHY_OVERRIDE_EN, 0 },

> +
> +struct m31_phy_regs m31_ipq5332_regs[] = {
> +	M31_REG(USB_PHY_CFG0, UTMI_PHY_OVERRIDE_EN, 0),
> +	M31_REG(USB_PHY_UTMI_CTRL5, POR_EN, 15),
> +	M31_REG(USB_PHY_FSEL_SEL, FREQ_SEL, 0),
> +	M31_REG(USB_PHY_HS_PHY_CTRL_COMMON0,
> +		COMMONONN | FREQ_24MHZ | RETENABLEN, 0),

The guideline is to keep under 80 characters, but if it improves
readability you may use up to 100. So this line can be unwrapped.

> +	M31_REG(USB_PHY_UTMI_CTRL5, POR_EN, 0),
> +	M31_REG(USB_PHY_HS_PHY_CTRL2,
> +		USB2_SUSPEND_N_SEL | USB2_SUSPEND_N | USB2_UTMI_CLK_EN, 0),

This one as well.

> +	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_11,
> +		XCFG_COARSE_TUNE_NUM  | XCFG_FINE_TUNE_NUM, 0),
> +	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_4,
> +		HSTX_SLEW_RATE_565PS | PLL_CHARGING_PUMP_CURRENT_35UA |
> +		ODT_VALUE_38_02_OHM, 0),
> +	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_1, USB2_0_TX_ENABLE, 0),
> +	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_5,
> +		ODT_VALUE_45_02_OHM | HSTX_PRE_EMPHASIS_LEVEL_0_55MA, 4),

And this one, if you drop the M32_REG macro.

> +	M31_REG(USB_PHY_UTMI_CTRL5, 0x0, 0),
> +	M31_REG(USB_PHY_HS_PHY_CTRL2, USB2_SUSPEND_N | USB2_UTMI_CLK_EN, 0),
> +};
> +
> +struct m31usb_phy {
> +	struct phy			*phy;
> +	void __iomem			*base;
> +	const struct m31_phy_regs	*regs;
> +	int				nregs;
> +
> +	struct clk			*cfg_ahb_clk;

If you only have one clock, you can name it "clk" without causing
confusion.

> +	struct reset_control		*phy_reset;

Same here, if there's only one reset, then "reset" is a sufficient name.

> +
> +	bool				cable_connected;

Unused

> +	bool				suspended;

Unused, are you missing/omitting some relevant code here?

> +	bool				ulpi_mode;
> +};
> +
> +static void m31usb_phy_enable_clock(struct m31usb_phy *qphy)

It wouldn't hurt to just move this code into m31usb_phy_init().

> +{
> +	const struct m31_phy_regs *regs = qphy->regs;
> +	int i;
> +
> +	for (i = 0; i < qphy->nregs; i++) {
> +		writel(regs[i].val, qphy->base + regs[i].off);
> +		if (regs[i].delay)
> +			udelay(regs[i].delay);
> +	}
> +}
> +
> +static int m31usb_phy_init(struct phy *phy)
> +{
> +	int ret;
> +	struct m31usb_phy *qphy = phy_get_drvdata(phy);
> +
> +	ret = clk_prepare_enable(qphy->cfg_ahb_clk);
> +	if (ret) {
> +		dev_err(&phy->dev, "failed to enable cfg ahb clock, %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Perform phy reset */
> +	reset_control_assert(qphy->phy_reset);
> +	udelay(5);
> +	reset_control_deassert(qphy->phy_reset);
> +
> +	/* configure for ULPI mode if requested */
> +	if (qphy->ulpi_mode)
> +		writel(0x0, qphy->base + USB2PHY_PORT_UTMI_CTRL2);
> +
> +	/* Enable the PHY */
> +	writel(POWER_UP, qphy->base + USB2PHY_PORT_POWERDOWN);
> +
> +	/* Make sure above write completed */

That's not what wmb() does. It ensures that any writes prior to this
line is completed before any writes after this.

But m31usb_phy_enable_clock() is using writel(), which does the same
already.

If you really need the previous writes to have completed (get through
the write buffer etc), then you need to readl() the same register back.
But I see no reason for that.

> +	wmb();

So I think this should go.

> +
> +	/* Turn on phy ref clock */
> +	m31usb_phy_enable_clock(qphy);
> +
> +	return 0;
> +}
> +
> +static int m31usb_phy_shutdown(struct phy *phy)
> +{
> +	struct m31usb_phy *qphy = phy_get_drvdata(phy);
> +
> +	/* Disable the PHY */
> +	writel_relaxed(POWER_DOWN, qphy->base + USB2PHY_PORT_POWERDOWN);
> +	/* Make sure above write completed */
> +	wmb();

Same as above.

Although here you would assume that clk_disable_unprepare() uses
writel() and doesn't overtake your power-down write, but that sounds
like reasonable assumptions to make.

> +
> +	clk_disable_unprepare(qphy->cfg_ahb_clk);
> +
> +	return 0;
> +}
> +
> +static const struct phy_ops m31usb_phy_gen_ops = {
> +	.power_on	= m31usb_phy_init,
> +	.power_off	= m31usb_phy_shutdown,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static int m31usb_phy_probe(struct platform_device *pdev)
> +{
> +	const struct m31_priv_data *data;
> +	struct device *dev = &pdev->dev;
> +	struct m31usb_phy *qphy;
> +	struct phy_provider *phy_provider;
> +	int ret;

Please sort these in descending length order (reverse xmas tree).

> +
> +	qphy = devm_kzalloc(dev, sizeof(*qphy), GFP_KERNEL);
> +	if (!qphy)
> +		return -ENOMEM;
> +
> +	qphy->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(qphy->base))
> +		return PTR_ERR(qphy->base);
> +
> +	qphy->phy_reset = devm_reset_control_get_exclusive_by_index(dev, 0);
> +	if (IS_ERR(qphy->phy_reset))
> +		return PTR_ERR(qphy->phy_reset);
> +
> +	qphy->cfg_ahb_clk = devm_clk_get(dev, "cfg_ahb");

If you only have one clock, then omitting clock-names and do
devm_clk_get(dev, NULL) works well.

> +	if (IS_ERR(qphy->cfg_ahb_clk)) {
> +		ret = PTR_ERR(qphy->cfg_ahb_clk);
> +		dev_err(dev, "failed to get cfg_ahb clock, %d\n", ret);
> +		return ret;

Replace these tree lines with:

return dev_err_probe(dev, PTR_ERR(qphy->cfg_ahb_clk), "failed to get cfg_ahb_clk\n");

> +	}
> +
> +	platform_set_drvdata(pdev, qphy);

There are no platform_get_drvdata() calls in this driver, so you can
omit this.

> +
> +	data = of_device_get_match_data(dev);
> +	qphy->regs		= data->regs;
> +	qphy->nregs		= data->nregs;
> +	qphy->ulpi_mode		= data->ulpi_mode;
> +
> +	qphy->phy = devm_phy_create(dev, NULL, &m31usb_phy_gen_ops);
> +	if (IS_ERR(qphy->phy)) {
> +		ret = PTR_ERR(qphy->phy);
> +		dev_err(dev, "failed to create phy, %d\n", ret);
> +		qphy->phy = NULL;

qphy is inaccessible, and freed after the next line, no need to clear
this.

> +		return ret;
> +	}
> +
> +	dev_set_drvdata(dev, qphy);

This is exactly he same as platform_set_drvdata() above, so per the same
logic you can drop this as well.

> +	phy_set_drvdata(qphy->phy, qphy);
> +
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	if (!IS_ERR(phy_provider))
> +		dev_info(dev, "Registered M31 USB phy\n");
> +
> +	return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static const struct m31_priv_data m31_ipq5332_data = {
> +	.ulpi_mode = false,
> +	.regs = m31_ipq5332_regs,
> +	.nregs = ARRAY_SIZE(m31_ipq5332_regs),
> +};
> +
> +static const struct of_device_id m31usb_phy_id_table[] = {
> +	{ .compatible = "qcom,ipq5332-usb-hsphy", .data = &m31_ipq5332_data },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, m31usb_phy_id_table);
> +
> +static struct platform_driver m31usb_phy_driver = {
> +	.probe		= m31usb_phy_probe,

Please drop the indentation in these.

Regards,
Bjorn

Re: [PATCH v5 3/6] phy: qcom-m31: Introduce qcom,m31 USB phy driver

[Bjorn Andersson]

On Fri, Jul 21, 2023 at 10:05:28AM +0530, Varadarajan Narayanan wrote:
> Add the M31 USB2 phy driver.

Is it possible to extend this description a little bit? E.g. mention
where this PHY is used.

> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v5:
> 	Kconfig and Makefile:- place snippet according to sorted order
> 	Use generic phy instead of usb-phy
> 	Use ARRAY_SIZE for reg init instead of blank last entry
> 	Fix copyright year
> 
> v4:
> 	Remove unused enum
> 	Error handling for devm_clk_get
> v1:
> 	Combine driver, makefile and kconfig into 1 patch
> 	Remove 'qscratch' region and its usage. The controller driver takes care
> 	of those settings
> 	Use compatible/data to handle ipq5332 init
> 	Drop the default case
> 	Get resources by index instead of name as there is only one resource
> 	Add clock
> 	Fix review comments in the driver
> ---
>  drivers/phy/qualcomm/Kconfig        |  11 ++
>  drivers/phy/qualcomm/Makefile       |   1 +
>  drivers/phy/qualcomm/phy-qcom-m31.c | 264 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 276 insertions(+)
>  create mode 100644 drivers/phy/qualcomm/phy-qcom-m31.c
> 
> diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
> index ced6038..d638906 100644
> --- a/drivers/phy/qualcomm/Kconfig
> +++ b/drivers/phy/qualcomm/Kconfig
> @@ -143,6 +143,17 @@ config PHY_QCOM_EUSB2_REPEATER
>  	  PMICs. The repeater is paired with a Synopsys eUSB2 Phy
>  	  on Qualcomm SOCs.
>  
> +config PHY_QCOM_M31_USB
> +	tristate "Qualcomm M31 HS PHY driver support"
> +	depends on (USB || USB_GADGET) && ARCH_QCOM

The USB part seems reasonable, but it's not clear to me why you depend
on USB_GADGET. And this is the only option in this directory that
depends on it's consumer subsystem...

It is reasonable help the user not choose the driver if ARCH_QCOM is
disabled, but most other options in this file allow compilation during
COMPILE_TEST. This provides additional build coverage, so perhaps you
could adopt this as well?

> +	select USB_PHY

USB_PHY is for drivers in drivers/usb/phy, but you're using the generic
PHY framework. Is this really needed, or just a remnant from downstream?

You should however select GENERIC_PHY, like all other options in this
file.

> +	help
> +	  Enable this to support M31 HS PHY transceivers on Qualcomm chips
> +	  with DWC3 USB core. It handles PHY initialization, clock
> +	  management required after resetting the hardware and power
> +	  management. This driver is required even for peripheral only or
> +	  host only mode configurations.
> +
>  config PHY_QCOM_USB_HS
>  	tristate "Qualcomm USB HS PHY module"
>  	depends on USB_ULPI_BUS
> diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
> index df94581..ffd609a 100644
> --- a/drivers/phy/qualcomm/Makefile
> +++ b/drivers/phy/qualcomm/Makefile
> @@ -4,6 +4,7 @@ obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)	+= phy-qcom-apq8064-sata.o
>  obj-$(CONFIG_PHY_QCOM_EDP)		+= phy-qcom-edp.o
>  obj-$(CONFIG_PHY_QCOM_IPQ4019_USB)	+= phy-qcom-ipq4019-usb.o
>  obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)	+= phy-qcom-ipq806x-sata.o
> +obj-$(CONFIG_PHY_QCOM_M31_USB)		+= phy-qcom-m31.o
>  obj-$(CONFIG_PHY_QCOM_PCIE2)		+= phy-qcom-pcie2.o
>  
>  obj-$(CONFIG_PHY_QCOM_QMP_COMBO)	+= phy-qcom-qmp-combo.o
> diff --git a/drivers/phy/qualcomm/phy-qcom-m31.c b/drivers/phy/qualcomm/phy-qcom-m31.c
> new file mode 100644
> index 0000000..4627425
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-m31.c
> @@ -0,0 +1,264 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2014-2016, 2020-2023, The Linux Foundation. All rights reserved.

Please fix this.

> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>

Rob Herring has posted a bunch of patches lately removing uses of
of_device.h, so please don't introduce a new one.

of.h and of_platform.h are to be used instead.

> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +#include <linux/usb/of.h>
> +#include <linux/usb/phy.h>

I don't think you need the last two, in a generic phy driver.

> +
> +#define USB2PHY_PORT_UTMI_CTRL1		0x40
> +
> +#define USB2PHY_PORT_UTMI_CTRL2		0x44
> + #define UTMI_ULPI_SEL			BIT(7)
> + #define UTMI_TEST_MUX_SEL		BIT(6)
> +
> +#define HS_PHY_CTRL_REG			0x10
> + #define UTMI_OTG_VBUS_VALID		BIT(20)
> + #define SW_SESSVLD_SEL			BIT(28)
> +
> +#define USB_PHY_UTMI_CTRL0		0x3c
> +
> +#define USB_PHY_UTMI_CTRL5		0x50
> + #define POR_EN				BIT(1)
> +
> +#define USB_PHY_HS_PHY_CTRL_COMMON0	0x54
> + #define COMMONONN			BIT(7)
> + #define FSEL				BIT(4)
> + #define RETENABLEN			BIT(3)
> + #define FREQ_24MHZ			(GENMASK(6, 6) | GENMASK(4, 4))
> +
> +#define USB_PHY_HS_PHY_CTRL2		0x64
> + #define USB2_SUSPEND_N_SEL		BIT(3)
> + #define USB2_SUSPEND_N			BIT(2)
> + #define USB2_UTMI_CLK_EN		BIT(1)
> +
> +#define USB_PHY_CFG0			0x94
> + #define UTMI_PHY_OVERRIDE_EN		BIT(1)
> +
> +#define USB_PHY_REFCLK_CTRL		0xa0
> + #define CLKCORE			BIT(1)
> +
> +#define USB2PHY_PORT_POWERDOWN		0xa4
> + #define POWER_UP			BIT(0)
> + #define POWER_DOWN			0
> +
> +#define USB_PHY_FSEL_SEL		0xb8
> + #define FREQ_SEL			BIT(0)
> +
> +#define USB2PHY_USB_PHY_M31_XCFGI_1	0xbc
> + #define USB2_0_TX_ENABLE		BIT(2)
> +
> +#define USB2PHY_USB_PHY_M31_XCFGI_4	0xc8
> + #define HSTX_SLEW_RATE_565PS		GENMASK(1, 0)
> + #define PLL_CHARGING_PUMP_CURRENT_35UA	GENMASK(4, 3)
> + #define ODT_VALUE_38_02_OHM		GENMASK(7, 6)
> +
> +#define USB2PHY_USB_PHY_M31_XCFGI_5	0xcc
> + #define ODT_VALUE_45_02_OHM		BIT(2)
> + #define HSTX_PRE_EMPHASIS_LEVEL_0_55MA	BIT(0)
> +
> +#define USB2PHY_USB_PHY_M31_XCFGI_11	0xe4
> + #define XCFG_COARSE_TUNE_NUM		BIT(1)
> + #define XCFG_FINE_TUNE_NUM		BIT(3)
> +
> +struct m31_phy_regs {
> +	u32 off;
> +	u32 val;
> +	u32 delay;
> +};
> +
> +struct m31_priv_data {
> +	bool				ulpi_mode;
> +	const struct m31_phy_regs	*regs;
> +	const int			nregs;

Doesn't hurt to spell out that this is an unsigned int, and you don't
need the const on the nregs.

> +};
> +
> +#define M31_REG(__o, __v, __d)	\
> +	{ .off = __o, .val = __v, .delay = __d }

This macro doesn't add any value.

Compare:
	M31_REG(USB_PHY_CFG0, UTMI_PHY_OVERRIDE_EN, 0),
with:
	{ USB_PHY_CFG0, UTMI_PHY_OVERRIDE_EN, 0 },

> +
> +struct m31_phy_regs m31_ipq5332_regs[] = {
> +	M31_REG(USB_PHY_CFG0, UTMI_PHY_OVERRIDE_EN, 0),
> +	M31_REG(USB_PHY_UTMI_CTRL5, POR_EN, 15),
> +	M31_REG(USB_PHY_FSEL_SEL, FREQ_SEL, 0),
> +	M31_REG(USB_PHY_HS_PHY_CTRL_COMMON0,
> +		COMMONONN | FREQ_24MHZ | RETENABLEN, 0),

The guideline is to keep under 80 characters, but if it improves
readability you may use up to 100. So this line can be unwrapped.

> +	M31_REG(USB_PHY_UTMI_CTRL5, POR_EN, 0),
> +	M31_REG(USB_PHY_HS_PHY_CTRL2,
> +		USB2_SUSPEND_N_SEL | USB2_SUSPEND_N | USB2_UTMI_CLK_EN, 0),

This one as well.

> +	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_11,
> +		XCFG_COARSE_TUNE_NUM  | XCFG_FINE_TUNE_NUM, 0),
> +	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_4,
> +		HSTX_SLEW_RATE_565PS | PLL_CHARGING_PUMP_CURRENT_35UA |
> +		ODT_VALUE_38_02_OHM, 0),
> +	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_1, USB2_0_TX_ENABLE, 0),
> +	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_5,
> +		ODT_VALUE_45_02_OHM | HSTX_PRE_EMPHASIS_LEVEL_0_55MA, 4),

And this one, if you drop the M32_REG macro.

> +	M31_REG(USB_PHY_UTMI_CTRL5, 0x0, 0),
> +	M31_REG(USB_PHY_HS_PHY_CTRL2, USB2_SUSPEND_N | USB2_UTMI_CLK_EN, 0),
> +};
> +
> +struct m31usb_phy {
> +	struct phy			*phy;
> +	void __iomem			*base;
> +	const struct m31_phy_regs	*regs;
> +	int				nregs;
> +
> +	struct clk			*cfg_ahb_clk;

If you only have one clock, you can name it "clk" without causing
confusion.

> +	struct reset_control		*phy_reset;

Same here, if there's only one reset, then "reset" is a sufficient name.

> +
> +	bool				cable_connected;

Unused

> +	bool				suspended;

Unused, are you missing/omitting some relevant code here?

> +	bool				ulpi_mode;
> +};
> +
> +static void m31usb_phy_enable_clock(struct m31usb_phy *qphy)

It wouldn't hurt to just move this code into m31usb_phy_init().

> +{
> +	const struct m31_phy_regs *regs = qphy->regs;
> +	int i;
> +
> +	for (i = 0; i < qphy->nregs; i++) {
> +		writel(regs[i].val, qphy->base + regs[i].off);
> +		if (regs[i].delay)
> +			udelay(regs[i].delay);
> +	}
> +}
> +
> +static int m31usb_phy_init(struct phy *phy)
> +{
> +	int ret;
> +	struct m31usb_phy *qphy = phy_get_drvdata(phy);
> +
> +	ret = clk_prepare_enable(qphy->cfg_ahb_clk);
> +	if (ret) {
> +		dev_err(&phy->dev, "failed to enable cfg ahb clock, %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Perform phy reset */
> +	reset_control_assert(qphy->phy_reset);
> +	udelay(5);
> +	reset_control_deassert(qphy->phy_reset);
> +
> +	/* configure for ULPI mode if requested */
> +	if (qphy->ulpi_mode)
> +		writel(0x0, qphy->base + USB2PHY_PORT_UTMI_CTRL2);
> +
> +	/* Enable the PHY */
> +	writel(POWER_UP, qphy->base + USB2PHY_PORT_POWERDOWN);
> +
> +	/* Make sure above write completed */

That's not what wmb() does. It ensures that any writes prior to this
line is completed before any writes after this.

But m31usb_phy_enable_clock() is using writel(), which does the same
already.

If you really need the previous writes to have completed (get through
the write buffer etc), then you need to readl() the same register back.
But I see no reason for that.

> +	wmb();

So I think this should go.

> +
> +	/* Turn on phy ref clock */
> +	m31usb_phy_enable_clock(qphy);
> +
> +	return 0;
> +}
> +
> +static int m31usb_phy_shutdown(struct phy *phy)
> +{
> +	struct m31usb_phy *qphy = phy_get_drvdata(phy);
> +
> +	/* Disable the PHY */
> +	writel_relaxed(POWER_DOWN, qphy->base + USB2PHY_PORT_POWERDOWN);
> +	/* Make sure above write completed */
> +	wmb();

Same as above.

Although here you would assume that clk_disable_unprepare() uses
writel() and doesn't overtake your power-down write, but that sounds
like reasonable assumptions to make.

> +
> +	clk_disable_unprepare(qphy->cfg_ahb_clk);
> +
> +	return 0;
> +}
> +
> +static const struct phy_ops m31usb_phy_gen_ops = {
> +	.power_on	= m31usb_phy_init,
> +	.power_off	= m31usb_phy_shutdown,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static int m31usb_phy_probe(struct platform_device *pdev)
> +{
> +	const struct m31_priv_data *data;
> +	struct device *dev = &pdev->dev;
> +	struct m31usb_phy *qphy;
> +	struct phy_provider *phy_provider;
> +	int ret;

Please sort these in descending length order (reverse xmas tree).

> +
> +	qphy = devm_kzalloc(dev, sizeof(*qphy), GFP_KERNEL);
> +	if (!qphy)
> +		return -ENOMEM;
> +
> +	qphy->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(qphy->base))
> +		return PTR_ERR(qphy->base);
> +
> +	qphy->phy_reset = devm_reset_control_get_exclusive_by_index(dev, 0);
> +	if (IS_ERR(qphy->phy_reset))
> +		return PTR_ERR(qphy->phy_reset);
> +
> +	qphy->cfg_ahb_clk = devm_clk_get(dev, "cfg_ahb");

If you only have one clock, then omitting clock-names and do
devm_clk_get(dev, NULL) works well.

> +	if (IS_ERR(qphy->cfg_ahb_clk)) {
> +		ret = PTR_ERR(qphy->cfg_ahb_clk);
> +		dev_err(dev, "failed to get cfg_ahb clock, %d\n", ret);
> +		return ret;

Replace these tree lines with:

return dev_err_probe(dev, PTR_ERR(qphy->cfg_ahb_clk), "failed to get cfg_ahb_clk\n");

> +	}
> +
> +	platform_set_drvdata(pdev, qphy);

There are no platform_get_drvdata() calls in this driver, so you can
omit this.

> +
> +	data = of_device_get_match_data(dev);
> +	qphy->regs		= data->regs;
> +	qphy->nregs		= data->nregs;
> +	qphy->ulpi_mode		= data->ulpi_mode;
> +
> +	qphy->phy = devm_phy_create(dev, NULL, &m31usb_phy_gen_ops);
> +	if (IS_ERR(qphy->phy)) {
> +		ret = PTR_ERR(qphy->phy);
> +		dev_err(dev, "failed to create phy, %d\n", ret);
> +		qphy->phy = NULL;

qphy is inaccessible, and freed after the next line, no need to clear
this.

> +		return ret;
> +	}
> +
> +	dev_set_drvdata(dev, qphy);

This is exactly he same as platform_set_drvdata() above, so per the same
logic you can drop this as well.

> +	phy_set_drvdata(qphy->phy, qphy);
> +
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	if (!IS_ERR(phy_provider))
> +		dev_info(dev, "Registered M31 USB phy\n");
> +
> +	return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static const struct m31_priv_data m31_ipq5332_data = {
> +	.ulpi_mode = false,
> +	.regs = m31_ipq5332_regs,
> +	.nregs = ARRAY_SIZE(m31_ipq5332_regs),
> +};
> +
> +static const struct of_device_id m31usb_phy_id_table[] = {
> +	{ .compatible = "qcom,ipq5332-usb-hsphy", .data = &m31_ipq5332_data },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, m31usb_phy_id_table);
> +
> +static struct platform_driver m31usb_phy_driver = {
> +	.probe		= m31usb_phy_probe,

Please drop the indentation in these.

Regards,
Bjorn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 3/6] phy: qcom-m31: Introduce qcom,m31 USB phy driver

[Bjorn Andersson]

On Fri, Jul 21, 2023 at 10:05:28AM +0530, Varadarajan Narayanan wrote:
> Add the M31 USB2 phy driver.

Is it possible to extend this description a little bit? E.g. mention
where this PHY is used.

> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v5:
> 	Kconfig and Makefile:- place snippet according to sorted order
> 	Use generic phy instead of usb-phy
> 	Use ARRAY_SIZE for reg init instead of blank last entry
> 	Fix copyright year
> 
> v4:
> 	Remove unused enum
> 	Error handling for devm_clk_get
> v1:
> 	Combine driver, makefile and kconfig into 1 patch
> 	Remove 'qscratch' region and its usage. The controller driver takes care
> 	of those settings
> 	Use compatible/data to handle ipq5332 init
> 	Drop the default case
> 	Get resources by index instead of name as there is only one resource
> 	Add clock
> 	Fix review comments in the driver
> ---
>  drivers/phy/qualcomm/Kconfig        |  11 ++
>  drivers/phy/qualcomm/Makefile       |   1 +
>  drivers/phy/qualcomm/phy-qcom-m31.c | 264 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 276 insertions(+)
>  create mode 100644 drivers/phy/qualcomm/phy-qcom-m31.c
> 
> diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
> index ced6038..d638906 100644
> --- a/drivers/phy/qualcomm/Kconfig
> +++ b/drivers/phy/qualcomm/Kconfig
> @@ -143,6 +143,17 @@ config PHY_QCOM_EUSB2_REPEATER
>  	  PMICs. The repeater is paired with a Synopsys eUSB2 Phy
>  	  on Qualcomm SOCs.
>  
> +config PHY_QCOM_M31_USB
> +	tristate "Qualcomm M31 HS PHY driver support"
> +	depends on (USB || USB_GADGET) && ARCH_QCOM

The USB part seems reasonable, but it's not clear to me why you depend
on USB_GADGET. And this is the only option in this directory that
depends on it's consumer subsystem...

It is reasonable help the user not choose the driver if ARCH_QCOM is
disabled, but most other options in this file allow compilation during
COMPILE_TEST. This provides additional build coverage, so perhaps you
could adopt this as well?

> +	select USB_PHY

USB_PHY is for drivers in drivers/usb/phy, but you're using the generic
PHY framework. Is this really needed, or just a remnant from downstream?

You should however select GENERIC_PHY, like all other options in this
file.

> +	help
> +	  Enable this to support M31 HS PHY transceivers on Qualcomm chips
> +	  with DWC3 USB core. It handles PHY initialization, clock
> +	  management required after resetting the hardware and power
> +	  management. This driver is required even for peripheral only or
> +	  host only mode configurations.
> +
>  config PHY_QCOM_USB_HS
>  	tristate "Qualcomm USB HS PHY module"
>  	depends on USB_ULPI_BUS
> diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
> index df94581..ffd609a 100644
> --- a/drivers/phy/qualcomm/Makefile
> +++ b/drivers/phy/qualcomm/Makefile
> @@ -4,6 +4,7 @@ obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)	+= phy-qcom-apq8064-sata.o
>  obj-$(CONFIG_PHY_QCOM_EDP)		+= phy-qcom-edp.o
>  obj-$(CONFIG_PHY_QCOM_IPQ4019_USB)	+= phy-qcom-ipq4019-usb.o
>  obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)	+= phy-qcom-ipq806x-sata.o
> +obj-$(CONFIG_PHY_QCOM_M31_USB)		+= phy-qcom-m31.o
>  obj-$(CONFIG_PHY_QCOM_PCIE2)		+= phy-qcom-pcie2.o
>  
>  obj-$(CONFIG_PHY_QCOM_QMP_COMBO)	+= phy-qcom-qmp-combo.o
> diff --git a/drivers/phy/qualcomm/phy-qcom-m31.c b/drivers/phy/qualcomm/phy-qcom-m31.c
> new file mode 100644
> index 0000000..4627425
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-m31.c
> @@ -0,0 +1,264 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2014-2016, 2020-2023, The Linux Foundation. All rights reserved.

Please fix this.

> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>

Rob Herring has posted a bunch of patches lately removing uses of
of_device.h, so please don't introduce a new one.

of.h and of_platform.h are to be used instead.

> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +#include <linux/usb/of.h>
> +#include <linux/usb/phy.h>

I don't think you need the last two, in a generic phy driver.

> +
> +#define USB2PHY_PORT_UTMI_CTRL1		0x40
> +
> +#define USB2PHY_PORT_UTMI_CTRL2		0x44
> + #define UTMI_ULPI_SEL			BIT(7)
> + #define UTMI_TEST_MUX_SEL		BIT(6)
> +
> +#define HS_PHY_CTRL_REG			0x10
> + #define UTMI_OTG_VBUS_VALID		BIT(20)
> + #define SW_SESSVLD_SEL			BIT(28)
> +
> +#define USB_PHY_UTMI_CTRL0		0x3c
> +
> +#define USB_PHY_UTMI_CTRL5		0x50
> + #define POR_EN				BIT(1)
> +
> +#define USB_PHY_HS_PHY_CTRL_COMMON0	0x54
> + #define COMMONONN			BIT(7)
> + #define FSEL				BIT(4)
> + #define RETENABLEN			BIT(3)
> + #define FREQ_24MHZ			(GENMASK(6, 6) | GENMASK(4, 4))
> +
> +#define USB_PHY_HS_PHY_CTRL2		0x64
> + #define USB2_SUSPEND_N_SEL		BIT(3)
> + #define USB2_SUSPEND_N			BIT(2)
> + #define USB2_UTMI_CLK_EN		BIT(1)
> +
> +#define USB_PHY_CFG0			0x94
> + #define UTMI_PHY_OVERRIDE_EN		BIT(1)
> +
> +#define USB_PHY_REFCLK_CTRL		0xa0
> + #define CLKCORE			BIT(1)
> +
> +#define USB2PHY_PORT_POWERDOWN		0xa4
> + #define POWER_UP			BIT(0)
> + #define POWER_DOWN			0
> +
> +#define USB_PHY_FSEL_SEL		0xb8
> + #define FREQ_SEL			BIT(0)
> +
> +#define USB2PHY_USB_PHY_M31_XCFGI_1	0xbc
> + #define USB2_0_TX_ENABLE		BIT(2)
> +
> +#define USB2PHY_USB_PHY_M31_XCFGI_4	0xc8
> + #define HSTX_SLEW_RATE_565PS		GENMASK(1, 0)
> + #define PLL_CHARGING_PUMP_CURRENT_35UA	GENMASK(4, 3)
> + #define ODT_VALUE_38_02_OHM		GENMASK(7, 6)
> +
> +#define USB2PHY_USB_PHY_M31_XCFGI_5	0xcc
> + #define ODT_VALUE_45_02_OHM		BIT(2)
> + #define HSTX_PRE_EMPHASIS_LEVEL_0_55MA	BIT(0)
> +
> +#define USB2PHY_USB_PHY_M31_XCFGI_11	0xe4
> + #define XCFG_COARSE_TUNE_NUM		BIT(1)
> + #define XCFG_FINE_TUNE_NUM		BIT(3)
> +
> +struct m31_phy_regs {
> +	u32 off;
> +	u32 val;
> +	u32 delay;
> +};
> +
> +struct m31_priv_data {
> +	bool				ulpi_mode;
> +	const struct m31_phy_regs	*regs;
> +	const int			nregs;

Doesn't hurt to spell out that this is an unsigned int, and you don't
need the const on the nregs.

> +};
> +
> +#define M31_REG(__o, __v, __d)	\
> +	{ .off = __o, .val = __v, .delay = __d }

This macro doesn't add any value.

Compare:
	M31_REG(USB_PHY_CFG0, UTMI_PHY_OVERRIDE_EN, 0),
with:
	{ USB_PHY_CFG0, UTMI_PHY_OVERRIDE_EN, 0 },

> +
> +struct m31_phy_regs m31_ipq5332_regs[] = {
> +	M31_REG(USB_PHY_CFG0, UTMI_PHY_OVERRIDE_EN, 0),
> +	M31_REG(USB_PHY_UTMI_CTRL5, POR_EN, 15),
> +	M31_REG(USB_PHY_FSEL_SEL, FREQ_SEL, 0),
> +	M31_REG(USB_PHY_HS_PHY_CTRL_COMMON0,
> +		COMMONONN | FREQ_24MHZ | RETENABLEN, 0),

The guideline is to keep under 80 characters, but if it improves
readability you may use up to 100. So this line can be unwrapped.

> +	M31_REG(USB_PHY_UTMI_CTRL5, POR_EN, 0),
> +	M31_REG(USB_PHY_HS_PHY_CTRL2,
> +		USB2_SUSPEND_N_SEL | USB2_SUSPEND_N | USB2_UTMI_CLK_EN, 0),

This one as well.

> +	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_11,
> +		XCFG_COARSE_TUNE_NUM  | XCFG_FINE_TUNE_NUM, 0),
> +	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_4,
> +		HSTX_SLEW_RATE_565PS | PLL_CHARGING_PUMP_CURRENT_35UA |
> +		ODT_VALUE_38_02_OHM, 0),
> +	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_1, USB2_0_TX_ENABLE, 0),
> +	M31_REG(USB2PHY_USB_PHY_M31_XCFGI_5,
> +		ODT_VALUE_45_02_OHM | HSTX_PRE_EMPHASIS_LEVEL_0_55MA, 4),

And this one, if you drop the M32_REG macro.

> +	M31_REG(USB_PHY_UTMI_CTRL5, 0x0, 0),
> +	M31_REG(USB_PHY_HS_PHY_CTRL2, USB2_SUSPEND_N | USB2_UTMI_CLK_EN, 0),
> +};
> +
> +struct m31usb_phy {
> +	struct phy			*phy;
> +	void __iomem			*base;
> +	const struct m31_phy_regs	*regs;
> +	int				nregs;
> +
> +	struct clk			*cfg_ahb_clk;

If you only have one clock, you can name it "clk" without causing
confusion.

> +	struct reset_control		*phy_reset;

Same here, if there's only one reset, then "reset" is a sufficient name.

> +
> +	bool				cable_connected;

Unused

> +	bool				suspended;

Unused, are you missing/omitting some relevant code here?

> +	bool				ulpi_mode;
> +};
> +
> +static void m31usb_phy_enable_clock(struct m31usb_phy *qphy)

It wouldn't hurt to just move this code into m31usb_phy_init().

> +{
> +	const struct m31_phy_regs *regs = qphy->regs;
> +	int i;
> +
> +	for (i = 0; i < qphy->nregs; i++) {
> +		writel(regs[i].val, qphy->base + regs[i].off);
> +		if (regs[i].delay)
> +			udelay(regs[i].delay);
> +	}
> +}
> +
> +static int m31usb_phy_init(struct phy *phy)
> +{
> +	int ret;
> +	struct m31usb_phy *qphy = phy_get_drvdata(phy);
> +
> +	ret = clk_prepare_enable(qphy->cfg_ahb_clk);
> +	if (ret) {
> +		dev_err(&phy->dev, "failed to enable cfg ahb clock, %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Perform phy reset */
> +	reset_control_assert(qphy->phy_reset);
> +	udelay(5);
> +	reset_control_deassert(qphy->phy_reset);
> +
> +	/* configure for ULPI mode if requested */
> +	if (qphy->ulpi_mode)
> +		writel(0x0, qphy->base + USB2PHY_PORT_UTMI_CTRL2);
> +
> +	/* Enable the PHY */
> +	writel(POWER_UP, qphy->base + USB2PHY_PORT_POWERDOWN);
> +
> +	/* Make sure above write completed */

That's not what wmb() does. It ensures that any writes prior to this
line is completed before any writes after this.

But m31usb_phy_enable_clock() is using writel(), which does the same
already.

If you really need the previous writes to have completed (get through
the write buffer etc), then you need to readl() the same register back.
But I see no reason for that.

> +	wmb();

So I think this should go.

> +
> +	/* Turn on phy ref clock */
> +	m31usb_phy_enable_clock(qphy);
> +
> +	return 0;
> +}
> +
> +static int m31usb_phy_shutdown(struct phy *phy)
> +{
> +	struct m31usb_phy *qphy = phy_get_drvdata(phy);
> +
> +	/* Disable the PHY */
> +	writel_relaxed(POWER_DOWN, qphy->base + USB2PHY_PORT_POWERDOWN);
> +	/* Make sure above write completed */
> +	wmb();

Same as above.

Although here you would assume that clk_disable_unprepare() uses
writel() and doesn't overtake your power-down write, but that sounds
like reasonable assumptions to make.

> +
> +	clk_disable_unprepare(qphy->cfg_ahb_clk);
> +
> +	return 0;
> +}
> +
> +static const struct phy_ops m31usb_phy_gen_ops = {
> +	.power_on	= m31usb_phy_init,
> +	.power_off	= m31usb_phy_shutdown,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static int m31usb_phy_probe(struct platform_device *pdev)
> +{
> +	const struct m31_priv_data *data;
> +	struct device *dev = &pdev->dev;
> +	struct m31usb_phy *qphy;
> +	struct phy_provider *phy_provider;
> +	int ret;

Please sort these in descending length order (reverse xmas tree).

> +
> +	qphy = devm_kzalloc(dev, sizeof(*qphy), GFP_KERNEL);
> +	if (!qphy)
> +		return -ENOMEM;
> +
> +	qphy->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(qphy->base))
> +		return PTR_ERR(qphy->base);
> +
> +	qphy->phy_reset = devm_reset_control_get_exclusive_by_index(dev, 0);
> +	if (IS_ERR(qphy->phy_reset))
> +		return PTR_ERR(qphy->phy_reset);
> +
> +	qphy->cfg_ahb_clk = devm_clk_get(dev, "cfg_ahb");

If you only have one clock, then omitting clock-names and do
devm_clk_get(dev, NULL) works well.

> +	if (IS_ERR(qphy->cfg_ahb_clk)) {
> +		ret = PTR_ERR(qphy->cfg_ahb_clk);
> +		dev_err(dev, "failed to get cfg_ahb clock, %d\n", ret);
> +		return ret;

Replace these tree lines with:

return dev_err_probe(dev, PTR_ERR(qphy->cfg_ahb_clk), "failed to get cfg_ahb_clk\n");

> +	}
> +
> +	platform_set_drvdata(pdev, qphy);

There are no platform_get_drvdata() calls in this driver, so you can
omit this.

> +
> +	data = of_device_get_match_data(dev);
> +	qphy->regs		= data->regs;
> +	qphy->nregs		= data->nregs;
> +	qphy->ulpi_mode		= data->ulpi_mode;
> +
> +	qphy->phy = devm_phy_create(dev, NULL, &m31usb_phy_gen_ops);
> +	if (IS_ERR(qphy->phy)) {
> +		ret = PTR_ERR(qphy->phy);
> +		dev_err(dev, "failed to create phy, %d\n", ret);
> +		qphy->phy = NULL;

qphy is inaccessible, and freed after the next line, no need to clear
this.

> +		return ret;
> +	}
> +
> +	dev_set_drvdata(dev, qphy);

This is exactly he same as platform_set_drvdata() above, so per the same
logic you can drop this as well.

> +	phy_set_drvdata(qphy->phy, qphy);
> +
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	if (!IS_ERR(phy_provider))
> +		dev_info(dev, "Registered M31 USB phy\n");
> +
> +	return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static const struct m31_priv_data m31_ipq5332_data = {
> +	.ulpi_mode = false,
> +	.regs = m31_ipq5332_regs,
> +	.nregs = ARRAY_SIZE(m31_ipq5332_regs),
> +};
> +
> +static const struct of_device_id m31usb_phy_id_table[] = {
> +	{ .compatible = "qcom,ipq5332-usb-hsphy", .data = &m31_ipq5332_data },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, m31usb_phy_id_table);
> +
> +static struct platform_driver m31usb_phy_driver = {
> +	.probe		= m31usb_phy_probe,

Please drop the indentation in these.

Regards,
Bjorn

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v5 4/6] arm64: dts: qcom: ipq5332: Add USB related nodes

[Varadarajan Narayanan]

Add USB phy and controller nodes.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v5:
	Use generic phy instead of usb-phy
	'make CHECK_DTBS=y DT_SCHEMA_FILES=qcom dtbs_check' passed
	'DT_CHECKER_FLAGS='-v -m' DT_SCHEMA_FILES=qcom dt_binding_check' passed
v4:
	Change node name
	Remove blank line
	'make CHECK_DTBS=y DT_SCHEMA_FILES=qcom qcom/ipq5332-rdp441.dtb' passed
v1:
	Rename phy node
	Change compatible from m31,ipq5332-usb-hsphy -> qcom,ipq5332-usb-hsphy
	Remove 'qscratch' from phy node
	Fix alignment and upper-case hex no.s
	Add clock definition for the phy
	Remove snps,ref-clock-period-ns as it is not used. dwc3_ref_clk_period()
	in dwc3/core.c takes the frequency from ref clock and calculates fladj
	as appropriate.
---
 arch/arm64/boot/dts/qcom/ipq5332.dtsi | 56 +++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
index 8bfc2db..a22f340 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
@@ -405,6 +405,62 @@
 				status = "disabled";
 			};
 		};
+
+		usbphy0: usb-phy@7b000 {
+			compatible = "qcom,ipq5332-usb-hsphy";
+			reg = <0x0007b000 0x12c>;
+
+			clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>;
+			clock-names = "cfg_ahb";
+
+			resets = <&gcc GCC_QUSB2_0_PHY_BCR>;
+
+			#phy-cells = <0>;
+
+			status = "disabled";
+		};
+
+		usb2: usb2@8a00000 {
+			compatible = "qcom,ipq5332-dwc3", "qcom,dwc3";
+			reg = <0x08af8800 0x400>;
+
+			interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "hs_phy_irq";
+
+			clocks = <&gcc GCC_USB0_MASTER_CLK>,
+				 <&gcc GCC_SNOC_USB_CLK>,
+				 <&gcc GCC_USB0_SLEEP_CLK>,
+				 <&gcc GCC_USB0_MOCK_UTMI_CLK>;
+			clock-names = "core",
+				      "iface",
+				      "sleep",
+				      "mock_utmi";
+
+			resets = <&gcc GCC_USB_BCR>;
+
+			qcom,select-utmi-as-pipe-clk;
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+
+			status = "disabled";
+
+			usb2_0_dwc: usb@8a00000 {
+				compatible = "snps,dwc3";
+				reg = <0x08a00000 0xe000>;
+				clocks = <&gcc GCC_USB0_MOCK_UTMI_CLK>;
+				clock-names = "ref";
+				interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
+				phy-names = "usb2-phy";
+				phys = <&usbphy0>;
+				tx-fifo-resize;
+				snps,is-utmi-l1-suspend;
+				snps,hird-threshold = /bits/ 8 <0x0>;
+				snps,dis_u2_susphy_quirk;
+				snps,dis_u3_susphy_quirk;
+			};
+		};
 	};
 
 	timer {
-- 
2.7.4

[PATCH v5 4/6] arm64: dts: qcom: ipq5332: Add USB related nodes

[Varadarajan Narayanan]

Add USB phy and controller nodes.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v5:
	Use generic phy instead of usb-phy
	'make CHECK_DTBS=y DT_SCHEMA_FILES=qcom dtbs_check' passed
	'DT_CHECKER_FLAGS='-v -m' DT_SCHEMA_FILES=qcom dt_binding_check' passed
v4:
	Change node name
	Remove blank line
	'make CHECK_DTBS=y DT_SCHEMA_FILES=qcom qcom/ipq5332-rdp441.dtb' passed
v1:
	Rename phy node
	Change compatible from m31,ipq5332-usb-hsphy -> qcom,ipq5332-usb-hsphy
	Remove 'qscratch' from phy node
	Fix alignment and upper-case hex no.s
	Add clock definition for the phy
	Remove snps,ref-clock-period-ns as it is not used. dwc3_ref_clk_period()
	in dwc3/core.c takes the frequency from ref clock and calculates fladj
	as appropriate.
---
 arch/arm64/boot/dts/qcom/ipq5332.dtsi | 56 +++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
index 8bfc2db..a22f340 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
@@ -405,6 +405,62 @@
 				status = "disabled";
 			};
 		};
+
+		usbphy0: usb-phy@7b000 {
+			compatible = "qcom,ipq5332-usb-hsphy";
+			reg = <0x0007b000 0x12c>;
+
+			clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>;
+			clock-names = "cfg_ahb";
+
+			resets = <&gcc GCC_QUSB2_0_PHY_BCR>;
+
+			#phy-cells = <0>;
+
+			status = "disabled";
+		};
+
+		usb2: usb2@8a00000 {
+			compatible = "qcom,ipq5332-dwc3", "qcom,dwc3";
+			reg = <0x08af8800 0x400>;
+
+			interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "hs_phy_irq";
+
+			clocks = <&gcc GCC_USB0_MASTER_CLK>,
+				 <&gcc GCC_SNOC_USB_CLK>,
+				 <&gcc GCC_USB0_SLEEP_CLK>,
+				 <&gcc GCC_USB0_MOCK_UTMI_CLK>;
+			clock-names = "core",
+				      "iface",
+				      "sleep",
+				      "mock_utmi";
+
+			resets = <&gcc GCC_USB_BCR>;
+
+			qcom,select-utmi-as-pipe-clk;
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+
+			status = "disabled";
+
+			usb2_0_dwc: usb@8a00000 {
+				compatible = "snps,dwc3";
+				reg = <0x08a00000 0xe000>;
+				clocks = <&gcc GCC_USB0_MOCK_UTMI_CLK>;
+				clock-names = "ref";
+				interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
+				phy-names = "usb2-phy";
+				phys = <&usbphy0>;
+				tx-fifo-resize;
+				snps,is-utmi-l1-suspend;
+				snps,hird-threshold = /bits/ 8 <0x0>;
+				snps,dis_u2_susphy_quirk;
+				snps,dis_u3_susphy_quirk;
+			};
+		};
 	};
 
 	timer {
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 4/6] arm64: dts: qcom: ipq5332: Add USB related nodes

[Varadarajan Narayanan]

Add USB phy and controller nodes.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v5:
	Use generic phy instead of usb-phy
	'make CHECK_DTBS=y DT_SCHEMA_FILES=qcom dtbs_check' passed
	'DT_CHECKER_FLAGS='-v -m' DT_SCHEMA_FILES=qcom dt_binding_check' passed
v4:
	Change node name
	Remove blank line
	'make CHECK_DTBS=y DT_SCHEMA_FILES=qcom qcom/ipq5332-rdp441.dtb' passed
v1:
	Rename phy node
	Change compatible from m31,ipq5332-usb-hsphy -> qcom,ipq5332-usb-hsphy
	Remove 'qscratch' from phy node
	Fix alignment and upper-case hex no.s
	Add clock definition for the phy
	Remove snps,ref-clock-period-ns as it is not used. dwc3_ref_clk_period()
	in dwc3/core.c takes the frequency from ref clock and calculates fladj
	as appropriate.
---
 arch/arm64/boot/dts/qcom/ipq5332.dtsi | 56 +++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
index 8bfc2db..a22f340 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
@@ -405,6 +405,62 @@
 				status = "disabled";
 			};
 		};
+
+		usbphy0: usb-phy@7b000 {
+			compatible = "qcom,ipq5332-usb-hsphy";
+			reg = <0x0007b000 0x12c>;
+
+			clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>;
+			clock-names = "cfg_ahb";
+
+			resets = <&gcc GCC_QUSB2_0_PHY_BCR>;
+
+			#phy-cells = <0>;
+
+			status = "disabled";
+		};
+
+		usb2: usb2@8a00000 {
+			compatible = "qcom,ipq5332-dwc3", "qcom,dwc3";
+			reg = <0x08af8800 0x400>;
+
+			interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "hs_phy_irq";
+
+			clocks = <&gcc GCC_USB0_MASTER_CLK>,
+				 <&gcc GCC_SNOC_USB_CLK>,
+				 <&gcc GCC_USB0_SLEEP_CLK>,
+				 <&gcc GCC_USB0_MOCK_UTMI_CLK>;
+			clock-names = "core",
+				      "iface",
+				      "sleep",
+				      "mock_utmi";
+
+			resets = <&gcc GCC_USB_BCR>;
+
+			qcom,select-utmi-as-pipe-clk;
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges;
+
+			status = "disabled";
+
+			usb2_0_dwc: usb@8a00000 {
+				compatible = "snps,dwc3";
+				reg = <0x08a00000 0xe000>;
+				clocks = <&gcc GCC_USB0_MOCK_UTMI_CLK>;
+				clock-names = "ref";
+				interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
+				phy-names = "usb2-phy";
+				phys = <&usbphy0>;
+				tx-fifo-resize;
+				snps,is-utmi-l1-suspend;
+				snps,hird-threshold = /bits/ 8 <0x0>;
+				snps,dis_u2_susphy_quirk;
+				snps,dis_u3_susphy_quirk;
+			};
+		};
 	};
 
 	timer {
-- 
2.7.4


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 4/6] arm64: dts: qcom: ipq5332: Add USB related nodes

[Bjorn Andersson]

On Fri, Jul 21, 2023 at 10:05:29AM +0530, Varadarajan Narayanan wrote:
> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> index 8bfc2db..a22f340 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> @@ -405,6 +405,62 @@
>  				status = "disabled";
>  			};
>  		};
> +
> +		usbphy0: usb-phy@7b000 {

s/usb-phy/phy/

> +			compatible = "qcom,ipq5332-usb-hsphy";
> +			reg = <0x0007b000 0x12c>;

Sort your nodes under /soc by their address.

> +
> +			clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>;
> +			clock-names = "cfg_ahb";
> +
> +			resets = <&gcc GCC_QUSB2_0_PHY_BCR>;
> +
> +			#phy-cells = <0>;
> +
> +			status = "disabled";
> +		};
> +
> +		usb2: usb2@8a00000 {

usb@... drop the '2' in the node name (not in the label). Although, why
does usb2 use the phy labeled usb0?

Regards,
Bjorn

Re: [PATCH v5 4/6] arm64: dts: qcom: ipq5332: Add USB related nodes

[Bjorn Andersson]

On Fri, Jul 21, 2023 at 10:05:29AM +0530, Varadarajan Narayanan wrote:
> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> index 8bfc2db..a22f340 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> @@ -405,6 +405,62 @@
>  				status = "disabled";
>  			};
>  		};
> +
> +		usbphy0: usb-phy@7b000 {

s/usb-phy/phy/

> +			compatible = "qcom,ipq5332-usb-hsphy";
> +			reg = <0x0007b000 0x12c>;

Sort your nodes under /soc by their address.

> +
> +			clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>;
> +			clock-names = "cfg_ahb";
> +
> +			resets = <&gcc GCC_QUSB2_0_PHY_BCR>;
> +
> +			#phy-cells = <0>;
> +
> +			status = "disabled";
> +		};
> +
> +		usb2: usb2@8a00000 {

usb@... drop the '2' in the node name (not in the label). Although, why
does usb2 use the phy labeled usb0?

Regards,
Bjorn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 4/6] arm64: dts: qcom: ipq5332: Add USB related nodes

[Bjorn Andersson]

On Fri, Jul 21, 2023 at 10:05:29AM +0530, Varadarajan Narayanan wrote:
> diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> index 8bfc2db..a22f340 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
> @@ -405,6 +405,62 @@
>  				status = "disabled";
>  			};
>  		};
> +
> +		usbphy0: usb-phy@7b000 {

s/usb-phy/phy/

> +			compatible = "qcom,ipq5332-usb-hsphy";
> +			reg = <0x0007b000 0x12c>;

Sort your nodes under /soc by their address.

> +
> +			clocks = <&gcc GCC_USB0_PHY_CFG_AHB_CLK>;
> +			clock-names = "cfg_ahb";
> +
> +			resets = <&gcc GCC_QUSB2_0_PHY_BCR>;
> +
> +			#phy-cells = <0>;
> +
> +			status = "disabled";
> +		};
> +
> +		usb2: usb2@8a00000 {

usb@... drop the '2' in the node name (not in the label). Although, why
does usb2 use the phy labeled usb0?

Regards,
Bjorn

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v5 5/6] arm64: dts: qcom: ipq5332: Enable USB

[Varadarajan Narayanan]

Enable USB2 in host mode.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v1:
	Enable usb-phy node
---
 arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
index f96b0c8..f5dea16 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
+++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
@@ -79,3 +79,15 @@
 		bias-pull-up;
 	};
 };
+
+&usbphy0 {
+	status = "okay";
+};
+
+&usb2 {
+	status = "okay";
+};
+
+&usb2_0_dwc {
+	dr_mode = "host";
+};
-- 
2.7.4

[PATCH v5 5/6] arm64: dts: qcom: ipq5332: Enable USB

[Varadarajan Narayanan]

Enable USB2 in host mode.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v1:
	Enable usb-phy node
---
 arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
index f96b0c8..f5dea16 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
+++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
@@ -79,3 +79,15 @@
 		bias-pull-up;
 	};
 };
+
+&usbphy0 {
+	status = "okay";
+};
+
+&usb2 {
+	status = "okay";
+};
+
+&usb2_0_dwc {
+	dr_mode = "host";
+};
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 5/6] arm64: dts: qcom: ipq5332: Enable USB

[Varadarajan Narayanan]

Enable USB2 in host mode.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v1:
	Enable usb-phy node
---
 arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
index f96b0c8..f5dea16 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
+++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
@@ -79,3 +79,15 @@
 		bias-pull-up;
 	};
 };
+
+&usbphy0 {
+	status = "okay";
+};
+
+&usb2 {
+	status = "okay";
+};
+
+&usb2_0_dwc {
+	dr_mode = "host";
+};
-- 
2.7.4


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v5 5/6] arm64: dts: qcom: ipq5332: Enable USB

[Bjorn Andersson]

On Fri, Jul 21, 2023 at 10:05:30AM +0530, Varadarajan Narayanan wrote:
> Enable USB2 in host mode.
> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v1:
> 	Enable usb-phy node
> ---
>  arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
> index f96b0c8..f5dea16 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
> +++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
> @@ -79,3 +79,15 @@
>  		bias-pull-up;
>  	};
>  };
> +
> +&usbphy0 {
> +	status = "okay";

Is this phy not powered? Should there be some regulators controlled?

Regards,
Bjorn

Re: [PATCH v5 5/6] arm64: dts: qcom: ipq5332: Enable USB

[Bjorn Andersson]

On Fri, Jul 21, 2023 at 10:05:30AM +0530, Varadarajan Narayanan wrote:
> Enable USB2 in host mode.
> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v1:
> 	Enable usb-phy node
> ---
>  arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
> index f96b0c8..f5dea16 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
> +++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
> @@ -79,3 +79,15 @@
>  		bias-pull-up;
>  	};
>  };
> +
> +&usbphy0 {
> +	status = "okay";

Is this phy not powered? Should there be some regulators controlled?

Regards,
Bjorn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 5/6] arm64: dts: qcom: ipq5332: Enable USB

[Bjorn Andersson]

On Fri, Jul 21, 2023 at 10:05:30AM +0530, Varadarajan Narayanan wrote:
> Enable USB2 in host mode.
> 
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v1:
> 	Enable usb-phy node
> ---
>  arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
> index f96b0c8..f5dea16 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
> +++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
> @@ -79,3 +79,15 @@
>  		bias-pull-up;
>  	};
>  };
> +
> +&usbphy0 {
> +	status = "okay";

Is this phy not powered? Should there be some regulators controlled?

Regards,
Bjorn

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v5 6/6] arm64: defconfig: Enable M31 USB phy driver

[Varadarajan Narayanan]

Enable M31 USB phy driver present in IPQ5332.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v2:
	Add full stop to commit log.
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 6cbf6eb..e6d2012 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1389,6 +1389,7 @@ CONFIG_PHY_QCOM_USB_SNPS_FEMTO_V2=m
 CONFIG_PHY_QCOM_USB_HS_28NM=m
 CONFIG_PHY_QCOM_USB_SS=m
 CONFIG_PHY_QCOM_SGMII_ETH=m
+CONFIG_PHY_QCOM_M31_USB=m
 CONFIG_PHY_R8A779F0_ETHERNET_SERDES=y
 CONFIG_PHY_RCAR_GEN3_PCIE=y
 CONFIG_PHY_RCAR_GEN3_USB2=y
-- 
2.7.4

[PATCH v5 6/6] arm64: defconfig: Enable M31 USB phy driver

[Varadarajan Narayanan]

Enable M31 USB phy driver present in IPQ5332.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v2:
	Add full stop to commit log.
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 6cbf6eb..e6d2012 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1389,6 +1389,7 @@ CONFIG_PHY_QCOM_USB_SNPS_FEMTO_V2=m
 CONFIG_PHY_QCOM_USB_HS_28NM=m
 CONFIG_PHY_QCOM_USB_SS=m
 CONFIG_PHY_QCOM_SGMII_ETH=m
+CONFIG_PHY_QCOM_M31_USB=m
 CONFIG_PHY_R8A779F0_ETHERNET_SERDES=y
 CONFIG_PHY_RCAR_GEN3_PCIE=y
 CONFIG_PHY_RCAR_GEN3_USB2=y
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v5 6/6] arm64: defconfig: Enable M31 USB phy driver

[Varadarajan Narayanan]

Enable M31 USB phy driver present in IPQ5332.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v2:
	Add full stop to commit log.
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 6cbf6eb..e6d2012 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1389,6 +1389,7 @@ CONFIG_PHY_QCOM_USB_SNPS_FEMTO_V2=m
 CONFIG_PHY_QCOM_USB_HS_28NM=m
 CONFIG_PHY_QCOM_USB_SS=m
 CONFIG_PHY_QCOM_SGMII_ETH=m
+CONFIG_PHY_QCOM_M31_USB=m
 CONFIG_PHY_R8A779F0_ETHERNET_SERDES=y
 CONFIG_PHY_RCAR_GEN3_PCIE=y
 CONFIG_PHY_RCAR_GEN3_USB2=y
-- 
2.7.4


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy