Re: [PATCH 2/2] USB: pl2303: Print an error message if break fails.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2011-11-03 12:04 -0700, Greg KH wrote:
> On Thu, Nov 03, 2011 at 02:40:56PM -0400, Nick Bowler wrote:
> > Silently doing nothing when a break is requested but fails is not the
> > most user-friendly behaviour.  Convert the existing debug message to a
> > proper error message.
> 
> What we really need to do is propagate that error back to userspace,
> unfortunately we forgot to do that, so we need to change the function
> signature.

OK, this actually looks pretty straightforward to do.  I'll try to look
into it over the weekend.

> >  	if (result)
> > -		dbg("%s - error sending break = %d", __func__, result);
> > +		dev_err(&port->dev, "failed to send break: %d\n", result);
> 
> If you do this, it's now trivial for userspace to spam your logs if you
> have this type of hardware, so we should leave it at debug, and just fix
> the ability to return the error properly to userspace, which can then
> handle it correctly.

Assuming userspace has sufficient privileges to access the serial port.
But you're right, it's a problem.

However, it seems that most existing userspace fails to actually check
tcsendbreak for errors. :(

What if we keep the debug message as-is, and instead add a new message
that gets printed only once per device?  That should avoid the log spam
and allow people with old userspace to at least have some indication of
what's going on.  The message can be removed after wiring up the error
returns and giving userspace some time to handle these new errors.

Cheers,
-- 
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux