Re: [PATCH 3/9] firmware: imx: ele: Add API functions for OCOTP fuse access

[Peng Fan <peng.fan@oss.nxp.com>]

On Thu, Jun 18, 2026 at 07:52:16AM +0200, Frieder Schrempf wrote:
>On 17.06.26 21:56, Frank Li wrote:
>> On Wed, Jun 17, 2026 at 08:54:35AM +0200, Frieder Schrempf wrote:
>>> On 16.06.26 22:05, Frank Li wrote:
>>>> On Tue, Jun 16, 2026 at 07:59:54PM +0200, Frieder Schrempf wrote:
>>>>> On 16.06.26 17:36, Frank Li wrote:
>>>>>> On Tue, Jun 16, 2026 at 01:52:18PM +0200, Frieder Schrempf wrote:
>>>>>>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>>>>>>
>>>>>>> The ELE S400 API provides read and write access to the OCOTP fuse
>>>>>>> registers. This adds the necessary API functions imx_se_read_fuse()
>>>>>>> and imx_se_write_fuse() to be used by other drivers such as the
>>>>>>> OCOTP S400 NVMEM driver.
>>>>>>>
>>>>>>> This is ported from the downstream vendor kernel.
>>>>>>>
>>>>>>> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
>>>>>>> ---
>>>>>>>  drivers/firmware/imx/ele_base_msg.c | 122 ++++++++++++++++++++++++++++++++++++
>>>>>>>  drivers/firmware/imx/ele_base_msg.h |   6 ++
>>>>>>>  include/linux/firmware/imx/se_api.h |   3 +
>>>>>>>  3 files changed, 131 insertions(+)
>>>>>>>
>>>>>> ...
>>>>>>> +++ b/include/linux/firmware/imx/se_api.h
>>>>>>> @@ -11,4 +11,7 @@
>>>>>>>  #define SOC_ID_OF_IMX8ULP		0x084d
>>>>>>>  #define SOC_ID_OF_IMX93			0x9300
>>>>>>>
>>>>>>> +int imx_se_read_fuse(void *se_if_data, uint16_t fuse_id, u32 *value);
>>>>>>> +int imx_se_write_fuse(void *se_if_data, uint16_t fuse_id, u32 value);
>>>>>>> +
>>>>>>
>>>>>> This API should implement in fuse drivers. Other consume should use standard
>>>>>> fuse API to get value. If put here, it may bypass fuse driver.
>>>>>
>>>>> The reason this is here, is the downstream implementation in linux-imx
>>>>> and the current code organization.
>>>>
>>>> Downstream may not good enough, sometime, it is quick solution.
>>>
>>> Ok, but the code structure and API design has been upstreamed like this
>>> and the refactoring could have been done before, if downstream is known
>>> to not be well organized.
>>>
>>>>
>>>>> I thought there is some good reason
>>>>> to have shared functions and it looks like Pankaj structured it like
>>>>> this so all API functions live in ele_base_msg.c and the internal
>>>>> structs and defines in ele_base_msg.h and se_ctrl.h are not exposed to
>>>>> other drivers.
>>>>>
>>>>> If I would move this into imx-ocotp-ele.c, then I would also need to
>>>>> change how the code is organized and make the internal se_api functions
>>>>> exposed to other drivers. I don't know if that is really a good idea.
>>>>>
>>>>> I get your point but it looks like this contradicts the intention of
>>>>> having a clean API in the firmware driver.
>>>>
>>>> You can refer imx-ocotp-scu.c, structure should be similar, only difference
>>>> is that lower transfer APIs.
>>> Ok, this would mean that I expose the generic SE functions and structs
>>> required for fuse handling. In practice, I would remove
>>> imx_se_read_fuse() and imx_se_write_fuse() from se_api.h and instead add
>>> the following:
>>>
>>> struct se_msg_hdr { ... };
>>> struct se_api_msg { ... };
>>> struct se_if_priv;
>>> se_fill_cmd_msg_hdr( ... );
>>> se_msg_send_rcv( ... );
>>> se_val_rsp_hdr_n_status( ... );
>>>
>>> Then I would export the functions in ele_common.c and put the fuse
>>> read/write functions in the NVMEM driver.
>>>
>>> Is that what you want me to do?
>> 
>> Yes, Idealy, it should be children device under ele, ELE like a bus, which
>> previous lower level data transfer, ocotp should be base on top then it.
>> like spi/i2c, which provide low level data transfer.
>
>Ok, please also see the discussion with Krzysztof on the bindings patch.
>The problem is that this driver uses both, MMIO and firmware interface.
>Therefore putting a child node in the ELE device node is probably not
>correct, either!?
>
>In general I think it's a good idea as the fuses actually live inside
>the ELE block so this would properly describe the hardware, but again
>this would create a hard dependency on the closed source ELE firmware
>which I don't like that much.

The current ele driver does not export API for others to fill the structure
as i.MX8 imx_scu_call_rpc(). Actually I agree with Frank's idea regarding
ELE firmware provides low level data transfer, such as ELE firmware driver
providing an API such as imx_se_call_rpc() or imx_ele_call_rpc().

Regards
Peng