ASoC: mxs: Several cleanups and bugfixes for the MXS sound driver

PATCH Applied Lothar Waßmann 3 participants 19 messages Dec 9, 2011 – Dec 12, 2011

Subsystems (4)

Sound Arm i.MX and Layerscape ASoC NXP SGTL5000 driver

lore

Share & tools

lore

Lothar Waßmann

author

2011-12-09 13:38 UTC #0 lore raw

cover ASoC: mxs: Several cleanups and bugfixes for the MXS sound driver

Lothar WaÃŸmann (5):
General cleanups
Add .owner to struct snd_soc_card
Export symbols 'mxs_saif_put_mclk()' and 'mxs_saif_get_mclk()' Add missing MODULE_LICENSE("GPL")
Add appropriate MODULE_ALIAS()

 sound/soc/codecs/sgtl5000.c  |   13 ++++++-------
 sound/soc/mxs/mxs-pcm.c      |    3 +++
 sound/soc/mxs/mxs-saif.c     |    4 +++-
 sound/soc/mxs/mxs-sgtl5000.c |    2 ++
 4 files changed, 14 insertions(+), 8 deletions(-)

Lothar Waßmann

author

2011-12-09 13:38 UTC #1 lore raw

PATCH 1/5 ASoC: mxs: General cleanups

Some textual cleanups without functional changes

Signed-off-by Lothar Waßmann <[email protected]>

---
 sound/soc/codecs/sgtl5000.c |   13 ++++++-------
 sound/soc/mxs/mxs-saif.c    |    2 +-
 2 files changed, 7 insertions(+), 8 deletions(-)

sound/soc/codecs/sgtl5000.c +6 -7

index bbcf921..fd0489b 100644
@@ -73,7 +73,7 @@ enum sgtl5000_regulator_supplies {
 static const char *supply_names[SGTL5000_SUPPLY_NUM] = {
 	"VDDA",
 	"VDDIO",
-	"VDDD"
+	"VDDD",
 };
 
 #define LDO_CONSUMER_NAME	"VDDD_LDO"

@@ -1278,7 +1278,6 @@ err_regulator_free:
 	if (external_vddd)
 		ldo_regulator_remove(codec);
 	return ret;
-
 }
 
 static int sgtl5000_probe(struct snd_soc_codec *codec)

@@ -1431,7 +1430,7 @@ static __devexit int sgtl5000_i2c_remove(struct i2c_client *client)
 
 static const struct i2c_device_id sgtl5000_id[] = {
 	{"sgtl5000", 0},
-	{},
+	{ /* sentinel */ }
 };
 
 MODULE_DEVICE_TABLE(i2c, sgtl5000_id);

@@ -1444,10 +1443,10 @@ MODULE_DEVICE_TABLE(of, sgtl5000_dt_ids);
 
 static struct i2c_driver sgtl5000_i2c_driver = {
 	.driver = {
-		   .name = "sgtl5000",
-		   .owner = THIS_MODULE,
-		   .of_match_table = sgtl5000_dt_ids,
-		   },
+		.name = "sgtl5000",
+		.owner = THIS_MODULE,
+		.of_match_table = sgtl5000_dt_ids,
+	},
 	.probe = sgtl5000_i2c_probe,
 	.remove = __devexit_p(sgtl5000_i2c_remove),
 	.id_table = sgtl5000_id,

sound/soc/mxs/mxs-saif.c +1 -1

