Re: [PATCH v1 0/2] usb: dwc2: gadget: Fixes for LPM

[Stefan Wahren]

Hi Simon,


Am 07.05.2018 um 14:48 schrieb Simon Shields:

Hi Grigor,

On Wed, May 02, 2018 at 10:12:27AM +, Grigor Tovmasyan wrote:

Hi Simon,

On 4/21/2018 4:52 PM, Simon Shields wrote:

Hi Grigor,

On Fri, Apr 20, 2018 at 01:00:16PM +, Grigor Tovmasyan wrote:

Hi Simon,

On 4/19/2018 8:31 PM, Simon Shields wrote:

Hi all,

On 10/04/2018 10:21 PM, Grigor Tovmasyan wrote:

Here are two little fixes for LPM feature.

First one is coverity warning fix.

The Second one was asserted by Stefan Wahren.

Changes from version 0:

1/2:
   - Instead of converting parameter in the CHECK_RANGE macro
 to int, changed hird_threshold type from u8 to int.


Grigor Tovmasyan (2):
  usb: dwc2: gadget: Fix coverity issue
  usb: dwc2: gadget: Change LPM default values

 drivers/usb/dwc2/core.h   | 2 +-
 drivers/usb/dwc2/params.c | 8 
 2 files changed, 5 insertions(+), 5 deletions(-)

The second patch in this series fixes a regression in 4.17-rc1 using dwc2 in 
gadget
mode on Exynos4412, introduced by commit 7455f8b7f0b3 ("usb: dwc2: Enable
LPM"). The regression is that using the cdc_acm serial gadget (and
presumably other gadgets) serial console output will only sporadically
show up on the host (it seems to only show up as input is sent).


The second patch is not fix for described by you issue. We will try to
reproduce your issue and provide fix. Could you provide some logs or usb
traces for issue?

Here's a log[0]. The log is pretty big: I generated it with both regular and 
verbose
debugging enabled for DWC2. However, I suspect the relevant line is
probably:

[   95.222330] dwc2 1248.hsotg: dwc2_hsotg_ep_queue: submit request only in 
active state

Which occurs whenever the controller is in LPM mode. I guess that input
makes it "work" because it reawakens the controller -- but I'm just
spitballing :-). Let me know if you need any more information from my
side.

[0]: 
https://urldefense.proofpoint.com/v2/url?u=https-3A__forkwhiletrue.me_-7Esimon_midas-5Fdwc2-5Flpm-5Fverbose.log=DwIBAg=DPL6_X_6JkXFx7AXWqB0tg=K1ULVL1slpLXpMJJlAXSOxws4tRq0IkTBqxDkyW2hUQ=fkdnWR9_dgHLGH5-mN9lDclJ58GWR__m7hS3zz3am28=aind-wCFaB40EyTcHvphrqt6SngQTnf3CYXJxOfBjWc=


However, I'm unsure if completely disabling LPM is the correct fix, as the dwc2
revision in Exynos4412 (0x4f54281a) should support LPM according to the
source

Yes, we can enable LPM based on hardware configuration.


I'm not really sure how to debug this any further (vendor kernel
releases contain no mention of LPM in the gadget drivers), so any pointers
in that direction would be much appreciated.

Cheers,
Simon


Cheers,
Simon


Could you please revert "usb: dwc2: Add core state checking" patch and
try again.

This doesn't really fix the issue, but it does change the results. Now, input 
works
as expected, but output is still only shown after input is given (e.g.
the output of "dmesg" only shows if I press a key after executing dmesg).
However, after a few seconds the USB serial device disappears from the
host entirely. Once this has happened, reading the "regdump" of dwc2's debugfs
appears to hang the system (i.e. it no longer responds to UART). I've attached
another log[0].


the regdump is broken. Please try my patch [1]

Stefan

[1] - https://patchwork.kernel.org/patch/10381277/
--
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 v1 0/2] usb: dwc2: gadget: Fixes for LPM

[Simon Shields]

Hi Grigor,

