linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] arm64: meson: Add support for USB on Amlogic G12A
@ 2019-02-12 15:14 Neil Armstrong
  2019-02-12 15:14 ` [PATCH 1/8] dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings Neil Armstrong
                   ` (7 more replies)
  0 siblings, 8 replies; 25+ messages in thread
From: Neil Armstrong @ 2019-02-12 15:14 UTC (permalink / raw)
  To: gregkh, hminas, balbi, kishon
  Cc: Neil Armstrong, linux-amlogic, linux-usb, linux-arm-kernel, linux-kernel

This patchset adds support for USB on Amlogic G12A SoCs.

This patchset is composed with :
- bindings of the PHYs
- bindings of the USB Control Glue
- PHY Drivers
- USB Control Glue driver

Device Tree nodes will be added in a separate patchset.

The Amlogic G12A USB Complex is composed of :
- 2 USB Controllers :
 * DWC3 for USB2 and USB3 Host functionality
 * DWC2 for USB2 Peripheral functionality
- 2 USB2 OTG PHYs, only a single one will be routed to either DWC2 to DWC3
- 1 USB3 PHY shared with PCIE funcionnality
- A Glue to control PHY routing, setup and OTG detection

The Glue configures the UTMI 8bit interfaces for the USB2 PHYs, including
routing of the OTG PHY between the DWC3 and DWC2 controllers, and
setups the on-chip OTG mode selection for this PHY.

The PHYs are children of the Glue node since the Glue controls the interface
with the PHY, not the DWC3 controller.

The PHY interconnect is handled into ports subnodes, which eases describing
which PHY is enabled (like the USB3 shared PHY) and futures layouts on
derivatives of the G12A Family.

This drivers supports the on-probe setup of the OTG mode, and manually
via a debugfs interface. The IRQ mode change detect is yet to be added
in a future patchset, mainly due to lack of hardware to validate on.

Neil Armstrong (8):
  dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings
  dt-bindings: phy: Add Amlogic G12A USB3+PCIE Combo PHY Bindings
  dt-bindings: usb: dwc2: Add Amlogic G12A DWC2 Compatible
  dt-bindings: usb: dwc3: Add Amlogic G12A DWC3 Glue Bindings
  phy: amlogic: add Amlogic G12A USB2 PHY Driver
  phy: amlogic: Add Amlogic G12A USB3 + PCIE Combo PHY Driver
  usb: dwc2: Add Amlogic G12A DWC2 Params
  usb: dwc3: Add Amlogic G12A DWC3 glue

 .../bindings/phy/meson-g12a-usb2-phy.txt      |  22 +
 .../bindings/phy/meson-g12a-usb3-pcie-phy.txt |  25 +
 .../devicetree/bindings/usb/amlogic,dwc3.txt  | 109 +++
 .../devicetree/bindings/usb/dwc2.txt          |   1 +
 drivers/phy/amlogic/Kconfig                   |  24 +
 drivers/phy/amlogic/Makefile                  |   2 +
 drivers/phy/amlogic/phy-meson-g12a-usb2.c     | 191 +++++
 .../phy/amlogic/phy-meson-g12a-usb3-pcie.c    | 414 +++++++++++
 drivers/usb/dwc2/params.c                     |  12 +
 drivers/usb/dwc3/Kconfig                      |   9 +
 drivers/usb/dwc3/Makefile                     |   1 +
 drivers/usb/dwc3/dwc3-meson-g12a.c            | 650 ++++++++++++++++++
 12 files changed, 1460 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/meson-g12a-usb2-phy.txt
 create mode 100644 Documentation/devicetree/bindings/phy/meson-g12a-usb3-pcie-phy.txt
 create mode 100644 drivers/phy/amlogic/phy-meson-g12a-usb2.c
 create mode 100644 drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
 create mode 100644 drivers/usb/dwc3/dwc3-meson-g12a.c

-- 
2.20.1


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

* [PATCH 1/8] dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings
  2019-02-12 15:14 [PATCH 0/8] arm64: meson: Add support for USB on Amlogic G12A Neil Armstrong
@ 2019-02-12 15:14 ` Neil Armstrong
  2019-02-17 21:58   ` Martin Blumenstingl
  2019-02-28 15:18   ` Rob Herring
  2019-02-12 15:14 ` [PATCH 2/8] dt-bindings: phy: Add Amlogic G12A USB3+PCIE Combo " Neil Armstrong
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 25+ messages in thread
From: Neil Armstrong @ 2019-02-12 15:14 UTC (permalink / raw)
  To: gregkh, hminas, balbi, kishon, devicetree
  Cc: Neil Armstrong, linux-amlogic, linux-usb, linux-arm-kernel, linux-kernel

Add the Amlogic G12A Family USB2 OTG PHY Bindings

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../bindings/phy/meson-g12a-usb2-phy.txt      | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/meson-g12a-usb2-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/meson-g12a-usb2-phy.txt b/Documentation/devicetree/bindings/phy/meson-g12a-usb2-phy.txt
new file mode 100644
index 000000000000..a6ebc3dea159
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/meson-g12a-usb2-phy.txt
@@ -0,0 +1,22 @@
+* Amlogic G12A USB2 PHY binding
+
+Required properties:
+- compatible:	Should be "amlogic,meson-g12a-usb2-phy"
+- reg:		The base address and length of the registers
+- #phys-cells:	must be 0 (see phy-bindings.txt in this directory)
+- clocks:	a phandle to the clock of this PHY
+- clock-names:	must be "xtal"
+- resets:	a phandle to the reset line of this PHY
+- reset-names:	must be "phy"
+- phy-supply:	see phy-bindings.txt in this directory
+
+Example:
+	usb2_phy0: phy@36000 {
+		compatible = "amlogic,g12a-usb2-phy";
+		reg = <0x0 0x36000 0x0 0x2000>;
+		clocks = <&xtal>;
+		clock-names = "xtal";
+		resets = <&reset RESET_USB_PHY21>;
+		reset-names = "phy";
+		#phy-cells = <0>;
+	};
-- 
2.20.1


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

* [PATCH 2/8] dt-bindings: phy: Add Amlogic G12A USB3+PCIE Combo PHY Bindings
  2019-02-12 15:14 [PATCH 0/8] arm64: meson: Add support for USB on Amlogic G12A Neil Armstrong
  2019-02-12 15:14 ` [PATCH 1/8] dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings Neil Armstrong
@ 2019-02-12 15:14 ` Neil Armstrong
  2019-02-17 22:03   ` Martin Blumenstingl
  2019-02-12 15:14 ` [PATCH 3/8] dt-bindings: usb: dwc2: Add Amlogic G12A DWC2 Compatible Neil Armstrong
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 25+ messages in thread
From: Neil Armstrong @ 2019-02-12 15:14 UTC (permalink / raw)
  To: gregkh, hminas, balbi, kishon, devicetree
  Cc: Neil Armstrong, linux-amlogic, linux-usb, linux-arm-kernel, linux-kernel

Add the Amlogic G12A Family USB3 + PCIE Combo PHY Bindings.

This PHY can provide exclusively USB3 or PCIE support on shared I/Os.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../bindings/phy/meson-g12a-usb3-pcie-phy.txt | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/meson-g12a-usb3-pcie-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/meson-g12a-usb3-pcie-phy.txt b/Documentation/devicetree/bindings/phy/meson-g12a-usb3-pcie-phy.txt
new file mode 100644
index 000000000000..714d751091f5
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/meson-g12a-usb3-pcie-phy.txt
@@ -0,0 +1,25 @@
+* Amlogic G12A USB3 + PCIE Combo PHY binding
+
+Required properties:
+- compatible:	Should be "amlogic,meson-g12a-usb3-pcie-phy"
+- #phys-cells:	must be 1. The cell number is used to select the phy mode
+  as defined in <dt-bindings/phy/phy.h> between PHY_TYPE_USB3 and PHY_TYPE_PCIE
+- reg:		The base address and length of the registers
+- clocks:	a phandle to the 100MHz reference clock of this PHY
+- clock-names:	must be "ref_clk"
+- resets:	phandle to the reset lines for:
+		- the PHY control
+		- the USB3+PCIE PHY
+		- the PHY registers
+
+Example:
+	usb3_pcie_phy: phy@46000 {
+		compatible = "amlogic,g12a-usb3-pcie-phy";
+		reg = <0x0 0x46000 0x0 0x2000>;
+		clocks = <&clkc CLKID_PCIE_PLL>;
+		clock-names = "ref_clk";
+		resets = <&reset RESET_PCIE_CTRL_A>,
+			 <&reset RESET_PCIE_PHY>,
+			 <&reset RESET_PCIE_APB>;
+		#phy-cells = <1>;
+	};
-- 
2.20.1


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

* [PATCH 3/8] dt-bindings: usb: dwc2: Add Amlogic G12A DWC2 Compatible
  2019-02-12 15:14 [PATCH 0/8] arm64: meson: Add support for USB on Amlogic G12A Neil Armstrong
  2019-02-12 15:14 ` [PATCH 1/8] dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings Neil Armstrong
  2019-02-12 15:14 ` [PATCH 2/8] dt-bindings: phy: Add Amlogic G12A USB3+PCIE Combo " Neil Armstrong
@ 2019-02-12 15:14 ` Neil Armstrong
  2019-02-17 22:07   ` Martin Blumenstingl
  2019-02-28 16:14   ` Rob Herring
  2019-02-12 15:14 ` [PATCH 4/8] dt-bindings: usb: dwc3: Add Amlogic G12A DWC3 Glue Bindings Neil Armstrong
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 25+ messages in thread
From: Neil Armstrong @ 2019-02-12 15:14 UTC (permalink / raw)
  To: gregkh, hminas, balbi, kishon, devicetree
  Cc: Neil Armstrong, linux-amlogic, linux-usb, linux-arm-kernel, linux-kernel

Adds the specific compatible string for the DWC2 IP found in the
Amlogic G12A SoC Family.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index 6dc3c4a34483..e150b7b227c9 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -14,6 +14,7 @@ Required properties:
   - "amlogic,meson8-usb": The DWC2 USB controller instance in Amlogic Meson8 SoCs;
   - "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
   - "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
+  - "amlogic,meson-g12a-usb": The DWC2 USB controller instance in Amlogic G12A SoCs;
   - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
   - snps,dwc2: A generic DWC2 USB controller with default parameters.
   - "st,stm32f4x9-fsotg": The DWC2 USB FS/HS controller instance in STM32F4x9 SoCs
-- 
2.20.1


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

* [PATCH 4/8] dt-bindings: usb: dwc3: Add Amlogic G12A DWC3 Glue Bindings
  2019-02-12 15:14 [PATCH 0/8] arm64: meson: Add support for USB on Amlogic G12A Neil Armstrong
                   ` (2 preceding siblings ...)
  2019-02-12 15:14 ` [PATCH 3/8] dt-bindings: usb: dwc2: Add Amlogic G12A DWC2 Compatible Neil Armstrong
@ 2019-02-12 15:14 ` Neil Armstrong
  2019-02-24 19:52   ` Martin Blumenstingl
  2019-02-28 16:29   ` Rob Herring
  2019-02-12 15:14 ` [PATCH 5/8] phy: amlogic: add Amlogic G12A USB2 PHY Driver Neil Armstrong
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 25+ messages in thread
From: Neil Armstrong @ 2019-02-12 15:14 UTC (permalink / raw)
  To: gregkh, hminas, balbi, kishon, devicetree
  Cc: Neil Armstrong, linux-amlogic, linux-usb, linux-arm-kernel, linux-kernel

Adds the bindings for the Amlogic G12A USB Glue HW.

The Amlogic G12A SoC Family embeds 2 USB Controllers :
- a DWC3 IP configured as Host for USB2 and USB3
- a DWC2 IP configured as Peripheral USB2 Only

A glue connects these both controllers to 2 USB2 PHYs,
and optionnally to an USB3+PCIE Combo PHY shared with the PCIE controller.

The Glue configures the UTMI 8bit interfaces for the USB2 PHYs, including
routing of the OTG PHY between the DWC3 and DWC2 controllers, and
setups the on-chip OTG mode selection for this PHY.

The PHYs are children of the Glue node since the Glue controls the interface
with the PHY, not the DWC3 controller.

The PHY interconnect is handled into ports subnodes, which eases describing
which PHY is enabled (like the USB3 shared PHY) and futures layouts on
derivatives of the G12A Family.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../devicetree/bindings/usb/amlogic,dwc3.txt  | 109 ++++++++++++++++++
 1 file changed, 109 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt b/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
index 9a8b631904fd..c7c4726ef10d 100644
--- a/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
@@ -40,3 +40,112 @@ Example device nodes:
 				phy-names = "usb2-phy", "usb3-phy";
 			};
 		};
+
+Amlogic Meson G12A DWC3 USB SoC Controller Glue
+
+The Amlogic G12A embeds a DWC3 USB IP Core configured for USB2 and USB3
+in host-only mode, and a DWC2 IP Core configured for USB2 peripheral mode
+only.
+
+A glue connects the DWC3 core to USB2 PHYs and optionnaly to an USB3 PHY.
+
+One of the USB2 PHY can be re-routed in peripheral mode to a DWC2 USB IP.
+
+The DWC3 Glue controls the PHY routing and power, an interrupt line is
+connected to the Glue to serve as OTG ID change detection.
+
+Required properties:
+- compatible:	Should be "amlogic,meson-g12a-usb-ctrl"
+- clocks:	a handle for the "USB" clock
+- clock-names:	must be "usb"
+- resets:	a handle for the shared "USB" reset line
+- reset-names:	must be "usb"
+- reg:		The base address and length of the registers
+- interrupts:	the interrupt specifier for the OTG detection
+
+Required child nodes:
+
+USB Ports are described as child 'port' nodes grouped under a 'ports' node,
+with #address-cells, #size-cells specified.
+
+Each 'port' sub-node identifies a possible USB Port served by an USB PHY
+identified by the 'phy' property as decribed in ../phy/phy-bindings.txt
+
+Each 'port' is identified by a reg property to number the port.
+
+The following table lists for each supported model the port number
+corresponding to each PHY serving a physical USB Port.
+
+ Family	   Port 0     Port 1    Port 2    Port 3    Port 4
+---------------------------------------------------------------
+ G12A	   USBHOST_A  USBOTG_B  Reserved  Reserved  USB3_0
+
+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.
+
+A child node must exist to represent the core DWC2 IP block. The name of
+the node is not important. The content of the node is defined in dwc2.txt.
+
+PHY documentation is provided in the following places:
+- Documentation/devicetree/bindings/phy/meson-g12a-usb2-phy.txt
+- Documentation/devicetree/bindings/phy/meson-g12a-usb3-pcie-phy.txt
+
+
+Example device nodes:
+	usb: usb@ffe09000 {
+			compatible = "amlogic,meson-g12a-usb-ctrl";
+			reg = <0x0 0xffe09000 0x0 0xa0>;
+			interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
+
+			clocks = <&clkc CLKID_USB>;
+			clock-names = "usb";
+			resets = <&reset RESET_USB>;
+			reset-names = "usb";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				/* USB2 Port 0 */
+				usb20: port@0 {
+					reg = <0>;
+					phys = <&usb2_phy0>;
+				};
+
+				/* USB2 Port 1 */
+				usb21: port@1 {
+					reg = <1>;
+					phys = <&usb2_phy1>;
+				};
+
+				/* USB3 Port 0 */
+				usb3: port@4 {
+					reg = <4>;
+					phys = <&usb3_pcie_phy PHY_TYPE_USB3>;
+				};
+			};
+
+			dwc2: usb@ff400000 {
+				compatible = "amlogic,meson-g12a-usb", "snps,dwc2";
+				reg = <0x0 0xff400000 0x0 0x40000>;
+				interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clkc CLKID_USB1_DDR_BRIDGE>;
+				clock-names = "ddr";
+				dr_mode = "peripheral";
+				g-rx-fifo-size = <192>;
+				g-np-tx-fifo-size = <128>;
+				g-tx-fifo-size = <128 128 16 16 16>;
+			};
+
+			dwc3: dwc3@ff500000 {
+				compatible = "snps,dwc3";
+				reg = <0x0 0xff500000 0x0 0x100000>;
+				interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
+				dr_mode = "host";
+				snps,dis_u2_susphy_quirk;
+				snps,quirk-frame-length-adjustment;
+			};
+	};
-- 
2.20.1


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

* [PATCH 5/8] phy: amlogic: add Amlogic G12A USB2 PHY Driver
  2019-02-12 15:14 [PATCH 0/8] arm64: meson: Add support for USB on Amlogic G12A Neil Armstrong
                   ` (3 preceding siblings ...)
  2019-02-12 15:14 ` [PATCH 4/8] dt-bindings: usb: dwc3: Add Amlogic G12A DWC3 Glue Bindings Neil Armstrong
@ 2019-02-12 15:14 ` Neil Armstrong
  2019-02-17 22:24   ` Martin Blumenstingl
  2019-02-12 15:14 ` [PATCH 6/8] phy: amlogic: Add Amlogic G12A USB3 + PCIE Combo " Neil Armstrong
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 25+ messages in thread
From: Neil Armstrong @ 2019-02-12 15:14 UTC (permalink / raw)
  To: gregkh, hminas, balbi, kishon
  Cc: Neil Armstrong, linux-amlogic, linux-usb, linux-arm-kernel, linux-kernel

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

It supports Host and/or Peripheral mode, depending on it's position.
The first PHY is only used as Host, but the second supports Dual modes
defined by the USB Control Glue HW in front of the USB Controllers.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/phy/amlogic/Kconfig               |  12 ++
 drivers/phy/amlogic/Makefile              |   1 +
 drivers/phy/amlogic/phy-meson-g12a-usb2.c | 191 ++++++++++++++++++++++
 3 files changed, 204 insertions(+)
 create mode 100644 drivers/phy/amlogic/phy-meson-g12a-usb2.c

diff --git a/drivers/phy/amlogic/Kconfig b/drivers/phy/amlogic/Kconfig
index 23fe1cda2f70..78d6e194dce9 100644
--- a/drivers/phy/amlogic/Kconfig
+++ b/drivers/phy/amlogic/Kconfig
@@ -36,3 +36,15 @@ config PHY_MESON_GXL_USB3
 	  Enable this to support the Meson USB3 PHY and OTG detection
 	  IP block found in Meson GXL and GXM SoCs.
 	  If unsure, say N.
+
+config PHY_MESON_G12A_USB2
+	tristate "Meson G12A USB2 PHY drivers"
+	default ARCH_MESON
+	depends on OF && (ARCH_MESON || COMPILE_TEST)
+	depends on USB_SUPPORT
+	select GENERIC_PHY
+	select REGMAP_MMIO
+	help
+	  Enable this to support the Meson USB2 PHYs found in Meson
+	  G12A SoCs.
+	  If unsure, say N.
diff --git a/drivers/phy/amlogic/Makefile b/drivers/phy/amlogic/Makefile
index 4fd8848c194d..7d4d10f5a6b3 100644
--- a/drivers/phy/amlogic/Makefile
+++ b/drivers/phy/amlogic/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_PHY_MESON8B_USB2)		+= phy-meson8b-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-g12a-usb2.c b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
new file mode 100644
index 000000000000..3b6271a8be02
--- /dev/null
+++ b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Meson G12A USB2 PHY driver
+ *
+ * Copyright (C) 2017 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
+ * Copyright (C) 2017 Amlogic, Inc. All rights reserved
+ * Copyright (C) 2019 BayLibre, SAS
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+#include <linux/reset.h>
+#include <linux/phy/phy.h>
+#include <linux/usb/otg.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_R4						0x10
+#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_R14						0x38
+#define PHY_CTRL_R15						0x3c
+#define PHY_CTRL_R16						0x40
+#define PHY_CTRL_R17						0x44
+#define PHY_CTRL_R18						0x48
+#define PHY_CTRL_R19						0x4c
+#define PHY_CTRL_R20						0x50
+#define PHY_CTRL_R21						0x54
+#define PHY_CTRL_R22						0x58
+#define PHY_CTRL_R23						0x5c
+
+#define RESET_COMPLETE_TIME					1000
+#define PLL_RESET_COMPLETE_TIME					100
+
+struct phy_meson_g12a_usb2_priv {
+	struct device		*dev;
+	struct regmap		*regmap;
+	struct clk		*clk;
+	struct reset_control	*reset;
+};
+
+static const struct regmap_config phy_meson_g12a_usb2_regmap_conf = {
+	.reg_bits = 8,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.max_register = PHY_CTRL_R23,
+};
+
+static int phy_meson_g12a_usb2_init(struct phy *phy)
+{
+	struct phy_meson_g12a_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, BIT(2), 0);
+
+	/* PLL Setup : 24MHz * 20 / 1 = 480MHz */
+	regmap_write(priv->regmap, PHY_CTRL_R16, 0x39400414);
+	regmap_write(priv->regmap, PHY_CTRL_R17, 0x927e0000);
+	regmap_write(priv->regmap, PHY_CTRL_R18, 0xac5f49e5);
+
+	udelay(PLL_RESET_COMPLETE_TIME);
+
+	/* UnReset PLL */
+	regmap_write(priv->regmap, PHY_CTRL_R16, 0x19400414);
+
+	/* PHY Tuning */
+	regmap_write(priv->regmap, PHY_CTRL_R20, 0xfe18);
+	regmap_write(priv->regmap, PHY_CTRL_R4, 0x8000fff);
+
+	/* Tuning Disconnect Threshold */
+	regmap_write(priv->regmap, PHY_CTRL_R3, 0x34);
+
+	/* Analog Settings */
+	regmap_write(priv->regmap, PHY_CTRL_R14, 0);
+	regmap_write(priv->regmap, PHY_CTRL_R13, 0x78000);
+
+	return 0;
+}
+
+static int phy_meson_g12a_usb2_exit(struct phy *phy)
+{
+	struct phy_meson_g12a_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_g12a_usb2_ops = {
+	.init		= phy_meson_g12a_usb2_init,
+	.exit		= phy_meson_g12a_usb2_exit,
+	.owner		= THIS_MODULE,
+};
+
+static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct phy_provider *phy_provider;
+	struct resource *res;
+	struct phy_meson_g12a_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);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	priv->regmap = devm_regmap_init_mmio(dev, base,
+					     &phy_meson_g12a_usb2_regmap_conf);
+	if (IS_ERR(priv->regmap))
+		return PTR_ERR(priv->regmap);
+
+	priv->clk = devm_clk_get(dev, "xtal");
+	if (IS_ERR(priv->clk))
+		return PTR_ERR(priv->clk);
+
+	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_g12a_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_g12a_usb2_of_match[] = {
+	{ .compatible = "amlogic,g12a-usb2-phy", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, phy_meson_g12a_usb2_of_match);
+
+static struct platform_driver phy_meson_g12a_usb2_driver = {
+	.probe	= phy_meson_g12a_usb2_probe,
+	.driver	= {
+		.name		= "phy-meson-g12a-usb2",
+		.of_match_table	= phy_meson_g12a_usb2_of_match,
+	},
+};
+module_platform_driver(phy_meson_g12a_usb2_driver);
+
+MODULE_AUTHOR("Martin Blumenstingl <martin.blumenstingl@googlemail.com>");
+MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
+MODULE_DESCRIPTION("Meson G12A USB2 PHY driver");
+MODULE_LICENSE("GPL v2");
-- 
2.20.1


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

* [PATCH 6/8] phy: amlogic: Add Amlogic G12A USB3 + PCIE Combo PHY Driver
  2019-02-12 15:14 [PATCH 0/8] arm64: meson: Add support for USB on Amlogic G12A Neil Armstrong
                   ` (4 preceding siblings ...)
  2019-02-12 15:14 ` [PATCH 5/8] phy: amlogic: add Amlogic G12A USB2 PHY Driver Neil Armstrong
@ 2019-02-12 15:14 ` Neil Armstrong
  2019-02-24 19:40   ` Martin Blumenstingl
  2019-02-12 15:14 ` [PATCH 7/8] usb: dwc2: Add Amlogic G12A DWC2 Params Neil Armstrong
  2019-02-12 15:14 ` [PATCH 8/8] usb: dwc3: Add Amlogic G12A DWC3 glue Neil Armstrong
  7 siblings, 1 reply; 25+ messages in thread
From: Neil Armstrong @ 2019-02-12 15:14 UTC (permalink / raw)
  To: gregkh, hminas, balbi, kishon
  Cc: Neil Armstrong, linux-amlogic, linux-usb, linux-arm-kernel, linux-kernel

This adds support for the shared USB3 + PCIE PHY found in the
Amlogic G12A SoC Family.

It supports USB3 Host mode or PCIE 2.0 mode, depending on the layout of
the board.

Selection is done by the #phy-cells, making the mode static and exclusive.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/phy/amlogic/Kconfig                   |  12 +
 drivers/phy/amlogic/Makefile                  |   1 +
 .../phy/amlogic/phy-meson-g12a-usb3-pcie.c    | 414 ++++++++++++++++++
 3 files changed, 427 insertions(+)
 create mode 100644 drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c

diff --git a/drivers/phy/amlogic/Kconfig b/drivers/phy/amlogic/Kconfig
index 78d6e194dce9..7ccb9a756aba 100644
--- a/drivers/phy/amlogic/Kconfig
+++ b/drivers/phy/amlogic/Kconfig
@@ -48,3 +48,15 @@ config PHY_MESON_G12A_USB2
 	  Enable this to support the Meson USB2 PHYs found in Meson
 	  G12A SoCs.
 	  If unsure, say N.
+
+config PHY_MESON_G12A_USB3_PCIE
+	tristate "Meson G12A USB3+PCIE Combo PHY drivers"
+	default ARCH_MESON
+	depends on OF && (ARCH_MESON || COMPILE_TEST)
+	depends on USB_SUPPORT
+	select GENERIC_PHY
+	select REGMAP_MMIO
+	help
+	  Enable this to support the Meson USB3 + PCIE Combi PHY found
+	  in Meson G12A SoCs.
+	  If unsure, say N.
diff --git a/drivers/phy/amlogic/Makefile b/drivers/phy/amlogic/Makefile
index 7d4d10f5a6b3..fdd008e1b19b 100644
--- a/drivers/phy/amlogic/Makefile
+++ b/drivers/phy/amlogic/Makefile
@@ -2,3 +2,4 @@ obj-$(CONFIG_PHY_MESON8B_USB2)		+= phy-meson8b-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
+obj-$(CONFIG_PHY_MESON_G12A_USB3_PCIE)	+= phy-meson-g12a-usb3-pcie.o
diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c b/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
new file mode 100644
index 000000000000..59eae98928e9
--- /dev/null
+++ b/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
@@ -0,0 +1,414 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Amlogic G12A USB3 + PCIE Combo PHY driver
+ *
+ * Copyright (C) 2017 Amlogic, Inc. All rights reserved
+ * Copyright (C) 2019 BayLibre, SAS
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/phy/phy.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/platform_device.h>
+#include <dt-bindings/phy/phy.h>
+
+#define PHY_R0							0x00
+	#define PHY_R0_PCIE_POWER_STATE				GENMASK(4, 0)
+	#define PHY_R0_PCIE_USB3_SWITCH				GENMASK(6, 5)
+
+#define PHY_R1							0x04
+	#define PHY_R1_PHY_TX1_TERM_OFFSET			GENMASK(4, 0)
+	#define PHY_R1_PHY_TX0_TERM_OFFSET			GENMASK(9, 5)
+	#define PHY_R1_PHY_RX1_EQ				GENMASK(12, 10)
+	#define PHY_R1_PHY_RX0_EQ				GENMASK(15, 13)
+	#define PHY_R1_PHY_LOS_LEVEL				GENMASK(20, 16)
+	#define PHY_R1_PHY_LOS_BIAS				GENMASK(23, 21)
+	#define PHY_R1_PHY_REF_CLKDIV2				BIT(24)
+	#define PHY_R1_PHY_MPLL_MULTIPLIER			GENMASK(31, 25)
+
+#define PHY_R2							0x08
+	#define PHY_R2_PCS_TX_DEEMPH_GEN2_6DB			GENMASK(5, 0)
+	#define PHY_R2_PCS_TX_DEEMPH_GEN2_3P5DB			GENMASK(11, 6)
+	#define PHY_R2_PCS_TX_DEEMPH_GEN1			GENMASK(17, 12)
+	#define PHY_R2_PHY_TX_VBOOST_LVL			GENMASK(20, 18)
+
+#define PHY_R4							0x10
+	#define PHY_R4_PHY_CR_WRITE				BIT(0)
+	#define PHY_R4_PHY_CR_READ				BIT(1)
+	#define PHY_R4_PHY_CR_DATA_IN				GENMASK(17, 2)
+	#define PHY_R4_PHY_CR_CAP_DATA				BIT(18)
+	#define PHY_R4_PHY_CR_CAP_ADDR				BIT(19)
+
+#define PHY_R5							0x14
+	#define PHY_R5_PHY_CR_DATA_OUT				GENMASK(15, 0)
+	#define PHY_R5_PHY_CR_ACK				BIT(16)
+	#define PHY_R5_PHY_BS_OUT				BIT(17)
+
+struct phy_g12a_usb3_pcie_priv {
+	struct regmap		*regmap;
+	struct regmap		*regmap_cr;
+	struct clk		*clk_ref;
+	struct reset_control	*reset;
+	struct phy		*phy;
+	unsigned int		mode;
+};
+
+static const struct regmap_config phy_g12a_usb3_pcie_regmap_conf = {
+	.reg_bits = 8,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.max_register = PHY_R5,
+};
+
+static int phy_g12a_usb3_pcie_cr_bus_addr(struct phy_g12a_usb3_pcie_priv *priv,
+					  unsigned int addr)
+{
+	unsigned int val, reg;
+	int ret;
+
+	reg = FIELD_PREP(PHY_R4_PHY_CR_DATA_IN, addr);
+
+	regmap_write(priv->regmap, PHY_R4, reg);
+	regmap_write(priv->regmap, PHY_R4, reg);
+
+	regmap_write(priv->regmap, PHY_R4, reg | PHY_R4_PHY_CR_CAP_ADDR);
+
+	ret = regmap_read_poll_timeout(priv->regmap, PHY_R5, val,
+				       (val & PHY_R5_PHY_CR_ACK),
+				       5, 1000);
+	if (ret)
+		return ret;
+
+	regmap_write(priv->regmap, PHY_R4, reg);
+
+	ret = regmap_read_poll_timeout(priv->regmap, PHY_R5, val,
+				       !(val & PHY_R5_PHY_CR_ACK),
+				       5, 1000);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int phy_g12a_usb3_pcie_cr_bus_read(void *context, unsigned int addr,
+					  unsigned int *data)
+{
+	struct phy_g12a_usb3_pcie_priv *priv = context;
+	unsigned int val;
+	int ret;
+
+	ret = phy_g12a_usb3_pcie_cr_bus_addr(priv, addr);
+	if (ret)
+		return ret;
+
+	regmap_write(priv->regmap, PHY_R4, 0);
+	regmap_write(priv->regmap, PHY_R4, PHY_R4_PHY_CR_READ);
+
+	ret = regmap_read_poll_timeout(priv->regmap, PHY_R5, val,
+				       (val & PHY_R5_PHY_CR_ACK),
+				       5, 1000);
+	if (ret)
+		return ret;
+
+	*data = FIELD_GET(PHY_R5_PHY_CR_DATA_OUT, val);
+
+	regmap_write(priv->regmap, PHY_R4, 0);
+
+	ret = regmap_read_poll_timeout(priv->regmap, PHY_R5, val,
+				       !(val & PHY_R5_PHY_CR_ACK),
+				       5, 1000);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int phy_g12a_usb3_pcie_cr_bus_write(void *context, unsigned int addr,
+					   unsigned int data)
+{
+	struct phy_g12a_usb3_pcie_priv *priv = context;
+	unsigned int val, reg;
+	int ret;
+
+	ret = phy_g12a_usb3_pcie_cr_bus_addr(priv, addr);
+	if (ret)
+		return ret;
+
+	reg = FIELD_PREP(PHY_R4_PHY_CR_DATA_IN, data);
+
+	regmap_write(priv->regmap, PHY_R4, reg);
+	regmap_write(priv->regmap, PHY_R4, reg);
+
+	regmap_write(priv->regmap, PHY_R4, reg | PHY_R4_PHY_CR_CAP_DATA);
+
+	ret = regmap_read_poll_timeout(priv->regmap, PHY_R5, val,
+				       (val & PHY_R5_PHY_CR_ACK),
+				       5, 1000);
+	if (ret)
+		return ret;
+
+	regmap_write(priv->regmap, PHY_R4, reg);
+
+	ret = regmap_read_poll_timeout(priv->regmap, PHY_R5, val,
+				       (val & PHY_R5_PHY_CR_ACK) == 0,
+				       5, 1000);
+	if (ret)
+		return ret;
+
+	regmap_write(priv->regmap, PHY_R4, reg);
+
+	regmap_write(priv->regmap, PHY_R4, reg | PHY_R4_PHY_CR_WRITE);
+
+	ret = regmap_read_poll_timeout(priv->regmap, PHY_R5, val,
+				       (val & PHY_R5_PHY_CR_ACK),
+				       5, 1000);
+	if (ret)
+		return ret;
+
+	regmap_write(priv->regmap, PHY_R4, reg);
+
+	ret = regmap_read_poll_timeout(priv->regmap, PHY_R5, val,
+				       (val & PHY_R5_PHY_CR_ACK) == 0,
+				       5, 1000);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct regmap_config phy_g12a_usb3_pcie_cr_regmap_conf = {
+	.reg_bits = 16,
+	.val_bits = 16,
+	.reg_read = phy_g12a_usb3_pcie_cr_bus_read,
+	.reg_write = phy_g12a_usb3_pcie_cr_bus_write,
+	.max_register = 0xffff,
+	.fast_io = true,
+};
+
+static int phy_g12a_usb3_init(struct phy *phy)
+{
+	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
+	int data, ret;
+
+	/* Switch PHY to USB3 */
+	regmap_update_bits(priv->regmap, PHY_R0,
+			   PHY_R0_PCIE_USB3_SWITCH,
+			   PHY_R0_PCIE_USB3_SWITCH);
+
+	/*
+	 * WORKAROUND: There is SSPHY suspend bug due to
+	 * which USB enumerates
+	 * in HS mode instead of SS mode. Workaround it by asserting
+	 * LANE0.TX_ALT_BLOCK.EN_ALT_BUS to enable TX to use alt bus
+	 * mode
+	 */
+	ret = regmap_update_bits(priv->regmap_cr, 0x102d, BIT(7), BIT(7));
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(priv->regmap_cr, 0x1010, 0xff0, 20);
+	if (ret)
+		return ret;
+
+	/*
+	 * Fix RX Equalization setting as follows
+	 * LANE0.RX_OVRD_IN_HI. RX_EQ_EN set to 0
+	 * LANE0.RX_OVRD_IN_HI.RX_EQ_EN_OVRD set to 1
+	 * LANE0.RX_OVRD_IN_HI.RX_EQ set to 3
+	 * LANE0.RX_OVRD_IN_HI.RX_EQ_OVRD set to 1
+	 */
+	ret = regmap_read(priv->regmap_cr, 0x1006, &data);
+	if (ret)
+		return ret;
+
+	data &= ~BIT(6);
+	data |= BIT(7);
+	data &= ~(0x7 << 8);
+	data |= (0x3 << 8);
+	data |= (1 << 11);
+	ret = regmap_write(priv->regmap_cr, 0x1006, data);
+	if (ret)
+		return ret;
+
+	/*
+	 * Set EQ and TX launch amplitudes as follows
+	 * LANE0.TX_OVRD_DRV_LO.PREEMPH set to 22
+	 * LANE0.TX_OVRD_DRV_LO.AMPLITUDE set to 127
+	 * LANE0.TX_OVRD_DRV_LO.EN set to 1.
+	 */
+	ret = regmap_read(priv->regmap_cr, 0x1002, &data);
+	if (ret)
+		return ret;
+
+	data &= ~0x3f80;
+	data |= (0x16 << 7);
+	data &= ~0x7f;
+	data |= (0x7f | BIT(14));
+	ret = regmap_write(priv->regmap_cr, 0x1002, data);
+	if (ret)
+		return ret;
+
+	/*
+	 * MPLL_LOOP_CTL.PROP_CNTRL = 8
+	 */
+	ret = regmap_update_bits(priv->regmap_cr, 0x30, 0xf << 4, 8 << 4);
+	if (ret)
+		return ret;
+
+	regmap_update_bits(priv->regmap, PHY_R2,
+			PHY_R2_PHY_TX_VBOOST_LVL,
+			FIELD_PREP(PHY_R2_PHY_TX_VBOOST_LVL, 0x4));
+
+	regmap_update_bits(priv->regmap, PHY_R1,
+			PHY_R1_PHY_LOS_BIAS | PHY_R1_PHY_LOS_LEVEL,
+			FIELD_PREP(PHY_R1_PHY_LOS_BIAS, 4) |
+			FIELD_PREP(PHY_R1_PHY_LOS_LEVEL, 9));
+
+	return 0;
+}
+
+static int phy_g12a_usb3_pcie_init(struct phy *phy)
+{
+	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
+	int ret;
+
+	ret = reset_control_reset(priv->reset);
+	if (ret)
+		return ret;
+
+	if (priv->mode == PHY_TYPE_USB3)
+		return phy_g12a_usb3_init(phy);
+
+	/* Power UP PCIE */
+	regmap_update_bits(priv->regmap, PHY_R0,
+			   PHY_R0_PCIE_POWER_STATE,
+			   FIELD_PREP(PHY_R0_PCIE_POWER_STATE, 0x1c));
+
+	return 0;
+}
+
+static int phy_g12a_usb3_pcie_exit(struct phy *phy)
+{
+	struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
+	int ret;
+
+	return reset_control_reset(priv->reset);
+}
+
+static struct phy *phy_g12a_usb3_pcie_xlate(struct device *dev,
+					    struct of_phandle_args *args)
+{
+	struct phy_g12a_usb3_pcie_priv *priv = dev_get_drvdata(dev);
+	unsigned int mode;
+
+	if (args->args_count < 1) {
+		dev_err(dev, "invalid number of arguments\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	mode = args->args[0];
+
+	if (mode != PHY_TYPE_USB3 && mode != PHY_TYPE_PCIE) {
+		dev_err(dev, "invalid phy mode select argument\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	priv->mode = mode;
+
+	return priv->phy;
+}
+
+static const struct phy_ops phy_g12a_usb3_pcie_ops = {
+	.init		= phy_g12a_usb3_pcie_init,
+	.exit		= phy_g12a_usb3_pcie_exit,
+	.owner		= THIS_MODULE,
+};
+
+static int phy_g12a_usb3_pcie_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct phy_g12a_usb3_pcie_priv *priv;
+	struct resource *res;
+	struct phy_provider *phy_provider;
+	void __iomem *base;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	priv->regmap = devm_regmap_init_mmio(dev, base,
+					     &phy_g12a_usb3_pcie_regmap_conf);
+	if (IS_ERR(priv->regmap))
+		return PTR_ERR(priv->regmap);
+
+	priv->regmap_cr = devm_regmap_init(dev, NULL, priv,
+					   &phy_g12a_usb3_pcie_cr_regmap_conf);
+	if (IS_ERR(priv->regmap_cr))
+		return PTR_ERR(priv->regmap_cr);
+
+	priv->clk_ref = devm_clk_get(dev, "ref_clk");
+	if (IS_ERR(priv->clk_ref))
+		return PTR_ERR(priv->clk_ref);
+
+	ret = clk_prepare_enable(priv->clk_ref);
+	if (ret)
+		goto err_disable_clk_ref;
+
+	priv->reset = devm_reset_control_array_get(dev, false, false);
+	if (IS_ERR(priv->reset))
+		return PTR_ERR(priv->reset);
+
+	priv->phy = devm_phy_create(dev, np, &phy_g12a_usb3_pcie_ops);
+	if (IS_ERR(priv->phy)) {
+		ret = PTR_ERR(priv->phy);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to create PHY\n");
+
+		return ret;
+	}
+
+	phy_set_drvdata(priv->phy, priv);
+	dev_set_drvdata(dev, priv);
+
+	phy_provider = devm_of_phy_provider_register(dev,
+						     phy_g12a_usb3_pcie_xlate);
+
+	return PTR_ERR_OR_ZERO(phy_provider);
+
+err_disable_clk_ref:
+	clk_disable_unprepare(priv->clk_ref);
+
+	return ret;
+}
+
+static const struct of_device_id phy_g12a_usb3_pcie_of_match[] = {
+	{ .compatible = "amlogic,g12a-usb3-pcie-phy", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, phy_g12a_usb3_pcie_of_match);
+
+static struct platform_driver phy_g12a_usb3_pcie_driver = {
+	.probe	= phy_g12a_usb3_pcie_probe,
+	.driver	= {
+		.name		= "phy-g12a-usb3-pcie",
+		.of_match_table	= phy_g12a_usb3_pcie_of_match,
+	},
+};
+module_platform_driver(phy_g12a_usb3_pcie_driver);
+
+MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
+MODULE_DESCRIPTION("Amlogic G12A USB3 + PCIE Combo PHY driver");
+MODULE_LICENSE("GPL v2");
-- 
2.20.1


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

* [PATCH 7/8] usb: dwc2: Add Amlogic G12A DWC2 Params
  2019-02-12 15:14 [PATCH 0/8] arm64: meson: Add support for USB on Amlogic G12A Neil Armstrong
                   ` (5 preceding siblings ...)
  2019-02-12 15:14 ` [PATCH 6/8] phy: amlogic: Add Amlogic G12A USB3 + PCIE Combo " Neil Armstrong
@ 2019-02-12 15:14 ` Neil Armstrong
  2019-04-09  9:31   ` Minas Harutyunyan
  2019-02-12 15:14 ` [PATCH 8/8] usb: dwc3: Add Amlogic G12A DWC3 glue Neil Armstrong
  7 siblings, 1 reply; 25+ messages in thread
From: Neil Armstrong @ 2019-02-12 15:14 UTC (permalink / raw)
  To: gregkh, hminas, balbi, kishon
  Cc: Neil Armstrong, linux-amlogic, linux-usb, linux-arm-kernel, linux-kernel

This patchs sets the params for the DWC2 Controller found in the
Amlogic G12A SoC family.

It mainly sets the settings reported incorrect by the driver,
leaving the remaining detected automatically by the driver and
provided by the DT node.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/usb/dwc2/params.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 24ff5f21cb25..442113246cba 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -121,6 +121,16 @@ static void dwc2_set_amlogic_params(struct dwc2_hsotg *hsotg)
 	p->power_down = DWC2_POWER_DOWN_PARAM_NONE;
 }
 
+static void dwc2_set_amlogic_g12a_params(struct dwc2_hsotg *hsotg)
+{
+	struct dwc2_core_params *p = &hsotg->params;
+
+	p->lpm = false;
+	p->lpm_clock_gating = false;
+	p->besl = false;
+	p->hird_threshold_en = false;
+}
+
 static void dwc2_set_amcc_params(struct dwc2_hsotg *hsotg)
 {
 	struct dwc2_core_params *p = &hsotg->params;
@@ -167,6 +177,8 @@ const struct of_device_id dwc2_of_match_table[] = {
 	  .data = dwc2_set_amlogic_params },
 	{ .compatible = "amlogic,meson-gxbb-usb",
 	  .data = dwc2_set_amlogic_params },
+	{ .compatible = "amlogic,meson-g12a-usb",
+	  .data = dwc2_set_amlogic_g12a_params },
 	{ .compatible = "amcc,dwc-otg", .data = dwc2_set_amcc_params },
 	{ .compatible = "st,stm32f4x9-fsotg",
 	  .data = dwc2_set_stm32f4x9_fsotg_params },
-- 
2.20.1


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

* [PATCH 8/8] usb: dwc3: Add Amlogic G12A DWC3 glue
  2019-02-12 15:14 [PATCH 0/8] arm64: meson: Add support for USB on Amlogic G12A Neil Armstrong
                   ` (6 preceding siblings ...)
  2019-02-12 15:14 ` [PATCH 7/8] usb: dwc2: Add Amlogic G12A DWC2 Params Neil Armstrong
@ 2019-02-12 15:14 ` Neil Armstrong
  2019-02-24 20:40   ` Martin Blumenstingl
  7 siblings, 1 reply; 25+ messages in thread
From: Neil Armstrong @ 2019-02-12 15:14 UTC (permalink / raw)
  To: gregkh, hminas, balbi, kishon
  Cc: Neil Armstrong, linux-amlogic, linux-usb, linux-arm-kernel, linux-kernel

Adds support for Amlogic G12A USB Control Glue HW.

The Amlogic G12A SoC Family embeds 2 USB Controllers :
- a DWC3 IP configured as Host for USB2 and USB3
- a DWC2 IP configured as Peripheral USB2 Only

A glue connects these both controllers to 2 USB2 PHYs, and optionnally
to an USB3+PCIE Combo PHY shared with the PCIE controller.

The Glue configures the UTMI 8bit interfaces for the USB2 PHYs, including
routing of the OTG PHY between the DWC3 and DWC2 controllers, and
setups the on-chip OTG mode selection for this PHY.

The PHYs are childen of the Glue node since the Glue controls the interface
with the PHY, not the DWC3 controller.
The drivers collects the mode of each PHY and determine which PHY
is to be routed between the DWC2 and DWC3 controllers.

This drivers supports the on-probe setup of the OTG mode, and manually
via a debugfs interface. The IRQ mode change detect is yet to be added
in a future patchset, mainly due to lack of hardware to validate on.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/usb/dwc3/Kconfig           |   9 +
 drivers/usb/dwc3/Makefile          |   1 +
 drivers/usb/dwc3/dwc3-meson-g12a.c | 650 +++++++++++++++++++++++++++++
 3 files changed, 660 insertions(+)
 create mode 100644 drivers/usb/dwc3/dwc3-meson-g12a.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 1a0404fda596..4335e5e76bbb 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -93,6 +93,15 @@ config USB_DWC3_KEYSTONE
 	  Support of USB2/3 functionality in TI Keystone2 platforms.
 	  Say 'Y' or 'M' here if you have one such device
 
+config USB_DWC3_MESON_G12A
+       tristate "Amlogic Meson G12A Platforms"
+       depends on OF && COMMON_CLK
+       depends on ARCH_MESON || COMPILE_TEST
+       default USB_DWC3
+       help
+         Support USB2/3 functionality in Amlogic G12A platforms.
+	 Say 'Y' or 'M' if you have one such device.
+
 config USB_DWC3_OF_SIMPLE
        tristate "Generic OF Simple Glue Layer"
        depends on OF && COMMON_CLK
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index 6e3ef6144e5d..ae86da0dc5bd 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_G12A)	+= dwc3-meson-g12a.o
 obj-$(CONFIG_USB_DWC3_OF_SIMPLE)	+= dwc3-of-simple.o
 obj-$(CONFIG_USB_DWC3_ST)		+= dwc3-st.o
 obj-$(CONFIG_USB_DWC3_QCOM)		+= dwc3-qcom.o
diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c
new file mode 100644
index 000000000000..abeff2d56b1d
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-meson-g12a.c
@@ -0,0 +1,650 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * USB Glue for Amlogic G12A SoCs
+ *
+ * Copyright (c) 2019 BayLibre, SAS
+ * Author: Neil Armstrong <narmstrong@baylibre.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
+ * - SuperSpeed PHY can be enabled only if port is used
+ *
+ * TOFIX:
+ * - Add dynamic OTG switching with ID change interrupt
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/reset.h>
+#include <linux/of_graph.h>
+#include <linux/phy/phy.h>
+#include <linux/usb/otg.h>
+#include <linux/debugfs.h>
+
+/* USB Glue Control Registers */
+
+#define USB_R0							0x00
+	#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							0x04
+	#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							0x08
+	#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							0x0c
+	#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							0x10
+	#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							0x14
+	#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)
+
+/* USB2 Ports Control Registers */
+
+#define U2P_R0							0x0
+	#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							0x4
+	#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)
+
+#define MAX_PHY							5
+
+#define USB2_MAX_PHY						4
+#define USB3_PHY						4
+
+struct dwc3_meson_g12a {
+	struct device		*dev;
+	struct regmap		*regmap;
+	struct clk		*clk;
+	struct reset_control	*reset;
+	struct phy		*phys[5];
+	enum usb_dr_mode	phy_modes[5];
+	enum phy_mode		otg_phy_mode;
+	unsigned int		usb2_ports;
+	unsigned int		usb3_ports;
+	struct dentry		*root;
+};
+
+#define U2P_REG_SIZE						0x20
+#define USB_REG_OFFSET						0x80
+
+static void dwc3_meson_g12a_usb2_set_mode(struct dwc3_meson_g12a *priv,
+					  int i, enum usb_dr_mode mode)
+{
+	switch (mode) {
+	case USB_DR_MODE_HOST:
+	case USB_DR_MODE_OTG:
+	case USB_DR_MODE_UNKNOWN:
+		regmap_update_bits(priv->regmap, U2P_R0 + (U2P_REG_SIZE * i),
+				U2P_R0_HOST_DEVICE,
+				U2P_R0_HOST_DEVICE);
+		break;
+
+	case USB_DR_MODE_PERIPHERAL:
+		regmap_update_bits(priv->regmap, U2P_R0 + (U2P_REG_SIZE * i),
+				U2P_R0_HOST_DEVICE, 0);
+		break;
+	}
+}
+
+static int dwc3_meson_g12a_usb2_init(struct dwc3_meson_g12a *priv)
+{
+	enum usb_dr_mode id_mode;
+	u32 val;
+	int i;
+
+	/* Read ID current level */
+	regmap_read(priv->regmap, USB_R5, &val);
+	if (val & USB_R5_ID_DIG_CURR)
+		id_mode = USB_DR_MODE_PERIPHERAL;
+	else
+		id_mode = USB_DR_MODE_HOST;
+
+	dev_info(priv->dev, "ID mode %s\n",
+		 id_mode ==  USB_DR_MODE_HOST ? "host" : "peripheral");
+
+	for (i = 0 ; i < USB2_MAX_PHY ; ++i) {
+		if (!priv->phys[i])
+			continue;
+
+		regmap_update_bits(priv->regmap, U2P_R0 + (U2P_REG_SIZE * i),
+				   U2P_R0_POWER_ON_RESET,
+				   U2P_R0_POWER_ON_RESET);
+
+		if (priv->phy_modes[i] == USB_DR_MODE_PERIPHERAL ||
+		    (priv->phy_modes[i] == USB_DR_MODE_OTG &&
+		     id_mode == USB_DR_MODE_PERIPHERAL)) {
+			dwc3_meson_g12a_usb2_set_mode(priv, i,
+						      USB_DR_MODE_PERIPHERAL);
+
+			if (priv->phy_modes[i] == USB_DR_MODE_OTG)
+				priv->otg_phy_mode = PHY_MODE_USB_DEVICE;
+		} else {
+			dwc3_meson_g12a_usb2_set_mode(priv, i,
+						      USB_DR_MODE_HOST);
+
+			if (priv->phy_modes[i] == USB_DR_MODE_OTG)
+				priv->otg_phy_mode = PHY_MODE_USB_HOST;
+		}
+
+		regmap_update_bits(priv->regmap, U2P_R0 + (U2P_REG_SIZE * i),
+				   U2P_R0_POWER_ON_RESET, 0);
+	}
+
+	return 0;
+}
+
+static void dwc3_meson_g12a_usb3_init(struct dwc3_meson_g12a *priv)
+{
+	regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R3,
+			USB_R3_P30_SSC_RANGE_MASK |
+			USB_R3_P30_REF_SSP_EN,
+			USB_R3_P30_SSC_ENABLE |
+			FIELD_PREP(USB_R3_P30_SSC_RANGE_MASK, 2) |
+			USB_R3_P30_REF_SSP_EN);
+	udelay(2);
+
+	regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R2,
+			USB_R2_P30_PCS_TX_DEEMPH_3P5DB_MASK,
+			FIELD_PREP(USB_R2_P30_PCS_TX_DEEMPH_3P5DB_MASK, 0x15));
+
+	regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R2,
+			USB_R2_P30_PCS_TX_DEEMPH_6DB_MASK,
+			FIELD_PREP(USB_R2_P30_PCS_TX_DEEMPH_6DB_MASK, 0x20));
+
+	udelay(2);
+
+	regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R1,
+			USB_R1_U3H_HOST_PORT_POWER_CONTROL_PRESENT,
+			USB_R1_U3H_HOST_PORT_POWER_CONTROL_PRESENT);
+
+	regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R1,
+			USB_R1_P30_PCS_TX_SWING_FULL_MASK,
+			FIELD_PREP(USB_R1_P30_PCS_TX_SWING_FULL_MASK, 127));
+}
+
+static void dwc3_meson_g12a_usb_init_mode(struct dwc3_meson_g12a *priv,
+					  bool is_peripheral)
+{
+	if (is_peripheral) {
+		regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R0,
+				USB_R0_U2D_ACT, USB_R0_U2D_ACT);
+		regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R0,
+				USB_R0_U2D_SS_SCALEDOWN_MODE_MASK, 0);
+		regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R4,
+				USB_R4_P21_SLEEP_M0, USB_R4_P21_SLEEP_M0);
+	} else {
+		regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R0,
+				USB_R0_U2D_ACT, 0);
+		regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R4,
+				USB_R4_P21_SLEEP_M0, 0);
+	}
+}
+
+static int dwc3_meson_g12a_usb_init(struct dwc3_meson_g12a *priv)
+{
+	int ret;
+
+	ret = dwc3_meson_g12a_usb2_init(priv);
+	if (ret)
+		return ret;
+
+	regmap_update_bits(priv->regmap, USB_REG_OFFSET + 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_REG_OFFSET + USB_R5,
+			USB_R5_ID_DIG_EN_0,
+			USB_R5_ID_DIG_EN_0);
+	regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R5,
+			USB_R5_ID_DIG_EN_1,
+			USB_R5_ID_DIG_EN_1);
+	regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R5,
+			USB_R5_ID_DIG_TH_MASK,
+			FIELD_PREP(USB_R5_ID_DIG_TH_MASK, 0xff));
+
+	/* If we have an actual SuperSpeed port, initialize it */
+	if (priv->usb3_ports)
+		dwc3_meson_g12a_usb3_init(priv);
+
+	dwc3_meson_g12a_usb_init_mode(priv,
+				(priv->otg_phy_mode == PHY_MODE_USB_DEVICE));
+
+	return 0;
+}
+
+static const struct regmap_config phy_meson_g12a_usb3_regmap_conf = {
+	.reg_bits = 8,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.max_register = USB_REG_OFFSET + USB_R5,
+};
+
+static int dwc3_meson_g12a_get_phys(struct dwc3_meson_g12a *priv)
+{
+	struct device_node *port, *phy_node;
+	struct of_phandle_args args;
+	enum usb_dr_mode mode;
+	const char *dr_mode;
+	struct phy *phy;
+	int ret, i;
+
+	for (i = 0 ; i < MAX_PHY ; ++i) {
+		port = of_graph_get_port_by_id(priv->dev->of_node, i);
+
+		/* Ignore port if not defined or disabled */
+		if (!of_device_is_available(port)) {
+			of_node_put(port);
+			continue;
+		}
+
+		/* Get associated PHY */
+		phy = of_phy_get(port, NULL);
+		if (IS_ERR(phy)) {
+			of_node_put(port);
+			ret = PTR_ERR(phy);
+			goto err_phy_get;
+		}
+
+		of_node_put(port);
+
+		/* Get phy dr_mode */
+		ret = of_parse_phandle_with_args(port, "phys", "#phy-cells",
+						 0, &args);
+		if (ret) {
+			of_node_put(port);
+			goto err_phy_get;
+		}
+
+		phy_node = args.np;
+
+		ret = of_property_read_string(phy_node, "dr_mode", &dr_mode);
+		if (ret) {
+			dr_mode = "unknown";
+			mode = USB_DR_MODE_UNKNOWN;
+		} else {
+			if (!strcmp(dr_mode, "host"))
+				mode = USB_DR_MODE_HOST;
+			else if (!strcmp(dr_mode, "otg"))
+				mode = USB_DR_MODE_OTG;
+			else if (!strcmp(dr_mode, "peripheral"))
+				mode = USB_DR_MODE_PERIPHERAL;
+			else {
+				mode = USB_DR_MODE_UNKNOWN;
+				dr_mode = "unknown";
+			}
+		}
+
+		dev_info(priv->dev, "port%d: %s mode %s\n",
+			 i, of_node_full_name(phy_node), dr_mode);
+
+		of_node_put(phy_node);
+
+		priv->phy_modes[i] = mode;
+		priv->phys[i] = phy;
+
+		if (i == USB3_PHY)
+			priv->usb3_ports++;
+		else
+			priv->usb2_ports++;
+	}
+
+	dev_info(priv->dev, "usb2 ports: %d\n", priv->usb2_ports);
+	dev_info(priv->dev, "usb3 ports: %d\n", priv->usb3_ports);
+
+	return 0;
+
+err_phy_get:
+	for (i = 0 ; i < MAX_PHY ; ++i)
+		if (priv->phys[i])
+			phy_put(priv->phys[i]);
+
+	return ret;
+}
+
+static int dwc3_meson_g12a_mode_force_get(void *data, u64 *val)
+{
+	struct dwc3_meson_g12a *priv = data;
+
+	if (priv->otg_phy_mode == PHY_MODE_USB_HOST)
+		*val = 1;
+	else if (priv->otg_phy_mode == PHY_MODE_USB_DEVICE)
+		*val = 0;
+	else
+		return -EINVAL;
+
+	return 0;
+}
+
+static int dwc3_meson_g12a_mode_force_set(void *data, u64 val)
+{
+	struct dwc3_meson_g12a *priv = data;
+	int i;
+
+	if ((val && priv->otg_phy_mode == PHY_MODE_USB_HOST) ||
+	    (!val && priv->otg_phy_mode == PHY_MODE_USB_DEVICE))
+		return 0;
+
+	for (i = 0 ; i < USB2_MAX_PHY ; ++i) {
+		if (!priv->phys[i])
+			continue;
+
+		if (priv->phy_modes[i] != USB_DR_MODE_OTG)
+			continue;
+
+		if (val) {
+			dev_info(priv->dev, "switching to Host Mode\n");
+
+			dwc3_meson_g12a_usb2_set_mode(priv, i,
+						      USB_DR_MODE_HOST);
+
+			dwc3_meson_g12a_usb_init_mode(priv, false);
+
+			priv->otg_phy_mode = PHY_MODE_USB_HOST;
+		} else {
+			dev_info(priv->dev, "switching to Device Mode\n");
+
+			dwc3_meson_g12a_usb2_set_mode(priv, i,
+						      USB_DR_MODE_PERIPHERAL);
+
+			dwc3_meson_g12a_usb_init_mode(priv, true);
+
+			priv->otg_phy_mode = PHY_MODE_USB_DEVICE;
+		}
+
+		return phy_set_mode(priv->phys[i], priv->otg_phy_mode);
+	}
+
+	return -EINVAL;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(dwc3_meson_g12a_mode_force_fops,
+			 dwc3_meson_g12a_mode_force_get,
+			 dwc3_meson_g12a_mode_force_set, "%llu\n");
+
+static int dwc3_meson_g12a_otg_id_get(void *data, u64 *val)
+{
+	struct dwc3_meson_g12a *priv = data;
+	u32 reg;
+
+	regmap_read(priv->regmap, USB_R5, &reg);
+
+	*val = (reg & USB_R5_ID_DIG_CURR);
+
+	return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(dwc3_meson_g12a_otg_id_fops,
+			 dwc3_meson_g12a_otg_id_get, NULL, "%llu\n");
+
+/* We provide a DebugFS interface to get the ID value and force OTG switch */
+static int dwc3_meson_g12a_debugfs_init(struct dwc3_meson_g12a *priv)
+{
+	priv->root = debugfs_create_dir("dwc3-meson-g12a", NULL);
+	if (IS_ERR(priv->root))
+		return PTR_ERR(priv->root);
+
+	debugfs_create_file("mode_force", 0600, priv->root, priv,
+			    &dwc3_meson_g12a_mode_force_fops);
+
+	debugfs_create_file("otg_id", 0400, priv->root, priv,
+			    &dwc3_meson_g12a_otg_id_fops);
+
+	return 0;
+}
+
+static int dwc3_meson_g12a_probe(struct platform_device *pdev)
+{
+	struct dwc3_meson_g12a	*priv;
+	struct device		*dev = &pdev->dev;
+	struct device_node	*np = dev->of_node;
+	void __iomem *base;
+	struct resource *res;
+	int ret, i;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	priv->regmap = devm_regmap_init_mmio(dev, base,
+					     &phy_meson_g12a_usb3_regmap_conf);
+	if (IS_ERR(priv->regmap))
+		return PTR_ERR(priv->regmap);
+
+	priv->clk = devm_clk_get(dev, "usb");
+	if (IS_ERR(priv->clk))
+		return PTR_ERR(priv->clk);
+
+	ret = clk_prepare_enable(priv->clk);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, priv);
+	priv->dev = dev;
+
+	priv->reset = devm_reset_control_get(dev, "usb");
+	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_g12a_get_phys(priv);
+	if (ret)
+		return ret;
+
+	dwc3_meson_g12a_usb_init(priv);
+
+	/* Init PHYs */
+	for (i = 0 ; i < MAX_PHY ; ++i) {
+		if (priv->phys[i]) {
+			ret = phy_init(priv->phys[i]);
+			if (ret)
+				goto err_phys_put;
+		}
+	}
+
+	/* Set OTG PHY mode */
+	for (i = 0 ; i < MAX_PHY ; ++i) {
+		if (priv->phys[i] && priv->phy_modes[i] == USB_DR_MODE_OTG) {
+			ret = phy_set_mode(priv->phys[i], priv->otg_phy_mode);
+			if (ret)
+				goto err_phys_put;
+		}
+	}
+
+	ret = of_platform_populate(np, NULL, NULL, dev);
+	if (ret) {
+		clk_disable_unprepare(priv->clk);
+		clk_put(priv->clk);
+
+		goto err_phys_exit;
+	}
+
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+	pm_runtime_get_sync(dev);
+
+	if (dwc3_meson_g12a_debugfs_init(priv))
+		dev_dbg(dev, "Failed to add DebugFS interface\n");
+
+	return 0;
+
+err_phys_exit:
+	for (i = 0 ; i < MAX_PHY ; ++i)
+		if (priv->phys[i])
+			phy_exit(priv->phys[i]);
+
+err_phys_put:
+	for (i = 0 ; i < MAX_PHY ; ++i)
+		if (priv->phys[i])
+			phy_put(priv->phys[i]);
+
+	return ret;
+}
+
+static int dwc3_meson_g12a_remove(struct platform_device *pdev)
+{
+	struct dwc3_meson_g12a *priv = platform_get_drvdata(pdev);
+	struct device *dev = &pdev->dev;
+	int i;
+
+	debugfs_remove_recursive(priv->root);
+
+	of_platform_depopulate(dev);
+
+	for (i = 0 ; i < MAX_PHY ; ++i)
+		if (priv->phys[i])
+			phy_exit(priv->phys[i]);
+
+	for (i = 0 ; i < MAX_PHY ; ++i)
+		if (priv->phys[i])
+			phy_put(priv->phys[i]);
+
+	clk_disable_unprepare(priv->clk);
+	clk_put(priv->clk);
+
+	pm_runtime_disable(dev);
+	pm_runtime_put_noidle(dev);
+	pm_runtime_set_suspended(dev);
+
+	return 0;
+}
+
+static int __maybe_unused dwc3_meson_g12a_runtime_suspend(struct device *dev)
+{
+	struct dwc3_meson_g12a	*priv = dev_get_drvdata(dev);
+
+	clk_disable(priv->clk);
+
+	return 0;
+}
+
+static int __maybe_unused dwc3_meson_g12a_runtime_resume(struct device *dev)
+{
+	struct dwc3_meson_g12a	*priv = dev_get_drvdata(dev);
+
+	return clk_enable(priv->clk);
+}
+
+static int __maybe_unused dwc3_meson_g12a_suspend(struct device *dev)
+{
+	struct dwc3_meson_g12a *priv = dev_get_drvdata(dev);
+	int i;
+
+	for (i = 0 ; i < MAX_PHY ; ++i)
+		if (priv->phys[i])
+			phy_exit(priv->phys[i]);
+
+	reset_control_assert(priv->reset);
+
+	return 0;
+}
+
+static int __maybe_unused dwc3_meson_g12a_resume(struct device *dev)
+{
+	struct dwc3_meson_g12a *priv = dev_get_drvdata(dev);
+	int i, ret;
+
+	reset_control_deassert(priv->reset);
+
+	dwc3_meson_g12a_usb_init(priv);
+
+	/* Init PHYs */
+	for (i = 0 ; i < MAX_PHY ; ++i) {
+		if (priv->phys[i]) {
+			ret = phy_init(priv->phys[i]);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops dwc3_meson_g12a_dev_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(dwc3_meson_g12a_suspend, dwc3_meson_g12a_resume)
+	SET_RUNTIME_PM_OPS(dwc3_meson_g12a_runtime_suspend,
+			dwc3_meson_g12a_runtime_resume, NULL)
+};
+
+static const struct of_device_id dwc3_meson_g12a_match[] = {
+	{ .compatible = "amlogic,meson-g12a-usb-ctrl" },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, dwc3_meson_g12a_match);
+
+static struct platform_driver dwc3_meson_g12a_driver = {
+	.probe		= dwc3_meson_g12a_probe,
+	.remove		= dwc3_meson_g12a_remove,
+	.driver		= {
+		.name	= "dwc3-meson-g12a",
+		.of_match_table = dwc3_meson_g12a_match,
+		.pm	= &dwc3_meson_g12a_dev_pm_ops,
+	},
+};
+
+module_platform_driver(dwc3_meson_g12a_driver);
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Amlogic Meson G12A USB Glue Layer");
+MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
-- 
2.20.1


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

* Re: [PATCH 1/8] dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings
  2019-02-12 15:14 ` [PATCH 1/8] dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings Neil Armstrong
@ 2019-02-17 21:58   ` Martin Blumenstingl
  2019-02-28 15:18   ` Rob Herring
  1 sibling, 0 replies; 25+ messages in thread
From: Martin Blumenstingl @ 2019-02-17 21:58 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: gregkh, hminas, balbi, kishon, devicetree, linux-amlogic,
	linux-usb, linux-kernel, linux-arm-kernel

On Tue, Feb 12, 2019 at 4:14 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Add the Amlogic G12A Family USB2 OTG PHY Bindings
nit-pick: if you want to keep "OTG" in there then please add a short
description how OTG works on this PHY.
I would describe this as: "Add the Amlogic G12A Family USB2 PHY
Bindings. The PHY works for host or peripheral modes. Configuration of
the mode is part of the USBCTRL registers which are outside of the PHY
registers."

> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
with the patch description nit-pick addressed:
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>


Regards
Martin

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

* Re: [PATCH 2/8] dt-bindings: phy: Add Amlogic G12A USB3+PCIE Combo PHY Bindings
  2019-02-12 15:14 ` [PATCH 2/8] dt-bindings: phy: Add Amlogic G12A USB3+PCIE Combo " Neil Armstrong
@ 2019-02-17 22:03   ` Martin Blumenstingl
  2019-02-18 10:33     ` Neil Armstrong
  0 siblings, 1 reply; 25+ messages in thread
From: Martin Blumenstingl @ 2019-02-17 22:03 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: gregkh, hminas, balbi, kishon, devicetree, linux-amlogic,
	linux-usb, linux-kernel, linux-arm-kernel

On Tue, Feb 12, 2019 at 4:15 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Add the Amlogic G12A Family USB3 + PCIE Combo PHY Bindings.
>
> This PHY can provide exclusively USB3 or PCIE support on shared I/Os.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
one nit-pick below, but apart from that:
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

> ---
>  .../bindings/phy/meson-g12a-usb3-pcie-phy.txt | 25 +++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/meson-g12a-usb3-pcie-phy.txt
>
> diff --git a/Documentation/devicetree/bindings/phy/meson-g12a-usb3-pcie-phy.txt b/Documentation/devicetree/bindings/phy/meson-g12a-usb3-pcie-phy.txt
> new file mode 100644
> index 000000000000..714d751091f5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/meson-g12a-usb3-pcie-phy.txt
> @@ -0,0 +1,25 @@
> +* Amlogic G12A USB3 + PCIE Combo PHY binding
> +
> +Required properties:
> +- compatible:  Should be "amlogic,meson-g12a-usb3-pcie-phy"
> +- #phys-cells: must be 1. The cell number is used to select the phy mode
> +  as defined in <dt-bindings/phy/phy.h> between PHY_TYPE_USB3 and PHY_TYPE_PCIE
> +- reg:         The base address and length of the registers
> +- clocks:      a phandle to the 100MHz reference clock of this PHY
> +- clock-names: must be "ref_clk"
> +- resets:      phandle to the reset lines for:
> +               - the PHY control
> +               - the USB3+PCIE PHY
> +               - the PHY registers
no reset-names (like in the G12A USB2 PHY bindings) here?
even if you don't use them in the driver I suggest you add them for
consistency (and maybe to make it easier to compare the bindings with
the datasheet. I don't have access to the datasheet so I'm not sure if
having the reset-names is relevant for this case)


Regards
Martin

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

* Re: [PATCH 3/8] dt-bindings: usb: dwc2: Add Amlogic G12A DWC2 Compatible
  2019-02-12 15:14 ` [PATCH 3/8] dt-bindings: usb: dwc2: Add Amlogic G12A DWC2 Compatible Neil Armstrong
@ 2019-02-17 22:07   ` Martin Blumenstingl
  2019-02-28 16:14   ` Rob Herring
  1 sibling, 0 replies; 25+ messages in thread
From: Martin Blumenstingl @ 2019-02-17 22:07 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: gregkh, hminas, balbi, kishon, devicetree, linux-amlogic,
	linux-usb, linux-kernel, linux-arm-kernel

Hi Neil,

On Tue, Feb 12, 2019 at 4:15 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Adds the specific compatible string for the DWC2 IP found in the
> Amlogic G12A SoC Family.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

> ---
>  Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
> index 6dc3c4a34483..e150b7b227c9 100644
> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
> @@ -14,6 +14,7 @@ Required properties:
>    - "amlogic,meson8-usb": The DWC2 USB controller instance in Amlogic Meson8 SoCs;
>    - "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
>    - "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
> +  - "amlogic,meson-g12a-usb": The DWC2 USB controller instance in Amlogic G12A SoCs;
if anyone is curious: starting with GXL (not supported by the dwc2
driver yet) the dwc2 core is "peripheral mode" only while previous
SoCs had one host-only dwc2 instance and another OTG capable dwc2
instance

I also discussed the compatible string with Neil off-list because I
was not sure if we have unique compatible names for the dwc2
controller and the "USB control" registers.
The "USB control" registers are named "USBCTRL" according to Neil (I
assume he got this information from the datasheet to which I don't
have access). Thus the compatible string for the "USB control" device
will be "amlogic,meson-g12a-usb-ctrl" - so we don't have any naming
conflict.


Regards
Martin

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

* Re: [PATCH 5/8] phy: amlogic: add Amlogic G12A USB2 PHY Driver
  2019-02-12 15:14 ` [PATCH 5/8] phy: amlogic: add Amlogic G12A USB2 PHY Driver Neil Armstrong
@ 2019-02-17 22:24   ` Martin Blumenstingl
  2019-02-18 10:38     ` Neil Armstrong
  0 siblings, 1 reply; 25+ messages in thread
From: Martin Blumenstingl @ 2019-02-17 22:24 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: gregkh, hminas, balbi, kishon, linux-amlogic, linux-usb,
	linux-kernel, linux-arm-kernel

Hi Neil,

On Tue, Feb 12, 2019 at 4:15 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> This adds support for the USB2 PHY found in the Amlogic G12A SoC Family.
>
> It supports Host and/or Peripheral mode, depending on it's position.
> The first PHY is only used as Host, but the second supports Dual modes
> defined by the USB Control Glue HW in front of the USB Controllers.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/phy/amlogic/Kconfig               |  12 ++
>  drivers/phy/amlogic/Makefile              |   1 +
>  drivers/phy/amlogic/phy-meson-g12a-usb2.c | 191 ++++++++++++++++++++++
>  3 files changed, 204 insertions(+)
>  create mode 100644 drivers/phy/amlogic/phy-meson-g12a-usb2.c
>
> diff --git a/drivers/phy/amlogic/Kconfig b/drivers/phy/amlogic/Kconfig
> index 23fe1cda2f70..78d6e194dce9 100644
> --- a/drivers/phy/amlogic/Kconfig
> +++ b/drivers/phy/amlogic/Kconfig
> @@ -36,3 +36,15 @@ config PHY_MESON_GXL_USB3
>           Enable this to support the Meson USB3 PHY and OTG detection
>           IP block found in Meson GXL and GXM SoCs.
>           If unsure, say N.
> +
> +config PHY_MESON_G12A_USB2
> +       tristate "Meson G12A USB2 PHY drivers"
aw, there's a typo in the GXL driver names (from which this new
Kconfig entry may have been copied): it should be "driver" instead of
"drivers"

> +       default ARCH_MESON
> +       depends on OF && (ARCH_MESON || COMPILE_TEST)
> +       depends on USB_SUPPORT
I don't think you need USB_SUPPORT (neither does PHY_MESON_GXL_USB2,
but it seems that I accidentally left it in from an early development
iteration), see below

> +       select GENERIC_PHY
> +       select REGMAP_MMIO
> +       help
> +         Enable this to support the Meson USB2 PHYs found in Meson
> +         G12A SoCs.
> +         If unsure, say N.
> diff --git a/drivers/phy/amlogic/Makefile b/drivers/phy/amlogic/Makefile
> index 4fd8848c194d..7d4d10f5a6b3 100644
> --- a/drivers/phy/amlogic/Makefile
> +++ b/drivers/phy/amlogic/Makefile
> @@ -1,3 +1,4 @@
>  obj-$(CONFIG_PHY_MESON8B_USB2)         += phy-meson8b-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-g12a-usb2.c b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
> new file mode 100644
> index 000000000000..3b6271a8be02
> --- /dev/null
> +++ b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
> @@ -0,0 +1,191 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Meson G12A USB2 PHY driver
> + *
> + * Copyright (C) 2017 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> + * Copyright (C) 2017 Amlogic, Inc. All rights reserved
> + * Copyright (C) 2019 BayLibre, SAS
> + * Author: Neil Armstrong <narmstrong@baylibre.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
I don't see any syscon usage in the driver so you can drop this

> +#include <linux/reset.h>
> +#include <linux/phy/phy.h>
> +#include <linux/usb/otg.h>
do you need this? if you drop "depends on USB_SUPPORT" from Kconfig
then you want to get rid of this as well

> +#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_R4                                            0x10
> +#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_R14                                           0x38
> +#define PHY_CTRL_R15                                           0x3c
> +#define PHY_CTRL_R16                                           0x40
> +#define PHY_CTRL_R17                                           0x44
> +#define PHY_CTRL_R18                                           0x48
> +#define PHY_CTRL_R19                                           0x4c
> +#define PHY_CTRL_R20                                           0x50
> +#define PHY_CTRL_R21                                           0x54
> +#define PHY_CTRL_R22                                           0x58
> +#define PHY_CTRL_R23                                           0x5c
> +
> +#define RESET_COMPLETE_TIME                                    1000
> +#define PLL_RESET_COMPLETE_TIME                                        100
> +
> +struct phy_meson_g12a_usb2_priv {
> +       struct device           *dev;
> +       struct regmap           *regmap;
> +       struct clk              *clk;
> +       struct reset_control    *reset;
> +};
> +
> +static const struct regmap_config phy_meson_g12a_usb2_regmap_conf = {
> +       .reg_bits = 8,
> +       .val_bits = 32,
> +       .reg_stride = 4,
> +       .max_register = PHY_CTRL_R23,
> +};
> +
> +static int phy_meson_g12a_usb2_init(struct phy *phy)
> +{
> +       struct phy_meson_g12a_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, BIT(2), 0);
do you have the name of the other register bits as well? having
#defines would make the code easier to read

> +
> +       /* PLL Setup : 24MHz * 20 / 1 = 480MHz */
> +       regmap_write(priv->regmap, PHY_CTRL_R16, 0x39400414);
> +       regmap_write(priv->regmap, PHY_CTRL_R17, 0x927e0000);
> +       regmap_write(priv->regmap, PHY_CTRL_R18, 0xac5f49e5);
> +
> +       udelay(PLL_RESET_COMPLETE_TIME);
> +
> +       /* UnReset PLL */
> +       regmap_write(priv->regmap, PHY_CTRL_R16, 0x19400414);
> +
> +       /* PHY Tuning */
> +       regmap_write(priv->regmap, PHY_CTRL_R20, 0xfe18);
> +       regmap_write(priv->regmap, PHY_CTRL_R4, 0x8000fff);
> +
> +       /* Tuning Disconnect Threshold */
> +       regmap_write(priv->regmap, PHY_CTRL_R3, 0x34);
> +
> +       /* Analog Settings */
> +       regmap_write(priv->regmap, PHY_CTRL_R14, 0);
> +       regmap_write(priv->regmap, PHY_CTRL_R13, 0x78000);
> +
> +       return 0;
> +}
> +
> +static int phy_meson_g12a_usb2_exit(struct phy *phy)
> +{
> +       struct phy_meson_g12a_usb2_priv *priv = phy_get_drvdata(phy);
> +
> +       return reset_control_reset(priv->reset);
do you have information on whether we should reset_control_assert here
instead of reset_control_reset?

> +}
> +
> +/* set_mode is not needed, mode setting is handled via the UTMI bus */
> +static const struct phy_ops phy_meson_g12a_usb2_ops = {
> +       .init           = phy_meson_g12a_usb2_init,
> +       .exit           = phy_meson_g12a_usb2_exit,
> +       .owner          = THIS_MODULE,
> +};
> +
> +static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct phy_provider *phy_provider;
> +       struct resource *res;
> +       struct phy_meson_g12a_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);
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       base = devm_ioremap_resource(dev, res);
> +       if (IS_ERR(base))
> +               return PTR_ERR(base);
> +
> +       priv->regmap = devm_regmap_init_mmio(dev, base,
> +                                            &phy_meson_g12a_usb2_regmap_conf);
> +       if (IS_ERR(priv->regmap))
> +               return PTR_ERR(priv->regmap);
> +
> +       priv->clk = devm_clk_get(dev, "xtal");
> +       if (IS_ERR(priv->clk))
> +               return PTR_ERR(priv->clk);
> +
> +       priv->reset = devm_reset_control_get(dev, "phy");
> +       if (IS_ERR(priv->reset))
> +               return PTR_ERR(priv->reset);
> +
> +       ret = reset_control_deassert(priv->reset);
should we move this to phy_meson_g12a_usb2_init (which currently uses
reset_control_reset)?
then we could also use reset_control_assert in phy_meson_g12a_usb2_exit

