Skip to contentSkip to conversation

MIPS: CI20: Add efuse driver for Ingenic JZ4780 and attach to DM9000 for stable MAC addresses

RFC Superseded H. Nikolaus Schaller 3 participants 14 messages Feb 16, 2020 – Feb 17, 2020

Subsystems (4)

v3 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

v3 cover MIPS: CI20: Add efuse driver for Ingenic JZ4780 and attach to DM9000 for stable MAC addresses

This series is based on and a follow up for

https://lore.kernel.org/patchwork/cover/868157/

("[v2,0/2] Add efuse driver for Ingenic JZ4780 SoC")

Original authors were
PrasannaKumar Muralidharan <[email protected]> Mathieu Malaterre <[email protected]>

and there are additions / code improvements by
H. Nikolaus Schaller <[email protected]>
Paul Cercueil <[email protected]>

This setup works, if the dm9000 driver is compiled as a module.

Therefore it is all RFC level. It is also not completely checkpatched.

H. Nikolaus Schaller (2):
MIPS: DTS: CI20: make DM9000 Ethernet controller use NVMEM to find the
default MAC address
MIPS: CI20 defconfig: DEMO HACK: make DM9000 a module

Mathieu Malaterre (1):
MIPS: CI20 defconfig: Probe efuse for CI20

Paul Cercueil (1):
rework to use regmap

PrasannaKumar Muralidharan (5):
nvmem: add driver for JZ4780 efuse
Bindings: nvmem: add bindings for JZ4780 efuse
Documentation: ABI: nvmem: add documentation for JZ4780 efuse ABI
nvmem: MAINTAINERS: add maintainer for JZ4780 efuse driver
MIPS: DTS: JZ4780: define node for JZ4780 efuse

 .../ABI/testing/sysfs-driver-jz4780-efuse     |  16 ++
 .../bindings/nvmem/ingenic,jz4780-efuse.txt   |  17 ++
 MAINTAINERS                                   |   5 +
 arch/mips/boot/dts/ingenic/ci20.dts           |   3 +
 arch/mips/boot/dts/ingenic/jz4780.dtsi        |  17 +-
 arch/mips/configs/ci20_defconfig              |   4 +-
 drivers/nvmem/Kconfig                         |  10 +
 drivers/nvmem/Makefile                        |   2 +
 drivers/nvmem/jz4780-efuse.c                  | 238 ++++++++++++++++++
 9 files changed, 310 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-jz4780-efuse
 create mode 100644 Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt
 create mode 100644 drivers/nvmem/jz4780-efuse.c
Signature · 1 line
2.23.0
#1 lore raw

RFC v3 1/9 nvmem: add driver for JZ4780 efuse

From: PrasannaKumar Muralidharan <[email protected]>

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.

(Signed-off-by: Paul Cercueil <[email protected]>)

---
 drivers/nvmem/Kconfig        |  10 ++
 drivers/nvmem/Makefile       |   2 +
 drivers/nvmem/jz4780-efuse.c | 249 +++++++++++++++++++++++++++++++++++
 3 files changed, 261 insertions(+)
 create mode 100644 drivers/nvmem/jz4780-efuse.c
drivers/nvmem/Kconfig +10 -0
index 35efab1ba8d9..10f8e08f5e31 100644
@@ -55,6 +55,16 @@ 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_JZ4780 || COMPILE_TEST
+	depends on HAS_IOMEM
+	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
drivers/nvmem/Makefile +2 -0
index 6b466cd1427b..65a268d17807 100644
@@ -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
drivers/nvmem/jz4780-efuse.c new file +249 -0

249 lines not on this page · raw message

Signature · 1 line
2.23.0
#2 lore raw

Re: [RFC v3 1/9] nvmem: add driver for JZ4780 efuse

Quoted 9 lines · H. Nikolaus Schaller

From: PrasannaKumar Muralidharan <[email protected]>

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.

Quoted 8 lines

(Signed-off-by: Paul Cercueil <[email protected]>)

---
  drivers/nvmem/Kconfig        |  10 ++
  drivers/nvmem/Makefile       |   2 +
  drivers/nvmem/jz4780-efuse.c | 249 +++++++++++++++++++++++++++++++++++
  3 files changed, 261 insertions(+)

create mode 100644 drivers/nvmem/jz4780-efuse.c

This patch along with 2/9 should be merged into single patch. Also please make sure you run checkpatch.pl before sending!

drivers/nvmem/Kconfig config NVMEM_IMX_OCOTP_SCU · line 60

Quoted 11 lines
drivers/nvmem/Kconfig +3 -0
index 35efab1ba8d9..10f8e08f5e31 100644
@@ -55,6 +55,16 @@ 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_JZ4780 || COMPILE_TEST

what is that this driver depends on MACH_JZ4780 board?

Quoted 52 lines
drivers/nvmem/Kconfig +7 -0
+	depends on HAS_IOMEM
+	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
drivers/nvmem/Makefile +2 -0
index 6b466cd1427b..65a268d17807 100644
@@ -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
drivers/nvmem/jz4780-efuse.c new file +23 -0
new file mode 100644
index 000000000000..ac03e1900ef9
@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * JZ4780 EFUSE Memory Support driver
+ *
+ * Copyright (c) 2017 PrasannaKumar Muralidharan <[email protected]>
+ * Copyright (c) 2020 H. Nikolaus Schaller <[email protected]>
+ */
+
+/*
+ * 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.
+ */
+

drivers/nvmem/jz4780-efuse.c line 31

Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +8 -0
+#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>

?? why do we need these two headers in this patch

Quoted 9 lines
drivers/nvmem/jz4780-efuse.c new file +9 -0
+
+#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)
+
+#define JZ_EFUSE_START_ADDR		0x200
+
+#define JZ_EFUSE_EFUCTRL_CS		BIT(30)

drivers/nvmem/jz4780-efuse.c line 50

Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10 -0
+#define JZ_EFUSE_EFUCTRL_ADDR_MASK	0x1FF
+#define JZ_EFUSE_EFUCTRL_ADDR_SHIFT	21
+#define JZ_EFUSE_EFUCTRL_LEN_MASK	0x1F
+#define JZ_EFUSE_EFUCTRL_LEN_SHIFT	16
+#define JZ_EFUSE_EFUCTRL_PG_EN		BIT(15)
+#define JZ_EFUSE_EFUCTRL_WR_EN		BIT(1)
+#define JZ_EFUSE_EFUCTRL_RD_EN		BIT(0)
+
+#define JZ_EFUSE_EFUCFG_INT_EN		BIT(31)
+#define JZ_EFUSE_EFUCFG_RD_ADJ_MASK	0xF

consider using GENMASK for these masks here.

Quoted 15 lines
drivers/nvmem/jz4780-efuse.c new file +15 -0
+#define JZ_EFUSE_EFUCFG_RD_ADJ_SHIFT	20
+#define JZ_EFUSE_EFUCFG_RD_STR_MASK	0xF
+#define JZ_EFUSE_EFUCFG_RD_STR_SHIFT	16
+#define JZ_EFUSE_EFUCFG_WR_ADJ_MASK	0xF
+#define JZ_EFUSE_EFUCFG_WR_ADJ_SHIFT	12
+#define JZ_EFUSE_EFUCFG_WR_STR_MASK	0xFFF
+#define JZ_EFUSE_EFUCFG_WR_STR_SHIFT	0
+
+#define JZ_EFUSE_EFUSTATE_WR_DONE	BIT(1)
+#define JZ_EFUSE_EFUSTATE_RD_DONE	BIT(0)
+
+struct jz4780_efuse {
+	struct device *dev;
+	void __iomem *iomem;
+	struct clk *clk;

drivers/nvmem/jz4780-efuse.c line 75

Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10 -0
+	unsigned int rd_adj;
+	unsigned int rd_strobe;
+};
+
+/* We read 32 byte chunks to avoid complexity in the driver. */
+static int jz4780_efuse_read_32bytes(struct jz4780_efuse *efuse, char *buf,
+		unsigned int addr)
+{
+	unsigned int tmp = 0;
+	int i = 0;

unnecessary initialization of both variables.

drivers/nvmem/jz4780-efuse.c line 77

Quoted 2 lines
drivers/nvmem/jz4780-efuse.c new file +2 -0
+	int timeout = 1000;
+	int size = 32;

better to #define this STRIDE/CHUNK size. this driver seems to use this value in multiple places.

drivers/nvmem/jz4780-efuse.c line 84

Quoted 7 lines
drivers/nvmem/jz4780-efuse.c new file +7 -0
+
+	/* 1. Set config register */
+	tmp = readl(efuse->iomem + JZ_EFUCFG);
+	tmp &= ~((JZ_EFUSE_EFUCFG_RD_ADJ_MASK << JZ_EFUSE_EFUCFG_RD_ADJ_SHIFT)
+	| (JZ_EFUSE_EFUCFG_RD_STR_MASK << JZ_EFUSE_EFUCFG_RD_STR_SHIFT));
+	tmp |= (efuse->rd_adj << JZ_EFUSE_EFUCFG_RD_ADJ_SHIFT)
+	| (efuse->rd_strobe << JZ_EFUSE_EFUCFG_RD_STR_SHIFT);

very odd indenting.

Quoted 29 lines
drivers/nvmem/jz4780-efuse.c new file +29 -0
+	writel(tmp, efuse->iomem + JZ_EFUCFG);
+
+	/*
+	 * 2. Set control register to indicate what to read data address,
+	 * read data numbers and read enable.
+	 */
+	tmp = readl(efuse->iomem + JZ_EFUCTRL);
+	tmp &= ~(JZ_EFUSE_EFUCFG_RD_STR_SHIFT
+		| (JZ_EFUSE_EFUCTRL_ADDR_MASK << JZ_EFUSE_EFUCTRL_ADDR_SHIFT)
+		| JZ_EFUSE_EFUCTRL_PG_EN | JZ_EFUSE_EFUCTRL_WR_EN
+		| JZ_EFUSE_EFUCTRL_WR_EN);
+
+	/* Need to select CS bit if address accesses upper 4Kbits memory */
+	if (addr >= (JZ_EFUSE_START_ADDR + 512))
+		tmp |= JZ_EFUSE_EFUCTRL_CS;
+
+	tmp |= (addr << JZ_EFUSE_EFUCTRL_ADDR_SHIFT)
+		| ((size - 1) << JZ_EFUSE_EFUCTRL_LEN_SHIFT)
+		| JZ_EFUSE_EFUCTRL_RD_EN;
+	writel(tmp, efuse->iomem + JZ_EFUCTRL);
+
+	/*
+	 * 3. Wait status register RD_DONE set to 1 or EFUSE interrupted,
+	 * software can read EFUSE data buffer 0 - 8 registers.
+	 */
+	do {
+		tmp = readl(efuse->iomem + JZ_EFUSTATE);
+		usleep_range(1000, 2000);
+		if (timeout--)

drivers/nvmem/jz4780-efuse.c line 123

Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10 -0
+			break;
+	} while (!(tmp & JZ_EFUSE_EFUSTATE_RD_DONE));
+
+	if (timeout <= 0) {
+		dev_err(efuse->dev, "Timed out while reading\n");
+		return -EAGAIN;
+	}
+
+	for (i = 0; i < (size / 4); i++)
+		*((unsigned int *)(buf + i * 4))

make "unsigned int *buf32" a local variable and use it here, makes it much readable code.

Quoted 31 lines
drivers/nvmem/jz4780-efuse.c new file +31 -0
+			 = readl(efuse->iomem + JZ_EFUDATA(i));
+
+	return 0;
+}
+
+/* main entry point */
+static int jz4780_efuse_read(void *context, unsigned int offset,
+					void *val, size_t bytes)
+{
+	struct jz4780_efuse *efuse = context;
+	int ret;
+
+	while (bytes > 0) {
+		unsigned int start = offset & ~(32 - 1);
+		unsigned chunk = min(bytes, (start + 32 - offset));
+
+		if (start == offset && chunk == 32) {
+			ret = jz4780_efuse_read_32bytes(efuse, val, start);
+			if (ret < 0)
+				return ret;
+
+		} else {
+			char buf[32];
+			ret = jz4780_efuse_read_32bytes(efuse, buf, start);
+			if (ret < 0)
+				return ret;
+
+			memcpy(val, &buf[offset - start], chunk);
+		}
+
+		val += chunk;

drivers/nvmem/jz4780-efuse.c line 164

Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10 -0
+		offset += chunk;
+		bytes -= chunk;
+	}
+
+	return 0;
+}
+
+static struct nvmem_config jz4780_efuse_nvmem_config = {
+	.name = "jz4780-efuse",
+	.read_only = true,

this value comes from device tree bindings, do we still need to specify this here?

Quoted 17 lines
drivers/nvmem/jz4780-efuse.c new file +17 -0
+	.size = 1024,
+	.word_size = 1,
+	.stride = 1,
+	.owner = THIS_MODULE,
+	.reg_read = jz4780_efuse_read,
+};
+
+static int jz4780_efuse_probe(struct platform_device *pdev)
+{
+	struct nvmem_device *nvmem;
+	struct jz4780_efuse *efuse;
+	struct resource *res;
+	unsigned long clk_rate;
+	struct device *dev = &pdev->dev;
+
+	efuse = devm_kzalloc(&pdev->dev, sizeof(*efuse), GFP_KERNEL);
+	if (!efuse)

drivers/nvmem/jz4780-efuse.c line 191

Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10 -0
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	efuse->iomem = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	if (IS_ERR(efuse->iomem))
+		return PTR_ERR(efuse->iomem);
+
+	efuse->clk = devm_clk_get(&pdev->dev, "bus_clk");
+	if (IS_ERR(efuse->clk))
+		return PTR_ERR(efuse->clk);

Who is enabling this clk?

Quoted 11 lines
drivers/nvmem/jz4780-efuse.c new file +11 -0
+
+	clk_rate = clk_get_rate(efuse->clk);
+	/*
+	 * rd_adj and rd_strobe are 4 bit values
+	 * bus clk period * (rd_adj + 1) > 6.5ns
+	 * bus clk period * (rd_adj + 5 + rd_strobe) > 35ns
+	 */
+	efuse->rd_adj = (((6500 * (clk_rate / 1000000)) / 1000000) + 1) - 1;
+	efuse->rd_strobe = ((((35000 * (clk_rate / 1000000)) / 1000000) + 1)
+						- 5 - efuse->rd_adj);
+

drivers/nvmem/jz4780-efuse.c line 212

Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10 -0
+	if ((efuse->rd_adj > 0x1F) || (efuse->rd_strobe > 0x1F)) {
+		dev_err(&pdev->dev, "Cannot set clock configuration\n");
+		return -EINVAL;
+	}
+	efuse->dev = dev;
+
+	jz4780_efuse_nvmem_config.dev = &pdev->dev;
+	jz4780_efuse_nvmem_config.priv = efuse;
+
+	nvmem = nvmem_register(&jz4780_efuse_nvmem_config);

devm variant here?

Quoted 38 lines
drivers/nvmem/jz4780-efuse.c new file +37 -0
+	if (IS_ERR(nvmem))
+		return PTR_ERR(nvmem);
+
+	platform_set_drvdata(pdev, nvmem);
+
+	return 0;
+}
+
+static int jz4780_efuse_remove(struct platform_device *pdev)
+{
+	struct nvmem_device *nvmem = platform_get_drvdata(pdev);
+
+	nvmem_unregister(nvmem);
+
+	return 0;
+}
+
+static const struct of_device_id jz4780_efuse_match[] = {
+	{ .compatible = "ingenic,jz4780-efuse" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, jz4780_efuse_match);
+
+static struct platform_driver jz4780_efuse_driver = {
+	.probe  = jz4780_efuse_probe,
+	.remove = jz4780_efuse_remove,
+	.driver = {
+		.name = "jz4780-efuse",
+		.of_match_table = jz4780_efuse_match,
+	},
+};
+module_platform_driver(jz4780_efuse_driver);
+
+MODULE_AUTHOR("PrasannaKumar Muralidharan <[email protected]>");
+MODULE_AUTHOR("H. Nikolaus Schaller <[email protected]>");
+MODULE_DESCRIPTION("Ingenic JZ4780 efuse driver");
+MODULE_LICENSE("GPL v2");
#3 lore raw

Re: [RFC v3 1/9] nvmem: add driver for JZ4780 efuse

Hi Srinivas Kandagatla,

Quoted 4 lines

Am 17.02.2020 um 12:36 schrieb Srinivas Kandagatla <[email protected]>:

Quoted 9 lines
Quoted 8 lines · H. Nikolaus Schaller

From: PrasannaKumar Muralidharan <[email protected]> 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.

(Signed-off-by: Paul Cercueil <[email protected]>)

@Paul: may I assume your signed off after squashing the patch 2/9?

Quoted 9 lines
Quoted 6 lines
---
 drivers/nvmem/Kconfig        |  10 ++
 drivers/nvmem/Makefile       |   2 +
 drivers/nvmem/jz4780-efuse.c | 249 +++++++++++++++++++++++++++++++++++
 3 files changed, 261 insertions(+)
 create mode 100644 drivers/nvmem/jz4780-efuse.c

This patch along with 2/9 should be merged into single patch. Also please make sure you run checkpatch.pl before sending!

Sure. I had noted that in the [0/9].

Quoted 13 lines

drivers/nvmem/Kconfig config NVMEM_IMX_OCOTP_SCU · line 59

Quoted 11 lines
drivers/nvmem/Kconfig +2 -0
index 35efab1ba8d9..10f8e08f5e31 100644
@@ -55,6 +55,16 @@ 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_JZ4780 || COMPILE_TEST

what is that this driver depends on MACH_JZ4780 board?

Hm. Good question. Well, it is only useful for the jz4780 SoC but since we match drivers through DTS there is probably no reason to make it depend on. And depend on COMPILE_TEST is also not needed if I understand correctly.

On the other hand this makes the option disappear for non-jz4780 SoC.

So I leave it for the moment but keep in mind.

Quoted 62 lines
Quoted 60 lines
drivers/nvmem/Kconfig +7 -0
+	depends on HAS_IOMEM
+	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
drivers/nvmem/Makefile +2 -0
index 6b466cd1427b..65a268d17807 100644
@@ -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
drivers/nvmem/jz4780-efuse.c new file +29 -0
new file mode 100644
index 000000000000..ac03e1900ef9
@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * JZ4780 EFUSE Memory Support driver
+ *
+ * Copyright (c) 2017 PrasannaKumar Muralidharan <[email protected]>
+ * Copyright (c) 2020 H. Nikolaus Schaller <[email protected]>
+ */
+
+/*
+ * 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>
Quoted 3 lines

drivers/nvmem/jz4780-efuse.c line 31

Quoted 2 lines
drivers/nvmem/jz4780-efuse.c new file +2 -0
+#include <linux/regmap.h>
+#include <linux/timer.h>

?? why do we need these two headers in this patch

regmap will be used after squashing 2/9.

Quoted 57 lines
Quoted 11 lines
drivers/nvmem/jz4780-efuse.c new file +9 -0
+
+#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)
+
+#define JZ_EFUSE_START_ADDR		0x200
+
+#define JZ_EFUSE_EFUCTRL_CS		BIT(30)

drivers/nvmem/jz4780-efuse.c line 50

Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10 -0
+#define JZ_EFUSE_EFUCTRL_ADDR_MASK	0x1FF
+#define JZ_EFUSE_EFUCTRL_ADDR_SHIFT	21
+#define JZ_EFUSE_EFUCTRL_LEN_MASK	0x1F
+#define JZ_EFUSE_EFUCTRL_LEN_SHIFT	16
+#define JZ_EFUSE_EFUCTRL_PG_EN		BIT(15)
+#define JZ_EFUSE_EFUCTRL_WR_EN		BIT(1)
+#define JZ_EFUSE_EFUCTRL_RD_EN		BIT(0)
+
+#define JZ_EFUSE_EFUCFG_INT_EN		BIT(31)
+#define JZ_EFUSE_EFUCFG_RD_ADJ_MASK	0xF

consider using GENMASK for these masks here.

Quoted 15 lines
drivers/nvmem/jz4780-efuse.c new file +15 -0
+#define JZ_EFUSE_EFUCFG_RD_ADJ_SHIFT	20
+#define JZ_EFUSE_EFUCFG_RD_STR_MASK	0xF
+#define JZ_EFUSE_EFUCFG_RD_STR_SHIFT	16
+#define JZ_EFUSE_EFUCFG_WR_ADJ_MASK	0xF
+#define JZ_EFUSE_EFUCFG_WR_ADJ_SHIFT	12
+#define JZ_EFUSE_EFUCFG_WR_STR_MASK	0xFFF
+#define JZ_EFUSE_EFUCFG_WR_STR_SHIFT	0
+
+#define JZ_EFUSE_EFUSTATE_WR_DONE	BIT(1)
+#define JZ_EFUSE_EFUSTATE_RD_DONE	BIT(0)
+
+struct jz4780_efuse {
+	struct device *dev;
+	void __iomem *iomem;
+	struct clk *clk;

drivers/nvmem/jz4780-efuse.c line 75

Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10 -0
+	unsigned int rd_adj;
+	unsigned int rd_strobe;
+};
+
+/* We read 32 byte chunks to avoid complexity in the driver. */
+static int jz4780_efuse_read_32bytes(struct jz4780_efuse *efuse, char *buf,
+		unsigned int addr)
+{
+	unsigned int tmp = 0;
+	int i = 0;

unnecessary initialization of both variables.

drivers/nvmem/jz4780-efuse.c line 77

Quoted 2 lines
drivers/nvmem/jz4780-efuse.c new file +2 -0
+	int timeout = 1000;
+	int size = 32;

better to #define this STRIDE/CHUNK size. this driver seems to use this value in multiple places.

Quoted 9 lines

drivers/nvmem/jz4780-efuse.c line 84

Quoted 7 lines
drivers/nvmem/jz4780-efuse.c new file +7 -0
+
+	/* 1. Set config register */
+	tmp = readl(efuse->iomem + JZ_EFUCFG);
+	tmp &= ~((JZ_EFUSE_EFUCFG_RD_ADJ_MASK << JZ_EFUSE_EFUCFG_RD_ADJ_SHIFT)
+	| (JZ_EFUSE_EFUCFG_RD_STR_MASK << JZ_EFUSE_EFUCFG_RD_STR_SHIFT));
+	tmp |= (efuse->rd_adj << JZ_EFUSE_EFUCFG_RD_ADJ_SHIFT)
+	| (efuse->rd_strobe << JZ_EFUSE_EFUCFG_RD_STR_SHIFT);

very odd indenting.

Will be replaced by 2/9.

Sorry for having you review this old version which is replaced by 2/9.

But it appeared more helpful to keep this as the 2018 original and then update in a separate patch for this RFC stage.

Quoted 78 lines
Quoted 30 lines
drivers/nvmem/jz4780-efuse.c new file +29 -0
+	writel(tmp, efuse->iomem + JZ_EFUCFG);
+
+	/*
+	 * 2. Set control register to indicate what to read data address,
+	 * read data numbers and read enable.
+	 */
+	tmp = readl(efuse->iomem + JZ_EFUCTRL);
+	tmp &= ~(JZ_EFUSE_EFUCFG_RD_STR_SHIFT
+		| (JZ_EFUSE_EFUCTRL_ADDR_MASK << JZ_EFUSE_EFUCTRL_ADDR_SHIFT)
+		| JZ_EFUSE_EFUCTRL_PG_EN | JZ_EFUSE_EFUCTRL_WR_EN
+		| JZ_EFUSE_EFUCTRL_WR_EN);
+
+	/* Need to select CS bit if address accesses upper 4Kbits memory */
+	if (addr >= (JZ_EFUSE_START_ADDR + 512))
+		tmp |= JZ_EFUSE_EFUCTRL_CS;
+
+	tmp |= (addr << JZ_EFUSE_EFUCTRL_ADDR_SHIFT)
+		| ((size - 1) << JZ_EFUSE_EFUCTRL_LEN_SHIFT)
+		| JZ_EFUSE_EFUCTRL_RD_EN;
+	writel(tmp, efuse->iomem + JZ_EFUCTRL);
+
+	/*
+	 * 3. Wait status register RD_DONE set to 1 or EFUSE interrupted,
+	 * software can read EFUSE data buffer 0 - 8 registers.
+	 */
+	do {
+		tmp = readl(efuse->iomem + JZ_EFUSTATE);
+		usleep_range(1000, 2000);
+		if (timeout--)

drivers/nvmem/jz4780-efuse.c line 123

Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10 -0
+			break;
+	} while (!(tmp & JZ_EFUSE_EFUSTATE_RD_DONE));
+
+	if (timeout <= 0) {
+		dev_err(efuse->dev, "Timed out while reading\n");
+		return -EAGAIN;
+	}
+
+	for (i = 0; i < (size / 4); i++)
+		*((unsigned int *)(buf + i * 4))

make "unsigned int *buf32" a local variable and use it here, makes it much readable code.

Quoted 35 lines
drivers/nvmem/jz4780-efuse.c new file +35 -0
+			 = readl(efuse->iomem + JZ_EFUDATA(i));
+
+	return 0;
+}
+
+/* main entry point */
+static int jz4780_efuse_read(void *context, unsigned int offset,
+					void *val, size_t bytes)
+{
+	struct jz4780_efuse *efuse = context;
+	int ret;
+
+	while (bytes > 0) {
+		unsigned int start = offset & ~(32 - 1);
+		unsigned chunk = min(bytes, (start + 32 - offset));
+
+		if (start == offset && chunk == 32) {
+			ret = jz4780_efuse_read_32bytes(efuse, val, start);
+			if (ret < 0)
+				return ret;
+
+		} else {
+			char buf[32];
+			ret = jz4780_efuse_read_32bytes(efuse, buf, start);
+			if (ret < 0)
+				return ret;
+
+			memcpy(val, &buf[offset - start], chunk);
+		}
+
+		val += chunk;
+		offset += chunk;
+		bytes -= chunk;
+	}
+
Quoted 8 lines

drivers/nvmem/jz4780-efuse.c line 164

Quoted 6 lines
drivers/nvmem/jz4780-efuse.c new file +6 -0
+	return 0;
+}
+
+static struct nvmem_config jz4780_efuse_nvmem_config = {
+	.name = "jz4780-efuse",
+	.read_only = true,

this value comes from device tree bindings, do we still need to specify this here?

Ok, have to check what is automatically taken from DT.

Quoted 48 lines
Quoted 19 lines
drivers/nvmem/jz4780-efuse.c new file +17 -0
+	.size = 1024,
+	.word_size = 1,
+	.stride = 1,
+	.owner = THIS_MODULE,
+	.reg_read = jz4780_efuse_read,
+};
+
+static int jz4780_efuse_probe(struct platform_device *pdev)
+{
+	struct nvmem_device *nvmem;
+	struct jz4780_efuse *efuse;
+	struct resource *res;
+	unsigned long clk_rate;
+	struct device *dev = &pdev->dev;
+
+	efuse = devm_kzalloc(&pdev->dev, sizeof(*efuse), GFP_KERNEL);
+	if (!efuse)

drivers/nvmem/jz4780-efuse.c line 191

Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10 -0
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	efuse->iomem = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	if (IS_ERR(efuse->iomem))
+		return PTR_ERR(efuse->iomem);
+
+	efuse->clk = devm_clk_get(&pdev->dev, "bus_clk");
+	if (IS_ERR(efuse->clk))
+		return PTR_ERR(efuse->clk);

Who is enabling this clk?

Quoted 15 lines
drivers/nvmem/jz4780-efuse.c new file +15 -0
+
+	clk_rate = clk_get_rate(efuse->clk);
+	/*
+	 * rd_adj and rd_strobe are 4 bit values
+	 * bus clk period * (rd_adj + 1) > 6.5ns
+	 * bus clk period * (rd_adj + 5 + rd_strobe) > 35ns
+	 */
+	efuse->rd_adj = (((6500 * (clk_rate / 1000000)) / 1000000) + 1) - 1;
+	efuse->rd_strobe = ((((35000 * (clk_rate / 1000000)) / 1000000) + 1)
+						- 5 - efuse->rd_adj);
+
+	if ((efuse->rd_adj > 0x1F) || (efuse->rd_strobe > 0x1F)) {
+		dev_err(&pdev->dev, "Cannot set clock configuration\n");
+		return -EINVAL;
+	}
Quoted 8 lines

drivers/nvmem/jz4780-efuse.c line 212

Quoted 6 lines
drivers/nvmem/jz4780-efuse.c new file +6 -0
+	efuse->dev = dev;
+
+	jz4780_efuse_nvmem_config.dev = &pdev->dev;
+	jz4780_efuse_nvmem_config.priv = efuse;
+
+	nvmem = nvmem_register(&jz4780_efuse_nvmem_config);

devm variant here?

Good idea! I didn't know that it exists. Allows us to remove jz4780_efuse_remove() below.

Quoted 31 lines
Quoted 31 lines
drivers/nvmem/jz4780-efuse.c new file +29 -0
+	if (IS_ERR(nvmem))
+		return PTR_ERR(nvmem);
+
+	platform_set_drvdata(pdev, nvmem);
+
+	return 0;
+}
+
+static int jz4780_efuse_remove(struct platform_device *pdev)
+{
+	struct nvmem_device *nvmem = platform_get_drvdata(pdev);
+
+	nvmem_unregister(nvmem);
+
+	return 0;
+}
+
+static const struct of_device_id jz4780_efuse_match[] = {
+	{ .compatible = "ingenic,jz4780-efuse" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, jz4780_efuse_match);
+
+static struct platform_driver jz4780_efuse_driver = {
+	.probe  = jz4780_efuse_probe,
+	.remove = jz4780_efuse_remove,
+	.driver = {
+		.name = "jz4780-efuse",
+		.of_match_table = jz4780_efuse_match,
Quoted 8 lines

drivers/nvmem/jz4780-efuse.c line 249

Quoted 8 lines
drivers/nvmem/jz4780-efuse.c new file +8 -0
+	},
+};
+module_platform_driver(jz4780_efuse_driver);
+
+MODULE_AUTHOR("PrasannaKumar Muralidharan <[email protected]>");
+MODULE_AUTHOR("H. Nikolaus Schaller <[email protected]>");
+MODULE_DESCRIPTION("Ingenic JZ4780 efuse driver");
+MODULE_LICENSE("GPL v2");

BR and thanks,
Nikolaus Schaller

maintainer
#4 lore raw

Re: [RFC v3 1/9] nvmem: add driver for JZ4780 efuse

Hi Nikolaus,

Quoted 20 lines · H. Nikolaus Schaller

Hi Srinivas Kandagatla,

Quoted 5 lines

Am 17.02.2020 um 12:36 schrieb Srinivas Kandagatla
<[email protected]>:

Quoted 12 lines
Quoted 11 lines · H. Nikolaus Schaller

From: PrasannaKumar Muralidharan <[email protected]> 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.

Signed-off-by PrasannaKumar Muralidharan

<[email protected]>

(Signed-off-by: Paul Cercueil <[email protected]>)

Quoted 1 line

@Paul: may I assume your signed off after squashing the patch 2/9?

Yes. As the Ingenic SoCs maintainer I'll still need to review though.

Quoted 28 lines
Quoted 11 lines
Quoted 8 lines

---

  drivers/nvmem/Kconfig        |  10 ++
  drivers/nvmem/Makefile       |   2 +
  drivers/nvmem/jz4780-efuse.c | 249 
+++++++++++++++++++++++++++++++++++
  3 files changed, 261 insertions(+)

create mode 100644 drivers/nvmem/jz4780-efuse.c

This patch along with 2/9 should be merged into single patch. Also please make sure you run checkpatch.pl before sending!

Sure. I had noted that in the [0/9].

Quoted 12 lines
Quoted 11 lines

diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index 35efab1ba8d9..10f8e08f5e31 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -55,6 +55,16 @@ 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_JZ4780 || COMPILE_TEST

Quoted 1 line

what is that this driver depends on MACH_JZ4780 board?

Quoted 8 lines

Hm. Good question. Well, it is only useful for the jz4780 SoC but since we match drivers through DTS there is probably no reason to make it depend on. And depend on COMPILE_TEST is also not needed if I understand correctly.

On the other hand this makes the option disappear for non-jz4780 SoC.

So I leave it for the moment but keep in mind.

Make it MACH_INGENIC || COMPILE_TEST.

* You want COMPILE_TEST so that the driver can be compile-tested. It should always be there unless the driver really isn't buildable if MACH_JZ4780 is not set.
* If you depend on MACH_JZ4780, then you cannot create a kernel that will work on anything else than a JZ4780. Depend on MACH_INGENIC instead.

Quoted 355 lines
Quoted 72 lines
Quoted 70 lines

+ depends on HAS_IOMEM
+ 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..ac03e1900ef9
--- /dev/null
+++ b/drivers/nvmem/jz4780-efuse.c
@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * JZ4780 EFUSE Memory Support driver
+ *
+ * Copyright (c) 2017 PrasannaKumar Muralidharan
<[email protected]>
+ * Copyright (c) 2020 H. Nikolaus Schaller <[email protected]>
+ */
+
+/*
+ * 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>

Quoted 3 lines
Quoted 2 lines

+#include <linux/regmap.h>
+#include <linux/timer.h>

?? why do we need these two headers in this patch

regmap will be used after squashing 2/9.

Quoted 68 lines
Quoted 13 lines

+
+#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)
+
+#define JZ_EFUSE_START_ADDR 0x200
+
+#define JZ_EFUSE_EFUCTRL_CS BIT(30)
+#define JZ_EFUSE_EFUCTRL_ADDR_MASK 0x1FF
+#define JZ_EFUSE_EFUCTRL_ADDR_SHIFT 21

Quoted 8 lines

+#define JZ_EFUSE_EFUCTRL_LEN_MASK 0x1F
+#define JZ_EFUSE_EFUCTRL_LEN_SHIFT 16
+#define JZ_EFUSE_EFUCTRL_PG_EN BIT(15)
+#define JZ_EFUSE_EFUCTRL_WR_EN BIT(1)
+#define JZ_EFUSE_EFUCTRL_RD_EN BIT(0)
+
+#define JZ_EFUSE_EFUCFG_INT_EN BIT(31)
+#define JZ_EFUSE_EFUCFG_RD_ADJ_MASK 0xF

consider using GENMASK for these masks here.

Quoted 18 lines

+#define JZ_EFUSE_EFUCFG_RD_ADJ_SHIFT 20
+#define JZ_EFUSE_EFUCFG_RD_STR_MASK 0xF
+#define JZ_EFUSE_EFUCFG_RD_STR_SHIFT 16
+#define JZ_EFUSE_EFUCFG_WR_ADJ_MASK 0xF
+#define JZ_EFUSE_EFUCFG_WR_ADJ_SHIFT 12
+#define JZ_EFUSE_EFUCFG_WR_STR_MASK 0xFFF
+#define JZ_EFUSE_EFUCFG_WR_STR_SHIFT 0
+
+#define JZ_EFUSE_EFUSTATE_WR_DONE BIT(1)
+#define JZ_EFUSE_EFUSTATE_RD_DONE BIT(0)
+
+struct jz4780_efuse {
+ struct device *dev;
+ void __iomem *iomem;
+ struct clk *clk;
+ unsigned int rd_adj;
+ unsigned int rd_strobe;
+};

Quoted 8 lines

+
+/* We read 32 byte chunks to avoid complexity in the driver. */ +static int jz4780_efuse_read_32bytes(struct jz4780_efuse *efuse,
char *buf,
+ unsigned int addr)
+{
+ unsigned int tmp = 0;
+ int i = 0;

unnecessary initialization of both variables.

Quoted 2 lines

+ int timeout = 1000;
+ int size = 32;

better to #define this STRIDE/CHUNK size. this driver seems to use
this value in multiple places.

Quoted 8 lines

+
+ /* 1. Set config register */
+ tmp = readl(efuse->iomem + JZ_EFUCFG);
+ tmp &= ~((JZ_EFUSE_EFUCFG_RD_ADJ_MASK <<
JZ_EFUSE_EFUCFG_RD_ADJ_SHIFT)
+ | (JZ_EFUSE_EFUCFG_RD_STR_MASK << JZ_EFUSE_EFUCFG_RD_STR_SHIFT));
+ tmp |= (efuse->rd_adj << JZ_EFUSE_EFUCFG_RD_ADJ_SHIFT)
+ | (efuse->rd_strobe << JZ_EFUSE_EFUCFG_RD_STR_SHIFT);

Quoted 1 line

very odd indenting.

Will be replaced by 2/9.

Sorry for having you review this old version which is replaced by 2/9.

But it appeared more helpful to keep this as the 2018 original and then update in a separate patch for this RFC stage.

Quoted 87 lines
Quoted 33 lines

+ writel(tmp, efuse->iomem + JZ_EFUCFG);
+
+ /*
+ * 2. Set control register to indicate what to read data address,
+ * read data numbers and read enable.
+ */
+ tmp = readl(efuse->iomem + JZ_EFUCTRL);
+ tmp &= ~(JZ_EFUSE_EFUCFG_RD_STR_SHIFT

 +		| (JZ_EFUSE_EFUCTRL_ADDR_MASK << JZ_EFUSE_EFUCTRL_ADDR_SHIFT)
 +		| JZ_EFUSE_EFUCTRL_PG_EN | JZ_EFUSE_EFUCTRL_WR_EN
 +		| JZ_EFUSE_EFUCTRL_WR_EN);

+
+ /* Need to select CS bit if address accesses upper 4Kbits memory
*/
+ if (addr >= (JZ_EFUSE_START_ADDR + 512))
+ tmp |= JZ_EFUSE_EFUCTRL_CS;
+
+ tmp |= (addr << JZ_EFUSE_EFUCTRL_ADDR_SHIFT)
+ | ((size - 1) << JZ_EFUSE_EFUCTRL_LEN_SHIFT)
+ | JZ_EFUSE_EFUCTRL_RD_EN;
+ writel(tmp, efuse->iomem + JZ_EFUCTRL);
+
+ /*
+ * 3. Wait status register RD_DONE set to 1 or EFUSE interrupted,
+ * software can read EFUSE data buffer 0 - 8 registers.
+ */
+ do {
+ tmp = readl(efuse->iomem + JZ_EFUSTATE);
+ usleep_range(1000, 2000);
+ if (timeout--)
+ break;
+ } while (!(tmp & JZ_EFUSE_EFUSTATE_RD_DONE));

Quoted 8 lines

+
+ if (timeout <= 0) {
+ dev_err(efuse->dev, "Timed out while reading\n");
+ return -EAGAIN;
+ }
+
+ for (i = 0; i < (size / 4); i++)
+ *((unsigned int *)(buf + i * 4))

make "unsigned int *buf32" a local variable and use it here, makes
it much readable code.

Quoted 33 lines

+ = readl(efuse->iomem + JZ_EFUDATA(i));
+
+ return 0;
+}
+
+/* main entry point */
+static int jz4780_efuse_read(void *context, unsigned int offset,
+ void *val, size_t bytes)
+{
+ struct jz4780_efuse *efuse = context;
+ int ret;
+
+ while (bytes > 0) {
+ unsigned int start = offset & ~(32 - 1);
+ unsigned chunk = min(bytes, (start + 32 - offset));
+
+ if (start == offset && chunk == 32) {
+ ret = jz4780_efuse_read_32bytes(efuse, val, start);
+ if (ret < 0)
+ return ret;
+
+ } else {
+ char buf[32];
+ ret = jz4780_efuse_read_32bytes(efuse, buf, start);
+ if (ret < 0)
+ return ret;
+
+ memcpy(val, &buf[offset - start], chunk);
+ }
+
+ val += chunk;
+ offset += chunk;
+ bytes -= chunk;

Quoted 8 lines

+ }
+
+ return 0;
+}
+
+static struct nvmem_config jz4780_efuse_nvmem_config = {
+ .name = "jz4780-efuse",
+ .read_only = true,

Quoted 2 lines

this value comes from device tree bindings, do we still need to
specify this here?

Ok, have to check what is automatically taken from DT.

Quoted 58 lines
Quoted 22 lines

+ .size = 1024,
+ .word_size = 1,
+ .stride = 1,
+ .owner = THIS_MODULE,
+ .reg_read = jz4780_efuse_read,
+};
+
+static int jz4780_efuse_probe(struct platform_device *pdev) +{
+ struct nvmem_device *nvmem;
+ struct jz4780_efuse *efuse;
+ struct resource *res;
+ unsigned long clk_rate;
+ struct device *dev = &pdev->dev;
+
+ efuse = devm_kzalloc(&pdev->dev, sizeof(*efuse), GFP_KERNEL);
+ if (!efuse)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

Quoted 8 lines

+ efuse->iomem = devm_ioremap(&pdev->dev, res->start,
resource_size(res));
+ if (IS_ERR(efuse->iomem))
+ return PTR_ERR(efuse->iomem);
+
+ efuse->clk = devm_clk_get(&pdev->dev, "bus_clk");
+ if (IS_ERR(efuse->clk))
+ return PTR_ERR(efuse->clk);

Who is enabling this clk?

Quoted 15 lines

+
+ clk_rate = clk_get_rate(efuse->clk);
+ /*
+ * rd_adj and rd_strobe are 4 bit values
+ * bus clk period * (rd_adj + 1) > 6.5ns
+ * bus clk period * (rd_adj + 5 + rd_strobe) > 35ns
+ */
+ efuse->rd_adj = (((6500 * (clk_rate / 1000000)) / 1000000) + 1)
- 1;
+ efuse->rd_strobe = ((((35000 * (clk_rate / 1000000)) / 1000000)
+ 1)
+ - 5 - efuse->rd_adj);
+
+ if ((efuse->rd_adj > 0x1F) || (efuse->rd_strobe > 0x1F)) {
+ dev_err(&pdev->dev, "Cannot set clock configuration\n");

Quoted 8 lines

+ return -EINVAL;
+ }
+ efuse->dev = dev;
+
+ jz4780_efuse_nvmem_config.dev = &pdev->dev;
+ jz4780_efuse_nvmem_config.priv = efuse;
+
+ nvmem = nvmem_register(&jz4780_efuse_nvmem_config);

Quoted 1 line

devm variant here?

Good idea! I didn't know that it exists. Allows us to remove jz4780_efuse_remove() below.

Quoted 32 lines
Quoted 32 lines

+ if (IS_ERR(nvmem))
+ return PTR_ERR(nvmem);
+
+ platform_set_drvdata(pdev, nvmem);
+
+ return 0;
+}
+
+static int jz4780_efuse_remove(struct platform_device *pdev) +{
+ struct nvmem_device *nvmem = platform_get_drvdata(pdev);
+
+ nvmem_unregister(nvmem);
+
+ return 0;
+}
+
+static const struct of_device_id jz4780_efuse_match[] = {
+ { .compatible = "ingenic,jz4780-efuse" },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, jz4780_efuse_match);
+
+static struct platform_driver jz4780_efuse_driver = {
+ .probe = jz4780_efuse_probe,
+ .remove = jz4780_efuse_remove,
+ .driver = {
+ .name = "jz4780-efuse",
+ .of_match_table = jz4780_efuse_match,
+ },

Quoted 8 lines
Quoted 8 lines

+};
+module_platform_driver(jz4780_efuse_driver);
+
+MODULE_AUTHOR("PrasannaKumar Muralidharan
<[email protected]>");
+MODULE_AUTHOR("H. Nikolaus Schaller <[email protected]>"); +MODULE_DESCRIPTION("Ingenic JZ4780 efuse driver"); +MODULE_LICENSE("GPL v2");

BR and thanks,
Nikolaus Schaller

maintainer
#5 lore raw

Re: [RFC v3 1/9] nvmem: add driver for JZ4780 efuse

In reply to Srinivas Kandagatla

Hi Srinivas,

Quoted 254 lines · Srinivas Kandagatla
Quoted 9 lines · H. Nikolaus Schaller

From: PrasannaKumar Muralidharan <[email protected]>

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.

Quoted 9 lines

(Signed-off-by: Paul Cercueil <[email protected]>)

---
  drivers/nvmem/Kconfig        |  10 ++
  drivers/nvmem/Makefile       |   2 +
  drivers/nvmem/jz4780-efuse.c | 249 
+++++++++++++++++++++++++++++++++++
  3 files changed, 261 insertions(+)

create mode 100644 drivers/nvmem/jz4780-efuse.c

This patch along with 2/9 should be merged into single patch. Also please make sure you run checkpatch.pl before sending!

drivers/nvmem/Kconfig config NVMEM_IMX_OCOTP_SCU · line 59

Quoted 10 lines
drivers/nvmem/Kconfig +2 -0
index 35efab1ba8d9..10f8e08f5e31 100644
@@ -55,6 +55,16 @@ 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_JZ4780 || COMPILE_TEST

what is that this driver depends on MACH_JZ4780 board?

Quoted 58 lines
drivers/nvmem/Kconfig +7 -0
+	depends on HAS_IOMEM
+	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
drivers/nvmem/Makefile +2 -0
index 6b466cd1427b..65a268d17807 100644
@@ -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
drivers/nvmem/jz4780-efuse.c new file +5 -0
new file mode 100644
index 000000000000..ac03e1900ef9
@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * JZ4780 EFUSE Memory Support driver
+ *
+ * Copyright (c) 2017 PrasannaKumar Muralidharan 

<[email protected]>
+ * Copyright (c) 2020 H. Nikolaus Schaller <[email protected]>
+ */

drivers/nvmem/jz4780-efuse.c new file +7 -1
+
+/*
+ * 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.
+ */

drivers/nvmem/jz4780-efuse.c new file +1 -0
+

drivers/nvmem/jz4780-efuse.c line 31

Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +8 -0
+#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>

?? why do we need these two headers in this patch

Quoted 9 lines
drivers/nvmem/jz4780-efuse.c new file +9 -0
+
+#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)
+
+#define JZ_EFUSE_START_ADDR		0x200
+
+#define JZ_EFUSE_EFUCTRL_CS		BIT(30)

drivers/nvmem/jz4780-efuse.c line 50

Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10 -0
+#define JZ_EFUSE_EFUCTRL_ADDR_MASK	0x1FF
+#define JZ_EFUSE_EFUCTRL_ADDR_SHIFT	21
+#define JZ_EFUSE_EFUCTRL_LEN_MASK	0x1F
+#define JZ_EFUSE_EFUCTRL_LEN_SHIFT	16
+#define JZ_EFUSE_EFUCTRL_PG_EN		BIT(15)
+#define JZ_EFUSE_EFUCTRL_WR_EN		BIT(1)
+#define JZ_EFUSE_EFUCTRL_RD_EN		BIT(0)
+
+#define JZ_EFUSE_EFUCFG_INT_EN		BIT(31)
+#define JZ_EFUSE_EFUCFG_RD_ADJ_MASK	0xF

consider using GENMASK for these masks here.

Quoted 22 lines
drivers/nvmem/jz4780-efuse.c new file +21 -0
+#define JZ_EFUSE_EFUCFG_RD_ADJ_SHIFT	20
+#define JZ_EFUSE_EFUCFG_RD_STR_MASK	0xF
+#define JZ_EFUSE_EFUCFG_RD_STR_SHIFT	16
+#define JZ_EFUSE_EFUCFG_WR_ADJ_MASK	0xF
+#define JZ_EFUSE_EFUCFG_WR_ADJ_SHIFT	12
+#define JZ_EFUSE_EFUCFG_WR_STR_MASK	0xFFF
+#define JZ_EFUSE_EFUCFG_WR_STR_SHIFT	0
+
+#define JZ_EFUSE_EFUSTATE_WR_DONE	BIT(1)
+#define JZ_EFUSE_EFUSTATE_RD_DONE	BIT(0)
+
+struct jz4780_efuse {
+	struct device *dev;
+	void __iomem *iomem;
+	struct clk *clk;
+	unsigned int rd_adj;
+	unsigned int rd_strobe;
+};
+
+/* We read 32 byte chunks to avoid complexity in the driver. */
+static int jz4780_efuse_read_32bytes(struct jz4780_efuse *efuse, 

char *buf,

drivers/nvmem/jz4780-efuse.c line 75

Quoted 4 lines
drivers/nvmem/jz4780-efuse.c new file +4 -0
+		unsigned int addr)
+{
+	unsigned int tmp = 0;
+	int i = 0;

unnecessary initialization of both variables.

drivers/nvmem/jz4780-efuse.c line 77

Quoted 2 lines
drivers/nvmem/jz4780-efuse.c new file +2 -0
+	int timeout = 1000;
+	int size = 32;

better to #define this STRIDE/CHUNK size. this driver seems to use this value in multiple places.

drivers/nvmem/jz4780-efuse.c line 84

Quoted 8 lines
drivers/nvmem/jz4780-efuse.c new file +4 -0
+
+	/* 1. Set config register */
+	tmp = readl(efuse->iomem + JZ_EFUCFG);
+	tmp &= ~((JZ_EFUSE_EFUCFG_RD_ADJ_MASK << 

JZ_EFUSE_EFUCFG_RD_ADJ_SHIFT)

drivers/nvmem/jz4780-efuse.c new file +3 -0
+	| (JZ_EFUSE_EFUCFG_RD_STR_MASK << JZ_EFUSE_EFUCFG_RD_STR_SHIFT));
+	tmp |= (efuse->rd_adj << JZ_EFUSE_EFUCFG_RD_ADJ_SHIFT)
+	| (efuse->rd_strobe << JZ_EFUSE_EFUCFG_RD_STR_SHIFT);

very odd indenting.

Quoted 29 lines
drivers/nvmem/jz4780-efuse.c new file +29 -0
+	writel(tmp, efuse->iomem + JZ_EFUCFG);
+
+	/*
+	 * 2. Set control register to indicate what to read data address,
+	 * read data numbers and read enable.
+	 */
+	tmp = readl(efuse->iomem + JZ_EFUCTRL);
+	tmp &= ~(JZ_EFUSE_EFUCFG_RD_STR_SHIFT
+		| (JZ_EFUSE_EFUCTRL_ADDR_MASK << JZ_EFUSE_EFUCTRL_ADDR_SHIFT)
+		| JZ_EFUSE_EFUCTRL_PG_EN | JZ_EFUSE_EFUCTRL_WR_EN
+		| JZ_EFUSE_EFUCTRL_WR_EN);
+
+	/* Need to select CS bit if address accesses upper 4Kbits memory */
+	if (addr >= (JZ_EFUSE_START_ADDR + 512))
+		tmp |= JZ_EFUSE_EFUCTRL_CS;
+
+	tmp |= (addr << JZ_EFUSE_EFUCTRL_ADDR_SHIFT)
+		| ((size - 1) << JZ_EFUSE_EFUCTRL_LEN_SHIFT)
+		| JZ_EFUSE_EFUCTRL_RD_EN;
+	writel(tmp, efuse->iomem + JZ_EFUCTRL);
+
+	/*
+	 * 3. Wait status register RD_DONE set to 1 or EFUSE interrupted,
+	 * software can read EFUSE data buffer 0 - 8 registers.
+	 */
+	do {
+		tmp = readl(efuse->iomem + JZ_EFUSTATE);
+		usleep_range(1000, 2000);
+		if (timeout--)

drivers/nvmem/jz4780-efuse.c line 123

Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10 -0
+			break;
+	} while (!(tmp & JZ_EFUSE_EFUSTATE_RD_DONE));
+
+	if (timeout <= 0) {
+		dev_err(efuse->dev, "Timed out while reading\n");
+		return -EAGAIN;
+	}
+
+	for (i = 0; i < (size / 4); i++)
+		*((unsigned int *)(buf + i * 4))

make "unsigned int *buf32" a local variable and use it here, makes it much readable code.

Quoted 35 lines
drivers/nvmem/jz4780-efuse.c new file +35 -0
+			 = readl(efuse->iomem + JZ_EFUDATA(i));
+
+	return 0;
+}
+
+/* main entry point */
+static int jz4780_efuse_read(void *context, unsigned int offset,
+					void *val, size_t bytes)
+{
+	struct jz4780_efuse *efuse = context;
+	int ret;
+
+	while (bytes > 0) {
+		unsigned int start = offset & ~(32 - 1);
+		unsigned chunk = min(bytes, (start + 32 - offset));
+
+		if (start == offset && chunk == 32) {
+			ret = jz4780_efuse_read_32bytes(efuse, val, start);
+			if (ret < 0)
+				return ret;
+
+		} else {
+			char buf[32];
+			ret = jz4780_efuse_read_32bytes(efuse, buf, start);
+			if (ret < 0)
+				return ret;
+
+			memcpy(val, &buf[offset - start], chunk);
+		}
+
+		val += chunk;
+		offset += chunk;
+		bytes -= chunk;
+	}
+
Quoted 8 lines

drivers/nvmem/jz4780-efuse.c line 164

Quoted 6 lines
drivers/nvmem/jz4780-efuse.c new file +6 -0
+	return 0;
+}
+
+static struct nvmem_config jz4780_efuse_nvmem_config = {
+	.name = "jz4780-efuse",
+	.read_only = true,

this value comes from device tree bindings, do we still need to specify this here?

I think its semantic is different - devicetree specifies that you want the nvmem to be readonly, this one specifies that the driver cannot do read-write anyway.

With that said, there is no .reg_write, so .read_only will be enforced to 'true' anyway, so it can be removed.

-Paul

Quoted 99 lines
Quoted 23 lines
drivers/nvmem/jz4780-efuse.c new file +21 -0
+	.size = 1024,
+	.word_size = 1,
+	.stride = 1,
+	.owner = THIS_MODULE,
+	.reg_read = jz4780_efuse_read,
+};
+
+static int jz4780_efuse_probe(struct platform_device *pdev)
+{
+	struct nvmem_device *nvmem;
+	struct jz4780_efuse *efuse;
+	struct resource *res;
+	unsigned long clk_rate;
+	struct device *dev = &pdev->dev;
+
+	efuse = devm_kzalloc(&pdev->dev, sizeof(*efuse), GFP_KERNEL);
+	if (!efuse)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	efuse->iomem = devm_ioremap(&pdev->dev, res->start, 

resource_size(res));

drivers/nvmem/jz4780-efuse.c line 191

Quoted 6 lines
drivers/nvmem/jz4780-efuse.c new file +6 -0
+	if (IS_ERR(efuse->iomem))
+		return PTR_ERR(efuse->iomem);
+
+	efuse->clk = devm_clk_get(&pdev->dev, "bus_clk");
+	if (IS_ERR(efuse->clk))
+		return PTR_ERR(efuse->clk);

Who is enabling this clk?

Quoted 13 lines
drivers/nvmem/jz4780-efuse.c new file +8 -0
+
+	clk_rate = clk_get_rate(efuse->clk);
+	/*
+	 * rd_adj and rd_strobe are 4 bit values
+	 * bus clk period * (rd_adj + 1) > 6.5ns
+	 * bus clk period * (rd_adj + 5 + rd_strobe) > 35ns
+	 */
+	efuse->rd_adj = (((6500 * (clk_rate / 1000000)) / 1000000) + 1) - 

1;

drivers/nvmem/jz4780-efuse.c new file +1 -0
+	efuse->rd_strobe = ((((35000 * (clk_rate / 1000000)) / 1000000) + 

1)

drivers/nvmem/jz4780-efuse.c new file +2 -0
+						- 5 - efuse->rd_adj);
+

drivers/nvmem/jz4780-efuse.c line 212

Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10 -0
+	if ((efuse->rd_adj > 0x1F) || (efuse->rd_strobe > 0x1F)) {
+		dev_err(&pdev->dev, "Cannot set clock configuration\n");
+		return -EINVAL;
+	}
+	efuse->dev = dev;
+
+	jz4780_efuse_nvmem_config.dev = &pdev->dev;
+	jz4780_efuse_nvmem_config.priv = efuse;
+
+	nvmem = nvmem_register(&jz4780_efuse_nvmem_config);

devm variant here?

Quoted 39 lines
drivers/nvmem/jz4780-efuse.c new file +34 -0
+	if (IS_ERR(nvmem))
+		return PTR_ERR(nvmem);
+
+	platform_set_drvdata(pdev, nvmem);
+
+	return 0;
+}
+
+static int jz4780_efuse_remove(struct platform_device *pdev)
+{
+	struct nvmem_device *nvmem = platform_get_drvdata(pdev);
+
+	nvmem_unregister(nvmem);
+
+	return 0;
+}
+
+static const struct of_device_id jz4780_efuse_match[] = {
+	{ .compatible = "ingenic,jz4780-efuse" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, jz4780_efuse_match);
+
+static struct platform_driver jz4780_efuse_driver = {
+	.probe  = jz4780_efuse_probe,
+	.remove = jz4780_efuse_remove,
+	.driver = {
+		.name = "jz4780-efuse",
+		.of_match_table = jz4780_efuse_match,
+	},
+};
+module_platform_driver(jz4780_efuse_driver);
+
+MODULE_AUTHOR("PrasannaKumar Muralidharan 

<[email protected]>");

drivers/nvmem/jz4780-efuse.c new file +3 -0
+MODULE_AUTHOR("H. Nikolaus Schaller <[email protected]>");
+MODULE_DESCRIPTION("Ingenic JZ4780 efuse driver");
+MODULE_LICENSE("GPL v2");
#6 lore raw

RFC v3 2/9 rework to use regmap

In reply to H. Nikolaus Schaller

From: Paul Cercueil <[email protected]>

should/will be squashed into the previous commit ("nvmem: add driver for JZ4780 efuse") but is kept separately for this RFC to make the modifications more transparent.

---
 drivers/nvmem/jz4780-efuse.c | 179 ++++++++++++++++-------------------
 1 file changed, 84 insertions(+), 95 deletions(-)
drivers/nvmem/jz4780-efuse.c +84 -95
index ac03e1900ef9..79dc42431bfe 100644
@@ -30,38 +30,35 @@
 #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)
-
-#define JZ_EFUSE_START_ADDR		0x200
-
-#define JZ_EFUSE_EFUCTRL_CS		BIT(30)
-#define JZ_EFUSE_EFUCTRL_ADDR_MASK	0x1FF
-#define JZ_EFUSE_EFUCTRL_ADDR_SHIFT	21
-#define JZ_EFUSE_EFUCTRL_LEN_MASK	0x1F
-#define JZ_EFUSE_EFUCTRL_LEN_SHIFT	16
-#define JZ_EFUSE_EFUCTRL_PG_EN		BIT(15)
-#define JZ_EFUSE_EFUCTRL_WR_EN		BIT(1)
-#define JZ_EFUSE_EFUCTRL_RD_EN		BIT(0)
-
-#define JZ_EFUSE_EFUCFG_INT_EN		BIT(31)
-#define JZ_EFUSE_EFUCFG_RD_ADJ_MASK	0xF
-#define JZ_EFUSE_EFUCFG_RD_ADJ_SHIFT	20
-#define JZ_EFUSE_EFUCFG_RD_STR_MASK	0xF
-#define JZ_EFUSE_EFUCFG_RD_STR_SHIFT	16
-#define JZ_EFUSE_EFUCFG_WR_ADJ_MASK	0xF
-#define JZ_EFUSE_EFUCFG_WR_ADJ_SHIFT	12
-#define JZ_EFUSE_EFUCFG_WR_STR_MASK	0xFFF
-#define JZ_EFUSE_EFUCFG_WR_STR_SHIFT	0
-
-#define JZ_EFUSE_EFUSTATE_WR_DONE	BIT(1)
-#define JZ_EFUSE_EFUSTATE_RD_DONE	BIT(0)
+#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)
+
+#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;
-	void __iomem *iomem;
+	struct regmap *map;
 	struct clk *clk;
 	unsigned int rd_adj;
 	unsigned int rd_strobe;
@@ -71,59 +68,29 @@ struct jz4780_efuse {
 static int jz4780_efuse_read_32bytes(struct jz4780_efuse *efuse, char *buf,
 		unsigned int addr)
 {
-	unsigned int tmp = 0;
-	int i = 0;
-	int timeout = 1000;
-	int size = 32;
-
-	/* 1. Set config register */
-	tmp = readl(efuse->iomem + JZ_EFUCFG);
-	tmp &= ~((JZ_EFUSE_EFUCFG_RD_ADJ_MASK << JZ_EFUSE_EFUCFG_RD_ADJ_SHIFT)
-	| (JZ_EFUSE_EFUCFG_RD_STR_MASK << JZ_EFUSE_EFUCFG_RD_STR_SHIFT));
-	tmp |= (efuse->rd_adj << JZ_EFUSE_EFUCFG_RD_ADJ_SHIFT)
-	| (efuse->rd_strobe << JZ_EFUSE_EFUCFG_RD_STR_SHIFT);
-	writel(tmp, efuse->iomem + JZ_EFUCFG);
-
-	/*
-	 * 2. Set control register to indicate what to read data address,
-	 * read data numbers and read enable.
-	 */
-	tmp = readl(efuse->iomem + JZ_EFUCTRL);
-	tmp &= ~(JZ_EFUSE_EFUCFG_RD_STR_SHIFT
-		| (JZ_EFUSE_EFUCTRL_ADDR_MASK << JZ_EFUSE_EFUCTRL_ADDR_SHIFT)
-		| JZ_EFUSE_EFUCTRL_PG_EN | JZ_EFUSE_EFUCTRL_WR_EN
-		| JZ_EFUSE_EFUCTRL_WR_EN);
-
-	/* Need to select CS bit if address accesses upper 4Kbits memory */
-	if (addr >= (JZ_EFUSE_START_ADDR + 512))
-		tmp |= JZ_EFUSE_EFUCTRL_CS;
-
-	tmp |= (addr << JZ_EFUSE_EFUCTRL_ADDR_SHIFT)
-		| ((size - 1) << JZ_EFUSE_EFUCTRL_LEN_SHIFT)
-		| JZ_EFUSE_EFUCTRL_RD_EN;
-	writel(tmp, efuse->iomem + JZ_EFUCTRL);
-
-	/*
-	 * 3. Wait status register RD_DONE set to 1 or EFUSE interrupted,
-	 * software can read EFUSE data buffer 0 - 8 registers.
-	 */
-	do {
-		tmp = readl(efuse->iomem + JZ_EFUSTATE);
-		usleep_range(1000, 2000);
-		if (timeout--)
-			break;
-	} while (!(tmp & JZ_EFUSE_EFUSTATE_RD_DONE));
-
-	if (timeout <= 0) {
-		dev_err(efuse->dev, "Timed out while reading\n");
-		return -EAGAIN;
+	unsigned int tmp;
+	u32 ctrl;
+	int ret;
+	const int size = 32;
+
+	ctrl = (addr << EFUCTRL_ADDR_SHIFT)
+		| ((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;
 	}
 
-	for (i = 0; i < (size / 4); i++)
-		*((unsigned int *)(buf + i * 4))
-			 = readl(efuse->iomem + JZ_EFUDATA(i));
-
-	return 0;
+	return regmap_bulk_read(efuse->map, JZ_EFUDATA(0), buf, size / sizeof(u32));
 }
 
 /* main entry point */
@@ -132,12 +99,13 @@ static int jz4780_efuse_read(void *context, unsigned int offset,
 {
 	struct jz4780_efuse *efuse = context;
 	int ret;
+	const int size = 32;
 
 	while (bytes > 0) {
-		unsigned int start = offset & ~(32 - 1);
-		unsigned chunk = min(bytes, (start + 32 - offset));
+		unsigned int start = offset & ~(size - 1);
+		unsigned chunk = min(bytes, (start + size) - offset);
 
-		if (start == offset && chunk == 32) {
+		if (start == offset && chunk == size) {
 			ret = jz4780_efuse_read_32bytes(efuse, val, start);
 			if (ret < 0)
 				return ret;
@@ -159,7 +127,7 @@ static int jz4780_efuse_read(void *context, unsigned int offset,
 	return 0;
 }
 
-static struct nvmem_config jz4780_efuse_nvmem_config = {
+static __initdata struct nvmem_config jz4780_efuse_nvmem_config = {
 	.name = "jz4780-efuse",
 	.read_only = true,
 	.size = 1024,
@@ -169,28 +137,42 @@ static struct nvmem_config jz4780_efuse_nvmem_config = {
 	.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 resource *res;
+	struct nvmem_config cfg;
 	unsigned long clk_rate;
 	struct device *dev = &pdev->dev;
+	void __iomem *regs;
 
-	efuse = devm_kzalloc(&pdev->dev, sizeof(*efuse), GFP_KERNEL);
+	efuse = devm_kzalloc(dev, sizeof(*efuse), GFP_KERNEL);
 	if (!efuse)
 		return -ENOMEM;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	efuse->iomem = devm_ioremap(&pdev->dev, res->start, resource_size(res));
-	if (IS_ERR(efuse->iomem))
-		return PTR_ERR(efuse->iomem);
+	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, "bus_clk");
+	efuse->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(efuse->clk))
 		return PTR_ERR(efuse->clk);
 
 	clk_rate = clk_get_rate(efuse->clk);
+
+	efuse->dev = dev;
 	/*
 	 * rd_adj and rd_strobe are 4 bit values
 	 * bus clk period * (rd_adj + 1) > 6.5ns
@@ -204,12 +186,18 @@ static int jz4780_efuse_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "Cannot set clock configuration\n");
 		return -EINVAL;
 	}
-	efuse->dev = dev;
 
-	jz4780_efuse_nvmem_config.dev = &pdev->dev;
-	jz4780_efuse_nvmem_config.priv = efuse;
+	regmap_update_bits(efuse->map, JZ_EFUCFG,
+			   (EFUCFG_RD_ADJ_MASK << EFUCFG_RD_ADJ_SHIFT) |
+			   (EFUCFG_RD_STR_MASK << EFUCFG_RD_STR_SHIFT),
+			   (efuse->rd_adj << EFUCFG_RD_ADJ_SHIFT) |
+			   (efuse->rd_strobe << EFUCFG_RD_STR_SHIFT));
+
+	cfg = jz4780_efuse_nvmem_config;
+	cfg.dev = &pdev->dev;
+	cfg.priv = efuse;
 
-	nvmem = nvmem_register(&jz4780_efuse_nvmem_config);
+	nvmem = nvmem_register(&cfg);
 	if (IS_ERR(nvmem))
 		return PTR_ERR(nvmem);
 
@@ -245,5 +233,6 @@ module_platform_driver(jz4780_efuse_driver);
 
 MODULE_AUTHOR("PrasannaKumar Muralidharan <[email protected]>");
 MODULE_AUTHOR("H. Nikolaus Schaller <[email protected]>");
+MODULE_AUTHOR("Paul Cercueil <[email protected]>");
 MODULE_DESCRIPTION("Ingenic JZ4780 efuse driver");
 MODULE_LICENSE("GPL v2");
Signature · 1 line
2.23.0
#7 lore raw

RFC v3 3/9 Bindings: nvmem: add bindings for JZ4780 efuse

In reply to H. Nikolaus Schaller

From: PrasannaKumar Muralidharan <[email protected]>

This patch brings support for the JZ4780 efuse. Currently it only expose a read only access to the entire 8K bits efuse memory.

---
 .../bindings/nvmem/ingenic,jz4780-efuse.txt     | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.txt new file +17 -0
new file mode 100644
index 000000000000..339e74daa9a9
@@ -0,0 +1,17 @@
+Ingenic JZ EFUSE driver bindings
+
+Required properties:
+- "compatible"		Must be set to "ingenic,jz4780-efuse"
+- "reg"			Register location and length
+- "clocks"		Handle for the ahb clock for the efuse.
+- "clock-names"		Must be "bus_clk"
+
+Example:
+
+efuse: efuse@134100d0 {
+	compatible = "ingenic,jz4780-efuse";
+	reg = <0x134100d0 0x2c>;
+
+	clocks = <&cgu JZ4780_CLK_AHB2>;
+	clock-names = "bus_clk";
+};
Signature · 1 line
2.23.0
#8 lore raw

RFC v3 4/9 Documentation: ABI: nvmem: add documentation for JZ4780 efuse ABI

In reply to H. Nikolaus Schaller

From: PrasannaKumar Muralidharan <[email protected]>

This patch brings support for the JZ4780 efuse. Currently it only expose a read only access to the entire 8K bits efuse memory.

---
 .../ABI/testing/sysfs-driver-jz4780-efuse        | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-jz4780-efuse
Documentation/ABI/testing/sysfs-driver-jz4780-efuse new file +16 -0
new file mode 100644
index 000000000000..bb6f5d6ceea0
@@ -0,0 +1,16 @@
+What:		/sys/devices/*/<our-device>/nvmem
+Date:		December 2017
+Contact:	PrasannaKumar Muralidharan <[email protected]>
+Description:	read-only access to the efuse on the Ingenic JZ4780 SoC
+		The SoC has a one time programmable 8K efuse that is
+		split into segments. The driver supports read only.
+		The segments are
+		0x000   64 bit Random Number
+		0x008  128 bit Ingenic Chip ID
+		0x018  128 bit Customer ID
+		0x028 3520 bit Reserved
+		0x1E0    8 bit Protect Segment
+		0x1E1 2296 bit HDMI Key
+		0x300 2048 bit Security boot key
+Users:		any user space application which wants to read the Chip
+		and Customer ID
Signature · 1 line
2.23.0
#9 lore raw

RFC v3 5/9 nvmem: MAINTAINERS: add maintainer for JZ4780 efuse driver

In reply to H. Nikolaus Schaller

From: PrasannaKumar Muralidharan <[email protected]>

This patch brings support for the JZ4780 efuse. Currently it only expose a read only access to the entire 8K bits efuse memory.

---
 MAINTAINERS | 5 +++++
 1 file changed, 5 insertions(+)
MAINTAINERS +5 -0
index 33f811f517d4..88f247456a48 100644
@@ -8270,6 +8270,11 @@ M:	Zubair Lutfullah Kakakhel <[email protected]>
 S:	Maintained
 F:	drivers/dma/dma-jz4780.c
 
+INGENIC JZ4780 EFUSE Driver
+M:	PrasannaKumar Muralidharan <[email protected]>
+S:	Maintained
+F:	drivers/nvmem/jz4780-efuse.c
+
 INGENIC JZ4780 NAND DRIVER
 M:	Harvey Hunt <[email protected]>
 L:	[email protected]
Signature · 1 line
2.23.0
#10 lore raw

RFC v3 6/9 MIPS: DTS: JZ4780: define node for JZ4780 efuse

In reply to H. Nikolaus Schaller

From: PrasannaKumar Muralidharan <[email protected]>

This patch brings support for the JZ4780 efuse. Currently it only exposes a read only access to the entire 8K bits efuse memory and the ethernet mac address for the davicom dm9000 chip on the CI20 board.

---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +16 -1
index 301b62da025e..4cc8abe675c2 100644
@@ -393,7 +393,7 @@
 
 	nemc: nemc@13410000 {
 		compatible = "ingenic,jz4780-nemc";
-		reg = <0x13410000 0x10000>;
+		reg = <0x13410000 0x4c>;
 		#address-cells = <2>;
 		#size-cells = <1>;
 		ranges = <1 0 0x1b000000 0x1000000
@@ -408,6 +408,21 @@
 		status = "disabled";
 	};
 
+	efuse: efuse@134100d0 {
+		compatible = "ingenic,jz4780-efuse";
+		reg = <0x134100d0 0x2c>;
+
+		clocks = <&cgu JZ4780_CLK_AHB2>;
+		clock-names = "bus_clk";
+
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		eth0_addr: eth-mac-addr@0x22 {
+			reg = <0x22 0x6>;
+		};
+	};
+
 	dma: dma@13420000 {
 		compatible = "ingenic,jz4780-dma";
 		reg = <0x13420000 0x400
Signature · 1 line
2.23.0
#11 lore raw

RFC v3 7/9 MIPS: DTS: CI20: make DM9000 Ethernet controller use NVMEM to find the default MAC address

In reply to H. Nikolaus Schaller

There is a default MAC address factory programmed into the eFuses of the JZ4780 chip. By using this every CI20 board has an individual but stable MAC address and DHCP can assing stable IP addresses.

---
 arch/mips/boot/dts/ingenic/ci20.dts | 3 +++
 1 file changed, 3 insertions(+)
arch/mips/boot/dts/ingenic/ci20.dts +3 -0
index 1166f3203ff8..63b4b53b5df5 100644
@@ -428,6 +428,9 @@ Optional input supply properties:
 
 		interrupt-parent = <&gpe>;
 		interrupts = <19 4>;
+
+		nvmem-cells = <&eth0_addr>;
+		nvmem-cell-names = "mac-address";
 	};
 };
 
Signature · 1 line
2.23.0
#12 lore raw

RFC v3 8/9 MIPS: CI20 defconfig: Probe efuse for CI20

In reply to H. Nikolaus Schaller

From: Mathieu Malaterre <[email protected]>

MIPS Creator CI20 comes with JZ4780 SoC. Provides access to the efuse block using jz4780 efuse driver.

---
 arch/mips/configs/ci20_defconfig | 2 ++
 1 file changed, 2 insertions(+)
arch/mips/configs/ci20_defconfig +2 -0
index 6e613bbb6807..614dc18211bc 100644
@@ -193,3 +193,5 @@ CONFIG_RC_DEVICES=y
 CONFIG_IR_GPIO_CIR=m
 CONFIG_IR_GPIO_TX=m
 CONFIG_RTC_DRV_PCF8563=m
+CONFIG_NVMEM=y
+CONFIG_JZ4780_EFUSE=y
Signature · 1 line
2.23.0
#13 lore raw

RFC v3 9/9 MIPS: CI20 defconfig: DEMO HACK: make DM9000 a module

In reply to H. Nikolaus Schaller

It appears that reading the ethernet mac address from NVMEM, i.e. through the jz4780-efuse driver, requires the jz4780-efuse driver to be up and running before the dm9000 driver is probed.

This is because there is a registration mechanism for nvmem devices and consumers scan the global table. If the provider has not yet been registered, the required matching entry is not found and the consumer assumes there is no nvram.

In the case of the dm9000 it will not wait but assign a random MAC address.

It appears that if the dm9000 is configured into the kernel binary, it will be probed first and the correct sequence is broken.

If it is a module (or even both), the way deferred probing works seems to serialize properly.

So this hack is for demo purposes only and makes the dm9000 probing being delayed.

A proper solution is to make sure the jz4780-efuse driver is probed earlier than the dm9000 driver (or mac address lookup).

---
 arch/mips/configs/ci20_defconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/configs/ci20_defconfig +1 -1
index 614dc18211bc..1d3d1d9b62bc 100644
@@ -59,7 +59,7 @@ CONFIG_MTD_UBI_FASTMAP=y
 CONFIG_NETDEVICES=y
 # CONFIG_NET_VENDOR_ARC is not set
 # CONFIG_NET_VENDOR_BROADCOM is not set
-CONFIG_DM9000=y
+CONFIG_DM9000=m
 CONFIG_DM9000_FORCE_SIMPLE_PHY_POLL=y
 # CONFIG_NET_VENDOR_INTEL is not set
 # CONFIG_NET_VENDOR_MARVELL is not set
Signature · 1 line
2.23.0