All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 2/2] ASoC: simple-card: Remove useless casts
  2014-11-09 19:40 ` Jean-Francois Moine
  (?)
@ 2014-11-09 11:38 ` Jean-Francois Moine
  2014-11-10 19:02   ` Mark Brown
  -1 siblings, 1 reply; 10+ messages in thread
From: Jean-Francois Moine @ 2014-11-09 11:38 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown
  Cc: Russell King, Lars-Peter Clausen, alsa-devel, linux-kernel

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


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

* [PATCH v2 1/2] ASoC: Remove 'const' from the device_node pointers
  2014-11-09 19:40 ` Jean-Francois Moine
  (?)
  (?)
@ 2014-11-09 19:33 ` Jean-Francois Moine
  2014-11-10 19:03   ` Mark Brown
  2014-11-10 19:37   ` Mark Brown
  -1 siblings, 2 replies; 10+ messages in thread
From: Jean-Francois Moine @ 2014-11-09 19:33 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown
  Cc: Russell King, Lars-Peter Clausen, alsa-devel, linux-kernel

>From Russell King:
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".

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 include/sound/soc.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 7ba7130..405f967 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -886,7 +886,7 @@ struct snd_soc_platform_driver {
 
 struct snd_soc_dai_link_component {
 	const char *name;
-	const struct device_node *of_node;
+	struct device_node *of_node;
 	const char *dai_name;
 };
 
@@ -990,7 +990,7 @@ struct snd_soc_codec_conf {
 	 * DT/OF node, but not both.
 	 */
 	const char *dev_name;
-	const struct device_node *of_node;
+	struct device_node *of_node;
 
 	/*
 	 * optional map of kcontrol, widget and path name prefixes that are
@@ -1007,7 +1007,7 @@ struct snd_soc_aux_dev {
 	 * DT/OF node, but not both.
 	 */
 	const char *codec_name;
-	const struct device_node *codec_of_node;
+	struct device_node *codec_of_node;
 
 	/* codec/machine specific init - e.g. add machine controls */
 	int (*init)(struct snd_soc_component *component);
-- 
2.1.3


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

* [PATCH v2 0/2] Fix some 'const' problems
@ 2014-11-09 19:40 ` Jean-Francois Moine
  0 siblings, 0 replies; 10+ messages in thread
From: Jean-Francois Moine @ 2014-11-09 19:40 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown
  Cc: Russell King, Lars-Peter Clausen, alsa-devel, linux-kernel

A cast was often needed to call some OF functions, removing
a 'const' attribute.
This patchset removes 'const' from the device_node pointers of the
sound/soc structures and also removes some useless casts
in the sound/soc tree.

v2: remove const from the pointers instead of modifying the
    OF functions (Russell King)

Jean-Francois Moine (2):
  ASoC: Remove 'const' from the device_node pointers
  ASoC: simple-card: Remove useless casts

 include/sound/soc.h                   | 6 +++---
 sound/soc/generic/simple-card.c       | 7 ++-----
 sound/soc/samsung/odroidx2_max98090.c | 4 ++--
 sound/soc/ux500/mop500.c              | 6 ++----
 4 files changed, 9 insertions(+), 14 deletions(-)

-- 
2.1.3


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

* [PATCH v2 0/2] Fix some 'const' problems
@ 2014-11-09 19:40 ` Jean-Francois Moine
  0 siblings, 0 replies; 10+ messages in thread
From: Jean-Francois Moine @ 2014-11-09 19:40 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown
  Cc: alsa-devel, Lars-Peter Clausen, Russell King, linux-kernel

A cast was often needed to call some OF functions, removing
a 'const' attribute.
This patchset removes 'const' from the device_node pointers of the
sound/soc structures and also removes some useless casts
in the sound/soc tree.

v2: remove const from the pointers instead of modifying the
    OF functions (Russell King)

Jean-Francois Moine (2):
  ASoC: Remove 'const' from the device_node pointers
  ASoC: simple-card: Remove useless casts

 include/sound/soc.h                   | 6 +++---
 sound/soc/generic/simple-card.c       | 7 ++-----
 sound/soc/samsung/odroidx2_max98090.c | 4 ++--
 sound/soc/ux500/mop500.c              | 6 ++----
 4 files changed, 9 insertions(+), 14 deletions(-)

-- 
2.1.3

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

* Re: [PATCH v2 2/2] ASoC: simple-card: Remove useless casts
  2014-11-09 11:38 ` [PATCH v2 2/2] ASoC: simple-card: Remove useless casts Jean-Francois Moine