> +       if (ret)
> +               return ret;
> +
> +       phy = devm_phy_create(dev, NULL, &phy_meson_g12a_usb2_ops);
> +       if (IS_ERR(phy)) {
> +               ret = PTR_ERR(phy);
> +               if (ret != -EPROBE_DEFER)
> +                       dev_err(dev, "failed to create PHY\n");
phy-core only returns EPROBE_DEFER in case the phy-supply is not ready yet.
we won't need a phy-supply (at least as far as I know, if we do then
please document this in the dt-bindings patch). for USB VBUS dwc2
support the "vbus-supply" property which we should use instead.
I leave it up to you to decide whether the check makes sense

> +               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_g12a_usb2_of_match[] = {
> +       { .compatible = "amlogic,g12a-usb2-phy", },
> +       { },
> +};
> +MODULE_DEVICE_TABLE(of, phy_meson_g12a_usb2_of_match);
> +
> +static struct platform_driver phy_meson_g12a_usb2_driver = {
> +       .probe  = phy_meson_g12a_usb2_probe,
> +       .driver = {
> +               .name           = "phy-meson-g12a-usb2",
> +               .of_match_table = phy_meson_g12a_usb2_of_match,
> +       },
> +};
> +module_platform_driver(phy_meson_g12a_usb2_driver);
> +
> +MODULE_AUTHOR("Martin Blumenstingl <martin.blumenstingl@googlemail.com>");
I haven't contributed any of the relevant code to this driver (I
assume you took one of our existing Amlogic PHY drivers as skeleton?).
it's up to you whether you want to keep me in here


Regards
Martin

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

* Re: [PATCH 2/8] dt-bindings: phy: Add Amlogic G12A USB3+PCIE Combo PHY Bindings
  2019-02-17 22:03   ` Martin Blumenstingl
@ 2019-02-18 10:33     ` Neil Armstrong
  0 siblings, 0 replies; 25+ messages in thread
From: Neil Armstrong @ 2019-02-18 10:33 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: gregkh, hminas, balbi, kishon, devicetree, linux-amlogic,
	linux-usb, linux-kernel, linux-arm-kernel

On 17/02/2019 23:03, Martin Blumenstingl wrote:
> On Tue, Feb 12, 2019 at 4:15 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> Add the Amlogic G12A Family USB3 + PCIE Combo PHY Bindings.
>>
>> This PHY can provide exclusively USB3 or PCIE support on shared I/Os.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> one nit-pick below, but apart from that:
> Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> 
>> ---
>>  .../bindings/phy/meson-g12a-usb3-pcie-phy.txt | 25 +++++++++++++++++++
>>  1 file changed, 25 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/phy/meson-g12a-usb3-pcie-phy.txt
>>
>> diff --git a/Documentation/devicetree/bindings/phy/meson-g12a-usb3-pcie-phy.txt b/Documentation/devicetree/bindings/phy/meson-g12a-usb3-pcie-phy.txt
>> new file mode 100644
>> index 000000000000..714d751091f5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/meson-g12a-usb3-pcie-phy.txt
>> @@ -0,0 +1,25 @@
>> +* Amlogic G12A USB3 + PCIE Combo PHY binding
>> +
>> +Required properties:
>> +- compatible:  Should be "amlogic,meson-g12a-usb3-pcie-phy"
>> +- #phys-cells: must be 1. The cell number is used to select the phy mode
>> +  as defined in <dt-bindings/phy/phy.h> between PHY_TYPE_USB3 and PHY_TYPE_PCIE
>> +- reg:         The base address and length of the registers
>> +- clocks:      a phandle to the 100MHz reference clock of this PHY
>> +- clock-names: must be "ref_clk"
>> +- resets:      phandle to the reset lines for:
>> +               - the PHY control
>> +               - the USB3+PCIE PHY
>> +               - the PHY registers
> no reset-names (like in the G12A USB2 PHY bindings) here?
> even if you don't use them in the driver I suggest you add them for
> consistency (and maybe to make it easier to compare the bindings with
> the datasheet. I don't have access to the datasheet so I'm not sure if
> having the reset-names is relevant for this case)

You're right, it's better to have names here !

Neil

> 
> 
> Regards
> Martin
> 


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

* Re: [PATCH 5/8] phy: amlogic: add Amlogic G12A USB2 PHY Driver
  2019-02-17 22:24   ` Martin Blumenstingl
@ 2019-02-18 10:38     ` Neil Armstrong
  0 siblings, 0 replies; 25+ messages in thread
From: Neil Armstrong @ 2019-02-18 10:38 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: gregkh, hminas, balbi, kishon, linux-amlogic, linux-usb,
	linux-kernel, linux-arm-kernel

Hi Martin,

On 17/02/2019 23:24, Martin Blumenstingl wrote:
> Hi Neil,
> 
> On Tue, Feb 12, 2019 at 4:15 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> This adds support for the USB2 PHY found in the Amlogic G12A SoC Family.
>>
>> It supports Host and/or Peripheral mode, depending on it's position.
>> The first PHY is only used as Host, but the second supports Dual modes
>> defined by the USB Control Glue HW in front of the USB Controllers.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  drivers/phy/amlogic/Kconfig               |  12 ++
>>  drivers/phy/amlogic/Makefile              |   1 +
>>  drivers/phy/amlogic/phy-meson-g12a-usb2.c | 191 ++++++++++++++++++++++
>>  3 files changed, 204 insertions(+)
>>  create mode 100644 drivers/phy/amlogic/phy-meson-g12a-usb2.c
>>
>> diff --git a/drivers/phy/amlogic/Kconfig b/drivers/phy/amlogic/Kconfig
>> index 23fe1cda2f70..78d6e194dce9 100644
>> --- a/drivers/phy/amlogic/Kconfig
>> +++ b/drivers/phy/amlogic/Kconfig
>> @@ -36,3 +36,15 @@ config PHY_MESON_GXL_USB3
>>           Enable this to support the Meson USB3 PHY and OTG detection
>>           IP block found in Meson GXL and GXM SoCs.
>>           If unsure, say N.
>> +
>> +config PHY_MESON_G12A_USB2
>> +       tristate "Meson G12A USB2 PHY drivers"
> aw, there's a typo in the GXL driver names (from which this new
> Kconfig entry may have been copied): it should be "driver" instead of
> "drivers"

Will fix...

> 
>> +       default ARCH_MESON
>> +       depends on OF && (ARCH_MESON || COMPILE_TEST)
>> +       depends on USB_SUPPORT
> I don't think you need USB_SUPPORT (neither does PHY_MESON_GXL_USB2,
> but it seems that I accidentally left it in from an early development
> iteration), see below

Indeed, thanks for pointing this

> 
>> +       select GENERIC_PHY
>> +       select REGMAP_MMIO
>> +       help
>> +         Enable this to support the Meson USB2 PHYs found in Meson
>> +         G12A SoCs.
>> +         If unsure, say N.
>> diff --git a/drivers/phy/amlogic/Makefile b/drivers/phy/amlogic/Makefile
>> index 4fd8848c194d..7d4d10f5a6b3 100644
>> --- a/drivers/phy/amlogic/Makefile
>> +++ b/drivers/phy/amlogic/Makefile
>> @@ -1,3 +1,4 @@
>>  obj-$(CONFIG_PHY_MESON8B_USB2)         += phy-meson8b-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-g12a-usb2.c b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
>> new file mode 100644
>> index 000000000000..3b6271a8be02
>> --- /dev/null
>> +++ b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
>> @@ -0,0 +1,191 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Meson G12A USB2 PHY driver
>> + *
>> + * Copyright (C) 2017 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> + * Copyright (C) 2017 Amlogic, Inc. All rights reserved
>> + * Copyright (C) 2019 BayLibre, SAS
>> + * Author: Neil Armstrong <narmstrong@baylibre.com>
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/mfd/syscon.h>
> I don't see any syscon usage in the driver so you can drop this

Forgot to remove it

> 
>> +#include <linux/reset.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/usb/otg.h>
> do you need this? if you drop "depends on USB_SUPPORT" from Kconfig
> then you want to get rid of this as well

Indeed, no need to this

> 
>> +#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_R4                                            0x10
>> +#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_R14                                           0x38
>> +#define PHY_CTRL_R15                                           0x3c
>> +#define PHY_CTRL_R16                                           0x40
>> +#define PHY_CTRL_R17                                           0x44
>> +#define PHY_CTRL_R18                                           0x48
>> +#define PHY_CTRL_R19                                           0x4c
>> +#define PHY_CTRL_R20                                           0x50
>> +#define PHY_CTRL_R21                                           0x54
>> +#define PHY_CTRL_R22                                           0x58
>> +#define PHY_CTRL_R23                                           0x5c
>> +
>> +#define RESET_COMPLETE_TIME                                    1000
>> +#define PLL_RESET_COMPLETE_TIME                                        100
>> +
>> +struct phy_meson_g12a_usb2_priv {
>> +       struct device           *dev;
>> +       struct regmap           *regmap;
>> +       struct clk              *clk;
>> +       struct reset_control    *reset;
>> +};
>> +
>> +static const struct regmap_config phy_meson_g12a_usb2_regmap_conf = {
>> +       .reg_bits = 8,
>> +       .val_bits = 32,
>> +       .reg_stride = 4,
>> +       .max_register = PHY_CTRL_R23,
>> +};
>> +
>> +static int phy_meson_g12a_usb2_init(struct phy *phy)
>> +{
>> +       struct phy_meson_g12a_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, BIT(2), 0);
> do you have the name of the other register bits as well? having
> #defines would make the code easier to read

It's on my TODO list...

> 
>> +
>> +       /* PLL Setup : 24MHz * 20 / 1 = 480MHz */
>> +       regmap_write(priv->regmap, PHY_CTRL_R16, 0x39400414);
>> +       regmap_write(priv->regmap, PHY_CTRL_R17, 0x927e0000);
>> +       regmap_write(priv->regmap, PHY_CTRL_R18, 0xac5f49e5);
>> +
>> +       udelay(PLL_RESET_COMPLETE_TIME);
>> +
>> +       /* UnReset PLL */
>> +       regmap_write(priv->regmap, PHY_CTRL_R16, 0x19400414);
>> +
>> +       /* PHY Tuning */
>> +       regmap_write(priv->regmap, PHY_CTRL_R20, 0xfe18);
>> +       regmap_write(priv->regmap, PHY_CTRL_R4, 0x8000fff);
>> +
>> +       /* Tuning Disconnect Threshold */
>> +       regmap_write(priv->regmap, PHY_CTRL_R3, 0x34);
>> +
>> +       /* Analog Settings */
>> +       regmap_write(priv->regmap, PHY_CTRL_R14, 0);
>> +       regmap_write(priv->regmap, PHY_CTRL_R13, 0x78000);
>> +
>> +       return 0;
>> +}
>> +
>> +static int phy_meson_g12a_usb2_exit(struct phy *phy)
>> +{
>> +       struct phy_meson_g12a_usb2_priv *priv = phy_get_drvdata(phy);
>> +
>> +       return reset_control_reset(priv->reset);
> do you have information on whether we should reset_control_assert here
> instead of reset_control_reset?

No, I don't have it, I haven't fully validated the remove/suspend path yet,
but this should at least put the PHY in an unconfigured mode.

> 
>> +}
>> +
>> +/* set_mode is not needed, mode setting is handled via the UTMI bus */
>> +static const struct phy_ops phy_meson_g12a_usb2_ops = {
>> +       .init           = phy_meson_g12a_usb2_init,
>> +       .exit           = phy_meson_g12a_usb2_exit,
>> +       .owner          = THIS_MODULE,
>> +};
>> +
>> +static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
>> +{
>> +       struct device *dev = &pdev->dev;
>> +       struct phy_provider *phy_provider;
>> +       struct resource *res;
>> +       struct phy_meson_g12a_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);
>> +
>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +       base = devm_ioremap_resource(dev, res);
>> +       if (IS_ERR(base))
>> +               return PTR_ERR(base);
>> +
>> +       priv->regmap = devm_regmap_init_mmio(dev, base,
>> +                                            &phy_meson_g12a_usb2_regmap_conf);
>> +       if (IS_ERR(priv->regmap))
>> +               return PTR_ERR(priv->regmap);
>> +
>> +       priv->clk = devm_clk_get(dev, "xtal");
>> +       if (IS_ERR(priv->clk))
>> +               return PTR_ERR(priv->clk);
>> +
>> +       priv->reset = devm_reset_control_get(dev, "phy");
>> +       if (IS_ERR(priv->reset))
>> +               return PTR_ERR(priv->reset);
>> +
>> +       ret = reset_control_deassert(priv->reset);
> should we move this to phy_meson_g12a_usb2_init (which currently uses
> reset_control_reset)?
> then we could also use reset_control_assert in phy_meson_g12a_usb2_exit

Maybe it would be better, indeed.

> 
>> +       if (ret)
>> +               return ret;
>> +
>> +       phy = devm_phy_create(dev, NULL, &phy_meson_g12a_usb2_ops);
>> +       if (IS_ERR(phy)) {
>> +               ret = PTR_ERR(phy);
>> +               if (ret != -EPROBE_DEFER)
>> +                       dev_err(dev, "failed to create PHY\n");
> phy-core only returns EPROBE_DEFER in case the phy-supply is not ready yet.
> we won't need a phy-supply (at least as far as I know, if we do then
> please document this in the dt-bindings patch). for USB VBUS dwc2
> support the "vbus-supply" property which we should use instead.
> I leave it up to you to decide whether the check makes sense

It's always better to handle these return cases, better to leave it.

> 
>> +               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_g12a_usb2_of_match[] = {
>> +       { .compatible = "amlogic,g12a-usb2-phy", },
>> +       { },
>> +};
>> +MODULE_DEVICE_TABLE(of, phy_meson_g12a_usb2_of_match);
>> +
>> +static struct platform_driver phy_meson_g12a_usb2_driver = {
>> +       .probe  = phy_meson_g12a_usb2_probe,
>> +       .driver = {
>> +               .name           = "phy-meson-g12a-usb2",
>> +               .of_match_table = phy_meson_g12a_usb2_of_match,
>> +       },
>> +};
>> +module_platform_driver(phy_meson_g12a_usb2_driver);
>> +
>> +MODULE_AUTHOR("Martin Blumenstingl <martin.blumenstingl@googlemail.com>");
> I haven't contributed any of the relevant code to this driver (I
> assume you took one of our existing Amlogic PHY drivers as skeleton?).
> it's up to you whether you want to keep me in here

Yep it's for the skeleton ! No harm to keep you in !

> 
> 
> Regards
> Martin
> 

Thanks,
Neil


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

* Re: [PATCH 6/8] phy: amlogic: Add Amlogic G12A USB3 + PCIE Combo PHY Driver
  2019-02-12 15:14 ` [PATCH 6/8] phy: amlogic: Add Amlogic G12A USB3 + PCIE Combo " Neil Armstrong
@ 2019-02-24 19:40   ` Martin Blumenstingl
  0 siblings, 0 replies; 25+ messages in thread
From: Martin Blumenstingl @ 2019-02-24 19:40 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: gregkh, hminas, balbi, kishon, linux-amlogic, linux-usb,
	linux-kernel, linux-arm-kernel

Hi Neil,

On Tue, Feb 12, 2019 at 4:16 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> This adds support for the shared USB3 + PCIE PHY found in the
> Amlogic G12A SoC Family.
>
> It supports USB3 Host mode or PCIE 2.0 mode, depending on the layout of
> the board.
>
> Selection is done by the #phy-cells, making the mode static and exclusive.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/phy/amlogic/Kconfig                   |  12 +
>  drivers/phy/amlogic/Makefile                  |   1 +
>  .../phy/amlogic/phy-meson-g12a-usb3-pcie.c    | 414 ++++++++++++++++++
>  3 files changed, 427 insertions(+)
>  create mode 100644 drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
>
> diff --git a/drivers/phy/amlogic/Kconfig b/drivers/phy/amlogic/Kconfig
> index 78d6e194dce9..7ccb9a756aba 100644
> --- a/drivers/phy/amlogic/Kconfig
> +++ b/drivers/phy/amlogic/Kconfig
> @@ -48,3 +48,15 @@ config PHY_MESON_G12A_USB2
>           Enable this to support the Meson USB2 PHYs found in Meson
>           G12A SoCs.
>           If unsure, say N.
> +
> +config PHY_MESON_G12A_USB3_PCIE
> +       tristate "Meson G12A USB3+PCIE Combo PHY drivers"
nit-pick: s/drivers/driver/

