[PATCH v2 0/8] phy: samsung: add Exynos2200 SNPS eUSB2 driver

[PATCH v2 0/8] phy: samsung: add Exynos2200 SNPS eUSB2 driver

[Ivaylo Ivanov]

Hey folks,

In the vendor kernel, everything is handled in a single phy driver,
with helpers for functions outside it. Clocks and regulators are
specified and enabled in one node, which makes it difficult to
separate what clocks and regulators go where without access to
schematics or TRMs. The following gates are defined for USB:

CLK_BLK_HSI0_UID_USB32DRD_IPCLKPORT_I_USBSUBCTL_APB_PCLK
CLK_BLK_HSI0_UID_USB32DRD_IPCLKPORT_I_USBDPPHY_CTRL_PCLK
CLK_BLK_HSI0_UID_USB32DRD_IPCLKPORT_I_USBDPPHY_TCA_APB_CLK

CLK_BLK_HSI0_UID_USB32DRD_IPCLKPORT_I_USBLINK_ACLK
CLK_BLK_HSI0_UID_USB32DRD_IPCLKPORT_I_USB32DRD_REF_CLK_40

CLK_BLK_HSI0_UID_USB32DRD_IPCLKPORT_I_EUSB_CTRL_PCLK
CLK_BLK_HSI0_UID_USB32DRD_IPCLKPORT_I_EUSB_APB_CLK
CLK_BLK_HSI0_UID_AS_APB_EUSBPHY_HSI0_IPCLKPORT_PCLKM
CLK_BLK_HSI0_UID_RSTNSYNC_CLK_HSI0_EUSB_IPCLKPORT_CLK

The vendor kernel specifies 4 regulators, 2 of which are for eUSB
and the other 2 for the repeater. The rest of the PHYs and the dwc3
controller are on a single power domain (hsi0), so they're most likely
sharing power rails.

From what I've managed to understand, this SoC has 3 PHYs for USB:
 - a synopsys eusb2 phy
 - a synposys combophy for usbdp and superspeed
 - a usbcon phy, which acts as a "controller" for the other 2 phys

As Qualcomm is also using the eUSB2 IP, the approach taken here is to
rename the driver so that it can be used by other SoC vendors as well
while keeping compatibles SoC-vendor prefixed (different vendors have
different implementations of the IP with different register maps),
add support for exynos2200 in it and introduce a new USBCON phy driver,
modelled to take phandles to eUSB2 (and in the future, USBDP).

A new USBDP driver will be added later on, so that pipe3 and super-speed
can be configured.

The USBCON binding includes a header from the clocks [1] patchset.

Bindings have been tested:
$ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j4 dt_binding_check DT_SCHEMA_FILES="Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml"
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
/home/ivaylo/linux-devices/s22/linux/for-upstream/linux-next/Documentation/devicetree/bindings/iio/light/brcm,apds9160.yaml: ps-cancellation-current-picoamp: missing type definition
  CHKDT   ./Documentation/devicetree/bindings
  LINT    ./Documentation/devicetree/bindings
  DTEX    Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.example.dts
  DTC [C] Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.example.dtb

ivaylo@ivaylo-T580:~/linux-devices/s22/linux/for-upstream/linux-next$ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j4 dt_binding_check DT_SCHEMA_FILES="Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml"
  CHKDT   ./Documentation/devicetree/bindings
  LINT    ./Documentation/devicetree/bindings
  DTC [C] Documentation/devicetree/bindings/phy/snps,eusb2-phy.example.dtb

Best regards,
Ivaylo

Changes in v2:
USBCON changes:
- drop unused header includes
- sanitize the binding
- proper init and exit power management
- shorten some variables
- unrelax reads and writes
- update commit description
- remodel to take other phys
- drop specified regulators as these are for the repeater
- make the kconfig description better
- general cleanup
EUSB2 changes:
- merge the previous separate driver into the qualcomm one
- drop the previous model of taking usbcon phandle

[1] https://lore.kernel.org/all/20250223115601.723886-1-ivo.ivanov.ivanov1@gmail.com/

Ivaylo Ivanov (8):
  dt-bindings: phy: rename qcom,snps-eusb2-phy binding to snps,eusb2-phy
  dt-bindings: phy: snps-eusb2: add exynos2200 support
  dt-bindings: phy: add samsung,exynos2200-usbcon-phy schema file
  phy: move phy-qcom-snps-eusb2 out of its vendor sub-directory
  phy: phy-snps-eusb2: make repeater optional
  phy: phy-snps-eusb2: make reset control optional
  phy: phy-snps-eusb2: add support for exynos2200
  phy: samsung: add Exynos2200 usb phy controller

 .../phy/samsung,exynos2200-usbcon-phy.yaml    |  76 ++
 ...nps-eusb2-phy.yaml => snps,eusb2-phy.yaml} |  70 +-
 drivers/phy/Kconfig                           |   8 +
 drivers/phy/Makefile                          |   1 +
 drivers/phy/phy-snps-eusb2.c                  | 677 ++++++++++++++++++
 drivers/phy/qualcomm/Kconfig                  |   9 -
 drivers/phy/qualcomm/Makefile                 |   1 -
 drivers/phy/qualcomm/phy-qcom-snps-eusb2.c    | 442 ------------
 drivers/phy/samsung/Kconfig                   |  12 +
 drivers/phy/samsung/Makefile                  |   1 +
 drivers/phy/samsung/phy-exynos2200-usbcon.c   | 250 +++++++
 11 files changed, 1085 insertions(+), 462 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
 rename Documentation/devicetree/bindings/phy/{qcom,snps-eusb2-phy.yaml => snps,eusb2-phy.yaml} (57%)
 create mode 100644 drivers/phy/phy-snps-eusb2.c
 delete mode 100644 drivers/phy/qualcomm/phy-qcom-snps-eusb2.c
 create mode 100644 drivers/phy/samsung/phy-exynos2200-usbcon.c

-- 
2.43.0

[PATCH v2 1/8] dt-bindings: phy: rename qcom,snps-eusb2-phy binding to snps,eusb2-phy

[Ivaylo Ivanov]

As Samsung has been using the same Synopsys eUSB2 IP in Exynos2200,
albeit with a different register layout, rename qcom,snps-eusb2-phy
to snps,eusb2-phy and drop mentions of it being only for Qualcomm SoCs
in the binding description.

Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
---
 .../phy/{qcom,snps-eusb2-phy.yaml => snps,eusb2-phy.yaml}   | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
 rename Documentation/devicetree/bindings/phy/{qcom,snps-eusb2-phy.yaml => snps,eusb2-phy.yaml} (90%)

diff --git a/Documentation/devicetree/bindings/phy/qcom,snps-eusb2-phy.yaml b/Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml
similarity index 90%
rename from Documentation/devicetree/bindings/phy/qcom,snps-eusb2-phy.yaml
rename to Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml
index 142b3c883..22c77968f 100644
--- a/Documentation/devicetree/bindings/phy/qcom,snps-eusb2-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml
@@ -1,16 +1,16 @@
 # SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/phy/qcom,snps-eusb2-phy.yaml#
+$id: http://devicetree.org/schemas/phy/snps,eusb2-phy.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Qualcomm SNPS eUSB2 phy controller
+title: SNPS eUSB2 phy controller
 
 maintainers:
   - Abel Vesa <abel.vesa@linaro.org>
 
 description:
-  eUSB2 controller supports LS/FS/HS usb connectivity on Qualcomm chipsets.
+  eUSB2 controller supports LS/FS/HS usb connectivity.
 
 properties:
   compatible:
-- 
2.43.0

Re: [PATCH v2 1/8] dt-bindings: phy: rename qcom,snps-eusb2-phy binding to snps,eusb2-phy

[Krzysztof Kozlowski]

On Sun, Feb 23, 2025 at 02:22:20PM +0200, Ivaylo Ivanov wrote:
> As Samsung has been using the same Synopsys eUSB2 IP in Exynos2200,
> albeit with a different register layout, rename qcom,snps-eusb2-phy
> to snps,eusb2-phy and drop mentions of it being only for Qualcomm SoCs
> in the binding description.

Rename itself is pointless. One logical change is: you add here samsung,
this you rename it.

Best regards,
Krzysztof

[PATCH v2 2/8] dt-bindings: phy: snps-eusb2: add exynos2200 support

[Ivaylo Ivanov]

Exynos 2200 makes use of the Synposys eUSB2 IP, so document it in the
binding. Unlike the currently documented Qualcomm SoCs, it doesn't provide
reset lines for reset control and uses more clocks.

Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov@gmail.com>
---
 .../bindings/phy/snps,eusb2-phy.yaml          | 64 +++++++++++++++++--
 1 file changed, 57 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml b/Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml
index 22c77968f..f4164db71 100644
--- a/Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml
@@ -23,6 +23,7 @@ properties:
               - qcom,x1e80100-snps-eusb2-phy
           - const: qcom,sm8550-snps-eusb2-phy
       - const: qcom,sm8550-snps-eusb2-phy
+      - const: samsung,exynos2200-snps-eusb2-phy
 
   reg:
     maxItems: 1
@@ -31,12 +32,12 @@ properties:
     const: 0
 
   clocks:
-    items:
-      - description: ref
+    minItems: 1
+    maxItems: 3
 
   clock-names:
-    items:
-      - const: ref
+    minItems: 1
+    maxItems: 3
 
   resets:
     maxItems: 1
@@ -58,11 +59,60 @@ required:
   - compatible
   - reg
   - "#phy-cells"
-  - clocks
-  - clock-names
   - vdd-supply
   - vdda12-supply
-  - resets
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sm8550-snps-eusb2-phy
+
+    then:
+      properties:
+        reg:
+          maxItems: 1
+
+        clocks:
+          items:
+            - description: ref
+
+        clock-names:
+          items:
+            - const: ref
+
+      required:
+        - clocks
+        - clock-names
+        - resets
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - samsung,exynos2200-snps-eusb2-phy
+
+    then:
+      properties:
+
+        clocks:
+          items:
+            - description: Reference clock
+            - description: Bus (APB) clock
+            - description: Control clock
+
+        clock-names:
+          items:
+            - const: ref
+            - const: bus
+            - const: ctrl
+
+      required:
+        - clocks
+        - clock-names
 
 additionalProperties: false
 
-- 
2.43.0

Re: [PATCH v2 2/8] dt-bindings: phy: snps-eusb2: add exynos2200 support

[Dmitry Baryshkov]

On Sun, Feb 23, 2025 at 02:22:21PM +0200, Ivaylo Ivanov wrote:
> Exynos 2200 makes use of the Synposys eUSB2 IP, so document it in the
> binding. Unlike the currently documented Qualcomm SoCs, it doesn't provide
> reset lines for reset control and uses more clocks.
> 
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov@gmail.com>
> ---
>  .../bindings/phy/snps,eusb2-phy.yaml          | 64 +++++++++++++++++--
>  1 file changed, 57 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml b/Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml
> index 22c77968f..f4164db71 100644
> --- a/Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml
> @@ -23,6 +23,7 @@ properties:
>                - qcom,x1e80100-snps-eusb2-phy
>            - const: qcom,sm8550-snps-eusb2-phy
>        - const: qcom,sm8550-snps-eusb2-phy
> +      - const: samsung,exynos2200-snps-eusb2-phy
>  
>    reg:
>      maxItems: 1
> @@ -31,12 +32,12 @@ properties:
>      const: 0
>  
>    clocks:
> -    items:
> -      - description: ref
> +    minItems: 1
> +    maxItems: 3
>  
>    clock-names:
> -    items:
> -      - const: ref
> +    minItems: 1
> +    maxItems: 3
>  
>    resets:
>      maxItems: 1
> @@ -58,11 +59,60 @@ required:
>    - compatible
>    - reg
>    - "#phy-cells"
> -  - clocks
> -  - clock-names

Why? Clocks are required in both if clauses.

>    - vdd-supply
>    - vdda12-supply
> -  - resets
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sm8550-snps-eusb2-phy
> +
> +    then:
> +      properties:
> +        reg:
> +          maxItems: 1
> +
> +        clocks:
> +          items:
> +            - description: ref
> +
> +        clock-names:
> +          items:
> +            - const: ref
> +
> +      required:
> +        - clocks
> +        - clock-names
> +        - resets
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - samsung,exynos2200-snps-eusb2-phy
> +
> +    then:
> +      properties:
> +
> +        clocks:
> +          items:
> +            - description: Reference clock
> +            - description: Bus (APB) clock
> +            - description: Control clock
> +
> +        clock-names:
> +          items:
> +            - const: ref
> +            - const: bus
> +            - const: ctrl
> +
> +      required:
> +        - clocks
> +        - clock-names
>  
>  additionalProperties: false
>  
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 2/8] dt-bindings: phy: snps-eusb2: add exynos2200 support

[Ivaylo Ivanov]

On 2/24/25 01:43, Dmitry Baryshkov wrote:
> On Sun, Feb 23, 2025 at 02:22:21PM +0200, Ivaylo Ivanov wrote:
>> Exynos 2200 makes use of the Synposys eUSB2 IP, so document it in the
>> binding. Unlike the currently documented Qualcomm SoCs, it doesn't provide
>> reset lines for reset control and uses more clocks.
>>
>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov@gmail.com>
>> ---
>>  .../bindings/phy/snps,eusb2-phy.yaml          | 64 +++++++++++++++++--
>>  1 file changed, 57 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml b/Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml
>> index 22c77968f..f4164db71 100644
>> --- a/Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml
>> +++ b/Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml
>> @@ -23,6 +23,7 @@ properties:
>>                - qcom,x1e80100-snps-eusb2-phy
>>            - const: qcom,sm8550-snps-eusb2-phy
>>        - const: qcom,sm8550-snps-eusb2-phy
>> +      - const: samsung,exynos2200-snps-eusb2-phy
>>  
>>    reg:
>>      maxItems: 1
>> @@ -31,12 +32,12 @@ properties:
>>      const: 0
>>  
>>    clocks:
>> -    items:
>> -      - description: ref
>> +    minItems: 1
>> +    maxItems: 3
>>  
>>    clock-names:
>> -    items:
>> -      - const: ref
>> +    minItems: 1
>> +    maxItems: 3
>>  
>>    resets:
>>      maxItems: 1
>> @@ -58,11 +59,60 @@ required:
>>    - compatible
>>    - reg
>>    - "#phy-cells"
>> -  - clocks
>> -  - clock-names
> Why? Clocks are required in both if clauses.

Right. And "ref" is a requirement. I'll fix that in the next revision.

Best regards,
Ivaylo

>
>>    - vdd-supply
>>    - vdda12-supply
>> -  - resets
>> +
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,sm8550-snps-eusb2-phy
>> +
>> +    then:
>> +      properties:
>> +        reg:
>> +          maxItems: 1
>> +
>> +        clocks:
>> +          items:
>> +            - description: ref
>> +
>> +        clock-names:
>> +          items:
>> +            - const: ref
>> +
>> +      required:
>> +        - clocks
>> +        - clock-names
>> +        - resets
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - samsung,exynos2200-snps-eusb2-phy
>> +
>> +    then:
>> +      properties:
>> +
>> +        clocks:
>> +          items:
>> +            - description: Reference clock
>> +            - description: Bus (APB) clock
>> +            - description: Control clock
>> +
>> +        clock-names:
>> +          items:
>> +            - const: ref
>> +            - const: bus
>> +            - const: ctrl
>> +
>> +      required:
>> +        - clocks
>> +        - clock-names
>>  
>>  additionalProperties: false
>>  
>> -- 
>> 2.43.0
>>

Re: [PATCH v2 2/8] dt-bindings: phy: snps-eusb2: add exynos2200 support

[Krzysztof Kozlowski]

On Sun, Feb 23, 2025 at 02:22:21PM +0200, Ivaylo Ivanov wrote:
> Exynos 2200 makes use of the Synposys eUSB2 IP, so document it in the
> binding. Unlike the currently documented Qualcomm SoCs, it doesn't provide
> reset lines for reset control and uses more clocks.
> 
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov@gmail.com>

Mismatched sob. Be sure you run checkpatch before posting.

> ---
>  .../bindings/phy/snps,eusb2-phy.yaml          | 64 +++++++++++++++++--
>  1 file changed, 57 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml b/Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml
> index 22c77968f..f4164db71 100644
> --- a/Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/snps,eusb2-phy.yaml
> @@ -23,6 +23,7 @@ properties:
>                - qcom,x1e80100-snps-eusb2-phy
>            - const: qcom,sm8550-snps-eusb2-phy
>        - const: qcom,sm8550-snps-eusb2-phy
> +      - const: samsung,exynos2200-snps-eusb2-phy
>  
>    reg:
>      maxItems: 1
> @@ -31,12 +32,12 @@ properties:
>      const: 0
>  
>    clocks:
> -    items:
> -      - description: ref
> +    minItems: 1
> +    maxItems: 3
>  
>    clock-names:
> -    items:
> -      - const: ref
> +    minItems: 1
> +    maxItems: 3
>  
>    resets:
>      maxItems: 1
> @@ -58,11 +59,60 @@ required:
>    - compatible
>    - reg
>    - "#phy-cells"
> -  - clocks
> -  - clock-names
>    - vdd-supply
>    - vdda12-supply
> -  - resets
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sm8550-snps-eusb2-phy
> +
> +    then:
> +      properties:
> +        reg:
> +          maxItems: 1

I don't understand this. What's the top-level value here?

> +
> +        clocks:
> +          items:
> +            - description: ref
> +
> +        clock-names:
> +          items:
> +            - const: ref
> +
> +      required:
> +        - clocks
> +        - clock-names
> +        - resets
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - samsung,exynos2200-snps-eusb2-phy
> +
> +    then:
> +      properties:
> +

Drop blank line

Best regards,
Krzysztof

[PATCH v2 3/8] dt-bindings: phy: add samsung,exynos2200-usbcon-phy schema file

[Ivaylo Ivanov]

The Exynos2200 SoC has a USB controller PHY, which acts as an
intermediary between a USB controller (typically DWC3) and other PHYs
(UTMI, PIPE3). Add a dt-binding schema for it.

Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
---
 .../phy/samsung,exynos2200-usbcon-phy.yaml    | 76 +++++++++++++++++++
 1 file changed, 76 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
new file mode 100644
index 000000000..7d879ec8b
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/samsung,exynos2200-usbcon-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Exynos2200 USB controller PHY
+
+maintainers:
+  - Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
+
+description:
+  Exynos2200 USB controller PHY is an intermediary between a USB controller
+  (typically DWC3) and other PHYs (UTMI, PIPE3).
+
+properties:
+  compatible:
+    enum:
+      - samsung,exynos2200-usbcon-phy
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: bus clock
+
+  clock-names:
+    items:
+      - const: bus
+
+  "#phy-cells":
+    const: 0
+
+  phys:
+    maxItems: 1
+    description:
+      Phandle to eUSB2 PHY
+
+  samsung,pmu-syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    maxItems: 1
+    items:
+      minItems: 1
+      items:
+        - description: phandle for PMU system controller interface, used to
+                       control PMU registers bits for USBCON PHY
+        - description: offset of the pmu control register
+    description:
+      Should be phandle/offset pair. The phandle to System Register syscon node
+      and the offset for USBCON PHY.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - "#phy-cells"
+  - phys
+  - samsung,pmu-syscon
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/samsung,exynos2200-cmu.h>
+
+    usb_con_phy: phy@10aa0000 {
+        compatible = "samsung,exynos2200-usbcon-phy";
+        reg = <0x10aa0000 0x10000>;
+        clocks = <&cmu_hsi0 CLK_MOUT_HSI0_NOC>;
+        clock-names = "bus";
+        #phy-cells = <0>;
+        phys = <&usb_hs_phy>;
+        samsung,pmu-syscon = <&pmu_system_controller 0x72c>;
+    };
-- 
2.43.0

Re: [PATCH v2 3/8] dt-bindings: phy: add samsung,exynos2200-usbcon-phy schema file

[Rob Herring (Arm)]

On Sun, 23 Feb 2025 14:22:22 +0200, Ivaylo Ivanov wrote:
> The Exynos2200 SoC has a USB controller PHY, which acts as an
> intermediary between a USB controller (typically DWC3) and other PHYs
> (UTMI, PIPE3). Add a dt-binding schema for it.
> 
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> ---
>  .../phy/samsung,exynos2200-usbcon-phy.yaml    | 76 +++++++++++++++++++
>  1 file changed, 76 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.example.dts:18:18: fatal error: dt-bindings/clock/samsung,exynos2200-cmu.h: No such file or directory
   18 |         #include <dt-bindings/clock/samsung,exynos2200-cmu.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [scripts/Makefile.dtbs:131: Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1511: dt_binding_check] Error 2
make: *** [Makefile:251: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250223122227.725233-4-ivo.ivanov.ivanov1@gmail.com

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

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

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v2 3/8] dt-bindings: phy: add samsung,exynos2200-usbcon-phy schema file

[Krzysztof Kozlowski]

On Sun, Feb 23, 2025 at 02:22:22PM +0200, Ivaylo Ivanov wrote:
> The Exynos2200 SoC has a USB controller PHY, which acts as an
> intermediary between a USB controller (typically DWC3) and other PHYs
> (UTMI, PIPE3). Add a dt-binding schema for it.
> 
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> ---
>  .../phy/samsung,exynos2200-usbcon-phy.yaml    | 76 +++++++++++++++++++
>  1 file changed, 76 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml

You have undocumented dependencies which prevent merging this file.
First, dependencies have to be clearly expressed. Second, you should
rather decouple the code from header dependencies, otherwise this cannot
be merged for current release (just use clocks with long names, without IDs).

> 
> diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
> new file mode 100644
> index 000000000..7d879ec8b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
> @@ -0,0 +1,76 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/samsung,exynos2200-usbcon-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Exynos2200 USB controller PHY
> +
> +maintainers:
> +  - Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> +
> +description:
> +  Exynos2200 USB controller PHY is an intermediary between a USB controller
> +  (typically DWC3) and other PHYs (UTMI, PIPE3).

Isn't this the same as usbdrd phy? see: samsung,usb3-drd-phy.yaml

I think there is no PHY between DWC3 and UTMI/PIPE. There is a PHY
controller (so the samsung,usb3-drd-phy.yaml) which we call here the
phy.


Best regards,
Krzysztof

Re: [PATCH v2 3/8] dt-bindings: phy: add samsung,exynos2200-usbcon-phy schema file

[Ivaylo Ivanov]

On 2/24/25 10:56, Krzysztof Kozlowski wrote:
> On Sun, Feb 23, 2025 at 02:22:22PM +0200, Ivaylo Ivanov wrote:
>> The Exynos2200 SoC has a USB controller PHY, which acts as an
>> intermediary between a USB controller (typically DWC3) and other PHYs
>> (UTMI, PIPE3). Add a dt-binding schema for it.
>>
>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>> ---
>>  .../phy/samsung,exynos2200-usbcon-phy.yaml    | 76 +++++++++++++++++++
>>  1 file changed, 76 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
> You have undocumented dependencies which prevent merging this file.
> First, dependencies have to be clearly expressed.

They are, in the cover letter.

> Second, you should
> rather decouple the code from header dependencies, otherwise this cannot
> be merged for current release (just use clocks with long names, without IDs).

Sure

>
>> diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>> new file mode 100644
>> index 000000000..7d879ec8b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>> @@ -0,0 +1,76 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/phy/samsung,exynos2200-usbcon-phy.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Exynos2200 USB controller PHY
>> +
>> +maintainers:
>> +  - Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>> +
>> +description:
>> +  Exynos2200 USB controller PHY is an intermediary between a USB controller
>> +  (typically DWC3) and other PHYs (UTMI, PIPE3).
> Isn't this the same as usbdrd phy? see: samsung,usb3-drd-phy.yaml

It's not (I think). There's a few reasons I've decided to make this separate
from the usb3-drd-phy bindings and exynos5-usbdrd driver:

1. This PHY does not provide UTMI and PIPE3 on its own. There's no tuning
for them, and all that is needed from it is to disable HWACG, assert/
deassert reset and force bvalid/vbusvalid. After that SNPS eUSB2
initialization can be done and USB2 works. If the USBCON phy is not set
up before the eUSB2 one, the device hangs, so there is definitely a
dependancy between them. For PIPE3 we'd need to control the pipe3
attaching/deattaching and then initialize the synopsys USBDP combophy.

2. With the way it's modelled, we need to parse phandles from eUSB2 and
USBDP to the controller. Adding that to the usbdrd driver would be...
weird. It makes more sense to model it as a separate driver, because
it functions in a different way.

I've described this in the cover letter as well.

Best regards,
Ivaylo

>
> I think there is no PHY between DWC3 and UTMI/PIPE. There is a PHY
> controller (so the samsung,usb3-drd-phy.yaml) which we call here the
> phy.
>
>
> Best regards,
> Krzysztof
>

Re: [PATCH v2 3/8] dt-bindings: phy: add samsung,exynos2200-usbcon-phy schema file

[Krzysztof Kozlowski]

On 24/02/2025 11:48, Ivaylo Ivanov wrote:
> On 2/24/25 10:56, Krzysztof Kozlowski wrote:
>> On Sun, Feb 23, 2025 at 02:22:22PM +0200, Ivaylo Ivanov wrote:
>>> The Exynos2200 SoC has a USB controller PHY, which acts as an
>>> intermediary between a USB controller (typically DWC3) and other PHYs
>>> (UTMI, PIPE3). Add a dt-binding schema for it.
>>>
>>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>>> ---
>>>  .../phy/samsung,exynos2200-usbcon-phy.yaml    | 76 +++++++++++++++++++
>>>  1 file changed, 76 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>> You have undocumented dependencies which prevent merging this file.
>> First, dependencies have to be clearly expressed.
> 
> They are, in the cover letter.

Where? I read it twice. Dependencies is the most important thing and
should scream at beginning of the cover letter, so if you bury them
somewhere deep it also would not matter - just like they were missing.

> 
>> Second, you should
>> rather decouple the code from header dependencies, otherwise this cannot
>> be merged for current release (just use clocks with long names, without IDs).
> 
> Sure


> 
>>
>>> diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>> new file mode 100644
>>> index 000000000..7d879ec8b
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>> @@ -0,0 +1,76 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/phy/samsung,exynos2200-usbcon-phy.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Exynos2200 USB controller PHY
>>> +
>>> +maintainers:
>>> +  - Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>>> +
>>> +description:
>>> +  Exynos2200 USB controller PHY is an intermediary between a USB controller
>>> +  (typically DWC3) and other PHYs (UTMI, PIPE3).
>> Isn't this the same as usbdrd phy? see: samsung,usb3-drd-phy.yaml
> 
> It's not (I think). There's a few reasons I've decided to make this separate
> from the usb3-drd-phy bindings and exynos5-usbdrd driver:
> 
> 1. This PHY does not provide UTMI and PIPE3 on its own. There's no tuning

USBDRD phy does not provide UTMI and PIPE on its own either if you look
at diagram - they call it phy controller.

> for them, and all that is needed from it is to disable HWACG, assert/
> deassert reset and force bvalid/vbusvalid. After that SNPS eUSB2
> initialization can be done and USB2 works. If the USBCON phy is not set
> up before the eUSB2 one, the device hangs, so there is definitely a
> dependancy between them. For PIPE3 we'd need to control the pipe3
> attaching/deattaching and then initialize the synopsys USBDP combophy.

Does it mean there is no USB DRD phy controller as before?

Anyway the problem is you have DWC3 -> PHY -> PHY. Looks one phy too many.


> 
> 2. With the way it's modelled, we need to parse phandles from eUSB2 and
> USBDP to the controller. Adding that to the usbdrd driver would be...
> weird. It makes more sense to model it as a separate driver, because
> it functions in a different way.

Just to be clear: we don't talk about drivers here.


Best regards,
Krzysztof

Re: [PATCH v2 3/8] dt-bindings: phy: add samsung,exynos2200-usbcon-phy schema file

[Ivaylo Ivanov]

On 2/25/25 10:11, Krzysztof Kozlowski wrote:
> On 24/02/2025 11:48, Ivaylo Ivanov wrote:
>> On 2/24/25 10:56, Krzysztof Kozlowski wrote:
>>> On Sun, Feb 23, 2025 at 02:22:22PM +0200, Ivaylo Ivanov wrote:
>>>> The Exynos2200 SoC has a USB controller PHY, which acts as an
>>>> intermediary between a USB controller (typically DWC3) and other PHYs
>>>> (UTMI, PIPE3). Add a dt-binding schema for it.
>>>>
>>>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>>>> ---
>>>>  .../phy/samsung,exynos2200-usbcon-phy.yaml    | 76 +++++++++++++++++++
>>>>  1 file changed, 76 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>> You have undocumented dependencies which prevent merging this file.
>>> First, dependencies have to be clearly expressed.
>> They are, in the cover letter.
> Where? I read it twice. Dependencies is the most important thing and
> should scream at beginning of the cover letter, so if you bury them
> somewhere deep it also would not matter - just like they were missing.
>
>>> Second, you should
>>> rather decouple the code from header dependencies, otherwise this cannot
>>> be merged for current release (just use clocks with long names, without IDs).
>> Sure
>
>>>> diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>> new file mode 100644
>>>> index 000000000..7d879ec8b
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>> @@ -0,0 +1,76 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/phy/samsung,exynos2200-usbcon-phy.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Exynos2200 USB controller PHY
>>>> +
>>>> +maintainers:
>>>> +  - Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>>>> +
>>>> +description:
>>>> +  Exynos2200 USB controller PHY is an intermediary between a USB controller
>>>> +  (typically DWC3) and other PHYs (UTMI, PIPE3).
>>> Isn't this the same as usbdrd phy? see: samsung,usb3-drd-phy.yaml
>> It's not (I think). There's a few reasons I've decided to make this separate
>> from the usb3-drd-phy bindings and exynos5-usbdrd driver:
>>
>> 1. This PHY does not provide UTMI and PIPE3 on its own. There's no tuning
> USBDRD phy does not provide UTMI and PIPE on its own either if you look
> at diagram - they call it phy controller.

Ughm. What? So in most exynos cases, there's a combination of multiple phys?

>
>> for them, and all that is needed from it is to disable HWACG, assert/
>> deassert reset and force bvalid/vbusvalid. After that SNPS eUSB2
>> initialization can be done and USB2 works. If the USBCON phy is not set
>> up before the eUSB2 one, the device hangs, so there is definitely a
>> dependancy between them. For PIPE3 we'd need to control the pipe3
>> attaching/deattaching and then initialize the synopsys USBDP combophy.
> Does it mean there is no USB DRD phy controller as before?
>
> Anyway the problem is you have DWC3 -> PHY -> PHY. Looks one phy too many.

So...

DWC3 -> USBDRD (USBCON) -> PHYs?

...with usbdrd controller connecting and controlling the USB2 and USB3
phys, as well as dual role mode? Well, where is the DRD part in the exynos5
driver?

I guess it does perfectly fit the job of a usbdrd controller then (if it
even deals with DRD). But then again,  this brings up two questions:
1. Should this driver even be named exynos2200-usbcon and not, for
example, exynos2200-usbdrd?
2. Are the exynos5-usbdrd phys really only USBDRD, or do they implement
USB speed functionality? What is the UTMI/PIPE3 setup for then?

ps: dealing with this without any documentations sucks.

Best regards,
Ivaylo

>
>
>> 2. With the way it's modelled, we need to parse phandles from eUSB2 and
>> USBDP to the controller. Adding that to the usbdrd driver would be...
>> weird. It makes more sense to model it as a separate driver, because
>> it functions in a different way.
> Just to be clear: we don't talk about drivers here.
>
>
> Best regards,
> Krzysztof

Re: [PATCH v2 3/8] dt-bindings: phy: add samsung,exynos2200-usbcon-phy schema file

[Krzysztof Kozlowski]

On 02/03/2025 10:16, Ivaylo Ivanov wrote:
> On 2/25/25 10:11, Krzysztof Kozlowski wrote:
>> On 24/02/2025 11:48, Ivaylo Ivanov wrote:
>>> On 2/24/25 10:56, Krzysztof Kozlowski wrote:
>>>> On Sun, Feb 23, 2025 at 02:22:22PM +0200, Ivaylo Ivanov wrote:
>>>>> The Exynos2200 SoC has a USB controller PHY, which acts as an
>>>>> intermediary between a USB controller (typically DWC3) and other PHYs
>>>>> (UTMI, PIPE3). Add a dt-binding schema for it.
>>>>>
>>>>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>>>>> ---
>>>>>  .../phy/samsung,exynos2200-usbcon-phy.yaml    | 76 +++++++++++++++++++
>>>>>  1 file changed, 76 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>> You have undocumented dependencies which prevent merging this file.
>>>> First, dependencies have to be clearly expressed.
>>> They are, in the cover letter.
>> Where? I read it twice. Dependencies is the most important thing and
>> should scream at beginning of the cover letter, so if you bury them
>> somewhere deep it also would not matter - just like they were missing.
>>
>>>> Second, you should
>>>> rather decouple the code from header dependencies, otherwise this cannot
>>>> be merged for current release (just use clocks with long names, without IDs).
>>> Sure
>>
>>>>> diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>>> new file mode 100644
>>>>> index 000000000..7d879ec8b
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>>> @@ -0,0 +1,76 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/phy/samsung,exynos2200-usbcon-phy.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Exynos2200 USB controller PHY
>>>>> +
>>>>> +maintainers:
>>>>> +  - Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>>>>> +
>>>>> +description:
>>>>> +  Exynos2200 USB controller PHY is an intermediary between a USB controller
>>>>> +  (typically DWC3) and other PHYs (UTMI, PIPE3).
>>>> Isn't this the same as usbdrd phy? see: samsung,usb3-drd-phy.yaml
>>> It's not (I think). There's a few reasons I've decided to make this separate
>>> from the usb3-drd-phy bindings and exynos5-usbdrd driver:
>>>
>>> 1. This PHY does not provide UTMI and PIPE3 on its own. There's no tuning
>> USBDRD phy does not provide UTMI and PIPE on its own either if you look
>> at diagram - they call it phy controller.
> 
> Ughm. What? So in most exynos cases, there's a combination of multiple phys?


> 
>>
>>> for them, and all that is needed from it is to disable HWACG, assert/
>>> deassert reset and force bvalid/vbusvalid. After that SNPS eUSB2
>>> initialization can be done and USB2 works. If the USBCON phy is not set
>>> up before the eUSB2 one, the device hangs, so there is definitely a
>>> dependancy between them. For PIPE3 we'd need to control the pipe3
>>> attaching/deattaching and then initialize the synopsys USBDP combophy.
>> Does it mean there is no USB DRD phy controller as before?
>>
>> Anyway the problem is you have DWC3 -> PHY -> PHY. Looks one phy too many.
> 
> So...
> 
> DWC3 -> USBDRD (USBCON) -> PHYs?

No, drop last phy. You just wrote the same as me - two phys, because
usbdrd is the phy. In all existing designs there is no such controllable
object from the point of view of operating system.

> 
> ...with usbdrd controller connecting and controlling the USB2 and USB3
> phys, as well as dual role mode?

Yes.

> Well, where is the DRD part in the exynos5
> driver?

DRD? I believe it is part of DWC3, the same as in every other standard
implementation of Synopsys DWC3.

> 
> I guess it does perfectly fit the job of a usbdrd controller then (if it
> even deals with DRD). But then again,  this brings up two questions:
> 1. Should this driver even be named exynos2200-usbcon and not, for
> example, exynos2200-usbdrd?

Are you sure we talk about the same thing? USBDRD is IP block in the
Exynos and a device driver. Call your device as appropriate it is -
based on datasheet or downstream sources.

> 2. Are the exynos5-usbdrd phys really only USBDRD, or do they implement
> USB speed functionality? What is the UTMI/PIPE3 setup for then?

Dunno, I don't get what you mean by "exynos5-usbdrd phys really only
USBDRD". USBDRD is just the name of the device.

Best regards,
Krzysztof

Re: [PATCH v2 3/8] dt-bindings: phy: add samsung,exynos2200-usbcon-phy schema file

[Ivaylo Ivanov]

On 3/3/25 09:24, Krzysztof Kozlowski wrote:
> On 02/03/2025 10:16, Ivaylo Ivanov wrote:
>> On 2/25/25 10:11, Krzysztof Kozlowski wrote:
>>> On 24/02/2025 11:48, Ivaylo Ivanov wrote:
>>>> On 2/24/25 10:56, Krzysztof Kozlowski wrote:
>>>>> On Sun, Feb 23, 2025 at 02:22:22PM +0200, Ivaylo Ivanov wrote:
>>>>>> The Exynos2200 SoC has a USB controller PHY, which acts as an
>>>>>> intermediary between a USB controller (typically DWC3) and other PHYs
>>>>>> (UTMI, PIPE3). Add a dt-binding schema for it.
>>>>>>
>>>>>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>>>>>> ---
>>>>>>  .../phy/samsung,exynos2200-usbcon-phy.yaml    | 76 +++++++++++++++++++
>>>>>>  1 file changed, 76 insertions(+)
>>>>>>  create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>>> You have undocumented dependencies which prevent merging this file.
>>>>> First, dependencies have to be clearly expressed.
>>>> They are, in the cover letter.
>>> Where? I read it twice. Dependencies is the most important thing and
>>> should scream at beginning of the cover letter, so if you bury them
>>> somewhere deep it also would not matter - just like they were missing.
>>>
>>>>> Second, you should
>>>>> rather decouple the code from header dependencies, otherwise this cannot
>>>>> be merged for current release (just use clocks with long names, without IDs).
>>>> Sure
>>>>>> diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>>>> new file mode 100644
>>>>>> index 000000000..7d879ec8b
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>>>> @@ -0,0 +1,76 @@
>>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>>> +%YAML 1.2
>>>>>> +---
>>>>>> +$id: http://devicetree.org/schemas/phy/samsung,exynos2200-usbcon-phy.yaml#
>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>> +
>>>>>> +title: Exynos2200 USB controller PHY
>>>>>> +
>>>>>> +maintainers:
>>>>>> +  - Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>>>>>> +
>>>>>> +description:
>>>>>> +  Exynos2200 USB controller PHY is an intermediary between a USB controller
>>>>>> +  (typically DWC3) and other PHYs (UTMI, PIPE3).
>>>>> Isn't this the same as usbdrd phy? see: samsung,usb3-drd-phy.yaml
>>>> It's not (I think). There's a few reasons I've decided to make this separate
>>>> from the usb3-drd-phy bindings and exynos5-usbdrd driver:
>>>>
>>>> 1. This PHY does not provide UTMI and PIPE3 on its own. There's no tuning
>>> USBDRD phy does not provide UTMI and PIPE on its own either if you look
>>> at diagram - they call it phy controller.
>> Ughm. What? So in most exynos cases, there's a combination of multiple phys?
>
>>>> for them, and all that is needed from it is to disable HWACG, assert/
>>>> deassert reset and force bvalid/vbusvalid. After that SNPS eUSB2
>>>> initialization can be done and USB2 works. If the USBCON phy is not set
>>>> up before the eUSB2 one, the device hangs, so there is definitely a
>>>> dependancy between them. For PIPE3 we'd need to control the pipe3
>>>> attaching/deattaching and then initialize the synopsys USBDP combophy.
>>> Does it mean there is no USB DRD phy controller as before?
>>>
>>> Anyway the problem is you have DWC3 -> PHY -> PHY. Looks one phy too many.
>> So...
>>
>> DWC3 -> USBDRD (USBCON) -> PHYs?
> No, drop last phy. You just wrote the same as me - two phys, because
> usbdrd is the phy. In all existing designs there is no such controllable
> object from the point of view of operating system.

What? Per my understanding, the phy property should refer to whatever is
is connected to dwc3 UTMI. In this case it's the so-called USBDRD phy (called
usbcon in downstream). Considering that the eUSB2 IP definitely also has UTMI
that has to be connected to something, doesn't that mean we have clearly
separated hardware blocks? Now, I guess one could argue that this USBCON
hardware block could be classified as a syscon. But I don't see the problem
with the current binding description, nor the modelling, as it represents
how the hardware is (unless I've gotten it completely wrong).

Best regards,
Ivaylo

>
>> ...with usbdrd controller connecting and controlling the USB2 and USB3
>> phys, as well as dual role mode?
> Yes.
>
>> Well, where is the DRD part in the exynos5
>> driver?
> DRD? I believe it is part of DWC3, the same as in every other standard
> implementation of Synopsys DWC3.
>
>> I guess it does perfectly fit the job of a usbdrd controller then (if it
>> even deals with DRD). But then again,  this brings up two questions:
>> 1. Should this driver even be named exynos2200-usbcon and not, for
>> example, exynos2200-usbdrd?
> Are you sure we talk about the same thing? USBDRD is IP block in the
> Exynos and a device driver. Call your device as appropriate it is -
> based on datasheet or downstream sources.
>
>> 2. Are the exynos5-usbdrd phys really only USBDRD, or do they implement
>> USB speed functionality? What is the UTMI/PIPE3 setup for then?
> Dunno, I don't get what you mean by "exynos5-usbdrd phys really only
> USBDRD". USBDRD is just the name of the device.
>
> Best regards,
> Krzysztof

Re: [PATCH v2 3/8] dt-bindings: phy: add samsung,exynos2200-usbcon-phy schema file

[Krzysztof Kozlowski]

On 03/03/2025 18:18, Ivaylo Ivanov wrote:
> On 3/3/25 09:24, Krzysztof Kozlowski wrote:
>> On 02/03/2025 10:16, Ivaylo Ivanov wrote:
>>> On 2/25/25 10:11, Krzysztof Kozlowski wrote:
>>>> On 24/02/2025 11:48, Ivaylo Ivanov wrote:
>>>>> On 2/24/25 10:56, Krzysztof Kozlowski wrote:
>>>>>> On Sun, Feb 23, 2025 at 02:22:22PM +0200, Ivaylo Ivanov wrote:
>>>>>>> The Exynos2200 SoC has a USB controller PHY, which acts as an
>>>>>>> intermediary between a USB controller (typically DWC3) and other PHYs
>>>>>>> (UTMI, PIPE3). Add a dt-binding schema for it.
>>>>>>>
>>>>>>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>>>>>>> ---
>>>>>>>  .../phy/samsung,exynos2200-usbcon-phy.yaml    | 76 +++++++++++++++++++
>>>>>>>  1 file changed, 76 insertions(+)
>>>>>>>  create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>>>> You have undocumented dependencies which prevent merging this file.
>>>>>> First, dependencies have to be clearly expressed.
>>>>> They are, in the cover letter.
>>>> Where? I read it twice. Dependencies is the most important thing and
>>>> should scream at beginning of the cover letter, so if you bury them
>>>> somewhere deep it also would not matter - just like they were missing.
>>>>
>>>>>> Second, you should
>>>>>> rather decouple the code from header dependencies, otherwise this cannot
>>>>>> be merged for current release (just use clocks with long names, without IDs).
>>>>> Sure
>>>>>>> diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>>>>> new file mode 100644
>>>>>>> index 000000000..7d879ec8b
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>>>>> @@ -0,0 +1,76 @@
>>>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>>>> +%YAML 1.2
>>>>>>> +---
>>>>>>> +$id: http://devicetree.org/schemas/phy/samsung,exynos2200-usbcon-phy.yaml#
>>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>>> +
>>>>>>> +title: Exynos2200 USB controller PHY
>>>>>>> +
>>>>>>> +maintainers:
>>>>>>> +  - Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>>>>>>> +
>>>>>>> +description:
>>>>>>> +  Exynos2200 USB controller PHY is an intermediary between a USB controller
>>>>>>> +  (typically DWC3) and other PHYs (UTMI, PIPE3).
>>>>>> Isn't this the same as usbdrd phy? see: samsung,usb3-drd-phy.yaml
>>>>> It's not (I think). There's a few reasons I've decided to make this separate
>>>>> from the usb3-drd-phy bindings and exynos5-usbdrd driver:
>>>>>
>>>>> 1. This PHY does not provide UTMI and PIPE3 on its own. There's no tuning
>>>> USBDRD phy does not provide UTMI and PIPE on its own either if you look
>>>> at diagram - they call it phy controller.
>>> Ughm. What? So in most exynos cases, there's a combination of multiple phys?
>>
>>>>> for them, and all that is needed from it is to disable HWACG, assert/
>>>>> deassert reset and force bvalid/vbusvalid. After that SNPS eUSB2
>>>>> initialization can be done and USB2 works. If the USBCON phy is not set
>>>>> up before the eUSB2 one, the device hangs, so there is definitely a
>>>>> dependancy between them. For PIPE3 we'd need to control the pipe3
>>>>> attaching/deattaching and then initialize the synopsys USBDP combophy.
>>>> Does it mean there is no USB DRD phy controller as before?
>>>>
>>>> Anyway the problem is you have DWC3 -> PHY -> PHY. Looks one phy too many.
>>> So...
>>>
>>> DWC3 -> USBDRD (USBCON) -> PHYs?
>> No, drop last phy. You just wrote the same as me - two phys, because
>> usbdrd is the phy. In all existing designs there is no such controllable
>> object from the point of view of operating system.
> 
> What? Per my understanding, the phy property should refer to whatever is
> is connected to dwc3 UTMI. In this case it's the so-called USBDRD phy (called
> usbcon in downstream). Considering that the eUSB2 IP definitely also has UTMI
> that has to be connected to something, doesn't that mean we have clearly

The entire point is that eUSB2 is connected to DWC3, no? That's exactly
how it is done for example on Qualcomm SoC. Otherwise you claim that
DWC3 controls one phy, which controls another phy which controls UTMI...

> separated hardware blocks? Now, I guess one could argue that this USBCON
> hardware block could be classified as a syscon. But I don't see the problem
> with the current binding description, nor the modelling, as it represents
> how the hardware is (unless I've gotten it completely wrong).

It is the first time you use argument that it represents how the
hardware is and this is what we actually disagree. It is not like that.
You do not have chain of phys. Just look at any USB 3.0 DRD DWC diagram
from any Samsung SoC: where would you squeeze these two phys in relation
to what is called there "USB 3.0 PHY" which would be the third phy (!!!).

Best regards,
Krzysztof

Re: [PATCH v2 3/8] dt-bindings: phy: add samsung,exynos2200-usbcon-phy schema file

[Ivaylo Ivanov]

On 3/4/25 09:21, Krzysztof Kozlowski wrote:
> On 03/03/2025 18:18, Ivaylo Ivanov wrote:
>> On 3/3/25 09:24, Krzysztof Kozlowski wrote:
>>> On 02/03/2025 10:16, Ivaylo Ivanov wrote:
>>>> On 2/25/25 10:11, Krzysztof Kozlowski wrote:
>>>>> On 24/02/2025 11:48, Ivaylo Ivanov wrote:
>>>>>> On 2/24/25 10:56, Krzysztof Kozlowski wrote:
>>>>>>> On Sun, Feb 23, 2025 at 02:22:22PM +0200, Ivaylo Ivanov wrote:
>>>>>>>> The Exynos2200 SoC has a USB controller PHY, which acts as an
>>>>>>>> intermediary between a USB controller (typically DWC3) and other PHYs
>>>>>>>> (UTMI, PIPE3). Add a dt-binding schema for it.
>>>>>>>>
>>>>>>>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>>>>>>>> ---
>>>>>>>>  .../phy/samsung,exynos2200-usbcon-phy.yaml    | 76 +++++++++++++++++++
>>>>>>>>  1 file changed, 76 insertions(+)
>>>>>>>>  create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>>>>> You have undocumented dependencies which prevent merging this file.
>>>>>>> First, dependencies have to be clearly expressed.
>>>>>> They are, in the cover letter.
>>>>> Where? I read it twice. Dependencies is the most important thing and
>>>>> should scream at beginning of the cover letter, so if you bury them
>>>>> somewhere deep it also would not matter - just like they were missing.
>>>>>
>>>>>>> Second, you should
>>>>>>> rather decouple the code from header dependencies, otherwise this cannot
>>>>>>> be merged for current release (just use clocks with long names, without IDs).
>>>>>> Sure
>>>>>>>> diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>>>>>> new file mode 100644
>>>>>>>> index 000000000..7d879ec8b
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>>>>>> @@ -0,0 +1,76 @@
>>>>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>>>>> +%YAML 1.2
>>>>>>>> +---
>>>>>>>> +$id: http://devicetree.org/schemas/phy/samsung,exynos2200-usbcon-phy.yaml#
>>>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>>>> +
>>>>>>>> +title: Exynos2200 USB controller PHY
>>>>>>>> +
>>>>>>>> +maintainers:
>>>>>>>> +  - Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>>>>>>>> +
>>>>>>>> +description:
>>>>>>>> +  Exynos2200 USB controller PHY is an intermediary between a USB controller
>>>>>>>> +  (typically DWC3) and other PHYs (UTMI, PIPE3).
>>>>>>> Isn't this the same as usbdrd phy? see: samsung,usb3-drd-phy.yaml
>>>>>> It's not (I think). There's a few reasons I've decided to make this separate
>>>>>> from the usb3-drd-phy bindings and exynos5-usbdrd driver:
>>>>>>
>>>>>> 1. This PHY does not provide UTMI and PIPE3 on its own. There's no tuning
>>>>> USBDRD phy does not provide UTMI and PIPE on its own either if you look
>>>>> at diagram - they call it phy controller.
>>>> Ughm. What? So in most exynos cases, there's a combination of multiple phys?
>>>>>> for them, and all that is needed from it is to disable HWACG, assert/
>>>>>> deassert reset and force bvalid/vbusvalid. After that SNPS eUSB2
>>>>>> initialization can be done and USB2 works. If the USBCON phy is not set
>>>>>> up before the eUSB2 one, the device hangs, so there is definitely a
>>>>>> dependancy between them. For PIPE3 we'd need to control the pipe3
>>>>>> attaching/deattaching and then initialize the synopsys USBDP combophy.
>>>>> Does it mean there is no USB DRD phy controller as before?
>>>>>
>>>>> Anyway the problem is you have DWC3 -> PHY -> PHY. Looks one phy too many.
>>>> So...
>>>>
>>>> DWC3 -> USBDRD (USBCON) -> PHYs?
>>> No, drop last phy. You just wrote the same as me - two phys, because
>>> usbdrd is the phy. In all existing designs there is no such controllable
>>> object from the point of view of operating system.
>> What? Per my understanding, the phy property should refer to whatever is
>> is connected to dwc3 UTMI. In this case it's the so-called USBDRD phy (called
>> usbcon in downstream). Considering that the eUSB2 IP definitely also has UTMI
>> that has to be connected to something, doesn't that mean we have clearly
> The entire point is that eUSB2 is connected to DWC3, no? That's exactly
> how it is done for example on Qualcomm SoC. Otherwise you claim that
> DWC3 controls one phy, which controls another phy which controls UTMI...

But where does the USBCON fit? Is it just a side controller? Why's it needed
in the first place? This is what I don't understand.

>
>> separated hardware blocks? Now, I guess one could argue that this USBCON
>> hardware block could be classified as a syscon. But I don't see the problem
>> with the current binding description, nor the modelling, as it represents
>> how the hardware is (unless I've gotten it completely wrong).
> It is the first time you use argument that it represents how the
> hardware is and this is what we actually disagree. It is not like that.
> You do not have chain of phys. Just look at any USB 3.0 DRD DWC diagram
> from any Samsung SoC: where would you squeeze these two phys in relation
> to what is called there "USB 3.0 PHY" which would be the third phy (!!!).

Yeah, my point was that it was different from any previous design. Now,
I don't know if it's actually theoretically possible to design it like so. It's
hard to just guess how the hardware is designed without having access
to die shots, documentations or even just schematics.

Let's make it clear now, the changes your request are to document USBCON
in the existing exynos binding, as well as to correct all explanations of how
this block functions, right?

Best regards,
Ivaylo

>
> Best regards,
> Krzysztof

Re: [PATCH v2 3/8] dt-bindings: phy: add samsung,exynos2200-usbcon-phy schema file

[Krzysztof Kozlowski]

On 04/03/2025 10:09, Ivaylo Ivanov wrote:
> On 3/4/25 09:21, Krzysztof Kozlowski wrote:
>> On 03/03/2025 18:18, Ivaylo Ivanov wrote:
>>> On 3/3/25 09:24, Krzysztof Kozlowski wrote:
>>>> On 02/03/2025 10:16, Ivaylo Ivanov wrote:
>>>>> On 2/25/25 10:11, Krzysztof Kozlowski wrote:
>>>>>> On 24/02/2025 11:48, Ivaylo Ivanov wrote:
>>>>>>> On 2/24/25 10:56, Krzysztof Kozlowski wrote:
>>>>>>>> On Sun, Feb 23, 2025 at 02:22:22PM +0200, Ivaylo Ivanov wrote:
>>>>>>>>> The Exynos2200 SoC has a USB controller PHY, which acts as an
>>>>>>>>> intermediary between a USB controller (typically DWC3) and other PHYs
>>>>>>>>> (UTMI, PIPE3). Add a dt-binding schema for it.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>>>>>>>>> ---
>>>>>>>>>  .../phy/samsung,exynos2200-usbcon-phy.yaml    | 76 +++++++++++++++++++
>>>>>>>>>  1 file changed, 76 insertions(+)
>>>>>>>>>  create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>>>>>> You have undocumented dependencies which prevent merging this file.
>>>>>>>> First, dependencies have to be clearly expressed.
>>>>>>> They are, in the cover letter.
>>>>>> Where? I read it twice. Dependencies is the most important thing and
>>>>>> should scream at beginning of the cover letter, so if you bury them
>>>>>> somewhere deep it also would not matter - just like they were missing.
>>>>>>
>>>>>>>> Second, you should
>>>>>>>> rather decouple the code from header dependencies, otherwise this cannot
>>>>>>>> be merged for current release (just use clocks with long names, without IDs).
>>>>>>> Sure
>>>>>>>>> diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>>>>>>> new file mode 100644
>>>>>>>>> index 000000000..7d879ec8b
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>>>>>>> @@ -0,0 +1,76 @@
>>>>>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>>>>>> +%YAML 1.2
>>>>>>>>> +---
>>>>>>>>> +$id: http://devicetree.org/schemas/phy/samsung,exynos2200-usbcon-phy.yaml#
>>>>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>>>>> +
>>>>>>>>> +title: Exynos2200 USB controller PHY
>>>>>>>>> +
>>>>>>>>> +maintainers:
>>>>>>>>> +  - Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>>>>>>>>> +
>>>>>>>>> +description:
>>>>>>>>> +  Exynos2200 USB controller PHY is an intermediary between a USB controller
>>>>>>>>> +  (typically DWC3) and other PHYs (UTMI, PIPE3).
>>>>>>>> Isn't this the same as usbdrd phy? see: samsung,usb3-drd-phy.yaml
>>>>>>> It's not (I think). There's a few reasons I've decided to make this separate
>>>>>>> from the usb3-drd-phy bindings and exynos5-usbdrd driver:
>>>>>>>
>>>>>>> 1. This PHY does not provide UTMI and PIPE3 on its own. There's no tuning
>>>>>> USBDRD phy does not provide UTMI and PIPE on its own either if you look
>>>>>> at diagram - they call it phy controller.
>>>>> Ughm. What? So in most exynos cases, there's a combination of multiple phys?
>>>>>>> for them, and all that is needed from it is to disable HWACG, assert/
>>>>>>> deassert reset and force bvalid/vbusvalid. After that SNPS eUSB2
>>>>>>> initialization can be done and USB2 works. If the USBCON phy is not set
>>>>>>> up before the eUSB2 one, the device hangs, so there is definitely a
>>>>>>> dependancy between them. For PIPE3 we'd need to control the pipe3
>>>>>>> attaching/deattaching and then initialize the synopsys USBDP combophy.
>>>>>> Does it mean there is no USB DRD phy controller as before?
>>>>>>
>>>>>> Anyway the problem is you have DWC3 -> PHY -> PHY. Looks one phy too many.
>>>>> So...
>>>>>
>>>>> DWC3 -> USBDRD (USBCON) -> PHYs?
>>>> No, drop last phy. You just wrote the same as me - two phys, because
>>>> usbdrd is the phy. In all existing designs there is no such controllable
>>>> object from the point of view of operating system.
>>> What? Per my understanding, the phy property should refer to whatever is
>>> is connected to dwc3 UTMI. In this case it's the so-called USBDRD phy (called
>>> usbcon in downstream). Considering that the eUSB2 IP definitely also has UTMI
>>> that has to be connected to something, doesn't that mean we have clearly
>> The entire point is that eUSB2 is connected to DWC3, no? That's exactly
>> how it is done for example on Qualcomm SoC. Otherwise you claim that
>> DWC3 controls one phy, which controls another phy which controls UTMI...
> 
> But where does the USBCON fit? Is it just a side controller? Why's it needed
> in the first place? This is what I don't understand.

