mbox series

[0/5] mtd: core: OTP nvmem provider support

Message ID 20210416114928.27758-1-michael@walle.cc
Headers show
Series mtd: core: OTP nvmem provider support | expand

Message

Michael Walle April 16, 2021, 11:49 a.m. UTC
The goal is to fetch a (base) MAC address from the OTP region of a SPI NOR
flash.

This is the first part, where I try to add the nvmem provider support to
the MTD core.

I'm not sure about the device tree bindings. Consider the following two
variants:

(1)
    flash@0 {
        ..

        otp {
            compatible = "mtd-user-otp";
            #address-cells = <1>;
            #size-cells = <1>;

            serial-number@0 {
                reg = <0x0 0x8>;
            };
        };
    };

(2)
    flash@0 {
        ..

        otp {
            compatible = "mtd-user-otp";
            #address-cells = <1>;
            #size-cells = <1>;

			some-useful-name {
                compatible = "nvmem-cells";

                serial-number@0 {
                    reg = <0x0 0x8>;
                };
			};
        };
    };

Both bindings use a subnode "opt[-N]". We cannot have the nvmem cells as
children to the flash node because of the legacy partition binding.

(1) seems to be the form which is used almost everywhere in the kernel.
That is, the nvmem cells are just children of the parent node.

(2) seem to be more natural, because there might also be other properties
inside the otp subnode and might be more future-proof.

At the moment this patch implements (1).

Michael Walle (5):
  nvmem: core: allow specifying of_node
  dt-bindings: mtd: add YAML schema for the generic MTD bindings
  dt-bindings: mtd: add OTP bindings
  dt-bindings: mtd: spi-nor: add otp property
  mtd: core: add OTP nvmem provider support

 .../devicetree/bindings/mtd/common.txt        |  16 +-
 .../bindings/mtd/jedec,spi-nor.yaml           |   6 +
 .../devicetree/bindings/mtd/mtd.yaml          |  89 +++++++++++
 drivers/mtd/mtdcore.c                         | 149 ++++++++++++++++++
 drivers/nvmem/core.c                          |   4 +-
 include/linux/mtd/mtd.h                       |   2 +
 include/linux/nvmem-provider.h                |   2 +
 7 files changed, 252 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/mtd.yaml

Comments

Rob Herring (Arm) April 16, 2021, 6:44 p.m. UTC | #1
On Fri, Apr 16, 2021 at 01:49:23PM +0200, Michael Walle wrote:
> The goal is to fetch a (base) MAC address from the OTP region of a SPI NOR
> flash.
> 
> This is the first part, where I try to add the nvmem provider support to
> the MTD core.
> 
> I'm not sure about the device tree bindings. Consider the following two
> variants:
> 
> (1)
>     flash@0 {
>         ..
> 
>         otp {
>             compatible = "mtd-user-otp";

mtd is a linuxism. Why not just 'nvmem-cells' here or as a fallback if 
we come up with a better name? 

>             #address-cells = <1>;
>             #size-cells = <1>;
> 
>             serial-number@0 {
>                 reg = <0x0 0x8>;
>             };
>         };
>     };
> 
> (2)
>     flash@0 {
>         ..
> 
>         otp {
>             compatible = "mtd-user-otp";
>             #address-cells = <1>;
>             #size-cells = <1>;
> 
> 			some-useful-name {
>                 compatible = "nvmem-cells";
> 
>                 serial-number@0 {
>                     reg = <0x0 0x8>;
>                 };
> 			};
>         };
>     };
> 
> Both bindings use a subnode "opt[-N]". We cannot have the nvmem cells as
> children to the flash node because of the legacy partition binding.
> 
> (1) seems to be the form which is used almost everywhere in the kernel.
> That is, the nvmem cells are just children of the parent node.
> 
> (2) seem to be more natural, because there might also be other properties
> inside the otp subnode and might be more future-proof.
> 
> At the moment this patch implements (1).

I think approach (1) seems fine.

Rob
Michael Walle April 16, 2021, 7:26 p.m. UTC | #2
Hi Rob,

Am 2021-04-16 20:44, schrieb Rob Herring:
> On Fri, Apr 16, 2021 at 01:49:23PM +0200, Michael Walle wrote:
>> The goal is to fetch a (base) MAC address from the OTP region of a SPI 
>> NOR
>> flash.
>> 
>> This is the first part, where I try to add the nvmem provider support 
>> to
>> the MTD core.
>> 
>> I'm not sure about the device tree bindings. Consider the following 
>> two
>> variants:
>> 
>> (1)
>>     flash@0 {
>>         ..
>> 
>>         otp {
>>             compatible = "mtd-user-otp";
> 
> mtd is a linuxism. Why not just 'nvmem-cells' here or as a fallback if
> we come up with a better name?

There are two different compatibles: "mtd-user-otp" and 
"mtd-factory-otp"
to differentiate what kind of OTP should be used (and both are possible
at the same time). Thus nvmem-cells alone won't be enough. We could drop
the "mtd-" prefix though.

Is there a benefit of having the following?
   compatible = "user-otp", "nvmem-cells";


>>             #address-cells = <1>;
>>             #size-cells = <1>;
>> 
>>             serial-number@0 {
>>                 reg = <0x0 0x8>;
>>             };
>>         };
>>     };
>> 
>> (2)
>>     flash@0 {
>>         ..
>> 
>>         otp {
>>             compatible = "mtd-user-otp";
>>             #address-cells = <1>;
>>             #size-cells = <1>;
>> 
>> 			some-useful-name {
>>                 compatible = "nvmem-cells";
>> 
>>                 serial-number@0 {
>>                     reg = <0x0 0x8>;
>>                 };
>> 			};
>>         };
>>     };
>> 
>> Both bindings use a subnode "opt[-N]". We cannot have the nvmem cells 
>> as
>> children to the flash node because of the legacy partition binding.
>> 
>> (1) seems to be the form which is used almost everywhere in the 
>> kernel.
>> That is, the nvmem cells are just children of the parent node.
>> 
>> (2) seem to be more natural, because there might also be other 
>> properties
>> inside the otp subnode and might be more future-proof.
>> 
>> At the moment this patch implements (1).
> 
> I think approach (1) seems fine.

ok

-michael
Rob Herring (Arm) April 20, 2021, 2:08 p.m. UTC | #3
On Fri, Apr 16, 2021 at 09:26:03PM +0200, Michael Walle wrote:
> Hi Rob,
> 
> Am 2021-04-16 20:44, schrieb Rob Herring:
> > On Fri, Apr 16, 2021 at 01:49:23PM +0200, Michael Walle wrote:
> > > The goal is to fetch a (base) MAC address from the OTP region of a
> > > SPI NOR
> > > flash.
> > > 
> > > This is the first part, where I try to add the nvmem provider
> > > support to
> > > the MTD core.
> > > 
> > > I'm not sure about the device tree bindings. Consider the following
> > > two
> > > variants:
> > > 
> > > (1)
> > >     flash@0 {
> > >         ..
> > > 
> > >         otp {
> > >             compatible = "mtd-user-otp";
> > 
> > mtd is a linuxism. Why not just 'nvmem-cells' here or as a fallback if
> > we come up with a better name?
> 
> There are two different compatibles: "mtd-user-otp" and "mtd-factory-otp"
> to differentiate what kind of OTP should be used (and both are possible
> at the same time). Thus nvmem-cells alone won't be enough. We could drop
> the "mtd-" prefix though.
> 
> Is there a benefit of having the following?
>   compatible = "user-otp", "nvmem-cells";

Yes. I assume 'user-otp' tells you something about the region and 
'nvmem-cells' tells you that there are child nodes of nvmem data. Of 
course 'user-otp' could imply 'nvmem-cells' as you did. I'm fine with 
either way.

Rob
Michael Walle April 20, 2021, 3:03 p.m. UTC | #4
Hi Rob,

Am 2021-04-20 16:08, schrieb Rob Herring:
> On Fri, Apr 16, 2021 at 09:26:03PM +0200, Michael Walle wrote:
>> Am 2021-04-16 20:44, schrieb Rob Herring:
>> > On Fri, Apr 16, 2021 at 01:49:23PM +0200, Michael Walle wrote:
>> > > The goal is to fetch a (base) MAC address from the OTP region of a
>> > > SPI NOR
>> > > flash.
>> > >
>> > > This is the first part, where I try to add the nvmem provider
>> > > support to
>> > > the MTD core.
>> > >
>> > > I'm not sure about the device tree bindings. Consider the following
>> > > two
>> > > variants:
>> > >
>> > > (1)
>> > >     flash@0 {
>> > >         ..
>> > >
>> > >         otp {
>> > >             compatible = "mtd-user-otp";
>> >
>> > mtd is a linuxism. Why not just 'nvmem-cells' here or as a fallback if
>> > we come up with a better name?
>> 
>> There are two different compatibles: "mtd-user-otp" and 
>> "mtd-factory-otp"
>> to differentiate what kind of OTP should be used (and both are 
>> possible
>> at the same time). Thus nvmem-cells alone won't be enough. We could 
>> drop
>> the "mtd-" prefix though.
>> 
>> Is there a benefit of having the following?
>>   compatible = "user-otp", "nvmem-cells";
> 
> Yes. I assume 'user-otp' tells you something about the region and
> 'nvmem-cells' tells you that there are child nodes of nvmem data. Of
> course 'user-otp' could imply 'nvmem-cells' as you did. I'm fine with
> either way.

Ah, if I use both compatibles, then the
Documentation/devicetree/bindings/mtd/partitions/nvmem-cells.yaml
schema kicks in, which mandates 'compatible = "nvmem-cells";' and I
get the following errors:

   CHECK   Documentation/devicetree/bindings/mtd/mtd.example.dt.yaml
/home/mwalle/repos/b-linux-arm64/Documentation/devicetree/bindings/mtd/mtd.example.dt.yaml: 
otp-1: compatible:0: 'nvmem-cells' was expected
	From schema: 
/home/mwalle/repos/linux-mw/Documentation/devicetree/bindings/mtd/partitions/nvmem-cells.yaml
/home/mwalle/repos/b-linux-arm64/Documentation/devicetree/bindings/mtd/mtd.example.dt.yaml: 
otp-1: compatible: ['factory-otp', 'nvmem-cells'] is too long
	From schema: 
/home/mwalle/repos/linux-mw/Documentation/devicetree/bindings/mtd/partitions/nvmem-cells.yaml
/home/mwalle/repos/b-linux-arm64/Documentation/devicetree/bindings/mtd/mtd.example.dt.yaml: 
otp-1: compatible: Additional items are not allowed ('nvmem-cells' was 
unexpected)
	From schema: 
/home/mwalle/repos/linux-mw/Documentation/devicetree/bindings/mtd/partitions/nvmem-cells.yaml

Is there a way around that?

-michael