2023-04-18 11:34:11

by Dmitry Rokosov

[permalink] [raw]
Subject: [PATCH v2 0/5] arm64: meson: support Amlogic A1 USB OTG controller

This patch series introduces full support for the Amlogic A1 USB controller
in OTG mode (peripheral and host modes switching).

Previously, Amlogic's patch series [1] was applied to the upstream tree,
but it only had USB host mode support.
Furthermore, the device tree patchset [2] wasn't merged due to a missing
clk driver.
Patchset [2] has been completely reworked:
- changed register base offsets to proper values
- introduced dwc2 in peripheral mode
- OTG mode support
- the SoB of Amlogic authors still remain

Testing:
- USB OTG role switching between gadget and host - OK
- Peripheral mode - OK (tested with adb shell/push/pop)
- Host mode - OK (tested only USB enumeration and detection)

Changes v2 since v1 at [3]:
- as Martin suggested in v1, this commit completely removes
the 'otg_switch_supported' parameter from dwc3_meson_g12a_drvdata;
this parameter is no longer necessary as all IP versions now
support OTG switching
- this commit moves the USB PHY clkin enable/disable calls to
the PHY init/exit routines

Links:
[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
[3] https://lore.kernel.org/all/[email protected]/

Dmitry Rokosov (5):
phy: amlogic: enable/disable clkin during Amlogic USB PHY init/exit
usb: dwc2: support dwc2 IP for Amlogic A1 SoC family
dt-bindings: usb: dwc2: add support for Amlogic A1 SoC USB peripheral
usb: dwc3-meson-g12a: support OTG switch for all IP versions
arm64: dts: meson: a1: support USB controller in OTG mode

.../devicetree/bindings/usb/dwc2.yaml | 1 +
arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 59 +++++++++++++++++++
drivers/phy/amlogic/phy-meson-g12a-usb2.c | 13 +++-
drivers/usb/dwc2/params.c | 21 +++++++
drivers/usb/dwc3/dwc3-meson-g12a.c | 16 +----
5 files changed, 95 insertions(+), 15 deletions(-)

--
2.36.0


2023-04-18 11:34:33

by Dmitry Rokosov

[permalink] [raw]
Subject: [PATCH v2 4/5] usb: dwc3-meson-g12a: support OTG switch for all IP versions

From now, the Amlogic A1 USB controller is capable of switching between
host and gadget modes based on the status of the OTG_ID signal or via
manual USB role change.
Previously, only the Amlogic A1 IP version did not use OTG support for
host only mode, but this is no longer applicable.
Therefore, the 'otg_switch_supported' option can now be removed as
it is no longer required.

Signed-off-by: Dmitry Rokosov <[email protected]>
---
drivers/usb/dwc3/dwc3-meson-g12a.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c
index b282ad0e69c6..a13afdb219e8 100644
--- a/drivers/usb/dwc3/dwc3-meson-g12a.c
+++ b/drivers/usb/dwc3/dwc3-meson-g12a.c
@@ -140,7 +140,6 @@ static const char * const meson_a1_phy_names[] = {
struct dwc3_meson_g12a;

struct dwc3_meson_g12a_drvdata {
- bool otg_switch_supported;
bool otg_phy_host_port_disable;
struct clk_bulk_data *clks;
int num_clks;
@@ -189,7 +188,6 @@ static int dwc3_meson_gxl_usb_post_init(struct dwc3_meson_g12a *priv);
*/

static const struct dwc3_meson_g12a_drvdata gxl_drvdata = {
- .otg_switch_supported = true,
.otg_phy_host_port_disable = true,
.clks = meson_gxl_clocks,
.num_clks = ARRAY_SIZE(meson_g12a_clocks),
@@ -203,7 +201,6 @@ static const struct dwc3_meson_g12a_drvdata gxl_drvdata = {
};

static const struct dwc3_meson_g12a_drvdata gxm_drvdata = {
- .otg_switch_supported = true,
.otg_phy_host_port_disable = true,
.clks = meson_gxl_clocks,
.num_clks = ARRAY_SIZE(meson_g12a_clocks),
@@ -217,7 +214,6 @@ static const struct dwc3_meson_g12a_drvdata gxm_drvdata = {
};

static const struct dwc3_meson_g12a_drvdata axg_drvdata = {
- .otg_switch_supported = true,
.clks = meson_gxl_clocks,
.num_clks = ARRAY_SIZE(meson_gxl_clocks),
.phy_names = meson_a1_phy_names,
@@ -230,7 +226,6 @@ static const struct dwc3_meson_g12a_drvdata axg_drvdata = {
};

static const struct dwc3_meson_g12a_drvdata g12a_drvdata = {
- .otg_switch_supported = true,
.clks = meson_g12a_clocks,
.num_clks = ARRAY_SIZE(meson_g12a_clocks),
.phy_names = meson_g12a_phy_names,
@@ -242,7 +237,6 @@ static const struct dwc3_meson_g12a_drvdata g12a_drvdata = {
};

static const struct dwc3_meson_g12a_drvdata a1_drvdata = {
- .otg_switch_supported = false,
.clks = meson_a1_clocks,
.num_clks = ARRAY_SIZE(meson_a1_clocks),
.phy_names = meson_a1_phy_names,
@@ -307,7 +301,7 @@ static int dwc3_meson_g12a_usb2_init_phy(struct dwc3_meson_g12a *priv, int i,
U2P_R0_POWER_ON_RESET,
U2P_R0_POWER_ON_RESET);

- if (priv->drvdata->otg_switch_supported && i == USB2_OTG_PHY) {
+ if (i == USB2_OTG_PHY) {
regmap_update_bits(priv->u2p_regmap[i], U2P_R0,
U2P_R0_ID_PULLUP | U2P_R0_DRV_VBUS,
U2P_R0_ID_PULLUP | U2P_R0_DRV_VBUS);
@@ -490,7 +484,7 @@ static int dwc3_meson_g12a_otg_mode_set(struct dwc3_meson_g12a *priv,
{
int ret;

- if (!priv->drvdata->otg_switch_supported || !priv->phys[USB2_OTG_PHY])
+ if (!priv->phys[USB2_OTG_PHY])
return -EINVAL;

if (mode == PHY_MODE_USB_HOST)
@@ -589,9 +583,6 @@ static int dwc3_meson_g12a_otg_init(struct platform_device *pdev,
int ret, irq;
struct device *dev = &pdev->dev;

- if (!priv->drvdata->otg_switch_supported)
- return 0;
-
if (priv->otg_mode == USB_DR_MODE_OTG) {
/* Ack irq before registering */
regmap_update_bits(priv->usb_glue_regmap, USB_R5,
@@ -841,8 +832,7 @@ static int dwc3_meson_g12a_remove(struct platform_device *pdev)
struct device *dev = &pdev->dev;
int i;

- if (priv->drvdata->otg_switch_supported)
- usb_role_switch_unregister(priv->role_switch);
+ usb_role_switch_unregister(priv->role_switch);

of_platform_depopulate(dev);

--
2.36.0

2023-04-18 11:36:56

by Dmitry Rokosov

[permalink] [raw]
Subject: [PATCH v2 5/5] arm64: dts: meson: a1: support USB controller in OTG mode

Amlogic A1 SoC family has USB2.0 controller based on dwc2 and dwc3
heads. It supports otg/host/peripheral modes.

Signed-off-by: Yue Wang <[email protected]>
Signed-off-by: Hanjie Lin <[email protected]>
Signed-off-by: Dmitry Rokosov <[email protected]>
---
arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 59 +++++++++++++++++++++++
1 file changed, 59 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
index ae7d39cff07a..02af0aac6780 100644
--- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
@@ -8,6 +8,8 @@
#include <dt-bindings/gpio/meson-a1-gpio.h>
#include <dt-bindings/clock/amlogic,a1-pll-clkc.h>
#include <dt-bindings/clock/amlogic,a1-clkc.h>
+#include <dt-bindings/power/meson-a1-power.h>
+#include <dt-bindings/reset/amlogic,meson-a1-reset.h>

/ {
compatible = "amlogic,a1";
@@ -169,6 +171,17 @@ gpio_intc: interrupt-controller@0440 {
amlogic,channel-interrupts =
<49 50 51 52 53 54 55 56>;
};
+
+ usb2_phy1: phy@4000 {
+ compatible = "amlogic,a1-usb2-phy";
+ clocks = <&clkc CLKID_USB_PHY_IN>;
+ clock-names = "xtal";
+ reg = <0x0 0x4000 0x0 0x60>;
+ resets = <&reset RESET_USBPHY>;
+ reset-names = "phy";
+ #phy-cells = <0>;
+ power-domains = <&pwrc PWRC_USB_ID>;
+ };
};

gic: interrupt-controller@ff901000 {
@@ -192,6 +205,52 @@ spifc: spi@fd000400 {
#size-cells = <0>;
status = "disabled";
};
+
+ usb: usb@fe004400 {
+ status = "disabled";
+ compatible = "amlogic,meson-a1-usb-ctrl";
+ reg = <0x0 0xfe004400 0x0 0xa0>;
+ interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ clocks = <&clkc CLKID_USB_CTRL>,
+ <&clkc CLKID_USB_BUS>,
+ <&clkc CLKID_USB_CTRL_IN>;
+ clock-names = "usb_ctrl", "usb_bus", "xtal_usb_ctrl";
+ resets = <&reset RESET_USBCTRL>;
+ reset-name = "usb_ctrl";
+
+ dr_mode = "otg";
+
+ phys = <&usb2_phy1>;
+ phy-names = "usb2-phy1";
+
+ dwc2: usb@ff500000 {
+ compatible = "amlogic,meson-a1-usb", "snps,dwc2";
+ reg = <0x0 0xff500000 0x0 0x40000>;
+ interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
+ phys = <&usb2_phy1>;
+ phy-names = "usb2_phy";
+ clocks = <&clkc CLKID_USB_PHY>;
+ clock-names = "otg";
+ dr_mode = "peripheral";
+ g-rx-fifo-size = <192>;
+ g-np-tx-fifo-size = <128>;
+ g-tx-fifo-size = <128 128 16 16 16>;
+ };
+
+ dwc3: usb@ff400000 {
+ compatible = "snps,dwc3";
+ reg = <0x0 0xff400000 0x0 0x100000>;
+ interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
+ dr_mode = "host";
+ snps,dis_u2_susphy_quirk;
+ snps,quirk-frame-length-adjustment = <0x20>;
+ snps,parkmode-disable-ss-quirk;
+ };
+ };
};

timer {
--
2.36.0

2023-04-18 11:48:28

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] usb: dwc3-meson-g12a: support OTG switch for all IP versions

On 18/04/2023 13:16, Dmitry Rokosov wrote:
> From now, the Amlogic A1 USB controller is capable of switching between
> host and gadget modes based on the status of the OTG_ID signal or via
> manual USB role change.
> Previously, only the Amlogic A1 IP version did not use OTG support for
> host only mode, but this is no longer applicable.
> Therefore, the 'otg_switch_supported' option can now be removed as
> it is no longer required.
>
> Signed-off-by: Dmitry Rokosov <[email protected]>
> ---
> drivers/usb/dwc3/dwc3-meson-g12a.c | 16 +++-------------
> 1 file changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c
> index b282ad0e69c6..a13afdb219e8 100644
> --- a/drivers/usb/dwc3/dwc3-meson-g12a.c
> +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c
> @@ -140,7 +140,6 @@ static const char * const meson_a1_phy_names[] = {
> struct dwc3_meson_g12a;
>
> struct dwc3_meson_g12a_drvdata {
> - bool otg_switch_supported;
> bool otg_phy_host_port_disable;
> struct clk_bulk_data *clks;
> int num_clks;
> @@ -189,7 +188,6 @@ static int dwc3_meson_gxl_usb_post_init(struct dwc3_meson_g12a *priv);
> */
>
> static const struct dwc3_meson_g12a_drvdata gxl_drvdata = {
> - .otg_switch_supported = true,
> .otg_phy_host_port_disable = true,
> .clks = meson_gxl_clocks,
> .num_clks = ARRAY_SIZE(meson_g12a_clocks),
> @@ -203,7 +201,6 @@ static const struct dwc3_meson_g12a_drvdata gxl_drvdata = {
> };
>
> static const struct dwc3_meson_g12a_drvdata gxm_drvdata = {
> - .otg_switch_supported = true,
> .otg_phy_host_port_disable = true,
> .clks = meson_gxl_clocks,
> .num_clks = ARRAY_SIZE(meson_g12a_clocks),
> @@ -217,7 +214,6 @@ static const struct dwc3_meson_g12a_drvdata gxm_drvdata = {
> };
>
> static const struct dwc3_meson_g12a_drvdata axg_drvdata = {
> - .otg_switch_supported = true,
> .clks = meson_gxl_clocks,
> .num_clks = ARRAY_SIZE(meson_gxl_clocks),
> .phy_names = meson_a1_phy_names,
> @@ -230,7 +226,6 @@ static const struct dwc3_meson_g12a_drvdata axg_drvdata = {
> };
>
> static const struct dwc3_meson_g12a_drvdata g12a_drvdata = {
> - .otg_switch_supported = true,
> .clks = meson_g12a_clocks,
> .num_clks = ARRAY_SIZE(meson_g12a_clocks),
> .phy_names = meson_g12a_phy_names,
> @@ -242,7 +237,6 @@ static const struct dwc3_meson_g12a_drvdata g12a_drvdata = {
> };
>
> static const struct dwc3_meson_g12a_drvdata a1_drvdata = {
> - .otg_switch_supported = false,
> .clks = meson_a1_clocks,
> .num_clks = ARRAY_SIZE(meson_a1_clocks),
> .phy_names = meson_a1_phy_names,
> @@ -307,7 +301,7 @@ static int dwc3_meson_g12a_usb2_init_phy(struct dwc3_meson_g12a *priv, int i,
> U2P_R0_POWER_ON_RESET,
> U2P_R0_POWER_ON_RESET);
>
> - if (priv->drvdata->otg_switch_supported && i == USB2_OTG_PHY) {
> + if (i == USB2_OTG_PHY) {
> regmap_update_bits(priv->u2p_regmap[i], U2P_R0,
> U2P_R0_ID_PULLUP | U2P_R0_DRV_VBUS,
> U2P_R0_ID_PULLUP | U2P_R0_DRV_VBUS);
> @@ -490,7 +484,7 @@ static int dwc3_meson_g12a_otg_mode_set(struct dwc3_meson_g12a *priv,
> {
> int ret;
>
> - if (!priv->drvdata->otg_switch_supported || !priv->phys[USB2_OTG_PHY])
> + if (!priv->phys[USB2_OTG_PHY])
> return -EINVAL;
>
> if (mode == PHY_MODE_USB_HOST)
> @@ -589,9 +583,6 @@ static int dwc3_meson_g12a_otg_init(struct platform_device *pdev,
> int ret, irq;
> struct device *dev = &pdev->dev;
>
> - if (!priv->drvdata->otg_switch_supported)
> - return 0;
> -
> if (priv->otg_mode == USB_DR_MODE_OTG) {
> /* Ack irq before registering */
> regmap_update_bits(priv->usb_glue_regmap, USB_R5,
> @@ -841,8 +832,7 @@ static int dwc3_meson_g12a_remove(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> int i;
>
> - if (priv->drvdata->otg_switch_supported)
> - usb_role_switch_unregister(priv->role_switch);
> + usb_role_switch_unregister(priv->role_switch);
>
> of_platform_depopulate(dev);
>

Reviewed-by: Neil Armstrong <[email protected]>

2023-04-23 17:51:57

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] usb: dwc3-meson-g12a: support OTG switch for all IP versions

On Tue, Apr 18, 2023 at 1:16 PM Dmitry Rokosov <[email protected]> wrote:
>
> From now, the Amlogic A1 USB controller is capable of switching between
> host and gadget modes based on the status of the OTG_ID signal or via
> manual USB role change.
> Previously, only the Amlogic A1 IP version did not use OTG support for
> host only mode, but this is no longer applicable.
> Therefore, the 'otg_switch_supported' option can now be removed as
> it is no longer required.
>
> Signed-off-by: Dmitry Rokosov <[email protected]>
Reviewed-by: Martin Blumenstingl <[email protected]>

2023-04-23 17:55:07

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] arm64: dts: meson: a1: support USB controller in OTG mode

On Tue, Apr 18, 2023 at 1:16 PM Dmitry Rokosov <[email protected]> wrote:
[...]
> + usb2_phy1: phy@4000 {
> + compatible = "amlogic,a1-usb2-phy";
> + clocks = <&clkc CLKID_USB_PHY_IN>;
> + clock-names = "xtal";
Out of curiosity since there's also a CLKID_USB_PHY clock (which is
used for the dwc3 controller below):
Do we know that this part of the clock hierarchy is correct? I have no
way to check this myself, so I'm curious if you could verify this
somehow.

[...]
> + dwc2: usb@ff500000 {
> + compatible = "amlogic,meson-a1-usb", "snps,dwc2";
> + reg = <0x0 0xff500000 0x0 0x40000>;
> + interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> + phys = <&usb2_phy1>;
> + phy-names = "usb2_phy";
Documentation/devicetree/bindings/usb/dwc2.yaml only allows a
"usb2-phy" (dash instead of underscore).

[...]
> + dwc3: usb@ff400000 {
> + compatible = "snps,dwc3";
> + reg = <0x0 0xff400000 0x0 0x100000>;
Note to self: interesting that Amlogic swapped the register location
of the dwc2 and dwc3 controllers since the G12 generation.

2023-04-25 11:08:41

by Dmitry Rokosov

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] arm64: dts: meson: a1: support USB controller in OTG mode

On Sun, Apr 23, 2023 at 07:51:31PM +0200, Martin Blumenstingl wrote:
> On Tue, Apr 18, 2023 at 1:16 PM Dmitry Rokosov <[email protected]> wrote:
> [...]
> > + usb2_phy1: phy@4000 {
> > + compatible = "amlogic,a1-usb2-phy";
> > + clocks = <&clkc CLKID_USB_PHY_IN>;
> > + clock-names = "xtal";
> Out of curiosity since there's also a CLKID_USB_PHY clock (which is
> used for the dwc3 controller below):
> Do we know that this part of the clock hierarchy is correct? I have no
> way to check this myself, so I'm curious if you could verify this
> somehow.
>
> [...]

I've developed a clock driver for A1 and verified it against the Amlogic
custom driver and datasheet. As you pointed out, there are indeed two
USB phy clocks.
They are labeled as follows in my clock driver:
* CLKID_USB_PHY_IN (xtal -> usb_phy gated clock) - the phy input clock
* CLKID_USB_PHY (SYS_CLK_EN based gate) - the synopsys IP gated clock

The current representation of the USB phy clocks is solely based on
my technical opinion, as the datasheet does not provide any detailed
information about them.

Clock driver:
https://lore.kernel.org/all/[email protected]/

> > + dwc2: usb@ff500000 {
> > + compatible = "amlogic,meson-a1-usb", "snps,dwc2";
> > + reg = <0x0 0xff500000 0x0 0x40000>;
> > + interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > + phys = <&usb2_phy1>;
> > + phy-names = "usb2_phy";
> Documentation/devicetree/bindings/usb/dwc2.yaml only allows a
> "usb2-phy" (dash instead of underscore).
>
> [...]

Ah, my fault..

> > + dwc3: usb@ff400000 {
> > + compatible = "snps,dwc3";
> > + reg = <0x0 0xff400000 0x0 0x100000>;
> Note to self: interesting that Amlogic swapped the register location
> of the dwc2 and dwc3 controllers since the G12 generation.

Indeed, during the bringup process, I was surprised to discover that
the dwc2 engine wasn't starting properly. It was quite unexpected, but
also admittedly intriguing as I delved into the issue and tried to
understand the root cause.

--
Thank you,
Dmitry