[PATCH 0/2] Updates for Atmel SSC DAI

[PATCH 0/2] Updates for Atmel SSC DAI

[Andrei Simion]

This patch set includes two updates for the Atmel SSC DAI driver:
- Address the limitation with the S24_LE format.
- Add stream names for DPCM and future use-cases.

Codrin Ciubotariu (2):
  ASoC: atmel: atmel_ssc_dai: Add stream names
  ASoC: atmel: atmel_ssc_dai: Drop S24_LE support due to single channel
    limitation

 sound/soc/atmel/atmel_ssc_dai.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


base-commit: 5acd9952f95fb4b7da6d09a3be39195a80845eb6
-- 
2.34.1

[PATCH 1/2] ASoC: atmel: atmel_ssc_dai: Add stream names

[Andrei Simion]

From: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>

Add required stream names for DPCM and future use-cases.

[andrei.simion@microchip.com: Adjust commit title. Reword commit message.]

Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
---
 sound/soc/atmel/atmel_ssc_dai.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c
index 3763454436c1..7047f17fe7a8 100644
--- a/sound/soc/atmel/atmel_ssc_dai.c
+++ b/sound/soc/atmel/atmel_ssc_dai.c
@@ -836,6 +836,7 @@ static const struct snd_soc_dai_ops atmel_ssc_dai_ops = {
 
 static struct snd_soc_dai_driver atmel_ssc_dai = {
 		.playback = {
+			.stream_name = "Playback",
 			.channels_min = 1,
 			.channels_max = 2,
 			.rates = SNDRV_PCM_RATE_CONTINUOUS,
@@ -843,6 +844,7 @@ static struct snd_soc_dai_driver atmel_ssc_dai = {
 			.rate_max = 384000,
 			.formats = ATMEL_SSC_FORMATS,},
 		.capture = {
+			.stream_name = "Capture",
 			.channels_min = 1,
 			.channels_max = 2,
 			.rates = SNDRV_PCM_RATE_CONTINUOUS,
-- 
2.34.1

Re: [PATCH 1/2] ASoC: atmel: atmel_ssc_dai: Add stream names

[Alexandre Belloni]

On 16/09/2024 11:52:14+0300, Andrei Simion wrote:
> From: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> 
> Add required stream names for DPCM and future use-cases.
> 
> [andrei.simion@microchip.com: Adjust commit title. Reword commit message.]
> 
> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
>  sound/soc/atmel/atmel_ssc_dai.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c
> index 3763454436c1..7047f17fe7a8 100644
> --- a/sound/soc/atmel/atmel_ssc_dai.c
> +++ b/sound/soc/atmel/atmel_ssc_dai.c
> @@ -836,6 +836,7 @@ static const struct snd_soc_dai_ops atmel_ssc_dai_ops = {
>  
>  static struct snd_soc_dai_driver atmel_ssc_dai = {
>  		.playback = {
> +			.stream_name = "Playback",
>  			.channels_min = 1,
>  			.channels_max = 2,
>  			.rates = SNDRV_PCM_RATE_CONTINUOUS,
> @@ -843,6 +844,7 @@ static struct snd_soc_dai_driver atmel_ssc_dai = {
>  			.rate_max = 384000,
>  			.formats = ATMEL_SSC_FORMATS,},
>  		.capture = {
> +			.stream_name = "Capture",
>  			.channels_min = 1,
>  			.channels_max = 2,
>  			.rates = SNDRV_PCM_RATE_CONTINUOUS,
> -- 
> 2.34.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 2/2] ASoC: atmel: atmel_ssc_dai: Drop S24_LE support due to single channel limitation

[Andrei Simion]

On 16.09.2024 12:59, Alexandre Belloni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 16/09/2024 11:52:15+0300, Andrei Simion wrote:
>> From: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
>>
>> Drop S24_LE format because it is not supported if more than 2 channels
>> (of TDM slots) are used. This limitation makes it impractical for use cases
>> requiring more than 2 TDM slots, leading to potential issues in
>> multi-channel configurations.
>>
>> [andrei.simion@microchip.com: Reword the commit title and the commit
>> message.]
>>
>> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
>> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
>> ---
>>  sound/soc/atmel/atmel_ssc_dai.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c
>> index 7047f17fe7a8..475e7579c64c 100644
>> --- a/sound/soc/atmel/atmel_ssc_dai.c
>> +++ b/sound/soc/atmel/atmel_ssc_dai.c
>> @@ -822,7 +822,7 @@ static int atmel_ssc_resume(struct snd_soc_component *component)
>>  }
>>
>>  #define ATMEL_SSC_FORMATS (SNDRV_PCM_FMTBIT_S8     | SNDRV_PCM_FMTBIT_S16_LE |\
>> -                       SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
>> +                        SNDRV_PCM_FMTBIT_S32_LE)
>
> I believe this requires a comment. Else someone may add it back later
> on.
>

It is unclear to me. Would it be good to send v2 with this update?

>>
>>  static const struct snd_soc_dai_ops atmel_ssc_dai_ops = {
>>       .startup        = atmel_ssc_startup,
>> --
>> 2.34.1
>>
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Best regards,
Andrei Simion

Re: [PATCH 2/2] ASoC: atmel: atmel_ssc_dai: Drop S24_LE support due to single channel limitation

[Alexandre Belloni]

On 16/09/2024 14:32:05+0300, Andrei Simion wrote:
> On 16.09.2024 12:59, Alexandre Belloni wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > On 16/09/2024 11:52:15+0300, Andrei Simion wrote:
> >> From: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> >>
> >> Drop S24_LE format because it is not supported if more than 2 channels
> >> (of TDM slots) are used. This limitation makes it impractical for use cases
> >> requiring more than 2 TDM slots, leading to potential issues in
> >> multi-channel configurations.
> >>
> >> [andrei.simion@microchip.com: Reword the commit title and the commit
> >> message.]
> >>
> >> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> >> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
> >> ---
> >>  sound/soc/atmel/atmel_ssc_dai.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c
> >> index 7047f17fe7a8..475e7579c64c 100644
> >> --- a/sound/soc/atmel/atmel_ssc_dai.c
> >> +++ b/sound/soc/atmel/atmel_ssc_dai.c
> >> @@ -822,7 +822,7 @@ static int atmel_ssc_resume(struct snd_soc_component *component)
> >>  }
> >>
> >>  #define ATMEL_SSC_FORMATS (SNDRV_PCM_FMTBIT_S8     | SNDRV_PCM_FMTBIT_S16_LE |\
> >> -                       SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
> >> +                        SNDRV_PCM_FMTBIT_S32_LE)
> >
> > I believe this requires a comment. Else someone may add it back later
> > on.
> >
> 
> It is unclear to me. Would it be good to send v2 with this update?
> 

Yes, please send a new version. My point is that it is explained in the
commit log. This is fine when you are adding something because then
people will easily use git blame and find the commit and log. However,
because you are removing code, it is not obvious there is a reason why
it isn't there;

> >>
> >>  static const struct snd_soc_dai_ops atmel_ssc_dai_ops = {
> >>       .startup        = atmel_ssc_startup,
> >> --
> >> 2.34.1
> >>
> >
> > --
> > Alexandre Belloni, co-owner and COO, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
> 
> Best regards,
> Andrei Simion

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH 2/2] ASoC: atmel: atmel_ssc_dai: Drop S24_LE support due to single channel limitation

[Andrei Simion]

From: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>

Drop S24_LE format because it is not supported if more than 2 channels
(of TDM slots) are used. This limitation makes it impractical for use cases
requiring more than 2 TDM slots, leading to potential issues in
multi-channel configurations.

[andrei.simion@microchip.com: Reword the commit title and the commit
message.]

Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
---
 sound/soc/atmel/atmel_ssc_dai.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c
index 7047f17fe7a8..475e7579c64c 100644
--- a/sound/soc/atmel/atmel_ssc_dai.c
+++ b/sound/soc/atmel/atmel_ssc_dai.c
@@ -822,7 +822,7 @@ static int atmel_ssc_resume(struct snd_soc_component *component)
 }
 
 #define ATMEL_SSC_FORMATS (SNDRV_PCM_FMTBIT_S8     | SNDRV_PCM_FMTBIT_S16_LE |\
-			  SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
+			   SNDRV_PCM_FMTBIT_S32_LE)
 
 static const struct snd_soc_dai_ops atmel_ssc_dai_ops = {
 	.startup	= atmel_ssc_startup,
-- 
2.34.1

Re: [PATCH 2/2] ASoC: atmel: atmel_ssc_dai: Drop S24_LE support due to single channel limitation

[Alexandre Belloni]

On 16/09/2024 11:52:15+0300, Andrei Simion wrote:
> From: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> 
> Drop S24_LE format because it is not supported if more than 2 channels
> (of TDM slots) are used. This limitation makes it impractical for use cases
> requiring more than 2 TDM slots, leading to potential issues in
> multi-channel configurations.
> 
> [andrei.simion@microchip.com: Reword the commit title and the commit
> message.]
> 
> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
> ---
>  sound/soc/atmel/atmel_ssc_dai.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c
> index 7047f17fe7a8..475e7579c64c 100644
> --- a/sound/soc/atmel/atmel_ssc_dai.c
> +++ b/sound/soc/atmel/atmel_ssc_dai.c
> @@ -822,7 +822,7 @@ static int atmel_ssc_resume(struct snd_soc_component *component)
>  }
>  
>  #define ATMEL_SSC_FORMATS (SNDRV_PCM_FMTBIT_S8     | SNDRV_PCM_FMTBIT_S16_LE |\
> -			  SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
> +			   SNDRV_PCM_FMTBIT_S32_LE)

I believe this requires a comment. Else someone may add it back later
on.

>  
>  static const struct snd_soc_dai_ops atmel_ssc_dai_ops = {
>  	.startup	= atmel_ssc_startup,
> -- 
> 2.34.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com