diff mbox

[v3,01/10] ARM: tegra: Set the sound card model that alsaucm expects

Message ID 1422442278-10405-2-git-send-email-tomeu.vizoso@collabora.com
State Superseded, archived
Headers show

Commit Message

Tomeu Vizoso Jan. 28, 2015, 10:50 a.m. UTC
Patches are on its way to add a config file to alsaucm for the Nyan
boards. Use the same card ID that alsaucm will expect.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 arch/arm/boot/dts/tegra124-nyan-big.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Stephen Warren Feb. 2, 2015, 9:08 p.m. UTC | #1
On 01/28/2015 03:50 AM, Tomeu Vizoso wrote:
> Patches are on its way to add a config file to alsaucm for the Nyan
> boards. Use the same card ID that alsaucm will expect.

> diff --git a/arch/arm/boot/dts/tegra124-nyan-big.dts b/arch/arm/boot/dts/tegra124-nyan-big.dts

>   	sound {
> -		compatible = "nvidia,tegra-audio-max98090-nyan-big",
> +		compatible = "nvidia,tegra-audio-max98090-nyan",
>   			     "nvidia,tegra-audio-max98090";

I'm not convinced that removing the board-specific compatible value is a 
great idea. What if we find we need to distinguish between different 
boards that use this same binding in the future. That situation is 
exactly why we have board-/SoC-specific values in compatible even if we 
don't immediately use them.

> -		nvidia,model = "Acer Chromebook 13";
> +		nvidia,model = "GoogleNyan";

I believe this also technically breaks ABI, since some user-space tools 
use the model to look up saved state. Can we not leave this as is, and 
just have the UCM files know about both names?

Aside from that, I think the series looks OK at a quick glance.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomeu Vizoso Feb. 3, 2015, 1:13 p.m. UTC | #2
On 2 February 2015 at 22:08, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 01/28/2015 03:50 AM, Tomeu Vizoso wrote:
>>
>> Patches are on its way to add a config file to alsaucm for the Nyan
>> boards. Use the same card ID that alsaucm will expect.
>
>
>> diff --git a/arch/arm/boot/dts/tegra124-nyan-big.dts
>> b/arch/arm/boot/dts/tegra124-nyan-big.dts
>
>
>>         sound {
>> -               compatible = "nvidia,tegra-audio-max98090-nyan-big",
>> +               compatible = "nvidia,tegra-audio-max98090-nyan",
>>                              "nvidia,tegra-audio-max98090";
>
>
> I'm not convinced that removing the board-specific compatible value is a
> great idea. What if we find we need to distinguish between different boards
> that use this same binding in the future. That situation is exactly why we
> have board-/SoC-specific values in compatible even if we don't immediately
> use them.

I understand the need of naming each component variant so they can be
distinguished in the future, but in this case it's the exact same hw.

>> -               nvidia,model = "Acer Chromebook 13";
>> +               nvidia,model = "GoogleNyan";
>
>
> I believe this also technically breaks ABI, since some user-space tools use
> the model to look up saved state. Can we not leave this as is, and just have
> the UCM files know about both names?

Well, "A13" isn't a great card id. Given that there's no users yet, I
would prefer to take this chance to put a sane value in there. Btw,
alsa-lib has now a UCM config for this and it uses the GoogleNyan card
id (has been picked up already by OpenSUSE).

So in this case, I think it would be good to change the card id now
before people start to actually use it.

Regards,

Tomeu

> Aside from that, I think the series looks OK at a quick glance.
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren Feb. 3, 2015, 4:35 p.m. UTC | #3
On 02/03/2015 06:13 AM, Tomeu Vizoso wrote:
> On 2 February 2015 at 22:08, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 01/28/2015 03:50 AM, Tomeu Vizoso wrote:
>>>
>>> Patches are on its way to add a config file to alsaucm for the Nyan
>>> boards. Use the same card ID that alsaucm will expect.
>>
>>
>>> diff --git a/arch/arm/boot/dts/tegra124-nyan-big.dts
>>> b/arch/arm/boot/dts/tegra124-nyan-big.dts
>>
>>
>>>          sound {
>>> -               compatible = "nvidia,tegra-audio-max98090-nyan-big",
>>> +               compatible = "nvidia,tegra-audio-max98090-nyan",
>>>                               "nvidia,tegra-audio-max98090";
>>
>>
>> I'm not convinced that removing the board-specific compatible value is a
>> great idea. What if we find we need to distinguish between different boards
>> that use this same binding in the future. That situation is exactly why we
>> have board-/SoC-specific values in compatible even if we don't immediately
>> use them.
>
> I understand the need of naming each component variant so they can be
> distinguished in the future, but in this case it's the exact same hw.

That's not true. These are two different boards that are derived from 
the same base design. The intent may be that they are the same, but the 
whole point of board-specific compatible values is to cover the case 
where mistakes and exceptions appear later.

>>> -               nvidia,model = "Acer Chromebook 13";
>>> +               nvidia,model = "GoogleNyan";
>>
>>
>> I believe this also technically breaks ABI, since some user-space tools use
>> the model to look up saved state. Can we not leave this as is, and just have
>> the UCM files know about both names?
>
> Well, "A13" isn't a great card id. Given that there's no users yet, I
> would prefer to take this chance to put a sane value in there. Btw,
> alsa-lib has now a UCM config for this and it uses the GoogleNyan card
> id (has been picked up already by OpenSUSE).

"A13" didn't appear anywhere, did it? Perhaps that was a shorthand for 
"Acer Chromebook 13". Yes, I agree we should use the user-visible model 
number here; "Acer CB5" or perhaps "Acer CB5-xxx" whatever xxx actually is.

> So in this case, I think it would be good to change the card id now
> before people start to actually use it.


--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomeu Vizoso Feb. 4, 2015, 9:13 a.m. UTC | #4
On 02/03/2015 05:35 PM, Stephen Warren wrote:
> On 02/03/2015 06:13 AM, Tomeu Vizoso wrote:
>> On 2 February 2015 at 22:08, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 01/28/2015 03:50 AM, Tomeu Vizoso wrote:
>>>>
>>>> Patches are on its way to add a config file to alsaucm for the Nyan
>>>> boards. Use the same card ID that alsaucm will expect.
>>>
>>>
>>>> diff --git a/arch/arm/boot/dts/tegra124-nyan-big.dts
>>>> b/arch/arm/boot/dts/tegra124-nyan-big.dts
>>>
>>>
>>>>          sound {
>>>> -               compatible = "nvidia,tegra-audio-max98090-nyan-big",
>>>> +               compatible = "nvidia,tegra-audio-max98090-nyan",
>>>>                               "nvidia,tegra-audio-max98090";
>>>
>>>
>>> I'm not convinced that removing the board-specific compatible value is a
>>> great idea. What if we find we need to distinguish between different boards
>>> that use this same binding in the future. That situation is exactly why we
>>> have board-/SoC-specific values in compatible even if we don't immediately
>>> use them.
>>
>> I understand the need of naming each component variant so they can be
>> distinguished in the future, but in this case it's the exact same hw.
> 
> That's not true. These are two different boards that are derived from 
> the same base design. The intent may be that they are the same, but the 
> whole point of board-specific compatible values is to cover the case 
> where mistakes and exceptions appear later.

Fair enough.

>>>> -               nvidia,model = "Acer Chromebook 13";
>>>> +               nvidia,model = "GoogleNyan";
>>>
>>>
>>> I believe this also technically breaks ABI, since some user-space tools use
>>> the model to look up saved state. Can we not leave this as is, and just have
>>> the UCM files know about both names?
>>
>> Well, "A13" isn't a great card id. Given that there's no users yet, I
>> would prefer to take this chance to put a sane value in there. Btw,
>> alsa-lib has now a UCM config for this and it uses the GoogleNyan card
>> id (has been picked up already by OpenSUSE).
> 
> "A13" didn't appear anywhere, did it? Perhaps that was a shorthand for 
> "Acer Chromebook 13". Yes, I agree we should use the user-visible model 
> number here; "Acer CB5" or perhaps "Acer CB5-xxx" whatever xxx actually is.

Ok, so we can go with Acer-CB5-311 for the nyan big.

Unfortunately there doesn't exist a single product id that corresponds
with the nyan blaze, but at least four: K4K83UA, K4K11UA, K4K78UA and
K4K23UA. The commercial name of the series that contain a blaze board is
"HP Chromebook 14 G3", but that doesn't fit in the card id field (16 byte).

What do you think of using GoogleNyanBig and GoogleNyanBlaze? Or maybe
just NyanBig and NyanBlaze? IMO it's more important that the card ids
correspond 1-to-1 to the hw than their recognizability, as it's not
something that end users will be messing with.

Regards,

Tomeu

>> So in this case, I think it would be good to change the card id now
>> before people start to actually use it.
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren Feb. 4, 2015, 4:56 p.m. UTC | #5
On 02/04/2015 02:13 AM, Tomeu Vizoso wrote:
> On 02/03/2015 05:35 PM, Stephen Warren wrote:
>> On 02/03/2015 06:13 AM, Tomeu Vizoso wrote:
>>> On 2 February 2015 at 22:08, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> On 01/28/2015 03:50 AM, Tomeu Vizoso wrote:
>>>>>
>>>>> Patches are on its way to add a config file to alsaucm for the Nyan
>>>>> boards. Use the same card ID that alsaucm will expect.
>>>>
>>>>
>>>>> diff --git a/arch/arm/boot/dts/tegra124-nyan-big.dts
>>>>> b/arch/arm/boot/dts/tegra124-nyan-big.dts
>>>>
>>>>
>>>>>           sound {
>>>>> -               compatible = "nvidia,tegra-audio-max98090-nyan-big",
>>>>> +               compatible = "nvidia,tegra-audio-max98090-nyan",
>>>>>                                "nvidia,tegra-audio-max98090";
>>>>
>>>>
>>>> I'm not convinced that removing the board-specific compatible value is a
>>>> great idea. What if we find we need to distinguish between different boards
>>>> that use this same binding in the future. That situation is exactly why we
>>>> have board-/SoC-specific values in compatible even if we don't immediately
>>>> use them.
>>>
>>> I understand the need of naming each component variant so they can be
>>> distinguished in the future, but in this case it's the exact same hw.
>>
>> That's not true. These are two different boards that are derived from
>> the same base design. The intent may be that they are the same, but the
>> whole point of board-specific compatible values is to cover the case
>> where mistakes and exceptions appear later.
>
> Fair enough.
>
>>>>> -               nvidia,model = "Acer Chromebook 13";
>>>>> +               nvidia,model = "GoogleNyan";
>>>>
>>>>
>>>> I believe this also technically breaks ABI, since some user-space tools use
>>>> the model to look up saved state. Can we not leave this as is, and just have
>>>> the UCM files know about both names?
>>>
>>> Well, "A13" isn't a great card id. Given that there's no users yet, I
>>> would prefer to take this chance to put a sane value in there. Btw,
>>> alsa-lib has now a UCM config for this and it uses the GoogleNyan card
>>> id (has been picked up already by OpenSUSE).
>>
>> "A13" didn't appear anywhere, did it? Perhaps that was a shorthand for
>> "Acer Chromebook 13". Yes, I agree we should use the user-visible model
>> number here; "Acer CB5" or perhaps "Acer CB5-xxx" whatever xxx actually is.
>
> Ok, so we can go with Acer-CB5-311 for the nyan big.
>
> Unfortunately there doesn't exist a single product id that corresponds
> with the nyan blaze, but at least four: K4K83UA, K4K11UA, K4K78UA and
> K4K23UA. The commercial name of the series that contain a blaze board is
> "HP Chromebook 14 G3", but that doesn't fit in the card id field (16 byte).

Oh, the 16-byte limit is rather annoying:-( Some of our existing DT 
files don't fit into that limit.

> What do you think of using GoogleNyanBig and GoogleNyanBlaze? Or maybe
> just NyanBig and NyanBlaze? IMO it's more important that the card ids
> correspond 1-to-1 to the hw than their recognizability, as it's not
> something that end users will be messing with.

I'm not entirely sure that the user won't ever see those values; don't 
some X11 GUI tools display them? Still, as you say there isn't too much 
we can do, so since these names are unique/complete I figure they're OK.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/boot/dts/tegra124-nyan-big.dts b/arch/arm/boot/dts/tegra124-nyan-big.dts
index 004e8e4..eda6ae7 100644
--- a/arch/arm/boot/dts/tegra124-nyan-big.dts
+++ b/arch/arm/boot/dts/tegra124-nyan-big.dts
@@ -1108,9 +1108,9 @@ 
 	};
 
 	sound {
-		compatible = "nvidia,tegra-audio-max98090-nyan-big",
+		compatible = "nvidia,tegra-audio-max98090-nyan",
 			     "nvidia,tegra-audio-max98090";
-		nvidia,model = "Acer Chromebook 13";
+		nvidia,model = "GoogleNyan";
 
 		nvidia,audio-routing =
 			"Headphones", "HPR",