[PATCH v1 0/3] add UniPhier efuse support

[PATCH v1 0/3] add UniPhier efuse support

[Keiji Hayashibara]

This series adds support for eFuse implemented on UniPhier LD20 SoCs.
The eFuse device is under soc-glue and this register implements as read only.

Keiji Hayashibara (3):
  dt-bindings: nvmem: add description for UniPhier eFuse
  nvmem: uniphier: add UniPhier eFuse driver
  arm64: dts: uniphier: add efuse node for LD20

 .../devicetree/bindings/nvmem/uniphier-efuse.txt   | 45 ++++++++++
 arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi   | 14 ++++
 arch/arm64/configs/defconfig                       |  1 +
 drivers/nvmem/Kconfig                              | 11 +++
 drivers/nvmem/Makefile                             |  2 +
 drivers/nvmem/uniphier-efuse.c                     | 98 ++++++++++++++++++++++
 6 files changed, 171 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt
 create mode 100644 drivers/nvmem/uniphier-efuse.c

-- 
2.7.4

[PATCH v1 1/3] dt-bindings: nvmem: add description for UniPhier eFuse

[Keiji Hayashibara]

Add uniphier-efuse dt-bindings documentation.

Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
---
 .../devicetree/bindings/nvmem/uniphier-efuse.txt   | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt

diff --git a/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt b/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt
new file mode 100644
index 0000000..09024a2
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt
@@ -0,0 +1,45 @@
+= UniPhier eFuse device tree bindings =
+
+This UniPhier eFuse must be under soc-glue.
+
+Required properties:
+- compatible: should be "socionext,uniphier-efuse"
+- reg: should contain the register base and length
+
+= Data cells =
+Are child nodes of efuse, bindings of which as described in
+bindings/nvmem/nvmem.txt
+
+Example:
+
+	soc-glue@5f900000 {
+		compatible = "socionext,uniphier-ld20-soc-glue-debug",
+			     "simple-mfd";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0x0 0x5f900000 0x2000>;
+
+		efuse {
+			compatible = "socionext,uniphier-efuse",
+				     "syscon";
+			reg = <0x100 0xf00>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			/* Data cells */
+			usb_mon: usb_mon {
+				reg = <0x154 0xc>;
+			};
+		};
+	};
+
+= Data consumers =
+Are device nodes which consume nvmem data cells.
+
+Example:
+
+	usb {
+		...
+		nvmem-cells = <&usb_mon>;
+		nvmem-cell-names = "usb_mon";
+	}
-- 
2.7.4

[PATCH v1 2/3] nvmem: uniphier: add UniPhier eFuse driver

[Keiji Hayashibara]

Add eFuse driver for Socionext UniPhier series SoC.
Note that eFuse device is under soc-glue and this register
implements as read only.

Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
---
 arch/arm64/configs/defconfig   |  1 +
 drivers/nvmem/Kconfig          | 11 +++++
 drivers/nvmem/Makefile         |  2 +
 drivers/nvmem/uniphier-efuse.c | 98 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 112 insertions(+)
 create mode 100644 drivers/nvmem/uniphier-efuse.c

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 6c7d147..5c38b4a 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -514,6 +514,7 @@ CONFIG_PHY_XGENE=y
 CONFIG_PHY_TEGRA_XUSB=y
 CONFIG_QCOM_L2_PMU=y
 CONFIG_QCOM_L3_PMU=y
+CONFIG_UNIPHIER_EFUSE=y
 CONFIG_ARM_SCPI_PROTOCOL=y
 CONFIG_RASPBERRYPI_FIRMWARE=y
 CONFIG_EFI_CAPSULE_LOADER=y
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index 101ced4..aaeaebe 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -123,6 +123,17 @@ config NVMEM_SUNXI_SID
 	  This driver can also be built as a module. If so, the module
 	  will be called nvmem_sunxi_sid.
 
+config UNIPHIER_EFUSE
+	tristate "UniPhier SoCs eFuse support"
+	depends on ARCH_UNIPHIER || COMPILE_TEST
+	depends on OF && MFD_SYSCON
+	help
+	  This is a simple drive to dump specified values of UniPhier SoC
+	  from eFuse.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called nvmem_uniphier-efuse.
+
 config NVMEM_VF610_OCOTP
 	tristate "VF610 SoC OCOTP support"
 	depends on SOC_VF610 || COMPILE_TEST
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
index 1731406..f5277c3 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -26,6 +26,8 @@ obj-$(CONFIG_ROCKCHIP_EFUSE)	+= nvmem_rockchip_efuse.o
 nvmem_rockchip_efuse-y		:= rockchip-efuse.o
 obj-$(CONFIG_NVMEM_SUNXI_SID)	+= nvmem_sunxi_sid.o
 nvmem_sunxi_sid-y		:= sunxi_sid.o
+obj-$(CONFIG_UNIPHIER_EFUSE)	+= nvmem_uniphier-efuse.o
+nvmem_uniphier-efuse-y		:= uniphier-efuse.o
 obj-$(CONFIG_NVMEM_VF610_OCOTP)	+= nvmem-vf610-ocotp.o
 nvmem-vf610-ocotp-y		:= vf610-ocotp.o
 obj-$(CONFIG_MESON_EFUSE)	+= nvmem_meson_efuse.o