> +       default ARCH_MESON
> +       depends on OF && (ARCH_MESON || COMPILE_TEST)
> +       depends on USB_SUPPORT
> +       select GENERIC_PHY
> +       select REGMAP_MMIO
> +       help
> +         Enable this to support the Meson USB3 + PCIE Combi PHY found
> +         in Meson G12A SoCs.
> +         If unsure, say N.
> diff --git a/drivers/phy/amlogic/Makefile b/drivers/phy/amlogic/Makefile
> index 7d4d10f5a6b3..fdd008e1b19b 100644
> --- a/drivers/phy/amlogic/Makefile
> +++ b/drivers/phy/amlogic/Makefile
> @@ -2,3 +2,4 @@ obj-$(CONFIG_PHY_MESON8B_USB2)          += phy-meson8b-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
> +obj-$(CONFIG_PHY_MESON_G12A_USB3_PCIE) += phy-meson-g12a-usb3-pcie.o
> diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c b/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
> new file mode 100644
> index 000000000000..59eae98928e9
> --- /dev/null
> +++ b/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
> @@ -0,0 +1,414 @@
[...]
> +static int phy_g12a_usb3_init(struct phy *phy)
> +{
> +       struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
> +       int data, ret;
> +
> +       /* Switch PHY to USB3 */
> +       regmap_update_bits(priv->regmap, PHY_R0,
> +                          PHY_R0_PCIE_USB3_SWITCH,
> +                          PHY_R0_PCIE_USB3_SWITCH);
> +
> +       /*
> +        * WORKAROUND: There is SSPHY suspend bug due to
> +        * which USB enumerates
> +        * in HS mode instead of SS mode. Workaround it by asserting
> +        * LANE0.TX_ALT_BLOCK.EN_ALT_BUS to enable TX to use alt bus
> +        * mode
> +        */
> +       ret = regmap_update_bits(priv->regmap_cr, 0x102d, BIT(7), BIT(7));
does the datasheet have names for these registers / bits? it if does
then it would be great if you could introduce #defines for them

> +       if (ret)
> +               return ret;
> +
> +       ret = regmap_update_bits(priv->regmap_cr, 0x1010, 0xff0, 20);
> +       if (ret)
> +               return ret;
> +
> +       /*
> +        * Fix RX Equalization setting as follows
> +        * LANE0.RX_OVRD_IN_HI. RX_EQ_EN set to 0
> +        * LANE0.RX_OVRD_IN_HI.RX_EQ_EN_OVRD set to 1
> +        * LANE0.RX_OVRD_IN_HI.RX_EQ set to 3
> +        * LANE0.RX_OVRD_IN_HI.RX_EQ_OVRD set to 1
> +        */
> +       ret = regmap_read(priv->regmap_cr, 0x1006, &data);
> +       if (ret)
> +               return ret;
> +
> +       data &= ~BIT(6);
> +       data |= BIT(7);
> +       data &= ~(0x7 << 8);
> +       data |= (0x3 << 8);
> +       data |= (1 << 11);
> +       ret = regmap_write(priv->regmap_cr, 0x1006, data);
> +       if (ret)
> +               return ret;
> +
> +       /*
> +        * Set EQ and TX launch amplitudes as follows
> +        * LANE0.TX_OVRD_DRV_LO.PREEMPH set to 22
> +        * LANE0.TX_OVRD_DRV_LO.AMPLITUDE set to 127
> +        * LANE0.TX_OVRD_DRV_LO.EN set to 1.
> +        */
> +       ret = regmap_read(priv->regmap_cr, 0x1002, &data);
> +       if (ret)
> +               return ret;
> +
> +       data &= ~0x3f80;
> +       data |= (0x16 << 7);
> +       data &= ~0x7f;
> +       data |= (0x7f | BIT(14));
> +       ret = regmap_write(priv->regmap_cr, 0x1002, data);
> +       if (ret)
> +               return ret;
> +
> +       /*
> +        * MPLL_LOOP_CTL.PROP_CNTRL = 8
> +        */
why a multi-line comment here? "Switch PHY to USB3" above uses a
single-line comment

> +       ret = regmap_update_bits(priv->regmap_cr, 0x30, 0xf << 4, 8 << 4);
> +       if (ret)
> +               return ret;
> +
> +       regmap_update_bits(priv->regmap, PHY_R2,
> +                       PHY_R2_PHY_TX_VBOOST_LVL,
> +                       FIELD_PREP(PHY_R2_PHY_TX_VBOOST_LVL, 0x4));
> +
> +       regmap_update_bits(priv->regmap, PHY_R1,
> +                       PHY_R1_PHY_LOS_BIAS | PHY_R1_PHY_LOS_LEVEL,
> +                       FIELD_PREP(PHY_R1_PHY_LOS_BIAS, 4) |
> +                       FIELD_PREP(PHY_R1_PHY_LOS_LEVEL, 9));
> +
> +       return 0;
> +}
> +
> +static int phy_g12a_usb3_pcie_init(struct phy *phy)
> +{
> +       struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
> +       int ret;
> +
> +       ret = reset_control_reset(priv->reset);
> +       if (ret)
> +               return ret;
> +
> +       if (priv->mode == PHY_TYPE_USB3)
> +               return phy_g12a_usb3_init(phy);
> +
> +       /* Power UP PCIE */
> +       regmap_update_bits(priv->regmap, PHY_R0,
> +                          PHY_R0_PCIE_POWER_STATE,
> +                          FIELD_PREP(PHY_R0_PCIE_POWER_STATE, 0x1c));
do we also need this for USB mode?
also do we need to change the PHY_R2 register values
(PHY_R2_PHY_TX_VBOOST_LVL for example) for PCIe, for example if the
bootloader initialized the PHY in USB3 mode while the board actually
exposes a PCIe port?


Regards
Martin

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

* Re: [PATCH 4/8] dt-bindings: usb: dwc3: Add Amlogic G12A DWC3 Glue Bindings
  2019-02-12 15:14 ` [PATCH 4/8] dt-bindings: usb: dwc3: Add Amlogic G12A DWC3 Glue Bindings Neil Armstrong
@ 2019-02-24 19:52   ` Martin Blumenstingl
  2019-02-28 16:29   ` Rob Herring
  1 sibling, 0 replies; 25+ messages in thread
From: Martin Blumenstingl @ 2019-02-24 19:52 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: gregkh, hminas, balbi, kishon, devicetree, linux-amlogic,
	linux-usb, linux-kernel, linux-arm-kernel

Hi Neil,

On Tue, Feb 12, 2019 at 4:15 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
[...]
> +
> +Example device nodes:
> +       usb: usb@ffe09000 {
> +                       compatible = "amlogic,meson-g12a-usb-ctrl";
> +                       reg = <0x0 0xffe09000 0x0 0xa0>;
> +                       interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
> +                       #address-cells = <2>;
> +                       #size-cells = <2>;
> +                       ranges;
> +
> +                       clocks = <&clkc CLKID_USB>;
> +                       clock-names = "usb";
> +                       resets = <&reset RESET_USB>;
> +                       reset-names = "usb";
> +
> +                       ports {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +
> +                               /* USB2 Port 0 */
> +                               usb20: port@0 {
> +                                       reg = <0>;
> +                                       phys = <&usb2_phy0>;
> +                               };
> +
> +                               /* USB2 Port 1 */
> +                               usb21: port@1 {
> +                                       reg = <1>;
> +                                       phys = <&usb2_phy1>;
> +                               };
> +
> +                               /* USB3 Port 0 */
> +                               usb3: port@4 {
> +                                       reg = <4>;
> +                                       phys = <&usb3_pcie_phy PHY_TYPE_USB3>;
> +                               };
> +                       };
> +
> +                       dwc2: usb@ff400000 {
> +                               compatible = "amlogic,meson-g12a-usb", "snps,dwc2";
> +                               reg = <0x0 0xff400000 0x0 0x40000>;
> +                               interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
> +                               clocks = <&clkc CLKID_USB1_DDR_BRIDGE>;
> +                               clock-names = "ddr";
> +                               dr_mode = "peripheral";
> +                               g-rx-fifo-size = <192>;
> +                               g-np-tx-fifo-size = <128>;
> +                               g-tx-fifo-size = <128 128 16 16 16>;
> +                       };
you suggested (off-list) that the OTG capable PHY should be passed to
the dwc2 instance
I'm aware that this is an example - could you please still add it for
consistency?

> +
> +                       dwc3: dwc3@ff500000 {
> +                               compatible = "snps,dwc3";
> +                               reg = <0x0 0xff500000 0x0 0x100000>;
> +                               interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
> +                               dr_mode = "host";
> +                               snps,dis_u2_susphy_quirk;
> +                               snps,quirk-frame-length-adjustment;
> +                       };
maybe the phys should also be passed to the dwc3 instance?


Regards
Martin

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

* Re: [PATCH 8/8] usb: dwc3: Add Amlogic G12A DWC3 glue
  2019-02-12 15:14 ` [PATCH 8/8] usb: dwc3: Add Amlogic G12A DWC3 glue Neil Armstrong
@ 2019-02-24 20:40   ` Martin Blumenstingl
  2019-03-02 10:29     ` Martin Blumenstingl
  0 siblings, 1 reply; 25+ messages in thread
From: Martin Blumenstingl @ 2019-02-24 20:40 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: gregkh, hminas, balbi, kishon, linux-amlogic, linux-usb,
	linux-kernel, linux-arm-kernel

Hi Neil,

thank you for working on this!
I have few questions and comments below, but overall it looks good :)

On Tue, Feb 12, 2019 at 4:17 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Adds support for Amlogic G12A USB Control Glue HW.
>
> The Amlogic G12A SoC Family embeds 2 USB Controllers :
> - a DWC3 IP configured as Host for USB2 and USB3
> - a DWC2 IP configured as Peripheral USB2 Only
>
> A glue connects these both controllers to 2 USB2 PHYs, and optionnally
> to an USB3+PCIE Combo PHY shared with the PCIE controller.
>
> The Glue configures the UTMI 8bit interfaces for the USB2 PHYs, including
> routing of the OTG PHY between the DWC3 and DWC2 controllers, and
> setups the on-chip OTG mode selection for this PHY.
>
> The PHYs are childen of the Glue node since the Glue controls the interface
> with the PHY, not the DWC3 controller.
my initial impression is: "if we pass the PHYs [via device-tree] to
the dwc2 and dwc3 controllers then we can simplify the suspend
management here in the USBCTRL driver".
I assume there's a catch so it please add an explanation why it's not possible.

> The drivers collects the mode of each PHY and determine which PHY
> is to be routed between the DWC2 and DWC3 controllers.
>
> This drivers supports the on-probe setup of the OTG mode, and manually
> via a debugfs interface. The IRQ mode change detect is yet to be added
> in a future patchset, mainly due to lack of hardware to validate on.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/usb/dwc3/Kconfig           |   9 +
>  drivers/usb/dwc3/Makefile          |   1 +
>  drivers/usb/dwc3/dwc3-meson-g12a.c | 650 +++++++++++++++++++++++++++++
>  3 files changed, 660 insertions(+)
>  create mode 100644 drivers/usb/dwc3/dwc3-meson-g12a.c
>
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 1a0404fda596..4335e5e76bbb 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -93,6 +93,15 @@ config USB_DWC3_KEYSTONE
>           Support of USB2/3 functionality in TI Keystone2 platforms.
>           Say 'Y' or 'M' here if you have one such device
>
> +config USB_DWC3_MESON_G12A
> +       tristate "Amlogic Meson G12A Platforms"
> +       depends on OF && COMMON_CLK
> +       depends on ARCH_MESON || COMPILE_TEST
> +       default USB_DWC3
> +       help
> +         Support USB2/3 functionality in Amlogic G12A platforms.
> +        Say 'Y' or 'M' if you have one such device.
> +
>  config USB_DWC3_OF_SIMPLE
>         tristate "Generic OF Simple Glue Layer"
>         depends on OF && COMMON_CLK
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index 6e3ef6144e5d..ae86da0dc5bd 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_G12A)      += dwc3-meson-g12a.o
>  obj-$(CONFIG_USB_DWC3_OF_SIMPLE)       += dwc3-of-simple.o
>  obj-$(CONFIG_USB_DWC3_ST)              += dwc3-st.o
>  obj-$(CONFIG_USB_DWC3_QCOM)            += dwc3-qcom.o
> diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c
> new file mode 100644
> index 000000000000..abeff2d56b1d
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c
> @@ -0,0 +1,650 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * USB Glue for Amlogic G12A SoCs
> + *
> + * Copyright (c) 2019 BayLibre, SAS
> + * Author: Neil Armstrong <narmstrong@baylibre.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
> + * - SuperSpeed PHY can be enabled only if port is used
> + *
> + * TOFIX:
> + * - Add dynamic OTG switching with ID change interrupt
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/clk.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/reset.h>
> +#include <linux/of_graph.h>
> +#include <linux/phy/phy.h>
> +#include <linux/usb/otg.h>
> +#include <linux/debugfs.h>
> +
> +/* USB Glue Control Registers */
> +
> +#define USB_R0                                                 0x00
> +       #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                                                 0x04
> +       #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                                                 0x08
> +       #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                                                 0x0c
> +       #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                                                 0x10
> +       #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                                                 0x14
> +       #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)
> +
> +/* USB2 Ports Control Registers */
> +
> +#define U2P_R0                                                 0x0
> +       #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                                                 0x4
> +       #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)
> +
> +#define MAX_PHY                                                        5
> +
> +#define USB2_MAX_PHY                                           4
> +#define USB3_PHY                                               4
> +
> +struct dwc3_meson_g12a {
> +       struct device           *dev;
> +       struct regmap           *regmap;
> +       struct clk              *clk;
> +       struct reset_control    *reset;
> +       struct phy              *phys[5];
> +       enum usb_dr_mode        phy_modes[5];
use MAX_PHY instead of hardcoded 5 here

have you also considered moving the per-port values into a separate
struct struct dwc3_meson_g12a_port?
(I'll get back to this below)

> +       enum phy_mode           otg_phy_mode;
> +       unsigned int            usb2_ports;
> +       unsigned int            usb3_ports;
> +       struct dentry           *root;
> +};
> +
> +#define U2P_REG_SIZE                                           0x20
> +#define USB_REG_OFFSET                                         0x80
the USB_Rn registers are always accessed with USB_REG_OFFSET added to
their offset except in dwc3_meson_g12a_usb2_init.
can we add 0x80 to all USB_Rn #defines to avoid this?

> +
> +static void dwc3_meson_g12a_usb2_set_mode(struct dwc3_meson_g12a *priv,
> +                                         int i, enum usb_dr_mode mode)
> +{
> +       switch (mode) {
> +       case USB_DR_MODE_HOST:
> +       case USB_DR_MODE_OTG:
> +       case USB_DR_MODE_UNKNOWN:
> +               regmap_update_bits(priv->regmap, U2P_R0 + (U2P_REG_SIZE * i),
> +                               U2P_R0_HOST_DEVICE,
> +                               U2P_R0_HOST_DEVICE);
have you considered creating a separate regmap for each USB2 PHY
configuration register space?
that separate regmap could also be part of the per-port struct
(dwc3_meson_g12a_port)
the result might look like this:
               regmap_update_bits(priv->ports[i].regmap, U2P_R0,
                               U2P_R0_HOST_DEVICE,
                               U2P_R0_HOST_DEVICE);

> +               break;
> +
> +       case USB_DR_MODE_PERIPHERAL:
> +               regmap_update_bits(priv->regmap, U2P_R0 + (U2P_REG_SIZE * i),
> +                               U2P_R0_HOST_DEVICE, 0);
> +               break;
> +       }
> +}
> +
> +static int dwc3_meson_g12a_usb2_init(struct dwc3_meson_g12a *priv)
> +{
> +       enum usb_dr_mode id_mode;
> +       u32 val;
> +       int i;
> +
> +       /* Read ID current level */
> +       regmap_read(priv->regmap, USB_R5, &val);
do we need to add USB_REG_OFFSET to USB_R5 here or do we need to read
USB_R5 from the first USB2 PHY register space?
in case we need to express it as "USB_REG_OFFSET + USB_R5" then I have
two suggestions to avoid this kind of issues:
- either add 0x80 to all USB_Rn #defines and to remove USB_REG_OFFSET
- assuming you create a separate regmap for each USB2 PHY
configuration: have a separate regmap for the USB_Rn registers as well

> +       if (val & USB_R5_ID_DIG_CURR)
> +               id_mode = USB_DR_MODE_PERIPHERAL;
> +       else
> +               id_mode = USB_DR_MODE_HOST;
> +
> +       dev_info(priv->dev, "ID mode %s\n",
> +                id_mode ==  USB_DR_MODE_HOST ? "host" : "peripheral");
> +
> +       for (i = 0 ; i < USB2_MAX_PHY ; ++i) {
> +               if (!priv->phys[i])
> +                       continue;
> +
> +               regmap_update_bits(priv->regmap, U2P_R0 + (U2P_REG_SIZE * i),
> +                                  U2P_R0_POWER_ON_RESET,
> +                                  U2P_R0_POWER_ON_RESET);
> +
> +               if (priv->phy_modes[i] == USB_DR_MODE_PERIPHERAL ||
> +                   (priv->phy_modes[i] == USB_DR_MODE_OTG &&
> +                    id_mode == USB_DR_MODE_PERIPHERAL)) {
> +                       dwc3_meson_g12a_usb2_set_mode(priv, i,
> +                                                     USB_DR_MODE_PERIPHERAL);
> +
> +                       if (priv->phy_modes[i] == USB_DR_MODE_OTG)
> +                               priv->otg_phy_mode = PHY_MODE_USB_DEVICE;
> +               } else {
> +                       dwc3_meson_g12a_usb2_set_mode(priv, i,
> +                                                     USB_DR_MODE_HOST);
> +
> +                       if (priv->phy_modes[i] == USB_DR_MODE_OTG)
> +                               priv->otg_phy_mode = PHY_MODE_USB_HOST;
> +               }
> +
> +               regmap_update_bits(priv->regmap, U2P_R0 + (U2P_REG_SIZE * i),
> +                                  U2P_R0_POWER_ON_RESET, 0);
> +       }
> +
> +       return 0;
> +}
> +
> +static void dwc3_meson_g12a_usb3_init(struct dwc3_meson_g12a *priv)
> +{
> +       regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R3,
> +                       USB_R3_P30_SSC_RANGE_MASK |
> +                       USB_R3_P30_REF_SSP_EN,
> +                       USB_R3_P30_SSC_ENABLE |
> +                       FIELD_PREP(USB_R3_P30_SSC_RANGE_MASK, 2) |
> +                       USB_R3_P30_REF_SSP_EN);
> +       udelay(2);
> +
> +       regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R2,
> +                       USB_R2_P30_PCS_TX_DEEMPH_3P5DB_MASK,
> +                       FIELD_PREP(USB_R2_P30_PCS_TX_DEEMPH_3P5DB_MASK, 0x15));
> +
> +       regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R2,
> +                       USB_R2_P30_PCS_TX_DEEMPH_6DB_MASK,
> +                       FIELD_PREP(USB_R2_P30_PCS_TX_DEEMPH_6DB_MASK, 0x20));
> +
> +       udelay(2);
> +
> +       regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R1,
> +                       USB_R1_U3H_HOST_PORT_POWER_CONTROL_PRESENT,
> +                       USB_R1_U3H_HOST_PORT_POWER_CONTROL_PRESENT);
> +
> +       regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R1,
> +                       USB_R1_P30_PCS_TX_SWING_FULL_MASK,
> +                       FIELD_PREP(USB_R1_P30_PCS_TX_SWING_FULL_MASK, 127));
> +}
> +
> +static void dwc3_meson_g12a_usb_init_mode(struct dwc3_meson_g12a *priv,
> +                                         bool is_peripheral)
can you please make this consistent with dwc3_meson_g12a_usb2_set_mode
which takes enum usb_dr_mode instead of a bool?

also while reading the code the naming of
dwc3_meson_g12a_usb2_set_mode and dwc3_meson_g12a_usb_init_mode
confused me.
in retrospect dwc3_meson_g12a_usb2_set_mode seems good enough because
it indicates that it sets the mode for one one the USB2 ports (or
PHYs).
dwc3_meson_g12a_usb_init_mode however is not only used for
initialization but also for forcing the mode at runtime. I don't have
any suggestion for a better name yet

> +{
> +       if (is_peripheral) {
> +               regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R0,
> +                               USB_R0_U2D_ACT, USB_R0_U2D_ACT);
> +               regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R0,
> +                               USB_R0_U2D_SS_SCALEDOWN_MODE_MASK, 0);
> +               regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R4,
> +                               USB_R4_P21_SLEEP_M0, USB_R4_P21_SLEEP_M0);
> +       } else {
> +               regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R0,
> +                               USB_R0_U2D_ACT, 0);
> +               regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R4,
> +                               USB_R4_P21_SLEEP_M0, 0);
> +       }
> +}
> +
> +static int dwc3_meson_g12a_usb_init(struct dwc3_meson_g12a *priv)
> +{
> +       int ret;
> +
> +       ret = dwc3_meson_g12a_usb2_init(priv);
> +       if (ret)
> +               return ret;
> +
> +       regmap_update_bits(priv->regmap, USB_REG_OFFSET + 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_REG_OFFSET + USB_R5,
> +                       USB_R5_ID_DIG_EN_0,
> +                       USB_R5_ID_DIG_EN_0);
> +       regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R5,
> +                       USB_R5_ID_DIG_EN_1,
> +                       USB_R5_ID_DIG_EN_1);
> +       regmap_update_bits(priv->regmap, USB_REG_OFFSET + USB_R5,
> +                       USB_R5_ID_DIG_TH_MASK,
> +                       FIELD_PREP(USB_R5_ID_DIG_TH_MASK, 0xff));
> +
> +       /* If we have an actual SuperSpeed port, initialize it */
> +       if (priv->usb3_ports)
> +               dwc3_meson_g12a_usb3_init(priv);
> +
> +       dwc3_meson_g12a_usb_init_mode(priv,
> +                               (priv->otg_phy_mode == PHY_MODE_USB_DEVICE));
> +
> +       return 0;
> +}
> +
> +static const struct regmap_config phy_meson_g12a_usb3_regmap_conf = {
> +       .reg_bits = 8,
> +       .val_bits = 32,
> +       .reg_stride = 4,
> +       .max_register = USB_REG_OFFSET + USB_R5,
> +};
> +
> +static int dwc3_meson_g12a_get_phys(struct dwc3_meson_g12a *priv)
> +{
> +       struct device_node *port, *phy_node;
> +       struct of_phandle_args args;
> +       enum usb_dr_mode mode;
> +       const char *dr_mode;
> +       struct phy *phy;
> +       int ret, i;
> +
> +       for (i = 0 ; i < MAX_PHY ; ++i) {
> +               port = of_graph_get_port_by_id(priv->dev->of_node, i);
> +
> +               /* Ignore port if not defined or disabled */
> +               if (!of_device_is_available(port)) {
> +                       of_node_put(port);
> +                       continue;
> +               }
> +
> +               /* Get associated PHY */
> +               phy = of_phy_get(port, NULL);
> +               if (IS_ERR(phy)) {
> +                       of_node_put(port);
> +                       ret = PTR_ERR(phy);
> +                       goto err_phy_get;
> +               }
> +
> +               of_node_put(port);
> +
> +               /* Get phy dr_mode */
> +               ret = of_parse_phandle_with_args(port, "phys", "#phy-cells",
> +                                                0, &args);
> +               if (ret) {
> +                       of_node_put(port);
> +                       goto err_phy_get;
> +               }
> +
> +               phy_node = args.np;
> +
> +               ret = of_property_read_string(phy_node, "dr_mode", &dr_mode);
> +               if (ret) {
> +                       dr_mode = "unknown";
> +                       mode = USB_DR_MODE_UNKNOWN;
> +               } else {
> +                       if (!strcmp(dr_mode, "host"))
> +                               mode = USB_DR_MODE_HOST;
> +                       else if (!strcmp(dr_mode, "otg"))
> +                               mode = USB_DR_MODE_OTG;
> +                       else if (!strcmp(dr_mode, "peripheral"))
> +                               mode = USB_DR_MODE_PERIPHERAL;
> +                       else {
> +                               mode = USB_DR_MODE_UNKNOWN;
> +                               dr_mode = "unknown";
> +                       }
> +               }
> +
> +               dev_info(priv->dev, "port%d: %s mode %s\n",
> +                        i, of_node_full_name(phy_node), dr_mode);
> +
> +               of_node_put(phy_node);
> +
> +               priv->phy_modes[i] = mode;
> +               priv->phys[i] = phy;
> +
> +               if (i == USB3_PHY)
> +                       priv->usb3_ports++;
> +               else
> +                       priv->usb2_ports++;
> +       }
> +
> +       dev_info(priv->dev, "usb2 ports: %d\n", priv->usb2_ports);
> +       dev_info(priv->dev, "usb3 ports: %d\n", priv->usb3_ports);
nit-pick: USB2 and USB3 (instead of the lower-case variants)?

> +
> +       return 0;
> +
> +err_phy_get:
> +       for (i = 0 ; i < MAX_PHY ; ++i)
> +               if (priv->phys[i])
phy_put is NULL-safe, so you can drop this check

> +                       phy_put(priv->phys[i]);
> +
> +       return ret;
> +}
> +
> +static int dwc3_meson_g12a_mode_force_get(void *data, u64 *val)
> +{
> +       struct dwc3_meson_g12a *priv = data;
> +
> +       if (priv->otg_phy_mode == PHY_MODE_USB_HOST)
> +               *val = 1;
> +       else if (priv->otg_phy_mode == PHY_MODE_USB_DEVICE)
> +               *val = 0;
> +       else
> +               return -EINVAL;
> +
> +       return 0;
> +}
> +
> +static int dwc3_meson_g12a_mode_force_set(void *data, u64 val)
> +{
> +       struct dwc3_meson_g12a *priv = data;
> +       int i;
> +
> +       if ((val && priv->otg_phy_mode == PHY_MODE_USB_HOST) ||
> +           (!val && priv->otg_phy_mode == PHY_MODE_USB_DEVICE))
> +               return 0;
> +
> +       for (i = 0 ; i < USB2_MAX_PHY ; ++i) {
> +               if (!priv->phys[i])
> +                       continue;
> +
> +               if (priv->phy_modes[i] != USB_DR_MODE_OTG)
> +                       continue;
> +
> +               if (val) {
> +                       dev_info(priv->dev, "switching to Host Mode\n");
> +
> +                       dwc3_meson_g12a_usb2_set_mode(priv, i,
> +                                                     USB_DR_MODE_HOST);
> +
> +                       dwc3_meson_g12a_usb_init_mode(priv, false);
> +
> +                       priv->otg_phy_mode = PHY_MODE_USB_HOST;
> +               } else {
> +                       dev_info(priv->dev, "switching to Device Mode\n");
> +
> +                       dwc3_meson_g12a_usb2_set_mode(priv, i,
> +                                                     USB_DR_MODE_PERIPHERAL);
> +
> +                       dwc3_meson_g12a_usb_init_mode(priv, true);
> +
> +                       priv->otg_phy_mode = PHY_MODE_USB_DEVICE;
> +               }
> +
> +               return phy_set_mode(priv->phys[i], priv->otg_phy_mode);
none of the G12A USB PHY drivers implements phy_ops.set_mode. do you
plan to change that? if not: what's the purpose of calling
phy_set_mode?

> +       }
> +
> +       return -EINVAL;
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(dwc3_meson_g12a_mode_force_fops,
> +                        dwc3_meson_g12a_mode_force_get,
> +                        dwc3_meson_g12a_mode_force_set, "%llu\n");
> +
> +static int dwc3_meson_g12a_otg_id_get(void *data, u64 *val)
> +{
> +       struct dwc3_meson_g12a *priv = data;
> +       u32 reg;
> +
> +       regmap_read(priv->regmap, USB_R5, &reg);
> +
> +       *val = (reg & USB_R5_ID_DIG_CURR);
"mode_force" returns 0 or 1 while this returns either 0 or 0x40 (bit 6).
can you please express it as "!!(reg & USB_R5_ID_DIG_CURR)" to make
"otg_id" also return 0 or 1?

> +
> +       return 0;
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(dwc3_meson_g12a_otg_id_fops,
> +                        dwc3_meson_g12a_otg_id_get, NULL, "%llu\n");
> +
> +/* We provide a DebugFS interface to get the ID value and force OTG switch */
at first I was curious if having one debugfs attribute for the whole
USBCTL registers is enough.
my thought was that we may have to expose one attribute per port for
example if G12A uses port 1 for OTG, but G12B uses port 2 (this is a
theoretical case, I have no documentation that this will happen).
only later I noticed that the code already supports this by looking up
the PHY with USB_DR_MODE_OTG set.
so a single attribute (together with the dr_mode configuration in the
.dts) covers our requirements.

> +static int dwc3_meson_g12a_debugfs_init(struct dwc3_meson_g12a *priv)
> +{
> +       priv->root = debugfs_create_dir("dwc3-meson-g12a", NULL);
> +       if (IS_ERR(priv->root))
> +               return PTR_ERR(priv->root);
> +
> +       debugfs_create_file("mode_force", 0600, priv->root, priv,
> +                           &dwc3_meson_g12a_mode_force_fops);
> +
> +       debugfs_create_file("otg_id", 0400, priv->root, priv,
> +                           &dwc3_meson_g12a_otg_id_fops);
> +
> +       return 0;
> +}
> +
> +static int dwc3_meson_g12a_probe(struct platform_device *pdev)
> +{
> +       struct dwc3_meson_g12a  *priv;
> +       struct device           *dev = &pdev->dev;
> +       struct device_node      *np = dev->of_node;
> +       void __iomem *base;
> +       struct resource *res;
> +       int ret, i;
> +
> +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +       if (!priv)
> +               return -ENOMEM;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       base = devm_ioremap_resource(dev, res);
> +       if (IS_ERR(base))
> +               return PTR_ERR(base);
> +
> +       priv->regmap = devm_regmap_init_mmio(dev, base,
> +                                            &phy_meson_g12a_usb3_regmap_conf);
> +       if (IS_ERR(priv->regmap))
> +               return PTR_ERR(priv->regmap);
> +
> +       priv->clk = devm_clk_get(dev, "usb");
> +       if (IS_ERR(priv->clk))
> +               return PTR_ERR(priv->clk);
> +
> +       ret = clk_prepare_enable(priv->clk);
> +       if (ret)
> +               return ret;
> +
> +       platform_set_drvdata(pdev, priv);
> +       priv->dev = dev;
> +
> +       priv->reset = devm_reset_control_get(dev, "usb");
> +       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_g12a_get_phys(priv);
> +       if (ret)
> +               return ret;
> +
> +       dwc3_meson_g12a_usb_init(priv);
> +
> +       /* Init PHYs */
> +       for (i = 0 ; i < MAX_PHY ; ++i) {
> +               if (priv->phys[i]) {
> +                       ret = phy_init(priv->phys[i]);
> +                       if (ret)
> +                               goto err_phys_put;
> +               }
> +       }
> +
> +       /* Set OTG PHY mode */
none of this PHY drivers implements phy_ops.set_mode.
dwc3_meson_g12a_usb_init() above should have taken care of
initializing the mode already if I understand the code correctly
can you please check if the following block is needed:
> +       for (i = 0 ; i < MAX_PHY ; ++i) {
> +               if (priv->phys[i] && priv->phy_modes[i] == USB_DR_MODE_OTG) {
> +                       ret = phy_set_mode(priv->phys[i], priv->otg_phy_mode);
> +                       if (ret)
> +                               goto err_phys_put;
> +               }
> +       }
> +
> +       ret = of_platform_populate(np, NULL, NULL, dev);
> +       if (ret) {
> +               clk_disable_unprepare(priv->clk);
> +               clk_put(priv->clk);
> +
> +               goto err_phys_exit;
> +       }
> +
> +       pm_runtime_set_active(dev);
> +       pm_runtime_enable(dev);
> +       pm_runtime_get_sync(dev);
> +
> +       if (dwc3_meson_g12a_debugfs_init(priv))
> +               dev_dbg(dev, "Failed to add DebugFS interface\n");
> +
> +       return 0;
> +
> +err_phys_exit:
> +       for (i = 0 ; i < MAX_PHY ; ++i)
> +               if (priv->phys[i])
phy_exit is NULL-safe, so you can drop this check

> +                       phy_exit(priv->phys[i]);
> +
> +err_phys_put:
> +       for (i = 0 ; i < MAX_PHY ; ++i)
> +               if (priv->phys[i])
phy_put is NULL-safe so you can drop this check

> +                       phy_put(priv->phys[i]);
> +
> +       return ret;
> +}
> +
> +static int dwc3_meson_g12a_remove(struct platform_device *pdev)
> +{
> +       struct dwc3_meson_g12a *priv = platform_get_drvdata(pdev);
> +       struct device *dev = &pdev->dev;
> +       int i;
> +
> +       debugfs_remove_recursive(priv->root);
> +
> +       of_platform_depopulate(dev);
> +
> +       for (i = 0 ; i < MAX_PHY ; ++i)
> +               if (priv->phys[i])
phy_exit is NULL-safe, so you can drop this check

> +                       phy_exit(priv->phys[i]);
> +
> +       for (i = 0 ; i < MAX_PHY ; ++i)
> +               if (priv->phys[i])
phy_put is NULL-safe, so you can drop this check

> +                       phy_put(priv->phys[i]);
> +
> +       clk_disable_unprepare(priv->clk);
> +       clk_put(priv->clk);
the clock is obtained with devm_clk_get - doesn't that automatically
call clk_put upon removal?

> +
> +       pm_runtime_disable(dev);
> +       pm_runtime_put_noidle(dev);
> +       pm_runtime_set_suspended(dev);
(I'm new to the runtime PM logic, so the following questions might not
make sense)
will this also call dwc3_meson_g12a_runtime_suspend (defined below)?
if so: how do we ensure that we don't call clk_disable if we already
called clk_disable_unprepare?

> +       return 0;
> +}
> +
> +static int __maybe_unused dwc3_meson_g12a_runtime_suspend(struct device *dev)
> +{
> +       struct dwc3_meson_g12a  *priv = dev_get_drvdata(dev);
> +
> +       clk_disable(priv->clk);
> +
> +       return 0;
> +}
> +
> +static int __maybe_unused dwc3_meson_g12a_runtime_resume(struct device *dev)
> +{
> +       struct dwc3_meson_g12a  *priv = dev_get_drvdata(dev);
> +
> +       return clk_enable(priv->clk);
> +}
> +
> +static int __maybe_unused dwc3_meson_g12a_suspend(struct device *dev)
> +{
> +       struct dwc3_meson_g12a *priv = dev_get_drvdata(dev);
> +       int i;
> +
> +       for (i = 0 ; i < MAX_PHY ; ++i)
> +               if (priv->phys[i])
phy_exit is NULL-safe, so you can drop this check

> +                       phy_exit(priv->phys[i]);
> +
> +       reset_control_assert(priv->reset);
> +
> +       return 0;
> +}
> +
> +static int __maybe_unused dwc3_meson_g12a_resume(struct device *dev)
> +{
> +       struct dwc3_meson_g12a *priv = dev_get_drvdata(dev);
> +       int i, ret;
> +
> +       reset_control_deassert(priv->reset);
> +
> +       dwc3_meson_g12a_usb_init(priv);
> +
> +       /* Init PHYs */
> +       for (i = 0 ; i < MAX_PHY ; ++i) {
> +               if (priv->phys[i]) {
phy_init is NULL-safe, so you can drop this check
> +                       ret = phy_init(priv->phys[i]);
> +                       if (ret)
> +                               return ret;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct dev_pm_ops dwc3_meson_g12a_dev_pm_ops = {
> +       SET_SYSTEM_SLEEP_PM_OPS(dwc3_meson_g12a_suspend, dwc3_meson_g12a_resume)
> +       SET_RUNTIME_PM_OPS(dwc3_meson_g12a_runtime_suspend,
> +                       dwc3_meson_g12a_runtime_resume, NULL)
> +};
> +
> +static const struct of_device_id dwc3_meson_g12a_match[] = {
> +       { .compatible = "amlogic,meson-g12a-usb-ctrl" },
> +       { /* Sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, dwc3_meson_g12a_match);
> +
> +static struct platform_driver dwc3_meson_g12a_driver = {
> +       .probe          = dwc3_meson_g12a_probe,
> +       .remove         = dwc3_meson_g12a_remove,
> +       .driver         = {
> +               .name   = "dwc3-meson-g12a",
> +               .of_match_table = dwc3_meson_g12a_match,
> +               .pm     = &dwc3_meson_g12a_dev_pm_ops,
> +       },
> +};
> +
> +module_platform_driver(dwc3_meson_g12a_driver);
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Amlogic Meson G12A USB Glue Layer");
> +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
> --
> 2.20.1
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic


Regards

Martin

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

* Re: [PATCH 1/8] dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings
  2019-02-12 15:14 ` [PATCH 1/8] dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings Neil Armstrong
  2019-02-17 21:58   ` Martin Blumenstingl
@ 2019-02-28 15:18   ` Rob Herring
  1 sibling, 0 replies; 25+ messages in thread
From: Rob Herring @ 2019-02-28 15:18 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: gregkh, hminas, balbi, kishon, devicetree, Neil Armstrong,
	linux-amlogic, linux-usb, linux-arm-kernel, linux-kernel

On Tue, 12 Feb 2019 16:14:06 +0100, Neil Armstrong wrote:
> Add the Amlogic G12A Family USB2 OTG PHY Bindings
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  .../bindings/phy/meson-g12a-usb2-phy.txt      | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/meson-g12a-usb2-phy.txt
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH 3/8] dt-bindings: usb: dwc2: Add Amlogic G12A DWC2 Compatible
  2019-02-12 15:14 ` [PATCH 3/8] dt-bindings: usb: dwc2: Add Amlogic G12A DWC2 Compatible Neil Armstrong
  2019-02-17 22:07   ` Martin Blumenstingl
@ 2019-02-28 16:14   ` Rob Herring
  1 sibling, 0 replies; 25+ messages in thread
From: Rob Herring @ 2019-02-28 16:14 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: gregkh, hminas, balbi, kishon, devicetree, Neil Armstrong,
	linux-amlogic, linux-usb, linux-arm-kernel, linux-kernel

On Tue, 12 Feb 2019 16:14:08 +0100, Neil Armstrong wrote:
> Adds the specific compatible string for the DWC2 IP found in the
> Amlogic G12A SoC Family.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
>  1 file changed, 1 insertion(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH 4/8] dt-bindings: usb: dwc3: Add Amlogic G12A DWC3 Glue Bindings
  2019-02-12 15:14 ` [PATCH 4/8] dt-bindings: usb: dwc3: Add Amlogic G12A DWC3 Glue Bindings Neil Armstrong
  2019-02-24 19:52   ` Martin Blumenstingl
@ 2019-02-28 16:29   ` Rob Herring
  2019-03-01 10:25     ` Neil Armstrong
  1 sibling, 1 reply; 25+ messages in thread
From: Rob Herring @ 2019-02-28 16:29 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: gregkh, hminas, balbi, kishon, devicetree, linux-amlogic,
	linux-usb, linux-arm-kernel, linux-kernel

On Tue, Feb 12, 2019 at 04:14:09PM +0100, Neil Armstrong wrote:
> Adds the bindings for the Amlogic G12A USB Glue HW.
> 
> The Amlogic G12A SoC Family embeds 2 USB Controllers :
> - a DWC3 IP configured as Host for USB2 and USB3
> - a DWC2 IP configured as Peripheral USB2 Only
> 
> A glue connects these both controllers to 2 USB2 PHYs,
> and optionnally to an USB3+PCIE Combo PHY shared with the PCIE controller.
> 
> The Glue configures the UTMI 8bit interfaces for the USB2 PHYs, including
> routing of the OTG PHY between the DWC3 and DWC2 controllers, and
> setups the on-chip OTG mode selection for this PHY.
> 
> The PHYs are children of the Glue node since the Glue controls the interface
> with the PHY, not the DWC3 controller.
> 
> The PHY interconnect is handled into ports subnodes, which eases describing
> which PHY is enabled (like the USB3 shared PHY) and futures layouts on
> derivatives of the G12A Family.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  .../devicetree/bindings/usb/amlogic,dwc3.txt  | 109 ++++++++++++++++++
>  1 file changed, 109 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt b/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
> index 9a8b631904fd..c7c4726ef10d 100644
> --- a/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
> +++ b/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
> @@ -40,3 +40,112 @@ Example device nodes:
>  				phy-names = "usb2-phy", "usb3-phy";
>  			};
>  		};
> +
> +Amlogic Meson G12A DWC3 USB SoC Controller Glue
> +
> +The Amlogic G12A embeds a DWC3 USB IP Core configured for USB2 and USB3
> +in host-only mode, and a DWC2 IP Core configured for USB2 peripheral mode
> +only.
> +
> +A glue connects the DWC3 core to USB2 PHYs and optionnaly to an USB3 PHY.
> +
> +One of the USB2 PHY can be re-routed in peripheral mode to a DWC2 USB IP.
> +
> +The DWC3 Glue controls the PHY routing and power, an interrupt line is
> +connected to the Glue to serve as OTG ID change detection.
> +
> +Required properties:
> +- compatible:	Should be "amlogic,meson-g12a-usb-ctrl"
> +- clocks:	a handle for the "USB" clock
> +- clock-names:	must be "usb"
> +- resets:	a handle for the shared "USB" reset line
> +- reset-names:	must be "usb"

-name for a single entry is pointless.

> +- reg:		The base address and length of the registers
> +- interrupts:	the interrupt specifier for the OTG detection
> +
> +Required child nodes:
> +
> +USB Ports are described as child 'port' nodes grouped under a 'ports' node,
> +with #address-cells, #size-cells specified.
> +
> +Each 'port' sub-node identifies a possible USB Port served by an USB PHY
> +identified by the 'phy' property as decribed in ../phy/phy-bindings.txt
> +
> +Each 'port' is identified by a reg property to number the port.
> +
> +The following table lists for each supported model the port number
> +corresponding to each PHY serving a physical USB Port.
> +
> + Family	   Port 0     Port 1    Port 2    Port 3    Port 4
> +---------------------------------------------------------------
> + G12A	   USBHOST_A  USBOTG_B  Reserved  Reserved  USB3_0
> +
> +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.
> +
> +A child node must exist to represent the core DWC2 IP block. The name of
> +the node is not important. The content of the node is defined in dwc2.txt.
> +
> +PHY documentation is provided in the following places:
> +- Documentation/devicetree/bindings/phy/meson-g12a-usb2-phy.txt
> +- Documentation/devicetree/bindings/phy/meson-g12a-usb3-pcie-phy.txt
> +
> +
> +Example device nodes:
> +	usb: usb@ffe09000 {
> +			compatible = "amlogic,meson-g12a-usb-ctrl";
> +			reg = <0x0 0xffe09000 0x0 0xa0>;
> +			interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;
> +
> +			clocks = <&clkc CLKID_USB>;
> +			clock-names = "usb";
> +			resets = <&reset RESET_USB>;
> +			reset-names = "usb";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				/* USB2 Port 0 */
> +				usb20: port@0 {
> +					reg = <0>;
> +					phys = <&usb2_phy0>;

'ports' and 'port' are reserved for the graph binding. Don't use it for 
your own thing.

Can't you just make 'phys' a list using 0 phandle if you need to skip 
entries.

> +				};
> +
> +				/* USB2 Port 1 */
> +				usb21: port@1 {
> +					reg = <1>;
> +					phys = <&usb2_phy1>;
> +				};
> +
> +				/* USB3 Port 0 */
> +				usb3: port@4 {
> +					reg = <4>;
> +					phys = <&usb3_pcie_phy PHY_TYPE_USB3>;
> +				};
> +			};
> +
> +			dwc2: usb@ff400000 {
> +				compatible = "amlogic,meson-g12a-usb", "snps,dwc2";
> +				reg = <0x0 0xff400000 0x0 0x40000>;
> +				interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clkc CLKID_USB1_DDR_BRIDGE>;
> +				clock-names = "ddr";
> +				dr_mode = "peripheral";
> +				g-rx-fifo-size = <192>;
> +				g-np-tx-fifo-size = <128>;
> +				g-tx-fifo-size = <128 128 16 16 16>;
> +			};
> +
> +			dwc3: dwc3@ff500000 {

usb@... or usb3@...

> +				compatible = "snps,dwc3";
> +				reg = <0x0 0xff500000 0x0 0x100000>;
> +				interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
> +				dr_mode = "host";
> +				snps,dis_u2_susphy_quirk;
> +				snps,quirk-frame-length-adjustment;
> +			};
> +	};
> -- 
> 2.20.1
> 

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

* Re: [PATCH 4/8] dt-bindings: usb: dwc3: Add Amlogic G12A DWC3 Glue Bindings
  2019-02-28 16:29   ` Rob Herring
@ 2019-03-01 10:25     ` Neil Armstrong
  0 siblings, 0 replies; 25+ messages in thread
From: Neil Armstrong @ 2019-03-01 10:25 UTC (permalink / raw)
  To: Rob Herring
  Cc: gregkh, hminas, balbi, kishon, devicetree, linux-amlogic,
	linux-usb, linux-arm-kernel, linux-kernel

On 28/02/2019 17:29, Rob Herring wrote:
> On Tue, Feb 12, 2019 at 04:14:09PM +0100, Neil Armstrong wrote:
>> Adds the bindings for the Amlogic G12A USB Glue HW.
>>
>> The Amlogic G12A SoC Family embeds 2 USB Controllers :
>> - a DWC3 IP configured as Host for USB2 and USB3
>> - a DWC2 IP configured as Peripheral USB2 Only
>>
>> A glue connects these both controllers to 2 USB2 PHYs,
>> and optionnally to an USB3+PCIE Combo PHY shared with the PCIE controller.
>>
>> The Glue configures the UTMI 8bit interfaces for the USB2 PHYs, including
>> routing of the OTG PHY between the DWC3 and DWC2 controllers, and
>> setups the on-chip OTG mode selection for this PHY.
>>
>> The PHYs are children of the Glue node since the Glue controls the interface
>> with the PHY, not the DWC3 controller.
>>
>> The PHY interconnect is handled into ports subnodes, which eases describing
>> which PHY is enabled (like the USB3 shared PHY) and futures layouts on
>> derivatives of the G12A Family.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  .../devicetree/bindings/usb/amlogic,dwc3.txt  | 109 ++++++++++++++++++
>>  1 file changed, 109 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt b/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
>> index 9a8b631904fd..c7c4726ef10d 100644
>> --- a/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
>> +++ b/Documentation/devicetree/bindings/usb/amlogic,dwc3.txt
>> @@ -40,3 +40,112 @@ Example device nodes:
>>  				phy-names = "usb2-phy", "usb3-phy";
>>  			};
>>  		};
>> +
>> +Amlogic Meson G12A DWC3 USB SoC Controller Glue
>> +
>> +The Amlogic G12A embeds a DWC3 USB IP Core configured for USB2 and USB3
>> +in host-only mode, and a DWC2 IP Core configured for USB2 peripheral mode
>> +only.
>> +
>> +A glue connects the DWC3 core to USB2 PHYs and optionnaly to an USB3 PHY.
>> +
>> +One of the USB2 PHY can be re-routed in peripheral mode to a DWC2 USB IP.
>> +
>> +The DWC3 Glue controls the PHY routing and power, an interrupt line is
>> +connected to the Glue to serve as OTG ID change detection.
>> +
>> +Required properties:
>> +- compatible:	Should be "amlogic,meson-g12a-usb-ctrl"
>> +- clocks:	a handle for the "USB" clock
>> +- clock-names:	must be "usb"
>> +- resets:	a handle for the shared "USB" reset line
>> +- reset-names:	must be "usb"
> 
> -name for a single entry is pointless.
> 
>> +- reg:		The base address and length of the registers
>> +- interrupts:	the interrupt specifier for the OTG detection
>> +
>> +Required child nodes:
>> +
>> +USB Ports are described as child 'port' nodes grouped under a 'ports' node,
>> +with #address-cells, #size-cells specified.
>> +
>> +Each 'port' sub-node identifies a possible USB Port served by an USB PHY
>> +identified by the 'phy' property as decribed in ../phy/phy-bindings.txt
>> +
>> +Each 'port' is identified by a reg property to number the port.
>> +
>> +The following table lists for each supported model the port number
>> +corresponding to each PHY serving a physical USB Port.
>> +
>> + Family	   Port 0     Port 1    Port 2    Port 3    Port 4
>> +---------------------------------------------------------------
>> + G12A	   USBHOST_A  USBOTG_B  Reserved  Reserved  USB3_0
>> +
>> +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.
>> +
>> +A child node must exist to represent the core DWC2 IP block. The name of
>> +the node is not important. The content of the node is defined in dwc2.txt.
>> +
>> +PHY documentation is provided in the following places:
>> +- Documentation/devicetree/bindings/phy/meson-g12a-usb2-phy.txt
>> +- Documentation/devicetree/bindings/phy/meson-g12a-usb3-pcie-phy.txt
>> +
>> +
>> +Example device nodes:
>> +	usb: usb@ffe09000 {
>> +			compatible = "amlogic,meson-g12a-usb-ctrl";
>> +			reg = <0x0 0xffe09000 0x0 0xa0>;
>> +			interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
>> +			#address-cells = <2>;
>> +			#size-cells = <2>;
>> +			ranges;
>> +
>> +			clocks = <&clkc CLKID_USB>;
>> +			clock-names = "usb";
>> +			resets = <&reset RESET_USB>;
>> +			reset-names = "usb";
>> +
>> +			ports {
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +
>> +				/* USB2 Port 0 */
>> +				usb20: port@0 {
>> +					reg = <0>;
>> +					phys = <&usb2_phy0>;
> 
> 'ports' and 'port' are reserved for the graph binding. Don't use it for 
> your own thing.
> 
> Can't you just make 'phys' a list using 0 phandle if you need to skip 
> entries.

Yep, finally this would be simpler.

> 
>> +				};
>> +
>> +				/* USB2 Port 1 */
>> +				usb21: port@1 {
>> +					reg = <1>;
>> +					phys = <&usb2_phy1>;
>> +				};
>> +
>> +				/* USB3 Port 0 */
>> +				usb3: port@4 {
>> +					reg = <4>;
>> +					phys = <&usb3_pcie_phy PHY_TYPE_USB3>;
>> +				};
>> +			};
>> +
>> +			dwc2: usb@ff400000 {
>> +				compatible = "amlogic,meson-g12a-usb", "snps,dwc2";
>> +				reg = <0x0 0xff400000 0x0 0x40000>;
>> +				interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
>> +				clocks = <&clkc CLKID_USB1_DDR_BRIDGE>;
>> +				clock-names = "ddr";
>> +				dr_mode = "peripheral";
>> +				g-rx-fifo-size = <192>;
>> +				g-np-tx-fifo-size = <128>;
>> +				g-tx-fifo-size = <128 128 16 16 16>;
>> +			};
>> +
>> +			dwc3: dwc3@ff500000 {
> 
> usb@... or usb3@...

Ok

> 
>> +				compatible = "snps,dwc3";
>> +				reg = <0x0 0xff500000 0x0 0x100000>;
>> +				interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
>> +				dr_mode = "host";
>> +				snps,dis_u2_susphy_quirk;
>> +				snps,quirk-frame-length-adjustment;
>> +			};
>> +	};
>> -- 
>> 2.20.1
>>


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

* Re: [PATCH 8/8] usb: dwc3: Add Amlogic G12A DWC3 glue
  2019-02-24 20:40   ` Martin Blumenstingl
@ 2019-03-02 10:29     ` Martin Blumenstingl
  2019-03-04  9:33       ` Neil Armstrong
  0 siblings, 1 reply; 25+ messages in thread
From: Martin Blumenstingl @ 2019-03-02 10:29 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: gregkh, hminas, balbi, kishon, linux-amlogic, linux-usb,
	linux-kernel, linux-arm-kernel

Hi Neil,

> > +static int dwc3_meson_g12a_debugfs_init(struct dwc3_meson_g12a *priv)
> > +{
> > +       priv->root = debugfs_create_dir("dwc3-meson-g12a", NULL);
> > +       if (IS_ERR(priv->root))
> > +               return PTR_ERR(priv->root);
> > +
> > +       debugfs_create_file("mode_force", 0600, priv->root, priv,
> > +                           &dwc3_meson_g12a_mode_force_fops);
> > +
> > +       debugfs_create_file("otg_id", 0400, priv->root, priv,
> > +                           &dwc3_meson_g12a_otg_id_fops);
> > +
> > +       return 0;
> > +}
I just stumbled across the "USB role switch framework", see [0]
it seems to provide a userspace API as well and two in-kernel drivers
are using this framework already
(drivers/usb/gadget/udc/renesas_usb3.c,
drivers/usb/roles/intel-xhci-usb-role-switch.c)


[0] https://elixir.bootlin.com/linux/v5.0-rc8/source/drivers/usb/roles/class.c#L246

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

* Re: [PATCH 8/8] usb: dwc3: Add Amlogic G12A DWC3 glue
  2019-03-02 10:29     ` Martin Blumenstingl
@ 2019-03-04  9:33       ` Neil Armstrong
  0 siblings, 0 replies; 25+ messages in thread
From: Neil Armstrong @ 2019-03-04  9:33 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: gregkh, hminas, balbi, kishon, linux-amlogic, linux-usb,
	linux-kernel, linux-arm-kernel

Hi,

On 02/03/2019 11:29, Martin Blumenstingl wrote:
> Hi Neil,
> 
>>> +static int dwc3_meson_g12a_debugfs_init(struct dwc3_meson_g12a *priv)
>>> +{
>>> +       priv->root = debugfs_create_dir("dwc3-meson-g12a", NULL);
>>> +       if (IS_ERR(priv->root))
>>> +               return PTR_ERR(priv->root);
>>> +
>>> +       debugfs_create_file("mode_force", 0600, priv->root, priv,
>>> +                           &dwc3_meson_g12a_mode_force_fops);
>>> +
>>> +       debugfs_create_file("otg_id", 0400, priv->root, priv,
>>> +                           &dwc3_meson_g12a_otg_id_fops);
>>> +
>>> +       return 0;
>>> +}
> I just stumbled across the "USB role switch framework", see [0]
> it seems to provide a userspace API as well and two in-kernel drivers
> are using this framework already
> (drivers/usb/gadget/udc/renesas_usb3.c,
> drivers/usb/roles/intel-xhci-usb-role-switch.c)

Just added support for it, works perfectly :-)

Neil

> 
> 
> [0] https://elixir.bootlin.com/linux/v5.0-rc8/source/drivers/usb/roles/class.c#L246
> 


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

* RE: [PATCH 7/8] usb: dwc2: Add Amlogic G12A DWC2 Params
  2019-02-12 15:14 ` [PATCH 7/8] usb: dwc2: Add Amlogic G12A DWC2 Params Neil Armstrong
@ 2019-04-09  9:31   ` Minas Harutyunyan
  0 siblings, 0 replies; 25+ messages in thread
From: Minas Harutyunyan @ 2019-04-09  9:31 UTC (permalink / raw)
  To: Neil Armstrong, gregkh, minas.harutyunyan, balbi, kishon
  Cc: linux-amlogic, linux-usb, linux-arm-kernel, linux-kernel

> This patchs sets the params for the DWC2 Controller found in the Amlogic G12A SoC family.
> 
> It mainly sets the settings reported incorrect by the driver, leaving the remaining detected automatically by the driver and provided by the DT node.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

Acked-by: Minas Harutyunyan <hminas@synopsys.com>

> ---
>  drivers/usb/dwc2/params.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c index 24ff5f21cb25..442113246cba 100644
> --- a/drivers/usb/dwc2/params.c
> +++ b/drivers/usb/dwc2/params.c
> @@ -121,6 +121,16 @@ static void dwc2_set_amlogic_params(struct dwc2_hsotg *hsotg)
>  	p->power_down = DWC2_POWER_DOWN_PARAM_NONE;  }
>  
> +static void dwc2_set_amlogic_g12a_params(struct dwc2_hsotg *hsotg) {
> +	struct dwc2_core_params *p = &hsotg->params;
> +
> +	p->lpm = false;
> +	p->lpm_clock_gating = false;
> +	p->besl = false;
> +	p->hird_threshold_en = false;
> +}
> +
>  static void dwc2_set_amcc_params(struct dwc2_hsotg *hsotg)  {
>  	struct dwc2_core_params *p = &hsotg->params; @@ -167,6 +177,8 @@ const struct of_device_id dwc2_of_match_table[] = {
>  	  .data = dwc2_set_amlogic_params },
>  	{ .compatible = "amlogic,meson-gxbb-usb",
>  	  .data = dwc2_set_amlogic_params },
> +	{ .compatible = "amlogic,meson-g12a-usb",
> +	  .data = dwc2_set_amlogic_g12a_params },
>  	{ .compatible = "amcc,dwc-otg", .data = dwc2_set_amcc_params },
>  	{ .compatible = "st,stm32f4x9-fsotg",
>  	  .data = dwc2_set_stm32f4x9_fsotg_params },
> --
> 2.20.1
>

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

end of thread, other threads:[~2019-04-09  9:31 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-12 15:14 [PATCH 0/8] arm64: meson: Add support for USB on Amlogic G12A Neil Armstrong
2019-02-12 15:14 ` [PATCH 1/8] dt-bindings: phy: Add Amlogic G12A USB2 PHY Bindings Neil Armstrong
2019-02-17 21:58   ` Martin Blumenstingl
2019-02-28 15:18   ` Rob Herring
2019-02-12 15:14 ` [PATCH 2/8] dt-bindings: phy: Add Amlogic G12A USB3+PCIE Combo " Neil Armstrong
2019-02-17 22:03   ` Martin Blumenstingl
2019-02-18 10:33     ` Neil Armstrong
2019-02-12 15:14 ` [PATCH 3/8] dt-bindings: usb: dwc2: Add Amlogic G12A DWC2 Compatible Neil Armstrong
2019-02-17 22:07   ` Martin Blumenstingl
2019-02-28 16:14   ` Rob Herring
2019-02-12 15:14 ` [PATCH 4/8] dt-bindings: usb: dwc3: Add Amlogic G12A DWC3 Glue Bindings Neil Armstrong
2019-02-24 19:52   ` Martin Blumenstingl
2019-02-28 16:29   ` Rob Herring
2019-03-01 10:25     ` Neil Armstrong
2019-02-12 15:14 ` [PATCH 5/8] phy: amlogic: add Amlogic G12A USB2 PHY Driver Neil Armstrong
2019-02-17 22:24   ` Martin Blumenstingl
2019-02-18 10:38     ` Neil Armstrong
2019-02-12 15:14 ` [PATCH 6/8] phy: amlogic: Add Amlogic G12A USB3 + PCIE Combo " Neil Armstrong
2019-02-24 19:40   ` Martin Blumenstingl
2019-02-12 15:14 ` [PATCH 7/8] usb: dwc2: Add Amlogic G12A DWC2 Params Neil Armstrong
2019-04-09  9:31   ` Minas Harutyunyan
2019-02-12 15:14 ` [PATCH 8/8] usb: dwc3: Add Amlogic G12A DWC3 glue Neil Armstrong
2019-02-24 20:40   ` Martin Blumenstingl
2019-03-02 10:29     ` Martin Blumenstingl
2019-03-04  9:33       ` Neil Armstrong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).