All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support
@ 2021-12-16 16:05 ` Alexander Stein
  0 siblings, 0 replies; 24+ messages in thread
From: Alexander Stein @ 2021-12-16 16:05 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Vinod Koul, Rob Herring, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: Alexander Stein, NXP Linux Team, linux-phy, devicetree, linux-arm-kernel

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


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support
@ 2021-12-16 16:05 ` Alexander Stein
  0 siblings, 0 replies; 24+ messages in thread
From: Alexander Stein @ 2021-12-16 16:05 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Vinod Koul, Rob Herring, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: Alexander Stein, NXP Linux Team, linux-phy, devicetree, linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support
@ 2021-12-16 16:05 ` Alexander Stein
  0 siblings, 0 replies; 24+ messages in thread
From: Alexander Stein @ 2021-12-16 16:05 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Vinod Koul, Rob Herring, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: Alexander Stein, NXP Linux Team, linux-phy, devicetree, linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags
  2021-12-16 16:05 ` Alexander Stein
  (?)
@ 2021-12-16 16:05   ` Alexander Stein
  -1 siblings, 0 replies; 24+ messages in thread
From: Alexander Stein @ 2021-12-16 16:05 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Vinod Koul, Rob Herring, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: Alexander Stein, NXP Linux Team, linux-phy, devicetree, linux-arm-kernel

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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags
@ 2021-12-16 16:05   ` Alexander Stein
  0 siblings, 0 replies; 24+ messages in thread
From: Alexander Stein @ 2021-12-16 16:05 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Vinod Koul, Rob Herring, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: Alexander Stein, NXP Linux Team, linux-phy, devicetree, linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags
@ 2021-12-16 16:05   ` Alexander Stein
  0 siblings, 0 replies; 24+ messages in thread
From: Alexander Stein @ 2021-12-16 16:05 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Vinod Koul, Rob Herring, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: Alexander Stein, NXP Linux Team, linux-phy, devicetree, linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 2/3] phy: fsl-imx8mq-usb: Add support for setting fsl specific flags
  2021-12-16 16:05 ` Alexander Stein
  (?)
@ 2021-12-16 16:05   ` Alexander Stein
  -1 siblings, 0 replies; 24+ messages in thread
From: Alexander Stein @ 2021-12-16 16:05 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Vinod Koul, Rob Herring, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: Alexander Stein, NXP Linux Team, linux-phy, devicetree, linux-arm-kernel

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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 2/3] phy: fsl-imx8mq-usb: Add support for setting fsl specific flags
@ 2021-12-16 16:05   ` Alexander Stein
  0 siblings, 0 replies; 24+ messages in thread
From: Alexander Stein @ 2021-12-16 16:05 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Vinod Koul, Rob Herring, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: Alexander Stein, NXP Linux Team, linux-phy, devicetree, linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 2/3] phy: fsl-imx8mq-usb: Add support for setting fsl specific flags
@ 2021-12-16 16:05   ` Alexander Stein
  0 siblings, 0 replies; 24+ messages in thread
From: Alexander Stein @ 2021-12-16 16:05 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Vinod Koul, Rob Herring, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: Alexander Stein, NXP Linux Team, linux-phy, devicetree, linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 3/3] arm64: dts: imx8mp: Add memory for USB3 glue layer to usb3_phy nodes
  2021-12-16 16:05 ` Alexander Stein
  (?)
@ 2021-12-16 16:05   ` Alexander Stein
  -1 siblings, 0 replies; 24+ messages in thread
From: Alexander Stein @ 2021-12-16 16:05 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Vinod Koul, Rob Herring, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: Alexander Stein, NXP Linux Team, linux-phy, devicetree, linux-arm-kernel

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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 3/3] arm64: dts: imx8mp: Add memory for USB3 glue layer to usb3_phy nodes
@ 2021-12-16 16:05   ` Alexander Stein
  0 siblings, 0 replies; 24+ messages in thread
