Oops, I forgot Reply-All and some messages missed list. Sorry about that. Jonas and I have found NULL pointer dereference in rockchip_usb2phy_clkout_ctl: https://github.com/u-boot/u-boot/blob/df2ed552f0b05591090369a7fe7ddc92439dea5c/drivers/phy/rockchip/phy-rockchip-inno-usb2.c#L177
Here rockchip_usb2phy_clkout_ctl tries to read from `dev_get_priv(parent)->phy_cfg` but it is NULL because rockchip_usb2phy_probe is not yet called. Johnas, I have dug the problem a little further and found something interesting. We have circular dependency between rockchip-inno-usb2 clock/phy initialisation here. The rockchip-inno-usb2 phy node also acts as a parent clock for itself. Parent clocks now are enabled as part of the probe process before dev->probe call, but this particular clock depends on phy. Here is relevant code: https://github.com/u-boot/u-boot/blob/df2ed552f0b05591090369a7fe7ddc92439dea5c/drivers/core/device.c#L572 Quick and dirty solution would be to just ignore rockchip_usb2phy_clk_enable when the phy node is not yet probed. USB works fine with this check added, exactly as before the recent parent clock enablement commit that introduced this issue: ac30d90f3367 ("clk: Ensure the parent clocks are enabled while reparenting"). Clean solution would probably be to break dependency cycle, but I'm not proficient enough with uboot codebase to propose such solution. I've tried to move some of rockchip_usb2phy_probe initialisation code to rockchip_usb2phy_of_to_plat to make it run before clock initialisation. But I have failed on `priv->reg_base = syscon_get_regmap(dev_get_parent(dev))` which needs a parent node to be probed (I unsure is it OK to probe parent from of_to_plat). If we settle on a quick fix, I propose to add to rockchip_usb2phy_clk_enable something like (Johnas, please let me know if it have some flaw over your patch): struct rockchip_usb2phy *parent_priv = dev_get_priv(dev_get_parent(clk->dev)); if (!parent_priv->phy_cfg) { return 0; } On Sat, May 24, 2025 at 8:43 PM Jonas Karlman <jo...@kwiboo.se> wrote: > > On 2025-05-24 18:02, Jonas Karlman wrote: > > Hi Alex, > > > > On 2025-05-24 16:20, Alex Shumsky wrote: > >> Remove incorrect assigned-clock-parents in rk3328-u-boot.dtsi. > >> This incorrect parent has become a problem since recent commit with parent > >> clock enablement. > >> > >> Currently assigned-clock-parents is not a CLK node, but rk3328-usb2phy. > >> It looks like a mistake in dts made a long time ago (~8 years ago). > > > > No, this is very much correct, u2phy is a clk provider as can be seen > > by the use of the '#clock-cells' prop so being able to use it as a > > parent clk is allowed and the device tree is correct. > > > >> > >> => usb start > >> starting USB... > >> Bus usb@ff580000: "Synchronous Abort" handler, esr 0x96000210, far 0x4 > > I have now tried to re-create this issue using latest master branch on a > rk3328 board, however I was only able to re-create a different issue. > > On what board and U-Boot version/commit are you seeing this issue? > > The issue I faced was due to only enable usb20_otg and not u2phy_otg, > something that could be fixed with following diff: > > diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > index 88b33de1b2a0..d6ce12b5e83d 100644 > --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > @@ -538,7 +538,7 @@ U_BOOT_DRIVER(rockchip_usb2phy_clock) = { > > U_BOOT_DRIVER(rockchip_usb2phy) = { > .name = "rockchip_usb2phy", > - .id = UCLASS_PHY, > + .id = UCLASS_NOP, > .of_match = rockchip_usb2phy_ids, > .probe = rockchip_usb2phy_probe, > .bind = rockchip_usb2phy_bind, > > Regards, > Jonas > > >> > >> Linux dts source reference: > >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c60c0373a5e85d8bd0bb026cd5440576249d2299 > >> > >> Fixes: ac30d90f3367 ("clk: Ensure the parent clocks are enabled while > >> reparenting") > >> Signed-off-by: Alex Shumsky <alexthr...@gmail.com> > >> --- > >> > >> arch/arm/dts/rk3328-u-boot.dtsi | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/arch/arm/dts/rk3328-u-boot.dtsi > >> b/arch/arm/dts/rk3328-u-boot.dtsi > >> index b0e50a973a..39dcc2313b 100644 > >> --- a/arch/arm/dts/rk3328-u-boot.dtsi > >> +++ b/arch/arm/dts/rk3328-u-boot.dtsi > >> @@ -150,6 +150,10 @@ > >> bootph-all; > >> }; > >> > >> +&u2phy { > >> + /delete-property/ assigned-clock-parents; > >> +}; > > > > We should fix the driver code and not modify the device tree to fit the > > code. > > > > The u2phy ofnode is tied to multiple uclass drivers so is it possible > > that the parent clock lookup from commit ac30d90f3367 ("clk: Ensure the > > parent clocks are enabled while reparenting") is not getting the > > UCLASS_CLK driver? > > > > Regards, > > Jonas > > > >> + > >> #ifdef CONFIG_ROCKCHIP_SPI_IMAGE > >> &binman { > >> simple-bin-spi { > > >
