| Message ID | 20221214070650.703793-3-pumahsu@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add vendor hooks for usb suspend and resume | expand |
On Wed, Dec 14, 2022 at 03:06:50PM +0800, Puma Hsu wrote: > + /* > + * Note: Our private driver provides APIs to know the device is in audio playback > + * or capture. > + * > + * usb_playback = usb_audio_playback_enabled(); > + * usb_capture = usb_audio_capture_enabled(); > + */ There is no such thing as a "private driver" in Linux, sorry, you know this. Submit the real code please, otherwise this is just wasting everyone's time :( greg k-h
Hi Puma,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus linus/master v6.1 next-20221214]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Puma-Hsu/add-vendor-hooks-for-usb-suspend-and-resume/20221214-150942
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20221214070650.703793-3-pumahsu%40google.com
patch subject: [PATCH 2/2] usb: core: add implementations for usb suspend/resume hooks
config: x86_64-rhel-8.3-rust
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/de7c2e5c48eae351485a1bab4b8f92c399ea6c54
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Puma-Hsu/add-vendor-hooks-for-usb-suspend-and-resume/20221214-150942
git checkout de7c2e5c48eae351485a1bab4b8f92c399ea6c54
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/usb/core/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/usb/core/usb-hooks-impl-goog.c:69:5: warning: no previous prototype for 'usb_vendor_helper_init' [-Wmissing-prototypes]
69 | int usb_vendor_helper_init(void)
| ^~~~~~~~~~~~~~~~~~~~~~
vim +/usb_vendor_helper_init +69 drivers/usb/core/usb-hooks-impl-goog.c
68
> 69 int usb_vendor_helper_init(void)
Hi Puma,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus linus/master v6.1 next-20221219]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Puma-Hsu/add-vendor-hooks-for-usb-suspend-and-resume/20221214-150942
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20221214070650.703793-3-pumahsu%40google.com
patch subject: [PATCH 2/2] usb: core: add implementations for usb suspend/resume hooks
config: i386-randconfig-s003
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/de7c2e5c48eae351485a1bab4b8f92c399ea6c54
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Puma-Hsu/add-vendor-hooks-for-usb-suspend-and-resume/20221214-150942
git checkout de7c2e5c48eae351485a1bab4b8f92c399ea6c54
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 olddefconfig
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/usb/core/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/usb/core/usb-hooks-impl-goog.c:69:5: sparse: sparse: symbol 'usb_vendor_helper_init' was not declared. Should it be static?
diff --git a/drivers/usb/core/usb-hooks-impl-goog.c b/drivers/usb/core/usb-hooks-impl-goog.c new file mode 100644 index 000000000000..89dc360babed --- /dev/null +++ b/drivers/usb/core/usb-hooks-impl-goog.c @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2022 Google Corp. + * + * Author: + * Puma Hsu <pumahsu@google.com> + */ + +#include <linux/usb.h> +#include "usb.h" + +extern int usb_dev_register_vendor_ops(struct usb_device_vendor_ops *vendor_ops); + +static bool usb_device_vendor_suspend(struct usb_device *udev, pm_message_t msg) +{ + bool usb_playback = false; + bool usb_capture = false; + bool handled = false; + + if (!udev) + return handled; + + /* + * Note: Our private driver provides APIs to know the device is in audio playback + * or capture. + * + * usb_playback = usb_audio_playback_enabled(); + * usb_capture = usb_audio_capture_enabled(); + */ + + /* + * Note: When the USB audio is working, we will not let the usb device suspend. + * Return handled = true so that the System core can it's suspend process. + */ + if (usb_playback || usb_capture) { + dev_info(&udev->dev, "%s: skip suspend process (playback:%d,capture:%d)\n", + __func__, usb_playback, usb_capture); + handled = true; + } + + return handled; +} + +static bool usb_device_vendor_resume(struct usb_device *udev, pm_message_t msg) +{ + bool handled = false; + + if (!udev) + return handled; + + /* + * Note: If the udev didn't suspend actually, we don't need to do resume. + */ + if (udev->port_is_suspended || udev->state == USB_STATE_SUSPENDED) { + handled = false; + } else { + dev_info(&udev->dev, "%s: skip resume process\n", __func__); + handled = true; + } + + return handled; +} + +static struct usb_device_vendor_ops usb_dev_vendor_ops = { + .usb_dev_suspend = usb_device_vendor_suspend, + .usb_dev_resume = usb_device_vendor_resume, +}; + +int usb_vendor_helper_init(void) +{ + return usb_dev_register_vendor_ops(&usb_dev_vendor_ops); +}
In mobile, a co-processor can be used for USB audio. When the co-processor is working for USB audio, the co-processor is the user/owner of the USB driver, and the ACPU is able to sleep in such condition to improve power consumption. In order to support this, we implement the hooks to handle USB suspend/resume requests. This commit introduces two hook implementations: - usb_device_vendor_suspend() Determine whether we should skip suspend request according to the status of USB audio playback/capture. Return: - true: let driver.c know that we "handled" and it can just return succeeded to ACPU to continue system suspend process. - false: let driver.c know that it still run original suspend process. - usb_device_vendor_resume() Determine whether we should skip resume request according to the USB device's suspend state. Return: - true: let driver.c know that it doesn't need to run resume process. - false: let driver.c know that it still run original resume process. Signed-off-by: Puma Hsu <pumahsu@google.com> --- drivers/usb/core/usb-hooks-impl-goog.c | 72 ++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 drivers/usb/core/usb-hooks-impl-goog.c