On 2/2/24 20:53, Mathieu Poirier wrote:
> On Thu, Feb 01, 2024 at 07:33:35PM +0100, Arnaud POULIQUEN wrote:
>>
>>
>> On 2/1/24 17:02, Mathieu Poirier wrote:
>>> On Thu, Feb 01, 2024 at 04:06:37PM +0100, Arnaud POULIQUEN wrote:
>>>> hello Mathieu,
>>>>
>>>> On 1/31/24 19:52, Mathieu Poirier wrote:
>>>>> On Tue, Jan 30, 2024 at 10:13:48AM +0100, Arnaud POULIQUEN wrote:
>>>>>>
>>>>>>
>>>>>> On 1/26/24 18:11, Mathieu Poirier wrote:
>>>>>>> On Thu, Jan 18, 2024 at 11:04:33AM +0100, Arnaud Pouliquen wrote:
>>>>>>>> The new TEE remoteproc device is used to manage remote firmware in a
>>>>>>>> secure, trusted context. The 'st,stm32mp1-m4-tee' compatibility is
>>>>>>>> introduced to delegate the loading of the firmware to the trusted
>>>>>>>> execution context. In such cases, the firmware should be signed and
>>>>>>>> adhere to the image format defined by the TEE.
>>>>>>>>
>>>>>>>> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@xxxxxxxxxxx>
>>>>>>>> ---
>>>>>>>> V1 to V2 update:
>>>>>>>> - remove the select "TEE_REMOTEPROC" in STM32_RPROC config as detected by
>>>>>>>> the kernel test robot:
>>>>>>>> WARNING: unmet direct dependencies detected for TEE_REMOTEPROC
>>>>>>>> Depends on [n]: REMOTEPROC [=y] && OPTEE [=n]
>>>>>>>> Selected by [y]:
>>>>>>>> - STM32_RPROC [=y] && (ARCH_STM32 || COMPILE_TEST [=y]) && REMOTEPROC [=y]
>>>>>>>> - Fix initialized trproc variable in stm32_rproc_probe
>>>>>>>> ---
>>>>>>>> drivers/remoteproc/stm32_rproc.c | 149 +++++++++++++++++++++++++++++--
>>>>>>>> 1 file changed, 144 insertions(+), 5 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
>>>>>>>> index fcc0001e2657..cf6a21bac945 100644
>>>>>>>> --- a/drivers/remoteproc/stm32_rproc.c
>>>>>>>> +++ b/drivers/remoteproc/stm32_rproc.c
>>>>>>>> @@ -20,6 +20,7 @@
>>>>>>>> #include <linux/remoteproc.h>
>>>>>>>> #include <linux/reset.h>
>>>>>>>> #include <linux/slab.h>
>>>>>>>> +#include <linux/tee_remoteproc.h>
>>>>>>>> #include <linux/workqueue.h>
>>>>>>>>
>>>>>>>> #include "remoteproc_internal.h"
>>>>>>>> @@ -49,6 +50,9 @@
>>>>>>>> #define M4_STATE_STANDBY 4
>>>>>>>> #define M4_STATE_CRASH 5
>>>>>>>>
>>>>>>>> +/* Remote processor unique identifier aligned with the Trusted Execution Environment definitions */
>>>>>>>> +#define STM32_MP1_M4_PROC_ID 0
>>>>>>>> +
>>>>>>>> struct stm32_syscon {
>>>>>>>> struct regmap *map;
>>>>>>>> u32 reg;
>>>>>>>> @@ -90,6 +94,8 @@ struct stm32_rproc {
>>>>>>>> struct stm32_mbox mb[MBOX_NB_MBX];
>>>>>>>> struct workqueue_struct *workqueue;
>>>>>>>> bool hold_boot_smc;
>>>>>>>> + bool fw_loaded;
>>>>>>>> + struct tee_rproc *trproc;
>>>>>>>> void __iomem *rsc_va;
>>>>>>>> };
>>>>>>>>
>>>>>>>> @@ -257,6 +263,91 @@ static int stm32_rproc_release(struct rproc *rproc)
>>>>>>>> return err;
>>>>>>>> }
>>>>>>>>
>>>>>>>> +static int stm32_rproc_tee_elf_sanity_check(struct rproc *rproc,
>>>>>>>> + const struct firmware *fw)
>>>>>>>> +{
>>>>>>>> + struct stm32_rproc *ddata = rproc->priv;
>>>>>>>> + unsigned int ret = 0;
>>>>>>>> +
>>>>>>>> + if (rproc->state == RPROC_DETACHED)
>>>>>>>> + return 0;
>>>>>>>> +
>>>>>>>> + ret = tee_rproc_load_fw(ddata->trproc, fw);
>>>>>>>> + if (!ret)
>>>>>>>> + ddata->fw_loaded = true;
>>>>>>>> +
>>>>>>>> + return ret;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static int stm32_rproc_tee_elf_load(struct rproc *rproc,
>>>>>>>> + const struct firmware *fw)
>>>>>>>> +{
>>>>>>>> + struct stm32_rproc *ddata = rproc->priv;
>>>>>>>> + unsigned int ret;
>>>>>>>> +
>>>>>>>> + /*
>>>>>>>> + * This function can be called by remote proc for recovery
>>>>>>>> + * without the sanity check. In this case we need to load the firmware
>>>>>>>> + * else nothing done here as the firmware has been preloaded for the
>>>>>>>> + * sanity check to be able to parse it for the resource table.
>>>>>>>> + */
>>>>>>>
>>>>>>> This comment is very confusing - please consider refactoring.
>>>>>>>
>>>>>>>> + if (ddata->fw_loaded)
>>>>>>>> + return 0;
>>>>>>>> +
>>>>>>>
>>>>>>> I'm not sure about keeping a flag to indicate the status of the loaded firmware.
>>>>>>> It is not done for the non-secure method, I don't see why it would be needed for
>>>>>>> the secure one.
>>>>>>>
>>>>>>
>>>>>> The difference is on the sanity check.
>>>>>> - in rproc_elf_sanity_check we parse the elf file to verify that it is
>>>>>> valid.
>>>>>> - in stm32_rproc_tee_elf_sanity_check we have to do the same, that means to
>>>>>> authenticate it. the authentication is done during the load.
>>>>>>
>>>>>> So this flag is used to avoid to reload it twice time.
>>>>>> refactoring the comment should help to understand this flag
>>>>>>
>>>>>>
>>>>>> An alternative would be to bypass the sanity check. But this lead to same
>>>>>> limitation.
>>>>>> Before loading the firmware in remoteproc_core, we call rproc_parse_fw() that is
>>>>>> used to get the resource table address. To get it from tee we need to
>>>>>> authenticate the firmware so load it...
>>>>>>
>>>>>
>>>>> I spent a long time thinking about this patchset. Looking at the code as it
>>>>> is now, request_firmware() in rproc_boot() is called even when the TEE is
>>>>> responsible for loading the firmware. There should be some conditional code
>>>>> that calls either request_firmware() or tee_rproc_load_fw(). The latter should
>>>>> also be renamed to tee_rproc_request_firmware() to avoid confusion.
>>>>
>>>>
>>>> The request_firmware() call is needed in both cases to get the image from the
>>>> filesystem. The tee_rproc_load_fw() gets, as input, the struct firmware provided
>>>> by request_firmware().
>>>
>>> The cover letter clearly state the secure side is responsible for loading the
>>> firmware image but here you're telling me it has to be loaded twice. This is
>>> very confusing.
>>
>> Concerning the call of request_firmware()
>>
>> By "both cases" I would say that the call of request_firmware() is needed in
>> both modes:
>> - the ELF firmware is parsed and loaded by linux (legacy)
>> - the binary firmware is parsed and loaded by OP-TEE.
>>
>> The Op-TEE is not able to get the firmware image from the file system.
>>
>>
>> Concerning the call of tee_rproc_load_fw twice time
>>
>> There are 2 use cases:
>>
>> - First boot of the remote processor:
>>
>> 1) The Linux rproc gets the binary firmware image from the file system by
>> calling request_firmware(). A copy is stored in memory.
>
> Right. And I think tee_rproc_load_fw() should be called right after
> request_firmware() if rproc::tee_rproc_interface is valid. At that point the TEE
> app may or may not do the firmware authentication, that is application specific.
>
>> 2) the linux performs a sanity check on the firmware calling
>> rproc_fw_sanity_check()
>> => from OP-TEE point of view this means to autenticate the firmware
>> => let consider in this exemple that we bypass this step
>> (ops->sanity_check = NULL)
>
> Ok
>
>>
>> 3) the linux rproc call rproc_parse_fw() to get the resource table
>> => From OP-TEE point of view the resource table is available only when
>> the firmware is loaded
>
> Right, and it should have been loaded already. If it is not then the TEE should
> return an error.
>
>> => We need to call tee_rproc_load_fw() to be able then to get the
>> address of the resource table.
>
> See my comment above - at this point the TEE should already have the firmware.
> As such the only thing left is to get the address of the resource table, which
> you already do in rproc_tee_get_rsc_table(). The upper part of that function
> should be spun off in a new static function to deal with the TEE API, something
> like _rproc_tee_get_rsc_table(). The new function should also be called in
> tee_rproc_get_loaded_rsc_table() rather than keeping a cache value in
> trproc->rsc_va.
>
>> 4) The Linux rproc calls rproc_handle_resources() to parse the resource table.
>> 5) The linux rproc calls rproc_start()
>> - load the firrmware calling rproc_load_segments()
>> => we don't want to call tee_rproc_load_fw() it a second time
>
> And that is fine if the TEE app has already placed the program segments in
> memory.
>
>> - start the firmware calling ops->start()
>>
>> - Reboot on crash recovery using rproc_boot_recovery()
>>
>> 1) The Linux rproc gets the binary firmware image from the file system by
>> calling request_firmware(). A copy is stored in memory.
>> 5) The linux rproc calls rproc_start()
>> - load the firrmware calling rproc_load_segments()
>> => we have to call tee_rproc_load_fw() to reload the firmware
>
> Loading the firmware in the TEE should be done right after request_firmware()
> has been called, the same way it is done in the boot path. If there isn't a
> need to reload the TEE firmware than the TEE application should ignore the
> request.
I need to prototype to verify this proposal.
I will come back with a V3.
Thank you for the advice and review!
Regard,
Arnaud
>
>> - start the firmware calling ops->start()
>>
>> In first use case we have to load the firmware on rproc_parse_fw(), in second
>> usecase on rproc_load_segments().
>>
>> This is the point I have tried to solve with the ddata->fw_loaded variable.
>>
>>>
>>> I'm also confused as to why stm32_rproc_tee_elf_sanity_check() is calling
>>> tee_rproc_load_fw(). There should be one call to load the firmware and another
>>> to perform a sanity check on it. If the sanity check is done at load time by
>>> the secure world then ops::sanity_check() is NULL.
>>
>> Sure, make sense to remove the sanity_check ops
>>
>> Thanks,
>> Arnaud
>>
>>>
>>> Most of what this patchset does makes sense, but some of it needs to be moved
>>> around.
>>>
>>> Thanks,
>>> Mathieu
>>>
>>>>
>>>> If we want to integrate in remoteproc_core the solution could probably have to
>>>> create the equivalent of the rproc_fw_boot() to load the firmware with an
>>>> external method. Here is an example based on a new rproc_ops ( not tested)
>>>>
>>>> + static int rproc_fw_ext_boot(struct rproc *rproc, const struct firmware *fw)
>>>> + {
>>>> + struct device *dev = &rproc->dev;
>>>> + const char *name = rproc->firmware;
>>>> + int ret;
>>>> +
>>>> +
>>>> + dev_info(dev, "Booting fw image %s, size %zd\n", name, fw->size);
>>>> +
>>>> + /* ops to load and start the remoteprocessor */
>>>> + ret = rproc->ops->boot(rproc, fw);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + /*
>>>> + * if enabling an IOMMU isn't relevant for this rproc, this is
>>>> + * just a nop
>>>> + */
>>>> + ret = rproc_enable_iommu(rproc);
>>>> + if (ret) {
>>>> + dev_err(dev, "can't enable iommu: %d\n", ret);
>>>> + return ret;
>>>> + }
>>>> +
>>>> + /* Prepare rproc for firmware loading if needed */
>>>> + ret = rproc_prepare_device(rproc);
>>>> + if (ret) {
>>>> + dev_err(dev, "can't prepare rproc %s: %d\n", rproc->name, ret);
>>>> + goto disable_iommu;
>>>> + }
>>>> +
>>>> + ret = rproc_set_rsc_table(rproc);
>>>> + if (ret) {
>>>> + dev_err(dev, "can't load resource table: %d\n", ret);
>>>> + goto unprepare_device;
>>>> + }
>>>> +
>>>> +
>>>> + /* reset max_notifyid */
>>>> + rproc->max_notifyid = -1;
>>>> +
>>>> + /* reset handled vdev */
>>>> + rproc->nb_vdev = 0;
>>>> +
>>>> + /* handle fw resources which are required to boot rproc */
>>>> + ret = rproc_handle_resources(rproc, rproc_loading_handlers);
>>>> + if (ret) {
>>>> + dev_err(dev, "Failed to process resources: %d\n", ret);
>>>> + goto clean_up_resources;
>>>> + }
>>>> +
>>>> + /* Allocate carveout resources associated to rproc */
>>>> + ret = rproc_alloc_registered_carveouts(rproc);
>>>> + if (ret) {
>>>> + dev_err(dev, "Failed to allocate associated carveouts: %d\n",
>>>> + ret);
>>>> + goto clean_up_resources;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +
>>>> + clean_up_resources:
>>>> + rproc_resource_cleanup(rproc);
>>>> + unprepare_rproc:
>>>> + /* release HW resources if needed */
>>>> + rproc_unprepare_device(rproc);
>>>> + disable_iommu:
>>>> + rproc_disable_iommu(rproc);
>>>> + return ret;
>>>> + }
>>>>
>>>>
>>>> int rproc_boot(struct rproc *rproc)
>>>> {
>>>> [...]
>>>>
>>>> - ret = rproc_fw_boot(rproc, firmware_p);
>>>> + if(rproc->ops->boot)
>>>> + ret = rproc_fw_ext_boot(rproc, firmware_p);
>>>> + else
>>>> + ret = rproc_fw_boot(rproc, firmware_p);
>>>>
>>>> Another advantage of this solution is that it opens the framework to other
>>>> formats. For instance it could be a way to support dtb format requested in [RFC]
>>>> Passing device-tree to remoteproc [1].
>>>>
>>>> [1]
>>>> https://lore.kernel.org/linux-remoteproc/f67cd822-4e29-71f2-7c42-e11dbaa6cd8c@xxxxxxxxxxxxx/T/#t
>>>>
>>>> Thanks,
>>>> Arnaud
>>>>
>>>>
>>>>
>>>>>
>>>>> I touched on that before but please rename rproc_tee_get_rsc_table() to
>>>>> rproc_tee_elf_load_rsc_table(). I also suggest to introduce a new function,
>>>>> rproc_tee_get_loaded_rsc_table() that would be called from
>>>>> rproc_tee_elf_load_rsc_table(). That way we don't need trproc->rsc_va.
>>>>>
>>>>> I also think tee_rproc should be renamed to "rproc_tee_interface" and folded
>>>>> under struct rproc.
>>>>>
>>>>> With the above most of the problems with the current implementation should
>>>>> naturally go away.
>>>>>
>>>>> Thanks,
>>>>> Mathieu
>>>>>
>>>>>>
>>>>>>>> + ret = tee_rproc_load_fw(ddata->trproc, fw);
>>>>>>>> + if (ret)
>>>>>>>> + return ret;
>>>>>>>> + ddata->fw_loaded = true;
>>>>>>>> +
>>>>>>>> + /* Update the resource table parameters. */
>>>>>>>> + if (rproc_tee_get_rsc_table(ddata->trproc)) {
>>>>>>>> + /* No resource table: reset the related fields. */
>>>>>>>> + rproc->cached_table = NULL;
>>>>>>>> + rproc->table_ptr = NULL;
>>>>>>>> + rproc->table_sz = 0;
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + return 0;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static struct resource_table *
>>>>>>>> +stm32_rproc_tee_elf_find_loaded_rsc_table(struct rproc *rproc,
>>>>>>>> + const struct firmware *fw)
>>>>>>>> +{
>>>>>>>> + struct stm32_rproc *ddata = rproc->priv;
>>>>>>>> +
>>>>>>>> + return tee_rproc_get_loaded_rsc_table(ddata->trproc);
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static int stm32_rproc_tee_start(struct rproc *rproc)
>>>>>>>> +{
>>>>>>>> + struct stm32_rproc *ddata = rproc->priv;
>>>>>>>> +
>>>>>>>> + return tee_rproc_start(ddata->trproc);
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static int stm32_rproc_tee_attach(struct rproc *rproc)
>>>>>>>> +{
>>>>>>>> + /* Nothing to do, remote proc already started by the secured context. */
>>>>>>>> + return 0;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static int stm32_rproc_tee_stop(struct rproc *rproc)
>>>>>>>> +{
>>>>>>>> + struct stm32_rproc *ddata = rproc->priv;
>>>>>>>> + int err;
>>>>>>>> +
>>>>>>>> + stm32_rproc_request_shutdown(rproc);
>>>>>>>> +
>>>>>>>> + err = tee_rproc_stop(ddata->trproc);
>>>>>>>> + if (err)
>>>>>>>> + return err;
>>>>>>>> +
>>>>>>>> + ddata->fw_loaded = false;
>>>>>>>> +
>>>>>>>> + return stm32_rproc_release(rproc);
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> static int stm32_rproc_prepare(struct rproc *rproc)
>>>>>>>> {
>>>>>>>> struct device *dev = rproc->dev.parent;
>>>>>>>> @@ -319,7 +410,14 @@ static int stm32_rproc_prepare(struct rproc *rproc)
>>>>>>>>
>>>>>>>> static int stm32_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
>>>>>>>> {
>>>>>>>> - if (rproc_elf_load_rsc_table(rproc, fw))
>>>>>>>> + struct stm32_rproc *ddata = rproc->priv;
>>>>>>>> + int ret;
>>>>>>>> +
>>>>>>>> + if (ddata->trproc)
>>>>>>>> + ret = rproc_tee_get_rsc_table(ddata->trproc);
>>>>>>>> + else
>>>>>>>> + ret = rproc_elf_load_rsc_table(rproc, fw);
>>>>>>>> + if (ret)
>>>>>>>> dev_warn(&rproc->dev, "no resource table found for this firmware\n");
>>>>>>>>
>>>>>>>> return 0;
>>>>>>>> @@ -693,8 +791,22 @@ static const struct rproc_ops st_rproc_ops = {
>>>>>>>> .get_boot_addr = rproc_elf_get_boot_addr,
>>>>>>>> };
>>>>>>>>
>>>>>>>> +static const struct rproc_ops st_rproc_tee_ops = {
>>>>>>>> + .prepare = stm32_rproc_prepare,
>>>>>>>> + .start = stm32_rproc_tee_start,
>>>>>>>> + .stop = stm32_rproc_tee_stop,
>>>>>>>> + .attach = stm32_rproc_tee_attach,
>>>>>>>> + .kick = stm32_rproc_kick,
>>>>>>>> + .parse_fw = stm32_rproc_parse_fw,
>>>>>>>> + .find_loaded_rsc_table = stm32_rproc_tee_elf_find_loaded_rsc_table,
>>>>>>>> + .get_loaded_rsc_table = stm32_rproc_get_loaded_rsc_table,
>>>>>>>> + .sanity_check = stm32_rproc_tee_elf_sanity_check,
>>>>>>>> + .load = stm32_rproc_tee_elf_load,
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> static const struct of_device_id stm32_rproc_match[] = {
>>>>>>>> - { .compatible = "st,stm32mp1-m4" },
>>>>>>>> + {.compatible = "st,stm32mp1-m4",},
>>>>>>>> + {.compatible = "st,stm32mp1-m4-tee",},
>>>>>>>> {},
>>>>>>>> };
>>>>>>>> MODULE_DEVICE_TABLE(of, stm32_rproc_match);
>>>>>>>> @@ -853,6 +965,7 @@ static int stm32_rproc_probe(struct platform_device *pdev)
>>>>>>>> struct device *dev = &pdev->dev;
>>>>>>>> struct stm32_rproc *ddata;
>>>>>>>> struct device_node *np = dev->of_node;
>>>>>>>> + struct tee_rproc *trproc = NULL;
>>>>>>>> struct rproc *rproc;
>>>>>>>> unsigned int state;
>>>>>>>> int ret;
>>>>>>>> @@ -861,11 +974,31 @@ static int stm32_rproc_probe(struct platform_device *pdev)
>>>>>>>> if (ret)
>>>>>>>> return ret;
>>>>>>>>
>>>>>>>> - rproc = rproc_alloc(dev, np->name, &st_rproc_ops, NULL, sizeof(*ddata));
>>>>>>>> - if (!rproc)
>>>>>>>> - return -ENOMEM;
>>>>>>>> + if (of_device_is_compatible(np, "st,stm32mp1-m4-tee")) {
>>>>>>>> + trproc = tee_rproc_register(dev, STM32_MP1_M4_PROC_ID);
>>>>>>>> + if (IS_ERR(trproc)) {
>>>>>>>> + dev_err_probe(dev, PTR_ERR(trproc),
>>>>>>>> + "signed firmware not supported by TEE\n");
>>>>>>>> + return PTR_ERR(trproc);
>>>>>>>> + }
>>>>>>>> + /*
>>>>>>>> + * Delegate the firmware management to the secure context.
>>>>>>>> + * The firmware loaded has to be signed.
>>>>>>>> + */
>>>>>>>> + dev_info(dev, "Support of signed firmware only\n");
>>>>>>>
>>>>>>> Not sure what this adds. Please remove.
>>>>>>
>>>>>> This is used to inform the user that only a signed firmware can be loaded, not
>>>>>> an ELF file.
>>>>>> I have a patch in my pipe to provide the supported format in the debugfs. In a
>>>>>> first step, I can suppress this message and we can revisit the issue when I push
>>>>>> the debugfs proposal.
>>>>>>
>>>>>> Thanks,
>>>>>> Arnaud
>>>>>>
>>>>>>>
>>>>>>>> + }
>>>>>>>> + rproc = rproc_alloc(dev, np->name,
>>>>>>>> + trproc ? &st_rproc_tee_ops : &st_rproc_ops,
>>>>>>>> + NULL, sizeof(*ddata));
>>>>>>>> + if (!rproc) {
>>>>>>>> + ret = -ENOMEM;
>>>>>>>> + goto free_tee;
>>>>>>>> + }
>>>>>>>>
>>>>>>>> ddata = rproc->priv;
>>>>>>>> + ddata->trproc = trproc;
>>>>>>>> + if (trproc)
>>>>>>>> + trproc->rproc = rproc;
>>>>>>>>
>>>>>>>> rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
>>>>>>>>
>>>>>>>> @@ -916,6 +1049,10 @@ static int stm32_rproc_probe(struct platform_device *pdev)
>>>>>>>> device_init_wakeup(dev, false);
>>>>>>>> }
>>>>>>>> rproc_free(rproc);
>>>>>>>> +free_tee:
>>>>>>>> + if (trproc)
>>>>>>>> + tee_rproc_unregister(trproc);
>>>>>>>> +
>>>>>>>> return ret;
>>>>>>>> }
>>>>>>>>
>>>>>>>> @@ -937,6 +1074,8 @@ static void stm32_rproc_remove(struct platform_device *pdev)
>>>>>>>> device_init_wakeup(dev, false);
>>>>>>>> }
>>>>>>>> rproc_free(rproc);
>>>>>>>> + if (ddata->trproc)
>>>>>>>> + tee_rproc_unregister(ddata->trproc);
>>>>>>>> }
>>>>>>>>
>>>>>>>> static int stm32_rproc_suspend(struct device *dev)
>>>>>>>> --
>>>>>>>> 2.25.1
>>>>>>>>
