[alsa-devel] [PATCH] ASoC: nau8825: Add driver for headset chip Nuvoton 8825

[Anatol Pomozov]

Hi

>> 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()?

>> After discussing it with my teammates who works on Intel+NAU8825
>> platform I found that they use MCLK only at playback/capture time. The
>> rest of the time VCO is suppose to be sysclk source. Thus using
>> get_clk() is not flexible for all cases.
>
> 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.

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.

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.


>> 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

http://www.ti.com/product/ts3a227e


>> > This function doesn't appear to affect the hardware.  I would expect
>> > that the jack detection hardware is turned off when not in use.
>
>> Disabling jack detection means disable HeadsetCompletion IRQ. But the
>> IRQ handler does a bunch of manual stuff like grounding/ungrounding
>> jack pins. If IRQ handler is not called then neither playback nor
>> capture will not work.
>
> The assumption would be that if the system doesn't have or need jack
> detection then the jack pins won't be reconfigurable anyway.
>
>> 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.

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.