Re: [PATCH v20 3/7] firmware: imx: add driver for NXP EdgeLock Enclave

[Krzysztof Kozlowski <krzk@kernel.org>]

On 03/12/2025 17:59, Frank Li wrote:
> On Wed, Dec 03, 2025 at 08:20:50AM +0100, Krzysztof Kozlowski wrote:
>> On 03/12/2025 07:48, Pankaj Gupta wrote:
>>> Add driver for enabling MU based communication interface to secure-enclave.
>>>
>>> NXP hardware IP(s) for secure-enclaves like Edgelock Enclave(ELE), are
>>> embedded in the SoC to support the features like HSM, SHE & V2X, using
>>> message based communication interface.
>>>
>>> The secure enclave FW communicates with Linux over single or multiple
>>> dedicated messaging unit(MU) based interface(s).
>>> Exists on i.MX SoC(s) like i.MX8ULP, i.MX93, i.MX95 etc.
>>>
>>> For i.MX9x SoC(s) there is at least one dedicated ELE MU(s) for each
>>> world - Linux(one or more) and OPTEE-OS (one or more).
>>>
>>> Other dependent kernel drivers will be:
>>> - NVMEM: that supports non-volatile devices like EFUSES,
>>>          managed by NXP's secure-enclave.
>>>
>>> Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
>>> Reviewed-by: Frank Li <Frank.Li@nxp.com>
>>> ---
>>>  drivers/firmware/imx/Kconfig        |  13 ++
>>>  drivers/firmware/imx/Makefile       |   2 +
>>>  drivers/firmware/imx/ele_base_msg.c | 269 ++++++++++++++++++++++++
>>>  drivers/firmware/imx/ele_base_msg.h |  95 +++++++++
>>>  drivers/firmware/imx/ele_common.c   | 333 ++++++++++++++++++++++++++++++
>>>  drivers/firmware/imx/ele_common.h   |  45 ++++
>>>  drivers/firmware/imx/se_ctrl.c      | 401 ++++++++++++++++++++++++++++++++++++
>>>  drivers/firmware/imx/se_ctrl.h      |  86 ++++++++
>>>  include/linux/firmware/imx/se_api.h |  14 ++
>>>  9 files changed, 1258 insertions(+)
>>>
>>> diff --git a/drivers/firmware/imx/Kconfig b/drivers/firmware/imx/Kconfig
>>> index 127ad752acf8..5fe96299b704 100644
>>> --- a/drivers/firmware/imx/Kconfig
>>> +++ b/drivers/firmware/imx/Kconfig
>>> @@ -55,3 +55,16 @@ config IMX_SCMI_MISC_DRV
>>>  	  core that could provide misc functions such as board control.
>>>
>>>  	  This driver can also be built as a module.
>>> +
>>> +config IMX_SEC_ENCLAVE
>>> +	tristate "i.MX Embedded Secure Enclave - EdgeLock Enclave Firmware driver."
>>> +	depends on IMX_MBOX && ARCH_MXC && ARM64
>>> +	select FW_LOADER
>>> +	default m if ARCH_MXC
>>> +
>>> +	help
>>> +	  Exposes APIs supported by the iMX Secure Enclave HW IP called:
>>> +	  - EdgeLock Enclave Firmware (for i.MX8ULP, i.MX93),
>>> +	    like base, HSM, V2X & SHE using the SAB protocol via the shared Messaging
>>> +	    Unit. This driver exposes these interfaces via a set of file descriptors
>>> +	    allowing to configure shared memory, send and receive messages.
>>> diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
>>> index 3bbaffa6e347..4412b15846b1 100644
>>> --- a/drivers/firmware/imx/Makefile
>>> +++ b/drivers/firmware/imx/Makefile
>>> @@ -4,3 +4,5 @@ obj-$(CONFIG_IMX_SCU)		+= imx-scu.o misc.o imx-scu-irq.o rm.o imx-scu-soc.o
>>>  obj-${CONFIG_IMX_SCMI_CPU_DRV}	+= sm-cpu.o
>>>  obj-${CONFIG_IMX_SCMI_MISC_DRV}	+= sm-misc.o
>>>  obj-${CONFIG_IMX_SCMI_LMM_DRV}	+= sm-lmm.o
>>> +sec_enclave-objs		= se_ctrl.o ele_common.o ele_base_msg.o
>>> +obj-${CONFIG_IMX_SEC_ENCLAVE}	+= sec_enclave.o
>>> diff --git a/drivers/firmware/imx/ele_base_msg.c b/drivers/firmware/imx/ele_base_msg.c
>>> new file mode 100644
>>> index 000000000000..a070acbd895c
>>> --- /dev/null
>>> +++ b/drivers/firmware/imx/ele_base_msg.c
>>> @@ -0,0 +1,269 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * Copyright 2025 NXP
>>> + */
>>> +
>>> +#include <linux/types.h>
>>> +
>>> +#include <linux/completion.h>
>>> +#include <linux/dma-mapping.h>
>>> +#include <linux/genalloc.h>
>>> +
>>> +#include "ele_base_msg.h"
>>> +#include "ele_common.h"
>>> +
>>> +#define FW_DBG_DUMP_FIXED_STR		"ELE"
>>> +
>>> +int ele_get_info(struct se_if_priv *priv, struct ele_dev_info *s_info)
>>> +{
>>> +	struct se_api_msg *tx_msg __free(kfree) = NULL;
>>> +	struct se_api_msg *rx_msg __free(kfree) = NULL;
>>
>> No, don't use this syntax. This is explicitly discouraged.
>>
>> NAK
> 
> Add link for reference.
> 
> https://lore.kernel.org/all/CAHk-=whPZoi03ZwphxiW6cuWPtC3nyKYS8_BThgztCdgPWP1WA@mail.gmail.com/

This is since beginning documented in kernel, so contributor could read
cleanup docs before using them. Above Linus remark is nothing new, he
already wrote it ~2 years ago.

Best regards,
Krzysztof