From: Alexander Stein @ 2021-12-16 16:05 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Vinod Koul, Rob Herring, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: Alexander Stein, NXP Linux Team, linux-phy, devicetree, linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 3/3] arm64: dts: imx8mp: Add memory for USB3 glue layer to usb3_phy nodes
@ 2021-12-16 16:05   ` Alexander Stein
  0 siblings, 0 replies; 24+ messages in thread
From: Alexander Stein @ 2021-12-16 16:05 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Vinod Koul, Rob Herring, Shawn Guo,
	Sascha Hauer, Fabio Estevam
  Cc: Alexander Stein, NXP Linux Team, linux-phy, devicetree, linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags
  2021-12-16 16:05   ` Alexander Stein
  (?)
@ 2021-12-21 16:59     ` Rob Herring
  -1 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2021-12-21 16:59 UTC (permalink / raw)
  To: Alexander Stein
  Cc: Kishon Vijay Abraham I, Vinod Koul, Shawn Guo, Sascha Hauer,
	Fabio Estevam, NXP Linux Team, linux-phy, devicetree,
	linux-arm-kernel

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
> 
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags
@ 2021-12-21 16:59     ` Rob Herring
  0 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2021-12-21 16:59 UTC (permalink / raw)
  To: Alexander Stein
  Cc: Kishon Vijay Abraham I, Vinod Koul, Shawn Guo, Sascha Hauer,
	Fabio Estevam, NXP Linux Team, linux-phy, devicetree,
	linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags
@ 2021-12-21 16:59     ` Rob Herring
  0 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2021-12-21 16:59 UTC (permalink / raw)
  To: Alexander Stein
  Cc: Kishon Vijay Abraham I, Vinod Koul, Shawn Guo, Sascha Hauer,
	Fabio Estevam, NXP Linux Team, linux-phy, devicetree,
	linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* RE: [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support
  2021-12-16 16:05 ` Alexander Stein
  (?)
@ 2021-12-22  3:18   ` Jun Li
  -1 siblings, 0 replies; 24+ messages in thread
From: Jun Li @ 2021-12-22  3:18 UTC (permalink / raw)
  To: Alexander Stein, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
	Shawn Guo, Sascha Hauer, Fabio Estevam
  Cc: dl-linux-imx, linux-phy, devicetree, linux-arm-kernel



> -----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


^ permalink raw reply	[flat|nested] 24+ messages in thread

* RE: [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support
@ 2021-12-22  3:18   ` Jun Li
  0 siblings, 0 replies; 24+ messages in thread
From: Jun Li @ 2021-12-22  3:18 UTC (permalink / raw)
  To: Alexander Stein, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
	Shawn Guo, Sascha Hauer, Fabio Estevam
  Cc: dl-linux-imx, linux-phy, devicetree, linux-arm-kernel



> -----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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* RE: [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support
@ 2021-12-22  3:18   ` Jun Li
  0 siblings, 0 replies; 24+ messages in thread
From: Jun Li @ 2021-12-22  3:18 UTC (permalink / raw)
  To: Alexander Stein, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
	Shawn Guo, Sascha Hauer, Fabio Estevam
  Cc: dl-linux-imx, linux-phy, devicetree, linux-arm-kernel



> -----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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: (EXT) RE: [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support
  2021-12-22  3:18   ` Jun Li
  (?)
@ 2022-01-07 13:43     ` Alexander Stein
  -1 siblings, 0 replies; 24+ messages in thread
From: Alexander Stein @ 2022-01-07 13:43 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Vinod Koul, Rob Herring, Shawn Guo,
	Sascha Hauer, Fabio Estevam, linux-arm-kernel
  Cc: dl-linux-imx, linux-phy, devicetree, linux-arm-kernel, Jun Li

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




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: (EXT) RE: [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support
@ 2022-01-07 13:43     ` Alexander Stein
  0 siblings, 0 replies; 24+ messages in thread
From: Alexander Stein @ 2022-01-07 13:43 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Vinod Koul, Rob Herring, Shawn Guo,
	Sascha Hauer, Fabio Estevam, linux-arm-kernel
  Cc: dl-linux-imx, linux-phy, devicetree, linux-arm-kernel, Jun Li

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: (EXT) RE: [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support
@ 2022-01-07 13:43     ` Alexander Stein
  0 siblings, 0 replies; 24+ messages in thread
From: Alexander Stein @ 2022-01-07 13:43 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Vinod Koul, Rob Herring, Shawn Guo,
	Sascha Hauer, Fabio Estevam, linux-arm-kernel
  Cc: dl-linux-imx, linux-phy, devicetree, linux-arm-kernel, Jun Li

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: (EXT) Re: [PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags
  2021-12-21 16:59     ` Rob Herring
  (?)
@ 2022-01-07 13:50       ` Alexander Stein
  -1 siblings, 0 replies; 24+ messages in thread
From: Alexander Stein @ 2022-01-07 13:50 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Kishon Vijay Abraham I, Vinod Koul, Shawn Guo, Sascha Hauer,
	Fabio Estevam, NXP Linux Team, linux-phy, devicetree,
	linux-arm-kernel, Rob Herring

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




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: (EXT) Re: [PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags
@ 2022-01-07 13:50       ` Alexander Stein
  0 siblings, 0 replies; 24+ messages in thread
From: Alexander Stein @ 2022-01-07 13:50 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Kishon Vijay Abraham I, Vinod Koul, Shawn Guo, Sascha Hauer,
	Fabio Estevam, NXP Linux Team, linux-phy, devicetree,
	linux-arm-kernel, Rob Herring

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: (EXT) Re: [PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags
@ 2022-01-07 13:50       ` Alexander Stein
  0 siblings, 0 replies; 24+ messages in thread
From: Alexander Stein @ 2022-01-07 13:50 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Kishon Vijay Abraham I, Vinod Koul, Shawn Guo, Sascha Hauer,
	Fabio Estevam, NXP Linux Team, linux-phy, devicetree,
	linux-arm-kernel, Rob Herring

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2022-01-07 13:52 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-16 16:05 [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support Alexander Stein
2021-12-16 16:05 ` Alexander Stein
2021-12-16 16:05 ` Alexander Stein
2021-12-16 16:05 ` [PATCH v2 1/3] dt-bindings: phy: imx8mq-usb-phy: Add imx8mp specific flags Alexander Stein
2021-12-16 16:05   ` Alexander Stein
2021-12-16 16:05   ` Alexander Stein
2021-12-21 16:59   ` Rob Herring
2021-12-21 16:59     ` Rob Herring
2021-12-21 16:59     ` Rob Herring
2022-01-07 13:50     ` (EXT) " Alexander Stein
2022-01-07 13:50       ` Alexander Stein
2022-01-07 13:50       ` Alexander Stein
2021-12-16 16:05 ` [PATCH v2 2/3] phy: fsl-imx8mq-usb: Add support for setting fsl " Alexander Stein
2021-12-16 16:05   ` Alexander Stein
2021-12-16 16:05   ` Alexander Stein
2021-12-16 16:05 ` [PATCH v2 3/3] arm64: dts: imx8mp: Add memory for USB3 glue layer to usb3_phy nodes Alexander Stein
2021-12-16 16:05   ` Alexander Stein
2021-12-16 16:05   ` Alexander Stein
2021-12-22  3:18 ` [PATCH v2 0/3] i.MX8MP: more USB3 glue layer feature support Jun Li
2021-12-22  3:18   ` Jun Li
2021-12-22  3:18   ` Jun Li
2022-01-07 13:43   ` (EXT) " Alexander Stein
2022-01-07 13:43     ` Alexander Stein
2022-01-07 13:43     ` Alexander Stein

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.