Re: [PATCH 2/3] Revert "Input: hyperv-keyboard - register as a wakeup source"

From: Erni Sri Satya Vennela
Date: Thu Dec 12 2024 - 09:34:16 EST


On Mon, Dec 09, 2024 at 09:16:23AM -0800, Saurabh Singh Sengar wrote:
> 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
Thanks for the clarification Saurabh.
I'll be incorporating these changes in the next verison of the patch.

- Vennela