* [PATCH v2 0/5] Introduce Power domain based warming device driver
@ 2019-09-10 17:14 Thara Gopinath
2019-09-10 17:14 ` [PATCH v2 1/5] PM/Domains: Add support for retrieving genpd performance states information Thara Gopinath
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Thara Gopinath @ 2019-09-10 17:14 UTC (permalink / raw)
To: edubezval, rui.zhang, ulf.hansson, daniel.lezcano,
bjorn.andersson, robh+dt, agross
Cc: amit.kucheria, mark.rutland, rjw, linux-pm, devicetree,
linux-arm-msm, linux-kernel
Certain resources modeled as a generic power domain in linux kernel
can be used to warm up the SoC (mx power domain on sdm845)
if the temperature falls below certain threshold. These power domains
can be considered as thermal warming devices.
(opposite of thermal cooling devices).
In kernel, these warming devices can be modeled as a
thermal cooling device. In fact, since linux kernel today has
no instance of a resource modeled as a power domain acting as a
thermal warming device, a generic power domain based thermal warming device
driver that can be used pan-Socs is the approach taken in this
patch series. Since thermal warming devices can be thought of as the
mirror opposite of thermal cooling devices, this patch series re-uses
thermal cooling device framework. To use these power domains as warming
devices require further tweaks in the thermal framework which are out of
scope of this patch series.
The first patch in this series extends the genpd framework to export out
the performance states of a power domain so that when a power
domain is modeled as a cooling device, the number of possible states and
current state of the cooling device can be retrieved from the genpd
framework.
The second patch implements the newly added genpd callback for Qualcomm
RPMH power domain driver which hosts the mx power domain.
The third patch describes the dt binding required for a generic
power domain based warming device
The fourth patch introduces the generic power domain warming device driver
The fifth patch introduces the DT entreis for sdm845 to register mx power
domain as a thermal warming device
v1->v2:
- Rename the patch series from
"qcom: Model RPMH power domains as thermal cooling devices" to
"Introduce Power domain based thermal warming devices" as it is
more appropriate.
- Introduce a new patch(patch 3) describing the dt-bindings for generic power
domain warming device.
- Patch specific changes mentioned in respective patches.
Thara Gopinath (5):
PM/Domains: Add support for retrieving genpd performance states
information
soc: qcom: rpmhpd: Introduce function to retrieve power domain
performance state count
dt-bindings: thermal: Add generic power domain warming device binding
thermal: Add generic power domain warming device driver.
arm64: dts: qcom: Add node for RPMH power domain warming device on
sdm845.
.../bindings/thermal/pwr-domain-warming.txt | 32 ++++
arch/arm64/boot/dts/qcom/sdm845.dtsi | 7 +
drivers/base/power/domain.c | 37 +++++
drivers/soc/qcom/rpmhpd.c | 9 ++
drivers/thermal/Kconfig | 11 ++
drivers/thermal/Makefile | 2 +
drivers/thermal/pwr_domain_warming.c | 174 +++++++++++++++++++++
include/linux/pm_domain.h | 13 ++
8 files changed, 285 insertions(+)
create mode 100644 Documentation/devicetree/bindings/thermal/pwr-domain-warming.txt
create mode 100644 drivers/thermal/pwr_domain_warming.c
--
2.1.4
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v2 1/5] PM/Domains: Add support for retrieving genpd performance states information 2019-09-10 17:14 [PATCH v2 0/5] Introduce Power domain based warming device driver Thara Gopinath @ 2019-09-10 17:14 ` Thara Gopinath 2019-09-10 17:14 ` [PATCH v2 2/5] soc: qcom: rpmhpd: Introduce function to retrieve power domain performance state count Thara Gopinath ` (3 subsequent siblings) 4 siblings, 0 replies; 12+ messages in thread From: Thara Gopinath @ 2019-09-10 17:14 UTC (permalink / raw) To: edubezval, rui.zhang, ulf.hansson, daniel.lezcano, bjorn.andersson, robh+dt, agross Cc: amit.kucheria, mark.rutland, rjw, linux-pm, devicetree, linux-arm-msm, linux-kernel Add two new APIs in the genpd framework, dev_pm_genpd_get_performance_state to return the current performance state of a power domain and dev_pm_genpd_performance_state_count to return the total number of performance states supported by a power domain. Since the genpd framework does not maintain a count of number of performance states supported by a power domain, introduce a new callback(.get_performance_state_count) that can be used to retrieve this information from power domain drivers. These APIs are added to aid the implementation of a power domain as a warming device. Linux kernel cooling device framework(into which warming device can be plugged in) requires during initialization to be provided with the maximum number of states that can be supported. When a power domain acts as a warming device, the max state is the max number of perfomrance states supported by the power domain. The cooling device framework implements API to retrieve the current state of the cooling device. This in turn translates to the current performance state of the power domain. Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> --- v1->v2: - Use dev_to_genpd_safe when accessing genpd from a device pointer. - Change the format of dev_pm_genpd_get_performance_state and dev_pm_genpd_performance_state_count to return back the requested parameter rather than taking a point parameter as input. drivers/base/power/domain.c | 37 +++++++++++++++++++++++++++++++++++++ include/linux/pm_domain.h | 13 +++++++++++++ 2 files changed, 50 insertions(+) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 2650ae2..126710e 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -408,6 +408,43 @@ int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state) } EXPORT_SYMBOL_GPL(dev_pm_genpd_set_performance_state); +int dev_pm_genpd_get_performance_state(struct device *dev) +{ + struct generic_pm_domain *genpd; + unsigned int state; + + genpd = dev_to_genpd_safe(dev); + if (IS_ERR(genpd)) + return -ENODEV; + + genpd_lock(genpd); + state = genpd->performance_state; + genpd_unlock(genpd); + + return state; +} +EXPORT_SYMBOL_GPL(dev_pm_genpd_get_performance_state); + +int dev_pm_genpd_performance_state_count(struct device *dev) +{ + struct generic_pm_domain *genpd; + int count; + + genpd = dev_to_genpd_safe(dev); + if (IS_ERR(genpd)) + return -ENODEV; + + if (unlikely(!genpd->get_performance_state_count)) + return -EINVAL; + + genpd_lock(genpd); + count = genpd->get_performance_state_count(genpd); + genpd_unlock(genpd); + + return count; +} +EXPORT_SYMBOL_GPL(dev_pm_genpd_performance_state_count); + static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed) { unsigned int state_idx = genpd->state_idx; diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index baf02ff..e88e57f 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -117,6 +117,7 @@ struct generic_pm_domain { struct dev_pm_opp *opp); int (*set_performance_state)(struct generic_pm_domain *genpd, unsigned int state); + int (*get_performance_state_count)(struct generic_pm_domain *genpd); struct gpd_dev_ops dev_ops; s64 max_off_time_ns; /* Maximum allowed "suspended" time. */ bool max_off_time_changed; @@ -204,6 +205,8 @@ int pm_genpd_init(struct generic_pm_domain *genpd, struct dev_power_governor *gov, bool is_off); int pm_genpd_remove(struct generic_pm_domain *genpd); int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state); +int dev_pm_genpd_get_performance_state(struct device *dev); +int dev_pm_genpd_performance_state_count(struct device *dev); extern struct dev_power_governor simple_qos_governor; extern struct dev_power_governor pm_domain_always_on_gov; @@ -251,6 +254,16 @@ static inline int dev_pm_genpd_set_performance_state(struct device *dev, return -ENOTSUPP; } +static inline int dev_pm_genpd_get_performance_state(struct device *dev) +{ + return -ENOTSUPP; +} + +static inline int dev_pm_genpd_performance_state_count(struct device *dev) +{ + return -ENOTSUPP; +} + #define simple_qos_governor (*(struct dev_power_governor *)(NULL)) #define pm_domain_always_on_gov (*(struct dev_power_governor *)(NULL)) #endif -- 2.1.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/5] soc: qcom: rpmhpd: Introduce function to retrieve power domain performance state count 2019-09-10 17:14 [PATCH v2 0/5] Introduce Power domain based warming device driver Thara Gopinath 2019-09-10 17:14 ` [PATCH v2 1/5] PM/Domains: Add support for retrieving genpd performance states information Thara Gopinath @ 2019-09-10 17:14 ` Thara Gopinath 2019-09-10 17:14 ` [PATCH 3/5] dt-bindings: thermal: Add generic power domain warming device binding Thara Gopinath ` (2 subsequent siblings) 4 siblings, 0 replies; 12+ messages in thread From: Thara Gopinath @ 2019-09-10 17:14 UTC (permalink / raw) To: edubezval, rui.zhang, ulf.hansson, daniel.lezcano, bjorn.andersson, robh+dt, agross Cc: amit.kucheria, mark.rutland, rjw, linux-pm, devicetree, linux-arm-msm, linux-kernel Populate .get_performace_state_count in genpd ops to retrieve the count of performance states supported by a rpmh power domain. Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> --- v1->v2: - Changes ti incorporate the new format of get_performance_state_count. drivers/soc/qcom/rpmhpd.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/soc/qcom/rpmhpd.c b/drivers/soc/qcom/rpmhpd.c index 5741ec3..9d37534 100644 --- a/drivers/soc/qcom/rpmhpd.c +++ b/drivers/soc/qcom/rpmhpd.c @@ -285,6 +285,13 @@ static unsigned int rpmhpd_get_performance_state(struct generic_pm_domain *genpd return dev_pm_opp_get_level(opp); } +static int rpmhpd_performance_states_count(struct generic_pm_domain *domain) +{ + struct rpmhpd *pd = domain_to_rpmhpd(domain); + + return pd->level_count; +} + static int rpmhpd_update_level_mapping(struct rpmhpd *rpmhpd) { int i; @@ -373,6 +380,8 @@ static int rpmhpd_probe(struct platform_device *pdev) rpmhpds[i]->pd.power_on = rpmhpd_power_on; rpmhpds[i]->pd.set_performance_state = rpmhpd_set_performance_state; rpmhpds[i]->pd.opp_to_performance_state = rpmhpd_get_performance_state; + rpmhpds[i]->pd.get_performance_state_count = + rpmhpd_performance_states_count; pm_genpd_init(&rpmhpds[i]->pd, NULL, true); data->domains[i] = &rpmhpds[i]->pd; -- 2.1.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/5] dt-bindings: thermal: Add generic power domain warming device binding 2019-09-10 17:14 [PATCH v2 0/5] Introduce Power domain based warming device driver Thara Gopinath 2019-09-10 17:14 ` [PATCH v2 1/5] PM/Domains: Add support for retrieving genpd performance states information Thara Gopinath 2019-09-10 17:14 ` [PATCH v2 2/5] soc: qcom: rpmhpd: Introduce function to retrieve power domain performance state count Thara Gopinath @ 2019-09-10 17:14 ` Thara Gopinath 2019-09-30 14:42 ` Rob Herring 2019-09-10 17:14 ` [PATCH 4/5] thermal: Add generic power domain warming device driver Thara Gopinath 2019-09-10 17:14 ` [PATCH 5/5] arm64: dts: qcom: Add node for RPMH power domain warming device on sdm845 Thara Gopinath 4 siblings, 1 reply; 12+ messages in thread From: Thara Gopinath @ 2019-09-10 17:14 UTC (permalink / raw) To: edubezval, rui.zhang, ulf.hansson, daniel.lezcano, bjorn.andersson, robh+dt, agross Cc: amit.kucheria, mark.rutland, rjw, linux-pm, devicetree, linux-arm-msm, linux-kernel Add binding to define power domains as thermal warming devices. Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> --- .../bindings/thermal/pwr-domain-warming.txt | 32 ++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 Documentation/devicetree/bindings/thermal/pwr-domain-warming.txt diff --git a/Documentation/devicetree/bindings/thermal/pwr-domain-warming.txt b/Documentation/devicetree/bindings/thermal/pwr-domain-warming.txt new file mode 100644 index 0000000..25fc568 --- /dev/null +++ b/Documentation/devicetree/bindings/thermal/pwr-domain-warming.txt @@ -0,0 +1,32 @@ +* Generic power domain based thermal warming device. + +This binding describes the power domains that can be used as a +thermal warming device. + +- compatible: + Usage: required + Value type: <string> + Definition: must be "thermal-power-domain-wdev" + +- #temp-reg-cells: + Usage: required + Value type: <u32> + Definition: Must be 2 + +- power-domains: + Usage: required + Value type: <phandle> + Definition: reference to power-domains that match power-domain-names + +- power-domain-names: + Usage: required + Value type: <stringlist> + Definition: The power-domains that can behave as warming devices + +Example 1 +thermal_wdev: rpmhpd_mx_wdev { + compatible = "thermal-power-domain-wdev"; + #cooling-cells = <2>; + power-domains = <&rpmhpd SDM845_MX>; + power-domain-names = "mx"; + }; -- 2.1.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] dt-bindings: thermal: Add generic power domain warming device binding 2019-09-10 17:14 ` [PATCH 3/5] dt-bindings: thermal: Add generic power domain warming device binding Thara Gopinath @ 2019-09-30 14:42 ` Rob Herring 2019-10-09 12:18 ` Thara Gopinath 0 siblings, 1 reply; 12+ messages in thread From: Rob Herring @ 2019-09-30 14:42 UTC (permalink / raw) To: Thara Gopinath Cc: edubezval, rui.zhang, ulf.hansson, daniel.lezcano, bjorn.andersson, agross, amit.kucheria, mark.rutland, rjw, linux-pm, devicetree, linux-arm-msm, linux-kernel On Tue, Sep 10, 2019 at 01:14:34PM -0400, Thara Gopinath wrote: > Add binding to define power domains as thermal warming > devices. > > Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> > --- > .../bindings/thermal/pwr-domain-warming.txt | 32 ++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > create mode 100644 Documentation/devicetree/bindings/thermal/pwr-domain-warming.txt > > diff --git a/Documentation/devicetree/bindings/thermal/pwr-domain-warming.txt b/Documentation/devicetree/bindings/thermal/pwr-domain-warming.txt > new file mode 100644 > index 0000000..25fc568 > --- /dev/null > +++ b/Documentation/devicetree/bindings/thermal/pwr-domain-warming.txt > @@ -0,0 +1,32 @@ > +* Generic power domain based thermal warming device. > + > +This binding describes the power domains that can be used as a > +thermal warming device. This looks like just a gathering of properties and way to instantiate some driver. I think this all belongs in the power domain provider. Make it a cooling device and you should know which domains are relevant based on the compatible (though perhaps we could consider a list in DT). If you want to instantiate a separate driver to handle this, then make the power domain driver do that. Rob > + > +- compatible: > + Usage: required > + Value type: <string> > + Definition: must be "thermal-power-domain-wdev" > + > +- #temp-reg-cells: > + Usage: required > + Value type: <u32> > + Definition: Must be 2 > + > +- power-domains: > + Usage: required > + Value type: <phandle> > + Definition: reference to power-domains that match power-domain-names > + > +- power-domain-names: > + Usage: required > + Value type: <stringlist> > + Definition: The power-domains that can behave as warming devices > + > +Example 1 > +thermal_wdev: rpmhpd_mx_wdev { > + compatible = "thermal-power-domain-wdev"; > + #cooling-cells = <2>; > + power-domains = <&rpmhpd SDM845_MX>; > + power-domain-names = "mx"; > + }; > -- > 2.1.4 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] dt-bindings: thermal: Add generic power domain warming device binding 2019-09-30 14:42 ` Rob Herring @ 2019-10-09 12:18 ` Thara Gopinath 0 siblings, 0 replies; 12+ messages in thread From: Thara Gopinath @ 2019-10-09 12:18 UTC (permalink / raw) To: Rob Herring Cc: edubezval, rui.zhang, ulf.hansson, daniel.lezcano, bjorn.andersson, agross, amit.kucheria, mark.rutland, rjw, linux-pm, devicetree, linux-arm-msm, linux-kernel Hi Rob, Thanks for the review On 09/30/2019 10:42 AM, Rob Herring wrote: > On Tue, Sep 10, 2019 at 01:14:34PM -0400, Thara Gopinath wrote: >> Add binding to define power domains as thermal warming >> devices. >> >> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> >> --- >> .../bindings/thermal/pwr-domain-warming.txt | 32 ++++++++++++++++++++++ >> 1 file changed, 32 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/thermal/pwr-domain-warming.txt >> >> diff --git a/Documentation/devicetree/bindings/thermal/pwr-domain-warming.txt b/Documentation/devicetree/bindings/thermal/pwr-domain-warming.txt >> new file mode 100644 >> index 0000000..25fc568 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/thermal/pwr-domain-warming.txt >> @@ -0,0 +1,32 @@ >> +* Generic power domain based thermal warming device. >> + >> +This binding describes the power domains that can be used as a >> +thermal warming device. > > This looks like just a gathering of properties and way to instantiate > some driver. > > I think this all belongs in the power domain provider. Make it a cooling > device and you should know which domains are relevant based on the > compatible (though perhaps we could consider a list in DT). If you want > to instantiate a separate driver to handle this, then make the power > domain driver do that. This sounds fine. A list in DT might be needed though. I have a separate driver. I should be able to get the genpd provider driver call into it. I had asked this to Ulf and he seemed to be fine with it as well > > Rob > > -- Regards Thara ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/5] thermal: Add generic power domain warming device driver. 2019-09-10 17:14 [PATCH v2 0/5] Introduce Power domain based warming device driver Thara Gopinath ` (2 preceding siblings ...) 2019-09-10 17:14 ` [PATCH 3/5] dt-bindings: thermal: Add generic power domain warming device binding Thara Gopinath @ 2019-09-10 17:14 ` Thara Gopinath 2019-09-12 15:04 ` Ulf Hansson 2019-09-10 17:14 ` [PATCH 5/5] arm64: dts: qcom: Add node for RPMH power domain warming device on sdm845 Thara Gopinath 4 siblings, 1 reply; 12+ messages in thread From: Thara Gopinath @ 2019-09-10 17:14 UTC (permalink / raw) To: edubezval, rui.zhang, ulf.hansson, daniel.lezcano, bjorn.andersson, robh+dt, agross Cc: amit.kucheria, mark.rutland, rjw, linux-pm, devicetree, linux-arm-msm, linux-kernel Resources modeled as power domains in linux kenrel can be used to warm the SoC(eg. mx power domain on sdm845). To support this feature, introduce a generic power domain warming device driver that can be plugged into the thermal framework (The thermal framework itself requires further modifiction to support a warming device in place of a cooling device. Those extensions are not introduced in this patch series). Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> --- v1->v2: - Make power domain based warming device driver a generic driver in the thermal framework. v1 implemented this as a Qualcomm specific driver. - Rename certain variables as per review suggestions on the mailing list. drivers/thermal/Kconfig | 11 +++ drivers/thermal/Makefile | 2 + drivers/thermal/pwr_domain_warming.c | 174 +++++++++++++++++++++++++++++++++++ 3 files changed, 187 insertions(+) create mode 100644 drivers/thermal/pwr_domain_warming.c diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index 9966364..eeb6018 100644 --- a/drivers/thermal/Kconfig +++ b/drivers/thermal/Kconfig @@ -187,6 +187,17 @@ config DEVFREQ_THERMAL If you want this support, you should say Y here. +config PWR_DOMAIN_WARMING_THERMAL + bool "Power Domain based warming device" + depends on PM_GENERIC_DOMAINS + depends on PM_GENERIC_DOMAINS_OF + help + This implements the generic power domain based warming + mechanism through increasing the performance state of + a power domain. + + If you want this support, you should say Y here. + config THERMAL_EMULATION bool "Thermal emulation mode support" help diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile index 74a37c7..382c64a 100644 --- a/drivers/thermal/Makefile +++ b/drivers/thermal/Makefile @@ -27,6 +27,8 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL) += clock_cooling.o # devfreq cooling thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o +thermal_sys-$(CONFIG_PWR_DOMAIN_WARMING_THERMAL) += pwr_domain_warming.o + # platform thermal drivers obj-y += broadcom/ obj-$(CONFIG_THERMAL_MMIO) += thermal_mmio.o diff --git a/drivers/thermal/pwr_domain_warming.c b/drivers/thermal/pwr_domain_warming.c new file mode 100644 index 0000000..3dd792b --- /dev/null +++ b/drivers/thermal/pwr_domain_warming.c @@ -0,0 +1,174 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2019, Linaro Ltd + */ +#include <linux/err.h> +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/module.h> +#include <linux/pm_domain.h> +#include <linux/pm_runtime.h> +#include <linux/thermal.h> + +struct pd_warming_device { + struct thermal_cooling_device *cdev; + struct device *dev; + int max_state; + int cur_state; + bool runtime_resumed; +}; + +static const struct of_device_id pd_wdev_match_table[] = { + { .compatible = "thermal-power-domain-wdev", .data = NULL }, + { } +}; +MODULE_DEVICE_TABLE(of, pd_wdev_match_table); + +static int pd_wdev_get_max_state(struct thermal_cooling_device *cdev, + unsigned long *state) +{ + struct pd_warming_device *pd_wdev = cdev->devdata; + + *state = pd_wdev->max_state; + return 0; +} + +static int pd_wdev_get_cur_state(struct thermal_cooling_device *cdev, + unsigned long *state) +{ + struct pd_warming_device *pd_wdev = cdev->devdata; + + *state = dev_pm_genpd_get_performance_state(pd_wdev->dev); + + return 0; +} + +static int pd_wdev_set_cur_state(struct thermal_cooling_device *cdev, + unsigned long state) +{ + struct pd_warming_device *pd_wdev = cdev->devdata; + struct device *dev = pd_wdev->dev; + int ret; + + ret = dev_pm_genpd_set_performance_state(dev, state); + + if (ret) + return ret; + + if (state && !pd_wdev->runtime_resumed) { + ret = pm_runtime_get_sync(dev); + pd_wdev->runtime_resumed = true; + } else if (!state && pd_wdev->runtime_resumed) { + ret = pm_runtime_put(dev); + pd_wdev->runtime_resumed = false; + } + + return ret; +} + +static struct thermal_cooling_device_ops pd_warming_device_ops = { + .get_max_state = pd_wdev_get_max_state, + .get_cur_state = pd_wdev_get_cur_state, + .set_cur_state = pd_wdev_set_cur_state, +}; + +static int pd_wdev_create(struct device *dev, const char *name) +{ + struct pd_warming_device *pd_wdev; + int state_count; + + pd_wdev = devm_kzalloc(dev, sizeof(*pd_wdev), GFP_KERNEL); + if (!pd_wdev) + return -ENOMEM; + + state_count = dev_pm_genpd_performance_state_count(dev); + if (state_count < 0) + return state_count; + + pd_wdev->dev = dev; + pd_wdev->max_state = state_count - 1; + pd_wdev->runtime_resumed = false; + + pm_runtime_enable(dev); + + pd_wdev->cdev = thermal_of_cooling_device_register + (dev->of_node, name, + pd_wdev, + &pd_warming_device_ops); + if (IS_ERR(pd_wdev->cdev)) { + dev_err(dev, "unable to register %s cooling device\n", name); + pm_runtime_disable(dev); + + return PTR_ERR(pd_wdev->cdev); + } + + return 0; +} + +static int pd_wdev_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev, *pd_dev; + const char *pd_name; + int id, count, ret = 0; + + count = of_count_phandle_with_args(dev->of_node, "power-domains", + "#power-domain-cells"); + + if (count > 1) { + for (id = 0; id < count; id++) { + ret = of_property_read_string_index + (dev->of_node, "power-domain-names", + id, &pd_name); + if (ret) { + dev_err(dev, "Error reading the power domain name %d\n", ret); + continue; + } + + pd_dev = dev_pm_domain_attach_by_id(dev, id); + if (IS_ERR(pd_dev)) { + dev_err(dev, "Error attaching power domain %s %ld\n", pd_name, PTR_ERR(pd_dev)); + continue; + } + + ret = pd_wdev_create(pd_dev, pd_name); + if (ret) { + dev_err(dev, "Error building cooling device %s %d\n", pd_name, ret); + dev_pm_domain_detach(pd_dev, false); + continue; + } + } + } else if (count == 1) { + ret = of_property_read_string_index(dev->of_node, + "power-domain-names", + 0, &pd_name); + if (ret) { + dev_err(dev, "Error reading the power domain name %d\n", ret); + goto exit; + } + + ret = pd_wdev_create(dev, pd_name); + if (ret) { + dev_err(dev, "Error building cooling device %s %d\n", pd_name, ret); + goto exit; + } + } else { + ret = -EINVAL; + } + +exit: + return ret; +} + +static struct platform_driver pd_wdev_driver = { + .driver = { + .name = "qcom-rpmhpd-cdev", + .of_match_table = pd_wdev_match_table, + }, + .probe = pd_wdev_probe, +}; +module_platform_driver(pd_wdev_driver); + +MODULE_DESCRIPTION("Qualcomm RPMHPD cooling device driver"); +MODULE_LICENSE("GPL v2"); -- 2.1.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] thermal: Add generic power domain warming device driver. 2019-09-10 17:14 ` [PATCH 4/5] thermal: Add generic power domain warming device driver Thara Gopinath @ 2019-09-12 15:04 ` Ulf Hansson 2019-09-12 20:18 ` Thara Gopinath 0 siblings, 1 reply; 12+ messages in thread From: Ulf Hansson @ 2019-09-12 15:04 UTC (permalink / raw) To: Thara Gopinath Cc: Eduardo Valentin, Zhang Rui, Daniel Lezcano, Bjorn Andersson, Rob Herring, agross, amit.kucheria, Mark Rutland, Rafael J. Wysocki, Linux PM, DTML, linux-arm-msm, Linux Kernel Mailing List On Tue, 10 Sep 2019 at 19:14, Thara Gopinath <thara.gopinath@linaro.org> wrote: > > Resources modeled as power domains in linux kenrel > can be used to warm the SoC(eg. mx power domain on sdm845). > To support this feature, introduce a generic power domain > warming device driver that can be plugged into the thermal framework > (The thermal framework itself requires further modifiction to > support a warming device in place of a cooling device. > Those extensions are not introduced in this patch series). > > Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> > --- > v1->v2: > - Make power domain based warming device driver a generic > driver in the thermal framework. v1 implemented this as a > Qualcomm specific driver. > - Rename certain variables as per review suggestions on the > mailing list. > > drivers/thermal/Kconfig | 11 +++ > drivers/thermal/Makefile | 2 + > drivers/thermal/pwr_domain_warming.c | 174 +++++++++++++++++++++++++++++++++++ > 3 files changed, 187 insertions(+) > create mode 100644 drivers/thermal/pwr_domain_warming.c > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig > index 9966364..eeb6018 100644 > --- a/drivers/thermal/Kconfig > +++ b/drivers/thermal/Kconfig > @@ -187,6 +187,17 @@ config DEVFREQ_THERMAL > > If you want this support, you should say Y here. > > +config PWR_DOMAIN_WARMING_THERMAL > + bool "Power Domain based warming device" > + depends on PM_GENERIC_DOMAINS > + depends on PM_GENERIC_DOMAINS_OF PM_GENERIC_DOMAINS_OF can't be set unless PM_GENERIC_DOMAINS is set too. So I assume it's sufficient to depend on PM_GENERIC_DOMAINS_OF? > + help > + This implements the generic power domain based warming > + mechanism through increasing the performance state of > + a power domain. > + > + If you want this support, you should say Y here. > + > config THERMAL_EMULATION > bool "Thermal emulation mode support" > help > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile > index 74a37c7..382c64a 100644 > --- a/drivers/thermal/Makefile > +++ b/drivers/thermal/Makefile > @@ -27,6 +27,8 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL) += clock_cooling.o > # devfreq cooling > thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o > > +thermal_sys-$(CONFIG_PWR_DOMAIN_WARMING_THERMAL) += pwr_domain_warming.o > + > # platform thermal drivers > obj-y += broadcom/ > obj-$(CONFIG_THERMAL_MMIO) += thermal_mmio.o > diff --git a/drivers/thermal/pwr_domain_warming.c b/drivers/thermal/pwr_domain_warming.c > new file mode 100644 > index 0000000..3dd792b > --- /dev/null > +++ b/drivers/thermal/pwr_domain_warming.c > @@ -0,0 +1,174 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2019, Linaro Ltd > + */ > +#include <linux/err.h> > +#include <linux/kernel.h> > +#include <linux/init.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/module.h> > +#include <linux/pm_domain.h> > +#include <linux/pm_runtime.h> > +#include <linux/thermal.h> > + > +struct pd_warming_device { > + struct thermal_cooling_device *cdev; > + struct device *dev; > + int max_state; > + int cur_state; > + bool runtime_resumed; > +}; > + > +static const struct of_device_id pd_wdev_match_table[] = { > + { .compatible = "thermal-power-domain-wdev", .data = NULL }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, pd_wdev_match_table); > + > +static int pd_wdev_get_max_state(struct thermal_cooling_device *cdev, > + unsigned long *state) > +{ > + struct pd_warming_device *pd_wdev = cdev->devdata; > + > + *state = pd_wdev->max_state; > + return 0; > +} > + > +static int pd_wdev_get_cur_state(struct thermal_cooling_device *cdev, > + unsigned long *state) > +{ > + struct pd_warming_device *pd_wdev = cdev->devdata; > + > + *state = dev_pm_genpd_get_performance_state(pd_wdev->dev); > + > + return 0; > +} > + > +static int pd_wdev_set_cur_state(struct thermal_cooling_device *cdev, > + unsigned long state) > +{ > + struct pd_warming_device *pd_wdev = cdev->devdata; > + struct device *dev = pd_wdev->dev; > + int ret; > + > + ret = dev_pm_genpd_set_performance_state(dev, state); > + > + if (ret) > + return ret; > + > + if (state && !pd_wdev->runtime_resumed) { > + ret = pm_runtime_get_sync(dev); > + pd_wdev->runtime_resumed = true; > + } else if (!state && pd_wdev->runtime_resumed) { > + ret = pm_runtime_put(dev); > + pd_wdev->runtime_resumed = false; > + } > + > + return ret; > +} > + > +static struct thermal_cooling_device_ops pd_warming_device_ops = { > + .get_max_state = pd_wdev_get_max_state, > + .get_cur_state = pd_wdev_get_cur_state, > + .set_cur_state = pd_wdev_set_cur_state, > +}; > + > +static int pd_wdev_create(struct device *dev, const char *name) > +{ > + struct pd_warming_device *pd_wdev; > + int state_count; > + > + pd_wdev = devm_kzalloc(dev, sizeof(*pd_wdev), GFP_KERNEL); > + if (!pd_wdev) > + return -ENOMEM; > + > + state_count = dev_pm_genpd_performance_state_count(dev); > + if (state_count < 0) > + return state_count; > + > + pd_wdev->dev = dev; > + pd_wdev->max_state = state_count - 1; > + pd_wdev->runtime_resumed = false; > + > + pm_runtime_enable(dev); > + > + pd_wdev->cdev = thermal_of_cooling_device_register > + (dev->of_node, name, > + pd_wdev, > + &pd_warming_device_ops); > + if (IS_ERR(pd_wdev->cdev)) { > + dev_err(dev, "unable to register %s cooling device\n", name); > + pm_runtime_disable(dev); > + > + return PTR_ERR(pd_wdev->cdev); > + } > + > + return 0; > +} > + > +static int pd_wdev_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev, *pd_dev; > + const char *pd_name; > + int id, count, ret = 0; > + > + count = of_count_phandle_with_args(dev->of_node, "power-domains", > + "#power-domain-cells"); Perhaps this should be converted to genpd OF helper function instead, that allows the caller to know how many power-domains there are specified for a device node. > + > + if (count > 1) { > + for (id = 0; id < count; id++) { > + ret = of_property_read_string_index > + (dev->of_node, "power-domain-names", > + id, &pd_name); > + if (ret) { > + dev_err(dev, "Error reading the power domain name %d\n", ret); > + continue; > + } It looks a bit awkward that you want to re-use the power-domain-names as the name for the cooling (warming) device. This isn't really what we use the "*-names" bindings for in general, I think. Anyway, if you want a name corresponding to the actual attached PM domain, perhaps re-using "->name" from the struct generic_pm_domain is better. We can add a genpd helper for that, no problem. Of course it also means that you must call dev_pm_domain_attach_by_id() first, to attach the device and then get the name of the genpd, but that should be fine. > + > + pd_dev = dev_pm_domain_attach_by_id(dev, id); > + if (IS_ERR(pd_dev)) { > + dev_err(dev, "Error attaching power domain %s %ld\n", pd_name, PTR_ERR(pd_dev)); > + continue; > + } > + > + ret = pd_wdev_create(pd_dev, pd_name); > + if (ret) { > + dev_err(dev, "Error building cooling device %s %d\n", pd_name, ret); > + dev_pm_domain_detach(pd_dev, false); > + continue; > + } I am wondering about the use case of having multiple PM domains attached to the cooling (warming) device. Is that really needed? Perhaps you can elaborate on that a bit? > + } > + } else if (count == 1) { > + ret = of_property_read_string_index(dev->of_node, > + "power-domain-names", > + 0, &pd_name); > + if (ret) { > + dev_err(dev, "Error reading the power domain name %d\n", ret); > + goto exit; > + } According to my comment above, perhaps we simply don't have to use the "power-domain-names" binding at all. Also, I don't think this is really safe, as there is no guarantee that there is PM domain attached to the device, just because you found the DT property "power-domain-names". Probably better to check pm_domain pointer for the device. > + > + ret = pd_wdev_create(dev, pd_name); > + if (ret) { > + dev_err(dev, "Error building cooling device %s %d\n", pd_name, ret); > + goto exit; > + } > + } else { > + ret = -EINVAL; > + } > + > +exit: > + return ret; > +} > + > +static struct platform_driver pd_wdev_driver = { > + .driver = { > + .name = "qcom-rpmhpd-cdev", Probably rename to a more generic name (leftover from earlier version I assume). > + .of_match_table = pd_wdev_match_table, > + }, > + .probe = pd_wdev_probe, > +}; > +module_platform_driver(pd_wdev_driver); > + > +MODULE_DESCRIPTION("Qualcomm RPMHPD cooling device driver"); Ditto. > +MODULE_LICENSE("GPL v2"); > -- > 2.1.4 > Kind regards Uffe ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] thermal: Add generic power domain warming device driver. 2019-09-12 15:04 ` Ulf Hansson @ 2019-09-12 20:18 ` Thara Gopinath 2019-09-13 7:54 ` Ulf Hansson 0 siblings, 1 reply; 12+ messages in thread From: Thara Gopinath @ 2019-09-12 20:18 UTC (permalink / raw) To: Ulf Hansson Cc: Eduardo Valentin, Zhang Rui, Daniel Lezcano, Bjorn Andersson, Rob Herring, agross, amit.kucheria, Mark Rutland, Rafael J. Wysocki, Linux PM, DTML, linux-arm-msm, Linux Kernel Mailing List On 09/12/2019 11:04 AM, Ulf Hansson wrote: Hi Ulf, Thanks for the review. > On Tue, 10 Sep 2019 at 19:14, Thara Gopinath <thara.gopinath@linaro.org> wrote: >> >> Resources modeled as power domains in linux kenrel >> can be used to warm the SoC(eg. mx power domain on sdm845). >> To support this feature, introduce a generic power domain >> warming device driver that can be plugged into the thermal framework >> (The thermal framework itself requires further modifiction to >> support a warming device in place of a cooling device. >> Those extensions are not introduced in this patch series). >> >> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> >> --- >> v1->v2: >> - Make power domain based warming device driver a generic >> driver in the thermal framework. v1 implemented this as a >> Qualcomm specific driver. >> - Rename certain variables as per review suggestions on the >> mailing list. >> >> drivers/thermal/Kconfig | 11 +++ >> drivers/thermal/Makefile | 2 + >> drivers/thermal/pwr_domain_warming.c | 174 +++++++++++++++++++++++++++++++++++ >> 3 files changed, 187 insertions(+) >> create mode 100644 drivers/thermal/pwr_domain_warming.c >> >> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig >> index 9966364..eeb6018 100644 >> --- a/drivers/thermal/Kconfig >> +++ b/drivers/thermal/Kconfig >> @@ -187,6 +187,17 @@ config DEVFREQ_THERMAL >> >> If you want this support, you should say Y here. >> >> +config PWR_DOMAIN_WARMING_THERMAL >> + bool "Power Domain based warming device" >> + depends on PM_GENERIC_DOMAINS >> + depends on PM_GENERIC_DOMAINS_OF > > PM_GENERIC_DOMAINS_OF can't be set unless PM_GENERIC_DOMAINS is set too. > > So I assume it's sufficient to depend on PM_GENERIC_DOMAINS_OF? Yes, you are right. I will change it. > >> + help >> + This implements the generic power domain based warming >> + mechanism through increasing the performance state of >> + a power domain. >> + >> + If you want this support, you should say Y here. >> + >> config THERMAL_EMULATION >> bool "Thermal emulation mode support" >> help >> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile >> index 74a37c7..382c64a 100644 >> --- a/drivers/thermal/Makefile >> +++ b/drivers/thermal/Makefile >> @@ -27,6 +27,8 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL) += clock_cooling.o >> # devfreq cooling >> thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o >> >> +thermal_sys-$(CONFIG_PWR_DOMAIN_WARMING_THERMAL) += pwr_domain_warming.o >> + >> # platform thermal drivers >> obj-y += broadcom/ >> obj-$(CONFIG_THERMAL_MMIO) += thermal_mmio.o >> diff --git a/drivers/thermal/pwr_domain_warming.c b/drivers/thermal/pwr_domain_warming.c >> new file mode 100644 >> index 0000000..3dd792b >> --- /dev/null >> +++ b/drivers/thermal/pwr_domain_warming.c >> @@ -0,0 +1,174 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (c) 2019, Linaro Ltd >> + */ >> +#include <linux/err.h> >> +#include <linux/kernel.h> >> +#include <linux/init.h> >> +#include <linux/of_device.h> >> +#include <linux/platform_device.h> >> +#include <linux/module.h> >> +#include <linux/pm_domain.h> >> +#include <linux/pm_runtime.h> >> +#include <linux/thermal.h> >> + >> +struct pd_warming_device { >> + struct thermal_cooling_device *cdev; >> + struct device *dev; >> + int max_state; >> + int cur_state; >> + bool runtime_resumed; >> +}; >> + >> +static const struct of_device_id pd_wdev_match_table[] = { >> + { .compatible = "thermal-power-domain-wdev", .data = NULL }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(of, pd_wdev_match_table); >> + >> +static int pd_wdev_get_max_state(struct thermal_cooling_device *cdev, >> + unsigned long *state) >> +{ >> + struct pd_warming_device *pd_wdev = cdev->devdata; >> + >> + *state = pd_wdev->max_state; >> + return 0; >> +} >> + >> +static int pd_wdev_get_cur_state(struct thermal_cooling_device *cdev, >> + unsigned long *state) >> +{ >> + struct pd_warming_device *pd_wdev = cdev->devdata; >> + >> + *state = dev_pm_genpd_get_performance_state(pd_wdev->dev); >> + >> + return 0; >> +} >> + >> +static int pd_wdev_set_cur_state(struct thermal_cooling_device *cdev, >> + unsigned long state) >> +{ >> + struct pd_warming_device *pd_wdev = cdev->devdata; >> + struct device *dev = pd_wdev->dev; >> + int ret; >> + >> + ret = dev_pm_genpd_set_performance_state(dev, state); >> + >> + if (ret) >> + return ret; >> + >> + if (state && !pd_wdev->runtime_resumed) { >> + ret = pm_runtime_get_sync(dev); >> + pd_wdev->runtime_resumed = true; >> + } else if (!state && pd_wdev->runtime_resumed) { >> + ret = pm_runtime_put(dev); >> + pd_wdev->runtime_resumed = false; >> + } >> + >> + return ret; >> +} >> + >> +static struct thermal_cooling_device_ops pd_warming_device_ops = { >> + .get_max_state = pd_wdev_get_max_state, >> + .get_cur_state = pd_wdev_get_cur_state, >> + .set_cur_state = pd_wdev_set_cur_state, >> +}; >> + >> +static int pd_wdev_create(struct device *dev, const char *name) >> +{ >> + struct pd_warming_device *pd_wdev; >> + int state_count; >> + >> + pd_wdev = devm_kzalloc(dev, sizeof(*pd_wdev), GFP_KERNEL); >> + if (!pd_wdev) >> + return -ENOMEM; >> + >> + state_count = dev_pm_genpd_performance_state_count(dev); >> + if (state_count < 0) >> + return state_count; >> + >> + pd_wdev->dev = dev; >> + pd_wdev->max_state = state_count - 1; >> + pd_wdev->runtime_resumed = false; >> + >> + pm_runtime_enable(dev); >> + >> + pd_wdev->cdev = thermal_of_cooling_device_register >> + (dev->of_node, name, >> + pd_wdev, >> + &pd_warming_device_ops); >> + if (IS_ERR(pd_wdev->cdev)) { >> + dev_err(dev, "unable to register %s cooling device\n", name); >> + pm_runtime_disable(dev); >> + >> + return PTR_ERR(pd_wdev->cdev); >> + } >> + >> + return 0; >> +} >> + >> +static int pd_wdev_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev, *pd_dev; >> + const char *pd_name; >> + int id, count, ret = 0; >> + >> + count = of_count_phandle_with_args(dev->of_node, "power-domains", >> + "#power-domain-cells"); > > Perhaps this should be converted to genpd OF helper function instead, > that allows the caller to know how many power-domains there are > specified for a device node. I am ok with this if you think that a OF helper to get the number of power domains is a useful helper in the genpd framework. I can add it as part of the next revision. Or do you want me to send it across separate? > >> + >> + if (count > 1) { >> + for (id = 0; id < count; id++) { >> + ret = of_property_read_string_index >> + (dev->of_node, "power-domain-names", >> + id, &pd_name); >> + if (ret) { >> + dev_err(dev, "Error reading the power domain name %d\n", ret); >> + continue; >> + } > > It looks a bit awkward that you want to re-use the power-domain-names > as the name for the cooling (warming) device. This isn't really what > we use the "*-names" bindings for in general, I think. > > Anyway, if you want a name corresponding to the actual attached PM > domain, perhaps re-using "->name" from the struct generic_pm_domain is > better. We can add a genpd helper for that, no problem. Of course it > also means that you must call dev_pm_domain_attach_by_id() first, to > attach the device and then get the name of the genpd, but that should > be fine. Ya. I need a name corresponding to the power domain name (or something very close) to identify the actual warming device in the sysfs entries. I can use genpd->name and a helper function to achieve it. I can include it in Patch 1/5 where I add other helper functions. > >> + >> + pd_dev = dev_pm_domain_attach_by_id(dev, id); >> + if (IS_ERR(pd_dev)) { >> + dev_err(dev, "Error attaching power domain %s %ld\n", pd_name, PTR_ERR(pd_dev)); >> + continue; >> + } >> + >> + ret = pd_wdev_create(pd_dev, pd_name); >> + if (ret) { >> + dev_err(dev, "Error building cooling device %s %d\n", pd_name, ret); >> + dev_pm_domain_detach(pd_dev, false); >> + continue; >> + } > > I am wondering about the use case of having multiple PM domains > attached to the cooling (warming) device. Is that really needed? > Perhaps you can elaborate on that a bit? Ya. I though about this as well. I don't have a use case. In my current case it is just one power domain on the SoC. But considering this is now a generic driver, in my opinion this has to be a generic solution. So if you think about this, the device should be able to specify any number of power domains that can behave as a warming device since a SoC can have any number of power domain based warming devices. May be one to warm up the cpus, one for gpus etc. So another way of implementing this whole thing is to avoid having a special power domain warming device defined in the device tree. Instead, add a few new binding to the power-domain controller/provider entries to specify if a power domain controlled by the provider can act as a warming device or not. And have the initialization code for the power domain controller (of_genpd_add_provider_onecell or any other suitable API) register the specified power domain as a warming device. The DT entries should probably look something like below in the case. rpmhpd: power-controller { compatible = "qcom,sdm845-rpmhpd"; #power-domain-cells = <1>; hosts-warming-dev; warming-dev-names = "mx"; operating-points-v2 = <&rpmhpd_opp_table>; rpmhpd_opp_table: opp-table { compatible = "operating-points-v2"; .... And have the following in of_genpd_add_provider_onecell if (hosts-warming-dev) # loop through the warming-dev-names and register them as power domain warming devices. You think this is a better idea? > >> + } >> + } else if (count == 1) { >> + ret = of_property_read_string_index(dev->of_node, >> + "power-domain-names", >> + 0, &pd_name); >> + if (ret) { >> + dev_err(dev, "Error reading the power domain name %d\n", ret); >> + goto exit; >> + } > > According to my comment above, perhaps we simply don't have to use the > "power-domain-names" binding at all. I will use genpd->name > > Also, I don't think this is really safe, as there is no guarantee that > there is PM domain attached to the device, just because you found the > DT property "power-domain-names". > > Probably better to check pm_domain pointer for the device. > >> + >> + ret = pd_wdev_create(dev, pd_name); >> + if (ret) { >> + dev_err(dev, "Error building cooling device %s %d\n", pd_name, ret); >> + goto exit; >> + } >> + } else { >> + ret = -EINVAL; >> + } >> + >> +exit: >> + return ret; >> +} >> + >> +static struct platform_driver pd_wdev_driver = { >> + .driver = { >> + .name = "qcom-rpmhpd-cdev", > > Probably rename to a more generic name (leftover from earlier version I assume). Ya. I missed it . Will fix it. > >> + .of_match_table = pd_wdev_match_table, >> + }, >> + .probe = pd_wdev_probe, >> +}; >> +module_platform_driver(pd_wdev_driver); >> + >> +MODULE_DESCRIPTION("Qualcomm RPMHPD cooling device driver"); > > Ditto. Will fix it. > >> +MODULE_LICENSE("GPL v2"); >> -- >> 2.1.4 >> > > Kind regards > Uffe > -- Warm Regards Thara ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] thermal: Add generic power domain warming device driver. 2019-09-12 20:18 ` Thara Gopinath @ 2019-09-13 7:54 ` Ulf Hansson 2019-09-14 11:06 ` Thara Gopinath 0 siblings, 1 reply; 12+ messages in thread From: Ulf Hansson @ 2019-09-13 7:54 UTC (permalink / raw) To: Thara Gopinath Cc: Eduardo Valentin, Zhang Rui, Daniel Lezcano, Bjorn Andersson, Rob Herring, agross, amit.kucheria, Mark Rutland, Rafael J. Wysocki, Linux PM, DTML, linux-arm-msm, Linux Kernel Mailing List On Thu, 12 Sep 2019 at 22:18, Thara Gopinath <thara.gopinath@linaro.org> wrote: > > On 09/12/2019 11:04 AM, Ulf Hansson wrote: > > Hi Ulf, > > Thanks for the review. > > On Tue, 10 Sep 2019 at 19:14, Thara Gopinath <thara.gopinath@linaro.org> wrote: > >> > >> Resources modeled as power domains in linux kenrel > >> can be used to warm the SoC(eg. mx power domain on sdm845). > >> To support this feature, introduce a generic power domain > >> warming device driver that can be plugged into the thermal framework > >> (The thermal framework itself requires further modifiction to > >> support a warming device in place of a cooling device. > >> Those extensions are not introduced in this patch series). > >> > >> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> > >> --- > >> v1->v2: > >> - Make power domain based warming device driver a generic > >> driver in the thermal framework. v1 implemented this as a > >> Qualcomm specific driver. > >> - Rename certain variables as per review suggestions on the > >> mailing list. > >> > >> drivers/thermal/Kconfig | 11 +++ > >> drivers/thermal/Makefile | 2 + > >> drivers/thermal/pwr_domain_warming.c | 174 +++++++++++++++++++++++++++++++++++ > >> 3 files changed, 187 insertions(+) > >> create mode 100644 drivers/thermal/pwr_domain_warming.c > >> > >> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig > >> index 9966364..eeb6018 100644 > >> --- a/drivers/thermal/Kconfig > >> +++ b/drivers/thermal/Kconfig > >> @@ -187,6 +187,17 @@ config DEVFREQ_THERMAL > >> > >> If you want this support, you should say Y here. > >> > >> +config PWR_DOMAIN_WARMING_THERMAL > >> + bool "Power Domain based warming device" > >> + depends on PM_GENERIC_DOMAINS > >> + depends on PM_GENERIC_DOMAINS_OF > > > > PM_GENERIC_DOMAINS_OF can't be set unless PM_GENERIC_DOMAINS is set too. > > > > So I assume it's sufficient to depend on PM_GENERIC_DOMAINS_OF? > > Yes, you are right. I will change it. > > > >> + help > >> + This implements the generic power domain based warming > >> + mechanism through increasing the performance state of > >> + a power domain. > >> + > >> + If you want this support, you should say Y here. > >> + > >> config THERMAL_EMULATION > >> bool "Thermal emulation mode support" > >> help > >> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile > >> index 74a37c7..382c64a 100644 > >> --- a/drivers/thermal/Makefile > >> +++ b/drivers/thermal/Makefile > >> @@ -27,6 +27,8 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL) += clock_cooling.o > >> # devfreq cooling > >> thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o > >> > >> +thermal_sys-$(CONFIG_PWR_DOMAIN_WARMING_THERMAL) += pwr_domain_warming.o > >> + > >> # platform thermal drivers > >> obj-y += broadcom/ > >> obj-$(CONFIG_THERMAL_MMIO) += thermal_mmio.o > >> diff --git a/drivers/thermal/pwr_domain_warming.c b/drivers/thermal/pwr_domain_warming.c > >> new file mode 100644 > >> index 0000000..3dd792b > >> --- /dev/null > >> +++ b/drivers/thermal/pwr_domain_warming.c > >> @@ -0,0 +1,174 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> +/* > >> + * Copyright (c) 2019, Linaro Ltd > >> + */ > >> +#include <linux/err.h> > >> +#include <linux/kernel.h> > >> +#include <linux/init.h> > >> +#include <linux/of_device.h> > >> +#include <linux/platform_device.h> > >> +#include <linux/module.h> > >> +#include <linux/pm_domain.h> > >> +#include <linux/pm_runtime.h> > >> +#include <linux/thermal.h> > >> + > >> +struct pd_warming_device { > >> + struct thermal_cooling_device *cdev; > >> + struct device *dev; > >> + int max_state; > >> + int cur_state; > >> + bool runtime_resumed; > >> +}; > >> + > >> +static const struct of_device_id pd_wdev_match_table[] = { > >> + { .compatible = "thermal-power-domain-wdev", .data = NULL }, > >> + { } > >> +}; > >> +MODULE_DEVICE_TABLE(of, pd_wdev_match_table); > >> + > >> +static int pd_wdev_get_max_state(struct thermal_cooling_device *cdev, > >> + unsigned long *state) > >> +{ > >> + struct pd_warming_device *pd_wdev = cdev->devdata; > >> + > >> + *state = pd_wdev->max_state; > >> + return 0; > >> +} > >> + > >> +static int pd_wdev_get_cur_state(struct thermal_cooling_device *cdev, > >> + unsigned long *state) > >> +{ > >> + struct pd_warming_device *pd_wdev = cdev->devdata; > >> + > >> + *state = dev_pm_genpd_get_performance_state(pd_wdev->dev); > >> + > >> + return 0; > >> +} > >> + > >> +static int pd_wdev_set_cur_state(struct thermal_cooling_device *cdev, > >> + unsigned long state) > >> +{ > >> + struct pd_warming_device *pd_wdev = cdev->devdata; > >> + struct device *dev = pd_wdev->dev; > >> + int ret; > >> + > >> + ret = dev_pm_genpd_set_performance_state(dev, state); > >> + > >> + if (ret) > >> + return ret; > >> + > >> + if (state && !pd_wdev->runtime_resumed) { > >> + ret = pm_runtime_get_sync(dev); > >> + pd_wdev->runtime_resumed = true; > >> + } else if (!state && pd_wdev->runtime_resumed) { > >> + ret = pm_runtime_put(dev); > >> + pd_wdev->runtime_resumed = false; > >> + } > >> + > >> + return ret; > >> +} > >> + > >> +static struct thermal_cooling_device_ops pd_warming_device_ops = { > >> + .get_max_state = pd_wdev_get_max_state, > >> + .get_cur_state = pd_wdev_get_cur_state, > >> + .set_cur_state = pd_wdev_set_cur_state, > >> +}; > >> + > >> +static int pd_wdev_create(struct device *dev, const char *name) > >> +{ > >> + struct pd_warming_device *pd_wdev; > >> + int state_count; > >> + > >> + pd_wdev = devm_kzalloc(dev, sizeof(*pd_wdev), GFP_KERNEL); > >> + if (!pd_wdev) > >> + return -ENOMEM; > >> + > >> + state_count = dev_pm_genpd_performance_state_count(dev); > >> + if (state_count < 0) > >> + return state_count; > >> + > >> + pd_wdev->dev = dev; > >> + pd_wdev->max_state = state_count - 1; > >> + pd_wdev->runtime_resumed = false; > >> + > >> + pm_runtime_enable(dev); > >> + > >> + pd_wdev->cdev = thermal_of_cooling_device_register > >> + (dev->of_node, name, > >> + pd_wdev, > >> + &pd_warming_device_ops); > >> + if (IS_ERR(pd_wdev->cdev)) { > >> + dev_err(dev, "unable to register %s cooling device\n", name); > >> + pm_runtime_disable(dev); > >> + > >> + return PTR_ERR(pd_wdev->cdev); > >> + } > >> + > >> + return 0; > >> +} > >> + > >> +static int pd_wdev_probe(struct platform_device *pdev) > >> +{ > >> + struct device *dev = &pdev->dev, *pd_dev; > >> + const char *pd_name; > >> + int id, count, ret = 0; > >> + > >> + count = of_count_phandle_with_args(dev->of_node, "power-domains", > >> + "#power-domain-cells"); > > > > Perhaps this should be converted to genpd OF helper function instead, > > that allows the caller to know how many power-domains there are > > specified for a device node. > > I am ok with this if you think that a OF helper to get the number of > power domains is a useful helper in the genpd framework. I can add it as > part of the next revision. Or do you want me to send it across separate? Feel free to include in the next version of the series. In case it's needed. > > > >> + > >> + if (count > 1) { > >> + for (id = 0; id < count; id++) { > >> + ret = of_property_read_string_index > >> + (dev->of_node, "power-domain-names", > >> + id, &pd_name); > >> + if (ret) { > >> + dev_err(dev, "Error reading the power domain name %d\n", ret); > >> + continue; > >> + } > > > > It looks a bit awkward that you want to re-use the power-domain-names > > as the name for the cooling (warming) device. This isn't really what > > we use the "*-names" bindings for in general, I think. > > > > Anyway, if you want a name corresponding to the actual attached PM > > domain, perhaps re-using "->name" from the struct generic_pm_domain is > > better. We can add a genpd helper for that, no problem. Of course it > > also means that you must call dev_pm_domain_attach_by_id() first, to > > attach the device and then get the name of the genpd, but that should > > be fine. > > Ya. I need a name corresponding to the power domain name (or something > very close) to identify the actual warming device in the sysfs entries. > I can use genpd->name and a helper function to achieve it. I can include > it in Patch 1/5 where I add other helper functions. A separate patch please, but yeah, fold it in into @subject series. > > > >> + > >> + pd_dev = dev_pm_domain_attach_by_id(dev, id); > >> + if (IS_ERR(pd_dev)) { > >> + dev_err(dev, "Error attaching power domain %s %ld\n", pd_name, PTR_ERR(pd_dev)); > >> + continue; > >> + } > >> + > >> + ret = pd_wdev_create(pd_dev, pd_name); > >> + if (ret) { > >> + dev_err(dev, "Error building cooling device %s %d\n", pd_name, ret); > >> + dev_pm_domain_detach(pd_dev, false); > >> + continue; > >> + } > > > > I am wondering about the use case of having multiple PM domains > > attached to the cooling (warming) device. Is that really needed? > > Perhaps you can elaborate on that a bit? > Ya. I though about this as well. I don't have a use case. In my current > case it is just one power domain on the SoC. But considering this is now > a generic driver, in my opinion this has to be a generic solution. So if > you think about this, the device should be able to specify any number of > power domains that can behave as a warming device since a SoC can have > any number of power domain based warming devices. May be one to warm up > the cpus, one for gpus etc. I get that, but you can always have more than one warming device. Each warming device would then be attached to a single PM domain. Or is there a problem with that? In any case, if you don't have use case for multiple PM domains per warming device at this point, I would rather keep it simple and start to support only the single PM domain case. > > So another way of implementing this whole thing is to avoid having a > special power domain warming device defined in the device tree. Instead, > add a few new binding to the power-domain controller/provider entries > to specify if a power domain controlled by the provider can act as a > warming device or not. And have the initialization code for the power > domain controller (of_genpd_add_provider_onecell or any other suitable > API) register the specified power domain as a warming device. The DT > entries should probably look something like below in the case. > > rpmhpd: power-controller { > compatible = "qcom,sdm845-rpmhpd"; > #power-domain-cells = <1>; > hosts-warming-dev; > warming-dev-names = "mx"; > operating-points-v2 = <&rpmhpd_opp_table>; > > rpmhpd_opp_table: opp-table { > compatible = "operating-points-v2"; > .... > > And have the following in of_genpd_add_provider_onecell > > if (hosts-warming-dev) > # loop through the warming-dev-names and register them as power domain > warming devices. > > You think this is a better idea? Not really, but you need to re-direct that question to DT maintainers if want a better answer. > > > > >> + } > >> + } else if (count == 1) { > >> + ret = of_property_read_string_index(dev->of_node, > >> + "power-domain-names", > >> + 0, &pd_name); > >> + if (ret) { > >> + dev_err(dev, "Error reading the power domain name %d\n", ret); > >> + goto exit; > >> + } > > > > According to my comment above, perhaps we simply don't have to use the > > "power-domain-names" binding at all. > I will use genpd->name > [...] Kind regards Uffe ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] thermal: Add generic power domain warming device driver. 2019-09-13 7:54 ` Ulf Hansson @ 2019-09-14 11:06 ` Thara Gopinath 0 siblings, 0 replies; 12+ messages in thread From: Thara Gopinath @ 2019-09-14 11:06 UTC (permalink / raw) To: Ulf Hansson Cc: Eduardo Valentin, Zhang Rui, Daniel Lezcano, Bjorn Andersson, Rob Herring, agross, amit.kucheria, Mark Rutland, Rafael J. Wysocki, Linux PM, DTML, linux-arm-msm, Linux Kernel Mailing List On 09/13/2019 03:54 AM, Ulf Hansson wrote: > On Thu, 12 Sep 2019 at 22:18, Thara Gopinath <thara.gopinath@linaro.org> wrote: >> >> On 09/12/2019 11:04 AM, Ulf Hansson wrote: >> >> Hi Ulf, >> >> Thanks for the review. >>> On Tue, 10 Sep 2019 at 19:14, Thara Gopinath <thara.gopinath@linaro.org> wrote: >>>> >>>> Resources modeled as power domains in linux kenrel >>>> can be used to warm the SoC(eg. mx power domain on sdm845). >>>> To support this feature, introduce a generic power domain >>>> warming device driver that can be plugged into the thermal framework >>>> (The thermal framework itself requires further modifiction to >>>> support a warming device in place of a cooling device. >>>> Those extensions are not introduced in this patch series). >>>> >>>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> >>>> --- >>>> v1->v2: >>>> - Make power domain based warming device driver a generic >>>> driver in the thermal framework. v1 implemented this as a >>>> Qualcomm specific driver. >>>> - Rename certain variables as per review suggestions on the >>>> mailing list. >>>> >>>> drivers/thermal/Kconfig | 11 +++ >>>> drivers/thermal/Makefile | 2 + >>>> drivers/thermal/pwr_domain_warming.c | 174 +++++++++++++++++++++++++++++++++++ >>>> 3 files changed, 187 insertions(+) >>>> create mode 100644 drivers/thermal/pwr_domain_warming.c >>>> >>>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig >>>> index 9966364..eeb6018 100644 >>>> --- a/drivers/thermal/Kconfig >>>> +++ b/drivers/thermal/Kconfig >>>> @@ -187,6 +187,17 @@ config DEVFREQ_THERMAL >>>> >>>> If you want this support, you should say Y here. >>>> >>>> +config PWR_DOMAIN_WARMING_THERMAL >>>> + bool "Power Domain based warming device" >>>> + depends on PM_GENERIC_DOMAINS >>>> + depends on PM_GENERIC_DOMAINS_OF >>> >>> PM_GENERIC_DOMAINS_OF can't be set unless PM_GENERIC_DOMAINS is set too. >>> >>> So I assume it's sufficient to depend on PM_GENERIC_DOMAINS_OF? >> >> Yes, you are right. I will change it. >>> >>>> + help >>>> + This implements the generic power domain based warming >>>> + mechanism through increasing the performance state of >>>> + a power domain. >>>> + >>>> + If you want this support, you should say Y here. >>>> + >>>> config THERMAL_EMULATION >>>> bool "Thermal emulation mode support" >>>> help >>>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile >>>> index 74a37c7..382c64a 100644 >>>> --- a/drivers/thermal/Makefile >>>> +++ b/drivers/thermal/Makefile >>>> @@ -27,6 +27,8 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL) += clock_cooling.o >>>> # devfreq cooling >>>> thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o >>>> >>>> +thermal_sys-$(CONFIG_PWR_DOMAIN_WARMING_THERMAL) += pwr_domain_warming.o >>>> + >>>> # platform thermal drivers >>>> obj-y += broadcom/ >>>> obj-$(CONFIG_THERMAL_MMIO) += thermal_mmio.o >>>> diff --git a/drivers/thermal/pwr_domain_warming.c b/drivers/thermal/pwr_domain_warming.c >>>> new file mode 100644 >>>> index 0000000..3dd792b >>>> --- /dev/null >>>> +++ b/drivers/thermal/pwr_domain_warming.c >>>> @@ -0,0 +1,174 @@ >>>> +// SPDX-License-Identifier: GPL-2.0 >>>> +/* >>>> + * Copyright (c) 2019, Linaro Ltd >>>> + */ >>>> +#include <linux/err.h> >>>> +#include <linux/kernel.h> >>>> +#include <linux/init.h> >>>> +#include <linux/of_device.h> >>>> +#include <linux/platform_device.h> >>>> +#include <linux/module.h> >>>> +#include <linux/pm_domain.h> >>>> +#include <linux/pm_runtime.h> >>>> +#include <linux/thermal.h> >>>> + >>>> +struct pd_warming_device { >>>> + struct thermal_cooling_device *cdev; >>>> + struct device *dev; >>>> + int max_state; >>>> + int cur_state; >>>> + bool runtime_resumed; >>>> +}; >>>> + >>>> +static const struct of_device_id pd_wdev_match_table[] = { >>>> + { .compatible = "thermal-power-domain-wdev", .data = NULL }, >>>> + { } >>>> +}; >>>> +MODULE_DEVICE_TABLE(of, pd_wdev_match_table); >>>> + >>>> +static int pd_wdev_get_max_state(struct thermal_cooling_device *cdev, >>>> + unsigned long *state) >>>> +{ >>>> + struct pd_warming_device *pd_wdev = cdev->devdata; >>>> + >>>> + *state = pd_wdev->max_state; >>>> + return 0; >>>> +} >>>> + >>>> +static int pd_wdev_get_cur_state(struct thermal_cooling_device *cdev, >>>> + unsigned long *state) >>>> +{ >>>> + struct pd_warming_device *pd_wdev = cdev->devdata; >>>> + >>>> + *state = dev_pm_genpd_get_performance_state(pd_wdev->dev); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static int pd_wdev_set_cur_state(struct thermal_cooling_device *cdev, >>>> + unsigned long state) >>>> +{ >>>> + struct pd_warming_device *pd_wdev = cdev->devdata; >>>> + struct device *dev = pd_wdev->dev; >>>> + int ret; >>>> + >>>> + ret = dev_pm_genpd_set_performance_state(dev, state); >>>> + >>>> + if (ret) >>>> + return ret; >>>> + >>>> + if (state && !pd_wdev->runtime_resumed) { >>>> + ret = pm_runtime_get_sync(dev); >>>> + pd_wdev->runtime_resumed = true; >>>> + } else if (!state && pd_wdev->runtime_resumed) { >>>> + ret = pm_runtime_put(dev); >>>> + pd_wdev->runtime_resumed = false; >>>> + } >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static struct thermal_cooling_device_ops pd_warming_device_ops = { >>>> + .get_max_state = pd_wdev_get_max_state, >>>> + .get_cur_state = pd_wdev_get_cur_state, >>>> + .set_cur_state = pd_wdev_set_cur_state, >>>> +}; >>>> + >>>> +static int pd_wdev_create(struct device *dev, const char *name) >>>> +{ >>>> + struct pd_warming_device *pd_wdev; >>>> + int state_count; >>>> + >>>> + pd_wdev = devm_kzalloc(dev, sizeof(*pd_wdev), GFP_KERNEL); >>>> + if (!pd_wdev) >>>> + return -ENOMEM; >>>> + >>>> + state_count = dev_pm_genpd_performance_state_count(dev); >>>> + if (state_count < 0) >>>> + return state_count; >>>> + >>>> + pd_wdev->dev = dev; >>>> + pd_wdev->max_state = state_count - 1; >>>> + pd_wdev->runtime_resumed = false; >>>> + >>>> + pm_runtime_enable(dev); >>>> + >>>> + pd_wdev->cdev = thermal_of_cooling_device_register >>>> + (dev->of_node, name, >>>> + pd_wdev, >>>> + &pd_warming_device_ops); >>>> + if (IS_ERR(pd_wdev->cdev)) { >>>> + dev_err(dev, "unable to register %s cooling device\n", name); >>>> + pm_runtime_disable(dev); >>>> + >>>> + return PTR_ERR(pd_wdev->cdev); >>>> + } >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static int pd_wdev_probe(struct platform_device *pdev) >>>> +{ >>>> + struct device *dev = &pdev->dev, *pd_dev; >>>> + const char *pd_name; >>>> + int id, count, ret = 0; >>>> + >>>> + count = of_count_phandle_with_args(dev->of_node, "power-domains", >>>> + "#power-domain-cells"); >>> >>> Perhaps this should be converted to genpd OF helper function instead, >>> that allows the caller to know how many power-domains there are >>> specified for a device node. >> >> I am ok with this if you think that a OF helper to get the number of >> power domains is a useful helper in the genpd framework. I can add it as >> part of the next revision. Or do you want me to send it across separate? > > Feel free to include in the next version of the series. In case it's needed. Will do, if needed. (But as per below I am removing multiple PD support and hence this might not be needed) > >>> >>>> + >>>> + if (count > 1) { >>>> + for (id = 0; id < count; id++) { >>>> + ret = of_property_read_string_index >>>> + (dev->of_node, "power-domain-names", >>>> + id, &pd_name); >>>> + if (ret) { >>>> + dev_err(dev, "Error reading the power domain name %d\n", ret); >>>> + continue; >>>> + } >>> >>> It looks a bit awkward that you want to re-use the power-domain-names >>> as the name for the cooling (warming) device. This isn't really what >>> we use the "*-names" bindings for in general, I think. >>> >>> Anyway, if you want a name corresponding to the actual attached PM >>> domain, perhaps re-using "->name" from the struct generic_pm_domain is >>> better. We can add a genpd helper for that, no problem. Of course it >>> also means that you must call dev_pm_domain_attach_by_id() first, to >>> attach the device and then get the name of the genpd, but that should >>> be fine. >> >> Ya. I need a name corresponding to the power domain name (or something >> very close) to identify the actual warming device in the sysfs entries. >> I can use genpd->name and a helper function to achieve it. I can include >> it in Patch 1/5 where I add other helper functions. > > A separate patch please, but yeah, fold it in into @subject series. Sure! > >>> >>>> + >>>> + pd_dev = dev_pm_domain_attach_by_id(dev, id); >>>> + if (IS_ERR(pd_dev)) { >>>> + dev_err(dev, "Error attaching power domain %s %ld\n", pd_name, PTR_ERR(pd_dev)); >>>> + continue; >>>> + } >>>> + >>>> + ret = pd_wdev_create(pd_dev, pd_name); >>>> + if (ret) { >>>> + dev_err(dev, "Error building cooling device %s %d\n", pd_name, ret); >>>> + dev_pm_domain_detach(pd_dev, false); >>>> + continue; >>>> + } >>> >>> I am wondering about the use case of having multiple PM domains >>> attached to the cooling (warming) device. Is that really needed? >>> Perhaps you can elaborate on that a bit? >> Ya. I though about this as well. I don't have a use case. In my current >> case it is just one power domain on the SoC. But considering this is now >> a generic driver, in my opinion this has to be a generic solution. So if >> you think about this, the device should be able to specify any number of >> power domains that can behave as a warming device since a SoC can have >> any number of power domain based warming devices. May be one to warm up >> the cpus, one for gpus etc. > > I get that, but you can always have more than one warming device. Each > warming device would then be attached to a single PM domain. Or is > there a problem with that? > > In any case, if you don't have use case for multiple PM domains per > warming device at this point, I would rather keep it simple and start > to support only the single PM domain case. Ok. I will remove the support for multiple PM domains for now. > >> >> So another way of implementing this whole thing is to avoid having a >> special power domain warming device defined in the device tree. Instead, >> add a few new binding to the power-domain controller/provider entries >> to specify if a power domain controlled by the provider can act as a >> warming device or not. And have the initialization code for the power >> domain controller (of_genpd_add_provider_onecell or any other suitable >> API) register the specified power domain as a warming device. The DT >> entries should probably look something like below in the case. >> >> rpmhpd: power-controller { >> compatible = "qcom,sdm845-rpmhpd"; >> #power-domain-cells = <1>; >> hosts-warming-dev; >> warming-dev-names = "mx"; >> operating-points-v2 = <&rpmhpd_opp_table>; >> >> rpmhpd_opp_table: opp-table { >> compatible = "operating-points-v2"; >> .... >> >> And have the following in of_genpd_add_provider_onecell >> >> if (hosts-warming-dev) >> # loop through the warming-dev-names and register them as power domain >> warming devices. >> >> You think this is a better idea? > > Not really, but you need to re-direct that question to DT maintainers > if want a better answer. I will wait for the DT folks to take a look at this series. Hopefully DT folks will have some comments on the approach of a virtual device like this implementation vs specifying this info in the power domain controllers. I just wanted to run it by you to check whether you see any pros or cons from a genpd perspective. I will wait for a few more days for any additional review comments before sending v3 out. -- Warm Regards Thara ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 5/5] arm64: dts: qcom: Add node for RPMH power domain warming device on sdm845. 2019-09-10 17:14 [PATCH v2 0/5] Introduce Power domain based warming device driver Thara Gopinath ` (3 preceding siblings ...) 2019-09-10 17:14 ` [PATCH 4/5] thermal: Add generic power domain warming device driver Thara Gopinath @ 2019-09-10 17:14 ` Thara Gopinath 4 siblings, 0 replies; 12+ messages in thread From: Thara Gopinath @ 2019-09-10 17:14 UTC (permalink / raw) To: edubezval, rui.zhang, ulf.hansson, daniel.lezcano, bjorn.andersson, robh+dt, agross Cc: amit.kucheria, mark.rutland, rjw, linux-pm, devicetree, linux-arm-msm, linux-kernel RPMh hosts power domains that can be used to warm up the SoC. Add nodes for these domains on sdm845 (mx power domain). Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> --- v1->v2: - Change the dt-entries to reflect the newly introduced generic power domain warming device. arch/arm64/boot/dts/qcom/sdm845.dtsi | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index f406a43..0a83263 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -3284,6 +3284,13 @@ }; }; + thermal_wdev: rpmhpd_mx_wdev { + compatible = "thermal-power-domain-wdev"; + #cooling-cells = <2>; + power-domains = <&rpmhpd SDM845_MX>; + power-domain-names = "mx"; + }; + thermal-zones { cpu0-thermal { polling-delay-passive = <250>; -- 2.1.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-10-09 12:18 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-10 17:14 [PATCH v2 0/5] Introduce Power domain based warming device driver Thara Gopinath 2019-09-10 17:14 ` [PATCH v2 1/5] PM/Domains: Add support for retrieving genpd performance states information Thara Gopinath 2019-09-10 17:14 ` [PATCH v2 2/5] soc: qcom: rpmhpd: Introduce function to retrieve power domain performance state count Thara Gopinath 2019-09-10 17:14 ` [PATCH 3/5] dt-bindings: thermal: Add generic power domain warming device binding Thara Gopinath 2019-09-30 14:42 ` Rob Herring 2019-10-09 12:18 ` Thara Gopinath 2019-09-10 17:14 ` [PATCH 4/5] thermal: Add generic power domain warming device driver Thara Gopinath 2019-09-12 15:04 ` Ulf Hansson 2019-09-12 20:18 ` Thara Gopinath 2019-09-13 7:54 ` Ulf Hansson 2019-09-14 11:06 ` Thara Gopinath 2019-09-10 17:14 ` [PATCH 5/5] arm64: dts: qcom: Add node for RPMH power domain warming device on sdm845 Thara Gopinath
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).