? 2018?01?11? 03:36, Doug Anderson ??:
> Hi,
>
> On Wed, Jan 10, 2018 at 9:46 AM, Brian Norris <briannorris at chromium.org> wrote:
>>> */
>>> - timeout = jiffies + msecs_to_jiffies(50);
>>> - do {
>>> - udelay(1);
>>> -
>>> - regmap_read(rk_phy->reg_base,
>>> - rk_phy->reg_offset + GRF_EMMCPHY_STATUS,
>>> - &dllrdy);
>>> - 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),
>>> + 1, 50 * USEC_PER_MSEC);
> It seems a bit schizophrenic that one of our delay loops sleeps 1 us
> between loops and the other sleeps 5 us between loops.
>
> ...and, in fact, both of these numbers seem a little on the silly side
> of things. Assuming that the timer docs are up to date, usleep_range
> is intended for sleeping "10us - 20ms". Both 1 us and 5 us below that
> range and "1 us" is an order of magnitude below that range. ...your 1
> and 5 actually translate to usleep_range(1, 1) and usleep_range(3, 5).
>
> It seems like trying to do a sleep (the whole idea that some other
> process will get to run for some fraction of the 1 us) is just wasting
> cycles.
>
> So I'd say either:
>
> 1. Accept that we really expect this to be a long delay and change
> your delay to 10 us
>
> 2. Change the delay to 0 us and accept that you're busy waiting.
>
> I'd vote for #2 unless you have some evidence that we often need long
> delays and we've started calling this code all the time.
Agreed with #2
-Caesar
>
>
>>> + if (ret) {
>>> + pr_err("%s: dllrdy failed %d.\n", __func__, ret);
>>> + return ret;
>>> }
>>>
>>> return 0;
>>> --
>>> 1.9.1
>>>
>>>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
