From: Hiago De Franco <hiago.franco@toradex.com>

Move imx_sc_get_pd_power() from pmdomain/imx/scu-pd.c to
firmware/imx/misc.c and rename it to imx_sc_pm_get_resource_power_mode()
to maintain the same naming logic with other functions in misc.c.

This makes the API available for other use cases. For example,
remoteproc/imx_rproc.c can now use this function to check the power mode
of the remote core.

Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
---
v2: moved this patch to be the first one
---
 drivers/firmware/imx/misc.c           | 47 +++++++++++++++++++++++++++
 drivers/pmdomain/imx/scu-pd.c         | 29 ++++-------------
 include/linux/firmware/imx/svc/misc.h |  8 +++++
 3 files changed, 62 insertions(+), 22 deletions(-)

diff --git a/drivers/firmware/imx/misc.c b/drivers/firmware/imx/misc.c
index d073cb3ce699..61fcb0066ec9 100644
--- a/drivers/firmware/imx/misc.c
+++ b/drivers/firmware/imx/misc.c
@@ -37,6 +37,18 @@ struct imx_sc_msg_resp_misc_get_ctrl {
 	u32 val;
 } __packed __aligned(4);
 
+struct imx_sc_msg_req_get_resource_power_mode {
+	struct imx_sc_rpc_msg hdr;
+	union {
+		struct {
+			u16 resource;
+		} req;
+		struct {
+			u8 mode;
+		} resp;
+	} data;
+} __packed __aligned(4);
+
 /*
  * This function sets a miscellaneous control value.
  *
@@ -135,3 +147,38 @@ int imx_sc_pm_cpu_start(struct imx_sc_ipc *ipc, u32 resource,
 	return imx_scu_call_rpc(ipc, &msg, true);
 }
 EXPORT_SYMBOL(imx_sc_pm_cpu_start);
+
+/*
+ * This function gets the power mode from a given @resource
+ *
+ * @param[in]     ipc         IPC handle
+ * @param[in]     resource    resource to check the power mode
+ *
+ * @return Returns < 0 for errors or the following for success:
+ * IMX_SC_PM_PW_MODE_OFF  0	Power off
+ * IMX_SC_PM_PW_MODE_STBY 1	Power in standby
+ * IMX_SC_PM_PW_MODE_LP   2	Power in low-power
+ * IMX_SC_PM_PW_MODE_ON   3	Power on
+ *
+ * These are defined under firmware/imx/svc/pm.h
+ */
+int imx_sc_pm_get_resource_power_mode(struct imx_sc_ipc *ipc, u32 resource)
+{
+	struct imx_sc_msg_req_get_resource_power_mode msg;
+	struct imx_sc_rpc_msg *hdr = &msg.hdr;
+	int ret;
+
+	hdr->ver = IMX_SC_RPC_VERSION;
+	hdr->svc = IMX_SC_RPC_SVC_PM;
+	hdr->func = IMX_SC_PM_FUNC_GET_RESOURCE_POWER_MODE;
+	hdr->size = 2;
+
+	msg.data.req.resource = resource;
+
+	ret = imx_scu_call_rpc(ipc, &msg, true);
+	if (ret)
+		return ret;
+
+	return msg.data.resp.mode;
+}
+EXPORT_SYMBOL(imx_sc_pm_get_resource_power_mode);
diff --git a/drivers/pmdomain/imx/scu-pd.c b/drivers/pmdomain/imx/scu-pd.c
index 01d465d88f60..945f972e636f 100644
--- a/drivers/pmdomain/imx/scu-pd.c
+++ b/drivers/pmdomain/imx/scu-pd.c
@@ -54,6 +54,7 @@
 #include <dt-bindings/firmware/imx/rsrc.h>
 #include <linux/console.h>
 #include <linux/firmware/imx/sci.h>
+#include <linux/firmware/imx/svc/misc.h>
 #include <linux/firmware/imx/svc/rm.h>
 #include <linux/io.h>
 #include <linux/module.h>
@@ -328,27 +329,6 @@ static void imx_sc_pd_get_console_rsrc(void)
 	imx_con_rsrc = specs.args[0];
 }
 
