Skip to contentSkip to conversation

2 more hso.c patches

PATCH In review [email protected] 4 participants 9 messages Aug 20, 2008 – Aug 28, 2008

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover 2 more hso.c patches

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

#1 lore raw

PATCH 1/2 hso: icon 322 detection fix

From: Denis Joseph Barrow <[email protected]>

Fixes Icon-322 detection.

---
 drivers/net/usb/hso.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/usb/hso.c +1 -1
@@ -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 */
Signature · 4 lines
--
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
#3 lore raw

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

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

Quoted 6 lines
		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

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

#4 lore raw

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

In reply to Jeff Garzik

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

author
#5 lore raw

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

Quoted 2 lines · Denis Joseph Barrow

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

#6 lore raw

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

In reply to Denis Joseph Barrow
Quoted 4 lines

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.
--
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

#7 lore raw

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

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.

Quoted 12 lines · Alan Cox
Quoted 4 lines

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.

Signature · 2 lines
best regards,
D.J. Barrow
#8 lore raw

PATCH 2/2 hso: dev_kfree_skb crash fix

In reply to [email protected]

From: Denis Joseph Barrow <[email protected]>

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

---
 drivers/net/usb/hso.c |    1 +
 1 file changed, 1 insertion(+)
drivers/net/usb/hso.c +1 -0
@@ -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)
Signature · 4 lines
--
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