| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <7hsejzp4xg.fsf@baylibre.com>
Date: Mon, 16 Jun 2025 17:50:19 -0700
From: Kevin Hilman <khilman@...libre.com>
To: Ulf Hansson <ulf.hansson@...aro.org>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
devicetree@...r.kernel.org, linux-pm@...r.kernel.org,
arm-scmi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC v3 2/2] pmdomain: core: add support for subdomains
using power-domain-map
Ulf Hansson <ulf.hansson@...aro.org> writes:
--text follows this line--
> On Sat, 14 Jun 2025 at 00:39, Kevin Hilman <khilman@...libre.com> wrote:
>>
>> Currently, PM domains can only support hierarchy for simple
>> providers (e.g. ones with #power-domain-cells = 0).
>>
>> Add more generic support for hierarchy by using nexus node
>> maps (c.f. section 2.5.1 of the DT spec.)
>>
>> For example, we could describe SCMI PM domains with multiple parents
>> domains (MAIN_PD and WKUP_PD) like this:
>>
>> scmi_pds: protocol@11 {
>> reg = <0x11>;
>> #power-domain-cells = <1>;
>>
>> power-domain-map = <15 &MAIN_PD>,
>> <19 &WKUP_PD>;
>> };
>>
>> which should mean that <&scmi_pds 15> is a subdomain of MAIN_PD and
>> <&scmi_pds 19> is a subdomain of WKUP_PD.
>>
>> IOW, given an SCMI device which uses SCMI PM domains:
>>
>> main_timer0: timer@...0000 {
>> power-domains = <&scmi_pds 15>;
>> };
>>
>> it already implies that main_timer0 is PM domain <&scmi_pds 15>
>>
>> With the new map, this *also* now implies <&scmi_pds 15> is a
>> subdomain of MAIN_PD.
>>
>> Signed-off-by: Kevin Hilman <khilman@...libre.com>
>> ---
>> drivers/pmdomain/core.c | 24 ++++++++++++++++++++++--
>> 1 file changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
>> index d6c1ddb807b2..adf022b45d95 100644
>> --- a/drivers/pmdomain/core.c
>> +++ b/drivers/pmdomain/core.c
>> @@ -2998,8 +2998,8 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
>> unsigned int index, unsigned int num_domains,
>> bool power_on)
>> {
>> - struct of_phandle_args pd_args;
>> - struct generic_pm_domain *pd;
>> + struct of_phandle_args pd_args, parent_args;
>> + struct generic_pm_domain *pd, *parent_pd = NULL;
>> int ret;
>>
>> ret = of_parse_phandle_with_args(dev->of_node, "power-domains",
>> @@ -3039,6 +3039,22 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
>> goto err;
>> }
>>
>> + /*
>> + * Check for power-domain-map, which implies the primary
>> + * power-doamin is a subdomain of the parent found in the map.
>> + */
>> + ret = of_parse_phandle_with_args_map(dev->of_node, "power-domains",
>> + "power-domain", index, &parent_args);
>> + if (!ret && (pd_args.np != parent_args.np)) {
>> + parent_pd = genpd_get_from_provider(&parent_args);
>> + of_node_put(parent_args.np);
>> +
>> + ret = pm_genpd_add_subdomain(parent_pd, pd);
>> + if (!ret)
>> + dev_dbg(dev, "adding PM domain %s as subdomain of %s\n",
>> + pd->name, parent_pd->name);
>> + }
>
> Please move the above new code to a separate shared genpd helper
> function, that genpd providers can call build the topology. This, to
> be consistent with the current way for how we usually add
> parent/child-domains in genpd (see of_genpd_add_subdomain).
Yeah, you had the same comment on v2, and I'm not ignoring you. But I
thought that moving this code to when devices are attatched to domains
(instead of when providers are created) would solve that problem. IOW,
in this approach, `power-domain-map` is handled at the same time as a
devices `power-domains = ` property.
So, while I don't really understand the reason that every PM domain
provider has to handle this individually, I've given that a try (see
below.)
> Moreover, we also need a corresponding "cleanup" helper function to
> remove the child-domain (subdomain) correctly, similar to
> of_genpd_remove_subdomain().
Yes, I'll handle that better once I get through this RFC phase to make
sure I'm on th right path.
OK, so below[1] is a shot at just adding helpers to the PM domain core. I
will then uses these from the SCMI PM domains ->attach_dev() and
->detatch_dev callbacks.
If you think this is better, I'll send a v4 tomorrow.
Kevin
[1] NOTE: this is based on v6.12 because that's where I have a functioning BSP
for this SoC. If you're OK with this, I'll rebase to v6.15 and submit upstream.
>From 12a3e5669dc18f4a9fdf9f25398cba4245135a43 Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@...libre.com>
Date: Fri, 13 Jun 2025 13:49:45 -0700
Subject: [PATCH 2/3] pmdomain: core: add support for subdomains via
power-domain-map
---
drivers/pmdomain/core.c | 60 +++++++++++++++++++++++++++++++++++++++
include/linux/pm_domain.h | 11 +++++++
2 files changed, 71 insertions(+)
diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index 88819659df83..a0dc60d4160d 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -3100,6 +3100,66 @@ struct device *genpd_dev_pm_attach_by_name(struct device *dev, const char *name)
return genpd_dev_pm_attach_by_id(dev, index);
}
+/**
+ * genpd_dev_pm_attach_subdomain - Associate a PM domain with its parent domain
+ * @domain: The PM domain to lookup whether it has any parent
+ * @dev: The device being attached to the PM domain.
+ *
+ * Check if @domain has a power-domain-map. If present, use that map
+ * to determine the parent PM domain, and attach @domain as a
+ * subdomain to the parent PM domain.
+ *
+ * Intended to called from a PM domain provider's ->attach_dev()
+ * callback, where &gpd_list_lock will already be held by the genpd
+ * add_device() path.
+ */
+struct generic_pm_domain *
+genpd_dev_pm_attach_subdomain(struct generic_pm_domain *domain,
+ struct device *dev)
+{
+ struct of_phandle_args parent_args;
+ struct generic_pm_domain *parent_pd = NULL;
+ int ret;
+
+ /*
+ * Check for power-domain-map, which implies the primary
+ * power-doamin is a subdomain of the parent found in the map.
+ */
+ ret = of_parse_phandle_with_args_map(dev->of_node, "power-domains",
+ "power-domain", 0, &parent_args);
+ if (!ret && parent_args.np) {
+ parent_pd = genpd_get_from_provider(&parent_args);
+ of_node_put(parent_args.np);
+
+ ret = genpd_add_subdomain(parent_pd, domain);
+ if (!ret) {
+ dev_dbg(dev, "adding PM domain %s as subdomain of %s\n",
+ domain->name, parent_pd->name);
+ return parent_pd;
+ }
+ }
+
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(genpd_dev_pm_attach_subdomain);
+
+/**
+ * genpd_dev_pm_detach_subdomain - Detatch a PM domain from its parent domain
+ * @domain: The PM subdomain to detach
+ * @parent: The parent PM domain
+ * @dev: The device being attached to the PM subdomain.
+ *
+ * Remove @domain from @parent.
+ * Intended to cleanup after genpd_dev_pm_attach_subdomain()
+ */
+int genpd_dev_pm_detach_subdomain(struct generic_pm_domain *domain,
+ struct generic_pm_domain *parent,
+ struct device *dev)
+{
+ return pm_genpd_remove_subdomain(parent, domain);
+}
+EXPORT_SYMBOL_GPL(genpd_dev_pm_detach_subdomain);
+
static const struct of_device_id idle_state_match[] = {
{ .compatible = "domain-idle-state", },
{ }
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index cf4b11be3709..5d7eb3ae59dd 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -410,6 +410,11 @@ struct device *genpd_dev_pm_attach_by_id(struct device *dev,
unsigned int index);
struct device *genpd_dev_pm_attach_by_name(struct device *dev,
const char *name);
+struct generic_pm_domain *genpd_dev_pm_attach_subdomain(struct generic_pm_domain *domain,
+ struct device *dev);
+int genpd_dev_pm_detach_subdomain(struct generic_pm_domain *domain,
+ struct generic_pm_domain *parent,
+ struct device *dev);
#else /* !CONFIG_PM_GENERIC_DOMAINS_OF */
static inline int of_genpd_add_provider_simple(struct device_node *np,
struct generic_pm_domain *genpd)
@@ -466,6 +471,12 @@ static inline struct device *genpd_dev_pm_attach_by_name(struct device *dev,
return NULL;
}
+static inline
+struct generic_pm_domain *genpd_dev_pm_attach_subdomain(struct generic_pm_domain *domain,
+ struct device *dev)
+{
+ return NULL;
+}
static inline
struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
{
--
2.49.0
Powered by blists - more mailing lists