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

[Frieder Schrempf <frieder.schrempf@kontron.de>]

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.