From: Peng Fan <[email protected]>
Fix cm40 power domain, update to add more cm4 resources
Add resource owner check, to not register if not owned by Linux.
Peng Fan (4):
firmware: imx: scu-pd: fix cm40 power domain
firmware: imx: add resource management api
firmware: imx: scu-pd: ignore power domain not owned
firmware: imx: scu-pd: add more cm4 resources
drivers/firmware/imx/Makefile | 2 +-
drivers/firmware/imx/rm.c | 40 +++++++++++++++++++++
drivers/firmware/imx/scu-pd.c | 18 ++++++++--
include/linux/firmware/imx/sci.h | 1 +
include/linux/firmware/imx/svc/rm.h | 69 +++++++++++++++++++++++++++++++++++++
5 files changed, 127 insertions(+), 3 deletions(-)
create mode 100644 drivers/firmware/imx/rm.c
create mode 100644 include/linux/firmware/imx/svc/rm.h
--
2.16.4
From: Peng Fan <[email protected]>
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 <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
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
From: Peng Fan <[email protected]>
Add more cm4 resources, then linux could use cm4's i2c/lpuart and
could kick cm4 core.
Signed-off-by: Peng Fan <[email protected]>
---
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
> From: Peng Fan <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Peng Fan <[email protected]>
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
> From: Peng Fan <[email protected]>
> 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 <[email protected]>
> ---
> 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 <[email protected]>
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
> Subject: RE: [PATCH 2/4] firmware: imx: add resource management api
>
> > From: Peng Fan <[email protected]>
> > 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 <[email protected]>
> > Signed-off-by: Peng Fan <[email protected]>
>
> 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
> From: Peng Fan <[email protected]>
> Sent: Thursday, April 23, 2020 6:57 PM
> > > From: Peng Fan <[email protected]>
> > > 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 <[email protected]>
> > > Signed-off-by: Peng Fan <[email protected]>
> >
> > 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
> Subject: RE: [PATCH 2/4] firmware: imx: add resource management api
>
> > From: Peng Fan <[email protected]>
> > Sent: Thursday, April 23, 2020 6:57 PM
> > > > From: Peng Fan <[email protected]>
> > > > 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 <[email protected]>
> > > > Signed-off-by: Peng Fan <[email protected]>
> > >
> > > 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
> From: Peng Fan <[email protected]>
> Sent: Friday, April 24, 2020 9:12 AM
> >
> > > From: Peng Fan <[email protected]>
> > > Sent: Thursday, April 23, 2020 6:57 PM
> > > > > From: Peng Fan <[email protected]>
> > > > > 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 <[email protected]>
> > > > > Signed-off-by: Peng Fan <[email protected]>
> > > >
> > > > 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
> Subject: RE: [PATCH 2/4] firmware: imx: add resource management api
>
> > From: Peng Fan <[email protected]>
> > Sent: Friday, April 24, 2020 9:12 AM
> > >
> > > > From: Peng Fan <[email protected]>
> > > > Sent: Thursday, April 23, 2020 6:57 PM
> > > > > > From: Peng Fan <[email protected]>
> > > > > > 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 <[email protected]>
> > > > > > Signed-off-by: Peng Fan <[email protected]>
> > > > >
> > > > > 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
> From: Peng Fan <[email protected]>
> Sent: Monday, June 1, 2020 8:40 PM
> >
> > > From: Peng Fan <[email protected]>
> > > Sent: Friday, April 24, 2020 9:12 AM
> > > >
> > > > > From: Peng Fan <[email protected]>
> > > > > Sent: Thursday, April 23, 2020 6:57 PM
> > > > > > > From: Peng Fan <[email protected]>
> > > > > > > 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 <[email protected]>
> > > > > > > Signed-off-by: Peng Fan <[email protected]>
> > > > > >
> > > > > > 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
> From: Peng Fan <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> 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
> Subject: RE: [PATCH 2/4] firmware: imx: add resource management api
>
> > From: Peng Fan <[email protected]>
> > Sent: Monday, June 1, 2020 8:40 PM
> > >
> > > > From: Peng Fan <[email protected]>
> > > > Sent: Friday, April 24, 2020 9:12 AM
> > > > >
> > > > > > From: Peng Fan <[email protected]>
> > > > > > Sent: Thursday, April 23, 2020 6:57 PM
> > > > > > > > From: Peng Fan <[email protected]>
> > > > > > > > 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 <[email protected]>
> > > > > > > > Signed-off-by: Peng Fan <[email protected]>
> > > > > > >
> > > > > > > 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
> Subject: RE: [PATCH 2/4] firmware: imx: add resource management api
>
> > From: Peng Fan <[email protected]>
> > 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 <[email protected]>
> > Signed-off-by: Peng Fan <[email protected]>
> > ---
> > 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
> From: Peng Fan <[email protected]>
> Sent: Tuesday, June 2, 2020 1:24 PM
>
> > Subject: RE: [PATCH 2/4] firmware: imx: add resource management api
> >
> > > From: Peng Fan <[email protected]>
> > > 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 <[email protected]>
> > > Signed-off-by: Peng Fan <[email protected]>
> > > ---
> > > 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
> From: Peng Fan <[email protected]>
> Sent: Tuesday, June 2, 2020 12:51 PM
> >
> > > From: Peng Fan <[email protected]>
> > > Sent: Monday, June 1, 2020 8:40 PM
> > > >
> > > > > From: Peng Fan <[email protected]>
> > > > > Sent: Friday, April 24, 2020 9:12 AM
> > > > > >
> > > > > > > From: Peng Fan <[email protected]>
> > > > > > > Sent: Thursday, April 23, 2020 6:57 PM
> > > > > > > > > From: Peng Fan <[email protected]>
> > > > > > > > > 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 <[email protected]>
> > > > > > > > > Signed-off-by: Peng Fan <[email protected]>
> > > > > > > >
> > > > > > > > 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
> Subject: RE: [PATCH 2/4] firmware: imx: add resource management api
>
> > From: Peng Fan <[email protected]>
> > Sent: Tuesday, June 2, 2020 12:51 PM
> > >
> > > > From: Peng Fan <[email protected]>
> > > > Sent: Monday, June 1, 2020 8:40 PM
> > > > >
> > > > > > From: Peng Fan <[email protected]>
> > > > > > Sent: Friday, April 24, 2020 9:12 AM
> > > > > > >
> > > > > > > > From: Peng Fan <[email protected]>
> > > > > > > > Sent: Thursday, April 23, 2020 6:57 PM
> > > > > > > > > > From: Peng Fan <[email protected]>
> > > > > > > > > > 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 <[email protected]>
> > > > > > > > > > Signed-off-by: Peng Fan <[email protected]>
> > > > > > > > >
> > > > > > > > > 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
> From: Peng Fan <[email protected]>
> Sent: Tuesday, June 2, 2020 3:48 PM
> >
> > > From: Peng Fan <[email protected]>
> > > Sent: Tuesday, June 2, 2020 12:51 PM
> > > >
> > > > > From: Peng Fan <[email protected]>
> > > > > Sent: Monday, June 1, 2020 8:40 PM
> > > > > >
> > > > > > > From: Peng Fan <[email protected]>
> > > > > > > Sent: Friday, April 24, 2020 9:12 AM
> > > > > > > >
> > > > > > > > > From: Peng Fan <[email protected]>
> > > > > > > > > Sent: Thursday, April 23, 2020 6:57 PM
> > > > > > > > > > > From: Peng Fan <[email protected]>
> > > > > > > > > > > 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
> > > > > > > > > > > <[email protected]>
> > > > > > > > > > > Signed-off-by: Peng Fan <[email protected]>
> > > > > > > > > >
> > > > > > > > > > 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