[patch 00/02] 2 more hso.c patches

[patch 00/02] 2 more hso.c patches

[greg]

Jeff,

Here are two more patches for drivers/net/usb/hso.c that fix problems
people have reported.

I know you accepted the previously posted patches for this driver, but
they don't seem to be in Linus's tree yet, are they queued up for
inclusion before 2.6.27 is out?

thanks,

greg k-h

[patch 01/02] hso: icon 322 detection fix

[greg]

[-- Attachment #1: hso-icon-322-detection-fix.patch --]
[-- Type: text/plain, Size: 848 bytes --]

From: Denis Joseph Barrow <D.Barow@option.com>

Fixes Icon-322 detection.

Signed-off-by: Denis Joseph Barrow <D.Barow@option.com>
Cc: Jeff Garzik <jgarzik@pobox.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

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

--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -397,7 +397,7 @@ static const struct usb_device_id hso_id
 	{default_port_device(0x0af0, 0xc031)},	/* Icon-Edge */
 	{icon321_port_device(0x0af0, 0xd013)},	/* Module HSxPA */
 	{icon321_port_device(0x0af0, 0xd031)},	/* Icon-321 */
-	{default_port_device(0x0af0, 0xd033)},	/* Icon-322 */
+	{icon321_port_device(0x0af0, 0xd033)},	/* Icon-322 */
 	{USB_DEVICE(0x0af0, 0x7301)},		/* GE40x */
 	{USB_DEVICE(0x0af0, 0x7361)},		/* GE40x */
 	{USB_DEVICE(0x0af0, 0x7401)},		/* GI 0401 */

-- 

Re: [patch 01/02] hso: icon 322 detection fix

[Jeff Garzik]

applied 1-2

Re: hso driver dropping characters on serial port & hacky fix

[Denis Joseph Barrow]

Hi,
We have a problem with the hso driver serial ports.
The problem is a little complex but I hope I can explain it adequetely.
If you are unfamiliar with USB URBS are basically buffers we give to usb
hardware to recieve or transmit packets to a USB device.

The problem manifests itself when our hardware guys at option
have a userland diagnostics program going & on a system under
heavy load, the tty layers buffers belonging to our emulated serial port
fill up because the userland program gets no
chance to chance to empty data on the modems emulated serial port.

If we modify  put_rxbuf_data(struct urb *urb, struct hso_serial *serial)

with the copied check below we get printks under heavy load

> 		copied=tty_insert_flip_string(tty, urb->transfer_buffer,
> 				       urb->actual_length);
> 		if(copied<urb->actual_length)
> 			printk(KERN_ERR" hso: put_rxbuf_data did not "
> 			       "copy full rx buffer copied=%d asked to copy=%d\n",
> 			       copied,urb->actual_length);

In the current driver code you've got the driver drops packets silently.

I initially thought that this was not our problem/bug because I
thought we had no means for USB flow control & told our diagnostic
guys to go away & stop annoying me.
However I was talking to the firmware developer of our feama 3G modem
& he says that the modem will stall gracefully if it doesn't 
have outstanding bulk_urbs on the emulated serial ports.
i.e. we have a mechanism to do flow control on our emulated serial
ports & get away without dropping packets or data corruption.

If I redesign the code.
We run into a problem if we have no outstanding urbs on
the serial recieve channel as we will get no more read channel callback
interrupts. We need to poll the tty layer to
see if we have free space to put data from the
urbs we have not yet processed on the read channel or go with my hack.

The hack I'm suggesting which Filip Aben is worried that wont
get accepted by you is to put a change the line discipline function pointer
for the hso driver i.e. the bit that calls
disc->ops->receive_buf(tty, char_buf,flag_buf, count);
in
static void flush_to_ldisc(struct work_struct *work)
in
/drivers/char/tty_io.c

to point to our own special function in the hso driver where we call the original
disc->ops->receive_buf so we get automatically 
notified when we have free space in the hso drivers tty buffers.
Short of modifying the tty layer I can see no other way of doing this
efficently.

Is this acceptable to you guys.

This is the most elegant hack I can see to solve this problem,
if any of ye have a cleaner suggestion let me know.

Sincerely,
D.J. Barrow

question wrt /drivers/char/tty_io.c in pre 2.6.16 code

[Denis Joseph Barrow]

Hi all,
In kernels prior to 2.6.16 it appears to me have
no mechanism for finding out if the tty recieve buffers
are full. This is important for me in implementing
flow control in the hso serial driver I'm developing
I don't want to lose characters.

The snippet from allesandro rubini's book
doesn't will just blindly flip buffers
& overrun if data is pushed in too quickly
into the tty layer. 

for (i = 0; i < data_size; ++i) {
if (tty->flip.count >= TTY_FLIPBUF_SIZE)
tty_flip_buffer_push(tty);
tty_insert_flip_char(tty, data[i], TTY_NORMAL);
}
tty_flip_buffer_push(tty);

The new tty_insert_flip_string returns the
number of bytes successfully copied to tty recieve buffers
how do I implement this functionality in pre 2.6.16 code,
can it be done?

Thank you,
D.J. Barrow

Re: question wrt /drivers/char/tty_io.c in pre 2.6.16 code

[Greg KH]

On Wed, Aug 27, 2008 at 05:55:31PM +0200, Denis Joseph Barrow wrote:
> Hi all,
> In kernels prior to 2.6.16

Hm, why do we care about kernels that are 2 1/2 years old?  We can't do
anything to modify them at this point in time and the only ones
supporting them are the enterprise distros.

thanks,

greg k-h

Re: question wrt /drivers/char/tty_io.c in pre 2.6.16 code

[Alan Cox]

> The new tty_insert_flip_string returns the
> number of bytes successfully copied to tty recieve buffers
> how do I implement this functionality in pre 2.6.16 code,
> can it be done?

Probably but it'll be really ugly. You need to stick your nose into the
innards of struct tty_struct and check the space left in the current flip
buffer entry.

What you do if it is full is another question.

Alan.

Re: question wrt /drivers/char/tty_io.c in pre 2.6.16 code

[Denis Joseph Barrow]

Thanks Greg/Alan,
As for maintaining 2 and a half year old distros that's my job.
I think I figured out a way of doing the flow control.
I need to wrap the line discipline function pointer
disc->ops->receive_buf(tty, char_buf,flag_buf, count);
in /drivers/char/tty_io.c for my hso driver anyway
I can put in a counter in this wrapper decrementing
the buffers full & increment the counter each time
I call tty_flip_buffer_push(tty); which I will only
do if there are less than 2 buffers busy.

If this idea is plain wrong let me know.

Alan Cox wrote:
>> The new tty_insert_flip_string returns the
>> number of bytes successfully copied to tty recieve buffers
>> how do I implement this functionality in pre 2.6.16 code,
>> can it be done?
> 
> Probably but it'll be really ugly. You need to stick your nose into the
> innards of struct tty_struct and check the space left in the current flip
> buffer entry.
> 
> What you do if it is full is another question.
> 
> Alan.


-- 
best regards,
D.J. Barrow

[patch 02/02] hso: dev_kfree_skb crash fix

[greg]

[-- Attachment #1: hso-dev_kfree_skb-crash-fix.patch --]
[-- Type: text/plain, Size: 702 bytes --]

From: Denis Joseph Barrow <D.Barow@option.com>

Fixes dev_kfree_skb happening too many times when hso_start_net_device
is called from hso_resume.

Signed-off-by: Denis Joseph Barrow <D.Barow@option.com>
Cc: Jeff Garzik <jgarzik@pobox.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/net/usb/hso.c |    1 +
 1 file changed, 1 insertion(+)

--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2613,6 +2613,7 @@ static int hso_resume(struct usb_interfa
 					"Transmitting lingering data\n");
 				hso_net_start_xmit(hso_net->skb_tx_buf,
 						   hso_net->net);
+				hso_net->skb_tx_buf = NULL;
 			}
 			result = hso_start_net_device(network_table[i]);
 			if (result)

--