On 12-01-21, 17:52, Shawn Guo wrote: > On SDM845/850, running the following commands to put all cores in > freq-domain1 offline and then get one core back online, there will be > a request region error seen from qcom-hw driver. > > $ echo 0 > /sys/devices/system/cpu/cpu4/online > $ echo 0 > /sys/devices/system/cpu/cpu5/online > $ echo 0 > /sys/devices/system/cpu/cpu6/online > $ echo 0 > /sys/devices/system/cpu/cpu7/online > $ echo 1 > /sys/devices/system/cpu/cpu4/online > > [ 3395.915416] CPU4: shutdown > [ 3395.938185] psci: CPU4 killed (polled 0 ms) > [ 3399.071424] CPU5: shutdown > [ 3399.094316] psci: CPU5 killed (polled 0 ms) > [ 3402.139358] CPU6: shutdown > [ 3402.161705] psci: CPU6 killed (polled 0 ms) > [ 3404.742939] CPU7: shutdown > [ 3404.765592] psci: CPU7 killed (polled 0 ms) > [ 3411.492274] Detected VIPT I-cache on CPU4 > [ 3411.492337] GICv3: CPU4: found redistributor 400 region 0:0x0000000017ae0000 > [ 3411.492448] CPU4: Booted secondary processor 0x0000000400 [0x516f802d] > [ 3411.503654] qcom-cpufreq-hw 17d43000.cpufreq: can't request region for resource [mem 0x17d45800-0x17d46bff] > > The cause is that the memory region requested in .init hook doesn't get > released in .exit hook, and the subsequent call to .init will always fail > on this error. Let's break down the devm_platform_ioremap_resource() > call a bit, so that we can have the resource pointer to release memory > region from .exit hook. > > Signed-off-by: Shawn Guo <shawn.guo@xxxxxxxxxx> > --- > drivers/cpufreq/qcom-cpufreq-hw.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) FWIW, Ionela also sent a fix though I like this one better for the obvious reasons. https://lore.kernel.org/lkml/20210108151406.23595-1-ionela.voinescu@xxxxxxx/ > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > index 9ed5341dc515..315ee987d2d3 100644 > --- a/drivers/cpufreq/qcom-cpufreq-hw.c > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > @@ -32,6 +32,7 @@ struct qcom_cpufreq_soc_data { > > struct qcom_cpufreq_data { > void __iomem *base; > + struct resource *res; > const struct qcom_cpufreq_soc_data *soc_data; > }; > > @@ -280,6 +281,7 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) > struct of_phandle_args args; > struct device_node *cpu_np; > struct device *cpu_dev; > + struct resource *res; > void __iomem *base; > struct qcom_cpufreq_data *data; > int ret, index; > @@ -303,7 +305,8 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) > > index = args.args[0]; > > - base = devm_platform_ioremap_resource(pdev, index); > + res = platform_get_resource(pdev, IORESOURCE_MEM, index); > + base = devm_ioremap_resource(dev, res); > if (IS_ERR(base)) > return PTR_ERR(base); > > @@ -315,6 +318,7 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) > > data->soc_data = of_device_get_match_data(&pdev->dev); > data->base = base; > + data->res = res; > > /* HW should be in enabled state to proceed */ > if (!(readl_relaxed(base + data->soc_data->reg_enable) & 0x1)) { > @@ -358,11 +362,13 @@ static int qcom_cpufreq_hw_cpu_exit(struct cpufreq_policy *policy) > struct device *cpu_dev = get_cpu_device(policy->cpu); > struct qcom_cpufreq_data *data = policy->driver_data; > struct platform_device *pdev = cpufreq_get_driver_data(); > + struct resource *res = data->res; > > dev_pm_opp_remove_all_dynamic(cpu_dev); > dev_pm_opp_of_cpumask_remove_table(policy->related_cpus); > kfree(policy->freq_table); > devm_iounmap(&pdev->dev, data->base); > + devm_release_mem_region(&pdev->dev, res->start, resource_size(res)); > > return 0; > } > -- > 2.17.1 -- viresh
