mbox series

[RFC,0/4] mtd: core: OTP nvmem provider support

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

Message

Michael Walle March 22, 2021, 6:19 p.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 (4):
  nvmem: core: allow specifying of_node
  dt-bindings: mtd: add YAML schema for the generic MTD bindings
  dt-bindings: mtd: add OTP bindings
  mtd: core: add OTP nvmem provider support

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

Comments

Srinivas Kandagatla March 30, 2021, 9:42 a.m. UTC | #1
Hi Michael,

On 22/03/2021 18:19, 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";
>              #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).
> 


Have you looked at this series[1], are you both trying to do the same thing?

[1] 
https://lore.kernel.org/linux-mtd/20210312062830.20548-2-ansuelsmth@gmail.com/T/

--srini


> Michael Walle (4):
>    nvmem: core: allow specifying of_node
>    dt-bindings: mtd: add YAML schema for the generic MTD bindings
>    dt-bindings: mtd: add OTP bindings
>    mtd: core: add OTP nvmem provider support
> 
>   .../devicetree/bindings/mtd/common.txt        |  16 +-
>   .../devicetree/bindings/mtd/mtd.yaml          | 110 +++++++++++++
>   drivers/mtd/mtdcore.c                         | 149 ++++++++++++++++++
>   drivers/nvmem/core.c                          |   4 +-
>   include/linux/mtd/mtd.h                       |   2 +
>   include/linux/nvmem-provider.h                |   2 +
>   6 files changed, 267 insertions(+), 16 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/mtd/mtd.yaml
>
Michael Walle March 30, 2021, 9:49 a.m. UTC | #2
Hi Srinivas,

Am 2021-03-30 11:42, schrieb Srinivas Kandagatla:
> On 22/03/2021 18:19, 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";
>>              #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).
>> 
> 
> 
> Have you looked at this series[1], are you both trying to do the same 
> thing?

Yes, I've seen these, but they are for MTD partitions. OTP regions are 
not
MTD partitions (and cannot be mapped to them).

-michael

> 
> [1] 
> https://lore.kernel.org/linux-mtd/20210312062830.20548-2-ansuelsmth@gmail.com/T/
> 
> --srini
> 
> 
>> Michael Walle (4):
>>    nvmem: core: allow specifying of_node
>>    dt-bindings: mtd: add YAML schema for the generic MTD bindings
>>    dt-bindings: mtd: add OTP bindings
>>    mtd: core: add OTP nvmem provider support
>> 
>>   .../devicetree/bindings/mtd/common.txt        |  16 +-
>>   .../devicetree/bindings/mtd/mtd.yaml          | 110 +++++++++++++
>>   drivers/mtd/mtdcore.c                         | 149 
>> ++++++++++++++++++
>>   drivers/nvmem/core.c                          |   4 +-
>>   include/linux/mtd/mtd.h                       |   2 +
>>   include/linux/nvmem-provider.h                |   2 +
>>   6 files changed, 267 insertions(+), 16 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/mtd/mtd.yaml
>>
Srinivas Kandagatla March 30, 2021, 10:35 a.m. UTC | #3
On 22/03/2021 18:19, Michael Walle wrote:
> Until now, the of_node of the parent device is used. Some devices
> provide more than just the nvmem provider. To avoid name space clashes,
> add a way to allow specifying the nvmem cells in subnodes. Consider the
> following example:
> 
>      flash@0 {
>          compatible = "jedec,spi-nor";
> 
>          partitions {
>              compatible = "fixed-partitions";
>              #address-cells = <1>;
>              #size-cells = <1>;
> 
>              partition@0 {
>                  reg = <0x000000 0x010000>;
>              };
>          };
> 
>          otp {
>              compatible = "mtd-user-otp";

I would have expected this to come up as a proper device, but am not 
100% sure how MTD handles flashes and its partitions.

>              #address-cells = <1>;
>              #size-cells = <1>;
> 
>              serial-number@0 {
>                  reg = <0x0 0x8>;
>              };
>          };
>      };
> 
> There the nvmem provider might be the MTD partition or the OTP region of
> the flash.
> 
> Add a new config->of_node parameter, which if set, will be used instead
> of the parent's of_node.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---

Patch is fine as it its.
If you plan to take this via mtd tree here is my Ack

Acked-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>


--srini

>   drivers/nvmem/core.c           | 4 +++-
>   include/linux/nvmem-provider.h | 2 ++
>   2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index bca671ff4e54..62d363a399d3 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -789,7 +789,9 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
>   	nvmem->reg_write = config->reg_write;
>   	nvmem->keepout = config->keepout;
>   	nvmem->nkeepout = config->nkeepout;
> -	if (!config->no_of_node)
> +	if (config->of_node)
> +		nvmem->dev.of_node = config->of_node;
> +	else if (!config->no_of_node)
>   		nvmem->dev.of_node = config->dev->of_node;
>   
>   	switch (config->id) {
> diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
> index e162b757b6d5..471cb7b9e896 100644
> --- a/include/linux/nvmem-provider.h
> +++ b/include/linux/nvmem-provider.h
> @@ -57,6 +57,7 @@ struct nvmem_keepout {
>    * @type:	Type of the nvmem storage
>    * @read_only:	Device is read-only.
>    * @root_only:	Device is accessibly to root only.
> + * @of_node:	If given, this will be used instead of the parent's of_node.
>    * @no_of_node:	Device should not use the parent's of_node even if it's !NULL.
>    * @reg_read:	Callback to read data.
>    * @reg_write:	Callback to write data.
> @@ -86,6 +87,7 @@ struct nvmem_config {
>   	enum nvmem_type		type;
>   	bool			read_only;
>   	bool			root_only;
> +	struct device_node	*of_node;
>   	bool			no_of_node;
>   	nvmem_reg_read_t	reg_read;
>   	nvmem_reg_write_t	reg_write;
>