Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1360875 > unrolled thread

Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit()

Started byJohn Youn <John.Youn@synopsys.com>
First post2016-03-18 20:20 +0100
Last post2016-03-31 16:40 +0200
Articles 9 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit() John Youn <John.Youn@synopsys.com> - 2016-03-18 20:20 +0100
    Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit() Felipe Balbi <balbi@kernel.org> - 2016-03-21 08:20 +0100
    Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit() John Youn <John.Youn@synopsys.com> - 2016-03-21 21:00 +0100
      Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit() Felipe Balbi <balbi@kernel.org> - 2016-03-22 07:50 +0100
        Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit() John Youn <John.Youn@synopsys.com> - 2016-03-24 01:50 +0100
          Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit() Felipe Balbi <balbi@kernel.org> - 2016-03-24 08:00 +0100
            Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit() John Youn <John.Youn@synopsys.com> - 2016-03-30 20:50 +0200
              Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit() Roger Quadros <rogerq@ti.com> - 2016-03-31 16:10 +0200
                Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit() Felipe Balbi <balbi@kernel.org> - 2016-03-31 16:40 +0200

#1360875 — Re: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit()

FromJohn Youn <John.Youn@synopsys.com>
Date2016-03-18 20:20 +0100
SubjectRe: [PATCH 1/2] usb: dwc3: core: Introduce dwc3_device_reinit()
Message-ID<re86e-5Tk-7@gated-at.bofh.it>
On 3/16/2016 6:56 AM, Felipe Balbi wrote:
> 
> heh, +john
> 
> Felipe Balbi <balbi@kernel.org> writes:
>> [ text/plain ]
>>
>> Hi,
>>
>> Roger Quadros <rogerq@ti.com> writes:
>>> [ text/plain ]
>>> We will need this function for a workaround.
>>> The function issues a softreset only to the device
>>> controller and performs minimal re-initialization
>>> so that the device controller can be usable.
>>>
>>> As some code is similar to dwc3_core_init() take out
>>> common code into dwc3_get_gctl_quirks().
>>>
>>> We add a new member (prtcap_mode) to struct dwc3 to
>>> keep track of the current mode in the PRTCAPDIR register.
>>>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>
>> I must say, I don't like this at all :-p There's ONE known silicon which
>> needs this because of a poor silicon integration which took an IP with a
>> known erratum where it can't be made to work on lower speeds and STILL
>> was integrated without a superspeed PHY.
>>
>> There's a reason why I never tried to push this upstream myself ;-)
>>
>> I'm really thinking we might be better off adding a quirk flag to skip
>> the metastability workaround and allow this ONE silicon to set the
>> controller to lower speed.
>>
>> John, can you check with your colleagues if we would ever fall into
>> STAR#9000525659 if we set maximum speed to high speed during driver
>> probe and never touch it again ? I would assume we don't really fall
>> into the metastability workaround, right ? We're not doing any sort of
>> PM for dwc3...
>>

Hi Felipe,

I don't know much about this but I will check with the engineers and
get back to you.

John

[toc] | [next] | [standalone]


#1361558

FromFelipe Balbi <balbi@kernel.org>
Date2016-03-21 08:20 +0100
Message-ID<rf2i6-2fe-17@gated-at.bofh.it>
In reply to#1360875

[Multipart message — attachments visible in raw view] — view raw

Hi,

