Re: [PATCH] drivers/platform/x86: Add driver for INT0002 ACPI device

[Takashi Iwai <tiwai@suse.de>]

On Fri, 21 Apr 2017 11:02:20 +0200,
Andy Shevchenko wrote:
> 
> On Fri, Apr 21, 2017 at 11:38 AM, Takashi Iwai <tiwai@suse.de> wrote:
> > On Fri, 21 Apr 2017 10:27:32 +0200,
> > Andy Shevchenko wrote:
> >> On Fri, Apr 21, 2017 at 11:17 AM, Takashi Iwai <tiwai@suse.de> wrote:
> >> > On Fri, 21 Apr 2017 09:41:57 +0200,
> >> > Hans de Goede wrote:
> 
> >> >> +     char ev_name[5];
> >> >
> >> > Are 5 bytes enough?  I see the code below:
> >> >
> >> >> +             snprintf(data->ev_name, sizeof(data->ev_name), "_%c%02X",
> >> >> +                     res->data.gpio.triggering ? 'E' : 'L',
> >> >> +                     res->data.gpio.pin_table[0]);
> >> >
> >> > So it counts 6 including NUL.
> >>
> >> How? 4 + NUL = 5.
> >
> > Well, "_E00X" is 5 letters, IIUC.
> 
> Well, today is Friday, yes.
> But %02X means two capitalized hex characters.

Bah, of course, Friday morning, and I have to take a coffee.

> >> >> +static int int0002_runtime_resume(struct device *dev)
> >> >> +{
> >> >> +     return 0;
> >> >> +}
> >> >> +
> >> >> +static const struct dev_pm_ops int0002_pm_ops = {
> >> >> +     .runtime_suspend = int0002_runtime_suspend,
> >> >> +     .runtime_resume = int0002_runtime_resume,
> >> >> +};
> >> >
> >> > Do we need these runtime PM?  If not, we can remove the header
> >> > inclusion, too.
> >>
> >> Yeah, and it needs attention when built with !CONFIG_PM.
> >
> > Practically seen, we may build this only with CONFIG_PM, too.
> > The virtual GPIO thing happens only when the machine gets resumed.
> 
> Perhaps depend on PM then?

Yes, it may make our lives a bit easier.
We stumbled on this driver just because of the hang at resume, and who
wants Cherrytrail device without PM?


thanks,

Takashi