On Wed, Aug 12, 2015 at 04:08:31PM -0700, Anatol Pomozov wrote:
Moved it to ALSA controls. Though I still feel a bit uncomfortable that the default control value is not usable. I have to set "ADC Decimation Rate" and "DAC Oversampling Rate" controls in userspace from its defaults to "128".
Setting the default isn't too bad in this case (especially with a comment explaining why) - that doesn't preclude also providing user configuration.
What is the best way to set default control value? Just a regmap_write()?
Yes.
I'm sorry, I don't understand. What makes you say that "using get_clk() is not flexible for all cases", I can't quite parse that bit?
My initial assumption was that you are talking about moving complete clock handling (clock initialization, MCLK frequency calculation based on rate, policy of switching clock sources i.e. everything that platform drivers do) to the the driver code.
That is the ideal thing.
I've looked at other codec drivers and did not find many examples of using clk api. The drivers seems expect clock signal is pre-configured already. e.g. tegra has a utility function that configures audio clocks tegra_asoc_utils_set_rate() and it actively used in tegra-based platform drivers.
Most of these drivers predate the clock API, while there's no pressing need to update things (and it's pretty disruptive to existing systems especially older ones) if you're running into problems with new code that are best fixed with the clock API then you should do that.
Locally I tried to move clock initialization out of platform code to codec itself. Codec calls devm_clk_get() in init and sets frequency in nau8825_set_sysclk(). See it here https://github.com/anatol/linux/tree/nau8825 It works fine on a Tegra-based device. But I found that devm_clk_get() is DTS specific and does not work with ACPI. We need NAU8825 driver for Intel platform as well.
No, that is not the case at all - what makes you claim that devm_clk_get() is DT specific?
I was following code from ts3a227e driver that does the same.
That device is specifically tied to Chromebooks which have a defined mapping, as opposed to being a generic device which is usable on a range of devices.
I do not see anything Chromebook specific about this chip
My understanding is that the accessories it supports are Chrombook/Android ones (note that there's no configuration of ranges for detection in the code).
At this point this function just sets jack structure needed later in IRQ handler.
I can see what it's doing, I'd just also expect it to also be starting and stopping the detection in the hardware.
If we move IRQ enabling from init() function to enable_jack_detect() then it means chip will be unusable after the init(). It won't be able to playback nor capture audio. All developers will be forced to call nau88225_enable_jack_detect() from their platform driver to enable basic chip functionality. I think it is better if basic functionality is enabled during driver init and does not require additional steps.
I very much doubt that this will be a problem for all boards. To repeat what I said in my previous reply:
| The assumption would be that if the system doesn't have or need jack | detection then the jack pins won't be reconfigurable anyway.
Your board may be wired up with jack detection configured in a way that needs this, I would be surprised if all boards were.
It is better if I rename nau8825_enable_jack_detect() to nau8825_set_jack() to make cleaner what the function suppose to do. It sets jack structure that driver uses to report events to upper level. If the jack pointer is not set then driver still handles jack insert/eject internally but does not report it to the userspace.
No, that doesn't help.