* [PATCH v3 10/22] PM / devfreq: tegra30: Don't enable consecutive-down interrupt on startup
From: Dmitry Osipenko @ 2019-06-27 21:11 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
The consecutive-down event tells that we should perform frequency
de-boosting, but boosting is in a reset state on start and hence the
event won't do anything useful for us and it will be just a dummy
interrupt request.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index cca2fc15bafe..65f0363dbf1b 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -557,7 +557,6 @@ static void tegra_actmon_configure_device(struct tegra_devfreq *tegra,
<< ACTMON_DEV_CTRL_CONSECUTIVE_ABOVE_WMARK_NUM_SHIFT;
val |= ACTMON_DEV_CTRL_AVG_ABOVE_WMARK_EN;
val |= ACTMON_DEV_CTRL_AVG_BELOW_WMARK_EN;
- val |= ACTMON_DEV_CTRL_CONSECUTIVE_BELOW_WMARK_EN;
val |= ACTMON_DEV_CTRL_CONSECUTIVE_ABOVE_WMARK_EN;
val |= ACTMON_DEV_CTRL_ENB;
--
2.22.0
^ permalink raw reply related
* [PATCH v3 12/22] PM / devfreq: tegra30: Inline all one-line functions
From: Dmitry Osipenko @ 2019-06-27 21:11 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
Depending on a kernel's configuration, a single line functions may not be
inlined by compiler (like enabled ftracing for example). Let's inline such
functions explicitly for consistency.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 47a47be276ee..ecbd58504cd8 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -181,28 +181,29 @@ static struct tegra_actmon_emc_ratio actmon_emc_ratios[] = {
{ 250000, 100000 },
};
-static u32 actmon_readl(struct tegra_devfreq *tegra, u32 offset)
+static inline u32 actmon_readl(struct tegra_devfreq *tegra, u32 offset)
{
return readl_relaxed(tegra->regs + offset);
}
-static void actmon_writel(struct tegra_devfreq *tegra, u32 val, u32 offset)
+static inline void actmon_writel(struct tegra_devfreq *tegra,
+ u32 val, u32 offset)
{
writel_relaxed(val, tegra->regs + offset);
}
-static u32 device_readl(struct tegra_devfreq_device *dev, u32 offset)
+static inline u32 device_readl(struct tegra_devfreq_device *dev, u32 offset)
{
return readl_relaxed(dev->regs + offset);
}
-static void device_writel(struct tegra_devfreq_device *dev, u32 val,
- u32 offset)
+static inline void device_writel(struct tegra_devfreq_device *dev,
+ u32 val, u32 offset)
{
writel_relaxed(val, dev->regs + offset);
}
-static unsigned long do_percent(unsigned long val, unsigned int pct)
+static inline unsigned long do_percent(unsigned long val, unsigned int pct)
{
return val * pct / 100;
}
--
2.22.0
^ permalink raw reply related
* [PATCH v3 11/22] PM / devfreq: tegra30: Add debug messages
From: Dmitry Osipenko @ 2019-06-27 21:11 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
Add debug messages to know about what's happening in hardware and how
driver reacts.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 35 +++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 65f0363dbf1b..47a47be276ee 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -41,6 +41,7 @@
#define ACTMON_DEV_AVG_UPPER_WMARK 0x10
#define ACTMON_DEV_AVG_LOWER_WMARK 0x14
#define ACTMON_DEV_COUNT_WEIGHT 0x18
+#define ACTMON_DEV_COUNT 0x1c
#define ACTMON_DEV_AVG_COUNT 0x20
#define ACTMON_DEV_INTR_STATUS 0x24
@@ -276,6 +277,9 @@ static void tegra_actmon_get_lower_upper(struct tegra_devfreq *tegra,
unsigned long *lower,
unsigned long *upper)
{
+ struct device *ddev = tegra->devfreq->dev.parent;
+ u32 offset = dev->config->offset;
+
/*
* Memory frequencies are guaranteed to have 1MHz granularity
* and thus we need this rounding down to get a proper watermarks
@@ -288,6 +292,9 @@ static void tegra_actmon_get_lower_upper(struct tegra_devfreq *tegra,
*lower = tegra_actmon_lower_freq(tegra, target_freq);
*upper = tegra_actmon_upper_freq(tegra, target_freq);
+ dev_dbg(ddev, "%03x: target_freq %lu lower freq %lu upper freq %lu\n",
+ offset, target_freq, *lower, *upper);
+
*lower /= KHZ;
*upper /= KHZ;
@@ -367,11 +374,31 @@ static void tegra_devfreq_update_wmark(struct tegra_devfreq *tegra,
device_writel(dev, lower + delta, ACTMON_DEV_LOWER_WMARK);
}
+static void actmon_device_debug(struct tegra_devfreq *tegra,
+ struct tegra_devfreq_device *dev,
+ const char *prefix)
+{
+ dev_dbg(tegra->devfreq->dev.parent,
+ "%03x: %s: 0x%08x 0x%08x a %u %u %u c %u %u %u b %lu cpu %u\n",
+ dev->config->offset, prefix,
+ device_readl(dev, ACTMON_DEV_INTR_STATUS),
+ device_readl(dev, ACTMON_DEV_CTRL),
+ device_readl(dev, ACTMON_DEV_AVG_COUNT),
+ device_readl(dev, ACTMON_DEV_AVG_LOWER_WMARK),
+ device_readl(dev, ACTMON_DEV_AVG_UPPER_WMARK),
+ device_readl(dev, ACTMON_DEV_COUNT),
+ device_readl(dev, ACTMON_DEV_LOWER_WMARK),
+ device_readl(dev, ACTMON_DEV_UPPER_WMARK),
+ dev->boost_freq, cpufreq_get(0));
+}
+
static void actmon_isr_device(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
u32 intr_status, dev_ctrl, avg_intr_mask;
+ actmon_device_debug(tegra, dev, "isr+");
+
dev->avg_count = device_readl(dev, ACTMON_DEV_AVG_COUNT);
intr_status = device_readl(dev, ACTMON_DEV_INTR_STATUS);
dev_ctrl = device_readl(dev, ACTMON_DEV_CTRL);
@@ -422,6 +449,8 @@ static void actmon_isr_device(struct tegra_devfreq *tegra,
device_writel(dev, dev_ctrl, ACTMON_DEV_CTRL);
device_writel(dev, ACTMON_INTR_STATUS_CLEAR, ACTMON_DEV_INTR_STATUS);
+
+ actmon_device_debug(tegra, dev, "isr-");
}
static unsigned long actmon_update_target(struct tegra_devfreq *tegra,
@@ -711,6 +740,7 @@ static struct devfreq_dev_profile tegra_devfreq_profile = {
static int tegra_governor_get_target(struct devfreq *devfreq,
unsigned long *freq)
{
+ struct device *ddev = devfreq->dev.parent;
struct devfreq_dev_status *stat;
struct tegra_devfreq *tegra;
struct tegra_devfreq_device *dev;
@@ -733,6 +763,11 @@ static int tegra_governor_get_target(struct devfreq *devfreq,
dev_target_freq = actmon_update_target(tegra, dev);
target_freq = max(target_freq, dev_target_freq);
+
+ dev_dbg(ddev, "%03x: upd: dev_target_freq %lu\n",
+ dev->config->offset, dev_target_freq);
+
+ actmon_device_debug(tegra, dev, "upd");
}
*freq = target_freq * KHZ;
--
2.22.0
^ permalink raw reply related
* [PATCH v3 14/22] PM / devfreq: tegra30: Ensure that target freq won't overflow
From: Dmitry Osipenko @ 2019-06-27 21:11 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
Potentially very high boosting could cause an integer overflow for a
highly clocked memory after conversion to MHz.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index d3e117d827d2..f86593ab3008 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -459,6 +459,7 @@ static unsigned long actmon_update_target(struct tegra_devfreq *tegra,
{ unsigned long target_freq;
target_freq = dev->avg_count / ACTMON_SAMPLING_PERIOD + dev->boost_freq;
+ target_freq = min(target_freq, ULONG_MAX / KHZ);
target_freq = tegra_actmon_account_cpu_freq(tegra, dev, target_freq);
return target_freq;
--
2.22.0
^ permalink raw reply related
* [PATCH v3 13/22] PM / devfreq: tegra30: Constify structs
From: Dmitry Osipenko @ 2019-06-27 21:11 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
Constify unmodifiable structs for consistency.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index ecbd58504cd8..d3e117d827d2 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -106,7 +106,7 @@ enum tegra_actmon_device {
MCCPU,
};
-static struct tegra_devfreq_device_config actmon_device_configs[] = {
+static const struct tegra_devfreq_device_config actmon_device_configs[] = {
{
/* MCALL: All memory accesses (including from the CPUs) */
.offset = 0x1c0,
@@ -171,7 +171,7 @@ struct tegra_actmon_emc_ratio {
unsigned long emc_freq;
};
-static struct tegra_actmon_emc_ratio actmon_emc_ratios[] = {
+static const struct tegra_actmon_emc_ratio actmon_emc_ratios[] = {
{ 1400000, ULONG_MAX },
{ 1200000, 750000 },
{ 1100000, 600000 },
@@ -210,7 +210,7 @@ static inline unsigned long do_percent(unsigned long val, unsigned int pct)
static unsigned long actmon_cpu_to_emc_rate(struct tegra_devfreq *tegra)
{
- struct tegra_actmon_emc_ratio *ratio = actmon_emc_ratios;
+ const struct tegra_actmon_emc_ratio *ratio = actmon_emc_ratios;
unsigned int cpu_freq = cpufreq_get(0);
unsigned int i;
--
2.22.0
^ permalink raw reply related
* [PATCH v3 15/22] PM / devfreq: tegra30: Fix integer overflow on CPU's freq max out
From: Dmitry Osipenko @ 2019-06-27 21:11 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
There is another kHz-conversion bug in the code, resulting in integer
overflow. Although, this time the resulting value is 4294966296 and it's
close to ULONG_MAX, which is okay in this case.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index f86593ab3008..9a6ede689991 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -172,7 +172,7 @@ struct tegra_actmon_emc_ratio {
};
static const struct tegra_actmon_emc_ratio actmon_emc_ratios[] = {
- { 1400000, ULONG_MAX },
+ { 1400000, ULONG_MAX / KHZ },
{ 1200000, 750000 },
{ 1100000, 600000 },
{ 1000000, 500000 },
--
2.22.0
^ permalink raw reply related
* [PATCH v3 18/22] PM / devfreq: tegra30: Optimize CPUFreq notifier
From: Dmitry Osipenko @ 2019-06-27 21:11 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
When CPU's memory activity is low or memory activity is high such that
CPU's frequency contribution to the boosting is not taken into account,
then there is no need to schedule devfreq's update. This eliminates
unnecessary CPU activity during of idling caused by the scheduled work.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 73 +++++++++++++++++++++++++++----
1 file changed, 64 insertions(+), 9 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index c1ab7af07daa..e8f7cc56a340 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -216,10 +216,10 @@ static inline unsigned long do_percent(unsigned long val, unsigned int pct)
return val * pct / 100;
}
-static unsigned long actmon_cpu_to_emc_rate(struct tegra_devfreq *tegra)
+static unsigned long actmon_cpu_to_emc_rate(struct tegra_devfreq *tegra,
+ unsigned int cpu_freq)
{
const struct tegra_actmon_emc_ratio *ratio = actmon_emc_ratios;
- unsigned int cpu_freq = cpufreq_get(0);
unsigned int i;
for (i = 0; i < ARRAY_SIZE(actmon_emc_ratios); i++, ratio++) {
@@ -239,15 +239,15 @@ tegra_actmon_account_cpu_freq(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev,
unsigned long target_freq)
{
- unsigned long static_cpu_emc_freq;
+ unsigned long cpu_emc_freq = 0;
- if (dev->config->avg_dependency_threshold &&
- dev->config->avg_dependency_threshold < dev->avg_freq) {
- static_cpu_emc_freq = actmon_cpu_to_emc_rate(tegra);
- target_freq = max(target_freq, static_cpu_emc_freq);
- }
+ if (!dev->config->avg_dependency_threshold)
+ return target_freq;
- return target_freq;
+ if (dev->avg_freq > dev->config->avg_dependency_threshold)
+ cpu_emc_freq = actmon_cpu_to_emc_rate(tegra, cpufreq_get(0));
+
+ return max(target_freq, cpu_emc_freq);
}
static unsigned long tegra_actmon_lower_freq(struct tegra_devfreq *tegra,
@@ -530,16 +530,71 @@ static void tegra_actmon_delayed_update(struct work_struct *work)
mutex_unlock(&tegra->devfreq->lock);
}
+static unsigned long
+tegra_actmon_cpufreq_contribution(struct tegra_devfreq *tegra,
+ unsigned int cpu_freq)
+{
+ unsigned long freq, static_cpu_emc_freq;
+
+ /* check whether CPU's freq is taken into account at all */
+ if (tegra->devices[MCCPU].avg_freq <=
+ tegra->devices[MCCPU].config->avg_dependency_threshold)
+ return 0;
+
+ static_cpu_emc_freq = actmon_cpu_to_emc_rate(tegra, cpu_freq);
+
+ /* compare static CPU-EMC freq with MCALL */
+ freq = tegra->devices[MCALL].avg_freq +
+ tegra->devices[MCALL].boost_freq;
+
+ freq = tegra_actmon_upper_freq(tegra, freq);
+
+ if (freq == tegra->max_freq || freq >= static_cpu_emc_freq)
+ return 0;
+
+ /* compare static CPU-EMC freq with MCCPU */
+ freq = tegra->devices[MCCPU].avg_freq +
+ tegra->devices[MCCPU].boost_freq;
+
+ freq = tegra_actmon_upper_freq(tegra, freq);
+
+ if (freq == tegra->max_freq || freq >= static_cpu_emc_freq)
+ return 0;
+
+ return static_cpu_emc_freq;
+}
+
static int tegra_actmon_cpu_notify_cb(struct notifier_block *nb,
unsigned long action, void *ptr)
{
+ struct cpufreq_freqs *freqs = ptr;
struct tegra_devfreq *tegra;
+ unsigned long old, new;
if (action != CPUFREQ_POSTCHANGE)
return NOTIFY_OK;
tegra = container_of(nb, struct tegra_devfreq, cpu_rate_change_nb);
+ /*
+ * Quickly check whether CPU frequency should be taken into account
+ * at all, without blocking CPUFreq's core.
+ */
+ if (mutex_trylock(&tegra->devfreq->lock)) {
+ old = tegra_actmon_cpufreq_contribution(tegra, freqs->old);
+ new = tegra_actmon_cpufreq_contribution(tegra, freqs->new);
+ mutex_unlock(&tegra->devfreq->lock);
+
+ /*
+ * If CPU's frequency shouldn't be taken into account at
+ * the moment, then there is no need to update the devfreq's
+ * state because ISR will re-check CPU's frequency on the
+ * next interrupt.
+ */
+ if (old == new)
+ return NOTIFY_OK;
+ }
+
/*
* CPUFreq driver should support CPUFREQ_ASYNC_NOTIFICATION in order
* to allow asynchronous notifications. This means we can't block
--
2.22.0
^ permalink raw reply related
* [PATCH v3 17/22] PM / devfreq: tegra30: Use tracepoints for debugging
From: Dmitry Osipenko @ 2019-06-27 21:11 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
Debug messages create too much CPU and memory activity by themselves,
so it's difficult to debug lower rates and catch unwanted interrupts
that happen rarely. Tracepoints are ideal in that regards because they
do not contribute to the sampled date at all. This allowed me to catch
few problems which are fixed by the followup patches, without tracepoints
it would be much harder to do.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 43 +++-------
include/trace/events/tegra30_devfreq.h | 105 +++++++++++++++++++++++++
2 files changed, 117 insertions(+), 31 deletions(-)
create mode 100644 include/trace/events/tegra30_devfreq.h
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 61d9601766fc..c1ab7af07daa 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -19,6 +19,9 @@
#include <linux/reset.h>
#include <linux/workqueue.h>
+#define CREATE_TRACE_POINTS
+#include <trace/events/tegra30_devfreq.h>
+
#include "governor.h"
#define ACTMON_GLB_STATUS 0x0
@@ -283,9 +286,6 @@ static void tegra_actmon_get_lower_upper(struct tegra_devfreq *tegra,
unsigned long *lower,
unsigned long *upper)
{
- struct device *ddev = tegra->devfreq->dev.parent;
- u32 offset = dev->config->offset;
-
/*
* Memory frequencies are guaranteed to have 1MHz granularity
* and thus we need this rounding down to get a proper watermarks
@@ -298,8 +298,8 @@ static void tegra_actmon_get_lower_upper(struct tegra_devfreq *tegra,
*lower = tegra_actmon_lower_freq(tegra, target_freq);
*upper = tegra_actmon_upper_freq(tegra, target_freq);
- dev_dbg(ddev, "%03x: target_freq %lu lower freq %lu upper freq %lu\n",
- offset, target_freq, *lower, *upper);
+ trace_device_lower_upper(dev->config->offset, target_freq,
+ *lower, *upper);
/*
* The upper watermark should take into account CPU's frequency
@@ -377,30 +377,13 @@ static void tegra_devfreq_update_wmark(struct tegra_devfreq *tegra,
device_writel(dev, lower + delta, ACTMON_DEV_LOWER_WMARK);
}
-static void actmon_device_debug(struct tegra_devfreq *tegra,
- struct tegra_devfreq_device *dev,
- const char *prefix)
-{
- dev_dbg(tegra->devfreq->dev.parent,
- "%03x: %s: 0x%08x 0x%08x a %u %u %u c %u %u %u b %lu cpu %u\n",
- dev->config->offset, prefix,
- device_readl(dev, ACTMON_DEV_INTR_STATUS),
- device_readl(dev, ACTMON_DEV_CTRL),
- device_readl(dev, ACTMON_DEV_AVG_COUNT),
- device_readl(dev, ACTMON_DEV_AVG_LOWER_WMARK),
- device_readl(dev, ACTMON_DEV_AVG_UPPER_WMARK),
- device_readl(dev, ACTMON_DEV_COUNT),
- device_readl(dev, ACTMON_DEV_LOWER_WMARK),
- device_readl(dev, ACTMON_DEV_UPPER_WMARK),
- dev->boost_freq, cpufreq_get(0));
-}
-
static void actmon_isr_device(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
u32 intr_status, dev_ctrl, avg_intr_mask, avg_count;
- actmon_device_debug(tegra, dev, "isr+");
+ trace_device_isr_enter(tegra->regs, dev->config->offset,
+ dev->boost_freq, cpufreq_get(0));
intr_status = device_readl(dev, ACTMON_DEV_INTR_STATUS);
avg_count = device_readl(dev, ACTMON_DEV_AVG_COUNT);
@@ -455,7 +438,8 @@ static void actmon_isr_device(struct tegra_devfreq *tegra,
device_writel(dev, dev_ctrl, ACTMON_DEV_CTRL);
device_writel(dev, ACTMON_INTR_STATUS_CLEAR, ACTMON_DEV_INTR_STATUS);
- actmon_device_debug(tegra, dev, "isr-");
+ trace_device_isr_exit(tegra->regs, dev->config->offset,
+ dev->boost_freq, cpufreq_get(0));
}
static unsigned long actmon_update_target(struct tegra_devfreq *tegra,
@@ -748,7 +732,6 @@ static struct devfreq_dev_profile tegra_devfreq_profile = {
static int tegra_governor_get_target(struct devfreq *devfreq,
unsigned long *freq)
{
- struct device *ddev = devfreq->dev.parent;
struct devfreq_dev_status *stat;
struct tegra_devfreq *tegra;
struct tegra_devfreq_device *dev;
@@ -769,13 +752,11 @@ static int tegra_governor_get_target(struct devfreq *devfreq,
dev = &tegra->devices[i];
dev_target_freq = actmon_update_target(tegra, dev);
-
target_freq = max(target_freq, dev_target_freq);
- dev_dbg(ddev, "%03x: upd: dev_target_freq %lu\n",
- dev->config->offset, dev_target_freq);
-
- actmon_device_debug(tegra, dev, "upd");
+ trace_device_target_freq(dev->config->offset, dev_target_freq);
+ trace_device_target_update(tegra->regs, dev->config->offset,
+ dev->boost_freq, cpufreq_get(0));
}
*freq = target_freq;
diff --git a/include/trace/events/tegra30_devfreq.h b/include/trace/events/tegra30_devfreq.h
new file mode 100644
index 000000000000..8f264a489daf
--- /dev/null
+++ b/include/trace/events/tegra30_devfreq.h
@@ -0,0 +1,105 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM tegra30_devfreq
+
+#if !defined(_TRACE_TEGRA30_DEVFREQ_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_TEGRA30_DEVFREQ_H
+
+#include <linux/io.h>
+#include <linux/tracepoint.h>
+#include <linux/types.h>
+
+DECLARE_EVENT_CLASS(device_state,
+ TP_PROTO(void __iomem *base, u32 offset, u32 boost, u32 cpufreq),
+ TP_ARGS(base, offset, boost, cpufreq),
+ TP_STRUCT__entry(
+ __field(u32, offset)
+ __field(u32, intr_status)
+ __field(u32, ctrl)
+ __field(u32, avg_count)
+ __field(u32, avg_lower)
+ __field(u32, avg_upper)
+ __field(u32, count)
+ __field(u32, lower)
+ __field(u32, upper)
+ __field(u32, boost_freq)
+ __field(u32, cpu_freq)
+ ),
+ TP_fast_assign(
+ __entry->offset = offset;
+ __entry->intr_status = readl_relaxed(base + offset + 0x24);
+ __entry->ctrl = readl_relaxed(base + offset + 0x0);
+ __entry->avg_count = readl_relaxed(base + offset + 0x20);
+ __entry->avg_lower = readl_relaxed(base + offset + 0x14);
+ __entry->avg_upper = readl_relaxed(base + offset + 0x10);
+ __entry->count = readl_relaxed(base + offset + 0x1c);
+ __entry->lower = readl_relaxed(base + offset + 0x8);
+ __entry->upper = readl_relaxed(base + offset + 0x4);
+ __entry->boost_freq = boost;
+ __entry->cpu_freq = cpufreq;
+ ),
+ TP_printk("%03x: intr 0x%08x ctrl 0x%08x avg %010u %010u %010u cnt %010u %010u %010u boost %010u cpu %u",
+ __entry->offset,
+ __entry->intr_status,
+ __entry->ctrl,
+ __entry->avg_count,
+ __entry->avg_lower,
+ __entry->avg_upper,
+ __entry->count,
+ __entry->lower,
+ __entry->upper,
+ __entry->boost_freq,
+ __entry->cpu_freq)
+);
+
+DEFINE_EVENT(device_state, device_isr_enter,
+ TP_PROTO(void __iomem *base, u32 offset, u32 boost, u32 cpufreq),
+ TP_ARGS(base, offset, boost, cpufreq));
+
+DEFINE_EVENT(device_state, device_isr_exit,
+ TP_PROTO(void __iomem *base, u32 offset, u32 boost, u32 cpufreq),
+ TP_ARGS(base, offset, boost, cpufreq));
+
+DEFINE_EVENT(device_state, device_target_update,
+ TP_PROTO(void __iomem *base, u32 offset, u32 boost, u32 cpufreq),
+ TP_ARGS(base, offset, boost, cpufreq));
+
+TRACE_EVENT(device_lower_upper,
+ TP_PROTO(u32 offset, u32 target, u32 lower, u32 upper),
+ TP_ARGS(offset, target, lower, upper),
+ TP_STRUCT__entry(
+ __field(u32, offset)
+ __field(u32, target)
+ __field(u32, lower)
+ __field(u32, upper)
+ ),
+ TP_fast_assign(
+ __entry->offset = offset;
+ __entry->target = target;
+ __entry->lower = lower;
+ __entry->upper = upper;
+ ),
+ TP_printk("%03x: freq %010u lower freq %010u upper freq %010u",
+ __entry->offset,
+ __entry->target,
+ __entry->lower,
+ __entry->upper)
+);
+
+TRACE_EVENT(device_target_freq,
+ TP_PROTO(u32 offset, u32 target),
+ TP_ARGS(offset, target),
+ TP_STRUCT__entry(
+ __field(u32, offset)
+ __field(u32, target)
+ ),
+ TP_fast_assign(
+ __entry->offset = offset;
+ __entry->target = target;
+ ),
+ TP_printk("%03x: freq %010u", __entry->offset, __entry->target)
+);
+#endif /* _TRACE_TEGRA30_DEVFREQ_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
2.22.0
^ permalink raw reply related
* [PATCH v3 20/22] PM / devfreq: tegra30: Optimize upper average watermark selection
From: Dmitry Osipenko @ 2019-06-27 21:11 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
I noticed that CPU may be crossing the dependency threshold very
frequently for some workloads and this results in a lot of interrupts
which could be avoided if MCALL client is keeping actual EMC frequency
at a higher rate.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 1105104445a9..5b1feaf8c16e 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -314,7 +314,8 @@ static void tegra_actmon_get_lower_upper(struct tegra_devfreq *tegra,
}
static void tegra_devfreq_update_avg_wmark(struct tegra_devfreq *tegra,
- struct tegra_devfreq_device *dev)
+ struct tegra_devfreq_device *dev,
+ unsigned long freq)
{
unsigned long avg_threshold, lower, upper;
@@ -323,6 +324,15 @@ static void tegra_devfreq_update_avg_wmark(struct tegra_devfreq *tegra,
avg_threshold = dev->config->avg_dependency_threshold;
avg_threshold = avg_threshold * ACTMON_SAMPLING_PERIOD;
+ /*
+ * If cumulative EMC frequency selection is higher than the
+ * device's, then there is no need to set upper watermark to
+ * a lower value because it will result in unnecessary upper
+ * interrupts.
+ */
+ if (freq * ACTMON_SAMPLING_PERIOD > upper)
+ upper = freq * ACTMON_SAMPLING_PERIOD;
+
/*
* We want to get interrupts when MCCPU client crosses the
* dependency threshold in order to take into / out of account
@@ -392,6 +402,7 @@ static void actmon_isr_device(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
u32 intr_status, dev_ctrl, avg_intr_mask, avg_count;
+ unsigned long freq;
trace_device_isr_enter(tegra->regs, dev->config->offset,
dev->boost_freq, cpufreq_get(0));
@@ -405,8 +416,10 @@ static void actmon_isr_device(struct tegra_devfreq *tegra,
avg_intr_mask = ACTMON_DEV_INTR_AVG_BELOW_WMARK |
ACTMON_DEV_INTR_AVG_ABOVE_WMARK;
- if (intr_status & avg_intr_mask)
- tegra_devfreq_update_avg_wmark(tegra, dev);
+ if (intr_status & avg_intr_mask) {
+ freq = clk_get_rate(tegra->emc_clock) / KHZ;
+ tegra_devfreq_update_avg_wmark(tegra, dev, freq);
+ }
if (intr_status & ACTMON_DEV_INTR_CONSECUTIVE_UPPER) {
/*
@@ -524,7 +537,7 @@ static int tegra_actmon_clk_notify_cb(struct notifier_block *nb,
for (i = 0; i < ARRAY_SIZE(tegra->devices); i++) {
dev = &tegra->devices[i];
- tegra_devfreq_update_avg_wmark(tegra, dev);
+ tegra_devfreq_update_avg_wmark(tegra, dev, freq);
tegra_devfreq_update_wmark(tegra, dev, freq);
}
@@ -629,7 +642,7 @@ static void tegra_actmon_configure_device(struct tegra_devfreq *tegra,
device_writel(dev, dev->avg_freq * ACTMON_SAMPLING_PERIOD,
ACTMON_DEV_INIT_AVG);
- tegra_devfreq_update_avg_wmark(tegra, dev);
+ tegra_devfreq_update_avg_wmark(tegra, dev, dev->avg_freq);
tegra_devfreq_update_wmark(tegra, dev, dev->avg_freq);
device_writel(dev, ACTMON_COUNT_WEIGHT, ACTMON_DEV_COUNT_WEIGHT);
--
2.22.0
^ permalink raw reply related
* [PATCH v3 21/22] PM / devfreq: tegra30: Include appropriate header
From: Dmitry Osipenko @ 2019-06-27 21:11 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
It's not very correct to include mod_devicetable.h for the OF device
drivers and of_device.h should be included instead.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 5b1feaf8c16e..3ca227c95829 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -13,7 +13,7 @@
#include <linux/io.h>
#include <linux/irq.h>
#include <linux/module.h>
-#include <linux/mod_devicetable.h>
+#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/pm_opp.h>
#include <linux/reset.h>
--
2.22.0
^ permalink raw reply related
* [PATCH v3 19/22] PM / devfreq: tegra30: Optimize upper consecutive watermark selection
From: Dmitry Osipenko @ 2019-06-27 21:11 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
The memory activity counter may get a bit higher than a watermark which
is selected based on OPP that corresponds to a highest EMC rate, in this
case watermark is lower than the actual memory activity is and thus
results in unwanted "upper" interrupts.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index e8f7cc56a340..1105104445a9 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -363,7 +363,18 @@ static void tegra_devfreq_update_wmark(struct tegra_devfreq *tegra,
tegra_actmon_get_lower_upper(tegra, dev, freq - 1, &lower, &upper);
delta = do_percent(upper - lower, dev->config->boost_up_threshold);
- device_writel(dev, lower + delta, ACTMON_DEV_UPPER_WMARK);
+
+ /*
+ * The memory events count could go a bit higher than the maximum
+ * defined by the OPPs, hence make the upper watermark infinitely
+ * high to avoid unnecessary upper interrupts in that case.
+ */
+ if (freq == tegra->max_freq)
+ upper = ULONG_MAX;
+ else
+ upper = lower + delta;
+
+ device_writel(dev, upper, ACTMON_DEV_UPPER_WMARK);
/*
* Meanwhile the lower mark is based on the average value
--
2.22.0
^ permalink raw reply related
* [PATCH v3 22/22] PM / devfreq: tegra20/30: Add Dmitry as a maintainer
From: Dmitry Osipenko @ 2019-06-27 21:11 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
I was contributing to the NVIDIA Tegra20+ devfreq drivers recently and
want to help keep them working and evolving in the future.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
MAINTAINERS | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 792d2d927712..bfd827417a27 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10250,6 +10250,15 @@ F: include/linux/memblock.h
F: mm/memblock.c
F: Documentation/core-api/boot-time-mm.rst
+MEMORY FREQUENCY SCALING DRIVERS FOR NVIDIA TEGRA
+M: Dmitry Osipenko <digetx@gmail.com>
+L: linux-pm@vger.kernel.org
+L: linux-tegra@vger.kernel.org
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/mzx/devfreq.git
+S: Maintained
+F: drivers/devfreq/tegra20-devfreq.c
+F: drivers/devfreq/tegra30-devfreq.c
+
MEMORY MANAGEMENT
L: linux-mm@kvack.org
W: http://www.linux-mm.org
--
2.22.0
^ permalink raw reply related
* [PATCH v3 16/22] PM / devfreq: tegra30: Use kHz units uniformly in the code
From: Dmitry Osipenko @ 2019-06-27 21:11 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190627211115.21138-1-digetx@gmail.com>
Now that all kHz-conversion related bugs are fixed, we can use the kHz
uniformly. This makes code cleaner and avoids integer divisions in the
code, which is useful in a case of Tegra30 that has Cortex A9 CPU that
doesn't support integer division instructions, hence all divisions are
actually made in software mode. Another small benefit from this change
is that now powertop utility correctly displays devfreq's stats, for
some reason it expects them to be in kHz.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 78 +++++++++++++++++--------------
1 file changed, 43 insertions(+), 35 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 9a6ede689991..61d9601766fc 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -72,6 +72,8 @@
#define KHZ 1000
+#define KHZ_MAX (ULONG_MAX / KHZ)
+
/* Assume that the bus is saturated if the utilization is 25% */
#define BUS_SATURATION_RATIO 25
@@ -124,7 +126,7 @@ static const struct tegra_devfreq_device_config actmon_device_configs[] = {
.boost_down_coeff = 90,
.boost_up_threshold = 27,
.boost_down_threshold = 10,
- .avg_dependency_threshold = 50000,
+ .avg_dependency_threshold = 50000 / ACTMON_SAMPLING_PERIOD,
},
};
@@ -137,8 +139,11 @@ struct tegra_devfreq_device {
const struct tegra_devfreq_device_config *config;
void __iomem *regs;
- /* Average event count sampled in the last interrupt */
- u32 avg_count;
+ /*
+ * Average event count sampled in the last interrupt and converted
+ * to frequency value.
+ */
+ u32 avg_freq;
/*
* Extra frequency to increase the target by due to consecutive
@@ -172,7 +177,7 @@ struct tegra_actmon_emc_ratio {
};
static const struct tegra_actmon_emc_ratio actmon_emc_ratios[] = {
- { 1400000, ULONG_MAX / KHZ },
+ { 1400000, KHZ_MAX },
{ 1200000, 750000 },
{ 1100000, 600000 },
{ 1000000, 500000 },
@@ -234,7 +239,7 @@ tegra_actmon_account_cpu_freq(struct tegra_devfreq *tegra,
unsigned long static_cpu_emc_freq;
if (dev->config->avg_dependency_threshold &&
- dev->config->avg_dependency_threshold < dev->avg_count) {
+ dev->config->avg_dependency_threshold < dev->avg_freq) {
static_cpu_emc_freq = actmon_cpu_to_emc_rate(tegra);
target_freq = max(target_freq, static_cpu_emc_freq);
}
@@ -265,7 +270,7 @@ static unsigned long tegra_actmon_upper_freq(struct tegra_devfreq *tegra,
opp = dev_pm_opp_find_freq_ceil(tegra->devfreq->dev.parent, &upper);
if (IS_ERR(opp))
- upper = ULONG_MAX;
+ upper = KHZ_MAX;
else
dev_pm_opp_put(opp);
@@ -287,7 +292,7 @@ static void tegra_actmon_get_lower_upper(struct tegra_devfreq *tegra,
* range in a case where target_freq falls into a range of
* next_possible_opp_freq - 1MHz.
*/
- target_freq = round_down(target_freq, 1000000);
+ target_freq = round_down(target_freq, 1000);
/* watermarks are set at the borders of the corresponding OPPs */
*lower = tegra_actmon_lower_freq(tegra, target_freq);
@@ -296,9 +301,6 @@ static void tegra_actmon_get_lower_upper(struct tegra_devfreq *tegra,
dev_dbg(ddev, "%03x: target_freq %lu lower freq %lu upper freq %lu\n",
offset, target_freq, *lower, *upper);
- *lower /= KHZ;
- *upper /= KHZ;
-
/*
* The upper watermark should take into account CPU's frequency
* because cpu_to_emc_rate() may override the target_freq with
@@ -314,22 +316,23 @@ static void tegra_actmon_get_lower_upper(struct tegra_devfreq *tegra,
static void tegra_devfreq_update_avg_wmark(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
- unsigned long lower, upper, freq;
+ unsigned long avg_threshold, lower, upper;
- freq = dev->avg_count / ACTMON_SAMPLING_PERIOD * KHZ;
- tegra_actmon_get_lower_upper(tegra, dev, freq, &lower, &upper);
+ tegra_actmon_get_lower_upper(tegra, dev, dev->avg_freq, &lower, &upper);
+
+ avg_threshold = dev->config->avg_dependency_threshold;
+ avg_threshold = avg_threshold * ACTMON_SAMPLING_PERIOD;
/*
* We want to get interrupts when MCCPU client crosses the
* dependency threshold in order to take into / out of account
* the CPU's freq.
*/
- if (lower < dev->config->avg_dependency_threshold &&
- upper > dev->config->avg_dependency_threshold) {
- if (dev->avg_count < dev->config->avg_dependency_threshold)
- upper = dev->config->avg_dependency_threshold;
+ if (lower < avg_threshold && upper > avg_threshold) {
+ if (dev->avg_freq < dev->config->avg_dependency_threshold)
+ upper = avg_threshold;
else
- lower = dev->config->avg_dependency_threshold;
+ lower = avg_threshold;
}
device_writel(dev, lower, ACTMON_DEV_AVG_LOWER_WMARK);
@@ -368,8 +371,7 @@ static void tegra_devfreq_update_wmark(struct tegra_devfreq *tegra,
* device. Once that mark is hit and boosting is stopped, the
* interrupt is disabled by ISR.
*/
- freq = dev->avg_count / ACTMON_SAMPLING_PERIOD * KHZ;
- tegra_actmon_get_lower_upper(tegra, dev, freq, &lower, &upper);
+ tegra_actmon_get_lower_upper(tegra, dev, dev->avg_freq, &lower, &upper);
delta = do_percent(upper - lower, dev->config->boost_down_threshold);
device_writel(dev, lower + delta, ACTMON_DEV_LOWER_WMARK);
@@ -396,14 +398,16 @@ static void actmon_device_debug(struct tegra_devfreq *tegra,
static void actmon_isr_device(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
- u32 intr_status, dev_ctrl, avg_intr_mask;
+ u32 intr_status, dev_ctrl, avg_intr_mask, avg_count;
actmon_device_debug(tegra, dev, "isr+");
- dev->avg_count = device_readl(dev, ACTMON_DEV_AVG_COUNT);
intr_status = device_readl(dev, ACTMON_DEV_INTR_STATUS);
+ avg_count = device_readl(dev, ACTMON_DEV_AVG_COUNT);
dev_ctrl = device_readl(dev, ACTMON_DEV_CTRL);
+ dev->avg_freq = avg_count / ACTMON_SAMPLING_PERIOD;
+
avg_intr_mask = ACTMON_DEV_INTR_AVG_BELOW_WMARK |
ACTMON_DEV_INTR_AVG_ABOVE_WMARK;
@@ -458,8 +462,7 @@ static unsigned long actmon_update_target(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{ unsigned long target_freq;
- target_freq = dev->avg_count / ACTMON_SAMPLING_PERIOD + dev->boost_freq;
- target_freq = min(target_freq, ULONG_MAX / KHZ);
+ target_freq = min(dev->avg_freq + dev->boost_freq, KHZ_MAX);
target_freq = tegra_actmon_account_cpu_freq(tegra, dev, target_freq);
return target_freq;
@@ -496,6 +499,7 @@ static int tegra_actmon_clk_notify_cb(struct notifier_block *nb,
struct clk_notifier_data *data = ptr;
struct tegra_devfreq_device *dev;
struct tegra_devfreq *tegra;
+ unsigned long freq;
unsigned int i;
if (action != POST_RATE_CHANGE)
@@ -503,6 +507,8 @@ static int tegra_actmon_clk_notify_cb(struct notifier_block *nb,
tegra = container_of(nb, struct tegra_devfreq, clk_rate_change_nb);
+ freq = data->new_rate / KHZ;
+
/*
* EMC rate could change due to three reasons:
*
@@ -524,7 +530,7 @@ static int tegra_actmon_clk_notify_cb(struct notifier_block *nb,
dev = &tegra->devices[i];
tegra_devfreq_update_avg_wmark(tegra, dev);
- tegra_devfreq_update_wmark(tegra, dev, data->new_rate);
+ tegra_devfreq_update_wmark(tegra, dev, freq);
}
return NOTIFY_OK;
@@ -564,17 +570,17 @@ static int tegra_actmon_cpu_notify_cb(struct notifier_block *nb,
static void tegra_actmon_configure_device(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
- u32 val = 0, target_freq;
+ u32 val = 0;
/* we don't want boosting on restart */
dev->boost_freq = 0;
- target_freq = clk_get_rate(tegra->emc_clock) / KHZ;
- dev->avg_count = target_freq * ACTMON_SAMPLING_PERIOD;
- device_writel(dev, dev->avg_count, ACTMON_DEV_INIT_AVG);
+ dev->avg_freq = clk_get_rate(tegra->emc_clock) / KHZ;
+ device_writel(dev, dev->avg_freq * ACTMON_SAMPLING_PERIOD,
+ ACTMON_DEV_INIT_AVG);
tegra_devfreq_update_avg_wmark(tegra, dev);
- tegra_devfreq_update_wmark(tegra, dev, target_freq);
+ tegra_devfreq_update_wmark(tegra, dev, dev->avg_freq);
device_writel(dev, ACTMON_COUNT_WEIGHT, ACTMON_DEV_COUNT_WEIGHT);
device_writel(dev, ACTMON_INTR_STATUS_CLEAR, ACTMON_DEV_INTR_STATUS);
@@ -686,7 +692,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
rate = dev_pm_opp_get_freq(opp);
dev_pm_opp_put(opp);
- err = clk_set_min_rate(tegra->emc_clock, rate);
+ err = clk_set_min_rate(tegra->emc_clock, rate * KHZ);
if (err)
return err;
@@ -709,7 +715,7 @@ static int tegra_devfreq_get_dev_status(struct device *dev,
struct tegra_devfreq_device *actmon_dev;
unsigned long cur_freq;
- cur_freq = clk_get_rate(tegra->emc_clock);
+ cur_freq = clk_get_rate(tegra->emc_clock) / KHZ;
/* To be used by the tegra governor */
stat->private_data = tegra;
@@ -726,7 +732,7 @@ static int tegra_devfreq_get_dev_status(struct device *dev,
stat->busy_time *= 100 / BUS_SATURATION_RATIO;
/* Number of cycles in a sampling period */
- stat->total_time = cur_freq / KHZ * ACTMON_SAMPLING_PERIOD;
+ stat->total_time = cur_freq * ACTMON_SAMPLING_PERIOD;
stat->busy_time = min(stat->busy_time, stat->total_time);
@@ -772,7 +778,7 @@ static int tegra_governor_get_target(struct devfreq *devfreq,
actmon_device_debug(tegra, dev, "upd");
}
- *freq = target_freq * KHZ;
+ *freq = target_freq;
return 0;
}
@@ -908,7 +914,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
goto remove_opps;
}
- err = dev_pm_opp_add(&pdev->dev, rate, 0);
+ err = dev_pm_opp_add(&pdev->dev, rate / KHZ, 0);
if (err) {
dev_err(&pdev->dev, "Failed to add OPP: %d\n", err);
goto remove_opps;
@@ -928,6 +934,8 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
}
tegra_devfreq_profile.initial_freq = clk_get_rate(tegra->emc_clock);
+ tegra_devfreq_profile.initial_freq /= KHZ;
+
devfreq = devfreq_add_device(&pdev->dev, &tegra_devfreq_profile,
"tegra_actmon", NULL);
if (IS_ERR(tegra->devfreq)) {
--
2.22.0
^ permalink raw reply related
* [PATCH v3 00/22] More improvements for Tegra30 devfreq driver
From: Dmitry Osipenko @ 2019-06-27 21:10 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
Hello,
This series addresses some additional review comments that were made by
Thierry Reding to [1] and makes several important changes to the driver,
fixing excessive interrupts activity. In the end I'm proposing myself as
a maintainer for the Tegra devfreq drivers.
[1] https://lore.kernel.org/lkml/0fb50eb1-a173-1756-6889-2526a10ac707@gmail.com/T/
Changelog:
v3: Added support for tracepoints, replacing the debug messages.
Fixed few more bugs with the help of tracepoints.
New patches in this version:
PM / devfreq: tegra30: Use tracepoints for debugging
PM / devfreq: tegra30: Optimize CPUFreq notifier
PM / devfreq: tegra30: Optimize upper consecutive watermark selection
PM / devfreq: tegra30: Optimize upper average watermark selection
PM / devfreq: tegra30: Include appropriate header
Some of older patches of this series also got some extra minor polish.
v2: Added more patches that are cleaning driver's code further and
squashing another kHz conversion bug.
The patch "Rework frequency management logic" of the v1 series is now
converted to "Set up watermarks properly" because I found some problems
in the original patch and then realized that there is no need to change
the logic much. So the logic mostly preserved and only got improvements.
The series is based on the today's linux-next (25 Jun) and takes into
account minor changes that MyungJoo Ham made to the already queued
patches from the first batch [1].
Dmitry Osipenko (22):
PM / devfreq: tegra30: Change irq type to unsigned int
PM / devfreq: tegra30: Keep interrupt disabled while governor is
stopped
PM / devfreq: tegra30: Handle possible round-rate error
PM / devfreq: tegra30: Drop write-barrier
PM / devfreq: tegra30: Set up watermarks properly
PM / devfreq: tegra30: Tune up boosting thresholds
PM / devfreq: tegra30: Use CPUFreq notifier
PM / devfreq: tegra30: Move clk-notifier's registration to governor's
start
PM / devfreq: tegra30: Reset boosting on startup
PM / devfreq: tegra30: Don't enable consecutive-down interrupt on
startup
PM / devfreq: tegra30: Add debug messages
PM / devfreq: tegra30: Inline all one-line functions
PM / devfreq: tegra30: Constify structs
PM / devfreq: tegra30: Ensure that target freq won't overflow
PM / devfreq: tegra30: Fix integer overflow on CPU's freq max out
PM / devfreq: tegra30: Use kHz units uniformly in the code
PM / devfreq: tegra30: Use tracepoints for debugging
PM / devfreq: tegra30: Optimize CPUFreq notifier
PM / devfreq: tegra30: Optimize upper consecutive watermark selection
PM / devfreq: tegra30: Optimize upper average watermark selection
PM / devfreq: tegra30: Include appropriate header
PM / devfreq: tegra20/30: Add Dmitry as a maintainer
MAINTAINERS | 9 +
drivers/devfreq/tegra30-devfreq.c | 631 ++++++++++++++++++-------
include/trace/events/tegra30_devfreq.h | 105 ++++
3 files changed, 583 insertions(+), 162 deletions(-)
create mode 100644 include/trace/events/tegra30_devfreq.h
--
2.22.0
^ permalink raw reply
* [PATCH V4 2/3] thermal/drivers/cpu_cooling: Unregister with the policy
From: Daniel Lezcano @ 2019-06-27 21:02 UTC (permalink / raw)
To: viresh.kumar
Cc: rjw, edubezval, linux-kernel, Sudeep Holla, Amit Daniel Kachhap,
Javi Merino, Zhang Rui, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team, Keerthy,
open list:CPU FREQUENCY DRIVERS - ARM BIG LITTLE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list:TI BANDGAP AND THERMAL DRIVER
In-Reply-To: <20190627210209.32600-1-daniel.lezcano@linaro.org>
Currently the function cpufreq_cooling_register() returns a cooling
device pointer which is used back as a pointer to call the function
cpufreq_cooling_unregister(). Even if it is correct, it would make
sense to not leak the structure inside a cpufreq driver and keep the
code thermal code self-encapsulate. Moreover, that forces to add an
extra variable in each driver using this function.
Instead of passing the cooling device to unregister, pass the policy.
Because the cpufreq_cooling_unregister() function uses the policy to
unregister itself. The only purpose of the cooling device pointer is
to unregister the cpu cooling device.
As there is no more need of this pointer, remove it.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/arm_big_little.c | 9 ++--
drivers/cpufreq/cpufreq.c | 8 ++--
drivers/thermal/cpu_cooling.c | 42 +++++++++++--------
drivers/thermal/imx_thermal.c | 12 +++---
.../ti-soc-thermal/ti-thermal-common.c | 10 ++---
include/linux/cpu_cooling.h | 6 +--
include/linux/cpufreq.h | 3 --
7 files changed, 45 insertions(+), 45 deletions(-)
diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
index 7fe52fcddcf1..718c63231e66 100644
--- a/drivers/cpufreq/arm_big_little.c
+++ b/drivers/cpufreq/arm_big_little.c
@@ -56,7 +56,6 @@ static bool bL_switching_enabled;
#define ACTUAL_FREQ(cluster, freq) ((cluster == A7_CLUSTER) ? freq << 1 : freq)
#define VIRT_FREQ(cluster, freq) ((cluster == A7_CLUSTER) ? freq >> 1 : freq)
-static struct thermal_cooling_device *cdev[MAX_CLUSTERS];
static const struct cpufreq_arm_bL_ops *arm_bL_ops;
static struct clk *clk[MAX_CLUSTERS];
static struct cpufreq_frequency_table *freq_table[MAX_CLUSTERS + 1];
@@ -501,10 +500,8 @@ static int bL_cpufreq_exit(struct cpufreq_policy *policy)
struct device *cpu_dev;
int cur_cluster = cpu_to_cluster(policy->cpu);
- if (cur_cluster < MAX_CLUSTERS) {
- cpufreq_cooling_unregister(cdev[cur_cluster]);
- cdev[cur_cluster] = NULL;
- }
+ if (cur_cluster < MAX_CLUSTERS)
+ cpufreq_cooling_unregister(policy);
cpu_dev = get_cpu_device(policy->cpu);
if (!cpu_dev) {
@@ -527,7 +524,7 @@ static void bL_cpufreq_ready(struct cpufreq_policy *policy)
if (cur_cluster >= MAX_CLUSTERS)
return;
- cdev[cur_cluster] = of_cpufreq_cooling_register(policy);
+ of_cpufreq_cooling_register(policy);
}
static struct cpufreq_driver bL_cpufreq_driver = {
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index aee024e42618..1663a5601811 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1379,7 +1379,7 @@ static int cpufreq_online(unsigned int cpu)
cpufreq_driver->ready(policy);
if (cpufreq_thermal_control_enabled(cpufreq_driver))
- policy->cdev = of_cpufreq_cooling_register(policy);
+ of_cpufreq_cooling_register(policy);
pr_debug("initialization complete\n");
@@ -1468,10 +1468,8 @@ static int cpufreq_offline(unsigned int cpu)
goto unlock;
}
- if (cpufreq_thermal_control_enabled(cpufreq_driver)) {
- cpufreq_cooling_unregister(policy->cdev);
- policy->cdev = NULL;
- }
+ if (cpufreq_thermal_control_enabled(cpufreq_driver))
+ cpufreq_cooling_unregister(policy);
if (cpufreq_driver->stop_cpu)
cpufreq_driver->stop_cpu(policy);
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 83486775e593..be01546a656f 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -78,6 +78,7 @@ struct cpufreq_cooling_device {
struct cpufreq_policy *policy;
struct list_head node;
struct time_in_idle *idle_time;
+ struct thermal_cooling_device *cdev;
};
static DEFINE_IDA(cpufreq_ida);
@@ -606,6 +607,7 @@ __cpufreq_cooling_register(struct device_node *np,
goto remove_ida;
cpufreq_cdev->clipped_freq = get_state_freq(cpufreq_cdev, 0);
+ cpufreq_cdev->cdev = cdev;
mutex_lock(&cooling_list_lock);
/* Register the notifier for first cpufreq cooling device */
@@ -693,35 +695,41 @@ of_cpufreq_cooling_register(struct cpufreq_policy *policy)
}
EXPORT_SYMBOL_GPL(of_cpufreq_cooling_register);
+void __cpufreq_cooling_unregister(struct cpufreq_cooling_device *cpufreq_cdev, int last)
+{
+ /* Unregister the notifier for the last cpufreq cooling device */
+ if (last)
+ cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
+ CPUFREQ_POLICY_NOTIFIER);
+
+ thermal_cooling_device_unregister(cpufreq_cdev->cdev);
+ ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id);
+ kfree(cpufreq_cdev->idle_time);
+ kfree(cpufreq_cdev);
+}
+
/**
* cpufreq_cooling_unregister - function to remove cpufreq cooling device.
* @cdev: thermal cooling device pointer.
*
* This interface function unregisters the "thermal-cpufreq-%x" cooling device.
*/
-void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
+void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
{
struct cpufreq_cooling_device *cpufreq_cdev;
bool last;
- if (!cdev)
- return;
-
- cpufreq_cdev = cdev->devdata;
-
mutex_lock(&cooling_list_lock);
- list_del(&cpufreq_cdev->node);
- /* Unregister the notifier for the last cpufreq cooling device */
- last = list_empty(&cpufreq_cdev_list);
+ list_for_each_entry(cpufreq_cdev, &cpufreq_cdev_list, node) {
+ if (cpufreq_cdev->policy == policy) {
+ list_del(&cpufreq_cdev->node);
+ last = list_empty(&cpufreq_cdev_list);
+ break;
+ }
+ }
mutex_unlock(&cooling_list_lock);
- if (last)
- cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
- CPUFREQ_POLICY_NOTIFIER);
-
- thermal_cooling_device_unregister(cdev);
- ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id);
- kfree(cpufreq_cdev->idle_time);
- kfree(cpufreq_cdev);
+ if (cpufreq_cdev->policy == policy)
+ __cpufreq_cooling_unregister(cpufreq_cdev, last);
}
EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister);
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index bb6754a5342c..021c0948b740 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -203,7 +203,6 @@ static struct thermal_soc_data thermal_imx7d_data = {
struct imx_thermal_data {
struct cpufreq_policy *policy;
struct thermal_zone_device *tz;
- struct thermal_cooling_device *cdev;
enum thermal_device_mode mode;
struct regmap *tempmon;
u32 c1, c2; /* See formula in imx_init_calib() */
@@ -656,6 +655,7 @@ MODULE_DEVICE_TABLE(of, of_imx_thermal_match);
static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
{
struct device_node *np;
+ struct thermal_cooling_device *cdev;
int ret;
data->policy = cpufreq_cpu_get(0);
@@ -667,9 +667,9 @@ static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
np = of_get_cpu_node(data->policy->cpu, NULL);
if (!np || !of_find_property(np, "#cooling-cells", NULL)) {
- data->cdev = cpufreq_cooling_register(data->policy);
- if (IS_ERR(data->cdev)) {
- ret = PTR_ERR(data->cdev);
+ cdev = cpufreq_cooling_register(data->policy);
+ if (IS_ERR(cdev)) {
+ ret = PTR_ERR(cdev);
cpufreq_cpu_put(data->policy);
return ret;
}
@@ -680,7 +680,7 @@ static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
static void imx_thermal_unregister_legacy_cooling(struct imx_thermal_data *data)
{
- cpufreq_cooling_unregister(data->cdev);
+ cpufreq_cooling_unregister(data->policy);
cpufreq_cpu_put(data->policy);
}
@@ -872,7 +872,7 @@ static int imx_thermal_remove(struct platform_device *pdev)
clk_disable_unprepare(data->thermal_clk);
thermal_zone_device_unregister(data->tz);
- cpufreq_cooling_unregister(data->cdev);
+ cpufreq_cooling_unregister(data->policy);
cpufreq_cpu_put(data->policy);
return 0;
diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
index b4f981daeaf2..170b70b6ec61 100644
--- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
+++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
@@ -41,7 +41,6 @@ struct ti_thermal_data {
struct cpufreq_policy *policy;
struct thermal_zone_device *ti_thermal;
struct thermal_zone_device *pcb_tz;
- struct thermal_cooling_device *cool_dev;
struct ti_bandgap *bgp;
enum thermal_device_mode mode;
struct work_struct thermal_wq;
@@ -233,6 +232,7 @@ int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
{
struct ti_thermal_data *data;
struct device_node *np = bgp->dev->of_node;
+ struct thermal_cooling_device *cdev;
/*
* We are assuming here that if one deploys the zone
@@ -256,9 +256,9 @@ int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
}
/* Register cooling device */
- data->cool_dev = cpufreq_cooling_register(data->policy);
- if (IS_ERR(data->cool_dev)) {
- int ret = PTR_ERR(data->cool_dev);
+ cdev = cpufreq_cooling_register(data->policy);
+ if (IS_ERR(cdev)) {
+ int ret = PTR_ERR(cdev);
dev_err(bgp->dev, "Failed to register cpu cooling device %d\n",
ret);
cpufreq_cpu_put(data->policy);
@@ -277,7 +277,7 @@ int ti_thermal_unregister_cpu_cooling(struct ti_bandgap *bgp, int id)
data = ti_bandgap_get_sensor_data(bgp, id);
if (data) {
- cpufreq_cooling_unregister(data->cool_dev);
+ cpufreq_cooling_unregister(data->policy);
if (data->policy)
cpufreq_cpu_put(data->policy);
}
diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
index bae54bb7c048..89f469ee4be4 100644
--- a/include/linux/cpu_cooling.h
+++ b/include/linux/cpu_cooling.h
@@ -29,9 +29,9 @@ cpufreq_cooling_register(struct cpufreq_policy *policy);
/**
* cpufreq_cooling_unregister - function to remove cpufreq cooling device.
- * @cdev: thermal cooling device pointer.
+ * @policy: cpufreq policy
*/
-void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev);
+void cpufreq_cooling_unregister(struct cpufreq_policy *policy);
#else /* !CONFIG_CPU_THERMAL */
static inline struct thermal_cooling_device *
@@ -41,7 +41,7 @@ cpufreq_cooling_register(struct cpufreq_policy *policy)
}
static inline
-void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
+void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
{
return;
}
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index a1467aa7f58b..ce13204df972 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -144,9 +144,6 @@ struct cpufreq_policy {
/* For cpufreq driver's internal use */
void *driver_data;
-
- /* Pointer to the cooling device if used for thermal mitigation */
- struct thermal_cooling_device *cdev;
};
struct cpufreq_freqs {
--
2.17.1
^ permalink raw reply related
* [PATCH V4 3/3] thermal/drivers/cpu_cooling: cpufreq_cooling_register returns an int
From: Daniel Lezcano @ 2019-06-27 21:02 UTC (permalink / raw)
To: viresh.kumar
Cc: rjw, edubezval, linux-kernel, Amit Daniel Kachhap, Javi Merino,
Zhang Rui, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, NXP Linux Team, Keerthy,
open list:THERMAL/CPU_COOLING,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list:TI BANDGAP AND THERMAL DRIVER
In-Reply-To: <20190627210209.32600-1-daniel.lezcano@linaro.org>
It looks like after the changes in the patch the only reason for
returning (struct thermal_cooling_device *) from
cpufreq_cooling_register() is error checking, but it would be much
more straightforward to return int for this purpose.
Moreover, that would prevent the callers of it from doing incorrect
things with the returned pointers (like using it to unregister the
cooling device).
Replace the returned value an integer instead of a pointer to a
thermal cooling device structure.
Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/thermal/cpu_cooling.c | 63 +++++++++----------
drivers/thermal/imx_thermal.c | 6 +-
.../ti-soc-thermal/ti-thermal-common.c | 7 +--
include/linux/cpu_cooling.h | 16 ++---
4 files changed, 40 insertions(+), 52 deletions(-)
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index be01546a656f..0d5e39716542 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -530,13 +530,12 @@ static struct notifier_block thermal_cpufreq_notifier_block = {
* cooling devices. It also gives the opportunity to link the cooling device
* with a device tree node, in order to bind it via the thermal DT code.
*
- * Return: a valid struct thermal_cooling_device pointer on success,
- * on failure, it returns a corresponding ERR_PTR().
+ * Return: zero on success, less than zero corresponding to the
+ * negative error code.
*/
-static struct thermal_cooling_device *
-__cpufreq_cooling_register(struct device_node *np,
- struct cpufreq_policy *policy,
- struct em_perf_domain *em)
+static int __cpufreq_cooling_register(struct device_node *np,
+ struct cpufreq_policy *policy,
+ struct em_perf_domain *em)
{
struct thermal_cooling_device *cdev;
struct cpufreq_cooling_device *cpufreq_cdev;
@@ -548,19 +547,19 @@ __cpufreq_cooling_register(struct device_node *np,
if (IS_ERR_OR_NULL(policy)) {
pr_err("%s: cpufreq policy isn't valid: %p\n", __func__, policy);
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
}
i = cpufreq_table_count_valid_entries(policy);
if (!i) {
pr_debug("%s: CPUFreq table not found or has no valid entries\n",
__func__);
- return ERR_PTR(-ENODEV);
+ return -ENODEV;
}
cpufreq_cdev = kzalloc(sizeof(*cpufreq_cdev), GFP_KERNEL);
if (!cpufreq_cdev)
- return ERR_PTR(-ENOMEM);
+ return -ENOMEM;
cpufreq_cdev->policy = policy;
num_cpus = cpumask_weight(policy->related_cpus);
@@ -568,7 +567,7 @@ __cpufreq_cooling_register(struct device_node *np,
sizeof(*cpufreq_cdev->idle_time),
GFP_KERNEL);
if (!cpufreq_cdev->idle_time) {
- cdev = ERR_PTR(-ENOMEM);
+ ret = -ENOMEM;
goto free_cdev;
}
@@ -576,10 +575,8 @@ __cpufreq_cooling_register(struct device_node *np,
cpufreq_cdev->max_level = i - 1;
ret = ida_simple_get(&cpufreq_ida, 0, 0, GFP_KERNEL);
- if (ret < 0) {
- cdev = ERR_PTR(ret);
+ if (ret < 0)
goto free_idle_time;
- }
cpufreq_cdev->id = ret;
snprintf(dev_name, sizeof(dev_name), "thermal-cpufreq-%d",
@@ -597,14 +594,16 @@ __cpufreq_cooling_register(struct device_node *np,
if (policy->freq_table_sorted == CPUFREQ_TABLE_UNSORTED) {
pr_err("%s: unsorted frequency tables are not supported\n",
__func__);
- cdev = ERR_PTR(-EINVAL);
+ ret = -EINVAL;
goto remove_ida;
}
cdev = thermal_of_cooling_device_register(np, dev_name, cpufreq_cdev,
cooling_ops);
- if (IS_ERR(cdev))
+ if (IS_ERR(cdev)) {
+ ret = PTR_ERR(cdev);
goto remove_ida;
+ }
cpufreq_cdev->clipped_freq = get_state_freq(cpufreq_cdev, 0);
cpufreq_cdev->cdev = cdev;
@@ -619,7 +618,7 @@ __cpufreq_cooling_register(struct device_node *np,
cpufreq_register_notifier(&thermal_cpufreq_notifier_block,
CPUFREQ_POLICY_NOTIFIER);
- return cdev;
+ return 0;
remove_ida:
ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id);
@@ -627,7 +626,7 @@ __cpufreq_cooling_register(struct device_node *np,
kfree(cpufreq_cdev->idle_time);
free_cdev:
kfree(cpufreq_cdev);
- return cdev;
+ return ret;
}
/**
@@ -638,11 +637,10 @@ __cpufreq_cooling_register(struct device_node *np,
* "thermal-cpufreq-%x". This api can support multiple instances of cpufreq
* cooling devices.
*
- * Return: a valid struct thermal_cooling_device pointer on success,
- * on failure, it returns a corresponding ERR_PTR().
+ * Return: zero on success, less than zero corresponding to the
+ * negative error code.
*/
-struct thermal_cooling_device *
-cpufreq_cooling_register(struct cpufreq_policy *policy)
+int cpufreq_cooling_register(struct cpufreq_policy *policy)
{
return __cpufreq_cooling_register(NULL, policy, NULL);
}
@@ -664,34 +662,31 @@ EXPORT_SYMBOL_GPL(cpufreq_cooling_register);
* It also takes into account, if property present in policy CPU node, the
* static power consumed by the cpu.
*
- * Return: a valid struct thermal_cooling_device pointer on success,
- * and NULL on failure.
+ * Return: zero on success, less than zero corresponding to the
+ * negative error code.
*/
-struct thermal_cooling_device *
-of_cpufreq_cooling_register(struct cpufreq_policy *policy)
+int of_cpufreq_cooling_register(struct cpufreq_policy *policy)
{
struct device_node *np = of_get_cpu_node(policy->cpu, NULL);
- struct thermal_cooling_device *cdev = NULL;
+ int ret = -EINVAL;
if (!np) {
pr_err("cpu_cooling: OF node not available for cpu%d\n",
policy->cpu);
- return NULL;
+ return -EINVAL;
}
if (of_find_property(np, "#cooling-cells", NULL)) {
struct em_perf_domain *em = em_cpu_get(policy->cpu);
- cdev = __cpufreq_cooling_register(np, policy, em);
- if (IS_ERR(cdev)) {
- pr_err("cpu_cooling: cpu%d failed to register as cooling device: %ld\n",
- policy->cpu, PTR_ERR(cdev));
- cdev = NULL;
- }
+ ret = __cpufreq_cooling_register(np, policy, em);
+ if (ret)
+ pr_err("cpu_cooling: cpu%d failed to register as cooling device: %d\n",
+ policy->cpu, ret);
}
of_node_put(np);
- return cdev;
+ return ret;
}
EXPORT_SYMBOL_GPL(of_cpufreq_cooling_register);
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 021c0948b740..1c4b49b583bc 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -655,7 +655,6 @@ MODULE_DEVICE_TABLE(of, of_imx_thermal_match);
static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
{
struct device_node *np;
- struct thermal_cooling_device *cdev;
int ret;
data->policy = cpufreq_cpu_get(0);
@@ -667,9 +666,8 @@ static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
np = of_get_cpu_node(data->policy->cpu, NULL);
if (!np || !of_find_property(np, "#cooling-cells", NULL)) {
- cdev = cpufreq_cooling_register(data->policy);
- if (IS_ERR(cdev)) {
- ret = PTR_ERR(cdev);
+ ret = cpufreq_cooling_register(data->policy);
+ if (ret) {
cpufreq_cpu_put(data->policy);
return ret;
}
diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
index 170b70b6ec61..eacc46d7bd1c 100644
--- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
+++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
@@ -232,7 +232,7 @@ int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
{
struct ti_thermal_data *data;
struct device_node *np = bgp->dev->of_node;
- struct thermal_cooling_device *cdev;
+ int ret;
/*
* We are assuming here that if one deploys the zone
@@ -256,9 +256,8 @@ int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
}
/* Register cooling device */
- cdev = cpufreq_cooling_register(data->policy);
- if (IS_ERR(cdev)) {
- int ret = PTR_ERR(cdev);
+ ret = cpufreq_cooling_register(data->policy);
+ if (ret) {
dev_err(bgp->dev, "Failed to register cpu cooling device %d\n",
ret);
cpufreq_cpu_put(data->policy);
diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
index 89f469ee4be4..98f7c8a9cab6 100644
--- a/include/linux/cpu_cooling.h
+++ b/include/linux/cpu_cooling.h
@@ -24,8 +24,7 @@ struct cpufreq_policy;
* cpufreq_cooling_register - function to create cpufreq cooling device.
* @policy: cpufreq policy.
*/
-struct thermal_cooling_device *
-cpufreq_cooling_register(struct cpufreq_policy *policy);
+int cpufreq_cooling_register(struct cpufreq_policy *policy);
/**
* cpufreq_cooling_unregister - function to remove cpufreq cooling device.
@@ -34,10 +33,9 @@ cpufreq_cooling_register(struct cpufreq_policy *policy);
void cpufreq_cooling_unregister(struct cpufreq_policy *policy);
#else /* !CONFIG_CPU_THERMAL */
-static inline struct thermal_cooling_device *
-cpufreq_cooling_register(struct cpufreq_policy *policy)
+static inline int cpufreq_cooling_register(struct cpufreq_policy *policy)
{
- return ERR_PTR(-ENOSYS);
+ return -ENOSYS;
}
static inline
@@ -52,13 +50,11 @@ void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
* of_cpufreq_cooling_register - create cpufreq cooling device based on DT.
* @policy: cpufreq policy.
*/
-struct thermal_cooling_device *
-of_cpufreq_cooling_register(struct cpufreq_policy *policy);
+int of_cpufreq_cooling_register(struct cpufreq_policy *policy);
#else
-static inline struct thermal_cooling_device *
-of_cpufreq_cooling_register(struct cpufreq_policy *policy)
+static inline int of_cpufreq_cooling_register(struct cpufreq_policy *policy)
{
- return NULL;
+ return -ENOSYS;
}
#endif /* defined(CONFIG_THERMAL_OF) && defined(CONFIG_CPU_THERMAL) */
--
2.17.1
^ permalink raw reply related
* [PATCH V4 1/3] cpufreq: Move the IS_ENABLED(CPU_THERMAL) macro in a stub
From: Daniel Lezcano @ 2019-06-27 21:02 UTC (permalink / raw)
To: viresh.kumar
Cc: rjw, edubezval, linux-kernel,
open list:CPU FREQUENCY SCALING FRAMEWORK
The cpufreq_online and the cpufreq_offline [un]register the driver as
a cooling device. This is done if the driver is flagged as a cooling
device in addition with a IS_ENABLED macro to compile out the branching
code.
Group this test in a stub function added in the cpufreq header instead
of having the IS_ENABLED in the code path.
Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 6 ++----
include/linux/cpufreq.h | 6 ++++++
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 85ff958e01f1..aee024e42618 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1378,8 +1378,7 @@ static int cpufreq_online(unsigned int cpu)
if (cpufreq_driver->ready)
cpufreq_driver->ready(policy);
- if (IS_ENABLED(CONFIG_CPU_THERMAL) &&
- cpufreq_driver->flags & CPUFREQ_IS_COOLING_DEV)
+ if (cpufreq_thermal_control_enabled(cpufreq_driver))
policy->cdev = of_cpufreq_cooling_register(policy);
pr_debug("initialization complete\n");
@@ -1469,8 +1468,7 @@ static int cpufreq_offline(unsigned int cpu)
goto unlock;
}
- if (IS_ENABLED(CONFIG_CPU_THERMAL) &&
- cpufreq_driver->flags & CPUFREQ_IS_COOLING_DEV) {
+ if (cpufreq_thermal_control_enabled(cpufreq_driver)) {
cpufreq_cooling_unregister(policy->cdev);
policy->cdev = NULL;
}
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index d01a74fbc4db..a1467aa7f58b 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -409,6 +409,12 @@ int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
const char *cpufreq_get_current_driver(void);
void *cpufreq_get_driver_data(void);
+static inline int cpufreq_thermal_control_enabled(struct cpufreq_driver *drv)
+{
+ return IS_ENABLED(CONFIG_CPU_THERMAL) &&
+ (drv->flags & CPUFREQ_IS_COOLING_DEV);
+}
+
static inline void cpufreq_verify_within_limits(struct cpufreq_policy *policy,
unsigned int min, unsigned int max)
{
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 43/87] kernel: power: replace kmalloc and memset with kzalloc
From: Pavel Machek @ 2019-06-27 19:56 UTC (permalink / raw)
To: Fuqian Huang; +Cc: Rafael J. Wysocki, Len Brown, linux-pm, linux-kernel
In-Reply-To: <20190627174048.4234-1-huangfq.daxian@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 805 bytes --]
On Fri 2019-06-28 01:40:48, Fuqian Huang wrote:
> kmalloc + memset(0) -> kzalloc
>
> Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
> @@ -974,12 +974,11 @@ static int get_swap_reader(struct swap_map_handle *handle,
> last = handle->maps = NULL;
> offset = swsusp_header->image;
> while (offset) {
> - tmp = kmalloc(sizeof(*handle->maps), GFP_KERNEL);
> + tmp = kzalloc(sizeof(*handle->maps), GFP_KERNEL);
> if (!tmp) {
> release_swap_reader(handle);
> return -ENOMEM;
> }
> - memset(tmp, 0, sizeof(*tmp));
> if (!handle->maps)
> handle->maps = tmp;
> if (last)
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH v4 1/2] power: supply: add input power and voltage limit properties
From: Sebastian Reichel @ 2019-06-27 18:37 UTC (permalink / raw)
To: Enric Balletbo Serra
Cc: Benson Leung, Enric Balletbo i Serra, Linux PM list, Sameer Nanda,
Benson Leung, Rafael J. Wysocki, Gwendal Grignou, linux-kernel,
Len Brown, Guenter Roeck, Adam.Thomson.Opensource,
Collabora Kernel ML, Pavel Machek
In-Reply-To: <CAFqH_52mdHKGN6gA3HuRJrcBvG1GpE+LzAyLgJD_xeZAiHruFw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 8857 bytes --]
Hi,
I just queued both patches. Thanks for the detailed description and
documentation.
-- Sebastian
On Tue, Jun 25, 2019 at 12:04:13PM +0200, Enric Balletbo Serra wrote:
> Hi Sebastian, Pavel,
>
> Missatge de Benson Leung <bleung@google.com> del dia dj., 23 de maig
> 2019 a les 21:55:
> >
> > Hi Enric,
> >
> > On Tue, May 07, 2019 at 11:52:47AM +0200, Enric Balletbo i Serra wrote:
> > > For thermal management strategy you might be interested on limit the
> > > input power for a power supply. We already have current limit but
> > > basically what we probably want is to limit power. So, introduce the
> > > input_power_limit property.
> > >
> > > Although the common use case is limit the input power, in some
> > > specific cases it is the voltage that is problematic (i.e some regulators
> > > have different efficiencies at higher voltage resulting in more heat).
> > > So introduce also the input_voltage_limit property.
> > >
> > > This happens in one Chromebook and is used on the Pixel C's thermal
> > > management strategy to effectively limit the input power to 5V 3A when
> > > the screen is on. When the screen is on, the display, the CPU, and the GPU
> > > all contribute more heat to the system than while the screen is off, and
> > > we made a tradeoff to throttle the charger in order to give more of the
> > > thermal budget to those other components.
> > >
> > > So there's nothing fundamentally broken about the hardware that would
> > > cause the Pixel C to malfunction if we were charging at 9V or 12V instead
> > > of 5V when the screen is on, i.e. if userspace doesn't change this.
> > >
> > > What would happen is that you wouldn't meet Google's skin temperature
> > > targets on the system if the charger was allowed to run at 9V or 12V with
> > > the screen on.
> > >
> > > For folks hacking on Pixel Cs (which is now outside of Google's official
> > > support window for Android) and customizing their own kernel and userspace
> > > this would be acceptable, but we wanted to expose this feature in the
> > > power supply properties because the feature does exist in the Emedded
> > > Controller firmware of the Pixel C and all of Google's Chromebooks with
> > > USB-C made since 2015 in case someone running an up to date kernel wanted
> > > to limit the charging power for thermal or other reasons.
> > >
> > > This patch exposes a new property, similar to input current limit, to
> > > re-configure the maximum voltage from the external supply at runtime
> > > based on system-level knowledge or user input.
> > >
> > > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > > Reviewed-by: Guenter Roeck <groeck@chromium.org>
> > > Acked-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> >
> > Reviewed-by: Benson Leung <bleung@chromium.org>
> >
>
> We're close to the merge window so I'm wondering if there are more
> concerns on this patchset?
>
> Thanks,
> ~ Enric
>
> > > ---
> > >
> > > Changes in v4:
> > > - Add also input_power_limit.
> > >
> > > Changes in v3:
> > > - Improve commit log and documentation with Benson comments.
> > >
> > > Changes in v2:
> > > - Document the new property in ABI/testing/sysfs-class-power.
> > > - Add the Reviewed-by Guenter Roeck tag.
> > >
> > > Documentation/ABI/testing/sysfs-class-power | 32 +++++++++++++++++++++
> > > Documentation/power/power_supply_class.txt | 4 +++
> > > drivers/power/supply/power_supply_sysfs.c | 2 ++
> > > include/linux/power_supply.h | 2 ++
> > > 4 files changed, 40 insertions(+)
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
> > > index 5e23e22dce1b..962a27a1daf8 100644
> > > --- a/Documentation/ABI/testing/sysfs-class-power
> > > +++ b/Documentation/ABI/testing/sysfs-class-power
> > > @@ -331,10 +331,42 @@ Description:
> > > supply. Normally this is configured based on the type of
> > > connection made (e.g. A configured SDP should output a maximum
> > > of 500mA so the input current limit is set to the same value).
> > > + Use preferably input_power_limit, and for problems that can be
> > > + solved using power limit use input_current_limit.
> > >
> > > Access: Read, Write
> > > Valid values: Represented in microamps
> > >
> > > +What: /sys/class/power_supply/<supply_name>/input_voltage_limit
> > > +Date: May 2019
> > > +Contact: linux-pm@vger.kernel.org
> > > +Description:
> > > + This entry configures the incoming VBUS voltage limit currently
> > > + set in the supply. Normally this is configured based on
> > > + system-level knowledge or user input (e.g. This is part of the
> > > + Pixel C's thermal management strategy to effectively limit the
> > > + input power to 5V when the screen is on to meet Google's skin
> > > + temperature targets). Note that this feature should not be
> > > + used for safety critical things.
> > > + Use preferably input_power_limit, and for problems that can be
> > > + solved using power limit use input_voltage_limit.
> > > +
> > > + Access: Read, Write
> > > + Valid values: Represented in microvolts
> > > +
> > > +What: /sys/class/power_supply/<supply_name>/input_power_limit
> > > +Date: May 2019
> > > +Contact: linux-pm@vger.kernel.org
> > > +Description:
> > > + This entry configures the incoming power limit currently set
> > > + in the supply. Normally this is configured based on
> > > + system-level knowledge or user input. Use preferably this
> > > + feature to limit the incoming power and use current/voltage
> > > + limit only for problems that can be solved using power limit.
> > > +
> > > + Access: Read, Write
> > > + Valid values: Represented in microwatts
> > > +
> > > What: /sys/class/power_supply/<supply_name>/online,
> > > Date: May 2007
> > > Contact: linux-pm@vger.kernel.org
> > > diff --git a/Documentation/power/power_supply_class.txt b/Documentation/power/power_supply_class.txt
> > > index 300d37896e51..1e3c705111db 100644
> > > --- a/Documentation/power/power_supply_class.txt
> > > +++ b/Documentation/power/power_supply_class.txt
> > > @@ -137,6 +137,10 @@ power supply object.
> > >
> > > INPUT_CURRENT_LIMIT - input current limit programmed by charger. Indicates
> > > the current drawn from a charging source.
> > > +INPUT_VOLTAGE_LIMIT - input voltage limit programmed by charger. Indicates
> > > +the voltage limit from a charging source.
> > > +INPUT_POWER_LIMIT - input power limit programmed by charger. Indicates
> > > +the power limit from a charging source.
> > >
> > > CHARGE_CONTROL_LIMIT - current charge control limit setting
> > > CHARGE_CONTROL_LIMIT_MAX - maximum charge control limit setting
> > > diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> > > index 5358a80d854f..860db617d241 100644
> > > --- a/drivers/power/supply/power_supply_sysfs.c
> > > +++ b/drivers/power/supply/power_supply_sysfs.c
> > > @@ -275,6 +275,8 @@ static struct device_attribute power_supply_attrs[] = {
> > > POWER_SUPPLY_ATTR(charge_control_limit),
> > > POWER_SUPPLY_ATTR(charge_control_limit_max),
> > > POWER_SUPPLY_ATTR(input_current_limit),
> > > + POWER_SUPPLY_ATTR(input_voltage_limit),
> > > + POWER_SUPPLY_ATTR(input_power_limit),
> > > POWER_SUPPLY_ATTR(energy_full_design),
> > > POWER_SUPPLY_ATTR(energy_empty_design),
> > > POWER_SUPPLY_ATTR(energy_full),
> > > diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> > > index 2f9c201a54d1..ba135a5d8996 100644
> > > --- a/include/linux/power_supply.h
> > > +++ b/include/linux/power_supply.h
> > > @@ -122,6 +122,8 @@ enum power_supply_property {
> > > POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT,
> > > POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX,
> > > POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> > > + POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT,
> > > + POWER_SUPPLY_PROP_INPUT_POWER_LIMIT,
> > > POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
> > > POWER_SUPPLY_PROP_ENERGY_EMPTY_DESIGN,
> > > POWER_SUPPLY_PROP_ENERGY_FULL,
> > > --
> > > 2.20.1
> > >
> >
> > --
> > Benson Leung
> > Staff Software Engineer
> > Chrome OS Kernel
> > Google Inc.
> > bleung@google.com
> > Chromium OS Project
> > bleung@chromium.org
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v9 1/2] power_supply: wilco_ec: Add charging config driver
From: Sebastian Reichel @ 2019-06-27 18:36 UTC (permalink / raw)
To: Nick Crews
Cc: enric.balletbo, bleung, linux-pm, linux-kernel, dlaurie, lamzin,
bartfab, derat, dtor, sjg, jchwong, tbroch
In-Reply-To: <20190508203826.120741-1-ncrews@chromium.org>
[-- Attachment #1: Type: text/plain, Size: 11905 bytes --]
Hi,
On Wed, May 08, 2019 at 02:38:25PM -0600, Nick Crews wrote:
> Add a driver to control the charging algorithm used on Wilco
> devices. See Documentation/ABI/testing/sysfs-class-power-wilco
> for the userspace interface and other info.
>
> Signed-off-by: Nick Crews <ncrews@chromium.org>
> Reviewed-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Thanks, queued.
-- Sebastian
> ---
> v9 changes:
> -Split up adding the driver and adding the device.
> v8 changes:
> -Several documentation and comment fixups.
> v6 changes:
> -Remove CHARGE_MODE_ILLEGAL from enum charge_mode. It's not a
> public type, and error checking could be performed in other ways.
> -Split up the commit so properties are added in a first commit
> -Move CONFIG_CHARGER_WILCO to the power/supply Kconfig
> -Use PTR_ERR_OR_ZERO() macro in probe()
> v5 changes:
> -Remove OP_SYNC, it has no immediate use case.
> -Merge properties.h into wilco-ec.h
> -Remove enum get_set_sync_op from the public interface,
> since without OP_SYNC they are irrelevant.
> -Fix Kconfigs and Makefiles so they actually work
> with the v4 changes
> -Tweak some formatting, spacing, and comments
> -Fix validation of charge_type so illegal values
> can't be set. Before negative error codes were
> accidentally getting casted to positive numbers
> -Remove more unneeded parentheses.
> v4 changes:
> -Use put_unaligned_le32() to store PID in request.
> -Move implementation from
> drivers/platform/chrome/wilco_ec/charge_config.c to
> drivers/power/supply/wilco_charger.c
> -Move drivers/platform/chrome/wilco_ec/properties.h to
> include/linux/platform_data/wilco-ec-properties.h
> -Remove parentheses in switch statement in psp_val_to_charge_mode()
> -Check for any negatvie return code from psp_val_to_charge_mode()
> instead of just -EINVAL so its less brittle
> -Tweak comments in wilco-ec-properties.h
> v3 changes:
> -Add this changelog
> -Fix commit message tags
> v2 changes:
> -Update Documentation to say KernelVersion 5.2
> -Update Documentation to explain Trickle mode better.
> -rename things from using *PCC* to *CHARGE*
> -Split up conversions between POWER_SUPPLY_PROP_CHARGE_TYPE values
> and Wilco EC codes
> -Use devm_ flavor of power_supply_register(), which simplifies things
> -Add extra error checking on property messages received from the EC
> -Fix bug in memcpy() calls in properties.c
> -Refactor fill_property_id()
> -Add valid input checks to charge_type
> -Properly convert charge_type when get()ting
>
> .../ABI/testing/sysfs-class-power-wilco | 30 +++
> drivers/power/supply/Kconfig | 9 +
> drivers/power/supply/Makefile | 1 +
> drivers/power/supply/wilco-charger.c | 187 ++++++++++++++++++
> 4 files changed, 227 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-class-power-wilco
> create mode 100644 drivers/power/supply/wilco-charger.c
>
> diff --git a/Documentation/ABI/testing/sysfs-class-power-wilco b/Documentation/ABI/testing/sysfs-class-power-wilco
> new file mode 100644
> index 000000000000..da1d6ffe5e3c
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-power-wilco
> @@ -0,0 +1,30 @@
> +What: /sys/class/power_supply/wilco-charger/charge_type
> +Date: April 2019
> +KernelVersion: 5.2
> +Description:
> + What charging algorithm to use:
> +
> + Standard: Fully charges battery at a standard rate.
> + Adaptive: Battery settings adaptively optimized based on
> + typical battery usage pattern.
> + Fast: Battery charges over a shorter period.
> + Trickle: Extends battery lifespan, intended for users who
> + primarily use their Chromebook while connected to AC.
> + Custom: A low and high threshold percentage is specified.
> + Charging begins when level drops below
> + charge_control_start_threshold, and ceases when
> + level is above charge_control_end_threshold.
> +
> +What: /sys/class/power_supply/wilco-charger/charge_control_start_threshold
> +Date: April 2019
> +KernelVersion: 5.2
> +Description:
> + Used when charge_type="Custom", as described above. Measured in
> + percentages. The valid range is [50, 95].
> +
> +What: /sys/class/power_supply/wilco-charger/charge_control_end_threshold
> +Date: April 2019
> +KernelVersion: 5.2
> +Description:
> + Used when charge_type="Custom", as described above. Measured in
> + percentages. The valid range is [55, 100].
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index e901b9879e7e..0c67eff871c8 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -660,4 +660,13 @@ config FUEL_GAUGE_SC27XX
> Say Y here to enable support for fuel gauge with SC27XX
> PMIC chips.
>
> +config CHARGER_WILCO
> + tristate "Wilco EC based charger for ChromeOS"
> + depends on WILCO_EC
> + help
> + Say Y here to enable control of the charging routines performed
> + by the Embedded Controller on the Chromebook named Wilco. Further
> + information can be found in
> + Documentation/ABI/testing/sysfs-class-power-wilco
> +
> endif # POWER_SUPPLY
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index b731c2a9b695..2b603a142701 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -87,3 +87,4 @@ obj-$(CONFIG_AXP288_CHARGER) += axp288_charger.o
> obj-$(CONFIG_CHARGER_CROS_USBPD) += cros_usbpd-charger.o
> obj-$(CONFIG_CHARGER_SC2731) += sc2731_charger.o
> obj-$(CONFIG_FUEL_GAUGE_SC27XX) += sc27xx_fuel_gauge.o
> +obj-$(CONFIG_CHARGER_WILCO) += wilco-charger.o
> diff --git a/drivers/power/supply/wilco-charger.c b/drivers/power/supply/wilco-charger.c
> new file mode 100644
> index 000000000000..b3c6d7cdd731
> --- /dev/null
> +++ b/drivers/power/supply/wilco-charger.c
> @@ -0,0 +1,187 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Charging control driver for the Wilco EC
> + *
> + * Copyright 2019 Google LLC
> + *
> + * See Documentation/ABI/testing/sysfs-class-power and
> + * Documentation/ABI/testing/sysfs-class-power-wilco for userspace interface
> + * and other info.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/platform_data/wilco-ec.h>
> +#include <linux/power_supply.h>
> +
> +#define DRV_NAME "wilco-charger"
> +
> +/* Property IDs and related EC constants */
> +#define PID_CHARGE_MODE 0x0710
> +#define PID_CHARGE_LOWER_LIMIT 0x0711
> +#define PID_CHARGE_UPPER_LIMIT 0x0712
> +
> +enum charge_mode {
> + CHARGE_MODE_STD = 1, /* Used for Standard */
> + CHARGE_MODE_EXP = 2, /* Express Charge, used for Fast */
> + CHARGE_MODE_AC = 3, /* Mostly AC use, used for Trickle */
> + CHARGE_MODE_AUTO = 4, /* Used for Adaptive */
> + CHARGE_MODE_CUSTOM = 5, /* Used for Custom */
> +};
> +
> +#define CHARGE_LOWER_LIMIT_MIN 50
> +#define CHARGE_LOWER_LIMIT_MAX 95
> +#define CHARGE_UPPER_LIMIT_MIN 55
> +#define CHARGE_UPPER_LIMIT_MAX 100
> +
> +/* Convert from POWER_SUPPLY_PROP_CHARGE_TYPE value to the EC's charge mode */
> +static int psp_val_to_charge_mode(int psp_val)
> +{
> + switch (psp_val) {
> + case POWER_SUPPLY_CHARGE_TYPE_TRICKLE:
> + return CHARGE_MODE_AC;
> + case POWER_SUPPLY_CHARGE_TYPE_FAST:
> + return CHARGE_MODE_EXP;
> + case POWER_SUPPLY_CHARGE_TYPE_STANDARD:
> + return CHARGE_MODE_STD;
> + case POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE:
> + return CHARGE_MODE_AUTO;
> + case POWER_SUPPLY_CHARGE_TYPE_CUSTOM:
> + return CHARGE_MODE_CUSTOM;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +/* Convert from EC's charge mode to POWER_SUPPLY_PROP_CHARGE_TYPE value */
> +static int charge_mode_to_psp_val(enum charge_mode mode)
> +{
> + switch (mode) {
> + case CHARGE_MODE_AC:
> + return POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
> + case CHARGE_MODE_EXP:
> + return POWER_SUPPLY_CHARGE_TYPE_FAST;
> + case CHARGE_MODE_STD:
> + return POWER_SUPPLY_CHARGE_TYPE_STANDARD;
> + case CHARGE_MODE_AUTO:
> + return POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE;
> + case CHARGE_MODE_CUSTOM:
> + return POWER_SUPPLY_CHARGE_TYPE_CUSTOM;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static enum power_supply_property wilco_charge_props[] = {
> + POWER_SUPPLY_PROP_CHARGE_TYPE,
> + POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD,
> + POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD,
> +};
> +
> +static int wilco_charge_get_property(struct power_supply *psy,
> + enum power_supply_property psp,
> + union power_supply_propval *val)
> +{
> + struct wilco_ec_device *ec = power_supply_get_drvdata(psy);
> + u32 property_id;
> + int ret;
> + u8 raw;
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_CHARGE_TYPE:
> + property_id = PID_CHARGE_MODE;
> + break;
> + case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD:
> + property_id = PID_CHARGE_LOWER_LIMIT;
> + break;
> + case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
> + property_id = PID_CHARGE_UPPER_LIMIT;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + ret = wilco_ec_get_byte_property(ec, property_id, &raw);
> + if (ret < 0)
> + return ret;
> + if (property_id == PID_CHARGE_MODE) {
> + ret = charge_mode_to_psp_val(raw);
> + if (ret < 0)
> + return -EBADMSG;
> + raw = ret;
> + }
> + val->intval = raw;
> +
> + return 0;
> +}
> +
> +static int wilco_charge_set_property(struct power_supply *psy,
> + enum power_supply_property psp,
> + const union power_supply_propval *val)
> +{
> + struct wilco_ec_device *ec = power_supply_get_drvdata(psy);
> + int mode;
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_CHARGE_TYPE:
> + mode = psp_val_to_charge_mode(val->intval);
> + if (mode < 0)
> + return -EINVAL;
> + return wilco_ec_set_byte_property(ec, PID_CHARGE_MODE, mode);
> + case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD:
> + if (val->intval < CHARGE_LOWER_LIMIT_MIN ||
> + val->intval > CHARGE_LOWER_LIMIT_MAX)
> + return -EINVAL;
> + return wilco_ec_set_byte_property(ec, PID_CHARGE_LOWER_LIMIT,
> + val->intval);
> + case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
> + if (val->intval < CHARGE_UPPER_LIMIT_MIN ||
> + val->intval > CHARGE_UPPER_LIMIT_MAX)
> + return -EINVAL;
> + return wilco_ec_set_byte_property(ec, PID_CHARGE_UPPER_LIMIT,
> + val->intval);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int wilco_charge_property_is_writeable(struct power_supply *psy,
> + enum power_supply_property psp)
> +{
> + return 1;
> +}
> +
> +static const struct power_supply_desc wilco_ps_desc = {
> + .properties = wilco_charge_props,
> + .num_properties = ARRAY_SIZE(wilco_charge_props),
> + .get_property = wilco_charge_get_property,
> + .set_property = wilco_charge_set_property,
> + .property_is_writeable = wilco_charge_property_is_writeable,
> + .name = DRV_NAME,
> + .type = POWER_SUPPLY_TYPE_MAINS,
> +};
> +
> +static int wilco_charge_probe(struct platform_device *pdev)
> +{
> + struct wilco_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
> + struct power_supply_config psy_cfg = {};
> + struct power_supply *psy;
> +
> + psy_cfg.drv_data = ec;
> + psy = devm_power_supply_register(&pdev->dev, &wilco_ps_desc, &psy_cfg);
> +
> + return PTR_ERR_OR_ZERO(psy);
> +}
> +
> +static struct platform_driver wilco_charge_driver = {
> + .probe = wilco_charge_probe,
> + .driver = {
> + .name = DRV_NAME,
> + }
> +};
> +module_platform_driver(wilco_charge_driver);
> +
> +MODULE_ALIAS("platform:" DRV_NAME);
> +MODULE_AUTHOR("Nick Crews <ncrews@chromium.org>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Wilco EC charge control driver");
> --
> 2.20.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] power: supply: fix semicolon.cocci warnings
From: Sebastian Reichel @ 2019-06-27 18:33 UTC (permalink / raw)
To: kbuild test robot
Cc: Andrey Smirnov, kbuild-all, Enric Balletbo Serra, Guenter Roeck,
linux-pm, linux-kernel
In-Reply-To: <20190510180347.GA130528@lkp-kbuild21>
[-- Attachment #1: Type: text/plain, Size: 1150 bytes --]
Hi,
On Sat, May 11, 2019 at 02:03:47AM +0800, kbuild test robot wrote:
> From: kbuild test robot <lkp@intel.com>
>
> drivers/power/supply/ucs1002_power.c:339:2-3: Unneeded semicolon
>
>
> Remove unneeded semicolon.
>
> Generated by: scripts/coccinelle/misc/semicolon.cocci
Thanks, queued.
-- Sebastian
> Fixes: 9a2688e42638 ("power: supply: Add driver for Microchip UCS1002")
> CC: Andrey Smirnov <andrew.smirnov@gmail.com>
> Signed-off-by: kbuild test robot <lkp@intel.com>
> ---
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
> head: c8f3c9fe83cafc94679007b9c0083ee25d5735e0
> commit: 9a2688e42638f2e057dbdfce82e221b38bfba0a7 [42/44] power: supply: Add driver for Microchip UCS1002
>
> ucs1002_power.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- a/drivers/power/supply/ucs1002_power.c
> +++ b/drivers/power/supply/ucs1002_power.c
> @@ -336,7 +336,7 @@ static int ucs1002_get_usb_type(struct u
> case F_ACTIVE_MODE_BC12_CDP:
> type = POWER_SUPPLY_USB_TYPE_CDP;
> break;
> - };
> + }
>
> val->intval = type;
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v4 0/2] Use NVMEM as reboot-mode write interface
From: Sebastian Reichel @ 2019-06-27 18:33 UTC (permalink / raw)
To: Han Nandor
Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
robh+dt@kernel.org, mark.rutland@arm.com,
devicetree@vger.kernel.org
In-Reply-To: <20190515104658.25535-1-nandor.han@vaisala.com>
[-- Attachment #1: Type: text/plain, Size: 1938 bytes --]
Hi,
On Wed, May 15, 2019 at 10:47:14AM +0000, Han Nandor wrote:
> Description
> -----------
> Extend the reboot mode driver to use a NVMEM cell as writing interface.
>
> Testing
> -------
> The testing is done by configuring DT from a custom board.
> The NVMEM cell is configured in an RTC non-volatile memory.
> Kernel: 4.14.60 (the patchset was rebased on kernel master)
>
> DT configurations:
> `
> ...
> reboot-mode-nvmem@0 {
> compatible = "simple-mfd";
> reboot-mode {
> compatible = "nvmem-reboot-mode";
> nvmem-cells = <&reboot_mode>;
> nvmem-cell-names = "reboot-mode";
>
> mode-test = <0x21969147>;
> };
> };
> ...
> reboot_mode: nvmem_reboot_mode@0 {
> reg = <0x00 0x4>;
> };
> ...
> `
>
> 1. Reboot the system using the command `reboot test`
>
> 2. Verify that kernel logs show that reboot was done in mode `test`:
> PASS
> `[ 413.957172] reboot: Restarting system with command 'test' `
>
> 3. Stop in U-Boot and verify that mode `test` magic value is present
> in RTCs non-volatile memory: PASS
>
> Kernel: 5.1.0-rc3
>
> 1. Configure `arch/arm/configs/imx_v6_v7_defconfig` to contain
> `CONFIG_NVMEM_REBOOT_MODE=y`
> 2. Verify that Kernel compiles successful: PASS
> `
> make ARCH=arm CROSS_COMPILE=arm-linux-gnu- imx_v6_v7_defconfig zImage
> ...
> CC drivers/power/reset/nvmem-reboot-mode.o
> ...
> Kernel: arch/arm/boot/zImage is ready
> `
> Changes since v1:
> -----------------
> - split the documentation on a separate patch
> - add a missing header
>
> Changes since v2:
> ----------------
> - change the module license to GPL since GPL v2 is deprecated
>
> Changes since v3:
> ----------------
> - documentation updated according to the comments
Thanks, queued. Please fix your git/mail setup, I had to fix the
line endings (\r\n -> \n) to apply this.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 00/34] treewide: simplify getting the adapter of an I2C client
From: Sebastian Reichel @ 2019-06-27 18:28 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-i2c, linux-renesas-soc, devel, dri-devel, linux-arm-kernel,
linux-clk, linux-iio, linux-kernel, linux-leds, linux-media,
linux-mtd, linux-pm, linux-rtc, linux-usb
In-Reply-To: <20190608105619.593-1-wsa+renesas@sang-engineering.com>
[-- Attachment #1: Type: text/plain, Size: 5906 bytes --]
Hi,
On Sat, Jun 08, 2019 at 12:55:39PM +0200, Wolfram Sang wrote:
> While preparing a refactoring series, I noticed that some drivers use a
> complicated way of determining the adapter of a client. The easy way is
> to use the intended pointer: client->adapter
>
> These drivers do:
> to_i2c_adapter(client->dev.parent);
>
> The I2C core populates the parent pointer as:
> client->dev.parent = &client->adapter->dev;
>
> Now take into consideration that
> to_i2c_adapter(&adapter->dev);
>
> is a complicated way of saying 'adapter', then we can even formally
> prove that the complicated expression can be simplified by using
> client->adapter.
>
> The conversion was done using a coccinelle script with some manual
> indentation fixes applied on top.
>
> To avoid a brown paper bag mistake, I double checked this on a Renesas
> Salvator-XS board (R-Car M3N) and verified both expression result in the
> same pointer. Other than that, the series is only build tested.
>
> A branch can be found here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/no_to_adapter
>
> Please apply the patches to the individual subsystem trees. There are no
> dependencies.
>
> Thanks and kind regards,
>
> Wolfram
Thanks, I queued the patches prefixed with "power: supply: [...]".
-- Sebastian
> Wolfram Sang (34):
> clk: clk-cdce706: simplify getting the adapter of a client
> gpu: drm: bridge: sii9234: simplify getting the adapter of a client
> iio: light: bh1780: simplify getting the adapter of a client
> leds: leds-pca955x: simplify getting the adapter of a client
> leds: leds-tca6507: simplify getting the adapter of a client
> media: i2c: ak881x: simplify getting the adapter of a client
> media: i2c: mt9m001: simplify getting the adapter of a client
> media: i2c: mt9m111: simplify getting the adapter of a client
> media: i2c: mt9p031: simplify getting the adapter of a client
> media: i2c: ov2640: simplify getting the adapter of a client
> media: i2c: tw9910: simplify getting the adapter of a client
> misc: fsa9480: simplify getting the adapter of a client
> misc: isl29003: simplify getting the adapter of a client
> misc: tsl2550: simplify getting the adapter of a client
> mtd: maps: pismo: simplify getting the adapter of a client
> power: supply: bq24190_charger: simplify getting the adapter of a client
> power: supply: bq24257_charger: simplify getting the adapter of a client
> power: supply: bq25890_charger: simplify getting the adapter of a client
> power: supply: max14656_charger_detector: simplify getting the adapter
> of a client
> power: supply: max17040_battery: simplify getting the adapter of a client
> power: supply: max17042_battery: simplify getting the adapter of a client
> power: supply: rt5033_battery: simplify getting the adapter of a client
> power: supply: rt9455_charger: simplify getting the adapter of a client
> power: supply: sbs-manager: simplify getting the adapter of a client
> regulator: max8952: simplify getting the adapter of a client
> rtc: fm3130: simplify getting the adapter of a client
> rtc: m41t80: simplify getting the adapter of a client
> rtc: rv8803: simplify getting the adapter of a client
> rtc: rx8010: simplify getting the adapter of a client
> rtc: rx8025: simplify getting the adapter of a client
> staging: media: soc_camera: imx074: simplify getting the adapter of a client
> staging: media: soc_camera: mt9t031: simplify getting the adapter of a client
> staging: media: soc_camera: soc_mt9v022: simplify getting the adapter
> of a client
> usb: typec: tcpm: fusb302: simplify getting the adapter of a client
>
> drivers/clk/clk-cdce706.c | 2 +-
> drivers/gpu/drm/bridge/sii9234.c | 4 ++--
> drivers/iio/light/bh1780.c | 2 +-
> drivers/leds/leds-pca955x.c | 2 +-
> drivers/leds/leds-tca6507.c | 2 +-
> drivers/media/i2c/ak881x.c | 2 +-
> drivers/media/i2c/mt9m001.c | 2 +-
> drivers/media/i2c/mt9m111.c | 2 +-
> drivers/media/i2c/mt9p031.c | 2 +-
> drivers/media/i2c/ov2640.c | 2 +-
> drivers/media/i2c/tw9910.c | 3 +--
> drivers/misc/fsa9480.c | 2 +-
> drivers/misc/isl29003.c | 2 +-
> drivers/misc/tsl2550.c | 2 +-
> drivers/mtd/maps/pismo.c | 2 +-
> drivers/power/supply/bq24190_charger.c | 2 +-
> drivers/power/supply/bq24257_charger.c | 2 +-
> drivers/power/supply/bq25890_charger.c | 2 +-
> drivers/power/supply/max14656_charger_detector.c | 2 +-
> drivers/power/supply/max17040_battery.c | 2 +-
> drivers/power/supply/max17042_battery.c | 2 +-
> drivers/power/supply/rt5033_battery.c | 2 +-
> drivers/power/supply/rt9455_charger.c | 2 +-
> drivers/power/supply/sbs-manager.c | 2 +-
> drivers/regulator/max8952.c | 2 +-
> drivers/rtc/rtc-fm3130.c | 8 +++-----
> drivers/rtc/rtc-m41t80.c | 2 +-
> drivers/rtc/rtc-rv8803.c | 2 +-
> drivers/rtc/rtc-rx8010.c | 2 +-
> drivers/rtc/rtc-rx8025.c | 2 +-
> drivers/staging/media/soc_camera/imx074.c | 2 +-
> drivers/staging/media/soc_camera/mt9t031.c | 2 +-
> drivers/staging/media/soc_camera/soc_mt9v022.c | 2 +-
> drivers/usb/typec/tcpm/fusb302.c | 3 +--
> 34 files changed, 37 insertions(+), 41 deletions(-)
>
> --
> 2.19.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v4 1/1] power: supply: Add HWMON compatibility layer
From: Sebastian Reichel @ 2019-06-27 18:27 UTC (permalink / raw)
To: Andrey Smirnov
Cc: linux-pm, Guenter Roeck, Chris Healy, Cory Tusar, Lucas Stach,
Fabio Estevam, linux-kernel
In-Reply-To: <20190612084404.21685-2-andrew.smirnov@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 14360 bytes --]
Hi,
On Wed, Jun 12, 2019 at 01:44:04AM -0700, Andrey Smirnov wrote:
> Add code implementing HWMON adapter/compatibility layer to allow
> expositing various sensors present on power supply devices via HWMON
> subsystem. This is done in order to allow userspace to use single
> ABI/library(libsensors) to access/manipulate all of the sensors of the
> system.
>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> Tested-by: Chris Healy <cphealy@gmail.com>
> ---
Thanks, queued.
-- Sebastian
> drivers/power/supply/Kconfig | 14 +
> drivers/power/supply/Makefile | 1 +
> drivers/power/supply/power_supply_core.c | 7 +
> drivers/power/supply/power_supply_hwmon.c | 355 ++++++++++++++++++++++
> include/linux/power_supply.h | 13 +
> 5 files changed, 390 insertions(+)
> create mode 100644 drivers/power/supply/power_supply_hwmon.c
>
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index c614c8a196f3..0550cedb53c9 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -14,6 +14,20 @@ config POWER_SUPPLY_DEBUG
> Say Y here to enable debugging messages for power supply class
> and drivers.
>
> +config POWER_SUPPLY_HWMON
> + bool
> + prompt "Expose power supply sensors as hwmon device"
> + depends on HWMON=y || HWMON=POWER_SUPPLY
> + default y
> + help
> + This options enables API that allows sensors found on a
> + power supply device (current, voltage, temperature) to be
> + exposed as a hwmon device.
> +
> + Say 'Y' here if you want power supplies to
> + have hwmon sysfs interface too.
> +
> +
> config PDA_POWER
> tristate "Generic PDA/phone power driver"
> depends on !S390
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index c56803a9e4fe..0a87cfe49b21 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -6,6 +6,7 @@ power_supply-$(CONFIG_SYSFS) += power_supply_sysfs.o
> power_supply-$(CONFIG_LEDS_TRIGGERS) += power_supply_leds.o
>
> obj-$(CONFIG_POWER_SUPPLY) += power_supply.o
> +obj-$(CONFIG_POWER_SUPPLY_HWMON) += power_supply_hwmon.o
> obj-$(CONFIG_GENERIC_ADC_BATTERY) += generic-adc-battery.o
>
> obj-$(CONFIG_PDA_POWER) += pda_power.o
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index c917a8b43b2b..67e78538bb9c 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -1062,6 +1062,10 @@ __power_supply_register(struct device *parent,
> if (rc)
> goto create_triggers_failed;
>
> + rc = power_supply_add_hwmon_sysfs(psy);
> + if (rc)
> + goto add_hwmon_sysfs_failed;
> +
> /*
> * Update use_cnt after any uevents (most notably from device_add()).
> * We are here still during driver's probe but
> @@ -1080,6 +1084,8 @@ __power_supply_register(struct device *parent,
>
> return psy;
>
> +add_hwmon_sysfs_failed:
> + power_supply_remove_triggers(psy);
> create_triggers_failed:
> psy_unregister_cooler(psy);
> register_cooler_failed:
> @@ -1232,6 +1238,7 @@ void power_supply_unregister(struct power_supply *psy)
> cancel_work_sync(&psy->changed_work);
> cancel_delayed_work_sync(&psy->deferred_register_work);
> sysfs_remove_link(&psy->dev.kobj, "powers");
> + power_supply_remove_hwmon_sysfs(psy);
> power_supply_remove_triggers(psy);
> psy_unregister_cooler(psy);
> psy_unregister_thermal(psy);
> diff --git a/drivers/power/supply/power_supply_hwmon.c b/drivers/power/supply/power_supply_hwmon.c
> new file mode 100644
> index 000000000000..51fe60440d12
> --- /dev/null
> +++ b/drivers/power/supply/power_supply_hwmon.c
> @@ -0,0 +1,355 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * power_supply_hwmon.c - power supply hwmon support.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/power_supply.h>
> +#include <linux/slab.h>
> +
> +struct power_supply_hwmon {
> + struct power_supply *psy;
> + unsigned long *props;
> +};
> +
> +static int power_supply_hwmon_in_to_property(u32 attr)
> +{
> + switch (attr) {
> + case hwmon_in_average:
> + return POWER_SUPPLY_PROP_VOLTAGE_AVG;
> + case hwmon_in_min:
> + return POWER_SUPPLY_PROP_VOLTAGE_MIN;
> + case hwmon_in_max:
> + return POWER_SUPPLY_PROP_VOLTAGE_MAX;
> + case hwmon_in_input:
> + return POWER_SUPPLY_PROP_VOLTAGE_NOW;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int power_supply_hwmon_curr_to_property(u32 attr)
> +{
> + switch (attr) {
> + case hwmon_curr_average:
> + return POWER_SUPPLY_PROP_CURRENT_AVG;
> + case hwmon_curr_max:
> + return POWER_SUPPLY_PROP_CURRENT_MAX;
> + case hwmon_curr_input:
> + return POWER_SUPPLY_PROP_CURRENT_NOW;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int power_supply_hwmon_temp_to_property(u32 attr, int channel)
> +{
> + if (channel) {
> + switch (attr) {
> + case hwmon_temp_input:
> + return POWER_SUPPLY_PROP_TEMP_AMBIENT;
> + case hwmon_temp_min_alarm:
> + return POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MIN;
> + case hwmon_temp_max_alarm:
> + return POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MAX;
> + default:
> + break;
> + }
> + } else {
> + switch (attr) {
> + case hwmon_temp_input:
> + return POWER_SUPPLY_PROP_TEMP;
> + case hwmon_temp_max:
> + return POWER_SUPPLY_PROP_TEMP_MAX;
> + case hwmon_temp_min:
> + return POWER_SUPPLY_PROP_TEMP_MIN;
> + case hwmon_temp_min_alarm:
> + return POWER_SUPPLY_PROP_TEMP_ALERT_MIN;
> + case hwmon_temp_max_alarm:
> + return POWER_SUPPLY_PROP_TEMP_ALERT_MAX;
> + default:
> + break;
> + }
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int
> +power_supply_hwmon_to_property(enum hwmon_sensor_types type,
> + u32 attr, int channel)
> +{
> + switch (type) {
> + case hwmon_in:
> + return power_supply_hwmon_in_to_property(attr);
> + case hwmon_curr:
> + return power_supply_hwmon_curr_to_property(attr);
> + case hwmon_temp:
> + return power_supply_hwmon_temp_to_property(attr, channel);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static bool power_supply_hwmon_is_a_label(enum hwmon_sensor_types type,
> + u32 attr)
> +{
> + return type == hwmon_temp && attr == hwmon_temp_label;
> +}
> +
> +static bool power_supply_hwmon_is_writable(enum hwmon_sensor_types type,
> + u32 attr)
> +{
> + switch (type) {
> + case hwmon_in:
> + return attr == hwmon_in_min ||
> + attr == hwmon_in_max;
> + case hwmon_curr:
> + return attr == hwmon_curr_max;
> + case hwmon_temp:
> + return attr == hwmon_temp_max ||
> + attr == hwmon_temp_min ||
> + attr == hwmon_temp_min_alarm ||
> + attr == hwmon_temp_max_alarm;
> + default:
> + return false;
> + }
> +}
> +
> +static umode_t power_supply_hwmon_is_visible(const void *data,
> + enum hwmon_sensor_types type,
> + u32 attr, int channel)
> +{
> + const struct power_supply_hwmon *psyhw = data;
> + int prop;
> +
> +
> + if (power_supply_hwmon_is_a_label(type, attr))
> + return 0444;
> +
> + prop = power_supply_hwmon_to_property(type, attr, channel);
> + if (prop < 0 || !test_bit(prop, psyhw->props))
> + return 0;
> +
> + if (power_supply_property_is_writeable(psyhw->psy, prop) > 0 &&
> + power_supply_hwmon_is_writable(type, attr))
> + return 0644;
> +
> + return 0444;
> +}
> +
> +static int power_supply_hwmon_read_string(struct device *dev,
> + enum hwmon_sensor_types type,
> + u32 attr, int channel,
> + const char **str)
> +{
> + *str = channel ? "temp" : "temp ambient";
> + return 0;
> +}
> +
> +static int
> +power_supply_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + struct power_supply_hwmon *psyhw = dev_get_drvdata(dev);
> + struct power_supply *psy = psyhw->psy;
> + union power_supply_propval pspval;
> + int ret, prop;
> +
> + prop = power_supply_hwmon_to_property(type, attr, channel);
> + if (prop < 0)
> + return prop;
> +
> + ret = power_supply_get_property(psy, prop, &pspval);
> + if (ret)
> + return ret;
> +
> + switch (type) {
> + /*
> + * Both voltage and current is reported in units of
> + * microvolts/microamps, so we need to adjust it to
> + * milliamps(volts)
> + */
> + case hwmon_curr:
> + case hwmon_in:
> + pspval.intval = DIV_ROUND_CLOSEST(pspval.intval, 1000);
> + break;
> + /*
> + * Temp needs to be converted from 1/10 C to milli-C
> + */
> + case hwmon_temp:
> + if (check_mul_overflow(pspval.intval, 100,
> + &pspval.intval))
> + return -EOVERFLOW;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + *val = pspval.intval;
> +
> + return 0;
> +}
> +
> +static int
> +power_supply_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long val)
> +{
> + struct power_supply_hwmon *psyhw = dev_get_drvdata(dev);
> + struct power_supply *psy = psyhw->psy;
> + union power_supply_propval pspval;
> + int prop;
> +
> + prop = power_supply_hwmon_to_property(type, attr, channel);
> + if (prop < 0)
> + return prop;
> +
> + pspval.intval = val;
> +
> + switch (type) {
> + /*
> + * Both voltage and current is reported in units of
> + * microvolts/microamps, so we need to adjust it to
> + * milliamps(volts)
> + */
> + case hwmon_curr:
> + case hwmon_in:
> + if (check_mul_overflow(pspval.intval, 1000,
> + &pspval.intval))
> + return -EOVERFLOW;
> + break;
> + /*
> + * Temp needs to be converted from 1/10 C to milli-C
> + */
> + case hwmon_temp:
> + pspval.intval = DIV_ROUND_CLOSEST(pspval.intval, 100);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return power_supply_set_property(psy, prop, &pspval);
> +}
> +
> +static const struct hwmon_ops power_supply_hwmon_ops = {
> + .is_visible = power_supply_hwmon_is_visible,
> + .read = power_supply_hwmon_read,
> + .write = power_supply_hwmon_write,
> + .read_string = power_supply_hwmon_read_string,
> +};
> +
> +static const struct hwmon_channel_info *power_supply_hwmon_info[] = {
> + HWMON_CHANNEL_INFO(temp,
> + HWMON_T_LABEL |
> + HWMON_T_INPUT |
> + HWMON_T_MAX |
> + HWMON_T_MIN |
> + HWMON_T_MIN_ALARM |
> + HWMON_T_MIN_ALARM,
> +
> + HWMON_T_LABEL |
> + HWMON_T_INPUT |
> + HWMON_T_MIN_ALARM |
> + HWMON_T_LABEL |
> + HWMON_T_MAX_ALARM),
> +
> + HWMON_CHANNEL_INFO(curr,
> + HWMON_C_AVERAGE |
> + HWMON_C_MAX |
> + HWMON_C_INPUT),
> +
> + HWMON_CHANNEL_INFO(in,
> + HWMON_I_AVERAGE |
> + HWMON_I_MIN |
> + HWMON_I_MAX |
> + HWMON_I_INPUT),
> + NULL
> +};
> +
> +static const struct hwmon_chip_info power_supply_hwmon_chip_info = {
> + .ops = &power_supply_hwmon_ops,
> + .info = power_supply_hwmon_info,
> +};
> +
> +static void power_supply_hwmon_bitmap_free(void *data)
> +{
> + bitmap_free(data);
> +}
> +
> +int power_supply_add_hwmon_sysfs(struct power_supply *psy)
> +{
> + const struct power_supply_desc *desc = psy->desc;
> + struct power_supply_hwmon *psyhw;
> + struct device *dev = &psy->dev;
> + struct device *hwmon;
> + int ret, i;
> +
> + if (!devres_open_group(dev, power_supply_add_hwmon_sysfs,
> + GFP_KERNEL))
> + return -ENOMEM;
> +
> + psyhw = devm_kzalloc(dev, sizeof(*psyhw), GFP_KERNEL);
> + if (!psyhw) {
> + ret = -ENOMEM;
> + goto error;
> + }
> +
> + psyhw->psy = psy;
> + psyhw->props = bitmap_zalloc(POWER_SUPPLY_PROP_TIME_TO_FULL_AVG + 1,
> + GFP_KERNEL);
> + if (!psyhw->props) {
> + ret = -ENOMEM;
> + goto error;
> + }
> +
> + ret = devm_add_action(dev, power_supply_hwmon_bitmap_free,
> + psyhw->props);
> + if (ret)
> + goto error;
> +
> + for (i = 0; i < desc->num_properties; i++) {
> + const enum power_supply_property prop = desc->properties[i];
> +
> + switch (prop) {
> + case POWER_SUPPLY_PROP_CURRENT_AVG:
> + case POWER_SUPPLY_PROP_CURRENT_MAX:
> + case POWER_SUPPLY_PROP_CURRENT_NOW:
> + case POWER_SUPPLY_PROP_TEMP:
> + case POWER_SUPPLY_PROP_TEMP_MAX:
> + case POWER_SUPPLY_PROP_TEMP_MIN:
> + case POWER_SUPPLY_PROP_TEMP_ALERT_MIN:
> + case POWER_SUPPLY_PROP_TEMP_ALERT_MAX:
> + case POWER_SUPPLY_PROP_TEMP_AMBIENT:
> + case POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MIN:
> + case POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MAX:
> + case POWER_SUPPLY_PROP_VOLTAGE_AVG:
> + case POWER_SUPPLY_PROP_VOLTAGE_MIN:
> + case POWER_SUPPLY_PROP_VOLTAGE_MAX:
> + case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> + set_bit(prop, psyhw->props);
> + break;
> + default:
> + break;
> + }
> + }
> +
> + hwmon = devm_hwmon_device_register_with_info(dev, psy->desc->name,
> + psyhw,
> + &power_supply_hwmon_chip_info,
> + NULL);
> + ret = PTR_ERR_OR_ZERO(hwmon);
> + if (ret)
> + goto error;
> +
> + devres_close_group(dev, power_supply_add_hwmon_sysfs);
> + return 0;
> +error:
> + devres_release_group(dev, NULL);
> + return ret;
> +}
> +
> +void power_supply_remove_hwmon_sysfs(struct power_supply *psy)
> +{
> + devres_release_group(&psy->dev, power_supply_add_hwmon_sysfs);
> +}
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index bdab14c7ca4d..f850d4110935 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -475,4 +475,17 @@ static inline bool power_supply_is_watt_property(enum power_supply_property psp)
> return 0;
> }
>
> +#ifdef CONFIG_POWER_SUPPLY_HWMON
> +int power_supply_add_hwmon_sysfs(struct power_supply *psy);
> +void power_supply_remove_hwmon_sysfs(struct power_supply *psy);
> +#else
> +static inline int power_supply_add_hwmon_sysfs(struct power_supply *psy)
> +{
> + return 0;
> +}
> +
> +static inline
> +void power_supply_remove_hwmon_sysfs(struct power_supply *psy) {}
> +#endif
> +
> #endif /* __LINUX_POWER_SUPPLY_H__ */
> --
> 2.21.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] power: Do not clear events_check_enabled in pm_wakeup_pending()
From: Rafael J. Wysocki @ 2019-06-27 17:49 UTC (permalink / raw)
To: Ravi Chandra Sadineni
Cc: len.brown, pavel, gregkh, linux-pm, linux-kernel, tbroch, rajatja
In-Reply-To: <20190619175142.237794-1-ravisadineni@chromium.org>
On Wednesday, June 19, 2019 7:51:42 PM CEST Ravi Chandra Sadineni wrote:
> events_check_enabled bool is set when wakeup_count sysfs attribute
> is written. User level daemon is expected to write this attribute
> just before suspend.
>
> When this boolean is set, calls to pm_wakeup_event() will result in
> increment of per device and global wakeup count that helps in
> identifying the wake source. global wakeup count is also used by
> pm_wakeup_pending() to identify if there are any pending events that
> should result in an suspend abort.
>
> Currently calls to pm_wakeup_pending() also clears events_check_enabled.
> This can be a problem when there are multiple wake events or when the
> suspend is aborted due to an interrupt on a shared interrupt line.
> For example an Mfd device can create several platform devices which
> might fetch the state on resume in the driver resume method and increment
> the wakeup count if needed. But if events_check_enabled is cleared before
> resume methods get to execute, wakeup count will not be incremented. Thus
> let us not reset the bool here.
>
> Note that events_check_enabled is also cleared in suspend.c/enter_state()
> on every resume at the end.
>
> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> ---
> drivers/base/power/wakeup.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index 5b2b6a05a4f3..88aade871589 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -838,7 +838,6 @@ bool pm_wakeup_pending(void)
>
> split_counters(&cnt, &inpr);
> ret = (cnt != saved_count || inpr > 0);
> - events_check_enabled = !ret;
This effectively changes the meaning of the wakeup_count metric. so it cannot be applied.
> }
> raw_spin_unlock_irqrestore(&events_lock, flags);
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox