[1/3] ASoC: simple-card: Drop node->name checking
diff mbox

Message ID ef6f96d375c8771c93469168790e0098610bd250.1398337861.git.Guangyu.Chen@freescale.com
State Accepted
Commit 50e6c718a1eb2ae6d05f22615d8268b026175a4a
Headers show

Commit Message

Nicolin Chen April 24, 2014, 11:13 a.m. UTC
The current simple-card driver limits the DT node name to "sound".
Any of other names is forbidden while actually we should allow DT
to pass other node names.

And if this function is being called, the node must already have
the compatible "simple-audio-card" in DTB. So there should be no
need to check the name here.

Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
---
 sound/soc/generic/simple-card.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Jyri Sarha April 24, 2014, 12:47 p.m. UTC | #1
On 04/24/2014 02:13 PM, Nicolin Chen wrote:
> The current simple-card driver limits the DT node name to "sound".
> Any of other names is forbidden while actually we should allow DT
> to pass other node names.
>
> And if this function is being called, the node must already have
> the compatible "simple-audio-card" in DTB. So there should be no
> need to check the name here.
>
> Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
> ---
>   sound/soc/generic/simple-card.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
> index 3f2e580..383a4a1 100644
> --- a/sound/soc/generic/simple-card.c
> +++ b/sound/soc/generic/simple-card.c
> @@ -156,8 +156,7 @@ static int simple_card_dai_link_of(struct device_node *node,
>   	char *prefix = "";
>   	int ret;
>
> -	if (!strcmp("sound", node->name))
> -		prefix = "simple-audio-card,";
> +	prefix = "simple-audio-card,";
>
>   	daifmt = snd_soc_of_parse_daifmt(node, prefix,
>   					 &bitclkmaster, &framemaster);
>

I think you have missed the point of selecting the prefix based on the 
node name.

Before the change the "simple-audio-card,"-prefix was only needed for 
dai-link properties and subnodes if the dai-link node was omitted in a 
single dai-link setup.

After your change the prefix is also needed for the properties and 
subnodes inside dai-link subnodes.

See the details in: Documentation/devicetree/bindings/sound/simple-card.txt

Maybe the implementation could have been more explicit, but I think the 
old behavior is more convenient. If we anyway decide to go with this 
change then at least the DT binding document should be updated.

Best regards,
Jyri
Mark Brown April 24, 2014, 1:03 p.m. UTC | #2
On Thu, Apr 24, 2014 at 03:47:59PM +0300, Jyri Sarha wrote:
> On 04/24/2014 02:13 PM, Nicolin Chen wrote:

Please delete unneeded context from your mails...

> >-	if (!strcmp("sound", node->name))
> >-		prefix = "simple-audio-card,";
> >+	prefix = "simple-audio-card,";

> I think you have missed the point of selecting the prefix based on the node
> name.

...

> Maybe the implementation could have been more explicit, but I think the old
> behavior is more convenient. If we anyway decide to go with this change then
> at least the DT binding document should be updated.

Yes, the implementation needs to be *way* more explicit - I'd expect a
parameter/variable saying which level we're at depending on where we're
at in the parse rather than guessing based on the node name.
Jyri Sarha April 24, 2014, 1:05 p.m. UTC | #3
On 04/24/2014 04:03 PM, Mark Brown wrote:
> On Thu, Apr 24, 2014 at 03:47:59PM +0300, Jyri Sarha wrote:
>> On 04/24/2014 02:13 PM, Nicolin Chen wrote:
>
> Please delete unneeded context from your mails...
>
>>> -	if (!strcmp("sound", node->name))
>>> -		prefix = "simple-audio-card,";
>>> +	prefix = "simple-audio-card,";
>
>> I think you have missed the point of selecting the prefix based on the node
>> name.
>
> ...
>
>> Maybe the implementation could have been more explicit, but I think the old
>> behavior is more convenient. If we anyway decide to go with this change then
>> at least the DT binding document should be updated.
>
> Yes, the implementation needs to be *way* more explicit - I'd expect a
> parameter/variable saying which level we're at depending on where we're
> at in the parse rather than guessing based on the node name.
>

Ok, I'll come up with a patch soon.

Best regards,
Jyri

Patch
diff mbox

diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 3f2e580..383a4a1 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -156,8 +156,7 @@  static int simple_card_dai_link_of(struct device_node *node,
 	char *prefix = "";
 	int ret;
 
-	if (!strcmp("sound", node->name))
-		prefix = "simple-audio-card,";
+	prefix = "simple-audio-card,";
 
 	daifmt = snd_soc_of_parse_daifmt(node, prefix,
 					 &bitclkmaster, &framemaster);