diff mbox series

[v1,08/14] dt-bindings: mtd: relax the nvmem compatible string

Message ID 20220825214423.903672-9-michael@walle.cc (mailing list archive)
State New, archived
Headers show
Series nvmem: core: introduce NVMEM layouts | expand

Commit Message

Michael Walle Aug. 25, 2022, 9:44 p.m. UTC
The "user-otp" and "factory-otp" compatible string just depicts a
generic NVMEM device. But an actual device tree node might as well
contain a more specific compatible string. Make it possible to add
more specific binding elsewere and just match part of the compatibles
here.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 Documentation/devicetree/bindings/mtd/mtd.yaml | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Krzysztof Kozlowski Aug. 31, 2022, 7:37 a.m. UTC | #1
On 26/08/2022 00:44, Michael Walle wrote:
> The "user-otp" and "factory-otp" compatible string just depicts a
> generic NVMEM device. But an actual device tree node might as well
> contain a more specific compatible string. Make it possible to add
> more specific binding elsewere and just match part of the compatibles

typo: elsewhere

> here.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  Documentation/devicetree/bindings/mtd/mtd.yaml | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/mtd.yaml b/Documentation/devicetree/bindings/mtd/mtd.yaml
> index 376b679cfc70..0291e439b6a6 100644
> --- a/Documentation/devicetree/bindings/mtd/mtd.yaml
> +++ b/Documentation/devicetree/bindings/mtd/mtd.yaml
> @@ -33,9 +33,10 @@ patternProperties:
>  
>      properties:
>        compatible:
> -        enum:
> -          - user-otp
> -          - factory-otp
> +        contains:
> +          enum:
> +            - user-otp
> +            - factory-otp

This does not work in the "elsewhere" place. You need to use similar
approach as we do for syscon or primecell.

>  
>      required:
>        - compatible


Best regards,
Krzysztof
Michael Walle Aug. 31, 2022, 7:48 a.m. UTC | #2
Am 2022-08-31 09:37, schrieb Krzysztof Kozlowski:
> On 26/08/2022 00:44, Michael Walle wrote:
>> The "user-otp" and "factory-otp" compatible string just depicts a
>> generic NVMEM device. But an actual device tree node might as well
>> contain a more specific compatible string. Make it possible to add
>> more specific binding elsewere and just match part of the compatibles
> 
> typo: elsewhere
> 
>> here.
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>>  Documentation/devicetree/bindings/mtd/mtd.yaml | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/mtd/mtd.yaml 
>> b/Documentation/devicetree/bindings/mtd/mtd.yaml
>> index 376b679cfc70..0291e439b6a6 100644
>> --- a/Documentation/devicetree/bindings/mtd/mtd.yaml
>> +++ b/Documentation/devicetree/bindings/mtd/mtd.yaml
>> @@ -33,9 +33,10 @@ patternProperties:
>> 
>>      properties:
>>        compatible:
>> -        enum:
>> -          - user-otp
>> -          - factory-otp
>> +        contains:
>> +          enum:
>> +            - user-otp
>> +            - factory-otp
> 
> This does not work in the "elsewhere" place. You need to use similar
> approach as we do for syscon or primecell.

I'm a bit confused. Looking at
   Documentation/devicetree/bindings/arm/primecell.yaml
it is done in the same way as this binding.

Whereas, the syscon use a "select:" on top of it. I'm
pretty sure, I've tested it without the select and the
validator picked up the constraints.

Could you elaborate on what is wrong here? Select missing?

-michael
Krzysztof Kozlowski Aug. 31, 2022, 7:55 a.m. UTC | #3
On 31/08/2022 10:48, Michael Walle wrote:
> Am 2022-08-31 09:37, schrieb Krzysztof Kozlowski:
>> On 26/08/2022 00:44, Michael Walle wrote:
>>> The "user-otp" and "factory-otp" compatible string just depicts a
>>> generic NVMEM device. But an actual device tree node might as well
>>> contain a more specific compatible string. Make it possible to add
>>> more specific binding elsewere and just match part of the compatibles
>>
>> typo: elsewhere
>>
>>> here.
>>>
>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>> ---
>>>  Documentation/devicetree/bindings/mtd/mtd.yaml | 7 ++++---
>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/mtd.yaml 
>>> b/Documentation/devicetree/bindings/mtd/mtd.yaml
>>> index 376b679cfc70..0291e439b6a6 100644
>>> --- a/Documentation/devicetree/bindings/mtd/mtd.yaml
>>> +++ b/Documentation/devicetree/bindings/mtd/mtd.yaml
>>> @@ -33,9 +33,10 @@ patternProperties:
>>>
>>>      properties:
>>>        compatible:
>>> -        enum:
>>> -          - user-otp
>>> -          - factory-otp
>>> +        contains:
>>> +          enum:
>>> +            - user-otp
>>> +            - factory-otp
>>
>> This does not work in the "elsewhere" place. You need to use similar
>> approach as we do for syscon or primecell.
> 
> I'm a bit confused. Looking at
>    Documentation/devicetree/bindings/arm/primecell.yaml
> it is done in the same way as this binding.

