2021-03-22 18:22:59

by Michael Walle

[permalink] [raw]
Subject: [RFC PATCH 0/4] mtd: core: OTP nvmem provider support

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

--
2.20.1


2021-03-22 18:22:59

by Michael Walle

[permalink] [raw]
Subject: [RFC PATCH 4/4] mtd: core: add OTP nvmem provider support

Flash OTP regions can already be read via user space. Some boards have
their serial number or MAC addresses stored in the OTP regions. Add
support for them being a (read-only) nvmem provider.

The API to read the OTP data is already in place. It distinguishes
between factory and user OTP, thus there are up to two different
providers.

Signed-off-by: Michael Walle <[email protected]>
---
drivers/mtd/mtdcore.c | 149 ++++++++++++++++++++++++++++++++++++++++
include/linux/mtd/mtd.h | 2 +
2 files changed, 151 insertions(+)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 432769caae04..300340973561 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -776,6 +776,147 @@ static void mtd_set_dev_defaults(struct mtd_info *mtd)
mutex_init(&mtd->master.chrdev_lock);
}

+static ssize_t mtd_otp_size(struct mtd_info *mtd, bool is_user)
+{
+ struct otp_info *info = kmalloc(PAGE_SIZE, GFP_KERNEL);
+ ssize_t size = 0;
+ unsigned int i;
+ size_t retlen;
+ int ret;
+
+ if (is_user)
+ ret = mtd_get_user_prot_info(mtd, PAGE_SIZE, &retlen, info);
+ else
+ ret = mtd_get_fact_prot_info(mtd, PAGE_SIZE, &retlen, info);
+ if (ret)
+ goto err;
+
+ for (i = 0; i < retlen / sizeof(*info); i++) {
+ size += info->length;
+ info++;
+ }
+
+ kfree(info);
+ return size;
+
+err:
+ kfree(info);
+ return ret;
+}
+
+static struct nvmem_device *mtd_otp_nvmem_register(struct mtd_info *mtd,
+ const char *name, int size,
+ nvmem_reg_read_t reg_read,
+ const char *compatible)
+{
+ struct nvmem_device *nvmem = NULL;
+ struct nvmem_config config = {};
+ struct device_node *np;
+
+ /* DT binding is optional */
+ np = of_get_compatible_child(mtd->dev.of_node, compatible);
+
+ /* OTP nvmem will be registered on the physical device */
+ config.dev = mtd->dev.parent;
+ config.name = name;
+ config.id = NVMEM_DEVID_NONE;
+ config.owner = THIS_MODULE;
+ config.type = NVMEM_TYPE_OTP;
+ config.root_only = true;
+ config.reg_read = reg_read;
+ config.size = size;
+ config.of_node = np;
+ config.priv = mtd;
+
+ nvmem = nvmem_register(&config);
+ /* Just ignore if there is no NVMEM support in the kernel */
+ if (IS_ERR(nvmem) && PTR_ERR(nvmem) == -EOPNOTSUPP)
+ nvmem = NULL;
+
+ of_node_put(np);
+
+ return nvmem;
+}
+
+static int mtd_nvmem_user_otp_reg_read(void *priv, unsigned int offset,
+ void *val, size_t bytes)
+{
+ struct mtd_info *mtd = priv;
+ size_t retlen;
+ int ret;
+
+ ret = mtd_read_user_prot_reg(mtd, offset, bytes, &retlen, val);
+ if (ret)
+ return ret;
+
+ return retlen == bytes ? 0 : -EIO;
+}
+
+static int mtd_nvmem_fact_otp_reg_read(void *priv, unsigned int offset,
+ void *val, size_t bytes)
+{
+ struct mtd_info *mtd = priv;
+ size_t retlen;
+ int ret;
+
+ ret = mtd_read_fact_prot_reg(mtd, offset, bytes, &retlen, val);
+ if (ret)
+ return ret;
+
+ return retlen == bytes ? 0 : -EIO;
+}
+
+static int mtd_otp_nvmem_add(struct mtd_info *mtd)
+{
+ struct nvmem_device *nvmem;
+ ssize_t size;
+ int err;
+
+ if (mtd->_get_user_prot_info && mtd->_read_user_prot_reg) {
+ size = mtd_otp_size(mtd, true);
+ if (size < 0)
+ return size;
+
+ if (size > 0) {
+ nvmem = mtd_otp_nvmem_register(mtd, "user-otp", size,
+ mtd_nvmem_user_otp_reg_read,
+ "mtd-user-otp");
+ if (IS_ERR(nvmem)) {
+ dev_err(&mtd->dev, "Failed to register OTP NVMEM device\n");
+ return PTR_ERR(nvmem);
+ }
+ mtd->otp_user_nvmem = nvmem;
+ }
+ }
+
+ if (mtd->_get_fact_prot_info && mtd->_read_fact_prot_reg) {
+ size = mtd_otp_size(mtd, false);
+ if (size < 0) {
+ err = size;
+ goto err;
+ }
+
+ if (size > 0) {
+ nvmem = mtd_otp_nvmem_register(mtd, "factory-otp", size,
+ mtd_nvmem_fact_otp_reg_read,
+ "mtd-factory-otp");
+ if (IS_ERR(nvmem)) {
+ dev_err(&mtd->dev, "Failed to register OTP NVMEM device\n");
+ err = PTR_ERR(nvmem);
+ goto err;
+ }
+ mtd->otp_factory_nvmem = nvmem;
+ }
+ }
+
+ return 0;
+
+err:
+ if (mtd->otp_user_nvmem)
+ nvmem_unregister(mtd->otp_user_nvmem);
+ return err;
+}
+
/**
* mtd_device_parse_register - parse partitions and register an MTD device.
*
@@ -851,6 +992,8 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
register_reboot_notifier(&mtd->reboot_notifier);
}

+ ret = mtd_otp_nvmem_add(mtd);
+
out:
if (ret && device_is_registered(&mtd->dev))
del_mtd_device(mtd);
@@ -872,6 +1015,12 @@ int mtd_device_unregister(struct mtd_info *master)
if (master->_reboot)
unregister_reboot_notifier(&master->reboot_notifier);

+ if (master->otp_user_nvmem)
+ nvmem_unregister(master->otp_user_nvmem);
+
+ if (master->otp_factory_nvmem)
+ nvmem_unregister(master->otp_factory_nvmem);
+
err = del_mtd_partitions(master);
if (err)
return err;
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 4aac200ca8b5..71e751d18c22 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -379,6 +379,8 @@ struct mtd_info {
int usecount;
struct mtd_debug_info dbg;
struct nvmem_device *nvmem;
+ struct nvmem_device *otp_user_nvmem;
+ struct nvmem_device *otp_factory_nvmem;

/*
* Parent device from the MTD partition point of view.
--
2.20.1

2021-03-22 18:23:23

by Michael Walle

[permalink] [raw]
Subject: [RFC PATCH 3/4] dt-bindings: mtd: add OTP bindings

Flash devices can have one-time-programmable regions. Add a nvmem
binding so they can be used as a nvmem provider.

Signed-off-by: Michael Walle <[email protected]>
---
.../devicetree/bindings/mtd/mtd.yaml | 71 +++++++++++++++++++
1 file changed, 71 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/mtd.yaml b/Documentation/devicetree/bindings/mtd/mtd.yaml
index 321259aab0f6..2b852f91a6a9 100644
--- a/Documentation/devicetree/bindings/mtd/mtd.yaml
+++ b/Documentation/devicetree/bindings/mtd/mtd.yaml
@@ -21,6 +21,25 @@ properties:
based name) in order to ease flash device identification and/or
describe what they are used for.

+patternProperties:
+ "^otp(-[0-9]+)?":
+ type: object
+ $ref: ../nvmem/nvmem.yaml#
+
+ description: |
+ An OTP memory region. Some flashes provide a one-time-programmable
+ memory whose content can either be programmed by a user or is already
+ pre-programmed by the factory. Some flashes might provide both.
+
+ properties:
+ compatible:
+ enum:
+ - mtd-user-otp
+ - mtd-factory-otp
+
+ required:
+ - compatible
+
additionalProperties: true

examples:
@@ -36,4 +55,56 @@ examples:
};
};

+ - |
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ flash@0 {
+ reg = <0>;
+ compatible = "some,flash";
+
+ otp {
+ compatible = "mtd-user-otp";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ serial-number@0 {
+ reg = <0 16>;
+ };
+ };
+ };
+ };
+
+ - |
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ flash@0 {
+ reg = <0>;
+ compatible = "some,flash";
+
+ otp-1 {
+ compatible = "mtd-factory-otp";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ electronic-serial-number@0 {
+ reg = <0 8>;
+ };
+ };
+
+ otp-2 {
+ compatible = "mtd-user-otp";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ mac-address@0 {
+ reg = <0 6>;
+ };
+ };
+ };
+ };
+
...
--
2.20.1

2021-03-27 17:18:03

by Rob Herring

[permalink] [raw]
Subject: Re: [RFC PATCH 3/4] dt-bindings: mtd: add OTP bindings

On Mon, Mar 22, 2021 at 07:19:48PM +0100, Michael Walle wrote:
> Flash devices can have one-time-programmable regions. Add a nvmem
> binding so they can be used as a nvmem provider.
>
> Signed-off-by: Michael Walle <[email protected]>
> ---
> .../devicetree/bindings/mtd/mtd.yaml | 71 +++++++++++++++++++
> 1 file changed, 71 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/mtd.yaml b/Documentation/devicetree/bindings/mtd/mtd.yaml
> index 321259aab0f6..2b852f91a6a9 100644
> --- a/Documentation/devicetree/bindings/mtd/mtd.yaml
> +++ b/Documentation/devicetree/bindings/mtd/mtd.yaml
> @@ -21,6 +21,25 @@ properties:
> based name) in order to ease flash device identification and/or
> describe what they are used for.
>
> +patternProperties:
> + "^otp(-[0-9]+)?":

Needs '$' on the end.

> + type: object
> + $ref: ../nvmem/nvmem.yaml#
> +
> + description: |
> + An OTP memory region. Some flashes provide a one-time-programmable
> + memory whose content can either be programmed by a user or is already
> + pre-programmed by the factory. Some flashes might provide both.
> +
> + properties:
> + compatible:
> + enum:
> + - mtd-user-otp
> + - mtd-factory-otp
> +
> + required:
> + - compatible
> +
> additionalProperties: true
>
> examples:
> @@ -36,4 +55,56 @@ examples:
> };
> };
>
> + - |
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + flash@0 {
> + reg = <0>;
> + compatible = "some,flash";

Soon (in linux-next, but off by default) this will be a warning for
undocumented compatible string. Use a real device.

> +
> + otp {
> + compatible = "mtd-user-otp";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + serial-number@0 {
> + reg = <0 16>;
> + };
> + };
> + };
> + };
> +
> + - |
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + flash@0 {
> + reg = <0>;
> + compatible = "some,flash";
> +
> + otp-1 {
> + compatible = "mtd-factory-otp";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + electronic-serial-number@0 {
> + reg = <0 8>;
> + };
> + };
> +
> + otp-2 {
> + compatible = "mtd-user-otp";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + mac-address@0 {
> + reg = <0 6>;
> + };
> + };
> + };
> + };

The 2nd example is a superset of the 1st, so drop the first one.

Rob

2021-03-30 09:45:15

by Srinivas Kandagatla

[permalink] [raw]
Subject: Re: [RFC PATCH 0/4] mtd: core: OTP nvmem provider support

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/[email protected]/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
>

2021-03-30 09:51:10

by Michael Walle

[permalink] [raw]
Subject: Re: [RFC PATCH 0/4] mtd: core: OTP nvmem provider support

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/[email protected]/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
>>

2021-03-30 10:09:53

by Michael Walle

[permalink] [raw]
Subject: Re: [RFC PATCH 3/4] dt-bindings: mtd: add OTP bindings

Hi Rob,

Am 2021-03-27 18:09, schrieb Rob Herring:
>> + spi {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + flash@0 {
>> + reg = <0>;
>> + compatible = "some,flash";
>
> Soon (in linux-next, but off by default) this will be a warning for
> undocumented compatible string. Use a real device.

Two questions:
(1) I guess this is also true for "PATCH 2/4", where you already added
your Reviewed-by?
(2) I'd add the "jedec,spi-nor" because, that is the one I target. But
before doing so, I'd need to add the otp subnode to the spi-nor
schema, correct? Otherwise, the schema validation will fail. Eg.

--- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
+++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
@@ -9,6 +9,9 @@ title: SPI NOR flash ST M25Pxx (and similar) serial
flash chips
maintainers:
- Rob Herring <[email protected]>

+allOf:
+ - $ref: "mtd.yaml#"
+
properties:
compatible:
oneOf:
@@ -82,6 +85,9 @@ patternProperties:
'^partition@':
type: object

+ "^otp(-[0-9]+)?$":
+ type: object
+
additionalProperties: false

examples:

-michael