Linux USB
 help / color / mirror / Atom feed
* [PATCH v1 0/5] arm64: meson: support Amlogic A1 USB OTG controller
@ 2023-04-14 15:24 Dmitry Rokosov
  2023-04-14 15:24 ` [PATCH v1 1/5] phy: amlogic: during USB PHY clkin obtaining, enable it Dmitry Rokosov
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Dmitry Rokosov @ 2023-04-14 15:24 UTC (permalink / raw)
  To: gregkh, robh+dt, krzysztof.kozlowski+dt, neil.armstrong, khilman,
	jbrunet, martin.blumenstingl, mturquette, vkoul, kishon, hminas,
	Thinh.Nguyen
  Cc: yue.wang, hanjie.lin, kernel, rockosov, linux-usb, devicetree,
	linux-kernel, linux-arm-kernel, linux-amlogic, linux-phy,
	Dmitry Rokosov

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/1581990859-135234-1-git-send-email-hanjie.lin@amlogic.com/
    [2] https://lore.kernel.org/all/1581990859-135234-4-git-send-email-hanjie.lin@amlogic.com/

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


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

* [PATCH v1 1/5] phy: amlogic: during USB PHY clkin obtaining, enable it
  2023-04-14 15:24 [PATCH v1 0/5] arm64: meson: support Amlogic A1 USB OTG controller Dmitry Rokosov
@ 2023-04-14 15:24 ` Dmitry Rokosov
  2023-04-16 20:54   ` Martin Blumenstingl
  2023-04-14 15:24 ` [PATCH v1 2/5] usb: dwc2: support dwc2 IP for Amlogic A1 SoC family Dmitry Rokosov
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Dmitry Rokosov @ 2023-04-14 15:24 UTC (permalink / raw)
  To: gregkh, robh+dt, krzysztof.kozlowski+dt, neil.armstrong, khilman,
	jbrunet, martin.blumenstingl, mturquette, vkoul, kishon, hminas,
	Thinh.Nguyen
  Cc: yue.wang, hanjie.lin, kernel, rockosov, linux-usb, devicetree,
	linux-kernel, linux-arm-kernel, linux-amlogic, linux-phy,
	Dmitry Rokosov

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 <ddrokosov@sberdevices.ru>
---
 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


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

* [PATCH v1 2/5] usb: dwc2: support dwc2 IP for Amlogic A1 SoC family
  2023-04-14 15:24 [PATCH v1 0/5] arm64: meson: support Amlogic A1 USB OTG controller Dmitry Rokosov
  2023-04-14 15:24 ` [PATCH v1 1/5] phy: amlogic: during USB PHY clkin obtaining, enable it Dmitry Rokosov
@ 2023-04-14 15:24 ` Dmitry Rokosov
  2023-04-14 15:24 ` [PATCH v1 3/5] dt-bindings: usb: dwc2: add support for Amlogic A1 SoC USB peripheral Dmitry Rokosov
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Dmitry Rokosov @ 2023-04-14 15:24 UTC (permalink / raw)
  To: gregkh, robh+dt, krzysztof.kozlowski+dt, neil.armstrong, khilman,
	jbrunet, martin.blumenstingl, mturquette, vkoul, kishon, hminas,
	Thinh.Nguyen
  Cc: yue.wang, hanjie.lin, kernel, rockosov, linux-usb, devicetree,
	linux-kernel, linux-arm-kernel, linux-amlogic, linux-phy,
	Dmitry Rokosov

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 <ddrokosov@sberdevices.ru>
---
 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


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

* [PATCH v1 3/5] dt-bindings: usb: dwc2: add support for Amlogic A1 SoC USB peripheral
  2023-04-14 15:24 [PATCH v1 0/5] arm64: meson: support Amlogic A1 USB OTG controller Dmitry Rokosov
  2023-04-14 15:24 ` [PATCH v1 1/5] phy: amlogic: during USB PHY clkin obtaining, enable it Dmitry Rokosov
  2023-04-14 15:24 ` [PATCH v1 2/5] usb: dwc2: support dwc2 IP for Amlogic A1 SoC family Dmitry Rokosov
@ 2023-04-14 15:24 ` Dmitry Rokosov
  2023-04-14 15:24 ` [PATCH v1 4/5] usb: dwc3-meson-g12a: support OTG switch Dmitry Rokosov
  2023-04-14 15:24 ` [PATCH v1 5/5] arm64: dts: meson: a1: support USB controller in OTG mode Dmitry Rokosov
  4 siblings, 0 replies; 12+ messages in thread
From: Dmitry Rokosov @ 2023-04-14 15:24 UTC (permalink / raw)
  To: gregkh, robh+dt, krzysztof.kozlowski+dt, neil.armstrong, khilman,
	jbrunet, martin.blumenstingl, mturquette, vkoul, kishon, hminas,
	Thinh.Nguyen
  Cc: yue.wang, hanjie.lin, kernel, rockosov, linux-usb, devicetree,
	linux-kernel, linux-arm-kernel, linux-amlogic, linux-phy,
	Dmitry Rokosov

