Skip to contentSkip to conversation

ASoC: sigmadsp: remove I2C support

PATCH In review Arnd Bergmann 2 participants 5 messages Jun 5, 2014

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ASoC: sigmadsp: remove I2C support

The newly added ADAU1781 codec comes with separate drivers for I2C and SPI modes, which gives us a new variation of the dependency
problems: If SPI is enabled and the I2C core is a loadable module, selecting SND_SOC_ADAU1781_SPI as built-in leads to a link error because the common base code from sigmadsp.c now also gets built-in:

sound/built-in.o: In function `sigma_action_write_i2c':
:(.text+0x5d8d4): undefined reference to `i2c_master_send'

Rather than adding yet more complex Kconfig logic, this converts the last user of process_sigma_firmware() to process_sigma_firmware_regmap(), which seems trivial after the driver itself now uses regmap as well. With that done, there is no more linker dependency from SND_SOC_ADAU1781_SPI to i2c_master_send.

Cc Lars-Peter Clausen <[email protected]>

---
I hope this is the last ASoC patch I have for the moment, sorry for spamming everybody ;-)

 sound/soc/codecs/adau1701.c |  5 +++--
 sound/soc/codecs/sigmadsp.c | 27 ---------------------------
 sound/soc/codecs/sigmadsp.h |  3 ---
 3 files changed, 3 insertions(+), 32 deletions(-)
sound/soc/codecs/adau1701.c +3 -2
index d71c59c..1b5e306 100644
@@ -239,7 +239,6 @@ static int adau1701_reg_read(void *context, unsigned int reg,
 static int adau1701_reset(struct snd_soc_codec *codec, unsigned int clkdiv)
 {
 	struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec);
-	struct i2c_client *client = to_i2c_client(codec->dev);
 	int ret;
 
 	if (clkdiv != ADAU1707_CLKDIV_UNSET &&
@@ -282,7 +281,9 @@ static int adau1701_reset(struct snd_soc_codec *codec, unsigned int clkdiv)
 	 * know the correct PLL setup
 	 */
 	if (clkdiv != ADAU1707_CLKDIV_UNSET) {
-		ret = process_sigma_firmware(client, ADAU1701_FIRMWARE);
+		ret = process_sigma_firmware_regmap(codec->dev,
+						    adau1701->regmap,
+						    ADAU1701_FIRMWARE);
 		if (ret) {
 			dev_warn(codec->dev, "Failed to load firmware\n");
 			return ret;
sound/soc/codecs/sigmadsp.c +0 -27
index 4068f24..209f98f 100644
@@ -10,7 +10,6 @@
 #include <linux/delay.h>
 #include <linux/firmware.h>
 #include <linux/kernel.h>
-#include <linux/i2c.h>
 #include <linux/regmap.h>
 #include <linux/module.h>
 
@@ -198,30 +197,6 @@ static int _process_sigma_firmware(struct device *dev,
 	return ret;
 }
 
-#if IS_ENABLED(CONFIG_I2C)
-
-static int sigma_action_write_i2c(void *control_data,
-	const struct sigma_action *sa, size_t len)
-{
-	return i2c_master_send(control_data, (const unsigned char *)&sa->addr,
-		len);
-}
-
-int process_sigma_firmware(struct i2c_client *client, const char *name)
-{
-	struct sigma_firmware ssfw;
-
-	ssfw.control_data = client;
-	ssfw.write = sigma_action_write_i2c;
-
-	return _process_sigma_firmware(&client->dev, &ssfw, name);
-}
-EXPORT_SYMBOL(process_sigma_firmware);
-
-#endif
-
-#if IS_ENABLED(CONFIG_REGMAP)
-
 static int sigma_action_write_regmap(void *control_data,
 	const struct sigma_action *sa, size_t len)
 {
@@ -241,6 +216,4 @@ int process_sigma_firmware_regmap(struct device *dev, struct regmap *regmap,
 }
 EXPORT_SYMBOL(process_sigma_firmware_regmap);
 
-#endif
-
 MODULE_LICENSE("GPL");
sound/soc/codecs/sigmadsp.h +0 -3
index e439cbd..38a455b 100644
@@ -12,9 +12,6 @@
 #include <linux/device.h>
 #include <linux/regmap.h>
 
-struct i2c_client;
-
-extern int process_sigma_firmware(struct i2c_client *client, const char *name);
 extern int process_sigma_firmware_regmap(struct device *dev,
 		struct regmap *regmap, const char *name);
#1 lore raw
Quoted 9 lines · Arnd Bergmann

The newly added ADAU1781 codec comes with separate drivers for I2C and SPI modes, which gives us a new variation of the dependency
problems: If SPI is enabled and the I2C core is a loadable module, selecting SND_SOC_ADAU1781_SPI as built-in leads to a link error because the common base code from sigmadsp.c now also gets built-in:

sound/built-in.o: In function `sigma_action_write_i2c':
:(.text+0x5d8d4): undefined reference to `i2c_master_send'

Quoted 8 lines

Rather than adding yet more complex Kconfig logic, this converts the last user of process_sigma_firmware() to process_sigma_firmware_regmap(), which seems trivial after the driver itself now uses regmap as well. With that done, there is no more linker dependency from SND_SOC_ADAU1781_SPI to i2c_master_send.

Cc Lars-Peter Clausen <[email protected]>

No this patch won't work. Regmap support for the adau1701 is rather limited which is why we need native i2c support in the lib.

But yea, I just noticed the same problem an hour ago and was thinking hopefully I'll get to fixing this before it shows up in Arnd's randconfig builds ;)

- Lars

#2 lore raw
Quoted 16 lines · On Thursday 05 June 2014 16:55:47 Lars-Peter Clausen wrote:
Quoted 15 lines · Arnd Bergmann

The newly added ADAU1781 codec comes with separate drivers for I2C and SPI modes, which gives us a new variation of the dependency
problems: If SPI is enabled and the I2C core is a loadable module, selecting SND_SOC_ADAU1781_SPI as built-in leads to a link error because the common base code from sigmadsp.c now also gets built-in:

sound/built-in.o: In function `sigma_action_write_i2c':
:(.text+0x5d8d4): undefined reference to `i2c_master_send'

Rather than adding yet more complex Kconfig logic, this converts the last user of process_sigma_firmware() to process_sigma_firmware_regmap(), which seems trivial after the driver itself now uses regmap as well. With that done, there is no more linker dependency from SND_SOC_ADAU1781_SPI to i2c_master_send.

Quoted 5 lines
Quoted 2 lines
Cc Lars-Peter Clausen <[email protected]>

No this patch won't work. Regmap support for the adau1701 is rather limited which is why we need native i2c support in the lib.

Ah, I see.

Quoted 3 lines

But yea, I just noticed the same problem an hour ago and was thinking hopefully I'll get to fixing this before it shows up in Arnd's randconfig builds

It's actually my second attempt after I finished writing the first patch for it. Is that better?

8<---------
commit 75b46664e7c26cf044da3322c31ce8e7c4a16715

Author: Arnd Bergmann <[email protected]>
Date:   Thu Jun 5 15:47:35 2014 +0200

ASoC: fix ADAU 17x1 I2C dependencies

The newly added ADAU1761 and ADAU1781 codecs come with separate drivers for I2C and SPI modes, which gives us a new variation of the dependency
problems: If SPI is enabled and the I2C core is a loadable module, selecting SND_SOC_ADAU1781_SPI as built-in leads to a link error because the common base code from sigmadsp.c now also gets built-in:

sound/built-in.o: In function `sigma_action_write_i2c':
:(.text+0x5d8d4): undefined reference to `i2c_master_send'

To avoid that, this adds another Kconfig hack similar to the existing SND_SOC_I2C_AND_SPI symbol: the new SND_SOC_SPI_AND_MAYBE_I2C helps us enable the adau17x1-spi drivers as loadable modules in this case, but built-in when I2C is either disabled or built-in as well.

Cc Lars-Peter Clausen <[email protected]>
sound/soc/codecs/Kconfig +7 -2
index cbfa1e1..41bca14 100644
@@ -8,6 +8,11 @@ config SND_SOC_I2C_AND_SPI
 	default y if I2C=y
 	default y if SPI_MASTER=y
 
+config SND_SOC_SPI_AND_MAYBE_I2C
+	tristate
+	default m if SPI_MASTER=y && I2C=m
+	default y if SPI_MASTER=y
+
 menu "CODEC drivers"
 
 config SND_SOC_ALL_CODECS
@@ -24,9 +29,9 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_AD73311
 	select SND_SOC_ADAU1373 if I2C
 	select SND_SOC_ADAU1761_I2C if I2C
-	select SND_SOC_ADAU1761_SPI if SPI
+	select SND_SOC_ADAU1761_SPI if SND_SOC_SPI_AND_MAYBE_I2C
 	select SND_SOC_ADAU1781_I2C if I2C
-	select SND_SOC_ADAU1781_SPI if SPI
+	select SND_SOC_ADAU1781_SPI if SND_SOC_SPI_AND_MAYBE_I2C
 	select SND_SOC_ADAV801 if SPI_MASTER
 	select SND_SOC_ADAV803 if I2C
 	select SND_SOC_ADAU1977_SPI if SPI_MASTER
#3 lore raw
Quoted 23 lines · Arnd Bergmann
Quoted 16 lines · On Thursday 05 June 2014 16:55:47 Lars-Peter Clausen wrote:
Quoted 15 lines · Arnd Bergmann

The newly added ADAU1781 codec comes with separate drivers for I2C and SPI modes, which gives us a new variation of the dependency
problems: If SPI is enabled and the I2C core is a loadable module, selecting SND_SOC_ADAU1781_SPI as built-in leads to a link error because the common base code from sigmadsp.c now also gets built-in:

sound/built-in.o: In function `sigma_action_write_i2c':
:(.text+0x5d8d4): undefined reference to `i2c_master_send'

Rather than adding yet more complex Kconfig logic, this converts the last user of process_sigma_firmware() to process_sigma_firmware_regmap(), which seems trivial after the driver itself now uses regmap as well. With that done, there is no more linker dependency from SND_SOC_ADAU1781_SPI to i2c_master_send.

Quoted 5 lines
Quoted 2 lines
Cc Lars-Peter Clausen <[email protected]>

No this patch won't work. Regmap support for the adau1701 is rather limited which is why we need native i2c support in the lib.

Quoted 8 lines

Ah, I see.

Quoted 3 lines

But yea, I just noticed the same problem an hour ago and was thinking hopefully I'll get to fixing this before it shows up in Arnd's randconfig builds

It's actually my second attempt after I finished writing the first patch for it. Is that better?

That will fix the symptoms of the issue for SND_SOC_ALL_CODECS, but the problem is still there. I'll send a patch which factors the regmap and I2C sigmadsp code out into their own modules.

- Lars

#4 lore raw
Quoted 30 lines · On Thursday 05 June 2014 17:23:07 Lars-Peter Clausen wrote:
Quoted 29 lines · Arnd Bergmann
Quoted 16 lines · On Thursday 05 June 2014 16:55:47 Lars-Peter Clausen wrote:
Quoted 15 lines · Arnd Bergmann

The newly added ADAU1781 codec comes with separate drivers for I2C and SPI modes, which gives us a new variation of the dependency
problems: If SPI is enabled and the I2C core is a loadable module, selecting SND_SOC_ADAU1781_SPI as built-in leads to a link error because the common base code from sigmadsp.c now also gets built-in:

sound/built-in.o: In function `sigma_action_write_i2c':
:(.text+0x5d8d4): undefined reference to `i2c_master_send'

Rather than adding yet more complex Kconfig logic, this converts the last user of process_sigma_firmware() to process_sigma_firmware_regmap(), which seems trivial after the driver itself now uses regmap as well. With that done, there is no more linker dependency from SND_SOC_ADAU1781_SPI to i2c_master_send.

Quoted 5 lines
Quoted 2 lines
Cc Lars-Peter Clausen <[email protected]>

No this patch won't work. Regmap support for the adau1701 is rather limited which is why we need native i2c support in the lib.

Ah, I see.

Quoted 3 lines

But yea, I just noticed the same problem an hour ago and was thinking hopefully I'll get to fixing this before it shows up in Arnd's randconfig builds

Quoted 7 lines
Quoted 2 lines

It's actually my second attempt after I finished writing the first patch for it. Is that better?

That will fix the symptoms of the issue for SND_SOC_ALL_CODECS, but the problem is still there. I'll send a patch which factors the regmap and I2C sigmadsp code out into their own modules.

Ok, that sounds best, thanks!

Arnd