ASoC: da7210: Add support for spi regmap
Subsystems (3)
PATCH 1/1 ASoC: da7210: Add support for spi regmap
This patch adds support for spi regmap feature to existing da7210 driver.
--- sound/soc/codecs/da7210.c | 90 +++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 90 insertions(+), 0 deletions(-)
sound/soc/codecs/da7210.c +90 -0
@@ -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);
Signature · 1 line
1.7.1
sound/soc/codecs/da7210.c static int da7210_probe(struct snd_soc_codec *codec) · line 939
Quoted 6 lines · Ashish Chavan
sound/soc/codecs/da7210.c +6
-0+#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.
sound/soc/codecs/da7210.c static int da7210_probe(struct snd_soc_codec *codec) · line 1047
Quoted 3 lines
sound/soc/codecs/da7210.c +3
-0+#if defined(CONFIG_SPI_MASTER ) +snd_soc_write (codec,0x00 ,0x80 ); +#endif
Hrm?
sound/soc/codecs/da7210.c static int da7210_probe(struct snd_soc_codec *codec) · line 1057
Quoted 5 lines
sound/soc/codecs/da7210.c +0
-0/* 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.
sound/soc/codecs/da7210.c static struct i2c_driver da7210_i2c_driver = { · line 1164
Quoted 9 lines
sound/soc/codecs/da7210.c +9
-0+#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().
sound/soc/codecs/da7210.c static struct i2c_driver da7210_i2c_driver = { · line 1169
Quoted 2 lines
sound/soc/codecs/da7210.c +2
-0+ 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.
sound/soc/codecs/da7210.c static struct i2c_driver da7210_i2c_driver = { · line 1170
Quoted 1 line
sound/soc/codecs/da7210.c +1
-0+ da7210->regmap =regmap_init_spi (spi, &da7210_regmap);
devm_regmap_init_spi() (will come in in the merge window).
sound/soc/codecs/da7210.c static struct i2c_driver da7210_i2c_driver = { · line 1203
Quoted 3 lines
sound/soc/codecs/da7210.c +3
-0+static struct spi_driver da7210_spi_driver = { + .driver = { + .name ="da7210-codec" ,
No -codec.
Quoted 13 lines · Mark Brown
sound/soc/codecs/da7210.c static int da7210_probe(struct snd_soc_codec *codec) · line 939
Quoted 6 lines · Ashish Chavan
sound/soc/codecs/da7210.c +6
-0+#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.
Quoted 5 lines
sound/soc/codecs/da7210.c static int da7210_probe(struct snd_soc_codec *codec) · line 1047
Quoted 3 lines
sound/soc/codecs/da7210.c +3
-0+#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.
Quoted 9 lines
sound/soc/codecs/da7210.c static int da7210_probe(struct snd_soc_codec *codec) · line 1057
Quoted 6 lines
sound/soc/codecs/da7210.c +0
-0/* 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.
Quoted 12 lines
sound/soc/codecs/da7210.c static struct i2c_driver da7210_i2c_driver = { · line 1164
Quoted 10 lines
sound/soc/codecs/da7210.c +9
-0+#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.
Quoted 6 lines · Ashish Chavan
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
0x01to0x80fall in to first page. If we want to write to any register above0x80, 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.
Quoted 11 lines
Quoted 8 lines
sound/soc/codecs/da7210.c static int da7210_probe(struct snd_soc_codec *codec) · line 1057
Quoted 5 lines
sound/soc/codecs/da7210.c +0
-0/* 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...
Quoted 2 lines
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...
Quoted 6 lines
Quoted 3 lines
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.
Quoted 6 lines
Quoted 3 lines
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!
Quoted 6 lines · Ashish Chavan
Quoted 5 lines
Quoted 2 lines
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...
Quoted 6 lines
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.