I assume usbcon, so old usbdrd, is the second DWC3's phy, just like qcom
qmpphy.

> 
>>
>>> separated hardware blocks? Now, I guess one could argue that this USBCON
>>> hardware block could be classified as a syscon. But I don't see the problem
>>> with the current binding description, nor the modelling, as it represents
>>> how the hardware is (unless I've gotten it completely wrong).
>> It is the first time you use argument that it represents how the
>> hardware is and this is what we actually disagree. It is not like that.
>> You do not have chain of phys. Just look at any USB 3.0 DRD DWC diagram
>> from any Samsung SoC: where would you squeeze these two phys in relation
>> to what is called there "USB 3.0 PHY" which would be the third phy (!!!).
> 
> Yeah, my point was that it was different from any previous design. Now,
> I don't know if it's actually theoretically possible to design it like so. It's
> hard to just guess how the hardware is designed without having access
> to die shots, documentations or even just schematics.
> 
> Let's make it clear now, the changes your request are to document USBCON
> in the existing exynos binding, as well as to correct all explanations of how
> this block functions, right?

No, not necessarily. If USBCON is entirely different device than USBDRD
(different register layout, different features), then go ahead with a
new binding.

I was questioning your chain of PHYs and this should be investigated.


Best regards,
Krzysztof

Re: [PATCH v2 3/8] dt-bindings: phy: add samsung,exynos2200-usbcon-phy schema file

[Ivaylo Ivanov]

On 3/4/25 12:03, Krzysztof Kozlowski wrote:
> On 04/03/2025 10:09, Ivaylo Ivanov wrote:
>> On 3/4/25 09:21, Krzysztof Kozlowski wrote:
>>> On 03/03/2025 18:18, Ivaylo Ivanov wrote:
>>>> On 3/3/25 09:24, Krzysztof Kozlowski wrote:
>>>>> On 02/03/2025 10:16, Ivaylo Ivanov wrote:
>>>>>> On 2/25/25 10:11, Krzysztof Kozlowski wrote:
>>>>>>> On 24/02/2025 11:48, Ivaylo Ivanov wrote:
>>>>>>>> On 2/24/25 10:56, Krzysztof Kozlowski wrote:
>>>>>>>>> On Sun, Feb 23, 2025 at 02:22:22PM +0200, Ivaylo Ivanov wrote:
>>>>>>>>>> The Exynos2200 SoC has a USB controller PHY, which acts as an
>>>>>>>>>> intermediary between a USB controller (typically DWC3) and other PHYs
>>>>>>>>>> (UTMI, PIPE3). Add a dt-binding schema for it.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>>>>>>>>>> ---
>>>>>>>>>>  .../phy/samsung,exynos2200-usbcon-phy.yaml    | 76 +++++++++++++++++++
>>>>>>>>>>  1 file changed, 76 insertions(+)
>>>>>>>>>>  create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>>>>>>> You have undocumented dependencies which prevent merging this file.
>>>>>>>>> First, dependencies have to be clearly expressed.
>>>>>>>> They are, in the cover letter.
>>>>>>> Where? I read it twice. Dependencies is the most important thing and
>>>>>>> should scream at beginning of the cover letter, so if you bury them
>>>>>>> somewhere deep it also would not matter - just like they were missing.
>>>>>>>
>>>>>>>>> Second, you should
>>>>>>>>> rather decouple the code from header dependencies, otherwise this cannot
>>>>>>>>> be merged for current release (just use clocks with long names, without IDs).
>>>>>>>> Sure
>>>>>>>>>> diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>>>>>>>> new file mode 100644
>>>>>>>>>> index 000000000..7d879ec8b
>>>>>>>>>> --- /dev/null
>>>>>>>>>> +++ b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml
>>>>>>>>>> @@ -0,0 +1,76 @@
>>>>>>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>>>>>>> +%YAML 1.2
>>>>>>>>>> +---
>>>>>>>>>> +$id: http://devicetree.org/schemas/phy/samsung,exynos2200-usbcon-phy.yaml#
>>>>>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>>>>>> +
>>>>>>>>>> +title: Exynos2200 USB controller PHY
>>>>>>>>>> +
>>>>>>>>>> +maintainers:
>>>>>>>>>> +  - Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>>>>>>>>>> +
>>>>>>>>>> +description:
>>>>>>>>>> +  Exynos2200 USB controller PHY is an intermediary between a USB controller
>>>>>>>>>> +  (typically DWC3) and other PHYs (UTMI, PIPE3).
>>>>>>>>> Isn't this the same as usbdrd phy? see: samsung,usb3-drd-phy.yaml
>>>>>>>> It's not (I think). There's a few reasons I've decided to make this separate
>>>>>>>> from the usb3-drd-phy bindings and exynos5-usbdrd driver:
>>>>>>>>
>>>>>>>> 1. This PHY does not provide UTMI and PIPE3 on its own. There's no tuning
>>>>>>> USBDRD phy does not provide UTMI and PIPE on its own either if you look
>>>>>>> at diagram - they call it phy controller.
>>>>>> Ughm. What? So in most exynos cases, there's a combination of multiple phys?
>>>>>>>> for them, and all that is needed from it is to disable HWACG, assert/
>>>>>>>> deassert reset and force bvalid/vbusvalid. After that SNPS eUSB2
>>>>>>>> initialization can be done and USB2 works. If the USBCON phy is not set
>>>>>>>> up before the eUSB2 one, the device hangs, so there is definitely a
>>>>>>>> dependancy between them. For PIPE3 we'd need to control the pipe3
>>>>>>>> attaching/deattaching and then initialize the synopsys USBDP combophy.
>>>>>>> Does it mean there is no USB DRD phy controller as before?
>>>>>>>
>>>>>>> Anyway the problem is you have DWC3 -> PHY -> PHY. Looks one phy too many.
>>>>>> So...
>>>>>>
>>>>>> DWC3 -> USBDRD (USBCON) -> PHYs?
>>>>> No, drop last phy. You just wrote the same as me - two phys, because
>>>>> usbdrd is the phy. In all existing designs there is no such controllable
>>>>> object from the point of view of operating system.
>>>> What? Per my understanding, the phy property should refer to whatever is
>>>> is connected to dwc3 UTMI. In this case it's the so-called USBDRD phy (called
>>>> usbcon in downstream). Considering that the eUSB2 IP definitely also has UTMI
>>>> that has to be connected to something, doesn't that mean we have clearly
>>> The entire point is that eUSB2 is connected to DWC3, no? That's exactly
>>> how it is done for example on Qualcomm SoC. Otherwise you claim that
>>> DWC3 controls one phy, which controls another phy which controls UTMI...
>> But where does the USBCON fit? Is it just a side controller? Why's it needed
>> in the first place? This is what I don't understand.
> I assume usbcon, so old usbdrd, is the second DWC3's phy, just like qcom
> qmpphy.

Ugh. For qcoms, does the first phy depend on qmpphy? If we pass it as the second phy,
I don't know how the linkreset will happen. We also have a usbdp phy, which I
suspect is used for SS as well since it's a combophy by Synopsys.

https://gitlab.com/Mis012/sm-s908b-linux-source-code/-/blob/s22_restored_history/drivers/phy/samsung/phy-exynos-usbdrd-eusb.c#L1579

Best regards,
Ivaylo

>
>>>> separated hardware blocks? Now, I guess one could argue that this USBCON
>>>> hardware block could be classified as a syscon. But I don't see the problem
>>>> with the current binding description, nor the modelling, as it represents
>>>> how the hardware is (unless I've gotten it completely wrong).
>>> It is the first time you use argument that it represents how the
>>> hardware is and this is what we actually disagree. It is not like that.
>>> You do not have chain of phys. Just look at any USB 3.0 DRD DWC diagram
>>> from any Samsung SoC: where would you squeeze these two phys in relation
>>> to what is called there "USB 3.0 PHY" which would be the third phy (!!!).
>> Yeah, my point was that it was different from any previous design. Now,
>> I don't know if it's actually theoretically possible to design it like so. It's
>> hard to just guess how the hardware is designed without having access
>> to die shots, documentations or even just schematics.
>>
>> Let's make it clear now, the changes your request are to document USBCON
>> in the existing exynos binding, as well as to correct all explanations of how
>> this block functions, right?
> No, not necessarily. If USBCON is entirely different device than USBDRD
> (different register layout, different features), then go ahead with a
> new binding.
>
> I was questioning your chain of PHYs and this should be investigated.
>
>
> Best regards,
> Krzysztof

[PATCH v2 4/8] phy: move phy-qcom-snps-eusb2 out of its vendor sub-directory

[Ivaylo Ivanov]

As Samsung is using the same Synopsys eUSB2 IP in Exynos2200, albeit
with a different register layout, it only makes sense to implement
support for that in the existing eUSB2 driver.

To make room for new non-qcom SoCs, do the following:
1. Move phy-qcom-snps-eusb2.c to phy-snps-eusb2.c
2. Rename all qcom_snps_eusb2 functions and structs to snps_eusb2_phy
3. Add a prefix to the qcom-specific register offset definitions
4. Make a generic phy_ops init that sets up power before the SoC-specific
eUSB2 IP init
5. Introduce a driver data structure with init function and clocks

Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
Suggested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/phy/Kconfig                        |   8 +
 drivers/phy/Makefile                       |   1 +
 drivers/phy/phy-snps-eusb2.c               | 505 +++++++++++++++++++++
 drivers/phy/qualcomm/Kconfig               |   9 -
 drivers/phy/qualcomm/Makefile              |   1 -
 drivers/phy/qualcomm/phy-qcom-snps-eusb2.c | 442 ------------------
 6 files changed, 514 insertions(+), 452 deletions(-)
 create mode 100644 drivers/phy/phy-snps-eusb2.c
 delete mode 100644 drivers/phy/qualcomm/phy-qcom-snps-eusb2.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 8d58efe99..11c166204 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -43,6 +43,14 @@ config PHY_PISTACHIO_USB
 	help
 	  Enable this to support the USB2.0 PHY on the IMG Pistachio SoC.
 
+config PHY_SNPS_EUSB2
+	tristate "SNPS eUSB2 PHY Driver"
+	depends on OF && (ARCH_QCOM || COMPILE_TEST)
+	select GENERIC_PHY
+	help
+	  Enable support for the USB high-speed SNPS eUSB2 phy on select
+	  SoCs. The PHY is usually paired with a Synopsys DWC3 USB controller.
+
 config PHY_XGENE
 	tristate "APM X-Gene 15Gbps PHY support"
 	depends on HAS_IOMEM && OF && (ARCH_XGENE || COMPILE_TEST)
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index e281442ac..c670a8dac 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_PHY_CAN_TRANSCEIVER)	+= phy-can-transceiver.o
 obj-$(CONFIG_PHY_LPC18XX_USB_OTG)	+= phy-lpc18xx-usb-otg.o
 obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
 obj-$(CONFIG_PHY_PISTACHIO_USB)		+= phy-pistachio-usb.o
+obj-$(CONFIG_PHY_SNPS_EUSB2)		+= phy-snps-eusb2.o
 obj-$(CONFIG_USB_LGM_PHY)		+= phy-lgm-usb.o
 obj-$(CONFIG_PHY_AIROHA_PCIE)		+= phy-airoha-pcie.o
 obj-$(CONFIG_PHY_NXP_PTN3222)		+= phy-nxp-ptn3222.o
diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
new file mode 100644
index 000000000..4e5914a76
--- /dev/null
+++ b/drivers/phy/phy-snps-eusb2.c
@@ -0,0 +1,505 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2023, Linaro Limited
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/iopoll.h>
+#include <linux/mod_devicetable.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+
+#define QCOM_USB_PHY_UTMI_CTRL0		(0x3c)
+#define SLEEPM				BIT(0)
+#define OPMODE_MASK			GENMASK(4, 3)
+#define OPMODE_NONDRIVING		BIT(3)
+
+#define QCOM_USB_PHY_UTMI_CTRL5		(0x50)
+#define POR				BIT(1)
+
+#define QCOM_USB_PHY_HS_PHY_CTRL_COMMON0	(0x54)
+#define PHY_ENABLE			BIT(0)
+#define SIDDQ_SEL			BIT(1)
+#define SIDDQ				BIT(2)
+#define RETENABLEN			BIT(3)
+#define FSEL_MASK			GENMASK(6, 4)
+#define FSEL_19_2_MHZ_VAL		(0x0)
+#define FSEL_38_4_MHZ_VAL		(0x4)
+
+#define QCOM_USB_PHY_CFG_CTRL_1		(0x58)
+#define PHY_CFG_PLL_CPBIAS_CNTRL_MASK	GENMASK(7, 1)
+
+#define QCOM_USB_PHY_CFG_CTRL_2		(0x5c)
+#define PHY_CFG_PLL_FB_DIV_7_0_MASK	GENMASK(7, 0)
+#define DIV_7_0_19_2_MHZ_VAL		(0x90)
+#define DIV_7_0_38_4_MHZ_VAL		(0xc8)
+
+#define QCOM_USB_PHY_CFG_CTRL_3		(0x60)
+#define PHY_CFG_PLL_FB_DIV_11_8_MASK	GENMASK(3, 0)
+#define DIV_11_8_19_2_MHZ_VAL		(0x1)
+#define DIV_11_8_38_4_MHZ_VAL		(0x0)
+
+#define PHY_CFG_PLL_REF_DIV		GENMASK(7, 4)
+#define PLL_REF_DIV_VAL			(0x0)
+
+#define QCOM_USB_PHY_HS_PHY_CTRL2	(0x64)
+#define VBUSVLDEXT0			BIT(0)
+#define USB2_SUSPEND_N			BIT(2)
+#define USB2_SUSPEND_N_SEL		BIT(3)
+#define VBUS_DET_EXT_SEL		BIT(4)
+
+#define QCOM_USB_PHY_CFG_CTRL_4		(0x68)
+#define PHY_CFG_PLL_GMP_CNTRL_MASK	GENMASK(1, 0)
+#define PHY_CFG_PLL_INT_CNTRL_MASK	GENMASK(7, 2)
+
+#define QCOM_USB_PHY_CFG_CTRL_5		(0x6c)
+#define PHY_CFG_PLL_PROP_CNTRL_MASK	GENMASK(4, 0)
+#define PHY_CFG_PLL_VREF_TUNE_MASK	GENMASK(7, 6)
+
+#define QCOM_USB_PHY_CFG_CTRL_6		(0x70)
+#define PHY_CFG_PLL_VCO_CNTRL_MASK	GENMASK(2, 0)
+
+#define QCOM_USB_PHY_CFG_CTRL_7		(0x74)
+
+#define QCOM_USB_PHY_CFG_CTRL_8		(0x78)
+#define PHY_CFG_TX_FSLS_VREF_TUNE_MASK	GENMASK(1, 0)
+#define PHY_CFG_TX_FSLS_VREG_BYPASS	BIT(2)
+#define PHY_CFG_TX_HS_VREF_TUNE_MASK	GENMASK(5, 3)
+#define PHY_CFG_TX_HS_XV_TUNE_MASK	GENMASK(7, 6)
+
+#define QCOM_USB_PHY_CFG_CTRL_9		(0x7c)
+#define PHY_CFG_TX_PREEMP_TUNE_MASK	GENMASK(2, 0)
+#define PHY_CFG_TX_RES_TUNE_MASK	GENMASK(4, 3)
+#define PHY_CFG_TX_RISE_TUNE_MASK	GENMASK(6, 5)
+#define PHY_CFG_RCAL_BYPASS		BIT(7)
+
+#define QCOM_USB_PHY_CFG_CTRL_10	(0x80)
+
+#define QCOM_USB_PHY_CFG0		(0x94)
+#define DATAPATH_CTRL_OVERRIDE_EN	BIT(0)
+#define CMN_CTRL_OVERRIDE_EN		BIT(1)
+
+#define QCOM_UTMI_PHY_CMN_CTRL0		(0x98)
+#define TESTBURNIN			BIT(6)
+
+#define QCOM_USB_PHY_FSEL_SEL		(0xb8)
+#define FSEL_SEL			BIT(0)
+
+#define QCOM_USB_PHY_APB_ACCESS_CMD	(0x130)
+#define RW_ACCESS			BIT(0)
+#define APB_START_CMD			BIT(1)
+#define APB_LOGIC_RESET			BIT(2)
+
+#define QCOM_USB_PHY_APB_ACCESS_STATUS	(0x134)
+#define ACCESS_DONE			BIT(0)
+#define TIMED_OUT			BIT(1)
+#define ACCESS_ERROR			BIT(2)
+#define ACCESS_IN_PROGRESS		BIT(3)
+
+#define QCOM_USB_PHY_APB_ADDRESS	(0x138)
+#define APB_REG_ADDR_MASK		GENMASK(7, 0)
+
+#define QCOM_USB_PHY_APB_WRDATA_LSB	(0x13c)
+#define APB_REG_WRDATA_7_0_MASK		GENMASK(3, 0)
+
+#define QCOM_USB_PHY_APB_WRDATA_MSB	(0x140)
+#define APB_REG_WRDATA_15_8_MASK	GENMASK(7, 4)
+
+#define QCOM_USB_PHY_APB_RDDATA_LSB	(0x144)
+#define APB_REG_RDDATA_7_0_MASK		GENMASK(3, 0)
+
+#define QCOM_USB_PHY_APB_RDDATA_MSB	(0x148)
+#define APB_REG_RDDATA_15_8_MASK	GENMASK(7, 4)
+
+static const char * const eusb2_hsphy_vreg_names[] = {
+	"vdd", "vdda12",
+};
+
+#define EUSB2_NUM_VREGS		ARRAY_SIZE(eusb2_hsphy_vreg_names)
+
+struct snps_eusb2_phy_drvdata {
+	int (*phy_init)(struct phy *p);
+	const char * const *clk_names;
+	int num_clks;
+};
+
+struct snps_eusb2_hsphy {
+	struct phy *phy;
+	void __iomem *base;
+
+	struct clk *ref_clk;
+	struct clk_bulk_data *clks;
+
+	struct reset_control *phy_reset;
+
+	struct regulator_bulk_data vregs[EUSB2_NUM_VREGS];
+
+	enum phy_mode mode;
+
+	struct phy *repeater;
+
+	const struct snps_eusb2_phy_drvdata *data;
+};
+
+static int snps_eusb2_hsphy_set_mode(struct phy *p, enum phy_mode mode, int submode)
+{
+	struct snps_eusb2_hsphy *phy = phy_get_drvdata(p);
+
+	phy->mode = mode;
+
+	return phy_set_mode_ext(phy->repeater, mode, submode);
+}
+
+static void snps_eusb2_hsphy_write_mask(void __iomem *base, u32 offset,
+					u32 mask, u32 val)
+{
+	u32 reg;
+
+	reg = readl_relaxed(base + offset);
+	reg &= ~mask;
+	reg |= val & mask;
+	writel_relaxed(reg, base + offset);
+
+	/* Ensure above write is completed */
+	readl_relaxed(base + offset);
+}
+
+static void qcom_eusb2_default_parameters(struct snps_eusb2_hsphy *phy)
+{
+	/* default parameters: tx pre-emphasis */
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_9,
+				    PHY_CFG_TX_PREEMP_TUNE_MASK,
+				    FIELD_PREP(PHY_CFG_TX_PREEMP_TUNE_MASK, 0));
+
+	/* tx rise/fall time */
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_9,
+				    PHY_CFG_TX_RISE_TUNE_MASK,
+				    FIELD_PREP(PHY_CFG_TX_RISE_TUNE_MASK, 0x2));
+
+	/* source impedance adjustment */
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_9,
+				    PHY_CFG_TX_RES_TUNE_MASK,
+				    FIELD_PREP(PHY_CFG_TX_RES_TUNE_MASK, 0x1));
+
+	/* dc voltage level adjustement */
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_8,
+				    PHY_CFG_TX_HS_VREF_TUNE_MASK,
+				    FIELD_PREP(PHY_CFG_TX_HS_VREF_TUNE_MASK, 0x3));
+
+	/* transmitter HS crossover adjustement */
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_8,
+				    PHY_CFG_TX_HS_XV_TUNE_MASK,
+				    FIELD_PREP(PHY_CFG_TX_HS_XV_TUNE_MASK, 0x0));
+}
+
+static int qcom_eusb2_ref_clk_init(struct snps_eusb2_hsphy *phy)
+{
+	unsigned long ref_clk_freq = clk_get_rate(phy->ref_clk);
+
+	switch (ref_clk_freq) {
+	case 19200000:
+		snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_HS_PHY_CTRL_COMMON0,
+					    FSEL_MASK,
+					    FIELD_PREP(FSEL_MASK, FSEL_19_2_MHZ_VAL));
+
+		snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_2,
+					    PHY_CFG_PLL_FB_DIV_7_0_MASK,
+					    DIV_7_0_19_2_MHZ_VAL);
+
+		snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_3,
+					    PHY_CFG_PLL_FB_DIV_11_8_MASK,
+					    DIV_11_8_19_2_MHZ_VAL);
+		break;
+
+	case 38400000:
+		snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_HS_PHY_CTRL_COMMON0,
+					    FSEL_MASK,
+					    FIELD_PREP(FSEL_MASK, FSEL_38_4_MHZ_VAL));
+
+		snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_2,
+					    PHY_CFG_PLL_FB_DIV_7_0_MASK,
+					    DIV_7_0_38_4_MHZ_VAL);
+
+		snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_3,
+					    PHY_CFG_PLL_FB_DIV_11_8_MASK,
+					    DIV_11_8_38_4_MHZ_VAL);
+		break;
+
+	default:
+		dev_err(&phy->phy->dev, "unsupported ref_clk_freq:%lu\n", ref_clk_freq);
+		return -EINVAL;
+	}
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_3,
+				    PHY_CFG_PLL_REF_DIV, PLL_REF_DIV_VAL);
+
+	return 0;
+}
+
+static int qcom_snps_eusb2_hsphy_init(struct phy *p)
+{
+	struct snps_eusb2_hsphy *phy = phy_get_drvdata(p);
+	int ret;
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG0,
+				    CMN_CTRL_OVERRIDE_EN, CMN_CTRL_OVERRIDE_EN);
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_UTMI_CTRL5, POR, POR);
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_HS_PHY_CTRL_COMMON0,
+				    PHY_ENABLE | RETENABLEN, PHY_ENABLE | RETENABLEN);
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_APB_ACCESS_CMD,
+				    APB_LOGIC_RESET, APB_LOGIC_RESET);
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_UTMI_PHY_CMN_CTRL0, TESTBURNIN, 0);
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_FSEL_SEL,
+				    FSEL_SEL, FSEL_SEL);
+
+	/* update ref_clk related registers */
+	ret = qcom_eusb2_ref_clk_init(phy);
+	if (ret)
+		return ret;
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_1,
+				    PHY_CFG_PLL_CPBIAS_CNTRL_MASK,
+				    FIELD_PREP(PHY_CFG_PLL_CPBIAS_CNTRL_MASK, 0x1));
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_4,
+				    PHY_CFG_PLL_INT_CNTRL_MASK,
+				    FIELD_PREP(PHY_CFG_PLL_INT_CNTRL_MASK, 0x8));
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_4,
+				    PHY_CFG_PLL_GMP_CNTRL_MASK,
+				    FIELD_PREP(PHY_CFG_PLL_GMP_CNTRL_MASK, 0x1));
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_5,
+				    PHY_CFG_PLL_PROP_CNTRL_MASK,
+				    FIELD_PREP(PHY_CFG_PLL_PROP_CNTRL_MASK, 0x10));
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_6,
+				    PHY_CFG_PLL_VCO_CNTRL_MASK,
+				    FIELD_PREP(PHY_CFG_PLL_VCO_CNTRL_MASK, 0x0));
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_5,
+				    PHY_CFG_PLL_VREF_TUNE_MASK,
+				    FIELD_PREP(PHY_CFG_PLL_VREF_TUNE_MASK, 0x1));
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_HS_PHY_CTRL2,
+				    VBUS_DET_EXT_SEL, VBUS_DET_EXT_SEL);
+
+	/* set default parameters */
+	qcom_eusb2_default_parameters(phy);
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_HS_PHY_CTRL2,
+				    USB2_SUSPEND_N_SEL | USB2_SUSPEND_N,
+				    USB2_SUSPEND_N_SEL | USB2_SUSPEND_N);
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_UTMI_CTRL0, SLEEPM, SLEEPM);
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_HS_PHY_CTRL_COMMON0,
+				    SIDDQ_SEL, SIDDQ_SEL);
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_HS_PHY_CTRL_COMMON0,
+				    SIDDQ, 0);
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_UTMI_CTRL5, POR, 0);
+
+	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_HS_PHY_CTRL2,
+				    USB2_SUSPEND_N_SEL, 0);
+
+	return 0;
+}
+
+static const char * const qcom_eusb2_hsphy_clock_names[] = {
+	"ref",
+};
+
+static const struct snps_eusb2_phy_drvdata sm8550_snps_eusb2_phy = {
+	.phy_init	= qcom_snps_eusb2_hsphy_init,
+	.clk_names	= qcom_eusb2_hsphy_clock_names,
+	.num_clks	= ARRAY_SIZE(qcom_eusb2_hsphy_clock_names),
+};
+
+static int snps_eusb2_hsphy_init(struct phy *p)
+{
+	struct snps_eusb2_hsphy *phy = phy_get_drvdata(p);
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(phy->vregs), phy->vregs);
+	if (ret)
+		return ret;
+
+	ret = phy_init(phy->repeater);
+	if (ret) {
+		dev_err(&p->dev, "repeater init failed. %d\n", ret);
+		goto disable_vreg;
+	}
+
+	ret = clk_prepare_enable(phy->ref_clk);
+	if (ret) {
+		dev_err(&p->dev, "failed to enable ref clock, %d\n", ret);
+		goto disable_vreg;
+	}
+
+	ret = reset_control_assert(phy->phy_reset);
+	if (ret) {
+		dev_err(&p->dev, "failed to assert phy_reset, %d\n", ret);
+		goto disable_ref_clk;
+	}
+
+	usleep_range(100, 150);
+
+	ret = reset_control_deassert(phy->phy_reset);
+	if (ret) {
+		dev_err(&p->dev, "failed to de-assert phy_reset, %d\n", ret);
+		goto disable_ref_clk;
+	}
+
+	ret = phy->data->phy_init(p);
+	if (ret)
+		goto disable_ref_clk;
+
+	return 0;
+
+disable_ref_clk:
+	clk_disable_unprepare(phy->ref_clk);
+
+disable_vreg:
+	regulator_bulk_disable(ARRAY_SIZE(phy->vregs), phy->vregs);
+
+	return ret;
+}
+
+static int snps_eusb2_hsphy_exit(struct phy *p)
+{
+	struct snps_eusb2_hsphy *phy = phy_get_drvdata(p);
+
+	clk_disable_unprepare(phy->ref_clk);
+
+	regulator_bulk_disable(ARRAY_SIZE(phy->vregs), phy->vregs);
+
+	phy_exit(phy->repeater);
+
+	return 0;
+}
+
+static const struct phy_ops snps_eusb2_hsphy_ops = {
+	.init		= snps_eusb2_hsphy_init,
+	.exit		= snps_eusb2_hsphy_exit,
+	.set_mode	= snps_eusb2_hsphy_set_mode,
+	.owner		= THIS_MODULE,
+};
+
+static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct snps_eusb2_hsphy *phy;
+	struct phy_provider *phy_provider;
+	struct phy *generic_phy;
+	const struct snps_eusb2_phy_drvdata *drv_data;
+	int ret, i;
+	int num;
+
+	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
+	if (!phy)
+		return -ENOMEM;
+
+	drv_data = of_device_get_match_data(dev);
+	if (!drv_data)
+		return -EINVAL;
+	phy->data = drv_data;
+
+	phy->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(phy->base))
+		return PTR_ERR(phy->base);
+
+	phy->phy_reset = devm_reset_control_get_exclusive(dev, NULL);
+	if (IS_ERR(phy->phy_reset))
+		return PTR_ERR(phy->phy_reset);
+
+	phy->clks = devm_kcalloc(dev,
+				 phy->data->num_clks,
+				 sizeof(*phy->clks),
+				 GFP_KERNEL);
+	if (!phy->clks)
+		return -ENOMEM;
+
+	for (int i = 0; i < phy->data->num_clks; ++i)
+		phy->clks[i].id = phy->data->clk_names[i];
+
+	ret = devm_clk_bulk_get(dev, phy->data->num_clks,
+				phy->clks);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to get phy clock(s)\n");
+
+	phy->ref_clk = NULL;
+	for (int i = 0; i < phy->data->num_clks; ++i) {
+		if (!strcmp(phy->clks[i].id, "ref")) {
+			phy->ref_clk = phy->clks[i].clk;
+			break;
+		}
+	}
+
+	if (IS_ERR_OR_NULL(phy->ref_clk))
+		return dev_err_probe(dev, PTR_ERR(phy->ref_clk),
+				     "failed to get ref clk\n");
+
+	num = ARRAY_SIZE(phy->vregs);
+	for (i = 0; i < num; i++)
+		phy->vregs[i].supply = eusb2_hsphy_vreg_names[i];
+
+	ret = devm_regulator_bulk_get(dev, num, phy->vregs);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to get regulator supplies\n");
+
+	phy->repeater = devm_of_phy_get_by_index(dev, np, 0);
+	if (IS_ERR(phy->repeater))
+		return dev_err_probe(dev, PTR_ERR(phy->repeater),
+				     "failed to get repeater\n");
+
+	generic_phy = devm_phy_create(dev, NULL, &snps_eusb2_hsphy_ops);
+	if (IS_ERR(generic_phy)) {
+		dev_err(dev, "failed to create phy %d\n", ret);
+		return PTR_ERR(generic_phy);
+	}
+
+	dev_set_drvdata(dev, phy);
+	phy_set_drvdata(generic_phy, phy);
+
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	dev_info(dev, "Registered Snps-eUSB2 phy\n");
+
+	return 0;
+}
+
+static const struct of_device_id snps_eusb2_hsphy_of_match_table[] = {
+	{
+		.compatible = "qcom,sm8550-snps-eusb2-phy",
+		.data = &sm8550_snps_eusb2_phy,
+	}, { },
+};
+MODULE_DEVICE_TABLE(of, snps_eusb2_hsphy_of_match_table);
+
+static struct platform_driver snps_eusb2_hsphy_driver = {
+	.probe		= snps_eusb2_hsphy_probe,
+	.driver = {
+		.name	= "snps-eusb2-hsphy",
+		.of_match_table = snps_eusb2_hsphy_of_match_table,
+	},
+};
+
+module_platform_driver(snps_eusb2_hsphy_driver);
+MODULE_DESCRIPTION("SNPS eUSB2 HS PHY driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
index 846f8c995..914547068 100644
--- a/drivers/phy/qualcomm/Kconfig
+++ b/drivers/phy/qualcomm/Kconfig
@@ -125,15 +125,6 @@ config PHY_QCOM_QUSB2
 	  PHY which is usually paired with either the ChipIdea or Synopsys DWC3
 	  USB IPs on MSM SOCs.
 
-config PHY_QCOM_SNPS_EUSB2
-	tristate "Qualcomm SNPS eUSB2 PHY Driver"
-	depends on OF && (ARCH_QCOM || COMPILE_TEST)
-	select GENERIC_PHY
-	help
-	  Enable support for the USB high-speed SNPS eUSB2 phy on Qualcomm
-	  chipsets. The PHY is paired with a Synopsys DWC3 USB controller
-	  on Qualcomm SOCs.
-
 config PHY_QCOM_EUSB2_REPEATER
 	tristate "Qualcomm SNPS eUSB2 Repeater Driver"
 	depends on OF && (ARCH_QCOM || COMPILE_TEST)
diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
index eb60e950a..2121e92df 100644
--- a/drivers/phy/qualcomm/Makefile
+++ b/drivers/phy/qualcomm/Makefile
@@ -15,7 +15,6 @@ obj-$(CONFIG_PHY_QCOM_QMP_USB)		+= phy-qcom-qmp-usb.o
 obj-$(CONFIG_PHY_QCOM_QMP_USB_LEGACY)	+= phy-qcom-qmp-usb-legacy.o
 
 obj-$(CONFIG_PHY_QCOM_QUSB2)		+= phy-qcom-qusb2.o
-obj-$(CONFIG_PHY_QCOM_SNPS_EUSB2)	+= phy-qcom-snps-eusb2.o
 obj-$(CONFIG_PHY_QCOM_EUSB2_REPEATER)	+= phy-qcom-eusb2-repeater.o
 obj-$(CONFIG_PHY_QCOM_USB_HS) 		+= phy-qcom-usb-hs.o
 obj-$(CONFIG_PHY_QCOM_USB_HSIC) 	+= phy-qcom-usb-hsic.o
diff --git a/drivers/phy/qualcomm/phy-qcom-snps-eusb2.c b/drivers/phy/qualcomm/phy-qcom-snps-eusb2.c
deleted file mode 100644
index 1484691a4..000000000
--- a/drivers/phy/qualcomm/phy-qcom-snps-eusb2.c
+++ /dev/null
@@ -1,442 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (c) 2023, Linaro Limited
- */
-
-#include <linux/bitfield.h>
-#include <linux/clk.h>
-#include <linux/delay.h>
-#include <linux/iopoll.h>
-#include <linux/mod_devicetable.h>
-#include <linux/phy/phy.h>
-#include <linux/platform_device.h>
-#include <linux/regulator/consumer.h>
-#include <linux/reset.h>
-
-#define USB_PHY_UTMI_CTRL0		(0x3c)
-#define SLEEPM				BIT(0)
-#define OPMODE_MASK			GENMASK(4, 3)
-#define OPMODE_NONDRIVING		BIT(3)
-
-#define USB_PHY_UTMI_CTRL5		(0x50)
-#define POR				BIT(1)
-
-#define USB_PHY_HS_PHY_CTRL_COMMON0	(0x54)
-#define PHY_ENABLE			BIT(0)
-#define SIDDQ_SEL			BIT(1)
-#define SIDDQ				BIT(2)
-#define RETENABLEN			BIT(3)
-#define FSEL_MASK			GENMASK(6, 4)
-#define FSEL_19_2_MHZ_VAL		(0x0)
-#define FSEL_38_4_MHZ_VAL		(0x4)
-
-#define USB_PHY_CFG_CTRL_1		(0x58)
-#define PHY_CFG_PLL_CPBIAS_CNTRL_MASK	GENMASK(7, 1)
-
-#define USB_PHY_CFG_CTRL_2		(0x5c)
-#define PHY_CFG_PLL_FB_DIV_7_0_MASK	GENMASK(7, 0)
-#define DIV_7_0_19_2_MHZ_VAL		(0x90)
-#define DIV_7_0_38_4_MHZ_VAL		(0xc8)
-
-#define USB_PHY_CFG_CTRL_3		(0x60)
-#define PHY_CFG_PLL_FB_DIV_11_8_MASK	GENMASK(3, 0)
-#define DIV_11_8_19_2_MHZ_VAL		(0x1)
-#define DIV_11_8_38_4_MHZ_VAL		(0x0)
-
-#define PHY_CFG_PLL_REF_DIV		GENMASK(7, 4)
-#define PLL_REF_DIV_VAL			(0x0)
-
-#define USB_PHY_HS_PHY_CTRL2		(0x64)
-#define VBUSVLDEXT0			BIT(0)
-#define USB2_SUSPEND_N			BIT(2)
-#define USB2_SUSPEND_N_SEL		BIT(3)
-#define VBUS_DET_EXT_SEL		BIT(4)
-
-#define USB_PHY_CFG_CTRL_4		(0x68)
-#define PHY_CFG_PLL_GMP_CNTRL_MASK	GENMASK(1, 0)
-#define PHY_CFG_PLL_INT_CNTRL_MASK	GENMASK(7, 2)
-
-#define USB_PHY_CFG_CTRL_5		(0x6c)
-#define PHY_CFG_PLL_PROP_CNTRL_MASK	GENMASK(4, 0)
-#define PHY_CFG_PLL_VREF_TUNE_MASK	GENMASK(7, 6)
-
-#define USB_PHY_CFG_CTRL_6		(0x70)
-#define PHY_CFG_PLL_VCO_CNTRL_MASK	GENMASK(2, 0)
-
-#define USB_PHY_CFG_CTRL_7		(0x74)
-
-#define USB_PHY_CFG_CTRL_8		(0x78)
-#define PHY_CFG_TX_FSLS_VREF_TUNE_MASK	GENMASK(1, 0)
-#define PHY_CFG_TX_FSLS_VREG_BYPASS	BIT(2)
-#define PHY_CFG_TX_HS_VREF_TUNE_MASK	GENMASK(5, 3)
-#define PHY_CFG_TX_HS_XV_TUNE_MASK	GENMASK(7, 6)
-
-#define USB_PHY_CFG_CTRL_9		(0x7c)
-#define PHY_CFG_TX_PREEMP_TUNE_MASK	GENMASK(2, 0)
-#define PHY_CFG_TX_RES_TUNE_MASK	GENMASK(4, 3)
-#define PHY_CFG_TX_RISE_TUNE_MASK	GENMASK(6, 5)
-#define PHY_CFG_RCAL_BYPASS		BIT(7)
-
-#define USB_PHY_CFG_CTRL_10		(0x80)
-
-#define USB_PHY_CFG0			(0x94)
-#define DATAPATH_CTRL_OVERRIDE_EN	BIT(0)
-#define CMN_CTRL_OVERRIDE_EN		BIT(1)
-
-#define UTMI_PHY_CMN_CTRL0		(0x98)
-#define TESTBURNIN			BIT(6)
-
-#define USB_PHY_FSEL_SEL		(0xb8)
-#define FSEL_SEL			BIT(0)
-
-#define USB_PHY_APB_ACCESS_CMD		(0x130)
-#define RW_ACCESS			BIT(0)
-#define APB_START_CMD			BIT(1)
-#define APB_LOGIC_RESET			BIT(2)
-
-#define USB_PHY_APB_ACCESS_STATUS	(0x134)
-#define ACCESS_DONE			BIT(0)
-#define TIMED_OUT			BIT(1)
-#define ACCESS_ERROR			BIT(2)
-#define ACCESS_IN_PROGRESS		BIT(3)
-
-#define USB_PHY_APB_ADDRESS		(0x138)
-#define APB_REG_ADDR_MASK		GENMASK(7, 0)
-
-#define USB_PHY_APB_WRDATA_LSB		(0x13c)
-#define APB_REG_WRDATA_7_0_MASK		GENMASK(3, 0)
-
-#define USB_PHY_APB_WRDATA_MSB		(0x140)
-#define APB_REG_WRDATA_15_8_MASK	GENMASK(7, 4)
-
-#define USB_PHY_APB_RDDATA_LSB		(0x144)
-#define APB_REG_RDDATA_7_0_MASK		GENMASK(3, 0)
-
-#define USB_PHY_APB_RDDATA_MSB		(0x148)
-#define APB_REG_RDDATA_15_8_MASK	GENMASK(7, 4)
-
-static const char * const eusb2_hsphy_vreg_names[] = {
-	"vdd", "vdda12",
-};
-
-#define EUSB2_NUM_VREGS		ARRAY_SIZE(eusb2_hsphy_vreg_names)
-
-struct qcom_snps_eusb2_hsphy {
-	struct phy *phy;
-	void __iomem *base;
-
-	struct clk *ref_clk;
-	struct reset_control *phy_reset;
-
-	struct regulator_bulk_data vregs[EUSB2_NUM_VREGS];
-
-	enum phy_mode mode;
-
-	struct phy *repeater;
-};
-
-static int qcom_snps_eusb2_hsphy_set_mode(struct phy *p, enum phy_mode mode, int submode)
-{
-	struct qcom_snps_eusb2_hsphy *phy = phy_get_drvdata(p);
-
-	phy->mode = mode;
-
-	return phy_set_mode_ext(phy->repeater, mode, submode);
-}
-
-static void qcom_snps_eusb2_hsphy_write_mask(void __iomem *base, u32 offset,
-					     u32 mask, u32 val)
-{
-	u32 reg;
-
-	reg = readl_relaxed(base + offset);
-	reg &= ~mask;
-	reg |= val & mask;
-	writel_relaxed(reg, base + offset);
-
-	/* Ensure above write is completed */
-	readl_relaxed(base + offset);
-}
-
-static void qcom_eusb2_default_parameters(struct qcom_snps_eusb2_hsphy *phy)
-{
-	/* default parameters: tx pre-emphasis */
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_9,
-					 PHY_CFG_TX_PREEMP_TUNE_MASK,
-					 FIELD_PREP(PHY_CFG_TX_PREEMP_TUNE_MASK, 0));
-
-	/* tx rise/fall time */
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_9,
-					 PHY_CFG_TX_RISE_TUNE_MASK,
-					 FIELD_PREP(PHY_CFG_TX_RISE_TUNE_MASK, 0x2));
-
-	/* source impedance adjustment */
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_9,
-					 PHY_CFG_TX_RES_TUNE_MASK,
-					 FIELD_PREP(PHY_CFG_TX_RES_TUNE_MASK, 0x1));
-
-	/* dc voltage level adjustement */
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_8,
-					 PHY_CFG_TX_HS_VREF_TUNE_MASK,
-					 FIELD_PREP(PHY_CFG_TX_HS_VREF_TUNE_MASK, 0x3));
-
-	/* transmitter HS crossover adjustement */
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_8,
-					 PHY_CFG_TX_HS_XV_TUNE_MASK,
-					 FIELD_PREP(PHY_CFG_TX_HS_XV_TUNE_MASK, 0x0));
-}
-
-static int qcom_eusb2_ref_clk_init(struct qcom_snps_eusb2_hsphy *phy)
-{
-	unsigned long ref_clk_freq = clk_get_rate(phy->ref_clk);
-
-	switch (ref_clk_freq) {
-	case 19200000:
-		qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_HS_PHY_CTRL_COMMON0,
-						 FSEL_MASK,
-						 FIELD_PREP(FSEL_MASK, FSEL_19_2_MHZ_VAL));
-
-		qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_2,
-						 PHY_CFG_PLL_FB_DIV_7_0_MASK,
-						 DIV_7_0_19_2_MHZ_VAL);
-
-		qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_3,
-						 PHY_CFG_PLL_FB_DIV_11_8_MASK,
-						 DIV_11_8_19_2_MHZ_VAL);
-		break;
-
-	case 38400000:
-		qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_HS_PHY_CTRL_COMMON0,
-						 FSEL_MASK,
-						 FIELD_PREP(FSEL_MASK, FSEL_38_4_MHZ_VAL));
-
-		qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_2,
-						 PHY_CFG_PLL_FB_DIV_7_0_MASK,
-						 DIV_7_0_38_4_MHZ_VAL);
-
-		qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_3,
-						 PHY_CFG_PLL_FB_DIV_11_8_MASK,
-						 DIV_11_8_38_4_MHZ_VAL);
-		break;
-
-	default:
-		dev_err(&phy->phy->dev, "unsupported ref_clk_freq:%lu\n", ref_clk_freq);
-		return -EINVAL;
-	}
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_3,
-					 PHY_CFG_PLL_REF_DIV, PLL_REF_DIV_VAL);
-
-	return 0;
-}
-
-static int qcom_snps_eusb2_hsphy_init(struct phy *p)
-{
-	struct qcom_snps_eusb2_hsphy *phy = phy_get_drvdata(p);
-	int ret;
-
-	ret = regulator_bulk_enable(ARRAY_SIZE(phy->vregs), phy->vregs);
-	if (ret)
-		return ret;
-
-	ret = phy_init(phy->repeater);
-	if (ret) {
-		dev_err(&p->dev, "repeater init failed. %d\n", ret);
-		goto disable_vreg;
-	}
-
-	ret = clk_prepare_enable(phy->ref_clk);
-	if (ret) {
-		dev_err(&p->dev, "failed to enable ref clock, %d\n", ret);
-		goto disable_vreg;
-	}
-
-	ret = reset_control_assert(phy->phy_reset);
-	if (ret) {
-		dev_err(&p->dev, "failed to assert phy_reset, %d\n", ret);
-		goto disable_ref_clk;
-	}
-
-	usleep_range(100, 150);
-
-	ret = reset_control_deassert(phy->phy_reset);
-	if (ret) {
-		dev_err(&p->dev, "failed to de-assert phy_reset, %d\n", ret);
-		goto disable_ref_clk;
-	}
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG0,
-					 CMN_CTRL_OVERRIDE_EN, CMN_CTRL_OVERRIDE_EN);
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_UTMI_CTRL5, POR, POR);
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_HS_PHY_CTRL_COMMON0,
-					 PHY_ENABLE | RETENABLEN, PHY_ENABLE | RETENABLEN);
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_APB_ACCESS_CMD,
-					 APB_LOGIC_RESET, APB_LOGIC_RESET);
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, UTMI_PHY_CMN_CTRL0, TESTBURNIN, 0);
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_FSEL_SEL,
-					 FSEL_SEL, FSEL_SEL);
-
-	/* update ref_clk related registers */
-	ret = qcom_eusb2_ref_clk_init(phy);
-	if (ret)
-		goto disable_ref_clk;
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_1,
-					 PHY_CFG_PLL_CPBIAS_CNTRL_MASK,
-					 FIELD_PREP(PHY_CFG_PLL_CPBIAS_CNTRL_MASK, 0x1));
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_4,
-					 PHY_CFG_PLL_INT_CNTRL_MASK,
-					 FIELD_PREP(PHY_CFG_PLL_INT_CNTRL_MASK, 0x8));
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_4,
-					 PHY_CFG_PLL_GMP_CNTRL_MASK,
-					 FIELD_PREP(PHY_CFG_PLL_GMP_CNTRL_MASK, 0x1));
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_5,
-					 PHY_CFG_PLL_PROP_CNTRL_MASK,
-					 FIELD_PREP(PHY_CFG_PLL_PROP_CNTRL_MASK, 0x10));
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_6,
-					 PHY_CFG_PLL_VCO_CNTRL_MASK,
-					 FIELD_PREP(PHY_CFG_PLL_VCO_CNTRL_MASK, 0x0));
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_5,
-					 PHY_CFG_PLL_VREF_TUNE_MASK,
-					 FIELD_PREP(PHY_CFG_PLL_VREF_TUNE_MASK, 0x1));
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_HS_PHY_CTRL2,
-					 VBUS_DET_EXT_SEL, VBUS_DET_EXT_SEL);
-
-	/* set default parameters */
-	qcom_eusb2_default_parameters(phy);
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_HS_PHY_CTRL2,
-					 USB2_SUSPEND_N_SEL | USB2_SUSPEND_N,
-					 USB2_SUSPEND_N_SEL | USB2_SUSPEND_N);
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_UTMI_CTRL0, SLEEPM, SLEEPM);
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_HS_PHY_CTRL_COMMON0,
-					 SIDDQ_SEL, SIDDQ_SEL);
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_HS_PHY_CTRL_COMMON0,
-					 SIDDQ, 0);
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_UTMI_CTRL5, POR, 0);
-
-	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_HS_PHY_CTRL2,
-					 USB2_SUSPEND_N_SEL, 0);
-
-	return 0;
-
-disable_ref_clk:
-	clk_disable_unprepare(phy->ref_clk);
-
-disable_vreg:
-	regulator_bulk_disable(ARRAY_SIZE(phy->vregs), phy->vregs);
-
-	return ret;
-}
-
-static int qcom_snps_eusb2_hsphy_exit(struct phy *p)
-{
-	struct qcom_snps_eusb2_hsphy *phy = phy_get_drvdata(p);
-
-	clk_disable_unprepare(phy->ref_clk);
-
-	regulator_bulk_disable(ARRAY_SIZE(phy->vregs), phy->vregs);
-
-	phy_exit(phy->repeater);
-
-	return 0;
-}
-
-static const struct phy_ops qcom_snps_eusb2_hsphy_ops = {
-	.init		= qcom_snps_eusb2_hsphy_init,
-	.exit		= qcom_snps_eusb2_hsphy_exit,
-	.set_mode	= qcom_snps_eusb2_hsphy_set_mode,
-	.owner		= THIS_MODULE,
-};
-
-static int qcom_snps_eusb2_hsphy_probe(struct platform_device *pdev)
-{
-	struct device *dev = &pdev->dev;
-	struct device_node *np = dev->of_node;
-	struct qcom_snps_eusb2_hsphy *phy;
-	struct phy_provider *phy_provider;
-	struct phy *generic_phy;
-	int ret, i;
-	int num;
-
-	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
-	if (!phy)
-		return -ENOMEM;
-
-	phy->base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(phy->base))
-		return PTR_ERR(phy->base);
-
-	phy->phy_reset = devm_reset_control_get_exclusive(dev, NULL);
-	if (IS_ERR(phy->phy_reset))
-		return PTR_ERR(phy->phy_reset);
-
-	phy->ref_clk = devm_clk_get(dev, "ref");
-	if (IS_ERR(phy->ref_clk))
-		return dev_err_probe(dev, PTR_ERR(phy->ref_clk),
-				     "failed to get ref clk\n");
-
-	num = ARRAY_SIZE(phy->vregs);
-	for (i = 0; i < num; i++)
-		phy->vregs[i].supply = eusb2_hsphy_vreg_names[i];
-
-	ret = devm_regulator_bulk_get(dev, num, phy->vregs);
-	if (ret)
-		return dev_err_probe(dev, ret,
-				     "failed to get regulator supplies\n");
-
-	phy->repeater = devm_of_phy_get_by_index(dev, np, 0);
-	if (IS_ERR(phy->repeater))
-		return dev_err_probe(dev, PTR_ERR(phy->repeater),
-				     "failed to get repeater\n");
-
-	generic_phy = devm_phy_create(dev, NULL, &qcom_snps_eusb2_hsphy_ops);
-	if (IS_ERR(generic_phy)) {
-		dev_err(dev, "failed to create phy %d\n", ret);
-		return PTR_ERR(generic_phy);
-	}
-
-	dev_set_drvdata(dev, phy);
-	phy_set_drvdata(generic_phy, phy);
-
-	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
-	if (IS_ERR(phy_provider))
-		return PTR_ERR(phy_provider);
-
-	dev_info(dev, "Registered Qcom-eUSB2 phy\n");
-
-	return 0;
-}
-
-static const struct of_device_id qcom_snps_eusb2_hsphy_of_match_table[] = {
-	{ .compatible = "qcom,sm8550-snps-eusb2-phy", },
-	{ },
-};
-MODULE_DEVICE_TABLE(of, qcom_snps_eusb2_hsphy_of_match_table);
-
-static struct platform_driver qcom_snps_eusb2_hsphy_driver = {
-	.probe		= qcom_snps_eusb2_hsphy_probe,
-	.driver = {
-		.name	= "qcom-snps-eusb2-hsphy",
-		.of_match_table = qcom_snps_eusb2_hsphy_of_match_table,
-	},
-};
-
-module_platform_driver(qcom_snps_eusb2_hsphy_driver);
-MODULE_DESCRIPTION("Qualcomm SNPS eUSB2 HS PHY driver");
-MODULE_LICENSE("GPL");
-- 
2.43.0

Re: [PATCH v2 4/8] phy: move phy-qcom-snps-eusb2 out of its vendor sub-directory

[neil.armstrong]

Hi,

On 23/02/2025 13:22, Ivaylo Ivanov wrote:
> As Samsung is using the same Synopsys eUSB2 IP in Exynos2200, albeit
> with a different register layout, it only makes sense to implement
> support for that in the existing eUSB2 driver.
> 
> To make room for new non-qcom SoCs, do the following:
> 1. Move phy-qcom-snps-eusb2.c to phy-snps-eusb2.c
> 2. Rename all qcom_snps_eusb2 functions and structs to snps_eusb2_phy
> 3. Add a prefix to the qcom-specific register offset definitions
> 4. Make a generic phy_ops init that sets up power before the SoC-specific
> eUSB2 IP init
> 5. Introduce a driver data structure with init function and clocks

Please split this in multiples patches, because we can't check the actual
changes on the driver... so it's non reviewable in the current state.

Neil

