Re: power management problems in ehci-omap

[H. Nikolaus Schaller]

Hi,
Andreas seems to be travelling back from FOSDEM so I jump in with best of my 
knowledge.

> Am 04.02.2018 um 12:34 schrieb Michael Nazzareno Trimarchi 
> <mich...@amarulasolutions.com>:
> 
> Hi
> 
> On Sun, Feb 4, 2018 at 12:07 PM, H. Nikolaus Schaller <h...@goldelico.com> 
> wrote:
>> Hi Michael,
>> 
>>> Am 04.02.2018 um 11:55 schrieb Michael Nazzareno Trimarchi 
>>> <mich...@amarulasolutions.com>:
>>> 
>>> Hi Andreas
>>> 
>>> On Sun, Feb 4, 2018 at 11:50 AM, Andreas Kemnade <andr...@kemnade.info> 
>>> wrote:
>>>> Hi,
>>>> 
>>>> On Sun, 4 Feb 2018 09:43:45 +0100
>>>> Michael Nazzareno Trimarchi <mich...@amarulasolutions.com> wrote:
>>>> 
>>>>> Hi Andreas
>>>>> 
>>>>> On Sun, Feb 4, 2018 at 9:38 AM, Andreas Kemnade <andr...@kemnade.info> 
>>>>> wrote:
>>>>>> On Sun, 4 Feb 2018 00:10:50 +0100
>>>>>> Michael Nazzareno Trimarchi <mich...@amarulasolutions.com> wrote:
>>>>>> 
>>>>>>> Hi
>>>>>>> 
>>>>>>> On Sun, Feb 4, 2018 at 12:03 AM, Andreas Kemnade <andr...@kemnade.info> 
>>>>>>> wrote:
>>>>>>>> Hi,
>>>>>>>> 
>>>>>>>> I booted a 4.15 kernel without udev and loaded modules piece by piece 
>>>>>>>> to analyze
>>>>>>>> pm problems. modprobe ehci-omap increases current by around 35mA and
>>>>>>>> also rmmod ehci-omap does not let it go down at all.
>>>>>>>> 
>>>>>>>> I expect that removing hardware does the same thing
>>>>>> nonsense sentence from me, was to tired. I would expect that removing 
>>>>>> the modules
>>>>>> properly powers down the device.
>>>>>>>> 
>>>>>>>> Also suspend current increases by around 15mA if that module is loaded.
>>>>>>>> I tested with having everything disabled which is attached to that usb 
>>>>>>>> bus.
>>>>>>>> 
>>>>>>> 
>>>>>>> Do you have an LTE connected to the usb?
>>>>>>> 
>>>>>> Yes, there is a UMTS modem attached, but it was off during the tests.
>>>>>> It did not enumerate on the modem.
>>>>>> 
>>>>> 
>>>>> Just to understand if the suspend current drop was connected to the
>>>>> suspend of lte modem on your side.
>>>>> So you don't have anything connected on usb bus?
>>>>> 
>>>> Suspend current is increased when the ehci-omap module is loaded
>>>> in comparison to the state. I tested with the modem disabled, so there
>>>> is nothing on the bus. Increased suspend current is one thing,
>>>> current_before_modprobe_ehci_omap != current_after_rmmod_ehci_omap.
>>>> 
>>>> I am testing with init=some_testscript.sh, so no userspace
>>>> is doing strange things. No module autoload or something.
>>>> 
>>> 
>>> How many port are configured and how is the phy part connected to the
>>> ehci controller?
>>> Can you point me the schematic page?
>> 
>> it is essentially a copy from BeagleBoard XM.
>> 
>> Schematics of the GTA04 are here:
>> 
>>http://projects.goldelico.com/p/gta04-main/downloads/48/
>> 
>> USB PHY is on page 9.
>> 
> 
> I see. GPIO174 is controlled by you during boot or connected to the dts?
> I don't find in mainline the connection to the physical and it's own
> programming.
> Can you check if the phy is in reset when module is remove?

The reset gpio is defined in the respective board-DTS:

https://elixir.free-electrons.com/linux/v4.15/source/arch/arm/boot/dts/omap3-beagle-xm.dts#L88
https://elixir.free-electrons.com/linux/v4.15/source/arch/arm/boot/dts/omap3-gta04.dtsi#L120

BR,
Nikolaus

--
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: power management problems in ehci-omap

[H. Nikolaus Schaller]

Hi Michael,