John Youn <John.Youn@synopsys.com> writes:
> [ text/plain ]
> On 3/16/2016 6:56 AM, Felipe Balbi wrote:
>> 
>> heh, +john
>> 
>> Felipe Balbi <balbi@kernel.org> writes:
>>> [ text/plain ]
>>>
>>> Hi,
>>>
>>> Roger Quadros <rogerq@ti.com> writes:
>>>> [ text/plain ]
>>>> We will need this function for a workaround.
>>>> The function issues a softreset only to the device
>>>> controller and performs minimal re-initialization
>>>> so that the device controller can be usable.
>>>>
>>>> As some code is similar to dwc3_core_init() take out
>>>> common code into dwc3_get_gctl_quirks().
>>>>
>>>> We add a new member (prtcap_mode) to struct dwc3 to
>>>> keep track of the current mode in the PRTCAPDIR register.
>>>>
>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>
>>> I must say, I don't like this at all :-p There's ONE known silicon which
>>> needs this because of a poor silicon integration which took an IP with a
>>> known erratum where it can't be made to work on lower speeds and STILL
>>> was integrated without a superspeed PHY.
>>>
>>> There's a reason why I never tried to push this upstream myself ;-)
>>>
>>> I'm really thinking we might be better off adding a quirk flag to skip
>>> the metastability workaround and allow this ONE silicon to set the
>>> controller to lower speed.
>>>
>>> John, can you check with your colleagues if we would ever fall into
>>> STAR#9000525659 if we set maximum speed to high speed during driver
>>> probe and never touch it again ? I would assume we don't really fall
>>> into the metastability workaround, right ? We're not doing any sort of
>>> PM for dwc3...
>>>
>
> Hi Felipe,
>
> I don't know much about this but I will check with the engineers and
> get back to you.

Thank you John ;-)

-- 
balbi

[toc] | [prev] | [next] | [standalone]


#1362122

FromJohn Youn <John.Youn@synopsys.com>
Date2016-03-21 21:00 +0100
Message-ID<rfe9A-1Rp-1@gated-at.bofh.it>
In reply to#1360875
On 3/18/2016 12:17 PM, John Youn wrote:
> On 3/16/2016 6:56 AM, Felipe Balbi wrote:
>>
>> heh, +john
>>
>> Felipe Balbi <balbi@kernel.org> writes:
>>> [ text/plain ]
>>>
>>> Hi,
>>>
>>> Roger Quadros <rogerq@ti.com> writes:
>>>> [ text/plain ]
>>>> We will need this function for a workaround.
>>>> The function issues a softreset only to the device
>>>> controller and performs minimal re-initialization
>>>> so that the device controller can be usable.
>>>>
>>>> As some code is similar to dwc3_core_init() take out
>>>> common code into dwc3_get_gctl_quirks().
>>>>
>>>> We add a new member (prtcap_mode) to struct dwc3 to
>>>> keep track of the current mode in the PRTCAPDIR register.
>>>>
>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>
>>> I must say, I don't like this at all :-p There's ONE known silicon which
>>> needs this because of a poor silicon integration which took an IP with a
>>> known erratum where it can't be made to work on lower speeds and STILL
>>> was integrated without a superspeed PHY.
>>>
>>> There's a reason why I never tried to push this upstream myself ;-)
>>>
>>> I'm really thinking we might be better off adding a quirk flag to skip
>>> the metastability workaround and allow this ONE silicon to set the
>>> controller to lower speed.
>>>
>>> John, can you check with your colleagues if we would ever fall into
>>> STAR#9000525659 if we set maximum speed to high speed during driver
>>> probe and never touch it again ? I would assume we don't really fall
>>> into the metastability workaround, right ? We're not doing any sort of
>>> PM for dwc3...
>>>

Hi Felipe,

Do you mean to keep DCFG.speed to SS and set dwc->maximum_speed to HS?
I don't see an issue with that as long as we always ignore
dwc->maximum_speed when programming DCFG.speed for all affected
versions of the core. As long as the DCFG.speed = SS, you should not
hit the STAR.

John

[toc] | [prev] | [next] | [standalone]


#1362352

FromFelipe Balbi <balbi@kernel.org>
Date2016-03-22 07:50 +0100
Message-ID<rfoiC-C0-17@gated-at.bofh.it>
In reply to#1362122

[Multipart message — attachments visible in raw view] — view raw

Hi,

John Youn <John.Youn@synopsys.com> writes:
> [ text/plain ]
> On 3/18/2016 12:17 PM, John Youn wrote:
>> On 3/16/2016 6:56 AM, Felipe Balbi wrote:
>>>
>>> heh, +john
>>>
>>> Felipe Balbi <balbi@kernel.org> writes:
>>>> [ text/plain ]
>>>>
>>>> Hi,
>>>>
>>>> Roger Quadros <rogerq@ti.com> writes:
>>>>> [ text/plain ]
>>>>> We will need this function for a workaround.
>>>>> The function issues a softreset only to the device
>>>>> controller and performs minimal re-initialization
>>>>> so that the device controller can be usable.
>>>>>
>>>>> As some code is similar to dwc3_core_init() take out
>>>>> common code into dwc3_get_gctl_quirks().
>>>>>
>>>>> We add a new member (prtcap_mode) to struct dwc3 to
>>>>> keep track of the current mode in the PRTCAPDIR register.
>>>>>
>>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>>
>>>> I must say, I don't like this at all :-p There's ONE known silicon which
>>>> needs this because of a poor silicon integration which took an IP with a
>>>> known erratum where it can't be made to work on lower speeds and STILL
>>>> was integrated without a superspeed PHY.
>>>>
>>>> There's a reason why I never tried to push this upstream myself ;-)
>>>>
>>>> I'm really thinking we might be better off adding a quirk flag to skip
>>>> the metastability workaround and allow this ONE silicon to set the
>>>> controller to lower speed.
>>>>
>>>> John, can you check with your colleagues if we would ever fall into
>>>> STAR#9000525659 if we set maximum speed to high speed during driver
>>>> probe and never touch it again ? I would assume we don't really fall
>>>> into the metastability workaround, right ? We're not doing any sort of
>>>> PM for dwc3...
>>>>
>
> Hi Felipe,
>
> Do you mean to keep DCFG.speed to SS and set dwc->maximum_speed to HS?
> I don't see an issue with that as long as we always ignore
> dwc->maximum_speed when programming DCFG.speed for all affected
> versions of the core. As long as the DCFG.speed = SS, you should not
> hit the STAR.

I actually mean changing DCFG.speed during driver probe and never
touching it again. Would that still cause problems ?

-- 
balbi

[toc] | [prev] | [next] | [standalone]


#1363802

FromJohn Youn <John.Youn@synopsys.com>
Date2016-03-24 01:50 +0100
Message-ID<rg1Dj-30H-5@gated-at.bofh.it>
In reply to#1362352
On 3/21/2016 11:40 PM, Felipe Balbi wrote:
> 
> Hi,
> 
> John Youn <John.Youn@synopsys.com> writes:
>> [ text/plain ]
>> On 3/18/2016 12:17 PM, John Youn wrote:
>>> On 3/16/2016 6:56 AM, Felipe Balbi wrote:
>>>>
>>>> heh, +john
>>>>
>>>> Felipe Balbi <balbi@kernel.org> writes:
>>>>> [ text/plain ]
>>>>>
>>>>> Hi,
>>>>>
>>>>> Roger Quadros <rogerq@ti.com> writes:
>>>>>> [ text/plain ]
>>>>>> We will need this function for a workaround.
>>>>>> The function issues a softreset only to the device
>>>>>> controller and performs minimal re-initialization
>>>>>> so that the device controller can be usable.
>>>>>>
>>>>>> As some code is similar to dwc3_core_init() take out
>>>>>> common code into dwc3_get_gctl_quirks().
>>>>>>
>>>>>> We add a new member (prtcap_mode) to struct dwc3 to
>>>>>> keep track of the current mode in the PRTCAPDIR register.
>>>>>>
>>>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>>>
>>>>> I must say, I don't like this at all :-p There's ONE known silicon which
>>>>> needs this because of a poor silicon integration which took an IP with a
>>>>> known erratum where it can't be made to work on lower speeds and STILL
>>>>> was integrated without a superspeed PHY.
>>>>>
>>>>> There's a reason why I never tried to push this upstream myself ;-)
>>>>>
>>>>> I'm really thinking we might be better off adding a quirk flag to skip
>>>>> the metastability workaround and allow this ONE silicon to set the
>>>>> controller to lower speed.
>>>>>
>>>>> John, can you check with your colleagues if we would ever fall into
>>>>> STAR#9000525659 if we set maximum speed to high speed during driver
>>>>> probe and never touch it again ? I would assume we don't really fall
>>>>> into the metastability workaround, right ? We're not doing any sort of
>>>>> PM for dwc3...
>>>>>
>>
>> Hi Felipe,
>>
>> Do you mean to keep DCFG.speed to SS and set dwc->maximum_speed to HS?
>> I don't see an issue with that as long as we always ignore
>> dwc->maximum_speed when programming DCFG.speed for all affected
>> versions of the core. As long as the DCFG.speed = SS, you should not
>> hit the STAR.
> 
> I actually mean changing DCFG.speed during driver probe and never
> touching it again. Would that still cause problems ?
> 

In that case I'm not sure. The engineer who would know is off until
next week so I'll get back to you as soon as I can talk to him about
it.

Regards,
John

[toc] | [prev] | [next] | [standalone]


#1363959

FromFelipe Balbi <balbi@kernel.org>
Date2016-03-24 08:00 +0100
Message-ID<rg7po-7em-3@gated-at.bofh.it>
In reply to#1363802

[Multipart message — attachments visible in raw view] — view raw

Hi,

John Youn <John.Youn@synopsys.com> writes:
> [ text/plain ]
> On 3/21/2016 11:40 PM, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> John Youn <John.Youn@synopsys.com> writes:
>>> [ text/plain ]
>>> On 3/18/2016 12:17 PM, John Youn wrote:
>>>> On 3/16/2016 6:56 AM, Felipe Balbi wrote:
>>>>>
>>>>> heh, +john
>>>>>
>>>>> Felipe Balbi <balbi@kernel.org> writes:
>>>>>> [ text/plain ]
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Roger Quadros <rogerq@ti.com> writes:
>>>>>>> [ text/plain ]
>>>>>>> We will need this function for a workaround.
>>>>>>> The function issues a softreset only to the device
>>>>>>> controller and performs minimal re-initialization
>>>>>>> so that the device controller can be usable.
>>>>>>>
>>>>>>> As some code is similar to dwc3_core_init() take out
>>>>>>> common code into dwc3_get_gctl_quirks().
>>>>>>>
>>>>>>> We add a new member (prtcap_mode) to struct dwc3 to
>>>>>>> keep track of the current mode in the PRTCAPDIR register.
>>>>>>>
>>>>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>>>>
>>>>>> I must say, I don't like this at all :-p There's ONE known silicon which
>>>>>> needs this because of a poor silicon integration which took an IP with a
>>>>>> known erratum where it can't be made to work on lower speeds and STILL
>>>>>> was integrated without a superspeed PHY.
>>>>>>
>>>>>> There's a reason why I never tried to push this upstream myself ;-)
>>>>>>
>>>>>> I'm really thinking we might be better off adding a quirk flag to skip
>>>>>> the metastability workaround and allow this ONE silicon to set the
>>>>>> controller to lower speed.
>>>>>>
>>>>>> John, can you check with your colleagues if we would ever fall into
>>>>>> STAR#9000525659 if we set maximum speed to high speed during driver
>>>>>> probe and never touch it again ? I would assume we don't really fall
>>>>>> into the metastability workaround, right ? We're not doing any sort of
>>>>>> PM for dwc3...
>>>>>>
>>>
>>> Hi Felipe,
>>>
>>> Do you mean to keep DCFG.speed to SS and set dwc->maximum_speed to HS?
>>> I don't see an issue with that as long as we always ignore
>>> dwc->maximum_speed when programming DCFG.speed for all affected
>>> versions of the core. As long as the DCFG.speed = SS, you should not
>>> hit the STAR.
>> 
>> I actually mean changing DCFG.speed during driver probe and never
>> touching it again. Would that still cause problems ?
>> 
>
> In that case I'm not sure. The engineer who would know is off until
> next week so I'll get back to you as soon as I can talk to him about
> it.

