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)
Links:
[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
Dmitry Rokosov (5):
phy: amlogic: during USB PHY clkin obtaining, enable it
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
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 | 2 +-
drivers/usb/dwc2/params.c | 21 +++++++
drivers/usb/dwc3/dwc3-meson-g12a.c | 2 +-
5 files changed, 83 insertions(+), 2 deletions(-)
--
2.36.0
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
by manual usb role changing.
Signed-off-by: Dmitry Rokosov <[email protected]>
---
drivers/usb/dwc3/dwc3-meson-g12a.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c
index b282ad0e69c6..10469b95deb9 100644
--- a/drivers/usb/dwc3/dwc3-meson-g12a.c
+++ b/drivers/usb/dwc3/dwc3-meson-g12a.c
@@ -242,7 +242,7 @@ static const struct dwc3_meson_g12a_drvdata g12a_drvdata = {
};
static const struct dwc3_meson_g12a_drvdata a1_drvdata = {
- .otg_switch_supported = false,
+ .otg_switch_supported = true,
.clks = meson_a1_clocks,
.num_clks = ARRAY_SIZE(meson_a1_clocks),
.phy_names = meson_a1_phy_names,
--
2.36.0
The Amlogic A1 uses dwc2 Synopsys IP as its USB peripheral (gadget)
endpoint, with different DWC2 parameters when compared to previous
Amlogic SoCs.
Signed-off-by: Dmitry Rokosov <[email protected]>
---
drivers/usb/dwc2/params.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 9ed9fd956940..098fbfc774ab 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -161,6 +161,25 @@ static void dwc2_set_amlogic_g12a_params(struct dwc2_hsotg *hsotg)
p->hird_threshold_en = false;
}
+static void dwc2_set_amlogic_a1_params(struct dwc2_hsotg *hsotg)
+{
+ struct dwc2_core_params *p = &hsotg->params;
+
+ p->otg_caps.hnp_support = false;
+ p->otg_caps.srp_support = false;
+ p->speed = DWC2_SPEED_PARAM_HIGH;
+ p->host_rx_fifo_size = 192;
+ p->host_nperio_tx_fifo_size = 128;
+ p->host_perio_tx_fifo_size = 128;
+ p->phy_type = DWC2_PHY_TYPE_PARAM_UTMI;
+ p->phy_utmi_width = 8;
+ p->ahbcfg = GAHBCFG_HBSTLEN_INCR8 << GAHBCFG_HBSTLEN_SHIFT;
+ 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;
@@ -258,6 +277,8 @@ const struct of_device_id dwc2_of_match_table[] = {
.data = dwc2_set_amlogic_params },
{ .compatible = "amlogic,meson-g12a-usb",
.data = dwc2_set_amlogic_g12a_params },
+ { .compatible = "amlogic,meson-a1-usb",
+ .data = dwc2_set_amlogic_a1_params },
{ .compatible = "amcc,dwc-otg", .data = dwc2_set_amcc_params },
{ .compatible = "apm,apm82181-dwc-otg", .data = dwc2_set_amcc_params },
{ .compatible = "st,stm32f4x9-fsotg",
--
2.36.0
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
Previously, all Amlogic boards used the XTAL clock as the USB PHY input
clock, and it did not need to be enabled as it was the default board
clock. However, in new Amlogic SoCs such as the A1 family, USB PHY uses
a gated clock, so it is necessary to enable this gated clock during
probing.
Signed-off-by: Dmitry Rokosov <[email protected]>
---
drivers/phy/amlogic/phy-meson-g12a-usb2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb2.c b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
index 9d1efa0d9394..26b99fbe1026 100644
--- a/drivers/phy/amlogic/phy-meson-g12a-usb2.c
+++ b/drivers/phy/amlogic/phy-meson-g12a-usb2.c
@@ -315,7 +315,7 @@ static int phy_meson_g12a_usb2_probe(struct platform_device *pdev)
if (IS_ERR(priv->regmap))
return PTR_ERR(priv->regmap);
- priv->clk = devm_clk_get(dev, "xtal");
+ priv->clk = devm_clk_get_enabled(dev, "xtal");
if (IS_ERR(priv->clk))
return PTR_ERR(priv->clk);
--
2.36.0
Hi Dmitry,
On Fri, Apr 14, 2023 at 5:24 PM Dmitry Rokosov <[email protected]> wrote:
[...]
> - priv->clk = devm_clk_get(dev, "xtal");
> + priv->clk = devm_clk_get_enabled(dev, "xtal");
Generally this works fine but I wouldn't recommend this approach if:
- there's some required wait time after the clock has been enabled
(see phy_meson_g12a_usb2_init - there's already some required wait
time after triggering the reset)
- clock gating (for power saving) is needed when the dwc3 driver is
unloaded by the PHY driver is not
In this case: just manually manage the clock in phy_meson_g12a_usb2_{init,exit}
Best regards,
Martin
On Fri, Apr 14, 2023 at 5:24 PM Dmitry Rokosov <[email protected]> wrote:
[...]
> static const struct dwc3_meson_g12a_drvdata a1_drvdata = {
> - .otg_switch_supported = false,
> + .otg_switch_supported = true,
it would be great if you could also follow up with a patch that
removes otg_switch_supported.
A1 was the only variant that needed it and after this patch it's just dead code.
Best regards,
Martin
Hello Martin,
Thank you for quick review, appreciate it!
Please find my comments below and in the other replies.
On Sun, Apr 16, 2023 at 10:56:36PM +0200, Martin Blumenstingl wrote:
> On Fri, Apr 14, 2023 at 5:24 PM Dmitry Rokosov <[email protected]> wrote:
> [...]
> > static const struct dwc3_meson_g12a_drvdata a1_drvdata = {
> > - .otg_switch_supported = false,
> > + .otg_switch_supported = true,
> it would be great if you could also follow up with a patch that
> removes otg_switch_supported.
> A1 was the only variant that needed it and after this patch it's just dead code.
It makes sense. I thought about it before sending the first version, but
I found a counter-argument: future SoCs may use this parameter.
But if you ask, I will remove 'otg_switch_supported' in the next version
--
Thank you,
Dmitry
On Sun, Apr 16, 2023 at 10:54:17PM +0200, Martin Blumenstingl wrote:
> Hi Dmitry,
>
> On Fri, Apr 14, 2023 at 5:24 PM Dmitry Rokosov <[email protected]> wrote:
> [...]
> > - priv->clk = devm_clk_get(dev, "xtal");
> > + priv->clk = devm_clk_get_enabled(dev, "xtal");
> Generally this works fine but I wouldn't recommend this approach if:
> - there's some required wait time after the clock has been enabled
> (see phy_meson_g12a_usb2_init - there's already some required wait
> time after triggering the reset)
> - clock gating (for power saving) is needed when the dwc3 driver is
> unloaded by the PHY driver is not
>
> In this case: just manually manage the clock in phy_meson_g12a_usb2_{init,exit}
I'm sorry, but I'm not fully understanding your point. Currently, no
sleeps are required for this clock and we don't have any logic for
power saving (g12a phy_ops doesn't have power_on()/power_off()
implementation).
However, I believe all of your arguments make sense for the future
development of the phy_meson_g12a_usb2 driver. Is that correct?
--
Thank you,
Dmitry
On 17/04/2023 13:47, Dmitry Rokosov wrote:
> Hello Martin,
>
> Thank you for quick review, appreciate it!
> Please find my comments below and in the other replies.
>
> On Sun, Apr 16, 2023 at 10:56:36PM +0200, Martin Blumenstingl wrote:
>> On Fri, Apr 14, 2023 at 5:24 PM Dmitry Rokosov <[email protected]> wrote:
>> [...]
>>> static const struct dwc3_meson_g12a_drvdata a1_drvdata = {
>>> - .otg_switch_supported = false,
>>> + .otg_switch_supported = true,
>> it would be great if you could also follow up with a patch that
>> removes otg_switch_supported.
>> A1 was the only variant that needed it and after this patch it's just dead code.
>
> It makes sense. I thought about it before sending the first version, but
> I found a counter-argument: future SoCs may use this parameter.
> But if you ask, I will remove 'otg_switch_supported' in the next version
>
Please remove it, it's easy to add it again if needed.
Neil
Hello Neil,
Thank you for review, appreciate it!
On Mon, Apr 17, 2023 at 02:22:26PM +0200, [email protected] wrote:
> On 17/04/2023 13:47, Dmitry Rokosov wrote:
> > Hello Martin,
> >
> > Thank you for quick review, appreciate it!
> > Please find my comments below and in the other replies.
> >
> > On Sun, Apr 16, 2023 at 10:56:36PM +0200, Martin Blumenstingl wrote:
> > > On Fri, Apr 14, 2023 at 5:24 PM Dmitry Rokosov <[email protected]> wrote:
> > > [...]
> > > > static const struct dwc3_meson_g12a_drvdata a1_drvdata = {
> > > > - .otg_switch_supported = false,
> > > > + .otg_switch_supported = true,
> > > it would be great if you could also follow up with a patch that
> > > removes otg_switch_supported.
> > > A1 was the only variant that needed it and after this patch it's just dead code.
> >
> > It makes sense. I thought about it before sending the first version, but
> > I found a counter-argument: future SoCs may use this parameter.
> > But if you ask, I will remove 'otg_switch_supported' in the next version
> >
>
> Please remove it, it's easy to add it again if needed.
Sure, no problem. It will be removed in the next version.
--
Thank you,
Dmitry