[v2,2/2] usb: gadget: composite: Draw 100mA current if not configured

Message ID 1677142665-8686-3-git-send-email-quic_prashk@quicinc.com
State New
Headers
Series Fix vbus draw of dwc3 gadget |

Commit Message

Prashanth K Feb. 23, 2023, 8:57 a.m. UTC
  Currently we don't change the current value if device isn't in
configured state. But the battery charging specification says,
device can draw up to 100mA of current if its in unconfigured
state. Hence add a Vbus_draw work in composite_resume to draw
100mA if the device isn't configured.

Signed-off-by: Prashanth K <quic_prashk@quicinc.com>
---
 drivers/usb/gadget/composite.c | 3 +++
 1 file changed, 3 insertions(+)
  

Comments

kernel test robot Feb. 23, 2023, 11:57 a.m. UTC | #1
Hi Prashanth,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus linus/master v6.2]
[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/Prashanth-K/usb-dwc3-gadget-Change-condition-for-processing-suspend-event/20230223-165955
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/1677142665-8686-3-git-send-email-quic_prashk%40quicinc.com
patch subject: [PATCH v2 2/2] usb: gadget: composite: Draw 100mA current if not configured
config: hexagon-randconfig-r005-20230222 (https://download.01.org/0day-ci/archive/20230223/202302231910.xs35xNcG-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project db89896bbbd2251fff457699635acbbedeead27f)
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/19beaeb0554fc9c1556e8f7da85011f4267bd8fc
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Prashanth-K/usb-dwc3-gadget-Change-condition-for-processing-suspend-event/20230223-165955
        git checkout 19beaeb0554fc9c1556e8f7da85011f4267bd8fc
        # 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=hexagon olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/usb/gadget/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302231910.xs35xNcG-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from drivers/usb/gadget/composite.c:19:
   In file included from include/linux/usb/composite.h:27:
   In file included from include/linux/usb/gadget.h:24:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from drivers/usb/gadget/composite.c:19:
   In file included from include/linux/usb/composite.h:27:
   In file included from include/linux/usb/gadget.h:24:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from drivers/usb/gadget/composite.c:19:
   In file included from include/linux/usb/composite.h:27:
   In file included from include/linux/usb/gadget.h:24:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
>> drivers/usb/gadget/composite.c:2535:14: warning: comparison of distinct pointer types ('typeof (2) *' (aka 'int *') and 'typeof (100U) *' (aka 'unsigned int *')) [-Wcompare-distinct-pointer-types]
                   maxpower = min(CONFIG_USB_GADGET_VBUS_DRAW, 100U)
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:67:19: note: expanded from macro 'min'
   #define min(x, y)       __careful_cmp(x, y, <)
                           ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:36:24: note: expanded from macro '__careful_cmp'
           __builtin_choose_expr(__safe_cmp(x, y), \
                                 ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:26:4: note: expanded from macro '__safe_cmp'
                   (__typecheck(x, y) && __no_side_effects(x, y))
                    ^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:20:28: note: expanded from macro '__typecheck'
           (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                      ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
>> drivers/usb/gadget/composite.c:2535:52: error: expected ';' after expression
                   maxpower = min(CONFIG_USB_GADGET_VBUS_DRAW, 100U)
                                                                    ^
                                                                    ;
   7 warnings and 1 error generated.


vim +2535 drivers/usb/gadget/composite.c

  2504	
  2505	void composite_resume(struct usb_gadget *gadget)
  2506	{
  2507		struct usb_composite_dev	*cdev = get_gadget_data(gadget);
  2508		struct usb_function		*f;
  2509		unsigned			maxpower;
  2510	
  2511		/* REVISIT:  should we have config level
  2512		 * suspend/resume callbacks?
  2513		 */
  2514		DBG(cdev, "resume\n");
  2515		if (cdev->driver->resume)
  2516			cdev->driver->resume(cdev);
  2517		if (cdev->config) {
  2518			list_for_each_entry(f, &cdev->config->functions, list) {
  2519				if (f->resume)
  2520					f->resume(f);
  2521			}
  2522	
  2523			maxpower = cdev->config->MaxPower ?
  2524				cdev->config->MaxPower : CONFIG_USB_GADGET_VBUS_DRAW;
  2525			if (gadget->speed < USB_SPEED_SUPER)
  2526				maxpower = min(maxpower, 500U);
  2527			else
  2528				maxpower = min(maxpower, 900U);
  2529	
  2530			if (maxpower > USB_SELF_POWER_VBUS_MAX_DRAW)
  2531				usb_gadget_clear_selfpowered(gadget);
  2532	
  2533			usb_gadget_vbus_draw(gadget, maxpower);
  2534		} else {
> 2535			maxpower = min(CONFIG_USB_GADGET_VBUS_DRAW, 100U)
  2536			usb_gadget_vbus_draw(gadget, maxpower);
  2537		}
  2538	
  2539		cdev->suspended = 0;
  2540	}
  2541
  
kernel test robot Feb. 23, 2023, 12:09 p.m. UTC | #2
Hi Prashanth,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus linus/master v6.2 next-20230223]
[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/Prashanth-K/usb-dwc3-gadget-Change-condition-for-processing-suspend-event/20230223-165955
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/1677142665-8686-3-git-send-email-quic_prashk%40quicinc.com
patch subject: [PATCH v2 2/2] usb: gadget: composite: Draw 100mA current if not configured
config: x86_64-randconfig-a012 (https://download.01.org/0day-ci/archive/20230223/202302231957.9Vfsa8ln-lkp@intel.com/config)
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/19beaeb0554fc9c1556e8f7da85011f4267bd8fc
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Prashanth-K/usb-dwc3-gadget-Change-condition-for-processing-suspend-event/20230223-165955
        git checkout 19beaeb0554fc9c1556e8f7da85011f4267bd8fc
        # 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=x86_64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/usb/gadget/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302231957.9Vfsa8ln-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

>> drivers/usb/gadget/composite.c:2535:14: warning: comparison of distinct pointer types ('typeof (2) *' (aka 'int *') and 'typeof (100U) *' (aka 'unsigned int *')) [-Wcompare-distinct-pointer-types]
                   maxpower = min(CONFIG_USB_GADGET_VBUS_DRAW, 100U)
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:67:19: note: expanded from macro 'min'
   #define min(x, y)       __careful_cmp(x, y, <)
                           ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:36:24: note: expanded from macro '__careful_cmp'
           __builtin_choose_expr(__safe_cmp(x, y), \
                                 ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:26:4: note: expanded from macro '__safe_cmp'
                   (__typecheck(x, y) && __no_side_effects(x, y))
                    ^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:20:28: note: expanded from macro '__typecheck'
           (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                      ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
>> drivers/usb/gadget/composite.c:2535:52: error: expected ';' after expression
                   maxpower = min(CONFIG_USB_GADGET_VBUS_DRAW, 100U)
                                                                    ^
                                                                    ;
   1 warning and 1 error generated.


vim +2535 drivers/usb/gadget/composite.c

  2504	
  2505	void composite_resume(struct usb_gadget *gadget)
  2506	{
  2507		struct usb_composite_dev	*cdev = get_gadget_data(gadget);
  2508		struct usb_function		*f;
  2509		unsigned			maxpower;
  2510	
  2511		/* REVISIT:  should we have config level
  2512		 * suspend/resume callbacks?
  2513		 */
  2514		DBG(cdev, "resume\n");
  2515		if (cdev->driver->resume)
  2516			cdev->driver->resume(cdev);
  2517		if (cdev->config) {
  2518			list_for_each_entry(f, &cdev->config->functions, list) {
  2519				if (f->resume)
  2520					f->resume(f);
  2521			}
  2522	
  2523			maxpower = cdev->config->MaxPower ?
  2524				cdev->config->MaxPower : CONFIG_USB_GADGET_VBUS_DRAW;
  2525			if (gadget->speed < USB_SPEED_SUPER)
  2526				maxpower = min(maxpower, 500U);
  2527			else
  2528				maxpower = min(maxpower, 900U);
  2529	
  2530			if (maxpower > USB_SELF_POWER_VBUS_MAX_DRAW)
  2531				usb_gadget_clear_selfpowered(gadget);
  2532	
  2533			usb_gadget_vbus_draw(gadget, maxpower);
  2534		} else {
> 2535			maxpower = min(CONFIG_USB_GADGET_VBUS_DRAW, 100U)
  2536			usb_gadget_vbus_draw(gadget, maxpower);
  2537		}
  2538	
  2539		cdev->suspended = 0;
  2540	}
  2541
  

Patch

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 403563c..23b7347a8 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -2449,6 +2449,9 @@  void composite_resume(struct usb_gadget *gadget)
 			usb_gadget_clear_selfpowered(gadget);
 
 		usb_gadget_vbus_draw(gadget, maxpower);
+	} else {
+		maxpower = min(CONFIG_USB_GADGET_VBUS_DRAW, 100U)
+		usb_gadget_vbus_draw(gadget, maxpower);
 	}
 
 	cdev->suspended = 0;