On Wed, May 02, 2018 at 10:12:27AM +, Grigor Tovmasyan wrote:
> Hi Simon,
> 
> On 4/21/2018 4:52 PM, Simon Shields wrote:
> > Hi Grigor,
> > 
> > On Fri, Apr 20, 2018 at 01:00:16PM +, Grigor Tovmasyan wrote:
> >> Hi Simon,
> >>
> >> On 4/19/2018 8:31 PM, Simon Shields wrote:
> >>> Hi all,
> >>>
> >>> On 10/04/2018 10:21 PM, Grigor Tovmasyan wrote:
>  Here are two little fixes for LPM feature.
> 
>  First one is coverity warning fix.
> 
>  The Second one was asserted by Stefan Wahren.
> 
>  Changes from version 0:
> 
>  1/2:
>    - Instead of converting parameter in the CHECK_RANGE macro
>  to int, changed hird_threshold type from u8 to int.
> 
> 
>  Grigor Tovmasyan (2):
>   usb: dwc2: gadget: Fix coverity issue
>   usb: dwc2: gadget: Change LPM default values
> 
>  drivers/usb/dwc2/core.h   | 2 +-
>  drivers/usb/dwc2/params.c | 8 
>  2 files changed, 5 insertions(+), 5 deletions(-)
> >>>
> >>> The second patch in this series fixes a regression in 4.17-rc1 using dwc2 
> >>> in gadget
> >>> mode on Exynos4412, introduced by commit 7455f8b7f0b3 ("usb: dwc2: Enable
> >>> LPM"). The regression is that using the cdc_acm serial gadget (and
> >>> presumably other gadgets) serial console output will only sporadically
> >>> show up on the host (it seems to only show up as input is sent).
> >>>
> >> The second patch is not fix for described by you issue. We will try to
> >> reproduce your issue and provide fix. Could you provide some logs or usb
> >> traces for issue?
> > 
> > Here's a log[0]. The log is pretty big: I generated it with both regular 
> > and verbose
> > debugging enabled for DWC2. However, I suspect the relevant line is
> > probably:
> > 
> > [   95.222330] dwc2 1248.hsotg: dwc2_hsotg_ep_queue: submit request 
> > only in active state
> > 
> > Which occurs whenever the controller is in LPM mode. I guess that input
> > makes it "work" because it reawakens the controller -- but I'm just
> > spitballing :-). Let me know if you need any more information from my
> > side.
> > 
> > [0]: 
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__forkwhiletrue.me_-7Esimon_midas-5Fdwc2-5Flpm-5Fverbose.log=DwIBAg=DPL6_X_6JkXFx7AXWqB0tg=K1ULVL1slpLXpMJJlAXSOxws4tRq0IkTBqxDkyW2hUQ=fkdnWR9_dgHLGH5-mN9lDclJ58GWR__m7hS3zz3am28=aind-wCFaB40EyTcHvphrqt6SngQTnf3CYXJxOfBjWc=
> > 
> >>
> >>> However, I'm unsure if completely disabling LPM is the correct fix, as 
> >>> the dwc2
> >>> revision in Exynos4412 (0x4f54281a) should support LPM according to the
> >>> source
> >> Yes, we can enable LPM based on hardware configuration.
> >>
> >>> I'm not really sure how to debug this any further (vendor kernel
> >>> releases contain no mention of LPM in the gadget drivers), so any pointers
> >>> in that direction would be much appreciated.
> >>>
> >>> Cheers,
> >>> Simon
> >>>
> >>
> > 
> > Cheers,
> > Simon
> > 
> 
> Could you please revert "usb: dwc2: Add core state checking" patch and 
> try again.

This doesn't really fix the issue, but it does change the results. Now, input 
works
as expected, but output is still only shown after input is given (e.g.
the output of "dmesg" only shows if I press a key after executing dmesg).
However, after a few seconds the USB serial device disappears from the
host entirely. Once this has happened, reading the "regdump" of dwc2's debugfs
appears to hang the system (i.e. it no longer responds to UART). I've attached
another log[0].

[0]: https://forkwhiletrue.me/~simon/midas_dwc2_revert_verbose.log

> 
> Let me know about result.
> 
> Thanks,
> Grigor.

