[v3,3/7] clk: Add of_clk_hw_register() API for early clk drivers
diff mbox series

Message ID 20190404215344.6330-4-sboyd@kernel.org
State New
Headers show
Series
  • Rewrite clk parent handling
Related show

Commit Message

Stephen Boyd April 4, 2019, 9:53 p.m. UTC
In some circumstances drivers register clks early and don't have access
to a struct device because the device model isn't initialized yet. Add
an API to let drivers register clks associated with a struct device_node
so that these drivers can participate in getting parent clks through DT.

Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Jeffrey Hugo <jhugo@codeaurora.org>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
 drivers/clk/clk.c            | 26 +++++++++++++++++++++++---
 include/linux/clk-provider.h |  1 +
 2 files changed, 24 insertions(+), 3 deletions(-)

Comments

Jeffrey Hugo April 8, 2019, 9:46 p.m. UTC | #1
On 4/4/2019 3:53 PM, Stephen Boyd wrote:
> In some circumstances drivers register clks early and don't have access
> to a struct device because the device model isn't initialized yet. Add
> an API to let drivers register clks associated with a struct device_node
> so that these drivers can participate in getting parent clks through DT.

NACK.  This patch broke boot for me.  I had to pull the below from JTAG. 
  What do you need to debug this?

[    0.000000] Unable to handle kernel read from unreadable memory at 
virtual address 0000000000000280
[    0.000000] Mem abort info:
[    0.000000]   ESR = 0x96000004
[    0.000000]   Exception class = DABT (current EL), IL = 32 bits
[    0.000000]   SET = 0, FnV = 0
[    0.000000]   EA = 0, S1PTW = 0
[    0.000000] Data abort info:
[    0.000000]   ISV = 0, ISS = 0x00000004
[    0.000000]   CM = 0, WnR = 0
[    0.000000] [0000000000000280] user address but active_mm is swapper
[    0.000000] Internal error: Oops: 96000004 [#1] SMP
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.1.0-rc1+ #453
[    0.000000] Hardware name: Qualcomm Technologies, Inc. MSM8998 v1 MTP 
(DT)
[    0.000000] pstate: 60400085 (nZCv daIf +PAN -UAO)
[    0.000000] pc : clk_hw_register+0x24/0x40
[    0.000000] lr : clk_hw_register_fixed_rate_with_accuracy+0xac/0x100
[    0.000000] sp : ffff0000117d3df0
[    0.000000] x29: ffff0000117d3df0 x28: ffff8000fdc7cd28
[    0.000000] x27: dead000000000100 x26: dead000000000200
[    0.000000] x25: 0000000000000001 x24: ffff7dfffe80047c
[    0.000000] x23: 000000000124f800 x22: 0000000000000000
[    0.000000] x21: 0000000000000000 x20: ffff8000f9524c00
[    0.000000] x19: 0000000000000000 x18: ffffffffffffffff
[    0.000000] x17: 0000000000000000 x16: 0000000000000000
[    0.000000] x15: ffff0000117dc708 x14: ffffffffffffffff
[    0.000000] x13: 0000000000000020 x12: 0101010101010101
[    0.000000] x11: 7f7f7f7f7f7f7f7f x10: 02fefeff01fefeff
[    0.000000] x9 : 0000000000000000 x8 : ffff8000f9524c80
[    0.000000] x7 : 0000000000000000 x6 : 000000000000003f
[    0.000000] x5 : 0000000000000040 x4 : 0000000000000000
[    0.000000] x3 : 0000000000000000 x2 : ffff8000f9524c00
[    0.000000] x1 : ffff8000f9524c00 x0 : 0000000000000000
[    0.000000] Process swapper/0 (pid: 0, stack limit = 0x(____ptrval____))
[    0.000000] Call trace:
[    0.000000]  clk_hw_register+0x24/0x40
[    0.000000]  clk_hw_register_fixed_rate_with_accuracy+0xac/0x100
[    0.000000]  _of_fixed_clk_setup+0xa8/0x120
[    0.000000]  of_fixed_clk_setup+0x20/0x2c
[    0.000000]  of_clk_init+0x19c/0x254
[    0.000000]  time_init+0x18/0x4c
[    0.000000]  start_kernel+0x394/0x524
[    0.000000] Code: aa1e03e0 d503201f aa1403e2 aa1303e0 (f9414261)
[    0.000000] ---[ end trace 5d02af5e3cbab7bf ]---
[    0.000000] Kernel panic - not syncing: Attempted to kill the idle task!
[    0.000000] ---[ end Kernel panic - not syncing: Attempted to kill 
the idle task! ]---

> 
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Jeffrey Hugo <jhugo@codeaurora.org>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---
>   drivers/clk/clk.c            | 26 +++++++++++++++++++++++---
>   include/linux/clk-provider.h |  1 +
>   2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index d27775a73e67..4971e9651ca5 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -45,6 +45,7 @@ struct clk_core {
>   	struct clk_hw		*hw;
>   	struct module		*owner;
>   	struct device		*dev;
> +	struct device_node	*of_node;
>   	struct clk_core		*parent;
>   	const char		**parent_names;
>   	struct clk_core		**parents;
> @@ -3313,7 +3314,8 @@ struct clk *clk_hw_create_clk(struct device *dev, struct clk_hw *hw,
>   	return clk;
>   }
>   
> -static struct clk *__clk_register(struct device *dev, struct clk_hw *hw)
> +static struct clk *
> +__clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
>   {
>   	int i, ret;
>   	struct clk_core *core;
> @@ -3339,6 +3341,7 @@ static struct clk *__clk_register(struct device *dev, struct clk_hw *hw)
>   	if (dev && pm_runtime_enabled(dev))
>   		core->rpm_enabled = true;
>   	core->dev = dev;
> +	core->of_node = np;
>   	if (dev && dev->driver)
>   		core->owner = dev->driver->owner;
>   	core->hw = hw;
> @@ -3429,7 +3432,7 @@ static struct clk *__clk_register(struct device *dev, struct clk_hw *hw)
>    */
>   struct clk *clk_register(struct device *dev, struct clk_hw *hw)
>   {
> -	return __clk_register(dev, hw);
> +	return __clk_register(dev, dev->of_node, hw);
>   }
>   EXPORT_SYMBOL_GPL(clk_register);
>   
> @@ -3445,10 +3448,27 @@ EXPORT_SYMBOL_GPL(clk_register);
>    */
>   int clk_hw_register(struct device *dev, struct clk_hw *hw)
>   {
> -	return PTR_ERR_OR_ZERO(__clk_register(dev, hw));
> +	return PTR_ERR_OR_ZERO(__clk_register(dev, dev->of_node, hw));
>   }
>   EXPORT_SYMBOL_GPL(clk_hw_register);
>   
> +/*
> + * of_clk_hw_register - register a clk_hw and return an error code
> + * @node: device_node of device that is registering this clock
> + * @hw: link to hardware-specific clock data
> + *
> + * of_clk_hw_register() is the primary interface for populating the clock tree
> + * with new clock nodes when a struct device is not available, but a struct
> + * device_node is. It returns an integer equal to zero indicating success or
> + * less than zero indicating failure. Drivers must test for an error code after
> + * calling of_clk_hw_register().
> + */
> +int of_clk_hw_register(struct device_node *node, struct clk_hw *hw)
> +{
> +	return PTR_ERR_OR_ZERO(__clk_register(NULL, node, hw));
> +}
> +EXPORT_SYMBOL_GPL(of_clk_hw_register);
> +
>   /* Free memory allocated for a clock. */
>   static void __clk_release(struct kref *ref)
>   {
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index b7cf80a71293..7d2d97e15b76 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -773,6 +773,7 @@ struct clk *devm_clk_register(struct device *dev, struct clk_hw *hw);
>   
>   int __must_check clk_hw_register(struct device *dev, struct clk_hw *hw);
>   int __must_check devm_clk_hw_register(struct device *dev, struct clk_hw *hw);
> +int __must_check of_clk_hw_register(struct device_node *node, struct clk_hw *hw);
>   
>   void clk_unregister(struct clk *clk);
>   void devm_clk_unregister(struct device *dev, struct clk *clk);
>
Stephen Boyd April 10, 2019, 4:49 p.m. UTC | #2
Quoting Jeffrey Hugo (2019-04-08 14:46:11)
> On 4/4/2019 3:53 PM, Stephen Boyd wrote:
> > In some circumstances drivers register clks early and don't have access
> > to a struct device because the device model isn't initialized yet. Add
> > an API to let drivers register clks associated with a struct device_node
> > so that these drivers can participate in getting parent clks through DT.
> 
> NACK.  This patch broke boot for me.  I had to pull the below from JTAG. 

No early printk support on this board? Sadness.

>   What do you need to debug this?
> 

Hopefully nothing.

> [    0.000000] Unable to handle kernel read from unreadable memory at 
> virtual address 0000000000000280

I guess you pass NULL as dev to clk_hw_register(). Will fix! Thanks for
testing.

> [    0.000000] Mem abort info:
Stephen Boyd April 10, 2019, 4:53 p.m. UTC | #3
Quoting Jeffrey Hugo (2019-04-08 14:46:11)
> On 4/4/2019 3:53 PM, Stephen Boyd wrote:
> > In some circumstances drivers register clks early and don't have access
> > to a struct device because the device model isn't initialized yet. Add
> > an API to let drivers register clks associated with a struct device_node
> > so that these drivers can participate in getting parent clks through DT.
> 
> NACK.  This patch broke boot for me.  I had to pull the below from JTAG. 
>   What do you need to debug this?
> 

Here's a patch to try to squash in:

---8<----
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 709492d901a1..040ce083c89e 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3662,7 +3662,7 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
  */
 struct clk *clk_register(struct device *dev, struct clk_hw *hw)
 {
-	return __clk_register(dev, dev->of_node, hw);
+	return __clk_register(dev, dev_of_node(dev), hw);
 }
 EXPORT_SYMBOL_GPL(clk_register);
 
@@ -3678,7 +3678,7 @@ EXPORT_SYMBOL_GPL(clk_register);
  */
 int clk_hw_register(struct device *dev, struct clk_hw *hw)
 {
-	return PTR_ERR_OR_ZERO(__clk_register(dev, dev->of_node, hw));
+	return PTR_ERR_OR_ZERO(__clk_register(dev, dev_of_node(dev), hw));
 }
 EXPORT_SYMBOL_GPL(clk_hw_register);
Jeffrey Hugo April 10, 2019, 7:39 p.m. UTC | #4
On 4/10/2019 10:53 AM, Stephen Boyd wrote:
> Quoting Jeffrey Hugo (2019-04-08 14:46:11)
>> On 4/4/2019 3:53 PM, Stephen Boyd wrote:
>>> In some circumstances drivers register clks early and don't have access
>>> to a struct device because the device model isn't initialized yet. Add
>>> an API to let drivers register clks associated with a struct device_node
>>> so that these drivers can participate in getting parent clks through DT.
>>
>> NACK.  This patch broke boot for me.  I had to pull the below from JTAG.
>>    What do you need to debug this?
>>
> 
> Here's a patch to try to squash in:

No dice.  Same issue.

> 
> ---8<----
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 709492d901a1..040ce083c89e 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3662,7 +3662,7 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
>    */
>   struct clk *clk_register(struct device *dev, struct clk_hw *hw)
>   {
> -	return __clk_register(dev, dev->of_node, hw);
> +	return __clk_register(dev, dev_of_node(dev), hw);
>   }
>   EXPORT_SYMBOL_GPL(clk_register);
>   
> @@ -3678,7 +3678,7 @@ EXPORT_SYMBOL_GPL(clk_register);
>    */
>   int clk_hw_register(struct device *dev, struct clk_hw *hw)
>   {
> -	return PTR_ERR_OR_ZERO(__clk_register(dev, dev->of_node, hw));
> +	return PTR_ERR_OR_ZERO(__clk_register(dev, dev_of_node(dev), hw));
>   }
>   EXPORT_SYMBOL_GPL(clk_hw_register);
>   
>
Stephen Boyd April 10, 2019, 9:45 p.m. UTC | #5
Quoting Jeffrey Hugo (2019-04-10 12:39:16)
> On 4/10/2019 10:53 AM, Stephen Boyd wrote:
> > Quoting Jeffrey Hugo (2019-04-08 14:46:11)
> >> On 4/4/2019 3:53 PM, Stephen Boyd wrote:
> >>> In some circumstances drivers register clks early and don't have access
> >>> to a struct device because the device model isn't initialized yet. Add
> >>> an API to let drivers register clks associated with a struct device_node
> >>> so that these drivers can participate in getting parent clks through DT.
> >>
> >> NACK.  This patch broke boot for me.  I had to pull the below from JTAG.
> >>    What do you need to debug this?
> >>
> > 
> > Here's a patch to try to squash in:
> 
> No dice.  Same issue.

Argh! dev_of_node() doesn't check for NULL device. :-/ I want to be
extremely lazy! Let's get this merged too. Thanks for the testing.

---8<---
diff --git a/include/linux/device.h b/include/linux/device.h
index b425a7ee04ce..0370dd0b3ae7 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1231,7 +1231,7 @@ static inline void device_lock_assert(struct device *dev)
 
 static inline struct device_node *dev_of_node(struct device *dev)
 {
-	if (!IS_ENABLED(CONFIG_OF))
+	if (!IS_ENABLED(CONFIG_OF) || !dev)
 		return NULL;
 	return dev->of_node;
 }
Jeffrey Hugo April 10, 2019, 10:18 p.m. UTC | #6
On 4/10/2019 3:45 PM, Stephen Boyd wrote:
> Quoting Jeffrey Hugo (2019-04-10 12:39:16)
>> On 4/10/2019 10:53 AM, Stephen Boyd wrote:
>>> Quoting Jeffrey Hugo (2019-04-08 14:46:11)
>>>> On 4/4/2019 3:53 PM, Stephen Boyd wrote:
>>>>> In some circumstances drivers register clks early and don't have access
>>>>> to a struct device because the device model isn't initialized yet. Add
>>>>> an API to let drivers register clks associated with a struct device_node
>>>>> so that these drivers can participate in getting parent clks through DT.
>>>>
>>>> NACK.  This patch broke boot for me.  I had to pull the below from JTAG.
>>>>     What do you need to debug this?
>>>>
>>>
>>> Here's a patch to try to squash in:
>>
>> No dice.  Same issue.
> 
> Argh! dev_of_node() doesn't check for NULL device. :-/ I want to be
> extremely lazy! Let's get this merged too. Thanks for the testing.

Fixed.  Will continue to test the rest of the series and let you know.
Jeffrey Hugo April 11, 2019, 5:32 p.m. UTC | #7
On 4/10/2019 4:18 PM, Jeffrey Hugo wrote:
> On 4/10/2019 3:45 PM, Stephen Boyd wrote:
>> Quoting Jeffrey Hugo (2019-04-10 12:39:16)
>>> On 4/10/2019 10:53 AM, Stephen Boyd wrote:
>>>> Quoting Jeffrey Hugo (2019-04-08 14:46:11)
>>>>> On 4/4/2019 3:53 PM, Stephen Boyd wrote:
>>>>>> In some circumstances drivers register clks early and don't have 
>>>>>> access
>>>>>> to a struct device because the device model isn't initialized yet. 
>>>>>> Add
>>>>>> an API to let drivers register clks associated with a struct 
>>>>>> device_node
>>>>>> so that these drivers can participate in getting parent clks 
>>>>>> through DT.
>>>>>
>>>>> NACK.  This patch broke boot for me.  I had to pull the below from 
>>>>> JTAG.
>>>>>     What do you need to debug this?
>>>>>
>>>>
>>>> Here's a patch to try to squash in:
>>>
>>> No dice.  Same issue.
>>
>> Argh! dev_of_node() doesn't check for NULL device. :-/ I want to be
>> extremely lazy! Let's get this merged too. Thanks for the testing.
> 
> Fixed.  Will continue to test the rest of the series and let you know.
> 

With the two changes we discussed, for the entire series:
Tested-by: Jeffrey Hugo <jhugo@codeaurora.org>

Patch
diff mbox series

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index d27775a73e67..4971e9651ca5 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -45,6 +45,7 @@  struct clk_core {
 	struct clk_hw		*hw;
 	struct module		*owner;
 	struct device		*dev;
+	struct device_node	*of_node;
 	struct clk_core		*parent;
 	const char		**parent_names;
 	struct clk_core		**parents;
@@ -3313,7 +3314,8 @@  struct clk *clk_hw_create_clk(struct device *dev, struct clk_hw *hw,
 	return clk;
 }
 
-static struct clk *__clk_register(struct device *dev, struct clk_hw *hw)
+static struct clk *
+__clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
 {
 	int i, ret;
 	struct clk_core *core;
@@ -3339,6 +3341,7 @@  static struct clk *__clk_register(struct device *dev, struct clk_hw *hw)
 	if (dev && pm_runtime_enabled(dev))
 		core->rpm_enabled = true;
 	core->dev = dev;
+	core->of_node = np;
 	if (dev && dev->driver)
 		core->owner = dev->driver->owner;
 	core->hw = hw;
@@ -3429,7 +3432,7 @@  static struct clk *__clk_register(struct device *dev, struct clk_hw *hw)
  */
 struct clk *clk_register(struct device *dev, struct clk_hw *hw)
 {
-	return __clk_register(dev, hw);
+	return __clk_register(dev, dev->of_node, hw);
 }
 EXPORT_SYMBOL_GPL(clk_register);
 
@@ -3445,10 +3448,27 @@  EXPORT_SYMBOL_GPL(clk_register);
  */
 int clk_hw_register(struct device *dev, struct clk_hw *hw)
 {
-	return PTR_ERR_OR_ZERO(__clk_register(dev, hw));
+	return PTR_ERR_OR_ZERO(__clk_register(dev, dev->of_node, hw));
 }
 EXPORT_SYMBOL_GPL(clk_hw_register);
 
+/*
+ * of_clk_hw_register - register a clk_hw and return an error code
+ * @node: device_node of device that is registering this clock
+ * @hw: link to hardware-specific clock data
+ *
+ * of_clk_hw_register() is the primary interface for populating the clock tree
+ * with new clock nodes when a struct device is not available, but a struct
+ * device_node is. It returns an integer equal to zero indicating success or
+ * less than zero indicating failure. Drivers must test for an error code after
+ * calling of_clk_hw_register().
+ */
+int of_clk_hw_register(struct device_node *node, struct clk_hw *hw)
+{
+	return PTR_ERR_OR_ZERO(__clk_register(NULL, node, hw));
+}
+EXPORT_SYMBOL_GPL(of_clk_hw_register);
+
 /* Free memory allocated for a clock. */
 static void __clk_release(struct kref *ref)
 {
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index b7cf80a71293..7d2d97e15b76 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -773,6 +773,7 @@  struct clk *devm_clk_register(struct device *dev, struct clk_hw *hw);
 
 int __must_check clk_hw_register(struct device *dev, struct clk_hw *hw);
 int __must_check devm_clk_hw_register(struct device *dev, struct clk_hw *hw);
+int __must_check of_clk_hw_register(struct device_node *node, struct clk_hw *hw);
 
 void clk_unregister(struct clk *clk);
 void devm_clk_unregister(struct device *dev, struct clk *clk);