* [PATCH] ASoC: da7210: Add support for spi regmap
@ 2012-03-21 15:52 Ashish Chavan
2012-03-21 15:56 ` [alsa-devel] " Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Ashish Chavan @ 2012-03-21 15:52 UTC (permalink / raw)
To: Mark Brown, lrg, alsa-devel
Cc: linux-kernel, kuninori.morimoto.gx, David Dajun Chen
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);
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [alsa-devel] [PATCH] ASoC: da7210: Add support for spi regmap
2012-03-21 15:52 [PATCH] ASoC: da7210: Add support for spi regmap Ashish Chavan
@ 2012-03-21 15:56 ` Mark Brown
2012-03-21 16:41 ` Ashish Chavan
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2012-03-21 15:56 UTC (permalink / raw)
To: Ashish Chavan
Cc: lrg, alsa-devel, David Dajun Chen, kuninori.morimoto.gx,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1622 bytes --]
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.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ASoC: da7210: Add support for spi regmap
2012-03-21 15:56 ` [alsa-devel] " Mark Brown
@ 2012-03-21 16:41 ` Ashish Chavan
2012-03-21 16:38 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Ashish Chavan @ 2012-03-21 16:41 UTC (permalink / raw)
To: Mark Brown
Cc: alsa-devel, kuninori.morimoto.gx, linux-kernel, David, Chen, lrg
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ASoC: da7210: Add support for spi regmap
2012-03-21 16:41 ` Ashish Chavan
@ 2012-03-21 16:38 ` Mark Brown
2012-03-21 17:05 ` Ashish Chavan
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2012-03-21 16:38 UTC (permalink / raw)
To: Ashish Chavan
Cc: linux-kernel, alsa-devel, lrg, kuninori.morimoto.gx,
David Dajun Chen
[-- Attachment #1.1: Type: text/plain, Size: 1293 bytes --]
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...
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ASoC: da7210: Add support for spi regmap
2012-03-21 16:38 ` Mark Brown
@ 2012-03-21 17:05 ` Ashish Chavan
2012-03-21 17:22 ` [alsa-devel] " Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Ashish Chavan @ 2012-03-21 17:05 UTC (permalink / raw)
To: Mark Brown
Cc: alsa-devel, kuninori.morimoto.gx, linux-kernel, David, Chen, lrg
> > 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!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [alsa-devel] [PATCH] ASoC: da7210: Add support for spi regmap
2012-03-21 17:05 ` Ashish Chavan
@ 2012-03-21 17:22 ` Mark Brown
0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2012-03-21 17:22 UTC (permalink / raw)
To: Ashish Chavan
Cc: lrg, alsa-devel, David Dajun Chen, kuninori.morimoto.gx,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 959 bytes --]
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.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-03-21 17:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-21 15:52 [PATCH] ASoC: da7210: Add support for spi regmap Ashish Chavan
2012-03-21 15:56 ` [alsa-devel] " Mark Brown
2012-03-21 16:41 ` Ashish Chavan
2012-03-21 16:38 ` Mark Brown
2012-03-21 17:05 ` Ashish Chavan
2012-03-21 17:22 ` [alsa-devel] " Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).