On 7/1/2016 6:04 PM, Mark Brown wrote:
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.
Sometimes, we need to know the codec information and need read it from hardware, not cache. I'm afraid that it can't be done in this case.
- 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.
I know the reason and move it to DAPM.
- 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?
Yes, it can be done in hw_params().