| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <SJ0PR19MB4528872911CE42EC0C91AB1884619@SJ0PR19MB4528.namprd19.prod.outlook.com>
Date: Fri, 26 Mar 2021 07:20:11 +0000
From: "Yuan, Perry" <Perry.Yuan@...l.com>
To: Hans de Goede <hdegoede@...hat.com>,
Jaroslav Kysela <perex@...ex.cz>,
Mark Brown <broonie@...nel.org>,
"pierre-louis.bossart@...ux.intel.com"
<pierre-louis.bossart@...ux.intel.com>,
"Limonciello, Mario" <Mario.Limonciello@...l.com>
CC: "pobrn@...tonmail.com" <pobrn@...tonmail.com>,
"oder_chiou@...ltek.com" <oder_chiou@...ltek.com>,
"tiwai@...e.com" <tiwai@...e.com>,
"mgross@...ux.intel.com" <mgross@...ux.intel.com>,
"lgirdwood@...il.com" <lgirdwood@...il.com>,
"alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"platform-driver-x86@...r.kernel.org"
<platform-driver-x86@...r.kernel.org>
Subject: RE: [PATCH v4 2/2] ASoC: rt715:add micmute led state control supports
Hi Hans,Jaroslav
> -----Original Message-----
> From: Hans de Goede <hdegoede@...hat.com>
> Sent: 2021年3月25日 23:07
> To: Yuan, Perry; Jaroslav Kysela; Mark Brown; pierre-
> louis.bossart@...ux.intel.com; Limonciello, Mario
> Cc: pobrn@...tonmail.com; oder_chiou@...ltek.com; tiwai@...e.com;
> mgross@...ux.intel.com; lgirdwood@...il.com; alsa-devel@...a-project.org;
> linux-kernel@...r.kernel.org; platform-driver-x86@...r.kernel.org
> Subject: Re: [PATCH v4 2/2] ASoC: rt715:add micmute led state control
> supports
>
>
> [EXTERNAL EMAIL]
>
> Hi,
>
> On 3/25/21 3:11 PM, Yuan, Perry wrote:
> > Hi Hans
> >
> >> -----Original Message-----
> >> From: Hans de Goede <hdegoede@...hat.com>
> >> Sent: Monday, March 22, 2021 11:02 PM
> >> To: Jaroslav Kysela; Yuan, Perry; Mark Brown; pierre-
> >> louis.bossart@...ux.intel.com; Limonciello, Mario
> >> Cc: pobrn@...tonmail.com; oder_chiou@...ltek.com; tiwai@...e.com;
> >> mgross@...ux.intel.com; lgirdwood@...il.com;
> >> alsa-devel@...a-project.org; linux-kernel@...r.kernel.org;
> >> platform-driver-x86@...r.kernel.org
> >> Subject: Re: [PATCH v4 2/2] ASoC: rt715:add micmute led state control
> >> supports
> >>
> >>
> >> [EXTERNAL EMAIL]
> >>
> >> Hi,
> >>
> >> On 3/22/21 3:37 PM, Jaroslav Kysela wrote:
> >>> Dne 22. 03. 21 v 10:25 Yuan, Perry napsal(a):
> >>>> Hi Mark:
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Mark Brown <broonie@...nel.org>
> >>>>> Sent: Tuesday, March 9, 2021 1:24 AM
> >>>>> To: Yuan, Perry
> >>>>> Cc: pobrn@...tonmail.com; pierre-louis.bossart@...ux.intel.com;
> >>>>> oder_chiou@...ltek.com; perex@...ex.cz; tiwai@...e.com;
> >>>>> hdegoede@...hat.com; mgross@...ux.intel.com; Limonciello, Mario;
> >>>>> lgirdwood@...il.com; alsa-devel@...a-project.org; linux-
> >>>>> kernel@...r.kernel.org; platform-driver-x86@...r.kernel.org
> >>>>> Subject: Re: [PATCH v4 2/2] ASoC: rt715:add micmute led state
> >>>>> control supports
> >>>>>
> >>>>> On Mon, Mar 01, 2021 at 05:38:34PM +0800, Perry Yuan wrote:
> >>>>>
> >>>>>> + /* Micmute LED state changed by muted/unmute switch */
> >>>>>> + if (mc->invert) {
> >>>>>> + if (ucontrol->value.integer.value[0] || ucontrol-
> >>>>>> value.integer.value[1]) {
> >>>>>> + micmute_led = LED_OFF;
> >>>>>> + } else {
> >>>>>> + micmute_led = LED_ON;
> >>>>>> + }
> >>>>>> + ledtrig_audio_set(LED_AUDIO_MICMUTE,
> micmute_led);
> >>>>>> + }
> >>>>>
> >>>>> These conditionals on inversion seem weird and counterintuitive.
> >>>>> If we're going with this approach it would probably be clearer to
> >>>>> define a custom operation for the affected controls that wraps the
> >>>>> standard one and adds the LED setting rather than keying off
> >>>>> invert like
> >> this.
> >>>>
> >>>> Currently the sof soundwire driver has no generic led control yet.
> >>>> This patch can handle the led control needs for MIC mute LED,
> >>>> definitely
> >> the patch is a short term solution.
> >>>> There is a feature request discussion when we started to implement
> >>>> this
> >> solution.
> >>>> https://github.com/thesofproject/linux/issues/2496#issuecomment-
> >> 71389
> >>>> 2620
> >>>>
> >>>> The workable way for now is that we put the LED mute control to the
> >> codec driver.
> >>>> When there is new and full sound LED solution implemented, this
> >>>> part will
> >> be also optimized.
> >>>> The Hardware privacy feature needs this patch to handle the Mic
> >>>> mute led
> >> state change.
> >>>> Before that full solution ready in kernel, could we take this as
> >>>> short term
> >> solution?
> >>>
> >>> Perry, it's about the machine detection. Your code is too much
> >>> generic even for the top-level LED trigger implementation. We need
> >>> an extra check, if the proper LED's are really controlled on the
> >>> specific hardware. Other hardware may use RT715 for a different
> >>> purpose. Use DMI / ACPI checks to detect this hardware and don't
> >>> misuse the inversion
> >> flag to enable this code.
> >>
> >> I think this would be a goo candidate for the new generic LED handling:
> >>
> >> https://lore.kernel.org/alsa-devel/20210317172945.842280-1-
> >> perex@...ex.cz/
> >>
> >> And then use a udev-rule + hwdb and/or UCM profiles to configure the
> >> LED trigger for specific models from userspace ?
> >>
> >> Regards,
> >>
> >> Hans
> >>
> >>
> >>
> > Because the SOF SDW design has no mic mute led control yet.
> > So I add one short term solution to make Dell privacy working for now
> > Definitely , that is new solution I can add my patch on that to test as one
> user case .
> > We really need to take the short term solution work for some system
> > which support new SOF soundwire hardware which have dependence on
> the MIC mute feature Meanwhile we can continue working on the new "udev-
> rule + hwdb and/or UCM profiles" solution as to replace this one.
> > If we agree that, I will submit another V6 addressing new feedback.
>
> The triggering of the LED trigger and the code registering the led_classdev are
> in 2 separate subsystems; and should be merged separately.
>
> If you post a new version of patch 1/2 addressing my review remarks then I
> can merge that.
>
> For merging the sound side of this you need to talk to the sound folks
> (Jaroslav Kysela, Takashi Iwai, Mark Brown for files under sound/soc).
>
> Regards,
>
> Hans
Got it!
I am working the V6 today, will post it ASAP.
* addressing Hans Feedback
* add runtime machine detection with DMI checking (from Jaroslav Feedback)
Perry Yuan
Dell | Client Software Group | CDC Linux OS
Powered by blists - more mailing lists