> Am 04.02.2018 um 11:55 schrieb Michael Nazzareno Trimarchi 
> :
> 
> Hi Andreas
> 
> On Sun, Feb 4, 2018 at 11:50 AM, Andreas Kemnade  wrote:
>> Hi,
>> 
>> On Sun, 4 Feb 2018 09:43:45 +0100
>> Michael Nazzareno Trimarchi  wrote:
>> 
>>> Hi Andreas
>>> 
>>> On Sun, Feb 4, 2018 at 9:38 AM, Andreas Kemnade  
>>> wrote:
 On Sun, 4 Feb 2018 00:10:50 +0100
 Michael Nazzareno Trimarchi  wrote:
 
> Hi
> 
> On Sun, Feb 4, 2018 at 12:03 AM, Andreas Kemnade  
> wrote:
>> Hi,
>> 
>> I booted a 4.15 kernel without udev and loaded modules piece by piece to 
>> analyze
>> pm problems. modprobe ehci-omap increases current by around 35mA and
>> also rmmod ehci-omap does not let it go down at all.
>> 
>> I expect that removing hardware does the same thing
 nonsense sentence from me, was to tired. I would expect that removing the 
 modules
 properly powers down the device.
>> 
>> Also suspend current increases by around 15mA if that module is loaded.
>> I tested with having everything disabled which is attached to that usb 
>> bus.
>> 
> 
> Do you have an LTE connected to the usb?
> 
 Yes, there is a UMTS modem attached, but it was off during the tests.
 It did not enumerate on the modem.
 
>>> 
>>> Just to understand if the suspend current drop was connected to the
>>> suspend of lte modem on your side.
>>> So you don't have anything connected on usb bus?
>>> 
>> Suspend current is increased when the ehci-omap module is loaded
>> in comparison to the state. I tested with the modem disabled, so there
>> is nothing on the bus. Increased suspend current is one thing,
>> current_before_modprobe_ehci_omap != current_after_rmmod_ehci_omap.
>> 
>> I am testing with init=some_testscript.sh, so no userspace
>> is doing strange things. No module autoload or something.
>> 
> 
> How many port are configured and how is the phy part connected to the
> ehci controller?
> Can you point me the schematic page?

it is essentially a copy from BeagleBoard XM.

Schematics of the GTA04 are here:

http://projects.goldelico.com/p/gta04-main/downloads/48/

USB PHY is on page 9.

BR,
Nikolaus