Cheers,
Simon
--
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 v1 0/2] usb: dwc2: gadget: Fixes for LPM

[Grigor Tovmasyan]

Hi Simon,

On 4/21/2018 4:52 PM, Simon Shields wrote:
> Hi Grigor,
> 
> On Fri, Apr 20, 2018 at 01:00:16PM +, Grigor Tovmasyan wrote:
>> Hi Simon,
>>
>> On 4/19/2018 8:31 PM, Simon Shields wrote:
>>> Hi all,
>>>
>>> On 10/04/2018 10:21 PM, Grigor Tovmasyan wrote:
 Here are two little fixes for LPM feature.

 First one is coverity warning fix.

 The Second one was asserted by Stefan Wahren.

 Changes from version 0:

 1/2:
   - Instead of converting parameter in the CHECK_RANGE macro
 to int, changed hird_threshold type from u8 to int.


 Grigor Tovmasyan (2):
  usb: dwc2: gadget: Fix coverity issue
  usb: dwc2: gadget: Change LPM default values

 drivers/usb/dwc2/core.h   | 2 +-
 drivers/usb/dwc2/params.c | 8 
 2 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> The second patch in this series fixes a regression in 4.17-rc1 using dwc2 
>>> in gadget
>>> mode on Exynos4412, introduced by commit 7455f8b7f0b3 ("usb: dwc2: Enable
>>> LPM"). The regression is that using the cdc_acm serial gadget (and
>>> presumably other gadgets) serial console output will only sporadically
>>> show up on the host (it seems to only show up as input is sent).
>>>
>> The second patch is not fix for described by you issue. We will try to
>> reproduce your issue and provide fix. Could you provide some logs or usb
>> traces for issue?
> 
> Here's a log[0]. The log is pretty big: I generated it with both regular and 
> verbose
> debugging enabled for DWC2. However, I suspect the relevant line is
> probably:
> 
> [   95.222330] dwc2 1248.hsotg: dwc2_hsotg_ep_queue: submit request only 
> in active state
> 
> Which occurs whenever the controller is in LPM mode. I guess that input
> makes it "work" because it reawakens the controller -- but I'm just
> spitballing :-). Let me know if you need any more information from my
> side.
> 
> [0]: 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__forkwhiletrue.me_-7Esimon_midas-5Fdwc2-5Flpm-5Fverbose.log=DwIBAg=DPL6_X_6JkXFx7AXWqB0tg=K1ULVL1slpLXpMJJlAXSOxws4tRq0IkTBqxDkyW2hUQ=fkdnWR9_dgHLGH5-mN9lDclJ58GWR__m7hS3zz3am28=aind-wCFaB40EyTcHvphrqt6SngQTnf3CYXJxOfBjWc=
> 
>>
>>> However, I'm unsure if completely disabling LPM is the correct fix, as the 
>>> dwc2
>>> revision in Exynos4412 (0x4f54281a) should support LPM according to the
>>> source
>> Yes, we can enable LPM based on hardware configuration.
>>
>>> I'm not really sure how to debug this any further (vendor kernel
>>> releases contain no mention of LPM in the gadget drivers), so any pointers
>>> in that direction would be much appreciated.
>>>
>>> Cheers,
>>> Simon
>>>
>>
> 
> Cheers,
> Simon
> 

Could you please revert "usb: dwc2: Add core state checking" patch and 
try again.

Let me know about result.

Thanks,
Grigor.
--
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 v1 0/2] usb: dwc2: gadget: Fixes for LPM

[Simon Shields]

Hi Grigor,

On Fri, Apr 20, 2018 at 01:00:16PM +, Grigor Tovmasyan wrote:
> Hi Simon,
> 
> On 4/19/2018 8:31 PM, Simon Shields wrote:
> > Hi all,
> > 
> > On 10/04/2018 10:21 PM, Grigor Tovmasyan wrote:
> >> Here are two little fixes for LPM feature.
> >>
> >> First one is coverity warning fix.
> >>
> >> The Second one was asserted by Stefan Wahren.
> >>
> >> Changes from version 0:
> >>
> >> 1/2:
> >>  - Instead of converting parameter in the CHECK_RANGE macro
> >>to int, changed hird_threshold type from u8 to int.
> >>
> >>
> >> Grigor Tovmasyan (2):
> >> usb: dwc2: gadget: Fix coverity issue
> >> usb: dwc2: gadget: Change LPM default values
> >>
> >>drivers/usb/dwc2/core.h   | 2 +-
> >>drivers/usb/dwc2/params.c | 8 
> >>2 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > The second patch in this series fixes a regression in 4.17-rc1 using dwc2 
> > in gadget
> > mode on Exynos4412, introduced by commit 7455f8b7f0b3 ("usb: dwc2: Enable
> > LPM"). The regression is that using the cdc_acm serial gadget (and
> > presumably other gadgets) serial console output will only sporadically
> > show up on the host (it seems to only show up as input is sent).
> > 
> The second patch is not fix for described by you issue. We will try to 
> reproduce your issue and provide fix. Could you provide some logs or usb 
> traces for issue?

Here's a log[0]. The log is pretty big: I generated it with both regular and 
verbose
debugging enabled for DWC2. However, I suspect the relevant line is
probably:

[   95.222330] dwc2 1248.hsotg: dwc2_hsotg_ep_queue: submit request only in 
active state

Which occurs whenever the controller is in LPM mode. I guess that input
makes it "work" because it reawakens the controller -- but I'm just
spitballing :-). Let me know if you need any more information from my
side.

[0]: https://forkwhiletrue.me/~simon/midas_dwc2_lpm_verbose.log

> 
> > However, I'm unsure if completely disabling LPM is the correct fix, as the 
> > dwc2
> > revision in Exynos4412 (0x4f54281a) should support LPM according to the
> > source
> Yes, we can enable LPM based on hardware configuration.
> 
> > I'm not really sure how to debug this any further (vendor kernel
> > releases contain no mention of LPM in the gadget drivers), so any pointers
> > in that direction would be much appreciated.
> > 
> > Cheers,
> > Simon
> > 
> 

Cheers,
Simon
--
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 v1 0/2] usb: dwc2: gadget: Fixes for LPM

[Grigor Tovmasyan]

Hi Balbi,

On 4/10/2018 2:21 PM, Grigor Tovmasyan wrote:
> Here are two little fixes for LPM feature.
> 
> First one is coverity warning fix.
> 
> The Second one was asserted by Stefan Wahren.
> 
> Changes from version 0:
> 
> 1/2:
> - Instead of converting parameter in the CHECK_RANGE macro
>   to int, changed hird_threshold type from u8 to int.
> 
> 
> Grigor Tovmasyan (2):
>usb: dwc2: gadget: Fix coverity issue
>usb: dwc2: gadget: Change LPM default values
> 
>   drivers/usb/dwc2/core.h   | 2 +-
>   drivers/usb/dwc2/params.c | 8 
>   2 files changed, 5 insertions(+), 5 deletions(-)
> 

Please don't merge this patch yet. I need to review it again and made 
some updates to submit it as version 2.

Thanks,
Grigor.
--
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 v1 0/2] usb: dwc2: gadget: Fixes for LPM

[Grigor Tovmasyan]

Hi Simon,

On 4/19/2018 8:31 PM, Simon Shields wrote:
> Hi all,
> 
> On 10/04/2018 10:21 PM, Grigor Tovmasyan wrote:
>> Here are two little fixes for LPM feature.
>>
>> First one is coverity warning fix.
>>
>> The Second one was asserted by Stefan Wahren.
>>
>> Changes from version 0:
>>
>> 1/2:
>>  - Instead of converting parameter in the CHECK_RANGE macro
>>to int, changed hird_threshold type from u8 to int.
>>
>>
>> Grigor Tovmasyan (2):
>> usb: dwc2: gadget: Fix coverity issue
>> usb: dwc2: gadget: Change LPM default values
>>
>>drivers/usb/dwc2/core.h   | 2 +-
>>drivers/usb/dwc2/params.c | 8 
>>2 files changed, 5 insertions(+), 5 deletions(-)
> 
> The second patch in this series fixes a regression in 4.17-rc1 using dwc2 in 
> gadget
> mode on Exynos4412, introduced by commit 7455f8b7f0b3 ("usb: dwc2: Enable
> LPM"). The regression is that using the cdc_acm serial gadget (and
> presumably other gadgets) serial console output will only sporadically
> show up on the host (it seems to only show up as input is sent).
> 
The second patch is not fix for described by you issue. We will try to 
reproduce your issue and provide fix. Could you provide some logs or usb 
traces for issue?

> However, I'm unsure if completely disabling LPM is the correct fix, as the 
> dwc2
> revision in Exynos4412 (0x4f54281a) should support LPM according to the
> source
Yes, we can enable LPM based on hardware configuration.

> I'm not really sure how to debug this any further (vendor kernel
> releases contain no mention of LPM in the gadget drivers), so any pointers
> in that direction would be much appreciated.
> 
> Cheers,
> Simon
> 

--
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 v1 0/2] usb: dwc2: gadget: Fixes for LPM

[Simon Shields]

Hi all,

On 10/04/2018 10:21 PM, Grigor Tovmasyan wrote:
> Here are two little fixes for LPM feature.
>
> First one is coverity warning fix.
>
> The Second one was asserted by Stefan Wahren.
>
> Changes from version 0:
>
> 1/2:
> - Instead of converting parameter in the CHECK_RANGE macro
>   to int, changed hird_threshold type from u8 to int.
>
>
> Grigor Tovmasyan (2):
>usb: dwc2: gadget: Fix coverity issue
>usb: dwc2: gadget: Change LPM default values
>
>   drivers/usb/dwc2/core.h   | 2 +-
>   drivers/usb/dwc2/params.c | 8 
>   2 files changed, 5 insertions(+), 5 deletions(-)

The second patch in this series fixes a regression in 4.17-rc1 using dwc2 in 
gadget
mode on Exynos4412, introduced by commit 7455f8b7f0b3 ("usb: dwc2: Enable
LPM"). The regression is that using the cdc_acm serial gadget (and
presumably other gadgets) serial console output will only sporadically
show up on the host (it seems to only show up as input is sent).

However, I'm unsure if completely disabling LPM is the correct fix, as the dwc2
revision in Exynos4412 (0x4f54281a) should support LPM according to the
source.

I'm not really sure how to debug this any further (vendor kernel
releases contain no mention of LPM in the gadget drivers), so any pointers
in that direction would be much appreciated.

Cheers,
Simon
--
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 v1 0/2] usb: dwc2: gadget: Fixes for LPM

[Minas Harutyunyan]

On 4/10/2018 2:21 PM, Grigor Tovmasyan wrote:
> Here are two little fixes for LPM feature.
> 
> First one is coverity warning fix.
> 
> The Second one was asserted by Stefan Wahren.
> 
> Changes from version 0:
> 
> 1/2:
> - Instead of converting parameter in the CHECK_RANGE macro
>   to int, changed hird_threshold type from u8 to int.
> 
> 
> Grigor Tovmasyan (2):
>usb: dwc2: gadget: Fix coverity issue
>usb: dwc2: gadget: Change LPM default values
> 
>   drivers/usb/dwc2/core.h   | 2 +-
>   drivers/usb/dwc2/params.c | 8 
>   2 files changed, 5 insertions(+), 5 deletions(-)
> 

Acked-by: Minas Harutyunyan 
--
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 v1 0/2] usb: dwc2: gadget: Fixes for LPM

[Grigor Tovmasyan]

Here are two little fixes for LPM feature.

First one is coverity warning fix.

The Second one was asserted by Stefan Wahren.

Changes from version 0:

1/2:
   - Instead of converting parameter in the CHECK_RANGE macro 
 to int, changed hird_threshold type from u8 to int. 


Grigor Tovmasyan (2):
  usb: dwc2: gadget: Fix coverity issue
  usb: dwc2: gadget: Change LPM default values

 drivers/usb/dwc2/core.h   | 2 +-
 drivers/usb/dwc2/params.c | 8 
 2 files changed, 5 insertions(+), 5 deletions(-)

-- 
2.11.0

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