Re: [PATCH v3] cpufreq: tegra194: get consistent cpuinfo_cur_freq
Thank you. On 27/10/20 12:28 PM, Viresh Kumar wrote: External email: Use caution opening links or attachments On 27-10-20, 11:46, Sumit Gupta wrote: Ping. I was waiting for 5.10-rc1 to be released before I can start applying stuff for 5.11. Now that it is released, I will apply this. -- viresh
Re: [PATCH v3] cpufreq: tegra194: get consistent cpuinfo_cur_freq
Ping. Frequency returned by 'cpuinfo_cur_freq' using counters is not fixed and keeps changing slightly. This change returns a consistent value from freq_table. If the reconstructed frequency has acceptable delta from the last written value, then return the frequency corresponding to the last written ndiv value from freq_table. Otherwise, print a warning and return the reconstructed freq. Signed-off-by: Sumit Gupta --- Sending only this patch as other patch not required after the change to convert 'pr_warn' to 'pr_info' in cpufreq core for unlisted freq. Changelog v1[2] -> v3: - Removed unwanted checks for cpu_online and max cluster number - Used WARN_ON_ONCE to avoid print flooding. v1[1] -> v2: - Minor changes to improve comments and reduce debug prints. - Get freq table from cluster specific data instead of policy. [2] https://marc.info/?l=linux-tegra=160216218511280=2 [1] https://marc.info/?l=linux-arm-kernel=160028821117535=2 drivers/cpufreq/tegra194-cpufreq.c | 62 -- 1 file changed, 53 insertions(+), 9 deletions(-) diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c index e1d931c..7901587 100644 --- a/drivers/cpufreq/tegra194-cpufreq.c +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -180,9 +180,61 @@ static unsigned int tegra194_get_speed_common(u32 cpu, u32 delay) return (rate_mhz * KHZ); /* in KHz */ } +static void get_cpu_ndiv(void *ndiv) +{ + u64 ndiv_val; + + asm volatile("mrs %0, s3_0_c15_c0_4" : "=r" (ndiv_val) : ); + + *(u64 *)ndiv = ndiv_val; +} + +static void set_cpu_ndiv(void *data) +{ + struct cpufreq_frequency_table *tbl = data; + u64 ndiv_val = (u64)tbl->driver_data; + + asm volatile("msr s3_0_c15_c0_4, %0" : : "r" (ndiv_val)); +} + static unsigned int tegra194_get_speed(u32 cpu) { - return tegra194_get_speed_common(cpu, US_DELAY); + struct tegra194_cpufreq_data *data = cpufreq_get_driver_data(); + struct cpufreq_frequency_table *pos; + unsigned int rate; + u64 ndiv; + int ret; + u32 cl; + + smp_call_function_single(cpu, get_cpu_cluster, , true); + + /* reconstruct actual cpu freq using counters */ + rate = tegra194_get_speed_common(cpu, US_DELAY); + + /* get last written ndiv value */ + ret = smp_call_function_single(cpu, get_cpu_ndiv, , true); + if (WARN_ON_ONCE(ret)) + return rate; + + /* +* If the reconstructed frequency has acceptable delta from +* the last written value, then return freq corresponding +* to the last written ndiv value from freq_table. This is +* done to return consistent value. +*/ + cpufreq_for_each_valid_entry(pos, data->tables[cl]) { + if (pos->driver_data != ndiv) + continue; + + if (abs(pos->frequency - rate) > 115200) { + pr_warn("cpufreq: cpu%d,cur:%u,set:%u,set ndiv:%llu\n", + cpu, rate, pos->frequency, ndiv); + } else { + rate = pos->frequency; + } + break; + } + return rate; } static int tegra194_cpufreq_init(struct cpufreq_policy *policy) @@ -209,14 +261,6 @@ static int tegra194_cpufreq_init(struct cpufreq_policy *policy) return 0; } -static void set_cpu_ndiv(void *data) -{ - struct cpufreq_frequency_table *tbl = data; - u64 ndiv_val = (u64)tbl->driver_data; - - asm volatile("msr s3_0_c15_c0_4, %0" : : "r" (ndiv_val)); -} - static int tegra194_cpufreq_set_target(struct cpufreq_policy *policy, unsigned int index) { -- 2.7.4
Re: [PATCH] cpufreq: Improve code around unlisted freq check
On 13/10/20 10:42 AM, Viresh Kumar wrote: External email: Use caution opening links or attachments The cpufreq core checks if the frequency programmed by the bootloaders is not listed in the freq table and programs one from the table in such a case. This is done only if the driver has set the CPUFREQ_NEED_INITIAL_FREQ_CHECK flag. Currently we print two separate messages, with almost the same content, and do this with a pr_warn() which may be a bit too much as the driver only asked us to check this as it expected this to be the case. Lower down the severity of the print message by switching to pr_info() instead and print a single message only. Reviewed-by: Sumit Gupta Tested-by: Sumit Gupta Reported-by: Sumit Gupta Signed-off-by: Viresh Kumar --- drivers/cpufreq/cpufreq.c | 15 +++ 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 2ea245a6c0c0..99864afac272 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1461,14 +1461,13 @@ static int cpufreq_online(unsigned int cpu) */ if ((cpufreq_driver->flags & CPUFREQ_NEED_INITIAL_FREQ_CHECK) && has_target()) { + unsigned int old_freq = policy->cur; + /* Are we running at unknown frequency ? */ - ret = cpufreq_frequency_table_get_index(policy, policy->cur); + ret = cpufreq_frequency_table_get_index(policy, old_freq); if (ret == -EINVAL) { - /* Warn user and fix it */ - pr_warn("%s: CPU%d: Running at unlisted freq: %u KHz\n", - __func__, policy->cpu, policy->cur); - ret = __cpufreq_driver_target(policy, policy->cur - 1, - CPUFREQ_RELATION_L); + ret = __cpufreq_driver_target(policy, old_freq - 1, + CPUFREQ_RELATION_L); /* * Reaching here after boot in a few seconds may not @@ -1476,8 +1475,8 @@ static int cpufreq_online(unsigned int cpu) * frequency for longer duration. Hence, a BUG_ON(). */ BUG_ON(ret); - pr_warn("%s: CPU%d: Unlisted initial frequency changed to: %u KHz\n", - __func__, policy->cpu, policy->cur); + pr_info("%s: CPU%d: Running at unlisted initial frequency: %u KHz, changing to: %u KHz\n", + __func__, policy->cpu, old_freq, policy->cur); } } -- 2.25.0.rc1.19.g042ed3e048af
[PATCH v3] cpufreq: tegra194: get consistent cpuinfo_cur_freq
Frequency returned by 'cpuinfo_cur_freq' using counters is not fixed and keeps changing slightly. This change returns a consistent value from freq_table. If the reconstructed frequency has acceptable delta from the last written value, then return the frequency corresponding to the last written ndiv value from freq_table. Otherwise, print a warning and return the reconstructed freq. Signed-off-by: Sumit Gupta --- Sending only this patch as other patch not required after the change to convert 'pr_warn' to 'pr_info' in cpufreq core for unlisted freq. Changelog v1[2] -> v3: - Removed unwanted checks for cpu_online and max cluster number - Used WARN_ON_ONCE to avoid print flooding. v1[1] -> v2: - Minor changes to improve comments and reduce debug prints. - Get freq table from cluster specific data instead of policy. [2] https://marc.info/?l=linux-tegra=160216218511280=2 [1] https://marc.info/?l=linux-arm-kernel=160028821117535=2 drivers/cpufreq/tegra194-cpufreq.c | 62 -- 1 file changed, 53 insertions(+), 9 deletions(-) diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c index e1d931c..7901587 100644 --- a/drivers/cpufreq/tegra194-cpufreq.c +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -180,9 +180,61 @@ static unsigned int tegra194_get_speed_common(u32 cpu, u32 delay) return (rate_mhz * KHZ); /* in KHz */ } +static void get_cpu_ndiv(void *ndiv) +{ + u64 ndiv_val; + + asm volatile("mrs %0, s3_0_c15_c0_4" : "=r" (ndiv_val) : ); + + *(u64 *)ndiv = ndiv_val; +} + +static void set_cpu_ndiv(void *data) +{ + struct cpufreq_frequency_table *tbl = data; + u64 ndiv_val = (u64)tbl->driver_data; + + asm volatile("msr s3_0_c15_c0_4, %0" : : "r" (ndiv_val)); +} + static unsigned int tegra194_get_speed(u32 cpu) { - return tegra194_get_speed_common(cpu, US_DELAY); + struct tegra194_cpufreq_data *data = cpufreq_get_driver_data(); + struct cpufreq_frequency_table *pos; + unsigned int rate; + u64 ndiv; + int ret; + u32 cl; + + smp_call_function_single(cpu, get_cpu_cluster, , true); + + /* reconstruct actual cpu freq using counters */ + rate = tegra194_get_speed_common(cpu, US_DELAY); + + /* get last written ndiv value */ + ret = smp_call_function_single(cpu, get_cpu_ndiv, , true); + if (WARN_ON_ONCE(ret)) + return rate; + + /* +* If the reconstructed frequency has acceptable delta from +* the last written value, then return freq corresponding +* to the last written ndiv value from freq_table. This is +* done to return consistent value. +*/ + cpufreq_for_each_valid_entry(pos, data->tables[cl]) { + if (pos->driver_data != ndiv) + continue; + + if (abs(pos->frequency - rate) > 115200) { + pr_warn("cpufreq: cpu%d,cur:%u,set:%u,set ndiv:%llu\n", + cpu, rate, pos->frequency, ndiv); + } else { + rate = pos->frequency; + } + break; + } + return rate; } static int tegra194_cpufreq_init(struct cpufreq_policy *policy) @@ -209,14 +261,6 @@ static int tegra194_cpufreq_init(struct cpufreq_policy *policy) return 0; } -static void set_cpu_ndiv(void *data) -{ - struct cpufreq_frequency_table *tbl = data; - u64 ndiv_val = (u64)tbl->driver_data; - - asm volatile("msr s3_0_c15_c0_4, %0" : : "r" (ndiv_val)); -} - static int tegra194_cpufreq_set_target(struct cpufreq_policy *policy, unsigned int index) { -- 2.7.4
Re: [PATCH v2 2/2] cpufreq: tegra194: Fix unlisted boot freq warning
Warning coming during boot because the boot freq set by bootloader gets filtered out due to big freq steps while creating freq_table. Fix this by setting closest higher frequency from freq_table. Warning: cpufreq: cpufreq_online: CPU0: Running at unlisted freq cpufreq: cpufreq_online: CPU0: Unlisted initial frequency changed These warning messages also come during hotplug online of non-boot CPU's while exiting from 'Suspend-to-RAM'. This happens because during exit from 'Suspend-to-RAM', some time is taken to restore last software requested CPU frequency written in register before entering suspend. And who does this restoration ? To fix this, adding online hook to wait till the current frequency becomes equal or close to the last requested frequency. Fixes: df320f89359c ("cpufreq: Add Tegra194 cpufreq driver") Signed-off-by: Sumit Gupta --- drivers/cpufreq/tegra194-cpufreq.c | 86 ++ 1 file changed, 79 insertions(+), 7 deletions(-) diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c index d250e49..cc28b1e3 100644 --- a/drivers/cpufreq/tegra194-cpufreq.c +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -21,7 +22,6 @@ #define KHZ 1000 #define REF_CLK_MHZ 408 /* 408 MHz */ #define US_DELAY500 -#define US_DELAY_MIN2 #define CPUFREQ_TBL_STEP_HZ (50 * KHZ * KHZ) #define MAX_CNT ~0U @@ -249,17 +249,22 @@ static unsigned int tegra194_get_speed(u32 cpu) static int tegra194_cpufreq_init(struct cpufreq_policy *policy) { struct tegra194_cpufreq_data *data = cpufreq_get_driver_data(); - u32 cpu; + u32 cpu = policy->cpu; + int ret; u32 cl; - smp_call_function_single(policy->cpu, get_cpu_cluster, , true); + if (!cpu_online(cpu)) Not required to check this. OK. + return -EINVAL; + + ret = smp_call_function_single(cpu, get_cpu_cluster, , true); + if (ret) { Same as in the other patch. Got. + pr_err("cpufreq: Failed to get cluster for CPU%d\n", cpu); + return ret; + } if (cl >= data->num_clusters) return -EINVAL; - /* boot freq */ - policy->cur = tegra194_get_speed_common(policy->cpu, US_DELAY_MIN); - /* set same policy for all cpus in a cluster */ for (cpu = (cl * 2); cpu < ((cl + 1) * 2); cpu++) cpumask_set_cpu(cpu, policy->cpus); @@ -267,7 +272,23 @@ static int tegra194_cpufreq_init(struct cpufreq_policy *policy) policy->freq_table = data->tables[cl]; policy->cpuinfo.transition_latency = TEGRA_CPUFREQ_TRANSITION_LATENCY; - return 0; + policy->cur = tegra194_get_speed_common(policy->cpu, US_DELAY); + + ret = cpufreq_table_validate_and_sort(policy); + if (ret) + return ret; + + /* Are we running at unknown frequency ? */ + ret = cpufreq_frequency_table_get_index(policy, policy->cur); + if (ret == -EINVAL) { + ret = __cpufreq_driver_target(policy, policy->cur - 1, + CPUFREQ_RELATION_L); + if (ret) + return ret; + policy->cur = tegra194_get_speed_common(policy->cpu, US_DELAY); cpufreq-core will do this anyway, you don't need to do it. Got. + } + + return ret; } I wonder if I should change the pr_warn() in cpufreq-core to pr_info() instead, will that help you guys ? Will that still be a problem ? This is exactly same as what we do there. Yes, this will also work. Then we don't need the current patch. You want me to send a patch with change from pr_warn to pr_info? static int tegra194_cpufreq_set_target(struct cpufreq_policy *policy, @@ -285,6 +306,55 @@ static int tegra194_cpufreq_set_target(struct cpufreq_policy *policy, return 0; } +static int tegra194_cpufreq_online(struct cpufreq_policy *policy) +{ + unsigned int interm_freq, last_set_freq; + struct cpufreq_frequency_table *pos; + u64 ndiv; + int ret; + + if (!cpu_online(policy->cpu)) + return -EINVAL; + + /* get ndiv for the last frequency request from software */ + ret = smp_call_function_single(policy->cpu, get_cpu_ndiv, , true); + if (ret) { + pr_err("cpufreq: Failed to get ndiv for CPU%d\n", policy->cpu); + return ret; + } + + cpufreq_for_each_valid_entry(pos, policy->freq_table) { + if (pos->driver_data == ndiv) { + last_set_freq = pos->frequency; + break; + } + } + + policy->cur = tegra194_get_speed_common(policy->cpu, US_DELAY); + interm_freq = policy->cur; + + /* + * It takes some
Re: [PATCH v2 1/2] cpufreq: tegra194: get consistent cpuinfo_cur_freq
Frequency returned by 'cpuinfo_cur_freq' using counters is not fixed and keeps changing slightly. This change returns a consistent value from freq_table. If the reconstructed frequency has acceptable delta from the last written value, then return the frequency corresponding to the last written ndiv value from freq_table. Otherwise, print a warning and return the reconstructed freq. Signed-off-by: Sumit Gupta --- drivers/cpufreq/tegra194-cpufreq.c | 71 +- 1 file changed, 62 insertions(+), 9 deletions(-) diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c index e1d931c..d250e49 100644 --- a/drivers/cpufreq/tegra194-cpufreq.c +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -180,9 +180,70 @@ static unsigned int tegra194_get_speed_common(u32 cpu, u32 delay) return (rate_mhz * KHZ); /* in KHz */ } +static void get_cpu_ndiv(void *ndiv) +{ + u64 ndiv_val; + + asm volatile("mrs %0, s3_0_c15_c0_4" : "=r" (ndiv_val) : ); + + *(u64 *)ndiv = ndiv_val; +} + +static void set_cpu_ndiv(void *data) You weren't required to do this unnecessary change. ya, moved the function up to keep both {get_|set_} calls together. +{ + struct cpufreq_frequency_table *tbl = data; + u64 ndiv_val = (u64)tbl->driver_data; + + asm volatile("msr s3_0_c15_c0_4, %0" : : "r" (ndiv_val)); +} + static unsigned int tegra194_get_speed(u32 cpu) { - return tegra194_get_speed_common(cpu, US_DELAY); + struct tegra194_cpufreq_data *data = cpufreq_get_driver_data(); + struct cpufreq_frequency_table *pos; + unsigned int rate; + u64 ndiv; + int ret; + u32 cl; + + if (!cpu_online(cpu)) This isn't required. The CPU is guaranteed to be online here. OK, will remove this in next version. + return -EINVAL; + + smp_call_function_single(cpu, get_cpu_cluster, , true); + + if (cl >= data->num_clusters) Is it really possible here ? I meant you must have already checked this at cpufreq-init level already. Else mark it unlikely at least. Ya, will remove the check here. + return -EINVAL; + + /* reconstruct actual cpu freq using counters */ + rate = tegra194_get_speed_common(cpu, US_DELAY); + + /* get last written ndiv value */ + ret = smp_call_function_single(cpu, get_cpu_ndiv, , true); + if (ret) { What exactly can fail here ? get_cpu_ndiv() can't fail. Do we really need this check ? What about WARN_ON_ONCE() ? OK. + pr_err("cpufreq: Failed to get ndiv for CPU%d, ret:%d\n", +cpu, ret); + return rate; + } + + /* + * If the reconstructed frequency has acceptable delta from + * the last written value, then return freq corresponding + * to the last written ndiv value from freq_table. This is + * done to return consistent value. + */ + cpufreq_for_each_valid_entry(pos, data->tables[cl]) { + if (pos->driver_data != ndiv) + continue; + + if (abs(pos->frequency - rate) > 115200) { where does this 115200 comes from ? Strange that it matches tty's baud rate :) The value is equal to one freq step size. This is 115 MHz, right ? Isn't that too big of a delta ? The is the acceptable delta used during our testing keeping some margin. + pr_warn("cpufreq: cpu%d,cur:%u,set:%u,set ndiv:%llu\n", + cpu, rate, pos->frequency, ndiv); + } else { + rate = pos->frequency; + } + break; + } + return rate; } -- viresh
[PATCH v2 2/2] cpufreq: tegra194: Fix unlisted boot freq warning
Warning coming during boot because the boot freq set by bootloader gets filtered out due to big freq steps while creating freq_table. Fix this by setting closest higher frequency from freq_table. Warning: cpufreq: cpufreq_online: CPU0: Running at unlisted freq cpufreq: cpufreq_online: CPU0: Unlisted initial frequency changed These warning messages also come during hotplug online of non-boot CPU's while exiting from 'Suspend-to-RAM'. This happens because during exit from 'Suspend-to-RAM', some time is taken to restore last software requested CPU frequency written in register before entering suspend. To fix this, adding online hook to wait till the current frequency becomes equal or close to the last requested frequency. Fixes: df320f89359c ("cpufreq: Add Tegra194 cpufreq driver") Signed-off-by: Sumit Gupta --- drivers/cpufreq/tegra194-cpufreq.c | 86 ++ 1 file changed, 79 insertions(+), 7 deletions(-) diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c index d250e49..cc28b1e3 100644 --- a/drivers/cpufreq/tegra194-cpufreq.c +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -21,7 +22,6 @@ #define KHZ 1000 #define REF_CLK_MHZ 408 /* 408 MHz */ #define US_DELAY500 -#define US_DELAY_MIN2 #define CPUFREQ_TBL_STEP_HZ (50 * KHZ * KHZ) #define MAX_CNT ~0U @@ -249,17 +249,22 @@ static unsigned int tegra194_get_speed(u32 cpu) static int tegra194_cpufreq_init(struct cpufreq_policy *policy) { struct tegra194_cpufreq_data *data = cpufreq_get_driver_data(); - u32 cpu; + u32 cpu = policy->cpu; + int ret; u32 cl; - smp_call_function_single(policy->cpu, get_cpu_cluster, , true); + if (!cpu_online(cpu)) + return -EINVAL; + + ret = smp_call_function_single(cpu, get_cpu_cluster, , true); + if (ret) { + pr_err("cpufreq: Failed to get cluster for CPU%d\n", cpu); + return ret; + } if (cl >= data->num_clusters) return -EINVAL; - /* boot freq */ - policy->cur = tegra194_get_speed_common(policy->cpu, US_DELAY_MIN); - /* set same policy for all cpus in a cluster */ for (cpu = (cl * 2); cpu < ((cl + 1) * 2); cpu++) cpumask_set_cpu(cpu, policy->cpus); @@ -267,7 +272,23 @@ static int tegra194_cpufreq_init(struct cpufreq_policy *policy) policy->freq_table = data->tables[cl]; policy->cpuinfo.transition_latency = TEGRA_CPUFREQ_TRANSITION_LATENCY; - return 0; + policy->cur = tegra194_get_speed_common(policy->cpu, US_DELAY); + + ret = cpufreq_table_validate_and_sort(policy); + if (ret) + return ret; + + /* Are we running at unknown frequency ? */ + ret = cpufreq_frequency_table_get_index(policy, policy->cur); + if (ret == -EINVAL) { + ret = __cpufreq_driver_target(policy, policy->cur - 1, + CPUFREQ_RELATION_L); + if (ret) + return ret; + policy->cur = tegra194_get_speed_common(policy->cpu, US_DELAY); + } + + return ret; } static int tegra194_cpufreq_set_target(struct cpufreq_policy *policy, @@ -285,6 +306,55 @@ static int tegra194_cpufreq_set_target(struct cpufreq_policy *policy, return 0; } +static int tegra194_cpufreq_online(struct cpufreq_policy *policy) +{ + unsigned int interm_freq, last_set_freq; + struct cpufreq_frequency_table *pos; + u64 ndiv; + int ret; + + if (!cpu_online(policy->cpu)) + return -EINVAL; + + /* get ndiv for the last frequency request from software */ + ret = smp_call_function_single(policy->cpu, get_cpu_ndiv, , true); + if (ret) { + pr_err("cpufreq: Failed to get ndiv for CPU%d\n", policy->cpu); + return ret; + } + + cpufreq_for_each_valid_entry(pos, policy->freq_table) { + if (pos->driver_data == ndiv) { + last_set_freq = pos->frequency; + break; + } + } + + policy->cur = tegra194_get_speed_common(policy->cpu, US_DELAY); + interm_freq = policy->cur; + + /* +* It takes some time to restore the previous frequency while +* turning-on non-boot cores during exit from SC7(Suspend-to-RAM). +* So, wait till it reaches the previous value and timeout if the +* time taken to reach requested freq is >100ms +*/ + ret = read_poll_timeout(tegra194_get_speed_common, policy->cur, + abs(
[PATCH v2 1/2] cpufreq: tegra194: get consistent cpuinfo_cur_freq
Frequency returned by 'cpuinfo_cur_freq' using counters is not fixed and keeps changing slightly. This change returns a consistent value from freq_table. If the reconstructed frequency has acceptable delta from the last written value, then return the frequency corresponding to the last written ndiv value from freq_table. Otherwise, print a warning and return the reconstructed freq. Signed-off-by: Sumit Gupta --- drivers/cpufreq/tegra194-cpufreq.c | 71 +- 1 file changed, 62 insertions(+), 9 deletions(-) diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c index e1d931c..d250e49 100644 --- a/drivers/cpufreq/tegra194-cpufreq.c +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -180,9 +180,70 @@ static unsigned int tegra194_get_speed_common(u32 cpu, u32 delay) return (rate_mhz * KHZ); /* in KHz */ } +static void get_cpu_ndiv(void *ndiv) +{ + u64 ndiv_val; + + asm volatile("mrs %0, s3_0_c15_c0_4" : "=r" (ndiv_val) : ); + + *(u64 *)ndiv = ndiv_val; +} + +static void set_cpu_ndiv(void *data) +{ + struct cpufreq_frequency_table *tbl = data; + u64 ndiv_val = (u64)tbl->driver_data; + + asm volatile("msr s3_0_c15_c0_4, %0" : : "r" (ndiv_val)); +} + static unsigned int tegra194_get_speed(u32 cpu) { - return tegra194_get_speed_common(cpu, US_DELAY); + struct tegra194_cpufreq_data *data = cpufreq_get_driver_data(); + struct cpufreq_frequency_table *pos; + unsigned int rate; + u64 ndiv; + int ret; + u32 cl; + + if (!cpu_online(cpu)) + return -EINVAL; + + smp_call_function_single(cpu, get_cpu_cluster, , true); + + if (cl >= data->num_clusters) + return -EINVAL; + + /* reconstruct actual cpu freq using counters */ + rate = tegra194_get_speed_common(cpu, US_DELAY); + + /* get last written ndiv value */ + ret = smp_call_function_single(cpu, get_cpu_ndiv, , true); + if (ret) { + pr_err("cpufreq: Failed to get ndiv for CPU%d, ret:%d\n", + cpu, ret); + return rate; + } + + /* +* If the reconstructed frequency has acceptable delta from +* the last written value, then return freq corresponding +* to the last written ndiv value from freq_table. This is +* done to return consistent value. +*/ + cpufreq_for_each_valid_entry(pos, data->tables[cl]) { + if (pos->driver_data != ndiv) + continue; + + if (abs(pos->frequency - rate) > 115200) { + pr_warn("cpufreq: cpu%d,cur:%u,set:%u,set ndiv:%llu\n", + cpu, rate, pos->frequency, ndiv); + } else { + rate = pos->frequency; + } + break; + } + return rate; } static int tegra194_cpufreq_init(struct cpufreq_policy *policy) @@ -209,14 +270,6 @@ static int tegra194_cpufreq_init(struct cpufreq_policy *policy) return 0; } -static void set_cpu_ndiv(void *data) -{ - struct cpufreq_frequency_table *tbl = data; - u64 ndiv_val = (u64)tbl->driver_data; - - asm volatile("msr s3_0_c15_c0_4, %0" : : "r" (ndiv_val)); -} - static int tegra194_cpufreq_set_target(struct cpufreq_policy *policy, unsigned int index) { -- 2.7.4
[PATCH v2 0/2] Tegra194 cpufreq driver misc changes
This patch set has below two changes: 1) get consistent cpuinfo_cur_freq value from freq_table. 2) Fix unlisted boot freq warning by setting closest higher freq from freq_table if the boot frequency gets filtered while creating freq_table in kernel. v1[1] -> v2: - Minor changes to improve comments and reduce debug prints. - Get freq table from cluster specific data instead of policy. - Set a freq from freq_table if boot freq is not present in table. - Add online hook to fix unlisted boot freq warning in hotplug-on. Sumit Gupta (2): cpufreq: tegra194: get consistent cpuinfo_cur_freq cpufreq: tegra194: Fix unlisted boot freq warning drivers/cpufreq/tegra194-cpufreq.c | 153 + 1 file changed, 139 insertions(+), 14 deletions(-) [1] https://marc.info/?l=linux-arm-kernel=160028821117535=2 -- 2.7.4
Re: [Patch 2/2] cpufreq: tegra194: Fix unlisted boot freq warning
Warning coming during boot because the boot freq set by bootloader gets filtered out due to big freq steps while creating freq_table. Fixing this by setting closest ndiv value from freq_table. Warning: cpufreq: cpufreq_online: CPU0: Running at unlisted freq cpufreq: cpufreq_online: CPU0: Unlisted initial frequency changed Also, added change in init to wait till current frequency becomes equal or near to the previously requested frequency. This is done because it takes some time to restore the previous frequency while turning-on non-boot cores during exit from SC7(Suspend-to-RAM). So you are trying to figure if the frequency is listed in freq-table or not, otherwise setting the frequency to a value you think is appropriate. Right ? During boot, want to set the frequency from freq_table which is closest to the one set by bootloader. Right. During resume from suspend-to-idle, want to restore the frequency which was set on non-boot cores before they were hotplug powered off. Why exactly do you want to do that ? Rather you should provide online/offline hooks for the cpufreq driver and do light-weight suspend/resume as is done by cpufreq-dt.c as well. Thank you for pointer. Added online hook to avoid warning during hot-plug-on for the non-boot CPU's while exiting from Suspend-to-RAM. Will send new version with the changes. This is what the cpufreq core already does when it printed these boot time messages. Do we really need to add this much code in here ? We want to avoid the warning messages. Hmm, okay. If you really don't want to see the warning, how about fixing it the way cpufreq core does ? i.e. with this call: ret = __cpufreq_driver_target(policy, policy->cur - 1, CPUFREQ_RELATION_L); The cpufreq core change will help in bootup case but not during the case of resume. In this change, reading the previously stored value and restoring it will also fix the warning message during resume. You were getting the message during resume as well ? Why ? The firmware is updating the frequency to a previous value ? If that is so, you should just set the frequency again to some other value during resume to fix it. Yes, it boots at a predefined frequency and then some time is taken to restore the last frequency which software requested before entering Suspend-to-RAM. We don't need to re-write the register again. -- viresh
Re: [Patch 2/2] cpufreq: tegra194: Fix unlisted boot freq warning
Warning coming during boot because the boot freq set by bootloader gets filtered out due to big freq steps while creating freq_table. Fixing this by setting closest ndiv value from freq_table. Warning: cpufreq: cpufreq_online: CPU0: Running at unlisted freq cpufreq: cpufreq_online: CPU0: Unlisted initial frequency changed Also, added change in init to wait till current frequency becomes equal or near to the previously requested frequency. This is done because it takes some time to restore the previous frequency while turning-on non-boot cores during exit from SC7(Suspend-to-RAM). So you are trying to figure if the frequency is listed in freq-table or not, otherwise setting the frequency to a value you think is appropriate. Right ? During boot, want to set the frequency from freq_table which is closest to the one set by bootloader. During resume from suspend-to-idle, want to restore the frequency which was set on non-boot cores before they were hotplug powered off. This is what the cpufreq core already does when it printed these boot time messages. Do we really need to add this much code in here ? We want to avoid the warning messages. If you really don't want to see the warning, how about fixing it the way cpufreq core does ? i.e. with this call: ret = __cpufreq_driver_target(policy, policy->cur - 1, CPUFREQ_RELATION_L); The cpufreq core change will help in bootup case but not during the case of resume. In this change, reading the previously stored value and restoring it will also fix the warning message during resume. -- viresh
Re: [Patch 1/2] cpufreq: tegra194: get consistent cpuinfo_cur_freq
Frequency returned by 'cpuinfo_cur_freq' using counters is not fixed and keeps changing slightly. This change returns a consistent value from freq_table. If the reconstructed frequency has acceptable delta from the last written value, then return the frequency corresponding to the last written ndiv value from freq_table. Otherwise, print a warning and return the reconstructed freq. Signed-off-by: Sumit Gupta --- drivers/cpufreq/tegra194-cpufreq.c | 66 -- 1 file changed, 57 insertions(+), 9 deletions(-) diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c index e1d931c..d5b608d 100644 --- a/drivers/cpufreq/tegra194-cpufreq.c +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -180,9 +180,65 @@ static unsigned int tegra194_get_speed_common(u32 cpu, u32 delay) return (rate_mhz * KHZ); /* in KHz */ } +static void get_cpu_ndiv(void *ndiv) +{ + u64 ndiv_val; + + asm volatile("mrs %0, s3_0_c15_c0_4" : "=r" (ndiv_val) : ); + + *(u64 *)ndiv = ndiv_val; +} + +static void set_cpu_ndiv(void *data) +{ + struct cpufreq_frequency_table *tbl = data; + u64 ndiv_val = (u64)tbl->driver_data; + + asm volatile("msr s3_0_c15_c0_4, %0" : : "r" (ndiv_val)); +} + static unsigned int tegra194_get_speed(u32 cpu) { - return tegra194_get_speed_common(cpu, US_DELAY); + struct cpufreq_frequency_table *table, *pos; + struct cpufreq_policy policy; + unsigned int rate; + u64 ndiv; + int err; + + cpufreq_get_policy(, cpu); + table = policy.freq_table; Maybe getting freq-table from cluster specific data would be better/faster. will do the change in next patch version. + + /* reconstruct actual cpu freq using counters*/ + rate = tegra194_get_speed_common(cpu, US_DELAY); + + /* get last written ndiv value*/ + err = smp_call_function_single(cpu, get_cpu_ndiv, , true); + if (err) { + pr_err("cpufreq: Failed to get ndiv for CPU%d, ret:%d\n", +cpu, err); + return rate; + } + + /* if the reconstructed frequency has acceptable delta from Both have got the multi-line comments wrong. Format is wrong and every sentence needs to start with a capital letter. will correct. + * the last written value, then return freq corresponding + * to the last written ndiv value from freq_table. This is + * done to return consistent value. + */ + cpufreq_for_each_valid_entry(pos, table) { + if (pos->driver_data != ndiv) + continue; + + if (abs(pos->frequency - rate) > 115200) { + pr_warn("cpufreq: high delta (%d) on CPU%d\n", + abs(pos->frequency - rate), cpu); + pr_warn("cpufreq: cur:%u, set:%u, set ndiv:%llu\n", + rate, pos->frequency, ndiv); Both of these can be merged in a single line I think. There is no need to print delta as you already print both the frequencies. Will do this also in next patch version. + } else { + rate = pos->frequency; + } + break; + } + return rate; } static int tegra194_cpufreq_init(struct cpufreq_policy *policy) @@ -209,14 +265,6 @@ static int tegra194_cpufreq_init(struct cpufreq_policy *policy) return 0; } -static void set_cpu_ndiv(void *data) -{ - struct cpufreq_frequency_table *tbl = data; - u64 ndiv_val = (u64)tbl->driver_data; - - asm volatile("msr s3_0_c15_c0_4, %0" : : "r" (ndiv_val)); -} - static int tegra194_cpufreq_set_target(struct cpufreq_policy *policy, unsigned int index) { -- 2.7.4 -- viresh
[Patch 0/2] Tegra194 cpufreq driver misc changes
This patch set has below two changes: 1) get consistent cpuinfo_cur_freq value from freq_table. 2) Fix unlisted boot freq warning by setting closest ndiv value from freq_table if the boot frequency gets filtered while creating freq_table in kernel. Sumit Gupta (2): cpufreq: tegra194: get consistent cpuinfo_cur_freq cpufreq: tegra194: Fix unlisted boot freq warning drivers/cpufreq/tegra194-cpufreq.c | 182 ++--- 1 file changed, 167 insertions(+), 15 deletions(-) -- 2.7.4
[Patch 2/2] cpufreq: tegra194: Fix unlisted boot freq warning
Warning coming during boot because the boot freq set by bootloader gets filtered out due to big freq steps while creating freq_table. Fixing this by setting closest ndiv value from freq_table. Warning: cpufreq: cpufreq_online: CPU0: Running at unlisted freq cpufreq: cpufreq_online: CPU0: Unlisted initial frequency changed Also, added change in init to wait till current frequency becomes equal or near to the previously requested frequency. This is done because it takes some time to restore the previous frequency while turning-on non-boot cores during exit from SC7(Suspend-to-RAM). Signed-off-by: Sumit Gupta --- drivers/cpufreq/tegra194-cpufreq.c | 118 ++--- 1 file changed, 111 insertions(+), 7 deletions(-) diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c index d5b608d..c3c058a3 100644 --- a/drivers/cpufreq/tegra194-cpufreq.c +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -21,7 +22,6 @@ #define KHZ 1000 #define REF_CLK_MHZ 408 /* 408 MHz */ #define US_DELAY500 -#define US_DELAY_MIN2 #define CPUFREQ_TBL_STEP_HZ (50 * KHZ * KHZ) #define MAX_CNT ~0U @@ -241,26 +241,130 @@ static unsigned int tegra194_get_speed(u32 cpu) return rate; } +static int +freqtable_find_index_closest_ndiv(struct cpufreq_frequency_table *table, + unsigned int target_ndiv) +{ + struct cpufreq_frequency_table *pos; + unsigned int ndiv; + int idx, best = -1; + + cpufreq_for_each_valid_entry_idx(pos, table, idx) { + ndiv = pos->driver_data; + + if (ndiv == target_ndiv) + return idx; + + if (ndiv < target_ndiv) { + best = idx; + continue; + } + + /* No ndiv found below target_ndiv */ + if (best == -1) + return idx; + + /* Choose the closest ndiv */ + if (target_ndiv - table[best].driver_data > ndiv - target_ndiv) + return idx; + + return best; + } + + return best; +} + +static int +freqtable_set_closest_ndiv(struct cpufreq_frequency_table *freq_table, + int cpu) +{ + u64 ndiv; + int idx, ret; + + if (!cpu_online(cpu)) + return -EINVAL; + + /* get ndiv for the last frequency request from software */ + ret = smp_call_function_single(cpu, get_cpu_ndiv, , true); + if (ret) { + pr_err("cpufreq: Failed to get ndiv for CPU%d\n", cpu); + return ret; + } + + /* while creating freq_table during boot, if the ndiv value got +* filtered out due to large freq steps, then find closest ndiv +* from freq_table and set that. +*/ + idx = freqtable_find_index_closest_ndiv(freq_table, ndiv); + + if (ndiv != freq_table[idx].driver_data) { + pr_debug("cpufreq: new freq:%d ndiv:%d, old ndiv:%llu\n", +freq_table[idx].frequency, +freq_table[idx].driver_data, ndiv); + + ret = smp_call_function_single(cpu, set_cpu_ndiv, + freq_table + idx, true); + if (ret) { + pr_err("cpufreq: setting ndiv for CPU%d failed\n", + cpu); + return ret; + } + } + + return freq_table[idx].frequency; +} + static int tegra194_cpufreq_init(struct cpufreq_policy *policy) { struct tegra194_cpufreq_data *data = cpufreq_get_driver_data(); - u32 cpu; + u32 cpu = policy->cpu; + int new_freq, ret; u32 cl; - smp_call_function_single(policy->cpu, get_cpu_cluster, , true); + if (!cpu_online(cpu)) + return -EINVAL; + + ret = smp_call_function_single(cpu, get_cpu_cluster, , true); + if (ret) { + pr_err("cpufreq: Failed to get cluster for CPU%d\n", cpu); + return ret; + } if (cl >= data->num_clusters) return -EINVAL; - /* boot freq */ - policy->cur = tegra194_get_speed_common(policy->cpu, US_DELAY_MIN); - /* set same policy for all cpus in a cluster */ for (cpu = (cl * 2); cpu < ((cl + 1) * 2); cpu++) cpumask_set_cpu(cpu, policy->cpus); policy->freq_table = data->tables[cl]; - policy->cpuinfo.transition_latency = TEGRA_CPUFREQ_TRANSITION_LATENCY; + policy->cpuinfo.transition_latency = + TEGRA_CPUFREQ_TRANSITION_LATENCY; + + /* Find and set the closest ndiv fr
[Patch 1/2] cpufreq: tegra194: get consistent cpuinfo_cur_freq
Frequency returned by 'cpuinfo_cur_freq' using counters is not fixed and keeps changing slightly. This change returns a consistent value from freq_table. If the reconstructed frequency has acceptable delta from the last written value, then return the frequency corresponding to the last written ndiv value from freq_table. Otherwise, print a warning and return the reconstructed freq. Signed-off-by: Sumit Gupta --- drivers/cpufreq/tegra194-cpufreq.c | 66 -- 1 file changed, 57 insertions(+), 9 deletions(-) diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c index e1d931c..d5b608d 100644 --- a/drivers/cpufreq/tegra194-cpufreq.c +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -180,9 +180,65 @@ static unsigned int tegra194_get_speed_common(u32 cpu, u32 delay) return (rate_mhz * KHZ); /* in KHz */ } +static void get_cpu_ndiv(void *ndiv) +{ + u64 ndiv_val; + + asm volatile("mrs %0, s3_0_c15_c0_4" : "=r" (ndiv_val) : ); + + *(u64 *)ndiv = ndiv_val; +} + +static void set_cpu_ndiv(void *data) +{ + struct cpufreq_frequency_table *tbl = data; + u64 ndiv_val = (u64)tbl->driver_data; + + asm volatile("msr s3_0_c15_c0_4, %0" : : "r" (ndiv_val)); +} + static unsigned int tegra194_get_speed(u32 cpu) { - return tegra194_get_speed_common(cpu, US_DELAY); + struct cpufreq_frequency_table *table, *pos; + struct cpufreq_policy policy; + unsigned int rate; + u64 ndiv; + int err; + + cpufreq_get_policy(, cpu); + table = policy.freq_table; + + /* reconstruct actual cpu freq using counters*/ + rate = tegra194_get_speed_common(cpu, US_DELAY); + + /* get last written ndiv value*/ + err = smp_call_function_single(cpu, get_cpu_ndiv, , true); + if (err) { + pr_err("cpufreq: Failed to get ndiv for CPU%d, ret:%d\n", + cpu, err); + return rate; + } + + /* if the reconstructed frequency has acceptable delta from +* the last written value, then return freq corresponding +* to the last written ndiv value from freq_table. This is +* done to return consistent value. +*/ + cpufreq_for_each_valid_entry(pos, table) { + if (pos->driver_data != ndiv) + continue; + + if (abs(pos->frequency - rate) > 115200) { + pr_warn("cpufreq: high delta (%d) on CPU%d\n", + abs(pos->frequency - rate), cpu); + pr_warn("cpufreq: cur:%u, set:%u, set ndiv:%llu\n", + rate, pos->frequency, ndiv); + } else { + rate = pos->frequency; + } + break; + } + return rate; } static int tegra194_cpufreq_init(struct cpufreq_policy *policy) @@ -209,14 +265,6 @@ static int tegra194_cpufreq_init(struct cpufreq_policy *policy) return 0; } -static void set_cpu_ndiv(void *data) -{ - struct cpufreq_frequency_table *tbl = data; - u64 ndiv_val = (u64)tbl->driver_data; - - asm volatile("msr s3_0_c15_c0_4, %0" : : "r" (ndiv_val)); -} - static int tegra194_cpufreq_set_target(struct cpufreq_policy *policy, unsigned int index) { -- 2.7.4
Re: [PATCH -next] arm64: Export __cpu_logical_map
ERROR: modpost: "__cpu_logical_map" [drivers/cpufreq/tegra194-cpufreq.ko] undefined! ARM64 tegra194-cpufreq driver use cpu_logical_map, export __cpu_logical_map to fix build issue. I wonder why like other instances in the drivers, the mpidr is not get directly from the cpu. The cpufreq_driver->init call happens when the cpu is being brought online and is executed on the required cpu IIUC. Yes, this occurs during hotplug case. But in the case of system boot, 'cpufreq_driver->init' is called later during cpufreq platform driver's probe. The value of CPU in 'policy->cpu' can be different from the current CPU. That's why read_cpuid_mpidr() can't be used. Fair enough, why not do cross call like in set_target ? Since it is one-off in init, I don't see any issue when you are doing it runtime for set_target. read_cpuid_mpidr() is inline and avoids having to export the logical_cpu_map. Though we may not add physical hotplug anytime soon, less dependency on this cpu_logical_map is better given that we can resolve this without the need to access the map. To be honest, we have tried to remove all the dependency on cluster id in generic code as it is not well defined. This one is tegra specific driver so should be fine. But I am still bit nervous to export cpu_logical_map as we have no clue what that would mean for physical hotplug. As suggested, I have done below change to get the cluster number using read_cpuid_mpidr(). Please review and suggest if this looks ok? I will send formal patch if the change is fine. diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c index bae527e..06f5ccf 100644 --- a/drivers/cpufreq/tegra194-cpufreq.c +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -56,9 +56,11 @@ struct read_counters_work { static struct workqueue_struct *read_counters_wq; -static enum cluster get_cpu_cluster(u8 cpu) +static void get_cpu_cluster(void *cluster) { - return MPIDR_AFFINITY_LEVEL(cpu_logical_map(cpu), 1); + u64 mpidr = read_cpuid_mpidr() & MPIDR_HWID_BITMASK; + + *((uint32_t *) cluster) = MPIDR_AFFINITY_LEVEL(mpidr, 1); } /* @@ -186,8 +188,10 @@ static unsigned int tegra194_get_speed(u32 cpu) static int tegra194_cpufreq_init(struct cpufreq_policy *policy) { struct tegra194_cpufreq_data *data = cpufreq_get_driver_data(); - int cl = get_cpu_cluster(policy->cpu); u32 cpu; + u32 cl; + + smp_call_function_single(policy->cpu, get_cpu_cluster, , true); Thanks for this, looks good to me. You can add: Reviewed-by: Sudeep Holla I already merged Kefeng's __cpu_logical_map fix (commit eaecca9e7710) but if the above goes in, I can drop the EXPORT_SYMBOL part (and keep the rest as it's a good refactoring). I have posted the formal patch. Thanks, Sumit -- Catalin
[Patch] cpufreq: replace cpu_logical_map with read_cpuid_mpir
Commit eaecca9e7710 ("arm64: Fix __cpu_logical_map undefined issue") fixes the issue with building tegra194 cpufreq driver as module. But the fix might cause problem while supporting physical cpu hotplug[1]. This patch fixes the original problem by avoiding use of cpu_logical_map(). Instead calling read_cpuid_mpidr() to get MPIDR on target cpu. [1] https://lore.kernel.org/linux-arm-kernel/20200724131059.GB6521@bogus/ Reviewed-by: Sudeep Holla Signed-off-by: Sumit Gupta --- drivers/cpufreq/tegra194-cpufreq.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c index bae527e..e1d931c 100644 --- a/drivers/cpufreq/tegra194-cpufreq.c +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -56,9 +56,11 @@ struct read_counters_work { static struct workqueue_struct *read_counters_wq; -static enum cluster get_cpu_cluster(u8 cpu) +static void get_cpu_cluster(void *cluster) { - return MPIDR_AFFINITY_LEVEL(cpu_logical_map(cpu), 1); + u64 mpidr = read_cpuid_mpidr() & MPIDR_HWID_BITMASK; + + *((uint32_t *)cluster) = MPIDR_AFFINITY_LEVEL(mpidr, 1); } /* @@ -186,8 +188,10 @@ static unsigned int tegra194_get_speed(u32 cpu) static int tegra194_cpufreq_init(struct cpufreq_policy *policy) { struct tegra194_cpufreq_data *data = cpufreq_get_driver_data(); - int cl = get_cpu_cluster(policy->cpu); u32 cpu; + u32 cl; + + smp_call_function_single(policy->cpu, get_cpu_cluster, , true); if (cl >= data->num_clusters) return -EINVAL; -- 2.7.4
Re: [PATCH -next] arm64: Export __cpu_logical_map
ERROR: modpost: "__cpu_logical_map" [drivers/cpufreq/tegra194-cpufreq.ko] undefined! ARM64 tegra194-cpufreq driver use cpu_logical_map, export __cpu_logical_map to fix build issue. I wonder why like other instances in the drivers, the mpidr is not get directly from the cpu. The cpufreq_driver->init call happens when the cpu is being brought online and is executed on the required cpu IIUC. Yes, this occurs during hotplug case. But in the case of system boot, 'cpufreq_driver->init' is called later during cpufreq platform driver's probe. The value of CPU in 'policy->cpu' can be different from the current CPU. That's why read_cpuid_mpidr() can't be used. Fair enough, why not do cross call like in set_target ? Since it is one-off in init, I don't see any issue when you are doing it runtime for set_target. read_cpuid_mpidr() is inline and avoids having to export the logical_cpu_map. Though we may not add physical hotplug anytime soon, less dependency on this cpu_logical_map is better given that we can resolve this without the need to access the map. To be honest, we have tried to remove all the dependency on cluster id in generic code as it is not well defined. This one is tegra specific driver so should be fine. But I am still bit nervous to export cpu_logical_map as we have no clue what that would mean for physical hotplug. As suggested, I have done below change to get the cluster number using read_cpuid_mpidr(). Please review and suggest if this looks ok? I will send formal patch if the change is fine. Thanks, Sumit diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c index bae527e..06f5ccf 100644 --- a/drivers/cpufreq/tegra194-cpufreq.c +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -56,9 +56,11 @@ struct read_counters_work { static struct workqueue_struct *read_counters_wq; -static enum cluster get_cpu_cluster(u8 cpu) +static void get_cpu_cluster(void *cluster) { - return MPIDR_AFFINITY_LEVEL(cpu_logical_map(cpu), 1); + u64 mpidr = read_cpuid_mpidr() & MPIDR_HWID_BITMASK; + + *((uint32_t *) cluster) = MPIDR_AFFINITY_LEVEL(mpidr, 1); } /* @@ -186,8 +188,10 @@ static unsigned int tegra194_get_speed(u32 cpu) static int tegra194_cpufreq_init(struct cpufreq_policy *policy) { struct tegra194_cpufreq_data *data = cpufreq_get_driver_data(); - int cl = get_cpu_cluster(policy->cpu); u32 cpu; + u32 cl; + + smp_call_function_single(policy->cpu, get_cpu_cluster, , true); -- Regards, Sudeep
Re: [PATCH -next] arm64: Export __cpu_logical_map
ERROR: modpost: "__cpu_logical_map" [drivers/cpufreq/tegra194-cpufreq.ko] undefined! ARM64 tegra194-cpufreq driver use cpu_logical_map, export __cpu_logical_map to fix build issue. I wonder why like other instances in the drivers, the mpidr is not get directly from the cpu. The cpufreq_driver->init call happens when the cpu is being brought online and is executed on the required cpu IIUC. Yes, this occurs during hotplug case. But in the case of system boot, 'cpufreq_driver->init' is called later during cpufreq platform driver's probe. The value of CPU in 'policy->cpu' can be different from the current CPU. That's why read_cpuid_mpidr() can't be used. read_cpuid_mpidr() is inline and avoids having to export the logical_cpu_map. Though we may not add physical hotplug anytime soon, less dependency on this cpu_logical_map is better given that we can resolve this without the need to access the map. -- Regards, Sudeep
[TEGRA194_CPUFREQ PATCH v6.1 3/3] cpufreq: Add Tegra194 cpufreq driver
Add support for CPU frequency scaling on Tegra194. The frequency of each core can be adjusted by writing a clock divisor value to a MSR on the core. The range of valid divisors is queried from the BPMP. Signed-off-by: Mikko Perttunen Signed-off-by: Sumit Gupta --- drivers/cpufreq/Kconfig.arm| 7 + drivers/cpufreq/Makefile | 1 + drivers/cpufreq/tegra194-cpufreq.c | 390 + 3 files changed, 398 insertions(+) create mode 100644 drivers/cpufreq/tegra194-cpufreq.c diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 15c1a12..7e99a46 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -314,6 +314,13 @@ config ARM_TEGRA186_CPUFREQ help This adds the CPUFreq driver support for Tegra186 SOCs. +config ARM_TEGRA194_CPUFREQ + tristate "Tegra194 CPUFreq support" + depends on ARCH_TEGRA_194_SOC && TEGRA_BPMP + default y + help + This adds CPU frequency driver support for Tegra194 SOCs. + config ARM_TI_CPUFREQ bool "Texas Instruments CPUFreq support" depends on ARCH_OMAP2PLUS diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index f6670c4..66b5563 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -83,6 +83,7 @@ obj-$(CONFIG_ARM_TANGO_CPUFREQ) += tango-cpufreq.o obj-$(CONFIG_ARM_TEGRA20_CPUFREQ) += tegra20-cpufreq.o obj-$(CONFIG_ARM_TEGRA124_CPUFREQ) += tegra124-cpufreq.o obj-$(CONFIG_ARM_TEGRA186_CPUFREQ) += tegra186-cpufreq.o +obj-$(CONFIG_ARM_TEGRA194_CPUFREQ) += tegra194-cpufreq.o obj-$(CONFIG_ARM_TI_CPUFREQ) += ti-cpufreq.o obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c new file mode 100644 index 000..bae527e --- /dev/null +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -0,0 +1,390 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2020, NVIDIA CORPORATION. All rights reserved + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include +#include + +#define KHZ 1000 +#define REF_CLK_MHZ 408 /* 408 MHz */ +#define US_DELAY500 +#define US_DELAY_MIN2 +#define CPUFREQ_TBL_STEP_HZ (50 * KHZ * KHZ) +#define MAX_CNT ~0U + +/* cpufreq transisition latency */ +#define TEGRA_CPUFREQ_TRANSITION_LATENCY (300 * 1000) /* unit in nanoseconds */ + +enum cluster { + CLUSTER0, + CLUSTER1, + CLUSTER2, + CLUSTER3, + MAX_CLUSTERS, +}; + +struct tegra194_cpufreq_data { + void __iomem *regs; + size_t num_clusters; + struct cpufreq_frequency_table **tables; +}; + +struct tegra_cpu_ctr { + u32 cpu; + u32 delay; + u32 coreclk_cnt, last_coreclk_cnt; + u32 refclk_cnt, last_refclk_cnt; +}; + +struct read_counters_work { + struct work_struct work; + struct tegra_cpu_ctr c; +}; + +static struct workqueue_struct *read_counters_wq; + +static enum cluster get_cpu_cluster(u8 cpu) +{ + return MPIDR_AFFINITY_LEVEL(cpu_logical_map(cpu), 1); +} + +/* + * Read per-core Read-only system register NVFREQ_FEEDBACK_EL1. + * The register provides frequency feedback information to + * determine the average actual frequency a core has run at over + * a period of time. + * [31:0] PLLP counter: Counts at fixed frequency (408 MHz) + * [63:32] Core clock counter: counts on every core clock cycle + * where the core is architecturally clocking + */ +static u64 read_freq_feedback(void) +{ + u64 val = 0; + + asm volatile("mrs %0, s3_0_c15_c0_5" : "=r" (val) : ); + + return val; +} + +static inline u32 map_ndiv_to_freq(struct mrq_cpu_ndiv_limits_response + *nltbl, u16 ndiv) +{ + return nltbl->ref_clk_hz / KHZ * ndiv / (nltbl->pdiv * nltbl->mdiv); +} + +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, +
Re: [TEGRA194_CPUFREQ PATCH v6 3/3] cpufreq: Add Tegra194 cpufreq driver
Add support for CPU frequency scaling on Tegra194. The frequency of each core can be adjusted by writing a clock divisor value to a MSR on the core. The range of valid divisors is queried from the BPMP. Signed-off-by: Mikko Perttunen Signed-off-by: Sumit Gupta --- drivers/cpufreq/Kconfig.arm| 7 + drivers/cpufreq/Makefile | 1 + drivers/cpufreq/tegra194-cpufreq.c | 397 + 3 files changed, 405 insertions(+) create mode 100644 drivers/cpufreq/tegra194-cpufreq.c diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 15c1a12..7e99a46 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -314,6 +314,13 @@ config ARM_TEGRA186_CPUFREQ help This adds the CPUFreq driver support for Tegra186 SOCs. +config ARM_TEGRA194_CPUFREQ + tristate "Tegra194 CPUFreq support" + depends on ARCH_TEGRA_194_SOC && TEGRA_BPMP + default y + help + This adds CPU frequency driver support for Tegra194 SOCs. + config ARM_TI_CPUFREQ bool "Texas Instruments CPUFreq support" depends on ARCH_OMAP2PLUS diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index f6670c4..66b5563 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -83,6 +83,7 @@ obj-$(CONFIG_ARM_TANGO_CPUFREQ) += tango-cpufreq.o obj-$(CONFIG_ARM_TEGRA20_CPUFREQ) += tegra20-cpufreq.o obj-$(CONFIG_ARM_TEGRA124_CPUFREQ)+= tegra124-cpufreq.o obj-$(CONFIG_ARM_TEGRA186_CPUFREQ)+= tegra186-cpufreq.o +obj-$(CONFIG_ARM_TEGRA194_CPUFREQ) += tegra194-cpufreq.o obj-$(CONFIG_ARM_TI_CPUFREQ) += ti-cpufreq.o obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ)+= vexpress-spc-cpufreq.o diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c new file mode 100644 index 000..b52a5e2 --- /dev/null +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -0,0 +1,397 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2020, NVIDIA CORPORATION. All rights reserved + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include +#include + +#define KHZ 1000 +#define REF_CLK_MHZ 408 /* 408 MHz */ +#define US_DELAY500 +#define US_DELAY_MIN2 +#define CPUFREQ_TBL_STEP_HZ (50 * KHZ * KHZ) +#define MAX_CNT ~0U + +/* cpufreq transisition latency */ +#define TEGRA_CPUFREQ_TRANSITION_LATENCY (300 * 1000) /* unit in nanoseconds */ + +enum cluster { + CLUSTER0, + CLUSTER1, + CLUSTER2, + CLUSTER3, + MAX_CLUSTERS, +}; + +struct tegra194_cpufreq_data { + void __iomem *regs; + size_t num_clusters; + struct cpufreq_frequency_table **tables; +}; + +struct tegra_cpu_ctr { + u32 cpu; + u32 delay; + u32 coreclk_cnt, last_coreclk_cnt; + u32 refclk_cnt, last_refclk_cnt; +}; + +struct read_counters_work { + struct work_struct work; + struct tegra_cpu_ctr c; +}; + +static struct workqueue_struct *read_counters_wq; + +static enum cluster get_cpu_cluster(u8 cpu) +{ + return MPIDR_AFFINITY_LEVEL(cpu_logical_map(cpu), 1); +} + +/* + * Read per-core Read-only system register NVFREQ_FEEDBACK_EL1. + * The register provides frequency feedback information to + * determine the average actual frequency a core has run at over + * a period of time. + * [31:0] PLLP counter: Counts at fixed frequency (408 MHz) + * [63:32] Core clock counter: counts on every core clock cycle + * where the core is architecturally clocking + */ +static u64 read_freq_feedback(void) +{ + u64 val = 0; + + asm volatile("mrs %0, s3_0_c15_c0_5" : "=r" (val) : ); + + return val; +} + +static inline u32 map_ndiv_to_freq(struct mrq_cpu_ndiv_limits_response + *nltbl, u16 ndiv) +{ + return nltbl->ref_clk_hz / KHZ * ndiv / (nltbl->pdiv * nltbl->mdiv); +} + +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, +
[TEGRA194_CPUFREQ PATCH v6 2/3] arm64: tegra: Add t194 ccplex compatible and bpmp property
On Tegra194, data on valid operating points for the CPUs needs to be queried from BPMP. In T194, there is no node representing CPU complex. So, add compatible string to the 'cpus' node instead of using dummy node to bind cpufreq driver. Also, add reference to the BPMP instance for the CPU complex. Signed-off-by: Sumit Gupta --- arch/arm64/boot/dts/nvidia/tegra194.dtsi | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi index 7c9511a..0abf287 100644 --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi @@ -1764,6 +1764,8 @@ }; cpus { + compatible = "nvidia,tegra194-ccplex"; + nvidia,bpmp = <>; #address-cells = <1>; #size-cells = <0>; -- 2.7.4
[TEGRA194_CPUFREQ PATCH v6 3/3] cpufreq: Add Tegra194 cpufreq driver
Add support for CPU frequency scaling on Tegra194. The frequency of each core can be adjusted by writing a clock divisor value to a MSR on the core. The range of valid divisors is queried from the BPMP. Signed-off-by: Mikko Perttunen Signed-off-by: Sumit Gupta --- drivers/cpufreq/Kconfig.arm| 7 + drivers/cpufreq/Makefile | 1 + drivers/cpufreq/tegra194-cpufreq.c | 397 + 3 files changed, 405 insertions(+) create mode 100644 drivers/cpufreq/tegra194-cpufreq.c diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 15c1a12..7e99a46 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -314,6 +314,13 @@ config ARM_TEGRA186_CPUFREQ help This adds the CPUFreq driver support for Tegra186 SOCs. +config ARM_TEGRA194_CPUFREQ + tristate "Tegra194 CPUFreq support" + depends on ARCH_TEGRA_194_SOC && TEGRA_BPMP + default y + help + This adds CPU frequency driver support for Tegra194 SOCs. + config ARM_TI_CPUFREQ bool "Texas Instruments CPUFreq support" depends on ARCH_OMAP2PLUS diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index f6670c4..66b5563 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -83,6 +83,7 @@ obj-$(CONFIG_ARM_TANGO_CPUFREQ) += tango-cpufreq.o obj-$(CONFIG_ARM_TEGRA20_CPUFREQ) += tegra20-cpufreq.o obj-$(CONFIG_ARM_TEGRA124_CPUFREQ) += tegra124-cpufreq.o obj-$(CONFIG_ARM_TEGRA186_CPUFREQ) += tegra186-cpufreq.o +obj-$(CONFIG_ARM_TEGRA194_CPUFREQ) += tegra194-cpufreq.o obj-$(CONFIG_ARM_TI_CPUFREQ) += ti-cpufreq.o obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c new file mode 100644 index 000..b52a5e2 --- /dev/null +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -0,0 +1,397 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2020, NVIDIA CORPORATION. All rights reserved + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include +#include + +#define KHZ 1000 +#define REF_CLK_MHZ 408 /* 408 MHz */ +#define US_DELAY500 +#define US_DELAY_MIN2 +#define CPUFREQ_TBL_STEP_HZ (50 * KHZ * KHZ) +#define MAX_CNT ~0U + +/* cpufreq transisition latency */ +#define TEGRA_CPUFREQ_TRANSITION_LATENCY (300 * 1000) /* unit in nanoseconds */ + +enum cluster { + CLUSTER0, + CLUSTER1, + CLUSTER2, + CLUSTER3, + MAX_CLUSTERS, +}; + +struct tegra194_cpufreq_data { + void __iomem *regs; + size_t num_clusters; + struct cpufreq_frequency_table **tables; +}; + +struct tegra_cpu_ctr { + u32 cpu; + u32 delay; + u32 coreclk_cnt, last_coreclk_cnt; + u32 refclk_cnt, last_refclk_cnt; +}; + +struct read_counters_work { + struct work_struct work; + struct tegra_cpu_ctr c; +}; + +static struct workqueue_struct *read_counters_wq; + +static enum cluster get_cpu_cluster(u8 cpu) +{ + return MPIDR_AFFINITY_LEVEL(cpu_logical_map(cpu), 1); +} + +/* + * Read per-core Read-only system register NVFREQ_FEEDBACK_EL1. + * The register provides frequency feedback information to + * determine the average actual frequency a core has run at over + * a period of time. + * [31:0] PLLP counter: Counts at fixed frequency (408 MHz) + * [63:32] Core clock counter: counts on every core clock cycle + * where the core is architecturally clocking + */ +static u64 read_freq_feedback(void) +{ + u64 val = 0; + + asm volatile("mrs %0, s3_0_c15_c0_5" : "=r" (val) : ); + + return val; +} + +static inline u32 map_ndiv_to_freq(struct mrq_cpu_ndiv_limits_response + *nltbl, u16 ndiv) +{ + return nltbl->ref_clk_hz / KHZ * ndiv / (nltbl->pdiv * nltbl->mdiv); +} + +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, +
[TEGRA194_CPUFREQ PATCH v6 0/3] Add cpufreq driver for Tegra194
Hi Viresh & Rob, Have made the changes as per feedback. Please review/ack and consider this patch set for merging in 5.9. Thank you, Sumit --- The patch series adds cpufreq driver for Tegra194 SOC. v5[5] -> v6 - Add new schema file for 'nvidia,tegra194-ccplex'[Rob]. - Minor changes suggested in cpufreq driver[Viresh]. v4[4] -> v5 - Don't call destroy_workqueue() if alloc_workqueue() fails[Viresh] - Move CONFIG_ARM_TEGRA194_CPUFREQ enabling to soc/tegra/Kconfig[Viresh] - Add dependency of 'nvidia,bpmp' on 'compatible' in yaml file[Michal] - Fix typo in description causing dt_binding_check bot failure[Rob] v3[3] -> v4 - Open code LOOP_FOR_EACH_CPU_OF_CLUSTER macro[Viresh] - Delete unused funciton map_freq_to_ndiv[Viresh, kernel test bot] - Remove flush_workqueue from free_resources[Viresh] v2[2] -> v3 - Set same policy for all cpus in a cluster[Viresh]. - Add compatible string for CPU Complex under cpus node[Thierry]. - Add reference to bpmp node under cpus node[Thierry]. - Bind cpufreq driver to CPU Complex compatible string[Thierry]. - Remove patch to get bpmp data as now using cpus node to get that[Thierry]. v1[1] -> v2: - Remove cpufreq_lock mutex from tegra194_cpufreq_set_target [Viresh]. - Remove CPUFREQ_ASYNC_NOTIFICATION flag [Viresh]. - Remove redundant _begin|end() call from tegra194_cpufreq_set_target. - Rename opp_table to freq_table [Viresh]. Sumit Gupta (3): dt-bindings: arm: Add NVIDIA Tegra194 CPU Complex binding arm64: tegra: Add t194 ccplex compatible and bpmp property cpufreq: Add Tegra194 cpufreq driver .../bindings/arm/nvidia,tegra194-ccplex.yaml | 69 arch/arm64/boot/dts/nvidia/tegra194.dtsi | 2 + drivers/cpufreq/Kconfig.arm| 7 + drivers/cpufreq/Makefile | 1 + drivers/cpufreq/tegra194-cpufreq.c | 397 + 5 files changed, 476 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml create mode 100644 drivers/cpufreq/tegra194-cpufreq.c [1] https://marc.info/?t=15753945231=1=2 [2] https://marc.info/?l=linux-tegra=158602857106213=2 [3] https://marc.info/?l=linux-pm=159283376010084=2 [4] https://marc.info/?l=linux-tegra=159318640622917=2 [5] https://marc.info/?l=linux-tegra=159465409805593=2 -- 2.7.4
[TEGRA194_CPUFREQ PATCH v6 1/3] dt-bindings: arm: Add NVIDIA Tegra194 CPU Complex binding
Add device-tree binding documentation to represent Tegra194 CPU Complex with compatible string under 'cpus' node. This can be used by drivers like cpufreq which don't have their node or CPU Complex node to bind to. Also, documenting 'nvidia,bpmp' property which points to BPMP device. Signed-off-by: Sumit Gupta --- .../bindings/arm/nvidia,tegra194-ccplex.yaml | 69 ++ 1 file changed, 69 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml diff --git a/Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml b/Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml new file mode 100644 index 000..1043e4b --- /dev/null +++ b/Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml @@ -0,0 +1,69 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/arm/nvidia,tegra194-ccplex.yaml#; +$schema: "http://devicetree.org/meta-schemas/core.yaml#; + +title: NVIDIA Tegra194 CPU Complex device tree bindings + +maintainers: + - Thierry Reding + - Jonathan Hunter + - Sumit Gupta + +description: |+ + Tegra194 SOC has homogeneous architecture where each cluster has two + symmetric cores. Compatible string in "cpus" node represents the CPU + Complex having all clusters. + +properties: + $nodename: +const: cpus + + compatible: +enum: + - nvidia,tegra194-ccplex + + nvidia,bpmp: +$ref: '/schemas/types.yaml#/definitions/phandle' +description: | + Specifies the bpmp node that needs to be queried to get + operating point data for all CPUs. + +examples: + - | +cpus { + compatible = "nvidia,tegra194-ccplex"; + nvidia,bpmp = <>; + #address-cells = <1>; + #size-cells = <0>; + + cpu0_0: cpu@0 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x0>; +enable-method = "psci"; + }; + + cpu0_1: cpu@1 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x001>; +enable-method = "psci"; + }; + + cpu1_0: cpu@100 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x100>; +enable-method = "psci"; + }; + + cpu1_1: cpu@101 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x101>; +enable-method = "psci"; + }; +}; +... -- 2.7.4
Re: [TEGRA194_CPUFREQ PATCH v5 3/4] cpufreq: Add Tegra194 cpufreq driver
Thank you for the review, Add support for CPU frequency scaling on Tegra194. The frequency of each core can be adjusted by writing a clock divisor value to a MSR on the core. The range of valid divisors is queried from the BPMP. Signed-off-by: Mikko Perttunen Signed-off-by: Sumit Gupta --- drivers/cpufreq/Kconfig.arm| 6 + drivers/cpufreq/Makefile | 1 + drivers/cpufreq/tegra194-cpufreq.c | 397 + 3 files changed, 404 insertions(+) create mode 100644 drivers/cpufreq/tegra194-cpufreq.c diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 15c1a12..f3d8f09 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -314,6 +314,12 @@ config ARM_TEGRA186_CPUFREQ help This adds the CPUFreq driver support for Tegra186 SOCs. +config ARM_TEGRA194_CPUFREQ + tristate "Tegra194 CPUFreq support" + depends on ARCH_TEGRA && TEGRA_BPMP Shouldn't this depend on ARCH_TEGRA_194_SOC instead ? And I asked you to add a default y here itself instead of patch 4/4. Ok. + help + This adds CPU frequency driver support for Tegra194 SOCs. + config ARM_TI_CPUFREQ bool "Texas Instruments CPUFreq support" depends on ARCH_OMAP2PLUS diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index f6670c4..66b5563 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -83,6 +83,7 @@ obj-$(CONFIG_ARM_TANGO_CPUFREQ) += tango-cpufreq.o obj-$(CONFIG_ARM_TEGRA20_CPUFREQ)+= tegra20-cpufreq.o obj-$(CONFIG_ARM_TEGRA124_CPUFREQ) += tegra124-cpufreq.o obj-$(CONFIG_ARM_TEGRA186_CPUFREQ) += tegra186-cpufreq.o +obj-$(CONFIG_ARM_TEGRA194_CPUFREQ) += tegra194-cpufreq.o obj-$(CONFIG_ARM_TI_CPUFREQ) += ti-cpufreq.o obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c +static struct cpufreq_frequency_table * +init_freq_table(struct platform_device *pdev, struct tegra_bpmp *bpmp, + unsigned int cluster_id) +{ + struct cpufreq_frequency_table *freq_table; + struct mrq_cpu_ndiv_limits_response resp; + unsigned int num_freqs, ndiv, delta_ndiv; + struct mrq_cpu_ndiv_limits_request req; + struct tegra_bpmp_message msg; + u16 freq_table_step_size; + int err, index; + + memset(, 0, sizeof(req)); + req.cluster_id = cluster_id; + + memset(, 0, sizeof(msg)); + msg.mrq = MRQ_CPU_NDIV_LIMITS; + msg.tx.data = + msg.tx.size = sizeof(req); + msg.rx.data = + msg.rx.size = sizeof(resp); + + err = tegra_bpmp_transfer(bpmp, ); + if (err) + return ERR_PTR(err); + + /* + * Make sure frequency table step is a multiple of mdiv to match + * vhint table granularity. + */ + freq_table_step_size = resp.mdiv * + DIV_ROUND_UP(CPUFREQ_TBL_STEP_HZ, resp.ref_clk_hz); + + dev_dbg(>dev, "cluster %d: frequency table step size: %d\n", + cluster_id, freq_table_step_size); + + delta_ndiv = resp.ndiv_max - resp.ndiv_min; + + if (unlikely(delta_ndiv == 0)) + num_freqs = 1; + else + /* We store both ndiv_min and ndiv_max hence the +1 */ + num_freqs = delta_ndiv / freq_table_step_size + 1; You need {} in the if else blocks here because of the comment here. Ok. + + num_freqs += (delta_ndiv % freq_table_step_size) ? 1 : 0; + + freq_table = devm_kcalloc(>dev, num_freqs + 1, + sizeof(*freq_table), GFP_KERNEL); + if (!freq_table) + return ERR_PTR(-ENOMEM); + + for (index = 0, ndiv = resp.ndiv_min; + ndiv < resp.ndiv_max; + index++, ndiv += freq_table_step_size) { + freq_table[index].driver_data = ndiv; + freq_table[index].frequency = map_ndiv_to_freq(, ndiv); + } + + freq_table[index].driver_data = resp.ndiv_max; + freq_table[index++].frequency = map_ndiv_to_freq(, resp.ndiv_max); + freq_table[index].frequency = CPUFREQ_TABLE_END; + + return freq_table; +} + +static int tegra194_cpufreq_probe(struct platform_device *pdev) +{ + struct tegra194_cpufreq_data *data; + struct tegra_bpmp *bpmp; + int err, i; + + data = devm_kzalloc(>dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->num_clusters = MAX_CLUSTERS; + data->tables = devm_kcalloc(>dev, data->num_clusters, + sizeof(*data->tables), GFP_KERNEL); + if (!data->tables) + return -ENOMEM; + + platform_set_drvdata(pdev, data); + + bpmp = tegra_bpmp_get(>dev); + if (IS_ERR(bpmp)) + return PTR_ERR(bpmp); + + read_counters_wq = alloc_workqueue("read_counters_wq",
Re: [TEGRA194_CPUFREQ PATCH v6 1/4] dt-bindings: arm: Add NVIDIA Tegra194 CPU Complex binding
Thank you for the review. Add device-tree binding documentation to represent Tegra194 CPU Complex with compatible string under 'cpus' node. This can be used by drivers like cpufreq which don't have their node or CPU Complex node to bind to. Also, documenting 'nvidia,bpmp' property which points to BPMP device. Signed-off-by: Sumit Gupta --- .../bindings/arm/nvidia,tegra194-ccplex.yaml | 106 + 1 file changed, 106 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml diff --git a/Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml b/Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml new file mode 100644 index 000..06dbdaa --- /dev/null +++ b/Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml @@ -0,0 +1,106 @@ +# SPDX-License-Identifier: GPL-2.0 Dual license please. Ok. +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/arm/nvidia,tegra194-ccplex.yaml#; +$schema: "http://devicetree.org/meta-schemas/core.yaml#; + +title: NVIDIA Tegra194 CPU Complex device tree bindings + +maintainers: + - Thierry Reding + - Jonathan Hunter + - Sumit Gupta + +description: |+ + Tegra194 SOC has homogeneous architecture where each cluster has two + symmetric cores. Compatible string in "cpus" node represents the CPU + Complex having all clusters. + +properties: $nodename: const: cpus Ok. + compatible: +enum: + - nvidia,tegra194-ccplex + + nvidia,bpmp: +$ref: '/schemas/types.yaml#/definitions/phandle' +description: | + Specifies the bpmp node that needs to be queried to get + operating point data for all CPUs. + + Optional for systems that have a "compatible" + property value of "nvidia,tegra194-ccplex". The schema says this already. Removed this text from here. + + "#address-cells": +const: 1 This is wrong. The binding says it's 2 cells on aarch64 cpus though we don't enforce that. Removed. + + "#size-cells": +const: 0 + +dependencies: + nvidia,bpmp: [compatible] This is kind of redundant as 'compatible' is required in order to apply the schema. Removed this as well. + +examples: + - | +cpus { + compatible = "nvidia,tegra194-ccplex"; + nvidia,bpmp = <>; + #address-cells = <1>; + #size-cells = <0>; + + cpu0_0: cpu@0 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x0>; +enable-method = "psci"; + }; + + cpu0_1: cpu@1 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x001>; +enable-method = "psci"; + }; + + cpu1_0: cpu@100 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x100>; +enable-method = "psci"; + }; + + cpu1_1: cpu@101 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x101>; +enable-method = "psci"; + }; + + cpu2_0: cpu@200 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x200>; +enable-method = "psci"; + }; + + cpu2_1: cpu@201 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x201>; +enable-method = "psci"; + }; + + cpu3_0: cpu@300 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x300>; +enable-method = "psci"; + }; + + cpu3_1: cpu@301 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x301>; +enable-method = "psci"; + }; Not really that useful describing all these cpus. Ok. Kept first four cpu nodes only. +}; +... -- 2.7.4
Re: [TEGRA194_CPUFREQ PATCH v5 1/4] dt-bindings: arm: Add t194 ccplex compatible and bpmp property
The cpus.yaml binding documents what's in 'cpu' nodes, not 'cpus' node. AIUI, the latter is what you want. You should do your own schema file here. Do you mean to change existing file name from 'cpus.yaml' to 'cpu.yaml' and create new 'cpus.yaml' file? I think it's better to incorporate the change in existing 'cpus.yaml' file to keep both cpu@X and cpus node details together. Please suggest. No, I'm suggesting you create nvidia,tegra194-ccplex.yaml. Have posted new version of only this patch with new schema file. Please review. Thanks, Sumit
[TEGRA194_CPUFREQ PATCH v6 1/4] dt-bindings: arm: Add NVIDIA Tegra194 CPU Complex binding
Add device-tree binding documentation to represent Tegra194 CPU Complex with compatible string under 'cpus' node. This can be used by drivers like cpufreq which don't have their node or CPU Complex node to bind to. Also, documenting 'nvidia,bpmp' property which points to BPMP device. Signed-off-by: Sumit Gupta --- .../bindings/arm/nvidia,tegra194-ccplex.yaml | 106 + 1 file changed, 106 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml diff --git a/Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml b/Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml new file mode 100644 index 000..06dbdaa --- /dev/null +++ b/Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml @@ -0,0 +1,106 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/arm/nvidia,tegra194-ccplex.yaml#; +$schema: "http://devicetree.org/meta-schemas/core.yaml#; + +title: NVIDIA Tegra194 CPU Complex device tree bindings + +maintainers: + - Thierry Reding + - Jonathan Hunter + - Sumit Gupta + +description: |+ + Tegra194 SOC has homogeneous architecture where each cluster has two + symmetric cores. Compatible string in "cpus" node represents the CPU + Complex having all clusters. + +properties: + compatible: +enum: + - nvidia,tegra194-ccplex + + nvidia,bpmp: +$ref: '/schemas/types.yaml#/definitions/phandle' +description: | + Specifies the bpmp node that needs to be queried to get + operating point data for all CPUs. + + Optional for systems that have a "compatible" + property value of "nvidia,tegra194-ccplex". + + "#address-cells": +const: 1 + + "#size-cells": +const: 0 + +dependencies: + nvidia,bpmp: [compatible] + +examples: + - | +cpus { + compatible = "nvidia,tegra194-ccplex"; + nvidia,bpmp = <>; + #address-cells = <1>; + #size-cells = <0>; + + cpu0_0: cpu@0 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x0>; +enable-method = "psci"; + }; + + cpu0_1: cpu@1 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x001>; +enable-method = "psci"; + }; + + cpu1_0: cpu@100 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x100>; +enable-method = "psci"; + }; + + cpu1_1: cpu@101 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x101>; +enable-method = "psci"; + }; + + cpu2_0: cpu@200 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x200>; +enable-method = "psci"; + }; + + cpu2_1: cpu@201 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x201>; +enable-method = "psci"; + }; + + cpu3_0: cpu@300 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x300>; +enable-method = "psci"; + }; + + cpu3_1: cpu@301 { +compatible = "nvidia,tegra194-carmel"; +device_type = "cpu"; +reg = <0x301>; +enable-method = "psci"; + }; +}; +... -- 2.7.4
Re: [TEGRA194_CPUFREQ PATCH v5 1/4] dt-bindings: arm: Add t194 ccplex compatible and bpmp property
On Mon, Jul 13, 2020 at 07:36:46PM +0530, Sumit Gupta wrote: To do frequency scaling on all CPUs within T194 CPU Complex, we need to query BPMP for data on valid operating points. Document a compatible string under 'cpus' node to represent the CPU Complex for binding drivers like cpufreq which don't have their node or CPU Complex node to bind to. Also, document a property to point to the BPMP device that can be queried for all CPUs. The cpus.yaml binding documents what's in 'cpu' nodes, not 'cpus' node. AIUI, the latter is what you want. You should do your own schema file here. Do you mean to change existing file name from 'cpus.yaml' to 'cpu.yaml' and create new 'cpus.yaml' file? I think it's better to incorporate the change in existing 'cpus.yaml' file to keep both cpu@X and cpus node details together. Please suggest. Signed-off-by: Sumit Gupta --- Documentation/devicetree/bindings/arm/cpus.yaml | 11 +++ 1 file changed, 11 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/cpus.yaml b/Documentation/devicetree/bindings/arm/cpus.yaml index a018147..9b328e3 100644 --- a/Documentation/devicetree/bindings/arm/cpus.yaml +++ b/Documentation/devicetree/bindings/arm/cpus.yaml @@ -162,6 +162,7 @@ properties: - nvidia,tegra132-denver - nvidia,tegra186-denver - nvidia,tegra194-carmel + - nvidia,tegra194-ccplex Tegra194 has 2 different CPUs? No, T194 SOC has homogeneous architecture with four clusters where each cluster has two symmetric cores. 'nvidia,tegra194-carmel' compatible string represents each cpu. 'nvidia,tegra194-ccplex' string represents the CPU Complex to bind cpufreq driver. The change was done as per discussion [1] - qcom,krait - qcom,kryo - qcom,kryo260 @@ -255,6 +256,15 @@ properties: where voltage is in V, frequency is in MHz. + nvidia,bpmp: +$ref: '/schemas/types.yaml#/definitions/phandle' +description: | + Specifies the bpmp node that needs to be queried to get + operating point data for all CPUs. + + Optional for systems that have a "compatible" + property value of "nvidia,tegra194-ccplex". + power-domains: $ref: '/schemas/types.yaml#/definitions/phandle-array' description: @@ -340,6 +350,7 @@ required: dependencies: rockchip,pmu: [enable-method] + nvidia,bpmp: [compatible] examples: - | -- 2.7.4 [1] https://marc.info/?l=linux-arm-kernel=158999171528418=2
Re: [TEGRA194_CPUFREQ PATCH v4 3/4] cpufreq: Add Tegra194 cpufreq driver
On 26-06-20, 21:13, Sumit Gupta wrote: +static int tegra194_cpufreq_probe(struct platform_device *pdev) +{ + struct tegra194_cpufreq_data *data; + struct tegra_bpmp *bpmp; + int err, i; + + data = devm_kzalloc(>dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->num_clusters = MAX_CLUSTERS; + data->tables = devm_kcalloc(>dev, data->num_clusters, + sizeof(*data->tables), GFP_KERNEL); + if (!data->tables) + return -ENOMEM; + + platform_set_drvdata(pdev, data); + + bpmp = tegra_bpmp_get(>dev); + if (IS_ERR(bpmp)) + return PTR_ERR(bpmp); + + read_counters_wq = alloc_workqueue("read_counters_wq", __WQ_LEGACY, 1); + if (!read_counters_wq) { + dev_err(>dev, "fail to create_workqueue\n"); + err = -EINVAL; + goto put_bpmp; This will call destroy_workqueue() eventually and it will crash your kernel. Apart from this, this stuff looks okay. Don't resend the patch just yet (and if required, send only this patch using --in-reply-to flag for git send email). Lets wait for an Ack from Rob for the first two patches. Sorry for the delayed response as i was on PTO. Thank you for the feedback. Have posted a v5 based on v4 patch set. + } + -- viresh
[TEGRA194_CPUFREQ PATCH v5 4/4] soc/tegra: cpufreq: select cpufreq for Tegra194
Select ARM_TEGRA194_CPUFREQ by default to enable CPU frequency scaling support for Tegra194 SOC. Signed-off-by: Sumit Gupta --- drivers/soc/tegra/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/soc/tegra/Kconfig b/drivers/soc/tegra/Kconfig index 2e95809..6955cee 100644 --- a/drivers/soc/tegra/Kconfig +++ b/drivers/soc/tegra/Kconfig @@ -117,6 +117,7 @@ config ARCH_TEGRA_194_SOC select TEGRA_HSP_MBOX select TEGRA_IVC select SOC_TEGRA_PMC + select ARM_TEGRA194_CPUFREQ help Enable support for the NVIDIA Tegra194 SoC. -- 2.7.4
[TEGRA194_CPUFREQ PATCH v5 1/4] dt-bindings: arm: Add t194 ccplex compatible and bpmp property
To do frequency scaling on all CPUs within T194 CPU Complex, we need to query BPMP for data on valid operating points. Document a compatible string under 'cpus' node to represent the CPU Complex for binding drivers like cpufreq which don't have their node or CPU Complex node to bind to. Also, document a property to point to the BPMP device that can be queried for all CPUs. Signed-off-by: Sumit Gupta --- Documentation/devicetree/bindings/arm/cpus.yaml | 11 +++ 1 file changed, 11 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/cpus.yaml b/Documentation/devicetree/bindings/arm/cpus.yaml index a018147..9b328e3 100644 --- a/Documentation/devicetree/bindings/arm/cpus.yaml +++ b/Documentation/devicetree/bindings/arm/cpus.yaml @@ -162,6 +162,7 @@ properties: - nvidia,tegra132-denver - nvidia,tegra186-denver - nvidia,tegra194-carmel + - nvidia,tegra194-ccplex - qcom,krait - qcom,kryo - qcom,kryo260 @@ -255,6 +256,15 @@ properties: where voltage is in V, frequency is in MHz. + nvidia,bpmp: +$ref: '/schemas/types.yaml#/definitions/phandle' +description: | + Specifies the bpmp node that needs to be queried to get + operating point data for all CPUs. + + Optional for systems that have a "compatible" + property value of "nvidia,tegra194-ccplex". + power-domains: $ref: '/schemas/types.yaml#/definitions/phandle-array' description: @@ -340,6 +350,7 @@ required: dependencies: rockchip,pmu: [enable-method] + nvidia,bpmp: [compatible] examples: - | -- 2.7.4
[TEGRA194_CPUFREQ PATCH v5 2/4] arm64: tegra: Add t194 ccplex compatible and bpmp property
On Tegra194, data on valid operating points for the CPUs needs to be queried from BPMP. In T194, there is no node representing CPU complex. So, add compatible string to the 'cpus' node instead of using dummy node to bind cpufreq driver. Also, add reference to the BPMP instance for the CPU complex. Signed-off-by: Sumit Gupta --- arch/arm64/boot/dts/nvidia/tegra194.dtsi | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi index 7c9511a..0abf287 100644 --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi @@ -1764,6 +1764,8 @@ }; cpus { + compatible = "nvidia,tegra194-ccplex"; + nvidia,bpmp = <>; #address-cells = <1>; #size-cells = <0>; -- 2.7.4
[TEGRA194_CPUFREQ PATCH v5 3/4] cpufreq: Add Tegra194 cpufreq driver
Add support for CPU frequency scaling on Tegra194. The frequency of each core can be adjusted by writing a clock divisor value to a MSR on the core. The range of valid divisors is queried from the BPMP. Signed-off-by: Mikko Perttunen Signed-off-by: Sumit Gupta --- drivers/cpufreq/Kconfig.arm| 6 + drivers/cpufreq/Makefile | 1 + drivers/cpufreq/tegra194-cpufreq.c | 397 + 3 files changed, 404 insertions(+) create mode 100644 drivers/cpufreq/tegra194-cpufreq.c diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 15c1a12..f3d8f09 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -314,6 +314,12 @@ config ARM_TEGRA186_CPUFREQ help This adds the CPUFreq driver support for Tegra186 SOCs. +config ARM_TEGRA194_CPUFREQ + tristate "Tegra194 CPUFreq support" + depends on ARCH_TEGRA && TEGRA_BPMP + help + This adds CPU frequency driver support for Tegra194 SOCs. + config ARM_TI_CPUFREQ bool "Texas Instruments CPUFreq support" depends on ARCH_OMAP2PLUS diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index f6670c4..66b5563 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -83,6 +83,7 @@ obj-$(CONFIG_ARM_TANGO_CPUFREQ) += tango-cpufreq.o obj-$(CONFIG_ARM_TEGRA20_CPUFREQ) += tegra20-cpufreq.o obj-$(CONFIG_ARM_TEGRA124_CPUFREQ) += tegra124-cpufreq.o obj-$(CONFIG_ARM_TEGRA186_CPUFREQ) += tegra186-cpufreq.o +obj-$(CONFIG_ARM_TEGRA194_CPUFREQ) += tegra194-cpufreq.o obj-$(CONFIG_ARM_TI_CPUFREQ) += ti-cpufreq.o obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c new file mode 100644 index 000..450477f --- /dev/null +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -0,0 +1,397 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2020, NVIDIA CORPORATION. All rights reserved + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include +#include + +#define KHZ 1000 +#define REF_CLK_MHZ 408 /* 408 MHz */ +#define US_DELAY500 +#define US_DELAY_MIN2 +#define CPUFREQ_TBL_STEP_HZ (50 * KHZ * KHZ) +#define MAX_CNT ~0U + +/* cpufreq transisition latency */ +#define TEGRA_CPUFREQ_TRANSITION_LATENCY (300 * 1000) /* unit in nanoseconds */ + +enum cluster { + CLUSTER0, + CLUSTER1, + CLUSTER2, + CLUSTER3, + MAX_CLUSTERS, +}; + +struct tegra194_cpufreq_data { + void __iomem *regs; + size_t num_clusters; + struct cpufreq_frequency_table **tables; +}; + +struct tegra_cpu_ctr { + u32 cpu; + u32 delay; + u32 coreclk_cnt, last_coreclk_cnt; + u32 refclk_cnt, last_refclk_cnt; +}; + +struct read_counters_work { + struct work_struct work; + struct tegra_cpu_ctr c; +}; + +static struct workqueue_struct *read_counters_wq; + +static enum cluster get_cpu_cluster(u8 cpu) +{ + return MPIDR_AFFINITY_LEVEL(cpu_logical_map(cpu), 1); +} + +/* + * Read per-core Read-only system register NVFREQ_FEEDBACK_EL1. + * The register provides frequency feedback information to + * determine the average actual frequency a core has run at over + * a period of time. + * [31:0] PLLP counter: Counts at fixed frequency (408 MHz) + * [63:32] Core clock counter: counts on every core clock cycle + * where the core is architecturally clocking + */ +static u64 read_freq_feedback(void) +{ + u64 val = 0; + + asm volatile("mrs %0, s3_0_c15_c0_5" : "=r" (val) : ); + + return val; +} + +static inline u32 map_ndiv_to_freq(struct mrq_cpu_ndiv_limits_response + *nltbl, u16 ndiv) +{ + return nltbl->ref_clk_hz / KHZ * ndiv / (nltbl->pdiv * nltbl->mdiv); +} + +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 = _counters
[TEGRA194_CPUFREQ PATCH v5 0/4] Add cpufreq driver for Tegra194
Hi Viresh, The patch series adds cpufreq driver for Tegra194 SOC. Incorporated the feedback on previous version of patchset. Please consider this patch series for merging in 5.9. Hi Rob, Can you please review/ack DT patches (1-2). v4[4] -> v5 - Don't call destroy_workqueue() if alloc_workqueue() fails[Viresh] - Move CONFIG_ARM_TEGRA194_CPUFREQ enabling to soc/tegra/Kconfig[Viresh] - Add dependency of 'nvidia,bpmp' on 'compatible' in yaml file[Michal] - Fix typo in description causing dt_binding_check bot failure[Rob] v3[3] -> v4 - Open code LOOP_FOR_EACH_CPU_OF_CLUSTER macro[Viresh] - Delete unused funciton map_freq_to_ndiv[Viresh, kernel test bot] - Remove flush_workqueue from free_resources[Viresh] v2[2] -> v3 - Set same policy for all cpus in a cluster[Viresh]. - Add compatible string for CPU Complex under cpus node[Thierry]. - Add reference to bpmp node under cpus node[Thierry]. - Bind cpufreq driver to CPU Complex compatible string[Thierry]. - Remove patch to get bpmp data as now using cpus node to get that[Thierry]. v1[1] -> v2: - Remove cpufreq_lock mutex from tegra194_cpufreq_set_target [Viresh]. - Remove CPUFREQ_ASYNC_NOTIFICATION flag [Viresh]. - Remove redundant _begin|end() call from tegra194_cpufreq_set_target. - Rename opp_table to freq_table [Viresh]. Sumit Gupta (4): dt-bindings: arm: Add t194 ccplex compatible and bpmp property arm64: tegra: Add t194 ccplex compatible and bpmp property cpufreq: Add Tegra194 cpufreq driver soc/tegra: cpufreq: select cpufreq for Tegra194 Documentation/devicetree/bindings/arm/cpus.yaml | 11 + arch/arm64/boot/dts/nvidia/tegra194.dtsi| 2 + drivers/cpufreq/Kconfig.arm | 6 + drivers/cpufreq/Makefile| 1 + drivers/cpufreq/tegra194-cpufreq.c | 397 drivers/soc/tegra/Kconfig | 1 + 6 files changed, 418 insertions(+) create mode 100644 drivers/cpufreq/tegra194-cpufreq.c [1] https://marc.info/?t=15753945231=1=2 [2] https://marc.info/?l=linux-tegra=158602857106213=2 [3] https://marc.info/?l=linux-pm=159283376010084=2 [4] https://marc.info/?l=linux-tegra=159318640622917=2 -- 2.7.4
[TEGRA194_CPUFREQ PATCH v4 2/4] arm64: tegra: Add t194 ccplex compatible and bpmp property
On Tegra194, data on valid operating points for the CPUs needs to be queried from BPMP. In T194, there is no node representing CPU complex. So, add compatible string to the 'cpus' node instead of using dummy node to bind cpufreq driver. Also, add reference to the BPMP instance for the CPU complex. Signed-off-by: Sumit Gupta --- arch/arm64/boot/dts/nvidia/tegra194.dtsi | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi index 7c9511a..0abf287 100644 --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi @@ -1764,6 +1764,8 @@ }; cpus { + compatible = "nvidia,tegra194-ccplex"; + nvidia,bpmp = <>; #address-cells = <1>; #size-cells = <0>; -- 2.7.4
[TEGRA194_CPUFREQ PATCH v4 4/4] arm64: defconfig: Enable CONFIG_ARM_TEGRA194_CPUFREQ
Enable Tegra194 CPU frequency scaling support by default. Signed-off-by: Sumit Gupta --- arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index f9d378d..385bd35 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -91,6 +91,7 @@ CONFIG_ARM_QCOM_CPUFREQ_NVMEM=y CONFIG_ARM_QCOM_CPUFREQ_HW=y CONFIG_ARM_RASPBERRYPI_CPUFREQ=m CONFIG_ARM_TEGRA186_CPUFREQ=y +CONFIG_ARM_TEGRA194_CPUFREQ=y CONFIG_QORIQ_CPUFREQ=y CONFIG_ARM_SCPI_PROTOCOL=y CONFIG_RASPBERRYPI_FIRMWARE=y -- 2.7.4
[TEGRA194_CPUFREQ PATCH v4 3/4] cpufreq: Add Tegra194 cpufreq driver
Add support for CPU frequency scaling on Tegra194. The frequency of each core can be adjusted by writing a clock divisor value to a MSR on the core. The range of valid divisors is queried from the BPMP. Signed-off-by: Mikko Perttunen Signed-off-by: Sumit Gupta --- drivers/cpufreq/Kconfig.arm| 6 + drivers/cpufreq/Makefile | 1 + drivers/cpufreq/tegra194-cpufreq.c | 396 + 3 files changed, 403 insertions(+) create mode 100644 drivers/cpufreq/tegra194-cpufreq.c diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 15c1a12..f3d8f09 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -314,6 +314,12 @@ config ARM_TEGRA186_CPUFREQ help This adds the CPUFreq driver support for Tegra186 SOCs. +config ARM_TEGRA194_CPUFREQ + tristate "Tegra194 CPUFreq support" + depends on ARCH_TEGRA && TEGRA_BPMP + help + This adds CPU frequency driver support for Tegra194 SOCs. + config ARM_TI_CPUFREQ bool "Texas Instruments CPUFreq support" depends on ARCH_OMAP2PLUS diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index f6670c4..66b5563 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -83,6 +83,7 @@ obj-$(CONFIG_ARM_TANGO_CPUFREQ) += tango-cpufreq.o obj-$(CONFIG_ARM_TEGRA20_CPUFREQ) += tegra20-cpufreq.o obj-$(CONFIG_ARM_TEGRA124_CPUFREQ) += tegra124-cpufreq.o obj-$(CONFIG_ARM_TEGRA186_CPUFREQ) += tegra186-cpufreq.o +obj-$(CONFIG_ARM_TEGRA194_CPUFREQ) += tegra194-cpufreq.o obj-$(CONFIG_ARM_TI_CPUFREQ) += ti-cpufreq.o obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c new file mode 100644 index 000..77e422a --- /dev/null +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -0,0 +1,396 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2020, NVIDIA CORPORATION. All rights reserved + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include +#include + +#define KHZ 1000 +#define REF_CLK_MHZ 408 /* 408 MHz */ +#define US_DELAY500 +#define US_DELAY_MIN2 +#define CPUFREQ_TBL_STEP_HZ (50 * KHZ * KHZ) +#define MAX_CNT ~0U + +/* cpufreq transisition latency */ +#define TEGRA_CPUFREQ_TRANSITION_LATENCY (300 * 1000) /* unit in nanoseconds */ + +enum cluster { + CLUSTER0, + CLUSTER1, + CLUSTER2, + CLUSTER3, + MAX_CLUSTERS, +}; + +struct tegra194_cpufreq_data { + void __iomem *regs; + size_t num_clusters; + struct cpufreq_frequency_table **tables; +}; + +struct tegra_cpu_ctr { + u32 cpu; + u32 delay; + u32 coreclk_cnt, last_coreclk_cnt; + u32 refclk_cnt, last_refclk_cnt; +}; + +struct read_counters_work { + struct work_struct work; + struct tegra_cpu_ctr c; +}; + +static struct workqueue_struct *read_counters_wq; + +static enum cluster get_cpu_cluster(u8 cpu) +{ + return MPIDR_AFFINITY_LEVEL(cpu_logical_map(cpu), 1); +} + +/* + * Read per-core Read-only system register NVFREQ_FEEDBACK_EL1. + * The register provides frequency feedback information to + * determine the average actual frequency a core has run at over + * a period of time. + * [31:0] PLLP counter: Counts at fixed frequency (408 MHz) + * [63:32] Core clock counter: counts on every core clock cycle + * where the core is architecturally clocking + */ +static u64 read_freq_feedback(void) +{ + u64 val = 0; + + asm volatile("mrs %0, s3_0_c15_c0_5" : "=r" (val) : ); + + return val; +} + +static inline u32 map_ndiv_to_freq(struct mrq_cpu_ndiv_limits_response + *nltbl, u16 ndiv) +{ + return nltbl->ref_clk_hz / KHZ * ndiv / (nltbl->pdiv * nltbl->mdiv); +} + +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 = _counters
[TEGRA194_CPUFREQ PATCH v4 1/4] dt-bindings: arm: Add t194 ccplex compatible and bpmp property
To do frequency scaling on all CPUs within T194 CPU Complex, we need to query BPMP for data on valid operating points. Document a compatible string under 'cpus' node to represent the CPU Complex for binding drivers like cpufreq which don't have their node or CPU Complex node to bind to. Also, document a property to point to the BPMP device that can be queried for all CPUs. Signed-off-by: Sumit Gupta --- Documentation/devicetree/bindings/arm/cpus.yaml | 9 + 1 file changed, 9 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/cpus.yaml b/Documentation/devicetree/bindings/arm/cpus.yaml index a018147..737b55e 100644 --- a/Documentation/devicetree/bindings/arm/cpus.yaml +++ b/Documentation/devicetree/bindings/arm/cpus.yaml @@ -162,6 +162,7 @@ properties: - nvidia,tegra132-denver - nvidia,tegra186-denver - nvidia,tegra194-carmel + - nvidia,tegra194-ccplex - qcom,krait - qcom,kryo - qcom,kryo260 @@ -255,6 +256,14 @@ properties: where voltage is in V, frequency is in MHz. + nvidia,bpmp: +$ref: '/schemas/types.yaml#/definitions/phandle' +descrption: | + Specifies the bpmp node that needs to be queried to get + operating point data for all CPUs. + + Optional for NVIDIA Tegra194 Carmel CPUs + power-domains: $ref: '/schemas/types.yaml#/definitions/phandle-array' description: -- 2.7.4
[TEGRA194_CPUFREQ PATCH v4 0/4] Add cpufreq driver for Tegra194
The patch series adds cpufreq driver for Tegra194 SOC. Incorporated the feedback on previous version patchset. Please consider this patch series for merging in 5.9. v3[3] -> v4 - Open code LOOP_FOR_EACH_CPU_OF_CLUSTER macro[Viresh] - Delete unused funciton map_freq_to_ndiv[Viresh, kernel test bot] - Remove flush_workqueue from free_resources[Viresh] v2[2] -> v3 - Set same policy for all cpus in a cluster[Viresh]. - Add compatible string for CPU Complex under cpus node[Thierry]. - Add reference to bpmp node under cpus node[Thierry]. - Bind cpufreq driver to CPU Complex compatible string[Thierry]. - Remove patch to get bpmp data as now using cpus node to get that[Thierry]. v1[1] -> v2: - Remove cpufreq_lock mutex from tegra194_cpufreq_set_target [Viresh]. - Remove CPUFREQ_ASYNC_NOTIFICATION flag [Viresh]. - Remove redundant _begin|end() call from tegra194_cpufreq_set_target. - Rename opp_table to freq_table [Viresh]. Sumit Gupta (4): dt-bindings: arm: Add t194 ccplex compatible and bpmp property arm64: tegra: Add t194 ccplex compatible and bpmp property cpufreq: Add Tegra194 cpufreq driver arm64: defconfig: Enable CONFIG_ARM_TEGRA194_CPUFREQ Documentation/devicetree/bindings/arm/cpus.yaml | 9 + arch/arm64/boot/dts/nvidia/tegra194.dtsi| 2 + arch/arm64/configs/defconfig| 1 + drivers/cpufreq/Kconfig.arm | 6 + drivers/cpufreq/Makefile| 1 + drivers/cpufreq/tegra194-cpufreq.c | 396 6 files changed, 415 insertions(+) create mode 100644 drivers/cpufreq/tegra194-cpufreq.c [1] https://marc.info/?t=15753945231=1=2 [2] https://marc.info/?l=linux-tegra=158602857106213=2 [3] https://marc.info/?l=linux-pm=159283376010084=2 -- 2.7.4
Re: [TEGRA194_CPUFREQ Patch v3 1/4] dt-bindings: arm: Add t194 ccplex compatible and bpmp property
On 22/06/20 12:52 PM, Viresh Kumar wrote: External email: Use caution opening links or attachments On 22-06-20, 03:04, Sumit Gupta wrote: To do frequency scaling on all CPUs within T194 CPU Complex, we need to query BPMP for data on valid operating points. Document a compatible string under 'cpus' node to represent the CPU Complex for binding drivers like cpufreq which don't have their node or CPU Complex node to bind to. Also, document a property to point to the BPMP device that can be queried for all CPUs. You shouldn't be putting how linux is going to use this information and entries shouldn't be made just so cpufreq can bind to a driver. Though I see that this is a real hardware register which you can use to interact with the firmware ? And so it makes sense to have it, maybe in different form though. CPUFREQ driver doesn't communicate directly with BPMP firmware. It uses BPMP node's reference to call api's exported by BPMP driver which communicates with BPMP firmware. I will let Rob explain what would be the right way of doing this though. This is already discussed by Thierry with Rob. Please refer https://marc.info/?l=linux-arm-kernel=158999171528418=2 Signed-off-by: Sumit Gupta --- Documentation/devicetree/bindings/arm/cpus.yaml | 9 + 1 file changed, 9 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/cpus.yaml b/Documentation/devicetree/bindings/arm/cpus.yaml index a018147..737b55e 100644 --- a/Documentation/devicetree/bindings/arm/cpus.yaml +++ b/Documentation/devicetree/bindings/arm/cpus.yaml @@ -162,6 +162,7 @@ properties: - nvidia,tegra132-denver - nvidia,tegra186-denver - nvidia,tegra194-carmel + - nvidia,tegra194-ccplex - qcom,krait - qcom,kryo - qcom,kryo260 @@ -255,6 +256,14 @@ properties: where voltage is in V, frequency is in MHz. + nvidia,bpmp: +$ref: '/schemas/types.yaml#/definitions/phandle' +descrption: | + Specifies the bpmp node that needs to be queried to get + operating point data for all CPUs. + + Optional for NVIDIA Tegra194 Carmel CPUs + power-domains: $ref: '/schemas/types.yaml#/definitions/phandle-array' description: -- 2.7.4 -- viresh
Re: [TEGRA194_CPUFREQ Patch v3 3/4] cpufreq: Add Tegra194 cpufreq driver
Hi Viresh, Thank you for the review. please find my reply inline. +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -0,0 +1,403 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved 2020 + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include +#include + +#define KHZ 1000 +#define REF_CLK_MHZ 408 /* 408 MHz */ +#define US_DELAY500 +#define US_DELAY_MIN2 +#define CPUFREQ_TBL_STEP_HZ (50 * KHZ * KHZ) +#define MAX_CNT ~0U + +/* cpufreq transisition latency */ +#define TEGRA_CPUFREQ_TRANSITION_LATENCY (300 * 1000) /* unit in nanoseconds */ + +#define LOOP_FOR_EACH_CPU_OF_CLUSTER(cl) for (cpu = (cl * 2); \ + cpu < ((cl + 1) * 2); cpu++) Both latency and this loop are used only once in the code, maybe just open code it. Also you should have passed cpu as a parameter to the macro, even if it works fine without it, for better readability. Ok, i will open code the loop in next version. For latency value, i feel named macro makes readability better. So, prefer keeping it. + +u16 map_freq_to_ndiv(struct mrq_cpu_ndiv_limits_response *nltbl, u32 freq) Unused routine Sure, will remove it. +{ + return DIV_ROUND_UP(freq * nltbl->pdiv * nltbl->mdiv, + nltbl->ref_clk_hz / KHZ); +} +static int tegra194_cpufreq_init(struct cpufreq_policy *policy) +{ + struct tegra194_cpufreq_data *data = cpufreq_get_driver_data(); + int cl = get_cpu_cluster(policy->cpu); + u32 cpu; + + if (cl >= data->num_clusters) + return -EINVAL; + + policy->cur = tegra194_fast_get_speed(policy->cpu); /* boot freq */ + + /* set same policy for all cpus in a cluster */ + LOOP_FOR_EACH_CPU_OF_CLUSTER(cl) + cpumask_set_cpu(cpu, policy->cpus); + + policy->freq_table = data->tables[cl]; + policy->cpuinfo.transition_latency = TEGRA_CPUFREQ_TRANSITION_LATENCY; + + return 0; +} +static int tegra194_cpufreq_set_target(struct cpufreq_policy *policy, +unsigned int index) +{ + struct cpufreq_frequency_table *tbl = policy->freq_table + index; + + on_each_cpu_mask(policy->cpus, set_cpu_ndiv, tbl, true); I am still a bit confused. While setting the frequency you are calling this routine for each CPU of the policy (cluster). Does that mean that CPUs within a cluster can actually run at different frequencies at any given point of time ? If cpufreq terms, a cpufreq policy represents a group of CPUs that change frequency together, i.e. they share the clk line. If all CPUs in your system can do DVFS separately, then you must have policy per CPU, instead of cluster. T194 supports four CPU clusters, each with two cores. Each CPU cluster is capable of running at a specific frequency sourced by respective NAFLL to provide cluster specific clocks. Individual cores within a cluster write freq in per core register. Cluster h/w forwards the max(core0, core1) request to per cluster NAFLL. +static void tegra194_cpufreq_free_resources(void) +{ + flush_workqueue(read_counters_wq); Why is this required exactly? I see that you add the work request and immediately flush it, then why would you need to do this separately ? Ya, will remove flush_workqueue(). + destroy_workqueue(read_counters_wq); +} + +static struct cpufreq_frequency_table * +init_freq_table(struct platform_device *pdev, struct tegra_bpmp *bpmp, + unsigned int cluster_id) +{ + struct cpufreq_frequency_table *freq_table; + struct mrq_cpu_ndiv_limits_response resp; + unsigned int num_freqs, ndiv, delta_ndiv; + struct mrq_cpu_ndiv_limits_request req; + struct tegra_bpmp_message msg; + u16 freq_table_step_size; + int err, index; + + memset(, 0, sizeof(req)); + req.cluster_id = cluster_id; + + memset(, 0, sizeof(msg)); + msg.mrq = MRQ_CPU_NDIV_LIMITS; + msg.tx.data = + msg.tx.size = sizeof(req); + msg.rx.data = + msg.rx.size = sizeof(resp); + + err = tegra_bpmp_transfer(bpmp, ); So the firmware can actually return different frequency tables for the clusters, right ? Else you could have received the table only once and used it for all the CPUs. Yes, frequency tables are returned per cluster by BPMP firmware. In T194 SOC, currently same table values are used for all clusters. This might change in future. + if (err) + return ERR_PTR(err); + + /* + * Make sure frequency table step is a multiple of mdiv to match + * vhint table granularity. + */ + freq_table_step_size = resp.mdiv * + DIV_ROUND_UP(CPUFREQ_TBL_STEP_HZ, resp.ref_clk_hz); + + dev_dbg(>dev, "cluster %d: frequency table step size: %d\n", + cluster_id, freq_table_step_size);
[TEGRA194_CPUFREQ Patch v3 1/4] dt-bindings: arm: Add t194 ccplex compatible and bpmp property
To do frequency scaling on all CPUs within T194 CPU Complex, we need to query BPMP for data on valid operating points. Document a compatible string under 'cpus' node to represent the CPU Complex for binding drivers like cpufreq which don't have their node or CPU Complex node to bind to. Also, document a property to point to the BPMP device that can be queried for all CPUs. Signed-off-by: Sumit Gupta --- Documentation/devicetree/bindings/arm/cpus.yaml | 9 + 1 file changed, 9 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/cpus.yaml b/Documentation/devicetree/bindings/arm/cpus.yaml index a018147..737b55e 100644 --- a/Documentation/devicetree/bindings/arm/cpus.yaml +++ b/Documentation/devicetree/bindings/arm/cpus.yaml @@ -162,6 +162,7 @@ properties: - nvidia,tegra132-denver - nvidia,tegra186-denver - nvidia,tegra194-carmel + - nvidia,tegra194-ccplex - qcom,krait - qcom,kryo - qcom,kryo260 @@ -255,6 +256,14 @@ properties: where voltage is in V, frequency is in MHz. + nvidia,bpmp: +$ref: '/schemas/types.yaml#/definitions/phandle' +descrption: | + Specifies the bpmp node that needs to be queried to get + operating point data for all CPUs. + + Optional for NVIDIA Tegra194 Carmel CPUs + power-domains: $ref: '/schemas/types.yaml#/definitions/phandle-array' description: -- 2.7.4
[TEGRA194_CPUFREQ Patch v3 2/4] arm64: tegra: Add t194 ccplex compatible and bpmp property
On Tegra194, data on valid operating points for the CPUs needs to be queried from BPMP. In T194, there is no node representing CPU complex. So, add compatible string to the 'cpus' node instead of using dummy node to bind cpufreq driver. Also, add reference to the BPMP instance for the CPU complex. Signed-off-by: Sumit Gupta --- arch/arm64/boot/dts/nvidia/tegra194.dtsi | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi index 7c9511a..0abf287 100644 --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi @@ -1764,6 +1764,8 @@ }; cpus { + compatible = "nvidia,tegra194-ccplex"; + nvidia,bpmp = <>; #address-cells = <1>; #size-cells = <0>; -- 2.7.4
[TEGRA194_CPUFREQ Patch v3 3/4] cpufreq: Add Tegra194 cpufreq driver
Add support for CPU frequency scaling on Tegra194. The frequency of each core can be adjusted by writing a clock divisor value to a MSR on the core. The range of valid divisors is queried from the BPMP. Signed-off-by: Mikko Perttunen Signed-off-by: Sumit Gupta --- drivers/cpufreq/Kconfig.arm| 6 + drivers/cpufreq/Makefile | 1 + drivers/cpufreq/tegra194-cpufreq.c | 403 + 3 files changed, 410 insertions(+) create mode 100644 drivers/cpufreq/tegra194-cpufreq.c diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index 15c1a12..f3d8f09 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -314,6 +314,12 @@ config ARM_TEGRA186_CPUFREQ help This adds the CPUFreq driver support for Tegra186 SOCs. +config ARM_TEGRA194_CPUFREQ + tristate "Tegra194 CPUFreq support" + depends on ARCH_TEGRA && TEGRA_BPMP + help + This adds CPU frequency driver support for Tegra194 SOCs. + config ARM_TI_CPUFREQ bool "Texas Instruments CPUFreq support" depends on ARCH_OMAP2PLUS diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index f6670c4..66b5563 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -83,6 +83,7 @@ obj-$(CONFIG_ARM_TANGO_CPUFREQ) += tango-cpufreq.o obj-$(CONFIG_ARM_TEGRA20_CPUFREQ) += tegra20-cpufreq.o obj-$(CONFIG_ARM_TEGRA124_CPUFREQ) += tegra124-cpufreq.o obj-$(CONFIG_ARM_TEGRA186_CPUFREQ) += tegra186-cpufreq.o +obj-$(CONFIG_ARM_TEGRA194_CPUFREQ) += tegra194-cpufreq.o obj-$(CONFIG_ARM_TI_CPUFREQ) += ti-cpufreq.o obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c new file mode 100644 index 000..8de8000 --- /dev/null +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -0,0 +1,403 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include +#include + +#define KHZ 1000 +#define REF_CLK_MHZ 408 /* 408 MHz */ +#define US_DELAY500 +#define US_DELAY_MIN2 +#define CPUFREQ_TBL_STEP_HZ (50 * KHZ * KHZ) +#define MAX_CNT ~0U + +/* cpufreq transisition latency */ +#define TEGRA_CPUFREQ_TRANSITION_LATENCY (300 * 1000) /* unit in nanoseconds */ + +#define LOOP_FOR_EACH_CPU_OF_CLUSTER(cl) for (cpu = (cl * 2); \ + cpu < ((cl + 1) * 2); cpu++) + +enum cluster { + CLUSTER0, + CLUSTER1, + CLUSTER2, + CLUSTER3, + MAX_CLUSTERS, +}; + +struct tegra194_cpufreq_data { + void __iomem *regs; + size_t num_clusters; + struct cpufreq_frequency_table **tables; +}; + +struct tegra_cpu_ctr { + u32 cpu; + u32 delay; + u32 coreclk_cnt, last_coreclk_cnt; + u32 refclk_cnt, last_refclk_cnt; +}; + +struct read_counters_work { + struct work_struct work; + struct tegra_cpu_ctr c; +}; + +static struct workqueue_struct *read_counters_wq; + + +static enum cluster get_cpu_cluster(u8 cpu) +{ + return MPIDR_AFFINITY_LEVEL(cpu_logical_map(cpu), 1); +} + +/* + * Read per-core Read-only system register NVFREQ_FEEDBACK_EL1. + * The register provides frequency feedback information to + * determine the average actual frequency a core has run at over + * a period of time. + * [31:0] PLLP counter: Counts at fixed frequency (408 MHz) + * [63:32] Core clock counter: counts on every core clock cycle + * where the core is architecturally clocking + */ +static u64 read_freq_feedback(void) +{ + u64 val = 0; + + asm volatile("mrs %0, s3_0_c15_c0_5" : "=r" (val) : ); + + return val; +} + +u16 map_freq_to_ndiv(struct mrq_cpu_ndiv_limits_response *nltbl, u32 freq) +{ + return DIV_ROUND_UP(freq * nltbl->pdiv * nltbl->mdiv, + nltbl->ref_clk_hz / KHZ); +} + +static inline u32 map_ndiv_to_freq(struct mrq_cpu_ndiv_limits_response + *nltbl, u16 ndiv) +{ + return nltbl->ref_clk_hz / KHZ * ndiv / (nltbl->pdiv * nltbl->mdiv); +} + +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 ru
[TEGRA194_CPUFREQ Patch v3 4/4] arm64: defconfig: Enable CONFIG_ARM_TEGRA194_CPUFREQ
Enable Tegra194 CPU frequency scaling support by default. Signed-off-by: Sumit Gupta --- arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index f9d378d..385bd35 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -91,6 +91,7 @@ CONFIG_ARM_QCOM_CPUFREQ_NVMEM=y CONFIG_ARM_QCOM_CPUFREQ_HW=y CONFIG_ARM_RASPBERRYPI_CPUFREQ=m CONFIG_ARM_TEGRA186_CPUFREQ=y +CONFIG_ARM_TEGRA194_CPUFREQ=y CONFIG_QORIQ_CPUFREQ=y CONFIG_ARM_SCPI_PROTOCOL=y CONFIG_RASPBERRYPI_FIRMWARE=y -- 2.7.4
[TEGRA194_CPUFREQ Patch v3 0/4] Add cpufreq driver for Tegra194
The patch series adds cpufreq driver for Tegra194 SOC. v2[2] -> v3 - Set same policy for all cpus in a cluster[Viresh]. - Add compatible string for CPU Complex under cpus node[Thierry]. - Add reference to bpmp node under cpus node[Thierry]. - Bind cpufreq driver to CPU Complex compatible string[Thierry]. - Remove patch to get bpmp data as now using cpus node to get that[Thierry]. v1[1] -> v2: - Remove cpufreq_lock mutex from tegra194_cpufreq_set_target [Viresh]. - Remove CPUFREQ_ASYNC_NOTIFICATION flag [Viresh]. - Remove redundant _begin|end() call from tegra194_cpufreq_set_target. - Rename opp_table to freq_table [Viresh]. Sumit Gupta (4): dt-bindings: arm: Add t194 ccplex compatible and bpmp property arm64: tegra: Add t194 ccplex compatible and bpmp property cpufreq: Add Tegra194 cpufreq driver arm64: defconfig: Enable CONFIG_ARM_TEGRA194_CPUFREQ Documentation/devicetree/bindings/arm/cpus.yaml | 9 + arch/arm64/boot/dts/nvidia/tegra194.dtsi| 2 + arch/arm64/configs/defconfig| 1 + drivers/cpufreq/Kconfig.arm | 6 + drivers/cpufreq/Makefile| 1 + drivers/cpufreq/tegra194-cpufreq.c | 403 6 files changed, 422 insertions(+) create mode 100644 drivers/cpufreq/tegra194-cpufreq.c [1] https://marc.info/?t=15753945231=1=2 [2] https://marc.info/?l=linux-tegra=158602857106213=2 -- 2.7.4
Re: Re: [TEGRA194_CPUFREQ Patch 1/3] firmware: tegra: adding function to get BPMP data
On 27/04/20 12:48 PM, Thierry Reding wrote: On Tue, Apr 07, 2020 at 12:05:20PM +0200, Thierry Reding wrote: On Wed, Dec 04, 2019 at 03:21:38PM +0530, Viresh Kumar wrote: On 04-12-19, 10:33, Thierry Reding wrote: Yeah, the code that registers this device is in drivers/base/cpu.c in register_cpu(). It even retrieves the device tree node for the CPU from device tree and stores it in cpu->dev.of_node, so we should be able to just pass >dev to tegra_bpmp_get() in order to retrieve a reference to the BPMP. That said, I'm wondering if perhaps we could just add a compatible string to the /cpus node for cases like this where we don't have an actual device representing the CPU complex. There are a number of CPU frequency drivers that register dummy devices just so that they have something to bind a driver to. If we allow the /cpus node to represent the CPU complex (if no other "device" does that yet), we can add a compatible string and have the cpufreq driver match on that. Of course this would be slightly difficult to retrofit into existing drivers because they'd need to remain backwards compatible with existing device trees. But it would allow future drivers to do this a little more elegantly. For some SoCs this may not matter, but especially once you start depending on additional resources this would come in handy. Adding Rob and the device tree mailing list for feedback on this idea. Took some time to find this thread, but something around this was suggested by Rafael earlier. https://lore.kernel.org/lkml/8139001.q4ev8yg...@vostro.rjw.lan/ I gave this a try and came up with the following: --- >8 --- diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi index f4ede86e32b4..e4462f95f0b3 100644 --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi @@ -1764,6 +1764,9 @@ bpmp_thermal: thermal { }; cpus { + compatible = "nvidia,tegra194-ccplex"; + nvidia,bpmp = <>; + #address-cells = <1>; #size-cells = <0>; --- >8 --- Now I can do something rougly like this, although I have a more complete patch locally that also gets rid of all the global variables because we now actually have a struct platform_device that we can anchor everything at: --- >8 --- static const struct of_device_id tegra194_cpufreq_of_match[] = { { .compatible = "nvidia,tegra194-ccplex", }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, tegra194_cpufreq_of_match); static struct platform_driver tegra194_ccplex_driver = { .driver = { .name = "tegra194-cpufreq", .of_match_table = tegra194_cpufreq_of_match, }, .probe = tegra194_cpufreq_probe, .remove = tegra194_cpufreq_remove, }; module_platform_driver(tegra194_ccplex_driver); --- >8 --- I don't think that's exactly what Rafael (Cc'ed) had in mind, since the above thread seems to have mostly talked about binding a driver to each individual CPU. But this seems a lot better than having to instantiate a device from scratch just so that a driver can bind to it and it allows additional properties to be associated with the CCPLEX device. Rob, any thoughts on this from a device tree point of view? The /cpus bindings don't mention the compatible property, but there doesn't seem to be anything in the bindings that would prohibit its use. If we can agree on that, I can forward my local changes to Sumit for inclusion or reference. Rob, do you see any reason why we shouldn't be able to use a compatible string in the /cpus node for devices such as Tegra194 where there is no dedicated hardware block for the CCPLEX? Thierry Ping. Please suggest if we can add compatible string in the '/cpus' node. If not then i propose accepting the current patch to get BPMP data without adding any property in respective driver's DT node. We can push separate patch later if we need to add compatible string in the '/cpus' node or create new DT node for cpufreq. Regards, Sumit
[Patch V4] v4l2-core: fix use-after-free error
From: sumitg Fixing use-after-free within __v4l2_ctrl_handler_setup(). Memory is being freed with kfree(new_ref) for duplicate control reference entry but ctrl->cluster pointer is still referring to freed duplicate entry resulting in error on access. Change done to update cluster pointer only when new control reference is added. == BUG: KASAN: use-after-free in __v4l2_ctrl_handler_setup+0x388/0x428 Read of size 8 at addr ffc324e78618 by task systemd-udevd/312 Allocated by task 312: Freed by task 312: The buggy address belongs to the object at ffc324e78600 which belongs to the cache kmalloc-64 of size 64 The buggy address is located 24 bytes inside of 64-byte region [ffc324e78600, ffc324e78640) The buggy address belongs to the page: page:ffbf0c939e00 count:1 mapcount:0 mapping: (null) index:0xffc324e78f80 flags: 0x4100(slab) raw: 4100 ffc324e78f80 00018020001a raw: 00010001 ffc37040fb80 page dumped because: kasan: bad access detected Memory state around the buggy address: ffc324e78500: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc ffc324e78580: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc >ffc324e78600: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc ^ ffc324e78680: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc ffc324e78700: 00 00 00 00 00 fc fc fc fc fc fc fc fc fc fc fc == Suggested-by: Hans Verkuil Signed-off-by: Sumit Gupta --- v4: * update ctrl->cluster only when new control reference is added. v3: * update ctrl->cluster only when new control reference is added. * add new ctrl to handler only if the cluster points to an entry. v2: * update ctrl->cluster only when new control reference is added. * check ctrl->ncontrols to avoid illegal access when cluster has zero controls. drivers/media/v4l2-core/v4l2-ctrls.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index 5e3806f..956522c 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -2154,15 +2154,6 @@ static int handler_new_ref(struct v4l2_ctrl_handler *hdl, if (size_extra_req) new_ref->p_req.p = _ref[1]; - if (ctrl->handler == hdl) { - /* By default each control starts in a cluster of its own. - new_ref->ctrl is basically a cluster array with one - element, so that's perfect to use as the cluster pointer. - But only do this for the handler that owns the control. */ - ctrl->cluster = _ref->ctrl; - ctrl->ncontrols = 1; - } - INIT_LIST_HEAD(_ref->node); mutex_lock(hdl->lock); @@ -2195,6 +2186,15 @@ static int handler_new_ref(struct v4l2_ctrl_handler *hdl, hdl->buckets[bucket] = new_ref; if (ctrl_ref) *ctrl_ref = new_ref; + if (ctrl->handler == hdl) { + /* By default each control starts in a cluster of its own. +* new_ref->ctrl is basically a cluster array with one +* element, so that's perfect to use as the cluster pointer. +* But only do this for the handler that owns the control. +*/ + ctrl->cluster = _ref->ctrl; + ctrl->ncontrols = 1; + } unlock: mutex_unlock(hdl->lock); -- 2.7.4
[Patch V3] v4l2-core: fix use-after-free error
From: sumitg Fixing use-after-free within __v4l2_ctrl_handler_setup(). Memory is being freed with kfree(new_ref) for duplicate control reference entry but ctrl->cluster pointer is still referring to freed duplicate entry resulting in error on access. Change done to update cluster pointer only when new control reference is added. Also, added check to add new ctrl to handler only if the cluster points to an entry. == BUG: KASAN: use-after-free in __v4l2_ctrl_handler_setup+0x388/0x428 Read of size 8 at addr ffc324e78618 by task systemd-udevd/312 Allocated by task 312: Freed by task 312: The buggy address belongs to the object at ffc324e78600 which belongs to the cache kmalloc-64 of size 64 The buggy address is located 24 bytes inside of 64-byte region [ffc324e78600, ffc324e78640) The buggy address belongs to the page: page:ffbf0c939e00 count:1 mapcount:0 mapping: (null) index:0xffc324e78f80 flags: 0x4100(slab) raw: 4100 ffc324e78f80 00018020001a raw: 00010001 ffc37040fb80 page dumped because: kasan: bad access detected Memory state around the buggy address: ffc324e78500: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc ffc324e78580: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc >ffc324e78600: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc ^ ffc324e78680: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc ffc324e78700: 00 00 00 00 00 fc fc fc fc fc fc fc fc fc fc fc == Suggested-by: Hans Verkuil Signed-off-by: Sumit Gupta --- v3: * update ctrl->cluster only when new control reference is added. * add new ctrl to handler only if the cluster points to an entry. v2: * update ctrl->cluster only when new control reference is added. * check ctrl->ncontrols to avoid illegal access when cluster has zero controls. drivers/media/v4l2-core/v4l2-ctrls.c | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index 5e3806f..877c2ab 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -2154,15 +2154,6 @@ static int handler_new_ref(struct v4l2_ctrl_handler *hdl, if (size_extra_req) new_ref->p_req.p = _ref[1]; - if (ctrl->handler == hdl) { - /* By default each control starts in a cluster of its own. - new_ref->ctrl is basically a cluster array with one - element, so that's perfect to use as the cluster pointer. - But only do this for the handler that owns the control. */ - ctrl->cluster = _ref->ctrl; - ctrl->ncontrols = 1; - } - INIT_LIST_HEAD(_ref->node); mutex_lock(hdl->lock); @@ -2195,6 +2186,15 @@ static int handler_new_ref(struct v4l2_ctrl_handler *hdl, hdl->buckets[bucket] = new_ref; if (ctrl_ref) *ctrl_ref = new_ref; + if (ctrl->handler == hdl) { + /* By default each control starts in a cluster of its own. +* new_ref->ctrl is basically a cluster array with one +* element, so that's perfect to use as the cluster pointer. +* But only do this for the handler that owns the control. +*/ + ctrl->cluster = _ref->ctrl; + ctrl->ncontrols = 1; + } unlock: mutex_unlock(hdl->lock); @@ -2346,9 +2346,11 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, kvfree(ctrl); return NULL; } - mutex_lock(hdl->lock); - list_add_tail(>node, >ctrls); - mutex_unlock(hdl->lock); + if (ctrl->cluster) { + mutex_lock(hdl->lock); + list_add_tail(>node, >ctrls); + mutex_unlock(hdl->lock); + } return ctrl; } -- 2.7.4
RE: [PATCH ] drivers/base: cacheinfo: remove warning in resume
Hi Sudeep, Thank you for your comments. > > I understand the warning we get but the patch is completely wrong. > One it removes the feature of adding/removing the cache devices on cpu > hotplug events. Have you tested your patch with simple cpu hotplug and seen no > change before and after this change ? > I referred other node "cpufreq" under "cpu" node and that was also not getting deleted when core goes down. So, thought the behavior should be similar as entries are only used to read data and it won't change after boot. > On 29/08/16 08:20, Sumit Gupta wrote: > > CPU notifier is present for creating device entries for child node > > "cache" under parent node "cpu" as per DT. > > Again DT is only on few architectures not on all(e.g. x86) > > > During resume from > > suspend, while booting all non-boot CPU's, this notifier for adding > > cache device gets called before cpu device is added by device_resume. > > Because of this warning message of "parent should not be sleeping" > > comes during resume. > > > > Yes that's correct and needs to be fixed. I have seen this but haven't > spent much time to check in detail. It's harmless warning IMO. > See the comment in the code too:"This is a fib. But we'll allow new > children to be added below a resumed device, even if the device hasn't > been completed yet" > > CPU devices are special and they have separate hotplug paths. So we need > to consider that for cpu devices and set is_prepared quite early. > > > Removing the notifier to explicitly add/remove > > cache device as CPU and cache device get > > added/removed anyway as part of normal suspend > > resume sequence. > > dpm_resume_end - > dpm_resume -> device_resume > > > > Yes, but: > 1. the caches objects are visible even when the cpu is offline > 2. how is this handled for normal cpu-hotplug events ? This patch > breaks the existing feature. Compared with x86 now and there even cpufreq is getting deleted. As per the comments, right behavior seems to be where cache entries should be created during core on and removed during core off in hotplug. I will try to find other way of doing it. Also will see why cpufreq is not getting deleted as in x86. > > > Signed-off-by: Sumit Gupta <sum...@nvidia.com> > > --- > > drivers/base/cacheinfo.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c > > index e9fd32e91668..17d9c051a16f 100644 > > --- a/drivers/base/cacheinfo.c > > +++ b/drivers/base/cacheinfo.c > > @@ -544,8 +544,6 @@ static int __init cacheinfo_sysfs_init(void) > > goto out; > > } > > } > > - __hotcpu_notifier(cacheinfo_cpu_callback, 0); > > - > > out: > > cpu_notifier_register_done(); > > return rc; > > > > -- > Regards, > Sudeep
RE: [PATCH ] drivers/base: cacheinfo: remove warning in resume
Hi Sudeep, Thank you for your comments. > > I understand the warning we get but the patch is completely wrong. > One it removes the feature of adding/removing the cache devices on cpu > hotplug events. Have you tested your patch with simple cpu hotplug and seen no > change before and after this change ? > I referred other node "cpufreq" under "cpu" node and that was also not getting deleted when core goes down. So, thought the behavior should be similar as entries are only used to read data and it won't change after boot. > On 29/08/16 08:20, Sumit Gupta wrote: > > CPU notifier is present for creating device entries for child node > > "cache" under parent node "cpu" as per DT. > > Again DT is only on few architectures not on all(e.g. x86) > > > During resume from > > suspend, while booting all non-boot CPU's, this notifier for adding > > cache device gets called before cpu device is added by device_resume. > > Because of this warning message of "parent should not be sleeping" > > comes during resume. > > > > Yes that's correct and needs to be fixed. I have seen this but haven't > spent much time to check in detail. It's harmless warning IMO. > See the comment in the code too:"This is a fib. But we'll allow new > children to be added below a resumed device, even if the device hasn't > been completed yet" > > CPU devices are special and they have separate hotplug paths. So we need > to consider that for cpu devices and set is_prepared quite early. > > > Removing the notifier to explicitly add/remove > > cache device as CPU and cache device get > > added/removed anyway as part of normal suspend > > resume sequence. > > dpm_resume_end - > dpm_resume -> device_resume > > > > Yes, but: > 1. the caches objects are visible even when the cpu is offline > 2. how is this handled for normal cpu-hotplug events ? This patch > breaks the existing feature. Compared with x86 now and there even cpufreq is getting deleted. As per the comments, right behavior seems to be where cache entries should be created during core on and removed during core off in hotplug. I will try to find other way of doing it. Also will see why cpufreq is not getting deleted as in x86. > > > Signed-off-by: Sumit Gupta > > --- > > drivers/base/cacheinfo.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c > > index e9fd32e91668..17d9c051a16f 100644 > > --- a/drivers/base/cacheinfo.c > > +++ b/drivers/base/cacheinfo.c > > @@ -544,8 +544,6 @@ static int __init cacheinfo_sysfs_init(void) > > goto out; > > } > > } > > - __hotcpu_notifier(cacheinfo_cpu_callback, 0); > > - > > out: > > cpu_notifier_register_done(); > > return rc; > > > > -- > Regards, > Sudeep
RE: [PATCH ] arm64: cpuinfo: Add "model name" in /proc/cpuinfo for 64bit tasks also
> On Mon, Aug 29, 2016 at 02:32:25PM +0530, Sumit Gupta wrote: > > Removed restriction of displaying model name for 32 bit tasks only. > > Because of this Processor details were not displayed in "System > > setting -> Details" in Ubuntu model name display is generic and can be > > printed for 64 bit also. > > > > model name : ARMv8 Processor rev X (v8l) > > > > Signed-off-by: Sumit Gupta <sum...@nvidia.com> > > You didn't give a reason why this is needed. For 32-bit tasks, we did it for > backwards compatibility with code checking for it. > Within Ubuntu Home Screen "System setting -> details", processor details were coming as null. It seems those details are retrieved from "model name" in /proc/cpuinfo and because of check for 32 bit tasks only, "model name" was not getting displayed. On removing this check for model name, processor details are getting displayed in Ubuntu Home Screen. I think model name field is general and can be displayed irrespective of compatibility. Please suggest if I am missing anything. > -- > Catalin
RE: [PATCH ] arm64: cpuinfo: Add "model name" in /proc/cpuinfo for 64bit tasks also
> On Mon, Aug 29, 2016 at 02:32:25PM +0530, Sumit Gupta wrote: > > Removed restriction of displaying model name for 32 bit tasks only. > > Because of this Processor details were not displayed in "System > > setting -> Details" in Ubuntu model name display is generic and can be > > printed for 64 bit also. > > > > model name : ARMv8 Processor rev X (v8l) > > > > Signed-off-by: Sumit Gupta > > You didn't give a reason why this is needed. For 32-bit tasks, we did it for > backwards compatibility with code checking for it. > Within Ubuntu Home Screen "System setting -> details", processor details were coming as null. It seems those details are retrieved from "model name" in /proc/cpuinfo and because of check for 32 bit tasks only, "model name" was not getting displayed. On removing this check for model name, processor details are getting displayed in Ubuntu Home Screen. I think model name field is general and can be displayed irrespective of compatibility. Please suggest if I am missing anything. > -- > Catalin
[PATCH ] arm64: cpuinfo: Add "model name" in /proc/cpuinfo for 64bit tasks also
Removed restriction of displaying model name for 32 bit tasks only. Because of this Processor details were not displayed in "System setting -> Details" in Ubuntu model name display is generic and can be printed for 64 bit also. model name : ARMv8 Processor rev X (v8l) Signed-off-by: Sumit Gupta <sum...@nvidia.com> --- arch/arm64/kernel/cpuinfo.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c index ed1b84fe6925..13224f533ddb 100644 --- a/arch/arm64/kernel/cpuinfo.c +++ b/arch/arm64/kernel/cpuinfo.c @@ -118,8 +118,7 @@ static int c_show(struct seq_file *m, void *v) * "processor". Give glibc what it expects. */ seq_printf(m, "processor\t: %d\n", i); - if (compat) - seq_printf(m, "model name\t: ARMv8 Processor rev %d (%s)\n", + seq_printf(m, "model name\t: ARMv8 Processor rev %d (%s)\n", MIDR_REVISION(midr), COMPAT_ELF_PLATFORM); seq_printf(m, "BogoMIPS\t: %lu.%02lu\n", -- 2.1.4
[PATCH ] arm64: cpuinfo: Add "model name" in /proc/cpuinfo for 64bit tasks also
Removed restriction of displaying model name for 32 bit tasks only. Because of this Processor details were not displayed in "System setting -> Details" in Ubuntu model name display is generic and can be printed for 64 bit also. model name : ARMv8 Processor rev X (v8l) Signed-off-by: Sumit Gupta --- arch/arm64/kernel/cpuinfo.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c index ed1b84fe6925..13224f533ddb 100644 --- a/arch/arm64/kernel/cpuinfo.c +++ b/arch/arm64/kernel/cpuinfo.c @@ -118,8 +118,7 @@ static int c_show(struct seq_file *m, void *v) * "processor". Give glibc what it expects. */ seq_printf(m, "processor\t: %d\n", i); - if (compat) - seq_printf(m, "model name\t: ARMv8 Processor rev %d (%s)\n", + seq_printf(m, "model name\t: ARMv8 Processor rev %d (%s)\n", MIDR_REVISION(midr), COMPAT_ELF_PLATFORM); seq_printf(m, "BogoMIPS\t: %lu.%02lu\n", -- 2.1.4
[PATCH ] drivers/base: cacheinfo: remove warning in resume
CPU notifier is present for creating device entries for child node "cache" under parent node "cpu" as per DT. During resume from suspend, while booting all non-boot CPU's, this notifier for adding cache device gets called before cpu device is added by device_resume. Because of this warning message of "parent should not be sleeping" comes during resume. Removing the notifier to explicitly add/remove cache device as CPU and cache device get added/removed anyway as part of normal suspend resume sequence. dpm_resume_end - > dpm_resume -> device_resume Signed-off-by: Sumit Gupta <sum...@nvidia.com> --- drivers/base/cacheinfo.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c index e9fd32e91668..17d9c051a16f 100644 --- a/drivers/base/cacheinfo.c +++ b/drivers/base/cacheinfo.c @@ -544,8 +544,6 @@ static int __init cacheinfo_sysfs_init(void) goto out; } } - __hotcpu_notifier(cacheinfo_cpu_callback, 0); - out: cpu_notifier_register_done(); return rc; -- 2.1.4
[PATCH ] drivers/base: cacheinfo: remove warning in resume
CPU notifier is present for creating device entries for child node "cache" under parent node "cpu" as per DT. During resume from suspend, while booting all non-boot CPU's, this notifier for adding cache device gets called before cpu device is added by device_resume. Because of this warning message of "parent should not be sleeping" comes during resume. Removing the notifier to explicitly add/remove cache device as CPU and cache device get added/removed anyway as part of normal suspend resume sequence. dpm_resume_end - > dpm_resume -> device_resume Signed-off-by: Sumit Gupta --- drivers/base/cacheinfo.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c index e9fd32e91668..17d9c051a16f 100644 --- a/drivers/base/cacheinfo.c +++ b/drivers/base/cacheinfo.c @@ -544,8 +544,6 @@ static int __init cacheinfo_sysfs_init(void) goto out; } } - __hotcpu_notifier(cacheinfo_cpu_callback, 0); - out: cpu_notifier_register_done(); return rc; -- 2.1.4
Query about merging memblock and bootmem into one new alloc
Hi All, For ARM Linux, during booting first memblock reserves memory regions then bootmem allocator create node, mem_map, page bitmap data and then hands over to buddy. I have been thinking from some time about why we need two different allocators for this. Can we merge both into one(memblock into bootmem) or create a new allocator which can speed up the same thing which is easy to enhance in future. I am not sure about this and whether it's good idea or will it be fruitful. Please suggest and share your opinion. Thank you in advance for your help. Regards, Sumit Gupta -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Query about merging memblock and bootmem into one new alloc
Hi All, For ARM Linux, during booting first memblock reserves memory regions then bootmem allocator create node, mem_map, page bitmap data and then hands over to buddy. I have been thinking from some time about why we need two different allocators for this. Can we merge both into one(memblock into bootmem) or create a new allocator which can speed up the same thing which is easy to enhance in future. I am not sure about this and whether it's good idea or will it be fruitful. Please suggest and share your opinion. Thank you in advance for your help. Regards, Sumit Gupta -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
MM: Query about different memory types(mem_types)__mmu.c
Hi All, I have been exploring ARM reference manual about ARM weak memory model and mmu page table setting from some time. I think i understand different memory types, mmu settings for page/section, TEX, AP, B, C, S bits well. My target is to to dig further and fully understand setting of all parameters for different memory types in ARM [File mmu.c: "static struct mem_type mem_types"]. But i am not able to find any good source to refer for fully understanding all below parameters. Could you please help me to understand mappings for below mem types. If you can point me to some references which can help me understand more. Thank you in advance for your help. [MT_DEVICE] = { /* Strongly ordered / ARMv6 shared device */ .prot_pte = PROT_PTE_DEVICE | L_PTE_MT_DEV_SHARED | L_PTE_SHARED, .prot_pte_s2= s2_policy(PROT_PTE_S2_DEVICE) | s2_policy(L_PTE_S2_MT_DEV_SHARED) | L_PTE_SHARED, .prot_l1= PMD_TYPE_TABLE, .prot_sect = PROT_SECT_DEVICE | PMD_SECT_S, .domain = DOMAIN_IO, }, [MT_MEMORY_RW] = { .prot_pte = L_PTE_PRESENT | L_PTE_YOUNG | L_PTE_DIRTY | L_PTE_XN, .prot_l1 = PMD_TYPE_TABLE, .prot_sect = PMD_TYPE_SECT | PMD_SECT_AP_WRITE, .domain= DOMAIN_KERNEL, }, [MT_MEMORY_DMA_READY] = { .prot_pte = L_PTE_PRESENT | L_PTE_YOUNG | L_PTE_DIRTY | L_PTE_XN, .prot_l1 = PMD_TYPE_TABLE, .domain= DOMAIN_KERNEL, }, Regards, Sumit Gupta -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
MM: Query about different memory types(mem_types)__mmu.c
Hi All, I have been exploring ARM reference manual about ARM weak memory model and mmu page table setting from some time. I think i understand different memory types, mmu settings for page/section, TEX, AP, B, C, S bits well. My target is to to dig further and fully understand setting of all parameters for different memory types in ARM [File mmu.c: static struct mem_type mem_types]. But i am not able to find any good source to refer for fully understanding all below parameters. Could you please help me to understand mappings for below mem types. If you can point me to some references which can help me understand more. Thank you in advance for your help. [MT_DEVICE] = { /* Strongly ordered / ARMv6 shared device */ .prot_pte = PROT_PTE_DEVICE | L_PTE_MT_DEV_SHARED | L_PTE_SHARED, .prot_pte_s2= s2_policy(PROT_PTE_S2_DEVICE) | s2_policy(L_PTE_S2_MT_DEV_SHARED) | L_PTE_SHARED, .prot_l1= PMD_TYPE_TABLE, .prot_sect = PROT_SECT_DEVICE | PMD_SECT_S, .domain = DOMAIN_IO, }, [MT_MEMORY_RW] = { .prot_pte = L_PTE_PRESENT | L_PTE_YOUNG | L_PTE_DIRTY | L_PTE_XN, .prot_l1 = PMD_TYPE_TABLE, .prot_sect = PMD_TYPE_SECT | PMD_SECT_AP_WRITE, .domain= DOMAIN_KERNEL, }, [MT_MEMORY_DMA_READY] = { .prot_pte = L_PTE_PRESENT | L_PTE_YOUNG | L_PTE_DIRTY | L_PTE_XN, .prot_l1 = PMD_TYPE_TABLE, .domain= DOMAIN_KERNEL, }, Regards, Sumit Gupta -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/