[PATCH v2] rockchip: rockchip-inno-usb2: Fix Synchronous Abort on usb start

[Alex Shumsky]

Hi Jonas,

This time I had difficulties to reproduce Synchronous Abort on a real board, but
then I spotted a difference with my previous tests.
"usb start" abort happen when I boot upstream/current u-boot proper from
SD-card by older SPL/TPL installed on eMMC (stock/proprietary u-boot from 2019
installed by STB manufacturer).
There is no Sync Abort with upstream SPL/TPL. Looks like NULL dereference are
still there but some SPL/TPL init code prevents it from aborting.

On Thu, Jul 3, 2025 at 9:05 AM Alex Shumsky <alexthreed at gmail.com> wrote:
>
> Fix NULL pointer dereference that happen when rockchip-inno-usb2 clock
> enabled before device probe. This early clock enable call happen in process
> of parent clock activation added in ac30d90f3367.
>
> Fixes: 229218373c22 ("phy: rockchip-inno-usb2: Add support for clkout_ctl_phy").
> Fixes: ac30d90f3367 ("clk: Ensure the parent clocks are enabled while reparenting")
> Co-authored-by: Jonas Karlman <jonas at kwiboo.se>
> Signed-off-by: Alex Shumsky <alexthreed at gmail.com>
> ---
>
> Changes in v2:
> - rework patch (add NULL check to code instead of dts modification)
>
>  drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> index 88b33de1b2..3cc5956aed 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> @@ -167,20 +167,27 @@ static struct phy_ops rockchip_usb2phy_ops = {
>         .of_xlate = rockchip_usb2phy_of_xlate,
>  };
>
> -static void rockchip_usb2phy_clkout_ctl(struct clk *clk, struct regmap **base,
> -                                       const struct usb2phy_reg **clkout_ctl)
> +static int rockchip_usb2phy_clkout_ctl(struct clk *clk, struct regmap **base,
> +                                      const struct usb2phy_reg **clkout_ctl)
>  {
>         struct udevice *parent = dev_get_parent(clk->dev);
>         struct rockchip_usb2phy *priv = dev_get_priv(parent);
>         const struct rockchip_usb2phy_cfg *phy_cfg = priv->phy_cfg;
>
> -       if (priv->phy_cfg->clkout_ctl_phy.enable) {
> +       // phy_cfg can be NULL if this function called before probe (when parent
> +       // clocks are enabled)
> +       if (!phy_cfg)
> +               return -EINVAL;
> +
> +       if (phy_cfg->clkout_ctl_phy.enable) {
>                 *base = priv->phy_base;
>                 *clkout_ctl = &phy_cfg->clkout_ctl_phy;
>         } else {
>                 *base = priv->reg_base;
>                 *clkout_ctl = &phy_cfg->clkout_ctl;
>         }
> +
> +       return 0;
>  }
>
>  /**
> @@ -206,7 +213,8 @@ int rockchip_usb2phy_clk_enable(struct clk *clk)
>         const struct usb2phy_reg *clkout_ctl;
>         struct regmap *base;
>
> -       rockchip_usb2phy_clkout_ctl(clk, &base, &clkout_ctl);
> +       if (rockchip_usb2phy_clkout_ctl(clk, &base, &clkout_ctl))
> +               return -ENOSYS;
>
>         /* turn on 480m clk output if it is off */
>         if (!property_enabled(base, clkout_ctl)) {
> @@ -230,7 +238,8 @@ int rockchip_usb2phy_clk_disable(struct clk *clk)
>         const struct usb2phy_reg *clkout_ctl;
>         struct regmap *base;
>
> -       rockchip_usb2phy_clkout_ctl(clk, &base, &clkout_ctl);
> +       if (rockchip_usb2phy_clkout_ctl(clk, &base, &clkout_ctl))
> +               return -ENOSYS;
>
>         /* turn off 480m clk output */
>         property_enable(base, clkout_ctl, false);
> --
> 2.43.0
>
> base-commit: 7027b445cc0bfb86204ecb1f1fe596f5895048d9
> branch: fix-rk3328-usb2phy-clock-v2