On Wed, May 24, 2017 at 9:50 AM, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> Hi,
>
[cut]
>>> +static irqreturn_t int0002_irq(int irq, void *data)
>>> +{
>>> + struct gpio_chip *chip = data;
>>> + u32 gpe_sts_reg;
>>> +
>>> + gpe_sts_reg = inl(GPE0A_STS_PORT);
>>> + if (!(gpe_sts_reg & GPE0A_PME_B0_STS_BIT))
>>> + return IRQ_NONE;
>>> +
>>> + generic_handle_irq(irq_find_mapping(chip->irqdomain,
>>> + GPE0A_PME_B0_VIRT_GPIO_PIN));
>>> +
>>> + pm_wakeup_hard_event(chip->parent);
>>
>>
>> It may be better to just do pm_system_wakeup() here and drop the
>> device_init_wakeup() from _probe().
>
>
> Ok, I've given this a try and it works fine, so I will send a v4 with
> this changed.
OK
>> The reason why is that device_init_wakeup() allows user space to disable
>> this wakeup source via sysfs which probably never is a good idea, because
>> this thing is just for clearing PME status which should be done regardless
>> and the actual wakeup is triggered by something else.
>
>
> Ack.
>
>> Also in theory pm_system_wakeup() should not really be necessary for
>> wakeup via USB keyboard to work, because that should be signaled via
>> acpi_pm_notify_handler(), at least in principle.
>
>
> Earlier versions of the int0002 driver did not call any wakeup() function
> at all and with 4.11 that worked fine, my initial testing with 4.12-rc1 also
> did not include a wakeup() call but that resulted in USB-keyboard wakeup
> not working and the system not responding to other wakeup sources like
> the power-button after the attempted USB-keyb wakeup. To be sure I've
> retried my current int0002 code with the wakeup call commented out, that
> leads to the same result (unwakeable system) so it seems we really need
> to do the pm_system_wakeup() call.
That's fine, it doesn't hurt to put it in there ATM anyway.
Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
