diff mbox series

[v3,7/7] arm64: dts: allwinner: pinephone: Set audio card name

Message ID 20210430035859.3487-8-samuel@sholland.org (mailing list archive)
State New, archived
Headers show
Series PinePhone BT audio bringup | expand

Commit Message

Samuel Holland April 30, 2021, 3:58 a.m. UTC 
From: Arnaud Ferraris <arnaud.ferraris@collabora.com>

Add the "PinePhone" name to the sound card: this will make
upstreaming an ALSA UCM config easier as we can use a unique name.

It also avoids an issue where the default card name is truncated.

Signed-off-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
[Samuel: Split out change, updated commit message]
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi | 1 +
 1 file changed, 1 insertion(+)

Comments

Maxime Ripard May 7, 2021, 8:09 a.m. UTC | #1 
Hi,

On Thu, Apr 29, 2021 at 10:58:59PM -0500, Samuel Holland wrote:
> From: Arnaud Ferraris <arnaud.ferraris@collabora.com>
> 
> Add the "PinePhone" name to the sound card: this will make
> upstreaming an ALSA UCM config easier as we can use a unique name.
> 
> It also avoids an issue where the default card name is truncated.
> 
> Signed-off-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
> [Samuel: Split out change, updated commit message]
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> index 51cbfdc12936..02712f85f6bd 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> @@ -433,6 +433,7 @@ &reg_rtc_ldo {
>  
>  &sound {
>  	status = "okay";
> +	simple-audio-card,name = "PinePhone";
>  	simple-audio-card,aux-devs = <&codec_analog>, <&speaker_amp>;
>  	simple-audio-card,widgets = "Microphone", "Headset Microphone",
>  				    "Microphone", "Internal Microphone",

Isn't that reported to the userspace? I'm not sure we can just change it
without breaking it.

Maxime
Samuel Holland May 10, 2021, 1:54 a.m. UTC | #2 
On 5/7/21 3:09 AM, Maxime Ripard wrote:
> Hi,
> 
> On Thu, Apr 29, 2021 at 10:58:59PM -0500, Samuel Holland wrote:
>> From: Arnaud Ferraris <arnaud.ferraris@collabora.com>
>>
>> Add the "PinePhone" name to the sound card: this will make
>> upstreaming an ALSA UCM config easier as we can use a unique name.
>>
>> It also avoids an issue where the default card name is truncated.
>>
>> Signed-off-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
>> [Samuel: Split out change, updated commit message]
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>> ---
>>  arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
>> index 51cbfdc12936..02712f85f6bd 100644
>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
>> @@ -433,6 +433,7 @@ &reg_rtc_ldo {
>>  
>>  &sound {
>>  	status = "okay";
>> +	simple-audio-card,name = "PinePhone";
>>  	simple-audio-card,aux-devs = <&codec_analog>, <&speaker_amp>;
>>  	simple-audio-card,widgets = "Microphone", "Headset Microphone",
>>  				    "Microphone", "Internal Microphone",
> 
> Isn't that reported to the userspace? I'm not sure we can just change it
> without breaking it.

Yes, this is seen by userspace.

Ideally, we would add this property with a unique value when enabling
audio for any board, but as you mention, it would be a breaking change
to add those properties now.

In practice, all distributions supporting the PinePhone are already
carrying this patch, because the ALSA setup needed for the PinePhone is
very different from most other boards. So it would be similarly breaking
for them (and their users) to drop this patch.

Regards,
Samuel
Maxime Ripard May 24, 2021, 12:08 p.m. UTC | #3 
On Sun, May 09, 2021 at 08:54:36PM -0500, Samuel Holland wrote:
> On 5/7/21 3:09 AM, Maxime Ripard wrote:
> > Hi,
> > 
> > On Thu, Apr 29, 2021 at 10:58:59PM -0500, Samuel Holland wrote:
> >> From: Arnaud Ferraris <arnaud.ferraris@collabora.com>
> >>
> >> Add the "PinePhone" name to the sound card: this will make
> >> upstreaming an ALSA UCM config easier as we can use a unique name.
> >>
> >> It also avoids an issue where the default card name is truncated.
> >>
> >> Signed-off-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
> >> [Samuel: Split out change, updated commit message]
> >> Signed-off-by: Samuel Holland <samuel@sholland.org>
> >> ---
> >>  arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> >> index 51cbfdc12936..02712f85f6bd 100644
> >> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
g> >> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> >> @@ -433,6 +433,7 @@ &reg_rtc_ldo {
> >>  
> >>  &sound {
> >>  	status = "okay";
> >> +	simple-audio-card,name = "PinePhone";
> >>  	simple-audio-card,aux-devs = <&codec_analog>, <&speaker_amp>;
> >>  	simple-audio-card,widgets = "Microphone", "Headset Microphone",
> >>  				    "Microphone", "Internal Microphone",
> > 
> > Isn't that reported to the userspace? I'm not sure we can just change it
> > without breaking it.
> 
> Yes, this is seen by userspace.
> 
> Ideally, we would add this property with a unique value when enabling
> audio for any board, but as you mention, it would be a breaking change
> to add those properties now.
> 
> In practice, all distributions supporting the PinePhone are already
> carrying this patch, because the ALSA setup needed for the PinePhone is
> very different from most other boards. So it would be similarly breaking
> for them (and their users) to drop this patch.

Ack :)

Since no one else complained, I merged this patch too

Thanks!
Maxime
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
index 51cbfdc12936..02712f85f6bd 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
@@ -433,6 +433,7 @@  &reg_rtc_ldo {
 
 &sound {
 	status = "okay";
+	simple-audio-card,name = "PinePhone";
 	simple-audio-card,aux-devs = <&codec_analog>, <&speaker_amp>;
 	simple-audio-card,widgets = "Microphone", "Headset Microphone",
 				    "Microphone", "Internal Microphone",