> 
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> Suggested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>   drivers/phy/Kconfig                        |   8 +
>   drivers/phy/Makefile                       |   1 +
>   drivers/phy/phy-snps-eusb2.c               | 505 +++++++++++++++++++++
>   drivers/phy/qualcomm/Kconfig               |   9 -
>   drivers/phy/qualcomm/Makefile              |   1 -
>   drivers/phy/qualcomm/phy-qcom-snps-eusb2.c | 442 ------------------
>   6 files changed, 514 insertions(+), 452 deletions(-)
>   create mode 100644 drivers/phy/phy-snps-eusb2.c
>   delete mode 100644 drivers/phy/qualcomm/phy-qcom-snps-eusb2.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 8d58efe99..11c166204 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -43,6 +43,14 @@ config PHY_PISTACHIO_USB
>   	help
>   	  Enable this to support the USB2.0 PHY on the IMG Pistachio SoC.
>   
> +config PHY_SNPS_EUSB2
> +	tristate "SNPS eUSB2 PHY Driver"
> +	depends on OF && (ARCH_QCOM || COMPILE_TEST)
> +	select GENERIC_PHY
> +	help
> +	  Enable support for the USB high-speed SNPS eUSB2 phy on select
> +	  SoCs. The PHY is usually paired with a Synopsys DWC3 USB controller.
> +
>   config PHY_XGENE
>   	tristate "APM X-Gene 15Gbps PHY support"
>   	depends on HAS_IOMEM && OF && (ARCH_XGENE || COMPILE_TEST)
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index e281442ac..c670a8dac 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_PHY_CAN_TRANSCEIVER)	+= phy-can-transceiver.o
>   obj-$(CONFIG_PHY_LPC18XX_USB_OTG)	+= phy-lpc18xx-usb-otg.o
>   obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
>   obj-$(CONFIG_PHY_PISTACHIO_USB)		+= phy-pistachio-usb.o
> +obj-$(CONFIG_PHY_SNPS_EUSB2)		+= phy-snps-eusb2.o
>   obj-$(CONFIG_USB_LGM_PHY)		+= phy-lgm-usb.o
>   obj-$(CONFIG_PHY_AIROHA_PCIE)		+= phy-airoha-pcie.o
>   obj-$(CONFIG_PHY_NXP_PTN3222)		+= phy-nxp-ptn3222.o
> diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
> new file mode 100644
> index 000000000..4e5914a76
> --- /dev/null
> +++ b/drivers/phy/phy-snps-eusb2.c
> @@ -0,0 +1,505 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/iopoll.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +
> +#define QCOM_USB_PHY_UTMI_CTRL0		(0x3c)
> +#define SLEEPM				BIT(0)
> +#define OPMODE_MASK			GENMASK(4, 3)
> +#define OPMODE_NONDRIVING		BIT(3)
> +
> +#define QCOM_USB_PHY_UTMI_CTRL5		(0x50)
> +#define POR				BIT(1)
> +
> +#define QCOM_USB_PHY_HS_PHY_CTRL_COMMON0	(0x54)
> +#define PHY_ENABLE			BIT(0)
> +#define SIDDQ_SEL			BIT(1)
> +#define SIDDQ				BIT(2)
> +#define RETENABLEN			BIT(3)
> +#define FSEL_MASK			GENMASK(6, 4)
> +#define FSEL_19_2_MHZ_VAL		(0x0)
> +#define FSEL_38_4_MHZ_VAL		(0x4)
> +
> +#define QCOM_USB_PHY_CFG_CTRL_1		(0x58)
> +#define PHY_CFG_PLL_CPBIAS_CNTRL_MASK	GENMASK(7, 1)
> +
> +#define QCOM_USB_PHY_CFG_CTRL_2		(0x5c)
> +#define PHY_CFG_PLL_FB_DIV_7_0_MASK	GENMASK(7, 0)
> +#define DIV_7_0_19_2_MHZ_VAL		(0x90)
> +#define DIV_7_0_38_4_MHZ_VAL		(0xc8)
> +
> +#define QCOM_USB_PHY_CFG_CTRL_3		(0x60)
> +#define PHY_CFG_PLL_FB_DIV_11_8_MASK	GENMASK(3, 0)
> +#define DIV_11_8_19_2_MHZ_VAL		(0x1)
> +#define DIV_11_8_38_4_MHZ_VAL		(0x0)
> +
> +#define PHY_CFG_PLL_REF_DIV		GENMASK(7, 4)
> +#define PLL_REF_DIV_VAL			(0x0)
> +
> +#define QCOM_USB_PHY_HS_PHY_CTRL2	(0x64)
> +#define VBUSVLDEXT0			BIT(0)
> +#define USB2_SUSPEND_N			BIT(2)
> +#define USB2_SUSPEND_N_SEL		BIT(3)
> +#define VBUS_DET_EXT_SEL		BIT(4)
> +
> +#define QCOM_USB_PHY_CFG_CTRL_4		(0x68)
> +#define PHY_CFG_PLL_GMP_CNTRL_MASK	GENMASK(1, 0)
> +#define PHY_CFG_PLL_INT_CNTRL_MASK	GENMASK(7, 2)
> +
> +#define QCOM_USB_PHY_CFG_CTRL_5		(0x6c)
> +#define PHY_CFG_PLL_PROP_CNTRL_MASK	GENMASK(4, 0)
> +#define PHY_CFG_PLL_VREF_TUNE_MASK	GENMASK(7, 6)
> +
> +#define QCOM_USB_PHY_CFG_CTRL_6		(0x70)
> +#define PHY_CFG_PLL_VCO_CNTRL_MASK	GENMASK(2, 0)
> +
> +#define QCOM_USB_PHY_CFG_CTRL_7		(0x74)
> +
> +#define QCOM_USB_PHY_CFG_CTRL_8		(0x78)
> +#define PHY_CFG_TX_FSLS_VREF_TUNE_MASK	GENMASK(1, 0)
> +#define PHY_CFG_TX_FSLS_VREG_BYPASS	BIT(2)
> +#define PHY_CFG_TX_HS_VREF_TUNE_MASK	GENMASK(5, 3)
> +#define PHY_CFG_TX_HS_XV_TUNE_MASK	GENMASK(7, 6)
> +
> +#define QCOM_USB_PHY_CFG_CTRL_9		(0x7c)
> +#define PHY_CFG_TX_PREEMP_TUNE_MASK	GENMASK(2, 0)
> +#define PHY_CFG_TX_RES_TUNE_MASK	GENMASK(4, 3)
> +#define PHY_CFG_TX_RISE_TUNE_MASK	GENMASK(6, 5)
> +#define PHY_CFG_RCAL_BYPASS		BIT(7)
> +
> +#define QCOM_USB_PHY_CFG_CTRL_10	(0x80)
> +
> +#define QCOM_USB_PHY_CFG0		(0x94)
> +#define DATAPATH_CTRL_OVERRIDE_EN	BIT(0)
> +#define CMN_CTRL_OVERRIDE_EN		BIT(1)
> +
> +#define QCOM_UTMI_PHY_CMN_CTRL0		(0x98)
> +#define TESTBURNIN			BIT(6)
> +
> +#define QCOM_USB_PHY_FSEL_SEL		(0xb8)
> +#define FSEL_SEL			BIT(0)
> +
> +#define QCOM_USB_PHY_APB_ACCESS_CMD	(0x130)
> +#define RW_ACCESS			BIT(0)
> +#define APB_START_CMD			BIT(1)
> +#define APB_LOGIC_RESET			BIT(2)
> +
> +#define QCOM_USB_PHY_APB_ACCESS_STATUS	(0x134)
> +#define ACCESS_DONE			BIT(0)
> +#define TIMED_OUT			BIT(1)
> +#define ACCESS_ERROR			BIT(2)
> +#define ACCESS_IN_PROGRESS		BIT(3)
> +
> +#define QCOM_USB_PHY_APB_ADDRESS	(0x138)
> +#define APB_REG_ADDR_MASK		GENMASK(7, 0)
> +
> +#define QCOM_USB_PHY_APB_WRDATA_LSB	(0x13c)
> +#define APB_REG_WRDATA_7_0_MASK		GENMASK(3, 0)
> +
> +#define QCOM_USB_PHY_APB_WRDATA_MSB	(0x140)
> +#define APB_REG_WRDATA_15_8_MASK	GENMASK(7, 4)
> +
> +#define QCOM_USB_PHY_APB_RDDATA_LSB	(0x144)
> +#define APB_REG_RDDATA_7_0_MASK		GENMASK(3, 0)
> +
> +#define QCOM_USB_PHY_APB_RDDATA_MSB	(0x148)
> +#define APB_REG_RDDATA_15_8_MASK	GENMASK(7, 4)
> +
> +static const char * const eusb2_hsphy_vreg_names[] = {
> +	"vdd", "vdda12",
> +};
> +
> +#define EUSB2_NUM_VREGS		ARRAY_SIZE(eusb2_hsphy_vreg_names)
> +
> +struct snps_eusb2_phy_drvdata {
> +	int (*phy_init)(struct phy *p);
> +	const char * const *clk_names;
> +	int num_clks;
> +};
> +
> +struct snps_eusb2_hsphy {
> +	struct phy *phy;
> +	void __iomem *base;
> +
> +	struct clk *ref_clk;
> +	struct clk_bulk_data *clks;
> +
> +	struct reset_control *phy_reset;
> +
> +	struct regulator_bulk_data vregs[EUSB2_NUM_VREGS];
> +
> +	enum phy_mode mode;
> +
> +	struct phy *repeater;
> +
> +	const struct snps_eusb2_phy_drvdata *data;
> +};
> +
> +static int snps_eusb2_hsphy_set_mode(struct phy *p, enum phy_mode mode, int submode)
> +{
> +	struct snps_eusb2_hsphy *phy = phy_get_drvdata(p);
> +
> +	phy->mode = mode;
> +
> +	return phy_set_mode_ext(phy->repeater, mode, submode);
> +}
> +
> +static void snps_eusb2_hsphy_write_mask(void __iomem *base, u32 offset,
> +					u32 mask, u32 val)
> +{
> +	u32 reg;
> +
> +	reg = readl_relaxed(base + offset);
> +	reg &= ~mask;
> +	reg |= val & mask;
> +	writel_relaxed(reg, base + offset);
> +
> +	/* Ensure above write is completed */
> +	readl_relaxed(base + offset);
> +}
> +
> +static void qcom_eusb2_default_parameters(struct snps_eusb2_hsphy *phy)
> +{
> +	/* default parameters: tx pre-emphasis */
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_9,
> +				    PHY_CFG_TX_PREEMP_TUNE_MASK,
> +				    FIELD_PREP(PHY_CFG_TX_PREEMP_TUNE_MASK, 0));
> +
> +	/* tx rise/fall time */
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_9,
> +				    PHY_CFG_TX_RISE_TUNE_MASK,
> +				    FIELD_PREP(PHY_CFG_TX_RISE_TUNE_MASK, 0x2));
> +
> +	/* source impedance adjustment */
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_9,
> +				    PHY_CFG_TX_RES_TUNE_MASK,
> +				    FIELD_PREP(PHY_CFG_TX_RES_TUNE_MASK, 0x1));
> +
> +	/* dc voltage level adjustement */
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_8,
> +				    PHY_CFG_TX_HS_VREF_TUNE_MASK,
> +				    FIELD_PREP(PHY_CFG_TX_HS_VREF_TUNE_MASK, 0x3));
> +
> +	/* transmitter HS crossover adjustement */
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_8,
> +				    PHY_CFG_TX_HS_XV_TUNE_MASK,
> +				    FIELD_PREP(PHY_CFG_TX_HS_XV_TUNE_MASK, 0x0));
> +}
> +
> +static int qcom_eusb2_ref_clk_init(struct snps_eusb2_hsphy *phy)
> +{
> +	unsigned long ref_clk_freq = clk_get_rate(phy->ref_clk);
> +
> +	switch (ref_clk_freq) {
> +	case 19200000:
> +		snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_HS_PHY_CTRL_COMMON0,
> +					    FSEL_MASK,
> +					    FIELD_PREP(FSEL_MASK, FSEL_19_2_MHZ_VAL));
> +
> +		snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_2,
> +					    PHY_CFG_PLL_FB_DIV_7_0_MASK,
> +					    DIV_7_0_19_2_MHZ_VAL);
> +
> +		snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_3,
> +					    PHY_CFG_PLL_FB_DIV_11_8_MASK,
> +					    DIV_11_8_19_2_MHZ_VAL);
> +		break;
> +
> +	case 38400000:
> +		snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_HS_PHY_CTRL_COMMON0,
> +					    FSEL_MASK,
> +					    FIELD_PREP(FSEL_MASK, FSEL_38_4_MHZ_VAL));
> +
> +		snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_2,
> +					    PHY_CFG_PLL_FB_DIV_7_0_MASK,
> +					    DIV_7_0_38_4_MHZ_VAL);
> +
> +		snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_3,
> +					    PHY_CFG_PLL_FB_DIV_11_8_MASK,
> +					    DIV_11_8_38_4_MHZ_VAL);
> +		break;
> +
> +	default:
> +		dev_err(&phy->phy->dev, "unsupported ref_clk_freq:%lu\n", ref_clk_freq);
> +		return -EINVAL;
> +	}
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_3,
> +				    PHY_CFG_PLL_REF_DIV, PLL_REF_DIV_VAL);
> +
> +	return 0;
> +}
> +
> +static int qcom_snps_eusb2_hsphy_init(struct phy *p)
> +{
> +	struct snps_eusb2_hsphy *phy = phy_get_drvdata(p);
> +	int ret;
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG0,
> +				    CMN_CTRL_OVERRIDE_EN, CMN_CTRL_OVERRIDE_EN);
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_UTMI_CTRL5, POR, POR);
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_HS_PHY_CTRL_COMMON0,
> +				    PHY_ENABLE | RETENABLEN, PHY_ENABLE | RETENABLEN);
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_APB_ACCESS_CMD,
> +				    APB_LOGIC_RESET, APB_LOGIC_RESET);
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_UTMI_PHY_CMN_CTRL0, TESTBURNIN, 0);
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_FSEL_SEL,
> +				    FSEL_SEL, FSEL_SEL);
> +
> +	/* update ref_clk related registers */
> +	ret = qcom_eusb2_ref_clk_init(phy);
> +	if (ret)
> +		return ret;
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_1,
> +				    PHY_CFG_PLL_CPBIAS_CNTRL_MASK,
> +				    FIELD_PREP(PHY_CFG_PLL_CPBIAS_CNTRL_MASK, 0x1));
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_4,
> +				    PHY_CFG_PLL_INT_CNTRL_MASK,
> +				    FIELD_PREP(PHY_CFG_PLL_INT_CNTRL_MASK, 0x8));
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_4,
> +				    PHY_CFG_PLL_GMP_CNTRL_MASK,
> +				    FIELD_PREP(PHY_CFG_PLL_GMP_CNTRL_MASK, 0x1));
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_5,
> +				    PHY_CFG_PLL_PROP_CNTRL_MASK,
> +				    FIELD_PREP(PHY_CFG_PLL_PROP_CNTRL_MASK, 0x10));
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_6,
> +				    PHY_CFG_PLL_VCO_CNTRL_MASK,
> +				    FIELD_PREP(PHY_CFG_PLL_VCO_CNTRL_MASK, 0x0));
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_CFG_CTRL_5,
> +				    PHY_CFG_PLL_VREF_TUNE_MASK,
> +				    FIELD_PREP(PHY_CFG_PLL_VREF_TUNE_MASK, 0x1));
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_HS_PHY_CTRL2,
> +				    VBUS_DET_EXT_SEL, VBUS_DET_EXT_SEL);
> +
> +	/* set default parameters */
> +	qcom_eusb2_default_parameters(phy);
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_HS_PHY_CTRL2,
> +				    USB2_SUSPEND_N_SEL | USB2_SUSPEND_N,
> +				    USB2_SUSPEND_N_SEL | USB2_SUSPEND_N);
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_UTMI_CTRL0, SLEEPM, SLEEPM);
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_HS_PHY_CTRL_COMMON0,
> +				    SIDDQ_SEL, SIDDQ_SEL);
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_HS_PHY_CTRL_COMMON0,
> +				    SIDDQ, 0);
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_UTMI_CTRL5, POR, 0);
> +
> +	snps_eusb2_hsphy_write_mask(phy->base, QCOM_USB_PHY_HS_PHY_CTRL2,
> +				    USB2_SUSPEND_N_SEL, 0);
> +
> +	return 0;
> +}
> +
> +static const char * const qcom_eusb2_hsphy_clock_names[] = {
> +	"ref",
> +};
> +
> +static const struct snps_eusb2_phy_drvdata sm8550_snps_eusb2_phy = {
> +	.phy_init	= qcom_snps_eusb2_hsphy_init,
> +	.clk_names	= qcom_eusb2_hsphy_clock_names,
> +	.num_clks	= ARRAY_SIZE(qcom_eusb2_hsphy_clock_names),
> +};
> +
> +static int snps_eusb2_hsphy_init(struct phy *p)
> +{
> +	struct snps_eusb2_hsphy *phy = phy_get_drvdata(p);
> +	int ret;
> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(phy->vregs), phy->vregs);
> +	if (ret)
> +		return ret;
> +
> +	ret = phy_init(phy->repeater);
> +	if (ret) {
> +		dev_err(&p->dev, "repeater init failed. %d\n", ret);
> +		goto disable_vreg;
> +	}
> +
> +	ret = clk_prepare_enable(phy->ref_clk);
> +	if (ret) {
> +		dev_err(&p->dev, "failed to enable ref clock, %d\n", ret);
> +		goto disable_vreg;
> +	}
> +
> +	ret = reset_control_assert(phy->phy_reset);
> +	if (ret) {
> +		dev_err(&p->dev, "failed to assert phy_reset, %d\n", ret);
> +		goto disable_ref_clk;
> +	}
> +
> +	usleep_range(100, 150);
> +
> +	ret = reset_control_deassert(phy->phy_reset);
> +	if (ret) {
> +		dev_err(&p->dev, "failed to de-assert phy_reset, %d\n", ret);
> +		goto disable_ref_clk;
> +	}
> +
> +	ret = phy->data->phy_init(p);
> +	if (ret)
> +		goto disable_ref_clk;
> +
> +	return 0;
> +
> +disable_ref_clk:
> +	clk_disable_unprepare(phy->ref_clk);
> +
> +disable_vreg:
> +	regulator_bulk_disable(ARRAY_SIZE(phy->vregs), phy->vregs);
> +
> +	return ret;
> +}
> +
> +static int snps_eusb2_hsphy_exit(struct phy *p)
> +{
> +	struct snps_eusb2_hsphy *phy = phy_get_drvdata(p);
> +
> +	clk_disable_unprepare(phy->ref_clk);
> +
> +	regulator_bulk_disable(ARRAY_SIZE(phy->vregs), phy->vregs);
> +
> +	phy_exit(phy->repeater);
> +
> +	return 0;
> +}
> +
> +static const struct phy_ops snps_eusb2_hsphy_ops = {
> +	.init		= snps_eusb2_hsphy_init,
> +	.exit		= snps_eusb2_hsphy_exit,
> +	.set_mode	= snps_eusb2_hsphy_set_mode,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	struct snps_eusb2_hsphy *phy;
> +	struct phy_provider *phy_provider;
> +	struct phy *generic_phy;
> +	const struct snps_eusb2_phy_drvdata *drv_data;
> +	int ret, i;
> +	int num;
> +
> +	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
> +	if (!phy)
> +		return -ENOMEM;
> +
> +	drv_data = of_device_get_match_data(dev);
> +	if (!drv_data)
> +		return -EINVAL;
> +	phy->data = drv_data;
> +
> +	phy->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(phy->base))
> +		return PTR_ERR(phy->base);
> +
> +	phy->phy_reset = devm_reset_control_get_exclusive(dev, NULL);
> +	if (IS_ERR(phy->phy_reset))
> +		return PTR_ERR(phy->phy_reset);
> +
> +	phy->clks = devm_kcalloc(dev,
> +				 phy->data->num_clks,
> +				 sizeof(*phy->clks),
> +				 GFP_KERNEL);
> +	if (!phy->clks)
> +		return -ENOMEM;
> +
> +	for (int i = 0; i < phy->data->num_clks; ++i)
> +		phy->clks[i].id = phy->data->clk_names[i];
> +
> +	ret = devm_clk_bulk_get(dev, phy->data->num_clks,
> +				phy->clks);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to get phy clock(s)\n");
> +
> +	phy->ref_clk = NULL;
> +	for (int i = 0; i < phy->data->num_clks; ++i) {
> +		if (!strcmp(phy->clks[i].id, "ref")) {
> +			phy->ref_clk = phy->clks[i].clk;
> +			break;
> +		}
> +	}
> +
> +	if (IS_ERR_OR_NULL(phy->ref_clk))
> +		return dev_err_probe(dev, PTR_ERR(phy->ref_clk),
> +				     "failed to get ref clk\n");
> +
> +	num = ARRAY_SIZE(phy->vregs);
> +	for (i = 0; i < num; i++)
> +		phy->vregs[i].supply = eusb2_hsphy_vreg_names[i];
> +
> +	ret = devm_regulator_bulk_get(dev, num, phy->vregs);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to get regulator supplies\n");
> +
> +	phy->repeater = devm_of_phy_get_by_index(dev, np, 0);
> +	if (IS_ERR(phy->repeater))
> +		return dev_err_probe(dev, PTR_ERR(phy->repeater),
> +				     "failed to get repeater\n");
> +
> +	generic_phy = devm_phy_create(dev, NULL, &snps_eusb2_hsphy_ops);
> +	if (IS_ERR(generic_phy)) {
> +		dev_err(dev, "failed to create phy %d\n", ret);
> +		return PTR_ERR(generic_phy);
> +	}
> +
> +	dev_set_drvdata(dev, phy);
> +	phy_set_drvdata(generic_phy, phy);
> +
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	if (IS_ERR(phy_provider))
> +		return PTR_ERR(phy_provider);
> +
> +	dev_info(dev, "Registered Snps-eUSB2 phy\n");
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id snps_eusb2_hsphy_of_match_table[] = {
> +	{
> +		.compatible = "qcom,sm8550-snps-eusb2-phy",
> +		.data = &sm8550_snps_eusb2_phy,
> +	}, { },
> +};
> +MODULE_DEVICE_TABLE(of, snps_eusb2_hsphy_of_match_table);
> +
> +static struct platform_driver snps_eusb2_hsphy_driver = {
> +	.probe		= snps_eusb2_hsphy_probe,
> +	.driver = {
> +		.name	= "snps-eusb2-hsphy",
> +		.of_match_table = snps_eusb2_hsphy_of_match_table,
> +	},
> +};
> +
> +module_platform_driver(snps_eusb2_hsphy_driver);
> +MODULE_DESCRIPTION("SNPS eUSB2 HS PHY driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
> index 846f8c995..914547068 100644
> --- a/drivers/phy/qualcomm/Kconfig
> +++ b/drivers/phy/qualcomm/Kconfig
> @@ -125,15 +125,6 @@ config PHY_QCOM_QUSB2
>   	  PHY which is usually paired with either the ChipIdea or Synopsys DWC3
>   	  USB IPs on MSM SOCs.
>   
> -config PHY_QCOM_SNPS_EUSB2
> -	tristate "Qualcomm SNPS eUSB2 PHY Driver"
> -	depends on OF && (ARCH_QCOM || COMPILE_TEST)
> -	select GENERIC_PHY
> -	help
> -	  Enable support for the USB high-speed SNPS eUSB2 phy on Qualcomm
> -	  chipsets. The PHY is paired with a Synopsys DWC3 USB controller
> -	  on Qualcomm SOCs.
> -
>   config PHY_QCOM_EUSB2_REPEATER
>   	tristate "Qualcomm SNPS eUSB2 Repeater Driver"
>   	depends on OF && (ARCH_QCOM || COMPILE_TEST)
> diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
> index eb60e950a..2121e92df 100644
> --- a/drivers/phy/qualcomm/Makefile
> +++ b/drivers/phy/qualcomm/Makefile
> @@ -15,7 +15,6 @@ obj-$(CONFIG_PHY_QCOM_QMP_USB)		+= phy-qcom-qmp-usb.o
>   obj-$(CONFIG_PHY_QCOM_QMP_USB_LEGACY)	+= phy-qcom-qmp-usb-legacy.o
>   
>   obj-$(CONFIG_PHY_QCOM_QUSB2)		+= phy-qcom-qusb2.o
> -obj-$(CONFIG_PHY_QCOM_SNPS_EUSB2)	+= phy-qcom-snps-eusb2.o
>   obj-$(CONFIG_PHY_QCOM_EUSB2_REPEATER)	+= phy-qcom-eusb2-repeater.o
>   obj-$(CONFIG_PHY_QCOM_USB_HS) 		+= phy-qcom-usb-hs.o
>   obj-$(CONFIG_PHY_QCOM_USB_HSIC) 	+= phy-qcom-usb-hsic.o
> diff --git a/drivers/phy/qualcomm/phy-qcom-snps-eusb2.c b/drivers/phy/qualcomm/phy-qcom-snps-eusb2.c
> deleted file mode 100644
> index 1484691a4..000000000
> --- a/drivers/phy/qualcomm/phy-qcom-snps-eusb2.c
> +++ /dev/null
> @@ -1,442 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * Copyright (c) 2023, Linaro Limited
> - */
> -
> -#include <linux/bitfield.h>
> -#include <linux/clk.h>
> -#include <linux/delay.h>
> -#include <linux/iopoll.h>
> -#include <linux/mod_devicetable.h>
> -#include <linux/phy/phy.h>
> -#include <linux/platform_device.h>
> -#include <linux/regulator/consumer.h>
> -#include <linux/reset.h>
> -
> -#define USB_PHY_UTMI_CTRL0		(0x3c)
> -#define SLEEPM				BIT(0)
> -#define OPMODE_MASK			GENMASK(4, 3)
> -#define OPMODE_NONDRIVING		BIT(3)
> -
> -#define USB_PHY_UTMI_CTRL5		(0x50)
> -#define POR				BIT(1)
> -
> -#define USB_PHY_HS_PHY_CTRL_COMMON0	(0x54)
> -#define PHY_ENABLE			BIT(0)
> -#define SIDDQ_SEL			BIT(1)
> -#define SIDDQ				BIT(2)
> -#define RETENABLEN			BIT(3)
> -#define FSEL_MASK			GENMASK(6, 4)
> -#define FSEL_19_2_MHZ_VAL		(0x0)
> -#define FSEL_38_4_MHZ_VAL		(0x4)
> -
> -#define USB_PHY_CFG_CTRL_1		(0x58)
> -#define PHY_CFG_PLL_CPBIAS_CNTRL_MASK	GENMASK(7, 1)
> -
> -#define USB_PHY_CFG_CTRL_2		(0x5c)
> -#define PHY_CFG_PLL_FB_DIV_7_0_MASK	GENMASK(7, 0)
> -#define DIV_7_0_19_2_MHZ_VAL		(0x90)
> -#define DIV_7_0_38_4_MHZ_VAL		(0xc8)
> -
> -#define USB_PHY_CFG_CTRL_3		(0x60)
> -#define PHY_CFG_PLL_FB_DIV_11_8_MASK	GENMASK(3, 0)
> -#define DIV_11_8_19_2_MHZ_VAL		(0x1)
> -#define DIV_11_8_38_4_MHZ_VAL		(0x0)
> -
> -#define PHY_CFG_PLL_REF_DIV		GENMASK(7, 4)
> -#define PLL_REF_DIV_VAL			(0x0)
> -
> -#define USB_PHY_HS_PHY_CTRL2		(0x64)
> -#define VBUSVLDEXT0			BIT(0)
> -#define USB2_SUSPEND_N			BIT(2)
> -#define USB2_SUSPEND_N_SEL		BIT(3)
> -#define VBUS_DET_EXT_SEL		BIT(4)
> -
> -#define USB_PHY_CFG_CTRL_4		(0x68)
> -#define PHY_CFG_PLL_GMP_CNTRL_MASK	GENMASK(1, 0)
> -#define PHY_CFG_PLL_INT_CNTRL_MASK	GENMASK(7, 2)
> -
> -#define USB_PHY_CFG_CTRL_5		(0x6c)
> -#define PHY_CFG_PLL_PROP_CNTRL_MASK	GENMASK(4, 0)
> -#define PHY_CFG_PLL_VREF_TUNE_MASK	GENMASK(7, 6)
> -
> -#define USB_PHY_CFG_CTRL_6		(0x70)
> -#define PHY_CFG_PLL_VCO_CNTRL_MASK	GENMASK(2, 0)
> -
> -#define USB_PHY_CFG_CTRL_7		(0x74)
> -
> -#define USB_PHY_CFG_CTRL_8		(0x78)
> -#define PHY_CFG_TX_FSLS_VREF_TUNE_MASK	GENMASK(1, 0)
> -#define PHY_CFG_TX_FSLS_VREG_BYPASS	BIT(2)
> -#define PHY_CFG_TX_HS_VREF_TUNE_MASK	GENMASK(5, 3)
> -#define PHY_CFG_TX_HS_XV_TUNE_MASK	GENMASK(7, 6)
> -
> -#define USB_PHY_CFG_CTRL_9		(0x7c)
> -#define PHY_CFG_TX_PREEMP_TUNE_MASK	GENMASK(2, 0)
> -#define PHY_CFG_TX_RES_TUNE_MASK	GENMASK(4, 3)
> -#define PHY_CFG_TX_RISE_TUNE_MASK	GENMASK(6, 5)
> -#define PHY_CFG_RCAL_BYPASS		BIT(7)
> -
> -#define USB_PHY_CFG_CTRL_10		(0x80)
> -
> -#define USB_PHY_CFG0			(0x94)
> -#define DATAPATH_CTRL_OVERRIDE_EN	BIT(0)
> -#define CMN_CTRL_OVERRIDE_EN		BIT(1)
> -
> -#define UTMI_PHY_CMN_CTRL0		(0x98)
> -#define TESTBURNIN			BIT(6)
> -
> -#define USB_PHY_FSEL_SEL		(0xb8)
> -#define FSEL_SEL			BIT(0)
> -
> -#define USB_PHY_APB_ACCESS_CMD		(0x130)
> -#define RW_ACCESS			BIT(0)
> -#define APB_START_CMD			BIT(1)
> -#define APB_LOGIC_RESET			BIT(2)
> -
> -#define USB_PHY_APB_ACCESS_STATUS	(0x134)
> -#define ACCESS_DONE			BIT(0)
> -#define TIMED_OUT			BIT(1)
> -#define ACCESS_ERROR			BIT(2)
> -#define ACCESS_IN_PROGRESS		BIT(3)
> -
> -#define USB_PHY_APB_ADDRESS		(0x138)
> -#define APB_REG_ADDR_MASK		GENMASK(7, 0)
> -
> -#define USB_PHY_APB_WRDATA_LSB		(0x13c)
> -#define APB_REG_WRDATA_7_0_MASK		GENMASK(3, 0)
> -
> -#define USB_PHY_APB_WRDATA_MSB		(0x140)
> -#define APB_REG_WRDATA_15_8_MASK	GENMASK(7, 4)
> -
> -#define USB_PHY_APB_RDDATA_LSB		(0x144)
> -#define APB_REG_RDDATA_7_0_MASK		GENMASK(3, 0)
> -
> -#define USB_PHY_APB_RDDATA_MSB		(0x148)
> -#define APB_REG_RDDATA_15_8_MASK	GENMASK(7, 4)
> -
> -static const char * const eusb2_hsphy_vreg_names[] = {
> -	"vdd", "vdda12",
> -};
> -
> -#define EUSB2_NUM_VREGS		ARRAY_SIZE(eusb2_hsphy_vreg_names)
> -
> -struct qcom_snps_eusb2_hsphy {
> -	struct phy *phy;
> -	void __iomem *base;
> -
> -	struct clk *ref_clk;
> -	struct reset_control *phy_reset;
> -
> -	struct regulator_bulk_data vregs[EUSB2_NUM_VREGS];
> -
> -	enum phy_mode mode;
> -
> -	struct phy *repeater;
> -};
> -
> -static int qcom_snps_eusb2_hsphy_set_mode(struct phy *p, enum phy_mode mode, int submode)
> -{
> -	struct qcom_snps_eusb2_hsphy *phy = phy_get_drvdata(p);
> -
> -	phy->mode = mode;
> -
> -	return phy_set_mode_ext(phy->repeater, mode, submode);
> -}
> -
> -static void qcom_snps_eusb2_hsphy_write_mask(void __iomem *base, u32 offset,
> -					     u32 mask, u32 val)
> -{
> -	u32 reg;
> -
> -	reg = readl_relaxed(base + offset);
> -	reg &= ~mask;
> -	reg |= val & mask;
> -	writel_relaxed(reg, base + offset);
> -
> -	/* Ensure above write is completed */
> -	readl_relaxed(base + offset);
> -}
> -
> -static void qcom_eusb2_default_parameters(struct qcom_snps_eusb2_hsphy *phy)
> -{
> -	/* default parameters: tx pre-emphasis */
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_9,
> -					 PHY_CFG_TX_PREEMP_TUNE_MASK,
> -					 FIELD_PREP(PHY_CFG_TX_PREEMP_TUNE_MASK, 0));
> -
> -	/* tx rise/fall time */
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_9,
> -					 PHY_CFG_TX_RISE_TUNE_MASK,
> -					 FIELD_PREP(PHY_CFG_TX_RISE_TUNE_MASK, 0x2));
> -
> -	/* source impedance adjustment */
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_9,
> -					 PHY_CFG_TX_RES_TUNE_MASK,
> -					 FIELD_PREP(PHY_CFG_TX_RES_TUNE_MASK, 0x1));
> -
> -	/* dc voltage level adjustement */
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_8,
> -					 PHY_CFG_TX_HS_VREF_TUNE_MASK,
> -					 FIELD_PREP(PHY_CFG_TX_HS_VREF_TUNE_MASK, 0x3));
> -
> -	/* transmitter HS crossover adjustement */
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_8,
> -					 PHY_CFG_TX_HS_XV_TUNE_MASK,
> -					 FIELD_PREP(PHY_CFG_TX_HS_XV_TUNE_MASK, 0x0));
> -}
> -
> -static int qcom_eusb2_ref_clk_init(struct qcom_snps_eusb2_hsphy *phy)
> -{
> -	unsigned long ref_clk_freq = clk_get_rate(phy->ref_clk);
> -
> -	switch (ref_clk_freq) {
> -	case 19200000:
> -		qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_HS_PHY_CTRL_COMMON0,
> -						 FSEL_MASK,
> -						 FIELD_PREP(FSEL_MASK, FSEL_19_2_MHZ_VAL));
> -
> -		qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_2,
> -						 PHY_CFG_PLL_FB_DIV_7_0_MASK,
> -						 DIV_7_0_19_2_MHZ_VAL);
> -
> -		qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_3,
> -						 PHY_CFG_PLL_FB_DIV_11_8_MASK,
> -						 DIV_11_8_19_2_MHZ_VAL);
> -		break;
> -
> -	case 38400000:
> -		qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_HS_PHY_CTRL_COMMON0,
> -						 FSEL_MASK,
> -						 FIELD_PREP(FSEL_MASK, FSEL_38_4_MHZ_VAL));
> -
> -		qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_2,
> -						 PHY_CFG_PLL_FB_DIV_7_0_MASK,
> -						 DIV_7_0_38_4_MHZ_VAL);
> -
> -		qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_3,
> -						 PHY_CFG_PLL_FB_DIV_11_8_MASK,
> -						 DIV_11_8_38_4_MHZ_VAL);
> -		break;
> -
> -	default:
> -		dev_err(&phy->phy->dev, "unsupported ref_clk_freq:%lu\n", ref_clk_freq);
> -		return -EINVAL;
> -	}
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_3,
> -					 PHY_CFG_PLL_REF_DIV, PLL_REF_DIV_VAL);
> -
> -	return 0;
> -}
> -
> -static int qcom_snps_eusb2_hsphy_init(struct phy *p)
> -{
> -	struct qcom_snps_eusb2_hsphy *phy = phy_get_drvdata(p);
> -	int ret;
> -
> -	ret = regulator_bulk_enable(ARRAY_SIZE(phy->vregs), phy->vregs);
> -	if (ret)
> -		return ret;
> -
> -	ret = phy_init(phy->repeater);
> -	if (ret) {
> -		dev_err(&p->dev, "repeater init failed. %d\n", ret);
> -		goto disable_vreg;
> -	}
> -
> -	ret = clk_prepare_enable(phy->ref_clk);
> -	if (ret) {
> -		dev_err(&p->dev, "failed to enable ref clock, %d\n", ret);
> -		goto disable_vreg;
> -	}
> -
> -	ret = reset_control_assert(phy->phy_reset);
> -	if (ret) {
> -		dev_err(&p->dev, "failed to assert phy_reset, %d\n", ret);
> -		goto disable_ref_clk;
> -	}
> -
> -	usleep_range(100, 150);
> -
> -	ret = reset_control_deassert(phy->phy_reset);
> -	if (ret) {
> -		dev_err(&p->dev, "failed to de-assert phy_reset, %d\n", ret);
> -		goto disable_ref_clk;
> -	}
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG0,
> -					 CMN_CTRL_OVERRIDE_EN, CMN_CTRL_OVERRIDE_EN);
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_UTMI_CTRL5, POR, POR);
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_HS_PHY_CTRL_COMMON0,
> -					 PHY_ENABLE | RETENABLEN, PHY_ENABLE | RETENABLEN);
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_APB_ACCESS_CMD,
> -					 APB_LOGIC_RESET, APB_LOGIC_RESET);
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, UTMI_PHY_CMN_CTRL0, TESTBURNIN, 0);
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_FSEL_SEL,
> -					 FSEL_SEL, FSEL_SEL);
> -
> -	/* update ref_clk related registers */
> -	ret = qcom_eusb2_ref_clk_init(phy);
> -	if (ret)
> -		goto disable_ref_clk;
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_1,
> -					 PHY_CFG_PLL_CPBIAS_CNTRL_MASK,
> -					 FIELD_PREP(PHY_CFG_PLL_CPBIAS_CNTRL_MASK, 0x1));
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_4,
> -					 PHY_CFG_PLL_INT_CNTRL_MASK,
> -					 FIELD_PREP(PHY_CFG_PLL_INT_CNTRL_MASK, 0x8));
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_4,
> -					 PHY_CFG_PLL_GMP_CNTRL_MASK,
> -					 FIELD_PREP(PHY_CFG_PLL_GMP_CNTRL_MASK, 0x1));
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_5,
> -					 PHY_CFG_PLL_PROP_CNTRL_MASK,
> -					 FIELD_PREP(PHY_CFG_PLL_PROP_CNTRL_MASK, 0x10));
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_6,
> -					 PHY_CFG_PLL_VCO_CNTRL_MASK,
> -					 FIELD_PREP(PHY_CFG_PLL_VCO_CNTRL_MASK, 0x0));
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_CFG_CTRL_5,
> -					 PHY_CFG_PLL_VREF_TUNE_MASK,
> -					 FIELD_PREP(PHY_CFG_PLL_VREF_TUNE_MASK, 0x1));
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_HS_PHY_CTRL2,
> -					 VBUS_DET_EXT_SEL, VBUS_DET_EXT_SEL);
> -
> -	/* set default parameters */
> -	qcom_eusb2_default_parameters(phy);
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_HS_PHY_CTRL2,
> -					 USB2_SUSPEND_N_SEL | USB2_SUSPEND_N,
> -					 USB2_SUSPEND_N_SEL | USB2_SUSPEND_N);
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_UTMI_CTRL0, SLEEPM, SLEEPM);
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_HS_PHY_CTRL_COMMON0,
> -					 SIDDQ_SEL, SIDDQ_SEL);
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_HS_PHY_CTRL_COMMON0,
> -					 SIDDQ, 0);
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_UTMI_CTRL5, POR, 0);
> -
> -	qcom_snps_eusb2_hsphy_write_mask(phy->base, USB_PHY_HS_PHY_CTRL2,
> -					 USB2_SUSPEND_N_SEL, 0);
> -
> -	return 0;
> -
> -disable_ref_clk:
> -	clk_disable_unprepare(phy->ref_clk);
> -
> -disable_vreg:
> -	regulator_bulk_disable(ARRAY_SIZE(phy->vregs), phy->vregs);
> -
> -	return ret;
> -}
> -
> -static int qcom_snps_eusb2_hsphy_exit(struct phy *p)
> -{
> -	struct qcom_snps_eusb2_hsphy *phy = phy_get_drvdata(p);
> -
> -	clk_disable_unprepare(phy->ref_clk);
> -
> -	regulator_bulk_disable(ARRAY_SIZE(phy->vregs), phy->vregs);
> -
> -	phy_exit(phy->repeater);
> -
> -	return 0;
> -}
> -
> -static const struct phy_ops qcom_snps_eusb2_hsphy_ops = {
> -	.init		= qcom_snps_eusb2_hsphy_init,
> -	.exit		= qcom_snps_eusb2_hsphy_exit,
> -	.set_mode	= qcom_snps_eusb2_hsphy_set_mode,
> -	.owner		= THIS_MODULE,
> -};
> -
> -static int qcom_snps_eusb2_hsphy_probe(struct platform_device *pdev)
> -{
> -	struct device *dev = &pdev->dev;
> -	struct device_node *np = dev->of_node;
> -	struct qcom_snps_eusb2_hsphy *phy;
> -	struct phy_provider *phy_provider;
> -	struct phy *generic_phy;
> -	int ret, i;
> -	int num;
> -
> -	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
> -	if (!phy)
> -		return -ENOMEM;
> -
> -	phy->base = devm_platform_ioremap_resource(pdev, 0);
> -	if (IS_ERR(phy->base))
> -		return PTR_ERR(phy->base);
> -
> -	phy->phy_reset = devm_reset_control_get_exclusive(dev, NULL);
> -	if (IS_ERR(phy->phy_reset))
> -		return PTR_ERR(phy->phy_reset);
> -
> -	phy->ref_clk = devm_clk_get(dev, "ref");
> -	if (IS_ERR(phy->ref_clk))
> -		return dev_err_probe(dev, PTR_ERR(phy->ref_clk),
> -				     "failed to get ref clk\n");
> -
> -	num = ARRAY_SIZE(phy->vregs);
> -	for (i = 0; i < num; i++)
> -		phy->vregs[i].supply = eusb2_hsphy_vreg_names[i];
> -
> -	ret = devm_regulator_bulk_get(dev, num, phy->vregs);
> -	if (ret)
> -		return dev_err_probe(dev, ret,
> -				     "failed to get regulator supplies\n");
> -
> -	phy->repeater = devm_of_phy_get_by_index(dev, np, 0);
> -	if (IS_ERR(phy->repeater))
> -		return dev_err_probe(dev, PTR_ERR(phy->repeater),
> -				     "failed to get repeater\n");
> -
> -	generic_phy = devm_phy_create(dev, NULL, &qcom_snps_eusb2_hsphy_ops);
> -	if (IS_ERR(generic_phy)) {
> -		dev_err(dev, "failed to create phy %d\n", ret);
> -		return PTR_ERR(generic_phy);
> -	}
> -
> -	dev_set_drvdata(dev, phy);
> -	phy_set_drvdata(generic_phy, phy);
> -
> -	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> -	if (IS_ERR(phy_provider))
> -		return PTR_ERR(phy_provider);
> -
> -	dev_info(dev, "Registered Qcom-eUSB2 phy\n");
> -
> -	return 0;
> -}
> -
> -static const struct of_device_id qcom_snps_eusb2_hsphy_of_match_table[] = {
> -	{ .compatible = "qcom,sm8550-snps-eusb2-phy", },
> -	{ },
> -};
> -MODULE_DEVICE_TABLE(of, qcom_snps_eusb2_hsphy_of_match_table);
> -
> -static struct platform_driver qcom_snps_eusb2_hsphy_driver = {
> -	.probe		= qcom_snps_eusb2_hsphy_probe,
> -	.driver = {
> -		.name	= "qcom-snps-eusb2-hsphy",
> -		.of_match_table = qcom_snps_eusb2_hsphy_of_match_table,
> -	},
> -};
> -
> -module_platform_driver(qcom_snps_eusb2_hsphy_driver);
> -MODULE_DESCRIPTION("Qualcomm SNPS eUSB2 HS PHY driver");
> -MODULE_LICENSE("GPL");

Re: [PATCH v2 4/8] phy: move phy-qcom-snps-eusb2 out of its vendor sub-directory

[Dan Carpenter]

Hi Ivaylo,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ivaylo-Ivanov/dt-bindings-phy-rename-qcom-snps-eusb2-phy-binding-to-snps-eusb2-phy/20250223-202709
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20250223122227.725233-5-ivo.ivanov.ivanov1%40gmail.com
patch subject: [PATCH v2 4/8] phy: move phy-qcom-snps-eusb2 out of its vendor sub-directory
config: sh-randconfig-r073-20250307 (https://download.01.org/0day-ci/archive/20250307/202503072305.Osodtcnk-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 14.2.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202503072305.Osodtcnk-lkp@intel.com/

smatch warnings:
drivers/phy/phy-snps-eusb2.c:452 snps_eusb2_hsphy_probe() warn: passing zero to 'PTR_ERR'

vim +/PTR_ERR +452 drivers/phy/phy-snps-eusb2.c

15a1981f608b0f Ivaylo Ivanov 2025-02-23  399  static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
15a1981f608b0f Ivaylo Ivanov 2025-02-23  400  {
15a1981f608b0f Ivaylo Ivanov 2025-02-23  401  	struct device *dev = &pdev->dev;
15a1981f608b0f Ivaylo Ivanov 2025-02-23  402  	struct device_node *np = dev->of_node;
15a1981f608b0f Ivaylo Ivanov 2025-02-23  403  	struct snps_eusb2_hsphy *phy;
15a1981f608b0f Ivaylo Ivanov 2025-02-23  404  	struct phy_provider *phy_provider;
15a1981f608b0f Ivaylo Ivanov 2025-02-23  405  	struct phy *generic_phy;
15a1981f608b0f Ivaylo Ivanov 2025-02-23  406  	const struct snps_eusb2_phy_drvdata *drv_data;
15a1981f608b0f Ivaylo Ivanov 2025-02-23  407  	int ret, i;
15a1981f608b0f Ivaylo Ivanov 2025-02-23  408  	int num;
15a1981f608b0f Ivaylo Ivanov 2025-02-23  409  
15a1981f608b0f Ivaylo Ivanov 2025-02-23  410  	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
15a1981f608b0f Ivaylo Ivanov 2025-02-23  411  	if (!phy)
15a1981f608b0f Ivaylo Ivanov 2025-02-23  412  		return -ENOMEM;
15a1981f608b0f Ivaylo Ivanov 2025-02-23  413  
15a1981f608b0f Ivaylo Ivanov 2025-02-23  414  	drv_data = of_device_get_match_data(dev);
15a1981f608b0f Ivaylo Ivanov 2025-02-23  415  	if (!drv_data)
15a1981f608b0f Ivaylo Ivanov 2025-02-23  416  		return -EINVAL;
15a1981f608b0f Ivaylo Ivanov 2025-02-23  417  	phy->data = drv_data;
15a1981f608b0f Ivaylo Ivanov 2025-02-23  418  
15a1981f608b0f Ivaylo Ivanov 2025-02-23  419  	phy->base = devm_platform_ioremap_resource(pdev, 0);
15a1981f608b0f Ivaylo Ivanov 2025-02-23  420  	if (IS_ERR(phy->base))
15a1981f608b0f Ivaylo Ivanov 2025-02-23  421  		return PTR_ERR(phy->base);
15a1981f608b0f Ivaylo Ivanov 2025-02-23  422  
15a1981f608b0f Ivaylo Ivanov 2025-02-23  423  	phy->phy_reset = devm_reset_control_get_exclusive(dev, NULL);
15a1981f608b0f Ivaylo Ivanov 2025-02-23  424  	if (IS_ERR(phy->phy_reset))
15a1981f608b0f Ivaylo Ivanov 2025-02-23  425  		return PTR_ERR(phy->phy_reset);
15a1981f608b0f Ivaylo Ivanov 2025-02-23  426  
15a1981f608b0f Ivaylo Ivanov 2025-02-23  427  	phy->clks = devm_kcalloc(dev,
15a1981f608b0f Ivaylo Ivanov 2025-02-23  428  				 phy->data->num_clks,
15a1981f608b0f Ivaylo Ivanov 2025-02-23  429  				 sizeof(*phy->clks),
15a1981f608b0f Ivaylo Ivanov 2025-02-23  430  				 GFP_KERNEL);
15a1981f608b0f Ivaylo Ivanov 2025-02-23  431  	if (!phy->clks)
15a1981f608b0f Ivaylo Ivanov 2025-02-23  432  		return -ENOMEM;
15a1981f608b0f Ivaylo Ivanov 2025-02-23  433  
15a1981f608b0f Ivaylo Ivanov 2025-02-23  434  	for (int i = 0; i < phy->data->num_clks; ++i)
15a1981f608b0f Ivaylo Ivanov 2025-02-23  435  		phy->clks[i].id = phy->data->clk_names[i];
15a1981f608b0f Ivaylo Ivanov 2025-02-23  436  
15a1981f608b0f Ivaylo Ivanov 2025-02-23  437  	ret = devm_clk_bulk_get(dev, phy->data->num_clks,
15a1981f608b0f Ivaylo Ivanov 2025-02-23  438  				phy->clks);
15a1981f608b0f Ivaylo Ivanov 2025-02-23  439  	if (ret)
15a1981f608b0f Ivaylo Ivanov 2025-02-23  440  		return dev_err_probe(dev, ret,
15a1981f608b0f Ivaylo Ivanov 2025-02-23  441  				     "failed to get phy clock(s)\n");
15a1981f608b0f Ivaylo Ivanov 2025-02-23  442  
15a1981f608b0f Ivaylo Ivanov 2025-02-23  443  	phy->ref_clk = NULL;
15a1981f608b0f Ivaylo Ivanov 2025-02-23  444  	for (int i = 0; i < phy->data->num_clks; ++i) {
15a1981f608b0f Ivaylo Ivanov 2025-02-23  445  		if (!strcmp(phy->clks[i].id, "ref")) {
15a1981f608b0f Ivaylo Ivanov 2025-02-23  446  			phy->ref_clk = phy->clks[i].clk;
15a1981f608b0f Ivaylo Ivanov 2025-02-23  447  			break;
15a1981f608b0f Ivaylo Ivanov 2025-02-23  448  		}
15a1981f608b0f Ivaylo Ivanov 2025-02-23  449  	}
15a1981f608b0f Ivaylo Ivanov 2025-02-23  450  
15a1981f608b0f Ivaylo Ivanov 2025-02-23  451  	if (IS_ERR_OR_NULL(phy->ref_clk))
15a1981f608b0f Ivaylo Ivanov 2025-02-23 @452  		return dev_err_probe(dev, PTR_ERR(phy->ref_clk),

PTR_ERR(phy->ref_clk) is success.

15a1981f608b0f Ivaylo Ivanov 2025-02-23  453  				     "failed to get ref clk\n");
15a1981f608b0f Ivaylo Ivanov 2025-02-23  454  
15a1981f608b0f Ivaylo Ivanov 2025-02-23  455  	num = ARRAY_SIZE(phy->vregs);
15a1981f608b0f Ivaylo Ivanov 2025-02-23  456  	for (i = 0; i < num; i++)
15a1981f608b0f Ivaylo Ivanov 2025-02-23  457  		phy->vregs[i].supply = eusb2_hsphy_vreg_names[i];
15a1981f608b0f Ivaylo Ivanov 2025-02-23  458  
15a1981f608b0f Ivaylo Ivanov 2025-02-23  459  	ret = devm_regulator_bulk_get(dev, num, phy->vregs);
15a1981f608b0f Ivaylo Ivanov 2025-02-23  460  	if (ret)
15a1981f608b0f Ivaylo Ivanov 2025-02-23  461  		return dev_err_probe(dev, ret,
15a1981f608b0f Ivaylo Ivanov 2025-02-23  462  				     "failed to get regulator supplies\n");
15a1981f608b0f Ivaylo Ivanov 2025-02-23  463  
15a1981f608b0f Ivaylo Ivanov 2025-02-23  464  	phy->repeater = devm_of_phy_get_by_index(dev, np, 0);
15a1981f608b0f Ivaylo Ivanov 2025-02-23  465  	if (IS_ERR(phy->repeater))
15a1981f608b0f Ivaylo Ivanov 2025-02-23  466  		return dev_err_probe(dev, PTR_ERR(phy->repeater),
15a1981f608b0f Ivaylo Ivanov 2025-02-23  467  				     "failed to get repeater\n");
15a1981f608b0f Ivaylo Ivanov 2025-02-23  468  
15a1981f608b0f Ivaylo Ivanov 2025-02-23  469  	generic_phy = devm_phy_create(dev, NULL, &snps_eusb2_hsphy_ops);
15a1981f608b0f Ivaylo Ivanov 2025-02-23  470  	if (IS_ERR(generic_phy)) {
15a1981f608b0f Ivaylo Ivanov 2025-02-23  471  		dev_err(dev, "failed to create phy %d\n", ret);
15a1981f608b0f Ivaylo Ivanov 2025-02-23  472  		return PTR_ERR(generic_phy);
15a1981f608b0f Ivaylo Ivanov 2025-02-23  473  	}
15a1981f608b0f Ivaylo Ivanov 2025-02-23  474  
15a1981f608b0f Ivaylo Ivanov 2025-02-23  475  	dev_set_drvdata(dev, phy);
15a1981f608b0f Ivaylo Ivanov 2025-02-23  476  	phy_set_drvdata(generic_phy, phy);
15a1981f608b0f Ivaylo Ivanov 2025-02-23  477  
15a1981f608b0f Ivaylo Ivanov 2025-02-23  478  	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
15a1981f608b0f Ivaylo Ivanov 2025-02-23  479  	if (IS_ERR(phy_provider))
15a1981f608b0f Ivaylo Ivanov 2025-02-23  480  		return PTR_ERR(phy_provider);
15a1981f608b0f Ivaylo Ivanov 2025-02-23  481  
15a1981f608b0f Ivaylo Ivanov 2025-02-23  482  	dev_info(dev, "Registered Snps-eUSB2 phy\n");
15a1981f608b0f Ivaylo Ivanov 2025-02-23  483  
15a1981f608b0f Ivaylo Ivanov 2025-02-23  484  	return 0;
15a1981f608b0f Ivaylo Ivanov 2025-02-23  485  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v2 5/8] phy: phy-snps-eusb2: make repeater optional

[Ivaylo Ivanov]

Some platforms initialize their eUSB2 to USB repeater in the previous
stage bootloader and leave it in a working state for linux. Make the
repeater optional in order to allow for reusing that state until
proper repeater drivers are introduced.

Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
---
 drivers/phy/phy-snps-eusb2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
index 4e5914a76..dcc69c00a 100644
--- a/drivers/phy/phy-snps-eusb2.c
+++ b/drivers/phy/phy-snps-eusb2.c
@@ -461,7 +461,7 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, ret,
 				     "failed to get regulator supplies\n");
 
-	phy->repeater = devm_of_phy_get_by_index(dev, np, 0);
+	phy->repeater = devm_of_phy_optional_get(dev, np, 0);
 	if (IS_ERR(phy->repeater))
 		return dev_err_probe(dev, PTR_ERR(phy->repeater),
 				     "failed to get repeater\n");
-- 
2.43.0

Re: [PATCH v2 5/8] phy: phy-snps-eusb2: make repeater optional

[Abel Vesa]

On 25-02-23 14:22:24, Ivaylo Ivanov wrote:
> Some platforms initialize their eUSB2 to USB repeater in the previous
> stage bootloader and leave it in a working state for linux. Make the
> repeater optional in order to allow for reusing that state until
> proper repeater drivers are introduced.
> 
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> ---
>  drivers/phy/phy-snps-eusb2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
> index 4e5914a76..dcc69c00a 100644
> --- a/drivers/phy/phy-snps-eusb2.c
> +++ b/drivers/phy/phy-snps-eusb2.c
> @@ -461,7 +461,7 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, ret,
>  				     "failed to get regulator supplies\n");
>  
> -	phy->repeater = devm_of_phy_get_by_index(dev, np, 0);
> +	phy->repeater = devm_of_phy_optional_get(dev, np, 0);

Maybe make it optional based on compatible or something?

>  	if (IS_ERR(phy->repeater))
>  		return dev_err_probe(dev, PTR_ERR(phy->repeater),
>  				     "failed to get repeater\n");
> -- 
> 2.43.0
> 

Re: [PATCH v2 5/8] phy: phy-snps-eusb2: make repeater optional

[neil.armstrong]

On 24/02/2025 11:11, Abel Vesa wrote:
> On 25-02-23 14:22:24, Ivaylo Ivanov wrote:
>> Some platforms initialize their eUSB2 to USB repeater in the previous
>> stage bootloader and leave it in a working state for linux. Make the
>> repeater optional in order to allow for reusing that state until
>> proper repeater drivers are introduced.
>>
>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>> ---
>>   drivers/phy/phy-snps-eusb2.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
>> index 4e5914a76..dcc69c00a 100644
>> --- a/drivers/phy/phy-snps-eusb2.c
>> +++ b/drivers/phy/phy-snps-eusb2.c
>> @@ -461,7 +461,7 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
>>   		return dev_err_probe(dev, ret,
>>   				     "failed to get regulator supplies\n");
>>   
>> -	phy->repeater = devm_of_phy_get_by_index(dev, np, 0);
>> +	phy->repeater = devm_of_phy_optional_get(dev, np, 0);
> 
> Maybe make it optional based on compatible or something?

It's already optional in the bindings:
Documentation/devicetree/bindings/phy/qcom,snps-eusb2-phy.yaml

So it's:
Acked-by: Neil Armstrong <neil.armstrong@linaro.org>

> 
>>   	if (IS_ERR(phy->repeater))
>>   		return dev_err_probe(dev, PTR_ERR(phy->repeater),
>>   				     "failed to get repeater\n");
>> -- 
>> 2.43.0
>>
> 

Re: [PATCH v2 5/8] phy: phy-snps-eusb2: make repeater optional

[kernel test robot]

Hi Ivaylo,

kernel test robot noticed the following build warnings:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v6.14-rc4 next-20250228]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ivaylo-Ivanov/dt-bindings-phy-rename-qcom-snps-eusb2-phy-binding-to-snps-eusb2-phy/20250223-202709
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20250223122227.725233-6-ivo.ivanov.ivanov1%40gmail.com
patch subject: [PATCH v2 5/8] phy: phy-snps-eusb2: make repeater optional
config: microblaze-randconfig-r123-20250302 (https://download.01.org/0day-ci/archive/20250302/202503020920.Kw0H8Acs-lkp@intel.com/config)
compiler: microblaze-linux-gcc (GCC) 14.2.0
reproduce: (https://download.01.org/0day-ci/archive/20250302/202503020920.Kw0H8Acs-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503020920.Kw0H8Acs-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/phy/phy-snps-eusb2.c:464:59: sparse: sparse: Using plain integer as NULL pointer

vim +464 drivers/phy/phy-snps-eusb2.c

   398	
   399	static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
   400	{
   401		struct device *dev = &pdev->dev;
   402		struct device_node *np = dev->of_node;
   403		struct snps_eusb2_hsphy *phy;
   404		struct phy_provider *phy_provider;
   405		struct phy *generic_phy;
   406		const struct snps_eusb2_phy_drvdata *drv_data;
   407		int ret, i;
   408		int num;
   409	
   410		phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
   411		if (!phy)
   412			return -ENOMEM;
   413	
   414		drv_data = of_device_get_match_data(dev);
   415		if (!drv_data)
   416			return -EINVAL;
   417		phy->data = drv_data;
   418	
   419		phy->base = devm_platform_ioremap_resource(pdev, 0);
   420		if (IS_ERR(phy->base))
   421			return PTR_ERR(phy->base);
   422	
   423		phy->phy_reset = devm_reset_control_get_exclusive(dev, NULL);
   424		if (IS_ERR(phy->phy_reset))
   425			return PTR_ERR(phy->phy_reset);
   426	
   427		phy->clks = devm_kcalloc(dev,
   428					 phy->data->num_clks,
   429					 sizeof(*phy->clks),
   430					 GFP_KERNEL);
   431		if (!phy->clks)
   432			return -ENOMEM;
   433	
   434		for (int i = 0; i < phy->data->num_clks; ++i)
   435			phy->clks[i].id = phy->data->clk_names[i];
   436	
   437		ret = devm_clk_bulk_get(dev, phy->data->num_clks,
   438					phy->clks);
   439		if (ret)
   440			return dev_err_probe(dev, ret,
   441					     "failed to get phy clock(s)\n");
   442	
   443		phy->ref_clk = NULL;
   444		for (int i = 0; i < phy->data->num_clks; ++i) {
   445			if (!strcmp(phy->clks[i].id, "ref")) {
   446				phy->ref_clk = phy->clks[i].clk;
   447				break;
   448			}
   449		}
   450	
   451		if (IS_ERR_OR_NULL(phy->ref_clk))
   452			return dev_err_probe(dev, PTR_ERR(phy->ref_clk),
   453					     "failed to get ref clk\n");
   454	
   455		num = ARRAY_SIZE(phy->vregs);
   456		for (i = 0; i < num; i++)
   457			phy->vregs[i].supply = eusb2_hsphy_vreg_names[i];
   458	
   459		ret = devm_regulator_bulk_get(dev, num, phy->vregs);
   460		if (ret)
   461			return dev_err_probe(dev, ret,
   462					     "failed to get regulator supplies\n");
   463	
 > 464		phy->repeater = devm_of_phy_optional_get(dev, np, 0);
   465		if (IS_ERR(phy->repeater))
   466			return dev_err_probe(dev, PTR_ERR(phy->repeater),
   467					     "failed to get repeater\n");
   468	
   469		generic_phy = devm_phy_create(dev, NULL, &snps_eusb2_hsphy_ops);
   470		if (IS_ERR(generic_phy)) {
   471			dev_err(dev, "failed to create phy %d\n", ret);
   472			return PTR_ERR(generic_phy);
   473		}
   474	
   475		dev_set_drvdata(dev, phy);
   476		phy_set_drvdata(generic_phy, phy);
   477	
   478		phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
   479		if (IS_ERR(phy_provider))
   480			return PTR_ERR(phy_provider);
   481	
   482		dev_info(dev, "Registered Snps-eUSB2 phy\n");
   483	
   484		return 0;
   485	}
   486	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 5/8] phy: phy-snps-eusb2: make repeater optional

[Dmitry Baryshkov]

On Sun, Feb 23, 2025 at 02:22:24PM +0200, Ivaylo Ivanov wrote:
> Some platforms initialize their eUSB2 to USB repeater in the previous
> stage bootloader and leave it in a working state for linux. Make the
> repeater optional in order to allow for reusing that state until
> proper repeater drivers are introduced.

Generally "works as it is setup by the bootloader" is a very invalid
justification. Please don't do that. We should not be depending on the
way the bootlader sets up the devices, unless that _really_ makes sense.

> 
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> ---
>  drivers/phy/phy-snps-eusb2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
> index 4e5914a76..dcc69c00a 100644
> --- a/drivers/phy/phy-snps-eusb2.c
> +++ b/drivers/phy/phy-snps-eusb2.c
> @@ -461,7 +461,7 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, ret,
>  				     "failed to get regulator supplies\n");
>  
> -	phy->repeater = devm_of_phy_get_by_index(dev, np, 0);
> +	phy->repeater = devm_of_phy_optional_get(dev, np, 0);
>  	if (IS_ERR(phy->repeater))
>  		return dev_err_probe(dev, PTR_ERR(phy->repeater),
>  				     "failed to get repeater\n");
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 5/8] phy: phy-snps-eusb2: make repeater optional

[Ivaylo Ivanov]

On 3/19/25 13:08, Dmitry Baryshkov wrote:
> On Sun, Feb 23, 2025 at 02:22:24PM +0200, Ivaylo Ivanov wrote:
>> Some platforms initialize their eUSB2 to USB repeater in the previous
>> stage bootloader and leave it in a working state for linux. Make the
>> repeater optional in order to allow for reusing that state until
>> proper repeater drivers are introduced.
> Generally "works as it is setup by the bootloader" is a very invalid
> justification. Please don't do that. We should not be depending on the
> way the bootlader sets up the devices, unless that _really_ makes sense.

It does, doesn't it? We still don't even have i2c up on Exynos2200, so bringing up
the repeater before this patchset gets merged is a no-go. Either way, we should
follow what bindings say. I will change the commit description a bit.

Best regards,
Ivaylo

>
>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>> ---
>>  drivers/phy/phy-snps-eusb2.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
>> index 4e5914a76..dcc69c00a 100644
>> --- a/drivers/phy/phy-snps-eusb2.c
>> +++ b/drivers/phy/phy-snps-eusb2.c
>> @@ -461,7 +461,7 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
>>  		return dev_err_probe(dev, ret,
>>  				     "failed to get regulator supplies\n");
>>  
>> -	phy->repeater = devm_of_phy_get_by_index(dev, np, 0);
>> +	phy->repeater = devm_of_phy_optional_get(dev, np, 0);
>>  	if (IS_ERR(phy->repeater))
>>  		return dev_err_probe(dev, PTR_ERR(phy->repeater),
>>  				     "failed to get repeater\n");
>> -- 
>> 2.43.0
>>

[PATCH v2 6/8] phy: phy-snps-eusb2: make reset control optional

[Ivaylo Ivanov]

Some SoCs don't provide explicit reset lines, so make them optional.

Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
---
 drivers/phy/phy-snps-eusb2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
index dcc69c00a..7a242fe32 100644
--- a/drivers/phy/phy-snps-eusb2.c
+++ b/drivers/phy/phy-snps-eusb2.c
@@ -420,7 +420,7 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
 	if (IS_ERR(phy->base))
 		return PTR_ERR(phy->base);
 
-	phy->phy_reset = devm_reset_control_get_exclusive(dev, NULL);
+	phy->phy_reset = devm_reset_control_get_optional_exclusive(dev, NULL);
 	if (IS_ERR(phy->phy_reset))
 		return PTR_ERR(phy->phy_reset);
 
-- 
2.43.0

Re: [PATCH v2 6/8] phy: phy-snps-eusb2: make reset control optional

[Dmitry Baryshkov]

On Sun, Feb 23, 2025 at 02:22:25PM +0200, Ivaylo Ivanov wrote:
> Some SoCs don't provide explicit reset lines, so make them optional.

Is there an external reset or some other signal?

> 
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> ---
>  drivers/phy/phy-snps-eusb2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

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

-- 
With best wishes
Dmitry

Re: [PATCH v2 6/8] phy: phy-snps-eusb2: make reset control optional

[Ivaylo Ivanov]

On 2/24/25 01:48, Dmitry Baryshkov wrote:
> On Sun, Feb 23, 2025 at 02:22:25PM +0200, Ivaylo Ivanov wrote:
>> Some SoCs don't provide explicit reset lines, so make them optional.
> Is there an external reset or some other signal?

Well..  There probably are on a hardware level, but there's no interface
that exposes them to the kernel.. as far as I've seen. Resets are usually
managed via the blocks' registers. I can't say with certainty because I
don't have access to TRMs.

I can reword this commit message to make that clear.

Best regards,
Ivaylo

>
>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>> ---
>>  drivers/phy/phy-snps-eusb2.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>

[PATCH v2 7/8] phy: phy-snps-eusb2: add support for exynos2200

[Ivaylo Ivanov]

The Exynos2200 SoC reuses the Synopsis eUSB2 PHY IP, alongside an
external repeater, for USB 2.0. Add support for it to the existing
driver.

Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
---
 drivers/phy/Kconfig          |   2 +-
 drivers/phy/phy-snps-eusb2.c | 172 +++++++++++++++++++++++++++++++++++
 2 files changed, 173 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 11c166204..58c911e1b 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -45,7 +45,7 @@ config PHY_PISTACHIO_USB
 
 config PHY_SNPS_EUSB2
 	tristate "SNPS eUSB2 PHY Driver"
-	depends on OF && (ARCH_QCOM || COMPILE_TEST)
+	depends on OF && (ARCH_EXYNOS || ARCH_QCOM || COMPILE_TEST)
 	select GENERIC_PHY
 	help
 	  Enable support for the USB high-speed SNPS eUSB2 phy on select
diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
index 7a242fe32..67a19d671 100644
--- a/drivers/phy/phy-snps-eusb2.c
+++ b/drivers/phy/phy-snps-eusb2.c
@@ -13,6 +13,39 @@
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 
+#define EXYNOS_USB_PHY_HS_PHY_CTRL_RST	(0x0)
+#define USB_PHY_RST_MASK		GENMASK(1, 0)
+#define UTMI_PORT_RST_MASK		GENMASK(5, 4)
+
+#define EXYNOS_USB_PHY_HS_PHY_CTRL_COMMON	(0x4)
+#define RPTR_MODE			BIT(10)
+#define FSEL_20_MHZ_VAL			(0x1)
+#define FSEL_24_MHZ_VAL			(0x2)
+#define FSEL_26_MHZ_VAL			(0x3)
+#define FSEL_48_MHZ_VAL			(0x2)
+
+#define EXYNOS_USB_PHY_CFG_PLLCFG0	(0x8)
+#define PHY_CFG_PLL_FB_DIV_19_8_MASK	GENMASK(19, 8)
+#define DIV_19_8_19_2_MHZ_VAL		(0x170)
+#define DIV_19_8_20_MHZ_VAL		(0x160)
+#define DIV_19_8_24_MHZ_VAL		(0x120)
+#define DIV_19_8_26_MHZ_VAL		(0x107)
+#define DIV_19_8_48_MHZ_VAL		(0x120)
+
+#define EXYNOS_USB_PHY_CFG_PLLCFG1	(0xc)
+#define EXYNOS_PHY_CFG_PLL_FB_DIV_11_8_MASK	GENMASK(11, 8)
+#define EXYNOS_DIV_11_8_19_2_MHZ_VAL	(0x0)
+#define EXYNOS_DIV_11_8_20_MHZ_VAL	(0x0)
+#define EXYNOS_DIV_11_8_24_MHZ_VAL	(0x0)
+#define EXYNOS_DIV_11_8_26_MHZ_VAL	(0x0)
+#define EXYNOS_DIV_11_8_48_MHZ_VAL	(0x1)
+
+#define EXYNOS_PHY_CFG_TX		(0x14)
+#define EXYNOS_PHY_CFG_TX_FSLS_VREF_TUNE_MASK	GENMASK(2, 1)
+
+#define EXYNOS_USB_PHY_UTMI_TESTSE	(0x20)
+#define TEST_IDDQ			BIT(6)
+
 #define QCOM_USB_PHY_UTMI_CTRL0		(0x3c)
 #define SLEEPM				BIT(0)
 #define OPMODE_MASK			GENMASK(4, 3)
@@ -196,6 +229,93 @@ static void qcom_eusb2_default_parameters(struct snps_eusb2_hsphy *phy)
 				    FIELD_PREP(PHY_CFG_TX_HS_XV_TUNE_MASK, 0x0));
 }
 
+static int exynos_eusb2_ref_clk_init(struct snps_eusb2_hsphy *phy)
+{
+	unsigned long ref_clk_freq = clk_get_rate(phy->ref_clk);
+
+	switch (ref_clk_freq) {
+	case 19200000:
+		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_HS_PHY_CTRL_COMMON,
+					    FSEL_MASK,
+					    FIELD_PREP(FSEL_MASK, FSEL_19_2_MHZ_VAL));
+
+		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG0,
+					    PHY_CFG_PLL_FB_DIV_19_8_MASK,
+					    FIELD_PREP(PHY_CFG_PLL_FB_DIV_19_8_MASK,
+						       DIV_19_8_19_2_MHZ_VAL));
+
+		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG1,
+					    EXYNOS_PHY_CFG_PLL_FB_DIV_11_8_MASK,
+					    EXYNOS_DIV_11_8_19_2_MHZ_VAL);
+		break;
+
+	case 20000000:
+		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_HS_PHY_CTRL_COMMON,
+					    FSEL_MASK,
+					    FIELD_PREP(FSEL_MASK, FSEL_20_MHZ_VAL));
+
+		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG0,
+					    PHY_CFG_PLL_FB_DIV_19_8_MASK,
+					    FIELD_PREP(PHY_CFG_PLL_FB_DIV_19_8_MASK,
+						       DIV_19_8_20_MHZ_VAL));
+
+		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG1,
+					    EXYNOS_PHY_CFG_PLL_FB_DIV_11_8_MASK,
+					    EXYNOS_DIV_11_8_20_MHZ_VAL);
+		break;
+
+	case 24000000:
+		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_HS_PHY_CTRL_COMMON,
+					    FSEL_MASK,
+					    FIELD_PREP(FSEL_MASK, FSEL_24_MHZ_VAL));
+
+		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG0,
+					    PHY_CFG_PLL_FB_DIV_19_8_MASK,
+					    FIELD_PREP(PHY_CFG_PLL_FB_DIV_19_8_MASK,
+						       DIV_19_8_24_MHZ_VAL));
+
+		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG1,
+					    EXYNOS_PHY_CFG_PLL_FB_DIV_11_8_MASK,
+					    EXYNOS_DIV_11_8_24_MHZ_VAL);
+		break;
+
+	case 26000000:
+		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_HS_PHY_CTRL_COMMON,
+					    FSEL_MASK,
+					    FIELD_PREP(FSEL_MASK, FSEL_26_MHZ_VAL));
+
+		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG0,
+					    PHY_CFG_PLL_FB_DIV_19_8_MASK,
+					    FIELD_PREP(PHY_CFG_PLL_FB_DIV_19_8_MASK,
+						       DIV_19_8_26_MHZ_VAL));
+
+		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG1,
+					    EXYNOS_PHY_CFG_PLL_FB_DIV_11_8_MASK,
+					    EXYNOS_DIV_11_8_26_MHZ_VAL);
+		break;
+
+	case 48000000:
+		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_HS_PHY_CTRL_COMMON,
+					    FSEL_MASK,
+					    FIELD_PREP(FSEL_MASK, FSEL_48_MHZ_VAL));
+
+		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG0,
+					    PHY_CFG_PLL_FB_DIV_19_8_MASK,
+					    FIELD_PREP(PHY_CFG_PLL_FB_DIV_19_8_MASK,
+						       DIV_19_8_48_MHZ_VAL));
+
+		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG1,
+					    EXYNOS_PHY_CFG_PLL_FB_DIV_11_8_MASK,
+					    EXYNOS_DIV_11_8_48_MHZ_VAL);
+		break;
+	default:
+		dev_err(&phy->phy->dev, "unsupported ref_clk_freq:%lu\n", ref_clk_freq);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int qcom_eusb2_ref_clk_init(struct snps_eusb2_hsphy *phy)
 {
 	unsigned long ref_clk_freq = clk_get_rate(phy->ref_clk);
@@ -240,6 +360,55 @@ static int qcom_eusb2_ref_clk_init(struct snps_eusb2_hsphy *phy)
 	return 0;
 }
 
+static int exynos_snps_eusb2_hsphy_init(struct phy *p)
+{
+	struct snps_eusb2_hsphy *phy = phy_get_drvdata(p);
+	int ret;
+
+	snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_HS_PHY_CTRL_RST,
+				    USB_PHY_RST_MASK | UTMI_PORT_RST_MASK,
+				    USB_PHY_RST_MASK | UTMI_PORT_RST_MASK);
+	fsleep(50); /* required after holding phy in reset */
+
+	snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_HS_PHY_CTRL_COMMON,
+				    RPTR_MODE, RPTR_MODE);
+
+	/* update ref_clk related registers */
+	ret = exynos_eusb2_ref_clk_init(phy);
+	if (ret)
+		return ret;
+
+	/* default parameter: tx fsls-vref */
+	snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_PHY_CFG_TX,
+				    EXYNOS_PHY_CFG_TX_FSLS_VREF_TUNE_MASK,
+				    FIELD_PREP(EXYNOS_PHY_CFG_TX_FSLS_VREF_TUNE_MASK, 0x0));
+
+	snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_UTMI_TESTSE,
+				    TEST_IDDQ, 0);
+	fsleep(10); /* required after releasing test_iddq */
+
+	snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_HS_PHY_CTRL_RST,
+				    USB_PHY_RST_MASK, 0);
+
+	snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_HS_PHY_CTRL_COMMON,
+				    PHY_ENABLE, PHY_ENABLE);
+
+	snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_HS_PHY_CTRL_RST,
+				    UTMI_PORT_RST_MASK, 0);
+
+	return 0;
+}
+
+static const char * const exynos_eusb2_hsphy_clock_names[] = {
+	"ref", "bus", "ctrl",
+};
+
+static const struct snps_eusb2_phy_drvdata exynos2200_snps_eusb2_phy = {
+	.phy_init	= exynos_snps_eusb2_hsphy_init,
+	.clk_names	= exynos_eusb2_hsphy_clock_names,
+	.num_clks	= ARRAY_SIZE(exynos_eusb2_hsphy_clock_names),
+};
+
 static int qcom_snps_eusb2_hsphy_init(struct phy *p)
 {
 	struct snps_eusb2_hsphy *phy = phy_get_drvdata(p);
@@ -488,6 +657,9 @@ static const struct of_device_id snps_eusb2_hsphy_of_match_table[] = {
 	{
 		.compatible = "qcom,sm8550-snps-eusb2-phy",
 		.data = &sm8550_snps_eusb2_phy,
+	}, {
+		.compatible = "samsung,exynos2200-snps-eusb2-phy",
+		.data = &exynos2200_snps_eusb2_phy,
 	}, { },
 };
 MODULE_DEVICE_TABLE(of, snps_eusb2_hsphy_of_match_table);
-- 
2.43.0

Re: [PATCH v2 7/8] phy: phy-snps-eusb2: add support for exynos2200

[Dmitry Baryshkov]

On Sun, Feb 23, 2025 at 02:22:26PM +0200, Ivaylo Ivanov wrote:
> The Exynos2200 SoC reuses the Synopsis eUSB2 PHY IP, alongside an
> external repeater, for USB 2.0. Add support for it to the existing
> driver.
> 
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> ---
>  drivers/phy/Kconfig          |   2 +-
>  drivers/phy/phy-snps-eusb2.c | 172 +++++++++++++++++++++++++++++++++++
>  2 files changed, 173 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 11c166204..58c911e1b 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -45,7 +45,7 @@ config PHY_PISTACHIO_USB
>  
>  config PHY_SNPS_EUSB2
>  	tristate "SNPS eUSB2 PHY Driver"
> -	depends on OF && (ARCH_QCOM || COMPILE_TEST)
> +	depends on OF && (ARCH_EXYNOS || ARCH_QCOM || COMPILE_TEST)
>  	select GENERIC_PHY
>  	help
>  	  Enable support for the USB high-speed SNPS eUSB2 phy on select
> diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
> index 7a242fe32..67a19d671 100644
> --- a/drivers/phy/phy-snps-eusb2.c
> +++ b/drivers/phy/phy-snps-eusb2.c
> @@ -13,6 +13,39 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/reset.h>
>  
> +#define EXYNOS_USB_PHY_HS_PHY_CTRL_RST	(0x0)
> +#define USB_PHY_RST_MASK		GENMASK(1, 0)
> +#define UTMI_PORT_RST_MASK		GENMASK(5, 4)
> +
> +#define EXYNOS_USB_PHY_HS_PHY_CTRL_COMMON	(0x4)
> +#define RPTR_MODE			BIT(10)
> +#define FSEL_20_MHZ_VAL			(0x1)
> +#define FSEL_24_MHZ_VAL			(0x2)
> +#define FSEL_26_MHZ_VAL			(0x3)
> +#define FSEL_48_MHZ_VAL			(0x2)
> +
> +#define EXYNOS_USB_PHY_CFG_PLLCFG0	(0x8)
> +#define PHY_CFG_PLL_FB_DIV_19_8_MASK	GENMASK(19, 8)
> +#define DIV_19_8_19_2_MHZ_VAL		(0x170)
> +#define DIV_19_8_20_MHZ_VAL		(0x160)
> +#define DIV_19_8_24_MHZ_VAL		(0x120)
> +#define DIV_19_8_26_MHZ_VAL		(0x107)
> +#define DIV_19_8_48_MHZ_VAL		(0x120)
> +
> +#define EXYNOS_USB_PHY_CFG_PLLCFG1	(0xc)
> +#define EXYNOS_PHY_CFG_PLL_FB_DIV_11_8_MASK	GENMASK(11, 8)
> +#define EXYNOS_DIV_11_8_19_2_MHZ_VAL	(0x0)
> +#define EXYNOS_DIV_11_8_20_MHZ_VAL	(0x0)
> +#define EXYNOS_DIV_11_8_24_MHZ_VAL	(0x0)
> +#define EXYNOS_DIV_11_8_26_MHZ_VAL	(0x0)
> +#define EXYNOS_DIV_11_8_48_MHZ_VAL	(0x1)
> +
> +#define EXYNOS_PHY_CFG_TX		(0x14)
> +#define EXYNOS_PHY_CFG_TX_FSLS_VREF_TUNE_MASK	GENMASK(2, 1)
> +
> +#define EXYNOS_USB_PHY_UTMI_TESTSE	(0x20)
> +#define TEST_IDDQ			BIT(6)
> +
>  #define QCOM_USB_PHY_UTMI_CTRL0		(0x3c)
>  #define SLEEPM				BIT(0)
>  #define OPMODE_MASK			GENMASK(4, 3)
> @@ -196,6 +229,93 @@ static void qcom_eusb2_default_parameters(struct snps_eusb2_hsphy *phy)
>  				    FIELD_PREP(PHY_CFG_TX_HS_XV_TUNE_MASK, 0x0));
>  }
>  
> +static int exynos_eusb2_ref_clk_init(struct snps_eusb2_hsphy *phy)
> +{
> +	unsigned long ref_clk_freq = clk_get_rate(phy->ref_clk);
> +
> +	switch (ref_clk_freq) {
> +	case 19200000:
> +		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_HS_PHY_CTRL_COMMON,
> +					    FSEL_MASK,
> +					    FIELD_PREP(FSEL_MASK, FSEL_19_2_MHZ_VAL));
> +

Could you please unify the switchcase? assign the values to temp
variables, then program them from a single code stream. Or maybe even
replace switch-case with a table-based lookup.

(we probably should implement the similar change for qcom part. Maybe
you can refactor it too?)

Other than that LGTM.

> +		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG0,
> +					    PHY_CFG_PLL_FB_DIV_19_8_MASK,
> +					    FIELD_PREP(PHY_CFG_PLL_FB_DIV_19_8_MASK,
> +						       DIV_19_8_19_2_MHZ_VAL));
> +
> +		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG1,
> +					    EXYNOS_PHY_CFG_PLL_FB_DIV_11_8_MASK,
> +					    EXYNOS_DIV_11_8_19_2_MHZ_VAL);
> +		break;
> +
> +	case 20000000:
> +		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_HS_PHY_CTRL_COMMON,
> +					    FSEL_MASK,
> +					    FIELD_PREP(FSEL_MASK, FSEL_20_MHZ_VAL));
> +
> +		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG0,
> +					    PHY_CFG_PLL_FB_DIV_19_8_MASK,
> +					    FIELD_PREP(PHY_CFG_PLL_FB_DIV_19_8_MASK,
> +						       DIV_19_8_20_MHZ_VAL));
> +
> +		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG1,
> +					    EXYNOS_PHY_CFG_PLL_FB_DIV_11_8_MASK,
> +					    EXYNOS_DIV_11_8_20_MHZ_VAL);
> +		break;
> +

-- 
With best wishes
Dmitry

Re: [PATCH v2 7/8] phy: phy-snps-eusb2: add support for exynos2200

[Ivaylo Ivanov]

On 2/24/25 01:51, Dmitry Baryshkov wrote:
> On Sun, Feb 23, 2025 at 02:22:26PM +0200, Ivaylo Ivanov wrote:
>> The Exynos2200 SoC reuses the Synopsis eUSB2 PHY IP, alongside an
>> external repeater, for USB 2.0. Add support for it to the existing
>> driver.
>>
>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
>> ---
>>  drivers/phy/Kconfig          |   2 +-
>>  drivers/phy/phy-snps-eusb2.c | 172 +++++++++++++++++++++++++++++++++++
>>  2 files changed, 173 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index 11c166204..58c911e1b 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -45,7 +45,7 @@ config PHY_PISTACHIO_USB
>>  
>>  config PHY_SNPS_EUSB2
>>  	tristate "SNPS eUSB2 PHY Driver"
>> -	depends on OF && (ARCH_QCOM || COMPILE_TEST)
>> +	depends on OF && (ARCH_EXYNOS || ARCH_QCOM || COMPILE_TEST)
>>  	select GENERIC_PHY
>>  	help
>>  	  Enable support for the USB high-speed SNPS eUSB2 phy on select
>> diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
>> index 7a242fe32..67a19d671 100644
>> --- a/drivers/phy/phy-snps-eusb2.c
>> +++ b/drivers/phy/phy-snps-eusb2.c
>> @@ -13,6 +13,39 @@
>>  #include <linux/regulator/consumer.h>
>>  #include <linux/reset.h>
>>  
>> +#define EXYNOS_USB_PHY_HS_PHY_CTRL_RST	(0x0)
>> +#define USB_PHY_RST_MASK		GENMASK(1, 0)
>> +#define UTMI_PORT_RST_MASK		GENMASK(5, 4)
>> +
>> +#define EXYNOS_USB_PHY_HS_PHY_CTRL_COMMON	(0x4)
>> +#define RPTR_MODE			BIT(10)
>> +#define FSEL_20_MHZ_VAL			(0x1)
>> +#define FSEL_24_MHZ_VAL			(0x2)
>> +#define FSEL_26_MHZ_VAL			(0x3)
>> +#define FSEL_48_MHZ_VAL			(0x2)
>> +
>> +#define EXYNOS_USB_PHY_CFG_PLLCFG0	(0x8)
>> +#define PHY_CFG_PLL_FB_DIV_19_8_MASK	GENMASK(19, 8)
>> +#define DIV_19_8_19_2_MHZ_VAL		(0x170)
>> +#define DIV_19_8_20_MHZ_VAL		(0x160)
>> +#define DIV_19_8_24_MHZ_VAL		(0x120)
>> +#define DIV_19_8_26_MHZ_VAL		(0x107)
>> +#define DIV_19_8_48_MHZ_VAL		(0x120)
>> +
>> +#define EXYNOS_USB_PHY_CFG_PLLCFG1	(0xc)
>> +#define EXYNOS_PHY_CFG_PLL_FB_DIV_11_8_MASK	GENMASK(11, 8)
>> +#define EXYNOS_DIV_11_8_19_2_MHZ_VAL	(0x0)
>> +#define EXYNOS_DIV_11_8_20_MHZ_VAL	(0x0)
>> +#define EXYNOS_DIV_11_8_24_MHZ_VAL	(0x0)
>> +#define EXYNOS_DIV_11_8_26_MHZ_VAL	(0x0)
>> +#define EXYNOS_DIV_11_8_48_MHZ_VAL	(0x1)
>> +
>> +#define EXYNOS_PHY_CFG_TX		(0x14)
>> +#define EXYNOS_PHY_CFG_TX_FSLS_VREF_TUNE_MASK	GENMASK(2, 1)
>> +
>> +#define EXYNOS_USB_PHY_UTMI_TESTSE	(0x20)
>> +#define TEST_IDDQ			BIT(6)
>> +
>>  #define QCOM_USB_PHY_UTMI_CTRL0		(0x3c)
>>  #define SLEEPM				BIT(0)
>>  #define OPMODE_MASK			GENMASK(4, 3)
>> @@ -196,6 +229,93 @@ static void qcom_eusb2_default_parameters(struct snps_eusb2_hsphy *phy)
>>  				    FIELD_PREP(PHY_CFG_TX_HS_XV_TUNE_MASK, 0x0));
>>  }
>>  
>> +static int exynos_eusb2_ref_clk_init(struct snps_eusb2_hsphy *phy)
>> +{
>> +	unsigned long ref_clk_freq = clk_get_rate(phy->ref_clk);
>> +
>> +	switch (ref_clk_freq) {
>> +	case 19200000:
>> +		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_HS_PHY_CTRL_COMMON,
>> +					    FSEL_MASK,
>> +					    FIELD_PREP(FSEL_MASK, FSEL_19_2_MHZ_VAL));
>> +
> Could you please unify the switchcase? assign the values to temp
> variables, then program them from a single code stream. Or maybe even
> replace switch-case with a table-based lookup.
>
> (we probably should implement the similar change for qcom part. Maybe
> you can refactor it too?)

Alright. I'll do it for the Qualcomm part too in a separate commit.

Thanks for the feedback!

Best regards,
Ivaylo

> Other than that LGTM.
>
>> +		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG0,
>> +					    PHY_CFG_PLL_FB_DIV_19_8_MASK,
>> +					    FIELD_PREP(PHY_CFG_PLL_FB_DIV_19_8_MASK,
>> +						       DIV_19_8_19_2_MHZ_VAL));
>> +
>> +		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG1,
>> +					    EXYNOS_PHY_CFG_PLL_FB_DIV_11_8_MASK,
>> +					    EXYNOS_DIV_11_8_19_2_MHZ_VAL);
>> +		break;
>> +
>> +	case 20000000:
>> +		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_HS_PHY_CTRL_COMMON,
>> +					    FSEL_MASK,
>> +					    FIELD_PREP(FSEL_MASK, FSEL_20_MHZ_VAL));
>> +
>> +		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG0,
>> +					    PHY_CFG_PLL_FB_DIV_19_8_MASK,
>> +					    FIELD_PREP(PHY_CFG_PLL_FB_DIV_19_8_MASK,
>> +						       DIV_19_8_20_MHZ_VAL));
>> +
>> +		snps_eusb2_hsphy_write_mask(phy->base, EXYNOS_USB_PHY_CFG_PLLCFG1,
>> +					    EXYNOS_PHY_CFG_PLL_FB_DIV_11_8_MASK,
>> +					    EXYNOS_DIV_11_8_20_MHZ_VAL);
>> +		break;
>> +

[PATCH v2 8/8] phy: samsung: add Exynos2200 usb phy controller

[Ivaylo Ivanov]

The Exynos2200 SoC comes with a TI external repeater and 3 USB PHYs:
- snps eUSB2 for UTMI
- snps USBDP combophy for PIPE3 and DP
- samsung USBCON phy

The USBCON phy is an intermediary between the USB controller (DWC3)
and the underlying PHYs. Add a new driver for it, modelled to take
a phandle to the high-speed PHY for now.

Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
---
 drivers/phy/samsung/Kconfig                 |  12 +
 drivers/phy/samsung/Makefile                |   1 +
 drivers/phy/samsung/phy-exynos2200-usbcon.c | 250 ++++++++++++++++++++
 3 files changed, 263 insertions(+)
 create mode 100644 drivers/phy/samsung/phy-exynos2200-usbcon.c

diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig
index e2330b089..0f809a382 100644
--- a/drivers/phy/samsung/Kconfig
+++ b/drivers/phy/samsung/Kconfig
@@ -77,6 +77,18 @@ config PHY_S5PV210_USB2
 	  particular SoC is compiled in the driver. In case of S5PV210 two phys
 	  are available - device and host.
 
+config PHY_EXYNOS2200_USBCON
+	tristate "Exynos2200 USBCON PHY driver"
+	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
+	depends on HAS_IOMEM
+	select GENERIC_PHY
+	select MFD_SYSCON
+	default y
+	help
+	  Enable USBCON PHY support for Exynos2200 SoC.
+	  This driver provides PHY interface for the USBCON phy, which acts as
+	  an intermediary between the USB controller and underlying PHYs.
+
 config PHY_EXYNOS5_USBDRD
 	tristate "Exynos5 SoC series USB DRD PHY driver"
 	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
diff --git a/drivers/phy/samsung/Makefile b/drivers/phy/samsung/Makefile
index fea1f96d0..e2686a3f5 100644
--- a/drivers/phy/samsung/Makefile
+++ b/drivers/phy/samsung/Makefile
@@ -14,5 +14,6 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2)	+= phy-exynos4210-usb2.o
 phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
 phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
 phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)	+= phy-s5pv210-usb2.o
+obj-$(CONFIG_PHY_EXYNOS2200_USBCON)	+= phy-exynos2200-usbcon.o
 obj-$(CONFIG_PHY_EXYNOS5_USBDRD)	+= phy-exynos5-usbdrd.o
 obj-$(CONFIG_PHY_EXYNOS5250_SATA)	+= phy-exynos5250-sata.o
diff --git a/drivers/phy/samsung/phy-exynos2200-usbcon.c b/drivers/phy/samsung/phy-exynos2200-usbcon.c
new file mode 100644
index 000000000..aad59349e
--- /dev/null
+++ b/drivers/phy/samsung/phy-exynos2200-usbcon.c
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mod_devicetable.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/soc/samsung/exynos-regs-pmu.h>
+
+#define EXYNOS2200_USBCON_LINKCTRL		0x4
+#define LINKCTRL_FORCE_QACT			BIT(8)
+
+#define EXYNOS2200_USBCON_UTMI_CTRL		0x10
+#define UTMI_CTRL_FORCESLEEP			BIT(13)
+#define UTMI_CTRL_FORCESUSPEND			BIT(12)
+#define UTMI_CTRL_FORCE_VBUSVALID		BIT(1)
+#define UTMI_CTRL_FORCE_BVALID			BIT(0)
+
+#define EXYNOS2200_USBCON_LINK_CLKRST		0xc
+#define LINK_CLKRST_SW_RST			BIT(0)
+
+struct exynos2200_usbcon_phy_drvdata {
+	const char * const *clk_names;
+	int num_clks;
+};
+
+struct exynos2200_usbcon_phy {
+	struct phy *phy;
+	void __iomem *base;
+	struct regmap *reg_pmu;
+	struct clk_bulk_data *clks;
+	const struct exynos2200_usbcon_phy_drvdata *drv_data;
+	u32 pmu_offset;
+	struct phy *hs_phy;
+};
+
+static void exynos2200_usbcon_phy_isol(struct exynos2200_usbcon_phy *inst,
+				       bool isolate)
+{
+	unsigned int val;
+
+	if (!inst->reg_pmu)
+		return;
+
+	val = isolate ? 0 : EXYNOS4_PHY_ENABLE;
+
+	regmap_update_bits(inst->reg_pmu, inst->pmu_offset,
+			   EXYNOS4_PHY_ENABLE, val);
+}
+
+static void exynos2200_usbcon_phy_write_mask(void __iomem *base, u32 offset,
+					     u32 mask, u32 val)
+{
+	u32 reg;
+
+	reg = readl(base + offset);
+	reg &= ~mask;
+	reg |= val & mask;
+	writel(reg, base + offset);
+
+	/* Ensure above write is completed */
+	readl(base + offset);
+}
+
+static int exynos2200_usbcon_phy_init(struct phy *p)
+{
+	int ret;
+	struct exynos2200_usbcon_phy *phy = phy_get_drvdata(p);
+
+	/* Power-on PHY ... */
+	ret = clk_bulk_prepare_enable(phy->drv_data->num_clks, phy->clks);
+	if (ret)
+		return ret;
+
+	/*
+	 * ... and ungate power via PMU. Without this here, we can't access
+	 * registers
+	 */
+	exynos2200_usbcon_phy_isol(phy, false);
+
+	/*
+	 * Disable HWACG (hardware auto clock gating control). This will force
+	 * QACTIVE signal in Q-Channel interface to HIGH level, to make sure
+	 * the PHY clock is not gated by the hardware.
+	 */
+	exynos2200_usbcon_phy_write_mask(phy->base, EXYNOS2200_USBCON_LINKCTRL,
+					 LINKCTRL_FORCE_QACT,
+					 LINKCTRL_FORCE_QACT);
+
+	/* Reset Link */
+	exynos2200_usbcon_phy_write_mask(phy->base,
+					 EXYNOS2200_USBCON_LINK_CLKRST,
+					 LINK_CLKRST_SW_RST,
+					 LINK_CLKRST_SW_RST);
+
+	fsleep(10); /* required after POR high */
+	exynos2200_usbcon_phy_write_mask(phy->base,
+					 EXYNOS2200_USBCON_LINK_CLKRST,
+					 LINK_CLKRST_SW_RST, 0);
+
+	exynos2200_usbcon_phy_write_mask(phy->base,
+					 EXYNOS2200_USBCON_UTMI_CTRL,
+					 UTMI_CTRL_FORCESLEEP |
+					 UTMI_CTRL_FORCESUSPEND,
+					 0);
+
+	exynos2200_usbcon_phy_write_mask(phy->base,
+					 EXYNOS2200_USBCON_UTMI_CTRL,
+					 UTMI_CTRL_FORCE_BVALID |
+					 UTMI_CTRL_FORCE_VBUSVALID,
+					 UTMI_CTRL_FORCE_BVALID |
+					 UTMI_CTRL_FORCE_VBUSVALID);
+
+	return phy_init(phy->hs_phy);
+}
+
+static int exynos2200_usbcon_phy_exit(struct phy *p)
+{
+	struct exynos2200_usbcon_phy *phy = phy_get_drvdata(p);
+	int ret;
+
+	ret = phy_exit(phy->hs_phy);
+	if (ret)
+		return ret;
+
+	exynos2200_usbcon_phy_write_mask(phy->base,
+					 EXYNOS2200_USBCON_UTMI_CTRL,
+					 UTMI_CTRL_FORCESLEEP |
+					 UTMI_CTRL_FORCESUSPEND,
+					 UTMI_CTRL_FORCESLEEP |
+					 UTMI_CTRL_FORCESUSPEND);
+
+	exynos2200_usbcon_phy_write_mask(phy->base,
+					 EXYNOS2200_USBCON_LINK_CLKRST,
+					 LINK_CLKRST_SW_RST,
+					 LINK_CLKRST_SW_RST);
+
+	/* Gate power via PMU */
+	exynos2200_usbcon_phy_isol(phy, true);
+
+	clk_bulk_disable_unprepare(phy->drv_data->num_clks, phy->clks);
+
+	return 0;
+}
+
+static const struct phy_ops exynos2200_usbcon_phy_ops = {
+	.init		= exynos2200_usbcon_phy_init,
+	.exit		= exynos2200_usbcon_phy_exit,
+	.owner		= THIS_MODULE,
+};
+
+static int exynos2200_usbcon_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct exynos2200_usbcon_phy *phy;
+	const struct exynos2200_usbcon_phy_drvdata *drv_data;
+	struct phy_provider *phy_provider;
+	struct phy *generic_phy;
+	int ret;
+
+	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
+	if (!phy)
+		return -ENOMEM;
+
+	drv_data = of_device_get_match_data(dev);
+	if (!drv_data)
+		return -EINVAL;
+	phy->drv_data = drv_data;
+
+	phy->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(phy->base))
+		return PTR_ERR(phy->base);
+
+	phy->clks = devm_kcalloc(dev, drv_data->num_clks,
+				 sizeof(*phy->clks), GFP_KERNEL);
+	if (!phy->clks)
+		return -ENOMEM;
+
+	for (int i = 0; i < drv_data->num_clks; ++i)
+		phy->clks[i].id = drv_data->clk_names[i];
+
+	ret = devm_clk_bulk_get(dev, phy->drv_data->num_clks,
+				phy->clks);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to get phy clock(s)\n");
+
+	phy->reg_pmu = syscon_regmap_lookup_by_phandle_args(dev->of_node,
+							    "samsung,pmu-syscon",
+							    1, &phy->pmu_offset);
+	if (IS_ERR(phy->reg_pmu)) {
+		dev_err(dev, "Failed to lookup PMU regmap\n");
+		return PTR_ERR(phy->reg_pmu);
+	}
+
+	phy->hs_phy = devm_of_phy_get_by_index(dev, dev->of_node, 0);
+	if (IS_ERR(phy->hs_phy))
+		return dev_err_probe(dev, PTR_ERR(phy->hs_phy),
+				     "failed to get hs_phy\n");
+
+	generic_phy = devm_phy_create(dev, NULL, &exynos2200_usbcon_phy_ops);
+	if (IS_ERR(generic_phy))
+		return dev_err_probe(dev, PTR_ERR(generic_phy),
+				     "failed to create phy %d\n", ret);
+
+	dev_set_drvdata(dev, phy);
+	phy_set_drvdata(generic_phy, phy);
+
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return dev_err_probe(dev, PTR_ERR(phy_provider),
+				     "failed to register phy provider\n");
+
+	return 0;
+}
+
+static const char * const exynos2200_clk_names[] = {
+	"bus",
+};
+
+static const struct exynos2200_usbcon_phy_drvdata exynos2200_usbcon_phy = {
+	.clk_names		= exynos2200_clk_names,
+	.num_clks		= ARRAY_SIZE(exynos2200_clk_names),
+};
+
+static const struct of_device_id exynos2200_usbcon_phy_of_match_table[] = {
+	{
+		.compatible = "samsung,exynos2200-usbcon-phy",
+		.data = &exynos2200_usbcon_phy,
+	}, { },
+};
+MODULE_DEVICE_TABLE(of, exynos2200_usbcon_phy_of_match_table);
+
+static struct platform_driver exynos2200_usbcon_phy_driver = {
+	.probe		= exynos2200_usbcon_phy_probe,
+	.driver = {
+		.name	= "exynos2200-usbcon-phy",
+		.of_match_table = exynos2200_usbcon_phy_of_match_table,
+	},
+};
+
+module_platform_driver(exynos2200_usbcon_phy_driver);
+MODULE_DESCRIPTION("Exynos2200 USBCON PHY driver");
+MODULE_LICENSE("GPL");
-- 
2.43.0

Re: [PATCH v2 8/8] phy: samsung: add Exynos2200 usb phy controller

[Dmitry Baryshkov]

On Sun, Feb 23, 2025 at 02:22:27PM +0200, Ivaylo Ivanov wrote:
> The Exynos2200 SoC comes with a TI external repeater and 3 USB PHYs:
> - snps eUSB2 for UTMI
> - snps USBDP combophy for PIPE3 and DP
> - samsung USBCON phy
> 
> The USBCON phy is an intermediary between the USB controller (DWC3)
> and the underlying PHYs. Add a new driver for it, modelled to take
> a phandle to the high-speed PHY for now.
> 
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> ---
>  drivers/phy/samsung/Kconfig                 |  12 +
>  drivers/phy/samsung/Makefile                |   1 +
>  drivers/phy/samsung/phy-exynos2200-usbcon.c | 250 ++++++++++++++++++++
>  3 files changed, 263 insertions(+)
>  create mode 100644 drivers/phy/samsung/phy-exynos2200-usbcon.c
> 
> diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig
> index e2330b089..0f809a382 100644
> --- a/drivers/phy/samsung/Kconfig
> +++ b/drivers/phy/samsung/Kconfig
> @@ -77,6 +77,18 @@ config PHY_S5PV210_USB2
>  	  particular SoC is compiled in the driver. In case of S5PV210 two phys
>  	  are available - device and host.
>  
> +config PHY_EXYNOS2200_USBCON
> +	tristate "Exynos2200 USBCON PHY driver"
> +	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
> +	depends on HAS_IOMEM
> +	select GENERIC_PHY
> +	select MFD_SYSCON
> +	default y
> +	help
> +	  Enable USBCON PHY support for Exynos2200 SoC.
> +	  This driver provides PHY interface for the USBCON phy, which acts as
> +	  an intermediary between the USB controller and underlying PHYs.
> +
>  config PHY_EXYNOS5_USBDRD
>  	tristate "Exynos5 SoC series USB DRD PHY driver"
>  	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
> diff --git a/drivers/phy/samsung/Makefile b/drivers/phy/samsung/Makefile
> index fea1f96d0..e2686a3f5 100644
> --- a/drivers/phy/samsung/Makefile
> +++ b/drivers/phy/samsung/Makefile
> @@ -14,5 +14,6 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2)	+= phy-exynos4210-usb2.o
>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
>  phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)	+= phy-s5pv210-usb2.o
> +obj-$(CONFIG_PHY_EXYNOS2200_USBCON)	+= phy-exynos2200-usbcon.o
>  obj-$(CONFIG_PHY_EXYNOS5_USBDRD)	+= phy-exynos5-usbdrd.o
>  obj-$(CONFIG_PHY_EXYNOS5250_SATA)	+= phy-exynos5250-sata.o
> diff --git a/drivers/phy/samsung/phy-exynos2200-usbcon.c b/drivers/phy/samsung/phy-exynos2200-usbcon.c
> new file mode 100644
> index 000000000..aad59349e
> --- /dev/null
> +++ b/drivers/phy/samsung/phy-exynos2200-usbcon.c
> @@ -0,0 +1,250 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025, Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/soc/samsung/exynos-regs-pmu.h>
> +
> +#define EXYNOS2200_USBCON_LINKCTRL		0x4
> +#define LINKCTRL_FORCE_QACT			BIT(8)
> +
> +#define EXYNOS2200_USBCON_UTMI_CTRL		0x10
> +#define UTMI_CTRL_FORCESLEEP			BIT(13)
> +#define UTMI_CTRL_FORCESUSPEND			BIT(12)
> +#define UTMI_CTRL_FORCE_VBUSVALID		BIT(1)
> +#define UTMI_CTRL_FORCE_BVALID			BIT(0)
> +
> +#define EXYNOS2200_USBCON_LINK_CLKRST		0xc
> +#define LINK_CLKRST_SW_RST			BIT(0)
> +
> +struct exynos2200_usbcon_phy_drvdata {
> +	const char * const *clk_names;
> +	int num_clks;
> +};
> +
> +struct exynos2200_usbcon_phy {
> +	struct phy *phy;
> +	void __iomem *base;
> +	struct regmap *reg_pmu;
> +	struct clk_bulk_data *clks;
> +	const struct exynos2200_usbcon_phy_drvdata *drv_data;
> +	u32 pmu_offset;
> +	struct phy *hs_phy;
> +};
> +
> +static void exynos2200_usbcon_phy_isol(struct exynos2200_usbcon_phy *inst,
> +				       bool isolate)
> +{
> +	unsigned int val;
> +
> +	if (!inst->reg_pmu)
> +		return;
> +
> +	val = isolate ? 0 : EXYNOS4_PHY_ENABLE;
> +
> +	regmap_update_bits(inst->reg_pmu, inst->pmu_offset,
> +			   EXYNOS4_PHY_ENABLE, val);
> +}
> +
> +static void exynos2200_usbcon_phy_write_mask(void __iomem *base, u32 offset,
> +					     u32 mask, u32 val)
> +{
> +	u32 reg;
> +
> +	reg = readl(base + offset);
> +	reg &= ~mask;
> +	reg |= val & mask;
> +	writel(reg, base + offset);

Does this need any kind of locking? Maybe you can use regmap for this
access too?

> +
> +	/* Ensure above write is completed */
> +	readl(base + offset);
> +}

[...]

> +
> +	generic_phy = devm_phy_create(dev, NULL, &exynos2200_usbcon_phy_ops);
> +	if (IS_ERR(generic_phy))
> +		return dev_err_probe(dev, PTR_ERR(generic_phy),
> +				     "failed to create phy %d\n", ret);
> +
> +	dev_set_drvdata(dev, phy);

Nit: unused. LGTM otherwise.

> +	phy_set_drvdata(generic_phy, phy);
> +
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	if (IS_ERR(phy_provider))
> +		return dev_err_probe(dev, PTR_ERR(phy_provider),
> +				     "failed to register phy provider\n");
> +
> +	return 0;
> +}
> +
> +static const char * const exynos2200_clk_names[] = {
> +	"bus",
> +};
> +
> +static const struct exynos2200_usbcon_phy_drvdata exynos2200_usbcon_phy = {
> +	.clk_names		= exynos2200_clk_names,
> +	.num_clks		= ARRAY_SIZE(exynos2200_clk_names),
> +};
> +
> +static const struct of_device_id exynos2200_usbcon_phy_of_match_table[] = {
> +	{
> +		.compatible = "samsung,exynos2200-usbcon-phy",
> +		.data = &exynos2200_usbcon_phy,
> +	}, { },
> +};
> +MODULE_DEVICE_TABLE(of, exynos2200_usbcon_phy_of_match_table);
> +
> +static struct platform_driver exynos2200_usbcon_phy_driver = {
> +	.probe		= exynos2200_usbcon_phy_probe,
> +	.driver = {
> +		.name	= "exynos2200-usbcon-phy",
> +		.of_match_table = exynos2200_usbcon_phy_of_match_table,
> +	},
> +};
> +
> +module_platform_driver(exynos2200_usbcon_phy_driver);
> +MODULE_DESCRIPTION("Exynos2200 USBCON PHY driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry