* [PATCH 1/4] firmware: imx: scu-pd: fix cm40 power domain
2020-04-23 6:59 [PATCH 0/4] firmware: imx: fix/update cm4 and add resource check peng.fan
@ 2020-04-23 6:59 ` peng.fan
2020-04-23 10:02 ` Aisheng Dong
2020-04-23 6:59 ` [PATCH 2/4] firmware: imx: add resource management api peng.fan
` (2 subsequent siblings)
3 siblings, 1 reply; 22+ messages in thread
From: peng.fan @ 2020-04-23 6:59 UTC (permalink / raw)
To: shawnguo, s.hauer
Cc: kernel, festevam, linux, leonard.crestez, daniel.baluta,
aisheng.dong, qiangqing.zhang, linux-kernel, linux-arm-kernel,
linux-imx, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
The postfix needs to be false. Alought compiler use 0 for postfix now,
and take start_from as 0, it is better we add explicit false to postfix.
Fixes: 705dcca91d0a("firmware: imx: scu-pd: add power domain for I2C and INTMUX in CM40 SS")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/imx/scu-pd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/imx/scu-pd.c b/drivers/firmware/imx/scu-pd.c
index fb5523aa16ee..d1b313fa7b96 100644
--- a/drivers/firmware/imx/scu-pd.c
+++ b/drivers/firmware/imx/scu-pd.c
@@ -167,8 +167,8 @@ static const struct imx_sc_pd_range imx8qxp_scu_pd_ranges[] = {
{ "dc0-pll", IMX_SC_R_DC_0_PLL_0, 2, true, 0 },
/* CM40 SS */
- { "cm40_i2c", IMX_SC_R_M4_0_I2C, 1, 0 },
- { "cm40_intmux", IMX_SC_R_M4_0_INTMUX, 1, 0 },
+ { "cm40_i2c", IMX_SC_R_M4_0_I2C, 1, false, 0 },
+ { "cm40_intmux", IMX_SC_R_M4_0_INTMUX, 1, false, 0 },
};
static const struct imx_sc_pd_soc imx8qxp_scu_pd = {
--
2.16.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* RE: [PATCH 1/4] firmware: imx: scu-pd: fix cm40 power domain
2020-04-23 6:59 ` [PATCH 1/4] firmware: imx: scu-pd: fix cm40 power domain peng.fan
@ 2020-04-23 10:02 ` Aisheng Dong
0 siblings, 0 replies; 22+ messages in thread
From: Aisheng Dong @ 2020-04-23 10:02 UTC (permalink / raw)
To: Peng Fan, shawnguo, s.hauer
Cc: kernel, festevam, linux, Leonard Crestez, Daniel Baluta,
Joakim Zhang, linux-kernel, linux-arm-kernel, dl-linux-imx
> From: Peng Fan <peng.fan@nxp.com>
> Sent: Thursday, April 23, 2020 3:00 PM
>
> The postfix needs to be false. Alought compiler use 0 for postfix now, and take
> start_from as 0, it is better we add explicit false to postfix.
>
> Fixes: 705dcca91d0a("firmware: imx: scu-pd: add power domain for I2C and
> INTMUX in CM40 SS")
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
Regards
Aisheng
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/4] firmware: imx: add resource management api
2020-04-23 6:59 [PATCH 0/4] firmware: imx: fix/update cm4 and add resource check peng.fan
2020-04-23 6:59 ` [PATCH 1/4] firmware: imx: scu-pd: fix cm40 power domain peng.fan
@ 2020-04-23 6:59 ` peng.fan
2020-04-23 10:04 ` Aisheng Dong
2020-06-02 3:07 ` Aisheng Dong
2020-04-23 6:59 ` [PATCH 3/4] firmware: imx: scu-pd: ignore power domain not owned peng.fan
2020-04-23 6:59 ` [PATCH 4/4] firmware: imx: scu-pd: add more cm4 resources peng.fan
3 siblings, 2 replies; 22+ messages in thread
From: peng.fan @ 2020-04-23 6:59 UTC (permalink / raw)
To: shawnguo, s.hauer
Cc: kernel, festevam, linux, leonard.crestez, daniel.baluta,
aisheng.dong, qiangqing.zhang, linux-kernel, linux-arm-kernel,
linux-imx, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
Add resource management API, when we have multiple
partition running together, resources not owned to current
partition should not be used.
Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/imx/Makefile | 2 +-
drivers/firmware/imx/rm.c | 40 +++++++++++++++++++++
include/linux/firmware/imx/sci.h | 1 +
include/linux/firmware/imx/svc/rm.h | 69 +++++++++++++++++++++++++++++++++++++
4 files changed, 111 insertions(+), 1 deletion(-)
create mode 100644 drivers/firmware/imx/rm.c
create mode 100644 include/linux/firmware/imx/svc/rm.h
diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
index 08bc9ddfbdfb..17ea3613e142 100644
--- a/drivers/firmware/imx/Makefile
+++ b/drivers/firmware/imx/Makefile
@@ -1,4 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_IMX_DSP) += imx-dsp.o
-obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o
+obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o
obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
diff --git a/drivers/firmware/imx/rm.c b/drivers/firmware/imx/rm.c
new file mode 100644
index 000000000000..7b0334de5486
--- /dev/null
+++ b/drivers/firmware/imx/rm.c
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2020 NXP
+ *
+ * File containing client-side RPC functions for the RM service. These
+ * function are ported to clients that communicate to the SC.
+ */
+
+#include <linux/firmware/imx/svc/rm.h>
+
+struct imx_sc_msg_rm_rsrc_owned {
+ struct imx_sc_rpc_msg hdr;
+ u16 resource;
+} __packed __aligned(4);
+
+/*
+ * This function check @resource is owned by current partition or not
+ *
+ * @param[in] ipc IPC handle
+ * @param[in] resource resource the control is associated with
+ *
+ * @return Returns 0 for success and < 0 for errors.
+ */
+bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16 resource)
+{
+ struct imx_sc_msg_rm_rsrc_owned msg;
+ struct imx_sc_rpc_msg *hdr = &msg.hdr;
+
+ hdr->ver = IMX_SC_RPC_VERSION;
+ hdr->svc = IMX_SC_RPC_SVC_RM;
+ hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
+ hdr->size = 2;
+
+ msg.resource = resource;
+
+ imx_scu_call_rpc(ipc, &msg, true);
+
+ return hdr->func;
+}
+EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
diff --git a/include/linux/firmware/imx/sci.h b/include/linux/firmware/imx/sci.h
index 17ba4e405129..b5c5a56f29be 100644
--- a/include/linux/firmware/imx/sci.h
+++ b/include/linux/firmware/imx/sci.h
@@ -15,6 +15,7 @@
#include <linux/firmware/imx/svc/misc.h>
#include <linux/firmware/imx/svc/pm.h>
+#include <linux/firmware/imx/svc/rm.h>
int imx_scu_enable_general_irq_channel(struct device *dev);
int imx_scu_irq_register_notifier(struct notifier_block *nb);
diff --git a/include/linux/firmware/imx/svc/rm.h b/include/linux/firmware/imx/svc/rm.h
new file mode 100644
index 000000000000..fc6ea62d9d0e
--- /dev/null
+++ b/include/linux/firmware/imx/svc/rm.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017-2019 NXP
+ *
+ * Header file containing the public API for the System Controller (SC)
+ * Power Management (PM) function. This includes functions for power state
+ * control, clock control, reset control, and wake-up event control.
+ *
+ * RM_SVC (SVC) Resource Management Service
+ *
+ * Module for the Resource Management (RM) service.
+ */
+
+#ifndef _SC_RM_API_H
+#define _SC_RM_API_H
+
+#include <linux/firmware/imx/sci.h>
+
+/*
+ * This type is used to indicate RPC RM function calls.
+ */
+enum imx_sc_rm_func {
+ IMX_SC_RM_FUNC_UNKNOWN = 0,
+ IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
+ IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
+ IMX_SC_RM_FUNC_PARTITION_FREE = 2,
+ IMX_SC_RM_FUNC_GET_DID = 26,
+ IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
+ IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
+ IMX_SC_RM_FUNC_GET_PARTITION = 5,
+ IMX_SC_RM_FUNC_SET_PARENT = 6,
+ IMX_SC_RM_FUNC_MOVE_ALL = 7,
+ IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
+ IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
+ IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
+ IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
+ IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
+ IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS = 12,
+ IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
+ IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
+ IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
+ IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
+ IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
+ IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
+ IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
+ IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
+ IMX_SC_RM_FUNC_MEMREG_FREE = 18,
+ IMX_SC_RM_FUNC_FIND_MEMREG = 30,
+ IMX_SC_RM_FUNC_ASSIGN_MEMREG = 19,
+ IMX_SC_RM_FUNC_SET_MEMREG_PERMISSIONS = 20,
+ IMX_SC_RM_FUNC_IS_MEMREG_OWNED = 21,
+ IMX_SC_RM_FUNC_GET_MEMREG_INFO = 22,
+ IMX_SC_RM_FUNC_ASSIGN_PAD = 23,
+ IMX_SC_RM_FUNC_SET_PAD_MOVABLE = 24,
+ IMX_SC_RM_FUNC_IS_PAD_OWNED = 25,
+ IMX_SC_RM_FUNC_DUMP = 27,
+};
+
+#if IS_ENABLED(CONFIG_IMX_SCU)
+bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16 resource);
+#else
+static inline bool
+imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16 resource)
+{
+ return true;
+}
+#endif
+#endif
--
2.16.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* RE: [PATCH 2/4] firmware: imx: add resource management api
2020-04-23 6:59 ` [PATCH 2/4] firmware: imx: add resource management api peng.fan
@ 2020-04-23 10:04 ` Aisheng Dong
2020-04-23 10:57 ` Peng Fan
2020-06-02 3:07 ` Aisheng Dong
1 sibling, 1 reply; 22+ messages in thread
From: Aisheng Dong @ 2020-04-23 10:04 UTC (permalink / raw)
To: Peng Fan, shawnguo, s.hauer
Cc: kernel, festevam, linux, Leonard Crestez, Daniel Baluta,
Joakim Zhang, linux-kernel, linux-arm-kernel, dl-linux-imx
> From: Peng Fan <peng.fan@nxp.com>
> Sent: Thursday, April 23, 2020 3:00 PM
>
> Add resource management API, when we have multiple partition running
> together, resources not owned to current partition should not be used.
>
> Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
Right now I'm still not quite understand if we really need this.
As current resource is bound to power domains, if it's not owned by one specific
SW execution environment, power on will also fail.
Can you clarify if any exceptions?
Regards
Aisheng
> ---
> drivers/firmware/imx/Makefile | 2 +-
> drivers/firmware/imx/rm.c | 40 +++++++++++++++++++++
> include/linux/firmware/imx/sci.h | 1 +
> include/linux/firmware/imx/svc/rm.h | 69
> +++++++++++++++++++++++++++++++++++++
> 4 files changed, 111 insertions(+), 1 deletion(-) create mode 100644
> drivers/firmware/imx/rm.c create mode 100644
> include/linux/firmware/imx/svc/rm.h
>
> diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
> index 08bc9ddfbdfb..17ea3613e142 100644
> --- a/drivers/firmware/imx/Makefile
> +++ b/drivers/firmware/imx/Makefile
> @@ -1,4 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_IMX_DSP) += imx-dsp.o
> -obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o
> +obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o
> obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> diff --git a/drivers/firmware/imx/rm.c b/drivers/firmware/imx/rm.c new file
> mode 100644 index 000000000000..7b0334de5486
> --- /dev/null
> +++ b/drivers/firmware/imx/rm.c
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2020 NXP
> + *
> + * File containing client-side RPC functions for the RM service. These
> + * function are ported to clients that communicate to the SC.
> + */
> +
> +#include <linux/firmware/imx/svc/rm.h>
> +
> +struct imx_sc_msg_rm_rsrc_owned {
> + struct imx_sc_rpc_msg hdr;
> + u16 resource;
> +} __packed __aligned(4);
> +
> +/*
> + * This function check @resource is owned by current partition or not
> + *
> + * @param[in] ipc IPC handle
> + * @param[in] resource resource the control is associated with
> + *
> + * @return Returns 0 for success and < 0 for errors.
> + */
> +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16 resource)
> +{
> + struct imx_sc_msg_rm_rsrc_owned msg;
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_RM;
> + hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
> + hdr->size = 2;
> +
> + msg.resource = resource;
> +
> + imx_scu_call_rpc(ipc, &msg, true);
> +
> + return hdr->func;
> +}
> +EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
> diff --git a/include/linux/firmware/imx/sci.h b/include/linux/firmware/imx/sci.h
> index 17ba4e405129..b5c5a56f29be 100644
> --- a/include/linux/firmware/imx/sci.h
> +++ b/include/linux/firmware/imx/sci.h
> @@ -15,6 +15,7 @@
>
> #include <linux/firmware/imx/svc/misc.h> #include
> <linux/firmware/imx/svc/pm.h>
> +#include <linux/firmware/imx/svc/rm.h>
>
> int imx_scu_enable_general_irq_channel(struct device *dev); int
> imx_scu_irq_register_notifier(struct notifier_block *nb); diff --git
> a/include/linux/firmware/imx/svc/rm.h b/include/linux/firmware/imx/svc/rm.h
> new file mode 100644
> index 000000000000..fc6ea62d9d0e
> --- /dev/null
> +++ b/include/linux/firmware/imx/svc/rm.h
> @@ -0,0 +1,69 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + * Copyright 2017-2019 NXP
> + *
> + * Header file containing the public API for the System Controller (SC)
> + * Power Management (PM) function. This includes functions for power
> +state
> + * control, clock control, reset control, and wake-up event control.
> + *
> + * RM_SVC (SVC) Resource Management Service
> + *
> + * Module for the Resource Management (RM) service.
> + */
> +
> +#ifndef _SC_RM_API_H
> +#define _SC_RM_API_H
> +
> +#include <linux/firmware/imx/sci.h>
> +
> +/*
> + * This type is used to indicate RPC RM function calls.
> + */
> +enum imx_sc_rm_func {
> + IMX_SC_RM_FUNC_UNKNOWN = 0,
> + IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
> + IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
> + IMX_SC_RM_FUNC_PARTITION_FREE = 2,
> + IMX_SC_RM_FUNC_GET_DID = 26,
> + IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
> + IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
> + IMX_SC_RM_FUNC_GET_PARTITION = 5,
> + IMX_SC_RM_FUNC_SET_PARENT = 6,
> + IMX_SC_RM_FUNC_MOVE_ALL = 7,
> + IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
> + IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
> + IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
> + IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
> + IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
> + IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS = 12,
> + IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
> + IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
> + IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
> + IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
> + IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
> + IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
> + IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
> + IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
> + IMX_SC_RM_FUNC_MEMREG_FREE = 18,
> + IMX_SC_RM_FUNC_FIND_MEMREG = 30,
> + IMX_SC_RM_FUNC_ASSIGN_MEMREG = 19,
> + IMX_SC_RM_FUNC_SET_MEMREG_PERMISSIONS = 20,
> + IMX_SC_RM_FUNC_IS_MEMREG_OWNED = 21,
> + IMX_SC_RM_FUNC_GET_MEMREG_INFO = 22,
> + IMX_SC_RM_FUNC_ASSIGN_PAD = 23,
> + IMX_SC_RM_FUNC_SET_PAD_MOVABLE = 24,
> + IMX_SC_RM_FUNC_IS_PAD_OWNED = 25,
> + IMX_SC_RM_FUNC_DUMP = 27,
> +};
> +
> +#if IS_ENABLED(CONFIG_IMX_SCU)
> +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16 resource);
> +#else static inline bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc
> +*ipc, u16 resource) {
> + return true;
> +}
> +#endif
> +#endif
> --
> 2.16.4
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 2/4] firmware: imx: add resource management api
2020-04-23 10:04 ` Aisheng Dong
@ 2020-04-23 10:57 ` Peng Fan
2020-04-23 14:04 ` Aisheng Dong
0 siblings, 1 reply; 22+ messages in thread
From: Peng Fan @ 2020-04-23 10:57 UTC (permalink / raw)
To: Aisheng Dong, shawnguo, s.hauer
Cc: kernel, festevam, linux, Leonard Crestez, Daniel Baluta,
Joakim Zhang, linux-kernel, linux-arm-kernel, dl-linux-imx
> Subject: RE: [PATCH 2/4] firmware: imx: add resource management api
>
> > From: Peng Fan <peng.fan@nxp.com>
> > Sent: Thursday, April 23, 2020 3:00 PM
> >
> > Add resource management API, when we have multiple partition running
> > together, resources not owned to current partition should not be used.
> >
> > Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
>
> Right now I'm still not quite understand if we really need this.
> As current resource is bound to power domains, if it's not owned by one
> specific SW execution environment, power on will also fail.
> Can you clarify if any exceptions?
There will be lots of failures when boot Linux domu if no check.
Thanks,
Peng.
>
> Regards
> Aisheng
>
> > ---
> > drivers/firmware/imx/Makefile | 2 +-
> > drivers/firmware/imx/rm.c | 40 +++++++++++++++++++++
> > include/linux/firmware/imx/sci.h | 1 +
> > include/linux/firmware/imx/svc/rm.h | 69
> > +++++++++++++++++++++++++++++++++++++
> > 4 files changed, 111 insertions(+), 1 deletion(-) create mode 100644
> > drivers/firmware/imx/rm.c create mode 100644
> > include/linux/firmware/imx/svc/rm.h
> >
> > diff --git a/drivers/firmware/imx/Makefile
> > b/drivers/firmware/imx/Makefile index 08bc9ddfbdfb..17ea3613e142
> > 100644
> > --- a/drivers/firmware/imx/Makefile
> > +++ b/drivers/firmware/imx/Makefile
> > @@ -1,4 +1,4 @@
> > # SPDX-License-Identifier: GPL-2.0
> > obj-$(CONFIG_IMX_DSP) += imx-dsp.o
> > -obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o
> > +obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o
> > obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> > diff --git a/drivers/firmware/imx/rm.c b/drivers/firmware/imx/rm.c new
> > file mode 100644 index 000000000000..7b0334de5486
> > --- /dev/null
> > +++ b/drivers/firmware/imx/rm.c
> > @@ -0,0 +1,40 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2020 NXP
> > + *
> > + * File containing client-side RPC functions for the RM service.
> > +These
> > + * function are ported to clients that communicate to the SC.
> > + */
> > +
> > +#include <linux/firmware/imx/svc/rm.h>
> > +
> > +struct imx_sc_msg_rm_rsrc_owned {
> > + struct imx_sc_rpc_msg hdr;
> > + u16 resource;
> > +} __packed __aligned(4);
> > +
> > +/*
> > + * This function check @resource is owned by current partition or not
> > + *
> > + * @param[in] ipc IPC handle
> > + * @param[in] resource resource the control is associated with
> > + *
> > + * @return Returns 0 for success and < 0 for errors.
> > + */
> > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > +resource) {
> > + struct imx_sc_msg_rm_rsrc_owned msg;
> > + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > +
> > + hdr->ver = IMX_SC_RPC_VERSION;
> > + hdr->svc = IMX_SC_RPC_SVC_RM;
> > + hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
> > + hdr->size = 2;
> > +
> > + msg.resource = resource;
> > +
> > + imx_scu_call_rpc(ipc, &msg, true);
> > +
> > + return hdr->func;
> > +}
> > +EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
> > diff --git a/include/linux/firmware/imx/sci.h
> > b/include/linux/firmware/imx/sci.h
> > index 17ba4e405129..b5c5a56f29be 100644
> > --- a/include/linux/firmware/imx/sci.h
> > +++ b/include/linux/firmware/imx/sci.h
> > @@ -15,6 +15,7 @@
> >
> > #include <linux/firmware/imx/svc/misc.h> #include
> > <linux/firmware/imx/svc/pm.h>
> > +#include <linux/firmware/imx/svc/rm.h>
> >
> > int imx_scu_enable_general_irq_channel(struct device *dev); int
> > imx_scu_irq_register_notifier(struct notifier_block *nb); diff --git
> > a/include/linux/firmware/imx/svc/rm.h
> > b/include/linux/firmware/imx/svc/rm.h
> > new file mode 100644
> > index 000000000000..fc6ea62d9d0e
> > --- /dev/null
> > +++ b/include/linux/firmware/imx/svc/rm.h
> > @@ -0,0 +1,69 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > + * Copyright 2017-2019 NXP
> > + *
> > + * Header file containing the public API for the System Controller
> > +(SC)
> > + * Power Management (PM) function. This includes functions for power
> > +state
> > + * control, clock control, reset control, and wake-up event control.
> > + *
> > + * RM_SVC (SVC) Resource Management Service
> > + *
> > + * Module for the Resource Management (RM) service.
> > + */
> > +
> > +#ifndef _SC_RM_API_H
> > +#define _SC_RM_API_H
> > +
> > +#include <linux/firmware/imx/sci.h>
> > +
> > +/*
> > + * This type is used to indicate RPC RM function calls.
> > + */
> > +enum imx_sc_rm_func {
> > + IMX_SC_RM_FUNC_UNKNOWN = 0,
> > + IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
> > + IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
> > + IMX_SC_RM_FUNC_PARTITION_FREE = 2,
> > + IMX_SC_RM_FUNC_GET_DID = 26,
> > + IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
> > + IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
> > + IMX_SC_RM_FUNC_GET_PARTITION = 5,
> > + IMX_SC_RM_FUNC_SET_PARENT = 6,
> > + IMX_SC_RM_FUNC_MOVE_ALL = 7,
> > + IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
> > + IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
> > + IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
> > + IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
> > + IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
> > + IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS = 12,
> > + IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
> > + IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
> > + IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
> > + IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
> > + IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
> > + IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
> > + IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
> > + IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
> > + IMX_SC_RM_FUNC_MEMREG_FREE = 18,
> > + IMX_SC_RM_FUNC_FIND_MEMREG = 30,
> > + IMX_SC_RM_FUNC_ASSIGN_MEMREG = 19,
> > + IMX_SC_RM_FUNC_SET_MEMREG_PERMISSIONS = 20,
> > + IMX_SC_RM_FUNC_IS_MEMREG_OWNED = 21,
> > + IMX_SC_RM_FUNC_GET_MEMREG_INFO = 22,
> > + IMX_SC_RM_FUNC_ASSIGN_PAD = 23,
> > + IMX_SC_RM_FUNC_SET_PAD_MOVABLE = 24,
> > + IMX_SC_RM_FUNC_IS_PAD_OWNED = 25,
> > + IMX_SC_RM_FUNC_DUMP = 27,
> > +};
> > +
> > +#if IS_ENABLED(CONFIG_IMX_SCU)
> > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > +resource); #else static inline bool
> > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16 resource) {
> > + return true;
> > +}
> > +#endif
> > +#endif
> > --
> > 2.16.4
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 2/4] firmware: imx: add resource management api
2020-04-23 10:57 ` Peng Fan
@ 2020-04-23 14:04 ` Aisheng Dong
2020-04-24 1:11 ` Peng Fan
0 siblings, 1 reply; 22+ messages in thread
From: Aisheng Dong @ 2020-04-23 14:04 UTC (permalink / raw)
To: Peng Fan, shawnguo, s.hauer
Cc: kernel, festevam, linux, Leonard Crestez, Daniel Baluta,
Joakim Zhang, linux-kernel, linux-arm-kernel, dl-linux-imx
> From: Peng Fan <peng.fan@nxp.com>
> Sent: Thursday, April 23, 2020 6:57 PM
> > > From: Peng Fan <peng.fan@nxp.com>
> > > Sent: Thursday, April 23, 2020 3:00 PM
> > >
> > > Add resource management API, when we have multiple partition running
> > > together, resources not owned to current partition should not be used.
> > >
> > > Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >
> > Right now I'm still not quite understand if we really need this.
> > As current resource is bound to power domains, if it's not owned by
> > one specific SW execution environment, power on will also fail.
> > Can you clarify if any exceptions?
>
> There will be lots of failures when boot Linux domu if no check.
>
What kind of features did you mean?
Could you clarify a bit more? I'd like to understand this issue better.
Regards
Aisheng
> Thanks,
> Peng.
>
> >
> > Regards
> > Aisheng
> >
> > > ---
> > > drivers/firmware/imx/Makefile | 2 +-
> > > drivers/firmware/imx/rm.c | 40 +++++++++++++++++++++
> > > include/linux/firmware/imx/sci.h | 1 +
> > > include/linux/firmware/imx/svc/rm.h | 69
> > > +++++++++++++++++++++++++++++++++++++
> > > 4 files changed, 111 insertions(+), 1 deletion(-) create mode
> > > 100644 drivers/firmware/imx/rm.c create mode 100644
> > > include/linux/firmware/imx/svc/rm.h
> > >
> > > diff --git a/drivers/firmware/imx/Makefile
> > > b/drivers/firmware/imx/Makefile index 08bc9ddfbdfb..17ea3613e142
> > > 100644
> > > --- a/drivers/firmware/imx/Makefile
> > > +++ b/drivers/firmware/imx/Makefile
> > > @@ -1,4 +1,4 @@
> > > # SPDX-License-Identifier: GPL-2.0
> > > obj-$(CONFIG_IMX_DSP) += imx-dsp.o
> > > -obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o
> > > +obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o
> > > obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> > > diff --git a/drivers/firmware/imx/rm.c b/drivers/firmware/imx/rm.c
> > > new file mode 100644 index 000000000000..7b0334de5486
> > > --- /dev/null
> > > +++ b/drivers/firmware/imx/rm.c
> > > @@ -0,0 +1,40 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright 2020 NXP
> > > + *
> > > + * File containing client-side RPC functions for the RM service.
> > > +These
> > > + * function are ported to clients that communicate to the SC.
> > > + */
> > > +
> > > +#include <linux/firmware/imx/svc/rm.h>
> > > +
> > > +struct imx_sc_msg_rm_rsrc_owned {
> > > + struct imx_sc_rpc_msg hdr;
> > > + u16 resource;
> > > +} __packed __aligned(4);
> > > +
> > > +/*
> > > + * This function check @resource is owned by current partition or
> > > +not
> > > + *
> > > + * @param[in] ipc IPC handle
> > > + * @param[in] resource resource the control is associated with
> > > + *
> > > + * @return Returns 0 for success and < 0 for errors.
> > > + */
> > > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > +resource) {
> > > + struct imx_sc_msg_rm_rsrc_owned msg;
> > > + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > > +
> > > + hdr->ver = IMX_SC_RPC_VERSION;
> > > + hdr->svc = IMX_SC_RPC_SVC_RM;
> > > + hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
> > > + hdr->size = 2;
> > > +
> > > + msg.resource = resource;
> > > +
> > > + imx_scu_call_rpc(ipc, &msg, true);
> > > +
> > > + return hdr->func;
> > > +}
> > > +EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
> > > diff --git a/include/linux/firmware/imx/sci.h
> > > b/include/linux/firmware/imx/sci.h
> > > index 17ba4e405129..b5c5a56f29be 100644
> > > --- a/include/linux/firmware/imx/sci.h
> > > +++ b/include/linux/firmware/imx/sci.h
> > > @@ -15,6 +15,7 @@
> > >
> > > #include <linux/firmware/imx/svc/misc.h> #include
> > > <linux/firmware/imx/svc/pm.h>
> > > +#include <linux/firmware/imx/svc/rm.h>
> > >
> > > int imx_scu_enable_general_irq_channel(struct device *dev); int
> > > imx_scu_irq_register_notifier(struct notifier_block *nb); diff --git
> > > a/include/linux/firmware/imx/svc/rm.h
> > > b/include/linux/firmware/imx/svc/rm.h
> > > new file mode 100644
> > > index 000000000000..fc6ea62d9d0e
> > > --- /dev/null
> > > +++ b/include/linux/firmware/imx/svc/rm.h
> > > @@ -0,0 +1,69 @@
> > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > +/*
> > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > + * Copyright 2017-2019 NXP
> > > + *
> > > + * Header file containing the public API for the System Controller
> > > +(SC)
> > > + * Power Management (PM) function. This includes functions for
> > > +power state
> > > + * control, clock control, reset control, and wake-up event control.
> > > + *
> > > + * RM_SVC (SVC) Resource Management Service
> > > + *
> > > + * Module for the Resource Management (RM) service.
> > > + */
> > > +
> > > +#ifndef _SC_RM_API_H
> > > +#define _SC_RM_API_H
> > > +
> > > +#include <linux/firmware/imx/sci.h>
> > > +
> > > +/*
> > > + * This type is used to indicate RPC RM function calls.
> > > + */
> > > +enum imx_sc_rm_func {
> > > + IMX_SC_RM_FUNC_UNKNOWN = 0,
> > > + IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
> > > + IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
> > > + IMX_SC_RM_FUNC_PARTITION_FREE = 2,
> > > + IMX_SC_RM_FUNC_GET_DID = 26,
> > > + IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
> > > + IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
> > > + IMX_SC_RM_FUNC_GET_PARTITION = 5,
> > > + IMX_SC_RM_FUNC_SET_PARENT = 6,
> > > + IMX_SC_RM_FUNC_MOVE_ALL = 7,
> > > + IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
> > > + IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
> > > + IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
> > > + IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
> > > + IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
> > > + IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS = 12,
> > > + IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
> > > + IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
> > > + IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
> > > + IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
> > > + IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
> > > + IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
> > > + IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
> > > + IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
> > > + IMX_SC_RM_FUNC_MEMREG_FREE = 18,
> > > + IMX_SC_RM_FUNC_FIND_MEMREG = 30,
> > > + IMX_SC_RM_FUNC_ASSIGN_MEMREG = 19,
> > > + IMX_SC_RM_FUNC_SET_MEMREG_PERMISSIONS = 20,
> > > + IMX_SC_RM_FUNC_IS_MEMREG_OWNED = 21,
> > > + IMX_SC_RM_FUNC_GET_MEMREG_INFO = 22,
> > > + IMX_SC_RM_FUNC_ASSIGN_PAD = 23,
> > > + IMX_SC_RM_FUNC_SET_PAD_MOVABLE = 24,
> > > + IMX_SC_RM_FUNC_IS_PAD_OWNED = 25,
> > > + IMX_SC_RM_FUNC_DUMP = 27,
> > > +};
> > > +
> > > +#if IS_ENABLED(CONFIG_IMX_SCU)
> > > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > +resource); #else static inline bool
> > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16 resource) {
> > > + return true;
> > > +}
> > > +#endif
> > > +#endif
> > > --
> > > 2.16.4
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 2/4] firmware: imx: add resource management api
2020-04-23 14:04 ` Aisheng Dong
@ 2020-04-24 1:11 ` Peng Fan
2020-04-24 2:04 ` Aisheng Dong
0 siblings, 1 reply; 22+ messages in thread
From: Peng Fan @ 2020-04-24 1:11 UTC (permalink / raw)
To: Aisheng Dong, shawnguo, s.hauer
Cc: kernel, festevam, linux, Leonard Crestez, Daniel Baluta,
Joakim Zhang, linux-kernel, linux-arm-kernel, dl-linux-imx
> Subject: RE: [PATCH 2/4] firmware: imx: add resource management api
>
> > From: Peng Fan <peng.fan@nxp.com>
> > Sent: Thursday, April 23, 2020 6:57 PM
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > > Sent: Thursday, April 23, 2020 3:00 PM
> > > >
> > > > Add resource management API, when we have multiple partition
> > > > running together, resources not owned to current partition should not be
> used.
> > > >
> > > > Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > >
> > > Right now I'm still not quite understand if we really need this.
> > > As current resource is bound to power domains, if it's not owned by
> > > one specific SW execution environment, power on will also fail.
> > > Can you clarify if any exceptions?
> >
> > There will be lots of failures when boot Linux domu if no check.
> >
>
> What kind of features did you mean?
> Could you clarify a bit more? I'd like to understand this issue better.
When supporting hypervisor with dual Linux running, 1st Linux and the
2nd Linux will have their own dedicated resources.
If no resource check, that means 1st/2nd Linux will register all the
resources, then both will see fail logs when register resources not
owned to itself.
Same to partitioned m4 case.
Would it be better without the failure log?
Thanks,
Peng.
>
> Regards
> Aisheng
>
> > Thanks,
> > Peng.
> >
> > >
> > > Regards
> > > Aisheng
> > >
> > > > ---
> > > > drivers/firmware/imx/Makefile | 2 +-
> > > > drivers/firmware/imx/rm.c | 40
> +++++++++++++++++++++
> > > > include/linux/firmware/imx/sci.h | 1 +
> > > > include/linux/firmware/imx/svc/rm.h | 69
> > > > +++++++++++++++++++++++++++++++++++++
> > > > 4 files changed, 111 insertions(+), 1 deletion(-) create mode
> > > > 100644 drivers/firmware/imx/rm.c create mode 100644
> > > > include/linux/firmware/imx/svc/rm.h
> > > >
> > > > diff --git a/drivers/firmware/imx/Makefile
> > > > b/drivers/firmware/imx/Makefile index 08bc9ddfbdfb..17ea3613e142
> > > > 100644
> > > > --- a/drivers/firmware/imx/Makefile
> > > > +++ b/drivers/firmware/imx/Makefile
> > > > @@ -1,4 +1,4 @@
> > > > # SPDX-License-Identifier: GPL-2.0
> > > > obj-$(CONFIG_IMX_DSP) += imx-dsp.o
> > > > -obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o
> > > > +obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o
> > > > obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> > > > diff --git a/drivers/firmware/imx/rm.c b/drivers/firmware/imx/rm.c
> > > > new file mode 100644 index 000000000000..7b0334de5486
> > > > --- /dev/null
> > > > +++ b/drivers/firmware/imx/rm.c
> > > > @@ -0,0 +1,40 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +/*
> > > > + * Copyright 2020 NXP
> > > > + *
> > > > + * File containing client-side RPC functions for the RM service.
> > > > +These
> > > > + * function are ported to clients that communicate to the SC.
> > > > + */
> > > > +
> > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > +
> > > > +struct imx_sc_msg_rm_rsrc_owned {
> > > > + struct imx_sc_rpc_msg hdr;
> > > > + u16 resource;
> > > > +} __packed __aligned(4);
> > > > +
> > > > +/*
> > > > + * This function check @resource is owned by current partition or
> > > > +not
> > > > + *
> > > > + * @param[in] ipc IPC handle
> > > > + * @param[in] resource resource the control is associated
> with
> > > > + *
> > > > + * @return Returns 0 for success and < 0 for errors.
> > > > + */
> > > > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > > +resource) {
> > > > + struct imx_sc_msg_rm_rsrc_owned msg;
> > > > + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > > > +
> > > > + hdr->ver = IMX_SC_RPC_VERSION;
> > > > + hdr->svc = IMX_SC_RPC_SVC_RM;
> > > > + hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
> > > > + hdr->size = 2;
> > > > +
> > > > + msg.resource = resource;
> > > > +
> > > > + imx_scu_call_rpc(ipc, &msg, true);
> > > > +
> > > > + return hdr->func;
> > > > +}
> > > > +EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
> > > > diff --git a/include/linux/firmware/imx/sci.h
> > > > b/include/linux/firmware/imx/sci.h
> > > > index 17ba4e405129..b5c5a56f29be 100644
> > > > --- a/include/linux/firmware/imx/sci.h
> > > > +++ b/include/linux/firmware/imx/sci.h
> > > > @@ -15,6 +15,7 @@
> > > >
> > > > #include <linux/firmware/imx/svc/misc.h> #include
> > > > <linux/firmware/imx/svc/pm.h>
> > > > +#include <linux/firmware/imx/svc/rm.h>
> > > >
> > > > int imx_scu_enable_general_irq_channel(struct device *dev); int
> > > > imx_scu_irq_register_notifier(struct notifier_block *nb); diff
> > > > --git a/include/linux/firmware/imx/svc/rm.h
> > > > b/include/linux/firmware/imx/svc/rm.h
> > > > new file mode 100644
> > > > index 000000000000..fc6ea62d9d0e
> > > > --- /dev/null
> > > > +++ b/include/linux/firmware/imx/svc/rm.h
> > > > @@ -0,0 +1,69 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > +/*
> > > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > > + * Copyright 2017-2019 NXP
> > > > + *
> > > > + * Header file containing the public API for the System
> > > > +Controller
> > > > +(SC)
> > > > + * Power Management (PM) function. This includes functions for
> > > > +power state
> > > > + * control, clock control, reset control, and wake-up event control.
> > > > + *
> > > > + * RM_SVC (SVC) Resource Management Service
> > > > + *
> > > > + * Module for the Resource Management (RM) service.
> > > > + */
> > > > +
> > > > +#ifndef _SC_RM_API_H
> > > > +#define _SC_RM_API_H
> > > > +
> > > > +#include <linux/firmware/imx/sci.h>
> > > > +
> > > > +/*
> > > > + * This type is used to indicate RPC RM function calls.
> > > > + */
> > > > +enum imx_sc_rm_func {
> > > > + IMX_SC_RM_FUNC_UNKNOWN = 0,
> > > > + IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
> > > > + IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
> > > > + IMX_SC_RM_FUNC_PARTITION_FREE = 2,
> > > > + IMX_SC_RM_FUNC_GET_DID = 26,
> > > > + IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
> > > > + IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
> > > > + IMX_SC_RM_FUNC_GET_PARTITION = 5,
> > > > + IMX_SC_RM_FUNC_SET_PARENT = 6,
> > > > + IMX_SC_RM_FUNC_MOVE_ALL = 7,
> > > > + IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
> > > > + IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
> > > > + IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
> > > > + IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
> > > > + IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
> > > > + IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS = 12,
> > > > + IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
> > > > + IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
> > > > + IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
> > > > + IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
> > > > + IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
> > > > + IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
> > > > + IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
> > > > + IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
> > > > + IMX_SC_RM_FUNC_MEMREG_FREE = 18,
> > > > + IMX_SC_RM_FUNC_FIND_MEMREG = 30,
> > > > + IMX_SC_RM_FUNC_ASSIGN_MEMREG = 19,
> > > > + IMX_SC_RM_FUNC_SET_MEMREG_PERMISSIONS = 20,
> > > > + IMX_SC_RM_FUNC_IS_MEMREG_OWNED = 21,
> > > > + IMX_SC_RM_FUNC_GET_MEMREG_INFO = 22,
> > > > + IMX_SC_RM_FUNC_ASSIGN_PAD = 23,
> > > > + IMX_SC_RM_FUNC_SET_PAD_MOVABLE = 24,
> > > > + IMX_SC_RM_FUNC_IS_PAD_OWNED = 25,
> > > > + IMX_SC_RM_FUNC_DUMP = 27,
> > > > +};
> > > > +
> > > > +#if IS_ENABLED(CONFIG_IMX_SCU)
> > > > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > > +resource); #else static inline bool
> > > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16 resource) {
> > > > + return true;
> > > > +}
> > > > +#endif
> > > > +#endif
> > > > --
> > > > 2.16.4
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 2/4] firmware: imx: add resource management api
2020-04-24 1:11 ` Peng Fan
@ 2020-04-24 2:04 ` Aisheng Dong
2020-06-01 12:40 ` Peng Fan
0 siblings, 1 reply; 22+ messages in thread
From: Aisheng Dong @ 2020-04-24 2:04 UTC (permalink / raw)
To: Peng Fan, shawnguo, s.hauer
Cc: kernel, festevam, linux, Leonard Crestez, Daniel Baluta,
Joakim Zhang, linux-kernel, linux-arm-kernel, dl-linux-imx
> From: Peng Fan <peng.fan@nxp.com>
> Sent: Friday, April 24, 2020 9:12 AM
> >
> > > From: Peng Fan <peng.fan@nxp.com>
> > > Sent: Thursday, April 23, 2020 6:57 PM
> > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > Sent: Thursday, April 23, 2020 3:00 PM
> > > > >
> > > > > Add resource management API, when we have multiple partition
> > > > > running together, resources not owned to current partition
> > > > > should not be
> > used.
> > > > >
> > > > > Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > Right now I'm still not quite understand if we really need this.
> > > > As current resource is bound to power domains, if it's not owned
> > > > by one specific SW execution environment, power on will also fail.
> > > > Can you clarify if any exceptions?
> > >
> > > There will be lots of failures when boot Linux domu if no check.
> > >
> >
> > What kind of features did you mean?
> > Could you clarify a bit more? I'd like to understand this issue better.
>
> When supporting hypervisor with dual Linux running, 1st Linux and the 2nd
> Linux will have their own dedicated resources.
>
> If no resource check, that means 1st/2nd Linux will register all the resources,
> then both will see fail logs when register resources not owned to itself.
>
> Same to partitioned m4 case.
>
> Would it be better without the failure log?
>
Is it power up failure?
If yes, it's expected because we actually use power up to check if resources are owned by
this partition. It functions the same as calling resource check API.
That's current design.
Or it's other failures? Log?
Regards
Aisheng
> Thanks,
> Peng.
>
>
> >
> > Regards
> > Aisheng
> >
> > > Thanks,
> > > Peng.
> > >
> > > >
> > > > Regards
> > > > Aisheng
> > > >
> > > > > ---
> > > > > drivers/firmware/imx/Makefile | 2 +-
> > > > > drivers/firmware/imx/rm.c | 40
> > +++++++++++++++++++++
> > > > > include/linux/firmware/imx/sci.h | 1 +
> > > > > include/linux/firmware/imx/svc/rm.h | 69
> > > > > +++++++++++++++++++++++++++++++++++++
> > > > > 4 files changed, 111 insertions(+), 1 deletion(-) create mode
> > > > > 100644 drivers/firmware/imx/rm.c create mode 100644
> > > > > include/linux/firmware/imx/svc/rm.h
> > > > >
> > > > > diff --git a/drivers/firmware/imx/Makefile
> > > > > b/drivers/firmware/imx/Makefile index 08bc9ddfbdfb..17ea3613e142
> > > > > 100644
> > > > > --- a/drivers/firmware/imx/Makefile
> > > > > +++ b/drivers/firmware/imx/Makefile
> > > > > @@ -1,4 +1,4 @@
> > > > > # SPDX-License-Identifier: GPL-2.0
> > > > > obj-$(CONFIG_IMX_DSP) += imx-dsp.o
> > > > > -obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o
> > > > > +obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o
> > > > > obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> > > > > diff --git a/drivers/firmware/imx/rm.c
> > > > > b/drivers/firmware/imx/rm.c new file mode 100644 index
> > > > > 000000000000..7b0334de5486
> > > > > --- /dev/null
> > > > > +++ b/drivers/firmware/imx/rm.c
> > > > > @@ -0,0 +1,40 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > +/*
> > > > > + * Copyright 2020 NXP
> > > > > + *
> > > > > + * File containing client-side RPC functions for the RM service.
> > > > > +These
> > > > > + * function are ported to clients that communicate to the SC.
> > > > > + */
> > > > > +
> > > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > > +
> > > > > +struct imx_sc_msg_rm_rsrc_owned {
> > > > > + struct imx_sc_rpc_msg hdr;
> > > > > + u16 resource;
> > > > > +} __packed __aligned(4);
> > > > > +
> > > > > +/*
> > > > > + * This function check @resource is owned by current partition
> > > > > +or not
> > > > > + *
> > > > > + * @param[in] ipc IPC handle
> > > > > + * @param[in] resource resource the control is associated
> > with
> > > > > + *
> > > > > + * @return Returns 0 for success and < 0 for errors.
> > > > > + */
> > > > > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > > > +resource) {
> > > > > + struct imx_sc_msg_rm_rsrc_owned msg;
> > > > > + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > > > > +
> > > > > + hdr->ver = IMX_SC_RPC_VERSION;
> > > > > + hdr->svc = IMX_SC_RPC_SVC_RM;
> > > > > + hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
> > > > > + hdr->size = 2;
> > > > > +
> > > > > + msg.resource = resource;
> > > > > +
> > > > > + imx_scu_call_rpc(ipc, &msg, true);
> > > > > +
> > > > > + return hdr->func;
> > > > > +}
> > > > > +EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
> > > > > diff --git a/include/linux/firmware/imx/sci.h
> > > > > b/include/linux/firmware/imx/sci.h
> > > > > index 17ba4e405129..b5c5a56f29be 100644
> > > > > --- a/include/linux/firmware/imx/sci.h
> > > > > +++ b/include/linux/firmware/imx/sci.h
> > > > > @@ -15,6 +15,7 @@
> > > > >
> > > > > #include <linux/firmware/imx/svc/misc.h> #include
> > > > > <linux/firmware/imx/svc/pm.h>
> > > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > >
> > > > > int imx_scu_enable_general_irq_channel(struct device *dev);
> > > > > int imx_scu_irq_register_notifier(struct notifier_block *nb);
> > > > > diff --git a/include/linux/firmware/imx/svc/rm.h
> > > > > b/include/linux/firmware/imx/svc/rm.h
> > > > > new file mode 100644
> > > > > index 000000000000..fc6ea62d9d0e
> > > > > --- /dev/null
> > > > > +++ b/include/linux/firmware/imx/svc/rm.h
> > > > > @@ -0,0 +1,69 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > +/*
> > > > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > > > + * Copyright 2017-2019 NXP
> > > > > + *
> > > > > + * Header file containing the public API for the System
> > > > > +Controller
> > > > > +(SC)
> > > > > + * Power Management (PM) function. This includes functions for
> > > > > +power state
> > > > > + * control, clock control, reset control, and wake-up event control.
> > > > > + *
> > > > > + * RM_SVC (SVC) Resource Management Service
> > > > > + *
> > > > > + * Module for the Resource Management (RM) service.
> > > > > + */
> > > > > +
> > > > > +#ifndef _SC_RM_API_H
> > > > > +#define _SC_RM_API_H
> > > > > +
> > > > > +#include <linux/firmware/imx/sci.h>
> > > > > +
> > > > > +/*
> > > > > + * This type is used to indicate RPC RM function calls.
> > > > > + */
> > > > > +enum imx_sc_rm_func {
> > > > > + IMX_SC_RM_FUNC_UNKNOWN = 0,
> > > > > + IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
> > > > > + IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
> > > > > + IMX_SC_RM_FUNC_PARTITION_FREE = 2,
> > > > > + IMX_SC_RM_FUNC_GET_DID = 26,
> > > > > + IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
> > > > > + IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
> > > > > + IMX_SC_RM_FUNC_GET_PARTITION = 5,
> > > > > + IMX_SC_RM_FUNC_SET_PARENT = 6,
> > > > > + IMX_SC_RM_FUNC_MOVE_ALL = 7,
> > > > > + IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
> > > > > + IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
> > > > > + IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
> > > > > + IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
> > > > > + IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
> > > > > + IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS = 12,
> > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
> > > > > + IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
> > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
> > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
> > > > > + IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
> > > > > + IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
> > > > > + IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
> > > > > + IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
> > > > > + IMX_SC_RM_FUNC_MEMREG_FREE = 18,
> > > > > + IMX_SC_RM_FUNC_FIND_MEMREG = 30,
> > > > > + IMX_SC_RM_FUNC_ASSIGN_MEMREG = 19,
> > > > > + IMX_SC_RM_FUNC_SET_MEMREG_PERMISSIONS = 20,
> > > > > + IMX_SC_RM_FUNC_IS_MEMREG_OWNED = 21,
> > > > > + IMX_SC_RM_FUNC_GET_MEMREG_INFO = 22,
> > > > > + IMX_SC_RM_FUNC_ASSIGN_PAD = 23,
> > > > > + IMX_SC_RM_FUNC_SET_PAD_MOVABLE = 24,
> > > > > + IMX_SC_RM_FUNC_IS_PAD_OWNED = 25,
> > > > > + IMX_SC_RM_FUNC_DUMP = 27,
> > > > > +};
> > > > > +
> > > > > +#if IS_ENABLED(CONFIG_IMX_SCU)
> > > > > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > > > +resource); #else static inline bool
> > > > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16 resource) {
> > > > > + return true;
> > > > > +}
> > > > > +#endif
> > > > > +#endif
> > > > > --
> > > > > 2.16.4
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 2/4] firmware: imx: add resource management api
2020-04-24 2:04 ` Aisheng Dong
@ 2020-06-01 12:40 ` Peng Fan
2020-06-02 2:41 ` Aisheng Dong
0 siblings, 1 reply; 22+ messages in thread
From: Peng Fan @ 2020-06-01 12:40 UTC (permalink / raw)
To: Aisheng Dong, shawnguo, s.hauer
Cc: kernel, festevam, linux, Leonard Crestez, Daniel Baluta,
Joakim Zhang, linux-kernel, linux-arm-kernel, dl-linux-imx
> Subject: RE: [PATCH 2/4] firmware: imx: add resource management api
>
> > From: Peng Fan <peng.fan@nxp.com>
> > Sent: Friday, April 24, 2020 9:12 AM
> > >
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > > Sent: Thursday, April 23, 2020 6:57 PM
> > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > Sent: Thursday, April 23, 2020 3:00 PM
> > > > > >
> > > > > > Add resource management API, when we have multiple partition
> > > > > > running together, resources not owned to current partition
> > > > > > should not be
> > > used.
> > > > > >
> > > > > > Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > >
> > > > > Right now I'm still not quite understand if we really need this.
> > > > > As current resource is bound to power domains, if it's not owned
> > > > > by one specific SW execution environment, power on will also fail.
> > > > > Can you clarify if any exceptions?
> > > >
> > > > There will be lots of failures when boot Linux domu if no check.
> > > >
> > >
> > > What kind of features did you mean?
> > > Could you clarify a bit more? I'd like to understand this issue better.
> >
> > When supporting hypervisor with dual Linux running, 1st Linux and the
> > 2nd Linux will have their own dedicated resources.
> >
> > If no resource check, that means 1st/2nd Linux will register all the
> > resources, then both will see fail logs when register resources not owned to
> itself.
> >
> > Same to partitioned m4 case.
> >
> > Would it be better without the failure log?
> >
>
> Is it power up failure?
> If yes, it's expected because we actually use power up to check if resources
> are owned by this partition. It functions the same as calling resource check
> API.
> That's current design.
>
> Or it's other failures? Log?
Sorry for long late reply.
Part of my XEN DomU log, there are lots of failure log. I think better not
have such logs by add a resource own check.
[ 2.034774] imx6q-pcie 5f000000.pcie: IO 0x6ff80000..0x6ff8ffff -> 0x00000000
[ 2.034801] imx6q-pcie 5f000000.pcie: MEM 0x60000000..0x6fefffff -> 0x60000000
[ 2.035072] sdhc1: failed to power up resource 249 ret -13
[ 2.035619] sdhc2: failed to power up resource 250 ret -13
[ 2.036126] enet0: failed to power up resource 251 ret -13
[ 2.036584] enet1: failed to power up resource 252 ret -13
[ 2.037040] mlb0: failed to power up resource 253 ret -13
[ 2.037495] nand: failed to power up resource 265 ret -13
[ 2.037951] nand: failed to power up resource 265 ret -13
[ 2.038416] pwm0: failed to power up resource 191 ret -13
[ 2.038868] pwm1: failed to power up resource 192 ret -13
[ 2.039320] pwm2: failed to power up resource 193 ret -13
[ 2.039786] pwm3: failed to power up resource 194 ret -13
[ 2.040239] pwm4: failed to power up resource 195 ret -13
[ 2.040692] pwm5: failed to power up resource 196 ret -13
[ 2.041142] pwm6: failed to power up resource 197 ret -13
[ 2.041596] pwm7: failed to power up resource 198 ret -13
[ 2.042073] amix: failed to power up resource 458 ret -13
[ 2.042558] lpspi0: failed to power up resource 53 ret -13
[ 2.043033] lpspi1: failed to power up resource 54 ret -13
[ 2.043501] lpspi2: failed to power up resource 55 ret -13
[ 2.043992] lpspi3: failed to power up resource 56 ret -13
[ 2.044460] lpuart0: failed to power up resource 57 ret -13
[ 2.044935] lpuart2: failed to power up resource 59 ret -13
[ 2.045409] lpuart3: failed to power up resource 60 ret -13
[ 2.055014] sim0: failed to power up resource 62 ret -13
[ 2.055510] adc0: failed to power up resource 101 ret -13
[ 2.056467] lpi2c0: failed to power up resource 96 ret -13
[ 2.056946] lpi2c1: failed to power up resource 97 ret -13
[ 2.057424] lpi2c2: failed to power up resource 98 ret -13
[ 2.057898] lpi2c3: failed to power up resource 99 ret -13
[ 2.058371] can0: failed to power up resource 105 ret -13
[ 2.059289] can1: failed to power up resource 106 ret -13
[ 2.059801] can2: failed to power up resource 107 ret -13
[ 2.060281] nand: failed to power up resource 265 ret -13
[ 2.062764] dpu-core 56180000.dpu: driver probed
Thanks,
Peng.
>
> Regards
> Aisheng
>
> > Thanks,
> > Peng.
> >
> >
> > >
> > > Regards
> > > Aisheng
> > >
> > > > Thanks,
> > > > Peng.
> > > >
> > > > >
> > > > > Regards
> > > > > Aisheng
> > > > >
> > > > > > ---
> > > > > > drivers/firmware/imx/Makefile | 2 +-
> > > > > > drivers/firmware/imx/rm.c | 40
> > > +++++++++++++++++++++
> > > > > > include/linux/firmware/imx/sci.h | 1 +
> > > > > > include/linux/firmware/imx/svc/rm.h | 69
> > > > > > +++++++++++++++++++++++++++++++++++++
> > > > > > 4 files changed, 111 insertions(+), 1 deletion(-) create
> > > > > > mode
> > > > > > 100644 drivers/firmware/imx/rm.c create mode 100644
> > > > > > include/linux/firmware/imx/svc/rm.h
> > > > > >
> > > > > > diff --git a/drivers/firmware/imx/Makefile
> > > > > > b/drivers/firmware/imx/Makefile index
> > > > > > 08bc9ddfbdfb..17ea3613e142
> > > > > > 100644
> > > > > > --- a/drivers/firmware/imx/Makefile
> > > > > > +++ b/drivers/firmware/imx/Makefile
> > > > > > @@ -1,4 +1,4 @@
> > > > > > # SPDX-License-Identifier: GPL-2.0
> > > > > > obj-$(CONFIG_IMX_DSP) += imx-dsp.o
> > > > > > -obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o
> > > > > > +obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o
> rm.o
> > > > > > obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> > > > > > diff --git a/drivers/firmware/imx/rm.c
> > > > > > b/drivers/firmware/imx/rm.c new file mode 100644 index
> > > > > > 000000000000..7b0334de5486
> > > > > > --- /dev/null
> > > > > > +++ b/drivers/firmware/imx/rm.c
> > > > > > @@ -0,0 +1,40 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > +/*
> > > > > > + * Copyright 2020 NXP
> > > > > > + *
> > > > > > + * File containing client-side RPC functions for the RM service.
> > > > > > +These
> > > > > > + * function are ported to clients that communicate to the SC.
> > > > > > + */
> > > > > > +
> > > > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > > > +
> > > > > > +struct imx_sc_msg_rm_rsrc_owned {
> > > > > > + struct imx_sc_rpc_msg hdr;
> > > > > > + u16 resource;
> > > > > > +} __packed __aligned(4);
> > > > > > +
> > > > > > +/*
> > > > > > + * This function check @resource is owned by current
> > > > > > +partition or not
> > > > > > + *
> > > > > > + * @param[in] ipc IPC handle
> > > > > > + * @param[in] resource resource the control is
> associated
> > > with
> > > > > > + *
> > > > > > + * @return Returns 0 for success and < 0 for errors.
> > > > > > + */
> > > > > > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > > > > +resource) {
> > > > > > + struct imx_sc_msg_rm_rsrc_owned msg;
> > > > > > + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > > > > > +
> > > > > > + hdr->ver = IMX_SC_RPC_VERSION;
> > > > > > + hdr->svc = IMX_SC_RPC_SVC_RM;
> > > > > > + hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
> > > > > > + hdr->size = 2;
> > > > > > +
> > > > > > + msg.resource = resource;
> > > > > > +
> > > > > > + imx_scu_call_rpc(ipc, &msg, true);
> > > > > > +
> > > > > > + return hdr->func;
> > > > > > +}
> > > > > > +EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
> > > > > > diff --git a/include/linux/firmware/imx/sci.h
> > > > > > b/include/linux/firmware/imx/sci.h
> > > > > > index 17ba4e405129..b5c5a56f29be 100644
> > > > > > --- a/include/linux/firmware/imx/sci.h
> > > > > > +++ b/include/linux/firmware/imx/sci.h
> > > > > > @@ -15,6 +15,7 @@
> > > > > >
> > > > > > #include <linux/firmware/imx/svc/misc.h> #include
> > > > > > <linux/firmware/imx/svc/pm.h>
> > > > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > > >
> > > > > > int imx_scu_enable_general_irq_channel(struct device *dev);
> > > > > > int imx_scu_irq_register_notifier(struct notifier_block *nb);
> > > > > > diff --git a/include/linux/firmware/imx/svc/rm.h
> > > > > > b/include/linux/firmware/imx/svc/rm.h
> > > > > > new file mode 100644
> > > > > > index 000000000000..fc6ea62d9d0e
> > > > > > --- /dev/null
> > > > > > +++ b/include/linux/firmware/imx/svc/rm.h
> > > > > > @@ -0,0 +1,69 @@
> > > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > > +/*
> > > > > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > > > > + * Copyright 2017-2019 NXP
> > > > > > + *
> > > > > > + * Header file containing the public API for the System
> > > > > > +Controller
> > > > > > +(SC)
> > > > > > + * Power Management (PM) function. This includes functions
> > > > > > +for power state
> > > > > > + * control, clock control, reset control, and wake-up event control.
> > > > > > + *
> > > > > > + * RM_SVC (SVC) Resource Management Service
> > > > > > + *
> > > > > > + * Module for the Resource Management (RM) service.
> > > > > > + */
> > > > > > +
> > > > > > +#ifndef _SC_RM_API_H
> > > > > > +#define _SC_RM_API_H
> > > > > > +
> > > > > > +#include <linux/firmware/imx/sci.h>
> > > > > > +
> > > > > > +/*
> > > > > > + * This type is used to indicate RPC RM function calls.
> > > > > > + */
> > > > > > +enum imx_sc_rm_func {
> > > > > > + IMX_SC_RM_FUNC_UNKNOWN = 0,
> > > > > > + IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
> > > > > > + IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
> > > > > > + IMX_SC_RM_FUNC_PARTITION_FREE = 2,
> > > > > > + IMX_SC_RM_FUNC_GET_DID = 26,
> > > > > > + IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
> > > > > > + IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
> > > > > > + IMX_SC_RM_FUNC_GET_PARTITION = 5,
> > > > > > + IMX_SC_RM_FUNC_SET_PARENT = 6,
> > > > > > + IMX_SC_RM_FUNC_MOVE_ALL = 7,
> > > > > > + IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
> > > > > > + IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
> > > > > > + IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
> > > > > > + IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
> > > > > > + IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
> > > > > > + IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS = 12,
> > > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
> > > > > > + IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
> > > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
> > > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
> > > > > > + IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
> > > > > > + IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
> > > > > > + IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
> > > > > > + IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
> > > > > > + IMX_SC_RM_FUNC_MEMREG_FREE = 18,
> > > > > > + IMX_SC_RM_FUNC_FIND_MEMREG = 30,
> > > > > > + IMX_SC_RM_FUNC_ASSIGN_MEMREG = 19,
> > > > > > + IMX_SC_RM_FUNC_SET_MEMREG_PERMISSIONS = 20,
> > > > > > + IMX_SC_RM_FUNC_IS_MEMREG_OWNED = 21,
> > > > > > + IMX_SC_RM_FUNC_GET_MEMREG_INFO = 22,
> > > > > > + IMX_SC_RM_FUNC_ASSIGN_PAD = 23,
> > > > > > + IMX_SC_RM_FUNC_SET_PAD_MOVABLE = 24,
> > > > > > + IMX_SC_RM_FUNC_IS_PAD_OWNED = 25,
> > > > > > + IMX_SC_RM_FUNC_DUMP = 27,
> > > > > > +};
> > > > > > +
> > > > > > +#if IS_ENABLED(CONFIG_IMX_SCU) bool
> > > > > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > > > > +resource); #else static inline bool
> > > > > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> resource) {
> > > > > > + return true;
> > > > > > +}
> > > > > > +#endif
> > > > > > +#endif
> > > > > > --
> > > > > > 2.16.4
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 2/4] firmware: imx: add resource management api
2020-06-01 12:40 ` Peng Fan
@ 2020-06-02 2:41 ` Aisheng Dong
2020-06-02 4:50 ` Peng Fan
0 siblings, 1 reply; 22+ messages in thread
From: Aisheng Dong @ 2020-06-02 2:41 UTC (permalink / raw)
To: Peng Fan, shawnguo, s.hauer
Cc: kernel, festevam, linux, Leonard Crestez, Daniel Baluta,
Joakim Zhang, linux-kernel, linux-arm-kernel, dl-linux-imx
> From: Peng Fan <peng.fan@nxp.com>
> Sent: Monday, June 1, 2020 8:40 PM
> >
> > > From: Peng Fan <peng.fan@nxp.com>
> > > Sent: Friday, April 24, 2020 9:12 AM
> > > >
> > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > Sent: Thursday, April 23, 2020 6:57 PM
> > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > Sent: Thursday, April 23, 2020 3:00 PM
> > > > > > >
> > > > > > > Add resource management API, when we have multiple partition
> > > > > > > running together, resources not owned to current partition
> > > > > > > should not be
> > > > used.
> > > > > > >
> > > > > > > Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > >
> > > > > > Right now I'm still not quite understand if we really need this.
> > > > > > As current resource is bound to power domains, if it's not
> > > > > > owned by one specific SW execution environment, power on will also
> fail.
> > > > > > Can you clarify if any exceptions?
> > > > >
> > > > > There will be lots of failures when boot Linux domu if no check.
> > > > >
> > > >
> > > > What kind of features did you mean?
> > > > Could you clarify a bit more? I'd like to understand this issue better.
> > >
> > > When supporting hypervisor with dual Linux running, 1st Linux and
> > > the 2nd Linux will have their own dedicated resources.
> > >
> > > If no resource check, that means 1st/2nd Linux will register all the
> > > resources, then both will see fail logs when register resources not
> > > owned to
> > itself.
> > >
> > > Same to partitioned m4 case.
> > >
> > > Would it be better without the failure log?
> > >
> >
> > Is it power up failure?
> > If yes, it's expected because we actually use power up to check if
> > resources are owned by this partition. It functions the same as
> > calling resource check API.
> > That's current design.
> >
> > Or it's other failures? Log?
> Sorry for long late reply.
>
> Part of my XEN DomU log, there are lots of failure log. I think better not have
> such logs by add a resource own check.
Those error logs are intended.
Resource owner check actually behaves the same as power up.
So not quite necessary to add a double check.
If we're concerning about the error log, we can change it to dev_dbg().
BTW, as resource management will be needed by seco drivers later,
So I will continue to review the patch.
Regards
Aisheng
>
> [ 2.034774] imx6q-pcie 5f000000.pcie: IO 0x6ff80000..0x6ff8ffff ->
> 0x00000000
> [ 2.034801] imx6q-pcie 5f000000.pcie: MEM 0x60000000..0x6fefffff ->
> 0x60000000
> [ 2.035072] sdhc1: failed to power up resource 249 ret -13
> [ 2.035619] sdhc2: failed to power up resource 250 ret -13
> [ 2.036126] enet0: failed to power up resource 251 ret -13
> [ 2.036584] enet1: failed to power up resource 252 ret -13
> [ 2.037040] mlb0: failed to power up resource 253 ret -13
> [ 2.037495] nand: failed to power up resource 265 ret -13
> [ 2.037951] nand: failed to power up resource 265 ret -13
> [ 2.038416] pwm0: failed to power up resource 191 ret -13
> [ 2.038868] pwm1: failed to power up resource 192 ret -13
> [ 2.039320] pwm2: failed to power up resource 193 ret -13
> [ 2.039786] pwm3: failed to power up resource 194 ret -13
> [ 2.040239] pwm4: failed to power up resource 195 ret -13
> [ 2.040692] pwm5: failed to power up resource 196 ret -13
> [ 2.041142] pwm6: failed to power up resource 197 ret -13
> [ 2.041596] pwm7: failed to power up resource 198 ret -13
> [ 2.042073] amix: failed to power up resource 458 ret -13
> [ 2.042558] lpspi0: failed to power up resource 53 ret -13
> [ 2.043033] lpspi1: failed to power up resource 54 ret -13
> [ 2.043501] lpspi2: failed to power up resource 55 ret -13
> [ 2.043992] lpspi3: failed to power up resource 56 ret -13
> [ 2.044460] lpuart0: failed to power up resource 57 ret -13
> [ 2.044935] lpuart2: failed to power up resource 59 ret -13
> [ 2.045409] lpuart3: failed to power up resource 60 ret -13
> [ 2.055014] sim0: failed to power up resource 62 ret -13
> [ 2.055510] adc0: failed to power up resource 101 ret -13
> [ 2.056467] lpi2c0: failed to power up resource 96 ret -13
> [ 2.056946] lpi2c1: failed to power up resource 97 ret -13
> [ 2.057424] lpi2c2: failed to power up resource 98 ret -13
> [ 2.057898] lpi2c3: failed to power up resource 99 ret -13
> [ 2.058371] can0: failed to power up resource 105 ret -13
> [ 2.059289] can1: failed to power up resource 106 ret -13
> [ 2.059801] can2: failed to power up resource 107 ret -13
> [ 2.060281] nand: failed to power up resource 265 ret -13
> [ 2.062764] dpu-core 56180000.dpu: driver probed
>
> Thanks,
> Peng.
>
> >
> > Regards
> > Aisheng
> >
> > > Thanks,
> > > Peng.
> > >
> > >
> > > >
> > > > Regards
> > > > Aisheng
> > > >
> > > > > Thanks,
> > > > > Peng.
> > > > >
> > > > > >
> > > > > > Regards
> > > > > > Aisheng
> > > > > >
> > > > > > > ---
> > > > > > > drivers/firmware/imx/Makefile | 2 +-
> > > > > > > drivers/firmware/imx/rm.c | 40
> > > > +++++++++++++++++++++
> > > > > > > include/linux/firmware/imx/sci.h | 1 +
> > > > > > > include/linux/firmware/imx/svc/rm.h | 69
> > > > > > > +++++++++++++++++++++++++++++++++++++
> > > > > > > 4 files changed, 111 insertions(+), 1 deletion(-) create
> > > > > > > mode
> > > > > > > 100644 drivers/firmware/imx/rm.c create mode 100644
> > > > > > > include/linux/firmware/imx/svc/rm.h
> > > > > > >
> > > > > > > diff --git a/drivers/firmware/imx/Makefile
> > > > > > > b/drivers/firmware/imx/Makefile index
> > > > > > > 08bc9ddfbdfb..17ea3613e142
> > > > > > > 100644
> > > > > > > --- a/drivers/firmware/imx/Makefile
> > > > > > > +++ b/drivers/firmware/imx/Makefile
> > > > > > > @@ -1,4 +1,4 @@
> > > > > > > # SPDX-License-Identifier: GPL-2.0
> > > > > > > obj-$(CONFIG_IMX_DSP) += imx-dsp.o
> > > > > > > -obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o
> > > > > > > +obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o
> > rm.o
> > > > > > > obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> > > > > > > diff --git a/drivers/firmware/imx/rm.c
> > > > > > > b/drivers/firmware/imx/rm.c new file mode 100644 index
> > > > > > > 000000000000..7b0334de5486
> > > > > > > --- /dev/null
> > > > > > > +++ b/drivers/firmware/imx/rm.c
> > > > > > > @@ -0,0 +1,40 @@
> > > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > > +/*
> > > > > > > + * Copyright 2020 NXP
> > > > > > > + *
> > > > > > > + * File containing client-side RPC functions for the RM service.
> > > > > > > +These
> > > > > > > + * function are ported to clients that communicate to the SC.
> > > > > > > + */
> > > > > > > +
> > > > > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > > > > +
> > > > > > > +struct imx_sc_msg_rm_rsrc_owned {
> > > > > > > + struct imx_sc_rpc_msg hdr;
> > > > > > > + u16 resource;
> > > > > > > +} __packed __aligned(4);
> > > > > > > +
> > > > > > > +/*
> > > > > > > + * This function check @resource is owned by current
> > > > > > > +partition or not
> > > > > > > + *
> > > > > > > + * @param[in] ipc IPC handle
> > > > > > > + * @param[in] resource resource the control is
> > associated
> > > > with
> > > > > > > + *
> > > > > > > + * @return Returns 0 for success and < 0 for errors.
> > > > > > > + */
> > > > > > > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc,
> > > > > > > +u16
> > > > > > > +resource) {
> > > > > > > + struct imx_sc_msg_rm_rsrc_owned msg;
> > > > > > > + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > > > > > > +
> > > > > > > + hdr->ver = IMX_SC_RPC_VERSION;
> > > > > > > + hdr->svc = IMX_SC_RPC_SVC_RM;
> > > > > > > + hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
> > > > > > > + hdr->size = 2;
> > > > > > > +
> > > > > > > + msg.resource = resource;
> > > > > > > +
> > > > > > > + imx_scu_call_rpc(ipc, &msg, true);
> > > > > > > +
> > > > > > > + return hdr->func;
> > > > > > > +}
> > > > > > > +EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
> > > > > > > diff --git a/include/linux/firmware/imx/sci.h
> > > > > > > b/include/linux/firmware/imx/sci.h
> > > > > > > index 17ba4e405129..b5c5a56f29be 100644
> > > > > > > --- a/include/linux/firmware/imx/sci.h
> > > > > > > +++ b/include/linux/firmware/imx/sci.h
> > > > > > > @@ -15,6 +15,7 @@
> > > > > > >
> > > > > > > #include <linux/firmware/imx/svc/misc.h> #include
> > > > > > > <linux/firmware/imx/svc/pm.h>
> > > > > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > > > >
> > > > > > > int imx_scu_enable_general_irq_channel(struct device *dev);
> > > > > > > int imx_scu_irq_register_notifier(struct notifier_block
> > > > > > > *nb); diff --git a/include/linux/firmware/imx/svc/rm.h
> > > > > > > b/include/linux/firmware/imx/svc/rm.h
> > > > > > > new file mode 100644
> > > > > > > index 000000000000..fc6ea62d9d0e
> > > > > > > --- /dev/null
> > > > > > > +++ b/include/linux/firmware/imx/svc/rm.h
> > > > > > > @@ -0,0 +1,69 @@
> > > > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > > > +/*
> > > > > > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > > > > > + * Copyright 2017-2019 NXP
> > > > > > > + *
> > > > > > > + * Header file containing the public API for the System
> > > > > > > +Controller
> > > > > > > +(SC)
> > > > > > > + * Power Management (PM) function. This includes functions
> > > > > > > +for power state
> > > > > > > + * control, clock control, reset control, and wake-up event control.
> > > > > > > + *
> > > > > > > + * RM_SVC (SVC) Resource Management Service
> > > > > > > + *
> > > > > > > + * Module for the Resource Management (RM) service.
> > > > > > > + */
> > > > > > > +
> > > > > > > +#ifndef _SC_RM_API_H
> > > > > > > +#define _SC_RM_API_H
> > > > > > > +
> > > > > > > +#include <linux/firmware/imx/sci.h>
> > > > > > > +
> > > > > > > +/*
> > > > > > > + * This type is used to indicate RPC RM function calls.
> > > > > > > + */
> > > > > > > +enum imx_sc_rm_func {
> > > > > > > + IMX_SC_RM_FUNC_UNKNOWN = 0,
> > > > > > > + IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
> > > > > > > + IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
> > > > > > > + IMX_SC_RM_FUNC_PARTITION_FREE = 2,
> > > > > > > + IMX_SC_RM_FUNC_GET_DID = 26,
> > > > > > > + IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
> > > > > > > + IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
> > > > > > > + IMX_SC_RM_FUNC_GET_PARTITION = 5,
> > > > > > > + IMX_SC_RM_FUNC_SET_PARENT = 6,
> > > > > > > + IMX_SC_RM_FUNC_MOVE_ALL = 7,
> > > > > > > + IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
> > > > > > > + IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
> > > > > > > + IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
> > > > > > > + IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
> > > > > > > + IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
> > > > > > > + IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS = 12,
> > > > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
> > > > > > > + IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
> > > > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
> > > > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
> > > > > > > + IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
> > > > > > > + IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
> > > > > > > + IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
> > > > > > > + IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
> > > > > > > + IMX_SC_RM_FUNC_MEMREG_FREE = 18,
> > > > > > > + IMX_SC_RM_FUNC_FIND_MEMREG = 30,
> > > > > > > + IMX_SC_RM_FUNC_ASSIGN_MEMREG = 19,
> > > > > > > + IMX_SC_RM_FUNC_SET_MEMREG_PERMISSIONS = 20,
> > > > > > > + IMX_SC_RM_FUNC_IS_MEMREG_OWNED = 21,
> > > > > > > + IMX_SC_RM_FUNC_GET_MEMREG_INFO = 22,
> > > > > > > + IMX_SC_RM_FUNC_ASSIGN_PAD = 23,
> > > > > > > + IMX_SC_RM_FUNC_SET_PAD_MOVABLE = 24,
> > > > > > > + IMX_SC_RM_FUNC_IS_PAD_OWNED = 25,
> > > > > > > + IMX_SC_RM_FUNC_DUMP = 27,
> > > > > > > +};
> > > > > > > +
> > > > > > > +#if IS_ENABLED(CONFIG_IMX_SCU) bool
> > > > > > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > > > > > +resource); #else static inline bool
> > > > > > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > resource) {
> > > > > > > + return true;
> > > > > > > +}
> > > > > > > +#endif
> > > > > > > +#endif
> > > > > > > --
> > > > > > > 2.16.4
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 2/4] firmware: imx: add resource management api
2020-06-02 2:41 ` Aisheng Dong
@ 2020-06-02 4:50 ` Peng Fan
2020-06-02 7:28 ` Aisheng Dong
0 siblings, 1 reply; 22+ messages in thread
From: Peng Fan @ 2020-06-02 4:50 UTC (permalink / raw)
To: Aisheng Dong, shawnguo, s.hauer
Cc: kernel, festevam, linux, Leonard Crestez, Daniel Baluta,
Joakim Zhang, linux-kernel, linux-arm-kernel, dl-linux-imx
> Subject: RE: [PATCH 2/4] firmware: imx: add resource management api
>
> > From: Peng Fan <peng.fan@nxp.com>
> > Sent: Monday, June 1, 2020 8:40 PM
> > >
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > > Sent: Friday, April 24, 2020 9:12 AM
> > > > >
> > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > Sent: Thursday, April 23, 2020 6:57 PM
> > > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > > Sent: Thursday, April 23, 2020 3:00 PM
> > > > > > > >
> > > > > > > > Add resource management API, when we have multiple
> > > > > > > > partition running together, resources not owned to current
> > > > > > > > partition should not be
> > > > > used.
> > > > > > > >
> > > > > > > > Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> > > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > > >
> > > > > > > Right now I'm still not quite understand if we really need this.
> > > > > > > As current resource is bound to power domains, if it's not
> > > > > > > owned by one specific SW execution environment, power on
> > > > > > > will also
> > fail.
> > > > > > > Can you clarify if any exceptions?
> > > > > >
> > > > > > There will be lots of failures when boot Linux domu if no check.
> > > > > >
> > > > >
> > > > > What kind of features did you mean?
> > > > > Could you clarify a bit more? I'd like to understand this issue better.
> > > >
> > > > When supporting hypervisor with dual Linux running, 1st Linux and
> > > > the 2nd Linux will have their own dedicated resources.
> > > >
> > > > If no resource check, that means 1st/2nd Linux will register all
> > > > the resources, then both will see fail logs when register
> > > > resources not owned to
> > > itself.
> > > >
> > > > Same to partitioned m4 case.
> > > >
> > > > Would it be better without the failure log?
> > > >
> > >
> > > Is it power up failure?
> > > If yes, it's expected because we actually use power up to check if
> > > resources are owned by this partition. It functions the same as
> > > calling resource check API.
> > > That's current design.
> > >
> > > Or it's other failures? Log?
> > Sorry for long late reply.
> >
> > Part of my XEN DomU log, there are lots of failure log. I think better
> > not have such logs by add a resource own check.
>
> Those error logs are intended.
> Resource owner check actually behaves the same as power up.
If resource is not owned, it will not register the power domain.
Without resource own check, it will continue register the power domain
and hook it into genpd list.
I prefer we not expose the power domain not owned to current
partition and keep the err msg for people to easy
see where it goes wrong.
Regards,
Peng.
> So not quite necessary to add a double check.
> If we're concerning about the error log, we can change it to dev_dbg().
>
> BTW, as resource management will be needed by seco drivers later, So I will
> continue to review the patch.
>
> Regards
> Aisheng
>
> >
> > [ 2.034774] imx6q-pcie 5f000000.pcie: IO 0x6ff80000..0x6ff8ffff ->
> > 0x00000000
> > [ 2.034801] imx6q-pcie 5f000000.pcie: MEM 0x60000000..0x6fefffff
> ->
> > 0x60000000
> > [ 2.035072] sdhc1: failed to power up resource 249 ret -13
> > [ 2.035619] sdhc2: failed to power up resource 250 ret -13
> > [ 2.036126] enet0: failed to power up resource 251 ret -13
> > [ 2.036584] enet1: failed to power up resource 252 ret -13
> > [ 2.037040] mlb0: failed to power up resource 253 ret -13
> > [ 2.037495] nand: failed to power up resource 265 ret -13
> > [ 2.037951] nand: failed to power up resource 265 ret -13
> > [ 2.038416] pwm0: failed to power up resource 191 ret -13
> > [ 2.038868] pwm1: failed to power up resource 192 ret -13
> > [ 2.039320] pwm2: failed to power up resource 193 ret -13
> > [ 2.039786] pwm3: failed to power up resource 194 ret -13
> > [ 2.040239] pwm4: failed to power up resource 195 ret -13
> > [ 2.040692] pwm5: failed to power up resource 196 ret -13
> > [ 2.041142] pwm6: failed to power up resource 197 ret -13
> > [ 2.041596] pwm7: failed to power up resource 198 ret -13
> > [ 2.042073] amix: failed to power up resource 458 ret -13
> > [ 2.042558] lpspi0: failed to power up resource 53 ret -13
> > [ 2.043033] lpspi1: failed to power up resource 54 ret -13
> > [ 2.043501] lpspi2: failed to power up resource 55 ret -13
> > [ 2.043992] lpspi3: failed to power up resource 56 ret -13
> > [ 2.044460] lpuart0: failed to power up resource 57 ret -13
> > [ 2.044935] lpuart2: failed to power up resource 59 ret -13
> > [ 2.045409] lpuart3: failed to power up resource 60 ret -13
> > [ 2.055014] sim0: failed to power up resource 62 ret -13
> > [ 2.055510] adc0: failed to power up resource 101 ret -13
> > [ 2.056467] lpi2c0: failed to power up resource 96 ret -13
> > [ 2.056946] lpi2c1: failed to power up resource 97 ret -13
> > [ 2.057424] lpi2c2: failed to power up resource 98 ret -13
> > [ 2.057898] lpi2c3: failed to power up resource 99 ret -13
> > [ 2.058371] can0: failed to power up resource 105 ret -13
> > [ 2.059289] can1: failed to power up resource 106 ret -13
> > [ 2.059801] can2: failed to power up resource 107 ret -13
> > [ 2.060281] nand: failed to power up resource 265 ret -13
> > [ 2.062764] dpu-core 56180000.dpu: driver probed
> >
> > Thanks,
> > Peng.
> >
> > >
> > > Regards
> > > Aisheng
> > >
> > > > Thanks,
> > > > Peng.
> > > >
> > > >
> > > > >
> > > > > Regards
> > > > > Aisheng
> > > > >
> > > > > > Thanks,
> > > > > > Peng.
> > > > > >
> > > > > > >
> > > > > > > Regards
> > > > > > > Aisheng
> > > > > > >
> > > > > > > > ---
> > > > > > > > drivers/firmware/imx/Makefile | 2 +-
> > > > > > > > drivers/firmware/imx/rm.c | 40
> > > > > +++++++++++++++++++++
> > > > > > > > include/linux/firmware/imx/sci.h | 1 +
> > > > > > > > include/linux/firmware/imx/svc/rm.h | 69
> > > > > > > > +++++++++++++++++++++++++++++++++++++
> > > > > > > > 4 files changed, 111 insertions(+), 1 deletion(-) create
> > > > > > > > mode
> > > > > > > > 100644 drivers/firmware/imx/rm.c create mode 100644
> > > > > > > > include/linux/firmware/imx/svc/rm.h
> > > > > > > >
> > > > > > > > diff --git a/drivers/firmware/imx/Makefile
> > > > > > > > b/drivers/firmware/imx/Makefile index
> > > > > > > > 08bc9ddfbdfb..17ea3613e142
> > > > > > > > 100644
> > > > > > > > --- a/drivers/firmware/imx/Makefile
> > > > > > > > +++ b/drivers/firmware/imx/Makefile
> > > > > > > > @@ -1,4 +1,4 @@
> > > > > > > > # SPDX-License-Identifier: GPL-2.0
> > > > > > > > obj-$(CONFIG_IMX_DSP) += imx-dsp.o
> > > > > > > > -obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o
> > > > > > > > +obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o
> imx-scu-irq.o
> > > rm.o
> > > > > > > > obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> > > > > > > > diff --git a/drivers/firmware/imx/rm.c
> > > > > > > > b/drivers/firmware/imx/rm.c new file mode 100644 index
> > > > > > > > 000000000000..7b0334de5486
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/drivers/firmware/imx/rm.c
> > > > > > > > @@ -0,0 +1,40 @@
> > > > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > > > +/*
> > > > > > > > + * Copyright 2020 NXP
> > > > > > > > + *
> > > > > > > > + * File containing client-side RPC functions for the RM service.
> > > > > > > > +These
> > > > > > > > + * function are ported to clients that communicate to the SC.
> > > > > > > > + */
> > > > > > > > +
> > > > > > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > > > > > +
> > > > > > > > +struct imx_sc_msg_rm_rsrc_owned {
> > > > > > > > + struct imx_sc_rpc_msg hdr;
> > > > > > > > + u16 resource;
> > > > > > > > +} __packed __aligned(4);
> > > > > > > > +
> > > > > > > > +/*
> > > > > > > > + * This function check @resource is owned by current
> > > > > > > > +partition or not
> > > > > > > > + *
> > > > > > > > + * @param[in] ipc IPC handle
> > > > > > > > + * @param[in] resource resource the control is
> > > associated
> > > > > with
> > > > > > > > + *
> > > > > > > > + * @return Returns 0 for success and < 0 for errors.
> > > > > > > > + */
> > > > > > > > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc,
> > > > > > > > +u16
> > > > > > > > +resource) {
> > > > > > > > + struct imx_sc_msg_rm_rsrc_owned msg;
> > > > > > > > + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > > > > > > > +
> > > > > > > > + hdr->ver = IMX_SC_RPC_VERSION;
> > > > > > > > + hdr->svc = IMX_SC_RPC_SVC_RM;
> > > > > > > > + hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
> > > > > > > > + hdr->size = 2;
> > > > > > > > +
> > > > > > > > + msg.resource = resource;
> > > > > > > > +
> > > > > > > > + imx_scu_call_rpc(ipc, &msg, true);
> > > > > > > > +
> > > > > > > > + return hdr->func;
> > > > > > > > +}
> > > > > > > > +EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
> > > > > > > > diff --git a/include/linux/firmware/imx/sci.h
> > > > > > > > b/include/linux/firmware/imx/sci.h
> > > > > > > > index 17ba4e405129..b5c5a56f29be 100644
> > > > > > > > --- a/include/linux/firmware/imx/sci.h
> > > > > > > > +++ b/include/linux/firmware/imx/sci.h
> > > > > > > > @@ -15,6 +15,7 @@
> > > > > > > >
> > > > > > > > #include <linux/firmware/imx/svc/misc.h> #include
> > > > > > > > <linux/firmware/imx/svc/pm.h>
> > > > > > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > > > > >
> > > > > > > > int imx_scu_enable_general_irq_channel(struct device
> > > > > > > > *dev); int imx_scu_irq_register_notifier(struct
> > > > > > > > notifier_block *nb); diff --git
> > > > > > > > a/include/linux/firmware/imx/svc/rm.h
> > > > > > > > b/include/linux/firmware/imx/svc/rm.h
> > > > > > > > new file mode 100644
> > > > > > > > index 000000000000..fc6ea62d9d0e
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/include/linux/firmware/imx/svc/rm.h
> > > > > > > > @@ -0,0 +1,69 @@
> > > > > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > > > > +/*
> > > > > > > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > > > > > > + * Copyright 2017-2019 NXP
> > > > > > > > + *
> > > > > > > > + * Header file containing the public API for the System
> > > > > > > > +Controller
> > > > > > > > +(SC)
> > > > > > > > + * Power Management (PM) function. This includes
> > > > > > > > +functions for power state
> > > > > > > > + * control, clock control, reset control, and wake-up event
> control.
> > > > > > > > + *
> > > > > > > > + * RM_SVC (SVC) Resource Management Service
> > > > > > > > + *
> > > > > > > > + * Module for the Resource Management (RM) service.
> > > > > > > > + */
> > > > > > > > +
> > > > > > > > +#ifndef _SC_RM_API_H
> > > > > > > > +#define _SC_RM_API_H
> > > > > > > > +
> > > > > > > > +#include <linux/firmware/imx/sci.h>
> > > > > > > > +
> > > > > > > > +/*
> > > > > > > > + * This type is used to indicate RPC RM function calls.
> > > > > > > > + */
> > > > > > > > +enum imx_sc_rm_func {
> > > > > > > > + IMX_SC_RM_FUNC_UNKNOWN = 0,
> > > > > > > > + IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
> > > > > > > > + IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
> > > > > > > > + IMX_SC_RM_FUNC_PARTITION_FREE = 2,
> > > > > > > > + IMX_SC_RM_FUNC_GET_DID = 26,
> > > > > > > > + IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
> > > > > > > > + IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
> > > > > > > > + IMX_SC_RM_FUNC_GET_PARTITION = 5,
> > > > > > > > + IMX_SC_RM_FUNC_SET_PARENT = 6,
> > > > > > > > + IMX_SC_RM_FUNC_MOVE_ALL = 7,
> > > > > > > > + IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
> > > > > > > > + IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
> > > > > > > > + IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
> > > > > > > > + IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
> > > > > > > > + IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
> > > > > > > > + IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS = 12,
> > > > > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
> > > > > > > > + IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
> > > > > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
> > > > > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
> > > > > > > > + IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
> > > > > > > > + IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
> > > > > > > > + IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
> > > > > > > > + IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
> > > > > > > > + IMX_SC_RM_FUNC_MEMREG_FREE = 18,
> > > > > > > > + IMX_SC_RM_FUNC_FIND_MEMREG = 30,
> > > > > > > > + IMX_SC_RM_FUNC_ASSIGN_MEMREG = 19,
> > > > > > > > + IMX_SC_RM_FUNC_SET_MEMREG_PERMISSIONS = 20,
> > > > > > > > + IMX_SC_RM_FUNC_IS_MEMREG_OWNED = 21,
> > > > > > > > + IMX_SC_RM_FUNC_GET_MEMREG_INFO = 22,
> > > > > > > > + IMX_SC_RM_FUNC_ASSIGN_PAD = 23,
> > > > > > > > + IMX_SC_RM_FUNC_SET_PAD_MOVABLE = 24,
> > > > > > > > + IMX_SC_RM_FUNC_IS_PAD_OWNED = 25,
> > > > > > > > + IMX_SC_RM_FUNC_DUMP = 27, };
> > > > > > > > +
> > > > > > > > +#if IS_ENABLED(CONFIG_IMX_SCU) bool
> > > > > > > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > > > > > > +resource); #else static inline bool
> > > > > > > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > resource) {
> > > > > > > > + return true;
> > > > > > > > +}
> > > > > > > > +#endif
> > > > > > > > +#endif
> > > > > > > > --
> > > > > > > > 2.16.4
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 2/4] firmware: imx: add resource management api
2020-06-02 4:50 ` Peng Fan
@ 2020-06-02 7:28 ` Aisheng Dong
2020-06-02 7:47 ` Peng Fan
0 siblings, 1 reply; 22+ messages in thread
From: Aisheng Dong @ 2020-06-02 7:28 UTC (permalink / raw)
To: Peng Fan, shawnguo, s.hauer
Cc: kernel, festevam, linux, Leonard Crestez, Daniel Baluta,
Joakim Zhang, linux-kernel, linux-arm-kernel, dl-linux-imx
> From: Peng Fan <peng.fan@nxp.com>
> Sent: Tuesday, June 2, 2020 12:51 PM
> >
> > > From: Peng Fan <peng.fan@nxp.com>
> > > Sent: Monday, June 1, 2020 8:40 PM
> > > >
> > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > Sent: Friday, April 24, 2020 9:12 AM
> > > > > >
> > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > Sent: Thursday, April 23, 2020 6:57 PM
> > > > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > > > Sent: Thursday, April 23, 2020 3:00 PM
> > > > > > > > >
> > > > > > > > > Add resource management API, when we have multiple
> > > > > > > > > partition running together, resources not owned to
> > > > > > > > > current partition should not be
> > > > > > used.
> > > > > > > > >
> > > > > > > > > Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> > > > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > > > >
> > > > > > > > Right now I'm still not quite understand if we really need this.
> > > > > > > > As current resource is bound to power domains, if it's not
> > > > > > > > owned by one specific SW execution environment, power on
> > > > > > > > will also
> > > fail.
> > > > > > > > Can you clarify if any exceptions?
> > > > > > >
> > > > > > > There will be lots of failures when boot Linux domu if no check.
> > > > > > >
> > > > > >
> > > > > > What kind of features did you mean?
> > > > > > Could you clarify a bit more? I'd like to understand this issue better.
> > > > >
> > > > > When supporting hypervisor with dual Linux running, 1st Linux
> > > > > and the 2nd Linux will have their own dedicated resources.
> > > > >
> > > > > If no resource check, that means 1st/2nd Linux will register all
> > > > > the resources, then both will see fail logs when register
> > > > > resources not owned to
> > > > itself.
> > > > >
> > > > > Same to partitioned m4 case.
> > > > >
> > > > > Would it be better without the failure log?
> > > > >
> > > >
> > > > Is it power up failure?
> > > > If yes, it's expected because we actually use power up to check if
> > > > resources are owned by this partition. It functions the same as
> > > > calling resource check API.
> > > > That's current design.
> > > >
> > > > Or it's other failures? Log?
> > > Sorry for long late reply.
> > >
> > > Part of my XEN DomU log, there are lots of failure log. I think
> > > better not have such logs by add a resource own check.
> >
> > Those error logs are intended.
> > Resource owner check actually behaves the same as power up.
>
> If resource is not owned, it will not register the power domain.
>
> Without resource own check, it will continue register the power domain and
> hook it into genpd list.
>
That's the intended behavior to save the resource owner checking as it's very time cost
to send SCU cmd for each domain during booting which benefits nothing except not exposing
them in genpd list.
> I prefer we not expose the power domain not owned to current partition and
> keep the err msg for people to easy see where it goes wrong.
If we really want to to do, I wonder probably another better approach is trying to re-use the partition
Information built by bootloader as uboot already did that one time, not necessary to re-do
It again for kernel as it's time cost.
e.g. introduce a resource partition property in dt and initialized and passed by bootloarder
to kernel to use later.
Then we can still save those huge number of resource owner check CMDs.
Regards
Aisheng
>
> Regards,
> Peng.
> > So not quite necessary to add a double check.
> > If we're concerning about the error log, we can change it to dev_dbg().
> >
> > BTW, as resource management will be needed by seco drivers later, So I
> > will continue to review the patch.
> >
> > Regards
> > Aisheng
> >
> > >
> > > [ 2.034774] imx6q-pcie 5f000000.pcie: IO 0x6ff80000..0x6ff8ffff
> ->
> > > 0x00000000
> > > [ 2.034801] imx6q-pcie 5f000000.pcie: MEM
> 0x60000000..0x6fefffff
> > ->
> > > 0x60000000
> > > [ 2.035072] sdhc1: failed to power up resource 249 ret -13
> > > [ 2.035619] sdhc2: failed to power up resource 250 ret -13
> > > [ 2.036126] enet0: failed to power up resource 251 ret -13
> > > [ 2.036584] enet1: failed to power up resource 252 ret -13
> > > [ 2.037040] mlb0: failed to power up resource 253 ret -13
> > > [ 2.037495] nand: failed to power up resource 265 ret -13
> > > [ 2.037951] nand: failed to power up resource 265 ret -13
> > > [ 2.038416] pwm0: failed to power up resource 191 ret -13
> > > [ 2.038868] pwm1: failed to power up resource 192 ret -13
> > > [ 2.039320] pwm2: failed to power up resource 193 ret -13
> > > [ 2.039786] pwm3: failed to power up resource 194 ret -13
> > > [ 2.040239] pwm4: failed to power up resource 195 ret -13
> > > [ 2.040692] pwm5: failed to power up resource 196 ret -13
> > > [ 2.041142] pwm6: failed to power up resource 197 ret -13
> > > [ 2.041596] pwm7: failed to power up resource 198 ret -13
> > > [ 2.042073] amix: failed to power up resource 458 ret -13
> > > [ 2.042558] lpspi0: failed to power up resource 53 ret -13
> > > [ 2.043033] lpspi1: failed to power up resource 54 ret -13
> > > [ 2.043501] lpspi2: failed to power up resource 55 ret -13
> > > [ 2.043992] lpspi3: failed to power up resource 56 ret -13
> > > [ 2.044460] lpuart0: failed to power up resource 57 ret -13
> > > [ 2.044935] lpuart2: failed to power up resource 59 ret -13
> > > [ 2.045409] lpuart3: failed to power up resource 60 ret -13
> > > [ 2.055014] sim0: failed to power up resource 62 ret -13
> > > [ 2.055510] adc0: failed to power up resource 101 ret -13
> > > [ 2.056467] lpi2c0: failed to power up resource 96 ret -13
> > > [ 2.056946] lpi2c1: failed to power up resource 97 ret -13
> > > [ 2.057424] lpi2c2: failed to power up resource 98 ret -13
> > > [ 2.057898] lpi2c3: failed to power up resource 99 ret -13
> > > [ 2.058371] can0: failed to power up resource 105 ret -13
> > > [ 2.059289] can1: failed to power up resource 106 ret -13
> > > [ 2.059801] can2: failed to power up resource 107 ret -13
> > > [ 2.060281] nand: failed to power up resource 265 ret -13
> > > [ 2.062764] dpu-core 56180000.dpu: driver probed
> > >
> > > Thanks,
> > > Peng.
> > >
> > > >
> > > > Regards
> > > > Aisheng
> > > >
> > > > > Thanks,
> > > > > Peng.
> > > > >
> > > > >
> > > > > >
> > > > > > Regards
> > > > > > Aisheng
> > > > > >
> > > > > > > Thanks,
> > > > > > > Peng.
> > > > > > >
> > > > > > > >
> > > > > > > > Regards
> > > > > > > > Aisheng
> > > > > > > >
> > > > > > > > > ---
> > > > > > > > > drivers/firmware/imx/Makefile | 2 +-
> > > > > > > > > drivers/firmware/imx/rm.c | 40
> > > > > > +++++++++++++++++++++
> > > > > > > > > include/linux/firmware/imx/sci.h | 1 +
> > > > > > > > > include/linux/firmware/imx/svc/rm.h | 69
> > > > > > > > > +++++++++++++++++++++++++++++++++++++
> > > > > > > > > 4 files changed, 111 insertions(+), 1 deletion(-)
> > > > > > > > > create mode
> > > > > > > > > 100644 drivers/firmware/imx/rm.c create mode 100644
> > > > > > > > > include/linux/firmware/imx/svc/rm.h
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/firmware/imx/Makefile
> > > > > > > > > b/drivers/firmware/imx/Makefile index
> > > > > > > > > 08bc9ddfbdfb..17ea3613e142
> > > > > > > > > 100644
> > > > > > > > > --- a/drivers/firmware/imx/Makefile
> > > > > > > > > +++ b/drivers/firmware/imx/Makefile
> > > > > > > > > @@ -1,4 +1,4 @@
> > > > > > > > > # SPDX-License-Identifier: GPL-2.0
> > > > > > > > > obj-$(CONFIG_IMX_DSP) += imx-dsp.o
> > > > > > > > > -obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o
> imx-scu-irq.o
> > > > > > > > > +obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o
> > imx-scu-irq.o
> > > > rm.o
> > > > > > > > > obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> > > > > > > > > diff --git a/drivers/firmware/imx/rm.c
> > > > > > > > > b/drivers/firmware/imx/rm.c new file mode 100644 index
> > > > > > > > > 000000000000..7b0334de5486
> > > > > > > > > --- /dev/null
> > > > > > > > > +++ b/drivers/firmware/imx/rm.c
> > > > > > > > > @@ -0,0 +1,40 @@
> > > > > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > > > > +/*
> > > > > > > > > + * Copyright 2020 NXP
> > > > > > > > > + *
> > > > > > > > > + * File containing client-side RPC functions for the RM service.
> > > > > > > > > +These
> > > > > > > > > + * function are ported to clients that communicate to the SC.
> > > > > > > > > + */
> > > > > > > > > +
> > > > > > > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > > > > > > +
> > > > > > > > > +struct imx_sc_msg_rm_rsrc_owned {
> > > > > > > > > + struct imx_sc_rpc_msg hdr;
> > > > > > > > > + u16 resource;
> > > > > > > > > +} __packed __aligned(4);
> > > > > > > > > +
> > > > > > > > > +/*
> > > > > > > > > + * This function check @resource is owned by current
> > > > > > > > > +partition or not
> > > > > > > > > + *
> > > > > > > > > + * @param[in] ipc IPC handle
> > > > > > > > > + * @param[in] resource resource the control is
> > > > associated
> > > > > > with
> > > > > > > > > + *
> > > > > > > > > + * @return Returns 0 for success and < 0 for errors.
> > > > > > > > > + */
> > > > > > > > > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc
> > > > > > > > > +*ipc,
> > > > > > > > > +u16
> > > > > > > > > +resource) {
> > > > > > > > > + struct imx_sc_msg_rm_rsrc_owned msg;
> > > > > > > > > + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > > > > > > > > +
> > > > > > > > > + hdr->ver = IMX_SC_RPC_VERSION;
> > > > > > > > > + hdr->svc = IMX_SC_RPC_SVC_RM;
> > > > > > > > > + hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
> > > > > > > > > + hdr->size = 2;
> > > > > > > > > +
> > > > > > > > > + msg.resource = resource;
> > > > > > > > > +
> > > > > > > > > + imx_scu_call_rpc(ipc, &msg, true);
> > > > > > > > > +
> > > > > > > > > + return hdr->func;
> > > > > > > > > +}
> > > > > > > > > +EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
> > > > > > > > > diff --git a/include/linux/firmware/imx/sci.h
> > > > > > > > > b/include/linux/firmware/imx/sci.h
> > > > > > > > > index 17ba4e405129..b5c5a56f29be 100644
> > > > > > > > > --- a/include/linux/firmware/imx/sci.h
> > > > > > > > > +++ b/include/linux/firmware/imx/sci.h
> > > > > > > > > @@ -15,6 +15,7 @@
> > > > > > > > >
> > > > > > > > > #include <linux/firmware/imx/svc/misc.h> #include
> > > > > > > > > <linux/firmware/imx/svc/pm.h>
> > > > > > > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > > > > > >
> > > > > > > > > int imx_scu_enable_general_irq_channel(struct device
> > > > > > > > > *dev); int imx_scu_irq_register_notifier(struct
> > > > > > > > > notifier_block *nb); diff --git
> > > > > > > > > a/include/linux/firmware/imx/svc/rm.h
> > > > > > > > > b/include/linux/firmware/imx/svc/rm.h
> > > > > > > > > new file mode 100644
> > > > > > > > > index 000000000000..fc6ea62d9d0e
> > > > > > > > > --- /dev/null
> > > > > > > > > +++ b/include/linux/firmware/imx/svc/rm.h
> > > > > > > > > @@ -0,0 +1,69 @@
> > > > > > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > > > > > +/*
> > > > > > > > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > > > > > > > + * Copyright 2017-2019 NXP
> > > > > > > > > + *
> > > > > > > > > + * Header file containing the public API for the System
> > > > > > > > > +Controller
> > > > > > > > > +(SC)
> > > > > > > > > + * Power Management (PM) function. This includes
> > > > > > > > > +functions for power state
> > > > > > > > > + * control, clock control, reset control, and wake-up
> > > > > > > > > +event
> > control.
> > > > > > > > > + *
> > > > > > > > > + * RM_SVC (SVC) Resource Management Service
> > > > > > > > > + *
> > > > > > > > > + * Module for the Resource Management (RM) service.
> > > > > > > > > + */
> > > > > > > > > +
> > > > > > > > > +#ifndef _SC_RM_API_H
> > > > > > > > > +#define _SC_RM_API_H
> > > > > > > > > +
> > > > > > > > > +#include <linux/firmware/imx/sci.h>
> > > > > > > > > +
> > > > > > > > > +/*
> > > > > > > > > + * This type is used to indicate RPC RM function calls.
> > > > > > > > > + */
> > > > > > > > > +enum imx_sc_rm_func {
> > > > > > > > > + IMX_SC_RM_FUNC_UNKNOWN = 0,
> > > > > > > > > + IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
> > > > > > > > > + IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
> > > > > > > > > + IMX_SC_RM_FUNC_PARTITION_FREE = 2,
> > > > > > > > > + IMX_SC_RM_FUNC_GET_DID = 26,
> > > > > > > > > + IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
> > > > > > > > > + IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
> > > > > > > > > + IMX_SC_RM_FUNC_GET_PARTITION = 5,
> > > > > > > > > + IMX_SC_RM_FUNC_SET_PARENT = 6,
> > > > > > > > > + IMX_SC_RM_FUNC_MOVE_ALL = 7,
> > > > > > > > > + IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
> > > > > > > > > + IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
> > > > > > > > > + IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
> > > > > > > > > + IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
> > > > > > > > > + IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
> > > > > > > > > + IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS = 12,
> > > > > > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
> > > > > > > > > + IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
> > > > > > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
> > > > > > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
> > > > > > > > > + IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
> > > > > > > > > + IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
> > > > > > > > > + IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
> > > > > > > > > + IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
> > > > > > > > > + IMX_SC_RM_FUNC_MEMREG_FREE = 18,
> > > > > > > > > + IMX_SC_RM_FUNC_FIND_MEMREG = 30,
> > > > > > > > > + IMX_SC_RM_FUNC_ASSIGN_MEMREG = 19,
> > > > > > > > > + IMX_SC_RM_FUNC_SET_MEMREG_PERMISSIONS = 20,
> > > > > > > > > + IMX_SC_RM_FUNC_IS_MEMREG_OWNED = 21,
> > > > > > > > > + IMX_SC_RM_FUNC_GET_MEMREG_INFO = 22,
> > > > > > > > > + IMX_SC_RM_FUNC_ASSIGN_PAD = 23,
> > > > > > > > > + IMX_SC_RM_FUNC_SET_PAD_MOVABLE = 24,
> > > > > > > > > + IMX_SC_RM_FUNC_IS_PAD_OWNED = 25,
> > > > > > > > > + IMX_SC_RM_FUNC_DUMP = 27, };
> > > > > > > > > +
> > > > > > > > > +#if IS_ENABLED(CONFIG_IMX_SCU) bool
> > > > > > > > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > > > > > > > +resource); #else static inline bool
> > > > > > > > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > > resource) {
> > > > > > > > > + return true;
> > > > > > > > > +}
> > > > > > > > > +#endif
> > > > > > > > > +#endif
> > > > > > > > > --
> > > > > > > > > 2.16.4
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 2/4] firmware: imx: add resource management api
2020-06-02 7:28 ` Aisheng Dong
@ 2020-06-02 7:47 ` Peng Fan
2020-06-02 9:07 ` Aisheng Dong
0 siblings, 1 reply; 22+ messages in thread
From: Peng Fan @ 2020-06-02 7:47 UTC (permalink / raw)
To: Aisheng Dong, shawnguo, s.hauer
Cc: kernel, festevam, linux, Leonard Crestez, Daniel Baluta,
Joakim Zhang, linux-kernel, linux-arm-kernel, dl-linux-imx
> Subject: RE: [PATCH 2/4] firmware: imx: add resource management api
>
> > From: Peng Fan <peng.fan@nxp.com>
> > Sent: Tuesday, June 2, 2020 12:51 PM
> > >
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > > Sent: Monday, June 1, 2020 8:40 PM
> > > > >
> > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > Sent: Friday, April 24, 2020 9:12 AM
> > > > > > >
> > > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > > Sent: Thursday, April 23, 2020 6:57 PM
> > > > > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > > > > Sent: Thursday, April 23, 2020 3:00 PM
> > > > > > > > > >
> > > > > > > > > > Add resource management API, when we have multiple
> > > > > > > > > > partition running together, resources not owned to
> > > > > > > > > > current partition should not be
> > > > > > > used.
> > > > > > > > > >
> > > > > > > > > > Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> > > > > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > > > > >
> > > > > > > > > Right now I'm still not quite understand if we really need this.
> > > > > > > > > As current resource is bound to power domains, if it's
> > > > > > > > > not owned by one specific SW execution environment,
> > > > > > > > > power on will also
> > > > fail.
> > > > > > > > > Can you clarify if any exceptions?
> > > > > > > >
> > > > > > > > There will be lots of failures when boot Linux domu if no check.
> > > > > > > >
> > > > > > >
> > > > > > > What kind of features did you mean?
> > > > > > > Could you clarify a bit more? I'd like to understand this issue
> better.
> > > > > >
> > > > > > When supporting hypervisor with dual Linux running, 1st Linux
> > > > > > and the 2nd Linux will have their own dedicated resources.
> > > > > >
> > > > > > If no resource check, that means 1st/2nd Linux will register
> > > > > > all the resources, then both will see fail logs when register
> > > > > > resources not owned to
> > > > > itself.
> > > > > >
> > > > > > Same to partitioned m4 case.
> > > > > >
> > > > > > Would it be better without the failure log?
> > > > > >
> > > > >
> > > > > Is it power up failure?
> > > > > If yes, it's expected because we actually use power up to check
> > > > > if resources are owned by this partition. It functions the same
> > > > > as calling resource check API.
> > > > > That's current design.
> > > > >
> > > > > Or it's other failures? Log?
> > > > Sorry for long late reply.
> > > >
> > > > Part of my XEN DomU log, there are lots of failure log. I think
> > > > better not have such logs by add a resource own check.
> > >
> > > Those error logs are intended.
> > > Resource owner check actually behaves the same as power up.
> >
> > If resource is not owned, it will not register the power domain.
> >
> > Without resource own check, it will continue register the power domain
> > and hook it into genpd list.
> >
>
> That's the intended behavior to save the resource owner checking as it's very
> time cost to send SCU cmd for each domain during booting which benefits
> nothing except not exposing them in genpd list.
>
> > I prefer we not expose the power domain not owned to current partition
> > and keep the err msg for people to easy see where it goes wrong.
>
> If we really want to to do, I wonder probably another better approach is
> trying to re-use the partition Information built by bootloader as uboot already
> did that one time, not necessary to re-do It again for kernel as it's time cost.
> e.g. introduce a resource partition property in dt and initialized and passed by
> bootloarder to kernel to use later.
This will not work for hypervisor based VM runtime partition create and resource
assignment.
> Then we can still save those huge number of resource owner check CMDs.
So we have to live with them even I only need one SDHC for a VM? With
so many clk-scu entries and power domain entries there.
Thanks,
Peng.
>
> Regards
> Aisheng
>
> >
> > Regards,
> > Peng.
> > > So not quite necessary to add a double check.
> > > If we're concerning about the error log, we can change it to dev_dbg().
> > >
> > > BTW, as resource management will be needed by seco drivers later, So
> > > I will continue to review the patch.
> > >
> > > Regards
> > > Aisheng
> > >
> > > >
> > > > [ 2.034774] imx6q-pcie 5f000000.pcie: IO
> 0x6ff80000..0x6ff8ffff
> > ->
> > > > 0x00000000
> > > > [ 2.034801] imx6q-pcie 5f000000.pcie: MEM
> > 0x60000000..0x6fefffff
> > > ->
> > > > 0x60000000
> > > > [ 2.035072] sdhc1: failed to power up resource 249 ret -13
> > > > [ 2.035619] sdhc2: failed to power up resource 250 ret -13
> > > > [ 2.036126] enet0: failed to power up resource 251 ret -13
> > > > [ 2.036584] enet1: failed to power up resource 252 ret -13
> > > > [ 2.037040] mlb0: failed to power up resource 253 ret -13
> > > > [ 2.037495] nand: failed to power up resource 265 ret -13
> > > > [ 2.037951] nand: failed to power up resource 265 ret -13
> > > > [ 2.038416] pwm0: failed to power up resource 191 ret -13
> > > > [ 2.038868] pwm1: failed to power up resource 192 ret -13
> > > > [ 2.039320] pwm2: failed to power up resource 193 ret -13
> > > > [ 2.039786] pwm3: failed to power up resource 194 ret -13
> > > > [ 2.040239] pwm4: failed to power up resource 195 ret -13
> > > > [ 2.040692] pwm5: failed to power up resource 196 ret -13
> > > > [ 2.041142] pwm6: failed to power up resource 197 ret -13
> > > > [ 2.041596] pwm7: failed to power up resource 198 ret -13
> > > > [ 2.042073] amix: failed to power up resource 458 ret -13
> > > > [ 2.042558] lpspi0: failed to power up resource 53 ret -13
> > > > [ 2.043033] lpspi1: failed to power up resource 54 ret -13
> > > > [ 2.043501] lpspi2: failed to power up resource 55 ret -13
> > > > [ 2.043992] lpspi3: failed to power up resource 56 ret -13
> > > > [ 2.044460] lpuart0: failed to power up resource 57 ret -13
> > > > [ 2.044935] lpuart2: failed to power up resource 59 ret -13
> > > > [ 2.045409] lpuart3: failed to power up resource 60 ret -13
> > > > [ 2.055014] sim0: failed to power up resource 62 ret -13
> > > > [ 2.055510] adc0: failed to power up resource 101 ret -13
> > > > [ 2.056467] lpi2c0: failed to power up resource 96 ret -13
> > > > [ 2.056946] lpi2c1: failed to power up resource 97 ret -13
> > > > [ 2.057424] lpi2c2: failed to power up resource 98 ret -13
> > > > [ 2.057898] lpi2c3: failed to power up resource 99 ret -13
> > > > [ 2.058371] can0: failed to power up resource 105 ret -13
> > > > [ 2.059289] can1: failed to power up resource 106 ret -13
> > > > [ 2.059801] can2: failed to power up resource 107 ret -13
> > > > [ 2.060281] nand: failed to power up resource 265 ret -13
> > > > [ 2.062764] dpu-core 56180000.dpu: driver probed
> > > >
> > > > Thanks,
> > > > Peng.
> > > >
> > > > >
> > > > > Regards
> > > > > Aisheng
> > > > >
> > > > > > Thanks,
> > > > > > Peng.
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > Regards
> > > > > > > Aisheng
> > > > > > >
> > > > > > > > Thanks,
> > > > > > > > Peng.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > Regards
> > > > > > > > > Aisheng
> > > > > > > > >
> > > > > > > > > > ---
> > > > > > > > > > drivers/firmware/imx/Makefile | 2 +-
> > > > > > > > > > drivers/firmware/imx/rm.c | 40
> > > > > > > +++++++++++++++++++++
> > > > > > > > > > include/linux/firmware/imx/sci.h | 1 +
> > > > > > > > > > include/linux/firmware/imx/svc/rm.h | 69
> > > > > > > > > > +++++++++++++++++++++++++++++++++++++
> > > > > > > > > > 4 files changed, 111 insertions(+), 1 deletion(-)
> > > > > > > > > > create mode
> > > > > > > > > > 100644 drivers/firmware/imx/rm.c create mode 100644
> > > > > > > > > > include/linux/firmware/imx/svc/rm.h
> > > > > > > > > >
> > > > > > > > > > diff --git a/drivers/firmware/imx/Makefile
> > > > > > > > > > b/drivers/firmware/imx/Makefile index
> > > > > > > > > > 08bc9ddfbdfb..17ea3613e142
> > > > > > > > > > 100644
> > > > > > > > > > --- a/drivers/firmware/imx/Makefile
> > > > > > > > > > +++ b/drivers/firmware/imx/Makefile
> > > > > > > > > > @@ -1,4 +1,4 @@
> > > > > > > > > > # SPDX-License-Identifier: GPL-2.0
> > > > > > > > > > obj-$(CONFIG_IMX_DSP) += imx-dsp.o
> > > > > > > > > > -obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o
> > imx-scu-irq.o
> > > > > > > > > > +obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o
> > > imx-scu-irq.o
> > > > > rm.o
> > > > > > > > > > obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> > > > > > > > > > diff --git a/drivers/firmware/imx/rm.c
> > > > > > > > > > b/drivers/firmware/imx/rm.c new file mode 100644 index
> > > > > > > > > > 000000000000..7b0334de5486
> > > > > > > > > > --- /dev/null
> > > > > > > > > > +++ b/drivers/firmware/imx/rm.c
> > > > > > > > > > @@ -0,0 +1,40 @@
> > > > > > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > > > > > +/*
> > > > > > > > > > + * Copyright 2020 NXP
> > > > > > > > > > + *
> > > > > > > > > > + * File containing client-side RPC functions for the RM
> service.
> > > > > > > > > > +These
> > > > > > > > > > + * function are ported to clients that communicate to the SC.
> > > > > > > > > > + */
> > > > > > > > > > +
> > > > > > > > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > > > > > > > +
> > > > > > > > > > +struct imx_sc_msg_rm_rsrc_owned {
> > > > > > > > > > + struct imx_sc_rpc_msg hdr;
> > > > > > > > > > + u16 resource;
> > > > > > > > > > +} __packed __aligned(4);
> > > > > > > > > > +
> > > > > > > > > > +/*
> > > > > > > > > > + * This function check @resource is owned by current
> > > > > > > > > > +partition or not
> > > > > > > > > > + *
> > > > > > > > > > + * @param[in] ipc IPC handle
> > > > > > > > > > + * @param[in] resource resource the control is
> > > > > associated
> > > > > > > with
> > > > > > > > > > + *
> > > > > > > > > > + * @return Returns 0 for success and < 0 for errors.
> > > > > > > > > > + */
> > > > > > > > > > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc
> > > > > > > > > > +*ipc,
> > > > > > > > > > +u16
> > > > > > > > > > +resource) {
> > > > > > > > > > + struct imx_sc_msg_rm_rsrc_owned msg;
> > > > > > > > > > + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > > > > > > > > > +
> > > > > > > > > > + hdr->ver = IMX_SC_RPC_VERSION;
> > > > > > > > > > + hdr->svc = IMX_SC_RPC_SVC_RM;
> > > > > > > > > > + hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
> > > > > > > > > > + hdr->size = 2;
> > > > > > > > > > +
> > > > > > > > > > + msg.resource = resource;
> > > > > > > > > > +
> > > > > > > > > > + imx_scu_call_rpc(ipc, &msg, true);
> > > > > > > > > > +
> > > > > > > > > > + return hdr->func;
> > > > > > > > > > +}
> > > > > > > > > > +EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
> > > > > > > > > > diff --git a/include/linux/firmware/imx/sci.h
> > > > > > > > > > b/include/linux/firmware/imx/sci.h
> > > > > > > > > > index 17ba4e405129..b5c5a56f29be 100644
> > > > > > > > > > --- a/include/linux/firmware/imx/sci.h
> > > > > > > > > > +++ b/include/linux/firmware/imx/sci.h
> > > > > > > > > > @@ -15,6 +15,7 @@
> > > > > > > > > >
> > > > > > > > > > #include <linux/firmware/imx/svc/misc.h> #include
> > > > > > > > > > <linux/firmware/imx/svc/pm.h>
> > > > > > > > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > > > > > > >
> > > > > > > > > > int imx_scu_enable_general_irq_channel(struct device
> > > > > > > > > > *dev); int imx_scu_irq_register_notifier(struct
> > > > > > > > > > notifier_block *nb); diff --git
> > > > > > > > > > a/include/linux/firmware/imx/svc/rm.h
> > > > > > > > > > b/include/linux/firmware/imx/svc/rm.h
> > > > > > > > > > new file mode 100644
> > > > > > > > > > index 000000000000..fc6ea62d9d0e
> > > > > > > > > > --- /dev/null
> > > > > > > > > > +++ b/include/linux/firmware/imx/svc/rm.h
> > > > > > > > > > @@ -0,0 +1,69 @@
> > > > > > > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > > > > > > +/*
> > > > > > > > > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > > > > > > > > + * Copyright 2017-2019 NXP
> > > > > > > > > > + *
> > > > > > > > > > + * Header file containing the public API for the
> > > > > > > > > > +System Controller
> > > > > > > > > > +(SC)
> > > > > > > > > > + * Power Management (PM) function. This includes
> > > > > > > > > > +functions for power state
> > > > > > > > > > + * control, clock control, reset control, and wake-up
> > > > > > > > > > +event
> > > control.
> > > > > > > > > > + *
> > > > > > > > > > + * RM_SVC (SVC) Resource Management Service
> > > > > > > > > > + *
> > > > > > > > > > + * Module for the Resource Management (RM) service.
> > > > > > > > > > + */
> > > > > > > > > > +
> > > > > > > > > > +#ifndef _SC_RM_API_H
> > > > > > > > > > +#define _SC_RM_API_H
> > > > > > > > > > +
> > > > > > > > > > +#include <linux/firmware/imx/sci.h>
> > > > > > > > > > +
> > > > > > > > > > +/*
> > > > > > > > > > + * This type is used to indicate RPC RM function calls.
> > > > > > > > > > + */
> > > > > > > > > > +enum imx_sc_rm_func {
> > > > > > > > > > + IMX_SC_RM_FUNC_UNKNOWN = 0,
> > > > > > > > > > + IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
> > > > > > > > > > + IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
> > > > > > > > > > + IMX_SC_RM_FUNC_PARTITION_FREE = 2,
> > > > > > > > > > + IMX_SC_RM_FUNC_GET_DID = 26,
> > > > > > > > > > + IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
> > > > > > > > > > + IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
> > > > > > > > > > + IMX_SC_RM_FUNC_GET_PARTITION = 5,
> > > > > > > > > > + IMX_SC_RM_FUNC_SET_PARENT = 6,
> > > > > > > > > > + IMX_SC_RM_FUNC_MOVE_ALL = 7,
> > > > > > > > > > + IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
> > > > > > > > > > + IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
> > > > > > > > > > + IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
> > > > > > > > > > + IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
> > > > > > > > > > + IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
> > > > > > > > > > + IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS =
> 12,
> > > > > > > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
> > > > > > > > > > + IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
> > > > > > > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
> > > > > > > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
> > > > > > > > > > + IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
> > > > > > > > > > + IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
> > > > > > > > > > + IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
> > > > > > > > > > + IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
> > > > > > > > > > + IMX_SC_RM_FUNC_MEMREG_FREE = 18,
> > > > > > > > > > + IMX_SC_RM_FUNC_FIND_MEMREG = 30,
> > > > > > > > > > + IMX_SC_RM_FUNC_ASSIGN_MEMREG = 19,
> > > > > > > > > > + IMX_SC_RM_FUNC_SET_MEMREG_PERMISSIONS = 20,
> > > > > > > > > > + IMX_SC_RM_FUNC_IS_MEMREG_OWNED = 21,
> > > > > > > > > > + IMX_SC_RM_FUNC_GET_MEMREG_INFO = 22,
> > > > > > > > > > + IMX_SC_RM_FUNC_ASSIGN_PAD = 23,
> > > > > > > > > > + IMX_SC_RM_FUNC_SET_PAD_MOVABLE = 24,
> > > > > > > > > > + IMX_SC_RM_FUNC_IS_PAD_OWNED = 25,
> > > > > > > > > > + IMX_SC_RM_FUNC_DUMP = 27, };
> > > > > > > > > > +
> > > > > > > > > > +#if IS_ENABLED(CONFIG_IMX_SCU) bool
> > > > > > > > > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc,
> > > > > > > > > > +u16 resource); #else static inline bool
> > > > > > > > > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc,
> > > > > > > > > > +u16
> > > > > resource) {
> > > > > > > > > > + return true;
> > > > > > > > > > +}
> > > > > > > > > > +#endif
> > > > > > > > > > +#endif
> > > > > > > > > > --
> > > > > > > > > > 2.16.4
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 2/4] firmware: imx: add resource management api
2020-06-02 7:47 ` Peng Fan
@ 2020-06-02 9:07 ` Aisheng Dong
0 siblings, 0 replies; 22+ messages in thread
From: Aisheng Dong @ 2020-06-02 9:07 UTC (permalink / raw)
To: Peng Fan, shawnguo, s.hauer
Cc: kernel, festevam, linux, Leonard Crestez, Daniel Baluta,
Joakim Zhang, linux-kernel, linux-arm-kernel, dl-linux-imx
> From: Peng Fan <peng.fan@nxp.com>
> Sent: Tuesday, June 2, 2020 3:48 PM
> >
> > > From: Peng Fan <peng.fan@nxp.com>
> > > Sent: Tuesday, June 2, 2020 12:51 PM
> > > >
> > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > Sent: Monday, June 1, 2020 8:40 PM
> > > > > >
> > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > Sent: Friday, April 24, 2020 9:12 AM
> > > > > > > >
> > > > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > > > Sent: Thursday, April 23, 2020 6:57 PM
> > > > > > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > > > > > Sent: Thursday, April 23, 2020 3:00 PM
> > > > > > > > > > >
> > > > > > > > > > > Add resource management API, when we have multiple
> > > > > > > > > > > partition running together, resources not owned to
> > > > > > > > > > > current partition should not be
> > > > > > > > used.
> > > > > > > > > > >
> > > > > > > > > > > Reviewed-by: Leonard Crestez
> > > > > > > > > > > <leonard.crestez@nxp.com>
> > > > > > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > > > > > >
> > > > > > > > > > Right now I'm still not quite understand if we really need this.
> > > > > > > > > > As current resource is bound to power domains, if it's
> > > > > > > > > > not owned by one specific SW execution environment,
> > > > > > > > > > power on will also
> > > > > fail.
> > > > > > > > > > Can you clarify if any exceptions?
> > > > > > > > >
> > > > > > > > > There will be lots of failures when boot Linux domu if no check.
> > > > > > > > >
> > > > > > > >
> > > > > > > > What kind of features did you mean?
> > > > > > > > Could you clarify a bit more? I'd like to understand this
> > > > > > > > issue
> > better.
> > > > > > >
> > > > > > > When supporting hypervisor with dual Linux running, 1st
> > > > > > > Linux and the 2nd Linux will have their own dedicated resources.
> > > > > > >
> > > > > > > If no resource check, that means 1st/2nd Linux will register
> > > > > > > all the resources, then both will see fail logs when
> > > > > > > register resources not owned to
> > > > > > itself.
> > > > > > >
> > > > > > > Same to partitioned m4 case.
> > > > > > >
> > > > > > > Would it be better without the failure log?
> > > > > > >
> > > > > >
> > > > > > Is it power up failure?
> > > > > > If yes, it's expected because we actually use power up to
> > > > > > check if resources are owned by this partition. It functions
> > > > > > the same as calling resource check API.
> > > > > > That's current design.
> > > > > >
> > > > > > Or it's other failures? Log?
> > > > > Sorry for long late reply.
> > > > >
> > > > > Part of my XEN DomU log, there are lots of failure log. I think
> > > > > better not have such logs by add a resource own check.
> > > >
> > > > Those error logs are intended.
> > > > Resource owner check actually behaves the same as power up.
> > >
> > > If resource is not owned, it will not register the power domain.
> > >
> > > Without resource own check, it will continue register the power
> > > domain and hook it into genpd list.
> > >
> >
> > That's the intended behavior to save the resource owner checking as
> > it's very time cost to send SCU cmd for each domain during booting
> > which benefits nothing except not exposing them in genpd list.
> >
> > > I prefer we not expose the power domain not owned to current
> > > partition and keep the err msg for people to easy see where it goes wrong.
> >
> > If we really want to to do, I wonder probably another better approach
> > is trying to re-use the partition Information built by bootloader as
> > uboot already did that one time, not necessary to re-do It again for kernel as
> it's time cost.
> > e.g. introduce a resource partition property in dt and initialized and
> > passed by bootloarder to kernel to use later.
>
> This will not work for hypervisor based VM runtime partition create and
> resource assignment.
>
For VM case, can't we define them in DT if it's statically defined?
> > Then we can still save those huge number of resource owner check CMDs.
>
> So we have to live with them even I only need one SDHC for a VM? With so
> many clk-scu entries and power domain entries there.
No, clk-scu register will fail automatically if not owned by this partition.
Regards
Aisheng
>
> Thanks,
> Peng.
>
> >
> > Regards
> > Aisheng
> >
> > >
> > > Regards,
> > > Peng.
> > > > So not quite necessary to add a double check.
> > > > If we're concerning about the error log, we can change it to dev_dbg().
> > > >
> > > > BTW, as resource management will be needed by seco drivers later,
> > > > So I will continue to review the patch.
> > > >
> > > > Regards
> > > > Aisheng
> > > >
> > > > >
> > > > > [ 2.034774] imx6q-pcie 5f000000.pcie: IO
> > 0x6ff80000..0x6ff8ffff
> > > ->
> > > > > 0x00000000
> > > > > [ 2.034801] imx6q-pcie 5f000000.pcie: MEM
> > > 0x60000000..0x6fefffff
> > > > ->
> > > > > 0x60000000
> > > > > [ 2.035072] sdhc1: failed to power up resource 249 ret -13
> > > > > [ 2.035619] sdhc2: failed to power up resource 250 ret -13
> > > > > [ 2.036126] enet0: failed to power up resource 251 ret -13
> > > > > [ 2.036584] enet1: failed to power up resource 252 ret -13
> > > > > [ 2.037040] mlb0: failed to power up resource 253 ret -13
> > > > > [ 2.037495] nand: failed to power up resource 265 ret -13
> > > > > [ 2.037951] nand: failed to power up resource 265 ret -13
> > > > > [ 2.038416] pwm0: failed to power up resource 191 ret -13
> > > > > [ 2.038868] pwm1: failed to power up resource 192 ret -13
> > > > > [ 2.039320] pwm2: failed to power up resource 193 ret -13
> > > > > [ 2.039786] pwm3: failed to power up resource 194 ret -13
> > > > > [ 2.040239] pwm4: failed to power up resource 195 ret -13
> > > > > [ 2.040692] pwm5: failed to power up resource 196 ret -13
> > > > > [ 2.041142] pwm6: failed to power up resource 197 ret -13
> > > > > [ 2.041596] pwm7: failed to power up resource 198 ret -13
> > > > > [ 2.042073] amix: failed to power up resource 458 ret -13
> > > > > [ 2.042558] lpspi0: failed to power up resource 53 ret -13
> > > > > [ 2.043033] lpspi1: failed to power up resource 54 ret -13
> > > > > [ 2.043501] lpspi2: failed to power up resource 55 ret -13
> > > > > [ 2.043992] lpspi3: failed to power up resource 56 ret -13
> > > > > [ 2.044460] lpuart0: failed to power up resource 57 ret -13
> > > > > [ 2.044935] lpuart2: failed to power up resource 59 ret -13
> > > > > [ 2.045409] lpuart3: failed to power up resource 60 ret -13
> > > > > [ 2.055014] sim0: failed to power up resource 62 ret -13
> > > > > [ 2.055510] adc0: failed to power up resource 101 ret -13
> > > > > [ 2.056467] lpi2c0: failed to power up resource 96 ret -13
> > > > > [ 2.056946] lpi2c1: failed to power up resource 97 ret -13
> > > > > [ 2.057424] lpi2c2: failed to power up resource 98 ret -13
> > > > > [ 2.057898] lpi2c3: failed to power up resource 99 ret -13
> > > > > [ 2.058371] can0: failed to power up resource 105 ret -13
> > > > > [ 2.059289] can1: failed to power up resource 106 ret -13
> > > > > [ 2.059801] can2: failed to power up resource 107 ret -13
> > > > > [ 2.060281] nand: failed to power up resource 265 ret -13
> > > > > [ 2.062764] dpu-core 56180000.dpu: driver probed
> > > > >
> > > > > Thanks,
> > > > > Peng.
> > > > >
> > > > > >
> > > > > > Regards
> > > > > > Aisheng
> > > > > >
> > > > > > > Thanks,
> > > > > > > Peng.
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > Regards
> > > > > > > > Aisheng
> > > > > > > >
> > > > > > > > > Thanks,
> > > > > > > > > Peng.
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Regards
> > > > > > > > > > Aisheng
> > > > > > > > > >
> > > > > > > > > > > ---
> > > > > > > > > > > drivers/firmware/imx/Makefile | 2 +-
> > > > > > > > > > > drivers/firmware/imx/rm.c | 40
> > > > > > > > +++++++++++++++++++++
> > > > > > > > > > > include/linux/firmware/imx/sci.h | 1 +
> > > > > > > > > > > include/linux/firmware/imx/svc/rm.h | 69
> > > > > > > > > > > +++++++++++++++++++++++++++++++++++++
> > > > > > > > > > > 4 files changed, 111 insertions(+), 1 deletion(-)
> > > > > > > > > > > create mode
> > > > > > > > > > > 100644 drivers/firmware/imx/rm.c create mode 100644
> > > > > > > > > > > include/linux/firmware/imx/svc/rm.h
> > > > > > > > > > >
> > > > > > > > > > > diff --git a/drivers/firmware/imx/Makefile
> > > > > > > > > > > b/drivers/firmware/imx/Makefile index
> > > > > > > > > > > 08bc9ddfbdfb..17ea3613e142
> > > > > > > > > > > 100644
> > > > > > > > > > > --- a/drivers/firmware/imx/Makefile
> > > > > > > > > > > +++ b/drivers/firmware/imx/Makefile
> > > > > > > > > > > @@ -1,4 +1,4 @@
> > > > > > > > > > > # SPDX-License-Identifier: GPL-2.0
> > > > > > > > > > > obj-$(CONFIG_IMX_DSP) += imx-dsp.o
> > > > > > > > > > > -obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o
> > > imx-scu-irq.o
> > > > > > > > > > > +obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o
> > > > imx-scu-irq.o
> > > > > > rm.o
> > > > > > > > > > > obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> > > > > > > > > > > diff --git a/drivers/firmware/imx/rm.c
> > > > > > > > > > > b/drivers/firmware/imx/rm.c new file mode 100644
> > > > > > > > > > > index
> > > > > > > > > > > 000000000000..7b0334de5486
> > > > > > > > > > > --- /dev/null
> > > > > > > > > > > +++ b/drivers/firmware/imx/rm.c
> > > > > > > > > > > @@ -0,0 +1,40 @@
> > > > > > > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > > > > > > +/*
> > > > > > > > > > > + * Copyright 2020 NXP
> > > > > > > > > > > + *
> > > > > > > > > > > + * File containing client-side RPC functions for
> > > > > > > > > > > +the RM
> > service.
> > > > > > > > > > > +These
> > > > > > > > > > > + * function are ported to clients that communicate to the SC.
> > > > > > > > > > > + */
> > > > > > > > > > > +
> > > > > > > > > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > > > > > > > > +
> > > > > > > > > > > +struct imx_sc_msg_rm_rsrc_owned {
> > > > > > > > > > > + struct imx_sc_rpc_msg hdr;
> > > > > > > > > > > + u16 resource;
> > > > > > > > > > > +} __packed __aligned(4);
> > > > > > > > > > > +
> > > > > > > > > > > +/*
> > > > > > > > > > > + * This function check @resource is owned by
> > > > > > > > > > > +current partition or not
> > > > > > > > > > > + *
> > > > > > > > > > > + * @param[in] ipc IPC handle
> > > > > > > > > > > + * @param[in] resource resource the control is
> > > > > > associated
> > > > > > > > with
> > > > > > > > > > > + *
> > > > > > > > > > > + * @return Returns 0 for success and < 0 for errors.
> > > > > > > > > > > + */
> > > > > > > > > > > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc
> > > > > > > > > > > +*ipc,
> > > > > > > > > > > +u16
> > > > > > > > > > > +resource) {
> > > > > > > > > > > + struct imx_sc_msg_rm_rsrc_owned msg;
> > > > > > > > > > > + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > > > > > > > > > > +
> > > > > > > > > > > + hdr->ver = IMX_SC_RPC_VERSION;
> > > > > > > > > > > + hdr->svc = IMX_SC_RPC_SVC_RM;
> > > > > > > > > > > + hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
> > > > > > > > > > > + hdr->size = 2;
> > > > > > > > > > > +
> > > > > > > > > > > + msg.resource = resource;
> > > > > > > > > > > +
> > > > > > > > > > > + imx_scu_call_rpc(ipc, &msg, true);
> > > > > > > > > > > +
> > > > > > > > > > > + return hdr->func;
> > > > > > > > > > > +}
> > > > > > > > > > > +EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
> > > > > > > > > > > diff --git a/include/linux/firmware/imx/sci.h
> > > > > > > > > > > b/include/linux/firmware/imx/sci.h
> > > > > > > > > > > index 17ba4e405129..b5c5a56f29be 100644
> > > > > > > > > > > --- a/include/linux/firmware/imx/sci.h
> > > > > > > > > > > +++ b/include/linux/firmware/imx/sci.h
> > > > > > > > > > > @@ -15,6 +15,7 @@
> > > > > > > > > > >
> > > > > > > > > > > #include <linux/firmware/imx/svc/misc.h> #include
> > > > > > > > > > > <linux/firmware/imx/svc/pm.h>
> > > > > > > > > > > +#include <linux/firmware/imx/svc/rm.h>
> > > > > > > > > > >
> > > > > > > > > > > int imx_scu_enable_general_irq_channel(struct
> > > > > > > > > > > device *dev); int
> > > > > > > > > > > imx_scu_irq_register_notifier(struct
> > > > > > > > > > > notifier_block *nb); diff --git
> > > > > > > > > > > a/include/linux/firmware/imx/svc/rm.h
> > > > > > > > > > > b/include/linux/firmware/imx/svc/rm.h
> > > > > > > > > > > new file mode 100644 index
> > > > > > > > > > > 000000000000..fc6ea62d9d0e
> > > > > > > > > > > --- /dev/null
> > > > > > > > > > > +++ b/include/linux/firmware/imx/svc/rm.h
> > > > > > > > > > > @@ -0,0 +1,69 @@
> > > > > > > > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > > > > > > > +/*
> > > > > > > > > > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > > > > > > > > > + * Copyright 2017-2019 NXP
> > > > > > > > > > > + *
> > > > > > > > > > > + * Header file containing the public API for the
> > > > > > > > > > > +System Controller
> > > > > > > > > > > +(SC)
> > > > > > > > > > > + * Power Management (PM) function. This includes
> > > > > > > > > > > +functions for power state
> > > > > > > > > > > + * control, clock control, reset control, and
> > > > > > > > > > > +wake-up event
> > > > control.
> > > > > > > > > > > + *
> > > > > > > > > > > + * RM_SVC (SVC) Resource Management Service
> > > > > > > > > > > + *
> > > > > > > > > > > + * Module for the Resource Management (RM) service.
> > > > > > > > > > > + */
> > > > > > > > > > > +
> > > > > > > > > > > +#ifndef _SC_RM_API_H #define _SC_RM_API_H
> > > > > > > > > > > +
> > > > > > > > > > > +#include <linux/firmware/imx/sci.h>
> > > > > > > > > > > +
> > > > > > > > > > > +/*
> > > > > > > > > > > + * This type is used to indicate RPC RM function calls.
> > > > > > > > > > > + */
> > > > > > > > > > > +enum imx_sc_rm_func {
> > > > > > > > > > > + IMX_SC_RM_FUNC_UNKNOWN = 0,
> > > > > > > > > > > + IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
> > > > > > > > > > > + IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
> > > > > > > > > > > + IMX_SC_RM_FUNC_PARTITION_FREE = 2,
> > > > > > > > > > > + IMX_SC_RM_FUNC_GET_DID = 26,
> > > > > > > > > > > + IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
> > > > > > > > > > > + IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
> > > > > > > > > > > + IMX_SC_RM_FUNC_GET_PARTITION = 5,
> > > > > > > > > > > + IMX_SC_RM_FUNC_SET_PARENT = 6,
> > > > > > > > > > > + IMX_SC_RM_FUNC_MOVE_ALL = 7,
> > > > > > > > > > > + IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
> > > > > > > > > > > + IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
> > > > > > > > > > > + IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
> > > > > > > > > > > + IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
> > > > > > > > > > > + IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
> > > > > > > > > > > + IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS =
> > 12,
> > > > > > > > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
> > > > > > > > > > > + IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
> > > > > > > > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
> > > > > > > > > > > + IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
> > > > > > > > > > > + IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
> > > > > > > > > > > + IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
> > > > > > > > > > > + IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
> > > > > > > > > > > + IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
> > > > > > > > > > > + IMX_SC_RM_FUNC_MEMREG_FREE = 18,
> > > > > > > > > > > + IMX_SC_RM_FUNC_FIND_MEMREG = 30,
> > > > > > > > > > > + IMX_SC_RM_FUNC_ASSIGN_MEMREG = 19,
> > > > > > > > > > > + IMX_SC_RM_FUNC_SET_MEMREG_PERMISSIONS = 20,
> > > > > > > > > > > + IMX_SC_RM_FUNC_IS_MEMREG_OWNED = 21,
> > > > > > > > > > > + IMX_SC_RM_FUNC_GET_MEMREG_INFO = 22,
> > > > > > > > > > > + IMX_SC_RM_FUNC_ASSIGN_PAD = 23,
> > > > > > > > > > > + IMX_SC_RM_FUNC_SET_PAD_MOVABLE = 24,
> > > > > > > > > > > + IMX_SC_RM_FUNC_IS_PAD_OWNED = 25,
> > > > > > > > > > > + IMX_SC_RM_FUNC_DUMP = 27, };
> > > > > > > > > > > +
> > > > > > > > > > > +#if IS_ENABLED(CONFIG_IMX_SCU) bool
> > > > > > > > > > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc,
> > > > > > > > > > > +u16 resource); #else static inline bool
> > > > > > > > > > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc,
> > > > > > > > > > > +u16
> > > > > > resource) {
> > > > > > > > > > > + return true;
> > > > > > > > > > > +}
> > > > > > > > > > > +#endif
> > > > > > > > > > > +#endif
> > > > > > > > > > > --
> > > > > > > > > > > 2.16.4
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH 2/4] firmware: imx: add resource management api
2020-04-23 6:59 ` [PATCH 2/4] firmware: imx: add resource management api peng.fan
2020-04-23 10:04 ` Aisheng Dong
@ 2020-06-02 3:07 ` Aisheng Dong
2020-06-02 5:23 ` Peng Fan
1 sibling, 1 reply; 22+ messages in thread
From: Aisheng Dong @ 2020-06-02 3:07 UTC (permalink / raw)
To: Peng Fan, shawnguo, s.hauer
Cc: kernel, festevam, linux, Leonard Crestez, Daniel Baluta,
Joakim Zhang, linux-kernel, linux-arm-kernel, dl-linux-imx
> From: Peng Fan <peng.fan@nxp.com>
> Sent: Thursday, April 23, 2020 3:00 PM
>
> Add resource management API, when we have multiple partition running
> together, resources not owned to current partition should not be used.
>
> Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/firmware/imx/Makefile | 2 +-
> drivers/firmware/imx/rm.c | 40 +++++++++++++++++++++
> include/linux/firmware/imx/sci.h | 1 +
> include/linux/firmware/imx/svc/rm.h | 69
> +++++++++++++++++++++++++++++++++++++
> 4 files changed, 111 insertions(+), 1 deletion(-) create mode 100644
> drivers/firmware/imx/rm.c create mode 100644
> include/linux/firmware/imx/svc/rm.h
>
> diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
> index 08bc9ddfbdfb..17ea3613e142 100644
> --- a/drivers/firmware/imx/Makefile
> +++ b/drivers/firmware/imx/Makefile
> @@ -1,4 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_IMX_DSP) += imx-dsp.o
> -obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o
> +obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o
> obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> diff --git a/drivers/firmware/imx/rm.c b/drivers/firmware/imx/rm.c new file
> mode 100644 index 000000000000..7b0334de5486
> --- /dev/null
> +++ b/drivers/firmware/imx/rm.c
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2020 NXP
> + *
> + * File containing client-side RPC functions for the RM service. These
> + * function are ported to clients that communicate to the SC.
> + */
> +
> +#include <linux/firmware/imx/svc/rm.h>
> +
> +struct imx_sc_msg_rm_rsrc_owned {
> + struct imx_sc_rpc_msg hdr;
> + u16 resource;
> +} __packed __aligned(4);
> +
> +/*
> + * This function check @resource is owned by current partition or not
> + *
> + * @param[in] ipc IPC handle
> + * @param[in] resource resource the control is associated with
> + *
> + * @return Returns 0 for success and < 0 for errors.
> + */
> +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16 resource)
> +{
> + struct imx_sc_msg_rm_rsrc_owned msg;
> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> +
> + hdr->ver = IMX_SC_RPC_VERSION;
> + hdr->svc = IMX_SC_RPC_SVC_RM;
> + hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
> + hdr->size = 2;
> +
> + msg.resource = resource;
> +
> + imx_scu_call_rpc(ipc, &msg, true);
No need check err return?
> +
> + return hdr->func;
> +}
> +EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
> diff --git a/include/linux/firmware/imx/sci.h b/include/linux/firmware/imx/sci.h
> index 17ba4e405129..b5c5a56f29be 100644
> --- a/include/linux/firmware/imx/sci.h
> +++ b/include/linux/firmware/imx/sci.h
> @@ -15,6 +15,7 @@
>
> #include <linux/firmware/imx/svc/misc.h> #include
> <linux/firmware/imx/svc/pm.h>
> +#include <linux/firmware/imx/svc/rm.h>
>
> int imx_scu_enable_general_irq_channel(struct device *dev); int
> imx_scu_irq_register_notifier(struct notifier_block *nb); diff --git
> a/include/linux/firmware/imx/svc/rm.h b/include/linux/firmware/imx/svc/rm.h
> new file mode 100644
> index 000000000000..fc6ea62d9d0e
> --- /dev/null
> +++ b/include/linux/firmware/imx/svc/rm.h
> @@ -0,0 +1,69 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + * Copyright 2017-2019 NXP
Update copyright
> + *
> + * Header file containing the public API for the System Controller (SC)
> + * Power Management (PM) function. This includes functions for power
> +state
> + * control, clock control, reset control, and wake-up event control.
Fix the code comments
Otherwise, I'm fine with this patch.
Regards
Aisheng
> + *
> + * RM_SVC (SVC) Resource Management Service
> + *
> + * Module for the Resource Management (RM) service.
> + */
> +
> +#ifndef _SC_RM_API_H
> +#define _SC_RM_API_H
> +
> +#include <linux/firmware/imx/sci.h>
> +
> +/*
> + * This type is used to indicate RPC RM function calls.
> + */
> +enum imx_sc_rm_func {
> + IMX_SC_RM_FUNC_UNKNOWN = 0,
> + IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
> + IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
> + IMX_SC_RM_FUNC_PARTITION_FREE = 2,
> + IMX_SC_RM_FUNC_GET_DID = 26,
> + IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
> + IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
> + IMX_SC_RM_FUNC_GET_PARTITION = 5,
> + IMX_SC_RM_FUNC_SET_PARENT = 6,
> + IMX_SC_RM_FUNC_MOVE_ALL = 7,
> + IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
> + IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
> + IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
> + IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
> + IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
> + IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS = 12,
> + IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
> + IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
> + IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
> + IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
> + IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
> + IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
> + IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
> + IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
> + IMX_SC_RM_FUNC_MEMREG_FREE = 18,
> + IMX_SC_RM_FUNC_FIND_MEMREG = 30,
> + IMX_SC_RM_FUNC_ASSIGN_MEMREG = 19,
> + IMX_SC_RM_FUNC_SET_MEMREG_PERMISSIONS = 20,
> + IMX_SC_RM_FUNC_IS_MEMREG_OWNED = 21,
> + IMX_SC_RM_FUNC_GET_MEMREG_INFO = 22,
> + IMX_SC_RM_FUNC_ASSIGN_PAD = 23,
> + IMX_SC_RM_FUNC_SET_PAD_MOVABLE = 24,
> + IMX_SC_RM_FUNC_IS_PAD_OWNED = 25,
> + IMX_SC_RM_FUNC_DUMP = 27,
> +};
> +
> +#if IS_ENABLED(CONFIG_IMX_SCU)
> +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16 resource);
> +#else static inline bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc
> +*ipc, u16 resource) {
> + return true;
> +}
> +#endif
> +#endif
> --
> 2.16.4
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 2/4] firmware: imx: add resource management api
2020-06-02 3:07 ` Aisheng Dong
@ 2020-06-02 5:23 ` Peng Fan
2020-06-02 7:12 ` Aisheng Dong
0 siblings, 1 reply; 22+ messages in thread
From: Peng Fan @ 2020-06-02 5:23 UTC (permalink / raw)
To: Aisheng Dong, shawnguo, s.hauer
Cc: kernel, festevam, linux, Leonard Crestez, Daniel Baluta,
Joakim Zhang, linux-kernel, linux-arm-kernel, dl-linux-imx
> Subject: RE: [PATCH 2/4] firmware: imx: add resource management api
>
> > From: Peng Fan <peng.fan@nxp.com>
> > Sent: Thursday, April 23, 2020 3:00 PM
> >
> > Add resource management API, when we have multiple partition running
> > together, resources not owned to current partition should not be used.
> >
> > Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> > drivers/firmware/imx/Makefile | 2 +-
> > drivers/firmware/imx/rm.c | 40 +++++++++++++++++++++
> > include/linux/firmware/imx/sci.h | 1 +
> > include/linux/firmware/imx/svc/rm.h | 69
> > +++++++++++++++++++++++++++++++++++++
> > 4 files changed, 111 insertions(+), 1 deletion(-) create mode 100644
> > drivers/firmware/imx/rm.c create mode 100644
> > include/linux/firmware/imx/svc/rm.h
> >
> > diff --git a/drivers/firmware/imx/Makefile
> > b/drivers/firmware/imx/Makefile index 08bc9ddfbdfb..17ea3613e142
> > 100644
> > --- a/drivers/firmware/imx/Makefile
> > +++ b/drivers/firmware/imx/Makefile
> > @@ -1,4 +1,4 @@
> > # SPDX-License-Identifier: GPL-2.0
> > obj-$(CONFIG_IMX_DSP) += imx-dsp.o
> > -obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o
> > +obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o
> > obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> > diff --git a/drivers/firmware/imx/rm.c b/drivers/firmware/imx/rm.c new
> > file mode 100644 index 000000000000..7b0334de5486
> > --- /dev/null
> > +++ b/drivers/firmware/imx/rm.c
> > @@ -0,0 +1,40 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2020 NXP
> > + *
> > + * File containing client-side RPC functions for the RM service.
> > +These
> > + * function are ported to clients that communicate to the SC.
> > + */
> > +
> > +#include <linux/firmware/imx/svc/rm.h>
> > +
> > +struct imx_sc_msg_rm_rsrc_owned {
> > + struct imx_sc_rpc_msg hdr;
> > + u16 resource;
> > +} __packed __aligned(4);
> > +
> > +/*
> > + * This function check @resource is owned by current partition or not
> > + *
> > + * @param[in] ipc IPC handle
> > + * @param[in] resource resource the control is associated with
> > + *
> > + * @return Returns 0 for success and < 0 for errors.
> > + */
> > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > +resource) {
> > + struct imx_sc_msg_rm_rsrc_owned msg;
> > + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > +
> > + hdr->ver = IMX_SC_RPC_VERSION;
> > + hdr->svc = IMX_SC_RPC_SVC_RM;
> > + hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
> > + hdr->size = 2;
> > +
> > + msg.resource = resource;
> > +
> > + imx_scu_call_rpc(ipc, &msg, true);
>
> No need check err return?
No. it use hdr->func as the resource owner bool.
However imx_scu_call_rpc also use hdr->func
to check error value or not.
When hdr->func is 1, imx_sc_to_linux_errno
will report it -EINVAL. However here 1 means
not owned.
>
> > +
> > + return hdr->func;
> > +}
> > +EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
> > diff --git a/include/linux/firmware/imx/sci.h
> > b/include/linux/firmware/imx/sci.h
> > index 17ba4e405129..b5c5a56f29be 100644
> > --- a/include/linux/firmware/imx/sci.h
> > +++ b/include/linux/firmware/imx/sci.h
> > @@ -15,6 +15,7 @@
> >
> > #include <linux/firmware/imx/svc/misc.h> #include
> > <linux/firmware/imx/svc/pm.h>
> > +#include <linux/firmware/imx/svc/rm.h>
> >
> > int imx_scu_enable_general_irq_channel(struct device *dev); int
> > imx_scu_irq_register_notifier(struct notifier_block *nb); diff --git
> > a/include/linux/firmware/imx/svc/rm.h
> > b/include/linux/firmware/imx/svc/rm.h
> > new file mode 100644
> > index 000000000000..fc6ea62d9d0e
> > --- /dev/null
> > +++ b/include/linux/firmware/imx/svc/rm.h
> > @@ -0,0 +1,69 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > + * Copyright 2017-2019 NXP
>
> Update copyright
ok
>
> > + *
> > + * Header file containing the public API for the System Controller
> > +(SC)
> > + * Power Management (PM) function. This includes functions for power
> > +state
> > + * control, clock control, reset control, and wake-up event control.
>
> Fix the code comments
>
> Otherwise, I'm fine with this patch.
Ok.
Thanks,
Peng.
>
> Regards
> Aisheng
>
> > + *
> > + * RM_SVC (SVC) Resource Management Service
> > + *
> > + * Module for the Resource Management (RM) service.
> > + */
> > +
> > +#ifndef _SC_RM_API_H
> > +#define _SC_RM_API_H
> > +
> > +#include <linux/firmware/imx/sci.h>
> > +
> > +/*
> > + * This type is used to indicate RPC RM function calls.
> > + */
> > +enum imx_sc_rm_func {
> > + IMX_SC_RM_FUNC_UNKNOWN = 0,
> > + IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
> > + IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
> > + IMX_SC_RM_FUNC_PARTITION_FREE = 2,
> > + IMX_SC_RM_FUNC_GET_DID = 26,
> > + IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
> > + IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
> > + IMX_SC_RM_FUNC_GET_PARTITION = 5,
> > + IMX_SC_RM_FUNC_SET_PARENT = 6,
> > + IMX_SC_RM_FUNC_MOVE_ALL = 7,
> > + IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
> > + IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
> > + IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
> > + IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
> > + IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
> > + IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS = 12,
> > + IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
> > + IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
> > + IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
> > + IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
> > + IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
> > + IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
> > + IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
> > + IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
> > + IMX_SC_RM_FUNC_MEMREG_FREE = 18,
> > + IMX_SC_RM_FUNC_FIND_MEMREG = 30,
> > + IMX_SC_RM_FUNC_ASSIGN_MEMREG = 19,
> > + IMX_SC_RM_FUNC_SET_MEMREG_PERMISSIONS = 20,
> > + IMX_SC_RM_FUNC_IS_MEMREG_OWNED = 21,
> > + IMX_SC_RM_FUNC_GET_MEMREG_INFO = 22,
> > + IMX_SC_RM_FUNC_ASSIGN_PAD = 23,
> > + IMX_SC_RM_FUNC_SET_PAD_MOVABLE = 24,
> > + IMX_SC_RM_FUNC_IS_PAD_OWNED = 25,
> > + IMX_SC_RM_FUNC_DUMP = 27,
> > +};
> > +
> > +#if IS_ENABLED(CONFIG_IMX_SCU)
> > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > +resource); #else static inline bool
> > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16 resource) {
> > + return true;
> > +}
> > +#endif
> > +#endif
> > --
> > 2.16.4
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 2/4] firmware: imx: add resource management api
2020-06-02 5:23 ` Peng Fan
@ 2020-06-02 7:12 ` Aisheng Dong
0 siblings, 0 replies; 22+ messages in thread
From: Aisheng Dong @ 2020-06-02 7:12 UTC (permalink / raw)
To: Peng Fan, shawnguo, s.hauer
Cc: kernel, festevam, linux, Leonard Crestez, Daniel Baluta,
Joakim Zhang, linux-kernel, linux-arm-kernel, dl-linux-imx
> From: Peng Fan <peng.fan@nxp.com>
> Sent: Tuesday, June 2, 2020 1:24 PM
>
> > Subject: RE: [PATCH 2/4] firmware: imx: add resource management api
> >
> > > From: Peng Fan <peng.fan@nxp.com>
> > > Sent: Thursday, April 23, 2020 3:00 PM
> > >
> > > Add resource management API, when we have multiple partition running
> > > together, resources not owned to current partition should not be used.
> > >
> > > Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > > drivers/firmware/imx/Makefile | 2 +-
> > > drivers/firmware/imx/rm.c | 40 +++++++++++++++++++++
> > > include/linux/firmware/imx/sci.h | 1 +
> > > include/linux/firmware/imx/svc/rm.h | 69
> > > +++++++++++++++++++++++++++++++++++++
> > > 4 files changed, 111 insertions(+), 1 deletion(-) create mode
> > > 100644 drivers/firmware/imx/rm.c create mode 100644
> > > include/linux/firmware/imx/svc/rm.h
> > >
> > > diff --git a/drivers/firmware/imx/Makefile
> > > b/drivers/firmware/imx/Makefile index 08bc9ddfbdfb..17ea3613e142
> > > 100644
> > > --- a/drivers/firmware/imx/Makefile
> > > +++ b/drivers/firmware/imx/Makefile
> > > @@ -1,4 +1,4 @@
> > > # SPDX-License-Identifier: GPL-2.0
> > > obj-$(CONFIG_IMX_DSP) += imx-dsp.o
> > > -obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o
> > > +obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o
> > > obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
> > > diff --git a/drivers/firmware/imx/rm.c b/drivers/firmware/imx/rm.c
> > > new file mode 100644 index 000000000000..7b0334de5486
> > > --- /dev/null
> > > +++ b/drivers/firmware/imx/rm.c
> > > @@ -0,0 +1,40 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright 2020 NXP
> > > + *
> > > + * File containing client-side RPC functions for the RM service.
> > > +These
> > > + * function are ported to clients that communicate to the SC.
> > > + */
> > > +
> > > +#include <linux/firmware/imx/svc/rm.h>
> > > +
> > > +struct imx_sc_msg_rm_rsrc_owned {
> > > + struct imx_sc_rpc_msg hdr;
> > > + u16 resource;
> > > +} __packed __aligned(4);
> > > +
> > > +/*
> > > + * This function check @resource is owned by current partition or
> > > +not
> > > + *
> > > + * @param[in] ipc IPC handle
> > > + * @param[in] resource resource the control is associated with
> > > + *
> > > + * @return Returns 0 for success and < 0 for errors.
> > > + */
> > > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > +resource) {
> > > + struct imx_sc_msg_rm_rsrc_owned msg;
> > > + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> > > +
> > > + hdr->ver = IMX_SC_RPC_VERSION;
> > > + hdr->svc = IMX_SC_RPC_SVC_RM;
> > > + hdr->func = IMX_SC_RM_FUNC_IS_RESOURCE_OWNED;
> > > + hdr->size = 2;
> > > +
> > > + msg.resource = resource;
> > > +
> > > + imx_scu_call_rpc(ipc, &msg, true);
> >
> > No need check err return?
>
> No. it use hdr->func as the resource owner bool.
> However imx_scu_call_rpc also use hdr->func to check error value or not.
>
> When hdr->func is 1, imx_sc_to_linux_errno will report it -EINVAL. However
> here 1 means not owned.
For this special case, pls add a code comment about it.
Refer to:
drivers/input/keyboard/imx_sc_pwrkey.c
Regards
Aisheng
>
> >
> > > +
> > > + return hdr->func;
> > > +}
> > > +EXPORT_SYMBOL(imx_sc_rm_is_resource_owned);
> > > diff --git a/include/linux/firmware/imx/sci.h
> > > b/include/linux/firmware/imx/sci.h
> > > index 17ba4e405129..b5c5a56f29be 100644
> > > --- a/include/linux/firmware/imx/sci.h
> > > +++ b/include/linux/firmware/imx/sci.h
> > > @@ -15,6 +15,7 @@
> > >
> > > #include <linux/firmware/imx/svc/misc.h> #include
> > > <linux/firmware/imx/svc/pm.h>
> > > +#include <linux/firmware/imx/svc/rm.h>
> > >
> > > int imx_scu_enable_general_irq_channel(struct device *dev); int
> > > imx_scu_irq_register_notifier(struct notifier_block *nb); diff --git
> > > a/include/linux/firmware/imx/svc/rm.h
> > > b/include/linux/firmware/imx/svc/rm.h
> > > new file mode 100644
> > > index 000000000000..fc6ea62d9d0e
> > > --- /dev/null
> > > +++ b/include/linux/firmware/imx/svc/rm.h
> > > @@ -0,0 +1,69 @@
> > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > +/*
> > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > + * Copyright 2017-2019 NXP
> >
> > Update copyright
>
> ok
>
> >
> > > + *
> > > + * Header file containing the public API for the System Controller
> > > +(SC)
> > > + * Power Management (PM) function. This includes functions for
> > > +power state
> > > + * control, clock control, reset control, and wake-up event control.
> >
> > Fix the code comments
> >
> > Otherwise, I'm fine with this patch.
> Ok.
>
> Thanks,
> Peng.
>
> >
> > Regards
> > Aisheng
> >
> > > + *
> > > + * RM_SVC (SVC) Resource Management Service
> > > + *
> > > + * Module for the Resource Management (RM) service.
> > > + */
> > > +
> > > +#ifndef _SC_RM_API_H
> > > +#define _SC_RM_API_H
> > > +
> > > +#include <linux/firmware/imx/sci.h>
> > > +
> > > +/*
> > > + * This type is used to indicate RPC RM function calls.
> > > + */
> > > +enum imx_sc_rm_func {
> > > + IMX_SC_RM_FUNC_UNKNOWN = 0,
> > > + IMX_SC_RM_FUNC_PARTITION_ALLOC = 1,
> > > + IMX_SC_RM_FUNC_SET_CONFIDENTIAL = 31,
> > > + IMX_SC_RM_FUNC_PARTITION_FREE = 2,
> > > + IMX_SC_RM_FUNC_GET_DID = 26,
> > > + IMX_SC_RM_FUNC_PARTITION_STATIC = 3,
> > > + IMX_SC_RM_FUNC_PARTITION_LOCK = 4,
> > > + IMX_SC_RM_FUNC_GET_PARTITION = 5,
> > > + IMX_SC_RM_FUNC_SET_PARENT = 6,
> > > + IMX_SC_RM_FUNC_MOVE_ALL = 7,
> > > + IMX_SC_RM_FUNC_ASSIGN_RESOURCE = 8,
> > > + IMX_SC_RM_FUNC_SET_RESOURCE_MOVABLE = 9,
> > > + IMX_SC_RM_FUNC_SET_SUBSYS_RSRC_MOVABLE = 28,
> > > + IMX_SC_RM_FUNC_SET_MASTER_ATTRIBUTES = 10,
> > > + IMX_SC_RM_FUNC_SET_MASTER_SID = 11,
> > > + IMX_SC_RM_FUNC_SET_PERIPHERAL_PERMISSIONS = 12,
> > > + IMX_SC_RM_FUNC_IS_RESOURCE_OWNED = 13,
> > > + IMX_SC_RM_FUNC_GET_RESOURCE_OWNER = 33,
> > > + IMX_SC_RM_FUNC_IS_RESOURCE_MASTER = 14,
> > > + IMX_SC_RM_FUNC_IS_RESOURCE_PERIPHERAL = 15,
> > > + IMX_SC_RM_FUNC_GET_RESOURCE_INFO = 16,
> > > + IMX_SC_RM_FUNC_MEMREG_ALLOC = 17,
> > > + IMX_SC_RM_FUNC_MEMREG_SPLIT = 29,
> > > + IMX_SC_RM_FUNC_MEMREG_FRAG = 32,
> > > + IMX_SC_RM_FUNC_MEMREG_FREE = 18,
> > > + IMX_SC_RM_FUNC_FIND_MEMREG = 30,
> > > + IMX_SC_RM_FUNC_ASSIGN_MEMREG = 19,
> > > + IMX_SC_RM_FUNC_SET_MEMREG_PERMISSIONS = 20,
> > > + IMX_SC_RM_FUNC_IS_MEMREG_OWNED = 21,
> > > + IMX_SC_RM_FUNC_GET_MEMREG_INFO = 22,
> > > + IMX_SC_RM_FUNC_ASSIGN_PAD = 23,
> > > + IMX_SC_RM_FUNC_SET_PAD_MOVABLE = 24,
> > > + IMX_SC_RM_FUNC_IS_PAD_OWNED = 25,
> > > + IMX_SC_RM_FUNC_DUMP = 27,
> > > +};
> > > +
> > > +#if IS_ENABLED(CONFIG_IMX_SCU)
> > > +bool imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16
> > > +resource); #else static inline bool
> > > +imx_sc_rm_is_resource_owned(struct imx_sc_ipc *ipc, u16 resource) {
> > > + return true;
> > > +}
> > > +#endif
> > > +#endif
> > > --
> > > 2.16.4
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/4] firmware: imx: scu-pd: ignore power domain not owned
2020-04-23 6:59 [PATCH 0/4] firmware: imx: fix/update cm4 and add resource check peng.fan
2020-04-23 6:59 ` [PATCH 1/4] firmware: imx: scu-pd: fix cm40 power domain peng.fan
2020-04-23 6:59 ` [PATCH 2/4] firmware: imx: add resource management api peng.fan
@ 2020-04-23 6:59 ` peng.fan
2020-04-23 10:06 ` Aisheng Dong
2020-04-23 6:59 ` [PATCH 4/4] firmware: imx: scu-pd: add more cm4 resources peng.fan
3 siblings, 1 reply; 22+ messages in thread
From: peng.fan @ 2020-04-23 6:59 UTC (permalink / raw)
To: shawnguo, s.hauer
Cc: kernel, festevam, linux, leonard.crestez, daniel.baluta,
aisheng.dong, qiangqing.zhang, linux-kernel, linux-arm-kernel,
linux-imx, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
Should not register power domain that not owned by current
partition.
Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/imx/scu-pd.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/firmware/imx/scu-pd.c b/drivers/firmware/imx/scu-pd.c
index d1b313fa7b96..7b43bd63cc1e 100644
--- a/drivers/firmware/imx/scu-pd.c
+++ b/drivers/firmware/imx/scu-pd.c
@@ -46,6 +46,7 @@
#include <dt-bindings/firmware/imx/rsrc.h>
#include <linux/firmware/imx/sci.h>
+#include <linux/firmware/imx/svc/rm.h>
#include <linux/io.h>
#include <linux/module.h>
#include <linux/of.h>
@@ -246,6 +247,9 @@ imx_scu_add_pm_domain(struct device *dev, int idx,
struct imx_sc_pm_domain *sc_pd;
int ret;
+ if (!imx_sc_rm_is_resource_owned(pm_ipc_handle, pd_ranges->rsrc + idx))
+ return NULL;
+
sc_pd = devm_kzalloc(dev, sizeof(*sc_pd), GFP_KERNEL);
if (!sc_pd)
return ERR_PTR(-ENOMEM);
--
2.16.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* RE: [PATCH 3/4] firmware: imx: scu-pd: ignore power domain not owned
2020-04-23 6:59 ` [PATCH 3/4] firmware: imx: scu-pd: ignore power domain not owned peng.fan
@ 2020-04-23 10:06 ` Aisheng Dong
0 siblings, 0 replies; 22+ messages in thread
From: Aisheng Dong @ 2020-04-23 10:06 UTC (permalink / raw)
To: Peng Fan, shawnguo, s.hauer
Cc: kernel, festevam, linux, Leonard Crestez, Daniel Baluta,
Joakim Zhang, linux-kernel, linux-arm-kernel, dl-linux-imx
> From: Peng Fan <peng.fan@nxp.com>
> Sent: Thursday, April 23, 2020 3:00 PM
>
> Should not register power domain that not owned by current partition.
>
> Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/firmware/imx/scu-pd.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/firmware/imx/scu-pd.c b/drivers/firmware/imx/scu-pd.c
> index d1b313fa7b96..7b43bd63cc1e 100644
> --- a/drivers/firmware/imx/scu-pd.c
> +++ b/drivers/firmware/imx/scu-pd.c
> @@ -46,6 +46,7 @@
>
> #include <dt-bindings/firmware/imx/rsrc.h> #include
> <linux/firmware/imx/sci.h>
> +#include <linux/firmware/imx/svc/rm.h>
> #include <linux/io.h>
> #include <linux/module.h>
> #include <linux/of.h>
> @@ -246,6 +247,9 @@ imx_scu_add_pm_domain(struct device *dev, int idx,
> struct imx_sc_pm_domain *sc_pd;
> int ret;
>
> + if (!imx_sc_rm_is_resource_owned(pm_ipc_handle, pd_ranges->rsrc + idx))
> + return NULL;
> +
Same question.
See my comment on patch 2/4.
Regards
Aisheng
> sc_pd = devm_kzalloc(dev, sizeof(*sc_pd), GFP_KERNEL);
> if (!sc_pd)
> return ERR_PTR(-ENOMEM);
> --
> 2.16.4
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/4] firmware: imx: scu-pd: add more cm4 resources
2020-04-23 6:59 [PATCH 0/4] firmware: imx: fix/update cm4 and add resource check peng.fan
` (2 preceding siblings ...)
2020-04-23 6:59 ` [PATCH 3/4] firmware: imx: scu-pd: ignore power domain not owned peng.fan
@ 2020-04-23 6:59 ` peng.fan
2020-04-23 10:33 ` Aisheng Dong
3 siblings, 1 reply; 22+ messages in thread
From: peng.fan @ 2020-04-23 6:59 UTC (permalink / raw)
To: shawnguo, s.hauer
Cc: kernel, festevam, linux, leonard.crestez, daniel.baluta,
aisheng.dong, qiangqing.zhang, linux-kernel, linux-arm-kernel,
linux-imx, Peng Fan
From: Peng Fan <peng.fan@nxp.com>
Add more cm4 resources, then linux could use cm4's i2c/lpuart and
could kick cm4 core.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/firmware/imx/scu-pd.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/firmware/imx/scu-pd.c b/drivers/firmware/imx/scu-pd.c
index 7b43bd63cc1e..2fa86be2027a 100644
--- a/drivers/firmware/imx/scu-pd.c
+++ b/drivers/firmware/imx/scu-pd.c
@@ -170,6 +170,16 @@ static const struct imx_sc_pd_range imx8qxp_scu_pd_ranges[] = {
/* CM40 SS */
{ "cm40_i2c", IMX_SC_R_M4_0_I2C, 1, false, 0 },
{ "cm40_intmux", IMX_SC_R_M4_0_INTMUX, 1, false, 0 },
+ { "cm40_pid", IMX_SC_R_M4_0_PID0, 5, true, 0},
+ { "cm40_mu1a", IMX_SC_R_M4_0_MU_1A, 1, false, 0},
+ { "cm40_lpuart", IMX_SC_R_M4_0_UART, 1, false, 0},
+
+ /* CM41 SS */
+ { "cm41_i2c", IMX_SC_R_M4_1_I2C, 1, false, 0 },
+ { "cm41_intmux", IMX_SC_R_M4_1_INTMUX, 1, false, 0 },
+ { "cm41_pid", IMX_SC_R_M4_1_PID0, 5, true, 0},
+ { "cm41_mu1a", IMX_SC_R_M4_1_MU_1A, 1, false, 0},
+ { "cm41_lpuart", IMX_SC_R_M4_1_UART, 1, false, 0},
};
static const struct imx_sc_pd_soc imx8qxp_scu_pd = {
--
2.16.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* RE: [PATCH 4/4] firmware: imx: scu-pd: add more cm4 resources
2020-04-23 6:59 ` [PATCH 4/4] firmware: imx: scu-pd: add more cm4 resources peng.fan
@ 2020-04-23 10:33 ` Aisheng Dong
0 siblings, 0 replies; 22+ messages in thread
From: Aisheng Dong @ 2020-04-23 10:33 UTC (permalink / raw)
To: Peng Fan, shawnguo, s.hauer
Cc: kernel, festevam, linux, Leonard Crestez, Daniel Baluta,
Joakim Zhang, linux-kernel, linux-arm-kernel, dl-linux-imx
> From: Peng Fan <peng.fan@nxp.com>
> Sent: Thursday, April 23, 2020 3:00 PM
>
> Add more cm4 resources, then linux could use cm4's i2c/lpuart and could kick
> cm4 core.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/firmware/imx/scu-pd.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/firmware/imx/scu-pd.c b/drivers/firmware/imx/scu-pd.c
> index 7b43bd63cc1e..2fa86be2027a 100644
> --- a/drivers/firmware/imx/scu-pd.c
> +++ b/drivers/firmware/imx/scu-pd.c
> @@ -170,6 +170,16 @@ static const struct imx_sc_pd_range
> imx8qxp_scu_pd_ranges[] = {
> /* CM40 SS */
> { "cm40_i2c", IMX_SC_R_M4_0_I2C, 1, false, 0 },
> { "cm40_intmux", IMX_SC_R_M4_0_INTMUX, 1, false, 0 },
> + { "cm40_pid", IMX_SC_R_M4_0_PID0, 5, true, 0},
> + { "cm40_mu1a", IMX_SC_R_M4_0_MU_1A, 1, false, 0},
> + { "cm40_lpuart", IMX_SC_R_M4_0_UART, 1, false, 0},
For consistency,
s/cm40_pid/cm40-pid
s/cm40_mu1a/cm40-mu-a1
s/cm40_lpuart/cm40-lpuart
This also applies for the following part.
besides that, you can add my tag:
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
Regards
Aisheng
A
> +
> + /* CM41 SS */
> + { "cm41_i2c", IMX_SC_R_M4_1_I2C, 1, false, 0 },
> + { "cm41_intmux", IMX_SC_R_M4_1_INTMUX, 1, false, 0 },
> + { "cm41_pid", IMX_SC_R_M4_1_PID0, 5, true, 0},
> + { "cm41_mu1a", IMX_SC_R_M4_1_MU_1A, 1, false, 0},
> + { "cm41_lpuart", IMX_SC_R_M4_1_UART, 1, false, 0},
> };
>
> static const struct imx_sc_pd_soc imx8qxp_scu_pd = {
> --
> 2.16.4
^ permalink raw reply [flat|nested] 22+ messages in thread