Re: [PATCH v2 2/2] usb: gadget serial: Honour termios CLOCAL on disconnect

2014-11-03 Thread Kyösti Mälkki

On 11/03/2014 05:33 PM, Peter Hurley wrote:

Hi Kyösti,

On 11/03/2014 10:18 AM, Kyösti Mälkki wrote:

There are applications where it is desirable to not hangup ttyGS* when
USB disconnect is detected. USB host side of communication may
power-cycle periodically or there may be the actual need to physically
disconnect and reconnect USB cable temporarily.

USB disconnects on serial gadget are comparable to loss of Carrier Detect
of conventional UARTs. With the change, if ttyGS* has termios CLOCAL flag
set, disconnect on USB does not hangup the TTY.

Signed-off-by: Kyösti Mälkki 
---
  drivers/usb/gadget/function/u_serial.c | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/u_serial.c 
b/drivers/usb/gadget/function/u_serial.c
index 491082a..dabc165 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1253,9 +1253,16 @@ void gserial_disconnect(struct gserial *gser)
port->port_usb = NULL;
gser->ioport = NULL;
if (port->port.count > 0 || port->openclose) {
+   struct tty_struct *tty;
+
wake_up_interruptible(&port->drain_wait);
-   if (port->port.tty)
-   tty_hangup(port->port.tty);
+   tty = port->port.tty;
+   if (tty) {
+   if (tty->termios.c_cflag & CLOCAL)
+   stop_tty(tty);
+   else
+   tty_hangup(tty);


It seems you missed my earlier email: what happens if you leave
out the stop_tty() call here?



Hi Peter

I did not miss it. My intro to the patch series says I am not familiar 
with tty infra, so I did not realize the question was directed to me.
Also my setup with serial gadget runs kernel 3.12.6 so I cannot really 
tell or test if tty changes since that would make my patch invalid or 
unnecessary.


In the previous change for gserial_disconnect(), where the tty_hangup() 
call was added, commit message described it fixes an endless loop on tty 
device read and select. Would we be back at that situation without call 
to stop_tty()?



I ask because the tty is still restartable from userspace after you
stop_tty() here. So if your goal is to prevent write() from
happening, this won't work.


There was no goal to prevent write() on the tty device, should there be? 
Or should it silently just drop any characters while in the disconnected 
state instead of buffering (some of) them?


Thanks,
KM



Regards,
Peter Hurley


+   }
}
spin_unlock_irqrestore(&port->port_lock, flags);






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


[PATCH v2 1/2] usb: gadget dbgp: Fix endpoint config after USB disconnect

2014-11-03 Thread Kyösti Mälkki
SET_FEATURE request with DEBUG_MODE only worked the first time after module
initialisation. Per the USB 2.0 debug device specification, said request
is to be treated as if it were a SET_CONFIGURATION request, i.e. endpoint
must be re-configured.

As configure_endpoints() may now get called multiple times, move it outside
__init and move serial_alloc_tty() call into __init.

Code has assumption that endpoint mapping remains unchanged with consecutive
calls of configure_endpoints().

Signed-off-by: Kyösti Mälkki 
---
 drivers/usb/gadget/legacy/dbgp.c | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/gadget/legacy/dbgp.c b/drivers/usb/gadget/legacy/dbgp.c
index 1b07513..633683a 100644
--- a/drivers/usb/gadget/legacy/dbgp.c
+++ b/drivers/usb/gadget/legacy/dbgp.c
@@ -237,7 +237,7 @@ static void dbgp_unbind(struct usb_gadget *gadget)
 static unsigned char tty_line;
 #endif
 
-static int __init dbgp_configure_endpoints(struct usb_gadget *gadget)
+static int dbgp_configure_endpoints(struct usb_gadget *gadget)
 {
int stp;
 
@@ -273,19 +273,10 @@ static int __init dbgp_configure_endpoints(struct 
usb_gadget *gadget)
 
dbgp.serial->in->desc = &i_desc;
dbgp.serial->out->desc = &o_desc;
-
-   if (gserial_alloc_line(&tty_line)) {
-   stp = 3;
-   goto fail_3;
-   }
+#endif
 
return 0;
 
-fail_3:
-   dbgp.o_ep->driver_data = NULL;
-#else
-   return 0;
-#endif
 fail_2:
dbgp.i_ep->driver_data = NULL;
 fail_1:
@@ -324,10 +315,17 @@ static int __init dbgp_bind(struct usb_gadget *gadget,
err = -ENOMEM;
goto fail;
}
+
+   if (gserial_alloc_line(&tty_line)) {
+   stp = 4;
+   err = -ENODEV;
+   goto fail;
+   }
 #endif
+
err = dbgp_configure_endpoints(gadget);
if (err < 0) {
-   stp = 4;
+   stp = 5;
goto fail;
}
 
@@ -383,6 +381,10 @@ static int dbgp_setup(struct usb_gadget *gadget,
 #ifdef CONFIG_USB_G_DBGP_PRINTK
err = dbgp_enable_ep();
 #else
+   err = dbgp_configure_endpoints(gadget);
+   if (err < 0) {
+   goto fail;
+   }
err = gserial_connect(dbgp.serial, tty_line);
 #endif
if (err < 0)
-- 
1.8.1.1

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


usb: gadget: Fixes to receive from USB EHCI Debug

2014-11-03 Thread Kyösti Mälkki
USB gadget driver dbgp can be used as an EHCI debug dongle in replacement
for product like Net20DC. With it one can receive early kernel messages
from remote targets over USB. See parameter earlyprintk=dbgp for target
kernel requirements and configuration.
 
The two patches are required for dbgp gadget driver to work. Longterm
plan: I think G_DBGP_PRINTK is an odd hack and could be removed.
Exposing the debug descriptor can be implemented in cdc_acm so that we
could seemlessly reconfigure endpoint from the 8 byte payload limitation
EHCI Debug Port has, to 512 bytes, once ehci-hcd with DMA is available.


Handling of the termios CLOCAL flag calls for some discussion and I am
not that familiar with the tty infra u_serial uses. See the previous
change around gserial_disconnect():
  http://permalink.gmane.org/gmane.linux.usb.general/4260

Regards,
  Kyösti
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 2/2] usb: gadget serial: Honour termios CLOCAL on disconnect

2014-11-03 Thread Kyösti Mälkki
There are applications where it is desirable to not hangup ttyGS* when
USB disconnect is detected. USB host side of communication may
power-cycle periodically or there may be the actual need to physically
disconnect and reconnect USB cable temporarily.

USB disconnects on serial gadget are comparable to loss of Carrier Detect
of conventional UARTs. With the change, if ttyGS* has termios CLOCAL flag
set, disconnect on USB does not hangup the TTY.

Signed-off-by: Kyösti Mälkki 
---
 drivers/usb/gadget/function/u_serial.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/u_serial.c 
b/drivers/usb/gadget/function/u_serial.c
index 491082a..dabc165 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1253,9 +1253,16 @@ void gserial_disconnect(struct gserial *gser)
port->port_usb = NULL;
gser->ioport = NULL;
if (port->port.count > 0 || port->openclose) {
+   struct tty_struct *tty;
+
wake_up_interruptible(&port->drain_wait);
-   if (port->port.tty)
-   tty_hangup(port->port.tty);
+   tty = port->port.tty;
+   if (tty) {
+   if (tty->termios.c_cflag & CLOCAL)
+   stop_tty(tty);
+   else
+   tty_hangup(tty);
+   }
}
spin_unlock_irqrestore(&port->port_lock, flags);
 
-- 
1.8.1.1

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


Re: [PATCH 1/2] usb: gadget dbgp: Fix endpoint config after USB disconnect

2014-10-27 Thread Kyösti Mälkki

On 10/27/2014 05:22 PM, Felipe Balbi wrote:

On Mon, Oct 27, 2014 at 05:15:16PM +0200, Kyösti Mälkki wrote:

On 10/27/2014 03:35 PM, Felipe Balbi wrote:

Hi,

On Sun, Oct 26, 2014 at 08:01:29PM +0200, Kyösti Mälkki wrote:

SET_FEATURE request for DEBUG_MODE only worked the first time after module
initialisation. On USB host reset or cable disconnect gserial_disconnect()
clears the associations dbgp.serial->in->desc and dbgp_serial->out->desc.

Per the USB 2.0 debug device specification, SET_FEATURE with DEBUG_MODE
is to be treated as if it were a SET_CONFIGURATION request. Redo endpoint
configuration on this request.

Code has assumption that endpoint mapping remains unchanged from what was
previously returned as a GET_DESCRIPTOR DT_DEBUG response.

Signed-off-by: Kyösti Mälkki 


you're doing many things in one patch and I can't accept this.


---
  drivers/usb/gadget/legacy/dbgp.c | 39 ++-
  1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/gadget/legacy/dbgp.c b/drivers/usb/gadget/legacy/dbgp.c
index 1b07513..afb49ef 100644
--- a/drivers/usb/gadget/legacy/dbgp.c
+++ b/drivers/usb/gadget/legacy/dbgp.c
@@ -214,6 +214,7 @@ static void dbgp_disconnect(struct usb_gadget *gadget)
  #ifdef CONFIG_USB_G_DBGP_PRINTK
dbgp_disable_ep();
  #else
+   dev_dbg(&dbgp.gadget->dev, "disconnected\n");


first you add this debugging message which can definitely wait for
v3.19, not a fix.


gserial_disconnect(dbgp.serial);
  #endif
  }
@@ -237,7 +238,7 @@ static void dbgp_unbind(struct usb_gadget *gadget)
  static unsigned char tty_line;
  #endif

-static int __init dbgp_configure_endpoints(struct usb_gadget *gadget)
+static int dbgp_configure_endpoints(struct usb_gadget *gadget)


then you remove __init here without ever mentioning why. Separate patch.



This is the handler for an equivalent of a SET_CONFIGURATION request the
gadget will receive after every bus reset or connect. Change is relevant
once we actually do call the handler with the reception of said request,
instead of configuring the endpoints only once at module init.


@@ -273,19 +274,10 @@ static int __init dbgp_configure_endpoints(struct 
usb_gadget *gadget)

dbgp.serial->in->desc = &i_desc;
dbgp.serial->out->desc = &o_desc;
-
-   if (gserial_alloc_line(&tty_line)) {
-   stp = 3;
-   goto fail_3;
-   }


why are you removing this ?


To allocate the TTY only once. Change is relevant once
dbgp_configure_endpoints() is called more than once.


+#endif

return 0;

-fail_3:
-   dbgp.o_ep->driver_data = NULL;
-#else
-   return 0;
-#endif
  fail_2:
dbgp.i_ep->driver_data = NULL;
  fail_1:
@@ -324,10 +316,17 @@ static int __init dbgp_bind(struct usb_gadget *gadget,
err = -ENOMEM;
goto fail;
}
+
+   if (gserial_alloc_line(&tty_line)) {
+   stp = 4;
+   err = -ENODEV;
+   goto fail;
+   }


why do you need this here ?


  #endif
+
err = dbgp_configure_endpoints(gadget);
if (err < 0) {
-   stp = 4;
+   stp = 5;
goto fail;
}

