[PATCH 0/2] of: Fix some 'const' problems

[PATCH 0/2] of: Fix some 'const' problems

[Jean-Francois Moine]

A cast is often needed to call some OF functions, removing
the 'const' attribute.
This patchset makes 'const' the device node pointers of the functions
of_clk_get() and of_node_put() and also removes some useless casts
in the sound/soc tree.

Jean-Francois Moine (2):
  of: Make const the device node pointers in of_clk_get and of_node_put
  ASoC: Remove useless casts

 drivers/clk/clkdev.c                  | 2 +-
 drivers/of/dynamic.c                  | 2 +-
 include/linux/clk.h                   | 4 ++--
 include/linux/of.h                    | 4 ++--
 sound/soc/generic/simple-card.c       | 7 ++-----
 sound/soc/samsung/odroidx2_max98090.c | 4 ++--
 sound/soc/ux500/mop500.c              | 6 ++----
 7 files changed, 12 insertions(+), 17 deletions(-)

-- 
2.1.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] of: Make const the device node pointers in of_clk_get and of_node_put

[Jean-Francois Moine]

Some device nodes are sometimes referenced by const pointers.

This patch avoids to cast these pointers when calling the functions
of_clk_get() and of_node_put().

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 drivers/clk/clkdev.c | 2 +-
 drivers/of/dynamic.c | 2 +-
 include/linux/clk.h  | 4 ++--
 include/linux/of.h   | 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index da4bda8..eaee11e 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -53,7 +53,7 @@ struct clk *of_clk_get_by_clkspec(struct of_phandle_args *clkspec)
 	return clk;
 }
 
-struct clk *of_clk_get(struct device_node *np, int index)
+struct clk *of_clk_get(const struct device_node *np, int index)
 {
 	struct of_phandle_args clkspec;
 	struct clk *clk;
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index f297891..64eb5ba 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -34,7 +34,7 @@ EXPORT_SYMBOL(of_node_get);
  * @node:	Node to dec refcount, NULL is supported to simplify writing of
  *		callers
  */
-void of_node_put(struct device_node *node)
+void of_node_put(const struct device_node *node)
 {
 	if (node)
 		kobject_put(&node->kobj);
diff --git a/include/linux/clk.h b/include/linux/clk.h
index c7f258a..4e077a4 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -425,11 +425,11 @@ struct device_node;
 struct of_phandle_args;
 
 #if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK)
-struct clk *of_clk_get(struct device_node *np, int index);
+struct clk *of_clk_get(const struct device_node *np, int index);
 struct clk *of_clk_get_by_name(struct device_node *np, const char *name);
 struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec);
 #else
-static inline struct clk *of_clk_get(struct device_node *np, int index)
+static inline struct clk *of_clk_get(const struct device_node *np, int index)
 {
 	return ERR_PTR(-ENOENT);
 }
diff --git a/include/linux/of.h b/include/linux/of.h
index 6545e7a..b738681 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -95,14 +95,14 @@ static inline int of_node_is_attached(struct device_node *node)
 
 #ifdef CONFIG_OF_DYNAMIC
 extern struct device_node *of_node_get(struct device_node *node);
-extern void of_node_put(struct device_node *node);
+extern void of_node_put(const struct device_node *node);
 #else /* CONFIG_OF_DYNAMIC */
 /* Dummy ref counting routines - to be implemented later */
 static inline struct device_node *of_node_get(struct device_node *node)
 {
 	return node;
 }
-static inline void of_node_put(struct device_node *node) { }
+static inline void of_node_put(const struct device_node *node) { }
 #endif /* !CONFIG_OF_DYNAMIC */
 
 #ifdef CONFIG_OF
-- 
2.1.3

Re: [PATCH 1/2] of: Make const the device node pointers in of_clk_get and of_node_put

[Russell King - ARM Linux]

On Sat, Nov 08, 2014 at 07:33:22PM +0100, Jean-Francois Moine wrote:
> Some device nodes are sometimes referenced by const pointers.
> 
> This patch avoids to cast these pointers when calling the functions
> of_clk_get() and of_node_put().

NAK.  Firstly, of_node_put contains a kref, which is /definitely/ not
a const item when kobject_put() is called on it.  Inside the kobject
is a kref, which kobject_put() will call kref_put() on.  kref_put()
in turn will internally call atomic_sub_and_test() on a value embedded
within the kref struct.

Ergo, of_node_put() modifies the struct device_node contents.  Therefore,
of_node_put() definitely not treating the data pointed to as read-only,
and therefore it is completely inappropriate for it to be marked "const".

What this then means is that it fundamentally undermines the idea of
storing the pointer to a device_node as a const pointer, as the device
node must always be modified when you're done with it (because it's a
ref-counted structure.)  So, having it const in your code is a bug.

What this also means is that every other place that you've added const
below is also very dubious.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] of: Make const the device node pointers in of_clk_get and of_node_put

[Jean-Francois Moine]

On Sun, 9 Nov 2014 18:50:19 +0000
Russell King - ARM Linux <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org> wrote:

> What this then means is that it fundamentally undermines the idea of
> storing the pointer to a device_node as a const pointer, as the device
> node must always be modified when you're done with it (because it's a
> ref-counted structure.)  So, having it const in your code is a bug.
> 
> What this also means is that every other place that you've added const
> below is also very dubious.

Thanks for the explanation.

The correct patch will be simpler: in the ASoC structures, I found only
three places where the device_node pointer is const.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] ASoC: Remove useless casts

[Jean-Francois Moine]

There is no need to cast the cpu_of_node or codec_of_node of the
dai_links when calling of_put_node.

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 sound/soc/generic/simple-card.c       | 7 ++-----
 sound/soc/samsung/odroidx2_max98090.c | 4 ++--
 sound/soc/ux500/mop500.c              | 6 ++----
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index cd49d50..3e3fec4 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -457,16 +457,13 @@ static int asoc_simple_card_unref(struct platform_device *pdev)
 {
 	struct snd_soc_card *card = platform_get_drvdata(pdev);
 	struct snd_soc_dai_link *dai_link;
-	struct device_node *np;
 	int num_links;
 
 	for (num_links = 0, dai_link = card->dai_link;
 	     num_links < card->num_links;
 	     num_links++, dai_link++) {
-		np = (struct device_node *) dai_link->cpu_of_node;
-		of_node_put(np);
-		np = (struct device_node *) dai_link->codec_of_node;
-		of_node_put(np);
+		of_node_put(dai_link->cpu_of_node);
+		of_node_put(dai_link->codec_of_node);
 	}
 	return 0;
 }
diff --git a/sound/soc/samsung/odroidx2_max98090.c b/sound/soc/samsung/odroidx2_max98090.c
index 3c8f604..d7640e7 100644
--- a/sound/soc/samsung/odroidx2_max98090.c
+++ b/sound/soc/samsung/odroidx2_max98090.c
@@ -153,8 +153,8 @@ static int odroidx2_audio_remove(struct platform_device *pdev)
 
 	snd_soc_unregister_card(card);
 
-	of_node_put((struct device_node *)odroidx2_dai[0].cpu_of_node);
-	of_node_put((struct device_node *)odroidx2_dai[0].codec_of_node);
+	of_node_put(odroidx2_dai[0].cpu_of_node);
+	of_node_put(odroidx2_dai[0].codec_of_node);
 
 	return 0;
 }
diff --git a/sound/soc/ux500/mop500.c b/sound/soc/ux500/mop500.c
index b3b66aa..ea9ba284 100644
--- a/sound/soc/ux500/mop500.c
+++ b/sound/soc/ux500/mop500.c
@@ -64,11 +64,9 @@ static void mop500_of_node_put(void)
 
 	for (i = 0; i < 2; i++) {
 		if (mop500_dai_links[i].cpu_of_node)
-			of_node_put((struct device_node *)
-				mop500_dai_links[i].cpu_of_node);
+			of_node_put(mop500_dai_links[i].cpu_of_node);
 		if (mop500_dai_links[i].codec_of_node)
-			of_node_put((struct device_node *)
-				mop500_dai_links[i].codec_of_node);
+			of_node_put(mop500_dai_links[i].codec_of_node);
 	}
 }
 
-- 
2.1.3