[PATCH 0/2] usb: serial: handle -ENODEV and -EPROTO quietly

[PATCH 0/2] usb: serial: handle -ENODEV and -EPROTO quietly

[Jeremiah Mahler]

If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
unplugged, a bunch of -ENODEV and -EPROTO errors will be produced in the
logs.  This patch set quiets these messages without changing the
original behavior.

This change is beneficial when using daemons such as slcand, which is
similar to pppd or slip, that cannot determine whether they should exit
until after the USB serial device is unplugged.  Producing these error
messages for a normal use case is not helpful.

Jeremiah Mahler (2):
  usb: serial: handle -EPROTO quietly in generic_read_bulk
  usb: serial: handle -ENODEV quietly in generic_submit_read_urb

 drivers/usb/serial/generic.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

-- 
2.1.3

[PATCH 1/2] usb: serial: handle -EPROTO quietly in generic_read_bulk

[Jeremiah Mahler]

If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
unplugged, a bunch of -EPROTO (71) error messages will be produced by
usb_serial_generic_read_bulk_callback() as it tries to resubmit the
request.

  usb_serial_generic_read_bulk_callback - nonzero urb status: -71

Keep the same functionality, resubmit after an -EPROTO error, but change
message log level to debug instead of error so they are handled quietly
by default.

Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>
---
 drivers/usb/serial/generic.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 1bd1922..98fe718 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -372,6 +372,10 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
 		dev_err(&port->dev, "%s - urb stopped: %d\n",
 							__func__, urb->status);
 		return;
+	case -EPROTO:
+		dev_dbg(&port->dev, "%s - urb resubmit: %d\n",
+							__func__, urb->status);
+		goto resubmit;
 	default:
 		dev_err(&port->dev, "%s - nonzero urb status: %d\n",
 							__func__, urb->status);
-- 
2.1.3

[PATCH 2/2] usb: serial: handle -ENODEV quietly in generic_submit_read_urb

[Jeremiah Mahler]

If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
unplugged, an -ENODEV (19) error will be produced after it gives up
trying to resubmit a read.

  usb_serial_generic_submit_read_urb - usb_submit_urb failed: -19

