[v3,01/15] phy: rockchip-emmc: give DLL some extra time to be ready
diff mbox

Message ID 1466445414-11974-2-git-send-email-dianders@chromium.org
State New
Headers show

Commit Message

Douglas Anderson June 20, 2016, 5:56 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>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
---
Changes in v3:
- Add Brian's PHY patches into my series

Changes in v2: None

 drivers/phy/phy-rockchip-emmc.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Guenter Roeck June 20, 2016, 7:23 p.m. UTC | #1
On Mon, Jun 20, 2016 at 10:56 AM, Douglas Anderson
<dianders@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

Nitpick, but description (20uS) and code (more than 25 uS) don't match.

> 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>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> ---
> Changes in v3:
> - Add Brian's PHY patches into my series
>
> Changes in v2: None
>
>  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);
> --
> 2.8.0.rc3.226.g39d4020
>
Douglas Anderson June 20, 2016, 7:30 p.m. UTC | #2
Hi,

On Mon, Jun 20, 2016 at 12:23 PM, Guenter Roeck <groeck@google.com> wrote:
> On Mon, Jun 20, 2016 at 10:56 AM, Douglas Anderson
> <dianders@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
>
> Nitpick, but description (20uS) and code (more than 25 uS) don't match.

Good catch.  If I spin the series I'll fix this, otherwise I'll assume
that the person applying this (Ulf?) can fix the description at their
discretion.

-Doug
Guenter Roeck June 20, 2016, 7:36 p.m. UTC | #3
On Mon, Jun 20, 2016 at 12:30 PM, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Mon, Jun 20, 2016 at 12:23 PM, Guenter Roeck <groeck@google.com> wrote:
>> On Mon, Jun 20, 2016 at 10:56 AM, Douglas Anderson
>> <dianders@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
>>
>> Nitpick, but description (20uS) and code (more than 25 uS) don't match.
>
> Good catch.  If I spin the series I'll fix this, otherwise I'll assume
> that the person applying this (Ulf?) can fix the description at their
> discretion.
>

Not that it matters much, as the code is replaced in a later patch
anyway. It might make more sense to merge the two patches into one.

Guenter
Douglas Anderson June 20, 2016, 7:38 p.m. UTC | #4
Hi,

On Mon, Jun 20, 2016 at 12:36 PM, Guenter Roeck <groeck@google.com> wrote:
> On Mon, Jun 20, 2016 at 12:30 PM, Doug Anderson <dianders@chromium.org> wrote:
>> Hi,
>>
>> On Mon, Jun 20, 2016 at 12:23 PM, Guenter Roeck <groeck@google.com> wrote:
>>> On Mon, Jun 20, 2016 at 10:56 AM, Douglas Anderson
>>> <dianders@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
>>>
>>> Nitpick, but description (20uS) and code (more than 25 uS) don't match.
>>
>> Good catch.  If I spin the series I'll fix this, otherwise I'll assume
>> that the person applying this (Ulf?) can fix the description at their
>> discretion.
>>
>
> Not that it matters much, as the code is replaced in a later patch
> anyway. It might make more sense to merge the two patches into one.

Yes.  At the time I wrote it it was unclear what the status of the
original patch was (whether it would be applied sooner vs. later).  At
this point it seemed expedient to continue to keep the patches
separate since they had been reviewed and tested separately.  Earlier
I made the offer to combine and re-spin and I'm happy to do that if
requested.

-Doug

Patch
diff mbox

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