linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Limonciello, Mario" <Mario.Limonciello@dell.com>
To: Matthew Garrett <mjg59@srcf.ucam.org>,
	"Yuan, Perry" <Perry.Yuan@dell.com>
Cc: "hdegoede@redhat.com" <hdegoede@redhat.com>,
	"mgross@linux.intel.com" <mgross@linux.intel.com>,
	"pali@kernel.org" <pali@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"platform-driver-x86@vger.kernel.org" 
	<platform-driver-x86@vger.kernel.org>
Subject: RE: [PATCH]  platform/x86: dell-privacy: Add support for new privacy driver
Date: Wed, 11 Nov 2020 14:30:21 +0000	[thread overview]
Message-ID: <DM6PR19MB2636956DB58B0E4ECAD43549FAE80@DM6PR19MB2636.namprd19.prod.outlook.com> (raw)
In-Reply-To: <20201111072456.tkwdzuq2wa7zvbod@srcf.ucam.org>

> 
> On Wed, Nov 11, 2020 at 07:21:07AM +0000, Yuan, Perry wrote:
> > > > +    status = acpi_evaluate_object(NULL, ACPI_PRIVACY_EC_ACK, NULL,
> > > NULL);
> > > > +    if (ACPI_FAILURE(status)) {
> > > > +        dev_err(led_cdev->dev, "Error setting privacy audio EC ack
> > > value: %d\n",status);
> > > > +        return -EIO;
> > > > +    }
> > > > +    return 0;
> > > > +}
> > >
> > > What's actually being set here? You don't seem to be passing any
> arguments.
> >
> > Yes,  it is a EC ack notification without any arguments needed.
> 
> I'm confused why it's being exposed as an LED device in that case -
> there's an expectation that this is something that actually controls a
> real LED, which means responding to state. Are you able to share the
> acpidump of a machine with this device?
> 
> --

Matthew,

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 leading to a
poor user experience.

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.  Later follow up work is already envisioned that if HW
mute is already enacted but SW mute is modified (IE LED notifier goes
through) that a message can come back out to userspace to tell the user
something along the lines of

"Your laptop mic is muted, press fn+f4 to unmute". 

I don't believe that will be part of the first commits to land, but that's
why an LED is used, to know the state of SW.

Perry,

Some suggestions for v2:
* You should better explain this hardware design in the commit
message.
* I think the codec changes should be in same patch series as 1/2 and this
be 2/2 rather than them going separately.  It won't make sense for one of them
to go in without the other.  For example if codec change goes in and dell-laptop
driver tries to change legacy LED it won't do anything.  And if this goes in
but codec driver doesn't, nothing will ever send EC ack.

  reply	other threads:[~2020-11-11 14:32 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-03 12:55 Perry Yuan
2020-11-03 16:22 ` kernel test robot
2020-11-03 16:50 ` kernel test robot
2020-11-03 19:14 ` Barnabás Pőcze
2020-11-04  1:49 ` Matthew Garrett
2020-11-11  7:21   ` Yuan, Perry
2020-11-11  7:24     ` Matthew Garrett
2020-11-11 14:30       ` Limonciello, Mario [this message]
2020-11-12 15:06         ` Enrico Weigelt, metux IT consult
2020-11-12 15:31           ` Limonciello, Mario
2020-11-12 15:55             ` Hans de Goede
2020-11-12 15:57               ` Limonciello, Mario
2020-11-04  6:43 ` kernel test robot
2020-11-09 11:16 ` Hans de Goede
2020-11-11  7:24   ` Yuan, Perry

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM6PR19MB2636956DB58B0E4ECAD43549FAE80@DM6PR19MB2636.namprd19.prod.outlook.com \
    --to=mario.limonciello@dell.com \
    --cc=Perry.Yuan@dell.com \
    --cc=hdegoede@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgross@linux.intel.com \
    --cc=mjg59@srcf.ucam.org \
    --cc=pali@kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --subject='RE: [PATCH]  platform/x86: dell-privacy: Add support for new privacy driver' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).