Re: [PATCH v7 2/7] nvmem: add driver for JZ4780 efuse

Am 28.02.2020 um 15:48 schrieb Paul Cercueil paul@crapouillou.net:

Hi Nikolaus,

Le ven., févr. 28, 2020 at 14:58, H. Nikolaus Schaller hns@goldelico.com a écrit :

...

From: PrasannaKumar Muralidharan prasannatsmkumar@gmail.com This patch brings support for the JZ4780 efuse. Currently it only exposes a read only access to the entire 8K bits efuse memory and nvmem cells. To fetch for example the MAC address: dd if=/sys/devices/platform/134100d0.efuse/jz4780-efuse0/nvmem bs=1 skip=34 count=6 status=none | xxd Tested-by: Mathieu Malaterre malat@debian.org Signed-off-by: PrasannaKumar Muralidharan prasannatsmkumar@gmail.com Signed-off-by: Mathieu Malaterre malat@debian.org Signed-off-by: H. Nikolaus Schaller hns@goldelico.com Signed-off-by: Paul Cercueil paul@crapouillou.net

---

drivers/nvmem/Kconfig | 12 ++ drivers/nvmem/Makefile | 2 + drivers/nvmem/jz4780-efuse.c | 234 +++++++++++++++++++++++++++++++++++ 3 files changed, 248 insertions(+) create mode 100644 drivers/nvmem/jz4780-efuse.c diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index 35efab1ba8d9..d7b7f6d688e7 100644 --- a/drivers/nvmem/Kconfig +++ b/drivers/nvmem/Kconfig @@ -55,6 +55,18 @@ config NVMEM_IMX_OCOTP_SCU This is a driver for the SCU On-Chip OTP Controller (OCOTP) available on i.MX8 SoCs. +config JZ4780_EFUSE

• tristate "JZ4780 EFUSE Memory Support"
• depends on MACH_INGENIC || COMPILE_TEST
• depends on HAS_IOMEM
• depends on OF
• select REGMAP_MMIO
• help

•  Say Y here to include support for JZ4780 efuse memory found on
  

•  all JZ4780 SoC based devices.
  

•  To compile this driver as a module, choose M here: the module
  

•  will be called nvmem_jz4780_efuse.
  

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 6b466cd1427b..65a268d17807 100644 --- a/drivers/nvmem/Makefile +++ b/drivers/nvmem/Makefile @@ -18,6 +18,8 @@ obj-$(CONFIG_NVMEM_IMX_OCOTP) += nvmem-imx-ocotp.o nvmem-imx-ocotp-y := imx-ocotp.o 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_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/jz4780-efuse.c b/drivers/nvmem/jz4780-efuse.c new file mode 100644 index 000000000000..4e9dd340e33a --- /dev/null +++ b/drivers/nvmem/jz4780-efuse.c @@ -0,0 +1,234 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*

• • JZ4780 EFUSE Memory Support driver
• • Copyright (c) 2017 PrasannaKumar Muralidharan prasannatsmkumar@gmail.com
• • Copyright (c) 2020 H. Nikolaus Schaller hns@goldelico.com
• */

+/*

• • Currently supports JZ4780 efuse which has 8K programmable bit.
• • Efuse is separated into seven segments as below:

• • ---

• • | 64 bit | 128 bit | 128 bit | 3520 bit | 8 bit | 2296 bit | 2048 bit |

• • ---

• • The rom itself is accessed using a 9 bit address line and an 8 word wide bus
• • which reads/writes based on strobes. The strobe is configured in the config
• • register and is based on number of cycles of the bus clock.
• • Driver supports read only as the writes are done in the Factory.
• */

+#include <linux/bitops.h> +#include <linux/clk.h> +#include <linux/module.h> +#include <linux/nvmem-provider.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/timer.h>

+#define JZ_EFUCTRL (0x0) /* Control Register */ +#define JZ_EFUCFG (0x4) /* Configure Register*/ +#define JZ_EFUSTATE (0x8) /* Status Register */ +#define JZ_EFUDATA(n) (0xC + (n) * 4)

+/* We read 32 byte chunks to avoid complexity in the driver. */ +#define JZ_EFU_READ_SIZE 32

+#define EFUCTRL_ADDR_MASK 0x3FF +#define EFUCTRL_ADDR_SHIFT 21 +#define EFUCTRL_LEN_MASK 0x1F +#define EFUCTRL_LEN_SHIFT 16 +#define EFUCTRL_PG_EN BIT(15) +#define EFUCTRL_WR_EN BIT(1) +#define EFUCTRL_RD_EN BIT(0)

+#define EFUCFG_INT_EN BIT(31) +#define EFUCFG_RD_ADJ_MASK 0xF +#define EFUCFG_RD_ADJ_SHIFT 20 +#define EFUCFG_RD_STR_MASK 0xF +#define EFUCFG_RD_STR_SHIFT 16 +#define EFUCFG_WR_ADJ_MASK 0xF +#define EFUCFG_WR_ADJ_SHIFT 12 +#define EFUCFG_WR_STR_MASK 0xFFF +#define EFUCFG_WR_STR_SHIFT 0

+#define EFUSTATE_WR_DONE BIT(1) +#define EFUSTATE_RD_DONE BIT(0)

+struct jz4780_efuse {

• struct device *dev;
• struct regmap *map;
• struct clk *clk;

+};

+/* main entry point */ +static int jz4780_efuse_read(void *context, unsigned int offset,

• 	     void *val, size_t bytes)
  

+{

• struct jz4780_efuse *efuse = context;
• while (bytes > 0) {

• unsigned int start = offset & ~(JZ_EFU_READ_SIZE - 1);
  

• unsigned int chunk = min(bytes, (start + JZ_EFU_READ_SIZE)
  

• 			 - offset);
  

• char buf[JZ_EFU_READ_SIZE];
  

• unsigned int tmp;
  

• u32 ctrl;
  

• int ret;
  

• ctrl = (start << EFUCTRL_ADDR_SHIFT)
  

• 	| ((JZ_EFU_READ_SIZE - 1) << EFUCTRL_LEN_SHIFT)
  

• 	| EFUCTRL_RD_EN;
  

• regmap_update_bits(efuse->map, JZ_EFUCTRL,
  

• 		   (EFUCTRL_ADDR_MASK << EFUCTRL_ADDR_SHIFT) |
  

• 		   (EFUCTRL_LEN_MASK << EFUCTRL_LEN_SHIFT) |
  

• 		   EFUCTRL_PG_EN | EFUCTRL_WR_EN |
  

• 		   EFUCTRL_RD_EN,
  

• 		   ctrl);
  

• ret = regmap_read_poll_timeout(efuse->map, JZ_EFUSTATE,
  

• 			       tmp, tmp & EFUSTATE_RD_DONE,
  

• 			       1 * MSEC_PER_SEC,
  

• 			       50 * MSEC_PER_SEC);
  

• if (ret < 0) {
  

• 	dev_err(efuse->dev, "Time out while reading efuse data");
  

• 	return ret;
  

• }
  

• ret = regmap_bulk_read(efuse->map, JZ_EFUDATA(0),
  

• 		       buf, JZ_EFU_READ_SIZE / sizeof(u32));
  

• if (ret < 0)
  

• 	return ret;
  

• memcpy(val, &buf[offset - start], chunk);
  

• val += chunk;
  

• offset += chunk;
  

• bytes -= chunk;
  

• }
• return 0;

+}

+static struct nvmem_config jz4780_efuse_nvmem_config = {

• .name = "jz4780-efuse",
• .size = 1024,
• .word_size = 1,
• .stride = 1,
• .owner = THIS_MODULE,
• .reg_read = jz4780_efuse_read,

+};

+static const struct regmap_config jz4780_efuse_regmap_config = {

• .reg_bits = 32,
• .val_bits = 32,
• .reg_stride = 4,
• .max_register = JZ_EFUDATA(7),

+};

+static int jz4780_efuse_probe(struct platform_device *pdev) +{

• struct nvmem_device *nvmem;
• struct jz4780_efuse *efuse;
• struct nvmem_config cfg;
• unsigned long clk_rate;
• unsigned long rd_adj;
• unsigned long rd_strobe;
• struct device *dev = &pdev->dev;
• void __iomem *regs;
• int ret;
• efuse = devm_kzalloc(dev, sizeof(*efuse), GFP_KERNEL);
• if (!efuse)

• return -ENOMEM;
  

• regs = devm_platform_ioremap_resource(pdev, 0);
• if (IS_ERR(regs))

• return PTR_ERR(regs);
  

• efuse->map = devm_regmap_init_mmio(dev, regs,

• 			   &jz4780_efuse_regmap_config);
  

• if (IS_ERR(efuse->map))

• return PTR_ERR(efuse->map);
  

• efuse->clk = devm_clk_get(&pdev->dev, NULL);
• if (IS_ERR(efuse->clk))

• return PTR_ERR(efuse->clk);
  

• ret = clk_prepare_enable(efuse->clk);
• if (ret < 0)

• return ret;
  

• ret = devm_add_action_or_reset(&pdev->dev,

• 		       &clk_disable_unprepare,
  

• 		       efuse->clk);
  

That's what I thought, this does not build:

CC drivers/nvmem/jz4780-efuse.o drivers/nvmem/jz4780-efuse.c: In function 'jz4780_efuse_probe': drivers/nvmem/jz4780-efuse.c:168:12: error: passing argument 2 of 'devm_add_action_or_reset' from incompatible pointer type [-Werror=incompatible-pointer-types] 168 | &clk_disable_unprepare, | ^~~~~~~~~~~~~~~~~~~~~~ | | | void (*)(struct clk *) In file included from ./include/linux/platform_device.h:13, from drivers/nvmem/jz4780-efuse.c:29: ./include/linux/device.h:256:16: note: expected 'void (*)(void *)' but argument is of type 'void (*)(struct clk *)' 256 | void (*action)(void *), void *data) | ~~~~~~~^~~~~~~~~~~~~~~ cc1: some warnings being treated as errors make[2]: *** [scripts/Makefile.build:268: drivers/nvmem/jz4780-efuse.o] Error 1 make[1]: *** [scripts/Makefile.build:505: drivers/nvmem] Error 2 make: *** [Makefile:1681: drivers] Error 2

You need a local function of type 'void foo(void *data)' that just calls clk_disable_unprepare(data) and the compiler will be happy. (btw - no need to deference functions pointers, since they are already pointers)

Well, this asks for a devm_clk_enable_prepare but it was apparently rejected...

https://lore.kernel.org/patchwork/patch/755667/

With that said, I expect you to at least compile-test the patchset before sending it upstream...

I have compiled and tested it on real hardware. But missed the warning because my cross-compiler seems to emit a warning only.

Sorry, I don't have a good day and after 7 rounds one wants to get things through the door and with least effort as quick as possible :)

BR, Nikolaus