diff mbox series

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

Message ID 5d54d2fd.1c69fb81.e13e5.7422@mx.google.com
State New
Headers show
Series clk/clk-next boot bisection: v5.3-rc1-79-g31f58d2f58cb on sun8i-h3-libretech-all-h3-cc | expand

Commit Message

kernelci.org bot Aug. 15, 2019, 3:35 a.m. UTC
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
* 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@kernelci.org>          *
*                                                               *
* Hope this helps!                                              *
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *

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

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@kernel.org>
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@linaro.org>
    Cc: Doug Anderson <dianders@chromium.org>
    Signed-off-by: Stephen Boyd <sboyd@kernel.org>

    Link: https://lkml.kernel.org/r/20190731193517.237136-10-sboyd@kernel.org
    Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>


-------------------------------------------------------------------------------


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()
-------------------------------------------------------------------------------

Comments

Stephen Boyd Aug. 15, 2019, 4:02 a.m. UTC | #1
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@kernelci.org>          *

> *                                                               *

> * 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@kernel.org>

> 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@linaro.org>

>     Cc: Doug Anderson <dianders@chromium.org>

>     Signed-off-by: Stephen Boyd <sboyd@kernel.org>

>     Link: https://lkml.kernel.org/r/20190731193517.237136-10-sboyd@kernel.org

>     Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

> 

> 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()

> -------------------------------------------------------------------------------
Mark Brown Aug. 15, 2019, 11:26 a.m. UTC | #2
On Wed, Aug 14, 2019 at 09:02:20PM -0700, Stephen Boyd wrote:
> Quoting kernelci.org bot (2019-08-14 20:35:25)


> > 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.


The infrastructure suppresses duplicate-seeming bisections so I'd not
count on it, check the reports on the web site.
Stephen Boyd Aug. 15, 2019, 3:16 p.m. UTC | #3
Quoting Mark Brown (2019-08-15 04:26:14)
> On Wed, Aug 14, 2019 at 09:02:20PM -0700, Stephen Boyd wrote:

> > Quoting kernelci.org bot (2019-08-14 20:35:25)

> 

> > > 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.

> 

> The infrastructure suppresses duplicate-seeming bisections so I'd not

> count on it, check the reports on the web site.


Hmm ok. I can remove the change from -next, but I'd still like to figure
out what is using the init pointer after registration. Is there a way to
get earlycon logs?
Stephen Boyd Aug. 15, 2019, 4:01 p.m. UTC | #4
Quoting Stephen Boyd (2019-08-15 08:16:15)
> Quoting Mark Brown (2019-08-15 04:26:14)

> > On Wed, Aug 14, 2019 at 09:02:20PM -0700, Stephen Boyd wrote:

> > > Quoting kernelci.org bot (2019-08-14 20:35:25)

> > 

> > > > 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.

> > 

> > The infrastructure suppresses duplicate-seeming bisections so I'd not

> > count on it, check the reports on the web site.

> 

> Hmm ok. I can remove the change from -next, but I'd still like to figure

> out what is using the init pointer after registration. Is there a way to

> get earlycon logs?


Ok I sent a patch series which may solve the problem for Allwinner.
Stephen Boyd Aug. 15, 2019, 6:02 p.m. UTC | #5
Quoting Mark Brown (2019-08-15 04:26:14)
> On Wed, Aug 14, 2019 at 09:02:20PM -0700, Stephen Boyd wrote:

> > Quoting kernelci.org bot (2019-08-14 20:35:25)

> 

> > > 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.

> 

> The infrastructure suppresses duplicate-seeming bisections so I'd not

> count on it, check the reports on the web site.


And there's something wrong with the OMAP4 panda board and tegra nyan.
Maybe Tony has seen it.
diff mbox series

Patch

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;