-static int imx_sc_get_pd_power(struct device *dev, u32 rsrc)
-{
-	struct imx_sc_msg_req_get_resource_power_mode msg;
-	struct imx_sc_rpc_msg *hdr = &msg.hdr;
-	int ret;
-
-	hdr->ver = IMX_SC_RPC_VERSION;
-	hdr->svc = IMX_SC_RPC_SVC_PM;
-	hdr->func = IMX_SC_PM_FUNC_GET_RESOURCE_POWER_MODE;
-	hdr->size = 2;
-
-	msg.data.req.resource = rsrc;
-
-	ret = imx_scu_call_rpc(pm_ipc_handle, &msg, true);
-	if (ret)
-		dev_err(dev, "failed to get power resource %d mode, ret %d\n",
-			rsrc, ret);
-
-	return msg.data.resp.mode;
-}
-
 static int imx_sc_pd_power(struct generic_pm_domain *domain, bool power_on)
 {
 	struct imx_sc_msg_req_set_resource_power_mode msg;
@@ -438,7 +418,12 @@ imx_scu_add_pm_domain(struct device *dev, int idx,
 	if (imx_con_rsrc == sc_pd->rsrc)
 		sc_pd->pd.flags = GENPD_FLAG_RPM_ALWAYS_ON;
 
-	mode = imx_sc_get_pd_power(dev, pd_ranges->rsrc + idx);
+	mode = imx_sc_pm_get_resource_power_mode(pm_ipc_handle,
+						 pd_ranges->rsrc + idx);
+	if (mode < 0)
+		dev_err(dev, "failed to get power resource %d mode, ret %d\n",
+			pd_ranges->rsrc + idx, mode);
+
 	if (mode == IMX_SC_PM_PW_MODE_ON)
 		is_off = false;
 	else
diff --git a/include/linux/firmware/imx/svc/misc.h b/include/linux/firmware/imx/svc/misc.h
index 760db08a67fc..376c800a88d0 100644
--- a/include/linux/firmware/imx/svc/misc.h
+++ b/include/linux/firmware/imx/svc/misc.h
@@ -55,6 +55,8 @@ int imx_sc_misc_get_control(struct imx_sc_ipc *ipc, u32 resource,
 
 int imx_sc_pm_cpu_start(struct imx_sc_ipc *ipc, u32 resource,
 			bool enable, u64 phys_addr);
+
+int imx_sc_pm_get_resource_power_mode(struct imx_sc_ipc *ipc, u32 resource);
 #else
 static inline int imx_sc_misc_set_control(struct imx_sc_ipc *ipc,
 					  u32 resource, u8 ctrl, u32 val)
@@ -73,5 +75,11 @@ static inline int imx_sc_pm_cpu_start(struct imx_sc_ipc *ipc, u32 resource,
 {
 	return -ENOTSUPP;
 }
+
+static inline int imx_sc_pm_get_resource_power_mode(struct imx_sc_ipc *ipc,
+						    u32 resource)
+{
+	return -ENOTSUPP;
+}
 #endif
 #endif /* _SC_MISC_API_H */
-- 
2.39.5

Hi Hiago,

On Wed, May 07, 2025 at 01:00:54PM -0300, Hiago De Franco wrote:
>From: Hiago De Franco <hiago.franco@toradex.com>
>
>Move imx_sc_get_pd_power() from pmdomain/imx/scu-pd.c to
>firmware/imx/misc.c and rename it to imx_sc_pm_get_resource_power_mode()
>to maintain the same naming logic with other functions in misc.c.
>
>This makes the API available for other use cases. For example,
>remoteproc/imx_rproc.c can now use this function to check the power mode
>of the remote core.
>
>Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
>---
>v2: moved this patch to be the first one
>---
> drivers/firmware/imx/misc.c           | 47 +++++++++++++++++++++++++++
> drivers/pmdomain/imx/scu-pd.c         | 29 ++++-------------
> include/linux/firmware/imx/svc/misc.h |  8 +++++
> 3 files changed, 62 insertions(+), 22 deletions(-)
>
>diff --git a/drivers/firmware/imx/misc.c b/drivers/firmware/imx/misc.c
>index d073cb3ce699..61fcb0066ec9 100644
>--- a/drivers/firmware/imx/misc.c
>+++ b/drivers/firmware/imx/misc.c
>@@ -37,6 +37,18 @@ struct imx_sc_msg_resp_misc_get_ctrl {
> 	u32 val;
> } __packed __aligned(4);
> 
>+struct imx_sc_msg_req_get_resource_power_mode {
>+	struct imx_sc_rpc_msg hdr;
>+	union {
>+		struct {
>+			u16 resource;
>+		} req;
>+		struct {
>+			u8 mode;
>+		} resp;
>+	} data;
>+} __packed __aligned(4);
>+
> /*
>  * This function sets a miscellaneous control value.
>  *
>@@ -135,3 +147,38 @@ int imx_sc_pm_cpu_start(struct imx_sc_ipc *ipc, u32 resource,
> 	return imx_scu_call_rpc(ipc, &msg, true);
> }
> EXPORT_SYMBOL(imx_sc_pm_cpu_start);
>+
>+/*
>+ * This function gets the power mode from a given @resource
>+ *
>+ * @param[in]     ipc         IPC handle
>+ * @param[in]     resource    resource to check the power mode
>+ *
>+ * @return Returns < 0 for errors or the following for success:
>+ * IMX_SC_PM_PW_MODE_OFF  0	Power off
>+ * IMX_SC_PM_PW_MODE_STBY 1	Power in standby
>+ * IMX_SC_PM_PW_MODE_LP   2	Power in low-power
>+ * IMX_SC_PM_PW_MODE_ON   3	Power on
>+ *
>+ * These are defined under firmware/imx/svc/pm.h
>+ */
>+int imx_sc_pm_get_resource_power_mode(struct imx_sc_ipc *ipc, u32 resource)
>+{
>+	struct imx_sc_msg_req_get_resource_power_mode msg;
>+	struct imx_sc_rpc_msg *hdr = &msg.hdr;
>+	int ret;
>+
>+	hdr->ver = IMX_SC_RPC_VERSION;
>+	hdr->svc = IMX_SC_RPC_SVC_PM;
>+	hdr->func = IMX_SC_PM_FUNC_GET_RESOURCE_POWER_MODE;
>+	hdr->size = 2;
>+
>+	msg.data.req.resource = resource;
>+
>+	ret = imx_scu_call_rpc(ipc, &msg, true);
>+	if (ret)
>+		return ret;
>+
>+	return msg.data.resp.mode;
>+}
>+EXPORT_SYMBOL(imx_sc_pm_get_resource_power_mode);
>diff --git a/drivers/pmdomain/imx/scu-pd.c b/drivers/pmdomain/imx/scu-pd.c
>index 01d465d88f60..945f972e636f 100644
>--- a/drivers/pmdomain/imx/scu-pd.c
>+++ b/drivers/pmdomain/imx/scu-pd.c
>@@ -54,6 +54,7 @@
> #include <dt-bindings/firmware/imx/rsrc.h>
> #include <linux/console.h>
> #include <linux/firmware/imx/sci.h>
>+#include <linux/firmware/imx/svc/misc.h>
> #include <linux/firmware/imx/svc/rm.h>
> #include <linux/io.h>
> #include <linux/module.h>
>@@ -328,27 +329,6 @@ static void imx_sc_pd_get_console_rsrc(void)
> 	imx_con_rsrc = specs.args[0];
> }
> 
>-static int imx_sc_get_pd_power(struct device *dev, u32 rsrc)
>-{
>-	struct imx_sc_msg_req_get_resource_power_mode msg;
>-	struct imx_sc_rpc_msg *hdr = &msg.hdr;
>-	int ret;
>-
>-	hdr->ver = IMX_SC_RPC_VERSION;
>-	hdr->svc = IMX_SC_RPC_SVC_PM;
>-	hdr->func = IMX_SC_PM_FUNC_GET_RESOURCE_POWER_MODE;
>-	hdr->size = 2;
>-
>-	msg.data.req.resource = rsrc;
>-
>-	ret = imx_scu_call_rpc(pm_ipc_handle, &msg, true);
>-	if (ret)
>-		dev_err(dev, "failed to get power resource %d mode, ret %d\n",
>-			rsrc, ret);
>-
>-	return msg.data.resp.mode;
>-}
>-
> static int imx_sc_pd_power(struct generic_pm_domain *domain, bool power_on)
> {
> 	struct imx_sc_msg_req_set_resource_power_mode msg;
>@@ -438,7 +418,12 @@ imx_scu_add_pm_domain(struct device *dev, int idx,
> 	if (imx_con_rsrc == sc_pd->rsrc)
> 		sc_pd->pd.flags = GENPD_FLAG_RPM_ALWAYS_ON;
> 
>-	mode = imx_sc_get_pd_power(dev, pd_ranges->rsrc + idx);
>+	mode = imx_sc_pm_get_resource_power_mode(pm_ipc_handle,

I mean, not change scu-pd.c in this patchset. After
imx_sc_pm_get_resource_power_mode landed, you could update scu-pd.c.

Otherwise you are touch three trees which are a bit hard to manage for
maintainers:
scu-pd.c under linux-pm tree
misc.c under linux-imx tree, I think using power.c should be better.
imx_rproc.c under linux-remoteproc tree

My suggestion was that
patch 1: firmware: imx: Introduce imx_sc_pm_get_resource_power_mode
patch 2: remoteproc: imx_rproc: Detecting power mode of M4 for i.MX8QX
Patch 3: pmdomain: scu-pd: migrate to use imx common pm API

Patch 3 could be separate out in future patchset to avoid handling three trees.

Regards,
Peng
>+						 pd_ranges->rsrc + idx);
>+	if (mode < 0)
>+		dev_err(dev, "failed to get power resource %d mode, ret %d\n",
>+			pd_ranges->rsrc + idx, mode);
>+
> 	if (mode == IMX_SC_PM_PW_MODE_ON)
> 		is_off = false;
> 	else
>diff --git a/include/linux/firmware/imx/svc/misc.h b/include/linux/firmware/imx/svc/misc.h
>index 760db08a67fc..376c800a88d0 100644
>--- a/include/linux/firmware/imx/svc/misc.h
>+++ b/include/linux/firmware/imx/svc/misc.h
>@@ -55,6 +55,8 @@ int imx_sc_misc_get_control(struct imx_sc_ipc *ipc, u32 resource,
> 
> int imx_sc_pm_cpu_start(struct imx_sc_ipc *ipc, u32 resource,
> 			bool enable, u64 phys_addr);
>+
>+int imx_sc_pm_get_resource_power_mode(struct imx_sc_ipc *ipc, u32 resource);
> #else
> static inline int imx_sc_misc_set_control(struct imx_sc_ipc *ipc,
> 					  u32 resource, u8 ctrl, u32 val)
>@@ -73,5 +75,11 @@ static inline int imx_sc_pm_cpu_start(struct imx_sc_ipc *ipc, u32 resource,
> {
> 	return -ENOTSUPP;
> }
>+
>+static inline int imx_sc_pm_get_resource_power_mode(struct imx_sc_ipc *ipc,
>+						    u32 resource)
>+{
>+	return -ENOTSUPP;
>+}
> #endif
> #endif /* _SC_MISC_API_H */
>-- 
>2.39.5
>

Hi Peng,

On Tue, May 13, 2025 at 03:43:37PM +0800, Peng Fan wrote:
> Hi Hiago,
> 
> On Wed, May 07, 2025 at 01:00:54PM -0300, Hiago De Franco wrote:
> >From: Hiago De Franco <hiago.franco@toradex.com>
> >
> >Move imx_sc_get_pd_power() from pmdomain/imx/scu-pd.c to
> >firmware/imx/misc.c and rename it to imx_sc_pm_get_resource_power_mode()
> >to maintain the same naming logic with other functions in misc.c.
> >
> >This makes the API available for other use cases. For example,
> >remoteproc/imx_rproc.c can now use this function to check the power mode
> >of the remote core.
> >
> >Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
> >---
> >v2: moved this patch to be the first one
> >---
> > drivers/firmware/imx/misc.c           | 47 +++++++++++++++++++++++++++
> > drivers/pmdomain/imx/scu-pd.c         | 29 ++++-------------
> > include/linux/firmware/imx/svc/misc.h |  8 +++++
> > 3 files changed, 62 insertions(+), 22 deletions(-)
> >
> >diff --git a/drivers/firmware/imx/misc.c b/drivers/firmware/imx/misc.c
> >index d073cb3ce699..61fcb0066ec9 100644
> >--- a/drivers/firmware/imx/misc.c
> >+++ b/drivers/firmware/imx/misc.c
> >@@ -37,6 +37,18 @@ struct imx_sc_msg_resp_misc_get_ctrl {
> > 	u32 val;
> > } __packed __aligned(4);
> > 
> >+struct imx_sc_msg_req_get_resource_power_mode {
> >+	struct imx_sc_rpc_msg hdr;
> >+	union {
> >+		struct {
> >+			u16 resource;
> >+		} req;
> >+		struct {
> >+			u8 mode;
> >+		} resp;
> >+	} data;
> >+} __packed __aligned(4);
> >+
> > /*
> >  * This function sets a miscellaneous control value.
> >  *
> >@@ -135,3 +147,38 @@ int imx_sc_pm_cpu_start(struct imx_sc_ipc *ipc, u32 resource,
> > 	return imx_scu_call_rpc(ipc, &msg, true);
> > }
> > EXPORT_SYMBOL(imx_sc_pm_cpu_start);
> >+
> >+/*
> >+ * This function gets the power mode from a given @resource
> >+ *
> >+ * @param[in]     ipc         IPC handle
> >+ * @param[in]     resource    resource to check the power mode
> >+ *
> >+ * @return Returns < 0 for errors or the following for success:
> >+ * IMX_SC_PM_PW_MODE_OFF  0	Power off
> >+ * IMX_SC_PM_PW_MODE_STBY 1	Power in standby
> >+ * IMX_SC_PM_PW_MODE_LP   2	Power in low-power
> >+ * IMX_SC_PM_PW_MODE_ON   3	Power on
> >+ *
> >+ * These are defined under firmware/imx/svc/pm.h
> >+ */
> >+int imx_sc_pm_get_resource_power_mode(struct imx_sc_ipc *ipc, u32 resource)
> >+{
> >+	struct imx_sc_msg_req_get_resource_power_mode msg;
> >+	struct imx_sc_rpc_msg *hdr = &msg.hdr;
> >+	int ret;
> >+
> >+	hdr->ver = IMX_SC_RPC_VERSION;
> >+	hdr->svc = IMX_SC_RPC_SVC_PM;
> >+	hdr->func = IMX_SC_PM_FUNC_GET_RESOURCE_POWER_MODE;
> >+	hdr->size = 2;
> >+
> >+	msg.data.req.resource = resource;
> >+
> >+	ret = imx_scu_call_rpc(ipc, &msg, true);
> >+	if (ret)
> >+		return ret;
> >+
> >+	return msg.data.resp.mode;
> >+}
> >+EXPORT_SYMBOL(imx_sc_pm_get_resource_power_mode);
> >diff --git a/drivers/pmdomain/imx/scu-pd.c b/drivers/pmdomain/imx/scu-pd.c
> >index 01d465d88f60..945f972e636f 100644
> >--- a/drivers/pmdomain/imx/scu-pd.c
> >+++ b/drivers/pmdomain/imx/scu-pd.c
> >@@ -54,6 +54,7 @@
> > #include <dt-bindings/firmware/imx/rsrc.h>
> > #include <linux/console.h>
> > #include <linux/firmware/imx/sci.h>
> >+#include <linux/firmware/imx/svc/misc.h>
> > #include <linux/firmware/imx/svc/rm.h>
> > #include <linux/io.h>
> > #include <linux/module.h>
> >@@ -328,27 +329,6 @@ static void imx_sc_pd_get_console_rsrc(void)
> > 	imx_con_rsrc = specs.args[0];
> > }
> > 
> >-static int imx_sc_get_pd_power(struct device *dev, u32 rsrc)
> >-{
> >-	struct imx_sc_msg_req_get_resource_power_mode msg;
> >-	struct imx_sc_rpc_msg *hdr = &msg.hdr;
> >-	int ret;
> >-
> >-	hdr->ver = IMX_SC_RPC_VERSION;
> >-	hdr->svc = IMX_SC_RPC_SVC_PM;
> >-	hdr->func = IMX_SC_PM_FUNC_GET_RESOURCE_POWER_MODE;
> >-	hdr->size = 2;
> >-
> >-	msg.data.req.resource = rsrc;
> >-
> >-	ret = imx_scu_call_rpc(pm_ipc_handle, &msg, true);
> >-	if (ret)
> >-		dev_err(dev, "failed to get power resource %d mode, ret %d\n",
> >-			rsrc, ret);
> >-
> >-	return msg.data.resp.mode;
> >-}
> >-
> > static int imx_sc_pd_power(struct generic_pm_domain *domain, bool power_on)
> > {
> > 	struct imx_sc_msg_req_set_resource_power_mode msg;
> >@@ -438,7 +418,12 @@ imx_scu_add_pm_domain(struct device *dev, int idx,
> > 	if (imx_con_rsrc == sc_pd->rsrc)
> > 		sc_pd->pd.flags = GENPD_FLAG_RPM_ALWAYS_ON;
> > 
> >-	mode = imx_sc_get_pd_power(dev, pd_ranges->rsrc + idx);
> >+	mode = imx_sc_pm_get_resource_power_mode(pm_ipc_handle,
> 
> I mean, not change scu-pd.c in this patchset. After
> imx_sc_pm_get_resource_power_mode landed, you could update scu-pd.c.
> 
> Otherwise you are touch three trees which are a bit hard to manage for
> maintainers:
> scu-pd.c under linux-pm tree
> misc.c under linux-imx tree, I think using power.c should be better.
> imx_rproc.c under linux-remoteproc tree
> 
> My suggestion was that
> patch 1: firmware: imx: Introduce imx_sc_pm_get_resource_power_mode
> patch 2: remoteproc: imx_rproc: Detecting power mode of M4 for i.MX8QX
> Patch 3: pmdomain: scu-pd: migrate to use imx common pm API
> 
> Patch 3 could be separate out in future patchset to avoid handling three trees.

Ok, got it. I will prepare the v3 with your suggestions. Thanks.

> 
> Regards,
> Peng
> >+						 pd_ranges->rsrc + idx);
> >+	if (mode < 0)
> >+		dev_err(dev, "failed to get power resource %d mode, ret %d\n",
> >+			pd_ranges->rsrc + idx, mode);
> >+
> > 	if (mode == IMX_SC_PM_PW_MODE_ON)
> > 		is_off = false;
> > 	else
> >diff --git a/include/linux/firmware/imx/svc/misc.h b/include/linux/firmware/imx/svc/misc.h
> >index 760db08a67fc..376c800a88d0 100644
> >--- a/include/linux/firmware/imx/svc/misc.h
> >+++ b/include/linux/firmware/imx/svc/misc.h
> >@@ -55,6 +55,8 @@ int imx_sc_misc_get_control(struct imx_sc_ipc *ipc, u32 resource,
> > 
> > int imx_sc_pm_cpu_start(struct imx_sc_ipc *ipc, u32 resource,
> > 			bool enable, u64 phys_addr);
> >+
> >+int imx_sc_pm_get_resource_power_mode(struct imx_sc_ipc *ipc, u32 resource);
> > #else
> > static inline int imx_sc_misc_set_control(struct imx_sc_ipc *ipc,
> > 					  u32 resource, u8 ctrl, u32 val)
> >@@ -73,5 +75,11 @@ static inline int imx_sc_pm_cpu_start(struct imx_sc_ipc *ipc, u32 resource,
> > {
> > 	return -ENOTSUPP;
> > }
> >+
> >+static inline int imx_sc_pm_get_resource_power_mode(struct imx_sc_ipc *ipc,
> >+						    u32 resource)
> >+{
> >+	return -ENOTSUPP;
> >+}
> > #endif
> > #endif /* _SC_MISC_API_H */
> >-- 
> >2.39.5
> >

Best Regards,
Hiago.