Message-ID: <2024071518-progeny-unsecured-2926@gregkh>
Date: Mon, 15 Jul 2024 17:43:18 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: ryan zhou <ryanzhou54@...il.com>
Cc: Alan Stern <stern@...land.harvard.edu>, jikos@...nel.org,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hid: usbhid: Enable remote wake-up based on device
 configuration

On Mon, Jul 15, 2024 at 11:36:57PM +0800, ryan zhou wrote:
> On Thu, Jul 11, 2024 at 3:41 PM Greg KH <gregkh@...uxfoundation.org> wrote:
> >
> > On Wed, Jul 10, 2024 at 09:47:39PM -0400, Alan Stern wrote:
> > > On Thu, Jul 11, 2024 at 07:16:06AM +0800, ryan wrote:
> > > > According to the USB protocol, the host should automatically
> > > > adapt the remote wake-up function based on the configuration
> > > > descriptor reported by the device, rather than only the default
> > > > keyboard support. Therefore, it's necessary to support other hid
> > > > devices, such as digital headsets,mice,etc.
> > >
> > > It's true that the host shouldn't try to enable remote wakeup if the
> > > configuration descriptor shows that the device doesn't support it.
> > >
> > > However, it's not true that the host should try to enable remote wakeup
> > > for devices other than keyboards with boot-protocol support.  History
> > > has shown that quite a few HID devices don't handle remote wakeup
> > > properly; the decision about whether to enable it should be left to the
> > > user.
> >
> > I agree, this patch isn't acceptable.  Ryan, why do you want this
> > applied?  What userspace control is missing to allow you to do this
> > today on your systems with no kernel changes for devices that you know
> > will work properly?
> >
> > thanks,
> >
> > greg k-h
> 
> 
> Many thanks to Greg KH and Alan Stern for reviewing the patch and
> replying to me.
> I'd like to start by asking Greg KH's question.
> 
> A1:This patch is expected to be applied to the USB digital headset,
> mouse, and keyboard,
> and we expect to wake up the system by operating them when the system
> has suspended.
> 
> A2:I've verified that user-space control does the trick, but
> Personally speaking, it's not a good solution.
> For each device plugged into the host, the user space needs to check whether
> it is one of the three and to enable wakeup.It may be better to enable
> wakeup when loading
> a HID class drivers, from my perspective. Could you please give me
> some advice if possible.

You can run anything you want at device-plugin-time in userspace by
writing a udev rule, that's exactly what that was designed for.  The
policy you decide is under your control in userspace, no need to do
anything special in the kernel at all.

> I have spent some time studying your responses, and learned a lot. I
> absolutely agree with many
> of your points, but still have some doubts.
> 
> Q1 for Alan Stern: Boot device includes a boot mouse and boot keyboard,
> why the patch(3d61510f4ecac) only enables boot keyboard by default,
> and in addation boot
> protocol is used in BIOS,why is it used as a wakeup judgment condition
> in the OS?
> 
> Q2: for Alan Stern:  As you comment 'History has shown that quite a
> few HID devices don't
> handle remote wakeup properly'  I consulted the USB20 Spec in Chapter
> 9.2.5.2 and it has
> this description:'If a device supports remote wakeup, it must also
> allow the capability to be
> enabled and disabled using the standard USB request'  So these devices
> that you're talking about
> are not compliant with the USB20 protocol specification to my mind. If
> so, shouldn't we
> support these non-standard devices.

If you do not support "non-standard" devices, your operating system will
not be used by anyone in the real-world as there are TONS of
"non-standard" devices out there, sorry.

try it and see, go to the local store and buy a shopping cart of cheap
mice and keyboards and see what happens...

thanks,

greg k-h

Powered by blists - more mailing lists