> 
> michael
> 
> 
>> Regards,
>> Andreas
> 
> 
> 
> -- 
> | Michael Nazzareno Trimarchi Amarula Solutions BV |
> | COO  -  Founder  Cruquiuskade 47 |
> | +31(0)851119172 Amsterdam 1018 AM NL |
> |  [`as] http://www.amarulasolutions.com   |
> ___
> http://projects.goldelico.com/p/gta04-kernel/
> Letux-kernel mailing list
> letux-ker...@openphoenux.org
> http://lists.goldelico.com/mailman/listinfo.cgi/letux-kernel

--
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: [Letux-kernel] [PATCH v2] musb: omap2430: do not assume balanced enable()/disable()

[H. Nikolaus Schaller]

Hi Tony,

> Am 04.08.2016 um 16:29 schrieb Tony Lindgren <t...@atomide.com>:
> 
> Hi,
> 
> * H. Nikolaus Schaller <h...@goldelico.com> [160803 10:07]:
>> All this prevents detection of cable plugin-events and VBUS measurement
>> and setting OTG_EN before charging is attempted.
> 
> So I gave this patch a try but it now blocks all deeper SoC idle
> states as the PHY stays active. I think the real fix is to make
> sure the charger behaves independent of the USB PHY state.

IMHO, plugin detection of the cable is a phy task and then it tells
the charger to start. This part works.

Charging did work up to kernel 4.3. It started to fail with 4.4-rc1
without obvious changes to the charger but many patches to phy
and musb. We had even backported the 4.7 charger driver
to 4.3 and it failed as well.

> So
> probably this needs to be fixed in phy-twl4030-usb.c and
> twl4030_charger.c instead. Now it sounds like we're also shutting
> down the charger with the USB PHY.

As a very deeply hidden side-effect the charger is shut down immediately
after being started. Because phy-twl4030-usb.c does not do what it is expected
and told to do.

I have developed a workaround for the charger driver but I do not consider it
as the solution.

http://git.goldelico.com/?p=gta04-kernel.git;a=commit;h=b8c538e75c6dd034889bdb0d66e00ca6e128e616

> power:twl4030_charger: hack to set POWER_CTRL_OTG_ENAB what twl4030-phy does 
> not do
> 
> This hack is a workaround in the charger driver to do what the  twl4030-usb
> driver is expected to have done. It is designed in a way that it can be
> removed after the twl4030-usb issue is solved, but it does not harm if it
> isn't removed immediately.
> 
> Rationale:
> 
> The charger driver calls pm_runtime_get_sync(bci->transceiver->dev);
> which should indirectly call twl4030_usb_set_mode to set the
> POWER_CTRL_OTG_ENAB bit. This enables the prescaler hardware
> for ADC8 (VBUS) channel. But this does not happen for reasons outside
> the charger driver.
> 
> If this bit is not set there are strange effects:
> * VBUS reports 0mV through MADC
> * the automatic charging stops after some ms
> * ITHEN is disabled automatically and the temperature
>   reports 56°C through MADC
> 
> The TPS65950 TRM says:
> "The prescaler for the ADCIN8 channel is in the On-The-Go (OTG) module of
> the USB subchip. This prescaler is enabled when the OTG is enabled by
> writing 1 to the OTG_EN bit of the POWER_CTRL register of the USB subchip."
> 
> and
> 
> "The software must set the POWER_CTRL[5] OTG_EN bit to 1 at least 50 ms
> before forcing the BCIMFSTS4[2] USBFASTMCHG bit to 1."
> 
> For unknown reasons this does not happen with current phy-twl4030-usb code.
> 
> Therefore we add a hack that ensures that this bit is set and the 50ms
> delay is done.
> 
> It appears as if this problem occurred for the first time in linux 4.4-rc1.


With that we have a workaround in the charger, but not a correct solution.
That is what Andreas is trying to fix. The charger driver seems to be ok to
me.

Hope this helps to better understand what is going wrong in phy4030 or musb.

BR,
Nikolaus

--
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: [Letux-kernel] [PATCH v2] musb: omap2430: do not assume balanced enable()/disable()

[H. Nikolaus Schaller]

> Am 03.08.2016 um 17:38 schrieb Andreas Kemnade :
> 
> The code assumes that omap2430_musb_enable() and
> omap2430_musb_disable() are called in a balanced way.
> That fact is broken by the fact that musb_init_controller() calls
> musb_platform_disable() to switch from unknown state to off state
> on initialisation.
> 
> That means that phy_power_off() is called first so that
> phy->power_count gets -1 and the phy is not enabled on phy_power_on().
> So when usb gadget is started the phy is not powered on.
> Depending on the phy used that caused various problems.
> Besides of causing usb problems, that can also have side effects.
> 
> In the case of using the phy_twl4030, that prevents also charging
> the battery via usb (using twl4030_charger) and so makes further
> kernel debugging hard.
> The problem was seen with 4.7 on an openphoenux gta04. It has a DM3730
> SoC and a TPS65950 companion.  phy->power never became 1

s/TPS65950/TPS65950 (twl4030)/

> and so the usb did get powered on.

s/did get/did not get/

maybe add:

All this prevents detection of cable plugin-events and VBUS measurement
and setting OTG_EN before charging is attempted.

> 
> The patch prevents phy_power_off() from being called when
> it is already off.
> 
> Signed-off-by: Andreas Kemnade 
> ---
> changes in v2:
> improved commit message
> 
> drivers/usb/musb/omap2430.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> index 0b4cec9..c1a2b7b 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -413,9 +413,10 @@ static void omap2430_musb_disable(struct musb *musb)
>   struct device *dev = musb->controller;
>   struct omap2430_glue *glue = dev_get_drvdata(dev->parent);
> 
> - if (!WARN_ON(!musb->phy))
> - phy_power_off(musb->phy);
> -
> + if (glue->enabled) {
> + if (!WARN_ON(!musb->phy))
> + phy_power_off(musb->phy);
> + }
>   if (glue->status != MUSB_UNKNOWN)
>   omap_control_usb_set_mode(glue->control_otghs,
>   USB_MODE_DISCONNECT);
> -- 
> 2.1.4
> 
> ___
> http://projects.goldelico.com/p/gta04-kernel/
> Letux-kernel mailing list
> letux-ker...@openphoenux.org
> http://lists.goldelico.com/mailman/listinfo.cgi/letux-kernel

--
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] Documentation: usb: serial: fixed how to provide vendor and product id

[Dr. H. Nikolaus Schaller]

Am 02.04.2015 um 12:35 schrieb Johan Hovold jo...@kernel.org:

 On Thu, Apr 02, 2015 at 12:12:37PM +0200, Jonathan Corbet wrote:
 On Sat, 28 Mar 2015 14:37:57 +0100
 Marek Belisko ma...@goldelico.com wrote:
 
 This patch changes the documentation file as needed.
 
 Applied to the docs tree (with the changelog typo fixed).
 
 This patch was incorrect and needed to be updated as discussed in the
 thread. Can you drop it or is your tree immutable?

Yes, please.
I did not find time to test Johan’s valuable comments.

BR,
Nikolaus

--
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] Documentation: usb: serial: fixed how to provide vendor and product id

[Dr. H. Nikolaus Schaller]

Hi,

Am 30.03.2015 um 12:08 schrieb Johan Hovold jo...@kernel.org:

 On Mon, Mar 30, 2015 at 11:32:39AM +0200, Dr. H. Nikolaus Schaller wrote:
 Hi Johan,
 
 Am 30.03.2015 um 09:45 schrieb Johan Hovold jo...@kernel.org:
 
 On Sat, Mar 28, 2015 at 02:37:57PM +0100, Marek Belisko wrote:
 From: H. Nikolaus Schaller h...@goldelico.com
 
 While trying to test the Pyra GSM/GPS/3G module I had reconfigured
 the USB interface by mistake and therefore needed to run a different
 USB driver than CSC-ACM. It turned out that I need the usbserial driver.
 
 CDC-ACM
 
 oh, thanks. There is always at least one more typo in any document…
 
 
 This file is an official description how to use it:
 Documentation/usb/usb-serial.txt
 
 But it is outdated. The parameters vendor= and product= are no longer
 available since ca. 3.12 which means that documentation is lagging behind
 quite some time.
 
 This is simply not true. The vendor and module parameters are still
 there for usbserial (i.e. the generic driver).
 
 Would you mind updating this patch to document the sysfs-method as an
 alternative instead? Using sysfs is preferred as that functionality can
 be used with any usb-serial driver and the drivers can also be compiled
 in.
 
 Hm. For me it did not work as described and therefore I searched for
 and found the solution cited below, which appears to confirm that it does
 not work any more. Only with these hints to use the /sys approach I
 could make it working immediately.
 
 Therefore I assumed that the documentation is outdated.
 
 But it might also be a hidden bug with the parameter passing.
 I had tested on 4.0-rc3.
 
 I can try a little more but it might be waste of time to find out why the old
 does not work, if there is a preferred method now.
 
 Please do try to figure out if it doesn't work as we have not deprecated
 this functionality yet. It should still work.

Good news:

I have tried again (this time with 4.0-rc5) and was successful. I have no
clue why it did not work with my tests with 4.0-rc3.

Here is the log:

 gta04 login: root
 Password: 
 Last login: Thu Apr  2 14:06:13 UTC 2015 on ttyO2
 Linux gta04 4.0.0-rc5-gta04+ #1022 SMP Mon Mar 23 08:13:29 CET 2015 armv7l
 
 The programs included with the Debian GNU/Linux system are free software;
 the exact distribution terms for each program are described in the
 individual files in /usr/share/doc/*/copyright.
 
 Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
 permitted by applicable law.
 root@gta04:~# lsusb
 Bus 001 Device 002: ID 0424:3503 Standard Microsystems Corp. 
 Bus 001 Device 003: ID 0424:9730 Standard Microsystems Corp. 
 Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
 root@gta04:~# rfkill unblock wwan
 [   40.443892] wwan_on_off_rfkill_set_block: blocked: 0
 [   40.449109] modem: set_power 0
 [   40.452318]   state 1
 [   40.454687] modem: send impulse
 [   41.174775] modem: done
 root@gta04:~# [   42.874901] usb 1-2.2: new high-speed USB device number 4 
 using ehci-omap
 [   43.008009] usb 1-2.2: New USB device found, idVendor=1e2d, idProduct=0053
 [   43.015211] usb 1-2.2: New USB device strings: Mfr=3, Product=2, 
 SerialNumber=0
 [   43.022859] usb 1-2.2: Product: PH8
 [   43.026525] usb 1-2.2: Manufacturer: Cinterion
 
 root@gta04:~# lsusb
 Bus 001 Device 002: ID 0424:3503 Standard Microsystems Corp. 
 Bus 001 Device 003: ID 0424:9730 Standard Microsystems Corp. 
 Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
 Bus 001 Device 004: ID 1e2d:0053  
 root@gta04:~# insmod 
 /lib/modules/4.0.0-rc5-gta04+/kernel/drivers/usb/serial/usbserial.ko 
 vendor=0x1e2d product=0x0053 
 [  172.699426] usbcore: registered new interface driver usbserial
 [  172.705714] usbcore: registered new interface driver usbserial_generic
 [  172.715819] usbserial: USB Serial support registered for generic
 [  172.722237] usbserial_generic 1-2.2:1.0: The generic usb-serial driver 
 is only for testing and one-off prototypes.
 [  172.733328] usbserial_generic 1-2.2:1.0: Tell linux-usb@vger.kernel.org to 
 add your device to a proper driver.
 [  172.743848] usbserial_generic 1-2.2:1.0: generic converter detected
 [  172.754681] usb 1-2.2: generic converter now attached to ttyUSB0
 [  172.761543] usbserial_generic 1-2.2:1.1: The generic usb-serial driver 
 is only for testing and one-off prototypes.
 [  172.772640] usbserial_generic 1-2.2:1.1: Tell linux-usb@vger.kernel.org to 
 add your device to a proper driver.
 [  172.783154] usbserial_generic 1-2.2:1.1: generic converter detected
 [  172.790765] usb 1-2.2: generic converter now attached to ttyUSB1
 [  172.797235] usbserial_generic 1-2.2:1.2: The generic usb-serial driver 
 is only for testing and one-off prototypes.
 [  172.808296] usbserial_generic 1-2.2:1.2: Tell linux-usb@vger.kernel.org to 
 add your device to a proper driver.
 [  172.818797] usbserial_generic 1-2.2:1.2: generic converter detected
 [  172.826135] usb 1-2.2: generic converter now attached to ttyUSB2

Re: [PATCH] Documentation: usb: serial: fixed how to provide vendor and product id

[Dr. H. Nikolaus Schaller]

Hi Johan,

Am 30.03.2015 um 09:45 schrieb Johan Hovold jo...@kernel.org:

 On Sat, Mar 28, 2015 at 02:37:57PM +0100, Marek Belisko wrote:
 From: H. Nikolaus Schaller h...@goldelico.com
 
 While trying to test the Pyra GSM/GPS/3G module I had reconfigured
 the USB interface by mistake and therefore needed to run a different
 USB driver than CSC-ACM. It turned out that I need the usbserial driver.
 
 CDC-ACM

oh, thanks. There is always at least one more typo in any document…

 
 This file is an official description how to use it:
 Documentation/usb/usb-serial.txt
 
 But it is outdated. The parameters vendor= and product= are no longer
 available since ca. 3.12 which means that documentation is lagging behind
 quite some time.
 
 This is simply not true. The vendor and module parameters are still
 there for usbserial (i.e. the generic driver).
 
 Would you mind updating this patch to document the sysfs-method as an
 alternative instead? Using sysfs is preferred as that functionality can
 be used with any usb-serial driver and the drivers can also be compiled
 in.

Hm. For me it did not work as described and therefore I searched for
and found the solution cited below, which appears to confirm that it does
not work any more. Only with these hints to use the /sys approach I
could make it working immediately.

Therefore I assumed that the documentation is outdated.

But it might also be a hidden bug with the parameter passing.
I had tested on 4.0-rc3.

I can try a little more but it might be waste of time to find out why the old
does not work, if there is a preferred method now.

 
 Here was the solution:
 
 https://bbs.archlinux.org/viewtopic.php?id=175499
 
  insmod usbserial vendor=0x product=0x
 
 becomes (first  is vendor, second is product)
 
  modprobe usbserial
  echo   /sys/bus/usb-serial/drivers/generic/new_id
 
 This patch changes the documentation file as needed.
 
 Signed-off-by: H. Nikolaus Schaller h...@goldelico.com
 Signed-off-by: Marek Belisko ma...@goldelico.com
 ---
 Documentation/usb/usb-serial.txt | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)
 
 diff --git a/Documentation/usb/usb-serial.txt 
 b/Documentation/usb/usb-serial.txt
 index 947fa62..9f184d8 100644
 --- a/Documentation/usb/usb-serial.txt
 +++ b/Documentation/usb/usb-serial.txt
 @@ -466,11 +466,13 @@ Generic Serial driver
   is required of your device is that it has at least one bulk in endpoint,
   or one bulk out endpoint. 
 
 -  To enable the generic driver to recognize your device, build the driver
 -  as a module and load it by the following invocation:
 -insmod usbserial vendor=0x product=0x
 -  where the  is replaced with the hex representation of your device's
 -  vendor id and product id.
 +  To enable the generic driver to recognize your device, configure
 +  USB_SERIAL_GENERIC=y and build the driver as a module. Then load it by the
 
 You don't need to build the driver as a module when using the sysfs
 method.

Ok, that is right and my formulation is too narrow on that.

So we just s/ as a module//.

 
 +  following invocation:
 +modprobe usbserial
 +echo   /sys/bus/usb-serial/drivers/generic/new_id
 +  where the the first  is replaced with the hex representation of your
 +  device's vendor id and the second  by the product id.
 
   This driver has been successfully used to connect to the NetChip USB
   development board, providing a way to develop USB firmware without
 
 Thanks,
 Johan

Thanks as well,
Nikolaus


--
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: [Gta04-owner] [PATCH] Documentation: usb: serial: fixed how to provide vendor and product id

[Dr. H. Nikolaus Schaller]

Am 30.03.2015 um 11:32 schrieb Dr. H. Nikolaus Schaller h...@goldelico.com:

 Hi Johan,
 
 Am 30.03.2015 um 09:45 schrieb Johan Hovold jo...@kernel.org:
 
 On Sat, Mar 28, 2015 at 02:37:57PM +0100, Marek Belisko wrote:
 From: H. Nikolaus Schaller h...@goldelico.com
 
 While trying to test the Pyra GSM/GPS/3G module I had reconfigured
 the USB interface by mistake and therefore needed to run a different
 USB driver than CSC-ACM. It turned out that I need the usbserial driver.
 
 CDC-ACM
 
 oh, thanks. There is always at least one more typo in any document…
 
 
 This file is an official description how to use it:
 Documentation/usb/usb-serial.txt
 
 But it is outdated. The parameters vendor= and product= are no longer
 available since ca. 3.12 which means that documentation is lagging behind
 quite some time.
 
 This is simply not true. The vendor and module parameters are still
 there for usbserial (i.e. the generic driver).
 
 Would you mind updating this patch to document the sysfs-method as an
 alternative instead? Using sysfs is preferred as that functionality can
 be used with any usb-serial driver and the drivers can also be compiled
 in.
 
 Hm. For me it did not work as described and therefore I searched for
 and found the solution cited below, which appears to confirm that it does
 not work any more. Only with these hints to use the /sys approach I
 could make it working immediately.
 
 Therefore I assumed that the documentation is outdated.
 
 But it might also be a hidden bug with the parameter passing.
 I had tested on 4.0-rc3.
 
 I can try a little more but it might be waste of time to find out why the old
 does not work, if there is a preferred method now.
 
 
 Here was the solution:
 
 https://bbs.archlinux.org/viewtopic.php?id=175499
 
 insmod usbserial vendor=0x product=0x
 
 becomes (first  is vendor, second is product)
 
 modprobe usbserial
 echo   /sys/bus/usb-serial/drivers/generic/new_id
 
 This patch changes the documentation file as needed.
 
 Signed-off-by: H. Nikolaus Schaller h...@goldelico.com
 Signed-off-by: Marek Belisko ma...@goldelico.com
 ---
 Documentation/usb/usb-serial.txt | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)
 
 diff --git a/Documentation/usb/usb-serial.txt 
 b/Documentation/usb/usb-serial.txt
 index 947fa62..9f184d8 100644
 --- a/Documentation/usb/usb-serial.txt
 +++ b/Documentation/usb/usb-serial.txt
 @@ -466,11 +466,13 @@ Generic Serial driver
  is required of your device is that it has at least one bulk in endpoint,
  or one bulk out endpoint. 
 
 -  To enable the generic driver to recognize your device, build the driver
 -  as a module and load it by the following invocation:
 -   insmod usbserial vendor=0x product=0x
 -  where the  is replaced with the hex representation of your device's
 -  vendor id and product id.
 +  To enable the generic driver to recognize your device, configure
 +  USB_SERIAL_GENERIC=y and build the driver as a module. Then load it by 
 the
 
 You don't need to build the driver as a module when using the sysfs
 method.
 
 Ok, that is right and my formulation is too narrow on that.

Well, I should add that not my formulation was too narrow but already the 
original.
 
 So we just s/ as a module//.
 
 
 +  following invocation:
 +   modprobe usbserial
 +   echo   /sys/bus/usb-serial/drivers/generic/new_id
 +  where the the first  is replaced with the hex representation of your
 +  device's vendor id and the second  by the product id.
 
  This driver has been successfully used to connect to the NetChip USB
  development board, providing a way to develop USB firmware without
 
 Thanks,
 Johan
 
 Thanks as well,
 Nikolaus
 
 
 ___
 Gta04-owner mailing list
 gta04-ow...@goldelico.com
 http://lists.goldelico.com/mailman/listinfo.cgi/gta04-owner

--
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] Documentation: usb: serial: fixed how to provide vendor and product id

[Dr. H. Nikolaus Schaller]

Am 26.03.2015 um 11:41 schrieb Belisko Marek marek.beli...@gmail.com:

 On Thu, Mar 26, 2015 at 10:53 AM, Greg KH gre...@linuxfoundation.org wrote:
 On Wed, Mar 25, 2015 at 09:53:34PM +0100, Marek Belisko wrote:
 From: H. Nikolaus Schaller h...@goldelico.com
 
 While trying to test the Pyra GSM/GPS/3G module I had reconfigured
 the USB interface by mistake and therefore needed to run a different
 USB driver than CSC-ACM. It turned out that I need the usbserial driver.
 
 This file is an official description how to use it:
 Documentation/usb/usb-serial.txt
 
 But it is outdated. The parameters vendor= and product= are no longer
 available since ca. 3.12 which means that documentation is lagging behind
 quite some time.
 
 Here was the solution:
 
 https://bbs.archlinux.org/viewtopic.php?id=175499
 
  insmod usbserial vendor=0x product=0x
 
 becomes (first  is vendor, second is product)
 
  modprobe usbserial
  echo   /sys/bus/usb-serial/drivers/generic/new_id
 
 This patch changes the documentation file as needed.
 
 Signed-off-by: H. Nikolaus Schaller h...@goldelico.com
 
 You are forwarding on a patch from someone else, why didn't you also
 sign-off on it?  Do you not agree with it?
 I wasn’t involved in patch development I just review it and forward.

Yes,
I asked Marek to submit this patch (from our local repo).

BR,
Nikolaus

 
 thanks,
 
 greg k-h
 
 BR,
 
 marek
 
 -- 
 as simple and primitive as possible
 -
 Marek Belisko - OPEN-NANDRA
 Freelance Developer
 
 Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
 Tel: +421 915 052 184
 skype: marekwhite
 twitter: #opennandra
 web: http://open-nandra.com

--
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 of 1]: musb: fixed a potential NULL pointer dereference.

[Dr. H. Nikolaus Schaller]

fixed a potential NULL pointer dereference.

Rationale:
this is the only location in the musb driver where the
otg-gadget pointer is dereferenced. Assuming that it
is never NULL is not only potentially unsafe but was
observed in the wild on a GTA04 (OMAP3/TPS65950 based
board) when trying to boot a device tree based 3.14-rc2
kernel with USB cable plugged in.

DT boot appears to modify the order in which components
(gadget driver) are loaded and linked and therefore
an early musb interrupt triggers with a NULL gadget
pointer ending in a kernel panic.

Since a non-existing gadget can never be active we
simply use a 0 value for musb-is_active.

Signed-off-by: H. Nikolaus Schaller h...@goldelico.com




0001-fixed-a-potential-NULL-pointer-dereference.patch
Description: Binary data

Re: [PATCH 1 of 1]: musb: fixed a potential NULL pointer dereference.

[Dr. H. Nikolaus Schaller]

Am 12.02.2014 um 15:29 schrieb Greg Kroah-Hartman:

 On Wed, Feb 12, 2014 at 11:08:22AM +0100, Dr. H. Nikolaus Schaller wrote:
 fixed a potential NULL pointer dereference.
 
Rationale:
this is the only location in the musb driver where the
otg-gadget pointer is dereferenced. Assuming that it
is never NULL is not only potentially unsafe but was
observed in the wild on a GTA04 (OMAP3/TPS65950 based
board) when trying to boot a device tree based 3.14-rc2
kernel with USB cable plugged in.
 
DT boot appears to modify the order in which components
(gadget driver) are loaded and linked and therefore
an early musb interrupt triggers with a NULL gadget
pointer ending in a kernel panic.
 
Since a non-existing gadget can never be active we
simply use a 0 value for musb-is_active.
 
Signed-off-by: H. Nikolaus Schaller h...@goldelico.com
 
 Please don't attach patches, our tools don't like them at all.

Sorry. That was by accident. Here is the patch appended.

 
 And shouldn't we fix the root problem here, not just gloss over the fact
 that this pointer is NULL at this point in time?  Fixing the real issue
 should be the correct solution, right?

Well,
I must admit that I have no idea what the root cause could be 
(because I neither know the musb nor the gadget subsystems)
or if it is intended and just something in the initialization
sequence has changed.

I have done one more test and it appears to be in a non-device-tree
3.14-rc1 as well. We did not see it in a 3.12.7 kernel on the same hardware.

 
 thanks,
 
 greg k-h


BR,
Nikolaus Schaller

---
 drivers/usb/musb/musb_core.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index df3f65d..f68afef 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -653,7 +653,8 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 
int_usb,
break;
case OTG_STATE_B_PERIPHERAL:
musb_g_suspend(musb);
-   musb-is_active = otg-gadget-b_hnp_enable;
+   musb-is_active =
+   otg-gadget ? otg-gadget-b_hnp_enable : 0;
if (musb-is_active) {
musb-xceiv-state = OTG_STATE_B_WAIT_ACON;
dev_dbg(musb-controller, HNP: Setting timer 
for b_ase0_brst\n);
-- 
1.7.7.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

Re: [PATCH 1/1] hso: fix problem with wrong status code sent by OPTION GTM601 during RING indication

[Dr. H. Nikolaus Schaller]

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

Re: [PATCH 1/1] hso: fix problem with wrong status code sent by OPTION GTM601 during RING indication

[Dr. H. Nikolaus Schaller]

Hi Dan,

Am 18.12.2013 um 18:49 schrieb Dan Williams:

 On Wed, 2013-12-18 at 14:16 +0100, Dr. H. Nikolaus Schaller wrote:
 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

Re: [PATCH 1/1] hso: fix problem with wrong status code sent by OPTION GTM601 during RING indication

[Dr. H. Nikolaus Schaller]

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

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.

 
 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

Re: [PATCH 1/1] hso: fix problem with wrong status code sent by OPTION GTM601 during RING indication

[Dr. H. Nikolaus Schaller]

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

BR,
Nikolaus

--
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/1] hso: fix problem with wrong status code sent by OPTION GTM601 during RING indication

[Dr. H. Nikolaus Schaller]

Am 03.10.2013 um 22:00 schrieb David Miller:

 From: Dr. H. Nikolaus Schaller h...@goldelico.com
 Date: Thu, 3 Oct 2013 21:40:34 +0200
 
 I have made the bug observation from debug log that this bit is set in a 
 response
 each time the modem has a RING message. It might be specific to this modem
 and firmware version, i.e. a firmware bug. But we have no means to verify or 
 even
 change it in the firmware.
 
 I.e. the driver must handle it in a better way.
 
 Because the notification is rejected by the driver, the driver will hang up 
 and the
 whole modem connection breaks down.
 
 With this patch, the problem was never observed again in ~2 years.
 
 I'd hope the maintainer of this driver can shed some light on it.
 
 I think if you suspect the bit is set in response to RING messages
 then you should define a macro so that the number is not just magic,
 and put a descriptrive comment above the macro definition for that
 bit.

Good idea! I will resend the patch.

--- hns--
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/1] hso: fix problem with wrong status code sent by OPTION GTM601 during RING indication

[Dr. H. Nikolaus Schaller]

Hi,

Am 03.10.2013 um 21:29 schrieb David Miller:

 From: Dr. H. Nikolaus Schaller h...@goldelico.com
 Date: Wed, 2 Oct 2013 09:00:18 +0200
 
 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
 
 I think we should look more deeply into what this bit might mean
 and why the firmware might be setting it before we even consider
 applying a patch like this one.

Yes, that would be the right way.

The problem is that the firmware is a black box and the packet format has no
(public) documentation (unless someone inside OPTION has access to it).

I have made the bug observation from debug log that this bit is set in a 
response
each time the modem has a RING message. It might be specific to this modem
and firmware version, i.e. a firmware bug. But we have no means to verify or 
even
change it in the firmware.

I.e. the driver must handle it in a better way.

Because the notification is rejected by the driver, the driver will hang up and 
the
whole modem connection breaks down.

With this patch, the problem was never observed again in ~2 years.

I'd hope the maintainer of this driver can shed some light on it.

BR,
Nikolaus Schaller

--
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/1] hso: fix problem with wrong status code sent by OPTION GTM601 during RING indication

[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


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