* [PATCH 0/4] Amlogic Meson6/Meson8/Meson8b efuse support @ 2017-10-01 12:56 Martin Blumenstingl [not found] ` <20171001125700.1520-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Martin Blumenstingl @ 2017-10-01 12:56 UTC (permalink / raw) To: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, carlo-KA+7E9HrN00dnm+yROfE0A, khilman-rdvid1DuHRBWk0Htik3J/w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Cc: linux-I+IVW8TIWO2tmTQ+vhA3Yw, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Martin Blumenstingl This series implements a driver for the efuse found on the Amlogic Meson6/Meson8/Meson8b SoCs. A new driver is needed because the existing "meson-efuse" driver communicates through the "secure monitor" firmware with the HW, while these older SoCs access the hardware registers directly. An additional patch also explicitly states that the existing "meson-efuse" driver is only for the Meson GX SoCs by adjusting the dt-binding docs, Kconfig help texts, etc. Please note that the dts patch ("ARM: dts: meson: add the efuse node") is built on top of my other series ("soc: amlogic: Add Meson6/8/8b SoC Information driver") from: [0] [0] http://lists.infradead.org/pipermail/linux-amlogic/2017-September/004772.html Martin Blumenstingl (4): dt-bindings: nvmem: Describe the Amlogic Meson6/Meson8/Meson8b efuse nvmem: meson-efuse: indicate that this driver is only for Meson GX SoCs nvmem: add a driver for the Amlogic Meson6/Meson8/Meson8b SoCs ARM: dts: meson: add the efuse node .../devicetree/bindings/nvmem/amlogic-efuse.txt | 2 +- .../bindings/nvmem/amlogic-meson-mx-efuse.txt | 22 ++ arch/arm/boot/dts/meson.dtsi | 13 + arch/arm/boot/dts/meson6.dtsi | 3 + arch/arm/boot/dts/meson8.dtsi | 6 + arch/arm/boot/dts/meson8b.dtsi | 7 + drivers/nvmem/Kconfig | 14 +- drivers/nvmem/Makefile | 2 + drivers/nvmem/meson-efuse.c | 4 +- drivers/nvmem/meson-mx-efuse.c | 272 +++++++++++++++++++++ 10 files changed, 340 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/nvmem/amlogic-meson-mx-efuse.txt create mode 100644 drivers/nvmem/meson-mx-efuse.c -- 2.14.2 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20171001125700.1520-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>]
* [PATCH 1/4] dt-bindings: nvmem: Describe the Amlogic Meson6/Meson8/Meson8b efuse [not found] ` <20171001125700.1520-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> @ 2017-10-01 12:56 ` Martin Blumenstingl 2017-10-01 12:56 ` [PATCH 2/4] nvmem: meson-efuse: indicate that this driver is only for Meson GX SoCs Martin Blumenstingl ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Martin Blumenstingl @ 2017-10-01 12:56 UTC (permalink / raw) To: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, carlo-KA+7E9HrN00dnm+yROfE0A, khilman-rdvid1DuHRBWk0Htik3J/w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Cc: linux-I+IVW8TIWO2tmTQ+vhA3Yw, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Martin Blumenstingl Amlogic Meson6, Meson8 and Meson8b SoCs have an efuse which contains calibration data from the factory (for the internal temperature sensor and some CVBS connector settings). Some manufacturers also store the MAC address or serial number in the efuse. This documents the devicetree bindings for the efuse on these SoCs. Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> --- .../bindings/nvmem/amlogic-meson-mx-efuse.txt | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 Documentation/devicetree/bindings/nvmem/amlogic-meson-mx-efuse.txt diff --git a/Documentation/devicetree/bindings/nvmem/amlogic-meson-mx-efuse.txt b/Documentation/devicetree/bindings/nvmem/amlogic-meson-mx-efuse.txt new file mode 100644 index 000000000000..a3c63954a1a4 --- /dev/null +++ b/Documentation/devicetree/bindings/nvmem/amlogic-meson-mx-efuse.txt @@ -0,0 +1,22 @@ +Amlogic Meson6/Meson8/Meson8b efuse + +Required Properties: +- compatible: depending on the SoC this should be one of: + - "amlogic,meson6-efuse" + - "amlogic,meson8-efuse" + - "amlogic,meson8b-efuse" +- reg: base address and size of the efuse registers +- clocks: a reference to the efuse core gate clock +- clock-names: must be "core" + +All properties and sub-nodes as well as the consumer bindings +defined in nvmem.txt in this directory are also supported. + + +Example: + efuse: nvmem@0 { + compatible = "amlogic,meson8-efuse"; + reg = <0x0 0x2000>; + clocks = <&clkc CLKID_EFUSE>; + clock-names = "core"; + }; -- 2.14.2 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/4] nvmem: meson-efuse: indicate that this driver is only for Meson GX SoCs [not found] ` <20171001125700.1520-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> 2017-10-01 12:56 ` [PATCH 1/4] dt-bindings: nvmem: Describe the Amlogic Meson6/Meson8/Meson8b efuse Martin Blumenstingl @ 2017-10-01 12:56 ` Martin Blumenstingl 2017-10-01 12:56 ` [PATCH 3/4] nvmem: add a driver for the Amlogic Meson6/Meson8/Meson8b SoCs Martin Blumenstingl 2017-10-01 12:57 ` [PATCH 4/4] ARM: dts: meson: add the efuse node Martin Blumenstingl 3 siblings, 0 replies; 10+ messages in thread From: Martin Blumenstingl @ 2017-10-01 12:56 UTC (permalink / raw) To: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, carlo-KA+7E9HrN00dnm+yROfE0A, khilman-rdvid1DuHRBWk0Htik3J/w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Cc: linux-I+IVW8TIWO2tmTQ+vhA3Yw, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Martin Blumenstingl The current Amlogic Meson eFuse driver only supports the 64-bit SoCs (GXBB and newer). Older SoCs cannot be supported by the same driver because they do not use the meson secure monitor firmware to access the hardware. Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> --- Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt | 2 +- drivers/nvmem/Kconfig | 4 ++-- drivers/nvmem/meson-efuse.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt b/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt index fafd85bd67a6..e3298e18de26 100644 --- a/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt +++ b/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt @@ -1,4 +1,4 @@ -= Amlogic eFuse device tree bindings = += Amlogic Meson GX eFuse device tree bindings = Required properties: - compatible: should be "amlogic,meson-gxbb-efuse" diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index 101ced4c84be..a21a781f587d 100644 --- a/drivers/nvmem/Kconfig +++ b/drivers/nvmem/Kconfig @@ -135,11 +135,11 @@ config NVMEM_VF610_OCOTP be called nvmem-vf610-ocotp. config MESON_EFUSE - tristate "Amlogic eFuse Support" + tristate "Amlogic Meson GX eFuse Support" depends on (ARCH_MESON || COMPILE_TEST) && MESON_SM help This is a driver to retrieve specific values from the eFuse found on - the Amlogic Meson SoCs. + the Amlogic Meson GX SoCs. This driver can also be built as a module. If so, the module will be called nvmem_meson_efuse. diff --git a/drivers/nvmem/meson-efuse.c b/drivers/nvmem/meson-efuse.c index 70bfc9839bb2..1ea3cd24a508 100644 --- a/drivers/nvmem/meson-efuse.c +++ b/drivers/nvmem/meson-efuse.c @@ -1,5 +1,5 @@ /* - * Amlogic eFuse Driver + * Amlogic Meson GX eFuse Driver * * Copyright (c) 2016 Endless Computers, Inc. * Author: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org> @@ -89,5 +89,5 @@ static struct platform_driver meson_efuse_driver = { module_platform_driver(meson_efuse_driver); MODULE_AUTHOR("Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>"); -MODULE_DESCRIPTION("Amlogic Meson NVMEM driver"); +MODULE_DESCRIPTION("Amlogic Meson GX NVMEM driver"); MODULE_LICENSE("GPL v2"); -- 2.14.2 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/4] nvmem: add a driver for the Amlogic Meson6/Meson8/Meson8b SoCs [not found] ` <20171001125700.1520-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> 2017-10-01 12:56 ` [PATCH 1/4] dt-bindings: nvmem: Describe the Amlogic Meson6/Meson8/Meson8b efuse Martin Blumenstingl 2017-10-01 12:56 ` [PATCH 2/4] nvmem: meson-efuse: indicate that this driver is only for Meson GX SoCs Martin Blumenstingl @ 2017-10-01 12:56 ` Martin Blumenstingl [not found] ` <20171001125700.1520-4-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> 2017-10-01 12:57 ` [PATCH 4/4] ARM: dts: meson: add the efuse node Martin Blumenstingl 3 siblings, 1 reply; 10+ messages in thread From: Martin Blumenstingl @ 2017-10-01 12:56 UTC (permalink / raw) To: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, carlo-KA+7E9HrN00dnm+yROfE0A, khilman-rdvid1DuHRBWk0Htik3J/w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Cc: linux-I+IVW8TIWO2tmTQ+vhA3Yw, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Martin Blumenstingl This adds a driver to access the efuse on Amlogic Meson6, Meson8 and Meson8b SoCs. These SoCs are accessing the efuse IP block directly through the registers in the "secbus" region. This makes it different from the Meson GX efuse driver which uses the "secure monitor" firmware to access the efuse. The efuse on Meson6 can only read one byte at a time, while the efuse on Meson8 and Meson8b always reads 4 bytes at a time. The new driver supports both, but due to lack of hardware Meson6 support was not tested. Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> --- drivers/nvmem/Kconfig | 10 ++ drivers/nvmem/Makefile | 2 + drivers/nvmem/meson-mx-efuse.c | 272 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 284 insertions(+) create mode 100644 drivers/nvmem/meson-mx-efuse.c diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index a21a781f587d..11f1313a03de 100644 --- a/drivers/nvmem/Kconfig +++ b/drivers/nvmem/Kconfig @@ -144,4 +144,14 @@ config MESON_EFUSE This driver can also be built as a module. If so, the module will be called nvmem_meson_efuse. +config MESON_MX_EFUSE + tristate "Amlogic Meson6/Meson8/Meson8b eFuse Support" + depends on ARCH_MESON || COMPILE_TEST + help + This is a driver to retrieve specific values from the eFuse found on + the Amlogic Meson6, Meson8 and Meson8b SoCs. + + This driver can also be built as a module. If so, the module + will be called nvmem_meson_mx_efuse. + endif diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile index 173140658693..1c2edf869f26 100644 --- a/drivers/nvmem/Makefile +++ b/drivers/nvmem/Makefile @@ -30,3 +30,5 @@ obj-$(CONFIG_NVMEM_VF610_OCOTP) += nvmem-vf610-ocotp.o nvmem-vf610-ocotp-y := vf610-ocotp.o obj-$(CONFIG_MESON_EFUSE) += nvmem_meson_efuse.o nvmem_meson_efuse-y := meson-efuse.o +obj-$(CONFIG_MESON_MX_EFUSE) += nvmem_meson_mx_efuse.o +nvmem_meson_mx_efuse-y := meson-mx-efuse.o diff --git a/drivers/nvmem/meson-mx-efuse.c b/drivers/nvmem/meson-mx-efuse.c new file mode 100644 index 000000000000..b7c75f3eadce --- /dev/null +++ b/drivers/nvmem/meson-mx-efuse.c @@ -0,0 +1,272 @@ +/* + * Amlogic Meson6, Meson8 and Meson8b eFuse Driver + * + * Copyright (c) 2017 Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of version 2 of the GNU General Public License as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include <linux/bitfield.h> +#include <linux/bitops.h> +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/io.h> +#include <linux/iopoll.h> +#include <linux/module.h> +#include <linux/nvmem-provider.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/sizes.h> +#include <linux/slab.h> + +#define MESON_MX_EFUSE_CNTL1 0x04 +#define MESON_MX_EFUSE_CNTL1_PD_ENABLE BIT(27) +#define MESON_MX_EFUSE_CNTL1_AUTO_RD_BUSY BIT(26) +#define MESON_MX_EFUSE_CNTL1_AUTO_RD_START BIT(25) +#define MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE BIT(24) +#define MESON_MX_EFUSE_CNTL1_BYTE_WR_DATA GENMASK(23, 16) +#define MESON_MX_EFUSE_CNTL1_AUTO_WR_BUSY BIT(14) +#define MESON_MX_EFUSE_CNTL1_AUTO_WR_START BIT(13) +#define MESON_MX_EFUSE_CNTL1_AUTO_WR_ENABLE BIT(12) +#define MESON_MX_EFUSE_CNTL1_BYTE_ADDR_SET BIT(11) +#define MESON_MX_EFUSE_CNTL1_BYTE_ADDR_MASK GENMASK(10, 0) + +#define MESON_MX_EFUSE_CNTL2 0x08 + +#define MESON_MX_EFUSE_CNTL4 0x10 +#define MESON_MX_EFUSE_CNTL4_ENCRYPT_ENABLE BIT(10) + +struct meson_mx_efuse_platform_data { + const char *name; + unsigned int word_size; +}; + +struct meson_mx_efuse { + void __iomem *base; + struct clk *core_clk; + struct nvmem_device *nvmem; + struct nvmem_config config; +}; + +static void meson_mx_efuse_mask_bits(struct meson_mx_efuse *efuse, u32 reg, + u32 mask, u32 set) +{ + u32 data; + + data = readl(efuse->base + reg); + data &= ~mask; + data |= (set & mask); + + writel(data, efuse->base + reg); +} + +static int meson_mx_efuse_hw_enable(struct meson_mx_efuse *efuse) +{ + int err; + + err = clk_prepare_enable(efuse->core_clk); + if (err) + return err; + + /* power up the efuse */ + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, + MESON_MX_EFUSE_CNTL1_PD_ENABLE, 0); + + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL4, + MESON_MX_EFUSE_CNTL4_ENCRYPT_ENABLE, 0); + + return 0; +} + +static void meson_mx_efuse_hw_disable(struct meson_mx_efuse *efuse) +{ + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, + MESON_MX_EFUSE_CNTL1_PD_ENABLE, + MESON_MX_EFUSE_CNTL1_PD_ENABLE); + + clk_disable_unprepare(efuse->core_clk); +} + +static int meson_mx_efuse_read_addr(struct meson_mx_efuse *efuse, + unsigned int addr, u32 *value) +{ + int err; + u32 regval; + + /* write the address to read */ + regval = FIELD_PREP(MESON_MX_EFUSE_CNTL1_BYTE_ADDR_MASK, addr); + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, + MESON_MX_EFUSE_CNTL1_BYTE_ADDR_MASK, regval); + + /* inform the hardware that we changed the address */ + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, + MESON_MX_EFUSE_CNTL1_BYTE_ADDR_SET, + MESON_MX_EFUSE_CNTL1_BYTE_ADDR_SET); + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, + MESON_MX_EFUSE_CNTL1_BYTE_ADDR_SET, 0); + + /* start the read process */ + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, + MESON_MX_EFUSE_CNTL1_AUTO_RD_START, + MESON_MX_EFUSE_CNTL1_AUTO_RD_START); + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, + MESON_MX_EFUSE_CNTL1_AUTO_RD_START, 0); + + /* + * perform a dummy read to ensure that the HW has the RD_BUSY bit set + * when polling for the status below. + */ + readl(efuse->base + MESON_MX_EFUSE_CNTL1); + + err = readl_poll_timeout_atomic(efuse->base + MESON_MX_EFUSE_CNTL1, + regval, + (!(regval & MESON_MX_EFUSE_CNTL1_AUTO_RD_BUSY)), + 1, 1000); + if (err) { + dev_err(efuse->config.dev, + "Timeout while reading efuse address %u\n", addr); + return err; + } + + *value = readl(efuse->base + MESON_MX_EFUSE_CNTL2); + + return 0; +} + +static int meson_mx_efuse_read(void *context, unsigned int offset, + void *buf, size_t bytes) +{ + struct meson_mx_efuse *efuse = context; + u32 tmp; + int err, i, addr; + + err = meson_mx_efuse_hw_enable(efuse); + if (err) + return err; + + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, + MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE, + MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE); + + for (i = offset; i < offset + bytes; i += efuse->config.word_size) { + addr = i / efuse->config.word_size; + + err = meson_mx_efuse_read_addr(efuse, addr, &tmp); + if (err) + break; + + memcpy(buf + i, &tmp, efuse->config.word_size); + } + + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, + MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE, 0); + + meson_mx_efuse_hw_disable(efuse); + + return err; +} + +static const struct meson_mx_efuse_platform_data meson6_efuse_data = { + .name = "meson6-efuse", + .word_size = 1, +}; + +static const struct meson_mx_efuse_platform_data meson8_efuse_data = { + .name = "meson8-efuse", + .word_size = 4, +}; + +static const struct meson_mx_efuse_platform_data meson8b_efuse_data = { + .name = "meson8b-efuse", + .word_size = 4, +}; + +static const struct of_device_id meson_mx_efuse_match[] = { + { .compatible = "amlogic,meson6-efuse", .data = &meson6_efuse_data }, + { .compatible = "amlogic,meson8-efuse", .data = &meson8_efuse_data }, + { .compatible = "amlogic,meson8b-efuse", .data = &meson8b_efuse_data }, + { /* sentinel */ }, +}; +MODULE_DEVICE_TABLE(of, meson_mx_efuse_match); + +static int meson_mx_efuse_probe(struct platform_device *pdev) +{ + const struct meson_mx_efuse_platform_data *drvdata; + struct meson_mx_efuse *efuse; + struct resource *res; + + drvdata = of_device_get_match_data(&pdev->dev); + if (!drvdata) + return -EINVAL; + + efuse = devm_kzalloc(&pdev->dev, sizeof(*efuse), GFP_KERNEL); + if (!efuse) + return -ENOMEM; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + efuse->base = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(efuse->base)) + return PTR_ERR(efuse->base); + + efuse->config.name = devm_kstrdup(&pdev->dev, drvdata->name, + GFP_KERNEL); + efuse->config.owner = THIS_MODULE; + efuse->config.dev = &pdev->dev; + efuse->config.priv = efuse; + efuse->config.stride = drvdata->word_size; + efuse->config.word_size = drvdata->word_size; + efuse->config.size = SZ_512; + efuse->config.read_only = true; + efuse->config.reg_read = meson_mx_efuse_read; + + efuse->core_clk = devm_clk_get(&pdev->dev, "core"); + if (IS_ERR(efuse->core_clk)) { + dev_err(&pdev->dev, "Failed to get core clock\n"); + return PTR_ERR(efuse->core_clk); + } + + efuse->nvmem = nvmem_register(&efuse->config); + if (IS_ERR(efuse->nvmem)) { + clk_unprepare(efuse->core_clk); + return PTR_ERR(efuse->nvmem); + } + + platform_set_drvdata(pdev, efuse); + + return 0; +} + +static int meson_mx_efuse_remove(struct platform_device *pdev) +{ + struct meson_mx_efuse *efuse = platform_get_drvdata(pdev); + int err; + + err = nvmem_unregister(efuse->nvmem); + + clk_unprepare(efuse->core_clk); + + return err; +} + +static struct platform_driver meson_mx_efuse_driver = { + .probe = meson_mx_efuse_probe, + .remove = meson_mx_efuse_remove, + .driver = { + .name = "meson-mx-efuse", + .of_match_table = meson_mx_efuse_match, + }, +}; + +module_platform_driver(meson_mx_efuse_driver); + +MODULE_AUTHOR("Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>"); +MODULE_DESCRIPTION("Amlogic Meson MX eFuse NVMEM driver"); +MODULE_LICENSE("GPL v2"); -- 2.14.2 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <20171001125700.1520-4-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 3/4] nvmem: add a driver for the Amlogic Meson6/Meson8/Meson8b SoCs [not found] ` <20171001125700.1520-4-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> @ 2017-10-02 15:28 ` Srinivas Kandagatla [not found] ` <8f1f5a45-951c-7434-44d5-4ef2a61ec038-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2017-10-03 4:48 ` Chris Moore 1 sibling, 1 reply; 10+ messages in thread From: Srinivas Kandagatla @ 2017-10-02 15:28 UTC (permalink / raw) To: Martin Blumenstingl, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, carlo-KA+7E9HrN00dnm+yROfE0A, khilman-rdvid1DuHRBWk0Htik3J/w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Cc: linux-I+IVW8TIWO2tmTQ+vhA3Yw, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Minor comments!! On 01/10/17 13:56, Martin Blumenstingl wrote: > This adds a driver to access the efuse on Amlogic Meson6, Meson8 and > Meson8b SoCs. > These SoCs are accessing the efuse IP block directly through the > registers in the "secbus" region. This makes it different from the Meson > GX efuse driver which uses the "secure monitor" firmware to access the > efuse. > > The efuse on Meson6 can only read one byte at a time, while the efuse on > Meson8 and Meson8b always reads 4 bytes at a time. The new driver > supports both, but due to lack of hardware Meson6 support was not tested. > > Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> > --- > drivers/nvmem/Kconfig | 10 ++ > drivers/nvmem/Makefile | 2 + > drivers/nvmem/meson-mx-efuse.c | 272 +++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 284 insertions(+) > create mode 100644 drivers/nvmem/meson-mx-efuse.c > > diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig > + > + > +static void meson_mx_efuse_mask_bits(struct meson_mx_efuse *efuse, u32 reg, > + u32 mask, u32 set) > +{ > + u32 data; > + > + data = readl(efuse->base + reg); > + data &= ~mask; > + data |= (set & mask); > + > + writel(data, efuse->base + reg); > +} > + > +static int meson_mx_efuse_hw_enable(struct meson_mx_efuse *efuse) > +{ > + int err; > + > + err = clk_prepare_enable(efuse->core_clk); > + if (err) > + return err; > + > + /* power up the efuse */ > + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, > + MESON_MX_EFUSE_CNTL1_PD_ENABLE, 0); > + > + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL4, > + MESON_MX_EFUSE_CNTL4_ENCRYPT_ENABLE, 0); > + > + return 0; > +} > + > +static void meson_mx_efuse_hw_disable(struct meson_mx_efuse *efuse) > +{ > + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, > + MESON_MX_EFUSE_CNTL1_PD_ENABLE, > + MESON_MX_EFUSE_CNTL1_PD_ENABLE); > + > + clk_disable_unprepare(efuse->core_clk); > +} > + > +static int meson_mx_efuse_read_addr(struct meson_mx_efuse *efuse, > + unsigned int addr, u32 *value) > +{ > + int err; > + u32 regval; > + > + /* write the address to read */ > + regval = FIELD_PREP(MESON_MX_EFUSE_CNTL1_BYTE_ADDR_MASK, addr); > + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, > + MESON_MX_EFUSE_CNTL1_BYTE_ADDR_MASK, regval); > + > + /* inform the hardware that we changed the address */ > + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, > + MESON_MX_EFUSE_CNTL1_BYTE_ADDR_SET, > + MESON_MX_EFUSE_CNTL1_BYTE_ADDR_SET); > + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, > + MESON_MX_EFUSE_CNTL1_BYTE_ADDR_SET, 0); > + > + /* start the read process */ > + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, > + MESON_MX_EFUSE_CNTL1_AUTO_RD_START, > + MESON_MX_EFUSE_CNTL1_AUTO_RD_START); > + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, > + MESON_MX_EFUSE_CNTL1_AUTO_RD_START, 0); > + > + /* > + * perform a dummy read to ensure that the HW has the RD_BUSY bit set > + * when polling for the status below. > + */ > + readl(efuse->base + MESON_MX_EFUSE_CNTL1); > + > + err = readl_poll_timeout_atomic(efuse->base + MESON_MX_EFUSE_CNTL1, > + regval, > + (!(regval & MESON_MX_EFUSE_CNTL1_AUTO_RD_BUSY)), > + 1, 1000); > + if (err) { > + dev_err(efuse->config.dev, > + "Timeout while reading efuse address %u\n", addr); > + return err; > + } > + > + *value = readl(efuse->base + MESON_MX_EFUSE_CNTL2); > + > + return 0; > +} > + > +static int meson_mx_efuse_read(void *context, unsigned int offset, > + void *buf, size_t bytes) > +{ > + struct meson_mx_efuse *efuse = context; > + u32 tmp; > + int err, i, addr; > + > + err = meson_mx_efuse_hw_enable(efuse); > + if (err) > + return err; > + > + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, > + MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE, > + MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE); > + > + for (i = offset; i < offset + bytes; i += efuse->config.word_size) { > + addr = i / efuse->config.word_size; > + > + err = meson_mx_efuse_read_addr(efuse, addr, &tmp); > + if (err) > + break; > + > + memcpy(buf + i, &tmp, efuse->config.word_size); > + } > + > + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, > + MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE, 0); > + > + meson_mx_efuse_hw_disable(efuse); > + > + return err; > +} ... > + > +static int meson_mx_efuse_probe(struct platform_device *pdev) > +{ > + const struct meson_mx_efuse_platform_data *drvdata; > + struct meson_mx_efuse *efuse; > + struct resource *res; > + ... > + > + efuse->core_clk = devm_clk_get(&pdev->dev, "core"); > + if (IS_ERR(efuse->core_clk)) { > + dev_err(&pdev->dev, "Failed to get core clock\n"); > + return PTR_ERR(efuse->core_clk); > + } > + > + efuse->nvmem = nvmem_register(&efuse->config); > + if (IS_ERR(efuse->nvmem)) { > + clk_unprepare(efuse->core_clk); Do you need this??? > + return PTR_ERR(efuse->nvmem); > + } > + > + platform_set_drvdata(pdev, efuse); > + > + return 0; > +} > + > +static int meson_mx_efuse_remove(struct platform_device *pdev) > +{ > + struct meson_mx_efuse *efuse = platform_get_drvdata(pdev); > + int err; > + > + err = nvmem_unregister(efuse->nvmem); > + > + clk_unprepare(efuse->core_clk); Do you need this disable here? AFAIU, this driver would never leave the clk prepared unless we are in the middle of read. > + > + return err; > +} > + -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <8f1f5a45-951c-7434-44d5-4ef2a61ec038-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 3/4] nvmem: add a driver for the Amlogic Meson6/Meson8/Meson8b SoCs [not found] ` <8f1f5a45-951c-7434-44d5-4ef2a61ec038-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> @ 2017-10-02 18:38 ` Martin Blumenstingl 0 siblings, 0 replies; 10+ messages in thread From: Martin Blumenstingl @ 2017-10-02 18:38 UTC (permalink / raw) To: Srinivas Kandagatla Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, carlo-KA+7E9HrN00dnm+yROfE0A, khilman-rdvid1DuHRBWk0Htik3J/w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-I+IVW8TIWO2tmTQ+vhA3Yw, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r On Mon, Oct 2, 2017 at 5:28 PM, Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote: > Minor comments!! that was a VERY quick code review (and by this I mean the time between sending the patch and getting valuable feedback) - many thanks for this! > > On 01/10/17 13:56, Martin Blumenstingl wrote: >> >> This adds a driver to access the efuse on Amlogic Meson6, Meson8 and >> Meson8b SoCs. >> These SoCs are accessing the efuse IP block directly through the >> registers in the "secbus" region. This makes it different from the Meson >> GX efuse driver which uses the "secure monitor" firmware to access the >> efuse. >> >> The efuse on Meson6 can only read one byte at a time, while the efuse on >> Meson8 and Meson8b always reads 4 bytes at a time. The new driver >> supports both, but due to lack of hardware Meson6 support was not tested. >> >> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> >> --- >> drivers/nvmem/Kconfig | 10 ++ >> drivers/nvmem/Makefile | 2 + >> drivers/nvmem/meson-mx-efuse.c | 272 >> +++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 284 insertions(+) >> create mode 100644 drivers/nvmem/meson-mx-efuse.c >> >> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig >> + >> + >> +static void meson_mx_efuse_mask_bits(struct meson_mx_efuse *efuse, u32 >> reg, >> + u32 mask, u32 set) >> +{ >> + u32 data; >> + >> + data = readl(efuse->base + reg); >> + data &= ~mask; >> + data |= (set & mask); >> + >> + writel(data, efuse->base + reg); >> +} >> + >> +static int meson_mx_efuse_hw_enable(struct meson_mx_efuse *efuse) >> +{ >> + int err; >> + >> + err = clk_prepare_enable(efuse->core_clk); >> + if (err) >> + return err; >> + >> + /* power up the efuse */ >> + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, >> + MESON_MX_EFUSE_CNTL1_PD_ENABLE, 0); >> + >> + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL4, >> + MESON_MX_EFUSE_CNTL4_ENCRYPT_ENABLE, 0); >> + >> + return 0; >> +} >> + >> +static void meson_mx_efuse_hw_disable(struct meson_mx_efuse *efuse) >> +{ >> + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, >> + MESON_MX_EFUSE_CNTL1_PD_ENABLE, >> + MESON_MX_EFUSE_CNTL1_PD_ENABLE); >> + >> + clk_disable_unprepare(efuse->core_clk); >> +} >> + >> +static int meson_mx_efuse_read_addr(struct meson_mx_efuse *efuse, >> + unsigned int addr, u32 *value) >> +{ >> + int err; >> + u32 regval; >> + >> + /* write the address to read */ >> + regval = FIELD_PREP(MESON_MX_EFUSE_CNTL1_BYTE_ADDR_MASK, addr); >> + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, >> + MESON_MX_EFUSE_CNTL1_BYTE_ADDR_MASK, >> regval); >> + >> + /* inform the hardware that we changed the address */ >> + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, >> + MESON_MX_EFUSE_CNTL1_BYTE_ADDR_SET, >> + MESON_MX_EFUSE_CNTL1_BYTE_ADDR_SET); >> + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, >> + MESON_MX_EFUSE_CNTL1_BYTE_ADDR_SET, 0); >> + >> + /* start the read process */ >> + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, >> + MESON_MX_EFUSE_CNTL1_AUTO_RD_START, >> + MESON_MX_EFUSE_CNTL1_AUTO_RD_START); >> + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, >> + MESON_MX_EFUSE_CNTL1_AUTO_RD_START, 0); >> + >> + /* >> + * perform a dummy read to ensure that the HW has the RD_BUSY bit >> set >> + * when polling for the status below. >> + */ >> + readl(efuse->base + MESON_MX_EFUSE_CNTL1); >> + >> + err = readl_poll_timeout_atomic(efuse->base + >> MESON_MX_EFUSE_CNTL1, >> + regval, >> + (!(regval & MESON_MX_EFUSE_CNTL1_AUTO_RD_BUSY)), >> + 1, 1000); >> + if (err) { >> + dev_err(efuse->config.dev, >> + "Timeout while reading efuse address %u\n", addr); >> + return err; >> + } >> + >> + *value = readl(efuse->base + MESON_MX_EFUSE_CNTL2); >> + >> + return 0; >> +} >> + >> +static int meson_mx_efuse_read(void *context, unsigned int offset, >> + void *buf, size_t bytes) >> +{ >> + struct meson_mx_efuse *efuse = context; >> + u32 tmp; >> + int err, i, addr; >> + >> + err = meson_mx_efuse_hw_enable(efuse); >> + if (err) >> + return err; >> + >> + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, >> + MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE, >> + MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE); >> + >> + for (i = offset; i < offset + bytes; i += efuse->config.word_size) >> { >> + addr = i / efuse->config.word_size; >> + >> + err = meson_mx_efuse_read_addr(efuse, addr, &tmp); >> + if (err) >> + break; >> + >> + memcpy(buf + i, &tmp, efuse->config.word_size); >> + } >> + >> + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, >> + MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE, 0); >> + >> + meson_mx_efuse_hw_disable(efuse); >> + >> + return err; >> +} > > ... >> >> + >> +static int meson_mx_efuse_probe(struct platform_device *pdev) >> +{ >> + const struct meson_mx_efuse_platform_data *drvdata; >> + struct meson_mx_efuse *efuse; >> + struct resource *res; >> + > > > ... > >> + >> + efuse->core_clk = devm_clk_get(&pdev->dev, "core"); >> + if (IS_ERR(efuse->core_clk)) { >> + dev_err(&pdev->dev, "Failed to get core clock\n"); >> + return PTR_ERR(efuse->core_clk); >> + } >> + >> + efuse->nvmem = nvmem_register(&efuse->config); >> + if (IS_ERR(efuse->nvmem)) { >> + clk_unprepare(efuse->core_clk); > > > Do you need this??? oh, this is a left-over from an earlier version where I only enabled the clock in .probe (and disabled it again in .remove) however, this could break during suspend and it keeps the clock unnecessarily enabled when it's not needed > >> + return PTR_ERR(efuse->nvmem); >> + } >> + >> + platform_set_drvdata(pdev, efuse); >> + >> + return 0; >> +} >> + >> +static int meson_mx_efuse_remove(struct platform_device *pdev) >> +{ >> + struct meson_mx_efuse *efuse = platform_get_drvdata(pdev); >> + int err; >> + >> + err = nvmem_unregister(efuse->nvmem); >> + >> + clk_unprepare(efuse->core_clk); > > > Do you need this disable here? AFAIU, this driver would never leave the clk > prepared unless we are in the middle of read. yes, good catch here as well - this clk_unprepare call has to be removed > >> + >> + return err; >> +} >> + -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] nvmem: add a driver for the Amlogic Meson6/Meson8/Meson8b SoCs [not found] ` <20171001125700.1520-4-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> 2017-10-02 15:28 ` Srinivas Kandagatla @ 2017-10-03 4:48 ` Chris Moore [not found] ` <cd1e7f9c-5aff-8f4c-82b0-c58c2de4c253-GANU6spQydw@public.gmane.org> 1 sibling, 1 reply; 10+ messages in thread From: Chris Moore @ 2017-10-03 4:48 UTC (permalink / raw) To: Martin Blumenstingl, srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, carlo-KA+7E9HrN00dnm+yROfE0A, khilman-rdvid1DuHRBWk0Htik3J/w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Cc: linux-I+IVW8TIWO2tmTQ+vhA3Yw, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Hi, Sorry in advance if I am being extremely stupid here. Le 01/10/2017 à 14:56, Martin Blumenstingl a écrit : [snip] > +static int meson_mx_efuse_read(void *context, unsigned int offset, > + void *buf, size_t bytes) > +{ > + struct meson_mx_efuse *efuse = context; > + u32 tmp; > + int err, i, addr; > + > + err = meson_mx_efuse_hw_enable(efuse); > + if (err) > + return err; > + > + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, > + MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE, > + MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE); > + > + for (i = offset; i < offset + bytes; i += efuse->config.word_size) { > + addr = i / efuse->config.word_size; > + > + err = meson_mx_efuse_read_addr(efuse, addr, &tmp); > + if (err) > + break; > + > + memcpy(buf + i, &tmp, efuse->config.word_size); > + } This will not give the expected result if offset is not a multiple of efuse->config.word_size. This will cause buffer overflow if bytes is not a multiple of efuse->config.word_size. Shouldn't there at least be some sort of test on offset and bytes? Also this doesn't look endian-safe, but maybe it is. Old-timer remarks: - I try to avoid divisions where possible but I suppose they are very low-cost on recent hardware. - I don't like arithmetic on void pointers but I know it is supported by gcc (and possibly by recent C standards too). Sorry again for the noise if I am being stupid. Cheers, Chris -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <cd1e7f9c-5aff-8f4c-82b0-c58c2de4c253-GANU6spQydw@public.gmane.org>]
* Re: [PATCH 3/4] nvmem: add a driver for the Amlogic Meson6/Meson8/Meson8b SoCs [not found] ` <cd1e7f9c-5aff-8f4c-82b0-c58c2de4c253-GANU6spQydw@public.gmane.org> @ 2017-10-03 11:14 ` Martin Blumenstingl 2017-10-09 10:36 ` Srinivas Kandagatla 1 sibling, 0 replies; 10+ messages in thread From: Martin Blumenstingl @ 2017-10-03 11:14 UTC (permalink / raw) To: Chris Moore Cc: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, carlo-KA+7E9HrN00dnm+yROfE0A, khilman-rdvid1DuHRBWk0Htik3J/w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-I+IVW8TIWO2tmTQ+vhA3Yw, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Hi Chris, On Tue, Oct 3, 2017 at 6:48 AM, Chris Moore <moore-GANU6spQydw@public.gmane.org> wrote: > Hi, > > Sorry in advance if I am being extremely stupid here. > > Le 01/10/2017 à 14:56, Martin Blumenstingl a écrit : > > [snip] > >> +static int meson_mx_efuse_read(void *context, unsigned int offset, >> + void *buf, size_t bytes) >> +{ >> + struct meson_mx_efuse *efuse = context; >> + u32 tmp; >> + int err, i, addr; >> + >> + err = meson_mx_efuse_hw_enable(efuse); >> + if (err) >> + return err; >> + >> + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, >> + MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE, >> + MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE); >> + >> + for (i = offset; i < offset + bytes; i += efuse->config.word_size) >> { >> + addr = i / efuse->config.word_size; >> + >> + err = meson_mx_efuse_read_addr(efuse, addr, &tmp); >> + if (err) >> + break; >> + >> + memcpy(buf + i, &tmp, efuse->config.word_size); >> + } > > > This will not give the expected result if offset is not a multiple of > efuse->config.word_size. I had a look at mtk-efuse.c and bcm-ocotp.c and both are doing similar calculations thus I assumed that the nvmem core takes care of handling this > This will cause buffer overflow if bytes is not a multiple of > efuse->config.word_size. that would be very nasty indeed > Shouldn't there at least be some sort of test on offset and bytes? in the driver or in core? if possible these checks should be part of the core nvmem code to prevent code duplication across various drivers (and to prevent bugs if a specific driver forgets these checks) > Also this doesn't look endian-safe, but maybe it is. dumping the whole efuse via sysfs showed the same results as (the vendor command in Amlogic's) u-boot. > Old-timer remarks: > - I try to avoid divisions where possible but I suppose they are very > low-cost on recent hardware. > - I don't like arithmetic on void pointers but I know it is supported by gcc > (and possibly by recent C standards too). > > Sorry again for the noise if I am being stupid. your questions are reasonable this is my first nvmem driver, so there may be mistakes in the code - maybe Srinivas could also comment on your feedback also thank you for taking the time to review this! Regards, Martin -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] nvmem: add a driver for the Amlogic Meson6/Meson8/Meson8b SoCs [not found] ` <cd1e7f9c-5aff-8f4c-82b0-c58c2de4c253-GANU6spQydw@public.gmane.org> 2017-10-03 11:14 ` Martin Blumenstingl @ 2017-10-09 10:36 ` Srinivas Kandagatla 1 sibling, 0 replies; 10+ messages in thread From: Srinivas Kandagatla @ 2017-10-09 10:36 UTC (permalink / raw) To: Chris Moore, Martin Blumenstingl, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, carlo-KA+7E9HrN00dnm+yROfE0A, khilman-rdvid1DuHRBWk0Htik3J/w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Cc: linux-I+IVW8TIWO2tmTQ+vhA3Yw, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r On 03/10/17 05:48, Chris Moore wrote: > Hi, > > Sorry in advance if I am being extremely stupid here. > > Le 01/10/2017 à 14:56, Martin Blumenstingl a écrit : > > [snip] > >> +static int meson_mx_efuse_read(void *context, unsigned int offset, >> + void *buf, size_t bytes) >> +{ >> + struct meson_mx_efuse *efuse = context; >> + u32 tmp; >> + int err, i, addr; >> + >> + err = meson_mx_efuse_hw_enable(efuse); >> + if (err) >> + return err; >> + >> + meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1, >> + MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE, >> + MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE); >> + >> + for (i = offset; i < offset + bytes; i += efuse->config.word_size) { >> + addr = i / efuse->config.word_size; >> + >> + err = meson_mx_efuse_read_addr(efuse, addr, &tmp); >> + if (err) >> + break; >> + >> + memcpy(buf + i, &tmp, efuse->config.word_size); >> + } > > This will not give the expected result if offset is not a multiple of > efuse->config.word_size. Offset should be always multiple of stride which is enforced in nvmem core. nvmem_cell would return an -EINVAL if that is not the case. --srini > This will cause buffer overflow if bytes is not a multiple of > efuse->config.word_size. > Shouldn't there at least be some sort of test on offset and bytes? > > Also this doesn't look endian-safe, but maybe it is. > > Old-timer remarks: > - I try to avoid divisions where possible but I suppose they are very > low-cost on recent hardware. > - I don't like arithmetic on void pointers but I know it is supported by > gcc (and possibly by recent C standards too). > > Sorry again for the noise if I am being stupid. > > Cheers, > Chris > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/4] ARM: dts: meson: add the efuse node [not found] ` <20171001125700.1520-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> ` (2 preceding siblings ...) 2017-10-01 12:56 ` [PATCH 3/4] nvmem: add a driver for the Amlogic Meson6/Meson8/Meson8b SoCs Martin Blumenstingl @ 2017-10-01 12:57 ` Martin Blumenstingl 3 siblings, 0 replies; 10+ messages in thread From: Martin Blumenstingl @ 2017-10-01 12:57 UTC (permalink / raw) To: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, carlo-KA+7E9HrN00dnm+yROfE0A, khilman-rdvid1DuHRBWk0Htik3J/w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Cc: linux-I+IVW8TIWO2tmTQ+vhA3Yw, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Martin Blumenstingl Meson6, Meson8 and Meson8b use a similar IP block which has access to 512 bytes of efuse data. During SoC manufacturing some calibration settings for the CVBS connector and the internal temperature sensor are written to this efuse. On some boards it additionally stores for example the MAC addresses. The efuse is enabled on Meson8 and Meson8b but kept disabled on Meson6 since we do not have a clock driver there (which is required to read data from the efuse). Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> --- arch/arm/boot/dts/meson.dtsi | 13 +++++++++++++ arch/arm/boot/dts/meson6.dtsi | 3 +++ arch/arm/boot/dts/meson8.dtsi | 6 ++++++ arch/arm/boot/dts/meson8b.dtsi | 7 +++++++ 4 files changed, 29 insertions(+) diff --git a/arch/arm/boot/dts/meson.dtsi b/arch/arm/boot/dts/meson.dtsi index 163dac42bf2e..9c1b21be94f4 100644 --- a/arch/arm/boot/dts/meson.dtsi +++ b/arch/arm/boot/dts/meson.dtsi @@ -262,5 +262,18 @@ compatible = "amlogic,meson-mx-bootrom", "syscon"; reg = <0xd9040000 0x10000>; }; + + secbus: secbus@da000000 { + compatible = "simple-bus"; + reg = <0xda000000 0x6000>; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0xda000000 0x6000>; + + efuse: nvmem@0 { + compatible = "amlogic,meson6-efuse"; + reg = <0x0 0x2000>; + }; + }; }; }; /* end of / */ diff --git a/arch/arm/boot/dts/meson6.dtsi b/arch/arm/boot/dts/meson6.dtsi index ef281d290052..9b463211339f 100644 --- a/arch/arm/boot/dts/meson6.dtsi +++ b/arch/arm/boot/dts/meson6.dtsi @@ -84,6 +84,9 @@ }; }; /* end of / */ +&efuse { + status = "disabled"; +}; &uart_AO { clocks = <&xtal>, <&clk81>, <&clk81>; diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi index 2095a002ea8d..dad8b234c172 100644 --- a/arch/arm/boot/dts/meson8.dtsi +++ b/arch/arm/boot/dts/meson8.dtsi @@ -275,6 +275,12 @@ }; }; +&efuse { + compatible = "amlogic,meson8-efuse"; + clocks = <&clkc CLKID_EFUSE>; + clock-names = "core"; +}; + ðmac { clocks = <&clkc CLKID_ETH>; clock-names = "stmmaceth"; diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi index 111e01809ad7..4f8fc01ee0f0 100644 --- a/arch/arm/boot/dts/meson8b.dtsi +++ b/arch/arm/boot/dts/meson8b.dtsi @@ -183,6 +183,13 @@ }; }; + +&efuse { + compatible = "amlogic,meson8b-efuse"; + clocks = <&clkc CLKID_EFUSE>; + clock-names = "core"; +}; + ðmac { clocks = <&clkc CLKID_ETH>; clock-names = "stmmaceth"; -- 2.14.2 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-10-09 10:36 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-10-01 12:56 [PATCH 0/4] Amlogic Meson6/Meson8/Meson8b efuse support Martin Blumenstingl [not found] ` <20171001125700.1520-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> 2017-10-01 12:56 ` [PATCH 1/4] dt-bindings: nvmem: Describe the Amlogic Meson6/Meson8/Meson8b efuse Martin Blumenstingl 2017-10-01 12:56 ` [PATCH 2/4] nvmem: meson-efuse: indicate that this driver is only for Meson GX SoCs Martin Blumenstingl 2017-10-01 12:56 ` [PATCH 3/4] nvmem: add a driver for the Amlogic Meson6/Meson8/Meson8b SoCs Martin Blumenstingl [not found] ` <20171001125700.1520-4-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> 2017-10-02 15:28 ` Srinivas Kandagatla [not found] ` <8f1f5a45-951c-7434-44d5-4ef2a61ec038-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2017-10-02 18:38 ` Martin Blumenstingl 2017-10-03 4:48 ` Chris Moore [not found] ` <cd1e7f9c-5aff-8f4c-82b0-c58c2de4c253-GANU6spQydw@public.gmane.org> 2017-10-03 11:14 ` Martin Blumenstingl 2017-10-09 10:36 ` Srinivas Kandagatla 2017-10-01 12:57 ` [PATCH 4/4] ARM: dts: meson: add the efuse node Martin Blumenstingl
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).