Re: driver migration

2016-04-07 Thread tilman
Hello


Kind of Driver: Driver for a dongle usb-> serial 
Manufacturer: Io-Data
Device: USB-RSA (based on EZUSB-CHip)
Reference: My last post from 2016-04-02 19:49:40 GMT
Description:
The blocking behaviour that I reported in my last post seems to be induced
by the set_bit and submit_urb in usbrsa_write. If I comment them, the "echo"
or "stty" commands no longer block. If I only comment submit_urb, I
experience blocking.
For the complete source code, please refer to my posting from 2016-04-02
19:49:40 GMT

 usbrsa_write
.

dev_dbg(>dev, "%s - port %d;URB allocated=%d", __func__,
port->port_number, urb_index);
/* reserve urb */
/* if I comment set_bit and submit_urb, I do not observe 
*  blocking any longer 
*/ 
set_bit(urb_index, >write_urb_pool_lock);

urb = priv->write_urb_pool[urb_index];

spin_unlock_irqrestore(>lock, flags);

/* copy data from userspace into urb transfer buffer */
bytes = (nof_bytes_to_be_sent > port->bulk_out_size) ?
port->bulk_out_size : nof_bytes_to_be_sent;
...


dev_dbg(>dev, "%s - port %d;bytes in urb=%d", __func__,
port->port_number, bytes);
usb_serial_debug_data(>dev,
__func__, bytes, urb->transfer_buffer);
urb->dev = serial->dev;
urb->transfer_buffer_length = bytes;

/* sent urb to USB-RSA */
//retval = usb_submit_urb(urb, GFP_ATOMIC);
..


Thanks
Tilman




--
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: driver migration

2016-04-02 Thread tilman
[106202.140016] usbrsa ttyUSB0: usbrsa_ioctl() cmd 0x5401
[106202.140021] usbrsa ttyUSB0: usbrsa_ioctl(): TCGETS -- not implemented
[106202.140025] usbrsa ttyUSB0: usbrsa_ioctl() leaving
[106202.140030] usbrsa ttyUSB0: usbrsa_ioctl() cmd 0x5403
[106202.140035] usbrsa ttyUSB0: usbrsa_ioctl(): 0x5403 not supported
[106202.140038] usbrsa ttyUSB0: usbrsa_ioctl() leaving
[106204.139545] usbrsa ttyUSB0: usbrsa_status_callback():
usb_serial_port_softint

 (stty waits blocked on the command line) 

[106206.139617] usbrsa ttyUSB0: usbrsa_status_callback():
usb_serial_port_softint

... (repeats every 2 seconds until CTRL-C  is sent to the stty command)


[106428.147311] usbrsa ttyUSB0: usbrsa_status_callback():
usb_serial_port_softint
[106428.665722] usbrsa_close - start
[106428.665733] usbrsa ttyUSB0: usbrsa_close - port 0 start
[106428.665740] usbrsa ttyUSB0: send_baudrate_lcr_register() CFLAG=1207
SPEED=300 ep=5
[106428.665744] usbrsa ttyUSB0: send_baudrate_lcr_register()
ST16C550.DLL=74;ST16C550.DLM=6;ST16C550.LCR=3
[106428.665782] usbrsa ttyUSB0: usbrsa_baudrate_lcr_callback() - port = 0,
ep=0xc0028f00
[106428.665794] usbrsa ttyUSB0: send_baudrate_lcr_register() leaving
[106428.665797] usbrsa ttyUSB0: usbrsa_close - #retries=3
[106428.665799] usbrsa_close - end
[106428.665801] usbrsa ttyUSB0: usbrsa_close - end 
[106428.665824] usbrsa ttyUSB0: usbrsa_status_callback(): wake_up

... (stty returns to command line. return code = 130)




SOURCE CODE:

/*
 * Driver for IO-Data's USB RSA serial dongle
 *  Copyright (C) 2012
 * Tilman Glotzner(tilmanglotz...@gmail.com)
 *
 *
 * * This program is free software; you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation; either version 2 of the License, or
 * (at your option) any later version.
 *
 *
 *
 *  USB-RSA by IO-DATA
 *  USB to serial converter
 *  ===
 *
 *  Remark: The device uses an AN2131Q (Eazy USB) and a ST16C550(UART)
 *
 *  End Points:
 *  
 *
 *  End Point:  EP1OUT
 *  Direction:  Host to Device
 *  Type:   Bulk Transfer
 *  Size:   64 bytes
 *  Desc:   Data to be transmitted
 *  Format: 64 bytes of data
 *
 *
 *  End Point:  EP2OUT
 *  Direction:  Host to Device
 *  Type:   Bulk Transfer
 *  Size:   3 bytes
 *  Desc:   Set baud rate counter and Line Control Register
 *  of the ST16C550. Receiver Interrupts are turned
 *  ON, i.e.  Modem status IR, RX line status IR,
 *  and RX holding register IR
 *  Format:
 *  Byte0:  Low Byte of counter (DLL)
 *  Byte1:  High Byte of counter (DLM)
 *  Byte2:  Line Control Register (LCR)
 *  - Bit 0: Word Length 0
 *  - Bit 1: Word Length 1
 *  - Bit 2: Stop Bits
 *  - Bit 3: Parity Enable
 *  - Bit 4: Parity Even
 *  - Bit 5: Set Parity
 *  - Bit 6: Set Break
 *  - Bit 7: Counter Register Enable
 *  (needs to be set to '0')
 *
 *
 *  End Point:  EP3OUT
 *  Direction:  Host to Device
 *  Type:   Bulk Transfer
 *  Size:   1 bytes
 *  Desc:   Set modem control register of the ST16C550
 *  Format:
 *  Byte0:  Modem Control Register
 *  - Bit0: DTR
 *  - Bit1: RTS
 *  - Bit2: OP1
 *  - Bit3: OP2
 *  - Bit4: Diagnostics mode
 *
 *
 *  End Point:  EP4OUT
 *  Direction:  Host to Device
 *  Type:   Bulk Transfer
 *  Size:   0 bytes
 *  Desc:   Reset EP1OUT and EP1IN (Tx/Rx pipes)
 *  Format: n.a.
 *
 *
 *  End Point:  EP5OUT
 *  Direction:  Host to Device
 *  Type:   Bulk Transfer
 *  Size:   3 bytes
 *  Desc:   Set baud rate counter and Line Control Register
 *  of the ST16C550. Receiver Interrupts are turned
 *  OFF, i.e.  Modem status IR, RX line status IR,
 *  and RX holding register IR
 *  Format:
 *  Byte0:  Low Byte of counter (DLL)
 *  Byte1:  High Byte of counter (DLM)
 *  Byte2:  Line Control Register (LCR)
 *  - Bit 0: Word Length 0
 *  - Bit 1: Word Length 1
 *  - Bit 2: Stop Bits
 *  - Bit 3: Parity Enable
 *  - Bit 4: Parity Even
 *  - Bit 5: Set P

Re: driver migration

2016-04-02 Thread tilman
Konstantin Shkolnyy <Konstantin.Shkolnyy@...> writes:
> Just a wild guess - this 30-second delay before close can happen because
usbserial is waiting for data to be
> transmitted, before closing the port.
> If you know that your data have been transmitted, perhaps you didn't
properly tell usbserial that your
> write operation has finished.

Konstantin

thank you for your suggestion. 

I have implemented a dedicated write routine for the driver, and it is
returning the value 11 (for 11 sent characters downstream). The urb ia then
 put back into the pool, and then the blocking starts. stty -f /dev/ttyUSB0
300 seems never to finish, while echo "0123456789" > /dev/ttyUSB0
reproducibly finish after 30 seconds. I will post the complete source code
in response to Greg's posting. 

Tilman

= syslog ==
sbrsa_write; private struct =880307620e00
{103936.132220] usbrsa ttyUSB0: usbrsa_write - port 0; bytes=11 := count=11
: nofTxBytesFree=4085
[103936.132226] usbrsa ttyUSB0: usbrsa_write - poolsize  10, urb_index 0
[103936.132231] usbrsa ttyUSB0: usbrsa_write - port 0;URB allocated=0
[103936.132237] usbrsa ttyUSB0: usbrsa_write - port 0;bytes in urb=11
[103936.132245] usbrsa ttyUSB0: usbrsa_write - length = 11, data = 30 31 32
33 34 35 36 37 38 39 0a
[103936.132255] usbrsa ttyUSB0: usbrsa_write - port 0;sent=11;count=0;
nof_bytes_to_be_sent=0;bytes=11
[103936.132261] usbrsa ttyUSB0: usbrsa_write() End - sent=11
[103936.132306] usbrsa ttyUSB0: usbrsa_write_callback() - port = 0, ep
=0xc0018f80
[103936.132319] usbrsa ttyUSB0: usbrsa_write_callback - length = 11, data =
30 31 32 33 34 35 36 37 38 39 0a
[103936.132326] usbrsa ttyUSB0: usbrsa_write_callback(): Returned URB 0
sbrsa_status_callback: nofTxBytesFree=4074,nofRxBytesReceived=0
[103938.129140] usbrsa ttyUSB0: usbrsa_status_callback():
usb_serial_port_softint

..

[103966.130101] usbrsa ttyUSB0: usbrsa_status_callback():
usb_serial_port_softint
[103966.654158] usbrsa_close - start
[103966.654167] usbrsa ttyUSB0: usbrsa_close - port 0 start
[103966.654172] usbrsa ttyUSB0: send_baudrate_lcr_register()
.



--
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: driver migration

2016-03-29 Thread tilman
Greg KH <greg@...> writes:

Dear Greg

> > I moved the initialization and clean up code from the init_callback/release
> > callback to the port_init/port_remove callback.

I am referring to your last posting on Feb 25th:
gkh> Release your port data in the port_remove callback, not the release
gkh> callback.  The release callback is for when your whole device is
gkh> removed, not the individual ports.

In addition, I initialized a spinlock  as part of setting up the data
structures in the port_init callback routine. Since then, the driver is no
longer crashing, and also the load no longer slowly builds up causing the
machine to eventually freeze.

One problem solved.

Next problem: 
When issuing a write command to the driver via echo "text" > /dev/ttyUSB0,
there is a delay of about 30 seconds until echo completes (echo is blocking
for 30 seconds). The timestamp in the syslog also show this: usbrsa_close is
started only around 30 secs (see below below @ 7346.332061) after completion
of the write completion handlers. 
The routine usbrsa_status_callback that was started first by usbrsa_open,
and is then being continuously called by itself, does not block.
(For the source code of the involved routines, please see below)


With the ancient kernel version 3.17, this was working fine.
I would not think that the 30 seconds blockng are not random but are related
to a timeout of any kind. 

1) I wonder what the kernel is doing between usbrsa_write and usbrsa_close.
it seems not to be executing code within the driver. What can cause the 30
seconds of blocking/what is the kernel waiting for?

2) I have seen other drivers calling usb_serial_port_softint from various
places. What exaclty does usb_serial_port_softint/schedule_word that is
called by usb_serial_port_softint?



Many thanks

Tilman

Syslog:
===
...
[ 7315.882626] usbrsa_open - port=0
[ 7315.882666] usbrsa ttyUSB0: usbrsa_write_room(): Pool=640;usbrsa.tx=4084
[ 7315.882673] usbrsa ttyUSB0: usbrsa_write - port 0 START
[ 7315.882676] usbrsa_write; private struct =8803ff468c00
[ 7315.882682] usbrsa ttyUSB0: usbrsa_write - port 0; bytes=10 := count=10 :
nofTxBytesFree=4084
[ 7315.882688] usbrsa ttyUSB0: usbrsa_write - poolsize  10, urb_index 0
[ 7315.882694] usbrsa ttyUSB0: usbrsa_write - port 0;URB allocated=0
[ 7315.882699] usbrsa ttyUSB0: usbrsa_write - port 0;bytes in urb=10
[ 7315.882705] usbrsa ttyUSB0: usbrsa_write - length = 10, data = 31 32 33
34 35 36 37 38 39 30
[ 7315.882711] usbrsa ttyUSB0: usbrsa_write - port 0;sent=10;count=0;
nof_bytes_to_be_sent=0;bytes=10
[ 7315.882714] usbrsa ttyUSB0: usbrsa_write() End - sent=10
[ 7315.882722] usbrsa ttyUSB0: usbrsa_write_room(): Pool=576;usbrsa.tx=4084
[ 7315.882726] usbrsa ttyUSB0: usbrsa_write - port 0 START
[ 7315.882729] usbrsa_write; private struct =8803ff468c00
[ 7315.882734] usbrsa ttyUSB0: usbrsa_write - port 0; bytes=2 := count=2 :
nofTxBytesFree=4084
[ 7315.882738] usbrsa ttyUSB0: usbrsa_write - poolsize  10, urb_index 1
[ 7315.882742] usbrsa ttyUSB0: usbrsa_write - port 0;URB allocated=1
[ 7315.882745] usbrsa ttyUSB0: usbrsa_write - port 0;bytes in urb=2
[ 7315.882748] usbrsa ttyUSB0: usbrsa_write - length = 2, data = 0d 0a
[ 7315.882753] usbrsa ttyUSB0: usbrsa_write - port 0;sent=2;count=0;
nof_bytes_to_be_sent=0;bytes=2
[ 7315.882756] usbrsa ttyUSB0: usbrsa_write() End - sent=2
[ 7315.882769] usbrsa ttyUSB0: usbrsa_write_callback() - port = 0, ep
=0xc0018380
[ 7315.882776] usbrsa ttyUSB0: usbrsa_write_callback - length = 10, data =
31 32 33 34 35 36 37 38 39 30
[ 7315.882781] usbrsa ttyUSB0: usbrsa_write_callback(): Returned URB 0
[ 7315.882918] usbrsa ttyUSB0: usbrsa_write_callback() - port = 0, ep
=0xc0018380
[ 7315.882927] usbrsa ttyUSB0: usbrsa_write_callback - length = 2, data = 0d 0a
[ 7315.882932] usbrsa ttyUSB0: usbrsa_write_callback(): Returned URB 1
[ 7315.882938] usbrsa_status_callback: nofTxBytesFree=4074,nofRxBytesReceived=0 
[ 7315.883002] usbrsa_status_callback: nofTxBytesFree=4072,nofRxBytesReceived=0 
[ 7346.332061] usbrsa_close - start
[ 7346.332070] usbrsa ttyUSB0: usbrsa_close - port 0 start
[ 7346.332076] usbrsa ttyUSB0: send_baudrate_lcr_register() CFLAG=3261 
[ 7346.332080] usbrsa ttyUSB0: send_baudrate_lcr_register()
ST16C550.DLL=50;ST16C550.DLM=0;ST16C550.LCR=3
[ 7346.332130] usbrsa ttyUSB0: usbrsa_baudrate_lcr_callback() - port = 0,
ep=0xc0028300
[ 7346.332142] usbrsa ttyUSB0: send_baudrate_lcr_register() leaving
[ 7346.332146] usbrsa ttyUSB0: usbrsa_close - #retries=3
[ 7346.332148] usbrsa_close - end
[ 7346.332151] usbrsa ttyUSB0: usbrsa_close - end 
[ 7346.332163] usbrsa ttyUSB0: usbrsa_status_callback(): wake_up


Source code Excerpt:
=

static struct usb_serial_driver usbrsa_enumerated_device = 
{
.driver = {
.owner =THIS_MODULE,
.name = "usbrsa",
}, 
.description =  "IO-DATA - USB-RSA",

Re: driver migration

2016-03-27 Thread tilman
I moved the initialization and clean up code from the init_callback/release
callback to the port_init/port_remove callback. This did not help. What
prevents the driver from crashing is to initialize the spinlock via
spin_lock_init(>lock). Not sure if I originally had forgotten the
command, and the driver worked despite an uninitialized spinlock with 
kernel V3.17, or whether the spin_lock_init procedure was added in  kernel
version later than 3.17.

When issuing a write command to the driver via echo "text" > /dev/ttyUSB0,
there is a delay of about 30 Seconds until echo completes (echo is blocking
for 30 seconds). This is also observable in the syslog. usbrsa_close is
started only around 30 secs after completion of the write function
usbrsa_write and the write completion handlers. I would think this might be
related to a change in the scheduling mechanism since 3.17.  I wonder
whether this can be fixed by using usb_serial_port_softint(port) at the
right places. It apparently calls the kernel schedulding interface ( 
usb_serial_port_softint calls schedule_work). I would appreciate a hint on
how to prevent the blocking and an explanaion of what
usb_serial_port_softint is intented to do.

Many thanks

Tilman

[ 7315.882927] usbrsa ttyUSB0: usbrsa_write_callback - length = 2, data = 0d 0a
[ 7315.882932] usbrsa ttyUSB0: usbrsa_write_callback(): Returned URB 1
[ 7315.882938] usbrsa_status_callback: nofTxBytesFree=4074,nofRxBytesReceived=0 
[ 7315.883002] usbrsa_status_callback: nofTxBytesFree=4072,nofRxBytesReceived=0 
[ 7346.332061] usbrsa_close - start
[ 7346.332070] usbrsa ttyUSB0: usbrsa_close - port 0 start
[ 7346.332076] usbrsa ttyUSB0: send_baudrate_lcr_register() CFLAG=3261 
[ 7346.332080] usbrsa ttyUSB0: send_baudrate_lcr_register()
ST16C550.DLL=50;ST16C550.DLM=0;ST16C550.LCR=3
[ 7346.332130] usbrsa ttyUSB0: usbrsa_baudrate_lcr_callback() - port = 0,
ep=0xc0028300
[ 7346.332142] usbrsa ttyUSB0: send_baudrate_lcr_register() leaving
[ 7346.332146] usbrsa ttyUSB0: usbrsa_close - #retries=3
[ 7346.332148] usbrsa_close - end
[ 7346.332151] usbrsa ttyUSB0: usbrsa_close - end 
[ 7346.332163] usbrsa ttyUSB0: usbrsa_status_callback(): wake_up





--
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: driver migration

2016-02-24 Thread tilman

> I suggest staying away from eclipse, that's not needed for kernel
> development.  Other than that, it's just normal debugging, good luck
> with that :)
I resorted to good old vi as the editor that starts up  fastest :-)

And I traced down the problem -- but not the root cause:

In usbrsa_attach, I allocate a private struct for port specific/driver
specific data:

static int  usbrsa_attach(struct usb_serial *serial)
{
int   i = 0;
int   ret = 0;
struct usb_serial_port* serport;
struct usbrsa_port_private* priv = NULL;
...
for (i = 0; i < serial->num_ports; i++) 
{
serport = serial->port[i];

// allocate struct for driver's private data
priv = kmalloc(sizeof(struct usbrsa_port_private), GFP_KERNEL);

 usb_set_serial_port_data(serport,priv);
...
}


In usbrsa_release, the pointer to the private struct has become a NULL
pointer -- even though the pointer was not manipulated in between by
usbrsa_open or usbrsa_write. In fact, I have dumped the pointer in all other
functions like usbrsa_open, usbrsa_writeroom etc., and it is correct.  The
null pointer in usbrsa_release then leads to a pointer dereferencing error
in  release_baudrate_lcr_urb(priv);

static void  usbrsa_release(struct usb_serial *serial)
{
int i;
struct usbrsa_port_private* priv;
struct usb_serial_port* serport;
...

for (i = 0; i < serial->num_ports; i++) 
{
   printk("%s: Mark1\n",__func__);
serport = serial->port[i];
priv = usb_get_serial_port_data(serport);
printk("%s: Mark2\n",__func__);
printk("%s: private struct at %p\n",__func__,priv);
printk("%s: Mark2a ",__func__);
release_baudrate_lcr_urb(priv);

Here the kernel log
 [ 1856.366874] usbrsa_release: Mark2
 [ 1856.366876] usbrsa_release: private struct at (null)
 [ 1856.366878] usbrsa_release: Mark2a release_baudrate_lcr_urb: Start
 [ 1856.366904] BUG: unable to handle kernel NULL pointer dereference at
0102

This could mean that usb_get_serial_port_data does not always work correctly
-- which I find rather unlikely. Are there any limitations known, for
instance when using the usb_get_serial_port_data in an SMP environment? Any
suggestions in which direction to dig?

Thanks Tilman















--
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: driver migration

2016-02-21 Thread tilman
irqsave+0x37/0x40
[] usbrsa_attach+0x1b5/0x660 [usbrsa]
[] usb_serial_probe+0xdcf/0x1220 [usbserial]
[] ? __mutex_lock_slowpath+0x2f/0x110
[] ? ida_simple_get+0x85/0xe0
[] ? __pm_runtime_set_status+0x128/0x1e0
[] usb_probe_interface+0x1bf/0x310
[] driver_probe_device+0x239/0x460
[] __device_attach_driver+0x74/0x80
[] ? driver_allows_async_probing+0x30/0x30
[] bus_for_each_drv+0x58/0x90
[] __device_attach+0xbb/0x140
[] device_initial_probe+0x13/0x20
[] bus_probe_device+0x92/0xa0
[] device_add+0x422/0x610
[] usb_set_configuration+0x4f1/0x7f0
[] generic_probe+0x2e/0xa0
[] usb_probe_device+0x32/0x70
[] driver_probe_device+0x239/0x460
[] __device_attach_driver+0x74/0x80
[] ? driver_allows_async_probing+0x30/0x30
[] bus_for_each_drv+0x58/0x90
[] __device_attach+0xbb/0x140
[] device_initial_probe+0x13/0x20
[] bus_probe_device+0x92/0xa0
[] device_add+0x422/0x610
[] usb_new_device+0x276/0x4d0
[] hub_event+0xd7a/0x1430
[] process_one_work+0x151/0x3d0
[] worker_thread+0x12b/0x4b0
[] ? rescuer_thread+0x340/0x340
[] kthread+0xc9/0xe0
[] ? kthread_park+0x60/0x60
[] ret_from_fork+0x3f/0x70
[] ? kthread_park+0x60/0x60
Code: c1 e0 10 45 31 c9 85 c0 74 46 48 89 c2 c1 e8 12 48 c1 ea 0c 83 e8 01
83 e2 30 48 98 48 81 c2 40 76 01 00 48 03 14 c5 c0 d2 d3 81 <4c> 89 02 41 8b
40 08 85 c0 75 0a f3 90 41 8b 40 08 85 c0 74 f6 
 RIP  [] native_queued_spin_lock_slowpath+0x104/0x190
 RSP 
 CR2: 1ddc00017654
 ---[ end trace a78fe1b9025ad592 ]---
BUG: unable to handle kernel paging request at ffd8

Many thanks
Tilman



N�r��yb�X��ǧv�^�)޺{.n�+{��^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�

Re: driver migration

2016-02-19 Thread tilman
Greg KH <gregkh@...> writes:

> 
> On Fri, Feb 19, 2016 at 06:49:50AM +, tilman wrote:
> > Hello
> > 
> > I configured and setup a more recent kernel:  V4.5.0-rc4
> > 
> > The driver compiles and inserts.
> 
> What driver?  You have provided no context here :(
My apologies -- what I meant to say was: I inserted the compile usbrsa
driver. The code of the driver I posted on Feb 16th.

> 
> > When I plugin the USB-Dongle, the machine
> > freezes (not sure whether it freezes during the firmware download or after
> > it once it has reenumerated). 
> 
> Your kernel logs should show you this.

The kernel freezes before it can log the kernel oops:

Feb 19 07:26:35 btron kernel: [  241.247360] usbcore: registered new interface d
river usbrsa
Feb 19 07:26:35 btron kernel: [  241.247388] usbserial: USB Serial support regis
tered for IO-DATA - USB-RSA - (prerenumeration)
Feb 19 07:26:35 btron kernel: [  241.247412] usbserial: USB Serial support regis
tered for IO-DATA - USB-RSA

^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@


> The kernel logs can provide you with a full traceback, we can't do much
> without that.
I understand that and I would love to provide one -- there is however no
traceback logged. All I have is a kernel message on the console:

BUG: unable to handle kernel paging request at ...

When I had the driver running on the kernel version 3.13.0, things started
to go wrong in usbrsa_attach of the usbrsa driver (see first posting on Feb
16th)

I will comment out all functional parts of the driver, and check if it still
fails. If it no longer crashes, I will comment in procedure by procedure
until it crashes hoping that I can isolate the problem that way ... 
If there are better/faster debugging techniques I would love to learn about
them :-)

Thanks

Tilman



--
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: driver migration

2016-02-18 Thread tilman
Hello

I configured and setup a more recent kernel:  V4.5.0-rc4

The driver compiles and inserts. When I plugin the USB-Dongle, the machine
freezes (not sure whether it freezes during the firmware download or after
it once it has reenumerated). 

I could observe  a kernel oops message starting with BUG. Probably a null
pointer dereference...

Cheers
Tilman

--
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: driver migration

2016-02-15 Thread tilman
Greg KH <gregkh@...> writes:

> 
> On Mon, Feb 15, 2016 at 11:30:43PM +, tilman wrote:
> > Dear all
> > 
> > a couple of years ago I wrote a driver for a serial dongle.
> > I did not add it to the linux source because the dongle requires a firmware
> > to be downloaded to the device (ezusb).
> > The manufacturer, IO-DATA, did not want me to use
> > their firmware.
> 
> Why did they not want you to use their firmware in their device?
They did not say. I would think that they feared that users would come to
them in case the driver does not work. For  the successor model, the offer a
linux driver however. 
I have disassembled the firmware however. If I can make some time, I can
rewrite the firmware which would avoid this issue. First step however is to
get the driver working again.


> 
> It's impossible to say without seeing the code, sorry.
Below, it comes...

Thanks
Tilman

/// usbrsa.c

/*
 * Driver for IO-Data's USB RSA serial dongle
 *  Copyright (C) 2012
 * Tilman Glotzner(tilmanglotz...@gmail.com)
 *  
 *
 *  This program is free software; you can redistribute it and/or modify
 *  it under the terms of the GNU General Public License as published by
 *  the Free Software Foundation; either version 2 of the License, or
 *  (at your option) any later version.
 *  
 *
 *
 *  USB-RSA by IO-DATA
 *  USB to serial convertor
 *  ===
 *  
 *  Remark: The device uses an AN2131Q (Eazy USB) and a ST16C550(UART)
 *  
 *  End Points:
 *  
 *  
 *  End Point:  EP1OUT
 *  Direction:  Host to Device
 *  Type:   Bulk Transfer
 *  Size:   64 bytes
 *  Desc:   Data to be transmitted 
 *  Format: 64 bytes of data
 *  
 *  
 *  End Point:  EP2OUT
 *  Direction:  Host to Device
 *  Type:   Bulk Transfer
 *  Size:   3 bytes
 *  Desc:   Set baud rate counter and Line Control Register
 *  of the ST16C550. Receiver Interrupts are turned ON,
 *  i.e.  Modem status IR, RX line status IR, and 
 *  RX holding register IR
 *  Format:
 *  Byte0:  Low Byte of counter (DLL)
 *  Byte1:  High Byte of counter (DLM)
 *  Byte2:  Line Control Register (LCR)
 *  - Bit 0: Word Length 0
 *  - Bit 1: Word Length 1
 *  - Bit 2: Stop Bits
 *  - Bit 3: Parity Enable
 *  - Bit 4: Parity Even
 *  - Bit 5: Set Parity
 *  - Bit 6: Set Break
 *  - Bit 7: Counter Register Enable (needs to be set to 
'0')   
 *   
 *   
 *  End Point:  EP3OUT
 *  Direction:  Host to Device
 *  Type:   Bulk Transfer
 *  Size:   1 bytes
 *  Desc:   Set modem control register of the ST16C550
 *  Format: 
 *  Byte0:  Modem Control Register
 *  - Bit0: DTR
 *  - Bit1: RTS
 *  - Bit2: OP1
 *  - Bit3: OP2
 *  - Bit4: Diagnostics mode 
 *  
 *  
 *  End Point:  EP4OUT
 *  Direction:  Host to Device
 *  Type:   Bulk Transfer
 *  Size:   0 bytes
 *  Desc:   Reset EP1OUT and EP1IN (Tx/Rx pipes)
 *  Format: n.a.
 *  
 *  
 *  End Point:  EP5OUT
 *  Direction:  Host to Device
 *  Type:   Bulk Transfer
 *  Size:   3 bytes
 *  Desc:   Set baud rate counter and Line Control Register
 *  of the ST16C550. Receiver Interrupts are turned OFF,
 *  i.e.  Modem status IR, RX line status IR, and 
 *  RX holding register IR
 *  Format:
 *  Byte0:  Low Byte of counter (DLL)
 *  Byte1:  High Byte of counter (DLM)
 *  Byte2:  Line Control Register (LCR)
 *  - Bit 0: Word Length 0
 *  - Bit 1: Word Length 1
 *  - Bit 2: Stop Bits
 *  - Bit 3: Parity Enable
 *  - Bit 4: Parity Even
 *  - Bit 5: Set Parity
 *  - Bit 6: Set Break
 *  - Bit 7: Counter Register Enable (needs to be set to 
'0')
 *  
 *  
 *  End Point:  EP1IN
 *  Direction:  Device to Host 
 *  Type:   Bulk Transfer
 *  Size:   64 bytes
 *  Desc:   Data received by USB-RSA 
 *  Format: 64 bytes of data
 *  
 *  
 *  End Point:  EP2IN
 *  Direction:  Device to Host
 *  Type:   Bulk Transfer
 *  Size:   1 byte
 *  Desc:   Read Modem Status Register of ST16C550
 *  Format: 
 *  Byte0:  Modem Status Register 
 *  - Bit0: Delta CTS
 *

driver migration

2016-02-15 Thread tilman
Dear all

a couple of years ago I wrote a driver for a serial dongle.
I did not add it to the linux source because the dongle requires a firmware
to be downloaded to the device (ezusb).
The manufacturer, IO-DATA, did not want me to use
their firmware.

I am now in the process to move the driver from kernel version 3.0.x to
kernel version 3.13.0 (ubuntu 14.02 runs with it).
Once this works, I will migrate it once more to a kernel on the main line
(hoping that there are not to many changes in the framework between 3.13.0
and 4.5). 


a) I can compile the driver, and insert it. 
It downloads the firmware to the serial dongle. 
The device reenumerates then, but the driver fails to attach to the device.
It hangs while allocating the memory for the urbs of the write pool usb
messages. 
It then behaves like a time bomb: The load slowly
increase, and eventually, the system stops and I need to reboot the system.


Any thoughts on how to trace down the root cause and fix it ?

Here the sys log indicating this:
usb 3-3: usbrsa_attach
[ 5017.515778] usb-serial (null):allocate_write_urbs()
[ 5039.595625] [ cut here
[ 5039.595642] WARNING: CPU: 4 PID: 71 at

/build/linux-faWYrf/linux-3.13.0/kernel/watchdog.c:245

watchdog_overflow_callback+0x9c/0xd0()
[ 5039.595644] Watchdog detected hard LOCKUP on cpu 4



And here the part of the log after the enumeration sequence:
[  492.820632] usb 3-3: Product: USB-RS232C CONVERTER
[  492.820638] usb 3-3: Manufacturer: I-O DATA DEVICE,Inc.
[  492.820642] usb 3-3: SerialNumber: USB-RSA
Rev.1

[  492.821209] usbrsa 3-3:1.0: IO-DATA - USB-RSA converter detected
[  492.821225] usbrsa_attach start
[  492.821230] usb 3-3: usbrsa_attach


b) The driver requires the modules ezusb, and usb-serial.
 The usb serial driver framework does not offer to use the module_init
function any longer.
What would be the appropriate place the corresponding request_module commands?

Many thanks

Tilman 


--
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: Disconnecting an USB3 device from xhci-port isn't detected properly

2013-02-09 Thread Tilman Schmidt
Am 08.02.2013 20:36, schrieb Matthias Schniedermeyer:

 After updating from 3.7.2 to 3.7.6 disconnecting a USB3 device from a 
 xhci-port isn't detected properly anymore. After removing a 32GB stick 
 the only line in syslog i can see is:
 sdc: detected capacity change from 31625052160 to 0
 /dev/sdc is still there.
 Reconnecting the device results in usb 3-1: USB disconnect, device 
 number 2
 immediatly followed by the normal slew of new device found messages.
 
 Git bisect points to this commit:
 f7965c0846d74b270e246c1470ca955d5078eb07

Here's another regression report pointing to that same commit:
http://lkml.org/lkml/2013/1/28/546
external HDD in USB3 enclosure cannot be dynamically removed

-- 
Tilman SchmidtE-Mail: til...@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)





signature.asc
Description: OpenPGP digital signature


Re: usb serial driver: private data already deallocated when release function is called

2013-01-15 Thread Tilman

Greg KH greg@... writes:
  Btw. are there repositories that would be suitable to make
  highly experimental
  code available ? 
 
 Yes, that is what drivers/staging/ is for, why not submit your driver
 for inclusion there?
The driver is not even close from being finished... 


 Ugh, the whitehead driver was not a good role model.
Well,maybe, but it is the only driver for an eazy-usb chip, isn't it ?

  You should look at
 the changes that have gone into that driver over the past few releases,
 and make the same kind of changes to your driver.  That would be an easy
 way to bring it up to modern standards, and fix the tty port issue at
 the same time.
I got the driver working to the extend it was working before. The issue was a
pointer dereferencing issue in the allocation routine. Thanks for the advice.

Tilman


--
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: usb serial driver: private data already deallocated when release function is called

2013-01-13 Thread Tilman
Hello Greg

  
 Do you have a pointer to your code anywhere?  That would be the easiest
 way to help you out here.  Otherwise we are just guessing as to the
 issues involved.

Not really -- I hence pasted it into this posting. I hope that is OK and does
violate some etiquette of this list.
Btw. are there repositories that would be suitable to make highly experimental
code available ? 
 
Thanks

Tilman
/* File: usbrsatest.c
 * Driver for IO-Data's USB RSA serial dongle (test driver)
 *  Copyright (C) 2012
 * Tilman Glotzner
 *
 *
 *  This program is free software; you can redistribute it and/or modify
 *  it under the terms of the GNU General Public License as published by
 *  the Free Software Foundation; either version 2 of the License, or
 *  (at your option) any later version.
 *
 *
 *
 */


#include linux/kernel.h
#include linux/errno.h
#include linux/init.h
#include linux/slab.h
#include linux/tty.h
#include linux/tty_driver.h
#include linux/tty_flip.h
#include linux/module.h
#include linux/spinlock.h
#include linux/mutex.h
#include linux/uaccess.h
#include asm/termbits.h
#include linux/usb.h
#include linux/serial_reg.h
#include linux/serial.h
#include linux/usb/serial.h
//#include linux/firmware.h
#include linux/device.h
#include linux/usb/ezusb.h
//#include linux/ihex.h
#include usbrsa.h

#define CONFIG_DYNAMIC_PRINTK_DEBUG 1
#define CONFIG_USB_SERIAL_DEBUG 1
#ifdef CONFIG_USB_SERIAL_DEBUG
static int debug = 1;
#else
static int debug = 0;
#endif

char buffer_tx[65];
char buffer_rx[65];

/*
 * Version Information
 */
#define DRIVER_VERSION v0.1
#define DRIVER_AUTHOR Tilman Gloetzner tilmanglotz...@gmail.com
#define DRIVER_DESC IO-Data USB-RSA test driver


#define IODATA_VENDOR_ID0x4bb
#define IODATA_USBRSA_PREENUM_ID0xa01
#define IODATA_USBRSA_ID0xa03
#define IODATA_USBRSA_FW_STRING USB-RSA Test-FW

#define COMMAND_TIMEOUT (2*HZ)  /* 2 second timeout for a command */

/*
   Taken whiteheat driver as role model:
   ID tables for usb-rsa are unusual, because we want to different
   things for different versions of the device.  Eventually, this
   will be doable from a single table.  But, for now, we define two
   separate ID tables, and then a third table that combines them
   just for the purpose of exporting the autoloading information.
*/
static const struct usb_device_id id_table_std[] = {
{ USB_DEVICE(IODATA_VENDOR_ID, IODATA_USBRSA_ID) },
{ }  /* Terminating entry */
};

static const struct usb_device_id id_table_prerenumeration[] = {
{ USB_DEVICE(IODATA_VENDOR_ID, IODATA_USBRSA_PREENUM_ID) },
{ } /* Terminating entry */
};

static const struct usb_device_id id_table_combined[] = {
{ USB_DEVICE(IODATA_VENDOR_ID, IODATA_USBRSA_ID) },
{ USB_DEVICE(IODATA_VENDOR_ID, IODATA_USBRSA_PREENUM_ID) },
{ } /* Terminating entry */
};


MODULE_DEVICE_TABLE(usb, id_table_combined);


//   Preenumeration device




//   Function Prototypes

static int usbrsa_test_firmware_download(struct usb_serial *serial,
const struct usb_device_id *id);
static int usbrsa_test_firmware_attach(struct usb_serial *serial);

//   Driver object declaration

static struct usb_serial_driver usbrsa_test_preenum_device = {
.driver = {
.owner =THIS_MODULE,
.name = usbrsanofirm,
},
.description =  IO-DATA - USB-RSA - (test prerenumeration),
.id_table = id_table_prerenumeration,
.num_ports =1,
.probe =usbrsa_test_firmware_download,
.attach =   usbrsa_test_firmware_attach,
};



//   Functions



//
// Name: usbrsa_firmware_download
// Purpose: Downloads firmware to AN2134Q
//
//
static int usbrsa_test_firmware_download(struct usb_serial *serial,
const struct usb_device_id *id)
{
int response = -ENOENT;

dev_dbg(serial-dev-dev,%s, __func__

Re: usb bus monitoring

2012-12-11 Thread Tilman
Peter Stuge peter@... writes:


 Tilman wrote:

 What device is that? Is it a Cypress FX2 or an SiLabs C8051?
ezusb (Cypress AN2131)

  The urb eventually times out. Via debug FS I can see that the urb is
  indeed submitted. I suspect the firmware to be responsible for this.
 
 That's quite likely.
.. on the other hand, I do not see an obvious mistake when intializing the USB
core of the  AN2131 :-)

  I would like to exclude the linux test driver owever,
 
 That doesn't make a lot of sense - if you take the driver away then
 there is nothing that will talk to your device, so there will be no
 traffic.
I did not mean to take the driver away -- I would like to exclude it as error
source, i.e. I understand that debugfs taps the communication between the device
driver and the queue of the usb subsystem. It does not give me certainty that
the urb ever made it to the usb wire.

Tilman

--
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 bus monitoring

2012-12-09 Thread Tilman
Hello

I am in the process of writing firmware for an USB device based on an 8051 core.
I am testing it against linux kernel driver.
 
I am submitting an urb to the device via a linux kernel modul, and I get no
response. The urb eventually times out. Via debug FS I can see that the urb is
indeed submitted. I suspect the firmware to be responsible for this. 

I would like to exclude the linux test driver owever, and wonder if there is a
cheap way to monitor what is happening on the USB bus itself. USB protocol
analyzer start at 1100 USD which is more than I would be willing to invest.

Thanks

Tilman

--
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] bas_gigaset: fix pre_reset handling

2012-10-24 Thread Tilman Schmidt
The delayed work function int_in_work() may call usb_reset_device()
and thus, indirectly, the driver's pre_reset method. Trying to
cancel the work synchronously in that situation would deadlock.
Fix by avoiding cancel_work_sync() in the pre_reset method.

If the reset was NOT initiated by int_in_work() this might cause
int_in_work() to run after the post_reset method, with urb_int_in
already resubmitted, so handle that case gracefully.

Signed-off-by: Tilman Schmidt til...@imap.cc
---
 drivers/isdn/gigaset/bas-gigaset.c |   19 ---
 1 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/isdn/gigaset/bas-gigaset.c 
b/drivers/isdn/gigaset/bas-gigaset.c
index 5275887..c44950d 100644
--- a/drivers/isdn/gigaset/bas-gigaset.c
+++ b/drivers/isdn/gigaset/bas-gigaset.c
@@ -617,7 +617,13 @@ static void int_in_work(struct work_struct *work)
if (rc == 0)
/* success, resubmit interrupt read URB */
rc = usb_submit_urb(urb, GFP_ATOMIC);
-   if (rc != 0  rc != -ENODEV) {
+
+   switch (rc) {
+   case 0: /* success */
+   case -ENODEV:   /* device gone */
+   case -EINVAL:   /* URB already resubmitted, or terminal badness */
+   break;
+   default:/* failure: try to recover by resetting the device */
dev_err(cs-dev, clear halt failed: %s\n, get_usb_rcmsg(rc));
rc = usb_lock_device_for_reset(ucs-udev, ucs-interface);
if (rc == 0) {
@@ -2442,7 +2448,9 @@ static void gigaset_disconnect(struct usb_interface 
*interface)
 }
 
 /* gigaset_suspend
- * This function is called before the USB connection is suspended.
+ * This function is called before the USB connection is suspended
+ * or before the USB device is reset.
+ * In the latter case, message == PMSG_ON.
  */
 static int gigaset_suspend(struct usb_interface *intf, pm_message_t message)
 {
@@ -2498,7 +2506,12 @@ static int gigaset_suspend(struct usb_interface *intf, 
pm_message_t message)
del_timer_sync(ucs-timer_atrdy);
del_timer_sync(ucs-timer_cmd_in);
del_timer_sync(ucs-timer_int_in);
-   cancel_work_sync(ucs-int_in_wq);
+
+   /* don't try to cancel int_in_wq from within reset as it
+* might be the one requesting the reset
+*/
+   if (message.event != PM_EVENT_ON)
+   cancel_work_sync(ucs-int_in_wq);
 
gig_dbg(DEBUG_SUSPEND, suspend complete);
return 0;
-- 
1.7.3.4

--
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/RFC] Re: pre_reset() in bas-gigaset.c

2012-10-13 Thread Tilman Schmidt
Am 13.10.2012 05:01, schrieb Ming Lei:
 On Sat, Oct 13, 2012 at 6:11 AM, Tilman Schmidt til...@imap.cc wrote:

 There are two cases I have to worry about:
 (1) a reset triggered by int_in_work()
 (2) a reset triggered by some other source external to the driver

 In case (1) the current design will deadlock because pre_reset() is
 indirectly called from int_in_work() but calls cancel_work_sync()
 which will wait for this very work item to complete. This would be
 fixed by pre_reset() not calling cancel_work_sync() at all.
 
 Maybe you need to set a flag in int_in_work() to indicate that
 current reset is from itself, so you may fix both two cases.
 
 int_in_work() will then run to completion without resubmitting
 urb_int_in because it's already past that point once it calls
 usb_reset_device(), so there's no conflict with post_reset() doing
 that.

 In case (2) the current design will work fine, but if I remove the
 call to cancel_work_sync() from pre_reset() there's a slight chance
 that an uncompleted int_in_work() work item remains uncancelled.
 So any part of int_in_work() might run asynchronously after
 pre_reset(). The question is whether this might cause a problem.

 Which of the two cases does your argument address?
 
 I am saying the case 2, and the current design should be fine
 if the 1sec timeout can be tolerated in int_in_work().
 
 You are right on case 1.

Ok, what do you think about this patch? It uses the fact that
pre_reset() calls the suspend method with message=PMSG_ON, which
the USB framework never does, to skip the cancel_work_sync() in that
case. It also avoids the extraneous reset if usb_submit_urb() fails
because post_reset() has already submitted the interrupt URB.

-- 
Tilman SchmidtE-Mail: til...@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)
diff --git a/drivers/isdn/gigaset/bas-gigaset.c b/drivers/isdn/gigaset/bas-gigaset.c
index 5275887..3c69dbf 100644
--- a/drivers/isdn/gigaset/bas-gigaset.c
+++ b/drivers/isdn/gigaset/bas-gigaset.c
@@ -617,7 +617,11 @@ static void int_in_work(struct work_struct *work)
 	if (rc == 0)
 		/* success, resubmit interrupt read URB */
 		rc = usb_submit_urb(urb, GFP_ATOMIC);
-	if (rc != 0  rc != -ENODEV) {
+
+	/* try to recover from failures by resetting the device, except
+	 * for -ENODEV (device gone) and -EINVAL (URB already resubmitted)
+	 */
+	if (rc != 0  rc != -EINVAL  rc != -ENODEV) {
 		dev_err(cs-dev, clear halt failed: %s\n, get_usb_rcmsg(rc));
 		rc = usb_lock_device_for_reset(ucs-udev, ucs-interface);
 		if (rc == 0) {
@@ -2442,7 +2446,9 @@ static void gigaset_disconnect(struct usb_interface *interface)
 }
 
 /* gigaset_suspend
- * This function is called before the USB connection is suspended.
+ * This function is called before the USB connection is suspended
+ * or before the USB device is reset.
+ * In the latter case, message == PMSG_ON.
  */
 static int gigaset_suspend(struct usb_interface *intf, pm_message_t message)
 {
@@ -2498,7 +2504,12 @@ static int gigaset_suspend(struct usb_interface *intf, pm_message_t message)
 	del_timer_sync(ucs-timer_atrdy);
 	del_timer_sync(ucs-timer_cmd_in);
 	del_timer_sync(ucs-timer_int_in);
-	cancel_work_sync(ucs-int_in_wq);
+
+	/* don't try to cancel int_in_wq from within reset as it
+	 * might be the one requesting the reset
+	 */
+	if (message.event != PM_EVENT_ON)
+		cancel_work_sync(ucs-int_in_wq);
 
 	gig_dbg(DEBUG_SUSPEND, suspend complete);
 	return 0;


signature.asc
Description: OpenPGP digital signature


Re: pre_reset() in bas-gigaset.c

2012-10-12 Thread Tilman Schmidt
Hi Oliver,

Am 10.10.2012 20:51, schrieb Oliver Neukum:
 As pre_reset() and suspend() are identical there is a problem with
 resetting.
 
 /* kill all URBs and delayed work that might still be pending */
 usb_kill_urb(ucs-urb_ctrl);
 usb_kill_urb(ucs-urb_int_in);
 del_timer_sync(ucs-timer_ctrl);
 del_timer_sync(ucs-timer_atrdy);
 del_timer_sync(ucs-timer_cmd_in);
 del_timer_sync(ucs-timer_int_in);
 cancel_work_sync(ucs-int_in_wq);
 
 You are calling cancel_work_sync(). However int_in_work() may itself
 be resetting the device. This seems to be a situation that should be avoided.

Good point. I'll have to think about that.

At first sight, I can simply omit the cancel_work_sync() in
the pre_reset() case. In the worst case, the uncancelled
int_in_work() will call usb_clear_halt(), try to resubmit the
already submitted URB, fail, and trigger another reset
needlessly. Doesn't sound too bad?

The alternative would be to introduce a new flag in the device
state to skip the cancel_work_sync() call if int_in_work() is
in the call to usb_reset_device().

Thanks,
Tilman

-- 
Tilman SchmidtE-Mail: til...@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)



signature.asc
Description: OpenPGP digital signature


Re: pre_reset() in bas-gigaset.c

2012-10-12 Thread Tilman Schmidt
Am 12.10.2012 16:17, schrieb Ming Lei:
 On Fri, Oct 12, 2012 at 5:26 PM, Tilman Schmidt til...@imap.cc wrote:
 
 At first sight, I can simply omit the cancel_work_sync() in
 the pre_reset() case. In the worst case, the uncancelled
 int_in_work() will call usb_clear_halt(), try to resubmit the
 already submitted URB, fail, and trigger another reset
 needlessly. Doesn't sound too bad?

 The alternative would be to introduce a new flag in the device
 state to skip the cancel_work_sync() call if int_in_work() is
 in the call to usb_reset_device().
 
 The above is not good, running pre_reset() means that the
 device is being reset and the device lock has been held,
 so int_in_work() can't acquire the device lock and complete
 the reset.
 
 On easy solution is that skipping reset device in int_in_work()
 if the device is being reset.

I'm not sure I understand your argument.

There are two cases I have to worry about:
(1) a reset triggered by int_in_work()
(2) a reset triggered by some other source external to the driver

In case (1) the current design will deadlock because pre_reset() is
indirectly called from int_in_work() but calls cancel_work_sync()
which will wait for this very work item to complete. This would be
fixed by pre_reset() not calling cancel_work_sync() at all.
int_in_work() will then run to completion without resubmitting
urb_int_in because it's already past that point once it calls
usb_reset_device(), so there's no conflict with post_reset() doing
that.

In case (2) the current design will work fine, but if I remove the
call to cancel_work_sync() from pre_reset() there's a slight chance
that an uncompleted int_in_work() work item remains uncancelled.
So any part of int_in_work() might run asynchronously after
pre_reset(). The question is whether this might cause a problem.

Which of the two cases does your argument address?

Thanks,
Tilman

-- 
Tilman SchmidtE-Mail: til...@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)



signature.asc
Description: OpenPGP digital signature


Re: clutter in /var/log/messages (drivers/usb/serial/usb-serial.c)

2008-01-06 Thread Tilman Schmidt
On Sat, 05 Jan 2008 12:41:16 +0100, [EMAIL PROTECTED] said:
[...]
 
 modprobe -r keyspan:
  usbcore: deregistering interface driver keyspan
  drivers/usb/serial/usb-serial.c: USB Serial deregistering driver Keyspan
  - (without firmware)
  drivers/usb/serial/usb-serial.c: USB Serial deregistering driver Keyspan
  1 port adapter
  drivers/usb/serial/usb-serial.c: USB Serial deregistering driver Keyspan
  2 port adapter
  drivers/usb/serial/usb-serial.c: USB Serial deregistering driver Keyspan
  4 port adapter
  usbcore: deregistering interface driver usbserial_generic
  drivers/usb/serial/usb-serial.c: USB Serial deregistering driver generic
  usbcore: deregistering interface driver usbserial
 
 from my admin`s point of view, drivers/usb/serial/usb-serial.c is
 useless clutter which generates an overhead of ~30% here and makes
 reading dmesg more difficult.

I couldn't agree more. :-)

 is there a special reason why there is a path to a sourcefile being
 printed here ?

The reason is that this driver uses the info() etc. macros from
linux/usb.h
which in turn use the __FILE__ macro for showing the source file name.
With the current kernel build mechanism, that expands to the path you
are
seeing. (I gather it wasn't so in 2.4 times.) I tried to submit a patch
for that
but it got rejected because these macros are apparently deprecated.

 what`s the advantage of having this and not just usb-serial: instead ?

Easier coding - or rather, avoiding the work of re-coding. The info()
macro
is there, it works, why change it?

 can this probable be optimized (as most drivers only print  drivername:
 anyway) ?

See the redefinition of these macros in drivers/isdn/gigaset/gigaset.h
for
the alternative I proposed at the time.

HTH
Tilman
-- 
  Tilman Schmidt
  [EMAIL PROTECTED]

-
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