Yes. primecell is like your mtd here. And how are other places with
primcell (like other places with user-otp) done?

> 
> Whereas, the syscon use a "select:" on top of it. I'm
> pretty sure, I've tested it without the select and the
> validator picked up the constraints.
> 
> Could you elaborate on what is wrong here? Select missing?

You got warning from Rob, so run tests. I think you will see the errors,
just like bot reported them.

Best regards,
Krzysztof
Rob Herring (Arm) Aug. 31, 2022, 9:48 p.m. UTC | #4
On Thu, Aug 25, 2022 at 11:44:17PM +0200, Michael Walle wrote:
> The "user-otp" and "factory-otp" compatible string just depicts a
> generic NVMEM device. But an actual device tree node might as well
> contain a more specific compatible string. Make it possible to add
> more specific binding elsewere and just match part of the compatibles
> here.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  Documentation/devicetree/bindings/mtd/mtd.yaml | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

In hindsight it looks like we are mixing 2 different purposes of 'which 
instance is this' and 'what is this'. 'compatible' is supposed to be the 
latter.

Maybe there's a better way to handle user/factory? There's a similar 
need with partitions for A/B or factory/update.

Rob
Michael Walle Aug. 31, 2022, 10:30 p.m. UTC | #5
Am 2022-08-31 23:48, schrieb Rob Herring:
> On Thu, Aug 25, 2022 at 11:44:17PM +0200, Michael Walle wrote:
>> The "user-otp" and "factory-otp" compatible string just depicts a
>> generic NVMEM device. But an actual device tree node might as well
>> contain a more specific compatible string. Make it possible to add
>> more specific binding elsewere and just match part of the compatibles
>> here.
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>>  Documentation/devicetree/bindings/mtd/mtd.yaml | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> In hindsight it looks like we are mixing 2 different purposes of 'which
> instance is this' and 'what is this'. 'compatible' is supposed to be 
> the
> latter.
> 
> Maybe there's a better way to handle user/factory? There's a similar
> need with partitions for A/B or factory/update.

I'm not sure I understand what you mean. It has nothing to with
user and factory provisionings.

SPI flashes have a user programmable and a factory programmable
area, some have just one of them. Whereas with A/B you (as in the
user or the board manufacturer) defines an area within a memory device
to be either slot A or slot B. But here the flash dictates what's
factory and what's user storage. It's in the datasheet.

HTH
-michael
Rob Herring (Arm) Sept. 2, 2022, 2:46 p.m. UTC | #6
On Wed, Aug 31, 2022 at 5:30 PM Michael Walle <michael@walle.cc> wrote:
>
> Am 2022-08-31 23:48, schrieb Rob Herring:
> > On Thu, Aug 25, 2022 at 11:44:17PM +0200, Michael Walle wrote:
> >> The "user-otp" and "factory-otp" compatible string just depicts a
> >> generic NVMEM device. But an actual device tree node might as well
> >> contain a more specific compatible string. Make it possible to add
> >> more specific binding elsewere and just match part of the compatibles
> >> here.
> >>
> >> Signed-off-by: Michael Walle <michael@walle.cc>
> >> ---
> >>  Documentation/devicetree/bindings/mtd/mtd.yaml | 7 ++++---
> >>  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > In hindsight it looks like we are mixing 2 different purposes of 'which
> > instance is this' and 'what is this'. 'compatible' is supposed to be
> > the
> > latter.
> >
> > Maybe there's a better way to handle user/factory? There's a similar
> > need with partitions for A/B or factory/update.
>
> I'm not sure I understand what you mean. It has nothing to with
> user and factory provisionings.
>
> SPI flashes have a user programmable and a factory programmable
> area, some have just one of them. Whereas with A/B you (as in the
> user or the board manufacturer) defines an area within a memory device
> to be either slot A or slot B. But here the flash dictates what's
> factory and what's user storage. It's in the datasheet.

Ah, right. Nevermind...

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mtd/mtd.yaml b/Documentation/devicetree/bindings/mtd/mtd.yaml
index 376b679cfc70..0291e439b6a6 100644
--- a/Documentation/devicetree/bindings/mtd/mtd.yaml
+++ b/Documentation/devicetree/bindings/mtd/mtd.yaml
@@ -33,9 +33,10 @@  patternProperties:
 
     properties:
       compatible:
-        enum:
-          - user-otp
-          - factory-otp
+        contains:
+          enum:
+            - user-otp
+            - factory-otp
 
     required:
       - compatible