[alsa-devel] [PATCH] ASoC: nau8810: Add driver for Nuvoton codec chip NAU88C10

[Mark Brown]

On Fri, Jul 01, 2016 at 11:34:31AM +0800, John Hsu wrote:
> On 6/28/2016 1:15 AM, Mark Brown wrote:

> > This looks like a regmap with 7 bit registers and 9 bit value.  Why
> > aren't we just using the standard regmap support for this?

> Yes, that is the i2c format of this codec. The format is not common,
> and the register map only supports write but not supports read.
> The driver only can read information from cache, but it can't read
> the read-only register. Thus, we need to have our own read and write
> function for codec.

No, you don't - this is entirely normal for 7x9 regmaps, I've never seen
such a device that supported readback.  Look at devices like wm8731 for
examples.

> > > +	SOC_SINGLE("Digital Loopback Switch", NAU8810_REG_COMP,
> > > +		NAU8810_ADDAP_SFT, 1, 0),

> > This looks like it should be a DAPM control.

> The function is only for debug normally. The playback and capture
> shouldn't enable the function. Thus, we only put it in the user
> control.

If it's for routing it should go into DAPM, someone might find a use for
it and it'll stop confusion.

> > > +	nau8810->div_id = div_id;
> > > +	if (div_id != NAU8810_MCLK_DIV_MCLK)
> > > +		/* Defer the master clock prescaler configuration to DAI
> > > +		 * hardware parameter if master clock from MCLK because
> > > +		 * it needs runtime fs information to get the proper div.
> > > +		 */
> > > +		ret = nau8810_config_clkdiv(nau8810, div, 0);
> > > +
> > > +	return ret;
> > > +}

> > You shouldn't be implementing new set_clkdiv() operations, there's no
> > point in having each machine driver figure out the internal clocking of
> > the device.  Just specify the clocks coming into the device and have
> > the driver figure out what to do with them.

> We want to calculate the proper divide for MCLK as clock source.
> The design needs sampling rate information for the calculation.
> In the application sequence, there is no rate information in this
> stage and it should defer until codec hardware parameter.

That should be fine, you can do this in your hw_params() can't you?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20160701/3c4fb8c3/attachment.sig>