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

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

[Thomas Tuttle]

Signed-off-by: Thomas Tuttle <ttuttle-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 drivers/usb/serial/qcserial.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/serial/qcserial.c b/drivers/usb/serial/qcserial.c
index 26e3e30..5a817d6 100644
--- a/drivers/usb/serial/qcserial.c
+++ b/drivers/usb/serial/qcserial.c
@@ -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) &&
-- 
1.7.7.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Greg KH]

On Wed, Feb 01, 2012 at 04:07:23PM -0500, Thomas Tuttle wrote:
> Signed-off-by: Thomas Tuttle <ttuttle@chromium.org>
> ---
>  drivers/usb/serial/qcserial.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/usb/serial/qcserial.c b/drivers/usb/serial/qcserial.c
> index 26e3e30..5a817d6 100644
> --- a/drivers/usb/serial/qcserial.c
> +++ b/drivers/usb/serial/qcserial.c
> @@ -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

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

[ttuttle]

On Wed, Feb 1, 2012 at 16:11, Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
>> +             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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Greg KH]

On Wed, Feb 01, 2012 at 04:47:22PM -0500, ttuttle wrote:
> On Wed, Feb 1, 2012 at 16:11, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> +             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.

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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[ttuttle]

On Wed, Feb 1, 2012 at 17:02, Greg KH <gregkh@linuxfoundation.org> wrote:
> 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.

> So yes, please drop it.

Alright.

> 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.

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

[Greg KH]

On Wed, Feb 01, 2012 at 05:15:57PM -0500, ttuttle wrote:
> On Wed, Feb 1, 2012 at 17:02, Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
> > 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.
> 
> > So yes, please drop it.
> 
> Alright.
> 
> > 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[ttuttle]

On Wed, Feb 1, 2012 at 17:23, Greg KH <gregkh@linuxfoundation.org> wrote:
> No problem, I'll just pick the 2 good ones, thanks.

Thanks!

-- ttuttle