Re: [RFC 2/2] ASoC: rt5670: Add LED trigger support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dne 23. 02. 21 v 17:20 Takashi Iwai napsal(a):

> So do you mean that the LED feature should be selectively enabled like
> the current HD-audio?

Yes, it should be enabled only when the machine has the audio LEDs.

>>> Of course, this implementation would make the integration much easier,
>>> and that's a big benefit.  So I have a mixed feeling and not decided
>>> yet whether we should go for it right now...
>>
>> I think that we can reconsider the LED handling implementation later, when
>> someone brings something better on the table.
> 
> What worried me is the plan to expose this capability to user-space.
> If it's only a kernel-internal, we can fix it in the kernel and
> nothing else broken, but if it's a part of API, that's not easy.
> 
> So, if any, I'd like to avoid exposing to the user-space at first.
> (But then it comes to the question how to deal with a case like AMD
> ACP...)

I tried to propose a complete solution and the ACP was one strong reason for
this kernel / user space API. So without the user space support, it's just
a half solution for known issues.

Frankly, I don't see any drawback or a problem even if we remove this API
later. The LED group bits are just informal for the user space and it's
expected to create the user controls tied to this LED functionality only in
alsa-lib/plugins at the moment. The kernel may return an error when the user
space tries to set those new bits when the API is deprecated and I believe
that the hardware design faults like AMD ACP (without the hardware mute) are rare.

Initial alsa-lib support: https://github.com/alsa-project/alsa-lib/pull/121

						Jaroslav

-- 
Jaroslav Kysela <perex@xxxxxxxx>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux