linux-kernel - Re: [PATCH v4 1/3] pmdomain: core: introduce dev_pm_genpd_is

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <iuotfsnaft3623lchzop6sbu5ox56scdr57uia56qm6ummcvzt@yisczcdzbc3b>
Date: Wed, 11 Jun 2025 10:32:28 -0500
From: Bjorn Andersson <andersson@...nel.org>
To: Hiago De Franco <hiagofranco@...il.com>
Cc: Mathieu Poirier <mathieu.poirier@...aro.org>, 
	Ulf Hansson <ulf.hansson@...aro.org>, linux-pm@...r.kernel.org, linux-remoteproc@...r.kernel.org, 
	Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, 
	Hiago De Franco <hiago.franco@...adex.com>, imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, Peng Fan <peng.fan@....nxp.com>, daniel.baluta@....com, 
	iuliana.prodan@....nxp.com, "Rafael J . Wysocki" <rafael@...nel.org>
Subject: Re: [PATCH v4 1/3] pmdomain: core: introduce dev_pm_genpd_is_on

On Mon, Jun 02, 2025 at 10:19:03AM -0300, Hiago De Franco wrote:
> From: Hiago De Franco <hiago.franco@...adex.com>
> 
> This helper function returns the current power status of a given generic
> power domain.
> 

Please correct me if I'm wrong, but this returns the momentary status of
the device's associated genpd, and as genpds can be shared among devices
wouldn't there be a risk that you think the genpd is on but then that
other device powers it off?

> As example, remoteproc/imx_rproc.c can now use this function to check
> the power status of the remote core to properly set "attached" or
> "offline" modes.

I presume this example works because there is a dedicated, single usage,
genpd for the remoteproc instance?

> 
> Suggested-by: Ulf Hansson <ulf.hansson@...aro.org>
> Signed-off-by: Hiago De Franco <hiago.franco@...adex.com>
> ---
> v4: New patch.
> ---
>  drivers/pmdomain/core.c   | 27 +++++++++++++++++++++++++++
>  include/linux/pm_domain.h |  6 ++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> index ff5c7f2b69ce..bcb74d10960c 100644
> --- a/drivers/pmdomain/core.c
> +++ b/drivers/pmdomain/core.c
> @@ -758,6 +758,33 @@ int dev_pm_genpd_rpm_always_on(struct device *dev, bool on)
>  }
>  EXPORT_SYMBOL_GPL(dev_pm_genpd_rpm_always_on);
>  
> +/**
> + * dev_pm_genpd_is_on - Get device's power status

Functions in kernel-doc should have () prefix

> + *
> + * @dev: Device to get the current power status
> + *
> + * This function checks whether the generic power domain is on or not by
> + * verifying if genpd_status_on equals GENPD_STATE_ON.
> + *

If my understanding is correct, I'd like a warning here saying that this
is dangerous if the underlying genpd is shared.

Regards,
Bjorn

> + * Return: 'true' if the device's power domain is on, 'false' otherwise.
> + */
> +bool dev_pm_genpd_is_on(struct device *dev)
> +{
> +	struct generic_pm_domain *genpd;
> +	bool is_on;
> +
> +	genpd = dev_to_genpd_safe(dev);
> +	if (!genpd)
> +		return false;
> +
> +	genpd_lock(genpd);
> +	is_on = genpd_status_on(genpd);
> +	genpd_unlock(genpd);
> +
> +	return is_on;
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_genpd_is_on);
> +
>  /**
>   * pm_genpd_inc_rejected() - Adjust the rejected/usage counts for an idle-state.
>   *
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 0b18160901a2..c12580b6579b 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -301,6 +301,7 @@ void dev_pm_genpd_synced_poweroff(struct device *dev);
>  int dev_pm_genpd_set_hwmode(struct device *dev, bool enable);
>  bool dev_pm_genpd_get_hwmode(struct device *dev);
>  int dev_pm_genpd_rpm_always_on(struct device *dev, bool on);
> +bool dev_pm_genpd_is_on(struct device *dev);
>  
>  extern struct dev_power_governor simple_qos_governor;
>  extern struct dev_power_governor pm_domain_always_on_gov;
> @@ -393,6 +394,11 @@ static inline int dev_pm_genpd_rpm_always_on(struct device *dev, bool on)
>  	return -EOPNOTSUPP;
>  }
>  
> +static inline bool dev_pm_genpd_is_on(struct device *dev)
> +{
> +	return false;
> +}
> +
>  #define simple_qos_governor		(*(struct dev_power_governor *)(NULL))
>  #define pm_domain_always_on_gov		(*(struct dev_power_governor *)(NULL))
>  #endif
> -- 
> 2.39.5
>