| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <8722F126-2032-4AAF-BB99-82B3E9642818@goldelico.com>
Date: Fri, 28 Feb 2020 15:57:01 +0100
From: "H. Nikolaus Schaller" <hns@...delico.com>
To: Paul Cercueil <paul@...pouillou.net>
Cc: PrasannaKumar Muralidharan <prasannatsmkumar@...il.com>,
Andreas Kemnade <andreas@...nade.info>,
Mathieu Malaterre <malat@...ian.org>,
Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Ralf Baechle <ralf@...ux-mips.org>,
Paul Burton <paulburton@...nel.org>,
Mauro Carvalho Chehab <mchehab+samsung@...nel.org>,
"David S. Miller" <davem@...emloft.net>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>,
Krzysztof Kozlowski <krzk@...nel.org>,
Kees Cook <keescook@...omium.org>,
Andi Kleen <ak@...ux.intel.com>,
Geert Uytterhoeven <geert+renesas@...der.be>,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
linux-mips@...r.kernel.org, letux-kernel@...nphoenux.org,
kernel@...a-handheld.com
Subject: Re: [PATCH v7 2/7] nvmem: add driver for JZ4780 efuse
> Am 28.02.2020 um 15:48 schrieb Paul Cercueil <paul@...pouillou.net>:
>
> Hi Nikolaus,
>
> Le ven., févr. 28, 2020 at 14:58, H. Nikolaus Schaller <hns@...delico.com> a écrit :
>> From: PrasannaKumar Muralidharan <prasannatsmkumar@...il.com>
>> This patch brings support for the JZ4780 efuse. Currently it only exposes
>> a read only access to the entire 8K bits efuse memory and nvmem cells.
>> To fetch for example the MAC address:
>> dd if=/sys/devices/platform/134100d0.efuse/jz4780-efuse0/nvmem bs=1 skip=34 count=6 status=none | xxd
>> Tested-by: Mathieu Malaterre <malat@...ian.org>
>> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@...il.com>
>> Signed-off-by: Mathieu Malaterre <malat@...ian.org>
>> Signed-off-by: H. Nikolaus Schaller <hns@...delico.com>
>> Signed-off-by: Paul Cercueil <paul@...pouillou.net>
>> ---
>> drivers/nvmem/Kconfig | 12 ++
>> drivers/nvmem/Makefile | 2 +
>> drivers/nvmem/jz4780-efuse.c | 234 +++++++++++++++++++++++++++++++++++
>> 3 files changed, 248 insertions(+)
>> create mode 100644 drivers/nvmem/jz4780-efuse.c
>> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
>> index 35efab1ba8d9..d7b7f6d688e7 100644
>> --- a/drivers/nvmem/Kconfig
>> +++ b/drivers/nvmem/Kconfig
>> @@ -55,6 +55,18 @@ config NVMEM_IMX_OCOTP_SCU
>> This is a driver for the SCU On-Chip OTP Controller (OCOTP)
>> available on i.MX8 SoCs.
>> +config JZ4780_EFUSE
>> + tristate "JZ4780 EFUSE Memory Support"
>> + depends on MACH_INGENIC || COMPILE_TEST
>> + depends on HAS_IOMEM
>> + depends on OF
>> + select REGMAP_MMIO
>> + help
>> + Say Y here to include support for JZ4780 efuse memory found on
>> + all JZ4780 SoC based devices.
>> + To compile this driver as a module, choose M here: the module
>> + will be called nvmem_jz4780_efuse.
>> +
>> config NVMEM_LPC18XX_EEPROM
>> tristate "NXP LPC18XX EEPROM Memory Support"
>> depends on ARCH_LPC18XX || COMPILE_TEST
>> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
>> index 6b466cd1427b..65a268d17807 100644
>> --- a/drivers/nvmem/Makefile
>> +++ b/drivers/nvmem/Makefile
>> @@ -18,6 +18,8 @@ obj-$(CONFIG_NVMEM_IMX_OCOTP) += nvmem-imx-ocotp.o
>> nvmem-imx-ocotp-y := imx-ocotp.o
>> obj-$(CONFIG_NVMEM_IMX_OCOTP_SCU) += nvmem-imx-ocotp-scu.o
>> nvmem-imx-ocotp-scu-y := imx-ocotp-scu.o
>> +obj-$(CONFIG_JZ4780_EFUSE) += nvmem_jz4780_efuse.o
>> +nvmem_jz4780_efuse-y := jz4780-efuse.o
>> obj-$(CONFIG_NVMEM_LPC18XX_EEPROM) += nvmem_lpc18xx_eeprom.o
>> nvmem_lpc18xx_eeprom-y := lpc18xx_eeprom.o
>> obj-$(CONFIG_NVMEM_LPC18XX_OTP) += nvmem_lpc18xx_otp.o
>> diff --git a/drivers/nvmem/jz4780-efuse.c b/drivers/nvmem/jz4780-efuse.c
>> new file mode 100644
>> index 000000000000..4e9dd340e33a
>> --- /dev/null
>> +++ b/drivers/nvmem/jz4780-efuse.c
>> @@ -0,0 +1,234 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * JZ4780 EFUSE Memory Support driver
>> + *
>> + * Copyright (c) 2017 PrasannaKumar Muralidharan <prasannatsmkumar@...il.com>
>> + * Copyright (c) 2020 H. Nikolaus Schaller <hns@...delico.com>
>> + */
>> +
>> +/*
>> + * Currently supports JZ4780 efuse which has 8K programmable bit.
>> + * Efuse is separated into seven segments as below:
>> + *
>> + * -----------------------------------------------------------------------
>> + * | 64 bit | 128 bit | 128 bit | 3520 bit | 8 bit | 2296 bit | 2048 bit |
>> + * -----------------------------------------------------------------------
>> + *
>> + * The rom itself is accessed using a 9 bit address line and an 8 word wide bus
>> + * which reads/writes based on strobes. The strobe is configured in the config
>> + * register and is based on number of cycles of the bus clock.
>> + *
>> + * Driver supports read only as the writes are done in the Factory.
>> + */
>> +
>> +#include <linux/bitops.h>
>> +#include <linux/clk.h>
>> +#include <linux/module.h>
>> +#include <linux/nvmem-provider.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/timer.h>
>> +
>> +#define JZ_EFUCTRL (0x0) /* Control Register */
>> +#define JZ_EFUCFG (0x4) /* Configure Register*/
>> +#define JZ_EFUSTATE (0x8) /* Status Register */
>> +#define JZ_EFUDATA(n) (0xC + (n) * 4)
>> +
>> +/* We read 32 byte chunks to avoid complexity in the driver. */
>> +#define JZ_EFU_READ_SIZE 32
>> +
>> +#define EFUCTRL_ADDR_MASK 0x3FF
>> +#define EFUCTRL_ADDR_SHIFT 21
>> +#define EFUCTRL_LEN_MASK 0x1F
>> +#define EFUCTRL_LEN_SHIFT 16
>> +#define EFUCTRL_PG_EN BIT(15)
>> +#define EFUCTRL_WR_EN BIT(1)
>> +#define EFUCTRL_RD_EN BIT(0)
>> +
>> +#define EFUCFG_INT_EN BIT(31)
>> +#define EFUCFG_RD_ADJ_MASK 0xF
>> +#define EFUCFG_RD_ADJ_SHIFT 20
>> +#define EFUCFG_RD_STR_MASK 0xF
>> +#define EFUCFG_RD_STR_SHIFT 16
>> +#define EFUCFG_WR_ADJ_MASK 0xF
>> +#define EFUCFG_WR_ADJ_SHIFT 12
>> +#define EFUCFG_WR_STR_MASK 0xFFF
>> +#define EFUCFG_WR_STR_SHIFT 0
>> +
>> +#define EFUSTATE_WR_DONE BIT(1)
>> +#define EFUSTATE_RD_DONE BIT(0)
>> +
>> +struct jz4780_efuse {
>> + struct device *dev;
>> + struct regmap *map;
>> + struct clk *clk;
>> +};
>> +
>> +/* main entry point */
>> +static int jz4780_efuse_read(void *context, unsigned int offset,
>> + void *val, size_t bytes)
>> +{
>> + struct jz4780_efuse *efuse = context;
>> +
>> + while (bytes > 0) {
>> + unsigned int start = offset & ~(JZ_EFU_READ_SIZE - 1);
>> + unsigned int chunk = min(bytes, (start + JZ_EFU_READ_SIZE)
>> + - offset);
>> + char buf[JZ_EFU_READ_SIZE];
>> + unsigned int tmp;
>> + u32 ctrl;
>> + int ret;
>> +
>> + ctrl = (start << EFUCTRL_ADDR_SHIFT)
>> + | ((JZ_EFU_READ_SIZE - 1) << EFUCTRL_LEN_SHIFT)
>> + | EFUCTRL_RD_EN;
>> +
>> + regmap_update_bits(efuse->map, JZ_EFUCTRL,
>> + (EFUCTRL_ADDR_MASK << EFUCTRL_ADDR_SHIFT) |
>> + (EFUCTRL_LEN_MASK << EFUCTRL_LEN_SHIFT) |
>> + EFUCTRL_PG_EN | EFUCTRL_WR_EN |
>> + EFUCTRL_RD_EN,
>> + ctrl);
>> +
>> + ret = regmap_read_poll_timeout(efuse->map, JZ_EFUSTATE,
>> + tmp, tmp & EFUSTATE_RD_DONE,
>> + 1 * MSEC_PER_SEC,
>> + 50 * MSEC_PER_SEC);
>> + if (ret < 0) {
>> + dev_err(efuse->dev, "Time out while reading efuse data");
>> + return ret;
>> + }
>> +
>> + ret = regmap_bulk_read(efuse->map, JZ_EFUDATA(0),
>> + buf, JZ_EFU_READ_SIZE / sizeof(u32));
>> + if (ret < 0)
>> + return ret;
>> +
>> + memcpy(val, &buf[offset - start], chunk);
>> +
>> + val += chunk;
>> + offset += chunk;
>> + bytes -= chunk;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static struct nvmem_config jz4780_efuse_nvmem_config = {
>> + .name = "jz4780-efuse",
>> + .size = 1024,
>> + .word_size = 1,
>> + .stride = 1,
>> + .owner = THIS_MODULE,
>> + .reg_read = jz4780_efuse_read,
>> +};
>> +
>> +static const struct regmap_config jz4780_efuse_regmap_config = {
>> + .reg_bits = 32,
>> + .val_bits = 32,
>> + .reg_stride = 4,
>> + .max_register = JZ_EFUDATA(7),
>> +};
>> +
>> +static int jz4780_efuse_probe(struct platform_device *pdev)
>> +{
>> + struct nvmem_device *nvmem;
>> + struct jz4780_efuse *efuse;
>> + struct nvmem_config cfg;
>> + unsigned long clk_rate;
>> + unsigned long rd_adj;
>> + unsigned long rd_strobe;
>> + struct device *dev = &pdev->dev;
>> + void __iomem *regs;
>> + int ret;
>> +
>> + efuse = devm_kzalloc(dev, sizeof(*efuse), GFP_KERNEL);
>> + if (!efuse)
>> + return -ENOMEM;
>> +
>> + regs = devm_platform_ioremap_resource(pdev, 0);
>> + if (IS_ERR(regs))
>> + return PTR_ERR(regs);
>> +
>> + efuse->map = devm_regmap_init_mmio(dev, regs,
>> + &jz4780_efuse_regmap_config);
>> + if (IS_ERR(efuse->map))
>> + return PTR_ERR(efuse->map);
>> +
>> + efuse->clk = devm_clk_get(&pdev->dev, NULL);
>> + if (IS_ERR(efuse->clk))
>> + return PTR_ERR(efuse->clk);
>> +
>> + ret = clk_prepare_enable(efuse->clk);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = devm_add_action_or_reset(&pdev->dev,
>> + &clk_disable_unprepare,
>> + efuse->clk);
>
> That's what I thought, this does not build:
>
> CC drivers/nvmem/jz4780-efuse.o
> drivers/nvmem/jz4780-efuse.c: In function 'jz4780_efuse_probe':
> drivers/nvmem/jz4780-efuse.c:168:12: error: passing argument 2 of 'devm_add_action_or_reset' from incompatible pointer type [-Werror=incompatible-pointer-types]
> 168 | &clk_disable_unprepare,
> | ^~~~~~~~~~~~~~~~~~~~~~
> | |
> | void (*)(struct clk *)
> In file included from ./include/linux/platform_device.h:13,
> from drivers/nvmem/jz4780-efuse.c:29:
> ./include/linux/device.h:256:16: note: expected 'void (*)(void *)' but argument is of type 'void (*)(struct clk *)'
> 256 | void (*action)(void *), void *data)
> | ~~~~~~~^~~~~~~~~~~~~~~
> cc1: some warnings being treated as errors
> make[2]: *** [scripts/Makefile.build:268: drivers/nvmem/jz4780-efuse.o] Error 1
> make[1]: *** [scripts/Makefile.build:505: drivers/nvmem] Error 2
> make: *** [Makefile:1681: drivers] Error 2
>
>
> You need a local function of type 'void foo(void *data)' that just calls clk_disable_unprepare(data) and the compiler will be happy.
> (btw - no need to deference functions pointers, since they are already pointers)
Well, this asks for a devm_clk_enable_prepare but it was apparently rejected...
https://lore.kernel.org/patchwork/patch/755667/
>
> With that said, I expect you to at least compile-test the patchset before sending it upstream...
I have compiled and tested it on real hardware. But missed the warning because my cross-compiler seems to emit a warning only.
Sorry, I don't have a good day and after 7 rounds one wants to get things through the door and with least effort as quick as possible :)
BR,
Nikolaus
Powered by blists - more mailing lists