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

From: Aisheng Dong
Date: Tue Jun 02 2020 - 03:12:41 EST

Next message: Jason Wang: "Re: [PATCH 5/6] vdpa: introduce virtio pci driver"
Previous message: Gustavo Pimentel: "RE: [PATCH v1] PCI: dwc: convert to devm_platform_ioremap_resource_byname()"
In reply to: Peng Fan: "RE: [PATCH 2/4] firmware: imx: add resource management api"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

> From: Peng Fan <peng.fan@xxxxxxx>
> Sent: Tuesday, June 2, 2020 1:24 PM
>
> > Subject: RE: [PATCH 2/4] firmware: imx: add resource management api
> >
> > > 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>
> > > ---
> > > 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

Next message: Jason Wang: "Re: [PATCH 5/6] vdpa: introduce virtio pci driver"
Previous message: Gustavo Pimentel: "RE: [PATCH v1] PCI: dwc: convert to devm_platform_ioremap_resource_byname()"
In reply to: Peng Fan: "RE: [PATCH 2/4] firmware: imx: add resource management api"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]