Meson-8b and Meson-gxbb Fix some missing code

On Odroid C1+ and Odroid C2 USB feature is broken

It's being observed the after initiation of USB phy
the USB port goes in to suspend state, If we pass usbcore.autosuspend=-1 via command line USB hotplug seen to be working.

Another issue I observed is increase of USB interrupts event even if there is not much activity on USB ports.

$ cat /proc/interrupts | grep usb
 35:   26462800          0          0          0     GIC-0  63 Level
			c90c0000.usb, dwc2_hsotg:usb1
8
Changes added power node to usb phy and small code cleanup
in usb phy.

Previous version RFC.
[0] https://patchwork.kernel.org/project/linux-amlogic/cover/[email protected]/
Dopped the reorder of code changes as of now.

V1 > changes Fixed the GPIO input signal on Odroid C1+/C2
New patch added to fix Odroid C2.

[1] https://lore.kernel.org/linux-devicetree/[email protected]/

V2 > changes Fixed the GPIO polarity for Odroid C1
fix the power source from phy-supply to vbus-supply added new patches to fix resolve some issues.

Thanks
-Anand

Anand Moon (6):
ARM: dts: meson8b: odroidc1: Add usb phy power node
ARM: dts: meson8b: odroidc1: Set usb power source to always on
arm64: dts: amlogic: odroidc2: Fix the chip enable signal for usb
power
arm64: dts: amlogic: odroidc2: use vbus-supply for power source for
usb nodes
phy: amlogic: meson8b-usb2: Power off the PHY by putting it into reset
mode
phy: amlogic: meson8b-usb2: don't log an error on -EPROBE_DEFER

 arch/arm/boot/dts/meson8b-odroidc1.dts        | 21 ++++++++++++++++++-
 .../boot/dts/amlogic/meson-gxbb-odroidc2.dts  | 10 ++++-----
 drivers/phy/amlogic/phy-meson8b-usb2.c        |  8 +++++--
 3 files changed, 30 insertions(+), 9 deletions(-)

2.32.0

Add missing usb phy power node for usb node fix below warning. P5V0 regulator supply input voltage range to USB host controller. As described in the C1+ schematics, GPIO GPIOAO_5 is used to enable input power to USB ports, set it to Active Low.

[    1.260772] dwc2 c90c0000.usb: Looking up vbus-supply from device tree
[    1.260784] dwc2 c90c0000.usb: Looking up vbus-supply property in
		mode /soc/usb@c90c0000 failed

controller on Odroid-C1/C1+ board)

---
V2 > changes gpio polarity ACTIVE_HIGH to ACTIVE_LOW.
Fix the source power from phy-supply to vbus-supply.

[1] https://lore.kernel.org/linux-devicetree/[email protected]/

V1 > Fix the Input GPIO polarity from HIGH to LOW.

previous version
[0] https://patchwork.kernel.org/project/linux-amlogic/patch/[email protected]

changes fix the vbus-suppy to phy-supply, drop enable usb0

    USB_PWR                       2    1      0 unknown  5000mV     0mA  5000mV  5000mV
       phy-c1108820.phy.0-phy     2                                 0mA     0mV     0mV
---
 arch/arm/boot/dts/meson8b-odroidc1.dts | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

index c440ef94e082..30ec6a7f20c7 100644
@@ -32,6 +32,23 @@ emmc_pwrseq: emmc-pwrseq {
 		reset-gpios = <&gpio BOOT_9 GPIO_ACTIVE_LOW>;
 	};
 