Add -ENODEV as one of the permanent errors along with -EPERM that
usb_serial_generic_submit_read_urb() handles quietly without an error.

Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>
---
 drivers/usb/serial/generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 98fe718..cca81c4 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -286,7 +286,7 @@ static int usb_serial_generic_submit_read_urb(struct usb_serial_port *port,
 
 	res = usb_submit_urb(port->read_urbs[index], mem_flags);
 	if (res) {
-		if (res != -EPERM) {
+		if (res != -EPERM && res != -ENODEV) {
 			dev_err(&port->dev,
 					"%s - usb_submit_urb failed: %d\n",
 					__func__, res);
-- 
2.1.3

Re: [PATCH 0/2] usb: serial: handle -ENODEV and -EPROTO quietly

[Johan Hovold]

On Thu, Dec 11, 2014 at 03:29:52PM -0800, Jeremiah Mahler wrote:
> If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
> unplugged, a bunch of -ENODEV and -EPROTO errors will be produced in the
> logs.  This patch set quiets these messages without changing the
> original behavior.

Don't unplug devices that are in use then. ;)

> This change is beneficial when using daemons such as slcand, which is
> similar to pppd or slip, that cannot determine whether they should exit
> until after the USB serial device is unplugged.  Producing these error
> messages for a normal use case is not helpful.

Your patches would hide these errors when they occur during normal use
(e.g. EPROTO).

Receiving an error message when unplugging an active device should not
surprise anyone. And at least you know where it came from (and it's
right there in the logs as well).

Johan

Re: [PATCH 0/2] usb: serial: handle -ENODEV and -EPROTO quietly

[Jeremiah Mahler]

Johan,

On Mon, Dec 15, 2014 at 11:23:21AM +0100, Johan Hovold wrote:
> On Thu, Dec 11, 2014 at 03:29:52PM -0800, Jeremiah Mahler wrote:
> > If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
> > unplugged, a bunch of -ENODEV and -EPROTO errors will be produced in the
> > logs.  This patch set quiets these messages without changing the
> > original behavior.
> 
> Don't unplug devices that are in use then. ;)
> 
I knew someone was going to say that :-)

> > This change is beneficial when using daemons such as slcand, which is
> > similar to pppd or slip, that cannot determine whether they should exit
> > until after the USB serial device is unplugged.  Producing these error
> > messages for a normal use case is not helpful.
> 
> Your patches would hide these errors when they occur during normal use
> (e.g. EPROTO).
> 
> Receiving an error message when unplugging an active device should not
> surprise anyone. And at least you know where it came from (and it's
> right there in the logs as well).
> 
> Johan

Hmm.  Yes, I can see why quieting -EPROTO would be bad because it would
hide protocol errors which we want to know about.

I need to re-think this patch.
Nack.

Thanks for the review,
-- 
- Jeremiah Mahler

Re: [PATCH 0/2] usb: serial: handle -ENODEV and -EPROTO quietly

[Greg Kroah-Hartman]

On Mon, Dec 15, 2014 at 04:53:05AM -0800, Jeremiah Mahler wrote:
> Johan,
> 
> On Mon, Dec 15, 2014 at 11:23:21AM +0100, Johan Hovold wrote:
> > On Thu, Dec 11, 2014 at 03:29:52PM -0800, Jeremiah Mahler wrote:
> > > If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
> > > unplugged, a bunch of -ENODEV and -EPROTO errors will be produced in the
> > > logs.  This patch set quiets these messages without changing the
> > > original behavior.
> > 
> > Don't unplug devices that are in use then. ;)
> > 
> I knew someone was going to say that :-)
> 
> > > This change is beneficial when using daemons such as slcand, which is
> > > similar to pppd or slip, that cannot determine whether they should exit
> > > until after the USB serial device is unplugged.  Producing these error
> > > messages for a normal use case is not helpful.
> > 
> > Your patches would hide these errors when they occur during normal use
> > (e.g. EPROTO).
> > 
> > Receiving an error message when unplugging an active device should not
> > surprise anyone. And at least you know where it came from (and it's
> > right there in the logs as well).
> > 
> > Johan
> 
> Hmm.  Yes, I can see why quieting -EPROTO would be bad because it would
> hide protocol errors which we want to know about.

Do you really want to "know about" them?  What can a user do with them?
Nothing, so just resubmit and you should be fine.

> I need to re-think this patch.
> Nack.

I like this patch, putting crud in the kernel log that no one can do
anything with for a "normal" operation like yanking a USB device out
while it is open should not happen.

thanks,

greg k-h

Re: [PATCH 0/2] usb: serial: handle -ENODEV and -EPROTO quietly

[Jeremiah Mahler]

Greg,

On Mon, Dec 15, 2014 at 08:38:01AM -0800, Greg Kroah-Hartman wrote:
> On Mon, Dec 15, 2014 at 04:53:05AM -0800, Jeremiah Mahler wrote:
> > Johan,
> > 
> > On Mon, Dec 15, 2014 at 11:23:21AM +0100, Johan Hovold wrote:
> > > On Thu, Dec 11, 2014 at 03:29:52PM -0800, Jeremiah Mahler wrote:
> > > > If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
> > > > unplugged, a bunch of -ENODEV and -EPROTO errors will be produced in the
> > > > logs.  This patch set quiets these messages without changing the
> > > > original behavior.
> > > 
> > > Don't unplug devices that are in use then. ;)
> > > 
> > I knew someone was going to say that :-)
> > 
> > > > This change is beneficial when using daemons such as slcand, which is
> > > > similar to pppd or slip, that cannot determine whether they should exit
> > > > until after the USB serial device is unplugged.  Producing these error
> > > > messages for a normal use case is not helpful.
> > > 
> > > Your patches would hide these errors when they occur during normal use
> > > (e.g. EPROTO).
> > > 
> > > Receiving an error message when unplugging an active device should not
> > > surprise anyone. And at least you know where it came from (and it's
> > > right there in the logs as well).
> > > 
> > > Johan
> > 
> > Hmm.  Yes, I can see why quieting -EPROTO would be bad because it would
> > hide protocol errors which we want to know about.
> 
> Do you really want to "know about" them?  What can a user do with them?
> Nothing, so just resubmit and you should be fine.
> 
> > I need to re-think this patch.
> > Nack.
> 
> I like this patch, putting crud in the kernel log that no one can do
> anything with for a "normal" operation like yanking a USB device out
> while it is open should not happen.
> 
> thanks,
> 
> greg k-h

Perhaps something at a lower level could return a more apt error number
such as -ENODEV.  Then there would be no conflict with -EPROTO.

I will look in to it again and re-submit the patches.

-- 
- Jeremiah Mahler

Re: [PATCH 0/2] usb: serial: handle -ENODEV and -EPROTO quietly

[Johan Hovold]

On Mon, Dec 15, 2014 at 08:38:01AM -0800, Greg Kroah-Hartman wrote:
> On Mon, Dec 15, 2014 at 04:53:05AM -0800, Jeremiah Mahler wrote:
> > Johan,
> > 
> > On Mon, Dec 15, 2014 at 11:23:21AM +0100, Johan Hovold wrote:
> > > On Thu, Dec 11, 2014 at 03:29:52PM -0800, Jeremiah Mahler wrote:
> > > > If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
> > > > unplugged, a bunch of -ENODEV and -EPROTO errors will be produced in the
> > > > logs.  This patch set quiets these messages without changing the
> > > > original behavior.
> > > 
> > > Don't unplug devices that are in use then. ;)
> > > 
> > I knew someone was going to say that :-)
> > 
> > > > This change is beneficial when using daemons such as slcand, which is
> > > > similar to pppd or slip, that cannot determine whether they should exit
> > > > until after the USB serial device is unplugged.  Producing these error
> > > > messages for a normal use case is not helpful.
> > > 
> > > Your patches would hide these errors when they occur during normal use
> > > (e.g. EPROTO).
> > > 
> > > Receiving an error message when unplugging an active device should not
> > > surprise anyone. And at least you know where it came from (and it's
> > > right there in the logs as well).
> > > 
> > > Johan
> > 
> > Hmm.  Yes, I can see why quieting -EPROTO would be bad because it would
> > hide protocol errors which we want to know about.
> 
> Do you really want to "know about" them?  What can a user do with them?
> Nothing, so just resubmit and you should be fine.

Knowing that a device is flakey (and should be replaced) might be of
some worth?

And wouldn't silencing such errors mean that we could be quietly
dropping data?

> > I need to re-think this patch.
> > Nack.
> 
> I like this patch, putting crud in the kernel log that no one can do
> anything with for a "normal" operation like yanking a USB device out
> while it is open should not happen.

The problem is that several errors may be returned from the
host-controller driver as a consequence of disconnect (before the hub
driver can process the disconnect). At least -EPROTO, -EILSEQ, -ETIME
are -EPIPE explicitly listed in Documentation/usb/error-codes.txt for
this, and of those, -EPROTO, -EILSEQ could also indicate hardware
problems.

I don't see how we can get around the trade-off between having a few
error messages in the log in the short window prior to a processed (and
also logged) disconnect, and not reporting potential hardware issues.

Thanks,
Johan

Re: [PATCH 0/2] usb: serial: handle -ENODEV and -EPROTO quietly

[Johan Hovold]

On Mon, Dec 15, 2014 at 11:10:37PM -0800, Jeremiah Mahler wrote:

> Perhaps something at a lower level could return a more apt error number
> such as -ENODEV.  Then there would be no conflict with -EPROTO.

I'm afraid it's not possible to differentiate between a disconnected and
malfunctioning device in that short window before the disconnect is
reported and processes by the hub driver.

> I will look in to it again and re-submit the patches.

Let's see what comes out of this discussion first.

Thanks,
Johan

Re: [PATCH 2/2] usb: serial: handle -ENODEV quietly in generic_submit_read_urb

[Johan Hovold]

On Thu, Dec 11, 2014 at 03:29:54PM -0800, Jeremiah Mahler wrote:
> If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
> unplugged, an -ENODEV (19) error will be produced after it gives up
> trying to resubmit a read.
> 
>   usb_serial_generic_submit_read_urb - usb_submit_urb failed: -19
> 
> Add -ENODEV as one of the permanent errors along with -EPERM that
> usb_serial_generic_submit_read_urb() handles quietly without an error.
> 
> Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>

I'll apply this one once v3.19-rc1 is out.

Thanks,
Johan

[PATCH v2 0/2] usb: serial: handle -ENODEV and -EPROTO quietly

[Jeremiah Mahler]

NOTE: These patches can wait until after the merge window.  I just
wanted this slightly improved version to be available when the window
does close.

If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
unplugged, a bunch of -ENODEV and -EPROTO errors will be produced in the
logs.  This patch set quiets these messages without changing the
original behavior.

