Hi,
On Tue, Jan 9, 2018 at 11:37 PM, Caesar Wang <wxt at rock-chips.com> wrote:
> From: Shawn Lin <shawn.lin at rock-chips.com>
>
> It turns out that 5us isn't enough for all cases, so let's
> retry some more times to wait for caldone.
>
> Signed-off-by: Shawn Lin <shawn.lin at rock-chips.com>
> Tested-by: Ziyuan Xu <xzy.xu at rock-chips.com>
> Signed-off-by: Caesar Wang <wxt at rock-chips.com>
> ---
>
> Changes in v2:
> - print the return valut with regmap_read_poll_timeout failing.
I agree with Brian that it was quite confusing to see a v2 from both
you and Shawn.
> drivers/phy/rockchip/phy-rockchip-emmc.c | 27 +++++++++++++++++----------
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-emmc.c b/drivers/phy/rockchip/phy-rockchip-emmc.c
> index f1b24f1..574838f 100644
> --- a/drivers/phy/rockchip/phy-rockchip-emmc.c
> +++ b/drivers/phy/rockchip/phy-rockchip-emmc.c
> @@ -76,6 +76,10 @@
> #define PHYCTRL_OTAPDLYSEL_MASK 0xf
> #define PHYCTRL_OTAPDLYSEL_SHIFT 0x7
>
> +#define PHYCTRL_IS_CALDONE(x) \
> + ((((x) >> PHYCTRL_CALDONE_SHIFT) & \
> + PHYCTRL_CALDONE_MASK) == PHYCTRL_CALDONE_DONE)
> +
> struct rockchip_emmc_phy {
> unsigned int reg_offset;
> struct regmap *reg_base;
> @@ -90,6 +94,7 @@ static int rockchip_emmc_phy_power(struct phy *phy, bool on_off)
> unsigned int freqsel = PHYCTRL_FREQSEL_200M;
> unsigned long rate;
> unsigned long timeout;
> + int ret;
>
> /*
> * Keep phyctrl_pdb and phyctrl_endll low to allow
> @@ -160,17 +165,19 @@ static int rockchip_emmc_phy_power(struct phy *phy, bool on_off)
> PHYCTRL_PDB_SHIFT));
>
> /*
> - * According to the user manual, it asks driver to
> - * wait 5us for calpad busy trimming
> + * According to the user manual, it asks driver to wait 5us for
> + * calpad busy trimming. However it is documented that this value is
> + * PVT(A.K.A process,voltage and temperature) relevant, so some
> + * failure cases are found which indicates we should be more tolerant
> + * to calpad busy trimming.
> */
> - udelay(5);
> - regmap_read(rk_phy->reg_base,
> - rk_phy->reg_offset + GRF_EMMCPHY_STATUS,
> - &caldone);
> - caldone = (caldone >> PHYCTRL_CALDONE_SHIFT) & PHYCTRL_CALDONE_MASK;
> - if (caldone != PHYCTRL_CALDONE_DONE) {
> - pr_err("rockchip_emmc_phy_power: caldone timeout.\n");
> - return -ETIMEDOUT;
> + ret = regmap_read_poll_timeout(rk_phy->reg_base,
> + rk_phy->reg_offset + GRF_EMMCPHY_STATUS,
> + caldone, PHYCTRL_IS_CALDONE(caldone),
> + 5, 50);
>
> + if (ret) {
> + pr_err("%s: caldone timeout, ret=%d\n", __func__, ret);
In Shawn's v2, AKA <https://patchwork.kernel.org/patch/10154795/>,
this says "caldone failed", which I like better since not all failures
will be timeouts.
