[alsa-devel] [PATCH] ASoC: add ak4613 support

[Kuninori Morimoto]

Hi Mark

Thank you for your feedback
Will fix in v2

> This looks basically good, a few minor comments below but nothing major:
> 
> > --- a/sound/soc/codecs/Kconfig
> > +++ b/sound/soc/codecs/Kconfig
> > @@ -319,6 +319,10 @@ config SND_SOC_AK4535
> >  config SND_SOC_AK4554
> >  	tristate "AKM AK4554 CODEC"
> >  
> > +config SND_SOC_AK4613
> > +	tristate "AKM AK4613 CODEC"
> > +	depends on I2C
> > +
> >  config SND_SOC_AK4641
> >  	tristate
> >  
> 
> You should also add this to SND_SOC_ALL_CODECS.
> 
> > +static inline void ak4613_write(struct snd_soc_codec *codec, unsigned int reg,
> > +				unsigned int val)
> > +{
> > +	struct device *dev = codec->dev;
> > +
> > +	dev_dbg(dev, "reg %02x w %02x\n", reg, val);
> > +	snd_soc_write(codec, reg, val);
> > +}
> 
> Please just use snd_soc_write() natively - there's a lot of trace
> support in regmap already which really should be adequate.  If it's not
> then we need to review and improve that rather than open coding in
> drivers.
> 
> > +	spin_lock_irqsave(&priv->lock, flags);
> > +	if ((NO_FMT   == priv->fmt_ctrl) ||
> 
> Can this spinlock just be a mutex?
> 
> > +static int ak4613_resume(struct snd_soc_codec *codec)
> > +{
> > +	struct regmap *regmap = dev_get_regmap(codec->dev, NULL);
> > +
> > +	regcache_mark_dirty(regmap);
> > +	regcache_sync(regmap);
> 
> You should really check errors here.
> [2 Digital signature <application/pgp-signature (7bit)>]
> No public key for 24D68B725D5487D0 created at 2015-09-15T03:45:38+0900 using RSA


Best regards
---
Kuninori Morimoto