@@ -379,12 +378,26 @@ static int dbgp_setup(struct usb_gadget *gadget,
err = 0;
} else if (request == USB_REQ_SET_FEATURE &&
   value == USB_DEVICE_DEBUG_MODE) {
-   dev_dbg(&dbgp.gadget->dev, "setup: feat debug\n");
  #ifdef CONFIG_USB_G_DBGP_PRINTK
err = dbgp_enable_ep();
  #else
+   if (!(dbgp.serial->in && dbgp.serial->in->desc &&
+   dbgp.serial->out && dbgp.serial->out->desc)) {
+   dev_dbg(&dbgp.gadget->dev, "needs reconfiguration\n");
+   err = dbgp_configure_endpoints(gadget);
+   if (err < 0) {
+   goto fail;
+   }
+   }


now this is the only thing mentioned in your commit log and this is only
thing that should be in $subject.


Giving this another look. Endpoint data toggle / PID parity must be 
reset on SET_CONFIGURE request, right? So we should call the handler 
unconditionally instead?





+   if (dbgp.serial->in && dbgp.serial->in->desc &&
+   dbgp.serial->out && dbgp.serial->out->desc) {
+   dev_dbg(&dbgp.gadget->dev, "epIn %02x, epOut %02x\n",
+   dbgp.serial->in->desc->bEndpointAddress,
+   dbgp.serial->out->desc->bEndpointAddress);
+   }


why do you need this entire branch here just for a debugging message ?
Why didn't you add this debugging message

Re: [PATCH 2/2] usb: gadget serial: Honour termios CLOCAL on disconnect

2014-10-27 Thread Kyösti Mälkki

On 10/27/2014 03:38 PM, Felipe Balbi wrote:

Hi,

On Sun, Oct 26, 2014 at 08:01:30PM +0200, Kyösti Mälkki wrote:

There are applications where it is desirable to not hangup ttyGS* when
USB disconnect is detected. USB host side of communication may
power-cycle periodically or there may be the actual need to physically
disconnect and reconnect USB cable temporarily.

USB disconnects on serial gadget are comparable to loss of Carrier Detect
of conventional UARTs. With the change, if ttyGS* has termios CLOCAL flag
set, disconnect on USB does not hangup the TTY.

Signed-off-by: Kyösti Mälkki 
---
  drivers/usb/gadget/function/u_serial.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/u_serial.c 
b/drivers/usb/gadget/function/u_serial.c
index 491082a..e68ffd7 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1254,8 +1254,13 @@ void gserial_disconnect(struct gserial *gser)
gser->ioport = NULL;
if (port->port.count > 0 || port->openclose) {
wake_up_interruptible(&port->drain_wait);
-   if (port->port.tty)
-   tty_hangup(port->port.tty);
+   struct tty_struct *tty = port->port.tty;


declare above as Sergei said.


+   if (tty) {


is there any situation where tty would be NULL here ?


+   if (tty->termios.c_cflag & CLOCAL)
+   stop_tty(tty);
+   else
+   tty_hangup(tty);


this I'll defer to Greg who also maintains tty.



My main concern is if someone runs getty on ttyACM without explicitly 
clearing CLOCAL. With the patch here login session would no longer get 
SIGHUP if cable is disconnected. And it is not possible to tell if it is 
the same cable that is plugged back in.


KM




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


Re: [PATCH 1/2] usb: gadget dbgp: Fix endpoint config after USB disconnect

2014-10-27 Thread Kyösti Mälkki

On 10/27/2014 03:35 PM, Felipe Balbi wrote:

Hi,

On Sun, Oct 26, 2014 at 08:01:29PM +0200, Kyösti Mälkki wrote:

SET_FEATURE request for DEBUG_MODE only worked the first time after module
initialisation. On USB host reset or cable disconnect gserial_disconnect()
clears the associations dbgp.serial->in->desc and dbgp_serial->out->desc.

Per the USB 2.0 debug device specification, SET_FEATURE with DEBUG_MODE
is to be treated as if it were a SET_CONFIGURATION request. Redo endpoint
configuration on this request.

Code has assumption that endpoint mapping remains unchanged from what was
previously returned as a GET_DESCRIPTOR DT_DEBUG response.

Signed-off-by: Kyösti Mälkki 


you're doing many things in one patch and I can't accept this.


---
  drivers/usb/gadget/legacy/dbgp.c | 39 ++-
  1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/gadget/legacy/dbgp.c b/drivers/usb/gadget/legacy/dbgp.c
index 1b07513..afb49ef 100644
--- a/drivers/usb/gadget/legacy/dbgp.c
+++ b/drivers/usb/gadget/legacy/dbgp.c
@@ -214,6 +214,7 @@ static void dbgp_disconnect(struct usb_gadget *gadget)
  #ifdef CONFIG_USB_G_DBGP_PRINTK
dbgp_disable_ep();
  #else
+   dev_dbg(&dbgp.gadget->dev, "disconnected\n");


first you add this debugging message which can definitely wait for
v3.19, not a fix.


gserial_disconnect(dbgp.serial);
  #endif
  }
@@ -237,7 +238,7 @@ static void dbgp_unbind(struct usb_gadget *gadget)
  static unsigned char tty_line;
  #endif

-static int __init dbgp_configure_endpoints(struct usb_gadget *gadget)
+static int dbgp_configure_endpoints(struct usb_gadget *gadget)


then you remove __init here without ever mentioning why. Separate patch.



This is the handler for an equivalent of a SET_CONFIGURATION request the 
gadget will receive after every bus reset or connect. Change is relevant 
once we actually do call the handler with the reception of said request, 
instead of configuring the endpoints only once at module init.



@@ -273,19 +274,10 @@ static int __init dbgp_configure_endpoints(struct 
usb_gadget *gadget)

dbgp.serial->in->desc = &i_desc;
dbgp.serial->out->desc = &o_desc;
-
-   if (gserial_alloc_line(&tty_line)) {
-   stp = 3;
-   goto fail_3;
-   }


why are you removing this ?


To allocate the TTY only once. Change is relevant once 
dbgp_configure_endpoints() is called more than once.



+#endif

return 0;

-fail_3:
-   dbgp.o_ep->driver_data = NULL;
-#else
-   return 0;
-#endif
  fail_2:
dbgp.i_ep->driver_data = NULL;
  fail_1:
@@ -324,10 +316,17 @@ static int __init dbgp_bind(struct usb_gadget *gadget,
err = -ENOMEM;
goto fail;
}
+
+   if (gserial_alloc_line(&tty_line)) {
+   stp = 4;
+   err = -ENODEV;
+   goto fail;
+   }


why do you need this here ?


  #endif
+
err = dbgp_configure_endpoints(gadget);
if (err < 0) {
-   stp = 4;
+   stp = 5;
goto fail;
}

@@ -379,12 +378,26 @@ static int dbgp_setup(struct usb_gadget *gadget,
err = 0;
} else if (request == USB_REQ_SET_FEATURE &&
   value == USB_DEVICE_DEBUG_MODE) {
-   dev_dbg(&dbgp.gadget->dev, "setup: feat debug\n");
  #ifdef CONFIG_USB_G_DBGP_PRINTK
err = dbgp_enable_ep();
  #else
+   if (!(dbgp.serial->in && dbgp.serial->in->desc &&
+   dbgp.serial->out && dbgp.serial->out->desc)) {
+   dev_dbg(&dbgp.gadget->dev, "needs reconfiguration\n");
+   err = dbgp_configure_endpoints(gadget);
+   if (err < 0) {
+   goto fail;
+   }
+   }


now this is the only thing mentioned in your commit log and this is only
thing that should be in $subject.


+   if (dbgp.serial->in && dbgp.serial->in->desc &&
+   dbgp.serial->out && dbgp.serial->out->desc) {
+   dev_dbg(&dbgp.gadget->dev, "epIn %02x, epOut %02x\n",
+   dbgp.serial->in->desc->bEndpointAddress,
+   dbgp.serial->out->desc->bEndpointAddress);
+   }


why do you need this entire branch here just for a debugging message ?
Why didn't you add this debugging message right after
dbgp_configure_endpoints() above ?


err = gserial_connect(dbgp.serial, tty_line);
  #endif
+   dev_dbg(&dbgp.gadget->dev, "setup: feat debug (%d)\n", err);


another debugging message which an wait until v3.19.



Re: [PATCH 2/2] usb: gadget serial: Honour termios CLOCAL on disconnect

2014-10-27 Thread Kyösti Mälkki

On 10/27/2014 02:31 PM, Sergei Shtylyov wrote:

Hello.

On 10/26/2014 9:01 PM, Kyösti Mälkki wrote:


There are applications where it is desirable to not hangup ttyGS* when
USB disconnect is detected. USB host side of communication may
power-cycle periodically or there may be the actual need to physically
disconnect and reconnect USB cable temporarily.



USB disconnects on serial gadget are comparable to loss of Carrier Detect
of conventional UARTs. With the change, if ttyGS* has termios CLOCAL flag
set, disconnect on USB does not hangup the TTY.



Signed-off-by: Kyösti Mälkki 
---
  drivers/usb/gadget/function/u_serial.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)



diff --git a/drivers/usb/gadget/function/u_serial.c
b/drivers/usb/gadget/function/u_serial.c
index 491082a..e68ffd7 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1254,8 +1254,13 @@ void gserial_disconnect(struct gserial *gser)
  gser->ioport = NULL;
  if (port->port.count > 0 || port->openclose) {
  wake_up_interruptible(&port->drain_wait);
-if (port->port.tty)
-tty_hangup(port->port.tty);
+struct tty_struct *tty = port->port.tty;


Don't declare variables amidst the code. And please add empty line
after declaration.



Sloppy rebase there from my part :/
I'll update once there is advice on the CLOCAL matter itself.


+if (tty) {
+if (tty->termios.c_cflag & CLOCAL)
+stop_tty(tty);
+else
+tty_hangup(tty);
+}
  }


WBR, Sergei




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


usb: gadget: Fixes to receive from USB EHCI Debug

2014-10-26 Thread Kyösti Mälkki
USB gadget driver dbgp can be used as an EHCI debug dongle in replacement
for product like Net20DC. With it one can receive early kernel messages
from remote targets over USB. See parameter earlyprintk=dbgp for target
kernel requirements and configuration.
 
The two patches are required for dbgp gadget driver to work. Longterm
plan: I think G_DBGP_PRINTK is an odd hack and could be removed.
Exposing the debug descriptor can be implemented in cdc_acm so that we
could seemlessly reconfigure endpoint from the 8 byte payload limitation
EHCI Debug Port has, to 512 bytes, once ehci-hcd with DMA is available.


Handling of the termios CLOCAL flag calls for some discussion and I am
not that familiar with the tty infra u_serial uses. See the previous
change around gserial_disconnect():
  http://permalink.gmane.org/gmane.linux.usb.general/4260

Regards,
  Kyösti

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


[PATCH 1/2] usb: gadget dbgp: Fix endpoint config after USB disconnect

2014-10-26 Thread Kyösti Mälkki
SET_FEATURE request for DEBUG_MODE only worked the first time after module
initialisation. On USB host reset or cable disconnect gserial_disconnect()
clears the associations dbgp.serial->in->desc and dbgp_serial->out->desc.

Per the USB 2.0 debug device specification, SET_FEATURE with DEBUG_MODE
is to be treated as if it were a SET_CONFIGURATION request. Redo endpoint
configuration on this request.

Code has assumption that endpoint mapping remains unchanged from what was
previously returned as a GET_DESCRIPTOR DT_DEBUG response.

Signed-off-by: Kyösti Mälkki 
---
 drivers/usb/gadget/legacy/dbgp.c | 39 ++-
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/gadget/legacy/dbgp.c b/drivers/usb/gadget/legacy/dbgp.c
index 1b07513..afb49ef 100644
--- a/drivers/usb/gadget/legacy/dbgp.c
+++ b/drivers/usb/gadget/legacy/dbgp.c
@@ -214,6 +214,7 @@ static void dbgp_disconnect(struct usb_gadget *gadget)
 #ifdef CONFIG_USB_G_DBGP_PRINTK
dbgp_disable_ep();
 #else
+   dev_dbg(&dbgp.gadget->dev, "disconnected\n");
gserial_disconnect(dbgp.serial);
 #endif
 }
@@ -237,7 +238,7 @@ static void dbgp_unbind(struct usb_gadget *gadget)
 static unsigned char tty_line;
 #endif
 
-static int __init dbgp_configure_endpoints(struct usb_gadget *gadget)
+static int dbgp_configure_endpoints(struct usb_gadget *gadget)
 {
int stp;
 
@@ -273,19 +274,10 @@ static int __init dbgp_configure_endpoints(struct 
usb_gadget *gadget)
 
dbgp.serial->in->desc = &i_desc;
dbgp.serial->out->desc = &o_desc;
-
-   if (gserial_alloc_line(&tty_line)) {
-   stp = 3;
-   goto fail_3;
-   }
+#endif
 
return 0;
 
-fail_3:
-   dbgp.o_ep->driver_data = NULL;
-#else
-   return 0;
-#endif
 fail_2:
dbgp.i_ep->driver_data = NULL;
 fail_1:
@@ -324,10 +316,17 @@ static int __init dbgp_bind(struct usb_gadget *gadget,
err = -ENOMEM;
goto fail;
}
+
+   if (gserial_alloc_line(&tty_line)) {
+   stp = 4;
+   err = -ENODEV;
+   goto fail;
+   }
 #endif
+
err = dbgp_configure_endpoints(gadget);
if (err < 0) {
-   stp = 4;
+   stp = 5;
goto fail;
}
 
@@ -379,12 +378,26 @@ static int dbgp_setup(struct usb_gadget *gadget,
err = 0;
} else if (request == USB_REQ_SET_FEATURE &&
   value == USB_DEVICE_DEBUG_MODE) {
-   dev_dbg(&dbgp.gadget->dev, "setup: feat debug\n");
 #ifdef CONFIG_USB_G_DBGP_PRINTK
err = dbgp_enable_ep();
 #else
+   if (!(dbgp.serial->in && dbgp.serial->in->desc &&
+   dbgp.serial->out && dbgp.serial->out->desc)) {
+   dev_dbg(&dbgp.gadget->dev, "needs reconfiguration\n");
+   err = dbgp_configure_endpoints(gadget);
+   if (err < 0) {
+   goto fail;
+   }
+   }
+   if (dbgp.serial->in && dbgp.serial->in->desc &&
+   dbgp.serial->out && dbgp.serial->out->desc) {
+   dev_dbg(&dbgp.gadget->dev, "epIn %02x, epOut %02x\n",
+   dbgp.serial->in->desc->bEndpointAddress,
+   dbgp.serial->out->desc->bEndpointAddress);
+   }
err = gserial_connect(dbgp.serial, tty_line);
 #endif
+   dev_dbg(&dbgp.gadget->dev, "setup: feat debug (%d)\n", err);
if (err < 0)
goto fail;
} else
-- 
1.8.1.1

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


[PATCH 2/2] usb: gadget serial: Honour termios CLOCAL on disconnect

2014-10-26 Thread Kyösti Mälkki
There are applications where it is desirable to not hangup ttyGS* when
USB disconnect is detected. USB host side of communication may
power-cycle periodically or there may be the actual need to physically
disconnect and reconnect USB cable temporarily.

USB disconnects on serial gadget are comparable to loss of Carrier Detect
of conventional UARTs. With the change, if ttyGS* has termios CLOCAL flag
set, disconnect on USB does not hangup the TTY.

Signed-off-by: Kyösti Mälkki 
---
 drivers/usb/gadget/function/u_serial.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/u_serial.c 
b/drivers/usb/gadget/function/u_serial.c
index 491082a..e68ffd7 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1254,8 +1254,13 @@ void gserial_disconnect(struct gserial *gser)
gser->ioport = NULL;
if (port->port.count > 0 || port->openclose) {
wake_up_interruptible(&port->drain_wait);
-   if (port->port.tty)
-   tty_hangup(port->port.tty);
+   struct tty_struct *tty = port->port.tty;
+   if (tty) {
+   if (tty->termios.c_cflag & CLOCAL)
+   stop_tty(tty);
+   else
+   tty_hangup(tty);
+   }
}
spin_unlock_irqrestore(&port->port_lock, flags);
 
-- 
1.8.1.1

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


Re: [EHCI Debug Port] Linux fails to setup EHCI debug port

2014-04-24 Thread Kyösti Mälkki

Sorry for the late response, as I do not follow linux-usb list very closely.

I have discussed recent Intel chipsets and EHCI debug in some detail on 
the coreboot mailing list [1].


As for the discussion [2] that took place here in February, I noticed 
you tried with early_printk=dbgp which only tries USB bus 1 on EHCI 
0:1a.0. You need to use early_printk=dbgp1 to probe USB bus 2 (EHCI 
0:1d.0), lsusb output suggested that is the controller net20dc was 
plugged in.


Correct USB port would appear as "usb 1-1.2" or "usb 2-1.2" in the 
system log when you connect your USB device. EHCI debug mode will bypass 
the rate matching hubs.


While it does not appear to case on your setup, enabled XHCI can also 
prevent EHCI debug from working. And sometimes you see bad design where 
the USB port you need is either routed to mini-pci-e or ExpressCard slot 
or not connected at all.


Regards,
Kyösti

[1] http://www.coreboot.org/pipermail/coreboot/2014-January/077016.html
[2] http://marc.info/?t=13927060913&r=1&w=2

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


EHCI debug device gadgets and host

2013-06-30 Thread Kyösti Mälkki
Hi

Is someone actively working on or maintaining the EHCI debug device
gadget driver? I would like to see some fixes and/or improvements to it
but do not know the gadget framework and some of the required hardware
is not yet on my desk.

So the incomplete report here is from a friends' experiment.

The behaviour was experienced on both BeagleBone Black and GTA04 used as
the gadget side. These and other recent ARM boards could be used for
kernel debugging of x86 platforms where EHCI debug port is available and
there is no classic serial port.

The problem is with CONFIG_G_DBGP_SERIAL disconnecting the TTY on USB
resets. While this may be correct behaviour for most use cases, it means
when connected to a debug target with EHCI debug port in kernel, the TTY
link does not survive over target board USB reset signalling.
Also in my use case (debugging coreboot) we may need one or two USB
resets even before kernel.


For the debug target side, I have previously written some patches to get
compatibility with USB FX2 chips to be used as a replacement debug
device now that the Net20DC part is hard to get.

I hope we could discuss if some patches here are relevant for
upstreaming:

https://bitbucket.org/kmalkki/linux-kernel/commits/branch/ehci-dbgp-test2


Regards,
KM

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