Provide the appropriate compatible string for the DWC2 IP that is found
inside the Amlogic A1 SoC and used in peripheral mode.

Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
---
 Documentation/devicetree/bindings/usb/dwc2.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.yaml b/Documentation/devicetree/bindings/usb/dwc2.yaml
index 371ba93f3ce5..f70be397dac0 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.yaml
+++ b/Documentation/devicetree/bindings/usb/dwc2.yaml
@@ -53,6 +53,7 @@ properties:
               - amlogic,meson8b-usb
               - amlogic,meson-gxbb-usb
               - amlogic,meson-g12a-usb
+              - amlogic,meson-a1-usb
               - intel,socfpga-agilex-hsotg
           - const: snps,dwc2
       - const: amcc,dwc-otg
-- 
2.36.0


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

* [PATCH v1 4/5] usb: dwc3-meson-g12a: support OTG switch
  2023-04-14 15:24 [PATCH v1 0/5] arm64: meson: support Amlogic A1 USB OTG controller Dmitry Rokosov
                   ` (2 preceding siblings ...)
  2023-04-14 15:24 ` [PATCH v1 3/5] dt-bindings: usb: dwc2: add support for Amlogic A1 SoC USB peripheral Dmitry Rokosov
@ 2023-04-14 15:24 ` Dmitry Rokosov
  2023-04-16 20:56   ` Martin Blumenstingl
  2023-04-14 15:24 ` [PATCH v1 5/5] arm64: dts: meson: a1: support USB controller in OTG mode Dmitry Rokosov
  4 siblings, 1 reply; 12+ messages in thread
From: Dmitry Rokosov @ 2023-04-14 15:24 UTC (permalink / raw)
  To: gregkh, robh+dt, krzysztof.kozlowski+dt, neil.armstrong, khilman,
	jbrunet, martin.blumenstingl, mturquette, vkoul, kishon, hminas,
	Thinh.Nguyen
  Cc: yue.wang, hanjie.lin, kernel, rockosov, linux-usb, devicetree,
	linux-kernel, linux-arm-kernel, linux-amlogic, linux-phy,
	Dmitry Rokosov

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 <ddrokosov@sberdevices.ru>
---
 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


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

* [PATCH v1 5/5] arm64: dts: meson: a1: support USB controller in OTG mode
  2023-04-14 15:24 [PATCH v1 0/5] arm64: meson: support Amlogic A1 USB OTG controller Dmitry Rokosov
                   ` (3 preceding siblings ...)
  2023-04-14 15:24 ` [PATCH v1 4/5] usb: dwc3-meson-g12a: support OTG switch Dmitry Rokosov
@ 2023-04-14 15:24 ` Dmitry Rokosov
  4 siblings, 0 replies; 12+ messages in thread
From: Dmitry Rokosov @ 2023-04-14 15:24 UTC (permalink / raw)
  To: gregkh, robh+dt, krzysztof.kozlowski+dt, neil.armstrong, khilman,
	jbrunet, martin.blumenstingl, mturquette, vkoul, kishon, hminas,
	Thinh.Nguyen
  Cc: yue.wang, hanjie.lin, kernel, rockosov, linux-usb, devicetree,
	linux-kernel, linux-arm-kernel, linux-amlogic, linux-phy,
	Dmitry Rokosov

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 <yue.wang@amlogic.com>
Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
---
 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


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

* Re: [PATCH v1 1/5] phy: amlogic: during USB PHY clkin obtaining, enable it
  2023-04-14 15:24 ` [PATCH v1 1/5] phy: amlogic: during USB PHY clkin obtaining, enable it Dmitry Rokosov
@ 2023-04-16 20:54   ` Martin Blumenstingl
  2023-04-17 11:57     ` Dmitry Rokosov
  0 siblings, 1 reply; 12+ messages in thread
From: Martin Blumenstingl @ 2023-04-16 20:54 UTC (permalink / raw)
  To: Dmitry Rokosov
  Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, neil.armstrong, khilman,
	jbrunet, mturquette, vkoul, kishon, hminas, Thinh.Nguyen,
	yue.wang, hanjie.lin, kernel, rockosov, linux-usb, devicetree,
	linux-kernel, linux-arm-kernel, linux-amlogic, linux-phy

Hi Dmitry,

On Fri, Apr 14, 2023 at 5:24 PM Dmitry Rokosov <ddrokosov@sberdevices.ru> 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

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

* Re: [PATCH v1 4/5] usb: dwc3-meson-g12a: support OTG switch
  2023-04-14 15:24 ` [PATCH v1 4/5] usb: dwc3-meson-g12a: support OTG switch Dmitry Rokosov
@ 2023-04-16 20:56   ` Martin Blumenstingl
  2023-04-17 11:47     ` Dmitry Rokosov
  0 siblings, 1 reply; 12+ messages in thread
From: Martin Blumenstingl @ 2023-04-16 20:56 UTC (permalink / raw)
  To: Dmitry Rokosov
  Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, neil.armstrong, khilman,
	jbrunet, mturquette, vkoul, kishon, hminas, Thinh.Nguyen,
	yue.wang, hanjie.lin, kernel, rockosov, linux-usb, devicetree,
	linux-kernel, linux-arm-kernel, linux-amlogic, linux-phy

On Fri, Apr 14, 2023 at 5:24 PM Dmitry Rokosov <ddrokosov@sberdevices.ru> 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

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

* Re: [PATCH v1 4/5] usb: dwc3-meson-g12a: support OTG switch
  2023-04-16 20:56   ` Martin Blumenstingl
@ 2023-04-17 11:47     ` Dmitry Rokosov
  2023-04-17 12:22       ` neil.armstrong
  0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Rokosov @ 2023-04-17 11:47 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, neil.armstrong, khilman,
	jbrunet, mturquette, vkoul, kishon, hminas, Thinh.Nguyen,
	yue.wang, hanjie.lin, kernel, rockosov, linux-usb, devicetree,
	linux-kernel, linux-arm-kernel, linux-amlogic, linux-phy

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 <ddrokosov@sberdevices.ru> 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

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

* Re: [PATCH v1 1/5] phy: amlogic: during USB PHY clkin obtaining, enable it
  2023-04-16 20:54   ` Martin Blumenstingl
@ 2023-04-17 11:57     ` Dmitry Rokosov
  0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Rokosov @ 2023-04-17 11:57 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, neil.armstrong, khilman,
	jbrunet, mturquette, vkoul, kishon, hminas, Thinh.Nguyen,
	yue.wang, hanjie.lin, kernel, rockosov, linux-usb, devicetree,
	linux-kernel, linux-arm-kernel, linux-amlogic, linux-phy

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 <ddrokosov@sberdevices.ru> 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

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

* Re: [PATCH v1 4/5] usb: dwc3-meson-g12a: support OTG switch
  2023-04-17 11:47     ` Dmitry Rokosov
@ 2023-04-17 12:22       ` neil.armstrong
  2023-04-17 12:29         ` Dmitry Rokosov
  0 siblings, 1 reply; 12+ messages in thread
From: neil.armstrong @ 2023-04-17 12:22 UTC (permalink / raw)
  To: Dmitry Rokosov, Martin Blumenstingl
  Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, khilman, jbrunet,
	mturquette, vkoul, kishon, hminas, Thinh.Nguyen, yue.wang,
	hanjie.lin, kernel, rockosov, linux-usb, devicetree, linux-kernel,
	linux-arm-kernel, linux-amlogic, linux-phy

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 <ddrokosov@sberdevices.ru> 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

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

* Re: [PATCH v1 4/5] usb: dwc3-meson-g12a: support OTG switch
  2023-04-17 12:22       ` neil.armstrong
@ 2023-04-17 12:29         ` Dmitry Rokosov
  0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Rokosov @ 2023-04-17 12:29 UTC (permalink / raw)
  To: neil.armstrong
  Cc: Martin Blumenstingl, gregkh, robh+dt, krzysztof.kozlowski+dt,
	khilman, jbrunet, mturquette, vkoul, kishon, hminas, Thinh.Nguyen,
	yue.wang, hanjie.lin, kernel, rockosov, linux-usb, devicetree,
	linux-kernel, linux-arm-kernel, linux-amlogic, linux-phy

Hello Neil,

Thank you for review, appreciate it!

On Mon, Apr 17, 2023 at 02:22:26PM +0200, neil.armstrong@linaro.org 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 <ddrokosov@sberdevices.ru> 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

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

end of thread, other threads:[~2023-04-17 12:30 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-14 15:24 [PATCH v1 0/5] arm64: meson: support Amlogic A1 USB OTG controller Dmitry Rokosov
2023-04-14 15:24 ` [PATCH v1 1/5] phy: amlogic: during USB PHY clkin obtaining, enable it Dmitry Rokosov
2023-04-16 20:54   ` Martin Blumenstingl
2023-04-17 11:57     ` Dmitry Rokosov
2023-04-14 15:24 ` [PATCH v1 2/5] usb: dwc2: support dwc2 IP for Amlogic A1 SoC family Dmitry Rokosov
2023-04-14 15:24 ` [PATCH v1 3/5] dt-bindings: usb: dwc2: add support for Amlogic A1 SoC USB peripheral Dmitry Rokosov
2023-04-14 15:24 ` [PATCH v1 4/5] usb: dwc3-meson-g12a: support OTG switch Dmitry Rokosov
2023-04-16 20:56   ` Martin Blumenstingl
2023-04-17 11:47     ` Dmitry Rokosov
2023-04-17 12:22       ` neil.armstrong
2023-04-17 12:29         ` Dmitry Rokosov
2023-04-14 15:24 ` [PATCH v1 5/5] arm64: dts: meson: a1: support USB controller in OTG mode Dmitry Rokosov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox