Re: clk/clk-next boot bisection: v5.3-rc1-79-g31f58d2f58cb on sun8i-h3-libretech-all-h3-cc

[Stephen Boyd]

Quoting kernelci.org bot (2019-08-14 20:35:25)
> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> * This automated bisection report was sent to you on the basis  *
> * that you may be involved with the breaking commit it has      *
> * found.  No manual investigation has been done to verify it,   *
> * and the root cause of the problem may be somewhere else.      *
> *                                                               *
> * If you do send a fix, please include this trailer:            *
> *   Reported-by: "kernelci.org bot" <bot@xxxxxxxxxxxx>          *
> *                                                               *
> * Hope this helps!                                              *
> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> 
> clk/clk-next boot bisection: v5.3-rc1-79-g31f58d2f58cb on sun8i-h3-libretech-all-h3-cc

If this is the only board that failed, great! Must be something in a
sun8i driver that uses the init structure after registration.

> 
> Summary:
>   Start:      31f58d2f58cb Merge branch 'clk-meson' into clk-next
>   Details:    https://kernelci.org/boot/id/5d54b9d159b514324cf1226e
>   Plain log:  https://storage.kernelci.org//clk/clk-next/v5.3-rc1-79-g31f58d2f58cb/arm/multi_v7_defconfig+CONFIG_SMP=n/gcc-8/lab-baylibre/boot-sun8i-h3-libretech-all-h3-cc.txt
>   HTML log:   https://storage.kernelci.org//clk/clk-next/v5.3-rc1-79-g31f58d2f58cb/arm/multi_v7_defconfig+CONFIG_SMP=n/gcc-8/lab-baylibre/boot-sun8i-h3-libretech-all-h3-cc.html
>   Result:     c82987e740d1 clk: Overwrite clk_hw::init with NULL during clk_register()
> 
> Checks:
>   revert:     PASS
>   verify:     PASS
> 
> Parameters:
>   Tree:       clk
>   URL:        https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git
>   Branch:     clk-next
>   Target:     sun8i-h3-libretech-all-h3-cc
>   CPU arch:   arm
>   Lab:        lab-baylibre
>   Compiler:   gcc-8
>   Config:     multi_v7_defconfig+CONFIG_SMP=n
>   Test suite: boot
> 
> Breaking commit found:
> 
> -------------------------------------------------------------------------------
> commit c82987e740d12be98b8ae8aa9221b8b9e2541271
> Author: Stephen Boyd <sboyd@xxxxxxxxxx>
> Date:   Wed Jul 31 12:35:17 2019 -0700
> 
>     clk: Overwrite clk_hw::init with NULL during clk_register()
>     
>     We don't want clk provider drivers to use the init structure after clk
>     registration time, but we leave a dangling reference to it by means of
>     clk_hw::init. Let's overwrite the member with NULL during clk_register()
>     so that this can't be used anymore after registration time.
>     
>     Cc: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
>     Cc: Doug Anderson <dianders@xxxxxxxxxxxx>
>     Signed-off-by: Stephen Boyd <sboyd@xxxxxxxxxx>
>     Link: https://lkml.kernel.org/r/20190731193517.237136-10-sboyd@xxxxxxxxxx
>     Reviewed-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index c0990703ce54..efac620264a2 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3484,9 +3484,9 @@ static int clk_cpy_name(const char **dst_p, const char *src, bool must_exist)
>         return 0;
>  }
>  
> -static int clk_core_populate_parent_map(struct clk_core *core)
> +static int clk_core_populate_parent_map(struct clk_core *core,
> +                                       const struct clk_init_data *init)
>  {
> -       const struct clk_init_data *init = core->hw->init;
>         u8 num_parents = init->num_parents;
>         const char * const *parent_names = init->parent_names;
>         const struct clk_hw **parent_hws = init->parent_hws;
> @@ -3566,6 +3566,14 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
>  {
>         int ret;
>         struct clk_core *core;
> +       const struct clk_init_data *init = hw->init;
> +
> +       /*
> +        * The init data is not supposed to be used outside of registration path.
> +        * Set it to NULL so that provider drivers can't use it either and so that
> +        * we catch use of hw->init early on in the core.
> +        */
> +       hw->init = NULL;
>  
>         core = kzalloc(sizeof(*core), GFP_KERNEL);
>         if (!core) {
> @@ -3573,17 +3581,17 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
>                 goto fail_out;
>         }
>  
> -       core->name = kstrdup_const(hw->init->name, GFP_KERNEL);
> +       core->name = kstrdup_const(init->name, GFP_KERNEL);
>         if (!core->name) {
>                 ret = -ENOMEM;
>                 goto fail_name;
>         }
>  
> -       if (WARN_ON(!hw->init->ops)) {
> +       if (WARN_ON(!init->ops)) {
>                 ret = -EINVAL;
>                 goto fail_ops;
>         }
> -       core->ops = hw->init->ops;
> +       core->ops = init->ops;
>  
>         if (dev && pm_runtime_enabled(dev))
>                 core->rpm_enabled = true;
> @@ -3592,13 +3600,13 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
>         if (dev && dev->driver)
>                 core->owner = dev->driver->owner;
>         core->hw = hw;
> -       core->flags = hw->init->flags;
> -       core->num_parents = hw->init->num_parents;
> +       core->flags = init->flags;
> +       core->num_parents = init->num_parents;
>         core->min_rate = 0;
>         core->max_rate = ULONG_MAX;
>         hw->core = core;
>  
> -       ret = clk_core_populate_parent_map(core);
> +       ret = clk_core_populate_parent_map(core, init);
>         if (ret)
>                 goto fail_parents;
>  
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 2ae7604783dd..214c75ed62ae 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -299,7 +299,8 @@ struct clk_init_data {
>   * into the clk API
>   *
>   * @init: pointer to struct clk_init_data that contains the init data shared
> - * with the common clock framework.
> + * with the common clock framework. This pointer will be set to NULL once
> + * a clk_register() variant is called on this clk_hw pointer.
>   */
>  struct clk_hw {
>         struct clk_core *core;
> -------------------------------------------------------------------------------
> 
> 
> Git bisection log:
> 
> -------------------------------------------------------------------------------
> git bisect start
> # good: [21a2f76849f16d5a48d205b68e923694bc93aaf3] Merge branch 'clk-fixes' into clk-next
> git bisect good 21a2f76849f16d5a48d205b68e923694bc93aaf3
> # bad: [31f58d2f58cb0a8fbf58af88b6a5133bed23bf9b] Merge branch 'clk-meson' into clk-next
> git bisect bad 31f58d2f58cb0a8fbf58af88b6a5133bed23bf9b
> # good: [1d97657a4794ab23b47bd9921978ddd82569fcf4] Merge branch 'v5.4/dt' into v5.4/drivers
> git bisect good 1d97657a4794ab23b47bd9921978ddd82569fcf4
> # bad: [c82987e740d12be98b8ae8aa9221b8b9e2541271] clk: Overwrite clk_hw::init with NULL during clk_register()
> git bisect bad c82987e740d12be98b8ae8aa9221b8b9e2541271
> # good: [e22cce5f419f3c5aa07c8b0d2f8860d49980dbae] clk: qcom: Don't reference clk_init_data after registration
> git bisect good e22cce5f419f3c5aa07c8b0d2f8860d49980dbae
> # good: [3445b1287ac6cf410ecd4536b880172b98e6133d] clk: socfpga: Don't reference clk_init_data after registration
> git bisect good 3445b1287ac6cf410ecd4536b880172b98e6133d
> # good: [735822a8b114f73289679178ff075b73facd4571] phy: ti: am654-serdes: Don't reference clk_init_data after registration
> git bisect good 735822a8b114f73289679178ff075b73facd4571
> # first bad commit: [c82987e740d12be98b8ae8aa9221b8b9e2541271] clk: Overwrite clk_hw::init with NULL during clk_register()
> -------------------------------------------------------------------------------