Message-ID: <f2142d88-259f-302d-da61-e0fc39d1f041@suse.com>
Date:   Tue, 28 Feb 2023 10:03:22 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH] HID: usbhid: enable remote wakeup for mice
Content-Language: en-US
To:     "Limonciello, Mario" <Mario.Limonciello@amd.com>,
        Oliver Neukum <oneukum@suse.com>,
        Greg KH <gregkh@linuxfoundation.org>,
        Michael Wu <michael@allwinnertech.com>
Cc:     "jikos@kernel.org" <jikos@kernel.org>,
        "benjamin.tissoires@redhat.com" <benjamin.tissoires@redhat.com>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Gong, Richard" <Richard.Gong@amd.com>
References: <20230222013944.31095-1-michael@allwinnertech.com>
 <Y/WwXBF37hoZBbQa@kroah.com>
 <9bf4463c-6541-a6cb-9bbc-6d070118509a@allwinnertech.com>
 <Y/dMq2KKYfdMdrjh@kroah.com> <801338c3-7c0d-6b3e-eac8-872ee5add466@suse.com>
 <MN0PR12MB6101B1BF9748F0602684FED5E2AB9@MN0PR12MB6101.namprd12.prod.outlook.com>
From:   Oliver Neukum <oneukum@suse.com>
In-Reply-To: <MN0PR12MB6101B1BF9748F0602684FED5E2AB9@MN0PR12MB6101.namprd12.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Qk84cGFFazZCdlBBQWliRm95M3FuVWxDcEswcURFQVdMRlhUN2orTDVPeC9O?=
 =?utf-8?B?RkNuZktueTBlcURjWkt6cWJTVDZid09NYkNxNmJsNFdjaXlZazBGcnpTM2c4?=
 =?utf-8?B?c2lLRHQ3WGprUm5UNWZqbXhrWTJTR24zYXdwUWd1QWdCb0lGZ2k2N2ZzVkRE?=
 =?utf-8?B?WUJabU03WEJJQnllOTZLZDZsWlV6LzV3bWs2SmU4WkZPa1lhNVo5bndObXlU?=
 =?utf-8?B?QUUyblRWYXNlSit6cHl3WkpTbDBZcVF4bHprL3FkOUpXYmpUSkw1amx2ak1L?=
 =?utf-8?B?UVNoZTlxQUZlUUlxeXFmZDZsK1Bmc3ZNYno0RllqWWpkMGhMN0JNU3pMcG9j?=
 =?utf-8?B?UjNYTFpZNTA3aUFnL29XQXYxWkc5bnl6N0hOUkxWWjFUSjJCMFM3T1FRaTVn?=
 =?utf-8?B?Y3pwbFEyTnE0dnBraU5JMWxlRlIxYnk3UlM1MHE4ZGhJaW5FTEtlU0ZvUllM?=
 =?utf-8?B?ZkFuNlBqcC9ieTNtSGtZbWlEZnJGYU9xLzNVYmlSU2dDZ0poSWJNblk1cC91?=
 =?utf-8?B?eGRpNFdFeDNtclEyd0xrektoZkx3Sy8xUGQwNzJYMEFkdlBqZldXa3JzK2tl?=
 =?utf-8?B?U2piY0c5NzNnK1p4S0RDRzJxVmkvbEpDV1pjcU5QZ3RjRzZNdzN5cGlMQy9a?=
 =?utf-8?B?UmVUSTd1YW9XT2NEYWZuVkMya0FsNTVwcW4yMU9hYjRPaXNQcTNsMUJPQlh6?=
 =?utf-8?B?WVlaWXhuR0RsMUFiKzBNazJ3aDNIZjBFSWVYUWI1TWhLTDU0NnZyNVR2RFNj?=
 =?utf-8?B?eHRrVjZYNFIwQTVBQnZsRWZNZzc2YWlib2puc1ZFcDd2bXNDVmJCR212dU1U?=
 =?utf-8?B?MXF5Mjg0cnNMclh4UzVmOE1PTDFuR2M3MUtxZ3F3VG9kSWZLbDlEQ3grUWRx?=
 =?utf-8?B?S1VDT2FlRVc5RFlRaGVnRUVxdW5jT0VRNlRnbUhqY3NaZURaUkVBV0RWUlk2?=
 =?utf-8?B?WTFhWVl2SGpWczZmSTRkRFlNNGliNFVhUzJ6V0diVzZ3cEhnbTN4dWdNUnRj?=
 =?utf-8?B?M045K2hRWGNLVXA4UWVvK1YrZlUrTlhZdDdrbC9reXFkR0Fvb1IyQ25kb3dB?=
 =?utf-8?B?L1Zod0N0SUp6UWVMMysvQ2RzSGsvVXp5anZMemVNMktHMDdEMkRBY2JWQmpa?=
 =?utf-8?B?blhlRW9mQ05EQ0QxeG1rTkxubFNBSko2ZVkzSTdSenRpczZrVXBwS0RYQ0hn?=
 =?utf-8?B?MkZqbFl4MnZkOGVpVkRXVC9KWjVnOXFvN1hMQjVQS3pmUit3eHVsR0liWTRC?=
 =?utf-8?B?ZFIrM0xKcmxMSk9DVUlmZTd6QUp6V1NmTlZURkdOQnVENUFzSVN2V2xjbkFE?=
 =?utf-8?B?SzVIcXhKM0Z3UGw5bC9RU29GdDFRcS9zUytDQk4xNnUxRWthR28vK2Yzc0hS?=
 =?utf-8?B?dWxtQThOTVNybzRxbDkyWlhQV2txbVBGMU9kUndocm0ydFdnVGZMU1YyUDJh?=
 =?utf-8?B?WWxoOWFWZkR0RndKM1Q1anRTUlJBeWd5bUY4ZVhsVXI4aUNxV1JBTEp5UzlL?=
 =?utf-8?B?b1FzYjF5ZDBteW1pdEo5UVlrd2xPbFlNcjBRVE5yellOQTRCMWsyVnE3M1Qz?=
 =?utf-8?B?QXZjZDYxK2lIM054VzJTb1ZNL3VtaGttOXQydWR5SWxPcmYrb1lQN1FlcGhL?=
 =?utf-8?B?cDI0emZoa1pVN1FSUGdha0VGMjRZMVU4TFVIMk5sak1BN3JaVnJmVzVmSE9D?=
 =?utf-8?B?c2psdHN4UWg3R2tiYjYwQ2Q4cVM0Y3VCN05CbVNlMHZDTGdMbTBHTUJnTWJ4?=
 =?utf-8?B?VEVxd1BEMENGU3dMOEpQamgxeDAvejBhYnhQYXpmNkVQcjVycVYzcnlhbTdT?=
 =?utf-8?B?Y0FmZlpYL0p2aU44dGhEMnVxaGhPdmdCaWttVHQzVU54RmtwYTdUcUFHRjJo?=
 =?utf-8?B?Q0xEcVFJTDBCTU1pQWF0UWtqSGREU1lIKzlBV1B4K0FlYmZER1U0YW1UTUNN?=
 =?utf-8?B?aTcyaitlbjhCeDc4UTdocElDV2RiWlJBYlVudURqMlUwM2xvN3NPMzg2azhW?=
 =?utf-8?B?cXB5NkZpMG9lMjdhL2hEdFI1cExIbHdJRzBGR0YxYzFkeTFqejhJSlFjblFL?=
 =?utf-8?B?ZjkwRkZlMHdrc0dtSDVzU2lFRFdXTTdwTGZtL3VZTjVHN0xZUDlrM0RTVUpi?=
 =?utf-8?B?RHczckFxU00ycXExMUlFbkhlRFgvbVZ5U3E3VDJzQUNRSUtDbVFROHlUR1Ix?=
 =?utf-8?Q?16yLKoL9wdRabBstAh+G7xV1iP0N9ww/MKAxuL5QymRE?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8c638ed5-f18b-47c1-6bd5-08db196aa612
