[PATCH v1 01/11] clk: rockchip: rk3568: fix reset handler

[Simon Glass]

Hi Peter,

On Mon, 21 Feb 2022 at 18:31, Peter Geis <pgwipeout at gmail.com> wrote:
>
> The reset handler for rk3568 is missing its private data. This leads to
> an abort when a reset is triggered.
>
> Add the missing dev_set_priv to the rk3568 clk driver.
>
> Fixes: 4a262feba3a5 ("rockchip: rk3568: add clock driver")
>
> Signed-off-by: Peter Geis <pgwipeout at gmail.com>
> ---
>  drivers/clk/rockchip/clk_rk3568.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/clk/rockchip/clk_rk3568.c b/drivers/clk/rockchip/clk_rk3568.c
> index d5e45e7602c7..c83ae22dc302 100644
> --- a/drivers/clk/rockchip/clk_rk3568.c
> +++ b/drivers/clk/rockchip/clk_rk3568.c
> @@ -14,6 +14,7 @@
>  #include <asm/arch-rockchip/clock.h>
>  #include <asm/arch-rockchip/hardware.h>
>  #include <asm/io.h>
> +#include <dm/device-internal.h>
>  #include <dm/lists.h>
>  #include <dt-bindings/clock/rk3568-cru.h>
>
> @@ -2934,6 +2935,7 @@ static int rk3568_clk_bind(struct udevice *dev)
>                                                     glb_srst_fst);
>                 priv->glb_srst_snd_value = offsetof(struct rk3568_cru,
>                                                     glb_srsr_snd);
> +               dev_set_priv(sys_child, priv);
>         }
>
>  #if CONFIG_IS_ENABLED(RESET_ROCKCHIP)
> --
> 2.25.1
>

You must not access priv data in the bind() handler as it doesn't
exist yet. The malloc() in that function is incorrect.

You certainly must not set the priv data in there. See the lifecycle
docs in driver model for how things are supposed to work.

You can use plat data, so could move glb_srst_fst_value and
glb_srst_snd_value to struct rk3568_clk_plat, although oddly that
struct only exists if OF_PLATDATA is used.

Quite confusing.

Regards,
Simon