Messages in this thread | | | Subject | Re: [PATCH v2 4/6] nvmem: add support for the Khadas MCU Programmable User Memory | From | Srinivas Kandagatla <> | Date | Wed, 13 May 2020 11:34:13 +0100 |
| |
On 12/05/2020 14:26, Neil Armstrong wrote: > The new Khadas VIM2, VIM3 and Edge boards embeds an on-board microcontroller > offering a 56bytes User Programmable NVMEM array. > > This array needs a password to be writable, thus a password sysfs file > has been added on the device node to unlock the NVMEM.
Is this one time programmable? Or does it need to be unlocked for every read/write?
How can you make sure that the memory is unlocked before making attempt to read/write?
> > The default 6bytes password id: "Khadas" > > This implements the user NVMEM devices as cell of the Khadas MCU MFD driver. > > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > --- > drivers/nvmem/Kconfig | 8 ++ > drivers/nvmem/Makefile | 2 + > drivers/nvmem/khadas-mcu-user-mem.c | 128 ++++++++++++++++++++++++++++ > 3 files changed, 138 insertions(+) > create mode 100644 drivers/nvmem/khadas-mcu-user-mem.c > > diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig > index d7b7f6d688e7..92cd4f6aa931 100644 > --- a/drivers/nvmem/Kconfig > +++ b/drivers/nvmem/Kconfig > @@ -67,6 +67,14 @@ config JZ4780_EFUSE > To compile this driver as a module, choose M here: the module > will be called nvmem_jz4780_efuse. > > +config NVMEM_KHADAS_MCU_USER_MEM > + tristate "Khadas MCU User programmable memory support" > + depends on MFD_KHADAS_MCU > + depends on REGMAP > + help > + This is a driver for the MCU User programmable memory > + available on the Khadas VIM and Edge boards. > + > config NVMEM_LPC18XX_EEPROM > tristate "NXP LPC18XX EEPROM Memory Support" > depends on ARCH_LPC18XX || COMPILE_TEST > diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile > index a7c377218341..0516a309542d 100644 > --- a/drivers/nvmem/Makefile > +++ b/drivers/nvmem/Makefile > @@ -17,6 +17,8 @@ obj-$(CONFIG_NVMEM_IMX_OCOTP_SCU) += nvmem-imx-ocotp-scu.o > nvmem-imx-ocotp-scu-y := imx-ocotp-scu.o > obj-$(CONFIG_JZ4780_EFUSE) += nvmem_jz4780_efuse.o > nvmem_jz4780_efuse-y := jz4780-efuse.o > +obj-$(CONFIG_NVMEM_KHADAS_MCU_USER_MEM) += nvmem-khadas-mcu-user-mem.o > +nvmem-khadas-mcu-user-mem-y := khadas-mcu-user-mem.o > obj-$(CONFIG_NVMEM_LPC18XX_EEPROM) += nvmem_lpc18xx_eeprom.o > nvmem_lpc18xx_eeprom-y := lpc18xx_eeprom.o > obj-$(CONFIG_NVMEM_LPC18XX_OTP) += nvmem_lpc18xx_otp.o > diff --git a/drivers/nvmem/khadas-mcu-user-mem.c b/drivers/nvmem/khadas-mcu-user-mem.c > new file mode 100644 > index 000000000000..a1d5ae9a030c > --- /dev/null > +++ b/drivers/nvmem/khadas-mcu-user-mem.c > @@ -0,0 +1,128 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Driver for Khadas MCU User programmable Memory > + * > + * Copyright (C) 2020 BayLibre SAS > + * Author(s): Neil Armstrong <narmstrong@baylibre.com> > + */ > + > +#include <linux/clk.h>
Why do we need this header?
> +#include <linux/module.h> > +#include <linux/nvmem-provider.h> > +#include <linux/mfd/khadas-mcu.h> > +#include <linux/regmap.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > + > +static int khadas_mcu_user_mem_read(void *context, unsigned int offset, > + void *val, size_t bytes) > +{ > + struct khadas_mcu *khadas_mcu = context; > + > + return regmap_bulk_read(khadas_mcu->map, > + KHADAS_MCU_USER_DATA_0_REG + offset, > + val, bytes); > +} > + > +static int khadas_mcu_user_mem_write(void *context, unsigned int offset, > + void *val, size_t bytes) > +{ > + struct khadas_mcu *khadas_mcu = context; > + > + return regmap_bulk_write(khadas_mcu->map, > + KHADAS_MCU_USER_DATA_0_REG + offset, > + val, bytes); > +} > + > +static ssize_t password_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct khadas_mcu *khadas_mcu = dev_get_drvdata(dev); > + int i, ret; > + > + if (count < 6) > + return -EINVAL;
Possibly this magic 6 value needs proper define. An additional check here for > 6 would be better as well.
> + > + ret = regmap_write(khadas_mcu->map, KHADAS_MCU_PASSWD_START_REG, 1); > + if (ret) > + return ret; > + > + for (i = 0 ; i < 6 ; ++i) { > + ret = regmap_write(khadas_mcu->map, > + KHADAS_MCU_CHECK_USER_PASSWD_REG, > + buf[i]); > + if (ret) > + goto out; > + } > + > + ret = regmap_write(khadas_mcu->map, KHADAS_MCU_PASSWD_START_REG, 0); > + if (ret) > + return ret; > + > + return count; > +out: > + regmap_write(khadas_mcu->map, KHADAS_MCU_PASSWD_START_REG, 0); > + > + return ret; > +} > + > +static DEVICE_ATTR_WO(password); > + > +static struct attribute *khadas_mcu_user_mem_sysfs_attributes[] = { > + &dev_attr_password.attr, > + NULL, > +}; > + > +static const struct attribute_group khadas_mcu_user_mem_sysfs_attr_group = { > + .attrs = khadas_mcu_user_mem_sysfs_attributes, > +}; > + > +static int khadas_mcu_user_mem_probe(struct platform_device *pdev) > +{ > + struct khadas_mcu *khadas_mcu = dev_get_drvdata(pdev->dev.parent);
You could probably get away with dependency of this structure and the header itself by directly getting hold of regmap from parent device.
> + struct device *dev = &pdev->dev; > + struct nvmem_device *nvmem; > + struct nvmem_config *econfig; > + > + econfig = devm_kzalloc(dev, sizeof(*econfig), GFP_KERNEL); > + if (!econfig) > + return -ENOMEM; > + > + econfig->dev = pdev->dev.parent; > + econfig->name = dev_name(pdev->dev.parent); > + econfig->stride = 1; > + econfig->word_size = 1; > + econfig->reg_read = khadas_mcu_user_mem_read; > + econfig->reg_write = khadas_mcu_user_mem_write; > + econfig->size = 56; define 56 to make it more readable!
> + econfig->priv = khadas_mcu; > + > + platform_set_drvdata(pdev, khadas_mcu); > + > + nvmem = devm_nvmem_register(&pdev->dev, econfig); > + if (IS_ERR(nvmem)) > + return PTR_ERR(nvmem); > + > + return sysfs_create_group(&pdev->dev.kobj, > + &khadas_mcu_user_mem_sysfs_attr_group); > +} > + > +static const struct platform_device_id khadas_mcu_user_mem_id_table[] = { > + { .name = "khadas-mcu-user-mem", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(platform, khadas_mcu_user_mem_id_table); > + > +static struct platform_driver khadas_mcu_user_mem_driver = { > + .probe = khadas_mcu_user_mem_probe, > + .driver = { > + .name = "khadas-mcu-user-mem", > + }, > + .id_table = khadas_mcu_user_mem_id_table, > +}; > + > +module_platform_driver(khadas_mcu_user_mem_driver); > + > +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>"); > +MODULE_DESCRIPTION("Khadas MCU User MEM driver"); > +MODULE_LICENSE("GPL v2"); >
| |