Re: [PATCH 1/9] dt-bindings: nvmem: imx-ocotp: Add support for secure-enclave

[Krzysztof Kozlowski <krzk@kernel.org>]

On 17/06/2026 13:36, Frieder Schrempf wrote:
> On 17.06.26 12:49, Krzysztof Kozlowski wrote:
>> On Tue, Jun 16, 2026 at 01:52:16PM +0200, Frieder Schrempf wrote:
>>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>>
>>> Some SoCs like the i.MX9 family allow full access to the fuses only
>>> through the secure enclave firmware API. Add a property to reference
>>> the secure enclave node and let the driver use the API.
>>>
>>> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
>>> ---
>>>  Documentation/devicetree/bindings/nvmem/imx-ocotp.yaml | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/nvmem/imx-ocotp.yaml b/Documentation/devicetree/bindings/nvmem/imx-ocotp.yaml
>>> index a8076d0e2737..14a6429f4a4c 100644
>>> --- a/Documentation/devicetree/bindings/nvmem/imx-ocotp.yaml
>>> +++ b/Documentation/devicetree/bindings/nvmem/imx-ocotp.yaml
>>> @@ -53,6 +53,10 @@ properties:
>>>    reg:
>>>      maxItems: 1
>>>  
>>> +  secure-enclave:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>> +    description: A phandle to the secure enclave node
>>
>> Two things here:
>> 1. Here you describe what for is that phandle, how it is used by the
>> hardware. Currently the description repeats the property name and type,
>> so not much useful.
> 
> Ok, agree.
> 
>>
>> 2. If you access OTP via firmware, then this is completely different
>> interface than MMIO, thus:
>> A. reg is not appropriate
>> B. Device is very different thus it has different compatible and I even
>> claim should be in different binding. Devices having completely
>> different SW interface should not be in the same binding, at least
>> usually.
>>
>> If any of above is not accurate, then your commit msg should answer why
>> and give some background.
> 
> Thanks for the feedback!
> 
> The driver currently uses the limited MMIO (FSB) interface to access the
> OTPs. The intention is to support the firmware interface alongside the
> MMIO interface so the driver can pick the interface that is available
> (firmware might not be loaded) and fallback to MMIO.
> 
> Following your argument would mean a driver deciding by itself which
> interface to use at runtime is not something we want to have in general,
> right?

No, the property fits DT, but above information should be in commit msg.
If this SoC has indeed both interfaces - MMIO and firmware calls - then
everything is in general fine. I assumed that is not the case and MMIO
is not really working.

What was confusing is that it feels like you are changing existing
interface, but why wasn't all this documented in the beginning? There is
imx9 in this binding already, so was it working? Was it not working at
all? Commit msg must clarify that.

> 
> In turn this would mean we need two drivers, or at least two
> compatibles/bindings for something that is effectively the same hardware.

Driver design is orthogonal choice here.

It can reside in separate binding, if MMIO is still valid, but till
everything is not yet too complex can be also this binding file.

If it stays in this binding, then you need to restrict properties per
variant, so add if:then: block which will disallow the phandle for other
variants.
Best regards,
Krzysztof