@ 2014-11-10 19:02   ` Mark Brown
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2014-11-10 19:02 UTC (permalink / raw)
  To: Jean-Francois Moine
  Cc: Liam Girdwood, Russell King, Lars-Peter Clausen, alsa-devel,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 192 bytes --]

On Sun, Nov 09, 2014 at 12:38:56PM +0100, Jean-Francois Moine wrote:
> There is no need to cast the cpu_of_node or codec_of_node of the
> dai_links when calling of_put_node.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [PATCH v2 1/2] ASoC: Remove 'const' from the device_node pointers
  2014-11-09 19:33 ` [PATCH v2 1/2] ASoC: Remove 'const' from the device_node pointers Jean-Francois Moine
@ 2014-11-10 19:03   ` Mark Brown
  2014-11-10 19:12     ` Jean-Francois Moine
  2014-11-10 19:37   ` Mark Brown
  1 sibling, 1 reply; 10+ messages in thread
From: Mark Brown @ 2014-11-10 19:03 UTC (permalink / raw)
  To: Jean-Francois Moine
  Cc: Liam Girdwood, Russell King, Lars-Peter Clausen, alsa-devel,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 457 bytes --]

On Sun, Nov 09, 2014 at 08:33:45PM +0100, Jean-Francois Moine wrote:
> From Russell King:
> 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".

Did Russell write this patch (in which case it's missing a signoff and
his e-mail address above) or are you trying to say "Russell King
said..."?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [PATCH v2 1/2] ASoC: Remove 'const' from the device_node pointers
  2014-11-10 19:03   ` Mark Brown
@ 2014-11-10 19:12     ` Jean-Francois Moine
  2014-11-10 19:25       ` Mark Brown
  0 siblings, 1 reply; 10+ messages in thread
From: Jean-Francois Moine @ 2014-11-10 19:12 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, Russell King, Lars-Peter Clausen, alsa-devel,
	linux-kernel

On Mon, 10 Nov 2014 19:03:32 +0000
Mark Brown <broonie@kernel.org> wrote:

> On Sun, Nov 09, 2014 at 08:33:45PM +0100, Jean-Francois Moine wrote:
> > From Russell King:
> > 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".
> 
> Did Russell write this patch (in which case it's missing a signoff and
> his e-mail address above) or are you trying to say "Russell King
> said..."?

Sorry, that was "Russell said ...".
Do you want I resubmit the patch?

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

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

* Re: [PATCH v2 1/2] ASoC: Remove 'const' from the device_node pointers
  2014-11-10 19:12     ` Jean-Francois Moine
@ 2014-11-10 19:25       ` Mark Brown
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2014-11-10 19:25 UTC (permalink / raw)
  To: Jean-Francois Moine
  Cc: Liam Girdwood, Russell King, Lars-Peter Clausen, alsa-devel,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 497 bytes --]

On Mon, Nov 10, 2014 at 08:12:54PM +0100, Jean-Francois Moine wrote:
> Mark Brown <broonie@kernel.org> wrote:

> > Did Russell write this patch (in which case it's missing a signoff and
> > his e-mail address above) or are you trying to say "Russell King
> > said..."?

> Sorry, that was "Russell said ...".
> Do you want I resubmit the patch?

Let me look at the change, I didn't do that yet (what you wrote looks
awfully like the patch submission format for sending code someone else
wrote...).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [PATCH v2 1/2] ASoC: Remove 'const' from the device_node pointers
  2014-11-09 19:33 ` [PATCH v2 1/2] ASoC: Remove 'const' from the device_node pointers Jean-Francois Moine
  2014-11-10 19:03   ` Mark Brown
@ 2014-11-10 19:37   ` Mark Brown
  2014-11-10 19:51     ` Russell King - ARM Linux
  1 sibling, 1 reply; 10+ messages in thread
From: Mark Brown @ 2014-11-10 19:37 UTC (permalink / raw)
  To: Jean-Francois Moine
  Cc: Liam Girdwood, Russell King, Lars-Peter Clausen, alsa-devel,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 626 bytes --]

On Sun, Nov 09, 2014 at 08:33:45PM +0100, Jean-Francois Moine wrote:

> index 7ba7130..405f967 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -886,7 +886,7 @@ struct snd_soc_platform_driver {
>  
>  struct snd_soc_dai_link_component {
>  	const char *name;
> -	const struct device_node *of_node;
> +	struct device_node *of_node;
>  	const char *dai_name;
>  };
>  

The changelog talked about of_node_put() but I'm not seeing anything in
the code which calls that except snd_soc_of_get_dai_name()?  Everything
else just does comparisons of the pointer AFIACT from a quick scan
through.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [PATCH v2 1/2] ASoC: Remove 'const' from the device_node pointers
  2014-11-10 19:37   ` Mark Brown
@ 2014-11-10 19:51     ` Russell King - ARM Linux
  0 siblings, 0 replies; 10+ messages in thread
From: Russell King - ARM Linux @ 2014-11-10 19:51 UTC (permalink / raw)
  To: Mark Brown
  Cc: Jean-Francois Moine, Liam Girdwood, Lars-Peter Clausen,
	alsa-devel, linux-kernel

On Mon, Nov 10, 2014 at 07:37:04PM +0000, Mark Brown wrote:
> On Sun, Nov 09, 2014 at 08:33:45PM +0100, Jean-Francois Moine wrote:
> 
> > index 7ba7130..405f967 100644
> > --- a/include/sound/soc.h
> > +++ b/include/sound/soc.h
> > @@ -886,7 +886,7 @@ struct snd_soc_platform_driver {
> >  
> >  struct snd_soc_dai_link_component {
> >  	const char *name;
> > -	const struct device_node *of_node;
> > +	struct device_node *of_node;
> >  	const char *dai_name;
> >  };
> >  
> 
> The changelog talked about of_node_put() but I'm not seeing anything in
> the code which calls that except snd_soc_of_get_dai_name()?  Everything
> else just does comparisons of the pointer AFIACT from a quick scan
> through.

I think it comes from these files (from a previous patch from Jean
adding a load of inappropriate const's to places where they do not
belong):

 sound/soc/generic/simple-card.c       | 7 ++-----
 sound/soc/samsung/odroidx2_max98090.c | 4 ++--
 sound/soc/ux500/mop500.c              | 6 ++----

That patch was trying to do this to those three files:

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

having made of_node_put() take a const pointer.  Since of_node_put()
ultimately modifies the data pointed to by that pointer, making its
argument "const" is incorrect.

Moreover, Jean's latest patch is absolutely the right thing to do.
Had he quoted me fully, then the reason would become clear.

struct device_node is a ref-counted structure.  That means if you
store a reference to it, you should "get" it, and you should "put"
it once you've done.  The act of "put"ing the pointed-to structure
involves writing to that structure, so it is totally unappropriate
to store a device_node structure as a const pointer.  It forces you
to have to cast it back to a non-const pointer at various points
in time to use various OF function calls.

So, what you have is a pointer that you would /like/ to be const, but
ultimately you violate its const-ness at some point in time when you
need to call of_node_put() on it.  So it isn't really const at all.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

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

end of thread, other threads:[~2014-11-10 19:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-09 19:40 [PATCH v2 0/2] Fix some 'const' problems Jean-Francois Moine
2014-11-09 19:40 ` Jean-Francois Moine
2014-11-09 11:38 ` [PATCH v2 2/2] ASoC: simple-card: Remove useless casts Jean-Francois Moine
2014-11-10 19:02   ` Mark Brown
2014-11-09 19:33 ` [PATCH v2 1/2] ASoC: Remove 'const' from the device_node pointers Jean-Francois Moine
2014-11-10 19:03   ` Mark Brown
2014-11-10 19:12     ` Jean-Francois Moine
2014-11-10 19:25       ` Mark Brown
2014-11-10 19:37   ` Mark Brown
2014-11-10 19:51     ` Russell King - ARM Linux

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.