On Mon, 03 Dec 2018 16:46:01 +0100,
ayman.bagabas@xxxxxxxxx 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
