Fix some 'const' problems
Subsystems (4)
v2 cover Fix some 'const' problems
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(-)
Signature · 1 line
2.1.3
PATCH v2 1/2 ASoC: Remove 'const' from the device_node pointers
Quoted 1 line
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".
--- include/sound/soc.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
include/sound/soc.h +3 -3
@@ -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);
Signature · 1 line
2.1.3
Mark Brown
maintainerRe: [PATCH v2 1/2] ASoC: Remove 'const' from the device_node pointers
Quoted 4 lines · Jean-Francois Moine
From Russell King:
of_node_put()modifies the structdevice_nodecontents. 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..."?
Re: [PATCH v2 1/2] ASoC: Remove 'const' from the device_node pointers
Quoted 9 lines · On Mon, 10 Nov 2014 19:03:32 +0000 Mark Brown <[email protected]> wrote:
Quoted 4 lines · Jean-Francois Moine
From Russell King:
of_node_put()modifies the structdevice_nodecontents. 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?
Signature · 2 lines
Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/
Mark Brown
maintainerRe: [PATCH v2 1/2] ASoC: Remove 'const' from the device_node pointers
Quoted 8 lines · Jean-Francois Moine
Quoted 3 lines · Mark Brown
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...).
Mark Brown
maintainerRe: [PATCH v2 1/2] ASoC: Remove 'const' from the device_node pointers
In reply to Jean-Francois Moineinclude/sound/soc.h struct snd_soc_platform_driver { · line 892
Quoted 12 lines · Jean-Francois Moine
index 7ba7130..405f967 100644
include/sound/soc.h +1
-1@@ -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.
Re: [PATCH v2 1/2] ASoC: Remove 'const' from the device_node pointers
Quoted 7 lines · Mark Brown
Quoted 5 lines · Jean-Francois Moine
index 7ba7130..405f967 100644
include/sound/soc.h +0
-0@@ -886,7 +886,7 @@ struct snd_soc_platform_driver {
Quoted 12 lines
include/sound/soc.h struct snd_soc_platform_driver { · line 892
Quoted 7 lines
include/sound/soc.h +1
-1struct 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 exceptsnd_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.
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net.
PATCH v2 2/2 ASoC: simple-card: Remove useless casts
In reply to Jean-Francois MoineThere is no need to cast the cpu_of_node or codec_of_node of the dai_links when calling of_put_node.
--- 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(-)
sound/soc/generic/simple-card.c +2 -5
@@ -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 ; }
sound/soc/samsung/odroidx2_max98090.c +2 -2
@@ -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 ; }
sound/soc/ux500/mop500.c +2 -4
@@ -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); } }
Signature · 1 line
2.1.3
Re: [PATCH v2 2/2] ASoC: simple-card: Remove useless casts
Quoted 2 lines · Jean-Francois Moine
There is no need to cast the
cpu_of_nodeorcodec_of_nodeof thedai_linkswhen callingof_put_node.
Applied, thanks.