Hi,
On 2/16/21 8:24 AM, Perry Yuan wrote:
> Hi Hans:
>
> On 2021/1/13 2:37, Hans de Goede wrote:
>> Hi,
>>
>> I know there already is a v3 out and I will try to get around to reviewing
>> that soon, still 1 remark about the discussion surrounding v2:
>>
>> On 1/11/21 2:42 PM, Perry Yuan wrote:
>>
>> <snip>
>>
>>>>> *The flow is like this:
>>>>> 1) User presses key. HW does stuff with this key (timeout is started)
>>>>> 2) Event is emitted from FW
>>>>> 3) Event received by dell-privacy
>>>>> 4) KEY_MICMUTE emitted from dell-privacy
>>>>> 5) Userland picks up key and modifies kcontrol for SW mute
>>>>> 6) Codec kernel driver catches and calls ledtrig_audio_set, like this:
>>>>> ledtrig_audio_set(LED_AUDIO_MICMUTE,
>>>>> rt715->micmute_led ? LED_ON :LED_OFF);
>>>>> 7) If "LED" is set to on dell-privacy notifies ec,
>>>>> and timeout is cancelled,HW mic mute activated.
>>>>>
>>>> Please proofread the commit message again, and pay attention to capitalization
>>>> and spacing.
>>> I want to reformat it and move the commit info to cover letter.
>>
>> Please also put a copy of this as a comment in either the wmi or the
>> acpi driver (with a comment pointing to the comment in the other) this is
>> important info to have for someone reading the code and trying to understand
>> how this all fits together.
>>
>> Regards,
>>
>> Hans
>>
> Hans.
> I have added the comments to the dell-privacy driver file in V4
>
> -----------------------------------------------------------------------------------
> drivers/platform/x86/dell-privacy-wmi.c
>
> EXPORT_SYMBOL_GPL(dell_privacy_valid);
> /*
> * The flow of privacy event:
> * 1) User presses key. HW does stuff with this key (timeout is started)
> * 2) WMI event is emitted from BIOS
> * 3) WMI event is received by dell-privacy
> * 4) KEY_MICMUTE emitted from dell-privacy
> * 5) Userland picks up key and modifies kcontrol for SW mute
> * 6) Codec kernel driver catches and calls ledtrig_audio_set defined by
> * dell-privacy-acpi driver.
> * codec driver will call like this to switch micmute led state.
> * ledtrig_audio_set(LED_AUDIO_MICMUTE, micmute_led ? LED_ON :LED_OFF);
> * 7) If "LED" is set to on dell-privacy notifies EC,and timeout is cancelled,
> * HW mic mute activated.
> */
> void dell_privacy_process_event(int type, int code, int status)
> {
> struct privacy_wmi_data *priv;
> const struct key_entry *key;
>
> mutex_lock(&list_mutex);
> ....
>
> -----------------------------------------------------------------------------------
> drivers/platform/x86/dell-privacy-acpi.c
>
> /*
> * Pressing the mute key activates a time delayed circuit to physically cut
> * off the mute. The LED is in the same circuit, so it reflects the true
> * state of the HW mute. The reason for the EC "ack" is so that software
> * can first invoke a SW mute before the HW circuit is cut off. Without SW
> * cutting this off first does not affect the time delayed muting or status
> * of the LED but there is a possibility of a "popping" noise.
> *
> * If the EC receives the SW ack, the circuit will be activated before the
> * delay completed.
> *
> * Exposing as an LED device allows the codec drivers notification path to
> * EC ACK to work
> */
> static int dell_privacy_leds_setup(struct device *dev)
> {
> struct privacy_acpi_priv *priv = dev_get_drvdata(dev);
> int ret = 0;
>
> .....
>
This looks good, thank you.
Regards,
Hans
