[alsa-devel] [PATCH] ASoC: da7210: Add support for spi regmap

[Ashish Chavan]

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.