Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v5 0/6] arm64: meson: Add support for USB on Amlogic A1
@ 2020-01-10  5:42 Hanjie Lin
  2020-01-10  5:42 ` [PATCH v5 1/6] dt-bindings: phy: Add Amlogic A1 USB2 PHY Bindings Hanjie Lin
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Hanjie Lin @ 2020-01-10  5:42 UTC (permalink / raw)
  To: Jerome Brunet, Neil Armstrong, Rob Herring, Greg Kroah-Hartman,
	Kevin Hilman
  Cc: devicetree, Hanjie Lin, Victor Wan, Jianxin Pan, Stephen Boyd,
	Michael Turquette, linux-usb, Yue Wang, Martin Blumenstingl,
	Liang Yang, Qiufang Dai, Xingyu Chen, Carlo Caione,
	linux-amlogic, linux-arm-kernel, Jian Hu

This patchset adds support for USB on Amlogic A1 SoCs.

This patchset is composed with :
- bindings of the PHY
- bindings of the USB Control Glue
- PHY Driver
- USB Control Glue driver
- dts of the PHY
- dts of the USB Controller

The Amlogic A1 USB Complex is composed of :
- 1 DWC3 USB controller for USB2 Host functionality
- 1 USB2 PHY for USB2 Host functionality

The USB Control Glue setups the clocks and the reset about DWC3 USB
controller, and binds to the USB2 PHY. It also configures the 8bit
UTMI interfaces for the USB2 PHY, including setting USB2 phy mode.

The USB2 PHY driver initializes the phy analog settings, phy PLL 
setup and phy tuning.

This patchset is based on A1 clock/power domain/reset series at [0].

Changes since v1:[1]
 - integrate glue and phy drivers into g12a's
 
Changes since v2:[2]
 - modify amlogic,meson-g12a-usb-ctrl.yaml with dt_binding_check tool
 - phy/glue driver use of_device_get_match_data to distinguish A1 from G12A

Changes since v3:[3]
 - fix bindings mistakes of the PHY according Rob's comments
 - fix bindings mistakes of the USB Control Glue according Rob's comments
 - phy driver add xtal_usb_phy clock which moved from glue driver
 - glue driver use otg_mode instead of soc_id to support otg function

Changes since v4:[4]
 - phy driver revert reset-names changes
 - phy driver change clock name to "xtal" to consistent with g12a
 - glue driver add drvdata otg_switch_supported

[0]
https://patchwork.kernel.org/project/linux-amlogic/list/?series=185477
https://patchwork.kernel.org/project/linux-amlogic/list/?series=180055
https://patchwork.kernel.org/project/linux-amlogic/list/?series=189643

[1] : https://lore.kernel.org/linux-amlogic/1574405757-76184-1-git-send-email-hanjie.lin@amlogic.com

[2] : https://lore.kernel.org/linux-amlogic/1576636944-196192-1-git-send-email-hanjie.lin@amlogic.com

[3] : https://lore.kernel.org/linux-amlogic/1577428606-69855-1-git-send-email-hanjie.lin@amlogic.com

[4] : https://lore.kernel.org/linux-amlogic/1578537045-23260-1-git-send-email-hanjie.lin@amlogic.com

Hanjie Lin (6):
  dt-bindings: phy: Add Amlogic A1 USB2 PHY Bindings
  dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings
  phy: amlogic: Add Amlogic A1 USB2 PHY Driver
  usb: dwc3: Add Amlogic A1 DWC3 glue
  arm64: dts: meson: a1: Enable USB2 PHY
  arm64: dts: meson: a1: Enable DWC3 controller

 .../bindings/phy/amlogic,meson-a1-usb2-phy.yaml    | 62 +++++++++++++++
 .../bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml  | 11 +++
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi          | 44 +++++++++++
 drivers/phy/amlogic/phy-meson-g12a-usb2.c          | 85 ++++++++++++++-------
 drivers/usb/dwc3/dwc3-meson-g12a.c                 | 89 ++++++++++++++++------
 5 files changed, 243 insertions(+), 48 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/amlogic,meson-a1-usb2-phy.yaml

-- 
2.7.4


_______________________________________________
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] 18+ messages in thread

* [PATCH v5 1/6] dt-bindings: phy: Add Amlogic A1 USB2 PHY Bindings
  2020-01-10  5:42 [PATCH v5 0/6] arm64: meson: Add support for USB on Amlogic A1 Hanjie Lin
@ 2020-01-10  5:42 ` Hanjie Lin
  2020-01-11 20:54   ` Martin Blumenstingl
  2020-01-10  5:42 ` [PATCH v5 2/6] dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings Hanjie Lin
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Hanjie Lin @ 2020-01-10  5:42 UTC (permalink / raw)
  To: Jerome Brunet, Neil Armstrong, Rob Herring, Greg Kroah-Hartman,
	Kevin Hilman
  Cc: devicetree, Hanjie Lin, Victor Wan, Jianxin Pan, Stephen Boyd,
	Michael Turquette, linux-usb, Yue Wang, Martin Blumenstingl,
	Liang Yang, Qiufang Dai, Xingyu Chen, Carlo Caione,
	linux-amlogic, linux-arm-kernel, Jian Hu

Add the Amlogic A1 Family USB2 PHY Bindings

It supports Host mode only.

Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
Signed-off-by: Yue Wang <yue.wang@amlogic.com>
---
 .../bindings/phy/amlogic,meson-a1-usb2-phy.yaml    | 62 ++++++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/amlogic,meson-a1-usb2-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/amlogic,meson-a1-usb2-phy.yaml b/Documentation/devicetree/bindings/phy/amlogic,meson-a1-usb2-phy.yaml
new file mode 100644
index 00000000..8f8f5d3
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/amlogic,meson-a1-usb2-phy.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2019 Amlogic, Inc
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/phy/amlogic,meson-a1-usb2-phy.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Amlogic A1 USB2 PHY
+
+maintainers:
+  - Yue Wang <yue.wang@amlogic.com>
+
+properties:
+  compatible:
+    const: amlogic,meson-a1-usb2-phy
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: xtal
+
+  resets:
+    maxItems: 1
+
+  reset-names:
+    items:
+     - const: phy
+
+  "#phy-cells":
+    const: 0
+
+  power-domains:
+     maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+  - "#phy-cells"
+  - power-domains
+
+examples:
+  - |
+    usb2_phy1: phy@40000 {
+      status = "okay";
+      compatible = "amlogic,a1-usb2-phy";
+      clocks = <&clkc_periphs 2>;
+      clock-names = "xtal";
+      reg = <0x0 0x40000 0x0 0x2000>;
+      resets = <&reset RESET_USBPHY>;
+      reset-names = "phy";
+      #phy-cells = <0>;
+      power-domains = <&pwrc PWRC_USB_ID>;
+    };
-- 
2.7.4


_______________________________________________
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] 18+ messages in thread

* [PATCH v5 2/6] dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings
  2020-01-10  5:42 [PATCH v5 0/6] arm64: meson: Add support for USB on Amlogic A1 Hanjie Lin
  2020-01-10  5:42 ` [PATCH v5 1/6] dt-bindings: phy: Add Amlogic A1 USB2 PHY Bindings Hanjie Lin
@ 2020-01-10  5:42 ` Hanjie Lin
  2020-01-11 20:50   ` Martin Blumenstingl
  2020-01-10  5:42 ` [PATCH v5 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver Hanjie Lin
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Hanjie Lin @ 2020-01-10  5:42 UTC (permalink / raw)
  To: Jerome Brunet, Neil Armstrong, Rob Herring, Greg Kroah-Hartman,
	Kevin Hilman
  Cc: devicetree, Hanjie Lin, Victor Wan, Jianxin Pan, Stephen Boyd,
	Michael Turquette, linux-usb, Yue Wang, Martin Blumenstingl,
	Liang Yang, Qiufang Dai, Xingyu Chen, Carlo Caione,
	linux-amlogic, linux-arm-kernel, Jian Hu

The Amlogic A1 SoC Family embeds 1 USB Controllers:
 - a DWC3 IP configured as Host for USB2 and USB3

A glue connects the controllers to the USB2 PHY of A1 SoC.

Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
Signed-off-by: Yue Wang <yue.wang@amlogic.com>
---
 .../devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml  | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml b/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml
index 4efb77b..f4595a3 100644
--- a/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml
+++ b/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml
@@ -9,6 +9,8 @@ title: Amlogic Meson G12A DWC3 USB SoC Controller Glue
 
 maintainers:
   - Neil Armstrong <narmstrong@baylibre.com>
+  - Hanjie Lin <hanjie.lin@amlogic.com>
+  - Yue Wang <yue.wang@amlogic.com>
 
 description: |
   The Amlogic G12A embeds a DWC3 USB IP Core configured for USB2 and USB3
@@ -22,10 +24,14 @@ description: |
   The DWC3 Glue controls the PHY routing and power, an interrupt line is
   connected to the Glue to serve as OTG ID change detection.
 
+  The Amlogic A1 embeds a DWC3 USB IP Core configured for USB2 in
+  host-only mode.
+
 properties:
   compatible:
     enum:
       - amlogic,meson-g12a-usb-ctrl
+      - amlogic,meson-a1-usb-ctrl
 
   ranges: true
 
@@ -37,6 +43,11 @@ properties:
 
   clocks:
     minItems: 1
+    maxItems: 4
+
+  clock-names:
+    minItems: 1
+    maxItems: 4
 
   resets:
     minItems: 1
-- 
2.7.4


_______________________________________________
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] 18+ messages in thread

