[PATCH] rockchip: nanopi-r4s: Fix ehci usb error

[Justin Klaassen]

 Hi Jonas,

On Apr 18, 2024 at 07:52:03, Jonas Karlman <jonas at kwiboo.se> wrote:

> Hi Justin,
>
> On 2024-04-18 03:45, Justin Klaassen wrote:
>
> The ehci_generic driver always failed with the error:
>
> Bus usb at fe380000: ehci_generic usb at fe380000:
>
> ... Failed to get clocks (ret=-19)
>
> Port not available.
>
> Bus usb at fe3c0000: ehci_generic usb at fe3c0000:
>
> ... Failed to get clocks (ret=-19)
>
> Port not available.
>
>
> This error is resolved by enabling the CONFIG_PHY_ROCKCHIP_INNO_USB2
>
> and CONFIG_PHY_ROCKCHIP_TYPEC driver options.
>
>
> Is your issue also fixed with my series "rockchip: rk3399: Sync DT with
> linux v6.8 and update defconfigs" [1] containing patch [2]? A v2 of that
> series should hit the mailing list sometime tomorrow.
>
> [1] https://patchwork.ozlabs.org/cover/1918319/
> [2] https://patchwork.ozlabs.org/patch/1918346/
>
>
It looks like your patch does include all the necessary config changes from
my patch, so that should resolve the issue.


> The CONFIG_DM_RESET option must also be enabled, otherwise the
>
> ehci_generic driver will fail with the error:
>
> Bus usb at fe380000: ehci_generic usb at fe380000:
>
> ... Failed to get resets (err=-524)
>
> probe failed, error -524
>
> Bus usb at fe3c0000: ehci_generic usb at fe3c0000:
>
> ... Failed to get resets (err=-524)
>
> probe failed, error -524
>
>
> Additionally the u2phy1_host device was previously disabled in the
>
> nanopi-r4s device tree and must be enabled to avoid a crash in the
>
> ehci_generic driver:
>
> Bus usb at fe380000: USB EHCI 1.00
>
> Bus usb at fe3c0000: "Synchronous Abort" handler, esr 0x96000010, far 0x0
>
>
> With these changes the usb ports can now be initialized correctly by
>
> both the ehci_generic and xhci_generic drivers:
>
> Bus usb at fe380000: USB EHCI 1.00
>
> Bus usb at fe3c0000: USB EHCI 1.00
>
> Bus usb at fe800000: Register 2000140 NbrPorts 2
>
> Starting the controller
>
> USB XHCI 1.10
>
> Bus usb at fe900000: Register 2000140 NbrPorts 2
>
> Starting the controller
>
> USB XHCI 1.10
>
> scanning bus usb at fe380000 for devices... 1 USB Device(s) found
>
> scanning bus usb at fe3c0000 for devices... 1 USB Device(s) found
>
> scanning bus usb at fe800000 for devices... 1 USB Device(s) found
>
> scanning bus usb at fe900000 for devices... 1 USB Device(s) found
>
>        scanning usb for storage devices... 0 Storage Device(s) found
>
>
> Signed-off-by: Justin Klaassen <justin at tidylabs.app>
>
> Cc: Simon Glass <sjg at chromium.org>
>
> Cc: Philipp Tomsich <philipp.tomsich at vrull.eu>
>
> Cc: Kever Yang <kever.yang at rock-chips.com>
>
> ---
>
>  arch/arm/dts/rk3399-nanopi-r4s.dts  | 2 +-
>
>  configs/nanopi-r4s-rk3399_defconfig | 3 +++
>
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
>
> diff --git a/arch/arm/dts/rk3399-nanopi-r4s.dts
> b/arch/arm/dts/rk3399-nanopi-r4s.dts
>
> index cef4d18b599..a992a6ac5e9 100644
>
> --- a/arch/arm/dts/rk3399-nanopi-r4s.dts
>
> +++ b/arch/arm/dts/rk3399-nanopi-r4s.dts
>
> @@ -117,7 +117,7 @@
>
>  };
>
>
>
>  &u2phy1_host {
>
> - status = "disabled";
>
> + phy-supply = <&vdd_5v>;
>
>
> This does not match upstream linux device tree file. If this change is
> needed please submit a change to linux kernel, else make a U-Boot
> specific change in rk3399-nanopi-r4s-u-boot.dtsi.
>

I tested your patch (without this change) and it does result in the same
crash:

Bus usb at fe380000: USB EHCI 1.00
Bus usb at fe3c0000: "Synchronous Abort" handler, esr 0x96000010, far 0x0
elr: 0000000000241c7c lr : 0000000000241c68 (reloc)
elr: 00000000f3f72c7c lr : 00000000f3f72c68
x0 : 0000000000000000 x1 : 0000000000007d08
x2 : 0000000000007d0c x3 : 0000000000007d40
x4 : 000000000000f6c4 x5 : 0000000000010ab0
x6 : 0000000000007d0c x7 : 00000000f1f182e0
x8 : 0000000000007d08 x9 : 00000000f1f179dc
x10: 0000000000000002 x11: 0000000000007c8c
x12: 00000000f1f17aac x13: 00000000f1f182e0
x14: 0000000000000002 x15: 00000000f1f17684
x16: 00000000f3f71b94 x17: 00000000409ff249
x18: 00000000f1f28da0 x19: 00000000f1f5b2a0
x20: 0000000000000000 x21: 00000000f1f5b100
x22: 00000000f1f5b280 x23: 00000000f1f5b290
x24: 00000000f1f5b2a0 x25: 0000000000000000
x26: 00000000f3fd2d3d x27: 0000000000000001
x28: 0000000000000000 x29: 00000000f1f17ac0

While this change should probably be upstreamed to the linux device tree
file, I think we need something more immediate to prevent this crash/boot
loop once your change lands.

Do you prefer to incorporate this change into your patch or do you want me
to update this patch?

Thanks,
Justin


> Regards,
> Jonas
>
>  };
>
>
>
>  &uart0 {
>
> diff --git a/configs/nanopi-r4s-rk3399_defconfig
> b/configs/nanopi-r4s-rk3399_defconfig
>
> index ea01d323541..d8c34dc48cb 100644
>
> --- a/configs/nanopi-r4s-rk3399_defconfig
>
> +++ b/configs/nanopi-r4s-rk3399_defconfig
>
> @@ -5,6 +5,7 @@ CONFIG_ARCH_ROCKCHIP=y
>
>  CONFIG_NR_DRAM_BANKS=1
>
>  CONFIG_ENV_OFFSET=0x3F8000
>
>  CONFIG_DEFAULT_DEVICE_TREE="rk3399-nanopi-r4s"
>
> +CONFIG_DM_RESET=y
>
>  CONFIG_ROCKCHIP_RK3399=y
>
>  CONFIG_TARGET_EVB_RK3399=y
>
>  CONFIG_DEBUG_UART_BASE=0xFF1A0000
>
> @@ -36,6 +37,8 @@ CONFIG_MMC_SDHCI=y
>
>  CONFIG_MMC_SDHCI_ROCKCHIP=y
>
>  CONFIG_ETH_DESIGNWARE=y
>
>  CONFIG_GMAC_ROCKCHIP=y
>
> +CONFIG_PHY_ROCKCHIP_INNO_USB2=y
>
> +CONFIG_PHY_ROCKCHIP_TYPEC=y
>
>  CONFIG_PMIC_RK8XX=y
>
>  CONFIG_REGULATOR_PWM=y
>
>  CONFIG_REGULATOR_RK8XX=y
>
>
>