09.04.2020 20:52, Thierry Reding пишет:
...
> +static int tegra210_emc_set_rate(struct device *dev,
> + const struct tegra210_clk_emc_config *config)
> +{
> + struct tegra210_emc *emc = dev_get_drvdata(dev);
> + struct tegra210_emc_timing *timing = NULL;
> + unsigned long rate = config->rate;
> + s64 last_change_delay;
> + unsigned long flags;
> + unsigned int i;
> +
> + if (rate == emc->last->rate * 1000UL)
> + return 0;
Couldn't all the rates be expressed in Hz? Then you won't need all these
multiplications by 1000.
> + for (i = 0; i < emc->num_timings; i++) {
> + if (emc->timings[i].rate * 1000UL == rate) {
> + timing = &emc->timings[i];
> + break;
> + }
> + }
> +
> + if (!timing)
> + return -EINVAL;
> +
> + if (rate > 204000000 && !timing->trained)
> + return -EINVAL;
> +
> + emc->next = timing;
> + last_change_delay = ktime_us_delta(ktime_get(), emc->clkchange_time);
> +
> + /* XXX use non-busy-looping sleep? */
> + if ((last_change_delay >= 0) &&
> + (last_change_delay < emc->clkchange_delay))
> + udelay(emc->clkchange_delay - (int)last_change_delay);
> +
> + spin_lock_irqsave(&emc->lock, flags);
> + tegra210_emc_set_clock(emc, config->value);
> + emc->clkchange_time = ktime_get();
> + emc->last = timing;
> + spin_unlock_irqrestore(&emc->lock, flags);
> +
> + return 0;
> +}
I'd suggest to check how much time invocation of ktime_get() takes, at
least it came to a surprise to me in a case of the tegra-cpuidle driver.
It may be well over the emc->clkchange_delay.
...
> +static int tegra210_emc_probe(struct platform_device *pdev)
> +{
...
> + emc->clkchange_delay = 100;
> + emc->training_interval = 100;
Not sure why these aren't a constant with the code.. ?
