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

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

[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

Hanjie Lin (6):
  dt-bindings: phy: Add Amlogic G12A 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 +++
 .../devicetree/bindings/usb/amlogic,dwc3.txt       |  53 +++
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi          |  41 +++
 drivers/phy/amlogic/Kconfig                        |  13 +
 drivers/phy/amlogic/Makefile                       |   1 +
 drivers/phy/amlogic/phy-meson-a1-usb2.c            | 327 +++++++++++++++++
 drivers/usb/dwc3/Kconfig                           |  11 +
 drivers/usb/dwc3/Makefile                          |   1 +
 drivers/usb/dwc3/dwc3-meson-a1.c                   | 397 +++++++++++++++++++++
 9 files changed, 899 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/amlogic,meson-a1-usb2-phy.yaml
 create mode 100644 drivers/phy/amlogic/phy-meson-a1-usb2.c
 create mode 100644 drivers/usb/dwc3/dwc3-meson-a1.c

-- 
2.7.4


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

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

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


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

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

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,dwc3.txt       | 53 ++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt b/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
index 6ffb09b..63dc60b 100644
--- a/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
@@ -128,3 +128,56 @@ Example device nodes:
 				snps,quirk-frame-length-adjustment;
 			};
 	};
+
+Amlogic Meson A1 DWC3 USB SoC Controller Glue
+
+The Amlogic A1 embeds a DWC3 USB IP Core configured for USB2 in
+host-only mode.
+
+Required properties:
+- compatible:	Should be "amlogic,meson-a1-usb-ctrl"
+- clocks:       The clocks needed by the usb controller
+- clock-names:  Should contain the name of the clocks: "usb_ctrl", "usb_bus",
+                "xtal_usb_phy", "xtal_usb_ctrl"
+- resets:	a handle for the shared "USB" reset line
+- reg:		The base address and length of the registers
+- phys: 	handle to used PHYs on the system
+	- a <0> phandle can be used if a PHY is not used
+- phy-names:	names of the used PHYs on the system :
+	- "usb2-phy0" for USB2 PHY if USBHOST port is used
+
+Required child nodes:
+
+A child node must exist to represent the core DWC3 IP block. The name of
+the node is not important. The content of the node is defined in dwc3.txt.
+
+PHY documentation is provided in the following places:
+- Documentation/devicetree/bindings/phy/amlogic,meson-a1-usb2-phy.yaml
+
+Example device nodes:
+	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


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

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

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/Kconfig             |  13 ++
 drivers/phy/amlogic/Makefile            |   1 +
 drivers/phy/amlogic/phy-meson-a1-usb2.c | 327 ++++++++++++++++++++++++++++++++
 3 files changed, 341 insertions(+)
 create mode 100644 drivers/phy/amlogic/phy-meson-a1-usb2.c

diff --git a/drivers/phy/amlogic/Kconfig b/drivers/phy/amlogic/Kconfig
index af774ac..5b5affb 100644
--- a/drivers/phy/amlogic/Kconfig
+++ b/drivers/phy/amlogic/Kconfig
@@ -38,6 +38,19 @@ config PHY_MESON_GXL_USB3
 	  IP block found in Meson GXL and GXM SoCs.
 	  If unsure, say N.
 
+config PHY_MESON_A1_USB2
+	tristate "Meson A1 USB2 PHY driver"
+	default ARCH_MESON
+	depends on OF && (ARCH_MESON || COMPILE_TEST)
+	select GENERIC_PHY
+	select REGMAP_MMIO
+	help
+	  Enable this to support the Meson USB2 PHY found in Meson
+	  A1 SoCs.
+	  The MESON A1 USB2 PHY support a DWC3 USB IP Core configured
+	  for USB2 in host-only mode.
+	  If unsure, say N.
+
 config PHY_MESON_G12A_USB2
 	tristate "Meson G12A USB2 PHY driver"
 	default ARCH_MESON
diff --git a/drivers/phy/amlogic/Makefile b/drivers/phy/amlogic/Makefile
index 11d1c42..c9031c5 100644
--- a/drivers/phy/amlogic/Makefile
+++ b/drivers/phy/amlogic/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_PHY_MESON8B_USB2)		+= phy-meson8b-usb2.o
+obj-$(CONFIG_PHY_MESON_A1_USB2)		+= phy-meson-a1-usb2.o
 obj-$(CONFIG_PHY_MESON_GXL_USB2)	+= phy-meson-gxl-usb2.o
 obj-$(CONFIG_PHY_MESON_G12A_USB2)	+= phy-meson-g12a-usb2.o
 obj-$(CONFIG_PHY_MESON_GXL_USB3)	+= phy-meson-gxl-usb3.o
diff --git a/drivers/phy/amlogic/phy-meson-a1-usb2.c b/drivers/phy/amlogic/phy-meson-a1-usb2.c
new file mode 100644
index 00000000..28148b6
--- /dev/null
+++ b/drivers/phy/amlogic/phy-meson-a1-usb2.c
@@ -0,0 +1,327 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Meson A1 USB2 PHY driver
+ *
+ * Copyright (C) 2017 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
+ * Copyright (C) 2019 Amlogic, Inc. All rights reserved
+ * Author: Yue Wang <yue.wang@amlogic.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+
+#define PHY_CTRL_R0						0x0
+#define PHY_CTRL_R1						0x4
+#define PHY_CTRL_R2						0x8
+#define PHY_CTRL_R3						0xc
+	#define PHY_CTRL_R3_SQUELCH_REF				GENMASK(1, 0)
+	#define PHY_CTRL_R3_HSDIC_REF				GENMASK(3, 2)
+	#define PHY_CTRL_R3_DISC_THRESH				GENMASK(7, 4)
+
+#define PHY_CTRL_R4						0x10
+	#define PHY_CTRL_R4_CALIB_CODE_7_0			GENMASK(7, 0)
+	#define PHY_CTRL_R4_CALIB_CODE_15_8			GENMASK(15, 8)
+	#define PHY_CTRL_R4_CALIB_CODE_23_16			GENMASK(23, 16)
+	#define PHY_CTRL_R4_I_C2L_CAL_EN			BIT(24)
+	#define PHY_CTRL_R4_I_C2L_CAL_RESET_N			BIT(25)
+	#define PHY_CTRL_R4_I_C2L_CAL_DONE			BIT(26)
+	#define PHY_CTRL_R4_TEST_BYPASS_MODE_EN			BIT(27)
+	#define PHY_CTRL_R4_I_C2L_BIAS_TRIM_1_0			GENMASK(29, 28)
+	#define PHY_CTRL_R4_I_C2L_BIAS_TRIM_3_2			GENMASK(31, 30)
+
+#define PHY_CTRL_R5						0x14
+#define PHY_CTRL_R6						0x18
+#define PHY_CTRL_R7						0x1c
+#define PHY_CTRL_R8						0x20
+#define PHY_CTRL_R9						0x24
+#define PHY_CTRL_R10						0x28
+#define PHY_CTRL_R11						0x2c
+#define PHY_CTRL_R12						0x30
+#define PHY_CTRL_R13						0x34
+	#define PHY_CTRL_R13_CUSTOM_PATTERN_19			GENMASK(7, 0)
+	#define PHY_CTRL_R13_LOAD_STAT				BIT(14)
+	#define PHY_CTRL_R13_UPDATE_PMA_SIGNALS			BIT(15)
+	#define PHY_CTRL_R13_MIN_COUNT_FOR_SYNC_DET		GENMASK(20, 16)
+	#define PHY_CTRL_R13_CLEAR_HOLD_HS_DISCONNECT		BIT(21)
+	#define PHY_CTRL_R13_BYPASS_HOST_DISCONNECT_VAL		BIT(22)
+	#define PHY_CTRL_R13_BYPASS_HOST_DISCONNECT_EN		BIT(23)
+	#define PHY_CTRL_R13_I_C2L_HS_EN			BIT(24)
+	#define PHY_CTRL_R13_I_C2L_FS_EN			BIT(25)
+	#define PHY_CTRL_R13_I_C2L_LS_EN			BIT(26)
+	#define PHY_CTRL_R13_I_C2L_HS_OE			BIT(27)
+	#define PHY_CTRL_R13_I_C2L_FS_OE			BIT(28)
+	#define PHY_CTRL_R13_I_C2L_HS_RX_EN			BIT(29)
+	#define PHY_CTRL_R13_I_C2L_FSLS_RX_EN			BIT(30)
+
+#define PHY_CTRL_R14						0x38
+	#define PHY_CTRL_R14_I_RDP_EN				BIT(0)
+	#define PHY_CTRL_R14_I_RPU_SW1_EN			BIT(1)
+	#define PHY_CTRL_R14_I_RPU_SW2_EN			GENMASK(2, 3)
+	#define PHY_CTRL_R14_PG_RSTN				BIT(4)
+	#define PHY_CTRL_R14_I_C2L_DATA_16_8			BIT(5)
+	#define PHY_CTRL_R14_I_C2L_ASSERT_SINGLE_EN_ZERO	BIT(6)
+	#define PHY_CTRL_R14_BYPASS_CTRL_7_0			GENMASK(15, 8)
+	#define PHY_CTRL_R14_BYPASS_CTRL_15_8			GENMASK(23, 16)
+
+#define PHY_CTRL_R15						0x3c
+#define PHY_CTRL_R16						0x40
+	#define PHY_CTRL_R16_MPLL_M				GENMASK(8, 0)
+	#define PHY_CTRL_R16_MPLL_N				GENMASK(14, 10)
+	#define PHY_CTRL_R16_MPLL_TDC_MODE			BIT(20)
+	#define PHY_CTRL_R16_MPLL_SDM_EN			BIT(21)
+	#define PHY_CTRL_R16_MPLL_LOAD				BIT(22)
+	#define PHY_CTRL_R16_MPLL_DCO_SDM_EN			BIT(23)
+	#define PHY_CTRL_R16_MPLL_LOCK_LONG			GENMASK(25, 24)
+	#define PHY_CTRL_R16_MPLL_LOCK_F			BIT(26)
+	#define PHY_CTRL_R16_MPLL_FAST_LOCK			BIT(27)
+	#define PHY_CTRL_R16_MPLL_EN				BIT(28)
+	#define PHY_CTRL_R16_MPLL_RESET				BIT(29)
+	#define PHY_CTRL_R16_MPLL_LOCK				BIT(30)
+	#define PHY_CTRL_R16_MPLL_LOCK_DIG			BIT(31)
+
+#define PHY_CTRL_R17						0x44
+	#define PHY_CTRL_R17_MPLL_FRAC_IN			GENMASK(13, 0)
+	#define PHY_CTRL_R17_MPLL_FIX_EN			BIT(16)
+	#define PHY_CTRL_R17_MPLL_LAMBDA1			GENMASK(19, 17)
+	#define PHY_CTRL_R17_MPLL_LAMBDA0			GENMASK(22, 20)
+	#define PHY_CTRL_R17_MPLL_FILTER_MODE			BIT(23)
+	#define PHY_CTRL_R17_MPLL_FILTER_PVT2			GENMASK(27, 24)
+	#define PHY_CTRL_R17_MPLL_FILTER_PVT1			GENMASK(31, 28)
+
+#define PHY_CTRL_R18						0x48
+	#define PHY_CTRL_R18_MPLL_LKW_SEL			GENMASK(1, 0)
+	#define PHY_CTRL_R18_MPLL_LK_W				GENMASK(5, 2)
+	#define PHY_CTRL_R18_MPLL_LK_S				GENMASK(11, 6)
+	#define PHY_CTRL_R18_MPLL_DCO_M_EN			BIT(12)
+	#define PHY_CTRL_R18_MPLL_DCO_CLK_SEL			BIT(13)
+	#define PHY_CTRL_R18_MPLL_PFD_GAIN			GENMASK(15, 14)
+	#define PHY_CTRL_R18_MPLL_ROU				GENMASK(18, 16)
+	#define PHY_CTRL_R18_MPLL_DATA_SEL			GENMASK(21, 19)
+	#define PHY_CTRL_R18_MPLL_BIAS_ADJ			GENMASK(23, 22)
+	#define PHY_CTRL_R18_MPLL_BB_MODE			GENMASK(25, 24)
+	#define PHY_CTRL_R18_MPLL_ALPHA				GENMASK(28, 26)
+	#define PHY_CTRL_R18_MPLL_ADJ_LDO			GENMASK(30, 29)
+	#define PHY_CTRL_R18_MPLL_ACG_RANGE			BIT(31)
+
+#define PHY_CTRL_R19						0x4c
+#define PHY_CTRL_R20						0x50
+	#define PHY_CTRL_R20_USB2_IDDET_EN			BIT(0)
+	#define PHY_CTRL_R20_USB2_OTG_VBUS_TRIM_2_0		GENMASK(3, 1)
+	#define PHY_CTRL_R20_USB2_OTG_VBUSDET_EN		BIT(4)
+	#define PHY_CTRL_R20_USB2_AMON_EN			BIT(5)
+	#define PHY_CTRL_R20_USB2_CAL_CODE_R5			BIT(6)
+	#define PHY_CTRL_R20_BYPASS_OTG_DET			BIT(7)
+	#define PHY_CTRL_R20_USB2_DMON_EN			BIT(8)
+	#define PHY_CTRL_R20_USB2_DMON_SEL_3_0			GENMASK(12, 9)
+	#define PHY_CTRL_R20_USB2_EDGE_DRV_EN			BIT(13)
+	#define PHY_CTRL_R20_USB2_EDGE_DRV_TRIM_1_0		GENMASK(15, 14)
+	#define PHY_CTRL_R20_USB2_BGR_ADJ_4_0			GENMASK(20, 16)
+	#define PHY_CTRL_R20_USB2_BGR_START			BIT(21)
+	#define PHY_CTRL_R20_USB2_BGR_VREF_4_0			GENMASK(28, 24)
+	#define PHY_CTRL_R20_USB2_BGR_DBG_1_0			GENMASK(30, 29)
+	#define PHY_CTRL_R20_BYPASS_CAL_DONE_R5			BIT(31)
+
+#define PHY_CTRL_R21						0x54
+	#define PHY_CTRL_R21_USB2_BGR_FORCE			BIT(0)
+	#define PHY_CTRL_R21_USB2_CAL_ACK_EN			BIT(1)
+	#define PHY_CTRL_R21_USB2_OTG_ACA_EN			BIT(2)
+	#define PHY_CTRL_R21_USB2_TX_STRG_PD			BIT(3)
+	#define PHY_CTRL_R21_USB2_OTG_ACA_TRIM_1_0		GENMASK(5, 4)
+	#define PHY_CTRL_R21_BYPASS_UTMI_CNTR			GENMASK(15, 6)
+	#define PHY_CTRL_R21_BYPASS_UTMI_REG			GENMASK(25, 20)
+
+#define PHY_CTRL_R22						0x58
+#define PHY_CTRL_R23						0x5c
+
+#define RESET_COMPLETE_TIME					1000
+#define PLL_RESET_COMPLETE_TIME					100
+
+struct phy_meson_a1_usb2_priv {
+	struct device		*dev;
+	struct regmap		*regmap;
+	struct reset_control	*reset;
+};
+
+static const struct regmap_config phy_meson_a1_usb2_regmap_conf = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.max_register = PHY_CTRL_R23,
+};
+
+static int phy_meson_a1_usb2_init(struct phy *phy)
+{
+	struct phy_meson_a1_usb2_priv *priv = phy_get_drvdata(phy);
+	int ret;
+
+	ret = reset_control_reset(priv->reset);
+	if (ret)
+		return ret;
+
+	udelay(RESET_COMPLETE_TIME);
+
+	/* usb2_otg_aca_en == 0 */
+	regmap_update_bits(priv->regmap, PHY_CTRL_R21,
+			   PHY_CTRL_R21_USB2_OTG_ACA_EN, 0);
+
+	/* PLL Setup : 24MHz * 20 / 1 = 480MHz */
+	regmap_write(priv->regmap, PHY_CTRL_R16,
+		     FIELD_PREP(PHY_CTRL_R16_MPLL_M, 20) |
+		     FIELD_PREP(PHY_CTRL_R16_MPLL_N, 1) |
+		     PHY_CTRL_R16_MPLL_LOAD |
+		     FIELD_PREP(PHY_CTRL_R16_MPLL_LOCK_LONG, 1) |
+		     PHY_CTRL_R16_MPLL_FAST_LOCK |
+		     PHY_CTRL_R16_MPLL_EN |
+		     PHY_CTRL_R16_MPLL_RESET);
+
+	regmap_write(priv->regmap, PHY_CTRL_R17,
+		     FIELD_PREP(PHY_CTRL_R17_MPLL_FRAC_IN, 0) |
+		     FIELD_PREP(PHY_CTRL_R17_MPLL_LAMBDA1, 7) |
+		     FIELD_PREP(PHY_CTRL_R17_MPLL_LAMBDA0, 7) |
+		     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 |
+		     PHY_CTRL_R18_MPLL_DCO_CLK_SEL);
+
+	udelay(PLL_RESET_COMPLETE_TIME);
+
+	/* UnReset PLL */
+	regmap_write(priv->regmap, PHY_CTRL_R16,
+		     FIELD_PREP(PHY_CTRL_R16_MPLL_M, 20) |
+		     FIELD_PREP(PHY_CTRL_R16_MPLL_N, 1) |
+		     PHY_CTRL_R16_MPLL_LOAD |
+		     FIELD_PREP(PHY_CTRL_R16_MPLL_LOCK_LONG, 1) |
+		     PHY_CTRL_R16_MPLL_FAST_LOCK |
+		     PHY_CTRL_R16_MPLL_EN);
+
+	/* PHY Tuning */
+	regmap_write(priv->regmap, PHY_CTRL_R20,
+		     FIELD_PREP(PHY_CTRL_R20_USB2_OTG_VBUS_TRIM_2_0, 4) |
+		     PHY_CTRL_R20_USB2_OTG_VBUSDET_EN |
+		     FIELD_PREP(PHY_CTRL_R20_USB2_DMON_SEL_3_0, 15) |
+		     PHY_CTRL_R20_USB2_EDGE_DRV_EN |
+		     FIELD_PREP(PHY_CTRL_R20_USB2_EDGE_DRV_TRIM_1_0, 3) |
+		     FIELD_PREP(PHY_CTRL_R20_USB2_BGR_ADJ_4_0, 0) |
+		     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_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,
+		     FIELD_PREP(PHY_CTRL_R3_SQUELCH_REF, 0) |
+		     FIELD_PREP(PHY_CTRL_R3_HSDIC_REF, 1) |
+		     FIELD_PREP(PHY_CTRL_R3_DISC_THRESH, 3));
+
+	return 0;
+}
+
+static int phy_meson_a1_usb2_exit(struct phy *phy)
+{
+	struct phy_meson_a1_usb2_priv *priv = phy_get_drvdata(phy);
+
+	return reset_control_reset(priv->reset);
+}
+
+/* set_mode is not needed, mode setting is handled via the UTMI bus */
+static const struct phy_ops phy_meson_a1_usb2_ops = {
+	.init		= phy_meson_a1_usb2_init,
+	.exit		= phy_meson_a1_usb2_exit,
+	.owner		= THIS_MODULE,
+};
+
+static int phy_meson_a1_usb2_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct phy_provider *phy_provider;
+	struct phy_meson_a1_usb2_priv *priv;
+	struct phy *phy;
+	void __iomem *base;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+	platform_set_drvdata(pdev, priv);
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	priv->regmap = devm_regmap_init_mmio(dev, base,
+					     &phy_meson_a1_usb2_regmap_conf);
+	if (IS_ERR(priv->regmap))
+		return PTR_ERR(priv->regmap);
+
+	priv->reset = devm_reset_control_get(dev, "phy");
+	if (IS_ERR(priv->reset))
+		return PTR_ERR(priv->reset);
+
+	ret = reset_control_deassert(priv->reset);
+	if (ret)
+		return ret;
+
+	phy = devm_phy_create(dev, NULL, &phy_meson_a1_usb2_ops);
+	if (IS_ERR(phy)) {
+		ret = PTR_ERR(phy);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to create PHY\n");
+
+		return ret;
+	}
+
+	phy_set_bus_width(phy, 8);
+	phy_set_drvdata(phy, priv);
+
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id phy_meson_a1_usb2_of_match[] = {
+	{ .compatible = "amlogic,a1-usb2-phy", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, phy_meson_a1_usb2_of_match);
+
+static struct platform_driver phy_meson_a1_usb2_driver = {
+	.probe	= phy_meson_a1_usb2_probe,
+	.driver	= {
+		.name		= "phy-meson-a1-usb2",
+		.of_match_table	= phy_meson_a1_usb2_of_match,
+	},
+};
+module_platform_driver(phy_meson_a1_usb2_driver);
+
+MODULE_AUTHOR("Yue Wang <yue.wang@amlogic.com>");
+MODULE_DESCRIPTION("Meson A1 USB2 PHY driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


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

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

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/Kconfig         |  11 ++
 drivers/usb/dwc3/Makefile        |   1 +
 drivers/usb/dwc3/dwc3-meson-a1.c | 397 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 409 insertions(+)
 create mode 100644 drivers/usb/dwc3/dwc3-meson-a1.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 556a876..9bfb159 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -96,6 +96,17 @@ config USB_DWC3_KEYSTONE
 	  Support of USB2/3 functionality in TI Keystone2 and AM654 platforms.
 	  Say 'Y' or 'M' here if you have one such device
 
+config USB_DWC3_MESON_A1
+	tristate "Amlogic Meson A1 Platforms"
+	depends on OF && COMMON_CLK
+	depends on ARCH_MESON || COMPILE_TEST
+	default USB_DWC3
+	help
+	  Support USB2 functionality in MESON A1 platforms.
+	  The MESON A1 USB2 support a DWC3 USB IP Core configured for USB2 in
+	  host-only mode.
+	  Say 'Y' or 'M' if you have one such device.
+
 config USB_DWC3_MESON_G12A
        tristate "Amlogic Meson G12A Platforms"
        depends on OF && COMMON_CLK
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index ae86da0..a3fc655 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_USB_DWC3_EXYNOS)		+= dwc3-exynos.o
 obj-$(CONFIG_USB_DWC3_PCI)		+= dwc3-pci.o
 obj-$(CONFIG_USB_DWC3_HAPS)		+= dwc3-haps.o
 obj-$(CONFIG_USB_DWC3_KEYSTONE)		+= dwc3-keystone.o
+obj-$(CONFIG_USB_DWC3_MESON_A1)		+= dwc3-meson-a1.o
 obj-$(CONFIG_USB_DWC3_MESON_G12A)	+= dwc3-meson-g12a.o
 obj-$(CONFIG_USB_DWC3_OF_SIMPLE)	+= dwc3-of-simple.o
 obj-$(CONFIG_USB_DWC3_ST)		+= dwc3-st.o
diff --git a/drivers/usb/dwc3/dwc3-meson-a1.c b/drivers/usb/dwc3/dwc3-meson-a1.c
new file mode 100644
index 00000000..db2b99a
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-meson-a1.c
@@ -0,0 +1,397 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * USB Glue for Amlogic A1 SoCs
+ *
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved
+ * Author: Yue Wang <yue.wang@amlogic.com>
+ */
+
+/*
+ * The USB is organized with a glue around the DWC3 Controller IP as :
+ * - Control registers for each USB2 Ports
+ * - Control registers for the USB PHY layer
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/reset.h>
+#include <linux/phy/phy.h>
+#include <linux/usb/otg.h>
+#include <linux/usb/role.h>
+#include <linux/regulator/consumer.h>
+
+/* USB2 Ports Control Registers */
+#define U2P_R0							0x20
+	#define U2P_R0_HOST_DEVICE				BIT(0)
+	#define U2P_R0_POWER_OK					BIT(1)
+	#define U2P_R0_HAST_MODE				BIT(2)
+	#define U2P_R0_POWER_ON_RESET				BIT(3)
+	#define U2P_R0_ID_PULLUP				BIT(4)
+	#define U2P_R0_DRV_VBUS					BIT(5)
+
+#define U2P_R1							0x24
+	#define U2P_R1_PHY_READY				BIT(0)
+	#define U2P_R1_ID_DIG					BIT(1)
+	#define U2P_R1_OTG_SESSION_VALID			BIT(2)
+	#define U2P_R1_VBUS_VALID				BIT(3)
+
+/* USB Glue Control Registers */
+
+#define USB_R0							0x80
+	#define USB_R0_P30_LANE0_TX2RX_LOOPBACK			BIT(17)
+	#define USB_R0_P30_LANE0_EXT_PCLK_REQ			BIT(18)
+	#define USB_R0_P30_PCS_RX_LOS_MASK_VAL_MASK		GENMASK(28, 19)
+	#define USB_R0_U2D_SS_SCALEDOWN_MODE_MASK		GENMASK(30, 29)
+	#define USB_R0_U2D_ACT					BIT(31)
+
+#define USB_R1							0x84
+	#define USB_R1_U3H_BIGENDIAN_GS				BIT(0)
+	#define USB_R1_U3H_PME_ENABLE				BIT(1)
+	#define USB_R1_U3H_HUB_PORT_OVERCURRENT_MASK		GENMASK(4, 2)
+	#define USB_R1_U3H_HUB_PORT_PERM_ATTACH_MASK		GENMASK(9, 7)
+	#define USB_R1_U3H_HOST_U2_PORT_DISABLE_MASK		GENMASK(13, 12)
+	#define USB_R1_U3H_HOST_U3_PORT_DISABLE			BIT(16)
+	#define USB_R1_U3H_HOST_PORT_POWER_CONTROL_PRESENT	BIT(17)
+	#define USB_R1_U3H_HOST_MSI_ENABLE			BIT(18)
+	#define USB_R1_U3H_FLADJ_30MHZ_REG_MASK			GENMASK(24, 19)
+	#define USB_R1_P30_PCS_TX_SWING_FULL_MASK		GENMASK(31, 25)
+
+#define USB_R2							0x88
+	#define USB_R2_P30_PCS_TX_DEEMPH_3P5DB_MASK		GENMASK(25, 20)
+	#define USB_R2_P30_PCS_TX_DEEMPH_6DB_MASK		GENMASK(31, 26)
+
+#define USB_R3							0x8c
+	#define USB_R3_P30_SSC_ENABLE				BIT(0)
+	#define USB_R3_P30_SSC_RANGE_MASK			GENMASK(3, 1)
+	#define USB_R3_P30_SSC_REF_CLK_SEL_MASK			GENMASK(12, 4)
+	#define USB_R3_P30_REF_SSP_EN				BIT(13)
+
+#define USB_R4							0x90
+	#define USB_R4_P21_PORT_RESET_0				BIT(0)
+	#define USB_R4_P21_SLEEP_M0				BIT(1)
+	#define USB_R4_MEM_PD_MASK				GENMASK(3, 2)
+	#define USB_R4_P21_ONLY					BIT(4)
+
+#define USB_R5							0x94
+	#define USB_R5_ID_DIG_SYNC				BIT(0)
+	#define USB_R5_ID_DIG_REG				BIT(1)
+	#define USB_R5_ID_DIG_CFG_MASK				GENMASK(3, 2)
+	#define USB_R5_ID_DIG_EN_0				BIT(4)
+	#define USB_R5_ID_DIG_EN_1				BIT(5)
+	#define USB_R5_ID_DIG_CURR				BIT(6)
+	#define USB_R5_ID_DIG_IRQ				BIT(7)
+	#define USB_R5_ID_DIG_TH_MASK				GENMASK(15, 8)
+	#define USB_R5_ID_DIG_CNT_MASK				GENMASK(23, 16)
+
+static const char *phy_names = {
+	"usb2-phy0",
+};
+
+struct dwc3_meson_a1 {
+	struct device		*dev;
+	struct regmap		*regmap;
+	struct clk		*clk_usb_ctrl;
+	struct clk		*clk_usb_bus;
+	struct clk		*clk_xtal_usb_phy;
+	struct clk		*clk_xtal_usb_ctrl;
+	struct reset_control	*reset;
+	struct phy		*phys;
+	unsigned int		usb2_ports;
+};
+
+static void dwc3_meson_a1_usb_init(struct dwc3_meson_a1 *priv)
+{
+	regmap_update_bits(priv->regmap, U2P_R0,
+			   U2P_R0_POWER_ON_RESET,
+			   U2P_R0_POWER_ON_RESET);
+
+	regmap_update_bits(priv->regmap, U2P_R0,
+			   U2P_R0_HOST_DEVICE,
+			   U2P_R0_HOST_DEVICE);
+
+	regmap_update_bits(priv->regmap, U2P_R0,
+			   U2P_R0_POWER_ON_RESET, 0);
+
+	regmap_update_bits(priv->regmap, USB_R1,
+			   USB_R1_U3H_FLADJ_30MHZ_REG_MASK,
+			   FIELD_PREP(USB_R1_U3H_FLADJ_30MHZ_REG_MASK, 0x20));
+
+	regmap_update_bits(priv->regmap, USB_R0,
+			   USB_R0_U2D_ACT, 0);
+
+	regmap_update_bits(priv->regmap, USB_R4,
+			   USB_R4_P21_SLEEP_M0, 0);
+}
+
+static const struct regmap_config phy_meson_a1_usb_regmap_conf = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.max_register = USB_R5,
+};
+
+static int dwc3_meson_a1_get_phys(struct dwc3_meson_a1 *priv)
+{
+	priv->phys = devm_phy_optional_get(priv->dev, phy_names);
+	if (IS_ERR(priv->phys))
+		return PTR_ERR(priv->phys);
+
+	priv->usb2_ports++;
+
+	dev_info(priv->dev, "USB2 ports: %d\n", priv->usb2_ports);
+
+	return 0;
+}
+
+static int dwc3_meson_a1_enable_clk(struct dwc3_meson_a1 *priv)
+{
+	int ret;
+
+	ret = clk_prepare_enable(priv->clk_usb_ctrl);
+	if (ret < 0) {
+		dev_err(priv->dev, "can't enable usb_ctrl clock.\n");
+		return ret;
+	}
+
+	ret = clk_prepare_enable(priv->clk_usb_bus);
+	if (ret < 0) {
+		dev_err(priv->dev, "can't enable usb_bus clock.\n");
+		goto disable_clk_usb_ctrl;
+	}
+
+	ret = clk_prepare_enable(priv->clk_xtal_usb_phy);
+	if (ret < 0) {
+		dev_err(priv->dev, "can't enable xtal_usb_phy clock.\n");
+		goto disable_clk_usb_bus;
+	}
+
+	ret = clk_prepare_enable(priv->clk_xtal_usb_ctrl);
+	if (ret < 0) {
+		dev_err(priv->dev, "can't enable xtal_usb_ctrl clock.\n");
+		goto disable_clk_xtal_usb_phy;
+	}
+
+	return 0;
+
+disable_clk_xtal_usb_phy:
+	clk_disable_unprepare(priv->clk_xtal_usb_phy);
+disable_clk_usb_bus:
+	clk_disable_unprepare(priv->clk_usb_bus);
+disable_clk_usb_ctrl:
+	clk_disable_unprepare(priv->clk_usb_ctrl);
+
+	return ret;
+}
+
+static void dwc3_meson_a1_disable_clk(struct dwc3_meson_a1 *priv)
+{
+	clk_disable_unprepare(priv->clk_usb_ctrl);
+	clk_disable_unprepare(priv->clk_usb_bus);
+	clk_disable_unprepare(priv->clk_xtal_usb_phy);
+	clk_disable_unprepare(priv->clk_xtal_usb_ctrl);
+}
+
+static int dwc3_meson_a1_setup_clk(struct dwc3_meson_a1 *priv)
+{
+	int ret;
+
+	priv->clk_usb_ctrl = devm_clk_get(priv->dev, "usb_ctrl");
+	if (IS_ERR(priv->clk_usb_ctrl)) {
+		dev_err(priv->dev, "can't get usb_ctrl clock.\n");
+		return PTR_ERR(priv->clk_usb_ctrl);
+	}
+
+	priv->clk_usb_bus = devm_clk_get(priv->dev, "usb_bus");
+	if (IS_ERR(priv->clk_usb_bus)) {
+		dev_err(priv->dev, "can't get usb_bus clock.\n");
+		return PTR_ERR(priv->clk_usb_bus);
+	}
+
+	priv->clk_xtal_usb_phy = devm_clk_get(priv->dev, "xtal_usb_phy");
+	if (IS_ERR(priv->clk_xtal_usb_phy)) {
+		dev_err(priv->dev, "can't get xtal_usb_phy clock.\n");
+		return PTR_ERR(priv->clk_xtal_usb_phy);
+	}
+
+	priv->clk_xtal_usb_ctrl = devm_clk_get(priv->dev, "xtal_usb_ctrl");
+	if (IS_ERR(priv->clk_xtal_usb_ctrl)) {
+		dev_err(priv->dev, "can't get xtal_usb_ctrl clock.\n");
+		return PTR_ERR(priv->clk_xtal_usb_ctrl);
+	}
+
+	ret = dwc3_meson_a1_enable_clk(priv);
+	if (ret)
+		return ret;
+
+	devm_add_action_or_reset(priv->dev,
+				 (void(*)(void *))clk_disable_unprepare,
+				 priv->clk_usb_ctrl);
+	devm_add_action_or_reset(priv->dev,
+				 (void(*)(void *))clk_disable_unprepare,
+				 priv->clk_usb_bus);
+	devm_add_action_or_reset(priv->dev,
+				 (void(*)(void *))clk_disable_unprepare,
+				 priv->clk_xtal_usb_phy);
+	devm_add_action_or_reset(priv->dev,
+				 (void(*)(void *))clk_disable_unprepare,
+				 priv->clk_xtal_usb_ctrl);
+
+	return 0;
+}
+
+static int dwc3_meson_a1_probe(struct platform_device *pdev)
+{
+	struct dwc3_meson_a1	*priv;
+	struct device		*dev = &pdev->dev;
+	struct device_node	*np = dev->of_node;
+	void __iomem *base;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, priv);
+	priv->dev = dev;
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	priv->regmap = devm_regmap_init_mmio(dev, base,
+					     &phy_meson_a1_usb_regmap_conf);
+	if (IS_ERR(priv->regmap))
+		return PTR_ERR(priv->regmap);
+
+	ret = dwc3_meson_a1_setup_clk(priv);
+	if (ret)
+		return ret;
+
+	priv->reset = devm_reset_control_get(dev, NULL);
+	if (IS_ERR(priv->reset)) {
+		ret = PTR_ERR(priv->reset);
+		dev_err(dev, "failed to get device reset, err=%d\n", ret);
+		return ret;
+	}
+
+	ret = reset_control_reset(priv->reset);
+	if (ret)
+		return ret;
+
+	ret = dwc3_meson_a1_get_phys(priv);
+	if (ret)
+		return ret;
+
+	dwc3_meson_a1_usb_init(priv);
+
+	/* Init PHYs */
+	ret = phy_init(priv->phys);
+	if (ret)
+		return ret;
+
+	/* Set PHY Power */
+	ret = phy_power_on(priv->phys);
+	if (ret)
+		goto err_phys_exit;
+
+	ret = of_platform_populate(np, NULL, NULL, dev);
+	if (ret)
+		goto err_phys_power;
+
+	return 0;
+
+err_phys_power:
+	phy_power_off(priv->phys);
+
+err_phys_exit:
+	phy_exit(priv->phys);
+
+	return ret;
+}
+
+static int dwc3_meson_a1_remove(struct platform_device *pdev)
+{
+	struct dwc3_meson_a1 *priv = platform_get_drvdata(pdev);
+	struct device *dev = &pdev->dev;
+
+	of_platform_depopulate(dev);
+
+	phy_power_off(priv->phys);
+	phy_exit(priv->phys);
+
+	return 0;
+}
+
+static int __maybe_unused dwc3_meson_a1_suspend(struct device *dev)
+{
+	struct dwc3_meson_a1 *priv = dev_get_drvdata(dev);
+
+	phy_power_off(priv->phys);
+	phy_exit(priv->phys);
+
+	reset_control_assert(priv->reset);
+
+	dwc3_meson_a1_disable_clk(priv);
+
+	return 0;
+}
+
+static int __maybe_unused dwc3_meson_a1_resume(struct device *dev)
+{
+	struct dwc3_meson_a1 *priv = dev_get_drvdata(dev);
+	int ret;
+
+	ret = dwc3_meson_a1_enable_clk(priv);
+	if (ret)
+		return ret;
+
+	reset_control_deassert(priv->reset);
+
+	dwc3_meson_a1_usb_init(priv);
+
+	/* Init PHYs */
+	ret = phy_init(priv->phys);
+	if (ret)
+		return ret;
+
+	/* Set PHY Power */
+	ret = phy_power_on(priv->phys);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct dev_pm_ops dwc3_meson_a1_dev_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(dwc3_meson_a1_suspend, dwc3_meson_a1_resume)
+};
+
+static const struct of_device_id dwc3_meson_a1_match[] = {
+	{ .compatible = "amlogic,meson-a1-usb-ctrl" },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, dwc3_meson_a1_match);
+
+static struct platform_driver dwc3_meson_a1_driver = {
+	.probe		= dwc3_meson_a1_probe,
+	.remove		= dwc3_meson_a1_remove,
+	.driver		= {
+		.name	= "dwc3-meson-a1",
+		.of_match_table = dwc3_meson_a1_match,
+		.pm	= &dwc3_meson_a1_dev_pm_ops,
+	},
+};
+
+module_platform_driver(dwc3_meson_a1_driver);
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Amlogic Meson A1 USB Glue Layer");
+MODULE_AUTHOR("Yue Wang <yue.wang@amlogic.com>");
-- 
2.7.4


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

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

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

diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
index eaeae80..3b61717 100644
--- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
@@ -106,6 +106,16 @@
 				#power-domain-cells = <1>;
 				status = "okay";
 			};
+
+			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>;
+			};
 		};
 
 		gic: interrupt-controller@ff901000 {
-- 
2.7.4


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

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

Enable DWC3 controller for Meson A1 SoC.

Change-Id: Ibbbfb44fc211f926adf91a548776bc6bca6fa35c
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 3b61717..f3d93cd 100644
--- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
@@ -6,6 +6,9 @@
 #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/clock/a1-clkc.h>
+#include <dt-bindings/reset/amlogic,meson-a1-reset.h>
+#include <dt-bindings/clock/a1-pll-clkc.h>
 
 / {
 	compatible = "amlogic,a1";
@@ -130,6 +133,34 @@
 			#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>;
+			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>;
+			};
+		};
 	};
 
 	timer {
-- 
2.7.4


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

* Re: [PATCH 0/6] arm64: meson: Add support for USB on Amlogic A1
  2019-11-22  6:55 [PATCH 0/6] arm64: meson: Add support for USB on Amlogic A1 Hanjie Lin
                   ` (5 preceding siblings ...)
  2019-11-22  6:55 ` [PATCH 6/6] arm64: dts: meson: a1: Enable DWC3 controller Hanjie Lin
@ 2019-11-22  7:52 ` Martin Blumenstingl
  2019-11-25  7:53   ` Hanjie Lin
  6 siblings, 1 reply; 19+ messages in thread
From: Martin Blumenstingl @ 2019-11-22  7:52 UTC (permalink / raw)
  To: Hanjie Lin
  Cc: Jerome Brunet, Neil Armstrong, Rob Herring, Greg Kroah-Hartman,
	Felipe Balbi, Kevin Hilman, linux-amlogic, linux-arm-kernel,
	linux-usb, devicetree, Carlo Caione, Michael Turquette,
	Stephen Boyd, Liang Yang, Jianxin Pan, Qiufang Dai, Jian Hu,
	Victor Wan, Yue Wang, Xingyu Chen

Hello Hanjie,

On Fri, Nov 22, 2019 at 7:55 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
[...]
>   dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings
>   dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings
>   phy: amlogic: Add Amlogic A1 USB2 PHY Driver
drivers/phy/amlogic/phy-meson-g12a-usb2.c seems very similar to the A1
USB2 PHY you are introducing here.

>   usb: dwc3: Add Amlogic A1 DWC3 glue
drivers/usb/dwc3/dwc3-meson-g12a.c is also very similar to the dwc3 glue.

I have two questions:
- how is the PHY and the dwc3 glue different from G12A (or SM1)?
- why do we need a separate set of new drivers (instead of updating
the existing drivers)?

We try to use one driver for the same IP block, even if there are
several revisions with small differences (for example the SAR ADC
driver supports all SoC generations from Meson8 to G12A/G12B/SM1,
because 80-90% of the code is shared across all revisions).


Martin

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

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

Hi,


On 22/11/2019 07:55, 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>
> ---
>  .../devicetree/bindings/usb/amlogic,dwc3.txt       | 53 ++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt b/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
> index 6ffb09b..63dc60b 100644
> --- a/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
> +++ b/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
> @@ -128,3 +128,56 @@ Example device nodes:
>  				snps,quirk-frame-length-adjustment;
>  			};
>  	};
> +
> +Amlogic Meson A1 DWC3 USB SoC Controller Glue
> +
> +The Amlogic A1 embeds a DWC3 USB IP Core configured for USB2 in
> +host-only mode.
> +
> +Required properties:
> +- compatible:	Should be "amlogic,meson-a1-usb-ctrl"
> +- clocks:       The clocks needed by the usb controller
> +- clock-names:  Should contain the name of the clocks: "usb_ctrl", "usb_bus",
> +                "xtal_usb_phy", "xtal_usb_ctrl"
> +- resets:	a handle for the shared "USB" reset line
> +- reg:		The base address and length of the registers
> +- phys: 	handle to used PHYs on the system
> +	- a <0> phandle can be used if a PHY is not used
> +- phy-names:	names of the used PHYs on the system :
> +	- "usb2-phy0" for USB2 PHY if USBHOST port is used
> +
> +Required child nodes:
> +
> +A child node must exist to represent the core DWC3 IP block. The name of
> +the node is not important. The content of the node is defined in dwc3.txt.
> +
> +PHY documentation is provided in the following places:
> +- Documentation/devicetree/bindings/phy/amlogic,meson-a1-usb2-phy.yaml
> +
> +Example device nodes:
> +	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>;
> +			};
> +	};
> 

This seems very similar to the g12a bindings, seems you could update the yaml g12a bindings
with specific clocks and required for amlogic,meson-a1-usb-ctrl.

Neil

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

* Re: [PATCH 4/6] usb: dwc3: Add Amlogic A1 DWC3 glue
  2019-11-22  6:55 ` [PATCH 4/6] usb: dwc3: Add Amlogic A1 DWC3 glue Hanjie Lin
@ 2019-11-22  8:53   ` Neil Armstrong
  2019-11-25  7:53     ` Hanjie Lin
  0 siblings, 1 reply; 19+ messages in thread
From: Neil Armstrong @ 2019-11-22  8:53 UTC (permalink / raw)
  To: Hanjie Lin, Jerome Brunet, Rob Herring, Greg Kroah-Hartman,
	Felipe Balbi, Kevin Hilman
  Cc: Yue Wang, linux-amlogic, linux-arm-kernel, linux-usb, devicetree,
	Carlo Caione, Michael Turquette, Stephen Boyd,
	Martin Blumenstingl, Liang Yang, Jianxin Pan, Qiufang Dai,
	Jian Hu, Victor Wan, Xingyu Chen

Hi,

On 22/11/2019 07:55, 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/Kconfig         |  11 ++
>  drivers/usb/dwc3/Makefile        |   1 +
>  drivers/usb/dwc3/dwc3-meson-a1.c | 397 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 409 insertions(+)
>  create mode 100644 drivers/usb/dwc3/dwc3-meson-a1.c
> 
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 556a876..9bfb159 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -96,6 +96,17 @@ config USB_DWC3_KEYSTONE
>  	  Support of USB2/3 functionality in TI Keystone2 and AM654 platforms.
>  	  Say 'Y' or 'M' here if you have one such device
>  
> +config USB_DWC3_MESON_A1
> +	tristate "Amlogic Meson A1 Platforms"
> +	depends on OF && COMMON_CLK
> +	depends on ARCH_MESON || COMPILE_TEST
> +	default USB_DWC3
> +	help
> +	  Support USB2 functionality in MESON A1 platforms.
> +	  The MESON A1 USB2 support a DWC3 USB IP Core configured for USB2 in
> +	  host-only mode.
> +	  Say 'Y' or 'M' if you have one such device.
> +
>  config USB_DWC3_MESON_G12A
>         tristate "Amlogic Meson G12A Platforms"
>         depends on OF && COMMON_CLK
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index ae86da0..a3fc655 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -47,6 +47,7 @@ obj-$(CONFIG_USB_DWC3_EXYNOS)		+= dwc3-exynos.o
>  obj-$(CONFIG_USB_DWC3_PCI)		+= dwc3-pci.o
>  obj-$(CONFIG_USB_DWC3_HAPS)		+= dwc3-haps.o
>  obj-$(CONFIG_USB_DWC3_KEYSTONE)		+= dwc3-keystone.o
> +obj-$(CONFIG_USB_DWC3_MESON_A1)		+= dwc3-meson-a1.o
>  obj-$(CONFIG_USB_DWC3_MESON_G12A)	+= dwc3-meson-g12a.o
>  obj-$(CONFIG_USB_DWC3_OF_SIMPLE)	+= dwc3-of-simple.o
>  obj-$(CONFIG_USB_DWC3_ST)		+= dwc3-st.o
> diff --git a/drivers/usb/dwc3/dwc3-meson-a1.c b/drivers/usb/dwc3/dwc3-meson-a1.c
> new file mode 100644
> index 00000000..db2b99a
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-meson-a1.c
> @@ -0,0 +1,397 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * USB Glue for Amlogic A1 SoCs
> + *
> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved
> + * Author: Yue Wang <yue.wang@amlogic.com>
> + */
> +
> +/*
> + * The USB is organized with a glue around the DWC3 Controller IP as :
> + * - Control registers for each USB2 Ports
> + * - Control registers for the USB PHY layer
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/reset.h>
> +#include <linux/phy/phy.h>
> +#include <linux/usb/otg.h>
> +#include <linux/usb/role.h>
> +#include <linux/regulator/consumer.h>
> +
> +/* USB2 Ports Control Registers */
> +#define U2P_R0							0x20
> +	#define U2P_R0_HOST_DEVICE				BIT(0)
> +	#define U2P_R0_POWER_OK					BIT(1)
> +	#define U2P_R0_HAST_MODE				BIT(2)
> +	#define U2P_R0_POWER_ON_RESET				BIT(3)
> +	#define U2P_R0_ID_PULLUP				BIT(4)
> +	#define U2P_R0_DRV_VBUS					BIT(5)
> +
> +#define U2P_R1							0x24
> +	#define U2P_R1_PHY_READY				BIT(0)
> +	#define U2P_R1_ID_DIG					BIT(1)
> +	#define U2P_R1_OTG_SESSION_VALID			BIT(2)
> +	#define U2P_R1_VBUS_VALID				BIT(3)
> +
> +/* USB Glue Control Registers */
> +
> +#define USB_R0							0x80
> +	#define USB_R0_P30_LANE0_TX2RX_LOOPBACK			BIT(17)
> +	#define USB_R0_P30_LANE0_EXT_PCLK_REQ			BIT(18)
> +	#define USB_R0_P30_PCS_RX_LOS_MASK_VAL_MASK		GENMASK(28, 19)
> +	#define USB_R0_U2D_SS_SCALEDOWN_MODE_MASK		GENMASK(30, 29)
> +	#define USB_R0_U2D_ACT					BIT(31)
> +
> +#define USB_R1							0x84
> +	#define USB_R1_U3H_BIGENDIAN_GS				BIT(0)
> +	#define USB_R1_U3H_PME_ENABLE				BIT(1)
> +	#define USB_R1_U3H_HUB_PORT_OVERCURRENT_MASK		GENMASK(4, 2)
> +	#define USB_R1_U3H_HUB_PORT_PERM_ATTACH_MASK		GENMASK(9, 7)
> +	#define USB_R1_U3H_HOST_U2_PORT_DISABLE_MASK		GENMASK(13, 12)
> +	#define USB_R1_U3H_HOST_U3_PORT_DISABLE			BIT(16)
> +	#define USB_R1_U3H_HOST_PORT_POWER_CONTROL_PRESENT	BIT(17)
> +	#define USB_R1_U3H_HOST_MSI_ENABLE			BIT(18)
> +	#define USB_R1_U3H_FLADJ_30MHZ_REG_MASK			GENMASK(24, 19)
> +	#define USB_R1_P30_PCS_TX_SWING_FULL_MASK		GENMASK(31, 25)
> +
> +#define USB_R2							0x88
> +	#define USB_R2_P30_PCS_TX_DEEMPH_3P5DB_MASK		GENMASK(25, 20)
> +	#define USB_R2_P30_PCS_TX_DEEMPH_6DB_MASK		GENMASK(31, 26)
> +
> +#define USB_R3							0x8c
> +	#define USB_R3_P30_SSC_ENABLE				BIT(0)
> +	#define USB_R3_P30_SSC_RANGE_MASK			GENMASK(3, 1)
> +	#define USB_R3_P30_SSC_REF_CLK_SEL_MASK			GENMASK(12, 4)
> +	#define USB_R3_P30_REF_SSP_EN				BIT(13)
> +
> +#define USB_R4							0x90
> +	#define USB_R4_P21_PORT_RESET_0				BIT(0)
> +	#define USB_R4_P21_SLEEP_M0				BIT(1)
> +	#define USB_R4_MEM_PD_MASK				GENMASK(3, 2)
> +	#define USB_R4_P21_ONLY					BIT(4)
> +
> +#define USB_R5							0x94
> +	#define USB_R5_ID_DIG_SYNC				BIT(0)
> +	#define USB_R5_ID_DIG_REG				BIT(1)
> +	#define USB_R5_ID_DIG_CFG_MASK				GENMASK(3, 2)
> +	#define USB_R5_ID_DIG_EN_0				BIT(4)
> +	#define USB_R5_ID_DIG_EN_1				BIT(5)
> +	#define USB_R5_ID_DIG_CURR				BIT(6)
> +	#define USB_R5_ID_DIG_IRQ				BIT(7)
> +	#define USB_R5_ID_DIG_TH_MASK				GENMASK(15, 8)
> +	#define USB_R5_ID_DIG_CNT_MASK				GENMASK(23, 16)
> +
> +static const char *phy_names = {
> +	"usb2-phy0",
> +};
> +
> +struct dwc3_meson_a1 {
> +	struct device		*dev;
> +	struct regmap		*regmap;
> +	struct clk		*clk_usb_ctrl;
> +	struct clk		*clk_usb_bus;
> +	struct clk		*clk_xtal_usb_phy;
> +	struct clk		*clk_xtal_usb_ctrl;
> +	struct reset_control	*reset;
> +	struct phy		*phys;
> +	unsigned int		usb2_ports;
> +};
> +
> +static void dwc3_meson_a1_usb_init(struct dwc3_meson_a1 *priv)
> +{
> +	regmap_update_bits(priv->regmap, U2P_R0,
> +			   U2P_R0_POWER_ON_RESET,
> +			   U2P_R0_POWER_ON_RESET);
> +
> +	regmap_update_bits(priv->regmap, U2P_R0,
> +			   U2P_R0_HOST_DEVICE,
> +			   U2P_R0_HOST_DEVICE);
> +
> +	regmap_update_bits(priv->regmap, U2P_R0,
> +			   U2P_R0_POWER_ON_RESET, 0);
> +
> +	regmap_update_bits(priv->regmap, USB_R1,
> +			   USB_R1_U3H_FLADJ_30MHZ_REG_MASK,
> +			   FIELD_PREP(USB_R1_U3H_FLADJ_30MHZ_REG_MASK, 0x20));
> +
> +	regmap_update_bits(priv->regmap, USB_R0,
> +			   USB_R0_U2D_ACT, 0);
> +
> +	regmap_update_bits(priv->regmap, USB_R4,
> +			   USB_R4_P21_SLEEP_M0, 0);
> +}
> +
> +static const struct regmap_config phy_meson_a1_usb_regmap_conf = {
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +	.max_register = USB_R5,
> +};
> +
> +static int dwc3_meson_a1_get_phys(struct dwc3_meson_a1 *priv)
> +{
> +	priv->phys = devm_phy_optional_get(priv->dev, phy_names);
> +	if (IS_ERR(priv->phys))
> +		return PTR_ERR(priv->phys);
> +
> +	priv->usb2_ports++;
> +
> +	dev_info(priv->dev, "USB2 ports: %d\n", priv->usb2_ports);
> +
> +	return 0;
> +}
> +
> +static int dwc3_meson_a1_enable_clk(struct dwc3_meson_a1 *priv)
> +{
> +	int ret;
> +
> +	ret = clk_prepare_enable(priv->clk_usb_ctrl);
> +	if (ret < 0) {
> +		dev_err(priv->dev, "can't enable usb_ctrl clock.\n");
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(priv->clk_usb_bus);
> +	if (ret < 0) {
> +		dev_err(priv->dev, "can't enable usb_bus clock.\n");
> +		goto disable_clk_usb_ctrl;
> +	}
> +
> +	ret = clk_prepare_enable(priv->clk_xtal_usb_phy);
> +	if (ret < 0) {
> +		dev_err(priv->dev, "can't enable xtal_usb_phy clock.\n");
> +		goto disable_clk_usb_bus;
> +	}
> +
> +	ret = clk_prepare_enable(priv->clk_xtal_usb_ctrl);
> +	if (ret < 0) {
> +		dev_err(priv->dev, "can't enable xtal_usb_ctrl clock.\n");
> +		goto disable_clk_xtal_usb_phy;
> +	}
> +
> +	return 0;
> +
> +disable_clk_xtal_usb_phy:
> +	clk_disable_unprepare(priv->clk_xtal_usb_phy);
> +disable_clk_usb_bus:
> +	clk_disable_unprepare(priv->clk_usb_bus);
> +disable_clk_usb_ctrl:
> +	clk_disable_unprepare(priv->clk_usb_ctrl);
> +
> +	return ret;
> +}
> +
> +static void dwc3_meson_a1_disable_clk(struct dwc3_meson_a1 *priv)
> +{
> +	clk_disable_unprepare(priv->clk_usb_ctrl);
> +	clk_disable_unprepare(priv->clk_usb_bus);
> +	clk_disable_unprepare(priv->clk_xtal_usb_phy);
> +	clk_disable_unprepare(priv->clk_xtal_usb_ctrl);
> +}
> +
> +static int dwc3_meson_a1_setup_clk(struct dwc3_meson_a1 *priv)
> +{
> +	int ret;
> +
> +	priv->clk_usb_ctrl = devm_clk_get(priv->dev, "usb_ctrl");
> +	if (IS_ERR(priv->clk_usb_ctrl)) {
> +		dev_err(priv->dev, "can't get usb_ctrl clock.\n");
> +		return PTR_ERR(priv->clk_usb_ctrl);
> +	}
> +
> +	priv->clk_usb_bus = devm_clk_get(priv->dev, "usb_bus");
> +	if (IS_ERR(priv->clk_usb_bus)) {
> +		dev_err(priv->dev, "can't get usb_bus clock.\n");
> +		return PTR_ERR(priv->clk_usb_bus);
> +	}
> +
> +	priv->clk_xtal_usb_phy = devm_clk_get(priv->dev, "xtal_usb_phy");
> +	if (IS_ERR(priv->clk_xtal_usb_phy)) {
> +		dev_err(priv->dev, "can't get xtal_usb_phy clock.\n");
> +		return PTR_ERR(priv->clk_xtal_usb_phy);
> +	}
> +
> +	priv->clk_xtal_usb_ctrl = devm_clk_get(priv->dev, "xtal_usb_ctrl");
> +	if (IS_ERR(priv->clk_xtal_usb_ctrl)) {
> +		dev_err(priv->dev, "can't get xtal_usb_ctrl clock.\n");
> +		return PTR_ERR(priv->clk_xtal_usb_ctrl);
> +	}
> +
> +	ret = dwc3_meson_a1_enable_clk(priv);
> +	if (ret)
> +		return ret;
> +
> +	devm_add_action_or_reset(priv->dev,
> +				 (void(*)(void *))clk_disable_unprepare,
> +				 priv->clk_usb_ctrl);
> +	devm_add_action_or_reset(priv->dev,
> +				 (void(*)(void *))clk_disable_unprepare,
> +				 priv->clk_usb_bus);
> +	devm_add_action_or_reset(priv->dev,
> +				 (void(*)(void *))clk_disable_unprepare,
> +				 priv->clk_xtal_usb_phy);
> +	devm_add_action_or_reset(priv->dev,
> +				 (void(*)(void *))clk_disable_unprepare,
> +				 priv->clk_xtal_usb_ctrl);
> +
> +	return 0;
> +}
> +
> +static int dwc3_meson_a1_probe(struct platform_device *pdev)
> +{
> +	struct dwc3_meson_a1	*priv;
> +	struct device		*dev = &pdev->dev;
> +	struct device_node	*np = dev->of_node;
> +	void __iomem *base;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, priv);
> +	priv->dev = dev;
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	priv->regmap = devm_regmap_init_mmio(dev, base,
> +					     &phy_meson_a1_usb_regmap_conf);
> +	if (IS_ERR(priv->regmap))
> +		return PTR_ERR(priv->regmap);
> +
> +	ret = dwc3_meson_a1_setup_clk(priv);
> +	if (ret)
> +		return ret;
> +
> +	priv->reset = devm_reset_control_get(dev, NULL);
> +	if (IS_ERR(priv->reset)) {
> +		ret = PTR_ERR(priv->reset);
> +		dev_err(dev, "failed to get device reset, err=%d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = reset_control_reset(priv->reset);
> +	if (ret)
> +		return ret;
> +
> +	ret = dwc3_meson_a1_get_phys(priv);
> +	if (ret)
> +		return ret;
> +
> +	dwc3_meson_a1_usb_init(priv);
> +
> +	/* Init PHYs */
> +	ret = phy_init(priv->phys);
> +	if (ret)
> +		return ret;
> +
> +	/* Set PHY Power */
> +	ret = phy_power_on(priv->phys);
> +	if (ret)
> +		goto err_phys_exit;
> +
> +	ret = of_platform_populate(np, NULL, NULL, dev);
> +	if (ret)
> +		goto err_phys_power;
> +
> +	return 0;
> +
> +err_phys_power:
> +	phy_power_off(priv->phys);
> +
> +err_phys_exit:
> +	phy_exit(priv->phys);
> +
> +	return ret;
> +}
> +
> +static int dwc3_meson_a1_remove(struct platform_device *pdev)
> +{
> +	struct dwc3_meson_a1 *priv = platform_get_drvdata(pdev);
> +	struct device *dev = &pdev->dev;
> +
> +	of_platform_depopulate(dev);
> +
> +	phy_power_off(priv->phys);
> +	phy_exit(priv->phys);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused dwc3_meson_a1_suspend(struct device *dev)
> +{
> +	struct dwc3_meson_a1 *priv = dev_get_drvdata(dev);
> +
> +	phy_power_off(priv->phys);
> +	phy_exit(priv->phys);
> +
> +	reset_control_assert(priv->reset);
> +
> +	dwc3_meson_a1_disable_clk(priv);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused dwc3_meson_a1_resume(struct device *dev)
> +{
> +	struct dwc3_meson_a1 *priv = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = dwc3_meson_a1_enable_clk(priv);
> +	if (ret)
> +		return ret;
> +
> +	reset_control_deassert(priv->reset);
> +
> +	dwc3_meson_a1_usb_init(priv);
> +
> +	/* Init PHYs */
> +	ret = phy_init(priv->phys);
> +	if (ret)
> +		return ret;
> +
> +	/* Set PHY Power */
> +	ret = phy_power_on(priv->phys);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops dwc3_meson_a1_dev_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(dwc3_meson_a1_suspend, dwc3_meson_a1_resume)
> +};
> +
> +static const struct of_device_id dwc3_meson_a1_match[] = {
> +	{ .compatible = "amlogic,meson-a1-usb-ctrl" },
> +	{ /* Sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, dwc3_meson_a1_match);
> +
> +static struct platform_driver dwc3_meson_a1_driver = {
> +	.probe		= dwc3_meson_a1_probe,
> +	.remove		= dwc3_meson_a1_remove,
> +	.driver		= {
> +		.name	= "dwc3-meson-a1",
> +		.of_match_table = dwc3_meson_a1_match,
> +		.pm	= &dwc3_meson_a1_dev_pm_ops,
> +	},
> +};
> +
> +module_platform_driver(dwc3_meson_a1_driver);
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Amlogic Meson A1 USB Glue Layer");
> +MODULE_AUTHOR("Yue Wang <yue.wang@amlogic.com>");
> 

This driver looks very close to the g12a glue driver, could you reuse the g12a driver instead ?

Neil

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

* Re: [PATCH 1/6] dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings
  2019-11-22  6:55 ` [PATCH 1/6] dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings Hanjie Lin
@ 2019-11-22 22:52   ` Rob Herring
  0 siblings, 0 replies; 19+ messages in thread
From: Rob Herring @ 2019-11-22 22:52 UTC (permalink / raw)
  To: Hanjie Lin
  Cc: Jerome Brunet, Neil Armstrong, Greg Kroah-Hartman, Felipe Balbi,
	Kevin Hilman, 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

On Fri, 22 Nov 2019 14:55:52 +0800, Hanjie Lin 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    | 55 ++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/amlogic,meson-a1-usb2-phy.yaml
> 

Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.

If a tag was not added on purpose, please state why and what changed.

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

* Re: [PATCH 2/6] dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings
  2019-11-22  8:52   ` Neil Armstrong
@ 2019-11-25  7:52     ` Hanjie Lin
  2019-12-04 19:47       ` Rob Herring
  0 siblings, 1 reply; 19+ messages in thread
From: Hanjie Lin @ 2019-11-25  7:52 UTC (permalink / raw)
  To: Neil Armstrong, Jerome Brunet, Rob Herring, Greg Kroah-Hartman,
	Felipe Balbi, Kevin Hilman
  Cc: Yue Wang, linux-amlogic, linux-arm-kernel, linux-usb, devicetree,
	Carlo Caione, Michael Turquette, Stephen Boyd,
	Martin Blumenstingl, Liang Yang, Jianxin Pan, Qiufang Dai,
	Jian Hu, Victor Wan, Xingyu Chen



On 2019/11/22 16:52, Neil Armstrong wrote:
> Hi,
> 
> 
> On 22/11/2019 07:55, 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>
>> ---
>>  .../devicetree/bindings/usb/amlogic,dwc3.txt       | 53 ++++++++++++++++++++++
>>  1 file changed, 53 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt b/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
>> index 6ffb09b..63dc60b 100644
>> --- a/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
>> +++ b/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
>> @@ -128,3 +128,56 @@ Example device nodes:
>>  				snps,quirk-frame-length-adjustment;
>>  			};
>>  	};
>> +
>> +Amlogic Meson A1 DWC3 USB SoC Controller Glue
>> +
>> +The Amlogic A1 embeds a DWC3 USB IP Core configured for USB2 in
>> +host-only mode.
>> +
>> +Required properties:
>> +- compatible:	Should be "amlogic,meson-a1-usb-ctrl"
>> +- clocks:       The clocks needed by the usb controller
>> +- clock-names:  Should contain the name of the clocks: "usb_ctrl", "usb_bus",
>> +                "xtal_usb_phy", "xtal_usb_ctrl"
>> +- resets:	a handle for the shared "USB" reset line
>> +- reg:		The base address and length of the registers
>> +- phys: 	handle to used PHYs on the system
>> +	- a <0> phandle can be used if a PHY is not used
>> +- phy-names:	names of the used PHYs on the system :
>> +	- "usb2-phy0" for USB2 PHY if USBHOST port is used
>> +
>> +Required child nodes:
>> +
>> +A child node must exist to represent the core DWC3 IP block. The name of
>> +the node is not important. The content of the node is defined in dwc3.txt.
>> +
>> +PHY documentation is provided in the following places:
>> +- Documentation/devicetree/bindings/phy/amlogic,meson-a1-usb2-phy.yaml
>> +
>> +Example device nodes:
>> +	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>;
>> +			};
>> +	};
>>
> 
> This seems very similar to the g12a bindings, seems you could update the yaml g12a bindings
> with specific clocks and required for amlogic,meson-a1-usb-ctrl.
> 
> Neil
> 
> .
> 

Hi Neil
Thanks for the comment.

1, G12A have usb2-phy0/usb2-phy1/usb3-phy0 three phys and an interrupt to support host/peripheral/otg modes.
   A1 has one usb2-phy0 phy and only support host mode.
   
2, G12A glue/phy drivers are for G12A SoCs, there are some diffrences to A1.
   G12A glue driver have dr_mode and interrupts two attributes to support otg mode while A1 hasn't this requirement.
   G12A glue driver has a hard coding vbus regulator code to support otg mode while A1 hasn't this requirement.
   G12A glue driver has a hard coding support phys while A1 only supports host mode.
   	enum {
		USB2_HOST_PHY = 0,
		USB2_OTG_PHY,
		USB3_HOST_PHY,
		PHY_COUNT,
		};
   G12A glue driver only supports one clock while A1 needs four clocks.
   G12A and A1 phy drivers have different register configurations since hardware differences.
   
3, We have estimated these differences and we thought it's more clear and readable to have a dedicated glue/phy
   driver for A1 SoCs, so also dedicated dt-bindings.


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

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



