* [PATCH] drivers/usb/dwc3: Set PHY ready after soft reset done
@ 2021-06-11 7:18 Yuan Fang
2021-06-11 7:25 ` Greg KH
2021-06-11 13:02 ` Felipe Balbi
0 siblings, 2 replies; 7+ messages in thread
From: Yuan Fang @ 2021-06-11 7:18 UTC (permalink / raw)
To: linux-usb; +Cc: balbi, Yuan Fang
It's more safe to set PHY ready after soft reset done
Let's consider a test case like this:
I have a usb PHY which don't need SW initial before access
it, But i have a wrong PHY clock default setting in PHY
register which means PHY is not ready in fact.
in dwc3_core_init, dwc3_core_get_phy will return 0 despite
usb get PHYs with return -ENODEV, and set phys_ready=true
but at this point, we can not say phys is ready or at least
it not safe to do that.
then, go on with dwc3_core_soft_reset,dwc3 reset core and
PHY, as phy->init is NULL in this case, so, usb_phy_init
do nothing and go on with a while loop which cost 1000*
20 ms, and return with -ETIMEDOUT
check phys_ready is set to TRUE but actually not.
Move phys_ready after soft reset done is more reasonable
and don't see side-effect yet.
Signed-off-by: Yuan Fang <yuan.fang@verisilicon.com>
---
Hi,
please take a look at issue description above and kindly
review this CL if any side-effect, many thanks.
B.R
Yuan Fang
drivers/usb/dwc3/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index b6e53d8212cd..04b1bbaf694a 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -324,6 +324,7 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
if (DWC3_VER_IS_WITHIN(DWC31, ANY, 180A))
msleep(50);
+ dwc->phys_ready = true;
return 0;
}
@@ -979,7 +980,6 @@ static int dwc3_core_init(struct dwc3 *dwc)
ret = dwc3_core_get_phy(dwc);
if (ret)
goto err0a;
- dwc->phys_ready = true;
}
ret = dwc3_core_soft_reset(dwc);
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drivers/usb/dwc3: Set PHY ready after soft reset done
2021-06-11 7:18 [PATCH] drivers/usb/dwc3: Set PHY ready after soft reset done Yuan Fang
@ 2021-06-11 7:25 ` Greg KH
2021-06-11 7:34 ` 答复: " Fang, Yuan
2021-06-11 13:02 ` Felipe Balbi
1 sibling, 1 reply; 7+ messages in thread
From: Greg KH @ 2021-06-11 7:25 UTC (permalink / raw)
To: Yuan Fang; +Cc: linux-usb, balbi, Yuan Fang
On Fri, Jun 11, 2021 at 03:18:05PM +0800, Yuan Fang wrote:
> It's more safe to set PHY ready after soft reset done
>
> Let's consider a test case like this:
> I have a usb PHY which don't need SW initial before access
> it, But i have a wrong PHY clock default setting in PHY
> register which means PHY is not ready in fact.
> in dwc3_core_init, dwc3_core_get_phy will return 0 despite
> usb get PHYs with return -ENODEV, and set phys_ready=true
> but at this point, we can not say phys is ready or at least
> it not safe to do that.
> then, go on with dwc3_core_soft_reset,dwc3 reset core and
> PHY, as phy->init is NULL in this case, so, usb_phy_init
> do nothing and go on with a while loop which cost 1000*
> 20 ms, and return with -ETIMEDOUT
> check phys_ready is set to TRUE but actually not.
> Move phys_ready after soft reset done is more reasonable
> and don't see side-effect yet.
>
> Signed-off-by: Yuan Fang <yuan.fang@verisilicon.com>
What commit does this "fix"? Should it also be backported to older
kernels? If so, how far back?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
* 答复: [PATCH] drivers/usb/dwc3: Set PHY ready after soft reset done
2021-06-11 7:25 ` Greg KH
@ 2021-06-11 7:34 ` Fang, Yuan
0 siblings, 0 replies; 7+ messages in thread
From: Fang, Yuan @ 2021-06-11 7:34 UTC (permalink / raw)
To: Greg KH, Yuan Fang; +Cc: linux-usb, balbi
Hi Maintainer,
Thanks so much for the quick reply.
I would like to back ported to older kernels, but I can not
figure out to which version is suitable, I can have a check later
But merge into the new release version is also acceptable as I think.
Thanks again.
B.R
Yuan Fang
-----邮件原件-----
发件人: Greg KH [mailto:gregkh@linuxfoundation.org]
发送时间: 2021年6月11日 15:26
收件人: Yuan Fang
抄送: linux-usb@vger.kernel.org; balbi@kernel.org; Fang, Yuan
主题: Re: [PATCH] drivers/usb/dwc3: Set PHY ready after soft reset done
On Fri, Jun 11, 2021 at 03:18:05PM +0800, Yuan Fang wrote:
> It's more safe to set PHY ready after soft reset done
>
> Let's consider a test case like this:
> I have a usb PHY which don't need SW initial before access it, But i
> have a wrong PHY clock default setting in PHY register which means PHY
> is not ready in fact.
> in dwc3_core_init, dwc3_core_get_phy will return 0 despite usb get
> PHYs with return -ENODEV, and set phys_ready=true but at this point,
> we can not say phys is ready or at least it not safe to do that.
> then, go on with dwc3_core_soft_reset,dwc3 reset core and PHY, as
> phy->init is NULL in this case, so, usb_phy_init do nothing and go on
> with a while loop which cost 1000*
> 20 ms, and return with -ETIMEDOUT
> check phys_ready is set to TRUE but actually not.
> Move phys_ready after soft reset done is more reasonable and don't see
> side-effect yet.
>
> Signed-off-by: Yuan Fang <yuan.fang@verisilicon.com>
What commit does this "fix"? Should it also be backported to older kernels? If so, how far back?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drivers/usb/dwc3: Set PHY ready after soft reset done
2021-06-11 7:18 [PATCH] drivers/usb/dwc3: Set PHY ready after soft reset done Yuan Fang
2021-06-11 7:25 ` Greg KH
@ 2021-06-11 13:02 ` Felipe Balbi
2021-06-16 2:55 ` 答复: " Fang, Yuan
` (2 more replies)
1 sibling, 3 replies; 7+ messages in thread
From: Felipe Balbi @ 2021-06-11 13:02 UTC (permalink / raw)
To: Yuan Fang, linux-usb; +Cc: Yuan Fang
[-- Attachment #1: Type: text/plain, Size: 577 bytes --]
Yuan Fang <fangyuanseu@gmail.com> writes:
> It's more safe to set PHY ready after soft reset done
>
> Let's consider a test case like this:
> I have a usb PHY which don't need SW initial before access
> it, But i have a wrong PHY clock default setting in PHY
> register which means PHY is not ready in fact.
> in dwc3_core_init, dwc3_core_get_phy will return 0 despite
> usb get PHYs with return -ENODEV, and set phys_ready=true
your PHY driver shouldn't return -ENODEV here. Why are you returning
-ENODEV? The PHY isn't ready but it _does_ exist.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 511 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* 答复: [PATCH] drivers/usb/dwc3: Set PHY ready after soft reset done
2021-06-11 13:02 ` Felipe Balbi
@ 2021-06-16 2:55 ` Fang, Yuan
2021-06-29 2:42 ` Fang, Yuan
2021-09-07 11:01 ` Fang, Yuan
2 siblings, 0 replies; 7+ messages in thread
From: Fang, Yuan @ 2021-06-16 2:55 UTC (permalink / raw)
To: Felipe Balbi, Yuan Fang, linux-usb
Hi Balbi,
> It's more safe to set PHY ready after soft reset done
>
> Let's consider a test case like this:
> I have a usb PHY which don't need SW initial before access it, But i
> have a wrong PHY clock default setting in PHY register which means PHY
> is not ready in fact.
> in dwc3_core_init, dwc3_core_get_phy will return 0 despite usb get
> PHYs with return -ENODEV, and set phys_ready=true
> your PHY driver shouldn't return -ENODEV here. Why are you returning -ENODEV? The PHY isn't ready but it _does_ exist.
Sorry for the late reply, my explain may be not clear, let me describe it once more
1, I don’t coding a PHY driver, I have a PHY which do NOT need SW operation
All I should do is set the related PHY ctrl regs correctly. So I do not return anything to generic PHY framework.
2, In my above case, dwc3_core_get_phy will return 0, and set dwc->phys_ready=true, And then,
dwc3_core_soft_reset will cost retries*20ms to check if soft-reset is done , and return -ETIMEDOUT if time out, is that correct?
My point is, in this case(PHY do not set correctly, which is not rare during development stage I think),
PHY initial fail led to soft-reset timeout but dwc->phys_ready is true,
So I think is more safe to say "PHY is ready" after soft-reset done, please let me know your opinion, thanks a lot
Yuan Fang
-----邮件原件-----
发件人: Felipe Balbi [mailto:balbi@kernel.org]
发送时间: 2021年6月11日 21:03
收件人: Yuan Fang; linux-usb@vger.kernel.org
抄送: Fang, Yuan
主题: Re: [PATCH] drivers/usb/dwc3: Set PHY ready after soft reset done
Yuan Fang <fangyuanseu@gmail.com> writes:
> It's more safe to set PHY ready after soft reset done
>
> Let's consider a test case like this:
> I have a usb PHY which don't need SW initial before access it, But i
> have a wrong PHY clock default setting in PHY register which means PHY
> is not ready in fact.
> in dwc3_core_init, dwc3_core_get_phy will return 0 despite usb get
> PHYs with return -ENODEV, and set phys_ready=true
your PHY driver shouldn't return -ENODEV here. Why are you returning -ENODEV? The PHY isn't ready but it _does_ exist.
--
balbi
^ permalink raw reply [flat|nested] 7+ messages in thread
* 答复: [PATCH] drivers/usb/dwc3: Set PHY ready after soft reset done
2021-06-11 13:02 ` Felipe Balbi
2021-06-16 2:55 ` 答复: " Fang, Yuan
@ 2021-06-29 2:42 ` Fang, Yuan
2021-09-07 11:01 ` Fang, Yuan
2 siblings, 0 replies; 7+ messages in thread
From: Fang, Yuan @ 2021-06-29 2:42 UTC (permalink / raw)
To: Fang, Yuan, Felipe Balbi, Yuan Fang, linux-usb
Hi Maintainner,
Could you please share your thoughts with me?
Thanks a lot.
-----邮件原件-----
发件人: Fang, Yuan
发送时间: 2021年6月16日 10:56
收件人: 'Felipe Balbi'; Yuan Fang; linux-usb@vger.kernel.org
主题: 答复: [PATCH] drivers/usb/dwc3: Set PHY ready after soft reset done
Hi Balbi,
> It's more safe to set PHY ready after soft reset done
>
> Let's consider a test case like this:
> I have a usb PHY which don't need SW initial before access it, But i
> have a wrong PHY clock default setting in PHY register which means PHY
> is not ready in fact.
> in dwc3_core_init, dwc3_core_get_phy will return 0 despite usb get
> PHYs with return -ENODEV, and set phys_ready=true
> your PHY driver shouldn't return -ENODEV here. Why are you returning -ENODEV? The PHY isn't ready but it _does_ exist.
Sorry for the late reply, my explain may be not clear, let me describe it once more 1, I don’t coding a PHY driver, I have a PHY which do NOT need SW operation All I should do is set the related PHY ctrl regs correctly. So I do not return anything to generic PHY framework.
2, In my above case, dwc3_core_get_phy will return 0, and set dwc->phys_ready=true, And then, dwc3_core_soft_reset will cost retries*20ms to check if soft-reset is done , and return -ETIMEDOUT if time out, is that correct?
My point is, in this case(PHY do not set correctly, which is not rare during development stage I think), PHY initial fail led to soft-reset timeout but dwc->phys_ready is true, So I think is more safe to say "PHY is ready" after soft-reset done, please let me know your opinion, thanks a lot
Yuan Fang
-----邮件原件-----
发件人: Felipe Balbi [mailto:balbi@kernel.org]
发送时间: 2021年6月11日 21:03
收件人: Yuan Fang; linux-usb@vger.kernel.org
抄送: Fang, Yuan
主题: Re: [PATCH] drivers/usb/dwc3: Set PHY ready after soft reset done
Yuan Fang <fangyuanseu@gmail.com> writes:
> It's more safe to set PHY ready after soft reset done
>
> Let's consider a test case like this:
> I have a usb PHY which don't need SW initial before access it, But i
> have a wrong PHY clock default setting in PHY register which means PHY
> is not ready in fact.
> in dwc3_core_init, dwc3_core_get_phy will return 0 despite usb get
> PHYs with return -ENODEV, and set phys_ready=true
your PHY driver shouldn't return -ENODEV here. Why are you returning -ENODEV? The PHY isn't ready but it _does_ exist.
--
balbi
^ permalink raw reply [flat|nested] 7+ messages in thread
* 答复: [PATCH] drivers/usb/dwc3: Set PHY ready after soft reset done
2021-06-11 13:02 ` Felipe Balbi
2021-06-16 2:55 ` 答复: " Fang, Yuan
2021-06-29 2:42 ` Fang, Yuan
@ 2021-09-07 11:01 ` Fang, Yuan
2 siblings, 0 replies; 7+ messages in thread
From: Fang, Yuan @ 2021-09-07 11:01 UTC (permalink / raw)
To: Fang, Yuan, Felipe Balbi, Yuan Fang, linux-usb
Hi Maintainer,
May I know your thoughts on this topic?
Thanks a lot.
-----邮件原件-----
发件人: Fang, Yuan
发送时间: 2021年6月29日 10:42
收件人: Fang, Yuan; 'Felipe Balbi'; 'Yuan Fang'; 'linux-usb@vger.kernel.org'
主题: 答复: [PATCH] drivers/usb/dwc3: Set PHY ready after soft reset done
Hi Maintainner,
Could you please share your thoughts with me?
Thanks a lot.
-----邮件原件-----
发件人: Fang, Yuan
发送时间: 2021年6月16日 10:56
收件人: 'Felipe Balbi'; Yuan Fang; linux-usb@vger.kernel.org
主题: 答复: [PATCH] drivers/usb/dwc3: Set PHY ready after soft reset done
Hi Balbi,
> It's more safe to set PHY ready after soft reset done
>
> Let's consider a test case like this:
> I have a usb PHY which don't need SW initial before access it, But i
> have a wrong PHY clock default setting in PHY register which means PHY
> is not ready in fact.
> in dwc3_core_init, dwc3_core_get_phy will return 0 despite usb get
> PHYs with return -ENODEV, and set phys_ready=true
> your PHY driver shouldn't return -ENODEV here. Why are you returning -ENODEV? The PHY isn't ready but it _does_ exist.
Sorry for the late reply, my explain may be not clear, let me describe it once more 1, I don’t coding a PHY driver, I have a PHY which do NOT need SW operation All I should do is set the related PHY ctrl regs correctly. So I do not return anything to generic PHY framework.
2, In my above case, dwc3_core_get_phy will return 0, and set dwc->phys_ready=true, And then, dwc3_core_soft_reset will cost retries*20ms to check if soft-reset is done , and return -ETIMEDOUT if time out, is that correct?
My point is, in this case(PHY do not set correctly, which is not rare during development stage I think), PHY initial fail led to soft-reset timeout but dwc->phys_ready is true, So I think is more safe to say "PHY is ready" after soft-reset done, please let me know your opinion, thanks a lot
Yuan Fang
-----邮件原件-----
发件人: Felipe Balbi [mailto:balbi@kernel.org]
发送时间: 2021年6月11日 21:03
收件人: Yuan Fang; linux-usb@vger.kernel.org
抄送: Fang, Yuan
主题: Re: [PATCH] drivers/usb/dwc3: Set PHY ready after soft reset done
Yuan Fang <fangyuanseu@gmail.com> writes:
> It's more safe to set PHY ready after soft reset done
>
> Let's consider a test case like this:
> I have a usb PHY which don't need SW initial before access it, But i
> have a wrong PHY clock default setting in PHY register which means PHY
> is not ready in fact.
> in dwc3_core_init, dwc3_core_get_phy will return 0 despite usb get
> PHYs with return -ENODEV, and set phys_ready=true
your PHY driver shouldn't return -ENODEV here. Why are you returning -ENODEV? The PHY isn't ready but it _does_ exist.
--
balbi
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-09-07 11:06 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-11 7:18 [PATCH] drivers/usb/dwc3: Set PHY ready after soft reset done Yuan Fang
2021-06-11 7:25 ` Greg KH
2021-06-11 7:34 ` 答复: " Fang, Yuan
2021-06-11 13:02 ` Felipe Balbi
2021-06-16 2:55 ` 答复: " Fang, Yuan
2021-06-29 2:42 ` Fang, Yuan
2021-09-07 11:01 ` Fang, Yuan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).