This change is beneficial when using daemons such as slcand, which is
similar to pppd or slip, that cannot determine whether they should exit
until after the USB serial device is unplugged.  Producing these error
messages for a normal use case is not helpful.

Changes in v2:
  - Instead of handling -EPROTO specially, use dev_dbg instead of
    dev_err like other drivers do.

Jeremiah Mahler (2):
  usb: serial: handle -EPROTO quietly in generic_read_bulk
  usb: serial: handle -ENODEV quietly in generic_submit_read_urb

 drivers/usb/serial/generic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.1.3

[PATCH v2 1/2] usb: serial: handle -EPROTO quietly in generic_read_bulk

[Jeremiah Mahler]

If a USB serial device driver, which is built using the generic serial
driver, is unplugged while there is an active program using the device,
it will spam the logs with -EPROTO (71) messages as it attempts to
retry.

Most serial usb drivers (metro-usb, pl2303, mos7840, ...) only output
these messages for debugging.  The generic driver treats these as
errors.

Change the default output for the generic serial driver from error to
debug.

Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>
---
 drivers/usb/serial/generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 1bd1922..2d7207b 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -373,7 +373,7 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
 							__func__, urb->status);
 		return;
 	default:
-		dev_err(&port->dev, "%s - nonzero urb status: %d\n",
+		dev_dbg(&port->dev, "%s - nonzero urb status: %d\n",
 							__func__, urb->status);
 		goto resubmit;
 	}
-- 
2.1.3

[PATCH v2 2/2] usb: serial: handle -ENODEV quietly in generic_submit_read_urb

[Jeremiah Mahler]

If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
unplugged, an -ENODEV (19) error will be produced after it gives up
trying to resubmit a read.

  usb_serial_generic_submit_read_urb - usb_submit_urb failed: -19

Add -ENODEV as one of the permanent errors along with -EPERM that
usb_serial_generic_submit_read_urb() handles quietly without an error.

Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>
---
 drivers/usb/serial/generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 2d7207b..ccf1df7 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -286,7 +286,7 @@ static int usb_serial_generic_submit_read_urb(struct usb_serial_port *port,
 
 	res = usb_submit_urb(port->read_urbs[index], mem_flags);
 	if (res) {
-		if (res != -EPERM) {
+		if (res != -EPERM && res != -ENODEV) {
 			dev_err(&port->dev,
 					"%s - usb_submit_urb failed: %d\n",
 					__func__, res);
-- 
2.1.3

Re: [PATCH v2 1/2] usb: serial: handle -EPROTO quietly in generic_read_bulk

[Sergei Shtylyov]

Hello.

On 12/20/2014 12:11 PM, Jeremiah Mahler wrote:

> If a USB serial device driver, which is built using the generic serial
> driver, is unplugged while there is an active program using the device,

    Driver is unplugged? :-)

> it will spam the logs with -EPROTO (71) messages as it attempts to
> retry.

> Most serial usb drivers (metro-usb, pl2303, mos7840, ...) only output
> these messages for debugging.  The generic driver treats these as
> errors.

> Change the default output for the generic serial driver from error to
> debug.

> Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>

[...]

WBR, Sergei

Re: [PATCH v2 1/2] usb: serial: handle -EPROTO quietly in generic_read_bulk

[Jeremiah Mahler]

Sergei,

On Sat, Dec 20, 2014 at 03:32:42PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 12/20/2014 12:11 PM, Jeremiah Mahler wrote:
> 
> >If a USB serial device driver, which is built using the generic serial
> >driver, is unplugged while there is an active program using the device,
> 
>    Driver is unplugged? :-)
> 
Yes, that doesn't make sense.  Good catch, thanks :-)

> >it will spam the logs with -EPROTO (71) messages as it attempts to
> >retry.
> 
> >Most serial usb drivers (metro-usb, pl2303, mos7840, ...) only output
> >these messages for debugging.  The generic driver treats these as
> >errors.
> 
> >Change the default output for the generic serial driver from error to
> >debug.
> 
> >Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>
> 
> [...]
> 
> WBR, Sergei
> 

-- 
- Jeremiah Mahler

[PATCH v2b 1/2] usb: serial: handle -EPROTO quietly in generic_read_bulk

[Jeremiah Mahler]

If a USB serial device is unplugged while there is an active program
using the device it will spam the logs with -EPROTO (71) messages as it
attempts to retry.

Most serial usb drivers (metro-usb, pl2303, mos7840, ...) only output
these messages for debugging.  The generic driver treats these as
errors.

Change the default output for the generic serial driver from error to
debug.

Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>
---

Notes:
    Minor change just fixes the wording in the log message.

 drivers/usb/serial/generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 1bd1922..2d7207b 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -373,7 +373,7 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
 							__func__, urb->status);
 		return;
 	default:
-		dev_err(&port->dev, "%s - nonzero urb status: %d\n",
+		dev_dbg(&port->dev, "%s - nonzero urb status: %d\n",
 							__func__, urb->status);
 		goto resubmit;
 	}
-- 
2.1.3

[RESEND PATCH v2 0/2] usb: serial: handle -ENODEV and -EPROTO quietly

[Jeremiah Mahler]

(Jan 15) Just a resend of v2 [1].

  [1]: https://lkml.org/lkml/2014/12/20/16

If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
unplugged, a bunch of -ENODEV and -EPROTO errors will be produced in the
logs.  This patch set quiets these messages without changing the
original behavior.

This change is beneficial when using daemons such as slcand, which is
similar to pppd or slip, that cannot determine whether they should exit
until after the USB serial device is unplugged.  Producing these error
messages for a normal use case is not helpful.

Jeremiah Mahler (2):
  usb: serial: handle -EPROTO quietly in generic_read_bulk
  usb: serial: handle -ENODEV quietly in generic_submit_read_urb

 drivers/usb/serial/generic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.1.4

[RESEND PATCH v2 1/2] usb: serial: handle -EPROTO quietly in generic_read_bulk

[Jeremiah Mahler]

If a USB serial device is unplugged while there is an active program
using the device it will spam the logs with -EPROTO (71) messages as it
attempts to retry.

Most serial usb drivers (metro-usb, pl2303, mos7840, ...) only output
these messages for debugging.  The generic driver treats these as
errors.

Change the default output for the generic serial driver from error to
debug.

Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>
---
 drivers/usb/serial/generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 1bd1922..2d7207b 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -373,7 +373,7 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
 							__func__, urb->status);
 		return;
 	default:
-		dev_err(&port->dev, "%s - nonzero urb status: %d\n",
+		dev_dbg(&port->dev, "%s - nonzero urb status: %d\n",
 							__func__, urb->status);
 		goto resubmit;
 	}
-- 
2.1.4

[RESEND PATCH v2 2/2] usb: serial: handle -ENODEV quietly in generic_submit_read_urb

[Jeremiah Mahler]

If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
unplugged, an -ENODEV (19) error will be produced after it gives up
trying to resubmit a read.

  usb_serial_generic_submit_read_urb - usb_submit_urb failed: -19

Add -ENODEV as one of the permanent errors along with -EPERM that
usb_serial_generic_submit_read_urb() handles quietly without an error.

Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>
---
 drivers/usb/serial/generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 2d7207b..ccf1df7 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -286,7 +286,7 @@ static int usb_serial_generic_submit_read_urb(struct usb_serial_port *port,
 
 	res = usb_submit_urb(port->read_urbs[index], mem_flags);
 	if (res) {
-		if (res != -EPERM) {
+		if (res != -EPERM && res != -ENODEV) {
 			dev_err(&port->dev,
 					"%s - usb_submit_urb failed: %d\n",
 					__func__, res);
-- 
2.1.4

Re: [RESEND PATCH v2 1/2] usb: serial: handle -EPROTO quietly in generic_read_bulk

[Johan Hovold]

On Sat, Jan 10, 2015 at 04:44:32PM -0800, Jeremiah Mahler wrote:
> If a USB serial device is unplugged while there is an active program
> using the device it will spam the logs with -EPROTO (71) messages as it
> attempts to retry.

Can you change this to "might spam", as which error message, and if it is
at all printed, depends on host controller and what (hub) devices are
used.

> Most serial usb drivers (metro-usb, pl2303, mos7840, ...) only output
> these messages for debugging.  The generic driver treats these as
> errors.
> 
> Change the default output for the generic serial driver from error to
> debug.

Please also correct the commit summary (Subject) so that it matches what
this revised patch now does (you silence all non-critical error
messages).

Fix this up and I'll take both patches.

Thanks,
Johan

Re: [RESEND PATCH v2 1/2] usb: serial: handle -EPROTO quietly in generic_read_bulk

[Jeremiah Mahler]

Johan,

On Sun, Jan 11, 2015 at 12:36:18PM +0100, Johan Hovold wrote:
> On Sat, Jan 10, 2015 at 04:44:32PM -0800, Jeremiah Mahler wrote:
> > If a USB serial device is unplugged while there is an active program
> > using the device it will spam the logs with -EPROTO (71) messages as it
> > attempts to retry.
> 
> Can you change this to "might spam", as which error message, and if it is
> at all printed, depends on host controller and what (hub) devices are
> used.
> 
Ah, yes, good point.

> > Most serial usb drivers (metro-usb, pl2303, mos7840, ...) only output
> > these messages for debugging.  The generic driver treats these as
> > errors.
> > 
> > Change the default output for the generic serial driver from error to
> > debug.
> 
> Please also correct the commit summary (Subject) so that it matches what
> this revised patch now does (you silence all non-critical error
> messages).
> 
True, it is more than just -EPROTO now.

> Fix this up and I'll take both patches.
> 
> Thanks,
> Johan

Thanks for the suggestions.

v3 coming up.

-- 
- Jeremiah Mahler

[PATCH v3 0/2] usb: serial: silence non-critical unplug read errors

[Jeremiah Mahler]

If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
unplugged, a bunch of -ENODEV and -EPROTO errors may be produced in the
logs.  This patch set quiets these messages without changing the
original behavior.

This change is beneficial when using daemons such as slcand, which is
similar to pppd or slip, that cannot determine whether they should exit
until after the USB serial device is unplugged.  Producing these error
messages for a normal use case is not helpful.

Changes in v3:

  - change "will spam" to "might spam" since it is dependent
	upon what host controller and devices are used.

  - fix subject: it is silencing all errors, not just one as in
    previous versions.

Changes in v2:

  - Instead of handling -EPROTO specially, use dev_dbg instead of
    dev_err like other drivers do.

Jeremiah Mahler (2):
  usb: serial: silence all non-critical read errors
  usb: serial: handle -ENODEV quietly in generic_submit_read_urb

 drivers/usb/serial/generic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.1.4

[PATCH v3 1/2] usb: serial: silence all non-critical read errors

[Jeremiah Mahler]

If a USB serial device is unplugged while there is an active program
using the device it may spam the logs with -EPROTO (71) messages as it
attempts to retry.

Most serial usb drivers (metro-usb, pl2303, mos7840, ...) only output
these messages for debugging.  The generic driver treats these as
errors.

Change the default output for the generic serial driver from error to
debug to silence these non-critical errors.

Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>
---
 drivers/usb/serial/generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 1bd1922..2d7207b 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -373,7 +373,7 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
 							__func__, urb->status);
 		return;
 	default:
-		dev_err(&port->dev, "%s - nonzero urb status: %d\n",
+		dev_dbg(&port->dev, "%s - nonzero urb status: %d\n",
 							__func__, urb->status);
 		goto resubmit;
 	}
-- 
2.1.4

[PATCH v3 2/2] usb: serial: handle -ENODEV quietly in generic_submit_read_urb

[Jeremiah Mahler]

If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
unplugged, an -ENODEV (19) error will be produced after it gives up
trying to resubmit a read.

  usb_serial_generic_submit_read_urb - usb_submit_urb failed: -19

Add -ENODEV as one of the permanent errors along with -EPERM that
usb_serial_generic_submit_read_urb() handles quietly without an error.

Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>
---
 drivers/usb/serial/generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 2d7207b..ccf1df7 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -286,7 +286,7 @@ static int usb_serial_generic_submit_read_urb(struct usb_serial_port *port,
 
 	res = usb_submit_urb(port->read_urbs[index], mem_flags);
 	if (res) {
-		if (res != -EPERM) {
+		if (res != -EPERM && res != -ENODEV) {
 			dev_err(&port->dev,
 					"%s - usb_submit_urb failed: %d\n",
 					__func__, res);
-- 
2.1.4

Re: [PATCH v3 0/2] usb: serial: silence non-critical unplug read errors

[Johan Hovold]

On Sun, Jan 11, 2015 at 05:42:05AM -0800, Jeremiah Mahler wrote:
> If a USB serial device (e.g. /dev/ttyUSB0) with an active program is
> unplugged, a bunch of -ENODEV and -EPROTO errors may be produced in the
> logs.  This patch set quiets these messages without changing the
> original behavior.

Both patches applied, thanks.

Johan