[PATCH v2 0/6] arm64: meson: Add support for USB on Amlogic A1

[PATCH v2 0/6] arm64: meson: Add support for USB on Amlogic A1

[Hanjie Lin]

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

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

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    |  55 +++++++++++
 .../bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml  |  32 +++++++
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi          |  44 +++++++++
 drivers/phy/amlogic/phy-meson-g12a-usb2.c          | 102 +++++++++++++++------
 drivers/usb/dwc3/dwc3-meson-g12a.c                 |  69 ++++++++++----
 5 files changed, 254 insertions(+), 48 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/amlogic,meson-a1-usb2-phy.yaml

-- 
2.7.4

[PATCH v2 1/6] dt-bindings: phy: Add Amlogic A1 USB2 PHY Bindings

[Hanjie Lin]

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    | 55 ++++++++++++++++++++++
 1 file changed, 55 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..7a66e8a
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/amlogic,meson-a1-usb2-phy.yaml
@@ -0,0 +1,55 @@
+# 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:
+    enum:
+      - amlogic,meson-a1-usb2-phy
+
+  reg:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  reset-names:
+    items:
+      - const: phy
+
+  "#phy-cells":
+    const: 0
+
+  power-domains:
+     maxItems: 1
+     description:
+       a phandle to respective power domain node as described by generic
+       PM domain bindings (see power/power_domain.txt for more information).
+
+required:
+  - compatible
+  - reg
+  - resets
+  - reset-names
+  - "#phy-cells"
+  - power-domains
+
+examples:
+  - |
+    usb2_phy0: phy@40000 {
+      status = "okay";
+      compatible = "amlogic,a1-usb2-phy";
+      reg = <0x0 0x40000 0x0 0x2000>;
+      resets = <&reset RESET_USBPHY>;
+      reset-names = "phy";
+      #phy-cells = <0>;
+      power-domains = <&pwrc PWRC_USB_ID>;
+    };
-- 
2.7.4

[PATCH v2 2/6] dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings

[Hanjie Lin]

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>
---
 .../bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml  | 32 ++++++++++++++++++++++
 1 file changed, 32 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..9740027 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
 
@@ -124,4 +130,30 @@ examples:
               snps,quirk-frame-length-adjustment;
           };
     };
+  - |
+    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_PHY>,
+           <&clkc_periphs CLKID_XTAL_USB_CTRL>;
+          clock-names = "usb_ctrl", "usb_bus", "xtal_usb_phy", "xtal_usb_ctrl";
+          resets = <&reset RESET_USBCTRL>;
+          phys = <&usb2_phy0>;
+          phy-names = "usb2-phy0";
+
+          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>;
+          };
+  };
-- 
2.7.4

[PATCH v2 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver

[Hanjie Lin]

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 | 102 ++++++++++++++++++++++--------
 1 file changed, 74 insertions(+), 28 deletions(-)

diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb2.c b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
index 9065ffc..2c242d3 100644
--- a/drivers/phy/amlogic/phy-meson-g12a-usb2.c
+++ b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
@@ -146,11 +146,18 @@
 #define RESET_COMPLETE_TIME					1000
 #define PLL_RESET_COMPLETE_TIME					100
 
+enum {
+	MESON_USB2_PHY_VERSION_10 = 0,
+	MESON_USB2_PHY_VERSION_11,
+	MESON_USB2_PHY_VERSION_COUNT,
+};
+
 struct phy_meson_g12a_usb2_priv {
 	struct device		*dev;
 	struct regmap		*regmap;
 	struct clk		*clk;
 	struct reset_control	*reset;
+	int phy_version;
 };
 
 static const struct regmap_config phy_meson_g12a_usb2_regmap_conf = {
@@ -192,18 +199,33 @@ 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);
+	if (priv->phy_version == MESON_USB2_PHY_VERSION_10)
+		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);
+	else if (priv->phy_version == MESON_USB2_PHY_VERSION_11)
+		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 |
+			     PHY_CTRL_R18_MPLL_DCO_CLK_SEL);
 
 	udelay(PLL_RESET_COMPLETE_TIME);
 
@@ -227,13 +249,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->phy_version == MESON_USB2_PHY_VERSION_10)
+		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->phy_version == MESON_USB2_PHY_VERSION_11) {
+		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 +274,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->phy_version == MESON_USB2_PHY_VERSION_10) {
+		/* 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;
 }
@@ -271,6 +306,7 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct phy_meson_g12a_usb2_priv *priv;
 	struct phy *phy;
+	struct device_node *np = dev->of_node;
 	void __iomem *base;
 	int ret;
 
@@ -286,14 +322,23 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
+	if (of_device_is_compatible(np, "amlogic,g12a-usb2-phy"))
+		priv->phy_version = MESON_USB2_PHY_VERSION_10;
+	else if (of_device_is_compatible(np, "amlogic,a1-usb2-phy"))
+		priv->phy_version = MESON_USB2_PHY_VERSION_11;
+	else
+		return -EINVAL;
+
 	priv->regmap = devm_regmap_init_mmio(dev, base,
 					     &phy_meson_g12a_usb2_regmap_conf);
 	if (IS_ERR(priv->regmap))
 		return PTR_ERR(priv->regmap);
 
-	priv->clk = devm_clk_get(dev, "xtal");
-	if (IS_ERR(priv->clk))
-		return PTR_ERR(priv->clk);
+	if (priv->phy_version == MESON_USB2_PHY_VERSION_10) {
+		priv->clk = devm_clk_get(dev, "xtal");
+		if (IS_ERR(priv->clk))
+			return PTR_ERR(priv->clk);
+	}
 
 	priv->reset = devm_reset_control_get(dev, "phy");
 	if (IS_ERR(priv->reset))
@@ -322,7 +367,8 @@ 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,a1-usb2-phy", },
+	{ /* Sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, phy_meson_g12a_usb2_of_match);
 
-- 
2.7.4

[PATCH v2 4/6] usb: dwc3: Add Amlogic A1 DWC3 glue

[Hanjie Lin]

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 | 69 +++++++++++++++++++++++++++-----------
 1 file changed, 49 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c
index 8a3ec1a..3817daf 100644
--- a/drivers/usb/dwc3/dwc3-meson-g12a.c
+++ b/drivers/usb/dwc3/dwc3-meson-g12a.c
@@ -107,10 +107,22 @@ static const char *phy_names[PHY_COUNT] = {
 	"usb2-phy0", "usb2-phy1", "usb3-phy0",
 };
 
+static const struct clk_bulk_data meson_g12a_clocks[] = {
+	{ .id = NULL},
+};
+
+static const struct clk_bulk_data meson_a1_clocks[] = {
+	{ .id = "usb_ctrl"},
+	{ .id = "usb_bus"},
+	{ .id = "xtal_usb_phy"},
+	{ .id = "xtal_usb_ctrl"},
+};
+
 struct dwc3_meson_g12a {
 	struct device		*dev;
 	struct regmap		*regmap;
-	struct clk		*clk;
+	struct clk_bulk_data    *clks;
+	int num_clks;
 	struct reset_control	*reset;
 	struct phy		*phys[PHY_COUNT];
 	enum usb_dr_mode	otg_mode;
@@ -151,7 +163,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 (i == USB2_OTG_PHY && priv->otg_mode != USB_DR_MODE_HOST) {
 			regmap_update_bits(priv->regmap,
 				U2P_R0 + (U2P_REG_SIZE * i),
 				U2P_R0_ID_PULLUP | U2P_R0_DRV_VBUS,
@@ -295,7 +307,7 @@ static int dwc3_meson_g12a_otg_mode_set(struct dwc3_meson_g12a *priv,
 {
 	int ret;
 
-	if (!priv->phys[USB2_OTG_PHY])
+	if (!priv->phys[USB2_OTG_PHY] || priv->otg_mode == USB_DR_MODE_HOST)
 		return -EINVAL;
 
 	if (mode == PHY_MODE_USB_HOST)
@@ -409,17 +421,32 @@ 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->dev = dev;
+
+	if (of_device_is_compatible(np, "amlogic,meson-g12a-usb-ctrl")) {
+		priv->clks = devm_kmemdup(dev, meson_g12a_clocks,
+					  sizeof(meson_g12a_clocks),
+					  GFP_KERNEL);
+		priv->num_clks = ARRAY_SIZE(meson_g12a_clocks);
+	} else if (of_device_is_compatible(np, "amlogic,meson-a1-usb-ctrl")) {
+		priv->clks = devm_kmemdup(dev, meson_a1_clocks,
+					  sizeof(meson_a1_clocks),
+					  GFP_KERNEL);
+		priv->num_clks = ARRAY_SIZE(meson_a1_clocks);
+	} else {
+		return -EINVAL;
+	}
+
+	if (!priv->clks)
+		return -ENOMEM;
 
-	ret = clk_prepare_enable(priv->clk);
+	ret = devm_clk_bulk_get(dev, priv->num_clks, priv->clks);
 	if (ret)
 		return ret;
 
-	devm_add_action_or_reset(dev,
-				 (void(*)(void *))clk_disable_unprepare,
-				 priv->clk);
+	ret = clk_bulk_prepare_enable(priv->num_clks, priv->clks);
+	if (ret)
+		return ret;
 
 	platform_set_drvdata(pdev, priv);
 	priv->dev = dev;
@@ -433,16 +460,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 +485,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 +494,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 +505,8 @@ 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;
-	}
 
 	/* Setup OTG mode corresponding to the ID pin */
 	if (priv->otg_mode == USB_DR_MODE_OTG) {
@@ -518,6 +543,9 @@ 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->num_clks, priv->clks);
+
 	return ret;
 }
 
@@ -547,7 +575,7 @@ 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->num_clks, priv->clks);
 
 	return 0;
 }
@@ -556,7 +584,7 @@ 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->num_clks, priv->clks);
 }
 
 static int __maybe_unused dwc3_meson_g12a_suspend(struct device *dev)
@@ -620,6 +648,7 @@ 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-a1-usb-ctrl" },
 	{ /* Sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, dwc3_meson_g12a_match);
-- 
2.7.4

[PATCH v2 5/6] arm64: dts: meson: a1: Enable USB2 PHY

[Hanjie Lin]

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 | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
index 6fdc0dd..bd63374a 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,16 @@
 				#power-domain-cells = <1>;
 				status = "okay";
 			};
+
+			usb2_phy1: phy@40000 {
+				status = "okay";
+				compatible = "amlogic,a1-usb2-phy";
+				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

[PATCH v2 6/6] arm64: dts: meson: a1: Enable DWC3 controller

[Hanjie Lin]

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 | 33 +++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
index bd63374a..76f787d 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";
@@ -125,6 +127,37 @@
 			#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_PHY>,
+				 <&clkc_periphs CLKID_XTAL_USB_CTRL>;
+			clock-names = "usb_ctrl", "usb_bus", "xtal_usb_phy",
+					"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

Re: [PATCH v2 2/6] dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings

[Neil Armstrong]

Hi,

On 18/12/2019 03:42, Hanjie Lin wrote:
> 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>
> ---
>  .../bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml  | 32 ++++++++++++++++++++++
>  1 file changed, 32 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..9740027 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
>  
> @@ -124,4 +130,30 @@ examples:
>                snps,quirk-frame-length-adjustment;
>            };
>      };
> +  - |
> +    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_PHY>,
> +           <&clkc_periphs CLKID_XTAL_USB_CTRL>;
> +          clock-names = "usb_ctrl", "usb_bus", "xtal_usb_phy", "xtal_usb_ctrl";
> +          resets = <&reset RESET_USBCTRL>;
> +          phys = <&usb2_phy0>;
> +          phy-names = "usb2-phy0";
> +
> +          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>;
> +          };
> +  };
> 

I doubt this passed the dt_binding_check !


Please add the clock-names only for amlogic,meson-a1-usb-ctrl,
set the phys maxItems to 1 for amlogic,meson-a1-usb-ctrl,
and set dr_mode as host in the example or make it required only
for amlogic,meson-g12a-usb-ctrl.

Neil

Re: [PATCH v2 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver

[Neil Armstrong]

Hi,

On 18/12/2019 03: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 | 102 ++++++++++++++++++++++--------
>  1 file changed, 74 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb2.c b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
> index 9065ffc..2c242d3 100644
> --- a/drivers/phy/amlogic/phy-meson-g12a-usb2.c
> +++ b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
> @@ -146,11 +146,18 @@
>  #define RESET_COMPLETE_TIME					1000
>  #define PLL_RESET_COMPLETE_TIME					100
>  
> +enum {
> +	MESON_USB2_PHY_VERSION_10 = 0,
> +	MESON_USB2_PHY_VERSION_11,

Are these the real "versions" of the phy or it's made up ?

> +	MESON_USB2_PHY_VERSION_COUNT,
> +};
> +
>  struct phy_meson_g12a_usb2_priv {
>  	struct device		*dev;
>  	struct regmap		*regmap;
>  	struct clk		*clk;
>  	struct reset_control	*reset;
> +	int phy_version;
>  };
>  
>  static const struct regmap_config phy_meson_g12a_usb2_regmap_conf = {
> @@ -192,18 +199,33 @@ 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);
> +	if (priv->phy_version == MESON_USB2_PHY_VERSION_10)
> +		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);
> +	else if (priv->phy_version == MESON_USB2_PHY_VERSION_11)
> +		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 |
> +			     PHY_CTRL_R18_MPLL_DCO_CLK_SEL);

The only difference is PHY_CTRL_R18_MPLL_ACG_RANGE | PHY_CTRL_R18_MPLL_DCO_CLK_SEL,
you can easily simplify the code here by using a temp variable.

>  
>  	udelay(PLL_RESET_COMPLETE_TIME);
>  
> @@ -227,13 +249,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->phy_version == MESON_USB2_PHY_VERSION_10)
> +		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->phy_version == MESON_USB2_PHY_VERSION_11) {
> +		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 +274,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->phy_version == MESON_USB2_PHY_VERSION_10) {
> +		/* 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;
>  }
> @@ -271,6 +306,7 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	struct phy_meson_g12a_usb2_priv *priv;
>  	struct phy *phy;
> +	struct device_node *np = dev->of_node;
>  	void __iomem *base;
>  	int ret;
>  
> @@ -286,14 +322,23 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
>  	if (IS_ERR(base))
>  		return PTR_ERR(base);
>  
> +	if (of_device_is_compatible(np, "amlogic,g12a-usb2-phy"))
> +		priv->phy_version = MESON_USB2_PHY_VERSION_10;
> +	else if (of_device_is_compatible(np, "amlogic,a1-usb2-phy"))
> +		priv->phy_version = MESON_USB2_PHY_VERSION_11;
> +	else
> +		return -EINVAL;

Please use of_device_get_match_data() and a match data for each compatible instead.

> +
>  	priv->regmap = devm_regmap_init_mmio(dev, base,
>  					     &phy_meson_g12a_usb2_regmap_conf);
>  	if (IS_ERR(priv->regmap))
>  		return PTR_ERR(priv->regmap);
>  
> -	priv->clk = devm_clk_get(dev, "xtal");
> -	if (IS_ERR(priv->clk))
> -		return PTR_ERR(priv->clk);
> +	if (priv->phy_version == MESON_USB2_PHY_VERSION_10) {
> +		priv->clk = devm_clk_get(dev, "xtal");
> +		if (IS_ERR(priv->clk))
> +			return PTR_ERR(priv->clk);
> +	}
>  
>  	priv->reset = devm_reset_control_get(dev, "phy");
>  	if (IS_ERR(priv->reset))
> @@ -322,7 +367,8 @@ 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,a1-usb2-phy", },
> +	{ /* Sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, phy_meson_g12a_usb2_of_match);
>  
> 

Thanks,
Neil

Re: [PATCH v2 4/6] usb: dwc3: Add Amlogic A1 DWC3 glue

[Neil Armstrong]

Hi,

On 18/12/2019 03:42, Hanjie Lin wrote:
> 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 | 69 +++++++++++++++++++++++++++-----------
>  1 file changed, 49 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c
> index 8a3ec1a..3817daf 100644
> --- a/drivers/usb/dwc3/dwc3-meson-g12a.c
> +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c
> @@ -107,10 +107,22 @@ static const char *phy_names[PHY_COUNT] = {
>  	"usb2-phy0", "usb2-phy1", "usb3-phy0",
>  };
>  
> +static const struct clk_bulk_data meson_g12a_clocks[] = {
> +	{ .id = NULL},
> +};
> +
> +static const struct clk_bulk_data meson_a1_clocks[] = {
> +	{ .id = "usb_ctrl"},
> +	{ .id = "usb_bus"},
> +	{ .id = "xtal_usb_phy"},
> +	{ .id = "xtal_usb_ctrl"},
> +};
> +
>  struct dwc3_meson_g12a {
>  	struct device		*dev;
>  	struct regmap		*regmap;
> -	struct clk		*clk;
> +	struct clk_bulk_data    *clks;
> +	int num_clks;
>  	struct reset_control	*reset;
>  	struct phy		*phys[PHY_COUNT];
>  	enum usb_dr_mode	otg_mode;
> @@ -151,7 +163,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 (i == USB2_OTG_PHY && priv->otg_mode != USB_DR_MODE_HOST) {

This is wrong for G12A, please use something else to exclude PULLUPP/VBUS to be updated on A1.

>  			regmap_update_bits(priv->regmap,
>  				U2P_R0 + (U2P_REG_SIZE * i),
>  				U2P_R0_ID_PULLUP | U2P_R0_DRV_VBUS,
> @@ -295,7 +307,7 @@ static int dwc3_meson_g12a_otg_mode_set(struct dwc3_meson_g12a *priv,
>  {
>  	int ret;
>  
> -	if (!priv->phys[USB2_OTG_PHY])
> +	if (!priv->phys[USB2_OTG_PHY] || priv->otg_mode == USB_DR_MODE_HOST)

Same, this is wrong on G12A since a Host port can be switched to device, use something
else to disable this for A1, like setting a "otg_supported" flag into the match data,
and using it in dwc3_meson_g12a_usb2_init(), avoiding registering usb_role_switch, the
IRQ and updating the priv->otg_mode in probe().

>  		return -EINVAL;
>  
>  	if (mode == PHY_MODE_USB_HOST)
> @@ -409,17 +421,32 @@ 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->dev = dev;
> +
> +	if (of_device_is_compatible(np, "amlogic,meson-g12a-usb-ctrl")) {
> +		priv->clks = devm_kmemdup(dev, meson_g12a_clocks,
> +					  sizeof(meson_g12a_clocks),
> +					  GFP_KERNEL);
> +		priv->num_clks = ARRAY_SIZE(meson_g12a_clocks);
> +	} else if (of_device_is_compatible(np, "amlogic,meson-a1-usb-ctrl")) {
> +		priv->clks = devm_kmemdup(dev, meson_a1_clocks,
> +					  sizeof(meson_a1_clocks),
> +					  GFP_KERNEL);
> +		priv->num_clks = ARRAY_SIZE(meson_a1_clocks);
> +	} else {
> +		return -EINVAL;
> +	}

Like the PHY driver, please use of_device_get_match_data() and a match data for each compatible instead.

> +
> +	if (!priv->clks)
> +		return -ENOMEM;
>  
> -	ret = clk_prepare_enable(priv->clk);
> +	ret = devm_clk_bulk_get(dev, priv->num_clks, priv->clks);
>  	if (ret)
>  		return ret;
>  
> -	devm_add_action_or_reset(dev,
> -				 (void(*)(void *))clk_disable_unprepare,
> -				 priv->clk);
> +	ret = clk_bulk_prepare_enable(priv->num_clks, priv->clks);
> +	if (ret)
> +		return ret;
>  
>  	platform_set_drvdata(pdev, priv);
>  	priv->dev = dev;
> @@ -433,16 +460,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 +485,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 +494,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 +505,8 @@ 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;
> -	}
>  
>  	/* Setup OTG mode corresponding to the ID pin */
>  	if (priv->otg_mode == USB_DR_MODE_OTG) {
> @@ -518,6 +543,9 @@ 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->num_clks, priv->clks);
> +
>  	return ret;
>  }
>  
> @@ -547,7 +575,7 @@ 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->num_clks, priv->clks);
>  
>  	return 0;
>  }
> @@ -556,7 +584,7 @@ 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->num_clks, priv->clks);
>  }
>  
>  static int __maybe_unused dwc3_meson_g12a_suspend(struct device *dev)
> @@ -620,6 +648,7 @@ 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-a1-usb-ctrl" },
>  	{ /* Sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, dwc3_meson_g12a_match);
> 


Thanks,
Neil

Re: [PATCH v2 2/6] dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings

[Hanjie Lin]

On 2019/12/18 21:13, Neil Armstrong wrote:
> Hi,
> 
> On 18/12/2019 03:42, Hanjie Lin wrote:
>> 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>
>> ---
>>  .../bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml  | 32 ++++++++++++++++++++++
>>  1 file changed, 32 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..9740027 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
>>  
>> @@ -124,4 +130,30 @@ examples:
>>                snps,quirk-frame-length-adjustment;
>>            };
>>      };
>> +  - |
>> +    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_PHY>,
>> +           <&clkc_periphs CLKID_XTAL_USB_CTRL>;
>> +          clock-names = "usb_ctrl", "usb_bus", "xtal_usb_phy", "xtal_usb_ctrl";
>> +          resets = <&reset RESET_USBCTRL>;
>> +          phys = <&usb2_phy0>;
>> +          phy-names = "usb2-phy0";
>> +
>> +          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>;
>> +          };
>> +  };
>>
> 
> I doubt this passed the dt_binding_check !
> 
> 
> Please add the clock-names only for amlogic,meson-a1-usb-ctrl,
> set the phys maxItems to 1 for amlogic,meson-a1-usb-ctrl,
> and set dr_mode as host in the example or make it required only
> for amlogic,meson-g12a-usb-ctrl.
> 
> Neil
> 
> .
> 

Hi Neil,

It does report errors by dt_binding_check, I will check the list again.

Thanks

Hanjie.lin

Re: [PATCH v2 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver

[Hanjie Lin]

On 2019/12/18 21:17, Neil Armstrong wrote:
> Hi,
> 
> On 18/12/2019 03: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 | 102 ++++++++++++++++++++++--------
>>  1 file changed, 74 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb2.c b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
>> index 9065ffc..2c242d3 100644
>> --- a/drivers/phy/amlogic/phy-meson-g12a-usb2.c
>> +++ b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
>> @@ -146,11 +146,18 @@
>>  #define RESET_COMPLETE_TIME					1000
>>  #define PLL_RESET_COMPLETE_TIME					100
>>  
>> +enum {
>> +	MESON_USB2_PHY_VERSION_10 = 0,
>> +	MESON_USB2_PHY_VERSION_11,
> 
> Are these the real "versions" of the phy or it's made up ?
> 

This version is made up and only for distinguish a1 and g12a.

>> +	MESON_USB2_PHY_VERSION_COUNT,
>> +};
>> +
>>  struct phy_meson_g12a_usb2_priv {
>>  	struct device		*dev;
>>  	struct regmap		*regmap;
>>  	struct clk		*clk;
>>  	struct reset_control	*reset;
>> +	int phy_version;
>>  };
>>  
>>  static const struct regmap_config phy_meson_g12a_usb2_regmap_conf = {
>> @@ -192,18 +199,33 @@ 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);
>> +	if (priv->phy_version == MESON_USB2_PHY_VERSION_10)
>> +		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);
>> +	else if (priv->phy_version == MESON_USB2_PHY_VERSION_11)
>> +		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 |
>> +			     PHY_CTRL_R18_MPLL_DCO_CLK_SEL);
> 
> The only difference is PHY_CTRL_R18_MPLL_ACG_RANGE | PHY_CTRL_R18_MPLL_DCO_CLK_SEL,
> you can easily simplify the code here by using a temp variable.
> 

Yes, it will looks more clearly.

>>  
>>  	udelay(PLL_RESET_COMPLETE_TIME);
>>  
>> @@ -227,13 +249,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->phy_version == MESON_USB2_PHY_VERSION_10)
>> +		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->phy_version == MESON_USB2_PHY_VERSION_11) {
>> +		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 +274,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->phy_version == MESON_USB2_PHY_VERSION_10) {
>> +		/* 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;
>>  }
>> @@ -271,6 +306,7 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
>>  	struct resource *res;
>>  	struct phy_meson_g12a_usb2_priv *priv;
>>  	struct phy *phy;
>> +	struct device_node *np = dev->of_node;
>>  	void __iomem *base;
>>  	int ret;
>>  
>> @@ -286,14 +322,23 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
>>  	if (IS_ERR(base))
>>  		return PTR_ERR(base);
>>  
>> +	if (of_device_is_compatible(np, "amlogic,g12a-usb2-phy"))
>> +		priv->phy_version = MESON_USB2_PHY_VERSION_10;
>> +	else if (of_device_is_compatible(np, "amlogic,a1-usb2-phy"))
>> +		priv->phy_version = MESON_USB2_PHY_VERSION_11;
>> +	else
>> +		return -EINVAL;
> 
> Please use of_device_get_match_data() and a match data for each compatible instead.
> 

OK, I will fix it in next version.

>> +
>>  	priv->regmap = devm_regmap_init_mmio(dev, base,
>>  					     &phy_meson_g12a_usb2_regmap_conf);
>>  	if (IS_ERR(priv->regmap))
>>  		return PTR_ERR(priv->regmap);
>>  
>> -	priv->clk = devm_clk_get(dev, "xtal");
>> -	if (IS_ERR(priv->clk))
>> -		return PTR_ERR(priv->clk);
>> +	if (priv->phy_version == MESON_USB2_PHY_VERSION_10) {
>> +		priv->clk = devm_clk_get(dev, "xtal");
>> +		if (IS_ERR(priv->clk))
>> +			return PTR_ERR(priv->clk);
>> +	}
>>  
>>  	priv->reset = devm_reset_control_get(dev, "phy");
>>  	if (IS_ERR(priv->reset))
>> @@ -322,7 +367,8 @@ 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,a1-usb2-phy", },
>> +	{ /* Sentinel */ }
>>  };
>>  MODULE_DEVICE_TABLE(of, phy_meson_g12a_usb2_of_match);
>>  
>>
> 
> Thanks,
> Neil
> 
> .
> 

Thanks,
Hanjie.Lin

Re: [PATCH v2 4/6] usb: dwc3: Add Amlogic A1 DWC3 glue

[Hanjie Lin]

On 2019/12/18 21:23, Neil Armstrong wrote:
> Hi,
> 
> On 18/12/2019 03:42, Hanjie Lin wrote:
>> 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 | 69 +++++++++++++++++++++++++++-----------
>>  1 file changed, 49 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c
>> index 8a3ec1a..3817daf 100644
>> --- a/drivers/usb/dwc3/dwc3-meson-g12a.c
>> +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c
>> @@ -107,10 +107,22 @@ static const char *phy_names[PHY_COUNT] = {
>>  	"usb2-phy0", "usb2-phy1", "usb3-phy0",
>>  };
>>  
>> +static const struct clk_bulk_data meson_g12a_clocks[] = {
>> +	{ .id = NULL},
>> +};
>> +
>> +static const struct clk_bulk_data meson_a1_clocks[] = {
>> +	{ .id = "usb_ctrl"},
>> +	{ .id = "usb_bus"},
>> +	{ .id = "xtal_usb_phy"},
>> +	{ .id = "xtal_usb_ctrl"},
>> +};
>> +
>>  struct dwc3_meson_g12a {
>>  	struct device		*dev;
>>  	struct regmap		*regmap;
>> -	struct clk		*clk;
>> +	struct clk_bulk_data    *clks;
>> +	int num_clks;
>>  	struct reset_control	*reset;
>>  	struct phy		*phys[PHY_COUNT];
>>  	enum usb_dr_mode	otg_mode;
>> @@ -151,7 +163,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 (i == USB2_OTG_PHY && priv->otg_mode != USB_DR_MODE_HOST) {
> 
> This is wrong for G12A, please use something else to exclude PULLUPP/VBUS to be updated on A1.
> 

Yes, it's a mistake.
I will try to fix it in next version.

>>  			regmap_update_bits(priv->regmap,
>>  				U2P_R0 + (U2P_REG_SIZE * i),
>>  				U2P_R0_ID_PULLUP | U2P_R0_DRV_VBUS,
>> @@ -295,7 +307,7 @@ static int dwc3_meson_g12a_otg_mode_set(struct dwc3_meson_g12a *priv,
>>  {
>>  	int ret;
>>  
>> -	if (!priv->phys[USB2_OTG_PHY])
>> +	if (!priv->phys[USB2_OTG_PHY] || priv->otg_mode == USB_DR_MODE_HOST)
> 
> Same, this is wrong on G12A since a Host port can be switched to device, use something
> else to disable this for A1, like setting a "otg_supported" flag into the match data,
> and using it in dwc3_meson_g12a_usb2_init(), avoiding registering usb_role_switch, the
> IRQ and updating the priv->otg_mode in probe().
> 

Yes, thanks.

>>  		return -EINVAL;
>>  
>>  	if (mode == PHY_MODE_USB_HOST)
>> @@ -409,17 +421,32 @@ 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->dev = dev;
>> +
>> +	if (of_device_is_compatible(np, "amlogic,meson-g12a-usb-ctrl")) {
>> +		priv->clks = devm_kmemdup(dev, meson_g12a_clocks,
>> +					  sizeof(meson_g12a_clocks),
>> +					  GFP_KERNEL);
>> +		priv->num_clks = ARRAY_SIZE(meson_g12a_clocks);
>> +	} else if (of_device_is_compatible(np, "amlogic,meson-a1-usb-ctrl")) {
>> +		priv->clks = devm_kmemdup(dev, meson_a1_clocks,
>> +					  sizeof(meson_a1_clocks),
>> +					  GFP_KERNEL);
>> +		priv->num_clks = ARRAY_SIZE(meson_a1_clocks);
>> +	} else {
>> +		return -EINVAL;
>> +	}
> 
> Like the PHY driver, please use of_device_get_match_data() and a match data for each compatible instead.
> 

OK.

>> +
>> +	if (!priv->clks)
>> +		return -ENOMEM;
>>  
>> -	ret = clk_prepare_enable(priv->clk);
>> +	ret = devm_clk_bulk_get(dev, priv->num_clks, priv->clks);
>>  	if (ret)
>>  		return ret;
>>  
>> -	devm_add_action_or_reset(dev,
>> -				 (void(*)(void *))clk_disable_unprepare,
>> -				 priv->clk);
>> +	ret = clk_bulk_prepare_enable(priv->num_clks, priv->clks);
>> +	if (ret)
>> +		return ret;
>>  
>>  	platform_set_drvdata(pdev, priv);
>>  	priv->dev = dev;
>> @@ -433,16 +460,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 +485,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 +494,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 +505,8 @@ 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;
>> -	}
>>  
>>  	/* Setup OTG mode corresponding to the ID pin */
>>  	if (priv->otg_mode == USB_DR_MODE_OTG) {
>> @@ -518,6 +543,9 @@ 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->num_clks, priv->clks);
>> +
>>  	return ret;
>>  }
>>  
>> @@ -547,7 +575,7 @@ 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->num_clks, priv->clks);
>>  
>>  	return 0;
>>  }
>> @@ -556,7 +584,7 @@ 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->num_clks, priv->clks);
>>  }
>>  
>>  static int __maybe_unused dwc3_meson_g12a_suspend(struct device *dev)
>> @@ -620,6 +648,7 @@ 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-a1-usb-ctrl" },
>>  	{ /* Sentinel */ }
>>  };
>>  MODULE_DEVICE_TABLE(of, dwc3_meson_g12a_match);
>>
> 
> 
> Thanks,
> Neil
> 
> .
>

Thanks,
Hanjie.Lin
 

Re: [PATCH v2 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver

[Neil Armstrong]

On 19/12/2019 10:48, Hanjie Lin wrote:
> 
> 
> On 2019/12/18 21:17, Neil Armstrong wrote:
>> Hi,
>>
>> On 18/12/2019 03: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 | 102 ++++++++++++++++++++++--------
>>>  1 file changed, 74 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb2.c b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
>>> index 9065ffc..2c242d3 100644
>>> --- a/drivers/phy/amlogic/phy-meson-g12a-usb2.c
>>> +++ b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
>>> @@ -146,11 +146,18 @@
>>>  #define RESET_COMPLETE_TIME					1000
>>>  #define PLL_RESET_COMPLETE_TIME					100
>>>  
>>> +enum {
>>> +	MESON_USB2_PHY_VERSION_10 = 0,
>>> +	MESON_USB2_PHY_VERSION_11,
>>
>> Are these the real "versions" of the phy or it's made up ?
>>
> 
> This version is made up and only for distinguish a1 and g12a.

No problem, in this case simply use the SoC family instead of 10 and 11.

Neil

> 
>>> +	MESON_USB2_PHY_VERSION_COUNT,
>>> +};
>>> +
>>>  struct phy_meson_g12a_usb2_priv {
>>>  	struct device		*dev;
>>>  	struct regmap		*regmap;
>>>  	struct clk		*clk;
>>>  	struct reset_control	*reset;
>>> +	int phy_version;
>>>  };
>>>  
>>>  static const struct regmap_config phy_meson_g12a_usb2_regmap_conf = {
>>> @@ -192,18 +199,33 @@ 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);
>>> +	if (priv->phy_version == MESON_USB2_PHY_VERSION_10)
>>> +		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);
>>> +	else if (priv->phy_version == MESON_USB2_PHY_VERSION_11)
>>> +		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 |
>>> +			     PHY_CTRL_R18_MPLL_DCO_CLK_SEL);
>>
>> The only difference is PHY_CTRL_R18_MPLL_ACG_RANGE | PHY_CTRL_R18_MPLL_DCO_CLK_SEL,
>> you can easily simplify the code here by using a temp variable.
>>
> 
> Yes, it will looks more clearly.
> 
>>>  
>>>  	udelay(PLL_RESET_COMPLETE_TIME);
>>>  
>>> @@ -227,13 +249,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->phy_version == MESON_USB2_PHY_VERSION_10)
>>> +		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->phy_version == MESON_USB2_PHY_VERSION_11) {
>>> +		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 +274,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->phy_version == MESON_USB2_PHY_VERSION_10) {
>>> +		/* 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;
>>>  }
>>> @@ -271,6 +306,7 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
>>>  	struct resource *res;
>>>  	struct phy_meson_g12a_usb2_priv *priv;
>>>  	struct phy *phy;
>>> +	struct device_node *np = dev->of_node;
>>>  	void __iomem *base;
>>>  	int ret;
>>>  
>>> @@ -286,14 +322,23 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
>>>  	if (IS_ERR(base))
>>>  		return PTR_ERR(base);
>>>  
>>> +	if (of_device_is_compatible(np, "amlogic,g12a-usb2-phy"))
>>> +		priv->phy_version = MESON_USB2_PHY_VERSION_10;
>>> +	else if (of_device_is_compatible(np, "amlogic,a1-usb2-phy"))
>>> +		priv->phy_version = MESON_USB2_PHY_VERSION_11;
>>> +	else
>>> +		return -EINVAL;
>>
>> Please use of_device_get_match_data() and a match data for each compatible instead.
>>
> 
> OK, I will fix it in next version.
> 
>>> +
>>>  	priv->regmap = devm_regmap_init_mmio(dev, base,
>>>  					     &phy_meson_g12a_usb2_regmap_conf);
>>>  	if (IS_ERR(priv->regmap))
>>>  		return PTR_ERR(priv->regmap);
>>>  
>>> -	priv->clk = devm_clk_get(dev, "xtal");
>>> -	if (IS_ERR(priv->clk))
>>> -		return PTR_ERR(priv->clk);
>>> +	if (priv->phy_version == MESON_USB2_PHY_VERSION_10) {
>>> +		priv->clk = devm_clk_get(dev, "xtal");
>>> +		if (IS_ERR(priv->clk))
>>> +			return PTR_ERR(priv->clk);
>>> +	}
>>>  
>>>  	priv->reset = devm_reset_control_get(dev, "phy");
>>>  	if (IS_ERR(priv->reset))
>>> @@ -322,7 +367,8 @@ 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,a1-usb2-phy", },
>>> +	{ /* Sentinel */ }
>>>  };
>>>  MODULE_DEVICE_TABLE(of, phy_meson_g12a_usb2_of_match);
>>>  
>>>
>>
>> Thanks,
>> Neil
>>
>> .
>>
> 
> Thanks,
> Hanjie.Lin
> 

Re: [PATCH v2 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver

[Hanjie Lin]

On 2019/12/19 18:12, Neil Armstrong wrote:
> On 19/12/2019 10:48, Hanjie Lin wrote:
>>
>>
>> On 2019/12/18 21:17, Neil Armstrong wrote:
>>> Hi,
>>>
>>> On 18/12/2019 03: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 | 102 ++++++++++++++++++++++--------
>>>>  1 file changed, 74 insertions(+), 28 deletions(-)
>>>>
>>>> diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb2.c b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
>>>> index 9065ffc..2c242d3 100644
>>>> --- a/drivers/phy/amlogic/phy-meson-g12a-usb2.c
>>>> +++ b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
>>>> @@ -146,11 +146,18 @@
>>>>  #define RESET_COMPLETE_TIME					1000
>>>>  #define PLL_RESET_COMPLETE_TIME					100
>>>>  
>>>> +enum {
>>>> +	MESON_USB2_PHY_VERSION_10 = 0,
>>>> +	MESON_USB2_PHY_VERSION_11,
>>>
>>> Are these the real "versions" of the phy or it's made up ?
>>>
>>
>> This version is made up and only for distinguish a1 and g12a.
> 
> No problem, in this case simply use the SoC family instead of 10 and 11.
> 
> Neil
> 

Of course, it looks more accurate, I will get rid of phy version in next version.

>>
>>>> +	MESON_USB2_PHY_VERSION_COUNT,
>>>> +};
>>>> +
>>>>  struct phy_meson_g12a_usb2_priv {
>>>>  	struct device		*dev;
>>>>  	struct regmap		*regmap;
>>>>  	struct clk		*clk;
>>>>  	struct reset_control	*reset;
>>>> +	int phy_version;
>>>>  };
>>>>  
>>>>  static const struct regmap_config phy_meson_g12a_usb2_regmap_conf = {
>>>> @@ -192,18 +199,33 @@ 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);
>>>> +	if (priv->phy_version == MESON_USB2_PHY_VERSION_10)
>>>> +		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);
>>>> +	else if (priv->phy_version == MESON_USB2_PHY_VERSION_11)
>>>> +		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 |
>>>> +			     PHY_CTRL_R18_MPLL_DCO_CLK_SEL);
>>>
>>> The only difference is PHY_CTRL_R18_MPLL_ACG_RANGE | PHY_CTRL_R18_MPLL_DCO_CLK_SEL,
>>> you can easily simplify the code here by using a temp variable.
>>>
>>
>> Yes, it will looks more clearly.
>>
>>>>  
>>>>  	udelay(PLL_RESET_COMPLETE_TIME);
>>>>  
>>>> @@ -227,13 +249,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->phy_version == MESON_USB2_PHY_VERSION_10)
>>>> +		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->phy_version == MESON_USB2_PHY_VERSION_11) {
>>>> +		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 +274,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->phy_version == MESON_USB2_PHY_VERSION_10) {
>>>> +		/* 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;
>>>>  }
>>>> @@ -271,6 +306,7 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
>>>>  	struct resource *res;
>>>>  	struct phy_meson_g12a_usb2_priv *priv;
>>>>  	struct phy *phy;
>>>> +	struct device_node *np = dev->of_node;
>>>>  	void __iomem *base;
>>>>  	int ret;
>>>>  
>>>> @@ -286,14 +322,23 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
>>>>  	if (IS_ERR(base))
>>>>  		return PTR_ERR(base);
>>>>  
>>>> +	if (of_device_is_compatible(np, "amlogic,g12a-usb2-phy"))
>>>> +		priv->phy_version = MESON_USB2_PHY_VERSION_10;
>>>> +	else if (of_device_is_compatible(np, "amlogic,a1-usb2-phy"))
>>>> +		priv->phy_version = MESON_USB2_PHY_VERSION_11;
>>>> +	else
>>>> +		return -EINVAL;
>>>
>>> Please use of_device_get_match_data() and a match data for each compatible instead.
>>>
>>
>> OK, I will fix it in next version.
>>
>>>> +
>>>>  	priv->regmap = devm_regmap_init_mmio(dev, base,
>>>>  					     &phy_meson_g12a_usb2_regmap_conf);
>>>>  	if (IS_ERR(priv->regmap))
>>>>  		return PTR_ERR(priv->regmap);
>>>>  
>>>> -	priv->clk = devm_clk_get(dev, "xtal");
>>>> -	if (IS_ERR(priv->clk))
>>>> -		return PTR_ERR(priv->clk);
>>>> +	if (priv->phy_version == MESON_USB2_PHY_VERSION_10) {
>>>> +		priv->clk = devm_clk_get(dev, "xtal");
>>>> +		if (IS_ERR(priv->clk))
>>>> +			return PTR_ERR(priv->clk);
>>>> +	}
>>>>  
>>>>  	priv->reset = devm_reset_control_get(dev, "phy");
>>>>  	if (IS_ERR(priv->reset))
>>>> @@ -322,7 +367,8 @@ 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,a1-usb2-phy", },
>>>> +	{ /* Sentinel */ }
>>>>  };
>>>>  MODULE_DEVICE_TABLE(of, phy_meson_g12a_usb2_of_match);
>>>>  
>>>>
>>>
>>> Thanks,
>>> Neil
>>>
>>> .
>>>
>>
>> Thanks,
>> Hanjie.Lin
>>
> 
> .
> 


Thanks,
Hanjie.Lin