On 2019/11/22 16:53, Neil Armstrong wrote:
> Hi,
> 
> On 22/11/2019 07:55, 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/Kconfig         |  11 ++
>>  drivers/usb/dwc3/Makefile        |   1 +
>>  drivers/usb/dwc3/dwc3-meson-a1.c | 397 +++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 409 insertions(+)
>>  create mode 100644 drivers/usb/dwc3/dwc3-meson-a1.c
>>
>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
>> index 556a876..9bfb159 100644
>> --- a/drivers/usb/dwc3/Kconfig
>> +++ b/drivers/usb/dwc3/Kconfig
>> @@ -96,6 +96,17 @@ config USB_DWC3_KEYSTONE
>>  	  Support of USB2/3 functionality in TI Keystone2 and AM654 platforms.
>>  	  Say 'Y' or 'M' here if you have one such device
>>  
>> +config USB_DWC3_MESON_A1
>> +	tristate "Amlogic Meson A1 Platforms"
>> +	depends on OF && COMMON_CLK
>> +	depends on ARCH_MESON || COMPILE_TEST
>> +	default USB_DWC3
>> +	help
>> +	  Support USB2 functionality in MESON A1 platforms.
>> +	  The MESON A1 USB2 support a DWC3 USB IP Core configured for USB2 in
>> +	  host-only mode.
>> +	  Say 'Y' or 'M' if you have one such device.
>> +
>>  config USB_DWC3_MESON_G12A
>>         tristate "Amlogic Meson G12A Platforms"
>>         depends on OF && COMMON_CLK
>> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
>> index ae86da0..a3fc655 100644
>> --- a/drivers/usb/dwc3/Makefile
>> +++ b/drivers/usb/dwc3/Makefile
>> @@ -47,6 +47,7 @@ obj-$(CONFIG_USB_DWC3_EXYNOS)		+= dwc3-exynos.o
>>  obj-$(CONFIG_USB_DWC3_PCI)		+= dwc3-pci.o
>>  obj-$(CONFIG_USB_DWC3_HAPS)		+= dwc3-haps.o
>>  obj-$(CONFIG_USB_DWC3_KEYSTONE)		+= dwc3-keystone.o
>> +obj-$(CONFIG_USB_DWC3_MESON_A1)		+= dwc3-meson-a1.o
>>  obj-$(CONFIG_USB_DWC3_MESON_G12A)	+= dwc3-meson-g12a.o
>>  obj-$(CONFIG_USB_DWC3_OF_SIMPLE)	+= dwc3-of-simple.o
>>  obj-$(CONFIG_USB_DWC3_ST)		+= dwc3-st.o
>> diff --git a/drivers/usb/dwc3/dwc3-meson-a1.c b/drivers/usb/dwc3/dwc3-meson-a1.c
>> new file mode 100644
>> index 00000000..db2b99a
>> --- /dev/null
>> +++ b/drivers/usb/dwc3/dwc3-meson-a1.c
>> @@ -0,0 +1,397 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * USB Glue for Amlogic A1 SoCs
>> + *
>> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved
>> + * Author: Yue Wang <yue.wang@amlogic.com>
>> + */
>> +
>> +/*
>> + * The USB is organized with a glue around the DWC3 Controller IP as :
>> + * - Control registers for each USB2 Ports
>> + * - Control registers for the USB PHY layer
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/kernel.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/clk.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/pm_domain.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/regmap.h>
>> +#include <linux/bitfield.h>
>> +#include <linux/bitops.h>
>> +#include <linux/reset.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/usb/otg.h>
>> +#include <linux/usb/role.h>
>> +#include <linux/regulator/consumer.h>
>> +
>> +/* USB2 Ports Control Registers */
>> +#define U2P_R0							0x20
>> +	#define U2P_R0_HOST_DEVICE				BIT(0)
>> +	#define U2P_R0_POWER_OK					BIT(1)
>> +	#define U2P_R0_HAST_MODE				BIT(2)
>> +	#define U2P_R0_POWER_ON_RESET				BIT(3)
>> +	#define U2P_R0_ID_PULLUP				BIT(4)
>> +	#define U2P_R0_DRV_VBUS					BIT(5)
>> +
>> +#define U2P_R1							0x24
>> +	#define U2P_R1_PHY_READY				BIT(0)
>> +	#define U2P_R1_ID_DIG					BIT(1)
>> +	#define U2P_R1_OTG_SESSION_VALID			BIT(2)
>> +	#define U2P_R1_VBUS_VALID				BIT(3)
>> +
>> +/* USB Glue Control Registers */
>> +
>> +#define USB_R0							0x80
>> +	#define USB_R0_P30_LANE0_TX2RX_LOOPBACK			BIT(17)
>> +	#define USB_R0_P30_LANE0_EXT_PCLK_REQ			BIT(18)
>> +	#define USB_R0_P30_PCS_RX_LOS_MASK_VAL_MASK		GENMASK(28, 19)
>> +	#define USB_R0_U2D_SS_SCALEDOWN_MODE_MASK		GENMASK(30, 29)
>> +	#define USB_R0_U2D_ACT					BIT(31)
>> +
>> +#define USB_R1							0x84
>> +	#define USB_R1_U3H_BIGENDIAN_GS				BIT(0)
>> +	#define USB_R1_U3H_PME_ENABLE				BIT(1)
>> +	#define USB_R1_U3H_HUB_PORT_OVERCURRENT_MASK		GENMASK(4, 2)
>> +	#define USB_R1_U3H_HUB_PORT_PERM_ATTACH_MASK		GENMASK(9, 7)
>> +	#define USB_R1_U3H_HOST_U2_PORT_DISABLE_MASK		GENMASK(13, 12)
>> +	#define USB_R1_U3H_HOST_U3_PORT_DISABLE			BIT(16)
>> +	#define USB_R1_U3H_HOST_PORT_POWER_CONTROL_PRESENT	BIT(17)
>> +	#define USB_R1_U3H_HOST_MSI_ENABLE			BIT(18)
>> +	#define USB_R1_U3H_FLADJ_30MHZ_REG_MASK			GENMASK(24, 19)
>> +	#define USB_R1_P30_PCS_TX_SWING_FULL_MASK		GENMASK(31, 25)
>> +
>> +#define USB_R2							0x88
>> +	#define USB_R2_P30_PCS_TX_DEEMPH_3P5DB_MASK		GENMASK(25, 20)
>> +	#define USB_R2_P30_PCS_TX_DEEMPH_6DB_MASK		GENMASK(31, 26)
>> +
>> +#define USB_R3							0x8c
>> +	#define USB_R3_P30_SSC_ENABLE				BIT(0)
>> +	#define USB_R3_P30_SSC_RANGE_MASK			GENMASK(3, 1)
>> +	#define USB_R3_P30_SSC_REF_CLK_SEL_MASK			GENMASK(12, 4)
>> +	#define USB_R3_P30_REF_SSP_EN				BIT(13)
>> +
>> +#define USB_R4							0x90
>> +	#define USB_R4_P21_PORT_RESET_0				BIT(0)
>> +	#define USB_R4_P21_SLEEP_M0				BIT(1)
>> +	#define USB_R4_MEM_PD_MASK				GENMASK(3, 2)
>> +	#define USB_R4_P21_ONLY					BIT(4)
>> +
>> +#define USB_R5							0x94
>> +	#define USB_R5_ID_DIG_SYNC				BIT(0)
>> +	#define USB_R5_ID_DIG_REG				BIT(1)
>> +	#define USB_R5_ID_DIG_CFG_MASK				GENMASK(3, 2)
>> +	#define USB_R5_ID_DIG_EN_0				BIT(4)
>> +	#define USB_R5_ID_DIG_EN_1				BIT(5)
>> +	#define USB_R5_ID_DIG_CURR				BIT(6)
>> +	#define USB_R5_ID_DIG_IRQ				BIT(7)
>> +	#define USB_R5_ID_DIG_TH_MASK				GENMASK(15, 8)
>> +	#define USB_R5_ID_DIG_CNT_MASK				GENMASK(23, 16)
>> +
>> +static const char *phy_names = {
>> +	"usb2-phy0",
>> +};
>> +
>> +struct dwc3_meson_a1 {
>> +	struct device		*dev;
>> +	struct regmap		*regmap;
>> +	struct clk		*clk_usb_ctrl;
>> +	struct clk		*clk_usb_bus;
>> +	struct clk		*clk_xtal_usb_phy;
>> +	struct clk		*clk_xtal_usb_ctrl;
>> +	struct reset_control	*reset;
>> +	struct phy		*phys;
>> +	unsigned int		usb2_ports;
>> +};
>> +
>> +static void dwc3_meson_a1_usb_init(struct dwc3_meson_a1 *priv)
>> +{
>> +	regmap_update_bits(priv->regmap, U2P_R0,
>> +			   U2P_R0_POWER_ON_RESET,
>> +			   U2P_R0_POWER_ON_RESET);
>> +
>> +	regmap_update_bits(priv->regmap, U2P_R0,
>> +			   U2P_R0_HOST_DEVICE,
>> +			   U2P_R0_HOST_DEVICE);
>> +
>> +	regmap_update_bits(priv->regmap, U2P_R0,
>> +			   U2P_R0_POWER_ON_RESET, 0);
>> +
>> +	regmap_update_bits(priv->regmap, USB_R1,
>> +			   USB_R1_U3H_FLADJ_30MHZ_REG_MASK,
>> +			   FIELD_PREP(USB_R1_U3H_FLADJ_30MHZ_REG_MASK, 0x20));
>> +
>> +	regmap_update_bits(priv->regmap, USB_R0,
>> +			   USB_R0_U2D_ACT, 0);
>> +
>> +	regmap_update_bits(priv->regmap, USB_R4,
>> +			   USB_R4_P21_SLEEP_M0, 0);
>> +}
>> +
>> +static const struct regmap_config phy_meson_a1_usb_regmap_conf = {
>> +	.reg_bits = 32,
>> +	.val_bits = 32,
>> +	.reg_stride = 4,
>> +	.max_register = USB_R5,
>> +};
>> +
>> +static int dwc3_meson_a1_get_phys(struct dwc3_meson_a1 *priv)
>> +{
>> +	priv->phys = devm_phy_optional_get(priv->dev, phy_names);
>> +	if (IS_ERR(priv->phys))
>> +		return PTR_ERR(priv->phys);
>> +
>> +	priv->usb2_ports++;
>> +
>> +	dev_info(priv->dev, "USB2 ports: %d\n", priv->usb2_ports);
>> +
>> +	return 0;
>> +}
>> +
>> +static int dwc3_meson_a1_enable_clk(struct dwc3_meson_a1 *priv)
>> +{
>> +	int ret;
>> +
>> +	ret = clk_prepare_enable(priv->clk_usb_ctrl);
>> +	if (ret < 0) {
>> +		dev_err(priv->dev, "can't enable usb_ctrl clock.\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = clk_prepare_enable(priv->clk_usb_bus);
>> +	if (ret < 0) {
>> +		dev_err(priv->dev, "can't enable usb_bus clock.\n");
>> +		goto disable_clk_usb_ctrl;
>> +	}
>> +
>> +	ret = clk_prepare_enable(priv->clk_xtal_usb_phy);
>> +	if (ret < 0) {
>> +		dev_err(priv->dev, "can't enable xtal_usb_phy clock.\n");
>> +		goto disable_clk_usb_bus;
>> +	}
>> +
>> +	ret = clk_prepare_enable(priv->clk_xtal_usb_ctrl);
>> +	if (ret < 0) {
>> +		dev_err(priv->dev, "can't enable xtal_usb_ctrl clock.\n");
>> +		goto disable_clk_xtal_usb_phy;
>> +	}
>> +
>> +	return 0;
>> +
>> +disable_clk_xtal_usb_phy:
>> +	clk_disable_unprepare(priv->clk_xtal_usb_phy);
>> +disable_clk_usb_bus:
>> +	clk_disable_unprepare(priv->clk_usb_bus);
>> +disable_clk_usb_ctrl:
>> +	clk_disable_unprepare(priv->clk_usb_ctrl);
>> +
>> +	return ret;
>> +}
>> +
>> +static void dwc3_meson_a1_disable_clk(struct dwc3_meson_a1 *priv)
>> +{
>> +	clk_disable_unprepare(priv->clk_usb_ctrl);
>> +	clk_disable_unprepare(priv->clk_usb_bus);
>> +	clk_disable_unprepare(priv->clk_xtal_usb_phy);
>> +	clk_disable_unprepare(priv->clk_xtal_usb_ctrl);
>> +}
>> +
>> +static int dwc3_meson_a1_setup_clk(struct dwc3_meson_a1 *priv)
>> +{
>> +	int ret;
>> +
>> +	priv->clk_usb_ctrl = devm_clk_get(priv->dev, "usb_ctrl");
>> +	if (IS_ERR(priv->clk_usb_ctrl)) {
>> +		dev_err(priv->dev, "can't get usb_ctrl clock.\n");
>> +		return PTR_ERR(priv->clk_usb_ctrl);
>> +	}
>> +
>> +	priv->clk_usb_bus = devm_clk_get(priv->dev, "usb_bus");
>> +	if (IS_ERR(priv->clk_usb_bus)) {
>> +		dev_err(priv->dev, "can't get usb_bus clock.\n");
>> +		return PTR_ERR(priv->clk_usb_bus);
>> +	}
>> +
>> +	priv->clk_xtal_usb_phy = devm_clk_get(priv->dev, "xtal_usb_phy");
>> +	if (IS_ERR(priv->clk_xtal_usb_phy)) {
>> +		dev_err(priv->dev, "can't get xtal_usb_phy clock.\n");
>> +		return PTR_ERR(priv->clk_xtal_usb_phy);
>> +	}
>> +
>> +	priv->clk_xtal_usb_ctrl = devm_clk_get(priv->dev, "xtal_usb_ctrl");
>> +	if (IS_ERR(priv->clk_xtal_usb_ctrl)) {
>> +		dev_err(priv->dev, "can't get xtal_usb_ctrl clock.\n");
>> +		return PTR_ERR(priv->clk_xtal_usb_ctrl);
>> +	}
>> +
>> +	ret = dwc3_meson_a1_enable_clk(priv);
>> +	if (ret)
>> +		return ret;
>> +
>> +	devm_add_action_or_reset(priv->dev,
>> +				 (void(*)(void *))clk_disable_unprepare,
>> +				 priv->clk_usb_ctrl);
>> +	devm_add_action_or_reset(priv->dev,
>> +				 (void(*)(void *))clk_disable_unprepare,
>> +				 priv->clk_usb_bus);
>> +	devm_add_action_or_reset(priv->dev,
>> +				 (void(*)(void *))clk_disable_unprepare,
>> +				 priv->clk_xtal_usb_phy);
>> +	devm_add_action_or_reset(priv->dev,
>> +				 (void(*)(void *))clk_disable_unprepare,
>> +				 priv->clk_xtal_usb_ctrl);
>> +
>> +	return 0;
>> +}
>> +
>> +static int dwc3_meson_a1_probe(struct platform_device *pdev)
>> +{
>> +	struct dwc3_meson_a1	*priv;
>> +	struct device		*dev = &pdev->dev;
>> +	struct device_node	*np = dev->of_node;
>> +	void __iomem *base;
>> +	int ret;
>> +
>> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	platform_set_drvdata(pdev, priv);
>> +	priv->dev = dev;
>> +
>> +	base = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(base))
>> +		return PTR_ERR(base);
>> +
>> +	priv->regmap = devm_regmap_init_mmio(dev, base,
>> +					     &phy_meson_a1_usb_regmap_conf);
>> +	if (IS_ERR(priv->regmap))
>> +		return PTR_ERR(priv->regmap);
>> +
>> +	ret = dwc3_meson_a1_setup_clk(priv);
>> +	if (ret)
>> +		return ret;
>> +
>> +	priv->reset = devm_reset_control_get(dev, NULL);
>> +	if (IS_ERR(priv->reset)) {
>> +		ret = PTR_ERR(priv->reset);
>> +		dev_err(dev, "failed to get device reset, err=%d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = reset_control_reset(priv->reset);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = dwc3_meson_a1_get_phys(priv);
>> +	if (ret)
>> +		return ret;
>> +
>> +	dwc3_meson_a1_usb_init(priv);
>> +
>> +	/* Init PHYs */
>> +	ret = phy_init(priv->phys);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Set PHY Power */
>> +	ret = phy_power_on(priv->phys);
>> +	if (ret)
>> +		goto err_phys_exit;
>> +
>> +	ret = of_platform_populate(np, NULL, NULL, dev);
>> +	if (ret)
>> +		goto err_phys_power;
>> +
>> +	return 0;
>> +
>> +err_phys_power:
>> +	phy_power_off(priv->phys);
>> +
>> +err_phys_exit:
>> +	phy_exit(priv->phys);
>> +
>> +	return ret;
>> +}
>> +
>> +static int dwc3_meson_a1_remove(struct platform_device *pdev)
>> +{
>> +	struct dwc3_meson_a1 *priv = platform_get_drvdata(pdev);
>> +	struct device *dev = &pdev->dev;
>> +
>> +	of_platform_depopulate(dev);
>> +
>> +	phy_power_off(priv->phys);
>> +	phy_exit(priv->phys);
>> +
>> +	return 0;
>> +}
>> +
>> +static int __maybe_unused dwc3_meson_a1_suspend(struct device *dev)
>> +{
>> +	struct dwc3_meson_a1 *priv = dev_get_drvdata(dev);
>> +
>> +	phy_power_off(priv->phys);
>> +	phy_exit(priv->phys);
>> +
>> +	reset_control_assert(priv->reset);
>> +
>> +	dwc3_meson_a1_disable_clk(priv);
>> +
>> +	return 0;
>> +}
>> +
>> +static int __maybe_unused dwc3_meson_a1_resume(struct device *dev)
>> +{
>> +	struct dwc3_meson_a1 *priv = dev_get_drvdata(dev);
>> +	int ret;
>> +
>> +	ret = dwc3_meson_a1_enable_clk(priv);
>> +	if (ret)
>> +		return ret;
>> +
>> +	reset_control_deassert(priv->reset);
>> +
>> +	dwc3_meson_a1_usb_init(priv);
>> +
>> +	/* Init PHYs */
>> +	ret = phy_init(priv->phys);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Set PHY Power */
>> +	ret = phy_power_on(priv->phys);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct dev_pm_ops dwc3_meson_a1_dev_pm_ops = {
>> +	SET_SYSTEM_SLEEP_PM_OPS(dwc3_meson_a1_suspend, dwc3_meson_a1_resume)
>> +};
>> +
>> +static const struct of_device_id dwc3_meson_a1_match[] = {
>> +	{ .compatible = "amlogic,meson-a1-usb-ctrl" },
>> +	{ /* Sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, dwc3_meson_a1_match);
>> +
>> +static struct platform_driver dwc3_meson_a1_driver = {
>> +	.probe		= dwc3_meson_a1_probe,
>> +	.remove		= dwc3_meson_a1_remove,
>> +	.driver		= {
>> +		.name	= "dwc3-meson-a1",
>> +		.of_match_table = dwc3_meson_a1_match,
>> +		.pm	= &dwc3_meson_a1_dev_pm_ops,
>> +	},
>> +};
>> +
>> +module_platform_driver(dwc3_meson_a1_driver);
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("Amlogic Meson A1 USB Glue Layer");
>> +MODULE_AUTHOR("Yue Wang <yue.wang@amlogic.com>");
>>
> 
> This driver looks very close to the g12a glue driver, could you reuse the g12a driver instead ?
> 
> Neil
> 
> .
> 


Hi Neil,

thanks for the comment.

1, G12A have usb2-phy0/usb2-phy1/usb3-phy0 three phys and an interrupt to support host/peripheral/otg modes.
   A1 has one usb2-phy0 phy and only support host mode.
   
2, G12A glue/phy drivers are for G12A SoCs, there are some diffrences to A1.
   G12A glue driver have dr_mode and interrupts two attributes to support otg mode while A1 hasn't this requirement.
   G12A glue driver has a hard coding vbus regulator code to support otg mode while A1 hasn't this requirement.
   G12A glue driver has a hard coding support phys while A1 only supports host mode.
   	enum {
		USB2_HOST_PHY = 0,
		USB2_OTG_PHY,
		USB3_HOST_PHY,
		PHY_COUNT,
		};
   G12A glue driver only supports one clock while A1 needs four clocks.
   G12A and A1 phy drivers have different register configurations since hardware differences.
   
3, We have estimated these differences and we thought it's more clear and readable to have a dedicated glue/phy
   driver for A1 SoCs, so also dedicated dt-bindings.

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

* Re: [PATCH 0/6] arm64: meson: Add support for USB on Amlogic A1
  2019-11-22  7:52 ` [PATCH 0/6] arm64: meson: Add support for USB on Amlogic A1 Martin Blumenstingl
@ 2019-11-25  7:53   ` Hanjie Lin
  2019-11-25 22:02     ` Martin Blumenstingl
  0 siblings, 1 reply; 19+ messages in thread
From: Hanjie Lin @ 2019-11-25  7:53 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: Jerome Brunet, Neil Armstrong, Rob Herring, Greg Kroah-Hartman,
	Felipe Balbi, Kevin Hilman, linux-amlogic, linux-arm-kernel,
	linux-usb, devicetree, Carlo Caione, Michael Turquette,
	Stephen Boyd, Liang Yang, Jianxin Pan, Qiufang Dai, Jian Hu,
	Victor Wan, Yue Wang, Xingyu Chen



On 2019/11/22 15:52, Martin Blumenstingl wrote:
> Hello Hanjie,
> 
> On Fri, Nov 22, 2019 at 7:55 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
> [...]
>>   dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings
>>   dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings
>>   phy: amlogic: Add Amlogic A1 USB2 PHY Driver
> drivers/phy/amlogic/phy-meson-g12a-usb2.c seems very similar to the A1
> USB2 PHY you are introducing here.
> 
>>   usb: dwc3: Add Amlogic A1 DWC3 glue
> drivers/usb/dwc3/dwc3-meson-g12a.c is also very similar to the dwc3 glue.
> 
> I have two questions:
> - how is the PHY and the dwc3 glue different from G12A (or SM1)?
> - why do we need a separate set of new drivers (instead of updating
> the existing drivers)?
> 
> We try to use one driver for the same IP block, even if there are
> several revisions with small differences (for example the SAR ADC
> driver supports all SoC generations from Meson8 to G12A/G12B/SM1,
> because 80-90% of the code is shared across all revisions).
> 
> 
> Martin
> 
> .
> 

Hi Martin,

thanks for the comment.

1, G12A have usb2-phy0/usb2-phy1/usb3-phy0 three phys and an interrupt to support host/peripheral/otg modes.
   A1 has one usb2-phy0 phy and only support host mode.
   
2, G12A glue/phy drivers are for G12A SoCs, there are some diffrences to A1.
   G12A glue driver have dr_mode and interrupts two attributes to support otg mode while A1 hasn't this requirement.
   G12A glue driver has a hard coding vbus regulator code to support otg mode while A1 hasn't this requirement.
   G12A glue driver has a hard coding support phys while A1 only supports host mode.
   	enum {
		USB2_HOST_PHY = 0,
		USB2_OTG_PHY,
		USB3_HOST_PHY,
		PHY_COUNT,
		};
   G12A glue driver only supports one clock while A1 needs four clocks.
   G12A and A1 phy drivers have different register configurations since hardware differences.
   
3, We have estimated these differences and we thought it's more clear and readable to have a dedicated glue/phy
   driver for A1 SoCs, so also dedicated dt-bindings.

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

* Re: [PATCH 0/6] arm64: meson: Add support for USB on Amlogic A1
  2019-11-25  7:53   ` Hanjie Lin
@ 2019-11-25 22:02     ` Martin Blumenstingl
  2019-11-26 13:11       ` Neil Armstrong
  2019-11-27  7:11       ` Hanjie Lin
  0 siblings, 2 replies; 19+ messages in thread
From: Martin Blumenstingl @ 2019-11-25 22:02 UTC (permalink / raw)
  To: Hanjie Lin
  Cc: Jerome Brunet, Neil Armstrong, Rob Herring, Greg Kroah-Hartman,
	Felipe Balbi, Kevin Hilman, linux-amlogic, linux-arm-kernel,
	linux-usb, devicetree, Carlo Caione, Michael Turquette,
	Stephen Boyd, Liang Yang, Jianxin Pan, Qiufang Dai, Jian Hu,
	Victor Wan, Yue Wang, Xingyu Chen

Hi Hanjie,

On Mon, Nov 25, 2019 at 8:53 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
>
>
>
> On 2019/11/22 15:52, Martin Blumenstingl wrote:
> > Hello Hanjie,
> >
> > On Fri, Nov 22, 2019 at 7:55 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
> > [...]
> >>   dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings
> >>   dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings
> >>   phy: amlogic: Add Amlogic A1 USB2 PHY Driver
> > drivers/phy/amlogic/phy-meson-g12a-usb2.c seems very similar to the A1
> > USB2 PHY you are introducing here.
> >
> >>   usb: dwc3: Add Amlogic A1 DWC3 glue
> > drivers/usb/dwc3/dwc3-meson-g12a.c is also very similar to the dwc3 glue.
> >
> > I have two questions:
> > - how is the PHY and the dwc3 glue different from G12A (or SM1)?
> > - why do we need a separate set of new drivers (instead of updating
> > the existing drivers)?
> >
> > We try to use one driver for the same IP block, even if there are
> > several revisions with small differences (for example the SAR ADC
> > driver supports all SoC generations from Meson8 to G12A/G12B/SM1,
> > because 80-90% of the code is shared across all revisions).
> >
> >
> > Martin
> >
> > .
> >
>
> Hi Martin,
>
> thanks for the comment.
>
> 1, G12A have usb2-phy0/usb2-phy1/usb3-phy0 three phys and an interrupt to support host/peripheral/otg modes.
>    A1 has one usb2-phy0 phy and only support host mode.
dwc3-meson-g12a treats PHYs as optional
so if you only pass "usb2-phy0" and skip usb2-phy1/usb3-phy0 then it
will still work fine
(I didn't check whether the binding also reflects this)

> 2, G12A glue/phy drivers are for G12A SoCs, there are some diffrences to A1.
>    G12A glue driver have dr_mode and interrupts two attributes to support otg mode while A1 hasn't this requirement.
dwc3-meson-g12a ignores the interrupt for HOST-only mode
(I didn't check whether the IRQ is optional in the dt-binding)

>    G12A glue driver has a hard coding vbus regulator code to support otg mode while A1 hasn't this requirement.
my understanding is that whether a board has a VBUS regulator depends
on the board design. it has nothing to do with the SoC itself

>    G12A glue driver has a hard coding support phys while A1 only supports host mode.
>         enum {
>                 USB2_HOST_PHY = 0,
>                 USB2_OTG_PHY,
>                 USB3_HOST_PHY,
>                 PHY_COUNT,
>                 };
this goes together with comment #1 - you can skip USB2_OTG_PHY and
USB3_HOST_PHY and the driver should still work fine

>    G12A glue driver only supports one clock while A1 needs four clocks.
indeed, the dwc3-meson-g12a needs to be updated to support this
I don't think that I have used it myself yet but there's the
clk_bulk_data framework
it seems to fit this use-case pretty well: define an arbitrary number
of clocks for G12A/B an another set of clocks for A1 - then use the
clk_bulk_data framework to enable/disable them all at once

>    G12A and A1 phy drivers have different register configurations since hardware differences.
other drivers have similar requirements: (mostly) identical register
layout but different values per SoC
here are two examples (I'm not sure if they are good examples though):
Lantiq/Intel SoC [0] and Allwinner SoCs [1]

I compared your driver with phy-meson-g12a-usb2 and only found four differences:
1) PHY_CTRL_R18_MPLL_DCO_CLK_SEL is set for A1
2) PHY_CTRL_R13_UPDATE_PMA_SIGNALS is not set for A1
3) PHY_CTRL_R21 is updated twice for A1 (once for earlier gen SoCs)
4) A1 doesn't reference the "xtal" clock

Difference 4) seems to be a general problem because there seems to be
a PLL inside the PHY registers and that PLL must be fed by some input
clock
So I believe that there is some clock input (which is currently
missing from your A1 USB2 PHY driver)

> 3, We have estimated these differences and we thought it's more clear and readable to have a dedicated glue/phy
>    driver for A1 SoCs, so also dedicated dt-bindings.
I think we should separate the driver and dt-bindings

Based on what I have seen so far my preference for the PHY is:
- use the existing dt-binding, because it seems to be the same IP
block with different register configuration
- use the existing driver because there are only three different
register values (to me it feels like a dedicated driver for these
means more overhead for little benefit)

for the glue I think:
- extend the existing dt-bindings and make some of the PHYs and the
interrupt line optional. making the PHYs optional will be needed when
adding GXL/GXM/AXG support anyways
- use the existing driver and make the clock inputs depend on the SoC
- everything else should already work as is

please let me know if I missed something:
comparing/reviewing the new and existing drivers is harder than just
copying the existing one and modifying that copy
(this is one of the reasons why I think that duplicating code makes
the drivers harder to maintain)

I also thought about the negative consequences of extending the
existing driver(s).
modifying the existing code could break the driver for existing boards.
however, I think that is not a problem because BayLibre's Kernel CI
labs have good coverage for G12A, G12B and SM1.
so if you add some A1 boards there (or host your own lab with A1
boards) any breakage will be found early (the Kernel CI bot even does
git bisect and sends emails)


Martin


[0] https://github.com/torvalds/linux/blob/d2912cb15bdda8ba4a5dd73396ad62641af2f520/drivers/phy/lantiq/phy-lantiq-rcu-usb2.c#L47
[1] https://github.com/torvalds/linux/blob/c942fddf8793b2013be8c901b47d0a8dc02bf99f/drivers/phy/allwinner/phy-sun4i-usb.c#L862

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

* Re: [PATCH 0/6] arm64: meson: Add support for USB on Amlogic A1
  2019-11-25 22:02     ` Martin Blumenstingl
@ 2019-11-26 13:11       ` Neil Armstrong
  2019-11-27  7:11         ` Hanjie Lin
  2019-11-27  7:11       ` Hanjie Lin
  1 sibling, 1 reply; 19+ messages in thread
From: Neil Armstrong @ 2019-11-26 13:11 UTC (permalink / raw)
  To: Martin Blumenstingl, Hanjie Lin
  Cc: Jerome Brunet, Rob Herring, Greg Kroah-Hartman, Felipe Balbi,
	Kevin Hilman, linux-amlogic, linux-arm-kernel, linux-usb,
	devicetree, Carlo Caione, Michael Turquette, Stephen Boyd,
	Liang Yang, Jianxin Pan, Qiufang Dai, Jian Hu, Victor Wan,
	Yue Wang, Xingyu Chen

On 25/11/2019 23:02, Martin Blumenstingl wrote:
> Hi Hanjie,
> 
> On Mon, Nov 25, 2019 at 8:53 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
>>
>>
>>
>> On 2019/11/22 15:52, Martin Blumenstingl wrote:
>>> Hello Hanjie,
>>>
>>> On Fri, Nov 22, 2019 at 7:55 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
>>> [...]
>>>>   dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings
>>>>   dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings
>>>>   phy: amlogic: Add Amlogic A1 USB2 PHY Driver
>>> drivers/phy/amlogic/phy-meson-g12a-usb2.c seems very similar to the A1
>>> USB2 PHY you are introducing here.
>>>
>>>>   usb: dwc3: Add Amlogic A1 DWC3 glue
>>> drivers/usb/dwc3/dwc3-meson-g12a.c is also very similar to the dwc3 glue.
>>>
>>> I have two questions:
>>> - how is the PHY and the dwc3 glue different from G12A (or SM1)?
>>> - why do we need a separate set of new drivers (instead of updating
>>> the existing drivers)?
>>>
>>> We try to use one driver for the same IP block, even if there are
>>> several revisions with small differences (for example the SAR ADC
>>> driver supports all SoC generations from Meson8 to G12A/G12B/SM1,
>>> because 80-90% of the code is shared across all revisions).
>>>
>>>
>>> Martin
>>>
>>> .
>>>
>>
>> Hi Martin,
>>
>> thanks for the comment.
>>
>> 1, G12A have usb2-phy0/usb2-phy1/usb3-phy0 three phys and an interrupt to support host/peripheral/otg modes.
>>    A1 has one usb2-phy0 phy and only support host mode.
> dwc3-meson-g12a treats PHYs as optional
> so if you only pass "usb2-phy0" and skip usb2-phy1/usb3-phy0 then it
> will still work fine
> (I didn't check whether the binding also reflects this)

Exact, a simple match data could make max-phys to 1 for A1.

> 
>> 2, G12A glue/phy drivers are for G12A SoCs, there are some diffrences to A1.
>>    G12A glue driver have dr_mode and interrupts two attributes to support otg mode while A1 hasn't this requirement.
> dwc3-meson-g12a ignores the interrupt for HOST-only mode
> (I didn't check whether the IRQ is optional in the dt-binding)

Interrupt support and OTG manual switch is optional and can be easily bypassed.

> 
>>    G12A glue driver has a hard coding vbus regulator code to support otg mode while A1 hasn't this requirement.
> my understanding is that whether a board has a VBUS regulator depends
> on the board design. it has nothing to do with the SoC itself
> 
>>    G12A glue driver has a hard coding support phys while A1 only supports host mode.
>>         enum {
>>                 USB2_HOST_PHY = 0,
>>                 USB2_OTG_PHY,
>>                 USB3_HOST_PHY,
>>                 PHY_COUNT,
>>                 };
> this goes together with comment #1 - you can skip USB2_OTG_PHY and
> USB3_HOST_PHY and the driver should still work fine

Exact

> 
>>    G12A glue driver only supports one clock while A1 needs four clocks.
> indeed, the dwc3-meson-g12a needs to be updated to support this
> I don't think that I have used it myself yet but there's the
> clk_bulk_data framework
> it seems to fit this use-case pretty well: define an arbitrary number
> of clocks for G12A/B an another set of clocks for A1 - then use the
> clk_bulk_data framework to enable/disable them all at once

Exact, a simple conversion to clk_bulk_* would be enough

> 
>>    G12A and A1 phy drivers have different register configurations since hardware differences.
> other drivers have similar requirements: (mostly) identical register
> layout but different values per SoC
> here are two examples (I'm not sure if they are good examples though):
> Lantiq/Intel SoC [0] and Allwinner SoCs [1]
> 
> I compared your driver with phy-meson-g12a-usb2 and only found four differences:
> 1) PHY_CTRL_R18_MPLL_DCO_CLK_SEL is set for A1
> 2) PHY_CTRL_R13_UPDATE_PMA_SIGNALS is not set for A1
> 3) PHY_CTRL_R21 is updated twice for A1 (once for earlier gen SoCs)
> 4) A1 doesn't reference the "xtal" clock
> 
> Difference 4) seems to be a general problem because there seems to be
> a PLL inside the PHY registers and that PLL must be fed by some input
> clock
> So I believe that there is some clock input (which is currently
> missing from your A1 USB2 PHY driver)

These differences are trivial to add with a match data structure.

> 
>> 3, We have estimated these differences and we thought it's more clear and readable to have a dedicated glue/phy
>>    driver for A1 SoCs, so also dedicated dt-bindings.
> I think we should separate the driver and dt-bindings
> 
> Based on what I have seen so far my preference for the PHY is:
> - use the existing dt-binding, because it seems to be the same IP
> block with different register configuration
> - use the existing driver because there are only three different
> register values (to me it feels like a dedicated driver for these
> means more overhead for little benefit)
> 
> for the glue I think:
> - extend the existing dt-bindings and make some of the PHYs and the
> interrupt line optional. making the PHYs optional will be needed when
> adding GXL/GXM/AXG support anyways
> - use the existing driver and make the clock inputs depend on the SoC
> - everything else should already work as is
> 
> please let me know if I missed something:
> comparing/reviewing the new and existing drivers is harder than just
> copying the existing one and modifying that copy
> (this is one of the reasons why I think that duplicating code makes
> the drivers harder to maintain)
> 
> I also thought about the negative consequences of extending the
> existing driver(s).
> modifying the existing code could break the driver for existing boards.
> however, I think that is not a problem because BayLibre's Kernel CI
> labs have good coverage for G12A, G12B and SM1.
> so if you add some A1 boards there (or host your own lab with A1
> boards) any breakage will be found early (the Kernel CI bot even does
> git bisect and sends emails)
The overall architecture is the same since the GXL SoCs, we also plan to
move the GXL/GXM USB complex into the G12A usb-ctrl driver because the architecture
is very similar.

For A1, the changes will be quite minimal, please try and post a RFC version so we
can evaluate.

Thanks,

Neil

> 
> 
> Martin
> 
> 
> [0] https://github.com/torvalds/linux/blob/d2912cb15bdda8ba4a5dd73396ad62641af2f520/drivers/phy/lantiq/phy-lantiq-rcu-usb2.c#L47
> [1] https://github.com/torvalds/linux/blob/c942fddf8793b2013be8c901b47d0a8dc02bf99f/drivers/phy/allwinner/phy-sun4i-usb.c#L862
> 


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

* Re: [PATCH 0/6] arm64: meson: Add support for USB on Amlogic A1
  2019-11-25 22:02     ` Martin Blumenstingl
  2019-11-26 13:11       ` Neil Armstrong
@ 2019-11-27  7:11       ` Hanjie Lin
  1 sibling, 0 replies; 19+ messages in thread
From: Hanjie Lin @ 2019-11-27  7:11 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: Jerome Brunet, Neil Armstrong, Rob Herring, Greg Kroah-Hartman,
	Felipe Balbi, Kevin Hilman, linux-amlogic, linux-arm-kernel,
	linux-usb, devicetree, Carlo Caione, Michael Turquette,
	Stephen Boyd, Liang Yang, Jianxin Pan, Qiufang Dai, Jian Hu,
	Victor Wan, Yue Wang, Xingyu Chen



On 2019/11/26 6:02, Martin Blumenstingl wrote:
> Hi Hanjie,
> 
> On Mon, Nov 25, 2019 at 8:53 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
>>
>>
>>
>> On 2019/11/22 15:52, Martin Blumenstingl wrote:
>>> Hello Hanjie,
>>>
>>> On Fri, Nov 22, 2019 at 7:55 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
>>> [...]
>>>>   dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings
>>>>   dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings
>>>>   phy: amlogic: Add Amlogic A1 USB2 PHY Driver
>>> drivers/phy/amlogic/phy-meson-g12a-usb2.c seems very similar to the A1
>>> USB2 PHY you are introducing here.
>>>
>>>>   usb: dwc3: Add Amlogic A1 DWC3 glue
>>> drivers/usb/dwc3/dwc3-meson-g12a.c is also very similar to the dwc3 glue.
>>>
>>> I have two questions:
>>> - how is the PHY and the dwc3 glue different from G12A (or SM1)?
>>> - why do we need a separate set of new drivers (instead of updating
>>> the existing drivers)?
>>>
>>> We try to use one driver for the same IP block, even if there are
>>> several revisions with small differences (for example the SAR ADC
>>> driver supports all SoC generations from Meson8 to G12A/G12B/SM1,
>>> because 80-90% of the code is shared across all revisions).
>>>
>>>
>>> Martin
>>>
>>> .
>>>
>>
>> Hi Martin,
>>
>> thanks for the comment.
>>
>> 1, G12A have usb2-phy0/usb2-phy1/usb3-phy0 three phys and an interrupt to support host/peripheral/otg modes.
>>    A1 has one usb2-phy0 phy and only support host mode.
> dwc3-meson-g12a treats PHYs as optional
> so if you only pass "usb2-phy0" and skip usb2-phy1/usb3-phy0 then it
> will still work fine
> (I didn't check whether the binding also reflects this)
> 
>> 2, G12A glue/phy drivers are for G12A SoCs, there are some diffrences to A1.
>>    G12A glue driver have dr_mode and interrupts two attributes to support otg mode while A1 hasn't this requirement.
> dwc3-meson-g12a ignores the interrupt for HOST-only mode
> (I didn't check whether the IRQ is optional in the dt-binding)
> 
>>    G12A glue driver has a hard coding vbus regulator code to support otg mode while A1 hasn't this requirement.
> my understanding is that whether a board has a VBUS regulator depends
> on the board design. it has nothing to do with the SoC itself
> 
>>    G12A glue driver has a hard coding support phys while A1 only supports host mode.
>>         enum {
>>                 USB2_HOST_PHY = 0,
>>                 USB2_OTG_PHY,
>>                 USB3_HOST_PHY,
>>                 PHY_COUNT,
>>                 };
> this goes together with comment #1 - you can skip USB2_OTG_PHY and
> USB3_HOST_PHY and the driver should still work fine
> 
>>    G12A glue driver only supports one clock while A1 needs four clocks.
> indeed, the dwc3-meson-g12a needs to be updated to support this
> I don't think that I have used it myself yet but there's the
> clk_bulk_data framework
> it seems to fit this use-case pretty well: define an arbitrary number
> of clocks for G12A/B an another set of clocks for A1 - then use the
> clk_bulk_data framework to enable/disable them all at once
> 
>>    G12A and A1 phy drivers have different register configurations since hardware differences.
> other drivers have similar requirements: (mostly) identical register
> layout but different values per SoC
> here are two examples (I'm not sure if they are good examples though):
> Lantiq/Intel SoC [0] and Allwinner SoCs [1]
> 
> I compared your driver with phy-meson-g12a-usb2 and only found four differences:
> 1) PHY_CTRL_R18_MPLL_DCO_CLK_SEL is set for A1
> 2) PHY_CTRL_R13_UPDATE_PMA_SIGNALS is not set for A1
> 3) PHY_CTRL_R21 is updated twice for A1 (once for earlier gen SoCs)
> 4) A1 doesn't reference the "xtal" clock
> 
> Difference 4) seems to be a general problem because there seems to be
> a PLL inside the PHY registers and that PLL must be fed by some input
> clock
> So I believe that there is some clock input (which is currently
> missing from your A1 USB2 PHY driver)
> 
>> 3, We have estimated these differences and we thought it's more clear and readable to have a dedicated glue/phy
>>    driver for A1 SoCs, so also dedicated dt-bindings.
> I think we should separate the driver and dt-bindings
> 
> Based on what I have seen so far my preference for the PHY is:
> - use the existing dt-binding, because it seems to be the same IP
> block with different register configuration
> - use the existing driver because there are only three different
> register values (to me it feels like a dedicated driver for these
> means more overhead for little benefit)
> 
> for the glue I think:
> - extend the existing dt-bindings and make some of the PHYs and the
> interrupt line optional. making the PHYs optional will be needed when
> adding GXL/GXM/AXG support anyways
> - use the existing driver and make the clock inputs depend on the SoC
> - everything else should already work as is
> 
> please let me know if I missed something:
> comparing/reviewing the new and existing drivers is harder than just
> copying the existing one and modifying that copy
> (this is one of the reasons why I think that duplicating code makes
> the drivers harder to maintain)
> 
> I also thought about the negative consequences of extending the
> existing driver(s).
> modifying the existing code could break the driver for existing boards.
> however, I think that is not a problem because BayLibre's Kernel CI
> labs have good coverage for G12A, G12B and SM1.
> so if you add some A1 boards there (or host your own lab with A1
> boards) any breakage will be found early (the Kernel CI bot even does
> git bisect and sends emails)
> 
> 
> Martin
> 
> 
> [0] https://github.com/torvalds/linux/blob/d2912cb15bdda8ba4a5dd73396ad62641af2f520/drivers/phy/lantiq/phy-lantiq-rcu-usb2.c#L47
> [1] https://github.com/torvalds/linux/blob/c942fddf8793b2013be8c901b47d0a8dc02bf99f/drivers/phy/allwinner/phy-sun4i-usb.c#L862
> 
> .
> 

Hi Martin:

Okay.
We will try to move A1 usb phy/ctrl driver into G12A driver and send a V2 patch later.

Thanks.

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

* Re: [PATCH 0/6] arm64: meson: Add support for USB on Amlogic A1
  2019-11-26 13:11       ` Neil Armstrong
@ 2019-11-27  7:11         ` Hanjie Lin
  0 siblings, 0 replies; 19+ messages in thread
From: Hanjie Lin @ 2019-11-27  7:11 UTC (permalink / raw)
  To: Neil Armstrong, Martin Blumenstingl
  Cc: Jerome Brunet, Rob Herring, Greg Kroah-Hartman, Felipe Balbi,
	Kevin Hilman, linux-amlogic, linux-arm-kernel, linux-usb,
	devicetree, Carlo Caione, Michael Turquette, Stephen Boyd,
	Liang Yang, Jianxin Pan, Qiufang Dai, Jian Hu, Victor Wan,
	Yue Wang, Xingyu Chen



On 2019/11/26 21:11, Neil Armstrong wrote:
> On 25/11/2019 23:02, Martin Blumenstingl wrote:
>> Hi Hanjie,
>>
>> On Mon, Nov 25, 2019 at 8:53 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
>>>
>>>
>>>
>>> On 2019/11/22 15:52, Martin Blumenstingl wrote:
>>>> Hello Hanjie,
>>>>
>>>> On Fri, Nov 22, 2019 at 7:55 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
>>>> [...]
>>>>>   dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings
>>>>>   dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings
>>>>>   phy: amlogic: Add Amlogic A1 USB2 PHY Driver
>>>> drivers/phy/amlogic/phy-meson-g12a-usb2.c seems very similar to the A1
>>>> USB2 PHY you are introducing here.
>>>>
>>>>>   usb: dwc3: Add Amlogic A1 DWC3 glue
>>>> drivers/usb/dwc3/dwc3-meson-g12a.c is also very similar to the dwc3 glue.
>>>>
>>>> I have two questions:
>>>> - how is the PHY and the dwc3 glue different from G12A (or SM1)?
>>>> - why do we need a separate set of new drivers (instead of updating
>>>> the existing drivers)?
>>>>
>>>> We try to use one driver for the same IP block, even if there are
>>>> several revisions with small differences (for example the SAR ADC
>>>> driver supports all SoC generations from Meson8 to G12A/G12B/SM1,
>>>> because 80-90% of the code is shared across all revisions).
>>>>
>>>>
>>>> Martin
>>>>
>>>> .
>>>>
>>>
>>> Hi Martin,
>>>
>>> thanks for the comment.
>>>
>>> 1, G12A have usb2-phy0/usb2-phy1/usb3-phy0 three phys and an interrupt to support host/peripheral/otg modes.
>>>    A1 has one usb2-phy0 phy and only support host mode.
>> dwc3-meson-g12a treats PHYs as optional
>> so if you only pass "usb2-phy0" and skip usb2-phy1/usb3-phy0 then it
>> will still work fine
>> (I didn't check whether the binding also reflects this)
> 
> Exact, a simple match data could make max-phys to 1 for A1.
> 
>>
>>> 2, G12A glue/phy drivers are for G12A SoCs, there are some diffrences to A1.
>>>    G12A glue driver have dr_mode and interrupts two attributes to support otg mode while A1 hasn't this requirement.
>> dwc3-meson-g12a ignores the interrupt for HOST-only mode
>> (I didn't check whether the IRQ is optional in the dt-binding)
> 
> Interrupt support and OTG manual switch is optional and can be easily bypassed.
> 
>>
>>>    G12A glue driver has a hard coding vbus regulator code to support otg mode while A1 hasn't this requirement.
>> my understanding is that whether a board has a VBUS regulator depends
>> on the board design. it has nothing to do with the SoC itself
>>
>>>    G12A glue driver has a hard coding support phys while A1 only supports host mode.
>>>         enum {
>>>                 USB2_HOST_PHY = 0,
>>>                 USB2_OTG_PHY,
>>>                 USB3_HOST_PHY,
>>>                 PHY_COUNT,
>>>                 };
>> this goes together with comment #1 - you can skip USB2_OTG_PHY and
>> USB3_HOST_PHY and the driver should still work fine
> 
> Exact
> 
>>
>>>    G12A glue driver only supports one clock while A1 needs four clocks.
>> indeed, the dwc3-meson-g12a needs to be updated to support this
>> I don't think that I have used it myself yet but there's the
>> clk_bulk_data framework
>> it seems to fit this use-case pretty well: define an arbitrary number
>> of clocks for G12A/B an another set of clocks for A1 - then use the
>> clk_bulk_data framework to enable/disable them all at once
> 
> Exact, a simple conversion to clk_bulk_* would be enough
> 
>>
>>>    G12A and A1 phy drivers have different register configurations since hardware differences.
>> other drivers have similar requirements: (mostly) identical register
>> layout but different values per SoC
>> here are two examples (I'm not sure if they are good examples though):
>> Lantiq/Intel SoC [0] and Allwinner SoCs [1]
>>
>> I compared your driver with phy-meson-g12a-usb2 and only found four differences:
>> 1) PHY_CTRL_R18_MPLL_DCO_CLK_SEL is set for A1
>> 2) PHY_CTRL_R13_UPDATE_PMA_SIGNALS is not set for A1
>> 3) PHY_CTRL_R21 is updated twice for A1 (once for earlier gen SoCs)
>> 4) A1 doesn't reference the "xtal" clock
>>
>> Difference 4) seems to be a general problem because there seems to be
>> a PLL inside the PHY registers and that PLL must be fed by some input
>> clock
>> So I believe that there is some clock input (which is currently
>> missing from your A1 USB2 PHY driver)
> 
> These differences are trivial to add with a match data structure.
> 
>>
>>> 3, We have estimated these differences and we thought it's more clear and readable to have a dedicated glue/phy
>>>    driver for A1 SoCs, so also dedicated dt-bindings.
>> I think we should separate the driver and dt-bindings
>>
>> Based on what I have seen so far my preference for the PHY is:
>> - use the existing dt-binding, because it seems to be the same IP
>> block with different register configuration
>> - use the existing driver because there are only three different
>> register values (to me it feels like a dedicated driver for these
>> means more overhead for little benefit)
>>
>> for the glue I think:
>> - extend the existing dt-bindings and make some of the PHYs and the
>> interrupt line optional. making the PHYs optional will be needed when
>> adding GXL/GXM/AXG support anyways
>> - use the existing driver and make the clock inputs depend on the SoC
>> - everything else should already work as is
>>
>> please let me know if I missed something:
>> comparing/reviewing the new and existing drivers is harder than just
>> copying the existing one and modifying that copy
>> (this is one of the reasons why I think that duplicating code makes
>> the drivers harder to maintain)
>>
>> I also thought about the negative consequences of extending the
>> existing driver(s).
>> modifying the existing code could break the driver for existing boards.
>> however, I think that is not a problem because BayLibre's Kernel CI
>> labs have good coverage for G12A, G12B and SM1.
>> so if you add some A1 boards there (or host your own lab with A1
>> boards) any breakage will be found early (the Kernel CI bot even does
>> git bisect and sends emails)
> The overall architecture is the same since the GXL SoCs, we also plan to
> move the GXL/GXM USB complex into the G12A usb-ctrl driver because the architecture
> is very similar.
> 
> For A1, the changes will be quite minimal, please try and post a RFC version so we
> can evaluate.
> 
> Thanks,
> 
> Neil
> 
>>
>>
>> Martin
>>
>>
>> [0] https://github.com/torvalds/linux/blob/d2912cb15bdda8ba4a5dd73396ad62641af2f520/drivers/phy/lantiq/phy-lantiq-rcu-usb2.c#L47
>> [1] https://github.com/torvalds/linux/blob/c942fddf8793b2013be8c901b47d0a8dc02bf99f/drivers/phy/allwinner/phy-sun4i-usb.c#L862
>>
> 
> .
> 


Hi Neil:

Okay.
We will try to move A1 usb phy/ctrl driver into G12A driver and send a V2 patch later.

Thanks.

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

* Re: [PATCH 2/6] dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings
  2019-11-25  7:52     ` Hanjie Lin
@ 2019-12-04 19:47       ` Rob Herring
  0 siblings, 0 replies; 19+ messages in thread
From: Rob Herring @ 2019-12-04 19:47 UTC (permalink / raw)
  To: Hanjie Lin
  Cc: Neil Armstrong, Jerome Brunet, Greg Kroah-Hartman, Felipe Balbi,
	Kevin Hilman, Yue Wang, linux-amlogic, linux-arm-kernel,
	linux-usb, devicetree, Carlo Caione, Michael Turquette,
	Stephen Boyd, Martin Blumenstingl, Liang Yang, Jianxin Pan,
	Qiufang Dai, Jian Hu, Victor Wan, Xingyu Chen

On Mon, Nov 25, 2019 at 03:52:18PM +0800, Hanjie Lin wrote:
> 
> 
> On 2019/11/22 16:52, Neil Armstrong wrote:
> > Hi,
> > 
> > 
> > On 22/11/2019 07:55, 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>
> >> ---
> >>  .../devicetree/bindings/usb/amlogic,dwc3.txt       | 53 ++++++++++++++++++++++
> >>  1 file changed, 53 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt b/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
> >> index 6ffb09b..63dc60b 100644
> >> --- a/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
> >> +++ b/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
> >> @@ -128,3 +128,56 @@ Example device nodes:
> >>  				snps,quirk-frame-length-adjustment;
> >>  			};
> >>  	};
> >> +
> >> +Amlogic Meson A1 DWC3 USB SoC Controller Glue
> >> +
> >> +The Amlogic A1 embeds a DWC3 USB IP Core configured for USB2 in
> >> +host-only mode.
> >> +
> >> +Required properties:
> >> +- compatible:	Should be "amlogic,meson-a1-usb-ctrl"
> >> +- clocks:       The clocks needed by the usb controller
> >> +- clock-names:  Should contain the name of the clocks: "usb_ctrl", "usb_bus",
> >> +                "xtal_usb_phy", "xtal_usb_ctrl"
> >> +- resets:	a handle for the shared "USB" reset line
> >> +- reg:		The base address and length of the registers
> >> +- phys: 	handle to used PHYs on the system
> >> +	- a <0> phandle can be used if a PHY is not used
> >> +- phy-names:	names of the used PHYs on the system :
> >> +	- "usb2-phy0" for USB2 PHY if USBHOST port is used
> >> +
> >> +Required child nodes:
> >> +
> >> +A child node must exist to represent the core DWC3 IP block. The name of
> >> +the node is not important. The content of the node is defined in dwc3.txt.
> >> +
> >> +PHY documentation is provided in the following places:
> >> +- Documentation/devicetree/bindings/phy/amlogic,meson-a1-usb2-phy.yaml
> >> +
> >> +Example device nodes:
> >> +	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>;
> >> +			};
> >> +	};
> >>
> > 
> > This seems very similar to the g12a bindings, seems you could update the yaml g12a bindings
> > with specific clocks and required for amlogic,meson-a1-usb-ctrl.
> > 
> > Neil
> > 
> > .
> > 
> 
> Hi Neil
> Thanks for the comment.
> 
> 1, G12A have usb2-phy0/usb2-phy1/usb3-phy0 three phys and an interrupt to support host/peripheral/otg modes.
>    A1 has one usb2-phy0 phy and only support host mode.
>    
> 2, G12A glue/phy drivers are for G12A SoCs, there are some diffrences to A1.
>    G12A glue driver have dr_mode and interrupts two attributes to support otg mode while A1 hasn't this requirement.
>    G12A glue driver has a hard coding vbus regulator code to support otg mode while A1 hasn't this requirement.
>    G12A glue driver has a hard coding support phys while A1 only supports host mode.
>    	enum {
> 		USB2_HOST_PHY = 0,
> 		USB2_OTG_PHY,
> 		USB3_HOST_PHY,
> 		PHY_COUNT,
> 		};
>    G12A glue driver only supports one clock while A1 needs four clocks.
>    G12A and A1 phy drivers have different register configurations since hardware differences.
>    
> 3, We have estimated these differences and we thought it's more clear and readable to have a dedicated glue/phy
>    driver for A1 SoCs, so also dedicated dt-bindings.

Fair enough, I guess. But you're not sharing anything from the 
amlogic,dwc3.txt binding, so make a new doc. And please make it a DT 
schema.

Rob

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

end of thread, back to index

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-22  6:55 [PATCH 0/6] arm64: meson: Add support for USB on Amlogic A1 Hanjie Lin
2019-11-22  6:55 ` [PATCH 1/6] dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings Hanjie Lin
2019-11-22 22:52   ` Rob Herring
2019-11-22  6:55 ` [PATCH 2/6] dt-bindings: usb: dwc3: Add the Amlogic A1 Family DWC3 Glue Bindings Hanjie Lin
2019-11-22  8:52   ` Neil Armstrong
2019-11-25  7:52     ` Hanjie Lin
2019-12-04 19:47       ` Rob Herring
2019-11-22  6:55 ` [PATCH 3/6] phy: amlogic: Add Amlogic A1 USB2 PHY Driver Hanjie Lin
2019-11-22  6:55 ` [PATCH 4/6] usb: dwc3: Add Amlogic A1 DWC3 glue Hanjie Lin
2019-11-22  8:53   ` Neil Armstrong
2019-11-25  7:53     ` Hanjie Lin
2019-11-22  6:55 ` [PATCH 5/6] arm64: dts: meson: a1: Enable USB2 PHY Hanjie Lin
2019-11-22  6:55 ` [PATCH 6/6] arm64: dts: meson: a1: Enable DWC3 controller Hanjie Lin
2019-11-22  7:52 ` [PATCH 0/6] arm64: meson: Add support for USB on Amlogic A1 Martin Blumenstingl
2019-11-25  7:53   ` Hanjie Lin
2019-11-25 22:02     ` Martin Blumenstingl
2019-11-26 13:11       ` Neil Armstrong
2019-11-27  7:11         ` Hanjie Lin
2019-11-27  7:11       ` Hanjie Lin

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.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-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


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