Skip to contentSkip to conversation

qcserial: log when an interface has too many altsettings

PATCH In review Thomas Tuttle 4 participants 7 messages Feb 1, 2012 2 messages not in the archive

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 2/4 qcserial: log when an interface has too many altsettings

---
 drivers/usb/serial/qcserial.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)
drivers/usb/serial/qcserial.c +5 -1
index 26e3e30..5a817d6 100644
@@ -145,8 +145,12 @@ static int qcprobe(struct usb_serial *serial, const struct usb_device_id *id)
 		/* Gobi 2000 has a single altsetting, older ones have two */
 		if (serial->interface->num_altsetting == 2)
 			intf = &serial->interface->altsetting[1];
-		else if (serial->interface->num_altsetting > 2)
+		else if (serial->interface->num_altsetting > 2) {
+			dev_err(&serial->dev->dev,
+				"too many altsettings: %u",
+				serial->interface->num_altsetting);
 			break;
+		}
 
 		if (intf->desc.bNumEndpoints == 2 &&
 		    usb_endpoint_is_bulk_in(&intf->endpoint[0].desc) &&
Signature · 6 lines
1.7.7.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
maintainer
#1 lore raw
Quoted 10 lines · Thomas Tuttle
---
 drivers/usb/serial/qcserial.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)
drivers/usb/serial/qcserial.c +0 -0
index 26e3e30..5a817d6 100644

drivers/usb/serial/qcserial.c static int qcprobe(struct usb_serial *serial, const struct usb_device_id *id) · line 151

Quoted 8 lines
drivers/usb/serial/qcserial.c +4 -1
@@ -145,8 +145,12 @@ static int qcprobe(struct usb_serial *serial, const struct usb_device_id *id)
 		/* Gobi 2000 has a single altsetting, older ones have two */
 		if (serial->interface->num_altsetting == 2)
 			intf = &serial->interface->altsetting[1];
-		else if (serial->interface->num_altsetting > 2)
+		else if (serial->interface->num_altsetting > 2) {
+			dev_err(&serial->dev->dev,
+				"too many altsettings: %u",
+				serial->interface->num_altsetting);

Why would this happen? And what does spitting out an error do about it? What are we supposed to do with this information?

thanks,

greg k-h

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#3 lore raw
Quoted 7 lines · Greg KH
Quoted 4 lines

+             else if (serial->interface->num_altsetting > 2) {
+                     dev_err(&serial->dev->dev,
+                             "too many altsettings: %u",
+                             serial->interface->num_altsetting);

Why would this happen?  And what does spitting out an error do about it? What are we supposed to do with this information?

This would happen if we found a device that was returning an unexpected USB configuration. We've never actually seen this happen -- it was just there in case. I can drop it if you think it's not worth the trouble.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

maintainer
#4 lore raw
Quoted 6 lines · ttuttle
Quoted 5 lines · Greg KH
Quoted 4 lines

+             else if (serial->interface->num_altsetting > 2) {
+                     dev_err(&serial->dev->dev,
+                             "too many altsettings: %u",
+                             serial->interface->num_altsetting);

Quoted 7 lines
Quoted 2 lines

Why would this happen?  And what does spitting out an error do about it? What are we supposed to do with this information?

This would happen if we found a device that was returning an unexpected USB configuration. We've never actually seen this happen -- it was just there in case. I can drop it if you think it's not worth the trouble.

As the code properly handles such an error, my point is, if a user sees this new scary message, what are they supposed to do with it?

So yes, please drop it.

Are you going to resend this series based on the comments so far?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#5 lore raw
Quoted 2 lines · Greg KH

As the code properly handles such an error, my point is, if a user sees this new scary message, what are they supposed to do with it?

The intent was that when we saw it in logs, we'd know to look straight at the USB configuration, but okay.

Quoted 1 line

So yes, please drop it.

Alright.

Quoted 1 line

Are you going to resend this series based on the comments so far?

I can, if it's easier for you than just selectively applying the two remaining ones.

A message that is not in the archive (sent off-list or not yet received): CADyrwZQNMpxh5yTBnH68+0KK9r5NFvFMgad1K8qXouWAHY=1Rg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#7 lore raw
Quoted 7 lines · ttuttle
Quoted 2 lines · Greg KH

As the code properly handles such an error, my point is, if a user sees this new scary message, what are they supposed to do with it?

The intent was that when we saw it in logs, we'd know to look straight at the USB configuration, but okay.

Quoted 8 lines
Quoted 1 line

So yes, please drop it.

Alright.

Quoted 1 line

Are you going to resend this series based on the comments so far?

I can, if it's easier for you than just selectively applying the two remaining ones.

No problem, I'll just pick the 2 good ones, thanks.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#8 lore raw
Quoted 1 line · Greg KH

No problem, I'll just pick the 2 good ones, thanks.

Thanks!

-- ttuttle