diff --git a/drivers/nvmem/uniphier-efuse.c b/drivers/nvmem/uniphier-efuse.c
new file mode 100644
index 0000000..d553fa3
--- /dev/null
+++ b/drivers/nvmem/uniphier-efuse.c
@@ -0,0 +1,98 @@
+/*
+ * UniPhier eFuse driver
+ *
+ * Copyright (C) 2017 Socionext Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 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/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/nvmem-provider.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define UNIPHIER_EFUSE_WORD_SIZE	4
+#define UNIPHIER_EFUSE_STRIDE		4
+
+static int uniphier_reg_read(void *context,
+			     unsigned int offset, void *val, size_t bytes)
+{
+	int words = bytes / UNIPHIER_EFUSE_WORD_SIZE;
+
+	return regmap_bulk_read(context, offset, val, words);
+}
+
+static struct nvmem_config econfig = {
+	.name = "uniphier-efuse",
+	.owner = THIS_MODULE,
+	.read_only = true,
+	.reg_read = uniphier_reg_read,
+	.word_size = UNIPHIER_EFUSE_WORD_SIZE,
+	.stride = UNIPHIER_EFUSE_STRIDE,
+};
+
+static int uniphier_efuse_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct nvmem_device *nvmem;
+	struct regmap *regmap;
+	struct resource res;
+	int ret;
+
+	ret = of_address_to_resource(dev->of_node, 0, &res);
+	if (ret)
+		return ret;
+
+	regmap = syscon_node_to_regmap(dev->of_node);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	econfig.size = resource_size(&res);
+	econfig.priv = regmap;
+	econfig.dev = dev;
+
+	nvmem = nvmem_register(&econfig);
+	if (IS_ERR(nvmem))
+		return PTR_ERR(nvmem);
+
+	platform_set_drvdata(pdev, nvmem);
+
+	return 0;
+}
+
+static int uniphier_efuse_remove(struct platform_device *pdev)
+{
+	struct nvmem_device *nvmem = platform_get_drvdata(pdev);
+
+	return nvmem_unregister(nvmem);
+}
+
+static const struct of_device_id uniphier_efuse_of_match[] = {
+	{ .compatible = "socionext,uniphier-efuse",},
+	{/* sentinel */},
+};
+MODULE_DEVICE_TABLE(of, uniphier_efuse_of_match);
+
+static struct platform_driver uniphier_efuse_driver = {
+	.probe = uniphier_efuse_probe,
+	.remove = uniphier_efuse_remove,
+	.driver = {
+		.name = "uniphier-efuse",
+		.of_match_table = uniphier_efuse_of_match,
+	},
+};
+module_platform_driver(uniphier_efuse_driver);
+
+MODULE_AUTHOR("Keiji Hayashibara <hayashibara.keiji@socionext.com>");
+MODULE_DESCRIPTION("UniPhier eFuse driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH v1 3/3] arm64: dts: uniphier: add efuse node for LD20

[Keiji Hayashibara]

Add efuse node for UniPhier LD20 SoC.
This efuse node is included in soc-glue.

Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
---
 arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi b/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
index de1e753..8a1f5af 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
+++ b/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
@@ -365,6 +365,20 @@
 			};
 		};
 
