RE: [PATCH 2/4] firmware: imx: add resource management api

From: Aisheng Dong
Date: Thu Apr 23 2020 - 22:05:06 EST

Next message: Leizhen (ThunderTown): "Re: [PATCH 1/1] brd: remove a redundant check"
Previous message: Huang\, Ying: "Re: [PATCH v2] mm/swapfile.c: simplify the scan loop in scan_swap_map_slots()"
In reply to: Peng Fan: "RE: [PATCH 2/4] firmware: imx: add resource management api"
Next in thread: peng . fan: "[PATCH 3/4] firmware: imx: scu-pd: ignore power domain not owned"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

> From: Peng Fan <peng.fan@xxxxxxx>
> Sent: Friday, April 24, 2020 9:12 AM
> >
> > > From: Peng Fan <peng.fan@xxxxxxx>
> > > Sent: Thursday, April 23, 2020 6:57 PM
> > > > > From: Peng Fan <peng.fan@xxxxxxx>
> > > > > Sent: Thursday, April 23, 2020 3:00 PM
> > > > >
> > > > > Add resource management API, when we have multiple partition
> > > > > running together, resources not owned to current partition
> > > > > should not be
> > used.
> > > > >
> > > > > Reviewed-by: Leonard Crestez <leonard.crestez@xxxxxxx>
> > > > > Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> > > >
> > > > 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

Next message: Leizhen (ThunderTown): "Re: [PATCH 1/1] brd: remove a redundant check"
Previous message: Huang\, Ying: "Re: [PATCH v2] mm/swapfile.c: simplify the scan loop in scan_swap_map_slots()"
In reply to: Peng Fan: "RE: [PATCH 2/4] firmware: imx: add resource management api"
Next in thread: peng . fan: "[PATCH 3/4] firmware: imx: scu-pd: ignore power domain not owned"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]