* [PATCH v5 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver
  2020-01-10  5:42 [PATCH v5 0/6] arm64: meson: Add support for USB on Amlogic A1 Hanjie Lin
  2020-01-10  5:42 ` [PATCH v5 1/6] dt-bindings: phy: Add Amlogic A1 USB2 PHY Bindings Hanjie Lin
  2020-01-10  5:42 ` [PATCH v5 2/6] dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings Hanjie Lin
@ 2020-01-10  5:42 ` Hanjie Lin
  2020-01-11 20:36   ` Martin Blumenstingl
  2020-01-15  8:01   ` Neil Armstrong
  2020-01-10  5:42 ` [PATCH v5 4/6] usb: dwc3: Add Amlogic A1 DWC3 glue Hanjie Lin
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 18+ messages in thread
From: Hanjie Lin @ 2020-01-10  5:42 UTC (permalink / raw)
  To: Jerome Brunet, Neil Armstrong, Rob Herring, Greg Kroah-Hartman,
	Kevin Hilman
  Cc: devicetree, Hanjie Lin, Victor Wan, Jianxin Pan, Stephen Boyd,
	Michael Turquette, linux-usb, Yue Wang, Martin Blumenstingl,
	Liang Yang, Qiufang Dai, Xingyu Chen, Carlo Caione,
	linux-amlogic, linux-arm-kernel, Jian Hu

This adds support for the USB2 PHY found in the Amlogic A1 SoC Family.

It supports host mode only.

Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
Signed-off-by: Yue Wang <yue.wang@amlogic.com>
---
 drivers/phy/amlogic/phy-meson-g12a-usb2.c | 85 +++++++++++++++++++++----------
 1 file changed, 59 insertions(+), 26 deletions(-)

diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb2.c b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
index 9065ffc..33296f8 100644
--- a/drivers/phy/amlogic/phy-meson-g12a-usb2.c
+++ b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
@@ -146,11 +146,17 @@
 #define RESET_COMPLETE_TIME					1000
 #define PLL_RESET_COMPLETE_TIME					100
 
+enum meson_soc_id {
+	MESON_SOC_G12A  = 0,
+	MESON_SOC_A1,
+};
+
 struct phy_meson_g12a_usb2_priv {
 	struct device		*dev;
 	struct regmap		*regmap;
 	struct clk		*clk;
 	struct reset_control	*reset;
+	int                     soc_id;
 };
 
 static const struct regmap_config phy_meson_g12a_usb2_regmap_conf = {
@@ -164,6 +170,7 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
 {
 	struct phy_meson_g12a_usb2_priv *priv = phy_get_drvdata(phy);
 	int ret;
+	unsigned int value;
 
 	ret = reset_control_reset(priv->reset);
 	if (ret)
@@ -192,18 +199,22 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
 		     FIELD_PREP(PHY_CTRL_R17_MPLL_FILTER_PVT2, 2) |
 		     FIELD_PREP(PHY_CTRL_R17_MPLL_FILTER_PVT1, 9));
 
-	regmap_write(priv->regmap, PHY_CTRL_R18,
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_LKW_SEL, 1) |
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_LK_W, 9) |
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_LK_S, 0x27) |
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_PFD_GAIN, 1) |
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_ROU, 7) |
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_DATA_SEL, 3) |
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_BIAS_ADJ, 1) |
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_BB_MODE, 0) |
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_ALPHA, 3) |
-		     FIELD_PREP(PHY_CTRL_R18_MPLL_ADJ_LDO, 1) |
-		     PHY_CTRL_R18_MPLL_ACG_RANGE);
+	value = FIELD_PREP(PHY_CTRL_R18_MPLL_LKW_SEL, 1) |
+		FIELD_PREP(PHY_CTRL_R18_MPLL_LK_W, 9) |
+		FIELD_PREP(PHY_CTRL_R18_MPLL_LK_S, 0x27) |
+		FIELD_PREP(PHY_CTRL_R18_MPLL_PFD_GAIN, 1) |
+		FIELD_PREP(PHY_CTRL_R18_MPLL_ROU, 7) |
+		FIELD_PREP(PHY_CTRL_R18_MPLL_DATA_SEL, 3) |
+		FIELD_PREP(PHY_CTRL_R18_MPLL_BIAS_ADJ, 1) |
+		FIELD_PREP(PHY_CTRL_R18_MPLL_BB_MODE, 0) |
+		FIELD_PREP(PHY_CTRL_R18_MPLL_ALPHA, 3) |
+		FIELD_PREP(PHY_CTRL_R18_MPLL_ADJ_LDO, 1) |
+		PHY_CTRL_R18_MPLL_ACG_RANGE;
+
+	if (priv->soc_id == MESON_SOC_A1)
+		value |= PHY_CTRL_R18_MPLL_DCO_CLK_SEL;
+
+	regmap_write(priv->regmap, PHY_CTRL_R18, value);
 
 	udelay(PLL_RESET_COMPLETE_TIME);
 
@@ -227,13 +238,24 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
 		     FIELD_PREP(PHY_CTRL_R20_USB2_BGR_VREF_4_0, 0) |
 		     FIELD_PREP(PHY_CTRL_R20_USB2_BGR_DBG_1_0, 0));
 
-	regmap_write(priv->regmap, PHY_CTRL_R4,
-		     FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_7_0, 0xf) |
-		     FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_15_8, 0xf) |
-		     FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_23_16, 0xf) |
-		     PHY_CTRL_R4_TEST_BYPASS_MODE_EN |
-		     FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_1_0, 0) |
-		     FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_3_2, 0));
+	if (priv->soc_id == MESON_SOC_G12A)
+		regmap_write(priv->regmap, PHY_CTRL_R4,
+			     FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_7_0, 0xf) |
+			     FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_15_8, 0xf) |
+			     FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_23_16, 0xf) |
+			     PHY_CTRL_R4_TEST_BYPASS_MODE_EN |
+			     FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_1_0, 0) |
+			     FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_3_2, 0));
+	else if (priv->soc_id == MESON_SOC_A1) {
+		regmap_write(priv->regmap, PHY_CTRL_R21,
+			     PHY_CTRL_R21_USB2_CAL_ACK_EN |
+			     PHY_CTRL_R21_USB2_TX_STRG_PD |
+			     FIELD_PREP(PHY_CTRL_R21_USB2_OTG_ACA_TRIM_1_0, 2));
+
+		/* Analog Settings */
+		regmap_write(priv->regmap, PHY_CTRL_R13,
+			     FIELD_PREP(PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET, 7));
+	}
 
 	/* Tuning Disconnect Threshold */
 	regmap_write(priv->regmap, PHY_CTRL_R3,
@@ -241,11 +263,13 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
 		     FIELD_PREP(PHY_CTRL_R3_HSDIC_REF, 1) |
 		     FIELD_PREP(PHY_CTRL_R3_DISC_THRESH, 3));
 
