On Fri, Nov 08, 2024 at 02:47:41AM -0800, Erni Sri Satya Vennela wrote: > On Thu, Oct 17, 2024 at 06:44:38AM -0700, Erni Sri Satya Vennela wrote: > > On Fri, Oct 04, 2024 at 01:14:10AM -0700, Dmitry Torokhov wrote: > > > On Tue, Sep 24, 2024 at 03:28:51AM +0000, Srivatsa S. Bhat wrote: > > > > [+linux-pm, Rafael, Len, Pavel] > > > > > > > > On Thu, Sep 12, 2024 at 02:27:49PM -0700, Erni Sri Satya Vennela wrote: > > > > > This reverts commit 62238f3aadc9bc56da70100e19ec61b9f8d72a5f. > > > > > > > > > > Remove keyboard as wakeup source since Suspend-to-Idle feature > > > > > is disabled. > > > > > > > > > > Signed-off-by: Erni Sri Satya Vennela <ernis@xxxxxxxxxxxxxxxxxxx> > > > > > --- > > > > > drivers/input/serio/hyperv-keyboard.c | 12 ------------ > > > > > 1 file changed, 12 deletions(-) > > > > > > > > > > diff --git a/drivers/input/serio/hyperv-keyboard.c b/drivers/input/serio/hyperv-keyboard.c > > > > > index 31d9dacd2fd1..b42c546636bf 100644 > > > > > --- a/drivers/input/serio/hyperv-keyboard.c > > > > > +++ b/drivers/input/serio/hyperv-keyboard.c > > > > > @@ -162,15 +162,6 @@ static void hv_kbd_on_receive(struct hv_device *hv_dev, > > > > > serio_interrupt(kbd_dev->hv_serio, scan_code, 0); > > > > > } > > > > > spin_unlock_irqrestore(&kbd_dev->lock, flags); > > > > > - > > > > > - /* > > > > > - * Only trigger a wakeup on key down, otherwise > > > > > - * "echo freeze > /sys/power/state" can't really enter the > > > > > - * state because the Enter-UP can trigger a wakeup at once. > > > > > - */ > > > > > - if (!(info & IS_BREAK)) > > > > > - pm_wakeup_hard_event(&hv_dev->device); > > > > > - > > > > > break; > > > > > > > > > > default: > > > > > @@ -356,9 +347,6 @@ static int hv_kbd_probe(struct hv_device *hv_dev, > > > > > goto err_close_vmbus; > > > > > > > > > > serio_register_port(kbd_dev->hv_serio); > > > > > - > > > > > - device_init_wakeup(&hv_dev->device, true); > > > > > > If you do not want the keyboard to be a wakeup source by default maybe > > > change this to: > > > > > > device_set_wakeup_capable(&hv_dev->device, true); > > > > > > and leave the rest of the driver alone? > > > > > > Same for the HID change. > > > > > > Thanks. > > > > > device_set_wakeup_capable() sets the @dev's power.can_wakeup flag and > > adds wakeup-related attributes in sysfs. > > > > Could you please help me understand why explicitly calling this function > > can be helpful in our use case? > > > > > -- > > > Dmitry > Just following up on this patch. Could you please help me understand the > reason for the change? Vennela, There is a difference between "wakeup source registration" and "wakeup capable". For this there are two flags defined in power management framework: 1. power.wakeup 2. power.can_wakeup More details on these flags can be read here: https://www.kernel.org/doc/html/v6.12/driver-api/pm/devices.html 'device_init_wakeup(dev, true)' sets both; ie it registers the device as a wakeup source and marks it as wakeup capable too. In our case, the device is "wakeup capable" but we do not want to "register it as a wakeup source". 'device_set_wakeup_capable(dev, true)' is more appropriate because this marks the device as wakeup capable but doesn't register it as a wakeup source knowingly. I understand that Dimitry suggests not to revert the entire patch but to replace 'device_init_wakeup' with 'device_set_wakeup_capable', to mark the device as capable of wakeup but knowingly skipping the registering part. Requesting Dimitry to correct me if there is any misinterpretation. While fixing this in next version, please fix the kernel bot warings as well reported for 1/3 patch of this series. - Saurabh