+	usb_pwr_en: regulator-usb-pwr-en {
+		compatible = "regulator-fixed";
+
+		regulator-name = "USB_OTG_PWR";
+
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		/*
+		 * signal name from schematics: USB_POWER - P5V0
+		 */
+		vin-supply = <&p5v0>;
+		/*
+		 * signal name from schematics: PWREN - GPIOAO.BIT5
+		 */
+		gpios = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_LOW>;
+	};
+
 	leds {
 		compatible = "gpio-leds";
 		blue {

@@ -119,7 +136,6 @@ vcc_3v3: regulator-vcc-3v3 {
 		regulator-name = "VCC3V3";
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
-
 		vin-supply = <&p5v0>;
 	};
 

@@ -382,4 +398,5 @@ &usb1_phy {
 
 &usb1 {
 	status = "okay";
+	vbus-supply = <&usb_pwr_en>;
 };

2.32.0

Hi Anand,

sorry for the late reply
I have three very small comments below, apart from these, this is looking good!

I would phrase this last sentence as:
"enable USB VBUS on the Micro-USB port using an active high signal" My idea here is to 1) clarify that it's about enabling USB VBUS only on the Micro-USB port and 2) use "active high" like the changes inside the patch itself

please drop this empty line

[...]

Hi,

First of all, DT is not here to fix boot message.

Secondly, if the vbus-supply is optional, the message should be removed from the driver/regulator core instead.

Finally, I looked at the Odroid-C1 schematics and the GPIOAO.BIT5 is an input to the S805, and the PWREN signal is controlled by the USB Hub so this regulator should not be added at all.

Neil

Hi Neil,

Anand mentioned elsewhere that this is a debug/info message

I think there's a misunderstanding because there's two PWREN signals with different meanings.
The PWREN signal for the USB host ports is hard-wired and not connected to the SoC at all.
The PWREN signal for the Micro-USB port (which Anand is adding here) is controlled by GPIOAO_5. odroid-c1+_rev0.4_20150615.pdf [0] shows it as an input to "USB_OTG" on page 1. "USB_OTG" consists of a power switch and the connector itself as shown on page 28.

Personally I think that the change from Anand itself is good. If you feel otherwise then please speak up.
As I pointed out three smaller changes I am hoping that Anand will re-send the updated patch anyways. At that point he can also add the changes from your feedback.

Best regards,
Martin

[0] https://dn.odroid.com/S805/Schematics/odroid-c1+_rev0.4_20150615.pdf

Hi,

Ok thanks for the clarification, then the change is ok, but not the commit message.

is not a good reason for a DT change. A proper reason should be added.

And the commit message doesn't specify the change is for the Micro-USB port, this should be clarified.

Neil

Hi Neil / Martin,

Thanks for your review comments.

Ok I will try to address your feedback in the next version.

After enabling CONFIG_REGULATOR_DEBUG, with this patch applied I still not getting the USB regulator to enable.
Do you see different output at your end?

On Odroid C1+

[    5.737571] reg-fixed-voltage regulator-usb-pwr-en: GPIO lookup for
consumer (null)
[    5.737630] reg-fixed-voltage regulator-usb-pwr-en: using device
tree for GPIO lookup
[    5.737711] of_get_named_gpiod_flags: can't parse 'gpios' property
of node '/regulator-usb-pwr-en[0]'
[    5.737906] of_get_named_gpiod_flags: parsed 'gpio' property of
node '/regulator-usb-pwr-en[0]' - status (0)
[    5.738209] gpio_stub_drv gpiochip0: Persistence not supported for GPIO 5
[    5.738490] USB_OTG_PWR: 5000 mV, disabled
[    5.740313] reg-fixed-voltage regulator-usb-pwr-en: Looking up
vin-supply from device tree
[    5.740394] USB_OTG_PWR: supplied by P5V0
[    5.741235] reg-fixed-voltage regulator-usb-pwr-en: USB_OTG_PWR

supplying 5000000uV

Odroid N2.

[    3.047813] reg-fixed-voltage regulator-hub_5v: HUB_5V supplying 5000000uV
[    3.049282] reg-fixed-voltage regulator-usb_pwr_en: GPIO lookup for
consumer (null)
[    3.049305] reg-fixed-voltage regulator-usb_pwr_en: using device
tree for GPIO lookup
[    3.049370] of_get_named_gpiod_flags: can't parse 'gpios' property
of node '/regulator-usb_pwr_en[0]'
[    3.049500] of_get_named_gpiod_flags: parsed 'gpio' property of
node '/regulator-usb_pwr_en[0]' - status (0)
[    3.049622] gpio_stub_drv gpiochip0: Persistence not supported for GPIO 22
[    3.049759] USB_PWR_EN: 5000 mV, disabled
[    3.051257] reg-fixed-voltage regulator-usb_pwr_en: Looking up
vin-supply from device tree
[    3.051320] USB_PWR_EN: supplied by 5V

Thanks
-Anand

Hi Anand,

On Tue, Aug 31, 2021 at 10:48 PM Anand Moon <[email protected]> wrote:
[...]

I don't have much time for testing and debugging currently but I'll put it on my TODO-list
Until either of us has found the issue I suggest not merging this patch.

Best regards,
Martin

linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy

Hi Martin,

Ok no problem.

Basically, I have just roughly gone through the architecture of Amlogic's OTG framework.
Below is the global configuration registers for DWC2 OTG framer work

[0] https://github.com/hardkernel/linux/blob/odroidc-3.10.y/drivers/amlogic/usb/dwc_otg/310/dwc_otg_regs.h#L66-L151

Within some configurations, it helps tune the power for vbus and interrupt
For example
[1] https://github.com/hardkernel/linux/blob/odroidc-3.10.y/drivers/amlogic/usb/dwc_otg/310/dwc_otg_attr.c#L666-L717

Amlogic basically used the mode parameter to external tune the DWC2 driver it could help more fine-tuning the driver.

[2] https://github.com/hardkernel/linux/blob/odroidc-3.10.y/drivers/amlogic/usb/dwc_otg/310/dwc_otg_driver.c#L1461-L1703

I feel we need to identify many more PHY tuning parameters to make the USB work correctly.

Thanks
-Anand

Set P5V0 and vcc_3v3 power source to USB to always on so that regulator should not enter in suspend state.

---
New patch in this series.

---
 arch/arm/boot/dts/meson8b-odroidc1.dts | 2 ++
 1 file changed, 2 insertions(+)

index 30ec6a7f20c7..5e5953152452 100644
@@ -65,6 +65,7 @@ p5v0: regulator-p5v0 {
 		regulator-name = "P5V0";
 		regulator-min-microvolt = <5000000>;
 		regulator-max-microvolt = <5000000>;
+		regulator-always-on;
 	};
 
 	tflash_vdd: regulator-tflash_vdd {

@@ -136,6 +137,7 @@ vcc_3v3: regulator-vcc-3v3 {
 		regulator-name = "VCC3V3";
 		regulator-min-microvolt = <3300000>;
 		regulator-max-microvolt = <3300000>;
+		regulator-always-on;
 		vin-supply = <&p5v0>;
 	};
 

2.32.0

Hi Anand,

Neither of these two regulators can be controlled by Linux in any way (there's no GPIO to turn them off, no PWM signal to change the output voltage, etc.).
So can you please explain what this patch changes from a functional perspective?

Best regards,
Martin

Fix the chip enable signal changing from Active High to Active Low to enable input power to USB power. Also updated signal name as per the schematics.

---
v2:
Fix the typo: shematics -> schematics

---
 arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

index 201596247fd9..3f4f16a5dedd 100644
@@ -38,12 +38,11 @@ usb_otg_pwr: regulator-usb-pwrs {
 		regulator-max-microvolt = <5000000>;
 
 		/*
-		 * signal name from schematics: PWREN
+		 * signal name from schematics: PWREN - GPIOAO.BIT5
 		 */
-		gpio = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>;
-		enable-active-high;
+		gpios = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_LOW>;
 		/*
-		 * signal name from schematics: USB_POWER
+		 * signal name from schematics: USB_POWER - P5V0
 		 */
 		vin-supply = <&p5v0>;
 	};

2.32.0

Hi Anand,

according to [0] (page 12) Odroid-C2 uses an "RT9715EGB" for USB OTG power The datasheet for that power switch [1] mentions on page page 1 that the "E" stands for "E : 1.1A/Active High"

Can you please elaborate how you have tested this to confirm that active low is the correct polarity?

Best regards,
Martin

[0] https://dn.odroid.com/S905/Schematic/odroid-c2_rev0.1_20150930.pdf
[1] https://www.richtek.com/assets/product_file/RT9715/DS9715-03.pdf

Use vbus-supply instead of phy-supply as power source for dwc2 nodes. Drop vbus-supply for usb0 node, as it will handle later.

fix usb1 power supply")

---
New patch in this series.

---
 arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

index 3f4f16a5dedd..01bbfc4c091e 100644
@@ -400,12 +400,10 @@ &uart_AO {
 
 &usb0_phy {
 	status = "disabled";
-	phy-supply = <&usb_otg_pwr>;
 };
 
 &usb1_phy {
 	status = "okay";
-	phy-supply = <&usb_otg_pwr>;
 };
 
 &usb0 {

@@ -414,4 +412,5 @@ &usb0 {
 
 &usb1 {
 	status = "okay";
+	vbus-supply = <&usb_otg_pwr>;
 };

2.32.0

Hi Anand,

This is more of a question than a review comment:
Do you think that the USB power setup on Odroid-C1 and Odroid-C2 is different or the same?
On Odroid-C1 we know that only the PWREN signal which goes to the Micro-USB connector is configurable while VBUS for the USB host ports is always enabled.

most Linux commits which I have seen don't use a blank line here Also I think it's recommended not to break the "Fixes" tag into multiple lines, even if the text is long
see [0] for example

Best regards,
Martin

[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=425bec0032f59eeee12520085cd054fac09cc66e

Power off the PHY by putting it into reset mode.

---
v2 - None

---
 drivers/phy/amlogic/phy-meson8b-usb2.c | 4 ++++
 1 file changed, 4 insertions(+)

index 03c061dd5f0d..2aad45c55494 100644
@@ -219,6 +219,10 @@ static int phy_meson8b_usb2_power_off(struct phy *phy)
 	clk_disable_unprepare(priv->clk_usb);
 	clk_disable_unprepare(priv->clk_usb_general);
 
+	/* power off the PHY by putting it into reset mode */
+	regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_POWER_ON_RESET,
+			   REG_CTRL_POWER_ON_RESET);
+
 	return 0;
 }
 

2.32.0

Applied, thanks

~Vinod

devm_phy_create can return -EPROBE_DEFER if the vbus-supply is not ready yet. Silence this warning as the driver framework will re-attempt registering the PHY. Use dev_err_probe() for phy resources to indicate the deferral reason when waiting for the resource to come up.

---
None.

---
 drivers/phy/amlogic/phy-meson8b-usb2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 2aad45c55494..cf10bed40528 100644
@@ -277,8 +277,8 @@ static int phy_meson8b_usb2_probe(struct platform_device *pdev)
 
 	phy = devm_phy_create(&pdev->dev, NULL, &phy_meson8b_usb2_ops);
 	if (IS_ERR(phy)) {
-		dev_err(&pdev->dev, "failed to create PHY\n");
-		return PTR_ERR(phy);
+		return dev_err_probe(&pdev->dev, PTR_ERR(phy),
+				     "failed to create PHY\n");
 	}
 
 	phy_set_drvdata(phy, priv);

2.32.0

Applied, thanks

~Vinod