-	/* Analog Settings */
-	regmap_write(priv->regmap, PHY_CTRL_R14, 0);
-	regmap_write(priv->regmap, PHY_CTRL_R13,
-		     PHY_CTRL_R13_UPDATE_PMA_SIGNALS |
-		     FIELD_PREP(PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET, 7));
+	if (priv->soc_id == MESON_SOC_G12A) {
+		/* Analog Settings */
+		regmap_write(priv->regmap, PHY_CTRL_R14, 0);
+		regmap_write(priv->regmap, PHY_CTRL_R13,
+			     PHY_CTRL_R13_UPDATE_PMA_SIGNALS |
+			     FIELD_PREP(PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET, 7));
+	}
 
 	return 0;
 }
@@ -286,6 +310,8 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
+	priv->soc_id = (enum meson_soc_id)of_device_get_match_data(&pdev->dev);
+
 	priv->regmap = devm_regmap_init_mmio(dev, base,
 					     &phy_meson_g12a_usb2_regmap_conf);
 	if (IS_ERR(priv->regmap))
@@ -321,8 +347,15 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id phy_meson_g12a_usb2_of_match[] = {
-	{ .compatible = "amlogic,g12a-usb2-phy", },
-	{ },
+	{
+		.compatible = "amlogic,g12a-usb2-phy",
+		.data = (void *)MESON_SOC_G12A,
+	},
+	{
+		.compatible = "amlogic,a1-usb2-phy",
+		.data = (void *)MESON_SOC_A1,
+	},
+	{ /* Sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, phy_meson_g12a_usb2_of_match);
 
-- 
2.7.4


_______________________________________________
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] 18+ messages in thread

* [PATCH v5 4/6] usb: dwc3: Add Amlogic A1 DWC3 glue
  2020-01-10  5:42 [PATCH v5 0/6] arm64: meson: Add support for USB on Amlogic A1 Hanjie Lin
                   ` (2 preceding siblings ...)
  2020-01-10  5:42 ` [PATCH v5 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver Hanjie Lin
@ 2020-01-10  5:42 ` Hanjie Lin
  2020-01-11 20:45   ` Martin Blumenstingl
  2020-01-10  5:42 ` [PATCH v5 5/6] arm64: dts: meson: a1: Enable USB2 PHY Hanjie Lin
  2020-01-10  5:42 ` [PATCH v5 6/6] arm64: dts: meson: a1: Enable DWC3 controller Hanjie Lin
  5 siblings, 1 reply; 18+ messages in thread
From: Hanjie Lin @ 2020-01-10  5:42 UTC (permalink / raw)
  To: Jerome Brunet, Neil Armstrong, Rob Herring, Greg Kroah-Hartman,
	Kevin Hilman
  Cc: devicetree, Hanjie Lin, Victor Wan, Jianxin Pan, Stephen Boyd,
	Michael Turquette, linux-usb, Yue Wang, Martin Blumenstingl,
	Liang Yang, Qiufang Dai, Xingyu Chen, Carlo Caione,
	linux-amlogic, linux-arm-kernel, Jian Hu

Adds support for Amlogic A1 USB Control Glue HW.

The Amlogic A1 SoC Family embeds 1 USB Controllers:
- a DWC3 IP configured as Host for USB2 and USB3

A glue connects the controllers to the USB2 PHY of A1 SoC.

Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
Signed-off-by: Yue Wang <yue.wang@amlogic.com>
---
 drivers/usb/dwc3/dwc3-meson-g12a.c | 89 ++++++++++++++++++++++++++++----------
 1 file changed, 67 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c
index 8a3ec1a..9294fde 100644
--- a/drivers/usb/dwc3/dwc3-meson-g12a.c
+++ b/drivers/usb/dwc3/dwc3-meson-g12a.c
@@ -107,10 +107,37 @@ static const char *phy_names[PHY_COUNT] = {
 	"usb2-phy0", "usb2-phy1", "usb3-phy0",
 };
 
+static struct clk_bulk_data meson_g12a_clocks[] = {
+	{ .id = NULL },
+};
+
+static struct clk_bulk_data meson_a1_clocks[] = {
+	{ .id = "usb_ctrl" },
+	{ .id = "usb_bus" },
+	{ .id = "xtal_usb_ctrl" },
+};
+
+struct dwc3_meson_g12a_drvdata {
+	bool otg_switch_supported;
+	struct clk_bulk_data *clks;
+	int num_clks;
+};
+
+static struct dwc3_meson_g12a_drvdata g12a_drvdata = {
+	.otg_switch_supported = true,
+	.clks = meson_g12a_clocks,
+	.num_clks = ARRAY_SIZE(meson_g12a_clocks),
+};
+
+static struct dwc3_meson_g12a_drvdata a1_drvdata = {
+	.otg_switch_supported = false,
+	.clks = meson_a1_clocks,
+	.num_clks = ARRAY_SIZE(meson_a1_clocks),
+};
+
 struct dwc3_meson_g12a {
 	struct device		*dev;
 	struct regmap		*regmap;
-	struct clk		*clk;
 	struct reset_control	*reset;
 	struct phy		*phys[PHY_COUNT];
 	enum usb_dr_mode	otg_mode;
@@ -120,6 +147,7 @@ struct dwc3_meson_g12a {
 	struct regulator	*vbus;
 	struct usb_role_switch_desc switch_desc;
 	struct usb_role_switch	*role_switch;
+	const struct dwc3_meson_g12a_drvdata *drvdata;
 };
 
 static void dwc3_meson_g12a_usb2_set_mode(struct dwc3_meson_g12a *priv,
@@ -151,7 +179,7 @@ static int dwc3_meson_g12a_usb2_init(struct dwc3_meson_g12a *priv)
 				   U2P_R0_POWER_ON_RESET,
 				   U2P_R0_POWER_ON_RESET);
 
-		if (i == USB2_OTG_PHY) {
+		if (priv->drvdata->otg_switch_supported && i == USB2_OTG_PHY) {
 			regmap_update_bits(priv->regmap,
 				U2P_R0 + (U2P_REG_SIZE * i),
 				U2P_R0_ID_PULLUP | U2P_R0_DRV_VBUS,
@@ -295,7 +323,7 @@ static int dwc3_meson_g12a_otg_mode_set(struct dwc3_meson_g12a *priv,
 {
 	int ret;
 
-	if (!priv->phys[USB2_OTG_PHY])
+	if (!priv->drvdata->otg_switch_supported || !priv->phys[USB2_OTG_PHY])
 		return -EINVAL;
 
 	if (mode == PHY_MODE_USB_HOST)
@@ -409,17 +437,18 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev)
 		priv->vbus = NULL;
 	}
 
-	priv->clk = devm_clk_get(dev, NULL);
-	if (IS_ERR(priv->clk))
-		return PTR_ERR(priv->clk);
+	priv->drvdata = of_device_get_match_data(&pdev->dev);
 
-	ret = clk_prepare_enable(priv->clk);
+	ret = devm_clk_bulk_get(dev,
+				priv->drvdata->num_clks,
+				priv->drvdata->clks);
 	if (ret)
 		return ret;
 
-	devm_add_action_or_reset(dev,
-				 (void(*)(void *))clk_disable_unprepare,
-				 priv->clk);
+	ret = clk_bulk_prepare_enable(priv->drvdata->num_clks,
+				      priv->drvdata->clks);
+	if (ret)
+		return ret;
 
 	platform_set_drvdata(pdev, priv);
 	priv->dev = dev;
@@ -433,16 +462,16 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev)
 
 	ret = reset_control_reset(priv->reset);
 	if (ret)
-		return ret;
+		goto err_disable_clks;
 
 	ret = dwc3_meson_g12a_get_phys(priv);
 	if (ret)
-		return ret;
+		goto err_disable_clks;
 
 	if (priv->vbus) {
 		ret = regulator_enable(priv->vbus);
 		if (ret)
-			return ret;
+			goto err_disable_clks;
 	}
 
 	/* Get dr_mode */
@@ -458,7 +487,7 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev)
 						dwc3_meson_g12a_irq_thread,
 						IRQF_ONESHOT, pdev->name, priv);
 		if (ret)
-			return ret;
+			goto err_disable_clks;
 	}
 
 	dwc3_meson_g12a_usb_init(priv);
