Skip to contentSkip to conversation

ASoC: mxs: mxs-saif: Fix the playback/record capabilities

PATCH In review Fabio Estevam 3 participants 6 messages Jun 2, 2013 – Jun 3, 2013

Subsystems (3)
lore
Share & tools
lore

Fabio Estevam

author reviewer
#0 lore raw

PATCH 1/1 ASoC: mxs: mxs-saif: Fix the playback/record capabilities

From: Fabio Estevam <[email protected]>

Running "aplay -l" and "arecord -l" results in the following outputs:

$ aplay -l
**** List of PLAYBACK Hardware Devices ****
card 0: mxssgtl5000 [mxs_sgtl5000], device 0: Playback sgtl5000-0 []
  Subdevices: 1/1
  Subdevice #0: subdevice #0
card 0: mxssgtl5000 [mxs_sgtl5000], device 1: Capture sgtl5000-1 []
  Subdevices: 1/1
  Subdevice #0: subdevice #0
$ arecord -l
**** List of CAPTURE Hardware Devices ****
card 0: mxssgtl5000 [mxs_sgtl5000], device 0: Playback sgtl5000-0 []
  Subdevices: 1/1
  Subdevice #0: subdevice #0
card 0: mxssgtl5000 [mxs_sgtl5000], device 1: Capture sgtl5000-1 []
  Subdevices: 1/1
  Subdevice #0: subdevice #0

On mx28 there are two serial audio interface ports (SAIF0 and SAIF1) and each one of them are unidirectional.

Create a mxs_saif_dai array containing a playback element and a record element only in order to properly describe the dai links.

After this change we can correctly report the capabilities as follows:

$ aplay -l                                                      
**** List of PLAYBACK Hardware Devices ****                                     
card 0: mxssgtl5000 [mxs_sgtl5000], device 0: HiFi Playback sgtl5000-0 []       
  Subdevices: 1/1                                                               
  Subdevice #0: subdevice #0                                                    
$ arecord -l                                                    
**** List of CAPTURE Hardware Devices ****                                      
card 0: mxssgtl5000 [mxs_sgtl5000], device 1: HiFi Capture sgtl5000-1 []        
  Subdevices: 1/1                                                               
  Subdevice #0: subdevice #0                                                    

---
This patch also needs:
"[PATCH] ASoC: pcm: Require both CODEC and CPU support when declaring stream caps" from Mark Brown.

 sound/soc/mxs/mxs-saif.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)
sound/soc/mxs/mxs-saif.c +22 -16
index 49d8700..7f3e67a 100644
@@ -607,22 +607,28 @@ static int mxs_saif_dai_probe(struct snd_soc_dai *dai)
 	return 0;
 }
 
-static struct snd_soc_dai_driver mxs_saif_dai = {
-	.name = "mxs-saif",
-	.probe = mxs_saif_dai_probe,
-	.playback = {
-		.channels_min = 2,
-		.channels_max = 2,
-		.rates = MXS_SAIF_RATES,
-		.formats = MXS_SAIF_FORMATS,
+static struct snd_soc_dai_driver mxs_saif_dai[2] = {
+	{
+		.name = "saif0",
+		.probe = mxs_saif_dai_probe,
+		.playback = {
+			.channels_min = 2,
+			.channels_max = 2,
+			.rates = MXS_SAIF_RATES,
+			.formats = MXS_SAIF_FORMATS,
+		},
+		.ops = &mxs_saif_dai_ops,
+	}, {
+		.name = "saif1",
+		.probe = mxs_saif_dai_probe,
+		.capture = {
+			.channels_min = 2,
+			.channels_max = 2,
+			.rates = MXS_SAIF_RATES,
+			.formats = MXS_SAIF_FORMATS,
+		},
+		.ops = &mxs_saif_dai_ops,
 	},
-	.capture = {
-		.channels_min = 2,
-		.channels_max = 2,
-		.rates = MXS_SAIF_RATES,
-		.formats = MXS_SAIF_FORMATS,
-	},
-	.ops = &mxs_saif_dai_ops,
 };
 
 static const struct snd_soc_component_driver mxs_saif_component = {
@@ -735,7 +741,7 @@ static int mxs_saif_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, saif);
 
 	ret = snd_soc_register_component(&pdev->dev, &mxs_saif_component,
-					 &mxs_saif_dai, 1);
+					 &mxs_saif_dai[saif->id], 1);
 	if (ret) {
 		dev_err(&pdev->dev, "register DAI failed\n");
 		return ret;
Signature · 1 line
1.8.1.2
#1 lore raw
Quoted 36 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>

Running "aplay -l" and "arecord -l" results in the following outputs:

$ aplay -l
**** List of PLAYBACK Hardware Devices ****
card 0: mxssgtl5000 [mxs_sgtl5000], device 0: Playback sgtl5000-0 []
  Subdevices: 1/1
  Subdevice #0: subdevice #0
card 0: mxssgtl5000 [mxs_sgtl5000], device 1: Capture sgtl5000-1 []
  Subdevices: 1/1
  Subdevice #0: subdevice #0
$ arecord -l
**** List of CAPTURE Hardware Devices ****
card 0: mxssgtl5000 [mxs_sgtl5000], device 0: Playback sgtl5000-0 []
  Subdevices: 1/1
  Subdevice #0: subdevice #0
card 0: mxssgtl5000 [mxs_sgtl5000], device 1: Capture sgtl5000-1 []
  Subdevices: 1/1
  Subdevice #0: subdevice #0

On mx28 there are two serial audio interface ports (SAIF0 and SAIF1) and each one of them are unidirectional.

Create a mxs_saif_dai array containing a playback element and a record element only in order to properly describe the dai links.

After this change we can correctly report the capabilities as follows:

$ aplay -l                                                      
**** List of PLAYBACK Hardware Devices ****                                     
card 0: mxssgtl5000 [mxs_sgtl5000], device 0: HiFi Playback sgtl5000-0 []       
  Subdevices: 1/1                                                               
  Subdevice #0: subdevice #0                                                    
Quoted 7 lines
$ arecord -l                                                    
**** List of CAPTURE Hardware Devices ****                                      
card 0: mxssgtl5000 [mxs_sgtl5000], device 1: HiFi Capture sgtl5000-1 []        
  Subdevices: 1/1                                                               
  Subdevice #0: subdevice #0                                                    

Hi,

Not sure if this is the right approach, there is no limitation on either saif0 or saif1 that it can only be used for capture or playback. Some systems may use saif1 as playback and saif0 as capture, or both as playback and so on...

- Lars

Quoted 65 lines

---
This patch also needs:
"[PATCH] ASoC: pcm: Require both CODEC and CPU support when declaring stream caps" from Mark Brown.

 sound/soc/mxs/mxs-saif.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)
sound/soc/mxs/mxs-saif.c +22 -16
index 49d8700..7f3e67a 100644
@@ -607,22 +607,28 @@ static int mxs_saif_dai_probe(struct snd_soc_dai *dai)
 	return 0;
 }
 