Thank you :-)

-- 
balbi

[toc] | [prev] | [next] | [standalone]


#1367516

FromJohn Youn <John.Youn@synopsys.com>
Date2016-03-30 20:50 +0200
Message-ID<ritlL-15d-3@gated-at.bofh.it>
In reply to#1363959
On 3/23/2016 11:52 PM, Felipe Balbi wrote:
> 
> Hi,
> 
> John Youn <John.Youn@synopsys.com> writes:
>> [ text/plain ]
>> On 3/21/2016 11:40 PM, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> John Youn <John.Youn@synopsys.com> writes:
>>>> [ text/plain ]
>>>> On 3/18/2016 12:17 PM, John Youn wrote:
>>>>> On 3/16/2016 6:56 AM, Felipe Balbi wrote:
>>>>>>
>>>>>> heh, +john
>>>>>>
>>>>>> Felipe Balbi <balbi@kernel.org> writes:
>>>>>>> [ text/plain ]
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Roger Quadros <rogerq@ti.com> writes:
>>>>>>>> [ text/plain ]
>>>>>>>> We will need this function for a workaround.
>>>>>>>> The function issues a softreset only to the device
>>>>>>>> controller and performs minimal re-initialization
>>>>>>>> so that the device controller can be usable.
>>>>>>>>
>>>>>>>> As some code is similar to dwc3_core_init() take out
>>>>>>>> common code into dwc3_get_gctl_quirks().
>>>>>>>>
>>>>>>>> We add a new member (prtcap_mode) to struct dwc3 to
>>>>>>>> keep track of the current mode in the PRTCAPDIR register.
>>>>>>>>
>>>>>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>>>>>
>>>>>>> I must say, I don't like this at all :-p There's ONE known silicon which
>>>>>>> needs this because of a poor silicon integration which took an IP with a
>>>>>>> known erratum where it can't be made to work on lower speeds and STILL
>>>>>>> was integrated without a superspeed PHY.
>>>>>>>
>>>>>>> There's a reason why I never tried to push this upstream myself ;-)
>>>>>>>
>>>>>>> I'm really thinking we might be better off adding a quirk flag to skip
>>>>>>> the metastability workaround and allow this ONE silicon to set the
>>>>>>> controller to lower speed.
>>>>>>>
>>>>>>> John, can you check with your colleagues if we would ever fall into
>>>>>>> STAR#9000525659 if we set maximum speed to high speed during driver
>>>>>>> probe and never touch it again ? I would assume we don't really fall
>>>>>>> into the metastability workaround, right ? We're not doing any sort of
>>>>>>> PM for dwc3...
>>>>>>>
>>>>
>>>> Hi Felipe,
>>>>
>>>> Do you mean to keep DCFG.speed to SS and set dwc->maximum_speed to HS?
>>>> I don't see an issue with that as long as we always ignore
>>>> dwc->maximum_speed when programming DCFG.speed for all affected
>>>> versions of the core. As long as the DCFG.speed = SS, you should not
>>>> hit the STAR.
>>>
>>> I actually mean changing DCFG.speed during driver probe and never
>>> touching it again. Would that still cause problems ?
>>>
>>
>> In that case I'm not sure. The engineer who would know is off until
>> next week so I'll get back to you as soon as I can talk to him about
>> it.
> 

So the engineers said that this issue can occur while set to HS and
the run/stop bit is changed so it seems that won't work.

Regards,
John

[toc] | [prev] | [next] | [standalone]


#1368328

