MIPS: CI20: Add efuse driver for Ingenic JZ4780 and attach to DM9000 for stable MAC addresses
Subsystems (4)
v5 cover MIPS: CI20: Add efuse driver for Ingenic JZ4780 and attach to DM9000 for stable MAC addresses
* no longer RFC but PATCH
* add yaml bindings (by Andreas Kemnade <[email protected]>)
* fixes to yaml (suggested by Rob Herring <[email protected]>)
RFC V4 2020-02-17 17:55:35:
* removed read_only for nvmem config because there is no write method
* Kconfig: replaced depends MACH_JZ4780 with MACH_INGENIC
* run through checkpatch and fixed issues
* made use of devm_nvram_register() and get rid of jz4780_efuse_remove() (suggested by Srinivas Kandagatla <[email protected]>)
* squashed previous patch 1/9 and 2/9 into single (regmap based) driver
RFC V3 2020-02-16 20:20:59:
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 (1):
MIPS: DTS: CI20: make DM9000 Ethernet controller use NVMEM to find the
default MAC address
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.yaml | 50 ++++ MAINTAINERS | 5 + arch/mips/boot/dts/ingenic/ci20.dts | 3 + arch/mips/boot/dts/ingenic/jz4780.dtsi | 17 +- drivers/nvmem/Kconfig | 10 + drivers/nvmem/Makefile | 2 + drivers/nvmem/jz4780-efuse.c | 229 ++++++++++++++++++ 8 files changed, 331 insertions(+), 1 deletion(-) create mode 100644 Documentation/ABI/testing/sysfs-driver-jz4780-efuse create mode 100644 Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml create mode 100644 drivers/nvmem/jz4780-efuse.c
Signature · 1 line
2.23.0
PATCH v5 1/6 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.
--- drivers/nvmem/Kconfig | 10 ++ drivers/nvmem/Makefile | 2 + drivers/nvmem/jz4780-efuse.c | 229 +++++++++++++++++++++++++++++++++++ 3 files changed, 241 insertions(+) create mode 100644 drivers/nvmem/jz4780-efuse.c
drivers/nvmem/Kconfig +10 -0
@@ -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_INGENIC || 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
@@ -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 +229 -0
229 lines not on this page · raw message
Signature · 1 line
2.23.0
Paul Cercueil
maintainerRe: [PATCH v5 1/6] nvmem: add driver for JZ4780 efuse
Hi Nikolaus,
Quoted 23 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.
Tested-by Mathieu Malaterre <[email protected]>Signed-off-by PrasannaKumar Muralidharan <[email protected]>Signed-off-by Mathieu Malaterre <[email protected]>Signed-off-by H. Nikolaus Schaller <[email protected]>Signed-off-by Paul Cercueil <[email protected]>--- drivers/nvmem/Kconfig | 10 ++ drivers/nvmem/Makefile | 2 + drivers/nvmem/jz4780-efuse.c | 229 +++++++++++++++++++++++++++++++++++ 3 files changed, 241 insertions(+) create mode 100644 drivers/nvmem/jz4780-efuse.cdrivers/nvmem/Kconfig +0
-0
drivers/nvmem/Kconfig config NVMEM_IMX_OCOTP_SCU · line 61
Quoted 7 lines
drivers/nvmem/Kconfig +4
-0@@ -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_INGENIC || COMPILE_TEST + depends on HAS_IOMEM
You're using regmap, so you need to select REGMAP_MMIO here.
Quoted 91 lines
drivers/nvmem/Kconfig +6
-0+ 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_TESTdrivers/nvmem/Makefile +2
-0@@ -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.odrivers/nvmem/jz4780-efuse.c new file +5
-0@@ -0,0 +1,229 @@ +// 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 +35
-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> + +#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)
drivers/nvmem/jz4780-efuse.c line 67
Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10
-0+ +struct jz4780_efuse { +struct device *dev; +struct regmap *map; +struct clk *clk; +unsigned int rd_adj; +unsigned int rd_strobe; +}; + +/* We read 32 byte chunks to avoid complexity in the driver. */
I don't see how reading 32 byte chunks avoids complexity in the driver; the 'size' variable here could very well be an argument.
Quoted 31 lines
+static int jz4780_efuse_read_32bytes(struct
jz4780_efuse*efuse, char *buf,drivers/nvmem/jz4780-efuse.c new file +29
-0+ unsigned int addr) +{ +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; + } + +return regmap_bulk_read (efuse->map,JZ_EFUDATA (0 ), + buf, size /sizeof (u32)); +} + +/* main entry point */
drivers/nvmem/jz4780-efuse.c line 107
Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10
-0+static int jz4780_efuse_read (void *context,unsigned int offset, +void *val,size_t bytes) +{ +struct jz4780_efuse *efuse = context; +int ret; +const int size =32 ; + +while (bytes >0 ) { +unsigned int start = offset & ~(size -1 ); +unsigned int chunk =min (bytes, (start + size) - offset);
Why do you need to check for the address alignment?
Quoted 4 lines
drivers/nvmem/jz4780-efuse.c new file +4
-0+ +if (start == offset && chunk == size) { + ret =jz4780_efuse_read_32bytes (efuse, val, start); +if (ret <0 )
drivers/nvmem/jz4780-efuse.c line 121
Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10
-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);
Why do you need this cumbersome process of reading 32 bytes if you need less than that? This looks over-complex to me.
drivers/nvmem/jz4780-efuse.c line 134
Quoted 13 lines
drivers/nvmem/jz4780-efuse.c new file +13
-0+ } + + val += chunk; + offset += chunk; + bytes -= chunk; + } + +return 0 ; +} + +static struct nvmem_config jz4780_efuse_nvmem_config__initdata = { + .name ="jz4780-efuse" , + .size =1024 ,
The efuse is 8 KiB.
Quoted 30 lines
drivers/nvmem/jz4780-efuse.c new file +30
-0+ .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; +struct device *dev = &pdev->dev; +void __iomem *regs; + + 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); +
drivers/nvmem/jz4780-efuse.c line 174
Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10
-0+ 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); + + clk_rate =clk_get_rate (efuse->clk);
You didn't enable the clock before, so clk_get_rate can return a bogus value.
drivers/nvmem/jz4780-efuse.c line 182
Quoted 8 lines
drivers/nvmem/jz4780-efuse.c new file +8
-0+ + efuse->dev = dev; +/* +* 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 ;
I can't get my head around this calculus. Try this:
/* 1 / 6.5ns == 153846154 Hz */efuse->rd_adj = clk_rate / 153846154;
The efuse read should be *at least* 6.5ns, so no need to bother with the -1/+1.
drivers/nvmem/jz4780-efuse.c line 184
Quoted 2 lines
drivers/nvmem/jz4780-efuse.c new file +2
-0+ efuse->rd_strobe = ((((35000 * (clk_rate /1000000 )) /1000000 ) +1 ) + -5 - efuse->rd_adj);
/* 1 / 35ns == 28571429 Hz */efuse->rd_strobe = max(0, clk_rate / 28571429 + 1 - 5 - efuse->rd_adj);
Quoted 11 lines
drivers/nvmem/jz4780-efuse.c new file +11
-0+ +if (efuse->rd_adj >0x1F || efuse->rd_strobe >0x1F ) { +dev_err (&pdev->dev,"Cannot set clock configuration\n" ); +return -EINVAL ; + } + +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 ));
drivers/nvmem/jz4780-efuse.c line 205
Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10
-0+ + cfg = jz4780_efuse_nvmem_config; + cfg.dev = &pdev->dev; + cfg.priv = efuse; + + nvmem =devm_nvmem_register (dev, &cfg); +if (IS_ERR (nvmem)) +return PTR_ERR (nvmem); + +platform_set_drvdata (pdev, nvmem);
This isn't used anywhere.
Quoted 28 lines
drivers/nvmem/jz4780-efuse.c new file +20
-0+ +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, + .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 +4
-1+MODULE_AUTHOR ("H. Nikolaus Schaller <[email protected]>" ); +MODULE_AUTHOR ("Paul Cercueil <[email protected]>" ); +MODULE_DESCRIPTION ("Ingenic JZ4780 efuse driver" ); +MODULE_LICENSE ("GPL v2" ); --2.23.0
Re: [PATCH v5 1/6] nvmem: add driver for JZ4780 efuse
Hi Paul,
Quoted 25 lines
Am 22.02.2020 um 16:38 schrieb Paul Cercueil <[email protected]>:
Hi Nikolaus,
Quoted 19 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.
Tested-by Mathieu Malaterre <[email protected]>Signed-off-by PrasannaKumar Muralidharan <[email protected]>Signed-off-by Mathieu Malaterre <[email protected]>Signed-off-by H. Nikolaus Schaller <[email protected]>Signed-off-by Paul Cercueil <[email protected]>---
drivers/nvmem/Kconfig | 10 ++ drivers/nvmem/Makefile | 2 + drivers/nvmem/jz4780-efuse.c | 229 +++++++++++++++++++++++++++++++++++ 3 files changed, 241 insertions(+)create mode 100644
drivers/nvmem/jz4780-efuse.cdrivers/nvmem/Kconfig +0
-0@@ -55,6 +55,16 @@ config NVMEM_IMX_OCOTP_SCU
Quoted 8 lines
drivers/nvmem/Kconfig config NVMEM_IMX_OCOTP_SCU · line 61
Quoted 6 lines
drivers/nvmem/Kconfig +4
-0This 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_IOMEMYou're using regmap, so you need to select
REGMAP_MMIOhere.
Ok.
Quoted 90 lines
Quoted 90 lines
drivers/nvmem/Kconfig +6
-0+ 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_EEPROMtristate "NXP LPC18XX EEPROM Memory Support" depends on ARCH_LPC18XX || COMPILE_TESTdrivers/nvmem/Makefile +0
-0@@ -18,6 +18,8 @@ obj-$(CONFIG_NVMEM_IMX_OCOTP) += nvmem-imx-ocotp.onvmem-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.odrivers/nvmem/Makefile +2
-0+obj-$(CONFIG_JZ4780_EFUSE) += nvmem_jz4780_efuse.o +nvmem_jz4780_efuse-y := jz4780-efuse.oobj-$(CONFIG_NVMEM_LPC18XX_EEPROM) += nvmem_lpc18xx_eeprom.o
nvmem_lpc18xx_eeprom-y := lpc18xx_eeprom.o
obj-$(CONFIG_NVMEM_LPC18XX_OTP) += nvmem_lpc18xx_otp.odrivers/nvmem/jz4780-efuse.c new file +61
-0@@ -0,0 +1,229 @@ +// 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> +#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 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;
Quoted 8 lines
drivers/nvmem/jz4780-efuse.c line 67
Quoted 6 lines
drivers/nvmem/jz4780-efuse.c new file +6
-0+struct clk *clk; +unsigned int rd_adj; +unsigned int rd_strobe; +}; + +/* We read 32 byte chunks to avoid complexity in the driver. */ I don't see how reading 32 byte chunks avoids complexity in the driver; the 'size' variable here could very well be an argument.
I mixed your, mine and the original code.
One issue with a size argument is that regmap reads in 32bit words.
But reading the ethernet MAC address requires to fetch 6 bytes. AFAIK regmap can only read in multiples of the word size.
It must also be possible to read starting at any byte address.
Quoted 35 lines
Quoted 35 lines
drivers/nvmem/jz4780-efuse.c new file +34
-0+static int jz4780_efuse_read_32bytes (struct jz4780_efuse *efuse,char *buf, +unsigned int addr) +{ +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; + } + +return regmap_bulk_read (efuse->map,JZ_EFUDATA (0 ), + buf, size /sizeof (u32)); +} + +/* main entry point */ +static int jz4780_efuse_read (void *context,unsigned int offset, +void *val,size_t bytes) +{ +struct jz4780_efuse *efuse = context;
Quoted 8 lines
drivers/nvmem/jz4780-efuse.c line 107
Quoted 6 lines
drivers/nvmem/jz4780-efuse.c new file +6
-0+int ret; +const int size =32 ; + +while (bytes >0 ) { +unsigned int start = offset & ~(size -1 ); +unsigned int chunk =min (bytes, (start + size) - offset);Why do you need to check for the address alignment?
This is to decide if jz4780_efuse_read_32bytes() can directly read to the destination buffer or if we need a temporary buffer. This avoids to always use a temporary buffer.
Quoted 17 lines
Quoted 5 lines
drivers/nvmem/jz4780-efuse.c new file +4
-0+ +if (start == offset && chunk == size) { + ret =jz4780_efuse_read_32bytes (efuse, val, start); +if (ret <0 )drivers/nvmem/jz4780-efuse.c line 121
Quoted 10 lines
drivers/nvmem/jz4780-efuse.c new file +10
-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);Why do you need this cumbersome process of reading 32 bytes if you need less than that? This looks over-complex to me.
MAC address must allow to read 6 Bytes. I am not sure if regmap can do that.
According to the Programming Manual there are 8 registers with 32 bits each so it is probably wise to read all these 32 bytes in one big read into the regmap. So that it is never attempted to read smaller or not 32bit aligned registers. This also makes sure that bank address switching is not done for every byte.
Quoted 16 lines
Quoted 4 lines
drivers/nvmem/jz4780-efuse.c new file +3
-0+ } + + val += chunk;drivers/nvmem/jz4780-efuse.c line 134
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__initdata = { + .name ="jz4780-efuse" , + .size =1024 ,The efuse is 8 KiB.
I read the Programming Manual as 8 kBit = 1024 Byte. See for example Note 1. and 2. in section "26.2.4 EFUSE Data Register (EFUDATAn)"
Quoted 35 lines
Quoted 35 lines
drivers/nvmem/jz4780-efuse.c new file +34
-0+ .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; +struct device *dev = &pdev->dev; +void __iomem *regs; + + 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);
Quoted 8 lines
drivers/nvmem/jz4780-efuse.c line 174
Quoted 6 lines
drivers/nvmem/jz4780-efuse.c new file +6
-0+ + 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);You didn't enable the clock before, so
clk_get_ratecan return a bogus value.
Well, I copied that from your proposal for jz4780_efuse_probe(). So I don't know if that is correct or not...
How should it be solved? clk_enable() before and clk_disable() after? Or is it enabled somewhere else so that we break something if we explicitly disable?
Quoted 11 lines
drivers/nvmem/jz4780-efuse.c line 182
Quoted 9 lines
drivers/nvmem/jz4780-efuse.c new file +8
-0+ + efuse->dev = dev; +/* +* 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 ;I can't get my head around this calculus. Try this:
I would read it as: multiply clk_rate (in MHz units) by 6.5 and divide by another 1000.
So if clock rate is e.g. 500 Mhz we calculate 6.5 * 500 / 1000 => 3(.25). 500 Mhz is equivalent to 2 ns so 2 ns * 3.25 = 6.5 ns.
But that said: there seems to be something wrong with rounding. To fulfill the
Quoted 1 line
6.5 ns condition the 3.25 should be rounded up to 4.
But since the condition is defined for rd_adj + 1 it is ok to set rd_adj to 3 in this case.
Quoted 5 lines
/* 1 / 6.5ns == 153846154 Hz */
efuse->rd_adj=clk_rate/ 153846154;The efuse read should be *at least* 6.5ns, so no need to bother with the -1/+1.
Well, the (X + 1) - 1 looks very redundant.
So what about:
#define RD_ADJ_FACTOR (unsigned long)(10 * NSEC_PER_SEC / 65) /* 1 / 6.5ns == 153846154 Hz */
efuse->rd_adj = clk_rate / RD_ADJ_FACTOR;
Quoted 6 lines
drivers/nvmem/jz4780-efuse.c line 184
Quoted 3 lines
drivers/nvmem/jz4780-efuse.c new file +2
-0+ efuse->rd_strobe = ((((35000 * (clk_rate /1000000 )) /1000000 ) +1 ) + -5 - efuse->rd_adj);/* 1 / 35ns == 28571429 Hz */
efuse->rd_strobe=max(0, clk_rate / 28571429 + 1 - 5 - efuse->rd_adj);
I think the original author just wanted to make the magic values a little more transparent. 35000 / 1000000 is easier to recognise as 35 ns in the programming manual than 28571429.
Quoted 16 lines
Quoted 16 lines
drivers/nvmem/jz4780-efuse.c new file +15
-0+ +if (efuse->rd_adj >0x1F || efuse->rd_strobe >0x1F ) { +dev_err (&pdev->dev,"Cannot set clock configuration\n" ); +return -EINVAL ; + } + +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;
Quoted 8 lines
drivers/nvmem/jz4780-efuse.c line 205
Quoted 6 lines
drivers/nvmem/jz4780-efuse.c new file +6
-0+ + nvmem =devm_nvmem_register (dev, &cfg); +if (IS_ERR (nvmem)) +return PTR_ERR (nvmem); + +platform_set_drvdata (pdev, nvmem);This isn't used anywhere.
Also copied from your proposal...
But it is indeed not used anywhere.
A big question is why we need to store rd_adj and rd_strobe in the struct jz4780_efuse? They are local variables inside jz4780_efuse_probe().
So the only values used are efuse->map
Quoted 19 lines
Quoted 19 lines
drivers/nvmem/jz4780-efuse.c new file +18
-0+ +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, + .driver = { + .name ="jz4780-efuse" , + .of_match_table = jz4780_efuse_match, + }, +}; +module_platform_driver (jz4780_efuse_driver);
Quoted 8 lines
Quoted 8 lines
drivers/nvmem/jz4780-efuse.c new file +6
-1+ +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" ); --2.23.0
BR and thanks,
Nikolaus
Paul Cercueil
maintainerRe: [PATCH v5 1/6] nvmem: add driver for JZ4780 efuse
Hi Nikolaus,
Quoted 39 lines · H. Nikolaus Schaller
Hi Paul,
Quoted 37 lines
Am 22.02.2020 um 16:38 schrieb Paul Cercueil <[email protected]>:
Hi Nikolaus,
Quoted 29 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.Tested-by Mathieu Malaterre <[email protected]>Signed-off-by PrasannaKumar MuralidharanSigned-off-by Mathieu Malaterre <[email protected]>Signed-off-by H. Nikolaus Schaller <[email protected]>Signed-off-by Paul Cercueil <[email protected]>---
drivers/nvmem/Kconfig | 10 ++ drivers/nvmem/Makefile | 2 + drivers/nvmem/jz4780-efuse.c | 229 +++++++++++++++++++++++++++++++++++ 3 files changed, 241 insertions(+) create mode 100644 drivers/nvmem/jz4780-efuse.cdiff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index
35efab1ba8d9..8143e6e1dd82100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -55,6 +55,16 @@ configNVMEM_IMX_OCOTP_SCU(quoted diff) +0
-0This 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
Quoted 3 lines
Quoted 1 line
You're using regmap, so you need to select
REGMAP_MMIOhere.Ok.
I think you need to depend on OF too, since the driver only probes on devicetree.
Quoted 109 lines
Quoted 106 lines
Quoted 97 lines
+ 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.+
configNVMEM_LPC18XX_EEPROM(quoted diff) +0
-0tristate "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 --gita/drivers/nvmem/jz4780-efuse.cb/drivers/nvmem/jz4780-efuse.c
new file mode 100644
index 000000000000..08b63de0e9cc
---/dev/null
+++b/drivers/nvmem/jz4780-efuse.c
@@ -0,0 +1,229 @@
+// 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>
+#include <linux/regmap.h>
+#include <linux/timer.h>
+
+#defineJZ_EFUCTRL(0x0) /* Control Register */
+#defineJZ_EFUCFG(0x4) /* Configure Register*/
+#defineJZ_EFUSTATE(0x8) /* Status Register */
+#defineJZ_EFUDATA(n)(0xC+ (n) * 4)
+
+#defineEFUCTRL_ADDR_MASK0x3FF
+#defineEFUCTRL_ADDR_SHIFT21
+#defineEFUCTRL_LEN_MASK0x1F
+#defineEFUCTRL_LEN_SHIFT16
+#defineEFUCTRL_PG_ENBIT(15)
+#defineEFUCTRL_WR_ENBIT(1)
+#defineEFUCTRL_RD_ENBIT(0)
+
+#defineEFUCFG_INT_ENBIT(31)
+#defineEFUCFG_RD_ADJ_MASK0xF
+#defineEFUCFG_RD_ADJ_SHIFT20
+#defineEFUCFG_RD_STR_MASK0xF
+#defineEFUCFG_RD_STR_SHIFT16
+#defineEFUCFG_WR_ADJ_MASK0xF
+#defineEFUCFG_WR_ADJ_SHIFT12
+#defineEFUCFG_WR_STR_MASK0xFFF
+#defineEFUCFG_WR_STR_SHIFT0
+
+#defineEFUSTATE_WR_DONEBIT(1)
+#defineEFUSTATE_RD_DONEBIT(0)
+
+structjz4780_efuse{Quoted 8 lines
+ struct device *dev;
+ struct regmap *map;
+ struct clk *clk;
+ unsigned intrd_adj;
+ unsigned intrd_strobe;
+};
+
+/* We read 32 byte chunks to avoid complexity in the driver. */Quoted 2 lines
I don't see how reading 32 byte chunks avoids complexity in the
driver; the 'size' variable here could very well be an argument.
Quoted 8 lines
I mixed your, mine and the original code.
One issue with a size argument is that regmap reads in 32bit words.
But reading the ethernet MAC address requires to fetch 6 bytes. AFAIK regmap can only read in multiples of the word size.
It must also be possible to read starting at any byte address.
Right, I didn't think about byte access.
Quoted 43 lines
Quoted 43 lines
Quoted 42 lines
+static int jz4780_efuse_read_32bytes(struct
jz4780_efuse*efuse,
char *buf,
+ unsigned int addr)
+{
+ 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;
+ }
+
+ return regmap_bulk_read(efuse->map,JZ_EFUDATA(0),
+ buf, size / sizeof(u32));
+}
+
+/* main entry point */
+static int jz4780_efuse_read(void *context, unsigned int offset,
+ void *val,size_tbytes)
+{
+ structjz4780_efuse*efuse = context;
+ int ret;
+ const int size = 32;
+
+ while (bytes > 0) {
+ unsigned int start = offset & ~(size - 1);
+ unsigned int chunk =min(bytes, (start + size) - offset);
Quoted 5 lines
Quoted 1 line
Why do you need to check for the address alignment?
This is to decide if
jz4780_efuse_read_32bytes()can directly read to the destination buffer or if we need a temporary buffer. This avoids to always use a temporary buffer.
Just always use a temporary buffer. It's not like this will be called very often anyway. That makes the code simpler.
Quoted 19 lines
Quoted 16 lines
Quoted 15 lines
+
+ if (start == offset && chunk == size) {
+ 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);Quoted 2 lines
Why do you need this cumbersome process of reading 32 bytes if you
need less than that? This looks over-complex to me.
Quoted 7 lines
MAC address must allow to read 6 Bytes. I am not sure if regmap can do that.
According to the Programming Manual there are 8 registers with 32 bits each so it is probably wise to read all these 32 bytes in one big read into the regmap. So that it is never attempted to read smaller or not 32bit aligned registers. This also makes sure that bank address switching is not done for every byte.
Honestly, it doesn't have to be fast. You could very well read byte per byte. Then you wouldn't have to care about non-aligned addresses or sizes, and the driver would be much simpler.
Quoted 16 lines
Quoted 16 lines
Quoted 15 lines
+ }
+
+ val += chunk;
+ offset += chunk;
+ bytes -= chunk;
+ }
+
+ return 0;
+}
+
+static structnvmem_configjz4780_efuse_nvmem_config__initdata=
{
+ .name = "jz4780-efuse",
+ .size = 1024,
Quoted 4 lines
Quoted 1 line
The efuse is 8 KiB.
I read the Programming Manual as 8 kBit = 1024 Byte. See for example Note 1. and 2. in section "26.2.4 EFUSE Data Register (EFUDATAn)"
Right. 8 Kib, not KiB.
Quoted 42 lines
Quoted 42 lines
Quoted 41 lines
+ .word_size = 1,
+ .stride = 1,
+ .owner =THIS_MODULE,
+ .reg_read =jz4780_efuse_read,
+};
+
+static const structregmap_configjz4780_efuse_regmap_config= {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .max_register =JZ_EFUDATA(7),
+};
+
+static intjz4780_efuse_probe(struct platform_device *pdev)+{
+ structnvmem_device*nvmem;
+ structjz4780_efuse*efuse;
+ structnvmem_configcfg;
+ unsigned longclk_rate;
+ struct device *dev =&pdev->dev;
+ void__iomem*regs;
+
+ efuse =devm_kzalloc(dev, sizeof(*efuse), GFP_KERNEL);
+ if (!efuse)
+ return-ENOMEM;
+
+ regs =devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(regs))
+ returnPTR_ERR(regs);
+
+efuse->map= devm_regmap_init_mmio(dev, regs,
+&jz4780_efuse_regmap_config);
+ if (IS_ERR(efuse->map))
+ returnPTR_ERR(efuse->map);
+
+efuse->clk=devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(efuse->clk))
+ returnPTR_ERR(efuse->clk);
+
+clk_rate=clk_get_rate(efuse->clk);
Quoted 8 lines
Quoted 2 lines
You didn't enable the clock before, so
clk_get_ratecan return a
bogus value.Well, I copied that from your proposal for
jz4780_efuse_probe(). So I don't know if that is correct or not...
How should it be solved?clk_enable()before andclk_disable()after? Or is it enabled somewhere else so that we break something if we explicitly disable?
clk_prepare_enable() here, and clk_disable_unprepare() in the remove callback or in a function passed to devm_add_action_or_reset().
Quoted 11 lines
Quoted 11 lines
Quoted 10 lines
+
+efuse->dev= dev;
+ /*
+ *rd_adjandrd_strobeare 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;
Quoted 6 lines
Quoted 1 line
I can't get my head around this calculus. Try this:
I would read it as: multiply
clk_rate(in MHz units) by 6.5 and divide by another 1000.So if clock rate is e.g. 500 Mhz we calculate 6.5 * 500 / 1000 => 3(.25). 500 Mhz is equivalent to 2 ns so 2 ns * 3.25 = 6.5 ns.
Yes, I'm not saying that the calculus is wrong. But it multiplies picoseconds with MHz, then divides by 1000000, so it's not easy to understand.
Quoted 12 lines
But that said: there seems to be something wrong with rounding. To fulfill the
Quoted 1 line
6.5 ns condition the 3.25 should be rounded up to 4.
But since the condition is defined for
rd_adj+ 1 it is ok to setrd_adjto 3 in this case.Quoted 6 lines
/* 1 / 6.5ns == 153846154 Hz */
efuse->rd_adj=clk_rate/ 153846154;The efuse read should be *at least* 6.5ns, so no need to bother
with the -1/+1.
Quoted 7 lines
Well, the (X + 1) - 1 looks very redundant.
So what about:
#define
RD_ADJ_FACTOR(unsigned long)(10 *NSEC_PER_SEC/ 65) /* 1 / 6.5ns == 153846154 Hz */efuse->rd_adj = clk_rate / RD_ADJ_FACTOR;
Well, you still need the comment after the #define, so why use a #define in the first place?
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
+
efuse->rd_strobe= ((((35000 * (clk_rate/ 1000000)) / 1000000)
+ 1)
+ - 5 -efuse->rd_adj);
Quoted 7 lines
Quoted 3 lines
/* 1 / 35ns == 28571429 Hz */
efuse->rd_strobe= max(0,clk_rate/ 28571429 + 1 - 5 -efuse->rd_adj);I think the original author just wanted to make the magic values a little more transparent. 35000 / 1000000 is easier to recognise as 35 ns in the programming manual than 28571429.
35000 / 1000000 is 35ms, not 35ns.
28571429 Hz is easy to recognize as 35ns when there's a comment right above saying where that value comes from.
Quoted 24 lines
Quoted 24 lines
Quoted 23 lines
+
+ if (efuse->rd_adj>0x1F||efuse->rd_strobe>0x1F) {
+dev_err(&pdev->dev, "Cannot set clock configuration\n");
+ return-EINVAL;
+ }
+
+ 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 =devm_nvmem_register(dev, &cfg);
+ if (IS_ERR(nvmem))
+ returnPTR_ERR(nvmem);
+
+platform_set_drvdata(pdev, nvmem);
Quoted 4 lines
Quoted 1 line
This isn't used anywhere.
Also copied from your proposal...
But it is indeed not used anywhere.
My code wasn't upstream-ready, and wasn't even tested on real hardware. I'm reviewing the driver for mainline inclusion now.
Quoted 2 lines
A big question is why we need to store
rd_adjandrd_strobein the structjz4780_efuse? They are local variables insidejz4780_efuse_probe().
Maybe move the calculation of rd_adj / rd_strobe to a new function that's called from the probe, and have these local variables within this function.
Quoted 35 lines
So the only values used are
efuse->mapQuoted 20 lines
Quoted 20 lines
+
+ return 0;
+}
+
+static const structof_device_idjz4780_efuse_match[] = {
+ { .compatible = "ingenic,jz4780-efuse" },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of,jz4780_efuse_match);
+
+static structplatform_driverjz4780_efuse_driver= {
+ .probe =jz4780_efuse_probe,
+ .driver = {
+ .name = "jz4780-efuse",
+ .of_match_table =jz4780_efuse_match,
+ },
+};
+module_platform_driver(jz4780_efuse_driver);
+Quoted 8 lines
Quoted 8 lines
+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");
--
2.23.0BR and thanks,
Nikolaus
PATCH v5 2/6 Bindings: nvmem: add bindings for JZ4780 efuse
In reply to H. Nikolaus SchallerFrom: 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.
--- .../bindings/nvmem/ingenic,jz4780-efuse.yaml | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +50 -0
@@ -0,0 +1,50 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/nvmem/ingenic,jz4780-efuse.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Ingenic JZ EFUSE driver bindings + +maintainers: + - PrasannaKumar Muralidharan <[email protected]> + +allOf: + - $ref: "nvmem.yaml#" + +properties: + compatible: + enum: + - ingenic,jz4780-efuse + + reg: + maxItems: 1 + + clocks: + # Handle for the ahb for the efuse. + maxItems: 1 + + clock-names: + items: + - const: ahb2 + +required: + - compatible + - reg + - clock-names + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/clock/jz4780-cgu.h> + + efuse@134100d0 { + compatible = "ingenic,jz4780-efuse"; + reg = <0x134100d0 0x2c>; + + clocks = <&cgu JZ4780_CLK_AHB2>; + clock-names = "ahb2"; + }; + +...
Signature · 1 line
2.23.0
Re: [PATCH v5 2/6] Bindings: nvmem: add bindings for JZ4780 efuse
Quoted 46 lines · On Sat, 22 Feb 2020 11:25:37 +0100 "H. Nikolaus Schaller" <[email protected]>…
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.
Tested-by Mathieu Malaterre <[email protected]>Signed-off-by PrasannaKumar Muralidharan <[email protected]>Signed-off-by Mathieu Malaterre <[email protected]>Signed-off-by H. Nikolaus Schaller <[email protected]>[converted to yaml]Signed-off-by Andreas Kemnade <[email protected]>--- .../bindings/nvmem/ingenic,jz4780-efuse.yaml | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yamlDocumentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +24
-0@@ -0,0 +1,50 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/nvmem/ingenic,jz4780-efuse.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Ingenic JZ EFUSE driver bindings + +maintainers: + - PrasannaKumar Muralidharan <[email protected]> + +allOf: + - $ref: "nvmem.yaml#" + +properties: + compatible: + enum: + - ingenic,jz4780-efuse + + reg: + maxItems: 1 + + clocks: + # Handle for the ahb for the efuse.
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml line 34
Quoted 10 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +10
-0+ maxItems: 1 + + clock-names: + items: + - const: ahb2 + +required: + - compatible + - reg + - clock-names
- clocks ?
In my first try there was "clock" without s which
caused the trouble.
Regards,
Andreas
Re: [PATCH v5 2/6] Bindings: nvmem: add bindings for JZ4780 efuse
Quoted 64 lines
Am 22.02.2020 um 11:54 schrieb Andreas Kemnade <[email protected]>:
Quoted 46 lines · On Sat, 22 Feb 2020 11:25:37 +0100 "H. Nikolaus Schaller" <[email protected]>…
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.
Tested-by Mathieu Malaterre <[email protected]>Signed-off-by PrasannaKumar Muralidharan <[email protected]>Signed-off-by Mathieu Malaterre <[email protected]>Signed-off-by H. Nikolaus Schaller <[email protected]>[converted to yaml]Signed-off-by Andreas Kemnade <[email protected]>---
.../bindings/nvmem/ingenic,jz4780-efuse.yaml| 50 +++++++++++++++++++1 file changed, 50 insertions(+)create mode 100644
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yamlDocumentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +24
-0@@ -0,0 +1,50 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/nvmem/ingenic,jz4780-efuse.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Ingenic JZ EFUSE driver bindings + +maintainers: + - PrasannaKumar Muralidharan <[email protected]> + +allOf: + - $ref: "nvmem.yaml#" + +properties: + compatible: + enum: + - ingenic,jz4780-efuse + + reg: + maxItems: 1 + + clocks: + # Handle for the ahb for the efuse.Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml line 34
Quoted 10 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +10
-0+ maxItems: 1 + + clock-names: + items: + - const: ahb2 + +required: + - compatible + - reg + - clock-names- clocks ?
Quoted 2 lines
In my first try there was "clock" without s which
caused the trouble.
Ah, ok. Seems I have mis-interpreted this.
BR and thanks,
Nikolaus
Re: [PATCH v5 2/6] Bindings: nvmem: add bindings for JZ4780 efuse
In reply to H. Nikolaus SchallerQuoted 41 lines · On Sat, 22 Feb 2020 11:25:37 +0100 "H. Nikolaus Schaller" <[email protected]>…
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.
Tested-by Mathieu Malaterre <[email protected]>Signed-off-by PrasannaKumar Muralidharan <[email protected]>Signed-off-by Mathieu Malaterre <[email protected]>Signed-off-by H. Nikolaus Schaller <[email protected]>[converted to yaml]Signed-off-by Andreas Kemnade <[email protected]>--- .../bindings/nvmem/ingenic,jz4780-efuse.yaml | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yamlDocumentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +19
-0@@ -0,0 +1,50 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/nvmem/ingenic,jz4780-efuse.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Ingenic JZ EFUSE driver bindings + +maintainers: + - PrasannaKumar Muralidharan <[email protected]> + +allOf: + - $ref: "nvmem.yaml#" + +properties: + compatible: + enum: + - ingenic,jz4780-efuse +
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml line 29
Quoted 10 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +10
-0+ reg: + maxItems: 1 + + clocks: + # Handle for the ahb for the efuse. + maxItems: 1 + + clock-names: + items: + - const: ahb2
as Rob said: probably not needed, since it is a single
clock, and the driver uses devm_clk_get(dev, NULL), so it should be prepared for that without any extra work.
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml line 34
Quoted 5 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +5
-0+ +required: + - compatible + - reg + - clock-names
so it is not required here (but "- clocks" (not "- clock") as said in earlier mail).
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml line 47
Quoted 13 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +13
-0+ +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/clock/jz4780-cgu.h> + + efuse@134100d0 { + compatible = "ingenic,jz4780-efuse"; + reg = <0x134100d0 0x2c>; + + clocks = <&cgu JZ4780_CLK_AHB2>; + clock-names = "ahb2";
and not required here.
Regards,
Andreas
Re: [PATCH v5 2/6] Bindings: nvmem: add bindings for JZ4780 efuse
Quoted 52 lines
Am 22.02.2020 um 16:57 schrieb Andreas Kemnade <[email protected]>:
Quoted 46 lines · On Sat, 22 Feb 2020 11:25:37 +0100 "H. Nikolaus Schaller" <[email protected]>…
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.
Tested-by Mathieu Malaterre <[email protected]>Signed-off-by PrasannaKumar Muralidharan <[email protected]>Signed-off-by Mathieu Malaterre <[email protected]>Signed-off-by H. Nikolaus Schaller <[email protected]>[converted to yaml]Signed-off-by Andreas Kemnade <[email protected]>---
.../bindings/nvmem/ingenic,jz4780-efuse.yaml| 50 +++++++++++++++++++1 file changed, 50 insertions(+)create mode 100644
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yamlDocumentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +24
-0@@ -0,0 +1,50 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/nvmem/ingenic,jz4780-efuse.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Ingenic JZ EFUSE driver bindings + +maintainers: + - PrasannaKumar Muralidharan <[email protected]> + +allOf: + - $ref: "nvmem.yaml#" + +properties: + compatible: + enum: + - ingenic,jz4780-efuse + + reg: + maxItems: 1 + + clocks: + # Handle for the ahb for the efuse.
Quoted 8 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml line 29
Quoted 5 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +5
-0+ maxItems: 1 + + clock-names: + items: + - const: ahb2as Rob said: probably not needed, since it is a single
clock, and the driver usesdevm_clk_get(dev, NULL), so it should be prepared for that without any extra work.
The question is if a specific driver implementation should determine what the DT requires or the other way round. I don't know...
I did interpret Rob's comment differently: there was
Quoted 1 line
- "clock-names" Must be "
bus_clk"
and he did say:
'clk' is redundant. How about 'ahb'?
So I thought he refers to the _clk suffix?
Quoted 8 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml line 34
Quoted 6 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +5
-0+ +required: + - compatible + - reg + - clock-namesso it is not required here (but "- clocks" (not "- clock") as said in earlier mail).
Well, this is another example where I do not yet see any improvement by yaml. It is the same amount of guessing what should be written where. Is this to be added or not? When is it and why, when not and why?
BR and thanks,
Nikolaus
Re: [PATCH v5 2/6] Bindings: nvmem: add bindings for JZ4780 efuse
Quoted 65 lines · On Sat, 22 Feb 2020 17:34:06 +0100 "H. Nikolaus Schaller" <[email protected]>…
Quoted 51 lines
Am 22.02.2020 um 16:57 schrieb Andreas Kemnade <[email protected]>:
Quoted 46 lines · On Sat, 22 Feb 2020 11:25:37 +0100 "H. Nikolaus Schaller" <[email protected]>…
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.
Tested-by Mathieu Malaterre <[email protected]>Signed-off-by PrasannaKumar Muralidharan <[email protected]>Signed-off-by Mathieu Malaterre <[email protected]>Signed-off-by H. Nikolaus Schaller <[email protected]>[converted to yaml]Signed-off-by Andreas Kemnade <[email protected]>---
.../bindings/nvmem/ingenic,jz4780-efuse.yaml| 50 +++++++++++++++++++1 file changed, 50 insertions(+)create mode 100644
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yamlDocumentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +24
-0@@ -0,0 +1,50 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/nvmem/ingenic,jz4780-efuse.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Ingenic JZ EFUSE driver bindings + +maintainers: + - PrasannaKumar Muralidharan <[email protected]> + +allOf: + - $ref: "nvmem.yaml#" + +properties: + compatible: + enum: + - ingenic,jz4780-efuse + + reg: + maxItems: 1 + + clocks: + # Handle for the ahb for the efuse.Quoted 8 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml line 29
Quoted 5 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +5
-0+ maxItems: 1 + + clock-names: + items: + - const: ahb2as Rob said: probably not needed, since it is a single
clock, and the driver usesdevm_clk_get(dev, NULL), so it should be prepared for that without any extra work.The question is if a specific driver implementation should determine what the DT requires or the other way round. I don't know...
I did interpret Rob's comment differently: there was
Quoted 8 lines
Quoted 1 line
- "clock-names" Must be "
bus_clk"and he did say:
'clk' is redundant. How about 'ahb'?
So I thought he refers to the
_clksuffix?
and he said to my first try:
---
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml
Quoted 3 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +3
-0+ clock-names: + items: + - const: bus_clk
My prior comment still applies.
Also, for a single clock, you don't really need a name.
---
Regards,
Andreas
Re: [PATCH v5 2/6] Bindings: nvmem: add bindings for JZ4780 efuse
Quoted 82 lines
Am 22.02.2020 um 17:46 schrieb Andreas Kemnade <[email protected]>:
Quoted 65 lines · On Sat, 22 Feb 2020 17:34:06 +0100 "H. Nikolaus Schaller" <[email protected]>…
Quoted 51 lines
Am 22.02.2020 um 16:57 schrieb Andreas Kemnade <[email protected]>:
Quoted 46 lines · On Sat, 22 Feb 2020 11:25:37 +0100 "H. Nikolaus Schaller" <[email protected]>…
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.
Tested-by Mathieu Malaterre <[email protected]>Signed-off-by PrasannaKumar Muralidharan <[email protected]>Signed-off-by Mathieu Malaterre <[email protected]>Signed-off-by H. Nikolaus Schaller <[email protected]>[converted to yaml]Signed-off-by Andreas Kemnade <[email protected]>---
.../bindings/nvmem/ingenic,jz4780-efuse.yaml| 50 +++++++++++++++++++1 file changed, 50 insertions(+)create mode 100644
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yamlDocumentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +24
-0@@ -0,0 +1,50 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/nvmem/ingenic,jz4780-efuse.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Ingenic JZ EFUSE driver bindings + +maintainers: + - PrasannaKumar Muralidharan <[email protected]> + +allOf: + - $ref: "nvmem.yaml#" + +properties: + compatible: + enum: + - ingenic,jz4780-efuse + + reg: + maxItems: 1 + + clocks: + # Handle for the ahb for the efuse.Quoted 8 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml line 29
Quoted 5 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +5
-0+ maxItems: 1 + + clock-names: + items: + - const: ahb2as Rob said: probably not needed, since it is a single
clock, and the driver usesdevm_clk_get(dev, NULL), so it should be prepared for that without any extra work.The question is if a specific driver implementation should determine what the DT requires or the other way round. I don't know...
I did interpret Rob's comment differently: there was
Quoted 8 lines
Quoted 1 line
- "clock-names" Must be "
bus_clk"and he did say:
'clk' is redundant. How about 'ahb'?
So I thought he refers to the
_clksuffix?and he said to my first try:
---
Quoted 7 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml
Quoted 3 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +3
-0+ clock-names: + items: + - const: bus_clkMy prior comment still applies.
Also, for a single clock, you don't really need a name.
Ah, ok. I may have missed that.
BR and thanks,
Nikolaus
Re: [PATCH v5 2/6] Bindings: nvmem: add bindings for JZ4780 efuse
In reply to H. Nikolaus SchallerQuoted 82 lines · On Sat, 22 Feb 2020 17:34:06 +0100 "H. Nikolaus Schaller" <[email protected]>…
Quoted 51 lines
Am 22.02.2020 um 16:57 schrieb Andreas Kemnade <[email protected]>:
Quoted 46 lines · On Sat, 22 Feb 2020 11:25:37 +0100 "H. Nikolaus Schaller" <[email protected]>…
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.
Tested-by Mathieu Malaterre <[email protected]>Signed-off-by PrasannaKumar Muralidharan <[email protected]>Signed-off-by Mathieu Malaterre <[email protected]>Signed-off-by H. Nikolaus Schaller <[email protected]>[converted to yaml]Signed-off-by Andreas Kemnade <[email protected]>---
.../bindings/nvmem/ingenic,jz4780-efuse.yaml| 50 +++++++++++++++++++1 file changed, 50 insertions(+)create mode 100644
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yamlDocumentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +24
-0@@ -0,0 +1,50 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/nvmem/ingenic,jz4780-efuse.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Ingenic JZ EFUSE driver bindings + +maintainers: + - PrasannaKumar Muralidharan <[email protected]> + +allOf: + - $ref: "nvmem.yaml#" + +properties: + compatible: + enum: + - ingenic,jz4780-efuse + + reg: + maxItems: 1 + + clocks: + # Handle for the ahb for the efuse.Quoted 8 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml line 29
Quoted 5 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +5
-0+ maxItems: 1 + + clock-names: + items: + - const: ahb2as Rob said: probably not needed, since it is a single
clock, and the driver usesdevm_clk_get(dev, NULL), so it should be prepared for that without any extra work.The question is if a specific driver implementation should determine what the DT requires or the other way round. I don't know...
I did interpret Rob's comment differently: there was
Quoted 1 line
- "clock-names" Must be "
bus_clk"and he did say:
'clk' is redundant. How about 'ahb'?
So I thought he refers to the
_clksuffix?Quoted 8 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml line 34
Quoted 6 lines
Documentation/devicetree/bindings/nvmem/ingenic,jz4780-efuse.yaml new file +5
-0+ +required: + - compatible + - reg + - clock-namesso it is not required here (but "- clocks" (not "- clock") as said in earlier mail).
Quoted 4 lines
Well, this is another example where I do not yet see any improvement by yaml. It is the same amount of guessing what should be written where. Is this to be added or not? When is it and why, when not and why?
well, this is the list of required properties which have to be an any devicetree using that device, the conversion of the comments for the properties in the .txt-file.
So, if you do not declare a property before, you cannot specify that it is required. So if we drop the definition of clock-names we cannot declare it as required.
Regards,
Andreas
PATCH v5 3/6 Documentation: ABI: nvmem: add documentation for JZ4780 efuse ABI
In reply to H. Nikolaus SchallerFrom: 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.
--- .../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
@@ -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
PATCH v5 4/6 nvmem: MAINTAINERS: add maintainer for JZ4780 efuse driver
In reply to H. Nikolaus SchallerFrom: 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.
--- MAINTAINERS | 5 +++++ 1 file changed, 5 insertions(+)
MAINTAINERS +5 -0
@@ -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
PATCH v5 5/6 MIPS: DTS: JZ4780: define node for JZ4780 efuse
In reply to H. Nikolaus SchallerFrom: 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.
It also changes the nemc reg range to avoid overlap.
--- arch/mips/boot/dts/ingenic/jz4780.dtsi | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
arch/mips/boot/dts/ingenic/jz4780.dtsi +16 -1
@@ -358,7 +358,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
@@ -373,6 +373,21 @@ status = "disabled"; }; + efuse: efuse@134100d0 { + compatible = "ingenic,jz4780-efuse"; + reg = <0x134100d0 0x2c>; + + clocks = <&cgu JZ4780_CLK_AHB2>; + clock-names = "ahb2"; + + #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
PATCH v5 6/6 MIPS: DTS: CI20: make DM9000 Ethernet controller use NVMEM to find the default MAC address
In reply to H. Nikolaus SchallerThere is a unique MAC address programmed into the eFuses of the JZ4780 chip in the CI20 factory. By using this for initializing the DM9000 Ethernet controller, every CI20 board has an individual - but stable - MAC address and DHCP can assign 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
@@ -355,6 +355,9 @@ interrupt-parent = <&gpe>; interrupts = <19 4>; + + nvmem-cells = <ð0_addr>; + nvmem-cell-names = "mac-address"; }; };
Signature · 1 line
2.23.0