Hi Dan, Am 17.12.2013 um 23:27 schrieb Dan Williams:
> On Tue, 2013-12-17 at 20:56 +0100, Dr. H. Nikolaus Schaller wrote: >> Hi Dan, >> >> Am 16.12.2013 um 20:40 schrieb Dan Williams: >> >>> On Fri, 2013-12-13 at 15:43 +0100, Dr. H. Nikolaus Schaller wrote: >>>> Hi, >>>> >>>> Am 02.10.2013 um 09:00 schrieb Dr. H. Nikolaus Schaller: >>>> >>>>> Hi Jan, >>>>> >>>>> we are using a GTM601 modem (Firmware 1.7) for a while and have spotted an >>>>> issue that under some conditions the modem sends a packed wIndex over USB >>>>> that is rejected by the hso driver making troubles afterwards. Not >>>>> rejecting makes >>>>> it work fine. >>>>> >>>>> BR, >>>>> Nikolaus Schaller >>>>> >>>>> --- >>>>> >>>>> From f5c7e15b61f2ce4fe3105ff914f6bfaf5d74af0d Mon Sep 17 00:00:00 2001 >>>>> From: "H. Nikolaus Schaller" <h...@goldelico.com> >>>>> Date: Thu, 15 Nov 2012 14:40:57 +0100 >>>>> Subject: [PATCH 1/1] hso: fix problem with wrong status code sent by >>>>> OPTION >>>>> GTM601 during RING indication >>>>> >>>>> It has been observed that the GTM601 with 1.7 firmware sometimes sends a >>>>> value >>>>> wIndex that has bit 0x04 set instead of being reset as the code expects. >>>>> So we >>>>> mask it for the error check. >>>>> >>>>> See >>>>> http://lists.goldelico.com/pipermail/gta04-owner/2012-February/001643.html >>>>> >>>>> Signed-off-by: NeilBrown <ne...@suse.de> >>>>> Signed-off-by: H. Nikolaus Schaller <h...@goldelico.de> >>>>> --- >>>>> drivers/net/usb/hso.c | 3 ++- >>>>> 1 files changed, 2 insertions(+), 1 deletions(-) >>>>> >>>>> diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c >>>>> index cba1d46..d146e26 100644 >>>>> --- a/drivers/net/usb/hso.c >>>>> +++ b/drivers/net/usb/hso.c >>>>> @@ -1503,7 +1503,8 @@ static void tiocmget_intr_callback(struct urb *urb) >>>>> if (serial_state_notification->bmRequestType != BM_REQUEST_TYPE || >>>>> serial_state_notification->bNotification != B_NOTIFICATION || >>>>> le16_to_cpu(serial_state_notification->wValue) != W_VALUE || >>>>> - le16_to_cpu(serial_state_notification->wIndex) != W_INDEX || >>>>> + (le16_to_cpu(serial_state_notification->wIndex) & ~0x4) != >>>>> + W_INDEX || >>>>> le16_to_cpu(serial_state_notification->wLength) != W_LENGTH) { >>>>> dev_warn(&usb->dev, >>>>> "hso received invalid serial state notification\n"); >>>>> -- >>>>> 1.7.7.4 >>>>> >>>>> >>>> >>>> I have found this (better) proposal by OPTION, but wonder what did happen >>>> to it. >>>> It neither shows in mainline 3.13-rc nor linux-next: >>>> >>>> https://lkml.org/lkml/2013/10/9/263 >>> >>> Likely because nobody formally submitted the patch with a signed-off-by, >>> which indicates their intent that the patch is tested, correct, and can >>> be merged to the kernel. >> >> Ok, I see. I just wasn't aware of the proposal at all since I missed the >> discussion >> and wasn't on CC. >> >> Therefore I have added Eric to the CC. >> >>> >>> I looked at this today, and I'm left wondering how any port other than >>> HSO_PORT_MODEM can get the notification via tiocmget_intr_callback(). >>> "serial->tiocmget" is only created for HSO_PORT_MODEM serial ports (see >>> hso_create_bulk_serial_device()): >>> >>> if ((port & HSO_PORT_MASK) == HSO_PORT_MODEM) { >>> num_urbs = 2; >>> serial->tiocmget = kzalloc(sizeof(struct hso_tiocmget), >>> GFP_KERNEL); >>> >>> and the code in tiocmget_intr_callback() does this: >>> >>> tiocmget = serial->tiocmget; >>> if (!tiocmget) >>> return; >>> >>> which should mean that only a HSO_PORT_MODEM will ever process the >>> notification. Further, the tiocmget interrupt URB is only created and >>> submitted if serial->tiocmget != NULL, so only HSO_PORT_MODEM should >>> ever be calling into tiocmget_intr_callback(). >> >> Ok, that looks plausible. >> >>> >>> So I think Eric's patch is actually wrong because it will *always* pass >>> the new check. >>> >>> The original code had the correct intention, but the original code was >>> obviously wrong for newer devices where the port layout is read from >>> firmware and not from static tables, and thus for recent devices where >>> the modem interface is not USB interface #2. >> >> This explains why we did run into the problem with the GTM601. >> >>> >>> Can you confirm/deny that the 'modem' interface for your GTM601 is USB >>> interface #6? For example, my Icon 452 has the following USB interface >>> layout: >>> >>> 0: Diag >>> 1: GPS >>> 2: GPS control >>> 3: Application >>> 4: Control >>> 5: Network >>> 6: Modem >>> >>> So like the GTM601, I would expect any RING notifications to appear for >>> wIndex=0x06. >> >> Interestingly I see: >> >> 0: Diagnostic >> 1: GPS >> 2: GPS Control >> 3: Application >> 4: Control >> 5: Modem >> >> I.e. there is no Network interface. Originally we did focus on /dev/ttyHS3 > > How are you determining the number here? Are you using: > > cat /sys/class/tty/ttyHS_Modem/device/bInterfaceNumber > > to determine the actual USB interface number associated with the Modem > port? Or are you going off the pre-udev-rename ttyHSx numbers? Ah, I did use root@gta04:~# ls -d /sys/class/tty/ttyHS*; cat /sys/class/tty/ttyHS*/hsotype /sys/class/tty/ttyHS0 /sys/class/tty/ttyHS1 /sys/class/tty/ttyHS2 /sys/class/tty/ttyHS3 /sys/class/tty/ttyHS4 /sys/class/tty/ttyHS5 Diagnostic GPS GPS Control Application Control Modem With bInterfaceNumber I get root@gta04:~# cat /sys/class/tty/ttyHS*/device/bInterfaceNumber 00 01 02 03 04 06 So the Modem is indeed interface 06 and there is just no ttyHS_Network. > >> to access the Application interface, but people reported that it is not >> stable >> or always so. Therefore we have some udev rule to add symbolic names >> (e.g. /dev/ttyHS_Application): >> >> http://git.goldelico.com/?p=gta04-kernel.git;a=blob;f=GTA04/udev-rules/hso.rules;hb=HEAD >> >> So I think the assignment is not even always the same on the same device. > > The assignment will always be the same on the same device *and the same > firmware*. That is because the hso driver asks the firmware for the > port layout, and that is what is put into the 'hsotype' file in sysfs > that the udev rules use. So if your telephony/WWAN stack is using the > udev symbolic links or reading 'hsotype' directly, you can be assured > that ttyHS_Modem is always the port you should be using for PPP. > >> And I am a little puzzled why we do see the wIndex == 6 if it is the >> interface >> number. Maybe it happens only in some scenario where the Modem becomes >> interface #6. > > I'm still not convinced that that it isn't #6 until we agree on how > we're figuring that out :) I may well assume to little of you, so > forgive me if you are actually reporting the real USB interface # of the > ports. But I've seen too many people confuse the tty numbers with USB > interface numbers, so I must ask. Yes, I did the mistake as you describe... > >>> >>> Does the following patch fix the problem for you? >> >> I will try but need a little time to setup a test scenario (because I need >> to trigger >> RING indications) and will then report. > > Here's a RING on the 452 with Modem @ usbif #6: > > [65808.711323] tiocmget_intr_callback: W_INDEX 0006 > [65808.711330] tiocmget_intr_callback: UART bits 000a > [65808.711333] tiocmget_intr_callback: Modem ifnum 06 > > and B_TX_CARRIER notification with GIO322 with Modem @ usbif #2: > > [66029.365441] tiocmget_intr_callback: W_INDEX 0002 > [66029.365444] tiocmget_intr_callback: UART bits 0002 > [66029.365445] tiocmget_intr_callback: Modem ifnum 02 Hm. I didn't see these messages (on 3.12). How do I enable them? BTW: the RING (or +CRING:) text messages do appear on the Application port, while the NO CARRIER after ATA and ending the call appears on the Modem port. BR, Nikolaus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