index 76dc74d..487d2ff 100644
@@ -481,7 +481,7 @@ static int mxs_saif_trigger(struct snd_pcm_substream *substream, int cmd,
 
 		/*
 		 * If the saif's master is not himself, we also need to enable
-		 * itself clk for its internal basic logic to work.
+		 * its clk for its internal basic logic to work.
 		 */
 		if (saif != master_saif) {
 			clk_enable(saif->clk);

Signature · 6 lines

1.5.6.5

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Lothar Waßmann

author

2011-12-09 13:38 UTC #2 lore raw

PATCH 2/5 ASoC: mxs: add .owner to struct snd_soc_card

This will prevent the module from being removed from underneath its users

Signed-off-by Lothar Waßmann <[email protected]>

---
 sound/soc/mxs/mxs-sgtl5000.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

sound/soc/mxs/mxs-sgtl5000.c +1 -0

index 7fbeaec..db5c184 100644
@@ -105,6 +105,7 @@ static struct snd_soc_dai_link mxs_sgtl5000_dai[] = {
 
 static struct snd_soc_card mxs_sgtl5000 = {
 	.name		= "mxs_sgtl5000",
+	.owner		= THIS_MODULE,
 	.dai_link	= mxs_sgtl5000_dai,
 	.num_links	= ARRAY_SIZE(mxs_sgtl5000_dai),
 };

Signature · 1 line

1.5.6.5

Lothar Waßmann

author

2011-12-09 13:38 UTC #3 lore raw

PATCH 3/5 ASoC: mxs: Export symbols 'mxs_saif_put_mclk()' and 'mxs_saif_get_mclk()'

Fixes the following errors with MXS sound support built as module:
|ERROR: "mxs_saif_put_mclk" [sound/soc/mxs/snd-soc-mxs-sgtl5000.ko] undefined! |ERROR: "mxs_saif_get_mclk" [sound/soc/mxs/snd-soc-mxs-sgtl5000.ko] undefined!

Signed-off-by Lothar Waßmann <[email protected]>

---
 sound/soc/mxs/mxs-saif.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

sound/soc/mxs/mxs-saif.c +2 -0

index 487d2ff..ec66a29 100644
@@ -221,6 +221,7 @@ int mxs_saif_put_mclk(unsigned int saif_id)
 	saif->mclk_in_use = 0;
 	return 0;
 }
+EXPORT_SYMBOL(mxs_saif_put_mclk);
 
 /*
  * Get MCLK and set clock rate, then enable it

@@ -274,6 +275,7 @@ int mxs_saif_get_mclk(unsigned int saif_id, unsigned int mclk,
 
 	return 0;
 }
+EXPORT_SYMBOL(mxs_saif_get_mclk);
 
 /*
  * SAIF DAI format configuration.

Signature · 6 lines

1.5.6.5

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Lothar Waßmann

author

2011-12-09 13:38 UTC #4 lore raw

PATCH 4/5 ASoC: mxs: Add missing MODULE_LICENSE("GPL")

The sound driver refuses to load as module, because of the missing MODULE_LICENSE("GPL").
The file header indicates that the driver is indeed published under the GPL.

Signed-off-by Lothar Waßmann <[email protected]>

---
 sound/soc/mxs/mxs-pcm.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

sound/soc/mxs/mxs-pcm.c +2 -0

index dea5aa4..8646913 100644
@@ -357,3 +357,5 @@ static void __exit snd_mxs_pcm_exit(void)
 	platform_driver_unregister(&mxs_pcm_driver);
 }
 module_exit(snd_mxs_pcm_exit);
+
+MODULE_LICENSE("GPL");

Signature · 1 line

1.5.6.5

Lothar Waßmann

author

2011-12-09 13:38 UTC #5 lore raw

PATCH 5/5 ASoC: mxs: Add appropriate MODULE_ALIAS()

Signed-off-by Lothar Waßmann <[email protected]>

---
 sound/soc/mxs/mxs-pcm.c      |    1 +
 sound/soc/mxs/mxs-sgtl5000.c |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

sound/soc/mxs/mxs-pcm.c +1 -0

index 8646913..f39d7dd 100644
@@ -359,3 +359,4 @@ static void __exit snd_mxs_pcm_exit(void)
 module_exit(snd_mxs_pcm_exit);
 
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:mxs-pcm-audio");

sound/soc/mxs/mxs-sgtl5000.c +1 -0

index db5c184..228bbc5 100644
@@ -172,3 +172,4 @@ module_exit(mxs_sgtl5000_exit);
 MODULE_AUTHOR("Freescale Semiconductor, Inc.");
 MODULE_DESCRIPTION("MXS ALSA SoC Machine driver");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:mxs-sgtl5000");

Signature · 6 lines

1.5.6.5

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Wolfram Sang

Acked-by on v1 5/5

2011-12-09 14:17 UTC #6 lore raw

Re: [alsa-devel] [PATCH 5/5] ASoC: mxs: Add appropriate MODULE_ALIAS()

Quoted 2 lines · Lothar Waßmann

Signed-off-by Lothar Waßmann <[email protected]>

Acked-by Wolfram Sang <[email protected]>

Signature · 2 lines

Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Mark Brown

Applied

2011-12-11 03:00 UTC #7 lore raw

Re: [PATCH 5/5] ASoC: mxs: Add appropriate MODULE_ALIAS()

In reply to Lothar Waßmann

Quoted 2 lines · Lothar Waßmann

Signed-off-by Lothar Waßmann <[email protected]>

Applied, thanks.

Wolfram Sang

Acked-by on v1 4/5

2011-12-09 14:17 UTC #8 lore raw

Re: [PATCH 4/5] ASoC: mxs: Add missing MODULE_LICENSE("GPL")

In reply to Lothar Waßmann

Quoted 6 lines · Lothar Waßmann

The sound driver refuses to load as module, because of the missing MODULE_LICENSE("GPL").
The file header indicates that the driver is indeed published under the GPL.
Signed-off-by Lothar Waßmann <[email protected]>

Acked-by Wolfram Sang <[email protected]>

Sascha's code was also GPL and this code was derived from it.

Signature · 2 lines

Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Mark Brown

Applied

2011-12-11 02:59 UTC #9 lore raw

Re: [PATCH 4/5] ASoC: mxs: Add missing MODULE_LICENSE("GPL")

In reply to Lothar Waßmann

Quoted 4 lines · Lothar Waßmann

The sound driver refuses to load as module, because of the missing MODULE_LICENSE("GPL").
The file header indicates that the driver is indeed published under the GPL.

Applied, thanks.

Wolfram Sang

2011-12-09 14:15 UTC #10 lore raw

Re: [alsa-devel] [PATCH 3/5] ASoC: mxs: Export symbols 'mxs_saif_put_mclk()' and 'mxs_saif_get_mclk()'

In reply to Lothar Waßmann

Quoted 14 lines · Lothar Waßmann

Fixes the following errors with MXS sound support built as module:
|ERROR: "mxs_saif_put_mclk" [sound/soc/mxs/snd-soc-mxs-sgtl5000.ko] undefined! |ERROR: "mxs_saif_get_mclk" [sound/soc/mxs/snd-soc-mxs-sgtl5000.ko] undefined!
Signed-off-by Lothar Waßmann <[email protected]>
---
 sound/soc/mxs/mxs-saif.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
sound/soc/mxs/mxs-saif.c +0 -0
index 487d2ff..ec66a29 100644

sound/soc/mxs/mxs-saif.c int mxs_saif_put_mclk(unsigned int saif_id) · line 224

Quoted 4 lines

sound/soc/mxs/mxs-saif.c +1 -0

@@ -221,6 +221,7 @@ int mxs_saif_put_mclk(unsigned int saif_id)
 	saif->mclk_in_use = 0;
 	return 0;
 }
+EXPORT_SYMBOL(mxs_saif_put_mclk);

EXPORT_SYMBOL_GPL. I'd vote this is an implementation issue.

Signature · 2 lines

Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Mark Brown

2011-12-11 02:56 UTC #11 lore raw

Re: [alsa-devel] [PATCH 3/5] ASoC: mxs: Export symbols 'mxs_saif_put_mclk()' and 'mxs_saif_get_mclk()'

Quoted 5 lines · Wolfram Sang

Quoted 1 line · Lothar Waßmann
+EXPORT_SYMBOL(mxs_saif_put_mclk);
EXPORT_SYMBOL_GPL. I'd vote this is an implementation issue.

Yes, please - the entire subsystem is exported _GPL.

Wolfram Sang

2011-12-09 14:12 UTC #12 lore raw

Re: [alsa-devel] [PATCH 2/5] ASoC: mxs: add .owner to struct snd_soc_card

In reply to Lothar Waßmann

Quoted 4 lines · Lothar Waßmann

This will prevent the module from being removed from underneath its users
Signed-off-by Lothar Waßmann <[email protected]>

I can't find any other instance setting owner in snd_soc_card?

Signature · 2 lines

Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Lothar Waßmann

author

2011-12-09 14:36 UTC #13 lore raw

Re: [alsa-devel] [PATCH 2/5] ASoC: mxs: add .owner to struct snd_soc_card

Hi,

Quoted 8 lines · Wolfram Sang

Quoted 4 lines · Lothar Waßmann
This will prevent the module from being removed from underneath its users
Signed-off-by Lothar Waßmann <[email protected]>
I can't find any other instance setting owner in snd_soc_card?

Then that other uses are most probably faulty too.

You can easily test it by loading all necessary sound modules, starting some application that uses the sound device (e.g. alsamixer) and then removing the snd-soc-mxs-sgtl5000 module.

Without the patch the removal will succeed crashing the application. With this patch the refcount of the module will be

Quoted 1 line

0 as long as an application is using it, preventing the

removal.

Lothar Waßmann

Signature · 13 lines

___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | [email protected]
___________________________________________________________
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Mark Brown

2011-12-11 02:54 UTC #14 lore raw

Re: [alsa-devel] [PATCH 2/5] ASoC: mxs: add .owner to struct snd_soc_card

Quoted 5 lines · Lothar Waßmann

Quoted 1 line · Wolfram Sang
I can't find any other instance setting owner in snd_soc_card?
Then that other uses are most probably faulty too.

This needs to be fixed up generally, not just for a specific driver.

Lothar Waßmann

author

2011-12-12 09:10 UTC #15 lore raw

Re: [alsa-devel] [PATCH 2/5] ASoC: mxs: add .owner to struct snd_soc_card

Hi,

Quoted 9 lines · Mark Brown

Quoted 5 lines · Lothar Waßmann
Quoted 1 line · Wolfram Sang
I can't find any other instance setting owner in snd_soc_card?
Then that other uses are most probably faulty too.
This needs to be fixed up generally, not just for a specific driver.

There are 84 drivers affected by this. I did not check though whether all of them are compilable as module.
I could provide a patch for all of these, but I can at most do a compile testing for some of them.

Lothar Waßmann

Signature · 9 lines

___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | [email protected]
___________________________________________________________

Mark Brown

2011-12-12 10:04 UTC #16 lore raw

Re: [alsa-devel] [PATCH 2/5] ASoC: mxs: add .owner to struct snd_soc_card

Quoted 4 lines · Lothar Waßmann

There are 84 drivers affected by this. I did not check though whether all of them are compilable as module.
I could provide a patch for all of these, but I can at most do a compile testing for some of them.

Compile only testing is absolutely fine.

Wolfram Sang

Acked-by on v1 1/5

2011-12-09 14:07 UTC #17 lore raw

Re: [PATCH 1/5] ASoC: mxs: General cleanups

In reply to Lothar Waßmann

Quoted 7 lines · Lothar Waßmann

Some textual cleanups without functional changes
Signed-off-by Lothar Waßmann <[email protected]>
---
 sound/soc/codecs/sgtl5000.c |   13 ++++++-------
 sound/soc/mxs/mxs-saif.c    |    2 +-
 2 files changed, 7 insertions(+), 8 deletions(-)

I wonder if it is okay to change those two files in one patch, but for the changes itself:

Acked-by Wolfram Sang <[email protected]>

Signature · 2 lines

Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Mark Brown

2011-12-11 03:04 UTC #18 lore raw

Re: [PATCH 1/5] ASoC: mxs: General cleanups

In reply to Lothar Waßmann

Quoted 6 lines · Lothar Waßmann

Some textual cleanups without functional changes
Signed-off-by Lothar Waßmann <[email protected]>
---
 sound/soc/codecs/sgtl5000.c |   13 ++++++-------
 sound/soc/mxs/mxs-saif.c    |    2 +-

This is mostly a patch to a CODEC driver not anything to do with mxs as the subject says and

sound/soc/codecs/sgtl5000.c static __devexit int sgtl5000_i2c_remove(struct i2c_client *client) · line 1433

Quoted 4 lines

static const struct i2c_device_id sgtl5000_id[] = {
sound/soc/codecs/sgtl5000.c +1 -1
 	{"sgtl5000", 0},
-	{},
+	{ /* sentinel */ }

I'm not sure I actually find that an improvement; if we're going to actively go out and do that we should do it consistently.