Linux-Clk Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] clk: Use parent node pointer during registration if necessary
@ 2019-12-30 19:04 Stephen Boyd
  2020-01-03  0:17 ` Niklas Cassel
  2020-01-03  7:39 ` Bjorn Andersson
  0 siblings, 2 replies; 4+ messages in thread
From: Stephen Boyd @ 2019-12-30 19:04 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-kernel, linux-clk, Bjorn Andersson, Niklas Cassel

Sometimes clk drivers are attached to devices which are children of a
parent device that is connected to a node in DT. This happens when
devices are MFD-ish and the parent device driver mostly registers child
devices to match against drivers placed in their respective subsystem
directories like drivers/clk, drivers/regulator, etc. When the clk
driver calls clk_register() with a device pointer, that struct device
pointer won't have a device_node associated with it because it was
created purely in software as a way to partition logic to a subsystem.

This causes problems for the way we find parent clks for the clks
registered by these child devices because we look at the registering
device's device_node pointer to lookup 'clocks' and 'clock-names'
properties. Let's use the parent device's device_node pointer if the
registering device doesn't have a device_node but the parent does. This
simplifies clk registration code by avoiding the need to assign some
device_node to the device registering the clk.

Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Reported-by: Niklas Cassel <niklas.cassel@linaro.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---

I decided to introduce a new function instead of trying to jam it all
in the one line where we assign np. This way the function gets the 
true 'np' as an argument all the time.

 drivers/clk/clk.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index b68e200829f2..a743fffe8e46 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3719,6 +3719,28 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
 	return ERR_PTR(ret);
 }
 
