Re: [PATCH v3 2/2] phy: rockchip-emmc: use regmap_read_poll_timeout to poll dllrdy

[Doug Anderson]

Hi,

On Wed, Jan 10, 2018 at 6:40 PM, Caesar Wang  wrote:
> From: Shawn Lin 
>
> Just use the API instead of open-coding it, no functional change
> intended.
>
> Signed-off-by: Shawn Lin 
> Reviewed-by: Brian Norris 
> Signed-off-by: Caesar Wang 
>
> ---
>
> Changes in v3:
> - As Doug commented on https://patchwork.kernel.org/patch/10154797,
>   Change "1, 50" to "0, 50".
>
> Changes in v2:
> - As Brian commented on https://patchwork.kernel.org/patch/10139891/,
>   changed the note and added to print error value with
>   regmap_read_poll_timeout API.
>
>  drivers/phy/rockchip/phy-rockchip-emmc.c | 33 
> +++-
>  1 file changed, 11 insertions(+), 22 deletions(-)

Reviewed-by: Douglas Anderson 

Re: [PATCH v3 2/2] phy: rockchip-emmc: use regmap_read_poll_timeout to poll dllrdy

[Doug Anderson]

Hi,

On Wed, Jan 10, 2018 at 6:40 PM, Caesar Wang  wrote:
> From: Shawn Lin 
>
> Just use the API instead of open-coding it, no functional change
> intended.
>
> Signed-off-by: Shawn Lin 
> Reviewed-by: Brian Norris 
> Signed-off-by: Caesar Wang 
>
> ---
>
> Changes in v3:
> - As Doug commented on https://patchwork.kernel.org/patch/10154797,
>   Change "1, 50" to "0, 50".
>
> Changes in v2:
> - As Brian commented on https://patchwork.kernel.org/patch/10139891/,
>   changed the note and added to print error value with
>   regmap_read_poll_timeout API.
>
>  drivers/phy/rockchip/phy-rockchip-emmc.c | 33 
> +++-
>  1 file changed, 11 insertions(+), 22 deletions(-)

Reviewed-by: Douglas Anderson 

[PATCH v3 2/2] phy: rockchip-emmc: use regmap_read_poll_timeout to poll dllrdy

[Caesar Wang]

From: Shawn Lin 

Just use the API instead of open-coding it, no functional change
intended.

Signed-off-by: Shawn Lin 
Reviewed-by: Brian Norris 
Signed-off-by: Caesar Wang 

---

Changes in v3:
- As Doug commented on https://patchwork.kernel.org/patch/10154797,
  Change "1, 50" to "0, 50".

Changes in v2:
- As Brian commented on https://patchwork.kernel.org/patch/10139891/,
  changed the note and added to print error value with
  regmap_read_poll_timeout API.

 drivers/phy/rockchip/phy-rockchip-emmc.c | 33 +++-
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-emmc.c 
b/drivers/phy/rockchip/phy-rockchip-emmc.c
index b0d1093..b237360 100644
--- a/drivers/phy/rockchip/phy-rockchip-emmc.c
+++ b/drivers/phy/rockchip/phy-rockchip-emmc.c
@@ -79,6 +79,9 @@
 #define PHYCTRL_IS_CALDONE(x) \
x) >> PHYCTRL_CALDONE_SHIFT) & \
  PHYCTRL_CALDONE_MASK) == PHYCTRL_CALDONE_DONE)
+#define PHYCTRL_IS_DLLRDY(x) \
+   x) >> PHYCTRL_DLLRDY_SHIFT) & \
+ PHYCTRL_DLLRDY_MASK) == PHYCTRL_DLLRDY_DONE)
 
 struct rockchip_emmc_phy {
unsigned intreg_offset;
@@ -93,7 +96,6 @@ static int rockchip_emmc_phy_power(struct phy *phy, bool 
on_off)
unsigned int dllrdy;
unsigned int freqsel = PHYCTRL_FREQSEL_200M;
unsigned long rate;
-   unsigned long timeout;
int ret;
 
/*
@@ -217,28 +219,15 @@ static int rockchip_emmc_phy_power(struct phy *phy, bool 
on_off)
 * NOTE: There appear to be corner cases where the DLL seems to take
 * extra long to lock for reasons that aren't understood.  In some
 * extreme cases we've seen it take up to over 10ms (!).  We'll be
-* generous and give it 50ms.  We still busy wait here because:
-* - In most cases it should be super fast.
-* - This is not called lots during normal operation so it shouldn't
-*   be a power or performance problem to busy wait.  We expect it
-*   only at boot / resume.  In both cases, eMMC is probably on the
-*   critical path so busy waiting a little extra time should be OK.
+* generous and give it 50ms.
 */
-   timeout = jiffies + msecs_to_jiffies(50);
-   do {
-   udelay(1);
-
-   regmap_read(rk_phy->reg_base,
-   rk_phy->reg_offset + GRF_EMMCPHY_STATUS,
-   );
-   dllrdy = (dllrdy >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK;
-   if (dllrdy == PHYCTRL_DLLRDY_DONE)
-   break;
-   } while (!time_after(jiffies, timeout));
-
-   if (dllrdy != PHYCTRL_DLLRDY_DONE) {
-   pr_err("rockchip_emmc_phy_power: dllrdy timeout.\n");
-   return -ETIMEDOUT;
+   ret = regmap_read_poll_timeout(rk_phy->reg_base,
+  rk_phy->reg_offset + GRF_EMMCPHY_STATUS,
+  dllrdy, PHYCTRL_IS_DLLRDY(dllrdy),
+  0, 50 * USEC_PER_MSEC);
+   if (ret) {
+   pr_err("%s: dllrdy failed. ret=%d\n", __func__, ret);
+   return ret;
}
 
return 0;
-- 
2.7.4

[PATCH v3 2/2] phy: rockchip-emmc: use regmap_read_poll_timeout to poll dllrdy

[Caesar Wang]

From: Shawn Lin 

Just use the API instead of open-coding it, no functional change
intended.

Signed-off-by: Shawn Lin 
Reviewed-by: Brian Norris 
Signed-off-by: Caesar Wang 

---

Changes in v3:
- As Doug commented on https://patchwork.kernel.org/patch/10154797,
  Change "1, 50" to "0, 50".

Changes in v2:
- As Brian commented on https://patchwork.kernel.org/patch/10139891/,
  changed the note and added to print error value with
  regmap_read_poll_timeout API.

 drivers/phy/rockchip/phy-rockchip-emmc.c | 33 +++-
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-emmc.c 
b/drivers/phy/rockchip/phy-rockchip-emmc.c
index b0d1093..b237360 100644
--- a/drivers/phy/rockchip/phy-rockchip-emmc.c
+++ b/drivers/phy/rockchip/phy-rockchip-emmc.c
@@ -79,6 +79,9 @@
 #define PHYCTRL_IS_CALDONE(x) \
x) >> PHYCTRL_CALDONE_SHIFT) & \
  PHYCTRL_CALDONE_MASK) == PHYCTRL_CALDONE_DONE)
+#define PHYCTRL_IS_DLLRDY(x) \
+   x) >> PHYCTRL_DLLRDY_SHIFT) & \
+ PHYCTRL_DLLRDY_MASK) == PHYCTRL_DLLRDY_DONE)
 
 struct rockchip_emmc_phy {
unsigned intreg_offset;
@@ -93,7 +96,6 @@ static int rockchip_emmc_phy_power(struct phy *phy, bool 
on_off)
unsigned int dllrdy;
unsigned int freqsel = PHYCTRL_FREQSEL_200M;
unsigned long rate;
-   unsigned long timeout;
int ret;
 
/*
@@ -217,28 +219,15 @@ static int rockchip_emmc_phy_power(struct phy *phy, bool 
on_off)
 * NOTE: There appear to be corner cases where the DLL seems to take
 * extra long to lock for reasons that aren't understood.  In some
 * extreme cases we've seen it take up to over 10ms (!).  We'll be
-* generous and give it 50ms.  We still busy wait here because:
-* - In most cases it should be super fast.
-* - This is not called lots during normal operation so it shouldn't
-*   be a power or performance problem to busy wait.  We expect it
-*   only at boot / resume.  In both cases, eMMC is probably on the
-*   critical path so busy waiting a little extra time should be OK.
+* generous and give it 50ms.
 */
-   timeout = jiffies + msecs_to_jiffies(50);
-   do {
-   udelay(1);
-
-   regmap_read(rk_phy->reg_base,
-   rk_phy->reg_offset + GRF_EMMCPHY_STATUS,
-   );
-   dllrdy = (dllrdy >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK;
-   if (dllrdy == PHYCTRL_DLLRDY_DONE)
-   break;
-   } while (!time_after(jiffies, timeout));
-
-   if (dllrdy != PHYCTRL_DLLRDY_DONE) {
-   pr_err("rockchip_emmc_phy_power: dllrdy timeout.\n");
-   return -ETIMEDOUT;
+   ret = regmap_read_poll_timeout(rk_phy->reg_base,
+  rk_phy->reg_offset + GRF_EMMCPHY_STATUS,
+  dllrdy, PHYCTRL_IS_DLLRDY(dllrdy),
+  0, 50 * USEC_PER_MSEC);
+   if (ret) {
+   pr_err("%s: dllrdy failed. ret=%d\n", __func__, ret);
+   return ret;
}
 
return 0;
-- 
2.7.4