@@ -467,7 +496,7 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev)
 	for (i = 0 ; i < PHY_COUNT ; ++i) {
 		ret = phy_init(priv->phys[i]);
 		if (ret)
-			return ret;
+			goto err_disable_clks;
 	}
 
 	/* Set PHY Power */
@@ -478,10 +507,11 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev)
 	}
 
 	ret = of_platform_populate(np, NULL, NULL, dev);
-	if (ret) {
-		clk_disable_unprepare(priv->clk);
+	if (ret)
 		goto err_phys_power;
-	}
+
+	if (!priv->drvdata->otg_switch_supported)
+		goto setup_pm_runtime;
 
 	/* Setup OTG mode corresponding to the ID pin */
 	if (priv->otg_mode == USB_DR_MODE_OTG) {
@@ -504,6 +534,7 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->role_switch))
 		dev_warn(dev, "Unable to register Role Switch\n");
 
+setup_pm_runtime:
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
 	pm_runtime_get_sync(dev);
@@ -518,6 +549,10 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev)
 	for (i = 0 ; i < PHY_COUNT ; ++i)
 		phy_exit(priv->phys[i]);
 
+err_disable_clks:
+	clk_bulk_disable_unprepare(priv->drvdata->num_clks,
+				   priv->drvdata->clks);
+
 	return ret;
 }
 
