[PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support

[PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support

[Alexander Stein]

This patchset aims to support flags for e.g. over-current active low or port
permanantly attached which are provided in the USB3 glue layer.

There is already a glue layer driver dwc3-imx8mp, but unfortunately this driver
does not use the glue area at all, it only handles wakeup-support which is
done in the HSIO BLK_CTRL area (0x32f10100), accordingly the driver only uses
the hsio clock.

The driver which actually uses the USB3 glue area is phy-fsl-imx8mq-usb. As the
name indicates PHY is configured in the corresponding registers, which are part
of the USB3 glue layer.

This make is it unclear for me which driver should handle the required features
above.
dwc3-imx8mp, the glue layer driver, does not touch the glue area at all,
but the HSIO BLK_CTRL area.
phy-fsl-imx8mq-usb only touches the PHY registers in the glue layer.
Neither does map the USB3 control register from the glue layer.

Thanks for any feedback and best regards,
Alexander

Alexander Stein (3):
  dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags
  phy: fsl-imx8mq-usb: Add support for setting fsl specific flags
  arm64: dts: imx8mp: Add memory for USB3 glue layer to usb3_phy nodes

 .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 52 +++++++++++++++-
 arch/arm64/boot/dts/freescale/imx8mp.dtsi     |  6 +-
 drivers/phy/freescale/phy-fsl-imx8mq-usb.c    | 61 +++++++++++++++++++
 3 files changed, 116 insertions(+), 3 deletions(-)

-- 
2.25.1

[PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support

[Alexander Stein]

This patchset aims to support flags for e.g. over-current active low or port
permanantly attached which are provided in the USB3 glue layer.

There is already a glue layer driver dwc3-imx8mp, but unfortunately this driver
does not use the glue area at all, it only handles wakeup-support which is
done in the HSIO BLK_CTRL area (0x32f10100), accordingly the driver only uses
the hsio clock.

The driver which actually uses the USB3 glue area is phy-fsl-imx8mq-usb. As the
name indicates PHY is configured in the corresponding registers, which are part
of the USB3 glue layer.

This make is it unclear for me which driver should handle the required features
above.
dwc3-imx8mp, the glue layer driver, does not touch the glue area at all,
but the HSIO BLK_CTRL area.
phy-fsl-imx8mq-usb only touches the PHY registers in the glue layer.
Neither does map the USB3 control register from the glue layer.

Thanks for any feedback and best regards,
Alexander

Alexander Stein (3):
  dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags
  phy: fsl-imx8mq-usb: Add support for setting fsl specific flags
  arm64: dts: imx8mp: Add memory for USB3 glue layer to usb3_phy nodes

 .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 52 +++++++++++++++-
 arch/arm64/boot/dts/freescale/imx8mp.dtsi     |  6 +-
 drivers/phy/freescale/phy-fsl-imx8mq-usb.c    | 61 +++++++++++++++++++
 3 files changed, 116 insertions(+), 3 deletions(-)

-- 
2.25.1


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

[PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support

[Alexander Stein]

This patchset aims to support flags for e.g. over-current active low or port
permanantly attached which are provided in the USB3 glue layer.

There is already a glue layer driver dwc3-imx8mp, but unfortunately this driver
does not use the glue area at all, it only handles wakeup-support which is
done in the HSIO BLK_CTRL area (0x32f10100), accordingly the driver only uses
the hsio clock.

The driver which actually uses the USB3 glue area is phy-fsl-imx8mq-usb. As the
name indicates PHY is configured in the corresponding registers, which are part
of the USB3 glue layer.

This make is it unclear for me which driver should handle the required features
above.
dwc3-imx8mp, the glue layer driver, does not touch the glue area at all,
but the HSIO BLK_CTRL area.
phy-fsl-imx8mq-usb only touches the PHY registers in the glue layer.
Neither does map the USB3 control register from the glue layer.

Thanks for any feedback and best regards,
Alexander

Alexander Stein (3):
  dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags
  phy: fsl-imx8mq-usb: Add support for setting fsl specific flags
  arm64: dts: imx8mp: Add memory for USB3 glue layer to usb3_phy nodes

 .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 52 +++++++++++++++-
 arch/arm64/boot/dts/freescale/imx8mp.dtsi     |  6 +-
 drivers/phy/freescale/phy-fsl-imx8mq-usb.c    | 61 +++++++++++++++++++
 3 files changed, 116 insertions(+), 3 deletions(-)

-- 
2.25.1


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

[PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags

[Alexander Stein]

This adds bindings for features only available on imx8mp. They allow
setting polarity of PWR and OC as well as disabling port power control.
Also permanently atteched can be annotated as well.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Adding properties specific to one compatible globally and disabling them on
other compatibles is the way to go?

Are there any best practices on the usage of '-' and/or '_' in property names?

 .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 52 ++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
index 2936f3510a6a..1d28b7d1c413 100644
--- a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
@@ -16,7 +16,8 @@ properties:
       - fsl,imx8mp-usb-phy
 
   reg:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   "#phy-cells":
     const: 0
@@ -32,6 +33,28 @@ properties:
     description:
       A phandle to the regulator for USB VBUS.
 
+  fsl,permanently-attached:
+    type: boolean
+    description:
+      Indicates if the device atached to a downstream port is
+      permanently attached.
+
+  fsl,disable-port-power-control:
+    type: boolean
+    description:
+      Indicates whether the host controller implementation includes port
+      power control. Defines Bit 3 in capability register (HCCPARAMS).
+
+  fsl,over-current-active-low:
+    type: boolean
+    description:
+      Over current signal polarity is active low.
+
+  fsl,power-active-low:
+    type: boolean
+    description:
+      Power pad (PWR) polarity is active low.
+
 required:
   - compatible
   - reg
@@ -39,6 +62,33 @@ required:
   - clocks
   - clock-names
 
+if:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - fsl,imx8mp-usb-phy
+
+then:
+  properties:
+    reg:
+      minItems: 2
+      maxItems: 2
+      items:
+        - description: PHY register base address
+        - description: Glue layer base address
+
+else:
+  properties:
+    reg:
+      maxItems: 1
+      items:
+        - description: PHY register base address
+    fsl,permanently-attached: false
+    fsl,disable-port-power-control: false
+    fsl,over-current-active-low: false
+    fsl,power-active-low: false
+
 additionalProperties: false
 
 examples:
-- 
2.25.1

[PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags

[Alexander Stein]

This adds bindings for features only available on imx8mp. They allow
setting polarity of PWR and OC as well as disabling port power control.
Also permanently atteched can be annotated as well.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Adding properties specific to one compatible globally and disabling them on
other compatibles is the way to go?

Are there any best practices on the usage of '-' and/or '_' in property names?

 .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 52 ++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
index 2936f3510a6a..1d28b7d1c413 100644
--- a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
@@ -16,7 +16,8 @@ properties:
       - fsl,imx8mp-usb-phy
 
   reg:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   "#phy-cells":
     const: 0
@@ -32,6 +33,28 @@ properties:
     description:
       A phandle to the regulator for USB VBUS.
 
+  fsl,permanently-attached:
+    type: boolean
+    description:
+      Indicates if the device atached to a downstream port is
+      permanently attached.
+
+  fsl,disable-port-power-control:
+    type: boolean
+    description:
+      Indicates whether the host controller implementation includes port
+      power control. Defines Bit 3 in capability register (HCCPARAMS).
+
+  fsl,over-current-active-low:
+    type: boolean
+    description:
+      Over current signal polarity is active low.
+
+  fsl,power-active-low:
+    type: boolean
+    description:
+      Power pad (PWR) polarity is active low.
+
 required:
   - compatible
   - reg
@@ -39,6 +62,33 @@ required:
   - clocks
   - clock-names
 
+if:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - fsl,imx8mp-usb-phy
+
+then:
+  properties:
+    reg:
+      minItems: 2
+      maxItems: 2
+      items:
+        - description: PHY register base address
+        - description: Glue layer base address
+
+else:
+  properties:
+    reg:
+      maxItems: 1
+      items:
+        - description: PHY register base address
+    fsl,permanently-attached: false
+    fsl,disable-port-power-control: false
+    fsl,over-current-active-low: false
+    fsl,power-active-low: false
+
 additionalProperties: false
 
 examples:
-- 
2.25.1


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

[PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags

[Alexander Stein]

This adds bindings for features only available on imx8mp. They allow
setting polarity of PWR and OC as well as disabling port power control.
Also permanently atteched can be annotated as well.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
Adding properties specific to one compatible globally and disabling them on
other compatibles is the way to go?

Are there any best practices on the usage of '-' and/or '_' in property names?

 .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 52 ++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
index 2936f3510a6a..1d28b7d1c413 100644
--- a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
@@ -16,7 +16,8 @@ properties:
       - fsl,imx8mp-usb-phy
 
   reg:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   "#phy-cells":
     const: 0
@@ -32,6 +33,28 @@ properties:
     description:
       A phandle to the regulator for USB VBUS.
 
+  fsl,permanently-attached:
+    type: boolean
+    description:
+      Indicates if the device atached to a downstream port is
+      permanently attached.
+
+  fsl,disable-port-power-control:
+    type: boolean
+    description:
+      Indicates whether the host controller implementation includes port
+      power control. Defines Bit 3 in capability register (HCCPARAMS).
+
+  fsl,over-current-active-low:
+    type: boolean
+    description:
+      Over current signal polarity is active low.
+
+  fsl,power-active-low:
+    type: boolean
+    description:
+      Power pad (PWR) polarity is active low.
+
 required:
   - compatible
   - reg
@@ -39,6 +62,33 @@ required:
   - clocks
   - clock-names
 
+if:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - fsl,imx8mp-usb-phy
+
+then:
+  properties:
+    reg:
+      minItems: 2
+      maxItems: 2
+      items:
+        - description: PHY register base address
+        - description: Glue layer base address
+
+else:
+  properties:
+    reg:
+      maxItems: 1
+      items:
+        - description: PHY register base address
+    fsl,permanently-attached: false
+    fsl,disable-port-power-control: false
+    fsl,over-current-active-low: false
+    fsl,power-active-low: false
+
 additionalProperties: false
 
 examples:
-- 
2.25.1


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

Re: [PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags

[Rob Herring]

On Thu, Dec 16, 2021 at 05:05:39PM +0100, Alexander Stein wrote:
> This adds bindings for features only available on imx8mp. They allow
> setting polarity of PWR and OC as well as disabling port power control.
> Also permanently atteched can be annotated as well.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> Adding properties specific to one compatible globally and disabling them on
> other compatibles is the way to go?
> 
> Are there any best practices on the usage of '-' and/or '_' in property names?

Yes, don't use '_'.

> 
>  .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 52 ++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> index 2936f3510a6a..1d28b7d1c413 100644
> --- a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> @@ -16,7 +16,8 @@ properties:
>        - fsl,imx8mp-usb-phy
>  
>    reg:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
>  
>    "#phy-cells":
>      const: 0
> @@ -32,6 +33,28 @@ properties:
>      description:
>        A phandle to the regulator for USB VBUS.
>  
> +  fsl,permanently-attached:
> +    type: boolean
> +    description:
> +      Indicates if the device atached to a downstream port is
> +      permanently attached.

Wouldn't just describing the downstream device be enough to indicate 
this? Though that is in the host controller rather than the phy.

> +
> +  fsl,disable-port-power-control:
> +    type: boolean
> +    description:
> +      Indicates whether the host controller implementation includes port
> +      power control. Defines Bit 3 in capability register (HCCPARAMS).
> +
> +  fsl,over-current-active-low:
> +    type: boolean
> +    description:
> +      Over current signal polarity is active low.
> +
> +  fsl,power-active-low:
> +    type: boolean
> +    description:
> +      Power pad (PWR) polarity is active low.
> +
>  required:
>    - compatible
>    - reg
> @@ -39,6 +62,33 @@ required:
>    - clocks
>    - clock-names
>  
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - fsl,imx8mp-usb-phy
> +
> +then:
> +  properties:
> +    reg:
> +      minItems: 2
> +      maxItems: 2
> +      items:
> +        - description: PHY register base address
> +        - description: Glue layer base address

Move 'items' to the top level and then here you only need 'minItems: 2'.

> +
> +else:
> +  properties:
> +    reg:
> +      maxItems: 1
> +      items:
> +        - description: PHY register base address

And just 'maxItems' here.

> +    fsl,permanently-attached: false
> +    fsl,disable-port-power-control: false
> +    fsl,over-current-active-low: false
> +    fsl,power-active-low: false
> +
>  additionalProperties: false
>  
>  examples:
> -- 
> 2.25.1
> 
> 

Re: [PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags

[Rob Herring]

On Thu, Dec 16, 2021 at 05:05:39PM +0100, Alexander Stein wrote:
> This adds bindings for features only available on imx8mp. They allow
> setting polarity of PWR and OC as well as disabling port power control.
> Also permanently atteched can be annotated as well.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> Adding properties specific to one compatible globally and disabling them on
> other compatibles is the way to go?
> 
> Are there any best practices on the usage of '-' and/or '_' in property names?

Yes, don't use '_'.

> 
>  .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 52 ++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> index 2936f3510a6a..1d28b7d1c413 100644
> --- a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> @@ -16,7 +16,8 @@ properties:
>        - fsl,imx8mp-usb-phy
>  
>    reg:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
>  
>    "#phy-cells":
>      const: 0
> @@ -32,6 +33,28 @@ properties:
>      description:
>        A phandle to the regulator for USB VBUS.
>  
> +  fsl,permanently-attached:
> +    type: boolean
> +    description:
> +      Indicates if the device atached to a downstream port is
> +      permanently attached.

Wouldn't just describing the downstream device be enough to indicate 
this? Though that is in the host controller rather than the phy.

> +
> +  fsl,disable-port-power-control:
> +    type: boolean
> +    description:
> +      Indicates whether the host controller implementation includes port
> +      power control. Defines Bit 3 in capability register (HCCPARAMS).
> +
> +  fsl,over-current-active-low:
> +    type: boolean
> +    description:
> +      Over current signal polarity is active low.
> +
> +  fsl,power-active-low:
> +    type: boolean
> +    description:
> +      Power pad (PWR) polarity is active low.
> +
>  required:
>    - compatible
>    - reg
> @@ -39,6 +62,33 @@ required:
>    - clocks
>    - clock-names
>  
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - fsl,imx8mp-usb-phy
> +
> +then:
> +  properties:
> +    reg:
> +      minItems: 2
> +      maxItems: 2
> +      items:
> +        - description: PHY register base address
> +        - description: Glue layer base address

Move 'items' to the top level and then here you only need 'minItems: 2'.

> +
> +else:
> +  properties:
> +    reg:
> +      maxItems: 1
> +      items:
> +        - description: PHY register base address

And just 'maxItems' here.

> +    fsl,permanently-attached: false
> +    fsl,disable-port-power-control: false
> +    fsl,over-current-active-low: false
> +    fsl,power-active-low: false
> +
>  additionalProperties: false
>  
>  examples:
> -- 
> 2.25.1
> 
> 

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

Re: [PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags

[Rob Herring]

On Thu, Dec 16, 2021 at 05:05:39PM +0100, Alexander Stein wrote:
> This adds bindings for features only available on imx8mp. They allow
> setting polarity of PWR and OC as well as disabling port power control.
> Also permanently atteched can be annotated as well.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> Adding properties specific to one compatible globally and disabling them on
> other compatibles is the way to go?
> 
> Are there any best practices on the usage of '-' and/or '_' in property names?

Yes, don't use '_'.

> 
>  .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 52 ++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> index 2936f3510a6a..1d28b7d1c413 100644
> --- a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> @@ -16,7 +16,8 @@ properties:
>        - fsl,imx8mp-usb-phy
>  
>    reg:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
>  
>    "#phy-cells":
>      const: 0
> @@ -32,6 +33,28 @@ properties:
>      description:
>        A phandle to the regulator for USB VBUS.
>  
> +  fsl,permanently-attached:
> +    type: boolean
> +    description:
> +      Indicates if the device atached to a downstream port is
> +      permanently attached.

Wouldn't just describing the downstream device be enough to indicate 
this? Though that is in the host controller rather than the phy.

> +
> +  fsl,disable-port-power-control:
> +    type: boolean
> +    description:
> +      Indicates whether the host controller implementation includes port
> +      power control. Defines Bit 3 in capability register (HCCPARAMS).
> +
> +  fsl,over-current-active-low:
> +    type: boolean
> +    description:
> +      Over current signal polarity is active low.
> +
> +  fsl,power-active-low:
> +    type: boolean
> +    description:
> +      Power pad (PWR) polarity is active low.
> +
>  required:
>    - compatible
>    - reg
> @@ -39,6 +62,33 @@ required:
>    - clocks
>    - clock-names
>  
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - fsl,imx8mp-usb-phy
> +
> +then:
> +  properties:
> +    reg:
> +      minItems: 2
> +      maxItems: 2
> +      items:
> +        - description: PHY register base address
> +        - description: Glue layer base address

Move 'items' to the top level and then here you only need 'minItems: 2'.

> +
> +else:
> +  properties:
> +    reg:
> +      maxItems: 1
> +      items:
> +        - description: PHY register base address

And just 'maxItems' here.

> +    fsl,permanently-attached: false
> +    fsl,disable-port-power-control: false
> +    fsl,over-current-active-low: false
> +    fsl,power-active-low: false
> +
>  additionalProperties: false
>  
>  examples:
> -- 
> 2.25.1
> 
> 

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

Re: (EXT) Re: [PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags

[Alexander Stein]

Hello,

thanks for the review.

Am Dienstag, 21. Dezember 2021, 17:59:52 CET schrieb Rob Herring:
> On Thu, Dec 16, 2021 at 05:05:39PM +0100, Alexander Stein wrote:
> > This adds bindings for features only available on imx8mp. They allow
> > setting polarity of PWR and OC as well as disabling port power control.
> > Also permanently atteched can be annotated as well.
> > 
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > ---
> > Adding properties specific to one compatible globally and disabling them
> > on
> > other compatibles is the way to go?
> > 
> > Are there any best practices on the usage of '-' and/or '_' in property
> > names?
> Yes, don't use '_'.

Alright, got it.

> >  .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 52 ++++++++++++++++++-
> >  1 file changed, 51 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> > b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml index
> > 2936f3510a6a..1d28b7d1c413 100644
> > --- a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> > 
> > @@ -16,7 +16,8 @@ properties:
> >        - fsl,imx8mp-usb-phy
> >    
> >    reg:
> > -    maxItems: 1
> > +    minItems: 1
> > +    maxItems: 2
> > 
> >    "#phy-cells":
> >      const: 0
> > 
> > @@ -32,6 +33,28 @@ properties:
> >      description:
> >        A phandle to the regulator for USB VBUS.
> > 
> > +  fsl,permanently-attached:
> > +    type: boolean
> > +    description:
> > +      Indicates if the device atached to a downstream port is
> > +      permanently attached.
> 
> Wouldn't just describing the downstream device be enough to indicate
> this? Though that is in the host controller rather than the phy.

You mean describing the downstream hub in device tree? I guess you are 
thinking about Documentation/devicetree/bindings/usb/usb-device.yaml, no?
I'll try using this.

This flag (and the others below) are used to set some specific flag in the 
host controller (not the PHY, see Li Jun's response). But I have to admit I do 
not know what they actually do. The description is pretty much everything 
written in the reference manual.

> > +
> > +  fsl,disable-port-power-control:
> > +    type: boolean
> > +    description:
> > +      Indicates whether the host controller implementation includes port
> > +      power control. Defines Bit 3 in capability register (HCCPARAMS).
> > +
> > +  fsl,over-current-active-low:
> > +    type: boolean
> > +    description:
> > +      Over current signal polarity is active low.
> > +
> > +  fsl,power-active-low:
> > +    type: boolean
> > +    description:
> > +      Power pad (PWR) polarity is active low.
> > +
> > 
> >  required:
> >    - compatible
> >    - reg
> > 
> > @@ -39,6 +62,33 @@ required:
> >    - clocks
> >    - clock-names
> > 
> > +if:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        enum:
> > +          - fsl,imx8mp-usb-phy
> > +
> > +then:
> > +  properties:
> > +    reg:
> > +      minItems: 2
> > +      maxItems: 2
> > +      items:
> > +        - description: PHY register base address
> > +        - description: Glue layer base address
> 
> Move 'items' to the top level and then here you only need 'minItems: 2'.
> 
> > +
> > +else:
> > +  properties:
> > +    reg:
> > +      maxItems: 1
> > +      items:
> > +        - description: PHY register base address
> 
> And just 'maxItems' here.

Thanks for the hints on how to write bindings, still fiddling with it.

Best regards,
Alexander

Re: (EXT) Re: [PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags

[Alexander Stein]

Hello,

thanks for the review.

Am Dienstag, 21. Dezember 2021, 17:59:52 CET schrieb Rob Herring:
> On Thu, Dec 16, 2021 at 05:05:39PM +0100, Alexander Stein wrote:
> > This adds bindings for features only available on imx8mp. They allow
> > setting polarity of PWR and OC as well as disabling port power control.
> > Also permanently atteched can be annotated as well.
> > 
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > ---
> > Adding properties specific to one compatible globally and disabling them
> > on
> > other compatibles is the way to go?
> > 
> > Are there any best practices on the usage of '-' and/or '_' in property
> > names?
> Yes, don't use '_'.

Alright, got it.

> >  .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 52 ++++++++++++++++++-
> >  1 file changed, 51 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> > b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml index
> > 2936f3510a6a..1d28b7d1c413 100644
> > --- a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> > 
> > @@ -16,7 +16,8 @@ properties:
> >        - fsl,imx8mp-usb-phy
> >    
> >    reg:
> > -    maxItems: 1
> > +    minItems: 1
> > +    maxItems: 2
> > 
> >    "#phy-cells":
> >      const: 0
> > 
> > @@ -32,6 +33,28 @@ properties:
> >      description:
> >        A phandle to the regulator for USB VBUS.
> > 
> > +  fsl,permanently-attached:
> > +    type: boolean
> > +    description:
> > +      Indicates if the device atached to a downstream port is
> > +      permanently attached.
> 
> Wouldn't just describing the downstream device be enough to indicate
> this? Though that is in the host controller rather than the phy.

You mean describing the downstream hub in device tree? I guess you are 
thinking about Documentation/devicetree/bindings/usb/usb-device.yaml, no?
I'll try using this.

This flag (and the others below) are used to set some specific flag in the 
host controller (not the PHY, see Li Jun's response). But I have to admit I do 
not know what they actually do. The description is pretty much everything 
written in the reference manual.

> > +
> > +  fsl,disable-port-power-control:
> > +    type: boolean
> > +    description:
> > +      Indicates whether the host controller implementation includes port
> > +      power control. Defines Bit 3 in capability register (HCCPARAMS).
> > +
> > +  fsl,over-current-active-low:
> > +    type: boolean
> > +    description:
> > +      Over current signal polarity is active low.
> > +
> > +  fsl,power-active-low:
> > +    type: boolean
> > +    description:
> > +      Power pad (PWR) polarity is active low.
> > +
> > 
> >  required:
> >    - compatible
> >    - reg
> > 
> > @@ -39,6 +62,33 @@ required:
> >    - clocks
> >    - clock-names
> > 
> > +if:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        enum:
> > +          - fsl,imx8mp-usb-phy
> > +
> > +then:
> > +  properties:
> > +    reg:
> > +      minItems: 2
> > +      maxItems: 2
> > +      items:
> > +        - description: PHY register base address
> > +        - description: Glue layer base address
> 
> Move 'items' to the top level and then here you only need 'minItems: 2'.
> 
> > +
> > +else:
> > +  properties:
> > +    reg:
> > +      maxItems: 1
> > +      items:
> > +        - description: PHY register base address
> 
> And just 'maxItems' here.

Thanks for the hints on how to write bindings, still fiddling with it.

Best regards,
Alexander




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

Re: (EXT) Re: [PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags

[Alexander Stein]

Hello,

thanks for the review.

Am Dienstag, 21. Dezember 2021, 17:59:52 CET schrieb Rob Herring:
> On Thu, Dec 16, 2021 at 05:05:39PM +0100, Alexander Stein wrote:
> > This adds bindings for features only available on imx8mp. They allow
> > setting polarity of PWR and OC as well as disabling port power control.
> > Also permanently atteched can be annotated as well.
> > 
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > ---
> > Adding properties specific to one compatible globally and disabling them
> > on
> > other compatibles is the way to go?
> > 
> > Are there any best practices on the usage of '-' and/or '_' in property
> > names?
> Yes, don't use '_'.

Alright, got it.

> >  .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 52 ++++++++++++++++++-
> >  1 file changed, 51 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> > b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml index
> > 2936f3510a6a..1d28b7d1c413 100644
> > --- a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> > 
> > @@ -16,7 +16,8 @@ properties:
> >        - fsl,imx8mp-usb-phy
> >    
> >    reg:
> > -    maxItems: 1
> > +    minItems: 1
> > +    maxItems: 2
> > 
> >    "#phy-cells":
> >      const: 0
> > 
> > @@ -32,6 +33,28 @@ properties:
> >      description:
> >        A phandle to the regulator for USB VBUS.
> > 
> > +  fsl,permanently-attached:
> > +    type: boolean
> > +    description:
> > +      Indicates if the device atached to a downstream port is
> > +      permanently attached.
> 
> Wouldn't just describing the downstream device be enough to indicate
> this? Though that is in the host controller rather than the phy.

You mean describing the downstream hub in device tree? I guess you are 
thinking about Documentation/devicetree/bindings/usb/usb-device.yaml, no?
I'll try using this.

This flag (and the others below) are used to set some specific flag in the 
host controller (not the PHY, see Li Jun's response). But I have to admit I do 
not know what they actually do. The description is pretty much everything 
written in the reference manual.

> > +
> > +  fsl,disable-port-power-control:
> > +    type: boolean
> > +    description:
> > +      Indicates whether the host controller implementation includes port
> > +      power control. Defines Bit 3 in capability register (HCCPARAMS).
> > +
> > +  fsl,over-current-active-low:
> > +    type: boolean
> > +    description:
> > +      Over current signal polarity is active low.
> > +
> > +  fsl,power-active-low:
> > +    type: boolean
> > +    description:
> > +      Power pad (PWR) polarity is active low.
> > +
> > 
> >  required:
> >    - compatible
> >    - reg
> > 
> > @@ -39,6 +62,33 @@ required:
> >    - clocks
> >    - clock-names
> > 
> > +if:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        enum:
> > +          - fsl,imx8mp-usb-phy
> > +
> > +then:
> > +  properties:
> > +    reg:
> > +      minItems: 2
> > +      maxItems: 2
> > +      items:
> > +        - description: PHY register base address
> > +        - description: Glue layer base address
> 
> Move 'items' to the top level and then here you only need 'minItems: 2'.
> 
> > +
> > +else:
> > +  properties:
> > +    reg:
> > +      maxItems: 1
> > +      items:
> > +        - description: PHY register base address
> 
> And just 'maxItems' here.

Thanks for the hints on how to write bindings, still fiddling with it.

Best regards,
Alexander




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

[PATCH v2 2/3] phy: fsl-imx8mq-usb: Add support for setting fsl specific flags

[Alexander Stein]

The i.MX8MP glue layer has support for the following flags:
* over-current polarity
* PWR pad polarity
* controlling PPC flag in HCCPARAMS register
* parmanent port attach for usb2 & usb3 port

Allow setting these flags by supporting specific flags in the glue node.
In order to get this to work an additional IORESOURCE_MEM is necessary
actually pointing to the glue layer. For backward compatibility this is
purely optional.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 61 ++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
index a29b4a6f7c24..86b61af6a949 100644
--- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
+++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
@@ -11,6 +11,18 @@
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 
+/* USB glue registers */
+#define USB_CTRL0		0x00
+#define USB_CTRL1		0x04
+
+#define USB_CTRL0_PORTPWR_EN	BIT(12) /* 1 - PPC enabled (default) */
+#define USB_CTRL0_USB3_FIXED	BIT(22) /* 1 - USB3 permanent attached */
+#define USB_CTRL0_USB2_FIXED	BIT(23) /* 1 - USB2 permanent attached */
+
+#define USB_CTRL1_OC_POLARITY	BIT(16) /* 0 - HIGH / 1 - LOW */
+#define USB_CTRL1_PWR_POLARITY	BIT(17) /* 0 - HIGH / 1 - LOW */
+
+/* USB phy registers */
 #define PHY_CTRL0			0x0
 #define PHY_CTRL0_REF_SSP_EN		BIT(2)
 #define PHY_CTRL0_FSEL_MASK		GENMASK(10, 5)
@@ -35,9 +47,46 @@ struct imx8mq_usb_phy {
 	struct phy *phy;
 	struct clk *clk;
 	void __iomem *base;
+	void __iomem *glue_base;
 	struct regulator *vbus;
 };
 
+static void imx8mp_configure_glue(struct imx8mq_usb_phy *dwc3_imx)
+{
+	struct device *dev = &dwc3_imx->phy->dev;
+	u32 value;
+
+	if (!dwc3_imx->glue_base)
+		return;
+
+	value = readl(dwc3_imx->glue_base + USB_CTRL0);
+
+	if (device_property_read_bool(dev, "fsl,permanently-attached"))
+		value |= (USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED);
+	else
+		value &= ~(USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED);
+
+	if (device_property_read_bool(dev, "fsl,disable-port-power-control"))
+		value &= ~(USB_CTRL0_PORTPWR_EN);
+	else
+		value |= USB_CTRL0_PORTPWR_EN;
+
+	writel(value, dwc3_imx->glue_base + USB_CTRL0);
+
+	value = readl(dwc3_imx->glue_base + USB_CTRL1);
+	if (device_property_read_bool(dev, "fsl,over-current-active-low"))
+		value |= USB_CTRL1_OC_POLARITY;
+	else
+		value &= ~USB_CTRL1_OC_POLARITY;
+
+	if (device_property_read_bool(dev, "fsl,power-active-low"))
+		value |= USB_CTRL1_PWR_POLARITY;
+	else
+		value &= ~USB_CTRL1_PWR_POLARITY;
+
+	writel(value, dwc3_imx->glue_base + USB_CTRL1);
+}
+
 static int imx8mq_usb_phy_init(struct phy *phy)
 {
 	struct imx8mq_usb_phy *imx_phy = phy_get_drvdata(phy);
@@ -69,6 +118,8 @@ static int imx8mp_usb_phy_init(struct phy *phy)
 	struct imx8mq_usb_phy *imx_phy = phy_get_drvdata(phy);
 	u32 value;
 
+	imx8mp_configure_glue(imx_phy);
+
 	/* USB3.0 PHY signal fsel for 24M ref */
 	value = readl(imx_phy->base + PHY_CTRL0);
 	value &= ~PHY_CTRL0_FSEL_MASK;
@@ -153,6 +204,7 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct imx8mq_usb_phy *imx_phy;
 	const struct phy_ops *phy_ops;
+	struct resource *res;
 
 	imx_phy = devm_kzalloc(dev, sizeof(*imx_phy), GFP_KERNEL);
 	if (!imx_phy)
@@ -168,6 +220,15 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(imx_phy->base))
 		return PTR_ERR(imx_phy->base);
 
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res) {
+		dev_warn(dev, "Base address for glue layer missing. Continuing without, some features are missing though.");
+	} else {
+		imx_phy->glue_base = devm_ioremap_resource(dev, res);
+		if (IS_ERR(imx_phy->glue_base))
+			return PTR_ERR(imx_phy->glue_base);
+	}
+
 	phy_ops = of_device_get_match_data(dev);
 	if (!phy_ops)
 		return -EINVAL;
-- 
2.25.1

[PATCH v2 2/3] phy: fsl-imx8mq-usb: Add support for setting fsl specific flags

[Alexander Stein]

The i.MX8MP glue layer has support for the following flags:
* over-current polarity
* PWR pad polarity
* controlling PPC flag in HCCPARAMS register
* parmanent port attach for usb2 & usb3 port

Allow setting these flags by supporting specific flags in the glue node.
In order to get this to work an additional IORESOURCE_MEM is necessary
actually pointing to the glue layer. For backward compatibility this is
purely optional.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 61 ++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
index a29b4a6f7c24..86b61af6a949 100644
--- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
+++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
@@ -11,6 +11,18 @@
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 
+/* USB glue registers */
+#define USB_CTRL0		0x00
+#define USB_CTRL1		0x04
+
+#define USB_CTRL0_PORTPWR_EN	BIT(12) /* 1 - PPC enabled (default) */
+#define USB_CTRL0_USB3_FIXED	BIT(22) /* 1 - USB3 permanent attached */
+#define USB_CTRL0_USB2_FIXED	BIT(23) /* 1 - USB2 permanent attached */
+
+#define USB_CTRL1_OC_POLARITY	BIT(16) /* 0 - HIGH / 1 - LOW */
+#define USB_CTRL1_PWR_POLARITY	BIT(17) /* 0 - HIGH / 1 - LOW */
+
+/* USB phy registers */
 #define PHY_CTRL0			0x0
 #define PHY_CTRL0_REF_SSP_EN		BIT(2)
 #define PHY_CTRL0_FSEL_MASK		GENMASK(10, 5)
@@ -35,9 +47,46 @@ struct imx8mq_usb_phy {
 	struct phy *phy;
 	struct clk *clk;
 	void __iomem *base;
+	void __iomem *glue_base;
 	struct regulator *vbus;
 };
 
+static void imx8mp_configure_glue(struct imx8mq_usb_phy *dwc3_imx)
+{
+	struct device *dev = &dwc3_imx->phy->dev;
+	u32 value;
+
+	if (!dwc3_imx->glue_base)
+		return;
+
+	value = readl(dwc3_imx->glue_base + USB_CTRL0);
+
+	if (device_property_read_bool(dev, "fsl,permanently-attached"))
+		value |= (USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED);
+	else
+		value &= ~(USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED);
+
+	if (device_property_read_bool(dev, "fsl,disable-port-power-control"))
+		value &= ~(USB_CTRL0_PORTPWR_EN);
+	else
+		value |= USB_CTRL0_PORTPWR_EN;
+
+	writel(value, dwc3_imx->glue_base + USB_CTRL0);
+
+	value = readl(dwc3_imx->glue_base + USB_CTRL1);
+	if (device_property_read_bool(dev, "fsl,over-current-active-low"))
+		value |= USB_CTRL1_OC_POLARITY;
+	else
+		value &= ~USB_CTRL1_OC_POLARITY;
+
+	if (device_property_read_bool(dev, "fsl,power-active-low"))
+		value |= USB_CTRL1_PWR_POLARITY;
+	else
+		value &= ~USB_CTRL1_PWR_POLARITY;
+
+	writel(value, dwc3_imx->glue_base + USB_CTRL1);
+}
+
 static int imx8mq_usb_phy_init(struct phy *phy)
 {
 	struct imx8mq_usb_phy *imx_phy = phy_get_drvdata(phy);
@@ -69,6 +118,8 @@ static int imx8mp_usb_phy_init(struct phy *phy)
 	struct imx8mq_usb_phy *imx_phy = phy_get_drvdata(phy);
 	u32 value;
 
+	imx8mp_configure_glue(imx_phy);
+
 	/* USB3.0 PHY signal fsel for 24M ref */
 	value = readl(imx_phy->base + PHY_CTRL0);
 	value &= ~PHY_CTRL0_FSEL_MASK;
@@ -153,6 +204,7 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct imx8mq_usb_phy *imx_phy;
 	const struct phy_ops *phy_ops;
+	struct resource *res;
 
 	imx_phy = devm_kzalloc(dev, sizeof(*imx_phy), GFP_KERNEL);
 	if (!imx_phy)
@@ -168,6 +220,15 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(imx_phy->base))
 		return PTR_ERR(imx_phy->base);
 
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res) {
+		dev_warn(dev, "Base address for glue layer missing. Continuing without, some features are missing though.");
+	} else {
+		imx_phy->glue_base = devm_ioremap_resource(dev, res);
+		if (IS_ERR(imx_phy->glue_base))
+			return PTR_ERR(imx_phy->glue_base);
+	}
+
 	phy_ops = of_device_get_match_data(dev);
 	if (!phy_ops)
 		return -EINVAL;
-- 
2.25.1


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

[PATCH v2 2/3] phy: fsl-imx8mq-usb: Add support for setting fsl specific flags

[Alexander Stein]

The i.MX8MP glue layer has support for the following flags:
* over-current polarity
* PWR pad polarity
* controlling PPC flag in HCCPARAMS register
* parmanent port attach for usb2 & usb3 port

Allow setting these flags by supporting specific flags in the glue node.
In order to get this to work an additional IORESOURCE_MEM is necessary
actually pointing to the glue layer. For backward compatibility this is
purely optional.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 61 ++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
index a29b4a6f7c24..86b61af6a949 100644
--- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
+++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
@@ -11,6 +11,18 @@
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 
+/* USB glue registers */
+#define USB_CTRL0		0x00
+#define USB_CTRL1		0x04
+
+#define USB_CTRL0_PORTPWR_EN	BIT(12) /* 1 - PPC enabled (default) */
+#define USB_CTRL0_USB3_FIXED	BIT(22) /* 1 - USB3 permanent attached */
+#define USB_CTRL0_USB2_FIXED	BIT(23) /* 1 - USB2 permanent attached */
+
+#define USB_CTRL1_OC_POLARITY	BIT(16) /* 0 - HIGH / 1 - LOW */
+#define USB_CTRL1_PWR_POLARITY	BIT(17) /* 0 - HIGH / 1 - LOW */
+
+/* USB phy registers */
 #define PHY_CTRL0			0x0
 #define PHY_CTRL0_REF_SSP_EN		BIT(2)
 #define PHY_CTRL0_FSEL_MASK		GENMASK(10, 5)
@@ -35,9 +47,46 @@ struct imx8mq_usb_phy {
 	struct phy *phy;
 	struct clk *clk;
 	void __iomem *base;
+	void __iomem *glue_base;
 	struct regulator *vbus;
 };
 
+static void imx8mp_configure_glue(struct imx8mq_usb_phy *dwc3_imx)
+{
+	struct device *dev = &dwc3_imx->phy->dev;
+	u32 value;
+
+	if (!dwc3_imx->glue_base)
+		return;
+
+	value = readl(dwc3_imx->glue_base + USB_CTRL0);
+
+	if (device_property_read_bool(dev, "fsl,permanently-attached"))
+		value |= (USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED);
+	else
+		value &= ~(USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED);
+
+	if (device_property_read_bool(dev, "fsl,disable-port-power-control"))
+		value &= ~(USB_CTRL0_PORTPWR_EN);
+	else
+		value |= USB_CTRL0_PORTPWR_EN;
+
+	writel(value, dwc3_imx->glue_base + USB_CTRL0);
+
+	value = readl(dwc3_imx->glue_base + USB_CTRL1);
+	if (device_property_read_bool(dev, "fsl,over-current-active-low"))
+		value |= USB_CTRL1_OC_POLARITY;
+	else
+		value &= ~USB_CTRL1_OC_POLARITY;
+
+	if (device_property_read_bool(dev, "fsl,power-active-low"))
+		value |= USB_CTRL1_PWR_POLARITY;
+	else
+		value &= ~USB_CTRL1_PWR_POLARITY;
+
+	writel(value, dwc3_imx->glue_base + USB_CTRL1);
+}
+
 static int imx8mq_usb_phy_init(struct phy *phy)
 {
 	struct imx8mq_usb_phy *imx_phy = phy_get_drvdata(phy);
@@ -69,6 +118,8 @@ static int imx8mp_usb_phy_init(struct phy *phy)
 	struct imx8mq_usb_phy *imx_phy = phy_get_drvdata(phy);
 	u32 value;
 
+	imx8mp_configure_glue(imx_phy);
+
 	/* USB3.0 PHY signal fsel for 24M ref */
 	value = readl(imx_phy->base + PHY_CTRL0);
 	value &= ~PHY_CTRL0_FSEL_MASK;
@@ -153,6 +204,7 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct imx8mq_usb_phy *imx_phy;
 	const struct phy_ops *phy_ops;
+	struct resource *res;
 
 	imx_phy = devm_kzalloc(dev, sizeof(*imx_phy), GFP_KERNEL);
 	if (!imx_phy)
@@ -168,6 +220,15 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(imx_phy->base))
 		return PTR_ERR(imx_phy->base);
 
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res) {
+		dev_warn(dev, "Base address for glue layer missing. Continuing without, some features are missing though.");
+	} else {
+		imx_phy->glue_base = devm_ioremap_resource(dev, res);
+		if (IS_ERR(imx_phy->glue_base))
+			return PTR_ERR(imx_phy->glue_base);
+	}
+
 	phy_ops = of_device_get_match_data(dev);
 	if (!phy_ops)
 		return -EINVAL;
-- 
2.25.1


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

[PATCH v2 3/3] arm64: dts: imx8mp: Add memory for USB3 glue layer to usb3_phy nodes

[Alexander Stein]

The USB3 glue layer has 2 areas in the register set, see RM Rev.1
section 11.2.5.2.1 GLUE_usb3 memory map:
* USB3 control/status
* PHY control/status

Provide the memory area to the usb3_phy nodes for accessing the features
in the USB3 control area.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 6b840c05dd77..4958142da1e4 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -910,7 +910,8 @@ ddr-pmu@3d800000 {
 
 		usb3_phy0: usb-phy@381f0040 {
 			compatible = "fsl,imx8mp-usb-phy";
-			reg = <0x381f0040 0x40>;
+			reg = <0x381f0040 0x40>,
+			      <0x381f0000 0x20>;
 			clocks = <&clk IMX8MP_CLK_USB_PHY_ROOT>;
 			clock-names = "phy";
 			assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
@@ -952,7 +953,8 @@ usb_dwc3_0: usb@38100000 {
 
 		usb3_phy1: usb-phy@382f0040 {
 			compatible = "fsl,imx8mp-usb-phy";
-			reg = <0x382f0040 0x40>;
+			reg = <0x382f0040 0x40>,
+			      <0x382f0000 0x20>;
 			clocks = <&clk IMX8MP_CLK_USB_PHY_ROOT>;
 			clock-names = "phy";
 			assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
-- 
2.25.1

[PATCH v2 3/3] arm64: dts: imx8mp: Add memory for USB3 glue layer to usb3_phy nodes

[Alexander Stein]

The USB3 glue layer has 2 areas in the register set, see RM Rev.1
section 11.2.5.2.1 GLUE_usb3 memory map:
* USB3 control/status
* PHY control/status

Provide the memory area to the usb3_phy nodes for accessing the features
in the USB3 control area.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 6b840c05dd77..4958142da1e4 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -910,7 +910,8 @@ ddr-pmu@3d800000 {
 
 		usb3_phy0: usb-phy@381f0040 {
 			compatible = "fsl,imx8mp-usb-phy";
-			reg = <0x381f0040 0x40>;
+			reg = <0x381f0040 0x40>,
+			      <0x381f0000 0x20>;
 			clocks = <&clk IMX8MP_CLK_USB_PHY_ROOT>;
 			clock-names = "phy";
 			assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
@@ -952,7 +953,8 @@ usb_dwc3_0: usb@38100000 {
 
 		usb3_phy1: usb-phy@382f0040 {
 			compatible = "fsl,imx8mp-usb-phy";
-			reg = <0x382f0040 0x40>;
+			reg = <0x382f0040 0x40>,
+			      <0x382f0000 0x20>;
 			clocks = <&clk IMX8MP_CLK_USB_PHY_ROOT>;
 			clock-names = "phy";
 			assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
-- 
2.25.1


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

[PATCH v2 3/3] arm64: dts: imx8mp: Add memory for USB3 glue layer to usb3_phy nodes

[Alexander Stein]

The USB3 glue layer has 2 areas in the register set, see RM Rev.1
section 11.2.5.2.1 GLUE_usb3 memory map:
* USB3 control/status
* PHY control/status

Provide the memory area to the usb3_phy nodes for accessing the features
in the USB3 control area.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 6b840c05dd77..4958142da1e4 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -910,7 +910,8 @@ ddr-pmu@3d800000 {
 
 		usb3_phy0: usb-phy@381f0040 {
 			compatible = "fsl,imx8mp-usb-phy";
-			reg = <0x381f0040 0x40>;
+			reg = <0x381f0040 0x40>,
+			      <0x381f0000 0x20>;
 			clocks = <&clk IMX8MP_CLK_USB_PHY_ROOT>;
 			clock-names = "phy";
 			assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
@@ -952,7 +953,8 @@ usb_dwc3_0: usb@38100000 {
 
 		usb3_phy1: usb-phy@382f0040 {
 			compatible = "fsl,imx8mp-usb-phy";
-			reg = <0x382f0040 0x40>;
+			reg = <0x382f0040 0x40>,
+			      <0x382f0000 0x20>;
 			clocks = <&clk IMX8MP_CLK_USB_PHY_ROOT>;
 			clock-names = "phy";
 			assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
-- 
2.25.1


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

RE: [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support

[Jun Li]

> -----Original Message-----
> From: Alexander Stein <alexander.stein@ew.tq-group.com>
> Sent: Friday, December 17, 2021 12:06 AM
> To: Kishon Vijay Abraham I <kishon@ti.com>; Vinod Koul <vkoul@kernel.org>;
> Rob Herring <robh+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha
> Hauer <s.hauer@pengutronix.de>; Fabio Estevam <festevam@gmail.com>
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>; dl-linux-imx
> <linux-imx@nxp.com>; linux-phy@lists.infradead.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Subject: [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support
> 
> This patchset aims to support flags for e.g. over-current active low or port
> permanantly attached which are provided in the USB3 glue layer.
> 
> There is already a glue layer driver dwc3-imx8mp, but unfortunately this
> driver does not use the glue area at all, it only handles wakeup-support
> which is done in the HSIO BLK_CTRL area (0x32f10100), accordingly the driver
> only uses the hsio clock.
> 
> The driver which actually uses the USB3 glue area is phy-fsl-imx8mq-usb.
> As the name indicates PHY is configured in the corresponding registers, which
> are part of the USB3 glue layer.
> 
> This make is it unclear for me which driver should handle the required features
> above.
> dwc3-imx8mp, the glue layer driver, does not touch the glue area at all,
> but the HSIO BLK_CTRL area.
> phy-fsl-imx8mq-usb only touches the PHY registers in the glue layer.
> Neither does map the USB3 control register from the glue layer.
> 
> Thanks for any feedback and best regards, Alexander

Which driver handle what function is decided by the driver *function*,
not where the actual HW logic is located, iMX8MP do have a "glue" layer
in SoC HW, some part is for phy config, and some part is for controller,
so we need put the part of phy config into the phy driver, the changes
you are adding is for controller so should be put in dwc3-imx8mp.c from
my point view.

Li Jun 
 
> 
> Alexander Stein (3):
>   dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags
>   phy: fsl-imx8mq-usb: Add support for setting fsl specific flags
>   arm64: dts: imx8mp: Add memory for USB3 glue layer to usb3_phy nodes
> 
>  .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 52 +++++++++++++++-
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi     |  6 +-
>  drivers/phy/freescale/phy-fsl-imx8mq-usb.c    | 61 +++++++++++++++++++
>  3 files changed, 116 insertions(+), 3 deletions(-)
> 
> --
> 2.25.1

RE: [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support

[Jun Li]

> -----Original Message-----
> From: Alexander Stein <alexander.stein@ew.tq-group.com>
> Sent: Friday, December 17, 2021 12:06 AM
> To: Kishon Vijay Abraham I <kishon@ti.com>; Vinod Koul <vkoul@kernel.org>;
> Rob Herring <robh+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha
> Hauer <s.hauer@pengutronix.de>; Fabio Estevam <festevam@gmail.com>
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>; dl-linux-imx
> <linux-imx@nxp.com>; linux-phy@lists.infradead.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Subject: [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support
> 
> This patchset aims to support flags for e.g. over-current active low or port
> permanantly attached which are provided in the USB3 glue layer.
> 
> There is already a glue layer driver dwc3-imx8mp, but unfortunately this
> driver does not use the glue area at all, it only handles wakeup-support
> which is done in the HSIO BLK_CTRL area (0x32f10100), accordingly the driver
> only uses the hsio clock.
> 
> The driver which actually uses the USB3 glue area is phy-fsl-imx8mq-usb.
> As the name indicates PHY is configured in the corresponding registers, which
> are part of the USB3 glue layer.
> 
> This make is it unclear for me which driver should handle the required features
> above.
> dwc3-imx8mp, the glue layer driver, does not touch the glue area at all,
> but the HSIO BLK_CTRL area.
> phy-fsl-imx8mq-usb only touches the PHY registers in the glue layer.
> Neither does map the USB3 control register from the glue layer.
> 
> Thanks for any feedback and best regards, Alexander

Which driver handle what function is decided by the driver *function*,
not where the actual HW logic is located, iMX8MP do have a "glue" layer
in SoC HW, some part is for phy config, and some part is for controller,
so we need put the part of phy config into the phy driver, the changes
you are adding is for controller so should be put in dwc3-imx8mp.c from
my point view.

Li Jun 
 
> 
> Alexander Stein (3):
>   dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags
>   phy: fsl-imx8mq-usb: Add support for setting fsl specific flags
>   arm64: dts: imx8mp: Add memory for USB3 glue layer to usb3_phy nodes
> 
>  .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 52 +++++++++++++++-
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi     |  6 +-
>  drivers/phy/freescale/phy-fsl-imx8mq-usb.c    | 61 +++++++++++++++++++
>  3 files changed, 116 insertions(+), 3 deletions(-)
> 
> --
> 2.25.1


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

RE: [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support

[Jun Li]

> -----Original Message-----
> From: Alexander Stein <alexander.stein@ew.tq-group.com>
> Sent: Friday, December 17, 2021 12:06 AM
> To: Kishon Vijay Abraham I <kishon@ti.com>; Vinod Koul <vkoul@kernel.org>;
> Rob Herring <robh+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha
> Hauer <s.hauer@pengutronix.de>; Fabio Estevam <festevam@gmail.com>
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>; dl-linux-imx
> <linux-imx@nxp.com>; linux-phy@lists.infradead.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Subject: [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support
> 
> This patchset aims to support flags for e.g. over-current active low or port
> permanantly attached which are provided in the USB3 glue layer.
> 
> There is already a glue layer driver dwc3-imx8mp, but unfortunately this
> driver does not use the glue area at all, it only handles wakeup-support
> which is done in the HSIO BLK_CTRL area (0x32f10100), accordingly the driver
> only uses the hsio clock.
> 
> The driver which actually uses the USB3 glue area is phy-fsl-imx8mq-usb.
> As the name indicates PHY is configured in the corresponding registers, which
> are part of the USB3 glue layer.
> 
> This make is it unclear for me which driver should handle the required features
> above.
> dwc3-imx8mp, the glue layer driver, does not touch the glue area at all,
> but the HSIO BLK_CTRL area.
> phy-fsl-imx8mq-usb only touches the PHY registers in the glue layer.
> Neither does map the USB3 control register from the glue layer.
> 
> Thanks for any feedback and best regards, Alexander

Which driver handle what function is decided by the driver *function*,
not where the actual HW logic is located, iMX8MP do have a "glue" layer
in SoC HW, some part is for phy config, and some part is for controller,
so we need put the part of phy config into the phy driver, the changes
you are adding is for controller so should be put in dwc3-imx8mp.c from
my point view.

Li Jun 
 
> 
> Alexander Stein (3):
>   dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags
>   phy: fsl-imx8mq-usb: Add support for setting fsl specific flags
>   arm64: dts: imx8mp: Add memory for USB3 glue layer to usb3_phy nodes
> 
>  .../bindings/phy/fsl,imx8mq-usb-phy.yaml      | 52 +++++++++++++++-
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi     |  6 +-
>  drivers/phy/freescale/phy-fsl-imx8mq-usb.c    | 61 +++++++++++++++++++
>  3 files changed, 116 insertions(+), 3 deletions(-)
> 
> --
> 2.25.1


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

Re: (EXT) RE: [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support

[Alexander Stein]

Hi,

Am Mittwoch, 22. Dezember 2021, 04:18:57 CET schrieb Jun Li:
> > -----Original Message-----
> > From: Alexander Stein <alexander.stein@ew.tq-group.com>
> > Sent: Friday, December 17, 2021 12:06 AM
> > To: Kishon Vijay Abraham I <kishon@ti.com>; Vinod Koul <vkoul@kernel.org>;
> > Rob Herring <robh+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha
> > Hauer <s.hauer@pengutronix.de>; Fabio Estevam <festevam@gmail.com>
> > Cc: Alexander Stein <alexander.stein@ew.tq-group.com>; dl-linux-imx
> > <linux-imx@nxp.com>; linux-phy@lists.infradead.org;
> > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> > Subject: [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support
> > 
> > This patchset aims to support flags for e.g. over-current active low or
> > port permanantly attached which are provided in the USB3 glue layer.
> > 
> > There is already a glue layer driver dwc3-imx8mp, but unfortunately this
> > driver does not use the glue area at all, it only handles wakeup-support
> > which is done in the HSIO BLK_CTRL area (0x32f10100), accordingly the
> > driver only uses the hsio clock.
> > 
> > The driver which actually uses the USB3 glue area is phy-fsl-imx8mq-usb.
> > As the name indicates PHY is configured in the corresponding registers,
> > which are part of the USB3 glue layer.
> > 
> > This make is it unclear for me which driver should handle the required
> > features above.
> > dwc3-imx8mp, the glue layer driver, does not touch the glue area at all,
> > but the HSIO BLK_CTRL area.
> > phy-fsl-imx8mq-usb only touches the PHY registers in the glue layer.
> > Neither does map the USB3 control register from the glue layer.
> > 
> > Thanks for any feedback and best regards, Alexander
> 
> Which driver handle what function is decided by the driver *function*,
> not where the actual HW logic is located, iMX8MP do have a "glue" layer
> in SoC HW, some part is for phy config, and some part is for controller,
> so we need put the part of phy config into the phy driver, the changes
> you are adding is for controller so should be put in dwc3-imx8mp.c from
> my point view.

Thanks for that feedback. This makes things clearer to me.
Yes, dwc3-imx8mp.c seems the right place for that. I'll do that.

Best regards,
Alexander

Re: (EXT) RE: [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support

[Alexander Stein]

Hi,

Am Mittwoch, 22. Dezember 2021, 04:18:57 CET schrieb Jun Li:
> > -----Original Message-----
> > From: Alexander Stein <alexander.stein@ew.tq-group.com>
> > Sent: Friday, December 17, 2021 12:06 AM
> > To: Kishon Vijay Abraham I <kishon@ti.com>; Vinod Koul <vkoul@kernel.org>;
> > Rob Herring <robh+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha
> > Hauer <s.hauer@pengutronix.de>; Fabio Estevam <festevam@gmail.com>
> > Cc: Alexander Stein <alexander.stein@ew.tq-group.com>; dl-linux-imx
> > <linux-imx@nxp.com>; linux-phy@lists.infradead.org;
> > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> > Subject: [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support
> > 
> > This patchset aims to support flags for e.g. over-current active low or
> > port permanantly attached which are provided in the USB3 glue layer.
> > 
> > There is already a glue layer driver dwc3-imx8mp, but unfortunately this
> > driver does not use the glue area at all, it only handles wakeup-support
> > which is done in the HSIO BLK_CTRL area (0x32f10100), accordingly the
> > driver only uses the hsio clock.
> > 
> > The driver which actually uses the USB3 glue area is phy-fsl-imx8mq-usb.
> > As the name indicates PHY is configured in the corresponding registers,
> > which are part of the USB3 glue layer.
> > 
> > This make is it unclear for me which driver should handle the required
> > features above.
> > dwc3-imx8mp, the glue layer driver, does not touch the glue area at all,
> > but the HSIO BLK_CTRL area.
> > phy-fsl-imx8mq-usb only touches the PHY registers in the glue layer.
> > Neither does map the USB3 control register from the glue layer.
> > 
> > Thanks for any feedback and best regards, Alexander
> 
> Which driver handle what function is decided by the driver *function*,
> not where the actual HW logic is located, iMX8MP do have a "glue" layer
> in SoC HW, some part is for phy config, and some part is for controller,
> so we need put the part of phy config into the phy driver, the changes
> you are adding is for controller so should be put in dwc3-imx8mp.c from
> my point view.

Thanks for that feedback. This makes things clearer to me.
Yes, dwc3-imx8mp.c seems the right place for that. I'll do that.

Best regards,
Alexander




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

Re: (EXT) RE: [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support

[Alexander Stein]

Hi,

Am Mittwoch, 22. Dezember 2021, 04:18:57 CET schrieb Jun Li:
> > -----Original Message-----
> > From: Alexander Stein <alexander.stein@ew.tq-group.com>
> > Sent: Friday, December 17, 2021 12:06 AM
> > To: Kishon Vijay Abraham I <kishon@ti.com>; Vinod Koul <vkoul@kernel.org>;
> > Rob Herring <robh+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha
> > Hauer <s.hauer@pengutronix.de>; Fabio Estevam <festevam@gmail.com>
> > Cc: Alexander Stein <alexander.stein@ew.tq-group.com>; dl-linux-imx
> > <linux-imx@nxp.com>; linux-phy@lists.infradead.org;
> > devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> > Subject: [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support
> > 
> > This patchset aims to support flags for e.g. over-current active low or
> > port permanantly attached which are provided in the USB3 glue layer.
> > 
> > There is already a glue layer driver dwc3-imx8mp, but unfortunately this
> > driver does not use the glue area at all, it only handles wakeup-support
> > which is done in the HSIO BLK_CTRL area (0x32f10100), accordingly the
> > driver only uses the hsio clock.
> > 
> > The driver which actually uses the USB3 glue area is phy-fsl-imx8mq-usb.
> > As the name indicates PHY is configured in the corresponding registers,
> > which are part of the USB3 glue layer.
> > 
> > This make is it unclear for me which driver should handle the required
> > features above.
> > dwc3-imx8mp, the glue layer driver, does not touch the glue area at all,
> > but the HSIO BLK_CTRL area.
> > phy-fsl-imx8mq-usb only touches the PHY registers in the glue layer.
> > Neither does map the USB3 control register from the glue layer.
> > 
> > Thanks for any feedback and best regards, Alexander
> 
> Which driver handle what function is decided by the driver *function*,
> not where the actual HW logic is located, iMX8MP do have a "glue" layer
> in SoC HW, some part is for phy config, and some part is for controller,
> so we need put the part of phy config into the phy driver, the changes
> you are adding is for controller so should be put in dwc3-imx8mp.c from
> my point view.

Thanks for that feedback. This makes things clearer to me.
Yes, dwc3-imx8mp.c seems the right place for that. I'll do that.

Best regards,
Alexander




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