Updates for Atmel SSC DAI

PATCH Superseded Andrei Simion 2 participants 7 messages Sep 16, 2024

Subsystems (4)

Sound ASoC Arm/Microchip (AT91) SoC support Microchip audio ASoC drivers

lore

Share & tools

lore

Andrei Simion

author maintainer

2024-09-16 08:53 UTC #0 lore raw

cover Updates for Atmel SSC DAI

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

Signature · 1 line

2.34.1

Andrei Simion

author maintainer

2024-09-16 08:53 UTC #1 lore raw

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

From: Codrin Ciubotariu <[email protected]>

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

[[email protected]: Adjust commit title. Reword commit message.]

Signed-off-by Codrin Ciubotariu <[email protected]>

Signed-off-by Andrei Simion <[email protected]>

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

sound/soc/atmel/atmel_ssc_dai.c +2 -0

index 3763454436c1..7047f17fe7a8 100644
@@ -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,

Signature · 1 line

2.34.1

Alexandre Belloni

maintainer Reviewed-by on v1 1/2

2024-09-16 10:00 UTC #2 lore raw

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

Quoted 8 lines · Andrei Simion

From: Codrin Ciubotariu <[email protected]>
Add required stream names for DPCM and future use-cases.
[[email protected]: Adjust commit title. Reword commit message.]
Signed-off-by Codrin Ciubotariu <[email protected]>
Signed-off-by Andrei Simion <[email protected]>

Reviewed-by Alexandre Belloni <[email protected]>

Quoted 27 lines

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

sound/soc/atmel/atmel_ssc_dai.c +2 -0

index 3763454436c1..7047f17fe7a8 100644
@@ -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,

Signature · 1 line

2.34.1

Signature · 3 lines

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

Andrei Simion

author maintainer

2024-09-16 11:34 UTC #3 lore raw

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

Quoted 26 lines · Alexandre Belloni

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
Quoted 23 lines · Andrei Simion
From: Codrin Ciubotariu <[email protected]>
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.
[[email protected]: Reword the commit title and the commit message.]
Signed-off-by Codrin Ciubotariu <[email protected]>
Signed-off-by Andrei Simion <[email protected]>
---
 sound/soc/atmel/atmel_ssc_dai.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/atmel/atmel_ssc_dai.c +0 -0
index 7047f17fe7a8..475e7579c64c 100644
@@ -822,7 +822,7 @@ static int atmel_ssc_resume(struct snd_soc_component *component)
 }
 

Quoted 7 lines

sound/soc/atmel/atmel_ssc_dai.c static int atmel_ssc_resume(struct snd_soc_component *component) · line 825
Quoted 3 lines
sound/soc/atmel/atmel_ssc_dai.c +1 -1
 #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?

Quoted 11 lines

Quoted 6 lines
sound/soc/atmel/atmel_ssc_dai.c +0 -1
 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

maintainer

2024-09-16 12:30 UTC #4 lore raw

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

Quoted 35 lines · Andrei Simion

Quoted 26 lines · Alexandre Belloni
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
Quoted 23 lines · Andrei Simion
From: Codrin Ciubotariu <[email protected]>
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.
[[email protected]: Reword the commit title and the commit message.]
Signed-off-by Codrin Ciubotariu <[email protected]>
Signed-off-by Andrei Simion <[email protected]>
---
 sound/soc/atmel/atmel_ssc_dai.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/atmel/atmel_ssc_dai.c +0 -0
index 7047f17fe7a8..475e7579c64c 100644
@@ -822,7 +822,7 @@ static int atmel_ssc_resume(struct snd_soc_component *component)
 }
 
Quoted 7 lines
sound/soc/atmel/atmel_ssc_dai.c static int atmel_ssc_resume(struct snd_soc_component *component) · line 825
Quoted 3 lines
sound/soc/atmel/atmel_ssc_dai.c +1 -1
 #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.

Quoted 2 lines

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;

Quoted 14 lines

Quoted 11 lines
Quoted 6 lines
sound/soc/atmel/atmel_ssc_dai.c +0 -1
 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

Signature · 3 lines

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

Andrei Simion

author maintainer

2024-09-16 08:53 UTC #5 lore raw

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

In reply to Andrei Simion

From: Codrin Ciubotariu <[email protected]>

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.

[[email protected]: Reword the commit title and the commit message.]

Signed-off-by Codrin Ciubotariu <[email protected]>

Signed-off-by Andrei Simion <[email protected]>

---
 sound/soc/atmel/atmel_ssc_dai.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

sound/soc/atmel/atmel_ssc_dai.c +1 -1

index 7047f17fe7a8..475e7579c64c 100644
@@ -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,

Signature · 1 line

2.34.1

Alexandre Belloni

maintainer

2024-09-16 09:59 UTC #6 lore raw

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

Quoted 21 lines · Andrei Simion

From: Codrin Ciubotariu <[email protected]>
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.
[[email protected]: Reword the commit title and the commit message.]
Signed-off-by Codrin Ciubotariu <[email protected]>
Signed-off-by Andrei Simion <[email protected]>
---
 sound/soc/atmel/atmel_ssc_dai.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/atmel/atmel_ssc_dai.c +0 -0
index 7047f17fe7a8..475e7579c64c 100644

sound/soc/atmel/atmel_ssc_dai.c static int atmel_ssc_resume(struct snd_soc_component *component) · line 825

Quoted 5 lines

sound/soc/atmel/atmel_ssc_dai.c +1 -1

@@ -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.

Quoted 6 lines

sound/soc/atmel/atmel_ssc_dai.c +0 -0
 static const struct snd_soc_dai_ops atmel_ssc_dai_ops = {
 	.startup	= atmel_ssc_startup,
Signature · 1 line
2.34.1

Signature · 3 lines

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