@@ -527,7 +562,8 @@ static int dwc3_meson_g12a_remove(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	int i;
 
-	usb_role_switch_unregister(priv->role_switch);
+	if (priv->drvdata->otg_switch_supported)
+		usb_role_switch_unregister(priv->role_switch);
 
 	of_platform_depopulate(dev);
 
@@ -547,7 +583,8 @@ static int __maybe_unused dwc3_meson_g12a_runtime_suspend(struct device *dev)
 {
 	struct dwc3_meson_g12a	*priv = dev_get_drvdata(dev);
 
-	clk_disable(priv->clk);
+	clk_bulk_disable_unprepare(priv->drvdata->num_clks,
+				   priv->drvdata->clks);
 
 	return 0;
 }
@@ -556,7 +593,8 @@ static int __maybe_unused dwc3_meson_g12a_runtime_resume(struct device *dev)
 {
 	struct dwc3_meson_g12a	*priv = dev_get_drvdata(dev);
 
-	return clk_enable(priv->clk);
+	return clk_bulk_prepare_enable(priv->drvdata->num_clks,
+				       priv->drvdata->clks);
 }
 
 static int __maybe_unused dwc3_meson_g12a_suspend(struct device *dev)
@@ -619,7 +657,14 @@ static const struct dev_pm_ops dwc3_meson_g12a_dev_pm_ops = {
 };
 
 static const struct of_device_id dwc3_meson_g12a_match[] = {
-	{ .compatible = "amlogic,meson-g12a-usb-ctrl" },
+	{
+		.compatible = "amlogic,meson-g12a-usb-ctrl",
+		.data = &g12a_drvdata,
+	},
+	{
+		.compatible = "amlogic,meson-a1-usb-ctrl",
+		.data = &a1_drvdata,
+	},
 	{ /* Sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, dwc3_meson_g12a_match);
-- 
2.7.4


_______________________________________________
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] 18+ messages in thread

* [PATCH v5 5/6] arm64: dts: meson: a1: Enable USB2 PHY
  2020-01-10  5:42 [PATCH v5 0/6] arm64: meson: Add support for USB on Amlogic A1 Hanjie Lin
                   ` (3 preceding siblings ...)
  2020-01-10  5:42 ` [PATCH v5 4/6] usb: dwc3: Add Amlogic A1 DWC3 glue Hanjie Lin
@ 2020-01-10  5:42 ` Hanjie Lin
  2020-01-10  5:42 ` [PATCH v5 6/6] arm64: dts: meson: a1: Enable DWC3 controller Hanjie Lin
  5 siblings, 0 replies; 18+ messages in thread
From: Hanjie Lin @ 2020-01-10  5:42 UTC (permalink / raw)
  To: Jerome Brunet, Neil Armstrong, Rob Herring, Greg Kroah-Hartman,
	Kevin Hilman
  Cc: devicetree, Hanjie Lin, Victor Wan, Jianxin Pan, Stephen Boyd,
	Michael Turquette, linux-usb, Yue Wang, Martin Blumenstingl,
	Liang Yang, Qiufang Dai, Xingyu Chen, Carlo Caione,
	linux-amlogic, linux-arm-kernel, Jian Hu

Enable USB2 PHY for Meson A1 SoC.

Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
Signed-off-by: Yue Wang <yue.wang@amlogic.com>
---
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
index 6fdc0dd..fb0ba85 100644
--- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
@@ -6,6 +6,7 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/power/meson-a1-power.h>
+#include <dt-bindings/reset/amlogic,meson-a1-reset.h>
 
 / {
 	compatible = "amlogic,a1";
@@ -100,6 +101,18 @@
 				#power-domain-cells = <1>;
 				status = "okay";
 			};
+
+			usb2_phy1: phy@40000 {
+				status = "okay";
+				compatible = "amlogic,a1-usb2-phy";
+				clocks = <&clkc_periphs CLKID_XTAL_USB_PHY>;
+				clock-names = "xtal";
+				reg = <0x0 0x40000 0x0 0x2000>;
+				resets = <&reset RESET_USBPHY>;
+				reset-names = "phy";
+				#phy-cells = <0>;
+				power-domains = <&pwrc PWRC_USB_ID>;
+			};
 		};
 
 		gic: interrupt-controller@ff901000 {
-- 
2.7.4


_______________________________________________
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] 18+ messages in thread

* [PATCH v5 6/6] arm64: dts: meson: a1: Enable DWC3 controller
  2020-01-10  5:42 [PATCH v5 0/6] arm64: meson: Add support for USB on Amlogic A1 Hanjie Lin
                   ` (4 preceding siblings ...)
  2020-01-10  5:42 ` [PATCH v5 5/6] arm64: dts: meson: a1: Enable USB2 PHY Hanjie Lin
@ 2020-01-10  5:42 ` Hanjie Lin
  5 siblings, 0 replies; 18+ messages in thread
From: Hanjie Lin @ 2020-01-10  5:42 UTC (permalink / raw)
  To: Jerome Brunet, Neil Armstrong, Rob Herring, Greg Kroah-Hartman,
	Kevin Hilman
  Cc: devicetree, Hanjie Lin, Victor Wan, Jianxin Pan, Stephen Boyd,
	Michael Turquette, linux-usb, Yue Wang, Martin Blumenstingl,
	Liang Yang, Qiufang Dai, Xingyu Chen, Carlo Caione,
	linux-amlogic, linux-arm-kernel, Jian Hu

Enable DWC3 controller for Meson A1 SoC.

Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
Signed-off-by: Yue Wang <yue.wang@amlogic.com>
---
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
index fb0ba85..9077ffa 100644
--- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
@@ -7,6 +7,8 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/power/meson-a1-power.h>
 #include <dt-bindings/reset/amlogic,meson-a1-reset.h>
+#include <dt-bindings/clock/a1-pll-clkc.h>
+#include <dt-bindings/clock/a1-clkc.h>
 
 / {
 	compatible = "amlogic,a1";
@@ -127,6 +129,35 @@
 			#interrupt-cells = <3>;
 			#address-cells = <0>;
 		};
+
+		usb: usb@ffe09000 {
+			status = "okay";
+			compatible = "amlogic,meson-a1-usb-ctrl";
+			reg = <0x0 0xffe09000 0x0 0xa0>;
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
+
+			clocks = <&clkc_periphs CLKID_USB_CTRL>,
+				 <&clkc_periphs CLKID_USB_BUS>,
+				 <&clkc_periphs CLKID_XTAL_USB_CTRL>;
+			clock-names = "usb_ctrl", "usb_bus", "xtal_usb_ctrl";
+			resets = <&reset RESET_USBCTRL>;
+
+			dr_mode = "host";
+
+			phys = <&usb2_phy1>;
+			phy-names = "usb2-phy1";
+
+			dwc3: usb@ff400000 {
+				compatible = "snps,dwc3";
+				reg = <0x0 0xff400000 0x0 0x100000>;
+				interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
+				dr_mode = "host";
+				snps,dis_u2_susphy_quirk;
+				snps,quirk-frame-length-adjustment = <0x20>;
+			};
+		};
 	};
 
 	timer {
-- 
2.7.4


_______________________________________________
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] 18+ messages in thread

* Re: [PATCH v5 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver
  2020-01-10  5:42 ` [PATCH v5 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver Hanjie Lin
@ 2020-01-11 20:36   ` Martin Blumenstingl
  2020-01-15  8:01   ` Neil Armstrong
  1 sibling, 0 replies; 18+ messages in thread
From: Martin Blumenstingl @ 2020-01-11 20:36 UTC (permalink / raw)
  To: Hanjie Lin
  Cc: Rob Herring, Victor Wan, Jianxin Pan, Neil Armstrong,
	Stephen Boyd, Kevin Hilman, Michael Turquette, linux-usb,
	Yue Wang, Qiufang Dai, devicetree, Liang Yang, Jian Hu,
	Xingyu Chen, Greg Kroah-Hartman, Carlo Caione, linux-amlogic,
	linux-arm-kernel, Jerome Brunet

On Fri, Jan 10, 2020 at 6:43 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
>
> This adds support for the USB2 PHY found in the Amlogic A1 SoC Family.
>
> It supports host mode only.
>
> Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
> Signed-off-by: Yue Wang <yue.wang@amlogic.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

_______________________________________________
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] 18+ messages in thread

* Re: [PATCH v5 4/6] usb: dwc3: Add Amlogic A1 DWC3 glue
  2020-01-10  5:42 ` [PATCH v5 4/6] usb: dwc3: Add Amlogic A1 DWC3 glue Hanjie Lin
@ 2020-01-11 20:45   ` Martin Blumenstingl
  2020-01-13  1:33     ` Hanjie Lin
  2020-01-15  8:01     ` Neil Armstrong
  0 siblings, 2 replies; 18+ messages in thread
From: Martin Blumenstingl @ 2020-01-11 20:45 UTC (permalink / raw)
  To: Hanjie Lin
  Cc: Rob Herring, Victor Wan, Jianxin Pan, Neil Armstrong,
	Stephen Boyd, Kevin Hilman, Michael Turquette, linux-usb,
	Yue Wang, Qiufang Dai, devicetree, Liang Yang, Jian Hu,
	Xingyu Chen, Greg Kroah-Hartman, Carlo Caione, linux-amlogic,
	linux-arm-kernel, Jerome Brunet

Hi Hanjie,

On Fri, Jan 10, 2020 at 6:43 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
[...]
> -       devm_add_action_or_reset(dev,
> -                                (void(*)(void *))clk_disable_unprepare,
> -                                priv->clk);
> +       ret = clk_bulk_prepare_enable(priv->drvdata->num_clks,
> +                                     priv->drvdata->clks);
I don't see clk_bulk_disable_unprepare in dwc3_meson_g12a_remove()
please test that the clocks are all disabled (see
/sys/kernel/debug/clk/clk_summary for example) when unloading all USB
related kernel modules

> +
> +       if (!priv->drvdata->otg_switch_supported)
> +               goto setup_pm_runtime;
my brain doesn't like the goto in this place because this is not an
error condition. I was about to write that
usb_role_switch_unregister() is now skipped even though we're calling
usb_role_switch_register().

I want to hear Neil's opinion on this because I got confused while
reading the code again.
my proposal is to move all of this OTG related code from
dwc3_meson_g12a_probe() into a new function, for example called
dwc3_meson_g12a_otg_init()
then only call that function when OTG switching is supported


Martin

_______________________________________________
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] 18+ messages in thread

* Re: [PATCH v5 2/6] dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings
  2020-01-10  5:42 ` [PATCH v5 2/6] dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings Hanjie Lin
@ 2020-01-11 20:50   ` Martin Blumenstingl
  2020-01-13  1:23     ` Hanjie Lin
  0 siblings, 1 reply; 18+ messages in thread
From: Martin Blumenstingl @ 2020-01-11 20:50 UTC (permalink / raw)
  To: Hanjie Lin
  Cc: Rob Herring, Victor Wan, Jianxin Pan, Neil Armstrong,
	Stephen Boyd, Kevin Hilman, Michael Turquette, linux-usb,
	Yue Wang, Qiufang Dai, devicetree, Liang Yang, Jian Hu,
	Xingyu Chen, Greg Kroah-Hartman, Carlo Caione, linux-amlogic,
	linux-arm-kernel, Jerome Brunet

Hi Hanjie,

On Fri, Jan 10, 2020 at 6:43 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
[...]
> @@ -37,6 +43,11 @@ properties:
>
>    clocks:
>      minItems: 1
> +    maxItems: 4
the driver parses one clock for G12A/G12B/SM1 and three clocks for A1
if there is a fourth clock: do we need to manage it in the driver?
(note: dt-bindings always represent the hardware, so if there's a
fourth clock which the driver doesn't need then it's perfectly valid
to describe it here. a comment which clock this is helps in the
code-review process)

> +  clock-names:
> +    minItems: 1
> +    maxItems: 4
I let Rob comment on this, personally I prefer naming the clocks explicitly
also I think clock-names has to be a mandatory property for A1 (see
Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-codec.yaml
for an example which makes properties mandatory depending on the
compatible string)


Martin

_______________________________________________
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] 18+ messages in thread

* Re: [PATCH v5 1/6] dt-bindings: phy: Add Amlogic A1 USB2 PHY Bindings
  2020-01-10  5:42 ` [PATCH v5 1/6] dt-bindings: phy: Add Amlogic A1 USB2 PHY Bindings Hanjie Lin
@ 2020-01-11 20:54   ` Martin Blumenstingl
  2020-01-13  1:13     ` Hanjie Lin
  0 siblings, 1 reply; 18+ messages in thread
From: Martin Blumenstingl @ 2020-01-11 20:54 UTC (permalink / raw)
  To: Hanjie Lin
  Cc: Rob Herring, Victor Wan, Jianxin Pan, Neil Armstrong,
	Stephen Boyd, Kevin Hilman, Michael Turquette, linux-usb,
	Yue Wang, Qiufang Dai, devicetree, Liang Yang, Jian Hu,
	Xingyu Chen, Greg Kroah-Hartman, Carlo Caione, linux-amlogic,
	linux-arm-kernel, Jerome Brunet

Hi Hanjie,

On Fri, Jan 10, 2020 at 6:43 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
>
> Add the Amlogic A1 Family USB2 PHY Bindings
>
> It supports Host mode only.
>
> Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
> Signed-off-by: Yue Wang <yue.wang@amlogic.com>
> ---
>  .../bindings/phy/amlogic,meson-a1-usb2-phy.yaml    | 62 ++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/amlogic,meson-a1-usb2-phy.yaml
there are only two differences to the existing
amlogic,meson-g12a-usb2-phy.yaml binding:
- different compatible string (the existing binding already has an
enum, so that would be easy to extend)
- new, mandatory power-domains property
(Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-codec.yaml
has examples how to make properties mandatory based on the compatible
string)

have you considered merging this with the existing
amlogic,meson-g12a-usb2-phy.yaml binding?
this is not a "must have" in my opinion, I still want to hear your
opinion on this topic!


Martin

_______________________________________________
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] 18+ messages in thread

* Re: [PATCH v5 1/6] dt-bindings: phy: Add Amlogic A1 USB2 PHY Bindings
  2020-01-11 20:54   ` Martin Blumenstingl
@ 2020-01-13  1:13     ` Hanjie Lin
  0 siblings, 0 replies; 18+ messages in thread
From: Hanjie Lin @ 2020-01-13  1:13 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: Rob Herring, Victor Wan, Jianxin Pan, Neil Armstrong,
	Stephen Boyd, Kevin Hilman, Michael Turquette, linux-usb,
	Yue Wang, Qiufang Dai, devicetree, Liang Yang, Jian Hu,
	Xingyu Chen, Greg Kroah-Hartman, Carlo Caione, linux-amlogic,
	linux-arm-kernel, Jerome Brunet



On 2020/1/12 4:54, Martin Blumenstingl wrote:
> Hi Hanjie,
> 
> On Fri, Jan 10, 2020 at 6:43 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
>>
>> Add the Amlogic A1 Family USB2 PHY Bindings
>>
>> It supports Host mode only.
>>
>> Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
>> Signed-off-by: Yue Wang <yue.wang@amlogic.com>
>> ---
>>  .../bindings/phy/amlogic,meson-a1-usb2-phy.yaml    | 62 ++++++++++++++++++++++
>>  1 file changed, 62 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/phy/amlogic,meson-a1-usb2-phy.yaml
> there are only two differences to the existing
> amlogic,meson-g12a-usb2-phy.yaml binding:
> - different compatible string (the existing binding already has an
> enum, so that would be easy to extend)
> - new, mandatory power-domains property
> (Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-codec.yaml
> has examples how to make properties mandatory based on the compatible
> string)
> 
> have you considered merging this with the existing
> amlogic,meson-g12a-usb2-phy.yaml binding?
> this is not a "must have" in my opinion, I still want to hear your
> opinion on this topic!
> 
> 
> Martin
> 
> .
> 


Hi Martin,

Thanks for your advice.

Of course, it should looks much better to have merging this into the existing
amlogic,meson-g12a-usb2-phy.yaml.
I will try to do it by following the examples.

thanks,
Hanjie

_______________________________________________
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] 18+ messages in thread

* Re: [PATCH v5 2/6] dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings
  2020-01-11 20:50   ` Martin Blumenstingl
@ 2020-01-13  1:23     ` Hanjie Lin
  2020-01-14 20:10       ` Martin Blumenstingl
  0 siblings, 1 reply; 18+ messages in thread
From: Hanjie Lin @ 2020-01-13  1:23 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: Rob Herring, Victor Wan, Jianxin Pan, Neil Armstrong,
	Stephen Boyd, Kevin Hilman, Michael Turquette, linux-usb,
	Yue Wang, Qiufang Dai, devicetree, Liang Yang, Jian Hu,
	Xingyu Chen, Greg Kroah-Hartman, Carlo Caione, linux-amlogic,
	linux-arm-kernel, Jerome Brunet



On 2020/1/12 4:50, Martin Blumenstingl wrote:
> Hi Hanjie,
> 
> On Fri, Jan 10, 2020 at 6:43 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
> [...]
>> @@ -37,6 +43,11 @@ properties:
>>
>>    clocks:
>>      minItems: 1
>> +    maxItems: 4
> the driver parses one clock for G12A/G12B/SM1 and three clocks for A1
> if there is a fourth clock: do we need to manage it in the driver?
> (note: dt-bindings always represent the hardware, so if there's a
> fourth clock which the driver doesn't need then it's perfectly valid
> to describe it here. a comment which clock this is helps in the
> code-review process)
> 

Hi Martin,

Sorry for this confusing, I moved xtal_usb_phy clock from glue driver to phy,
but I missed this binding modification. 
So actually A1 do only need these three clocks and no fourth clock exist, clock
and clock-names maxItems should be three here for A1.

>> +  clock-names:
>> +    minItems: 1
>> +    maxItems: 4
> I let Rob comment on this, personally I prefer naming the clocks explicitly
> also I think clock-names has to be a mandatory property for A1 (see
> Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-codec.yaml
> for an example which makes properties mandatory depending on the
> compatible string)
> 
> 
> Martin
> 
> .
> 

Thanks for your patient and detailed advice.

Do you mean something like this:

+allOf:
+  - if:
+      properties:
+        compatible:
+          enum:
+            - amlogic,meson-g12a-usb-ctrl
+
+    then:
+      properties:
+        clocks:
+         minItems: 1
+
+  - if:
+      properties:
+        compatible:
+          enum:
+            - amlogic,meson-a1-usb-ctrl
+
+    then:
+      properties:
+        clocks:
+          items:
+            minItems: 3
+       clock-names:
+          items:
+            - const: usb_ctrl
+            - const: usb_bus
+            - const: xtal_usb_ctrl
+      required:
+        - clock-names


Hanjie

_______________________________________________
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] 18+ messages in thread

* Re: [PATCH v5 4/6] usb: dwc3: Add Amlogic A1 DWC3 glue
  2020-01-11 20:45   ` Martin Blumenstingl
@ 2020-01-13  1:33     ` Hanjie Lin
  2020-01-15  8:01     ` Neil Armstrong
  1 sibling, 0 replies; 18+ messages in thread
From: Hanjie Lin @ 2020-01-13  1:33 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: Rob Herring, Victor Wan, Jianxin Pan, Neil Armstrong,
	Stephen Boyd, Kevin Hilman, Michael Turquette, linux-usb,
	Yue Wang, devicetree, Qiufang Dai, Jian Hu, linux-arm-kernel,
	Greg Kroah-Hartman, Carlo Caione, linux-amlogic, Liang Yang,
	Xingyu Chen, Jerome Brunet



On 2020/1/12 4:45, Martin Blumenstingl wrote:
> Hi Hanjie,
> 
> On Fri, Jan 10, 2020 at 6:43 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
> [...]
>> -       devm_add_action_or_reset(dev,
>> -                                (void(*)(void *))clk_disable_unprepare,
>> -                                priv->clk);
>> +       ret = clk_bulk_prepare_enable(priv->drvdata->num_clks,
>> +                                     priv->drvdata->clks);
> I don't see clk_bulk_disable_unprepare in dwc3_meson_g12a_remove()
> please test that the clocks are all disabled (see
> /sys/kernel/debug/clk/clk_summary for example) when unloading all USB
> related kernel modules
> 

Ok, 

I will check and fix this if missing here.

Thanks,

Hanjie


>> +
>> +       if (!priv->drvdata->otg_switch_supported)
>> +               goto setup_pm_runtime;
> my brain doesn't like the goto in this place because this is not an
> error condition. I was about to write that
> usb_role_switch_unregister() is now skipped even though we're calling
> usb_role_switch_register().
> 
> I want to hear Neil's opinion on this because I got confused while
> reading the code again.
> my proposal is to move all of this OTG related code from
> dwc3_meson_g12a_probe() into a new function, for example called
> dwc3_meson_g12a_otg_init()
> then only call that function when OTG switching is supported
> 
> 
> Martin
> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
> 
> .
> 

_______________________________________________
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] 18+ messages in thread

* Re: [PATCH v5 2/6] dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings
  2020-01-13  1:23     ` Hanjie Lin
@ 2020-01-14 20:10       ` Martin Blumenstingl
  0 siblings, 0 replies; 18+ messages in thread
From: Martin Blumenstingl @ 2020-01-14 20:10 UTC (permalink / raw)
  To: Hanjie Lin
  Cc: Rob Herring, Victor Wan, Jianxin Pan, Neil Armstrong,
	Stephen Boyd, Kevin Hilman, Michael Turquette, linux-usb,
	Yue Wang, Qiufang Dai, devicetree, Liang Yang, Jian Hu,
	Xingyu Chen, Greg Kroah-Hartman, Carlo Caione, linux-amlogic,
	linux-arm-kernel, Jerome Brunet

Hi Hanjie,

On Mon, Jan 13, 2020 at 2:23 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
>
>
>
> On 2020/1/12 4:50, Martin Blumenstingl wrote:
> > Hi Hanjie,
> >
> > On Fri, Jan 10, 2020 at 6:43 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
> > [...]
> >> @@ -37,6 +43,11 @@ properties:
> >>
> >>    clocks:
> >>      minItems: 1
> >> +    maxItems: 4
> > the driver parses one clock for G12A/G12B/SM1 and three clocks for A1
> > if there is a fourth clock: do we need to manage it in the driver?
> > (note: dt-bindings always represent the hardware, so if there's a
> > fourth clock which the driver doesn't need then it's perfectly valid
> > to describe it here. a comment which clock this is helps in the
> > code-review process)
> >
>
> Hi Martin,
>
> Sorry for this confusing, I moved xtal_usb_phy clock from glue driver to phy,
> but I missed this binding modification.
> So actually A1 do only need these three clocks and no fourth clock exist, clock
> and clock-names maxItems should be three here for A1.
I see, thank you for clarifying this!

[...]
> Do you mean something like this:
>
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - amlogic,meson-g12a-usb-ctrl
> +
> +    then:
> +      properties:
> +        clocks:
> +         minItems: 1
> +
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - amlogic,meson-a1-usb-ctrl
> +
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            minItems: 3
> +       clock-names:
> +          items:
> +            - const: usb_ctrl
> +            - const: usb_bus
> +            - const: xtal_usb_ctrl
> +      required:
> +        - clock-names
this looks good to me (but keep in mind that I'm no expert on these
yaml schemas)
I wonder if we are allowed to shorten this by having one clocks
property with minItems: 1 and maxItems: 3 (like you have in the
original patch) and then only have a
amlogic,meson-a1-usb-ctrl-specific "clock-names" property (and make
that mandatory for A1 SoCs)


Martin

_______________________________________________
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] 18+ messages in thread

* Re: [PATCH v5 4/6] usb: dwc3: Add Amlogic A1 DWC3 glue
  2020-01-11 20:45   ` Martin Blumenstingl
  2020-01-13  1:33     ` Hanjie Lin
@ 2020-01-15  8:01     ` Neil Armstrong
  2020-01-16  2:12       ` Hanjie Lin
  1 sibling, 1 reply; 18+ messages in thread
From: Neil Armstrong @ 2020-01-15  8:01 UTC (permalink / raw)
  To: Martin Blumenstingl, Hanjie Lin
  Cc: Rob Herring, Victor Wan, Jianxin Pan, devicetree, Stephen Boyd,
	Kevin Hilman, Michael Turquette, linux-usb, Yue Wang,
	Qiufang Dai, Liang Yang, Jian Hu, Xingyu Chen,
	Greg Kroah-Hartman, Carlo Caione, linux-amlogic,
	linux-arm-kernel, Jerome Brunet

On 11/01/2020 21:45, Martin Blumenstingl wrote:
> Hi Hanjie,
> 
> On Fri, Jan 10, 2020 at 6:43 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
> [...]
>> -       devm_add_action_or_reset(dev,
>> -                                (void(*)(void *))clk_disable_unprepare,
>> -                                priv->clk);
>> +       ret = clk_bulk_prepare_enable(priv->drvdata->num_clks,
>> +                                     priv->drvdata->clks);
> I don't see clk_bulk_disable_unprepare in dwc3_meson_g12a_remove()
> please test that the clocks are all disabled (see
> /sys/kernel/debug/clk/clk_summary for example) when unloading all USB
> related kernel modules
> 
>> +
>> +       if (!priv->drvdata->otg_switch_supported)
>> +               goto setup_pm_runtime;
> my brain doesn't like the goto in this place because this is not an
> error condition. I was about to write that
> usb_role_switch_unregister() is now skipped even though we're calling
> usb_role_switch_register().
> 
> I want to hear Neil's opinion on this because I got confused while
> reading the code again.
> my proposal is to move all of this OTG related code from
> dwc3_meson_g12a_probe() into a new function, for example called
> dwc3_meson_g12a_otg_init()
> then only call that function when OTG switching is supported

Indeed it's not cleanest way to do that, if you respin a v6, put all the OTG
setup and role switch register in a separate function.

with that and :clk_bulk_disable_unprepare() in remove:
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

Neil

> 
> 
> Martin
> 


_______________________________________________
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] 18+ messages in thread

* Re: [PATCH v5 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver
  2020-01-10  5:42 ` [PATCH v5 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver Hanjie Lin
  2020-01-11 20:36   ` Martin Blumenstingl
@ 2020-01-15  8:01   ` Neil Armstrong
  1 sibling, 0 replies; 18+ messages in thread
From: Neil Armstrong @ 2020-01-15  8:01 UTC (permalink / raw)
  To: Hanjie Lin, Jerome Brunet, Rob Herring, Greg Kroah-Hartman, Kevin Hilman
  Cc: devicetree, Victor Wan, Jianxin Pan, Stephen Boyd,
	Michael Turquette, linux-usb, Yue Wang, Martin Blumenstingl,
	Liang Yang, Qiufang Dai, Xingyu Chen, Carlo Caione,
	linux-amlogic, linux-arm-kernel, Jian Hu

On 10/01/2020 06:42, Hanjie Lin wrote:
> This adds support for the USB2 PHY found in the Amlogic A1 SoC Family.
> 
> It supports host mode only.
> 
> Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
> Signed-off-by: Yue Wang <yue.wang@amlogic.com>
> ---
>  drivers/phy/amlogic/phy-meson-g12a-usb2.c | 85 +++++++++++++++++++++----------
>  1 file changed, 59 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb2.c b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
> index 9065ffc..33296f8 100644
> --- a/drivers/phy/amlogic/phy-meson-g12a-usb2.c
> +++ b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
> @@ -146,11 +146,17 @@
>  #define RESET_COMPLETE_TIME					1000
>  #define PLL_RESET_COMPLETE_TIME					100
>  
> +enum meson_soc_id {
> +	MESON_SOC_G12A  = 0,
> +	MESON_SOC_A1,
> +};
> +
>  struct phy_meson_g12a_usb2_priv {
>  	struct device		*dev;
>  	struct regmap		*regmap;
>  	struct clk		*clk;
>  	struct reset_control	*reset;
> +	int                     soc_id;
>  };
>  
>  static const struct regmap_config phy_meson_g12a_usb2_regmap_conf = {
> @@ -164,6 +170,7 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
>  {
>  	struct phy_meson_g12a_usb2_priv *priv = phy_get_drvdata(phy);
>  	int ret;
> +	unsigned int value;
>  
>  	ret = reset_control_reset(priv->reset);
>  	if (ret)
> @@ -192,18 +199,22 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
>  		     FIELD_PREP(PHY_CTRL_R17_MPLL_FILTER_PVT2, 2) |
>  		     FIELD_PREP(PHY_CTRL_R17_MPLL_FILTER_PVT1, 9));
>  
> -	regmap_write(priv->regmap, PHY_CTRL_R18,
> -		     FIELD_PREP(PHY_CTRL_R18_MPLL_LKW_SEL, 1) |
> -		     FIELD_PREP(PHY_CTRL_R18_MPLL_LK_W, 9) |
> -		     FIELD_PREP(PHY_CTRL_R18_MPLL_LK_S, 0x27) |
> -		     FIELD_PREP(PHY_CTRL_R18_MPLL_PFD_GAIN, 1) |
> -		     FIELD_PREP(PHY_CTRL_R18_MPLL_ROU, 7) |
> -		     FIELD_PREP(PHY_CTRL_R18_MPLL_DATA_SEL, 3) |
> -		     FIELD_PREP(PHY_CTRL_R18_MPLL_BIAS_ADJ, 1) |
> -		     FIELD_PREP(PHY_CTRL_R18_MPLL_BB_MODE, 0) |
> -		     FIELD_PREP(PHY_CTRL_R18_MPLL_ALPHA, 3) |
> -		     FIELD_PREP(PHY_CTRL_R18_MPLL_ADJ_LDO, 1) |
> -		     PHY_CTRL_R18_MPLL_ACG_RANGE);
> +	value = FIELD_PREP(PHY_CTRL_R18_MPLL_LKW_SEL, 1) |
> +		FIELD_PREP(PHY_CTRL_R18_MPLL_LK_W, 9) |
> +		FIELD_PREP(PHY_CTRL_R18_MPLL_LK_S, 0x27) |
> +		FIELD_PREP(PHY_CTRL_R18_MPLL_PFD_GAIN, 1) |
> +		FIELD_PREP(PHY_CTRL_R18_MPLL_ROU, 7) |
> +		FIELD_PREP(PHY_CTRL_R18_MPLL_DATA_SEL, 3) |
> +		FIELD_PREP(PHY_CTRL_R18_MPLL_BIAS_ADJ, 1) |
> +		FIELD_PREP(PHY_CTRL_R18_MPLL_BB_MODE, 0) |
> +		FIELD_PREP(PHY_CTRL_R18_MPLL_ALPHA, 3) |
> +		FIELD_PREP(PHY_CTRL_R18_MPLL_ADJ_LDO, 1) |
> +		PHY_CTRL_R18_MPLL_ACG_RANGE;
> +
> +	if (priv->soc_id == MESON_SOC_A1)
> +		value |= PHY_CTRL_R18_MPLL_DCO_CLK_SEL;
> +
> +	regmap_write(priv->regmap, PHY_CTRL_R18, value);
>  
>  	udelay(PLL_RESET_COMPLETE_TIME);
>  
> @@ -227,13 +238,24 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
>  		     FIELD_PREP(PHY_CTRL_R20_USB2_BGR_VREF_4_0, 0) |
>  		     FIELD_PREP(PHY_CTRL_R20_USB2_BGR_DBG_1_0, 0));
>  
> -	regmap_write(priv->regmap, PHY_CTRL_R4,
> -		     FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_7_0, 0xf) |
> -		     FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_15_8, 0xf) |
> -		     FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_23_16, 0xf) |
> -		     PHY_CTRL_R4_TEST_BYPASS_MODE_EN |
> -		     FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_1_0, 0) |
> -		     FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_3_2, 0));
> +	if (priv->soc_id == MESON_SOC_G12A)
> +		regmap_write(priv->regmap, PHY_CTRL_R4,
> +			     FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_7_0, 0xf) |
> +			     FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_15_8, 0xf) |
> +			     FIELD_PREP(PHY_CTRL_R4_CALIB_CODE_23_16, 0xf) |
> +			     PHY_CTRL_R4_TEST_BYPASS_MODE_EN |
> +			     FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_1_0, 0) |
> +			     FIELD_PREP(PHY_CTRL_R4_I_C2L_BIAS_TRIM_3_2, 0));
> +	else if (priv->soc_id == MESON_SOC_A1) {
> +		regmap_write(priv->regmap, PHY_CTRL_R21,
> +			     PHY_CTRL_R21_USB2_CAL_ACK_EN |
> +			     PHY_CTRL_R21_USB2_TX_STRG_PD |
> +			     FIELD_PREP(PHY_CTRL_R21_USB2_OTG_ACA_TRIM_1_0, 2));
> +
> +		/* Analog Settings */
> +		regmap_write(priv->regmap, PHY_CTRL_R13,
> +			     FIELD_PREP(PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET, 7));
> +	}
>  
>  	/* Tuning Disconnect Threshold */
>  	regmap_write(priv->regmap, PHY_CTRL_R3,
> @@ -241,11 +263,13 @@ static int phy_meson_g12a_usb2_init(struct phy *phy)
>  		     FIELD_PREP(PHY_CTRL_R3_HSDIC_REF, 1) |
>  		     FIELD_PREP(PHY_CTRL_R3_DISC_THRESH, 3));
>  
> -	/* Analog Settings */
> -	regmap_write(priv->regmap, PHY_CTRL_R14, 0);
> -	regmap_write(priv->regmap, PHY_CTRL_R13,
> -		     PHY_CTRL_R13_UPDATE_PMA_SIGNALS |
> -		     FIELD_PREP(PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET, 7));
> +	if (priv->soc_id == MESON_SOC_G12A) {
> +		/* Analog Settings */
> +		regmap_write(priv->regmap, PHY_CTRL_R14, 0);
> +		regmap_write(priv->regmap, PHY_CTRL_R13,
> +			     PHY_CTRL_R13_UPDATE_PMA_SIGNALS |
> +			     FIELD_PREP(PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET, 7));
> +	}
>  
>  	return 0;
>  }
> @@ -286,6 +310,8 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
>  	if (IS_ERR(base))
>  		return PTR_ERR(base);
>  
> +	priv->soc_id = (enum meson_soc_id)of_device_get_match_data(&pdev->dev);
> +
>  	priv->regmap = devm_regmap_init_mmio(dev, base,
>  					     &phy_meson_g12a_usb2_regmap_conf);
>  	if (IS_ERR(priv->regmap))
> @@ -321,8 +347,15 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id phy_meson_g12a_usb2_of_match[] = {
> -	{ .compatible = "amlogic,g12a-usb2-phy", },
> -	{ },
> +	{
> +		.compatible = "amlogic,g12a-usb2-phy",
> +		.data = (void *)MESON_SOC_G12A,
> +	},
> +	{
> +		.compatible = "amlogic,a1-usb2-phy",
> +		.data = (void *)MESON_SOC_A1,
> +	},
> +	{ /* Sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, phy_meson_g12a_usb2_of_match);
>  
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

_______________________________________________
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] 18+ messages in thread

* Re: [PATCH v5 4/6] usb: dwc3: Add Amlogic A1 DWC3 glue
  2020-01-15  8:01     ` Neil Armstrong
@ 2020-01-16  2:12       ` Hanjie Lin
  0 siblings, 0 replies; 18+ messages in thread
From: Hanjie Lin @ 2020-01-16  2:12 UTC (permalink / raw)
  To: Neil Armstrong, Martin Blumenstingl
  Cc: Rob Herring, Victor Wan, Jianxin Pan, devicetree, Stephen Boyd,
	Kevin Hilman, Michael Turquette, linux-usb, Yue Wang,
	Qiufang Dai, Jian Hu, linux-arm-kernel, Greg Kroah-Hartman,
	Carlo Caione, linux-amlogic, Liang Yang, Xingyu Chen,
	Jerome Brunet



On 2020/1/15 16:01, Neil Armstrong wrote:
> On 11/01/2020 21:45, Martin Blumenstingl wrote:
>> Hi Hanjie,
>>
>> On Fri, Jan 10, 2020 at 6:43 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
>> [...]
>>> -       devm_add_action_or_reset(dev,
>>> -                                (void(*)(void *))clk_disable_unprepare,
>>> -                                priv->clk);
>>> +       ret = clk_bulk_prepare_enable(priv->drvdata->num_clks,
>>> +                                     priv->drvdata->clks);
>> I don't see clk_bulk_disable_unprepare in dwc3_meson_g12a_remove()
>> please test that the clocks are all disabled (see
>> /sys/kernel/debug/clk/clk_summary for example) when unloading all USB
>> related kernel modules
>>
>>> +
>>> +       if (!priv->drvdata->otg_switch_supported)
>>> +               goto setup_pm_runtime;
>> my brain doesn't like the goto in this place because this is not an
>> error condition. I was about to write that
>> usb_role_switch_unregister() is now skipped even though we're calling
>> usb_role_switch_register().
>>
>> I want to hear Neil's opinion on this because I got confused while
>> reading the code again.
>> my proposal is to move all of this OTG related code from
>> dwc3_meson_g12a_probe() into a new function, for example called
>> dwc3_meson_g12a_otg_init()
>> then only call that function when OTG switching is supported
> 
> Indeed it's not cleanest way to do that, if you respin a v6, put all the OTG
> setup and role switch register in a separate function.
> 
> with that and :clk_bulk_disable_unprepare() in remove:
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
> 
> Neil
> 

Ok, 

I will do it in v6 patch.

Thanks,
Hanjie

>>
>>
>> Martin
>>
> 
> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
> 
> .
> 

_______________________________________________
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] 18+ messages in thread

end of thread, back to index

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-10  5:42 [PATCH v5 0/6] arm64: meson: Add support for USB on Amlogic A1 Hanjie Lin
2020-01-10  5:42 ` [PATCH v5 1/6] dt-bindings: phy: Add Amlogic A1 USB2 PHY Bindings Hanjie Lin
2020-01-11 20:54   ` Martin Blumenstingl
2020-01-13  1:13     ` Hanjie Lin
2020-01-10  5:42 ` [PATCH v5 2/6] dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings Hanjie Lin
2020-01-11 20:50   ` Martin Blumenstingl
2020-01-13  1:23     ` Hanjie Lin
2020-01-14 20:10       ` Martin Blumenstingl
2020-01-10  5:42 ` [PATCH v5 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver Hanjie Lin
2020-01-11 20:36   ` Martin Blumenstingl
2020-01-15  8:01   ` Neil Armstrong
2020-01-10  5:42 ` [PATCH v5 4/6] usb: dwc3: Add Amlogic A1 DWC3 glue Hanjie Lin
2020-01-11 20:45   ` Martin Blumenstingl
2020-01-13  1:33     ` Hanjie Lin
2020-01-15  8:01     ` Neil Armstrong
2020-01-16  2:12       ` Hanjie Lin
2020-01-10  5:42 ` [PATCH v5 5/6] arm64: dts: meson: a1: Enable USB2 PHY Hanjie Lin
2020-01-10  5:42 ` [PATCH v5 6/6] arm64: dts: meson: a1: Enable DWC3 controller Hanjie Lin

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org
	public-inbox-index linux-arm-kernel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git