Re: [TEGRA194_CPUFREQ Patch v2 2/3] cpufreq: Add Tegra194 cpufreq driver

From: Dmitry Osipenko
Date: Sun Apr 05 2020 - 10:05:29 EST


04.04.2020 22:29, Sumit Gupta ÐÐÑÐÑ:
...
> +static void tegra_read_counters(struct work_struct *work)
> +{
> + struct read_counters_work *read_counters_work;
> + struct tegra_cpu_ctr *c;
> + u64 val;
> +
> + /*
> + * ref_clk_counter(32 bit counter) runs on constant clk,
> + * pll_p(408MHz).
> + * It will take = 2 ^ 32 / 408 MHz to overflow ref clk counter
> + * = 10526880 usec = 10.527 sec to overflow
> + *
> + * Like wise core_clk_counter(32 bit counter) runs on core clock.
> + * It's synchronized to crab_clk (cpu_crab_clk) which runs at
> + * freq of cluster. Assuming max cluster clock ~2000MHz,
> + * It will take = 2 ^ 32 / 2000 MHz to overflow core clk counter
> + * = ~2.147 sec to overflow
> + */
> + read_counters_work = container_of(work, struct read_counters_work,
> + work);
> + c = &read_counters_work->c;
> +
> + val = read_freq_feedback();
> + c->last_refclk_cnt = lower_32_bits(val);
> + c->last_coreclk_cnt = upper_32_bits(val);
> + udelay(c->delay);
> + val = read_freq_feedback();
> + c->refclk_cnt = lower_32_bits(val);
> + c->coreclk_cnt = upper_32_bits(val);
> +}
> +
> +/*
> + * Return instantaneous cpu speed
> + * Instantaneous freq is calculated as -
> + * -Takes sample on every query of getting the freq.
> + * - Read core and ref clock counters;
> + * - Delay for X us
> + * - Read above cycle counters again
> + * - Calculates freq by subtracting current and previous counters
> + * divided by the delay time or eqv. of ref_clk_counter in delta time
> + * - Return Kcycles/second, freq in KHz
> + *
> + * delta time period = x sec
> + * = delta ref_clk_counter / (408 * 10^6) sec
> + * freq in Hz = cycles/sec
> + * = (delta cycles / x sec
> + * = (delta cycles * 408 * 10^6) / delta ref_clk_counter
> + * in KHz = (delta cycles * 408 * 10^3) / delta ref_clk_counter
> + *
> + * @cpu - logical cpu whose freq to be updated


> + * Returns freq in KHz on success, 0 if cpu is offline

I don't see any checks in the code about whether CPU is offline.

Googling for "queue_work_on offline cpu" suggests that this function
should hang.

> + */
> +static unsigned int tegra194_get_speed_common(u32 cpu, u32 delay)
> +{
> + struct read_counters_work read_counters_work;
> + struct tegra_cpu_ctr c;
> + u32 delta_refcnt;
> + u32 delta_ccnt;
> + u32 rate_mhz;
> +
> + read_counters_work.c.cpu = cpu;
> + read_counters_work.c.delay = delay;
> + INIT_WORK_ONSTACK(&read_counters_work.work, tegra_read_counters);
> + queue_work_on(cpu, read_counters_wq, &read_counters_work.work);
> + flush_work(&read_counters_work.work);
> + c = read_counters_work.c;
> +
> + if (c.coreclk_cnt < c.last_coreclk_cnt)
> + delta_ccnt = c.coreclk_cnt + (MAX_CNT - c.last_coreclk_cnt);
> + else
> + delta_ccnt = c.coreclk_cnt - c.last_coreclk_cnt;
> + if (!delta_ccnt)
> + return 0;
> +
> + /* ref clock is 32 bits */
> + if (c.refclk_cnt < c.last_refclk_cnt)
> + delta_refcnt = c.refclk_cnt + (MAX_CNT - c.last_refclk_cnt);
> + else
> + delta_refcnt = c.refclk_cnt - c.last_refclk_cnt;
> + if (!delta_refcnt) {
> + pr_debug("cpufreq: %d is idle, delta_refcnt: 0\n", cpu);
> + return 0;
> + }
> + rate_mhz = ((unsigned long)(delta_ccnt * REF_CLK_MHZ)) / delta_refcnt;
> +
> + return (rate_mhz * KHZ); /* in KHz */
> +}