| Message ID | 20241004030512.2036-1-sakunix@yahoo.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | USB: atm: speedtch: do not use assignment in if condition | expand |
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
El jueves, 3 de octubre de 2024, 23:20:08 GMT-7, Greg KH <gregkh@linuxfoundation.org> escribió: 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? When I checked the patch it was not signed 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? I already had that blank line before the if, and the checkpatch script did not give me that error 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 Thanks for the info, should I correct the blank line and send you version 2 of the patch?
On Fri, Oct 04, 2024 at 09:07:15PM +0000, Manuel wrote: > El jueves, 3 de octubre de 2024, 23:20:08 GMT-7, Greg KH <gregkh@linuxfoundation.org> escribió: 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? > > When I checked the patch it was not signed twice. Odd quoting style... Anyway, when you look at the patch you sent, it was signed twice, so something went wrong on your end :( > Thanks for the info, should I correct the blank line and send you version 2 of the patch? Please get more experience in the drivers/staging/ portion of the kernel before going out beyond that portion. thanks, greg k-h
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
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
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
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; }