[PATCH] USB: atm: speedtch: do not use assignment in if condition

Manuel Quintero F posted 1 patch 1 month, 3 weeks ago
drivers/usb/atm/speedtch.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[PATCH] USB: atm: speedtch: do not use assignment in if condition
Posted by Manuel Quintero F 1 month, 3 weeks ago
Fix checkpatch error "do not use assignment in if condition"

Signed-off-by: Manuel Quintero F <sakunix@yahoo.com>
Signed-off-by: Manuel Quintero F <sakunix@yahoo.com>
---
 drivers/usb/atm/speedtch.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/atm/speedtch.c b/drivers/usb/atm/speedtch.c
index 973548b5c15c..dfd362abf602 100644
--- a/drivers/usb/atm/speedtch.c
+++ b/drivers/usb/atm/speedtch.c
@@ -324,7 +324,9 @@ static int speedtch_upload_firmware(struct speedtch_instance_data *instance,
 	   because we're in our own kernel thread anyway. */
 	msleep_interruptible(1000);
 
-	if ((ret = usb_set_interface(usb_dev, INTERFACE_DATA, instance->params.altsetting)) < 0) {
+	ret = usb_set_interface(usb_dev, INTERFACE_DATA, instance->params.altsetting
+
+	if (ret < 0) {
 		usb_err(usbatm, "%s: setting interface to %d failed (%d)!\n", __func__, instance->params.altsetting, ret);
 		goto out_free;
 	}
-- 
2.45.2
Re: [PATCH] USB: atm: speedtch: do not use assignment in if condition
Posted by kernel test robot 1 month, 3 weeks ago
Hi Manuel,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus linus/master v6.12-rc1 next-20241004]
[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/Manuel-Quintero-F/USB-atm-speedtch-do-not-use-assignment-in-if-condition/20241004-113643
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20241004030512.2036-1-sakunix%40yahoo.com
patch subject: [PATCH] USB: atm: speedtch: do not use assignment in if condition
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20241005/202410051833.7M7dq3rD-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project fef3566a25ff0e34fb87339ba5e13eca17cec00f)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241005/202410051833.7M7dq3rD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410051833.7M7dq3rD-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/usb/atm/speedtch.c:26:
   In file included from drivers/usb/atm/usbatm.h:14:
   In file included from include/linux/atmdev.h:9:
   In file included from include/linux/net.h:24:
   In file included from include/linux/mm.h:2213:
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   In file included from drivers/usb/atm/speedtch.c:26:
   In file included from drivers/usb/atm/usbatm.h:14:
   In file included from include/linux/atmdev.h:11:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         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'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from drivers/usb/atm/speedtch.c:26:
   In file included from drivers/usb/atm/usbatm.h:14:
   In file included from include/linux/atmdev.h:11:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         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'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from drivers/usb/atm/speedtch.c:26:
   In file included from drivers/usb/atm/usbatm.h:14:
   In file included from include/linux/atmdev.h:11:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
>> drivers/usb/atm/speedtch.c:329:2: error: expected ')'
     329 |         if (ret < 0) {
         |         ^
   drivers/usb/atm/speedtch.c:327:25: note: to match this '('
     327 |         ret = usb_set_interface(usb_dev, INTERFACE_DATA, instance->params.altsetting
         |                                ^
   7 warnings and 1 error generated.

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for MODVERSIONS
   Depends on [n]: MODULES [=y] && !COMPILE_TEST [=y]
   Selected by [y]:
   - RANDSTRUCT_FULL [=y] && (CC_HAS_RANDSTRUCT [=y] || GCC_PLUGINS [=n]) && MODULES [=y]
   WARNING: unmet direct dependencies detected for GET_FREE_REGION
   Depends on [n]: SPARSEMEM [=n]
   Selected by [m]:
   - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]


vim +329 drivers/usb/atm/speedtch.c

   229	
   230	static int speedtch_upload_firmware(struct speedtch_instance_data *instance,
   231					     const struct firmware *fw1,
   232					     const struct firmware *fw2)
   233	{
   234		unsigned char *buffer;
   235		struct usbatm_data *usbatm = instance->usbatm;
   236		struct usb_device *usb_dev = usbatm->usb_dev;
   237		int actual_length;
   238		int ret = 0;
   239		int offset;
   240	
   241		usb_dbg(usbatm, "%s entered\n", __func__);
   242	
   243		buffer = (unsigned char *)__get_free_page(GFP_KERNEL);
   244		if (!buffer) {
   245			ret = -ENOMEM;
   246			usb_dbg(usbatm, "%s: no memory for buffer!\n", __func__);
   247			goto out;
   248		}
   249	
   250		if (!usb_ifnum_to_if(usb_dev, 2)) {
   251			ret = -ENODEV;
   252			usb_dbg(usbatm, "%s: interface not found!\n", __func__);
   253			goto out_free;
   254		}
   255	
   256		/* URB 7 */
   257		if (dl_512_first) {	/* some modems need a read before writing the firmware */
   258			ret = usb_bulk_msg(usb_dev, usb_rcvbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
   259					   buffer, 0x200, &actual_length, 2000);
   260	
   261			if (ret < 0 && ret != -ETIMEDOUT)
   262				usb_warn(usbatm, "%s: read BLOCK0 from modem failed (%d)!\n", __func__, ret);
   263			else
   264				usb_dbg(usbatm, "%s: BLOCK0 downloaded (%d bytes)\n", __func__, ret);
   265		}
   266	
   267		/* URB 8 : both leds are static green */
   268		for (offset = 0; offset < fw1->size; offset += PAGE_SIZE) {
   269			int thislen = min_t(int, PAGE_SIZE, fw1->size - offset);
   270			memcpy(buffer, fw1->data + offset, thislen);
   271	
   272			ret = usb_bulk_msg(usb_dev, usb_sndbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
   273					   buffer, thislen, &actual_length, DATA_TIMEOUT);
   274	
   275			if (ret < 0) {
   276				usb_err(usbatm, "%s: write BLOCK1 to modem failed (%d)!\n", __func__, ret);
   277				goto out_free;
   278			}
   279			usb_dbg(usbatm, "%s: BLOCK1 uploaded (%zu bytes)\n", __func__, fw1->size);
   280		}
   281	
   282		/* USB led blinking green, ADSL led off */
   283	
   284		/* URB 11 */
   285		ret = usb_bulk_msg(usb_dev, usb_rcvbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
   286				   buffer, 0x200, &actual_length, DATA_TIMEOUT);
   287	
   288		if (ret < 0) {
   289			usb_err(usbatm, "%s: read BLOCK2 from modem failed (%d)!\n", __func__, ret);
   290			goto out_free;
   291		}
   292		usb_dbg(usbatm, "%s: BLOCK2 downloaded (%d bytes)\n", __func__, actual_length);
   293	
   294		/* URBs 12 to 139 - USB led blinking green, ADSL led off */
   295		for (offset = 0; offset < fw2->size; offset += PAGE_SIZE) {
   296			int thislen = min_t(int, PAGE_SIZE, fw2->size - offset);
   297			memcpy(buffer, fw2->data + offset, thislen);
   298	
   299			ret = usb_bulk_msg(usb_dev, usb_sndbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
   300					   buffer, thislen, &actual_length, DATA_TIMEOUT);
   301	
   302			if (ret < 0) {
   303				usb_err(usbatm, "%s: write BLOCK3 to modem failed (%d)!\n", __func__, ret);
   304				goto out_free;
   305			}
   306		}
   307		usb_dbg(usbatm, "%s: BLOCK3 uploaded (%zu bytes)\n", __func__, fw2->size);
   308	
   309		/* USB led static green, ADSL led static red */
   310	
   311		/* URB 142 */
   312		ret = usb_bulk_msg(usb_dev, usb_rcvbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
   313				   buffer, 0x200, &actual_length, DATA_TIMEOUT);
   314	
   315		if (ret < 0) {
   316			usb_err(usbatm, "%s: read BLOCK4 from modem failed (%d)!\n", __func__, ret);
   317			goto out_free;
   318		}
   319	
   320		/* success */
   321		usb_dbg(usbatm, "%s: BLOCK4 downloaded (%d bytes)\n", __func__, actual_length);
   322	
   323		/* Delay to allow firmware to start up. We can do this here
   324		   because we're in our own kernel thread anyway. */
   325		msleep_interruptible(1000);
   326	
   327		ret = usb_set_interface(usb_dev, INTERFACE_DATA, instance->params.altsetting
   328	
 > 329		if (ret < 0) {
   330			usb_err(usbatm, "%s: setting interface to %d failed (%d)!\n", __func__, instance->params.altsetting, ret);
   331			goto out_free;
   332		}
   333	
   334		/* Enable software buffering, if requested */
   335		if (sw_buffering)
   336			speedtch_set_swbuff(instance, 1);
   337	
   338		/* Magic spell; don't ask us what this does */
   339		speedtch_test_sequence(instance);
   340	
   341		ret = 0;
   342	
   343	out_free:
   344		free_page((unsigned long)buffer);
   345	out:
   346		return ret;
   347	}
   348	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] USB: atm: speedtch: do not use assignment in if condition
Posted by kernel test robot 1 month, 3 weeks ago
Hi Manuel,

kernel test robot noticed the following build warnings:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus linus/master v6.12-rc1 next-20241004]
[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/Manuel-Quintero-F/USB-atm-speedtch-do-not-use-assignment-in-if-condition/20241004-113643
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20241004030512.2036-1-sakunix%40yahoo.com
patch subject: [PATCH] USB: atm: speedtch: do not use assignment in if condition
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20241005/202410051744.I4PVYrMx-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241005/202410051744.I4PVYrMx-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410051744.I4PVYrMx-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/usb/atm/speedtch.c: In function 'speedtch_upload_firmware':
   drivers/usb/atm/speedtch.c:327:85: error: expected ')' before 'if'
     327 |         ret = usb_set_interface(usb_dev, INTERFACE_DATA, instance->params.altsetting
         |                                ~                                                    ^
         |                                                                                     )
     328 | 
     329 |         if (ret < 0) {
         |         ~~                                                                           
   drivers/usb/atm/speedtch.c:346:20: error: expected ';' before '}' token
     346 |         return ret;
         |                    ^
         |                    ;
     347 | }
         | ~                   
   drivers/usb/atm/speedtch.c:317:17: error: label 'out_free' used but not defined
     317 |                 goto out_free;
         |                 ^~~~
   drivers/usb/atm/speedtch.c:247:17: error: label 'out' used but not defined
     247 |                 goto out;
         |                 ^~~~
>> drivers/usb/atm/speedtch.c:347:1: warning: control reaches end of non-void function [-Wreturn-type]
     347 | }
         | ^
   drivers/usb/atm/speedtch.c: At top level:
   drivers/usb/atm/speedtch.c:160:13: warning: 'speedtch_test_sequence' defined but not used [-Wunused-function]
     160 | static void speedtch_test_sequence(struct speedtch_instance_data *instance)
         |             ^~~~~~~~~~~~~~~~~~~~~~
   drivers/usb/atm/speedtch.c:144:13: warning: 'speedtch_set_swbuff' defined but not used [-Wunused-function]
     144 | static void speedtch_set_swbuff(struct speedtch_instance_data *instance, int state)
         |             ^~~~~~~~~~~~~~~~~~~


vim +347 drivers/usb/atm/speedtch.c

^1da177e4c3f41 Linus Torvalds     2005-04-16  229  
48da7267ff1631 Duncan Sands       2005-05-11  230  static int speedtch_upload_firmware(struct speedtch_instance_data *instance,
48da7267ff1631 Duncan Sands       2005-05-11  231  				     const struct firmware *fw1,
48da7267ff1631 Duncan Sands       2005-05-11  232  				     const struct firmware *fw2)
^1da177e4c3f41 Linus Torvalds     2005-04-16  233  {
48da7267ff1631 Duncan Sands       2005-05-11  234  	unsigned char *buffer;
48da7267ff1631 Duncan Sands       2005-05-11  235  	struct usbatm_data *usbatm = instance->usbatm;
48da7267ff1631 Duncan Sands       2005-05-11  236  	struct usb_device *usb_dev = usbatm->usb_dev;
48da7267ff1631 Duncan Sands       2005-05-11  237  	int actual_length;
48da7267ff1631 Duncan Sands       2005-05-11  238  	int ret = 0;
48da7267ff1631 Duncan Sands       2005-05-11  239  	int offset;
48da7267ff1631 Duncan Sands       2005-05-11  240  
48da7267ff1631 Duncan Sands       2005-05-11  241  	usb_dbg(usbatm, "%s entered\n", __func__);
48da7267ff1631 Duncan Sands       2005-05-11  242  
3383ee4c3abf2e Greg Kroah-Hartman 2015-04-30  243  	buffer = (unsigned char *)__get_free_page(GFP_KERNEL);
3383ee4c3abf2e Greg Kroah-Hartman 2015-04-30  244  	if (!buffer) {
48da7267ff1631 Duncan Sands       2005-05-11  245  		ret = -ENOMEM;
48da7267ff1631 Duncan Sands       2005-05-11  246  		usb_dbg(usbatm, "%s: no memory for buffer!\n", __func__);
48da7267ff1631 Duncan Sands       2005-05-11  247  		goto out;
48da7267ff1631 Duncan Sands       2005-05-11  248  	}
48da7267ff1631 Duncan Sands       2005-05-11  249  
011db815231f40 Micah Gruber       2007-09-05  250  	if (!usb_ifnum_to_if(usb_dev, 2)) {
48da7267ff1631 Duncan Sands       2005-05-11  251  		ret = -ENODEV;
48da7267ff1631 Duncan Sands       2005-05-11  252  		usb_dbg(usbatm, "%s: interface not found!\n", __func__);
48da7267ff1631 Duncan Sands       2005-05-11  253  		goto out_free;
48da7267ff1631 Duncan Sands       2005-05-11  254  	}
48da7267ff1631 Duncan Sands       2005-05-11  255  
48da7267ff1631 Duncan Sands       2005-05-11  256  	/* URB 7 */
48da7267ff1631 Duncan Sands       2005-05-11  257  	if (dl_512_first) {	/* some modems need a read before writing the firmware */
48da7267ff1631 Duncan Sands       2005-05-11  258  		ret = usb_bulk_msg(usb_dev, usb_rcvbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
48da7267ff1631 Duncan Sands       2005-05-11  259  				   buffer, 0x200, &actual_length, 2000);
48da7267ff1631 Duncan Sands       2005-05-11  260  
48da7267ff1631 Duncan Sands       2005-05-11  261  		if (ret < 0 && ret != -ETIMEDOUT)
0ec3c7e856319b Duncan Sands       2006-01-17  262  			usb_warn(usbatm, "%s: read BLOCK0 from modem failed (%d)!\n", __func__, ret);
48da7267ff1631 Duncan Sands       2005-05-11  263  		else
48da7267ff1631 Duncan Sands       2005-05-11  264  			usb_dbg(usbatm, "%s: BLOCK0 downloaded (%d bytes)\n", __func__, ret);
48da7267ff1631 Duncan Sands       2005-05-11  265  	}
48da7267ff1631 Duncan Sands       2005-05-11  266  
48da7267ff1631 Duncan Sands       2005-05-11  267  	/* URB 8 : both leds are static green */
48da7267ff1631 Duncan Sands       2005-05-11  268  	for (offset = 0; offset < fw1->size; offset += PAGE_SIZE) {
48da7267ff1631 Duncan Sands       2005-05-11  269  		int thislen = min_t(int, PAGE_SIZE, fw1->size - offset);
48da7267ff1631 Duncan Sands       2005-05-11  270  		memcpy(buffer, fw1->data + offset, thislen);
48da7267ff1631 Duncan Sands       2005-05-11  271  
48da7267ff1631 Duncan Sands       2005-05-11  272  		ret = usb_bulk_msg(usb_dev, usb_sndbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
48da7267ff1631 Duncan Sands       2005-05-11  273  				   buffer, thislen, &actual_length, DATA_TIMEOUT);
48da7267ff1631 Duncan Sands       2005-05-11  274  
48da7267ff1631 Duncan Sands       2005-05-11  275  		if (ret < 0) {
0ec3c7e856319b Duncan Sands       2006-01-17  276  			usb_err(usbatm, "%s: write BLOCK1 to modem failed (%d)!\n", __func__, ret);
48da7267ff1631 Duncan Sands       2005-05-11  277  			goto out_free;
48da7267ff1631 Duncan Sands       2005-05-11  278  		}
48da7267ff1631 Duncan Sands       2005-05-11  279  		usb_dbg(usbatm, "%s: BLOCK1 uploaded (%zu bytes)\n", __func__, fw1->size);
48da7267ff1631 Duncan Sands       2005-05-11  280  	}
48da7267ff1631 Duncan Sands       2005-05-11  281  
48da7267ff1631 Duncan Sands       2005-05-11  282  	/* USB led blinking green, ADSL led off */
48da7267ff1631 Duncan Sands       2005-05-11  283  
48da7267ff1631 Duncan Sands       2005-05-11  284  	/* URB 11 */
48da7267ff1631 Duncan Sands       2005-05-11  285  	ret = usb_bulk_msg(usb_dev, usb_rcvbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
48da7267ff1631 Duncan Sands       2005-05-11  286  			   buffer, 0x200, &actual_length, DATA_TIMEOUT);
48da7267ff1631 Duncan Sands       2005-05-11  287  
48da7267ff1631 Duncan Sands       2005-05-11  288  	if (ret < 0) {
0ec3c7e856319b Duncan Sands       2006-01-17  289  		usb_err(usbatm, "%s: read BLOCK2 from modem failed (%d)!\n", __func__, ret);
48da7267ff1631 Duncan Sands       2005-05-11  290  		goto out_free;
48da7267ff1631 Duncan Sands       2005-05-11  291  	}
48da7267ff1631 Duncan Sands       2005-05-11  292  	usb_dbg(usbatm, "%s: BLOCK2 downloaded (%d bytes)\n", __func__, actual_length);
48da7267ff1631 Duncan Sands       2005-05-11  293  
48da7267ff1631 Duncan Sands       2005-05-11  294  	/* URBs 12 to 139 - USB led blinking green, ADSL led off */
48da7267ff1631 Duncan Sands       2005-05-11  295  	for (offset = 0; offset < fw2->size; offset += PAGE_SIZE) {
48da7267ff1631 Duncan Sands       2005-05-11  296  		int thislen = min_t(int, PAGE_SIZE, fw2->size - offset);
48da7267ff1631 Duncan Sands       2005-05-11  297  		memcpy(buffer, fw2->data + offset, thislen);
48da7267ff1631 Duncan Sands       2005-05-11  298  
48da7267ff1631 Duncan Sands       2005-05-11  299  		ret = usb_bulk_msg(usb_dev, usb_sndbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
48da7267ff1631 Duncan Sands       2005-05-11  300  				   buffer, thislen, &actual_length, DATA_TIMEOUT);
48da7267ff1631 Duncan Sands       2005-05-11  301  
48da7267ff1631 Duncan Sands       2005-05-11  302  		if (ret < 0) {
0ec3c7e856319b Duncan Sands       2006-01-17  303  			usb_err(usbatm, "%s: write BLOCK3 to modem failed (%d)!\n", __func__, ret);
48da7267ff1631 Duncan Sands       2005-05-11  304  			goto out_free;
48da7267ff1631 Duncan Sands       2005-05-11  305  		}
48da7267ff1631 Duncan Sands       2005-05-11  306  	}
48da7267ff1631 Duncan Sands       2005-05-11  307  	usb_dbg(usbatm, "%s: BLOCK3 uploaded (%zu bytes)\n", __func__, fw2->size);
48da7267ff1631 Duncan Sands       2005-05-11  308  
48da7267ff1631 Duncan Sands       2005-05-11  309  	/* USB led static green, ADSL led static red */
48da7267ff1631 Duncan Sands       2005-05-11  310  
48da7267ff1631 Duncan Sands       2005-05-11  311  	/* URB 142 */
48da7267ff1631 Duncan Sands       2005-05-11  312  	ret = usb_bulk_msg(usb_dev, usb_rcvbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
48da7267ff1631 Duncan Sands       2005-05-11  313  			   buffer, 0x200, &actual_length, DATA_TIMEOUT);
^1da177e4c3f41 Linus Torvalds     2005-04-16  314  
^1da177e4c3f41 Linus Torvalds     2005-04-16  315  	if (ret < 0) {
0ec3c7e856319b Duncan Sands       2006-01-17  316  		usb_err(usbatm, "%s: read BLOCK4 from modem failed (%d)!\n", __func__, ret);
48da7267ff1631 Duncan Sands       2005-05-11  317  		goto out_free;
48da7267ff1631 Duncan Sands       2005-05-11  318  	}
48da7267ff1631 Duncan Sands       2005-05-11  319  
48da7267ff1631 Duncan Sands       2005-05-11  320  	/* success */
48da7267ff1631 Duncan Sands       2005-05-11  321  	usb_dbg(usbatm, "%s: BLOCK4 downloaded (%d bytes)\n", __func__, actual_length);
48da7267ff1631 Duncan Sands       2005-05-11  322  
48da7267ff1631 Duncan Sands       2005-05-11  323  	/* Delay to allow firmware to start up. We can do this here
48da7267ff1631 Duncan Sands       2005-05-11  324  	   because we're in our own kernel thread anyway. */
48da7267ff1631 Duncan Sands       2005-05-11  325  	msleep_interruptible(1000);
48da7267ff1631 Duncan Sands       2005-05-11  326  
a23b48ae442d5f Manuel Quintero F  2024-10-03  327  	ret = usb_set_interface(usb_dev, INTERFACE_DATA, instance->params.altsetting
a23b48ae442d5f Manuel Quintero F  2024-10-03  328  
a23b48ae442d5f Manuel Quintero F  2024-10-03  329  	if (ret < 0) {
6a4f1b41357d2b Duncan Sands       2006-10-05  330  		usb_err(usbatm, "%s: setting interface to %d failed (%d)!\n", __func__, instance->params.altsetting, ret);
6f7494759870ec Duncan Sands       2006-01-13  331  		goto out_free;
6f7494759870ec Duncan Sands       2006-01-13  332  	}
6f7494759870ec Duncan Sands       2006-01-13  333  
48da7267ff1631 Duncan Sands       2005-05-11  334  	/* Enable software buffering, if requested */
48da7267ff1631 Duncan Sands       2005-05-11  335  	if (sw_buffering)
48da7267ff1631 Duncan Sands       2005-05-11  336  		speedtch_set_swbuff(instance, 1);
48da7267ff1631 Duncan Sands       2005-05-11  337  
48da7267ff1631 Duncan Sands       2005-05-11  338  	/* Magic spell; don't ask us what this does */
48da7267ff1631 Duncan Sands       2005-05-11  339  	speedtch_test_sequence(instance);
48da7267ff1631 Duncan Sands       2005-05-11  340  
48da7267ff1631 Duncan Sands       2005-05-11  341  	ret = 0;
48da7267ff1631 Duncan Sands       2005-05-11  342  
48da7267ff1631 Duncan Sands       2005-05-11  343  out_free:
48da7267ff1631 Duncan Sands       2005-05-11  344  	free_page((unsigned long)buffer);
48da7267ff1631 Duncan Sands       2005-05-11  345  out:
^1da177e4c3f41 Linus Torvalds     2005-04-16  346  	return ret;
^1da177e4c3f41 Linus Torvalds     2005-04-16 @347  }
^1da177e4c3f41 Linus Torvalds     2005-04-16  348  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] USB: atm: speedtch: do not use assignment in if condition
Posted by kernel test robot 1 month, 3 weeks ago
Hi Manuel,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus linus/master v6.12-rc1 next-20241004]
[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/Manuel-Quintero-F/USB-atm-speedtch-do-not-use-assignment-in-if-condition/20241004-113643
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20241004030512.2036-1-sakunix%40yahoo.com
patch subject: [PATCH] USB: atm: speedtch: do not use assignment in if condition
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20241005/202410051738.GFfOxDuk-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241005/202410051738.GFfOxDuk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410051738.GFfOxDuk-lkp@intel.com/

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

   drivers/usb/atm/speedtch.c: In function 'speedtch_upload_firmware':
>> drivers/usb/atm/speedtch.c:327:85: error: expected ')' before 'if'
     327 |         ret = usb_set_interface(usb_dev, INTERFACE_DATA, instance->params.altsetting
         |                                ~                                                    ^
         |                                                                                     )
     328 | 
     329 |         if (ret < 0) {
         |         ~~                                                                           
>> drivers/usb/atm/speedtch.c:346:20: error: expected ';' before '}' token
     346 |         return ret;
         |                    ^
         |                    ;
     347 | }
         | ~                   
>> drivers/usb/atm/speedtch.c:317:17: error: label 'out_free' used but not defined
     317 |                 goto out_free;
         |                 ^~~~
>> drivers/usb/atm/speedtch.c:247:17: error: label 'out' used but not defined
     247 |                 goto out;
         |                 ^~~~
>> drivers/usb/atm/speedtch.c:347:1: warning: no return statement in function returning non-void [-Wreturn-type]
     347 | }
         | ^
   drivers/usb/atm/speedtch.c: At top level:
>> drivers/usb/atm/speedtch.c:160:13: warning: 'speedtch_test_sequence' defined but not used [-Wunused-function]
     160 | static void speedtch_test_sequence(struct speedtch_instance_data *instance)
         |             ^~~~~~~~~~~~~~~~~~~~~~
>> drivers/usb/atm/speedtch.c:144:13: warning: 'speedtch_set_swbuff' defined but not used [-Wunused-function]
     144 | static void speedtch_set_swbuff(struct speedtch_instance_data *instance, int state)
         |             ^~~~~~~~~~~~~~~~~~~


vim +327 drivers/usb/atm/speedtch.c

   143	
 > 144	static void speedtch_set_swbuff(struct speedtch_instance_data *instance, int state)
   145	{
   146		struct usbatm_data *usbatm = instance->usbatm;
   147		struct usb_device *usb_dev = usbatm->usb_dev;
   148		int ret;
   149	
   150		ret = usb_control_msg(usb_dev, usb_sndctrlpipe(usb_dev, 0),
   151				      0x32, 0x40, state ? 0x01 : 0x00, 0x00, NULL, 0, CTRL_TIMEOUT);
   152		if (ret < 0)
   153			usb_warn(usbatm,
   154				 "%sabling SW buffering: usb_control_msg returned %d\n",
   155				 state ? "En" : "Dis", ret);
   156		else
   157			usb_dbg(usbatm, "speedtch_set_swbuff: %sbled SW buffering\n", state ? "En" : "Dis");
   158	}
   159	
 > 160	static void speedtch_test_sequence(struct speedtch_instance_data *instance)
   161	{
   162		struct usbatm_data *usbatm = instance->usbatm;
   163		struct usb_device *usb_dev = usbatm->usb_dev;
   164		unsigned char *buf = instance->scratch_buffer;
   165		int ret;
   166	
   167		/* URB 147 */
   168		buf[0] = 0x1c;
   169		buf[1] = 0x50;
   170		ret = usb_control_msg(usb_dev, usb_sndctrlpipe(usb_dev, 0),
   171				      0x01, 0x40, 0x0b, 0x00, buf, 2, CTRL_TIMEOUT);
   172		if (ret < 0)
   173			usb_warn(usbatm, "%s failed on URB147: %d\n", __func__, ret);
   174	
   175		/* URB 148 */
   176		buf[0] = 0x32;
   177		buf[1] = 0x00;
   178		ret = usb_control_msg(usb_dev, usb_sndctrlpipe(usb_dev, 0),
   179				      0x01, 0x40, 0x02, 0x00, buf, 2, CTRL_TIMEOUT);
   180		if (ret < 0)
   181			usb_warn(usbatm, "%s failed on URB148: %d\n", __func__, ret);
   182	
   183		/* URB 149 */
   184		buf[0] = 0x01;
   185		buf[1] = 0x00;
   186		buf[2] = 0x01;
   187		ret = usb_control_msg(usb_dev, usb_sndctrlpipe(usb_dev, 0),
   188				      0x01, 0x40, 0x03, 0x00, buf, 3, CTRL_TIMEOUT);
   189		if (ret < 0)
   190			usb_warn(usbatm, "%s failed on URB149: %d\n", __func__, ret);
   191	
   192		/* URB 150 */
   193		buf[0] = 0x01;
   194		buf[1] = 0x00;
   195		buf[2] = 0x01;
   196		ret = usb_control_msg(usb_dev, usb_sndctrlpipe(usb_dev, 0),
   197				      0x01, 0x40, 0x04, 0x00, buf, 3, CTRL_TIMEOUT);
   198		if (ret < 0)
   199			usb_warn(usbatm, "%s failed on URB150: %d\n", __func__, ret);
   200	
   201		/* Extra initialisation in recent drivers - gives higher speeds */
   202	
   203		/* URBext1 */
   204		buf[0] = instance->params.ModemMode;
   205		ret = usb_control_msg(usb_dev, usb_sndctrlpipe(usb_dev, 0),
   206				      0x01, 0x40, 0x11, 0x00, buf, 1, CTRL_TIMEOUT);
   207		if (ret < 0)
   208			usb_warn(usbatm, "%s failed on URBext1: %d\n", __func__, ret);
   209	
   210		/* URBext2 */
   211		/* This seems to be the one which actually triggers the higher sync
   212		   rate -- it does require the new firmware too, although it works OK
   213		   with older firmware */
   214		ret = usb_control_msg(usb_dev, usb_sndctrlpipe(usb_dev, 0),
   215				      0x01, 0x40, 0x14, 0x00,
   216				      instance->params.ModemOption,
   217				      MODEM_OPTION_LENGTH, CTRL_TIMEOUT);
   218		if (ret < 0)
   219			usb_warn(usbatm, "%s failed on URBext2: %d\n", __func__, ret);
   220	
   221		/* URBext3 */
   222		buf[0] = instance->params.BMaxDSL & 0xff;
   223		buf[1] = instance->params.BMaxDSL >> 8;
   224		ret = usb_control_msg(usb_dev, usb_sndctrlpipe(usb_dev, 0),
   225				      0x01, 0x40, 0x12, 0x00, buf, 2, CTRL_TIMEOUT);
   226		if (ret < 0)
   227			usb_warn(usbatm, "%s failed on URBext3: %d\n", __func__, ret);
   228	}
   229	
   230	static int speedtch_upload_firmware(struct speedtch_instance_data *instance,
   231					     const struct firmware *fw1,
   232					     const struct firmware *fw2)
   233	{
   234		unsigned char *buffer;
   235		struct usbatm_data *usbatm = instance->usbatm;
   236		struct usb_device *usb_dev = usbatm->usb_dev;
   237		int actual_length;
   238		int ret = 0;
   239		int offset;
   240	
   241		usb_dbg(usbatm, "%s entered\n", __func__);
   242	
   243		buffer = (unsigned char *)__get_free_page(GFP_KERNEL);
   244		if (!buffer) {
   245			ret = -ENOMEM;
   246			usb_dbg(usbatm, "%s: no memory for buffer!\n", __func__);
 > 247			goto out;
   248		}
   249	
   250		if (!usb_ifnum_to_if(usb_dev, 2)) {
   251			ret = -ENODEV;
   252			usb_dbg(usbatm, "%s: interface not found!\n", __func__);
   253			goto out_free;
   254		}
   255	
   256		/* URB 7 */
   257		if (dl_512_first) {	/* some modems need a read before writing the firmware */
   258			ret = usb_bulk_msg(usb_dev, usb_rcvbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
   259					   buffer, 0x200, &actual_length, 2000);
   260	
   261			if (ret < 0 && ret != -ETIMEDOUT)
   262				usb_warn(usbatm, "%s: read BLOCK0 from modem failed (%d)!\n", __func__, ret);
   263			else
   264				usb_dbg(usbatm, "%s: BLOCK0 downloaded (%d bytes)\n", __func__, ret);
   265		}
   266	
   267		/* URB 8 : both leds are static green */
   268		for (offset = 0; offset < fw1->size; offset += PAGE_SIZE) {
   269			int thislen = min_t(int, PAGE_SIZE, fw1->size - offset);
   270			memcpy(buffer, fw1->data + offset, thislen);
   271	
   272			ret = usb_bulk_msg(usb_dev, usb_sndbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
   273					   buffer, thislen, &actual_length, DATA_TIMEOUT);
   274	
   275			if (ret < 0) {
   276				usb_err(usbatm, "%s: write BLOCK1 to modem failed (%d)!\n", __func__, ret);
   277				goto out_free;
   278			}
   279			usb_dbg(usbatm, "%s: BLOCK1 uploaded (%zu bytes)\n", __func__, fw1->size);
   280		}
   281	
   282		/* USB led blinking green, ADSL led off */
   283	
   284		/* URB 11 */
   285		ret = usb_bulk_msg(usb_dev, usb_rcvbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
   286				   buffer, 0x200, &actual_length, DATA_TIMEOUT);
   287	
   288		if (ret < 0) {
   289			usb_err(usbatm, "%s: read BLOCK2 from modem failed (%d)!\n", __func__, ret);
   290			goto out_free;
   291		}
   292		usb_dbg(usbatm, "%s: BLOCK2 downloaded (%d bytes)\n", __func__, actual_length);
   293	
   294		/* URBs 12 to 139 - USB led blinking green, ADSL led off */
   295		for (offset = 0; offset < fw2->size; offset += PAGE_SIZE) {
   296			int thislen = min_t(int, PAGE_SIZE, fw2->size - offset);
   297			memcpy(buffer, fw2->data + offset, thislen);
   298	
   299			ret = usb_bulk_msg(usb_dev, usb_sndbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
   300					   buffer, thislen, &actual_length, DATA_TIMEOUT);
   301	
   302			if (ret < 0) {
   303				usb_err(usbatm, "%s: write BLOCK3 to modem failed (%d)!\n", __func__, ret);
   304				goto out_free;
   305			}
   306		}
   307		usb_dbg(usbatm, "%s: BLOCK3 uploaded (%zu bytes)\n", __func__, fw2->size);
   308	
   309		/* USB led static green, ADSL led static red */
   310	
   311		/* URB 142 */
   312		ret = usb_bulk_msg(usb_dev, usb_rcvbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
   313				   buffer, 0x200, &actual_length, DATA_TIMEOUT);
   314	
   315		if (ret < 0) {
   316			usb_err(usbatm, "%s: read BLOCK4 from modem failed (%d)!\n", __func__, ret);
 > 317			goto out_free;
   318		}
   319	
   320		/* success */
   321		usb_dbg(usbatm, "%s: BLOCK4 downloaded (%d bytes)\n", __func__, actual_length);
   322	
   323		/* Delay to allow firmware to start up. We can do this here
   324		   because we're in our own kernel thread anyway. */
   325		msleep_interruptible(1000);
   326	
 > 327		ret = usb_set_interface(usb_dev, INTERFACE_DATA, instance->params.altsetting
   328	
   329		if (ret < 0) {
   330			usb_err(usbatm, "%s: setting interface to %d failed (%d)!\n", __func__, instance->params.altsetting, ret);
   331			goto out_free;
   332		}
   333	
   334		/* Enable software buffering, if requested */
   335		if (sw_buffering)
   336			speedtch_set_swbuff(instance, 1);
   337	
   338		/* Magic spell; don't ask us what this does */
   339		speedtch_test_sequence(instance);
   340	
   341		ret = 0;
   342	
   343	out_free:
   344		free_page((unsigned long)buffer);
   345	out:
 > 346		return ret;
 > 347	}
   348	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] USB: atm: speedtch: do not use assignment in if condition
Posted by Greg KH 1 month, 3 weeks ago
On Thu, Oct 03, 2024 at 08:05:12PM -0700, Manuel Quintero F wrote:
> Fix checkpatch error "do not use assignment in if condition"
> 
> Signed-off-by: Manuel Quintero F <sakunix@yahoo.com>
> Signed-off-by: Manuel Quintero F <sakunix@yahoo.com>

Why twice?

> ---
>  drivers/usb/atm/speedtch.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/atm/speedtch.c b/drivers/usb/atm/speedtch.c
> index 973548b5c15c..dfd362abf602 100644
> --- a/drivers/usb/atm/speedtch.c
> +++ b/drivers/usb/atm/speedtch.c
> @@ -324,7 +324,9 @@ static int speedtch_upload_firmware(struct speedtch_instance_data *instance,
>  	   because we're in our own kernel thread anyway. */
>  	msleep_interruptible(1000);
>  
> -	if ((ret = usb_set_interface(usb_dev, INTERFACE_DATA, instance->params.altsetting)) < 0) {
> +	ret = usb_set_interface(usb_dev, INTERFACE_DATA, instance->params.altsetting
> +
> +	if (ret < 0) {

Why the extra blank line?

When learning to do kernel changes, I recommend doing so in
drivers/staging/ first, as that is what it is there for.  Only after
getting experience would I recommend doing this in other areas of the
kernel, and even then, only do checkpatch cleanups for code that you can
test, or that the subsystem maintainer has explicitly asked for.

good luck!

greg k-h