Hi,
On Sat, Apr 24, 2021 at 01:06:08PM +0200, Michael Walle wrote:
> 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 <michael@xxxxxxxx>
This patch causes a boot failure with one of my qemu tests.
With the patch in place, the flash fails to instantiate.
[ 1.156578] Creating 3 MTD partitions on "physmap-flash":
[ 1.157192] 0x000000000000-0x000000040000 : "U-Boot Bootloader"
[ 1.184632] 0x000000040000-0x000000060000 : "U-Boot Environment"
[ 1.201767] 0x000000060000-0x000000800000 : "Flash"
[ 1.222320] Deleting MTD partitions on "physmap-flash":
[ 1.222744] Deleting U-Boot Bootloader MTD partition
[ 1.303597] Deleting U-Boot Environment MTD partition
[ 1.368751] Deleting Flash MTD partition
[ 1.430619] physmap-flash: probe of physmap-flash failed with error -61
-61 is -ENODATA.
Other boot tests with different flash chips can still boot.
Reverting this patch (as well as the follow-up patches) fixes
the problem.
I do not know if this is a problem with qemu or a problem with the
patch, but, as I mentioned, other flash chips do still instantiate.
Do you have an idea what to look for when I try to track down the problem ?
Thanks,
Guenter
> ---
> Changes since v1:
> - combine name and compatible string in mtd_otp_nvmem_register()
>
> Changes since RFC:
> - none
>
> drivers/mtd/mtdcore.c | 148 ++++++++++++++++++++++++++++++++++++++++
> include/linux/mtd/mtd.h | 2 +
> 2 files changed, 150 insertions(+)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 9aaeadd53eb4..72e7000a86fd 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -777,6 +777,146 @@ 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 *compatible,
> + int size,
> + nvmem_reg_read_t reg_read)
> +{
> + 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;
> + /* just reuse the compatible as name */
> + config.name = compatible;
> + 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);
> + 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);
> + 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.
> *
> @@ -852,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);
> @@ -873,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 a89955f3cbc8..88227044fc86 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -380,6 +380,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.
