[1/2] usb: core: add vendor hook for usb suspend and resume

Add the hooks that vendor can design and bypass the suspend/resume.
When the handled is set, skip the original suspend/resume process.

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 original process, the ACPU will suspend/resume until the
USB suspend/resume. We add the hooks, so we can control USB suspend/resume
without affecting the ACPU.

Signed-off-by: Puma Hsu <pumahsu@google.com>
---
 drivers/usb/core/Makefile |  2 +-
 drivers/usb/core/driver.c | 36 ++++++++++++++++++++++++++++++++++++
 drivers/usb/core/usb.h    |  5 +++++
 3 files changed, 42 insertions(+), 1 deletion(-)

On Wed, Dec 14, 2022 at 3:07 PM Puma Hsu <pumahsu@google.com> wrote:
>
> Add the hooks that vendor can design and bypass the suspend/resume.
> When the handled is set, skip the original suspend/resume process.
>
> 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 original process, the ACPU will suspend/resume until the
> USB suspend/resume. We add the hooks, so we can control USB suspend/resume
> without affecting the ACPU.
>
> Signed-off-by: Puma Hsu <pumahsu@google.com>
> ---
>  drivers/usb/core/Makefile |  2 +-
>  drivers/usb/core/driver.c | 36 ++++++++++++++++++++++++++++++++++++
>  drivers/usb/core/usb.h    |  5 +++++
>  3 files changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
> index 7d338e9c0657..f48f646cd874 100644
> --- a/drivers/usb/core/Makefile
> +++ b/drivers/usb/core/Makefile
> @@ -3,7 +3,7 @@
>  # Makefile for USB Core files and filesystem
>  #
>
> -usbcore-y := usb.o hub.o hcd.o urb.o message.o driver.o
> +usbcore-y := usb.o hub.o hcd.o urb.o message.o driver.o usb-hooks-impl-goog.o
>  usbcore-y += config.o file.o buffer.o sysfs.o endpoint.o
>  usbcore-y += devio.o notify.o generic.o quirks.o devices.o
>  usbcore-y += phy.o port.o

Sorry I should not add usb-hooks-impl-goog to the makefile.
I will upload a version2

> diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> index 7e7e119c253f..3d2cfb6c2277 100644
> --- a/drivers/usb/core/driver.c
> +++ b/drivers/usb/core/driver.c
> @@ -35,6 +35,25 @@
>  #include "usb.h"
>
>
> +static struct usb_device_vendor_ops *usb_dev_vendor_ops;
> +
> +int usb_dev_register_vendor_ops(struct usb_device_vendor_ops *vendor_ops)
> +{
> +       if (vendor_ops == NULL)
> +               return -EINVAL;
> +
> +       usb_dev_vendor_ops = vendor_ops;
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_dev_register_vendor_ops);
> +
> +struct usb_device_vendor_ops *usb_vendor_get_ops(void)
> +{
> +       return usb_dev_vendor_ops;
> +}
> +EXPORT_SYMBOL_GPL(usb_vendor_get_ops);
> +
> +
>  /*
>   * Adds a new dynamic USBdevice ID to this driver,
>   * and cause the driver to probe for all devices again.
> @@ -1400,11 +1419,19 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
>         int                     status = 0;
>         int                     i = 0, n = 0;
>         struct usb_interface    *intf;
> +       bool                    handled;
> +       struct usb_device_vendor_ops *ops = usb_vendor_get_ops();
>
>         if (udev->state == USB_STATE_NOTATTACHED ||
>                         udev->state == USB_STATE_SUSPENDED)
>                 goto done;
>
> +       if (ops && ops->usb_dev_suspend) {
> +               handled = ops->usb_dev_suspend(udev, msg);
> +               if (handled)
> +                       goto done;
> +       }
> +
>         /* Suspend all the interfaces and then udev itself */
>         if (udev->actconfig) {
>                 n = udev->actconfig->desc.bNumInterfaces;
> @@ -1501,11 +1528,20 @@ static int usb_resume_both(struct usb_device *udev, pm_message_t msg)
>         int                     status = 0;
>         int                     i;
>         struct usb_interface    *intf;
> +       bool                    handled;
> +       struct usb_device_vendor_ops *ops = usb_vendor_get_ops();
>
>         if (udev->state == USB_STATE_NOTATTACHED) {
>                 status = -ENODEV;
>                 goto done;
>         }
> +
> +       if (ops && ops->usb_dev_resume) {
> +               handled = ops->usb_dev_resume(udev, msg);
> +               if (handled)
> +                       goto done;
> +       }
> +
>         udev->can_submit = 1;
>
>         /* Resume the device */
> diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
> index 82538daac8b8..9ccb8683071d 100644
> --- a/drivers/usb/core/usb.h
> +++ b/drivers/usb/core/usb.h
> @@ -220,3 +220,8 @@ extern acpi_handle usb_get_hub_port_acpi_handle(struct usb_device *hdev,
>  static inline int usb_acpi_register(void) { return 0; };
>  static inline void usb_acpi_unregister(void) { };
>  #endif
> +
> +struct usb_device_vendor_ops {
> +       bool (*usb_dev_suspend)(struct usb_device *udev, pm_message_t msg);
> +       bool (*usb_dev_resume)(struct usb_device *udev, pm_message_t msg);
> +};
> --
> 2.39.0.rc1.256.g54fd8350bd-goog
>

On Wed, Dec 14, 2022 at 03:06:49PM +0800, Puma Hsu wrote:
> Add the hooks that vendor can design and bypass the suspend/resume.

As Christoph said, there is no such thing in the kernel as a "vendor
hook".  That is a Google-only thing that was created for their Android
kernel for various reasons.

For obvious reaons, we can not take the same type of thing here, as that
makes no sense at all (and neither do you want us to.)

As explained each time this patch set has been sent in over the years,
the full implementation of a new callback must be submitted with the
patch series, otherwise it HAS to be rejected.  And again, you want this
rule in place, otherwise you would not even be able to use Linux for
your systems.

thanks,

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-2-pumahsu%40google.com
patch subject: [PATCH 1/2] usb: core: add vendor hook for usb suspend and resume
config: i386-randconfig-a002
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/c37166c62c494c4e2b7ebbb0a920a47a41a4500a
        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 c37166c62c494c4e2b7ebbb0a920a47a41a4500a
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 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>

All warnings (new ones prefixed by >>):

>> drivers/usb/core/driver.c:40:5: warning: no previous prototype for function 'usb_dev_register_vendor_ops' [-Wmissing-prototypes]
   int usb_dev_register_vendor_ops(struct usb_device_vendor_ops *vendor_ops)
       ^
   drivers/usb/core/driver.c:40:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int usb_dev_register_vendor_ops(struct usb_device_vendor_ops *vendor_ops)
   ^
   static 
>> drivers/usb/core/driver.c:50:31: warning: no previous prototype for function 'usb_vendor_get_ops' [-Wmissing-prototypes]
   struct usb_device_vendor_ops *usb_vendor_get_ops(void)
                                 ^
   drivers/usb/core/driver.c:50:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   struct usb_device_vendor_ops *usb_vendor_get_ops(void)
   ^
   static 
   2 warnings generated.


vim +/usb_dev_register_vendor_ops +40 drivers/usb/core/driver.c

    39	
  > 40	int usb_dev_register_vendor_ops(struct usb_device_vendor_ops *vendor_ops)
    41	{
    42		if (vendor_ops == NULL)
    43			return -EINVAL;
    44	
    45		usb_dev_vendor_ops = vendor_ops;
    46		return 0;
    47	}
    48	EXPORT_SYMBOL_GPL(usb_dev_register_vendor_ops);
    49	
  > 50	struct usb_device_vendor_ops *usb_vendor_get_ops(void)
    51	{
    52		return usb_dev_vendor_ops;
    53	}
    54	EXPORT_SYMBOL_GPL(usb_vendor_get_ops);
    55	
    56