diff mbox

[1/4] phy: rockchip-emmc: give DLL some extra time to be ready

Message ID 1463092986-61777-1-git-send-email-briannorris@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Brian Norris May 12, 2016, 10:43 p.m. UTC
From: Shawn Lin <shawn.lin@rock-chips.com>

According to the databook, 10.2us is the max time for dll to be ready to
work. However in testing, some chips need 20us for dll to be ready. This
patch adds some extra margin for dllrdy to be ready, fixing our
-ETIMEDOUT issues.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
 drivers/phy/phy-rockchip-emmc.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Doug Anderson May 13, 2016, 10:01 p.m. UTC | #1
Hi,

On Thu, May 12, 2016 at 3:43 PM, Brian Norris <briannorris@chromium.org> wrote:
> From: Shawn Lin <shawn.lin@rock-chips.com>
>
> According to the databook, 10.2us is the max time for dll to be ready to
> work. However in testing, some chips need 20us for dll to be ready. This
> patch adds some extra margin for dllrdy to be ready, fixing our
> -ETIMEDOUT issues.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
>  drivers/phy/phy-rockchip-emmc.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/phy/phy-rockchip-emmc.c b/drivers/phy/phy-rockchip-emmc.c
> index 6ebcf3e41c46..48cbe691a889 100644
> --- a/drivers/phy/phy-rockchip-emmc.c
> +++ b/drivers/phy/phy-rockchip-emmc.c
> @@ -119,10 +119,11 @@ static int rockchip_emmc_phy_power(struct rockchip_emmc_phy *rk_phy,
>                                    PHYCTRL_ENDLL_MASK,
>                                    PHYCTRL_ENDLL_SHIFT));
>         /*
> -        * After enable analog DLL circuits, we need extra 10.2us
> -        * for dll to be ready for work.
> +        * After enable analog DLL circuits, we need an extra 10.2us
> +        * for dll to be ready for work. But according to testing, we
> +        * find some chips need more than 25us.
>          */
> -       udelay(11);
> +       udelay(30);
>         regmap_read(rk_phy->reg_base,
>                     rk_phy->reg_offset + GRF_EMMCPHY_STATUS,
>                     &dllrdy);

Seems sane.  This is a "random delay" but you've documented it well,
and an extra 19 microseconds won't be the end of the world.

If we truly trusted the "DLLRDY" bit we could actually do a loop here,
but it's probably not worth it...

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Heiko Stübner June 16, 2016, 11:35 p.m. UTC | #2
Am Donnerstag, 12. Mai 2016, 15:43:03 schrieb Brian Norris:
> From: Shawn Lin <shawn.lin@rock-chips.com>
> 
> According to the databook, 10.2us is the max time for dll to be ready to
> work. However in testing, some chips need 20us for dll to be ready. This
> patch adds some extra margin for dllrdy to be ready, fixing our
> -ETIMEDOUT issues.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> Signed-off-by: Brian Norris <briannorris@chromium.org>

haven't looked to deep into the emmc-phy block, but on my rk3399-evb board 
the emmc still runs nicely with this patch, so

Tested-by: Heiko Stuebner <heiko@sntech.de>
Kishon Vijay Abraham I June 20, 2016, 1:11 p.m. UTC | #3
On Friday 13 May 2016 04:13 AM, Brian Norris wrote:
> From: Shawn Lin <shawn.lin@rock-chips.com>
> 
> According to the databook, 10.2us is the max time for dll to be ready to
> work. However in testing, some chips need 20us for dll to be ready. This
> patch adds some extra margin for dllrdy to be ready, fixing our
> -ETIMEDOUT issues.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> Signed-off-by: Brian Norris <briannorris@chromium.org>

Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/phy/phy-rockchip-emmc.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/phy/phy-rockchip-emmc.c b/drivers/phy/phy-rockchip-emmc.c
> index 6ebcf3e41c46..48cbe691a889 100644
> --- a/drivers/phy/phy-rockchip-emmc.c
> +++ b/drivers/phy/phy-rockchip-emmc.c
> @@ -119,10 +119,11 @@ static int rockchip_emmc_phy_power(struct rockchip_emmc_phy *rk_phy,
>  				   PHYCTRL_ENDLL_MASK,
>  				   PHYCTRL_ENDLL_SHIFT));
>  	/*
> -	 * After enable analog DLL circuits, we need extra 10.2us
> -	 * for dll to be ready for work.
> +	 * After enable analog DLL circuits, we need an extra 10.2us
> +	 * for dll to be ready for work. But according to testing, we
> +	 * find some chips need more than 25us.
>  	 */
> -	udelay(11);
> +	udelay(30);
>  	regmap_read(rk_phy->reg_base,
>  		    rk_phy->reg_offset + GRF_EMMCPHY_STATUS,
>  		    &dllrdy);
>
Brian Norris June 20, 2016, 4:19 p.m. UTC | #4
+ linux-mmc, Ulf

On Mon, Jun 20, 2016 at 06:41:26PM +0530, Kishon Vijay Abraham I wrote:
> On Friday 13 May 2016 04:13 AM, Brian Norris wrote:
> > From: Shawn Lin <shawn.lin@rock-chips.com>
> > 
> > According to the databook, 10.2us is the max time for dll to be ready to
> > work. However in testing, some chips need 20us for dll to be ready. This
> > patch adds some extra margin for dllrdy to be ready, fixing our
> > -ETIMEDOUT issues.
> > 
> > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> > Signed-off-by: Brian Norris <briannorris@chromium.org>
> 
> Acked-by: Kishon Vijay Abraham I <kishon@ti.com>

Thanks! I take it that to mean you're agreeing with Doug's suggestion
here:

https://lkml.org/lkml/2016/6/13/1067

"Since [the $subject series patches] aren't in 4.7-rc1 presumably they
would also make sense to take through the MMC tree if others agree."

Ulf,

I see I didn't CC you or linux-mmc on this patch series. Please let me
know if I should resend, or if you can pick these up off of LKML -- Doug
gave nice patchwork links in his linked series. NB: patch 2 has a
version 2, while the others were left unmodified.

Regards,
Brian
Doug Anderson June 20, 2016, 4:25 p.m. UTC | #5
Hi,

On Mon, Jun 20, 2016 at 9:19 AM, Brian Norris <briannorris@chromium.org> wrote:
> + linux-mmc, Ulf
>
> On Mon, Jun 20, 2016 at 06:41:26PM +0530, Kishon Vijay Abraham I wrote:
>> On Friday 13 May 2016 04:13 AM, Brian Norris wrote:
>> > From: Shawn Lin <shawn.lin@rock-chips.com>
>> >
>> > According to the databook, 10.2us is the max time for dll to be ready to
>> > work. However in testing, some chips need 20us for dll to be ready. This
>> > patch adds some extra margin for dllrdy to be ready, fixing our
>> > -ETIMEDOUT issues.
>> >
>> > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> > Signed-off-by: Brian Norris <briannorris@chromium.org>
>>
>> Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
>
> Thanks! I take it that to mean you're agreeing with Doug's suggestion
> here:
>
> https://lkml.org/lkml/2016/6/13/1067
>
> "Since [the $subject series patches] aren't in 4.7-rc1 presumably they
> would also make sense to take through the MMC tree if others agree."
>
> Ulf,
>
> I see I didn't CC you or linux-mmc on this patch series. Please let me
> know if I should resend, or if you can pick these up off of LKML -- Doug
> gave nice patchwork links in his linked series. NB: patch 2 has a
> version 2, while the others were left unmodified.

I actually need to spin my series to address a small bit of feedback
from Heiko.  Since Brian's original patches weren't on linux-mmc, I'll
include them in my series (along with collected tags) to make it easy.
Hope this is OK.

-Doug
Brian Norris June 20, 2016, 4:50 p.m. UTC | #6
On Mon, Jun 20, 2016 at 09:25:18AM -0700, Doug Anderson wrote:
> On Mon, Jun 20, 2016 at 9:19 AM, Brian Norris <briannorris@chromium.org> wrote:
> > Ulf,
> >
> > I see I didn't CC you or linux-mmc on this patch series. Please let me
> > know if I should resend, or if you can pick these up off of LKML -- Doug
> > gave nice patchwork links in his linked series. NB: patch 2 has a
> > version 2, while the others were left unmodified.
> 
> I actually need to spin my series to address a small bit of feedback
> from Heiko.  Since Brian's original patches weren't on linux-mmc, I'll
> include them in my series (along with collected tags) to make it easy.
> Hope this is OK.

Fine with me. Thanks.

Brian
diff mbox

Patch

diff --git a/drivers/phy/phy-rockchip-emmc.c b/drivers/phy/phy-rockchip-emmc.c
index 6ebcf3e41c46..48cbe691a889 100644
--- a/drivers/phy/phy-rockchip-emmc.c
+++ b/drivers/phy/phy-rockchip-emmc.c
@@ -119,10 +119,11 @@  static int rockchip_emmc_phy_power(struct rockchip_emmc_phy *rk_phy,
 				   PHYCTRL_ENDLL_MASK,
 				   PHYCTRL_ENDLL_SHIFT));
 	/*
-	 * After enable analog DLL circuits, we need extra 10.2us
-	 * for dll to be ready for work.
+	 * After enable analog DLL circuits, we need an extra 10.2us
+	 * for dll to be ready for work. But according to testing, we
+	 * find some chips need more than 25us.
 	 */
-	udelay(11);
+	udelay(30);
 	regmap_read(rk_phy->reg_base,
 		    rk_phy->reg_offset + GRF_EMMCPHY_STATUS,
 		    &dllrdy);