+		soc-glue@5f900000 {
+			compatible = "socionext,uniphier-ld20-soc-glue-debug",
+				     "simple-mfd";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges = <0x0 0x5f900000 0x2000>;
+
+			efuse {
+				compatible = "socionext,uniphier-efuse",
+					     "syscon";
+				reg = <0x100 0xf00>;
+			};
+		};
+
 		gic: interrupt-controller@5fe00000 {
 			compatible = "arm,gic-v3";
 			reg = <0x5fe00000 0x10000>,	/* GICD */
-- 
2.7.4

Re: [PATCH v1 1/3] dt-bindings: nvmem: add description for UniPhier eFuse

[Masahiro Yamada]

2017-09-01 8:20 GMT+09:00 Keiji Hayashibara <hayashibara.keiji@socionext.com>:
> Add uniphier-efuse dt-bindings documentation.
>
> Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
> ---
>  .../devicetree/bindings/nvmem/uniphier-efuse.txt   | 45 ++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt
>
> diff --git a/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt b/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt
> new file mode 100644
> index 0000000..09024a2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt
> @@ -0,0 +1,45 @@
> += UniPhier eFuse device tree bindings =
> +
> +This UniPhier eFuse must be under soc-glue.
> +
> +Required properties:
> +- compatible: should be "socionext,uniphier-efuse"
> +- reg: should contain the register base and length
> +
> += Data cells =
> +Are child nodes of efuse, bindings of which as described in
> +bindings/nvmem/nvmem.txt
> +
> +Example:
> +
> +       soc-glue@5f900000 {
> +               compatible = "socionext,uniphier-ld20-soc-glue-debug",
> +                            "simple-mfd";
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               ranges = <0x0 0x5f900000 0x2000>;


IMHO, I think an empty "ranges;" will clarify the code,
but it is up to your taste.


> +
> +               efuse {
> +                       compatible = "socionext,uniphier-efuse",
> +                                    "syscon";


You are adding a dedicated driver for "socionext,uniphier-efuse".

Then, "syscon" as well?




> +                       reg = <0x100 0xf00>;


Not so many efuse registers exist on the SoC.

reg = <0x100 0x200>; will be enough.


Or if you want to be strict to the hw spec, you can write as follows:

        soc-glue@5f900000 {
               compatible = "socionext,uniphier-ld20-soc-glue-debug";
                            "simple-mfd";
               #address-cells = <1>;
               #size-cells = <1>;
               ranges = <0x0 0x5f900000 0x2000>;

               efuse@100 {
                           compatible = "socionext,uniphier-efuse";
                           reg = <0x100 0x28>;
               };

               efuse@200 {
                           compatible = "socionext,uniphier-efuse";
                           reg = <0x200 0x68>;
               };
       };




> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +
> +                       /* Data cells */
> +                       usb_mon: usb_mon {
> +                               reg = <0x154 0xc>;
> +                       };


This <0x154 0xc> represents 0x5f900254 in CPU address view.
(0x5f900000 + 0x100 + 0x154)

So many ranges conversion, and how error-prone..





> +       };
> +
> += Data consumers =
> +Are device nodes which consume nvmem data cells.
> +
> +Example:
> +
> +       usb {
> +               ...
> +               nvmem-cells = <&usb_mon>;
> +               nvmem-cell-names = "usb_mon";
> +       }
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v1 2/3] nvmem: uniphier: add UniPhier eFuse driver

[Masahiro Yamada]

2017-09-01 8:20 GMT+09:00 Keiji Hayashibara <hayashibara.keiji@socionext.com>:
> Add eFuse driver for Socionext UniPhier series SoC.
> Note that eFuse device is under soc-glue and this register
> implements as read only.
>
> Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
> ---
>  arch/arm64/configs/defconfig   |  1 +
>  drivers/nvmem/Kconfig          | 11 +++++
>  drivers/nvmem/Makefile         |  2 +
>  drivers/nvmem/uniphier-efuse.c | 98 ++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 112 insertions(+)
>  create mode 100644 drivers/nvmem/uniphier-efuse.c
>
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 6c7d147..5c38b4a 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -514,6 +514,7 @@ CONFIG_PHY_XGENE=y
>  CONFIG_PHY_TEGRA_XUSB=y
>  CONFIG_QCOM_L2_PMU=y
>  CONFIG_QCOM_L3_PMU=y
> +CONFIG_UNIPHIER_EFUSE=y
>  CONFIG_ARM_SCPI_PROTOCOL=y
>  CONFIG_RASPBERRYPI_FIRMWARE=y
>  CONFIG_EFI_CAPSULE_LOADER=y


You need to send this separately
(after this driver is accepted)



> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index 101ced4..aaeaebe 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -123,6 +123,17 @@ config NVMEM_SUNXI_SID
>           This driver can also be built as a module. If so, the module
>           will be called nvmem_sunxi_sid.
>
> +config UNIPHIER_EFUSE
> +       tristate "UniPhier SoCs eFuse support"
> +       depends on ARCH_UNIPHIER || COMPILE_TEST
> +       depends on OF && MFD_SYSCON
> +       help
> +         This is a simple drive to dump specified values of UniPhier SoC
> +         from eFuse.


s/drive/driver/


I see the same typo in ROCKCHIP_EFUSE.

You copied it verbatim.




> +         This driver can also be built as a module. If so, the module
> +         will be called nvmem_uniphier-efuse.


I do not like a mixture of '_' and '-' though...





>  config NVMEM_VF610_OCOTP
>         tristate "VF610 SoC OCOTP support"
>         depends on SOC_VF610 || COMPILE_TEST
> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> index 1731406..f5277c3 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -26,6 +26,8 @@ obj-$(CONFIG_ROCKCHIP_EFUSE)  += nvmem_rockchip_efuse.o
>  nvmem_rockchip_efuse-y         := rockchip-efuse.o
>  obj-$(CONFIG_NVMEM_SUNXI_SID)  += nvmem_sunxi_sid.o
>  nvmem_sunxi_sid-y              := sunxi_sid.o
> +obj-$(CONFIG_UNIPHIER_EFUSE)   += nvmem_uniphier-efuse.o
> +nvmem_uniphier-efuse-y         := uniphier-efuse.o
>  obj-$(CONFIG_NVMEM_VF610_OCOTP)        += nvmem-vf610-ocotp.o
>  nvmem-vf610-ocotp-y            := vf610-ocotp.o
>  obj-$(CONFIG_MESON_EFUSE)      += nvmem_meson_efuse.o
> diff --git a/drivers/nvmem/uniphier-efuse.c b/drivers/nvmem/uniphier-efuse.c
> new file mode 100644
> index 0000000..d553fa3
> --- /dev/null
> +++ b/drivers/nvmem/uniphier-efuse.c
> @@ -0,0 +1,98 @@
> +/*
> + * UniPhier eFuse driver
> + *
> + * Copyright (C) 2017 Socionext Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 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/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-provider.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define UNIPHIER_EFUSE_WORD_SIZE       4
> +#define UNIPHIER_EFUSE_STRIDE          4
> +
> +static int uniphier_reg_read(void *context,
> +                            unsigned int offset, void *val, size_t bytes)
> +{
> +       int words = bytes / UNIPHIER_EFUSE_WORD_SIZE;
> +
> +       return regmap_bulk_read(context, offset, val, words);
> +}
> +
> +static struct nvmem_config econfig = {
> +       .name = "uniphier-efuse",
> +       .owner = THIS_MODULE,
> +       .read_only = true,
> +       .reg_read = uniphier_reg_read,
> +       .word_size = UNIPHIER_EFUSE_WORD_SIZE,
> +       .stride = UNIPHIER_EFUSE_STRIDE,
> +};



"struct nvmem_config" exists for the purpose of containing
driver-specific parameters.
Do you still want to use
UNIPHIER_EFUSE_WORD_SIZE / UNIPHIER_EFUSE_STRIDE macros here?




static struct nvmem_config econfig = {
       .name = "uniphier-efuse",
       .owner = THIS_MODULE,
       .read_only = true,
       .reg_read = uniphier_reg_read,
       .word_size = 4,
       .stride = 4,
};

looks nicer in my taste.



BTW, is there really no possibility
where the unphier_efuse_probe() is run concurrently?

Many other drivers pass statically allocated memory
to nvmem_register() like this...




> +static int uniphier_efuse_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct nvmem_device *nvmem;
> +       struct regmap *regmap;
> +       struct resource res;
> +       int ret;
> +
> +       ret = of_address_to_resource(dev->of_node, 0, &res);
> +       if (ret)
> +               return ret;
> +
> +       regmap = syscon_node_to_regmap(dev->of_node);
> +       if (IS_ERR(regmap))
> +               return PTR_ERR(regmap);


In my understanding,
syscon_node_to_regmap() is generally useful
to get regmap of another node,
i.e. it is often used together with of_get_parent / of_parse_phandle etc.

You can use devm_regmap_init_mmio()
to init regmap for itself.


BTW, what is your motivation of using regmap?


I think what you want to do is
the same as mtk-efuse.c, right?
(and qfprom.c is similar, the difference is just register width)






> +       econfig.size = resource_size(&res);
> +       econfig.priv = regmap;
> +       econfig.dev = dev;
> +
> +       nvmem = nvmem_register(&econfig);
> +       if (IS_ERR(nvmem))
> +               return PTR_ERR(nvmem);
> +
> +       platform_set_drvdata(pdev, nvmem);
> +
> +       return 0;
> +}
> +
> +static int uniphier_efuse_remove(struct platform_device *pdev)
> +{
> +       struct nvmem_device *nvmem = platform_get_drvdata(pdev);
> +
> +       return nvmem_unregister(nvmem);
> +}
> +
> +static const struct of_device_id uniphier_efuse_of_match[] = {
> +       { .compatible = "socionext,uniphier-efuse",},
> +       {/* sentinel */},
> +};
> +MODULE_DEVICE_TABLE(of, uniphier_efuse_of_match);
> +
> +static struct platform_driver uniphier_efuse_driver = {
> +       .probe = uniphier_efuse_probe,
> +       .remove = uniphier_efuse_remove,
> +       .driver = {
> +               .name = "uniphier-efuse",
> +               .of_match_table = uniphier_efuse_of_match,
> +       },
> +};
> +module_platform_driver(uniphier_efuse_driver);
> +
> +MODULE_AUTHOR("Keiji Hayashibara <hayashibara.keiji@socionext.com>");
> +MODULE_DESCRIPTION("UniPhier eFuse driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>



-- 
Best Regards
Masahiro Yamada

RE: [PATCH v1 1/3] dt-bindings: nvmem: add description for UniPhier eFuse

[Keiji Hayashibara]

Hello Yamada-san,

Thank you for your comment.

> From: Masahiro Yamada [mailto:yamada.masahiro@socionext.com]
> Sent: Monday, September 4, 2017 9:56 PM
> 
> 2017-09-01 8:20 GMT+09:00 Keiji Hayashibara
> <hayashibara.keiji@socionext.com>:
> > Add uniphier-efuse dt-bindings documentation.
> >
> > Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
> > ---
> >  .../devicetree/bindings/nvmem/uniphier-efuse.txt   | 45
> ++++++++++++++++++++++
> >  1 file changed, 45 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt
> > b/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt
> > new file mode 100644
> > index 0000000..09024a2
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt
> > @@ -0,0 +1,45 @@
> > += UniPhier eFuse device tree bindings =
> > +
> > +This UniPhier eFuse must be under soc-glue.
> > +
> > +Required properties:
> > +- compatible: should be "socionext,uniphier-efuse"
> > +- reg: should contain the register base and length
> > +
> > += Data cells =
> > +Are child nodes of efuse, bindings of which as described in
> > +bindings/nvmem/nvmem.txt
> > +
> > +Example:
> > +
> > +       soc-glue@5f900000 {
> > +               compatible = "socionext,uniphier-ld20-soc-glue-debug",
> > +                            "simple-mfd";
> > +               #address-cells = <1>;
> > +               #size-cells = <1>;
> > +               ranges = <0x0 0x5f900000 0x2000>;
> 
> 
> IMHO, I think an empty "ranges;" will clarify the code, but it is up to
> your taste.
> 
> 
> > +
> > +               efuse {
> > +                       compatible = "socionext,uniphier-efuse",
> > +                                    "syscon";
> 
> 
> You are adding a dedicated driver for "socionext,uniphier-efuse".
> 
> Then, "syscon" as well?
> 

Since I was using the syscon interface to implement the driver,
I specified "syscon". It's interface is syscon_node_to_regmap().

I will rethink this in v2.

> 
> 
> > +                       reg = <0x100 0xf00>;
> 
> 
> Not so many efuse registers exist on the SoC.
> 
> reg = <0x100 0x200>; will be enough.
> 
> 
> Or if you want to be strict to the hw spec, you can write as follows:
> 
>         soc-glue@5f900000 {
>                compatible = "socionext,uniphier-ld20-soc-glue-debug";
>                             "simple-mfd";
>                #address-cells = <1>;
>                #size-cells = <1>;
>                ranges = <0x0 0x5f900000 0x2000>;
> 
>                efuse@100 {
>                            compatible = "socionext,uniphier-efuse";
>                            reg = <0x100 0x28>;
>                };
> 
>                efuse@200 {
>                            compatible = "socionext,uniphier-efuse";
>                            reg = <0x200 0x68>;
>                };
>        };
> 
> 
> 
> 
> > +                       #address-cells = <1>;
> > +                       #size-cells = <1>;
> > +
> > +                       /* Data cells */
> > +                       usb_mon: usb_mon {
> > +                               reg = <0x154 0xc>;
> > +                       };
> 
> 
> This <0x154 0xc> represents 0x5f900254 in CPU address view.
> (0x5f900000 + 0x100 + 0x154)
> 
> So many ranges conversion, and how error-prone..
> 

Yes, indeed...
I will modify as below.

        soc-glue@5f900000 {
                compatible = "socionext,uniphier-ld20-soc-glue-debug",
                             "simple-mfd";
                #address-cells = <1>;
                #size-cells = <1>;
                ranges;

                efuse@5f900100 {
                        compatible = "socionext,uniphier-efuse";
                        reg = <0x5f900100 0x28>;
                };

                efuse@5f900200 {
                        compatible = "socionext,uniphier-efuse";
                        reg = <0x5f900200 0x68>;
                };
        };


> 
> 
> 
> > +       };
> > +
> > += Data consumers =
> > +Are device nodes which consume nvmem data cells.
> > +
> > +Example:
> > +
> > +       usb {
> > +               ...
> > +               nvmem-cells = <&usb_mon>;
> > +               nvmem-cell-names = "usb_mon";
> > +       }
> > --
> > 2.7.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe devicetree"
> > in the body of a message to majordomo@vger.kernel.org More majordomo
> > info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 
> --
> Best Regards
> Masahiro Yamada

Best Regards,
Keiji Hayashibara

RE: [PATCH v1 2/3] nvmem: uniphier: add UniPhier eFuse driver

[Keiji Hayashibara]

Hello Yamada-san,

Thank you for your comment.

> From: Masahiro Yamada [mailto:yamada.masahiro@socionext.com]
> Sent: Monday, September 4, 2017 9:58 PM
> 
> 2017-09-01 8:20 GMT+09:00 Keiji Hayashibara
> <hayashibara.keiji@socionext.com>:
> > Add eFuse driver for Socionext UniPhier series SoC.
> > Note that eFuse device is under soc-glue and this register implements
> > as read only.
> >
> > Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
> > ---
> >  arch/arm64/configs/defconfig   |  1 +
> >  drivers/nvmem/Kconfig          | 11 +++++
> >  drivers/nvmem/Makefile         |  2 +
> >  drivers/nvmem/uniphier-efuse.c | 98
> > ++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 112 insertions(+)
> >  create mode 100644 drivers/nvmem/uniphier-efuse.c
> >
> > diff --git a/arch/arm64/configs/defconfig
> > b/arch/arm64/configs/defconfig index 6c7d147..5c38b4a 100644
> > --- a/arch/arm64/configs/defconfig
> > +++ b/arch/arm64/configs/defconfig
> > @@ -514,6 +514,7 @@ CONFIG_PHY_XGENE=y  CONFIG_PHY_TEGRA_XUSB=y
> > CONFIG_QCOM_L2_PMU=y  CONFIG_QCOM_L3_PMU=y
> > +CONFIG_UNIPHIER_EFUSE=y
> >  CONFIG_ARM_SCPI_PROTOCOL=y
> >  CONFIG_RASPBERRYPI_FIRMWARE=y
> >  CONFIG_EFI_CAPSULE_LOADER=y
> 
> 
> You need to send this separately
> (after this driver is accepted)

I see.
Next I will post this separately.

> 
> > diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index
> > 101ced4..aaeaebe 100644
> > --- a/drivers/nvmem/Kconfig
> > +++ b/drivers/nvmem/Kconfig
> > @@ -123,6 +123,17 @@ config NVMEM_SUNXI_SID
> >           This driver can also be built as a module. If so, the module
> >           will be called nvmem_sunxi_sid.
> >
> > +config UNIPHIER_EFUSE
> > +       tristate "UniPhier SoCs eFuse support"
> > +       depends on ARCH_UNIPHIER || COMPILE_TEST
> > +       depends on OF && MFD_SYSCON
> > +       help
> > +         This is a simple drive to dump specified values of UniPhier
> SoC
> > +         from eFuse.
> 
> 
> s/drive/driver/
> 
> 
> I see the same typo in ROCKCHIP_EFUSE.
> 
> You copied it verbatim.
> 

I will fix it...

> 
> 
> > +         This driver can also be built as a module. If so, the module
> > +         will be called nvmem_uniphier-efuse.
> 
> 
> I do not like a mixture of '_' and '-' though...
> 

I will fix it.
> 
> 
> >  config NVMEM_VF610_OCOTP
> >         tristate "VF610 SoC OCOTP support"
> >         depends on SOC_VF610 || COMPILE_TEST diff --git
> > a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile index
> > 1731406..f5277c3 100644
> > --- a/drivers/nvmem/Makefile
> > +++ b/drivers/nvmem/Makefile
> > @@ -26,6 +26,8 @@ obj-$(CONFIG_ROCKCHIP_EFUSE)  +=
> nvmem_rockchip_efuse.o
> >  nvmem_rockchip_efuse-y         := rockchip-efuse.o
> >  obj-$(CONFIG_NVMEM_SUNXI_SID)  += nvmem_sunxi_sid.o
> >  nvmem_sunxi_sid-y              := sunxi_sid.o
> > +obj-$(CONFIG_UNIPHIER_EFUSE)   += nvmem_uniphier-efuse.o
> > +nvmem_uniphier-efuse-y         := uniphier-efuse.o
> >  obj-$(CONFIG_NVMEM_VF610_OCOTP)        += nvmem-vf610-ocotp.o
> >  nvmem-vf610-ocotp-y            := vf610-ocotp.o
> >  obj-$(CONFIG_MESON_EFUSE)      += nvmem_meson_efuse.o
> > diff --git a/drivers/nvmem/uniphier-efuse.c
> > b/drivers/nvmem/uniphier-efuse.c new file mode 100644 index
> > 0000000..d553fa3
> > --- /dev/null
> > +++ b/drivers/nvmem/uniphier-efuse.c
> > @@ -0,0 +1,98 @@
> > +/*
> > + * UniPhier eFuse driver
> > + *
> > + * Copyright (C) 2017 Socionext Inc.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > +modify
> > + * it under the terms of the GNU General Public License version 2 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/mfd/syscon.h>
> > +#include <linux/module.h>
> > +#include <linux/nvmem-provider.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +
> > +#define UNIPHIER_EFUSE_WORD_SIZE       4
> > +#define UNIPHIER_EFUSE_STRIDE          4
> > +
> > +static int uniphier_reg_read(void *context,
> > +                            unsigned int offset, void *val, size_t
> > +bytes) {
> > +       int words = bytes / UNIPHIER_EFUSE_WORD_SIZE;
> > +
> > +       return regmap_bulk_read(context, offset, val, words); }
> > +
> > +static struct nvmem_config econfig = {
> > +       .name = "uniphier-efuse",
> > +       .owner = THIS_MODULE,
> > +       .read_only = true,
> > +       .reg_read = uniphier_reg_read,
> > +       .word_size = UNIPHIER_EFUSE_WORD_SIZE,
> > +       .stride = UNIPHIER_EFUSE_STRIDE, };
> 
> 
> 
> "struct nvmem_config" exists for the purpose of containing driver-specific
> parameters.
> Do you still want to use
> UNIPHIER_EFUSE_WORD_SIZE / UNIPHIER_EFUSE_STRIDE macros here?
> 
> 
> 
> 
> static struct nvmem_config econfig = {
>        .name = "uniphier-efuse",
>        .owner = THIS_MODULE,
>        .read_only = true,
>        .reg_read = uniphier_reg_read,
>        .word_size = 4,
>        .stride = 4,
> };
> 
> looks nicer in my taste.
> 

I will fix it.

> 
> BTW, is there really no possibility
> where the unphier_efuse_probe() is run concurrently?
> 
> Many other drivers pass statically allocated memory to nvmem_register()
> like this...
> 
In the case of the uniphier system, since there is only one efuse device,
it will not run concurrently. 
I would like to fix it when I need it in the future.

> 
> > +static int uniphier_efuse_probe(struct platform_device *pdev) {
> > +       struct device *dev = &pdev->dev;
> > +       struct nvmem_device *nvmem;
> > +       struct regmap *regmap;
> > +       struct resource res;
> > +       int ret;
> > +
> > +       ret = of_address_to_resource(dev->of_node, 0, &res);
> > +       if (ret)
> > +               return ret;
> > +
> > +       regmap = syscon_node_to_regmap(dev->of_node);
> > +       if (IS_ERR(regmap))
> > +               return PTR_ERR(regmap);
> 
> 
> In my understanding,
> syscon_node_to_regmap() is generally useful to get regmap of another node,
> i.e. it is often used together with of_get_parent / of_parse_phandle etc.
> 
> You can use devm_regmap_init_mmio()
> to init regmap for itself.
> 
> 
> BTW, what is your motivation of using regmap?
>
> 
> I think what you want to do is
> the same as mtk-efuse.c, right?
> (and qfprom.c is similar, the difference is just register width)

Since redundancy can be omitted, implementation is easier, I think so.
However, since I can't find the merit of using regmap,
I will implement it in a method that does not use it.

> 
> 
> > +       econfig.size = resource_size(&res);
> > +       econfig.priv = regmap;
> > +       econfig.dev = dev;
> > +
> > +       nvmem = nvmem_register(&econfig);
> > +       if (IS_ERR(nvmem))
> > +               return PTR_ERR(nvmem);
> > +
> > +       platform_set_drvdata(pdev, nvmem);
> > +
> > +       return 0;
> > +}
> > +
> > +static int uniphier_efuse_remove(struct platform_device *pdev) {
> > +       struct nvmem_device *nvmem = platform_get_drvdata(pdev);
> > +
> > +       return nvmem_unregister(nvmem); }
> > +
> > +static const struct of_device_id uniphier_efuse_of_match[] = {
> > +       { .compatible = "socionext,uniphier-efuse",},
> > +       {/* sentinel */},
> > +};
> > +MODULE_DEVICE_TABLE(of, uniphier_efuse_of_match);
> > +
> > +static struct platform_driver uniphier_efuse_driver = {
> > +       .probe = uniphier_efuse_probe,
> > +       .remove = uniphier_efuse_remove,
> > +       .driver = {
> > +               .name = "uniphier-efuse",
> > +               .of_match_table = uniphier_efuse_of_match,
> > +       },
> > +};
> > +module_platform_driver(uniphier_efuse_driver);
> > +
> > +MODULE_AUTHOR("Keiji Hayashibara <hayashibara.keiji@socionext.com>");
> > +MODULE_DESCRIPTION("UniPhier eFuse driver"); MODULE_LICENSE("GPL
> > +v2");
> > --
> > 2.7.4
> >
> 
> 
> 
> --
> Best Regards
> Masahiro Yamada

Best Regards,
Keiji Hayashibara

Re: [PATCH v1 1/3] dt-bindings: nvmem: add description for UniPhier eFuse

[Rob Herring]

On Tue, Sep 05, 2017 at 04:04:31PM +0900, Keiji Hayashibara wrote:
> Hello Yamada-san,
> 
> Thank you for your comment.
> 
> > From: Masahiro Yamada [mailto:yamada.masahiro@socionext.com]
> > Sent: Monday, September 4, 2017 9:56 PM
> > 
> > 2017-09-01 8:20 GMT+09:00 Keiji Hayashibara
> > <hayashibara.keiji@socionext.com>:
> > > Add uniphier-efuse dt-bindings documentation.
> > >
> > > Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
> > > ---
> > >  .../devicetree/bindings/nvmem/uniphier-efuse.txt   | 45
> > ++++++++++++++++++++++
> > >  1 file changed, 45 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt

> > > +Example:
> > > +
> > > +       soc-glue@5f900000 {
> > > +               compatible = "socionext,uniphier-ld20-soc-glue-debug",
> > > +                            "simple-mfd";
> > > +               #address-cells = <1>;
> > > +               #size-cells = <1>;
> > > +               ranges = <0x0 0x5f900000 0x2000>;
> > 
> > 
> > IMHO, I think an empty "ranges;" will clarify the code, but it is up to
> > your taste.
> > 
> > 
> > > +
> > > +               efuse {
> > > +                       compatible = "socionext,uniphier-efuse",
> > > +                                    "syscon";
> > 
> > 
> > You are adding a dedicated driver for "socionext,uniphier-efuse".
> > 
> > Then, "syscon" as well?
> > 
> 
> Since I was using the syscon interface to implement the driver,
> I specified "syscon". It's interface is syscon_node_to_regmap().
> 
> I will rethink this in v2.
> 
> > 
> > 
> > > +                       reg = <0x100 0xf00>;
> > 
> > 
> > Not so many efuse registers exist on the SoC.
> > 
> > reg = <0x100 0x200>; will be enough.
> > 
> > 
> > Or if you want to be strict to the hw spec, you can write as follows:
> > 
> >         soc-glue@5f900000 {
> >                compatible = "socionext,uniphier-ld20-soc-glue-debug";
> >                             "simple-mfd";
> >                #address-cells = <1>;
> >                #size-cells = <1>;
> >                ranges = <0x0 0x5f900000 0x2000>;
> > 
> >                efuse@100 {
> >                            compatible = "socionext,uniphier-efuse";
> >                            reg = <0x100 0x28>;
> >                };
> > 
> >                efuse@200 {
> >                            compatible = "socionext,uniphier-efuse";
> >                            reg = <0x200 0x68>;
> >                };
> >        };
> > 
> > 
> > 
> > 
> > > +                       #address-cells = <1>;
> > > +                       #size-cells = <1>;
> > > +
> > > +                       /* Data cells */
> > > +                       usb_mon: usb_mon {
> > > +                               reg = <0x154 0xc>;
> > > +                       };
> > 
> > 
> > This <0x154 0xc> represents 0x5f900254 in CPU address view.
> > (0x5f900000 + 0x100 + 0x154)
> > 
> > So many ranges conversion, and how error-prone..
> > 
> 
> Yes, indeed...
> I will modify as below.

Please don't. A non-empty ranges is preferred. It limits the scope and 
chance for errors (smaller range allows fewer possible values and 
limits the chances of having address ranges duplicated in multiple 
nodes). But yes, it does add the requirement of doing addition and/or 
OR operations. I can't review whether the address ends up being correct 
either way, but having non-empty ranges helps enforce the other things.

Rob

RE: [PATCH v1 1/3] dt-bindings: nvmem: add description for UniPhier eFuse

[Keiji Hayashibara]

Hello Rob,

Thank you for your comment.

> From: Rob Herring [mailto:robh@kernel.org]
> Sent: Wednesday, September 13, 2017 1:16 AM
> 
> On Tue, Sep 05, 2017 at 04:04:31PM +0900, Keiji Hayashibara wrote:
> > Hello Yamada-san,
> >
> > Thank you for your comment.
> >
> > > From: Masahiro Yamada [mailto:yamada.masahiro@socionext.com]
> > > Sent: Monday, September 4, 2017 9:56 PM
> > >
> > > 2017-09-01 8:20 GMT+09:00 Keiji Hayashibara
> > > <hayashibara.keiji@socionext.com>:
> > > > Add uniphier-efuse dt-bindings documentation.
> > > >
> > > > Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
> > > > ---
> > > >  .../devicetree/bindings/nvmem/uniphier-efuse.txt   | 45
> > > ++++++++++++++++++++++
> > > >  1 file changed, 45 insertions(+)
> > > >  create mode 100644
> > > > Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt
> 
> > > > +Example:
> > > > +
> > > > +       soc-glue@5f900000 {
> > > > +               compatible =
> "socionext,uniphier-ld20-soc-glue-debug",
> > > > +                            "simple-mfd";
> > > > +               #address-cells = <1>;
> > > > +               #size-cells = <1>;
> > > > +               ranges = <0x0 0x5f900000 0x2000>;
> > >
> > >
> > > IMHO, I think an empty "ranges;" will clarify the code, but it is up
> > > to your taste.
> > >
> > >
> > > > +
> > > > +               efuse {
> > > > +                       compatible = "socionext,uniphier-efuse",
> > > > +                                    "syscon";
> > >
> > >
> > > You are adding a dedicated driver for "socionext,uniphier-efuse".
> > >
> > > Then, "syscon" as well?
> > >
> >
> > Since I was using the syscon interface to implement the driver, I
> > specified "syscon". It's interface is syscon_node_to_regmap().
> >
> > I will rethink this in v2.
> >
> > >
> > >
> > > > +                       reg = <0x100 0xf00>;
> > >
> > >
> > > Not so many efuse registers exist on the SoC.
> > >
> > > reg = <0x100 0x200>; will be enough.
> > >
> > >
> > > Or if you want to be strict to the hw spec, you can write as follows:
> > >
> > >         soc-glue@5f900000 {
> > >                compatible =
> "socionext,uniphier-ld20-soc-glue-debug";
> > >                             "simple-mfd";
> > >                #address-cells = <1>;
> > >                #size-cells = <1>;
> > >                ranges = <0x0 0x5f900000 0x2000>;
> > >
> > >                efuse@100 {
> > >                            compatible = "socionext,uniphier-efuse";
> > >                            reg = <0x100 0x28>;
> > >                };
> > >
> > >                efuse@200 {
> > >                            compatible = "socionext,uniphier-efuse";
> > >                            reg = <0x200 0x68>;
> > >                };
> > >        };
> > >
> > >
> > >
> > >
> > > > +                       #address-cells = <1>;
> > > > +                       #size-cells = <1>;
> > > > +
> > > > +                       /* Data cells */
> > > > +                       usb_mon: usb_mon {
> > > > +                               reg = <0x154 0xc>;
> > > > +                       };
> > >
> > >
> > > This <0x154 0xc> represents 0x5f900254 in CPU address view.
> > > (0x5f900000 + 0x100 + 0x154)
> > >
> > > So many ranges conversion, and how error-prone..
> > >
> >
> > Yes, indeed...
> > I will modify as below.
> 
> Please don't. A non-empty ranges is preferred. It limits the scope and
chance
> for errors (smaller range allows fewer possible values and limits the
> chances of having address ranges duplicated in multiple nodes). But yes,
> it does add the requirement of doing addition and/or OR operations. I
can't
> review whether the address ends up being correct either way, but having
> non-empty ranges helps enforce the other things.

I see. 
I will proceed with implementation by non-empty ranges.

Best Regards,
Keiji Hayashibara