em28xx-cards: don't print a misleading message
Subsystems (3)
PATCH 1/1 em28xx-cards: don't print a misleading message
There were some missing curly braces so it always says that the transfer mode changed even if it didn't. Also the indenting uses spaces instead of tabs.
drivers/media/usb/em28xx/em28xx-cards.c +5 -4
@@ -3329,10 +3329,11 @@ static int em28xx_usb_probe(struct usb_interface *interface,/* Select USB transfer types to use */ if (has_video) { -if (!dev->analog_ep_isoc || (try_bulk && dev->analog_ep_bulk)) - dev->analog_xfer_bulk =1 ; -em28xx_info ("analog set to %s mode.\n" , - dev->analog_xfer_bulk ?"bulk" :"isoc" ); +if (!dev->analog_ep_isoc || (try_bulk && dev->analog_ep_bulk)) { + dev->analog_xfer_bulk =1 ; +em28xx_info ("analog set to %s mode.\n" , + dev->analog_xfer_bulk ?"bulk" :"isoc" ); + } }if (has_dvb) {if (!dev->dvb_ep_isoc || (try_bulk && dev->dvb_ep_bulk))
Am 17.02.2014 21:04, schrieb Dan Carpenter:
Quoted 2 lines
There were some missing curly braces so it always says that the transfer mode changed even if it didn't.
It's not a transfer mode change, it's the initial selection (which never changes).
The intention of this section is to print which interface (type) is choosen for analog video.
The same is done for digital video (DVB) a few lines later. I think we should just switch from info to debug level.
Quoted 2 lines
Also the indenting uses spaces instead
of tabs.
Yes, indenting is wrong and that's what causes the confusion here. ;-)
Quoted 9 lines
Signed-off-by Dan Carpenter <[email protected]>drivers/media/usb/em28xx/em28xx-cards.c +0
-0@@ -3329,10 +3329,11 @@ static int em28xx_usb_probe(struct usb_interface *interface,
drivers/media/usb/em28xx/em28xx-cards.c static int em28xx_usb_probe(struct usb_interface *interface, · line 3335
Quoted 10 lines
drivers/media/usb/em28xx/em28xx-cards.c +4
-4/* Select USB transfer types to use */ if (has_video) { -if (!dev->analog_ep_isoc || (try_bulk && dev->analog_ep_bulk)) - dev->analog_xfer_bulk =1 ; -em28xx_info ("analog set to %s mode.\n" , - dev->analog_xfer_bulk ?"bulk" :"isoc" ); +if (!dev->analog_ep_isoc || (try_bulk && dev->analog_ep_bulk)) { + dev->analog_xfer_bulk =1 ; +em28xx_info ("analog set to %s mode.\n" , + dev->analog_xfer_bulk ?"bulk" :"isoc" );
Now you will never hit the "isoc" path.
Regards,
Frank
Quoted 8 lines
drivers/media/usb/em28xx/em28xx-cards.c +1
-1+ } }if (has_dvb) {if (!dev->dvb_ep_isoc || (try_bulk && dev->dvb_ep_bulk)) --To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Thank you so much for the review. I should have noticed that myself... I will send a patch to correct the indenting instead.
regards,
dan carpenter
PATCH v2 1/1 em28xx-cards: remove a wrong indent level
In reply to Frank SchäferThis code is correct but the indenting is wrong and triggers a static checker warning "add curly braces?".
---
v2: in v1 I added curly braces.
drivers/media/usb/em28xx/em28xx-cards.c +2 -2
@@ -3331,8 +3331,8 @@ static int em28xx_usb_probe(struct usb_interface *interface,if (has_video) {if (!dev->analog_ep_isoc || (try_bulk && dev->analog_ep_bulk)) dev->analog_xfer_bulk =1 ; -em28xx_info ("analog set to %s mode.\n" , - dev->analog_xfer_bulk ?"bulk" :"isoc" ); +em28xx_info ("analog set to %s mode.\n" , + dev->analog_xfer_bulk ?"bulk" :"isoc" ); }if (has_dvb) {if (!dev->dvb_ep_isoc || (try_bulk && dev->dvb_ep_bulk))
Re: [patch v2] [patch] [media] em28xx-cards: remove a wrong indent level
Am 05.03.2014 12:09, schrieb Dan Carpenter:
Quoted 12 lines
This code is correct but the indenting is wrong and triggers a static checker warning "add curly braces?".
Signed-off-by Dan Carpenter <[email protected]>---
v2: in v1 I added curly braces.drivers/media/usb/em28xx/em28xx-cards.c +0
-0
drivers/media/usb/em28xx/em28xx-cards.c static int em28xx_usb_probe(struct usb_interface *interface, · line 3335
Quoted 7 lines
drivers/media/usb/em28xx/em28xx-cards.c +2
-2@@ -3331,8 +3331,8 @@ static int em28xx_usb_probe(struct usb_interface *interface,if (has_video) {if (!dev->analog_ep_isoc || (try_bulk && dev->analog_ep_bulk)) dev->analog_xfer_bulk =1 ; -em28xx_info ("analog set to %s mode.\n" , - dev->analog_xfer_bulk ?"bulk" :"isoc" ); +em28xx_info ("analog set to %s mode.\n" , + dev->analog_xfer_bulk ?"bulk" :"isoc" );
Instead of moving em28xx_info(...) to the left the if section needs to be moved to the right:
- if (!dev->analog_ep_isoc || (try_bulk && dev->analog_ep_bulk))
- dev->analog_xfer_bulk = 1;
+ if (!dev->analog_ep_isoc || (try_bulk && dev->analog_ep_bulk))
+ dev->analog_xfer_bulk = 1;
While you are at it, could you also do fix the indention in the next paragraph ?
Thanks !
Regards,
Frank
Quoted 7 lines
drivers/media/usb/em28xx/em28xx-cards.c +0
-1}if (has_dvb) {if (!dev->dvb_ep_isoc || (try_bulk && dev->dvb_ep_bulk)) --To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch v2] [patch] [media] em28xx-cards: remove a wrong indent level
Quoted 25 lines · Frank Schäfer
Am 05.03.2014 12:09, schrieb Dan Carpenter:
Quoted 12 lines
This code is correct but the indenting is wrong and triggers a static checker warning "add curly braces?".
Signed-off-by Dan Carpenter <[email protected]>---
v2: in v1 I added curly braces.drivers/media/usb/em28xx/em28xx-cards.c +0
-0drivers/media/usb/em28xx/em28xx-cards.c static int em28xx_usb_probe(struct usb_interface *interface, · line 3335
Quoted 7 lines
drivers/media/usb/em28xx/em28xx-cards.c +2
-2@@ -3331,8 +3331,8 @@ static int em28xx_usb_probe(struct usb_interface *interface,if (has_video) {if (!dev->analog_ep_isoc || (try_bulk && dev->analog_ep_bulk)) dev->analog_xfer_bulk =1 ; -em28xx_info ("analog set to %s mode.\n" , - dev->analog_xfer_bulk ?"bulk" :"isoc" ); +em28xx_info ("analog set to %s mode.\n" , + dev->analog_xfer_bulk ?"bulk" :"isoc" );Instead of moving
em28xx_info(...)to the left the if section needs to be moved to the right:
Review
Quoted 4 lines
(quoted diff) +2
-2- if (!dev->analog_ep_isoc || (try_bulk && dev->analog_ep_bulk)) - dev->analog_xfer_bulk = 1; + if (!dev->analog_ep_isoc || (try_bulk && dev->analog_ep_bulk)) + dev->analog_xfer_bulk = 1;
Yes. I'd happy to.
Quoted 3 lines
While you are at it, could you also do fix the indention in the next paragraph ?
The next paragraph is almost identical but my static checker was ignoring the curly braces because of the blank line. I'll modify to complain about that as well.
I'll resend. Thanks for the review.
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v3 1/1 em28xx-cards: fix indenting in probe()
There was a mix of 4 space and tab indenting here which was confusing.
---
v3: Just fix all the surrounding indents as well.
v2: At first I thought the code was buggy and was missing curly braces
but it was just the indenting which was confusing.
drivers/media/usb/em28xx/em28xx-cards.c +6 -6
@@ -3416,14 +3416,14 @@ static int em28xx_usb_probe(struct usb_interface *interface,/* Select USB transfer types to use */ if (has_video) { -if (!dev->analog_ep_isoc || (try_bulk && dev->analog_ep_bulk)) - dev->analog_xfer_bulk =1 ; -em28xx_info ("analog set to %s mode.\n" , - dev->analog_xfer_bulk ?"bulk" :"isoc" ); +if (!dev->analog_ep_isoc || (try_bulk && dev->analog_ep_bulk)) + dev->analog_xfer_bulk =1 ; +em28xx_info ("analog set to %s mode.\n" , + dev->analog_xfer_bulk ?"bulk" :"isoc" ); }if (has_dvb) { -if (!dev->dvb_ep_isoc || (try_bulk && dev->dvb_ep_bulk)) - dev->dvb_xfer_bulk =1 ; +if (!dev->dvb_ep_isoc || (try_bulk && dev->dvb_ep_bulk)) + dev->dvb_xfer_bulk =1 ;em28xx_info ("dvb set to %s mode.\n" , dev->dvb_xfer_bulk ?"bulk" :"isoc" );