X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB7104.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Feb 2023 09:03:25.0259
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 8okfKpvL/IDphzU3ZvLMwNCXMcbk4kNYfm/C9fPx/D6sYnjm1JUgWAtg63ZRIUHjbIufWl9bmu8BfsyPyuXamw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXPR04MB8512
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 23.02.23 20:41, Limonciello, Mario wrote:

Hi,

>> As a system wakeup source a mouse that generates events when
>> it is moved, however, would make the system unsuspendable, whenever
>> even
>> a bit of vibration is acting on the system.
>> And as S4 is used in many setups to prevent an uncontrolled shutdown
>> at low power, this must work.
> 
> At least in my version of the series, this is part of the reason that it was
> only intended to be used with s2idle.

Yes, that is sensible. If these patches are to be taken at all, that will
be a necessary condition to meet. But it is not sufficient.
  
> The kernel driver is well aware of what power state you're in the suspend
> callback (pm_suspend_target_state).
> 
> What about if we agreed to treat this one special by examining that?
> 
> If the sysfs is set to "enabled"

If user space needs to manipulate sysfs at all, we can have user space
tell kernel space exactly what to do. Hence I see no point in
conditional interpretations values in sysfs at that point.

We are discussing the kernel's default here.

> * During suspend if your target is s2idle -> program it
> * During suspend if your target is mem -> disable it
> * During suspend if your target is hibernate -> disable it

To my mind these defaults make sense.
However, do they make much more sense than what we are doing now?

> With that type of policy on how to handle the suspend call in place
> perhaps we could set it to enabled by default?

It pains me to say, but I am afraid in that regard the only
decision that will not cause ugly surprises is to follow Windows.
Yet, what is wrong about the current defaults?
  
> Turning on "autosuspend" for USB mice makes them behave pretty
> similarly to how they work when they're marked for remote wakeup.

Because it is exactly the same mechanism.
  
> On some mice the lasers turn off, and they only wakeup when you
> press a button or roll a wheel.

Yes. And _some_ is the exact problem. If we could tell, _how_ mice
react, this discussion were unnecessary.

	Regards
		Oliver