Re: [PATCH] ASoC: nau8810: Add driver for Nuvoton codec chip NAU88C10

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1587 bytes --]

On Tue, Aug 16, 2016 at 11:57:17AM +0800, John Hsu wrote:
> On 8/15/2016 10:06 PM, Mark Brown wrote:
> > On Mon, Aug 15, 2016 at 05:02:23PM +0800, John Hsu wrote:

> > This looks very good overall, a few fairly small things but nothing too
> > major.

> > > +	val = (u16 *)ucontrol->value.bytes.data;
> > > +	reg = NAU8810_REG_EQ1;
> > > +	for (i = 0; i < params->max / sizeof(u16); i++) {
> > > +		regmap_read(nau8810->regmap, reg + i, &reg_val);
> > > +		reg_val = cpu_to_be16(reg_val);
> > > +		memcpy(val + i, &reg_val, sizeof(reg_val));
> > > +	}

> > This looks like it's trying to do regmap_raw_read()?  Raw I/O bypasses
> > all the endianness conversions.

> We also try regmap_raw_read() but it fails because the value width
> is 9 bits. Therefor, we make the functions by ourselves.

You're not open coding this, this is using regmap!

> > > +static struct snd_soc_codec_driver soc_codec_dev_nau8810 = {

> > > +	.controls = nau8810_snd_controls,
> > > +	.num_controls = ARRAY_SIZE(nau8810_snd_controls),
> > > +	.dapm_widgets = nau8810_dapm_widgets,
> > > +	.num_dapm_widgets = ARRAY_SIZE(nau8810_dapm_widgets),
> > > +	.dapm_routes = nau8810_dapm_routes,
> > > +	.num_dapm_routes = ARRAY_SIZE(nau8810_dapm_routes),

> > Move this data into the component driver please.

> I don't understand about it clearly. Is the snd_soc_codec_driver
> not component driver? Could you tell me more details? Thank you.

You need to set these values in the actual component driver struct, not
directly in the CODEC driver.  Look at the series of changes
Morimoto-san made recently for this.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]