RE: [EXT] Re: [PATCH v7 4/5] firmware: imx: add driver for NXP EdgeLock Enclave

[Pankaj Gupta]

-----Original Message-----
From: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> 
Sent: Tuesday, October 1, 2024 4:03 PM
To: Pankaj Gupta <pankaj.gupta@xxxxxxx>
Cc: Jonathan Corbet <corbet@xxxxxxx>; Rob Herring <robh@xxxxxxxxxx>;
Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>; Conor Dooley
<conor+dt@xxxxxxxxxx>; Shawn Guo <shawnguo@xxxxxxxxxx>; Pengutronix Kernel
Team <kernel@xxxxxxxxxxxxxx>; Fabio Estevam <festevam@xxxxxxxxx>; Rob
Herring <robh+dt@xxxxxxxxxx>; linux-doc@xxxxxxxxxxxxxxx;
linux-kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
imx@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
Subject: Re: [EXT] Re: [PATCH v7 4/5] firmware: imx: add driver for NXP
EdgeLock Enclave

Caution: This is an external email. Please take care when clicking links or
opening attachments. When in doubt, report the message using the 'Report
this email' button


On Tue, Oct 01, 2024 at 07:49:39AM +0000, Pankaj Gupta wrote:
>> >> >Either compile the firmware into the kernel or the ELE driver as
module.
>> >>
>> >> Cannot compile as part of Firmware.
>> >> There are OTA scenarios where the latest FW, that is downloaded to 
>> >> replace the image in RFS, and FW needs to be re-init with this new 
>> >> image, by putting the Linux to power-down state.
>> >
>> >> ELE driver is compiled as module only, by default. But if someone 
>> >> like to make it as in-line to kernel image, still it should work.
>>
>> > I am also not very happy with the situation that we can't compile 
>> > drivers
>> into the kernel and just get the firmware later once it is available.
>>
>> Driver is enabling the ROM API supports at probe time.
>> And, once the rootfs is available, and the Firmware image is loaded, 
>> then it will enable the complete set of FW API(s), along with ROM API(s).
>>
>> Hence, Driver can be compiled into the kernel to enable ELE-ROM API(s).

> I see what the code does, I just don't think that it's safe to assume that
the rootfs will be mounted after the 50*20ms timeout you use.

> I also think that it's a valid usecase for builtin code to retry firmware
loading after the rootfs has been mounted. This should be integrated into
the firmware loading code though and not be done as a driver specific hack.

> Anyway, it won't be me who merges this code and I am just telling you that
you'll likely have a problem getting this upstream as is.

I have another way to predictably handle in the current flow.
 -  Will add the check in the "se_ioctl_cmd_snd_rcv_rsp_handler", when the
first FW API,  is exercised, the FW will get loaded.

If you agree, I will make the change in V8 and send the patch.

> Sascha

--
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       |
https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pengutr
onix.de%2F&data=05%7C02%7Cpankaj.gupta%40nxp.com%7C7ce6df77007f47dfa74c08dce
2046437%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638633755716236681%7CUn
known%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX
VCI6Mn0%3D%7C0%7C%7C%7C&sdata=YU78kbX%2F9JXaI9WFN2RcIyvV23qU8TvqCjqDv15lO%2B
c%3D&reserved=0  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Attachment: smime.p7s
Description: S/MIME cryptographic signature