linux-kernel - Re: [PATCH v8 2/3] x86: add support for Huawei WMI hotkeys.

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 03 Dec 2018 16:51:43 +0100
From:   Takashi Iwai <tiwai@...e.de>
To:     ayman.bagabas@...il.com
Cc:     alsa-devel@...a-project.org, Hui Wang <hui.wang@...onical.com>,
        Andy Shevchenko <andy@...radead.org>,
        Darren Hart <dvhart@...radead.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Kailang Yang <kailang@...ltek.com>,
        linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v8 2/3] x86: add support for Huawei WMI hotkeys.

On Mon, 03 Dec 2018 16:46:01 +0100,
ayman.bagabas@...il.com wrote:
> 
> > > +	priv->cdev.name = "platform::micmute";
> > > +	priv->cdev.max_brightness = 1;
> > > +	priv->cdev.brightness_set_blocking =
> > > huawei_wmi_micmute_led_set;
> > > +	priv->cdev.default_trigger = "audio-micmute";
> > > +	priv->cdev.brightness = ledtrig_audio_get(LED_AUDIO_MICMUTE);
> > > +	priv->cdev.dev = &wdev->dev;
> > 
> > What about suspend/resume?
> > When the driver is bound wit HD-audio, the HD-audio will restore the
> > state at resume, so it would work.  But, by providing the LED class
> > device, it is supposed to work even without HD-audio, so it might
> > make
> > sense to pass LED_CORE_SUSPENDRESUME, too.
> 
> Besides that, is there anything needed for wmi_device suspend/resume?

AFAIK, the wmi_driver itself doesn't need anything.  The input also
doesn't need, so most likely only LED.

> > > +static int __init huawei_wmi_init(void)
> > > +{
> > > +	if (!(wmi_has_guid(WMI0_EVENT_GUID) ||
> > > wmi_has_guid(AMW0_EVENT_GUID))) {
> > > +		pr_debug("Compatible WMI GUID not found\n");
> > > +		return -ENODEV;
> > > +	}
> > 
> > This is superfluous when you implement with wmi_driver.
> > In theory, the supported GUID can be added dynamically via sysfs,
> > too.
> > 
> 
> I left it that way so it doesn't insert the module if these GUIDs were
> not found.

But they aren't loaded on such devices unless you do explicitly.
If they are done explicitly, there must be a reason to do so, hence
you don't need to block it :)

> Should I drop that and use module_wmi_driver instead?

Yes, that's cleaner.  Let's try to make it as simple as possible at
first.


thanks,

Takashi