From da55b1ad4b29b6ab44d5dc8ea8306260246d2699 Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Mon, 3 Jul 2017 07:59:26 -0500 Subject: PM / devfreq: rk3399_dmc: fix error return code in rk3399_dmcfreq_probe() platform_get_irq() returns an error code, but the rk3399_dmc driver ignores it and always returns -EINVAL. This is not correct, and prevents -EPROBE_DEFER from being propagated properly. Notice that platform_get_irq() no longer returns 0 on error: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e330b9a6bb35dc7097a4f02cb1ae7b6f96df92af Print and propagate the return value of platform_get_irq on failure. Reviewed-by: Chanwoo Choi Signed-off-by: Gustavo A. R. Silva Signed-off-by: MyungJoo Ham --- drivers/devfreq/rk3399_dmc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c index 40a2499730fc..1b89ebbad02c 100644 --- a/drivers/devfreq/rk3399_dmc.c +++ b/drivers/devfreq/rk3399_dmc.c @@ -336,8 +336,9 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev) irq = platform_get_irq(pdev, 0); if (irq < 0) { - dev_err(&pdev->dev, "Cannot get the dmc interrupt resource\n"); - return -EINVAL; + dev_err(&pdev->dev, + "Cannot get the dmc interrupt resource: %d\n", irq); + return irq; } data = devm_kzalloc(dev, sizeof(struct rk3399_dmcfreq), GFP_KERNEL); if (!data) -- cgit v1.2.3-59-g8ed1b From 9e578b37505018622dfafc40eed7cd78ff2af221 Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Mon, 3 Jul 2017 07:47:38 -0500 Subject: PM / devfreq: tegra: fix error return code in tegra_devfreq_probe() platform_get_irq() returns an error code, but the tegra-devfreq driver ignores it and always returns -ENODEV. This is not correct, and prevents -EPROBE_DEFER from being propagated properly. Notice that platform_get_irq() no longer returns 0 on error: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e330b9a6bb35dc7097a4f02cb1ae7b6f96df92af Print and propagate the return value of platform_get_irq on failure. Reviewed-by: Chanwoo Choi Signed-off-by: Gustavo A. R. Silva Signed-off-by: MyungJoo Ham --- drivers/devfreq/tegra-devfreq.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/devfreq/tegra-devfreq.c b/drivers/devfreq/tegra-devfreq.c index 214fff96fa4a..ae712159246f 100644 --- a/drivers/devfreq/tegra-devfreq.c +++ b/drivers/devfreq/tegra-devfreq.c @@ -688,9 +688,9 @@ static int tegra_devfreq_probe(struct platform_device *pdev) } irq = platform_get_irq(pdev, 0); - if (irq <= 0) { - dev_err(&pdev->dev, "Failed to get IRQ\n"); - return -ENODEV; + if (irq < 0) { + dev_err(&pdev->dev, "Failed to get IRQ: %d\n", irq); + return irq; } platform_set_drvdata(pdev, tegra); -- cgit v1.2.3-59-g8ed1b From 37d644aa01c092686af8a96f34df93c74e55b480 Mon Sep 17 00:00:00 2001 From: Arvind Yadav Date: Mon, 3 Jul 2017 15:40:04 +0530 Subject: PM / devfreq: constify attribute_group structures. attribute_groups are not supposed to change at runtime. All functions working with attribute_groups provided by work with const attribute_group. So mark the non-const structs as const. File size before: text data bss dec hex filename 621 176 0 797 31d drivers/devfreq/governor_userspace.o File size After adding 'const': text data bss dec hex filename 670 144 0 814 32e drivers/devfreq/governor_userspace.o Signed-off-by: Arvind Yadav Reviewed-by: Chanwoo Choi Signed-off-by: MyungJoo Ham --- drivers/devfreq/governor_userspace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c index 176976068bcd..77028c27593c 100644 --- a/drivers/devfreq/governor_userspace.c +++ b/drivers/devfreq/governor_userspace.c @@ -86,7 +86,7 @@ static struct attribute *dev_entries[] = { &dev_attr_set_freq.attr, NULL, }; -static struct attribute_group dev_attr_group = { +static const struct attribute_group dev_attr_group = { .name = "userspace", .attrs = dev_entries, }; -- cgit v1.2.3-59-g8ed1b From d4436c0dba8d4d780588179a2e192a867d266a10 Mon Sep 17 00:00:00 2001 From: Srinivas Pandruvada Date: Mon, 10 Jul 2017 16:23:52 -0700 Subject: cpufreq: intel_pstate: Fix ratio setting for min_perf_pct When the minimum performance limit percentage is set to the power-up default, it is possible that minimum performance ratio is off by one. In the set_policy() callback the minimum ratio is calculated by applying global.min_perf_pct to turbo_ratio and rounding up, but the power-up default global.min_perf_pct is already rounded up to the next percent in min_perf_pct_min(). That results in two round up operations, so for the default min_perf_pct one of them is not required. It is better to remove rounding up in min_perf_pct_min() as this matches the displayed min_perf_pct prior to commit c5a2ee7dde89 (cpufreq: intel_pstate: Active mode P-state limits rework) in 4.12. For example on a platform with max turbo ratio of 37 and minimum ratio of 10, the min_perf_pct resulted in 28 with the above commit. Before this commit it was 27 and it will be the same after this change. Fixes: 1a4fe38add8b (cpufreq: intel_pstate: Remove max/min fractions to limit performance) Reported-by: Artem Bityutskiy Signed-off-by: Srinivas Pandruvada Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/intel_pstate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index 756483251832..2386d7036e90 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -572,7 +572,7 @@ static int min_perf_pct_min(void) int turbo_pstate = cpu->pstate.turbo_pstate; return turbo_pstate ? - DIV_ROUND_UP(cpu->pstate.min_pstate * 100, turbo_pstate) : 0; + (cpu->pstate.min_pstate * 100 / turbo_pstate) : 0; } static s16 intel_pstate_get_epb(struct cpudata *cpu_data) -- cgit v1.2.3-59-g8ed1b From 2ca30331c156ca9e97643ad05dd8930b8fe78b01 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Mon, 10 Jul 2017 10:21:40 +0300 Subject: PM / QoS: return -EINVAL for bogus strings In the current code, if the user accidentally writes a bogus command to this sysfs file, then we set the latency tolerance to an uninitialized variable. Fixes: 2d984ad132a8 (PM / QoS: Introcuce latency tolerance device PM QoS type) Signed-off-by: Dan Carpenter Acked-by: Pavel Machek Cc: 3.15+ # 3.15+ Signed-off-by: Rafael J. Wysocki --- drivers/base/power/sysfs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c index 185a52581cfa..156ab57bca77 100644 --- a/drivers/base/power/sysfs.c +++ b/drivers/base/power/sysfs.c @@ -272,6 +272,8 @@ static ssize_t pm_qos_latency_tolerance_store(struct device *dev, value = PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT; else if (!strcmp(buf, "any") || !strcmp(buf, "any\n")) value = PM_QOS_LATENCY_ANY; + else + return -EINVAL; } ret = dev_pm_qos_update_user_latency_tolerance(dev, value); return ret < 0 ? ret : n; -- cgit v1.2.3-59-g8ed1b From ab2f7cf141aa6734c4ca7525132d8cc236efee77 Mon Sep 17 00:00:00 2001 From: Vikram Mulukutla Date: Thu, 6 Jul 2017 10:53:20 -0700 Subject: cpufreq: schedutil: Fix sugov_start() versus sugov_update_shared() race With a shared policy in place, when one of the CPUs in the policy is hotplugged out and then brought back online, sugov_stop() and sugov_start() are called in order. sugov_stop() removes utilization hooks for each CPU in the policy and does nothing else in the for_each_cpu() loop. sugov_start() on the other hand iterates through the CPUs in the policy and re-initializes the per-cpu structure _and_ adds the utilization hook. This implies that the scheduler is allowed to invoke a CPU's utilization update hook when the rest of the per-cpu structures have yet to be re-inited. Apart from some strange values in tracepoints this doesn't cause a problem, but if we do end up accessing a pointer from the per-cpu sugov_cpu structure somewhere in the sugov_update_shared() path, we will likely see crashes since the memset for another CPU in the policy is free to race with sugov_update_shared from the CPU that is ready to go. So let's fix this now to first init all per-cpu structures, and then add the per-cpu utilization update hooks all at once. Signed-off-by: Vikram Mulukutla Acked-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- kernel/sched/cpufreq_schedutil.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c index 076a2e31951c..29a397067ffa 100644 --- a/kernel/sched/cpufreq_schedutil.c +++ b/kernel/sched/cpufreq_schedutil.c @@ -610,6 +610,11 @@ static int sugov_start(struct cpufreq_policy *policy) sg_cpu->sg_policy = sg_policy; sg_cpu->flags = SCHED_CPUFREQ_RT; sg_cpu->iowait_boost_max = policy->cpuinfo.max_freq; + } + + for_each_cpu(cpu, policy->cpus) { + struct sugov_cpu *sg_cpu = &per_cpu(sugov_cpu, cpu); + cpufreq_add_update_util_hook(cpu, &sg_cpu->update_util, policy_is_shared(policy) ? sugov_update_shared : -- cgit v1.2.3-59-g8ed1b From 0ce3fcaff92908c370334ce3b9111aeea71159d6 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 12 Jul 2017 03:05:39 +0200 Subject: PCI / PM: Restore PME Enable after config space restoration Commit dc15e71eefc7 (PCI / PM: Restore PME Enable if skipping wakeup setup) introduced a mechanism by which the PME Enable bit can be restored by pci_enable_wake() if dev->wakeup_prepared is set in case it has been overwritten by PCI config space restoration. However, that commit overlooked the fact that on some systems (Dell XPS13 9360 in particular) the AML handling wakeup events checks PME Status and PME Enable and it won't trigger a Notify() for devices where those bits are not set while it is running. That happens during resume from suspend-to-idle when pci_restore_state() invoked by pci_pm_default_resume_early() clears PME Enable before the wakeup events are processed by AML, effectively causing those wakeup events to be ignored. Fix this issue by restoring the PME Enable configuration right after pci_restore_state() has been called instead of doing that in pci_enable_wake(). Fixes: dc15e71eefc7 (PCI / PM: Restore PME Enable if skipping wakeup setup) Signed-off-by: Rafael J. Wysocki Acked-by: Bjorn Helgaas --- drivers/pci/pci-driver.c | 2 ++ drivers/pci/pci.c | 16 ++++++++-------- drivers/pci/pci.h | 1 + 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index ffe7d54d9328..1a93b48b0dd9 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -506,6 +506,7 @@ static int pci_restore_standard_config(struct pci_dev *pci_dev) } pci_restore_state(pci_dev); + pci_pme_restore(pci_dev); return 0; } @@ -517,6 +518,7 @@ static void pci_pm_default_resume_early(struct pci_dev *pci_dev) { pci_power_up(pci_dev); pci_restore_state(pci_dev); + pci_pme_restore(pci_dev); pci_fixup_device(pci_fixup_resume_early, pci_dev); } diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 0b5302a9fdae..d1443a161b35 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1798,7 +1798,11 @@ static void __pci_pme_active(struct pci_dev *dev, bool enable) pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr); } -static void pci_pme_restore(struct pci_dev *dev) +/** + * pci_pme_restore - Restore PME configuration after config space restore. + * @dev: PCI device to update. + */ +void pci_pme_restore(struct pci_dev *dev) { u16 pmcsr; @@ -1808,6 +1812,7 @@ static void pci_pme_restore(struct pci_dev *dev) pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); if (dev->wakeup_prepared) { pmcsr |= PCI_PM_CTRL_PME_ENABLE; + pmcsr &= ~PCI_PM_CTRL_PME_STATUS; } else { pmcsr &= ~PCI_PM_CTRL_PME_ENABLE; pmcsr |= PCI_PM_CTRL_PME_STATUS; @@ -1904,14 +1909,9 @@ int pci_enable_wake(struct pci_dev *dev, pci_power_t state, bool enable) { int ret = 0; - /* - * Don't do the same thing twice in a row for one device, but restore - * PME Enable in case it has been updated by config space restoration. - */ - if (!!enable == !!dev->wakeup_prepared) { - pci_pme_restore(dev); + /* Don't do the same thing twice in a row for one device. */ + if (!!enable == !!dev->wakeup_prepared) return 0; - } /* * According to "PCI System Architecture" 4th ed. by Tom Shanley & Don diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 240b2c0fed4b..0a6e737f5325 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -71,6 +71,7 @@ void pci_power_up(struct pci_dev *dev); void pci_disable_enabled_device(struct pci_dev *dev); int pci_finish_runtime_suspend(struct pci_dev *dev); int __pci_pme_wakeup(struct pci_dev *dev, void *ign); +void pci_pme_restore(struct pci_dev *dev); bool pci_dev_keep_suspended(struct pci_dev *dev); void pci_dev_complete_resume(struct pci_dev *pci_dev); void pci_config_pm_runtime_get(struct pci_dev *dev); -- cgit v1.2.3-59-g8ed1b From c7b5a4e6e8fbef8fdf2c529f953000d24c561339 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 6 Jul 2017 02:32:02 +0200 Subject: PCI / PM: Fix native PME handling during system suspend/resume Commit 76cde7e49590 (PCI / PM: Make PCIe PME interrupts wake up from suspend-to-idle) went too far with preventing pcie_pme_work_fn() from clearing the root port's PME Status and re-enabling the PME interrupt which should be done for PMEs to work correctly after system resume. The failing scenario is as follows: 1. pcie_pme_suspend() finds that the PME IRQ should be designated for system wakeup, so it calls enable_irq_wake() and then sets data->suspend_level to PME_SUSPEND_WAKEUP. 2. PME interrupt happens at this point. 3. pcie_pme_irq() runs, disables the PME interrupt and queues up the execution of pcie_pme_work_fn(). 4. pcie_pme_work_fn() runs before pcie_pme_resume() and breaks out of the loop right away, because data->suspend_level is not PME_SUSPEND_NONE, and it doesn't re-enable the PME interrupt for the same reason. 5. pcie_pme_resume() runs and simply calls disable_irq_wake() without re-enabling the PME interrupt (because data->suspend_level is not PME_SUSPEND_NONE), so the PME interrupt remains disabled and the PME Status remains set. To fix this notice that there is no reason why pcie_pme_work_fn() should behave in a special way during system resume if the PME interrupt is not disabled by pcie_pme_suspend() and partially revert commit 76cde7e49590 and restore the previous (and correct) behavior of pcie_pme_work_fn(). Fixes: 76cde7e49590 (PCI / PM: Make PCIe PME interrupts wake up from suspend-to-idle) Reported-and-tested-by: Naresh Solanki Signed-off-by: Rafael J. Wysocki Acked-by: Bjorn Helgaas --- drivers/pci/pcie/pme.c | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c index 80e58d25006d..fafdb165dd2e 100644 --- a/drivers/pci/pcie/pme.c +++ b/drivers/pci/pcie/pme.c @@ -40,17 +40,11 @@ static int __init pcie_pme_setup(char *str) } __setup("pcie_pme=", pcie_pme_setup); -enum pme_suspend_level { - PME_SUSPEND_NONE = 0, - PME_SUSPEND_WAKEUP, - PME_SUSPEND_NOIRQ, -}; - struct pcie_pme_service_data { spinlock_t lock; struct pcie_device *srv; struct work_struct work; - enum pme_suspend_level suspend_level; + bool noirq; /* If set, keep the PME interrupt disabled. */ }; /** @@ -228,7 +222,7 @@ static void pcie_pme_work_fn(struct work_struct *work) spin_lock_irq(&data->lock); for (;;) { - if (data->suspend_level != PME_SUSPEND_NONE) + if (data->noirq) break; pcie_capability_read_dword(port, PCI_EXP_RTSTA, &rtsta); @@ -255,7 +249,7 @@ static void pcie_pme_work_fn(struct work_struct *work) spin_lock_irq(&data->lock); } - if (data->suspend_level == PME_SUSPEND_NONE) + if (!data->noirq) pcie_pme_interrupt_enable(port, true); spin_unlock_irq(&data->lock); @@ -378,7 +372,7 @@ static int pcie_pme_suspend(struct pcie_device *srv) { struct pcie_pme_service_data *data = get_service_data(srv); struct pci_dev *port = srv->port; - bool wakeup, wake_irq_enabled = false; + bool wakeup; int ret; if (device_may_wakeup(&port->dev)) { @@ -388,19 +382,16 @@ static int pcie_pme_suspend(struct pcie_device *srv) wakeup = pcie_pme_check_wakeup(port->subordinate); up_read(&pci_bus_sem); } - spin_lock_irq(&data->lock); if (wakeup) { ret = enable_irq_wake(srv->irq); - if (ret == 0) { - data->suspend_level = PME_SUSPEND_WAKEUP; - wake_irq_enabled = true; - } - } - if (!wake_irq_enabled) { - pcie_pme_interrupt_enable(port, false); - pcie_clear_root_pme_status(port); - data->suspend_level = PME_SUSPEND_NOIRQ; + if (!ret) + return 0; } + + spin_lock_irq(&data->lock); + pcie_pme_interrupt_enable(port, false); + pcie_clear_root_pme_status(port); + data->noirq = true; spin_unlock_irq(&data->lock); synchronize_irq(srv->irq); @@ -417,15 +408,15 @@ static int pcie_pme_resume(struct pcie_device *srv) struct pcie_pme_service_data *data = get_service_data(srv); spin_lock_irq(&data->lock); - if (data->suspend_level == PME_SUSPEND_NOIRQ) { + if (data->noirq) { struct pci_dev *port = srv->port; pcie_clear_root_pme_status(port); pcie_pme_interrupt_enable(port, true); + data->noirq = false; } else { disable_irq_wake(srv->irq); } - data->suspend_level = PME_SUSPEND_NONE; spin_unlock_irq(&data->lock); return 0; -- cgit v1.2.3-59-g8ed1b