+/**
+ * dev_or_parent_of_node - Get device node of @dev or @dev's parent
+ * @dev: Device to get device node of
+ *
+ * Returns: device node pointer of @dev, or the device node pointer of
+ * @dev->parent if dev doesn't have a device node, or NULL if neither
+ * @dev or @dev->parent have a device node.
+ */
+static struct device_node *dev_or_parent_of_node(struct device *dev)
+{
+	struct device_node *np;
+
+	if (!dev)
+		return NULL;
+
+	np = dev_of_node(dev);
+	if (!np)
+		np = dev_of_node(dev->parent);
+
+	return np;
+}
+
 /**
  * clk_register - allocate a new clock, register it and return an opaque cookie
  * @dev: device that is registering this clock
@@ -3734,7 +3756,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(dev), hw);
+	return __clk_register(dev, dev_or_parent_of_node(dev), hw);
 }
 EXPORT_SYMBOL_GPL(clk_register);
 
@@ -3750,7 +3772,8 @@ 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(dev), hw));
+	return PTR_ERR_OR_ZERO(__clk_register(dev, dev_or_parent_of_node(dev),
+			       hw));
 }
 EXPORT_SYMBOL_GPL(clk_hw_register);
 

base-commit: e42617b825f8073569da76dc4510bfa019b1c35a
-- 
Sent by a computer, using git, on the internet


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] clk: Use parent node pointer during registration if necessary
  2019-12-30 19:04 [PATCH] clk: Use parent node pointer during registration if necessary Stephen Boyd
@ 2020-01-03  0:17 ` Niklas Cassel
  2020-01-03  7:39 ` Bjorn Andersson
  1 sibling, 0 replies; 4+ messages in thread
From: Niklas Cassel @ 2020-01-03  0:17 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Michael Turquette, linux-kernel, linux-clk, Bjorn Andersson,
	Niklas Cassel

On Mon, Dec 30, 2019 at 11:04:55AM -0800, Stephen Boyd wrote:
> Sometimes clk drivers are attached to devices which are children of a
> parent device that is connected to a node in DT. This happens when
> devices are MFD-ish and the parent device driver mostly registers child
> devices to match against drivers placed in their respective subsystem
> directories like drivers/clk, drivers/regulator, etc. When the clk
> driver calls clk_register() with a device pointer, that struct device
> pointer won't have a device_node associated with it because it was
> created purely in software as a way to partition logic to a subsystem.
> 
> This causes problems for the way we find parent clks for the clks
> registered by these child devices because we look at the registering
> device's device_node pointer to lookup 'clocks' and 'clock-names'
> properties. Let's use the parent device's device_node pointer if the
> registering device doesn't have a device_node but the parent does. This
> simplifies clk registration code by avoiding the need to assign some
> device_node to the device registering the clk.
> 
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Reported-by: Niklas Cassel <niklas.cassel@linaro.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---
> 
> I decided to introduce a new function instead of trying to jam it all
> in the one line where we assign np. This way the function gets the 
> true 'np' as an argument all the time.
> 
>  drivers/clk/clk.c | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 
> 
> base-commit: e42617b825f8073569da76dc4510bfa019b1c35a
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index b68e200829f2..a743fffe8e46 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3719,6 +3719,28 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
>  	return ERR_PTR(ret);
>  }
>  
> +/**
> + * dev_or_parent_of_node - Get device node of @dev or @dev's parent
> + * @dev: Device to get device node of
> + *
> + * Returns: device node pointer of @dev, or the device node pointer of
> + * @dev->parent if dev doesn't have a device node, or NULL if neither
> + * @dev or @dev->parent have a device node.
> + */
> +static struct device_node *dev_or_parent_of_node(struct device *dev)
> +{
> +	struct device_node *np;
> +
> +	if (!dev)
> +		return NULL;
> +
> +	np = dev_of_node(dev);
> +	if (!np)
> +		np = dev_of_node(dev->parent);
> +
> +	return np;
> +}
> +
>  /**
>   * clk_register - allocate a new clock, register it and return an opaque cookie
>   * @dev: device that is registering this clock
> @@ -3734,7 +3756,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(dev), hw);
> +	return __clk_register(dev, dev_or_parent_of_node(dev), hw);
>  }
>  EXPORT_SYMBOL_GPL(clk_register);
>  
> @@ -3750,7 +3772,8 @@ 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(dev), hw));
> +	return PTR_ERR_OR_ZERO(__clk_register(dev, dev_or_parent_of_node(dev),
> +			       hw));
>  }
>  EXPORT_SYMBOL_GPL(clk_hw_register);
>  

Reviewed-by: Niklas Cassel <nks@flawful.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] clk: Use parent node pointer during registration if necessary
  2019-12-30 19:04 [PATCH] clk: Use parent node pointer during registration if necessary Stephen Boyd
  2020-01-03  0:17 ` Niklas Cassel
@ 2020-01-03  7:39 ` Bjorn Andersson
  2020-01-05  7:07   ` Stephen Boyd
  1 sibling, 1 reply; 4+ messages in thread
From: Bjorn Andersson @ 2020-01-03  7:39 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: Michael Turquette, linux-kernel, linux-clk, Niklas Cassel

On Mon 30 Dec 11:04 PST 2019, Stephen Boyd wrote:

> Sometimes clk drivers are attached to devices which are children of a
> parent device that is connected to a node in DT. This happens when
> devices are MFD-ish and the parent device driver mostly registers child
> devices to match against drivers placed in their respective subsystem
> directories like drivers/clk, drivers/regulator, etc. When the clk
> driver calls clk_register() with a device pointer, that struct device
> pointer won't have a device_node associated with it because it was
> created purely in software as a way to partition logic to a subsystem.
> 
> This causes problems for the way we find parent clks for the clks
> registered by these child devices because we look at the registering
> device's device_node pointer to lookup 'clocks' and 'clock-names'
> properties. Let's use the parent device's device_node pointer if the
> registering device doesn't have a device_node but the parent does. This
> simplifies clk registration code by avoiding the need to assign some
> device_node to the device registering the clk.
> 
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Reported-by: Niklas Cassel <niklas.cassel@linaro.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---
> 
> I decided to introduce a new function instead of trying to jam it all
> in the one line where we assign np. This way the function gets the 
> true 'np' as an argument all the time.
> 

Looks better.

>  drivers/clk/clk.c | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index b68e200829f2..a743fffe8e46 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3719,6 +3719,28 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
>  	return ERR_PTR(ret);
>  }
>  
> +/**
> + * dev_or_parent_of_node - Get device node of @dev or @dev's parent

()

> + * @dev: Device to get device node of
> + *
> + * Returns: device node pointer of @dev, or the device node pointer of

Return: (no 's')


With that,

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> + * @dev->parent if dev doesn't have a device node, or NULL if neither
> + * @dev or @dev->parent have a device node.
> + */
> +static struct device_node *dev_or_parent_of_node(struct device *dev)
> +{
> +	struct device_node *np;
> +
> +	if (!dev)
> +		return NULL;
> +
> +	np = dev_of_node(dev);
> +	if (!np)
> +		np = dev_of_node(dev->parent);
> +
> +	return np;
> +}
> +
>  /**
>   * clk_register - allocate a new clock, register it and return an opaque cookie
>   * @dev: device that is registering this clock
> @@ -3734,7 +3756,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(dev), hw);
> +	return __clk_register(dev, dev_or_parent_of_node(dev), hw);
>  }
>  EXPORT_SYMBOL_GPL(clk_register);
>  
> @@ -3750,7 +3772,8 @@ 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(dev), hw));
> +	return PTR_ERR_OR_ZERO(__clk_register(dev, dev_or_parent_of_node(dev),
> +			       hw));
>  }
>  EXPORT_SYMBOL_GPL(clk_hw_register);
>  
> 
> base-commit: e42617b825f8073569da76dc4510bfa019b1c35a
> -- 
> Sent by a computer, using git, on the internet
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] clk: Use parent node pointer during registration if necessary
  2020-01-03  7:39 ` Bjorn Andersson
@ 2020-01-05  7:07   ` Stephen Boyd
  0 siblings, 0 replies; 4+ messages in thread
From: Stephen Boyd @ 2020-01-05  7:07 UTC (permalink / raw)
  To: Bjorn Andersson; +Cc: Michael Turquette, linux-kernel, linux-clk, Niklas Cassel

Quoting Bjorn Andersson (2020-01-02 23:39:26)
> On Mon 30 Dec 11:04 PST 2019, Stephen Boyd wrote:
> 
> > Sometimes clk drivers are attached to devices which are children of a
> > parent device that is connected to a node in DT. This happens when
> > devices are MFD-ish and the parent device driver mostly registers child
> > devices to match against drivers placed in their respective subsystem
> > directories like drivers/clk, drivers/regulator, etc. When the clk
> > driver calls clk_register() with a device pointer, that struct device
> > pointer won't have a device_node associated with it because it was
> > created purely in software as a way to partition logic to a subsystem.
> > 
> > This causes problems for the way we find parent clks for the clks
> > registered by these child devices because we look at the registering
> > device's device_node pointer to lookup 'clocks' and 'clock-names'
> > properties. Let's use the parent device's device_node pointer if the
> > registering device doesn't have a device_node but the parent does. This
> > simplifies clk registration code by avoiding the need to assign some
> > device_node to the device registering the clk.
> > 
> > Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> > Reported-by: Niklas Cassel <niklas.cassel@linaro.org>
> > Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> > ---
> > 
> > I decided to introduce a new function instead of trying to jam it all
> > in the one line where we assign np. This way the function gets the 
> > true 'np' as an argument all the time.
> > 
> 
> Looks better.
> 
> >  drivers/clk/clk.c | 27 +++++++++++++++++++++++++--
> >  1 file changed, 25 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index b68e200829f2..a743fffe8e46 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -3719,6 +3719,28 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
> >       return ERR_PTR(ret);
> >  }
> >  
> > +/**
> > + * dev_or_parent_of_node - Get device node of @dev or @dev's parent
> 
> ()

This has been left out historically in this file. I'll add it but I
guess I should really take a pass at updating the docs in the whole
file!

> 
> > + * @dev: Device to get device node of
> > + *
> > + * Returns: device node pointer of @dev, or the device node pointer of
> 
> Return: (no 's')

Thanks.

> 
> 
> With that,
> 
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-30 19:04 [PATCH] clk: Use parent node pointer during registration if necessary Stephen Boyd
2020-01-03  0:17 ` Niklas Cassel
2020-01-03  7:39 ` Bjorn Andersson
2020-01-05  7:07   ` Stephen Boyd

Linux-Clk Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-clk/0 linux-clk/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-clk linux-clk/ https://lore.kernel.org/linux-clk \
		linux-clk@vger.kernel.org
	public-inbox-index linux-clk

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-clk


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git