FromRoger Quadros <rogerq@ti.com>
Date2016-03-31 16:10 +0200
Message-ID<riLsm-6eH-9@gated-at.bofh.it>
In reply to#1367516
On 30/03/16 21:44, John Youn wrote:
> On 3/23/2016 11:52 PM, Felipe Balbi wrote:
>>
>> Hi,
>>
>> John Youn <John.Youn@synopsys.com> writes:
>>> [ text/plain ]
>>> On 3/21/2016 11:40 PM, Felipe Balbi wrote:
>>>>
>>>> Hi,
>>>>
>>>> John Youn <John.Youn@synopsys.com> writes:
>>>>> [ text/plain ]
>>>>> On 3/18/2016 12:17 PM, John Youn wrote:
>>>>>> On 3/16/2016 6:56 AM, Felipe Balbi wrote:
>>>>>>>
>>>>>>> heh, +john
>>>>>>>
>>>>>>> Felipe Balbi <balbi@kernel.org> writes:
>>>>>>>> [ text/plain ]
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Roger Quadros <rogerq@ti.com> writes:
>>>>>>>>> [ text/plain ]
>>>>>>>>> We will need this function for a workaround.
>>>>>>>>> The function issues a softreset only to the device
>>>>>>>>> controller and performs minimal re-initialization
>>>>>>>>> so that the device controller can be usable.
>>>>>>>>>
>>>>>>>>> As some code is similar to dwc3_core_init() take out
>>>>>>>>> common code into dwc3_get_gctl_quirks().
>>>>>>>>>
>>>>>>>>> We add a new member (prtcap_mode) to struct dwc3 to
>>>>>>>>> keep track of the current mode in the PRTCAPDIR register.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>>>>>>
>>>>>>>> I must say, I don't like this at all :-p There's ONE known silicon which
>>>>>>>> needs this because of a poor silicon integration which took an IP with a
>>>>>>>> known erratum where it can't be made to work on lower speeds and STILL
>>>>>>>> was integrated without a superspeed PHY.
>>>>>>>>
>>>>>>>> There's a reason why I never tried to push this upstream myself ;-)
>>>>>>>>
>>>>>>>> I'm really thinking we might be better off adding a quirk flag to skip
>>>>>>>> the metastability workaround and allow this ONE silicon to set the
>>>>>>>> controller to lower speed.
>>>>>>>>
>>>>>>>> John, can you check with your colleagues if we would ever fall into
>>>>>>>> STAR#9000525659 if we set maximum speed to high speed during driver
>>>>>>>> probe and never touch it again ? I would assume we don't really fall
>>>>>>>> into the metastability workaround, right ? We're not doing any sort of
>>>>>>>> PM for dwc3...
>>>>>>>>
>>>>>
>>>>> Hi Felipe,
>>>>>
>>>>> Do you mean to keep DCFG.speed to SS and set dwc->maximum_speed to HS?
>>>>> I don't see an issue with that as long as we always ignore
>>>>> dwc->maximum_speed when programming DCFG.speed for all affected
>>>>> versions of the core. As long as the DCFG.speed = SS, you should not
>>>>> hit the STAR.
>>>>
>>>> I actually mean changing DCFG.speed during driver probe and never
>>>> touching it again. Would that still cause problems ?
>>>>
>>>
>>> In that case I'm not sure. The engineer who would know is off until
>>> next week so I'll get back to you as soon as I can talk to him about
>>> it.
>>
> 
> So the engineers said that this issue can occur while set to HS and
> the run/stop bit is changed so it seems that won't work.

Thanks John.

Felipe,

any suggestion how we can fix this upstream?

cheers,
-roger

[toc] | [prev] | [next] | [standalone]


#1368367

FromFelipe Balbi <balbi@kernel.org>
Date2016-03-31 16:40 +0200
Message-ID<riLVo-6qF-11@gated-at.bofh.it>
In reply to#1368328

[Multipart message — attachments visible in raw view] — view raw

Hi,

