Add RSC power domain support
v2 cover Add RSC power domain support
Changes in v2:
- Add Stephen's Reviewed-By to the first three patches
- Addressed Stephen's comments on fourth patch
- Include changes to connect rpmh domain to cpuidle and genpds
Resource State Coordinator (RSC) is responsible for powering off/lowering the requirements from CPU subsystem for the associated hardware like buses, clocks, and regulators when all CPUs and cluster is powered down.
RSC power domain uses last-man activities provided by genpd framework based on Ulf Hansoon's patch series[1], when the cluster of CPUs enter deepest idle states. As a part of domain poweroff, RSC can lower resource state requirements by flushing the cached sleep and wake state votes for resources.
Dependencies:
[1] https://lkml.org/lkml/2019/5/13/839
Maulik Shah (6):
drivers: qcom: rpmh: fix macro to accept NULL argument
drivers: qcom: rpmh: remove rpmh_flush export
dt-bindings: soc: qcom: Add RSC power domain specifier
drivers: qcom: rpmh-rsc: Add RSC power domain support
arm64: dts: Convert to the hierarchical CPU topology layout for sdm845
arm64: dts: Add rsc power domain for sdm845
.../devicetree/bindings/soc/qcom/rpmh-rsc.txt | 8 ++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 105 +++++++++++++----- drivers/soc/qcom/rpmh-internal.h | 3 + drivers/soc/qcom/rpmh-rsc.c | 84 ++++++++++++++ drivers/soc/qcom/rpmh.c | 22 ++-- include/soc/qcom/rpmh.h | 5 - 6 files changed, 185 insertions(+), 42 deletions(-)
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation.
PATCH v2 1/6 drivers: qcom: rpmh: fix macro to accept NULL argument
Device argument matches with dev variable declared in RPMH message. Compiler reports error when the argument is NULL since the argument matches the name of the property. Rename dev argument to device to fix this.
--- drivers/soc/qcom/rpmh.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/soc/qcom/rpmh.c +2 -2
@@ -23,7 +23,7 @@#define RPMH_TIMEOUT_MS msecs_to_jiffies(10000) -#define DEFINE_RPMH_MSG_ONSTACK (dev, s, q, name) \ +#define DEFINE_RPMH_MSG_ONSTACK (device, s, q, name) \ struct rpmh_request name = { \ .msg = { \ .state = s, \
@@ -33,7 +33,7 @@ }, \ .cmd = { { 0 } }, \ .completion = q, \ - .dev = dev, \ + .dev = device, \ .needs_free = false, \ }
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation.
PATCH v2 2/6 drivers: qcom: rpmh: remove rpmh_flush export
In reply to Maulik Shahrpmh_flush() was exported with the idea that an external entity operation during CPU idle would know when to flush the sleep and wake TCS. Since, this is not the case when defining a power domain for the RSC. Remove the function export and instead allow the function to be called internally.
--- drivers/soc/qcom/rpmh-internal.h | 1 + drivers/soc/qcom/rpmh.c | 18 ++++++++---------- include/soc/qcom/rpmh.h | 5 ----- 3 files changed, 9 insertions(+), 15 deletions(-)
drivers/soc/qcom/rpmh-internal.h +1 -0
@@ -110,5 +110,6 @@ int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv,int rpmh_rsc_invalidate (struct rsc_drv *drv);void rpmh_tx_done (const struct tcs_request *msg,int r); +int rpmh_flush (struct rpmh_ctrlr *ctrlr);#endif /* __RPM_INTERNAL_H__ */
drivers/soc/qcom/rpmh.c +8 -10
@@ -427,11 +427,10 @@ static int is_req_valid(struct cache_req *req) req->sleep_val != req->wake_val); } -static int send_single (const struct device *dev,enum rpmh_state state, +static int send_single (struct rpmh_ctrlr *ctrlr,enum rpmh_state state,u32 addr,u32 data) { -DEFINE_RPMH_MSG_ONSTACK (dev, state,NULL , rpm_msg); -struct rpmh_ctrlr *ctrlr =get_rpmh_ctrlr (dev); +DEFINE_RPMH_MSG_ONSTACK (NULL , state,NULL , rpm_msg);/* Wake sets are always complete and sleep sets are not */ rpm_msg.msg.wait_for_compl = (state ==RPMH_WAKE_ONLY_STATE );
@@ -445,7 +444,7 @@ static int send_single(const struct device *dev, enum rpmh_state state,/** * rpmh_flush: Flushes the buffered active and sleep sets to TCS * -* @dev: The device making the request +* @ctrlr: controller making request to flush cached data * * Return: -EBUSY if the controller is busy, probably waiting on a response * to a RPMH request sent earlier.
@@ -454,10 +453,9 @@ static int send_single(const struct device *dev, enum rpmh_state state,* that is powering down the entire system. Since no other RPMH API would be * executing at this time, it is safe to run lockless. */ -int rpmh_flush (const struct device *dev) +int rpmh_flush (struct rpmh_ctrlr *ctrlr) {struct cache_req *p; -struct rpmh_ctrlr *ctrlr =get_rpmh_ctrlr (dev);int ret;if (!ctrlr->dirty) {
@@ -480,11 +478,12 @@ int rpmh_flush(const struct device *dev) __func__, p->addr, p->sleep_val, p->wake_val);continue ; } - ret =send_single (dev,RPMH_SLEEP_STATE , p->addr, p->sleep_val); + ret =send_single (ctrlr,RPMH_SLEEP_STATE , p->addr, + p->sleep_val);if (ret)return ret; - ret =send_single (dev,RPMH_WAKE_ONLY_STATE , - p->addr, p->wake_val); + ret =send_single (ctrlr,RPMH_WAKE_ONLY_STATE , p->addr, + p->wake_val);if (ret)return ret; }
@@ -493,7 +492,6 @@ int rpmh_flush(const struct device *dev)return 0 ; } -EXPORT_SYMBOL (rpmh_flush);/** * rpmh_invalidate: Invalidate all sleep and active sets
include/soc/qcom/rpmh.h +0 -5
@@ -20,8 +20,6 @@ int rpmh_write_async(const struct device *dev, enum rpmh_state state,int rpmh_write_batch (const struct device *dev,enum rpmh_state state,const struct tcs_cmd *cmd,u32 *n); -int rpmh_flush (const struct device *dev); -int rpmh_invalidate (const struct device *dev);#else
@@ -40,9 +38,6 @@ static inline int rpmh_write_batch(const struct device *dev, const struct tcs_cmd *cmd, u32 *n) { return -ENODEV; } -static inline int rpmh_flush (const struct device *dev) -{return -ENODEV ; } -static inline int rpmh_invalidate (const struct device *dev) {return -ENODEV ; }
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation.
PATCH v2 3/6 dt-bindings: soc: qcom: Add RSC power domain specifier
In reply to Maulik ShahIn addition to transmitting resource state requests to the remote processor, the RSC is responsible for powering off/lowering the requirements from CPUs subsystem for the associated hardware like buses, clocks, and regulators when all CPUs and cluster is powered down.
The power domain is configured to a low power state and when all the CPUs are powered down, the RSC can lower resource state requirements and power down the rails that power the CPUs.
Add PM domain specifier property for RSC controller.
--- Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt | 8 ++++++++ 1 file changed, 8 insertions(+)
Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt +8 -0
@@ -83,6 +83,13 @@ Properties: Value type: <string> Definition: Name for the RSC. The name would be used in trace logs. +- #power-domain-cells: + Usage: optional + Value type: <u32> + Definition: Number of cells in power domain specifier. Optional for + controllers that may be in 'solver' state where they can + be in autonomous mode executing low power modes. + Drivers that want to use the RSC to communicate with RPMH must specify their bindings as child nodes of the RSC controllers they wish to communicate with.
@@ -112,6 +119,7 @@ TCS-OFFSET: 0xD00 <SLEEP_TCS 3>, <WAKE_TCS 3>, <CONTROL_TCS 1>; + #power-domain-cells = <0>; }; Example 2:
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation.
Rob Herring
maintainerRe: [PATCH v2 3/6] dt-bindings: soc: qcom: Add RSC power domain specifier
Quoted 23 lines · Maulik Shah
In addition to transmitting resource state requests to the remote processor, the RSC is responsible for powering off/lowering the requirements from CPUs subsystem for the associated hardware like buses, clocks, and regulators when all CPUs and cluster is powered down.
The power domain is configured to a low power state and when all the CPUs are powered down, the RSC can lower resource state requirements and power down the rails that power the CPUs.
Add PM domain specifier property for RSC controller.
Signed-off-by Maulik Shah <[email protected]>Reviewed-by Stephen Boyd <[email protected]>--- Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt | 8 ++++++++ 1 file changed, 8 insertions(+)Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt +0
-0
Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt Properties: · line 91
Quoted 9 lines
Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt +6
-0@@ -83,6 +83,13 @@ Properties: Value type: <string> Definition: Name for the RSC. The name would be used in trace logs. +- #power-domain-cells: + Usage: optional + Value type: <u32> + Definition: Number of cells in power domain specifier. Optional for + controllers that may be in 'solver' state where they can + be in autonomous mode executing low power modes.
What's the value? It's always 0?
Quoted 15 lines
Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt +2
-0+ Drivers that want to use the RSC to communicate with RPMH must specify their bindings as child nodes of the RSC controllers they wish to communicate with.@@ -112,6 +119,7 @@ TCS-OFFSET: 0xD00 <SLEEP_TCS 3>, <WAKE_TCS 3>, <CONTROL_TCS 1>; + #power-domain-cells = <0>; }; Example 2:Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation.
Re: [PATCH v2 3/6] dt-bindings: soc: qcom: Add RSC power domain specifier
Quoted 27 lines · Rob Herring
Quoted 26 lines · Maulik Shah
In addition to transmitting resource state requests to the remote processor, the RSC is responsible for powering off/lowering the requirements from CPUs subsystem for the associated hardware like buses, clocks, and regulators when all CPUs and cluster is powered down.
The power domain is configured to a low power state and when all the CPUs are powered down, the RSC can lower resource state requirements and power down the rails that power the CPUs.
Add PM domain specifier property for RSC controller.
Signed-off-by Maulik Shah <[email protected]>Reviewed-by Stephen Boyd <[email protected]>--- Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt | 8 ++++++++ 1 file changed, 8 insertions(+)Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt +0
-0@@ -83,6 +83,13 @@ Properties: Value type: <string> Definition: Name for the RSC. The name would be used in trace logs.
Quoted 7 lines
Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt Properties: · line 91
Quoted 6 lines
Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt +6
-0+- #power-domain-cells: + Usage: optional + Value type: <u32> + Definition: Number of cells in power domain specifier. Optional for + controllers that may be in 'solver' state where they can + be in autonomous mode executing low power modes.What's the value? It's always 0?
yes. its value is always 0. i will update definition to mention this in next version.
Quoted 15 lines
Quoted 15 lines
Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt +2
-0+ Drivers that want to use the RSC to communicate with RPMH must specify their bindings as child nodes of the RSC controllers they wish to communicate with.@@ -112,6 +119,7 @@ TCS-OFFSET: 0xD00 <SLEEP_TCS 3>, <WAKE_TCS 3>, <CONTROL_TCS 1>; + #power-domain-cells = <0>; }; Example 2:Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation.
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH v2 4/6 drivers: qcom: rpmh-rsc: Add RSC power domain support
In reply to Maulik ShahAdd RSC power domain support. RSC is top level power domain in hireachical CPU LPM modes. Once the rsc domain is down flush all cached sleep and wake votes from controller.
--- drivers/soc/qcom/rpmh-internal.h | 2 + drivers/soc/qcom/rpmh-rsc.c | 84 ++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+)
drivers/soc/qcom/rpmh-internal.h +2 -0
@@ -8,6 +8,7 @@#define __RPM_INTERNAL_H__#include <linux/bitmap.h> +#include <linux/pm_domain.h> #include <soc/qcom/tcs.h> #define TCS_TYPE_NR 4
@@ -102,6 +103,7 @@ struct rsc_drv {DECLARE_BITMAP (tcs_in_use,MAX_TCS_NR );spinlock_t lock;struct rpmh_ctrlr client; +struct generic_pm_domain rsc_pd; };int rpmh_rsc_send_data (struct rsc_drv *drv,const struct tcs_request *msg);
drivers/soc/qcom/rpmh-rsc.c +84 -0
102 lines not on this page · raw message
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation.
Re: [PATCH v2 4/6] drivers: qcom: rpmh-rsc: Add RSC power domain support
Quoting Maulik Shah (2019-08-23 01:17:01)
Quoted 5 lines
drivers/soc/qcom/rpmh-rsc.c +0
-0
drivers/soc/qcom/rpmh-rsc.c static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg) · line 506
Quoted 9 lines
drivers/soc/qcom/rpmh-rsc.c +6
-0@@ -498,6 +498,32 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg)return ret; } +/** +* rpmh_rsc_ctrlr_is_idle: Check if any of the AMCs are busy. +* +* @drv: The controller +* +* Returns false if the TCSes are engaged in handling requests,
Please use kernel-doc style for returns here.
drivers/soc/qcom/rpmh-rsc.c static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg) · line 515
Quoted 9 lines
drivers/soc/qcom/rpmh-rsc.c +9
-0+* True if controller is idle. +*/ +static bool rpmh_rsc_ctrlr_is_idle (struct rsc_drv *drv) +{ +int m; +struct tcs_group *tcs =get_tcs_of_type (drv,ACTIVE_TCS ); +bool ret = true; + +spin_lock (&drv->lock);
I think these need to be irqsave/restore still.
drivers/soc/qcom/rpmh-rsc.c static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg) · line 517
Quoted 2 lines
drivers/soc/qcom/rpmh-rsc.c +2
-0+for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) { +if (!tcs_is_free (drv, m)) {
This snippet is from tcs_invalidate(). Please collapse it into some sort of function or macro like for_each_tcs().
Quoted 15 lines
drivers/soc/qcom/rpmh-rsc.c +9
-0+ ret = false; +break ; + } + } +spin_unlock (&drv->lock); + +return ret; +} +/** * rpmh_rsc_write_ctrl_data: Write request to the controller * @@ -521,6 +547,53 @@ int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg)return tcs_ctrl_write (drv, msg); }
drivers/soc/qcom/rpmh-rsc.c int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg) · line 558
Quoted 10 lines
drivers/soc/qcom/rpmh-rsc.c +9
-0+static int rpmh_domain_power_off (struct generic_pm_domain *rsc_pd) +{ + struct rsc_drv *drv = container_of(rsc_pd, struct rsc_drv, rsc_pd); + +/* +* RPMh domain can not be powered off when there is pending ACK for +* ACTIVE_TCS request. Exit when controller is busy. +*/ +
Nitpick: Remove this extra newline.
Quoted 4 lines
drivers/soc/qcom/rpmh-rsc.c +4
-0+if (!rpmh_rsc_ctrlr_is_idle (drv)) +return -EBUSY ; + +return rpmh_flush (&drv->client);
drivers/soc/qcom/rpmh-rsc.c int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg) · line 572
Quoted 10 lines
drivers/soc/qcom/rpmh-rsc.c +10
-0+} + +static int rpmh_probe_power_domain (struct platform_device *pdev, +struct rsc_drv *drv) +{ +int ret; +struct generic_pm_domain *rsc_pd = &drv->rsc_pd; +struct device_node *dn = pdev->dev.of_node; + + rsc_pd->name =kasprintf (GFP_KERNEL ,"%s" , dn->name);
Maybe use devm_kasprintf?
Quoted 11 lines
drivers/soc/qcom/rpmh-rsc.c +11
-0+if (!rsc_pd->name) +return -ENOMEM ; + + rsc_pd->name =kbasename (rsc_pd->name); + rsc_pd->power_off = rpmh_domain_power_off; + rsc_pd->flags |=GENPD_FLAG_IRQ_SAFE ; + + ret =pm_genpd_init (rsc_pd,NULL , false); +if (ret) + goto free_name; +
drivers/soc/qcom/rpmh-rsc.c int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg) · line 593
Quoted 10 lines
drivers/soc/qcom/rpmh-rsc.c +10
-0+ ret =of_genpd_add_provider_simple (dn, rsc_pd); +if (ret) + goto remove_pd; + +return ret; + +remove_pd: +pm_genpd_remove (rsc_pd); +free_name: +kfree (rsc_pd->name);
And then drop this one?
Quoted 13 lines
drivers/soc/qcom/rpmh-rsc.c +6
-0+return ret; +} +static int rpmh_probe_tcs_config (struct platform_device *pdev,struct rsc_drv *drv) {@@ -650,6 +723,17 @@ static int rpmh_rsc_probe(struct platform_device *pdev)if (ret)return ret; +/* +* Power domain is not required for controllers that support 'solver' +* mode where they can be in autonomous mode executing low power mode
drivers/soc/qcom/rpmh-rsc.c static int rpmh_rsc_probe(struct platform_device *pdev) · line 738
Quoted 10 lines
drivers/soc/qcom/rpmh-rsc.c +8
-0+* to power down. +*/ +if (of_property_read_bool (dn,"#power-domain-cells" )) { + ret =rpmh_probe_power_domain (pdev, drv); +if (ret) +return ret; + } +spin_lock_init (&drv->lock);bitmap_zero (drv->tcs_in_use,MAX_TCS_NR );
What happens if it fails later on? The genpd provider is still sitting around and needs to be removed on probe failure later on in this function. It would be nicer if there wasn't another function to probe the power domain and it was just inlined here actually. That way we don't have to wonder about what's going on across two blocks of code.
Re: [PATCH v2 4/6] drivers: qcom: rpmh-rsc: Add RSC power domain support
Quoted 9 lines · Stephen Boyd
Quoting Maulik Shah (2019-08-23 01:17:01)
Quoted 8 lines
drivers/soc/qcom/rpmh-rsc.c +0
-0@@ -498,6 +498,32 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg)return ret; }
Quoted 7 lines
drivers/soc/qcom/rpmh-rsc.c static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg) · line 506
Quoted 6 lines
drivers/soc/qcom/rpmh-rsc.c +6
-0+/** +* rpmh_rsc_ctrlr_is_idle: Check if any of the AMCs are busy. +* +* @drv: The controller +* +* Returns false if the TCSes are engaged in handling requests, Please use kernel-doc style for returns here.
Done.
Quoted 10 lines
drivers/soc/qcom/rpmh-rsc.c static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg) · line 515
Quoted 9 lines
drivers/soc/qcom/rpmh-rsc.c +9
-0+* True if controller is idle. +*/ +static bool rpmh_rsc_ctrlr_is_idle (struct rsc_drv *drv) +{ +int m; +struct tcs_group *tcs =get_tcs_of_type (drv,ACTIVE_TCS ); +bool ret = true; + +spin_lock (&drv->lock);I think these need to be irqsave/restore still.
Done.
Quoted 4 lines
drivers/soc/qcom/rpmh-rsc.c static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg) · line 517
Quoted 2 lines
drivers/soc/qcom/rpmh-rsc.c +2
-0+for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) { +if (!tcs_is_free (drv, m)) {This snippet is from
tcs_invalidate(). Please collapse it into some sort of function or macro likefor_each_tcs().
Keeping it as it is, the snippet is actually little different from tcs_invalidate.
Quoted 16 lines
Quoted 16 lines
drivers/soc/qcom/rpmh-rsc.c +9
-0+ ret = false; +break ; + } + } +spin_unlock (&drv->lock); + +return ret; +} +/** * rpmh_rsc_write_ctrl_data: Write request to the controller * @@ -521,6 +547,53 @@ int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg)return tcs_ctrl_write (drv, msg); }
Quoted 10 lines
drivers/soc/qcom/rpmh-rsc.c int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg) · line 558
Quoted 9 lines
drivers/soc/qcom/rpmh-rsc.c +9
-0+static int rpmh_domain_power_off (struct generic_pm_domain *rsc_pd) +{ + struct rsc_drv *drv = container_of(rsc_pd, struct rsc_drv, rsc_pd); + +/* +* RPMh domain can not be powered off when there is pending ACK for +* ACTIVE_TCS request. Exit when controller is busy. +*/ +Nitpick: Remove this extra newline.
Done.
Quoted 7 lines
Quoted 7 lines
drivers/soc/qcom/rpmh-rsc.c +7
-0+if (!rpmh_rsc_ctrlr_is_idle (drv)) +return -EBUSY ; + +return rpmh_flush (&drv->client); +} + +static int rpmh_probe_power_domain(struct platform_device *pdev,
Quoted 8 lines
drivers/soc/qcom/rpmh-rsc.c int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg) · line 572
Quoted 7 lines
drivers/soc/qcom/rpmh-rsc.c +7
-0+ struct rsc_drv *drv) +{ +int ret; +struct generic_pm_domain *rsc_pd = &drv->rsc_pd; +struct device_node *dn = pdev->dev.of_node; + + rsc_pd->name =kasprintf (GFP_KERNEL ,"%s" , dn->name);Maybe use
devm_kasprintf?
Done.
Quoted 14 lines
Quoted 14 lines
drivers/soc/qcom/rpmh-rsc.c +14
-0+if (!rsc_pd->name) +return -ENOMEM ; + + rsc_pd->name =kbasename (rsc_pd->name); + rsc_pd->power_off = rpmh_domain_power_off; + rsc_pd->flags |=GENPD_FLAG_IRQ_SAFE ; + + ret =pm_genpd_init (rsc_pd,NULL , false); +if (ret) + goto free_name; + + ret =of_genpd_add_provider_simple (dn, rsc_pd); +if (ret) + goto remove_pd;
Quoted 8 lines
drivers/soc/qcom/rpmh-rsc.c int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg) · line 593
Quoted 7 lines
drivers/soc/qcom/rpmh-rsc.c +7
-0+ +return ret; + +remove_pd: +pm_genpd_remove (rsc_pd); +free_name: +kfree (rsc_pd->name);And then drop this one?
Done.
Quoted 20 lines
Quoted 20 lines
drivers/soc/qcom/rpmh-rsc.c +13
-0+return ret; +} +static int rpmh_probe_tcs_config (struct platform_device *pdev,struct rsc_drv *drv) {@@ -650,6 +723,17 @@ static int rpmh_rsc_probe(struct platform_device *pdev)if (ret)return ret; +/* +* Power domain is not required for controllers that support 'solver' +* mode where they can be in autonomous mode executing low power mode +* to power down. +*/ +if (of_property_read_bool (dn,"#power-domain-cells" )) { + ret =rpmh_probe_power_domain (pdev, drv); +if (ret) +return ret; + }
Quoted 8 lines
drivers/soc/qcom/rpmh-rsc.c static int rpmh_rsc_probe(struct platform_device *pdev) · line 738
Quoted 3 lines
drivers/soc/qcom/rpmh-rsc.c +1
-0+spin_lock_init (&drv->lock);bitmap_zero (drv->tcs_in_use,MAX_TCS_NR );What happens if it fails later on? The genpd provider is still sitting around and needs to be removed on probe failure later on in this function. It would be nicer if there wasn't another function to probe the power domain and it was just inlined here actually. That way we don't have to wonder about what's going on across two blocks of code.
Thanks for pointing this. Moved it at the end of probe to avoid this.
Quoted 1 line
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH v2 5/6 arm64: dts: Convert to the hierarchical CPU topology layout for sdm845
In reply to Maulik ShahIn the hierarchical layout, we are creating power domains around each CPU and describes the idle states for them inside the power domain provider node. Note that, the CPU's idle states still needs to be compatible with "arm,idle-state".
Furthermore, represent the CPU cluster as a separate master power domain, powering the CPU's power domains. The cluster node, contains the idle states for the cluster and each idle state needs to be compatible with the "domain-idle-state".
If the running platform is using a PSCI FW that supports the OS initiated CPU suspend mode, which likely should be the case unless the PSCI FW is very old, this change triggers the PSCI driver to enable it.
--- arch/arm64/boot/dts/qcom/sdm845.dtsi | 103 ++++++++++++++++++++------- 1 file changed, 78 insertions(+), 25 deletions(-)
arch/arm64/boot/dts/qcom/sdm845.dtsi +78 -25
@@ -190,9 +190,8 @@ compatible = "qcom,kryo385"; reg = <0x0 0x0>; enable-method = "psci"; - cpu-idle-states = <&LITTLE_CPU_SLEEP_0 - &LITTLE_CPU_SLEEP_1 - &CLUSTER_SLEEP_0>; + power-domains = <&CPU_PD0>; + power-domain-names = "psci"; capacity-dmips-mhz = <607>; qcom,freq-domain = <&cpufreq_hw 0>; #cooling-cells = <2>;
@@ -211,9 +210,8 @@ compatible = "qcom,kryo385"; reg = <0x0 0x100>; enable-method = "psci"; - cpu-idle-states = <&LITTLE_CPU_SLEEP_0 - &LITTLE_CPU_SLEEP_1 - &CLUSTER_SLEEP_0>; + power-domains = <&CPU_PD1>; + power-domain-names = "psci"; capacity-dmips-mhz = <607>; qcom,freq-domain = <&cpufreq_hw 0>; #cooling-cells = <2>;
@@ -229,9 +227,8 @@ compatible = "qcom,kryo385"; reg = <0x0 0x200>; enable-method = "psci"; - cpu-idle-states = <&LITTLE_CPU_SLEEP_0 - &LITTLE_CPU_SLEEP_1 - &CLUSTER_SLEEP_0>; + power-domains = <&CPU_PD2>; + power-domain-names = "psci"; capacity-dmips-mhz = <607>; qcom,freq-domain = <&cpufreq_hw 0>; #cooling-cells = <2>;
@@ -247,9 +244,8 @@ compatible = "qcom,kryo385"; reg = <0x0 0x300>; enable-method = "psci"; - cpu-idle-states = <&LITTLE_CPU_SLEEP_0 - &LITTLE_CPU_SLEEP_1 - &CLUSTER_SLEEP_0>; + power-domains = <&CPU_PD3>; + power-domain-names = "psci"; capacity-dmips-mhz = <607>; qcom,freq-domain = <&cpufreq_hw 0>; #cooling-cells = <2>;
@@ -266,9 +262,8 @@ reg = <0x0 0x400>; enable-method = "psci"; capacity-dmips-mhz = <1024>; - cpu-idle-states = <&BIG_CPU_SLEEP_0 - &BIG_CPU_SLEEP_1 - &CLUSTER_SLEEP_0>; + power-domains = <&CPU_PD4>; + power-domain-names = "psci"; qcom,freq-domain = <&cpufreq_hw 1>; #cooling-cells = <2>; next-level-cache = <&L2_400>;
@@ -284,9 +279,8 @@ reg = <0x0 0x500>; enable-method = "psci"; capacity-dmips-mhz = <1024>; - cpu-idle-states = <&BIG_CPU_SLEEP_0 - &BIG_CPU_SLEEP_1 - &CLUSTER_SLEEP_0>; + power-domains = <&CPU_PD5>; + power-domain-names = "psci"; qcom,freq-domain = <&cpufreq_hw 1>; #cooling-cells = <2>; next-level-cache = <&L2_500>;
@@ -302,9 +296,8 @@ reg = <0x0 0x600>; enable-method = "psci"; capacity-dmips-mhz = <1024>; - cpu-idle-states = <&BIG_CPU_SLEEP_0 - &BIG_CPU_SLEEP_1 - &CLUSTER_SLEEP_0>; + power-domains = <&CPU_PD6>; + power-domain-names = "psci"; qcom,freq-domain = <&cpufreq_hw 1>; #cooling-cells = <2>; next-level-cache = <&L2_600>;
@@ -320,9 +313,8 @@ reg = <0x0 0x700>; enable-method = "psci"; capacity-dmips-mhz = <1024>; - cpu-idle-states = <&BIG_CPU_SLEEP_0 - &BIG_CPU_SLEEP_1 - &CLUSTER_SLEEP_0>; + power-domains = <&CPU_PD7>; + power-domain-names = "psci"; qcom,freq-domain = <&cpufreq_hw 1>; #cooling-cells = <2>; next-level-cache = <&L2_700>;
@@ -412,7 +404,7 @@ }; CLUSTER_SLEEP_0: cluster-sleep-0 { - compatible = "arm,idle-state"; + compatible = "domain-idle-state"; idle-state-name = "cluster-power-down"; arm,psci-suspend-param = <0x400000F4>; entry-latency-us = <3263>;
@@ -618,6 +610,67 @@ psci { compatible = "arm,psci-1.0"; method = "smc"; + + CPU_PD0: cpu-pd0 { + #power-domain-cells = <0>; + power-domains = <&CLUSTER_PD>; + domain-idle-states = <&LITTLE_CPU_SLEEP_0>, + <&LITTLE_CPU_SLEEP_1>; + }; + + CPU_PD1: cpu-pd1 { + #power-domain-cells = <0>; + power-domains = <&CLUSTER_PD>; + domain-idle-states = <&LITTLE_CPU_SLEEP_0>, + <&LITTLE_CPU_SLEEP_1>; + }; + + CPU_PD2: cpu-pd2 { + #power-domain-cells = <0>; + power-domains = <&CLUSTER_PD>; + domain-idle-states = <&LITTLE_CPU_SLEEP_0>, + <&LITTLE_CPU_SLEEP_1>; + }; + + CPU_PD3: cpu-pd3 { + #power-domain-cells = <0>; + power-domains = <&CLUSTER_PD>; + domain-idle-states = <&LITTLE_CPU_SLEEP_0>, + <&LITTLE_CPU_SLEEP_1>; + }; + + CPU_PD4: cpu-pd4 { + #power-domain-cells = <0>; + power-domains = <&CLUSTER_PD>; + domain-idle-states = <&BIG_CPU_SLEEP_0>, + <&BIG_CPU_SLEEP_1>; + }; + + CPU_PD5: cpu-pd5 { + #power-domain-cells = <0>; + power-domains = <&CLUSTER_PD>; + domain-idle-states = <&BIG_CPU_SLEEP_0>, + <&BIG_CPU_SLEEP_1>; + }; + + CPU_PD6: cpu-pd6 { + #power-domain-cells = <0>; + power-domains = <&CLUSTER_PD>; + domain-idle-states = <&BIG_CPU_SLEEP_0>, + <&BIG_CPU_SLEEP_1>; + }; + + CPU_PD7: cpu-pd7 { + #power-domain-cells = <0>; + power-domains = <&CLUSTER_PD>; + domain-idle-states = <&BIG_CPU_SLEEP_0>, + <&BIG_CPU_SLEEP_1>; + }; + + CLUSTER_PD: cluster-pd { + #power-domain-cells = <0>; + domain-idle-states = <&CLUSTER_SLEEP_0>; + }; }; soc: soc {
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation.
PATCH v2 6/6 arm64: dts: Add rsc power domain for sdm845
In reply to Maulik ShahAdd rsc power domain to enable sending sleep and wake votes using generic power domain infrastructure.
--- arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 ++ 1 file changed, 2 insertions(+)
arch/arm64/boot/dts/qcom/sdm845.dtsi +2 -0
@@ -669,6 +669,7 @@ CLUSTER_PD: cluster-pd { #power-domain-cells = <0>; + power-domains = <&apps_rsc>; domain-idle-states = <&CLUSTER_SLEEP_0>; }; };
@@ -2587,6 +2588,7 @@ <SLEEP_TCS 3>, <WAKE_TCS 3>, <CONTROL_TCS 1>; + #power-domain-cells = <0>; rpmhcc: clock-controller { compatible = "qcom,sdm845-rpmh-clk";
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation.
Re: [PATCH v2 6/6] arm64: dts: Add rsc power domain for sdm845
Quoting Maulik Shah (2019-08-23 01:17:03)
Quoted 6 lines
Add rsc power domain to enable sending sleep and wake votes using generic power domain infrastructure.
Signed-off-by Maulik Shah <[email protected]>---
Can this be combined with the previous patch?
Hi Maulik,
What is the status of this series? It seems it hasn't been updated since you sent it in August last year. Do you plan to send a v3 in the near future to address the outstanding comments?
Thanks
Matthias
Quoted 37 lines · Maulik Shah
Changes in v2:
- Add Stephen's Reviewed-By to the first three patches
- Addressed Stephen's comments on fourth patch
- Include changes to connect rpmh domain to cpuidle and genpdsResource State Coordinator (RSC) is responsible for powering off/lowering the requirements from CPU subsystem for the associated hardware like buses, clocks, and regulators when all CPUs and cluster is powered down.
RSC power domain uses last-man activities provided by genpd framework based on Ulf Hansoon's patch series[1], when the cluster of CPUs enter deepest idle states. As a part of domain poweroff, RSC can lower resource state requirements by flushing the cached sleep and wake state votes for resources.
Dependencies:
[1] https://lkml.org/lkml/2019/5/13/839
Maulik Shah (6):
drivers: qcom: rpmh: fix macro to accept NULL argument
drivers: qcom: rpmh: removerpmh_flushexport
dt-bindings: soc: qcom: Add RSC power domain specifier
drivers: qcom: rpmh-rsc: Add RSC power domain support
arm64: dts: Convert to the hierarchical CPU topology layout for sdm845
arm64: dts: Add rsc power domain for sdm845.../devicetree/bindings/soc/qcom/rpmh-rsc.txt | 8 ++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 105 +++++++++++++----- drivers/soc/qcom/rpmh-internal.h | 3 + drivers/soc/qcom/rpmh-rsc.c | 84 ++++++++++++++ drivers/soc/qcom/rpmh.c | 22 ++-- include/soc/qcom/rpmh.h | 5 - 6 files changed, 185 insertions(+), 42 deletions(-)Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation.
Hi Matthias,
Yes i will soon post a v3 series addressing outstanding comments.
Thanks,
Maulik
Quoted 5 lines · Matthias Kaehlcke
Hi Maulik,
What is the status of this series? It seems it hasn't been updated since you sent it in August last year. Do you plan to send a v3 in the near future to address the outstanding comments?
Signature · 1 line
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation