[alsa-devel] [PATCH] ASoC: da7210: Add support for spi regmap
This patch adds support for spi regmap feature to existing da7210 driver.
Signed-off-by: Ashish Chavan ashish.chavan@kpitcummins.com Signed-off-by: David Dajun Chen dchen@diasemi.com --- sound/soc/codecs/da7210.c | 90 +++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 90 insertions(+), 0 deletions(-)
diff --git a/sound/soc/codecs/da7210.c b/sound/soc/codecs/da7210.c index 7843711..0f13fc4 100644 --- a/sound/soc/codecs/da7210.c +++ b/sound/soc/codecs/da7210.c @@ -17,6 +17,7 @@
#include <linux/delay.h> #include <linux/i2c.h> +#include <linux/spi/spi.h> #include <linux/regmap.h> #include <linux/slab.h> #include <linux/module.h> @@ -905,6 +906,9 @@ static int da7210_probe(struct snd_soc_codec *codec) { struct da7210_priv *da7210 = snd_soc_codec_get_drvdata(codec); int ret; +#if defined(CONFIG_SPI_MASTER) + unsigned int val; +#endif
dev_info(codec->dev, "DA7210 Audio Codec %s\n", DA7210_VERSION);
@@ -927,10 +931,23 @@ static int da7210_probe(struct snd_soc_codec *codec) * DA7210_PLL_DIV3 :: DA7210_MCLK_RANGExxx */
+#if defined(CONFIG_SPI_MASTER) + /* Dummy read to give two pulses over nCS */ + regmap_read(da7210->regmap, DA7210_STATUS, &val); + regmap_read(da7210->regmap, DA7210_STATUS, &val); + regmap_read(da7210->regmap, DA7210_STATUS, &val); +#endif + /* * make sure that DA7210 use bypass mode before start up */ snd_soc_write(codec, DA7210_STARTUP1, 0); + +#if defined(CONFIG_SPI_MASTER) + /* Dummy read to complete last write operation */ + regmap_read(da7210->regmap, DA7210_STATUS, &val); +#endif + snd_soc_write(codec, DA7210_PLL_DIV3, DA7210_MCLK_RANGE_10_20_MHZ | DA7210_PLL_BYP);
@@ -1025,6 +1042,10 @@ static int da7210_probe(struct snd_soc_codec *codec) DA7210_MCLK_RANGE_10_20_MHZ | DA7210_PLL_BYP); snd_soc_update_bits(codec, DA7210_PLL, DA7210_PLL_EN, DA7210_PLL_EN);
+#if defined(CONFIG_SPI_MASTER) + snd_soc_write(codec, 0x00, 0x80); +#endif + /* As suggested by Dialog */ /* unlock */ regmap_write(da7210->regmap, DA7210_A_HID_UNLOCK, 0x8B); @@ -1035,6 +1056,10 @@ static int da7210_probe(struct snd_soc_codec *codec) regmap_write(da7210->regmap, DA7210_A_HID_UNLOCK, 0x00); regmap_write(da7210->regmap, DA7210_A_TEST_UNLOCK, 0x00);
+#if defined(CONFIG_SPI_MASTER) + snd_soc_write(codec, 0x00, 0x00); +#endif + /* Activate all enabled subsystem */ snd_soc_write(codec, DA7210_STARTUP1, DA7210_SC_MST_EN);
@@ -1128,12 +1153,74 @@ static struct i2c_driver da7210_i2c_driver = { }; #endif
+#if defined(CONFIG_SPI_MASTER) +static int __devinit da7210_spi_probe(struct spi_device *spi) +{ + struct da7210_priv *da7210; + int ret; + + da7210 = kzalloc(sizeof(struct da7210_priv), GFP_KERNEL); + if (!da7210) + return -ENOMEM; + + spi_set_drvdata(spi, da7210); + + da7210_regmap.read_flag_mask = 0x01; + da7210_regmap.write_flag_mask = 0x00; + da7210->regmap = regmap_init_spi(spi, &da7210_regmap); + if (IS_ERR(da7210->regmap)) { + ret = PTR_ERR(da7210->regmap); + dev_err(&spi->dev, "Failed to register regmap: %d\n", ret); + goto err_alloc; + } + + ret = snd_soc_register_codec(&spi->dev, + &soc_codec_dev_da7210, &da7210_dai, 1); + if (ret < 0) + goto err_regmap; + + return ret; + +err_regmap: + regmap_exit(da7210->regmap); +err_alloc: + kfree(da7210); + + return ret; +} + +static int __devexit da7210_spi_remove(struct spi_device *spi) +{ + struct da7210_priv *da7210 = spi_get_drvdata(spi); + snd_soc_unregister_codec(&spi->dev); + regmap_exit(da7210->regmap); + kfree(da7210); + return 0; +} + +static struct spi_driver da7210_spi_driver = { + .driver = { + .name = "da7210-codec", + .owner = THIS_MODULE, + }, + .probe = da7210_spi_probe, + .remove = __devexit_p(da7210_spi_remove) +}; +#endif + static int __init da7210_modinit(void) { int ret = 0; #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE) ret = i2c_add_driver(&da7210_i2c_driver); #endif +#if defined(CONFIG_SPI_MASTER) + ret = spi_register_driver(&da7210_spi_driver); + if (ret) { + printk(KERN_ERR "Failed to register da7210 SPI driver: %d\n", + ret); + } +#endif return ret; } module_init(da7210_modinit); @@ -1143,6 +1230,9 @@ static void __exit da7210_exit(void) #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE) i2c_del_driver(&da7210_i2c_driver); #endif +#if defined(CONFIG_SPI_MASTER) + spi_unregister_driver(&da7210_spi_driver); +#endif } module_exit(da7210_exit);
On Wed, Mar 21, 2012 at 09:22:50PM +0530, Ashish Chavan wrote:
+#if defined(CONFIG_SPI_MASTER)
- /* Dummy read to give two pulses over nCS */
- regmap_read(da7210->regmap, DA7210_STATUS, &val);
- regmap_read(da7210->regmap, DA7210_STATUS, &val);
- regmap_read(da7210->regmap, DA7210_STATUS, &val);
+#endif
This ifdef stuff should all be runtime configured based on the bus type, unless the cost of the reads is considered immaterial in which case it should be unconditional.
+#if defined(CONFIG_SPI_MASTER)
- snd_soc_write(codec, 0x00, 0x80);
+#endif
Hrm?
/* unlock */ regmap_write(da7210->regmap, DA7210_A_HID_UNLOCK, 0x8B); @@ -1035,6 +1056,10 @@ static int da7210_probe(struct snd_soc_codec *codec) regmap_write(da7210->regmap, DA7210_A_HID_UNLOCK, 0x00); regmap_write(da7210->regmap, DA7210_A_TEST_UNLOCK, 0x00);
I also note that you've not yet updated this to use a regmap patch as was previously requested.
+#if defined(CONFIG_SPI_MASTER) +static int __devinit da7210_spi_probe(struct spi_device *spi) +{
- struct da7210_priv *da7210;
- int ret;
- da7210 = kzalloc(sizeof(struct da7210_priv), GFP_KERNEL);
- if (!da7210)
return -ENOMEM;
devm_kzalloc().
- da7210_regmap.read_flag_mask = 0x01;
- da7210_regmap.write_flag_mask = 0x00;
Just have a second, static, regmap variable. The regmap should be declared const.
- da7210->regmap = regmap_init_spi(spi, &da7210_regmap);
devm_regmap_init_spi() (will come in in the merge window).
+static struct spi_driver da7210_spi_driver = {
- .driver = {
.name = "da7210-codec",
No -codec.
On Wed, 2012-03-21 at 15:56 +0000, Mark Brown wrote:
On Wed, Mar 21, 2012 at 09:22:50PM +0530, Ashish Chavan wrote:
+#if defined(CONFIG_SPI_MASTER)
- /* Dummy read to give two pulses over nCS */
- regmap_read(da7210->regmap, DA7210_STATUS, &val);
- regmap_read(da7210->regmap, DA7210_STATUS, &val);
- regmap_read(da7210->regmap, DA7210_STATUS, &val);
+#endif
This ifdef stuff should all be runtime configured based on the bus type, unless the cost of the reads is considered immaterial in which case it should be unconditional.
OK, will make it runtime.
+#if defined(CONFIG_SPI_MASTER)
- snd_soc_write(codec, 0x00, 0x80);
+#endif
Hrm?
This is writing to page register. SPI register space is divided in to two pages. Registers from 0x01 to 0x80 fall in to first page. If we want to write to any register above 0x80, first we need to set page register with PAGE1. May be I should put comments to make it obvious.
/* unlock */ regmap_write(da7210->regmap, DA7210_A_HID_UNLOCK, 0x8B); @@ -1035,6 +1056,10 @@ static int da7210_probe(struct snd_soc_codec *codec) regmap_write(da7210->regmap, DA7210_A_HID_UNLOCK, 0x00); regmap_write(da7210->regmap, DA7210_A_TEST_UNLOCK, 0x00);
I also note that you've not yet updated this to use a regmap patch as was previously requested.
Actually I don't have enough details about the problem that you mentioned with this. I am waiting for inputs from some body else. Can you please elaborate the problem if you have details? so that I can correct it.
+#if defined(CONFIG_SPI_MASTER) +static int __devinit da7210_spi_probe(struct spi_device *spi) +{
- struct da7210_priv *da7210;
- int ret;
- da7210 = kzalloc(sizeof(struct da7210_priv), GFP_KERNEL);
- if (!da7210)
return -ENOMEM;
devm_kzalloc().
OK for this and rest of all comments. I will take care of them in next submission soon.
On Wed, Mar 21, 2012 at 10:11:20PM +0530, Ashish Chavan wrote:
On Wed, 2012-03-21 at 15:56 +0000, Mark Brown wrote:
This is writing to page register. SPI register space is divided in to two pages. Registers from 0x01 to 0x80 fall in to first page. If we want to write to any register above 0x80, first we need to set page register with PAGE1. May be I should put comments to make it obvious.
You also need to make sure that the register cache doesn't get confused.
/* unlock */ regmap_write(da7210->regmap, DA7210_A_HID_UNLOCK, 0x8B); @@ -1035,6 +1056,10 @@ static int da7210_probe(struct snd_soc_codec *codec) regmap_write(da7210->regmap, DA7210_A_HID_UNLOCK, 0x00); regmap_write(da7210->regmap, DA7210_A_TEST_UNLOCK, 0x00);
I also note that you've not yet updated this to use a regmap patch as was previously requested.
Actually I don't have enough details about the problem that you mentioned with this. I am waiting for inputs from some body else. Can
...and didn't bother asking any questions so it's unlikely anyone will say anything...
you please elaborate the problem if you have details? so that I can correct it.
What is unclear in the previous feedback? You should be converting this to use a regmap patch...
Actually I don't have enough details about the problem that you mentioned with this. I am waiting for inputs from some body else. Can
...and didn't bother asking any questions so it's unlikely anyone will say anything...
No, actually it's not like that. You mentioned that the sequence should be changed, which will fix an issue that the driver has with suspend and resume. I asked other users of this driver about the issue and waiting for their feedback. I just wanted to have clear picture about the issue before I commit any change there. Just to make sure that only changing of sequence is enough or some thing extra need to be done.
you please elaborate the problem if you have details? so that I can correct it.
What is unclear in the previous feedback? You should be converting this to use a regmap patch...
OK, thanks!
On Wed, Mar 21, 2012 at 10:35:53PM +0530, Ashish Chavan wrote:
Actually I don't have enough details about the problem that you mentioned with this. I am waiting for inputs from some body else. Can
...and didn't bother asking any questions so it's unlikely anyone will say anything...
No, actually it's not like that. You mentioned that the sequence should be changed, which will fix an issue that the driver has with suspend and resume. I asked other users of this driver about the issue and waiting for their feedback. I just wanted to have clear picture about the issue before I commit any change there. Just to make sure that only changing of sequence is enough or some thing extra need to be done.
No, I didn't say that the sequence should be changed. I said that it should be implemented using the relevant framework features rather than being open coded. The sequence itself is fine from a framework point of view.
participants (2)
-
Ashish Chavan
-
Mark Brown