-static struct snd_soc_dai_driver mxs_saif_dai = {
-	.name = "mxs-saif",
-	.probe = mxs_saif_dai_probe,
-	.playback = {
-		.channels_min = 2,
-		.channels_max = 2,
-		.rates = MXS_SAIF_RATES,
-		.formats = MXS_SAIF_FORMATS,
+static struct snd_soc_dai_driver mxs_saif_dai[2] = {
+	{
+		.name = "saif0",
+		.probe = mxs_saif_dai_probe,
+		.playback = {
+			.channels_min = 2,
+			.channels_max = 2,
+			.rates = MXS_SAIF_RATES,
+			.formats = MXS_SAIF_FORMATS,
+		},
+		.ops = &mxs_saif_dai_ops,
+	}, {
+		.name = "saif1",
+		.probe = mxs_saif_dai_probe,
+		.capture = {
+			.channels_min = 2,
+			.channels_max = 2,
+			.rates = MXS_SAIF_RATES,
+			.formats = MXS_SAIF_FORMATS,
+		},
+		.ops = &mxs_saif_dai_ops,
 	},
-	.capture = {
-		.channels_min = 2,
-		.channels_max = 2,
-		.rates = MXS_SAIF_RATES,
-		.formats = MXS_SAIF_FORMATS,
-	},
-	.ops = &mxs_saif_dai_ops,
 };
 
 static const struct snd_soc_component_driver mxs_saif_component = {
@@ -735,7 +741,7 @@ static int mxs_saif_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, saif);
 
 	ret = snd_soc_register_component(&pdev->dev, &mxs_saif_component,
-					 &mxs_saif_dai, 1);
+					 &mxs_saif_dai[saif->id], 1);
 	if (ret) {
 		dev_err(&pdev->dev, "register DAI failed\n");
 		return ret;
maintainer
#2 lore raw
Quoted 4 lines · Lars-Peter Clausen

Not sure if this is the right approach, there is no limitation on either saif0 or saif1 that it can only be used for capture or playback. Some systems may use saif1 as playback and saif0 as capture, or both as playback and so on...

Is there a limitation on the CODEC side or does the CODEC side have a single bidirectional interface?

#3 lore raw
Quoted 9 lines · Mark Brown
Quoted 4 lines · Lars-Peter Clausen

Not sure if this is the right approach, there is no limitation on either saif0 or saif1 that it can only be used for capture or playback. Some systems may use saif1 as playback and saif0 as capture, or both as playback and so on...

Is there a limitation on the CODEC side or does the CODEC side have a single bidirectional interface?

I think the sgtl5000 has a single bidirectional DAI, but I'd assume that it is not the only CODEC to be ever used with the mxs-saif. In my opinion a better solution is to add support for being able to specify if a snd_soc_dai_link is used playback, capture or both.

- Lars

author reviewer
#4 lore raw
Quoted 1 line · Lars-Peter Clausen

I think the sgtl5000 has a single bidirectional DAI, but I'd assume that it

Yes, correct.

Quoted 3 lines

is not the only CODEC to be ever used with the mxs-saif. In my opinion a better solution is to add support for being able to specify if a snd_soc_dai_link is used playback, capture or both.

Yes, I think that adding a device tree property specifying the SAIF direction could do the trick.

maintainer
#5 lore raw
Quoted 5 lines · Fabio Estevam
Quoted 1 line · Lars-Peter Clausen

I think the sgtl5000 has a single bidirectional DAI, but I'd assume that it

Yes, correct.

So this comes from the board wiring then.

Quoted 6 lines
Quoted 3 lines

is not the only CODEC to be ever used with the mxs-saif. In my opinion a better solution is to add support for being able to specify if a snd_soc_dai_link is used playback, capture or both.

Yes, I think that adding a device tree property specifying the SAIF direction could do the trick.

No, this should be done at the board level as Lars-Peter suggests - it's nothing to do with the SAIF driver. Probably doesn't even need to go into device tree, just do it in the machine driver.