Roger Quadros <rogerq@ti.com> writes:
>>>>>>>>>> We will need this function for a workaround.
>>>>>>>>>> The function issues a softreset only to the device
>>>>>>>>>> controller and performs minimal re-initialization
>>>>>>>>>> so that the device controller can be usable.
>>>>>>>>>>
>>>>>>>>>> As some code is similar to dwc3_core_init() take out
>>>>>>>>>> common code into dwc3_get_gctl_quirks().
>>>>>>>>>>
>>>>>>>>>> We add a new member (prtcap_mode) to struct dwc3 to
>>>>>>>>>> keep track of the current mode in the PRTCAPDIR register.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>>>>>>>
>>>>>>>>> I must say, I don't like this at all :-p There's ONE known silicon which
>>>>>>>>> needs this because of a poor silicon integration which took an IP with a
>>>>>>>>> known erratum where it can't be made to work on lower speeds and STILL
>>>>>>>>> was integrated without a superspeed PHY.
>>>>>>>>>
>>>>>>>>> There's a reason why I never tried to push this upstream myself ;-)
>>>>>>>>>
>>>>>>>>> I'm really thinking we might be better off adding a quirk flag to skip
>>>>>>>>> the metastability workaround and allow this ONE silicon to set the
>>>>>>>>> controller to lower speed.
>>>>>>>>>
>>>>>>>>> John, can you check with your colleagues if we would ever fall into
>>>>>>>>> STAR#9000525659 if we set maximum speed to high speed during driver
>>>>>>>>> probe and never touch it again ? I would assume we don't really fall
>>>>>>>>> into the metastability workaround, right ? We're not doing any sort of
>>>>>>>>> PM for dwc3...
>>>>>>>>>
>>>>>>
>>>>>> Hi Felipe,
>>>>>>
>>>>>> Do you mean to keep DCFG.speed to SS and set dwc->maximum_speed to HS?
>>>>>> I don't see an issue with that as long as we always ignore
>>>>>> dwc->maximum_speed when programming DCFG.speed for all affected
>>>>>> versions of the core. As long as the DCFG.speed = SS, you should not
>>>>>> hit the STAR.
>>>>>
>>>>> I actually mean changing DCFG.speed during driver probe and never
>>>>> touching it again. Would that still cause problems ?
>>>>>
>>>>
>>>> In that case I'm not sure. The engineer who would know is off until
>>>> next week so I'll get back to you as soon as I can talk to him about
>>>> it.
>>>
>> 
>> So the engineers said that this issue can occur while set to HS and
>> the run/stop bit is changed so it seems that won't work.
>
> Thanks John.
>
> Felipe,
>
> any suggestion how we can fix this upstream?

no idea, I don't have a lot of memory about this problem. I really don't
remember the details about this, is there an openly available errata
document which I could read ? /me goes search for it.

I found [1] which tells me, the following:


| i819        | A Device Control Bit Meta-Stability for USB3.0 Controller in USB2.0 Mode   |
|-------------+----------------------------------------------------------------------------|
| Criticality | Medium                                                                     |
|             |                                                                            |
| Descritiion | When USB3.0 controller core is programmed to be a USB 2.0-only device      |
|             | hardware meta-stability on USB_DCTL[31]RUNSTOP bit causing the core to     |
|             | attempt high speed as well as SuperSpeed connection or completely miss     |
|             | the attach request.                                                        |
|             |                                                                            |
| Workaround  | If the requirement is to always function in USB 2.0 mode, there is no      |
|             | workaround.                                                                |
|             | Otherwise, you can always program the USB controller core to be SuperSpeed |
|             | 3.0 capable (USB_DCFG[2:0]DEVSPD = 0x4)                                    |
|             |                                                                            |
| Revisions   | SR 1.1, 2.0                                                                |
| Impacted    |                                                                            |
|-------------+----------------------------------------------------------------------------|

So, TI's own documentation says that there is _no_ workaround. My
question is, then: How are you sure that resetting the device actually
solves the issue ? Did you really hit the metastability problem and
noted that it works after a soft-reset ? How did you verify that
Run/Stop was in a metastable state, considering that Run/Stop signal is
not visible outside the die ?

It seems to me that resetting the IP is just as "dangerous" as setting
the IP to High-speed in the first place. No ?

[1] http://www.ti.com/lit/er/sprz429h/sprz429h.pdf

-- 
balbi

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web