2019-05-05 13:29:23

by Peng Fan

[permalink] [raw]
Subject: [PATCH 1/4] dt-bindings: fsl: scu: add ocotp binding

NXP i.MX8QXP is an ARMv8 SoC with a Cortex-M4 core inside as
system controller(SCU), the ocotp controller is being controlled
by the SCU, so Linux need use RPC to SCU for ocotp handling. This
patch adds binding doc for i.MX8 SCU OCOTP driver.

Signed-off-by: Peng Fan <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Aisheng Dong <[email protected]>
Cc: Shawn Guo <[email protected]>
Cc: Ulf Hansson <[email protected]>
Cc: Stephen Boyd <[email protected]>
Cc: Anson Huang <[email protected]>
Cc: [email protected]
---
Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
index 5d7dbabbb784..9cb7d52bdf26 100644
--- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
+++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
@@ -100,6 +100,13 @@ ID in its "clocks" phandle cell.
See the full list of clock IDs from:
include/dt-bindings/clock/imx8qxp-clock.h

+OCOTP bindings based on SCU Message Protocol
+------------------------------------------------------------
+Required properties:
+- compatible: Should be "fsl,imx8qxp-ocotp"
+- #address-cells: Must be 1. Contains byte index
+- #size-cells: Must be 1. Contains byte length
+
Pinctrl bindings based on SCU Message Protocol
------------------------------------------------------------

@@ -177,6 +184,12 @@ firmware {
...
};

+ ocotp: imx8qx-ocotp {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "fsl,imx8qxp-ocotp";
+ };
+
pd: imx8qx-pd {
compatible = "fsl,imx8qxp-scu-pd", "fsl,scu-pd";
#power-domain-cells = <1>;
--
2.16.4


2019-05-05 13:29:30

by Peng Fan

[permalink] [raw]
Subject: [PATCH 2/4] nvmem: imx: add i.MX8 nvmem driver

This patch adds i.MX8 nvmem ocotp driver to access fuse via
RPC to i.MX8 system controller.

Signed-off-by: Peng Fan <[email protected]>
Cc: Srinivas Kandagatla <[email protected]>
Cc: Shawn Guo <[email protected]>
Cc: Sascha Hauer <[email protected]>
Cc: Pengutronix Kernel Team <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: NXP Linux Team <[email protected]>
Cc: [email protected]
---
drivers/nvmem/Kconfig | 7 +++
drivers/nvmem/Makefile | 2 +
drivers/nvmem/imx-ocotp-scu.c | 135 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 144 insertions(+)
create mode 100644 drivers/nvmem/imx-ocotp-scu.c

diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index 530d570724c9..0e705c04bd8c 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -36,6 +36,13 @@ config NVMEM_IMX_OCOTP
This driver can also be built as a module. If so, the module
will be called nvmem-imx-ocotp.

+config NVMEM_IMX_OCOTP_SCU
+ tristate "i.MX8 On-Chip OTP Controller support"
+ depends on IMX_SCU
+ help
+ This is a driver for the On-Chip OTP Controller (OCOTP)
+ available on i.MX8 SoCs.
+
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 2ece8ffffdda..30d653d34e57 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -13,6 +13,8 @@ obj-$(CONFIG_NVMEM_IMX_IIM) += nvmem-imx-iim.o
nvmem-imx-iim-y := imx-iim.o
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_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/imx-ocotp-scu.c b/drivers/nvmem/imx-ocotp-scu.c
new file mode 100644
index 000000000000..07e1eba385ac
--- /dev/null
+++ b/drivers/nvmem/imx-ocotp-scu.c
@@ -0,0 +1,135 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * i.MX8 OCOTP fusebox driver
+ *
+ * Copyright 2019 NXP
+ *
+ * Peng Fan <[email protected]>
+ */
+
+#include <linux/firmware/imx/sci.h>
+#include <linux/module.h>
+#include <linux/nvmem-provider.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+enum ocotp_devtype {
+ IMX8QXP,
+};
+
+struct ocotp_devtype_data {
+ int devtype;
+ int nregs;
+};
+
+struct ocotp_priv {
+ struct device *dev;
+ const struct ocotp_devtype_data *data;
+ struct imx_sc_ipc *nvmem_ipc;
+};
+
+static struct ocotp_devtype_data imx8qxp_data = {
+ .devtype = IMX8QXP,
+ .nregs = 800,
+};
+
+static int imx_scu_ocotp_read(void *context, unsigned int offset,
+ void *val, size_t bytes)
+{
+ struct ocotp_priv *priv = context;
+ u32 count, index, num_bytes;
+ u8 *buf, *p;
+ int i, ret;
+
+ index = offset >> 2;
+ num_bytes = round_up((offset % 4) + bytes, 4);
+ count = num_bytes >> 2;
+
+ if (count > (priv->data->nregs - index))
+ count = priv->data->nregs - index;
+
+ p = kzalloc(num_bytes, GFP_KERNEL);
+ if (!p)
+ return -ENOMEM;
+
+ buf = p;
+
+ for (i = index; i < (index + count); i++) {
+ if (priv->data->devtype == IMX8QXP) {
+ if ((i > 271) && (i < 544)) {
+ *(u32 *)buf = 0;
+ buf += 4;
+ continue;
+ }
+ }
+
+ ret = imx_sc_misc_otp_fuse_read(priv->nvmem_ipc, i,
+ (u32 *)buf);
+ if (ret) {
+ kfree(p);
+ return ret;
+ }
+ buf += 4;
+ }
+
+ index = offset % 4;
+ memcpy(val, &p[index], bytes);
+
+ kfree(p);
+
+ return 0;
+}
+
+static struct nvmem_config imx_scu_ocotp_nvmem_config = {
+ .name = "imx-ocotp",
+ .read_only = true,
+ .word_size = 4,
+ .stride = 1,
+ .owner = THIS_MODULE,
+ .reg_read = imx_scu_ocotp_read,
+};
+
+static const struct of_device_id imx_scu_ocotp_dt_ids[] = {
+ { .compatible = "fsl,imx8qxp-ocotp", (void *)&imx8qxp_data },
+ { },
+};
+MODULE_DEVICE_TABLE(of, imx_scu_ocotp_dt_ids);
+
+static int imx_scu_ocotp_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ocotp_priv *priv;
+ struct nvmem_device *nvmem;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ ret = imx_scu_get_handle(&priv->nvmem_ipc);
+ if (ret)
+ return ret;
+
+ priv->data = of_device_get_match_data(dev);
+ priv->dev = dev;
+ imx_scu_ocotp_nvmem_config.size = 4 * priv->data->nregs;
+ imx_scu_ocotp_nvmem_config.dev = dev;
+ imx_scu_ocotp_nvmem_config.priv = priv;
+ nvmem = devm_nvmem_register(dev, &imx_scu_ocotp_nvmem_config);
+
+ return PTR_ERR_OR_ZERO(nvmem);
+}
+
+static struct platform_driver imx_scu_ocotp_driver = {
+ .probe = imx_scu_ocotp_probe,
+ .driver = {
+ .name = "imx_scu_ocotp",
+ .of_match_table = imx_scu_ocotp_dt_ids,
+ },
+};
+module_platform_driver(imx_scu_ocotp_driver);
+
+MODULE_AUTHOR("Peng Fan <[email protected]>");
+MODULE_DESCRIPTION("i.MX8QM OCOTP fuse box driver");
+MODULE_LICENSE("GPL v2");
--
2.16.4

2019-05-05 13:31:16

by Peng Fan

[permalink] [raw]
Subject: [PATCH 3/4] defconfig: arm64: enable i.MX8 nvmem driver

Build in CONFIG_NVMEM_IMX_OCOTP_SCU.

Signed-off-by: Peng Fan <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Shawn Guo <[email protected]>
Cc: Andy Gross <[email protected]>
Cc: Maxime Ripard <[email protected]>
Cc: Olof Johansson <[email protected]>
Cc: Jagan Teki <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: Marc Gonzalez <[email protected]>
Cc: Enric Balletbo i Serra <[email protected]>
Cc: [email protected]
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index eb31c20e9914..9d8a512fc3d5 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -748,6 +748,7 @@ CONFIG_HISI_PMU=y
CONFIG_QCOM_L2_PMU=y
CONFIG_QCOM_L3_PMU=y
CONFIG_NVMEM_IMX_OCOTP=y
+CONFIG_NVMEM_IMX_OCOTP_SCU=y
CONFIG_QCOM_QFPROM=y
CONFIG_ROCKCHIP_EFUSE=y
CONFIG_UNIPHIER_EFUSE=y
--
2.16.4

2019-05-05 13:31:52

by Peng Fan

[permalink] [raw]
Subject: [PATCH 4/4] arm64: dts: imx: add i.MX8QXP ocotp support

Add i.MX8QXP ocotp node

Signed-off-by: Peng Fan <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Shawn Guo <[email protected]>
Cc: Sascha Hauer <[email protected]>
Cc: Pengutronix Kernel Team <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: NXP Linux Team <[email protected]>
Cc: Aisheng Dong <[email protected]>
Cc: Anson Huang <[email protected]>
Cc: Daniel Baluta <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index 0683ee2a48ae..f29998d7274a 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
@@ -141,6 +141,12 @@
compatible = "fsl,imx8qxp-iomuxc";
};

+ ocotp: imx8qx-ocotp {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "fsl,imx8qxp-ocotp";
+ };
+
pd: imx8qx-pd {
compatible = "fsl,imx8qxp-scu-pd";
#power-domain-cells = <1>;
--
2.16.4

2019-05-06 08:07:58

by Aisheng Dong

[permalink] [raw]
Subject: RE: [PATCH 1/4] dt-bindings: fsl: scu: add ocotp binding

> From: Peng Fan
> Sent: Sunday, May 5, 2019 9:28 PM
>
> NXP i.MX8QXP is an ARMv8 SoC with a Cortex-M4 core inside as system
> controller(SCU), the ocotp controller is being controlled by the SCU, so Linux
> need use RPC to SCU for ocotp handling. This patch adds binding doc for i.MX8
> SCU OCOTP driver.
>
> Signed-off-by: Peng Fan <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Aisheng Dong <[email protected]>
> Cc: Shawn Guo <[email protected]>
> Cc: Ulf Hansson <[email protected]>
> Cc: Stephen Boyd <[email protected]>
> Cc: Anson Huang <[email protected]>
> Cc: [email protected]
> ---
> Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt | 13
> +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> index 5d7dbabbb784..9cb7d52bdf26 100644
> --- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> @@ -100,6 +100,13 @@ ID in its "clocks" phandle cell.
> See the full list of clock IDs from:
> include/dt-bindings/clock/imx8qxp-clock.h
>
> +OCOTP bindings based on SCU Message Protocol
> +------------------------------------------------------------
> +Required properties:
> +- compatible: Should be "fsl,imx8qxp-ocotp"
> +- #address-cells: Must be 1. Contains byte index
> +- #size-cells: Must be 1. Contains byte length
> +

Please put this unimportant one to the last.
And it's better to mention the optional child nodes for data cells as
Above #address-cells and #size-cells are used for it.
Just like:
Documentation/devicetree/bindings/nvmem/imx-ocotp.txt

> Pinctrl bindings based on SCU Message Protocol
> ------------------------------------------------------------
>
> @@ -177,6 +184,12 @@ firmware {
> ...
> };
>
> + ocotp: imx8qx-ocotp {
> + #address-cells = <1>;
> + #size-cells = <1>;

Not sure if it's a free choice, but AFAIK we usually write #address-cells and #size-cells
under compatible string according to the example in DT spec and doc.
https://elinux.org/Device_Tree_Usage
https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.2

Maybe Rob can comment to avoid confusing.

Otherwise, this patch seems good to me.

Regards
Dong Aisheng

> + compatible = "fsl,imx8qxp-ocotp";
> + };
> +
> pd: imx8qx-pd {
> compatible = "fsl,imx8qxp-scu-pd", "fsl,scu-pd";
> #power-domain-cells = <1>;
> --
> 2.16.4

2019-05-06 08:35:30

by Aisheng Dong

[permalink] [raw]
Subject: RE: [PATCH 2/4] nvmem: imx: add i.MX8 nvmem driver

> From: Peng Fan
> Sent: Sunday, May 5, 2019 9:28 PM
> Subject: [PATCH 2/4] nvmem: imx: add i.MX8 nvmem driver
>
> This patch adds i.MX8 nvmem ocotp driver to access fuse via RPC to i.MX8
> system controller.
>
> Signed-off-by: Peng Fan <[email protected]>

Only a few minor comments.
Otherwise, this patch looks good to me.

First, the patch title probably better to be:
nvmem: imx: add i.MX8 SCU based ocotp driver support

> Cc: Srinivas Kandagatla <[email protected]>
> Cc: Shawn Guo <[email protected]>
> Cc: Sascha Hauer <[email protected]>
> Cc: Pengutronix Kernel Team <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: NXP Linux Team <[email protected]>
> Cc: [email protected]
> ---
> drivers/nvmem/Kconfig | 7 +++
> drivers/nvmem/Makefile | 2 +
> drivers/nvmem/imx-ocotp-scu.c | 135
> ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 144 insertions(+)
> create mode 100644 drivers/nvmem/imx-ocotp-scu.c
>
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index
> 530d570724c9..0e705c04bd8c 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -36,6 +36,13 @@ config NVMEM_IMX_OCOTP
> This driver can also be built as a module. If so, the module
> will be called nvmem-imx-ocotp.
>
> +config NVMEM_IMX_OCOTP_SCU
> + tristate "i.MX8 On-Chip OTP Controller support"

i.MX8 SCU On-Chip OTP Controller support

> + depends on IMX_SCU
> + help
> + This is a driver for the On-Chip OTP Controller (OCOTP)

SCU On-Chip OTP

> + available on i.MX8 SoCs.
> +
> 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
> 2ece8ffffdda..30d653d34e57 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -13,6 +13,8 @@ obj-$(CONFIG_NVMEM_IMX_IIM) +=
> nvmem-imx-iim.o
> nvmem-imx-iim-y := imx-iim.o
> 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_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/imx-ocotp-scu.c b/drivers/nvmem/imx-ocotp-scu.c
> new file mode 100644 index 000000000000..07e1eba385ac
> --- /dev/null
> +++ b/drivers/nvmem/imx-ocotp-scu.c
> @@ -0,0 +1,135 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * i.MX8 OCOTP fusebox driver
> + *
> + * Copyright 2019 NXP
> + *
> + * Peng Fan <[email protected]>
> + */
> +
> +#include <linux/firmware/imx/sci.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-provider.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +enum ocotp_devtype {
> + IMX8QXP,
> +};
> +
> +struct ocotp_devtype_data {
> + int devtype;
> + int nregs;
> +};
> +
> +struct ocotp_priv {
> + struct device *dev;
> + const struct ocotp_devtype_data *data;
> + struct imx_sc_ipc *nvmem_ipc;
> +};
> +
> +static struct ocotp_devtype_data imx8qxp_data = {
> + .devtype = IMX8QXP,
> + .nregs = 800,
> +};
> +
> +static int imx_scu_ocotp_read(void *context, unsigned int offset,
> + void *val, size_t bytes)
> +{
> + struct ocotp_priv *priv = context;
> + u32 count, index, num_bytes;
> + u8 *buf, *p;

It seems buf has never been used as u8.
So probably a better way is:
U32 *buf;
Void *p.
Then we can save all the explicit conversion of u32.

> + int i, ret;
> +
> + index = offset >> 2;
> + num_bytes = round_up((offset % 4) + bytes, 4);
> + count = num_bytes >> 2;
> +
> + if (count > (priv->data->nregs - index))
> + count = priv->data->nregs - index;
> +
> + p = kzalloc(num_bytes, GFP_KERNEL);
> + if (!p)
> + return -ENOMEM;
> +
> + buf = p;
> +
> + for (i = index; i < (index + count); i++) {
> + if (priv->data->devtype == IMX8QXP) {
> + if ((i > 271) && (i < 544)) {
> + *(u32 *)buf = 0;
> + buf += 4;
> + continue;
> + }
> + }
> +
> + ret = imx_sc_misc_otp_fuse_read(priv->nvmem_ipc, i,
> + (u32 *)buf);

Is this API already in kernel?

> + if (ret) {
> + kfree(p);
> + return ret;
> + }
> + buf += 4;
> + }
> +
> + index = offset % 4;
> + memcpy(val, &p[index], bytes);
> +
> + kfree(p);
> +
> + return 0;
> +}
> +
> +static struct nvmem_config imx_scu_ocotp_nvmem_config = {
> + .name = "imx-ocotp",

imx-scu-octop

> + .read_only = true,
> + .word_size = 4,
> + .stride = 1,
> + .owner = THIS_MODULE,
> + .reg_read = imx_scu_ocotp_read,
> +};
> +
> +static const struct of_device_id imx_scu_ocotp_dt_ids[] = {
> + { .compatible = "fsl,imx8qxp-ocotp", (void *)&imx8qxp_data },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, imx_scu_ocotp_dt_ids);
> +
> +static int imx_scu_ocotp_probe(struct platform_device *pdev) {
> + struct device *dev = &pdev->dev;
> + struct ocotp_priv *priv;
> + struct nvmem_device *nvmem;
> + int ret;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + ret = imx_scu_get_handle(&priv->nvmem_ipc);
> + if (ret)
> + return ret;
> +
> + priv->data = of_device_get_match_data(dev);
> + priv->dev = dev;
> + imx_scu_ocotp_nvmem_config.size = 4 * priv->data->nregs;
> + imx_scu_ocotp_nvmem_config.dev = dev;
> + imx_scu_ocotp_nvmem_config.priv = priv;
> + nvmem = devm_nvmem_register(dev, &imx_scu_ocotp_nvmem_config);
> +
> + return PTR_ERR_OR_ZERO(nvmem);
> +}
> +
> +static struct platform_driver imx_scu_ocotp_driver = {
> + .probe = imx_scu_ocotp_probe,
> + .driver = {
> + .name = "imx_scu_ocotp",
> + .of_match_table = imx_scu_ocotp_dt_ids,
> + },
> +};
> +module_platform_driver(imx_scu_ocotp_driver);
> +
> +MODULE_AUTHOR("Peng Fan <[email protected]>");
> +MODULE_DESCRIPTION("i.MX8QM OCOTP fuse box driver");

i.MX8 SCU OCOTP fuse box driver

Regards
Dong Aisheng

> +MODULE_LICENSE("GPL v2");
> --
> 2.16.4

2019-05-06 08:40:10

by Aisheng Dong

[permalink] [raw]
Subject: RE: [PATCH 3/4] defconfig: arm64: enable i.MX8 nvmem driver

> From: Peng Fan
> Sent: Sunday, May 5, 2019 9:28 PM
> Subject: [PATCH 3/4] defconfig: arm64: enable i.MX8 nvmem driver
>
> Build in CONFIG_NVMEM_IMX_OCOTP_SCU.
>
> Signed-off-by: Peng Fan <[email protected]>

defconfig: arm64: enable i.MX8 SCU octop driver

otherwise:
Reviewed-by: Dong Aisheng <[email protected]>

Regards
Dong Aisheng

> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Shawn Guo <[email protected]>
> Cc: Andy Gross <[email protected]>
> Cc: Maxime Ripard <[email protected]>
> Cc: Olof Johansson <[email protected]>
> Cc: Jagan Teki <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Leonard Crestez <[email protected]>
> Cc: Marc Gonzalez <[email protected]>
> Cc: Enric Balletbo i Serra <[email protected]>
> Cc: [email protected]
> ---
> arch/arm64/configs/defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index eb31c20e9914..9d8a512fc3d5 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -748,6 +748,7 @@ CONFIG_HISI_PMU=y
> CONFIG_QCOM_L2_PMU=y
> CONFIG_QCOM_L3_PMU=y
> CONFIG_NVMEM_IMX_OCOTP=y
> +CONFIG_NVMEM_IMX_OCOTP_SCU=y
> CONFIG_QCOM_QFPROM=y
> CONFIG_ROCKCHIP_EFUSE=y
> CONFIG_UNIPHIER_EFUSE=y
> --
> 2.16.4

2019-05-06 08:44:07

by Aisheng Dong

[permalink] [raw]
Subject: RE: [PATCH 4/4] arm64: dts: imx: add i.MX8QXP ocotp support

> From: Peng Fan
> Sent: Sunday, May 5, 2019 9:28 PM
> Subject: [PATCH 4/4] arm64: dts: imx: add i.MX8QXP ocotp support
>
> Add i.MX8QXP ocotp node
>
> Signed-off-by: Peng Fan <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Shawn Guo <[email protected]>
> Cc: Sascha Hauer <[email protected]>
> Cc: Pengutronix Kernel Team <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: NXP Linux Team <[email protected]>
> Cc: Aisheng Dong <[email protected]>
> Cc: Anson Huang <[email protected]>
> Cc: Daniel Baluta <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> index 0683ee2a48ae..f29998d7274a 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> @@ -141,6 +141,12 @@
> compatible = "fsl,imx8qxp-iomuxc";
> };
>
> + ocotp: imx8qx-ocotp {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "fsl,imx8qxp-ocotp";
> + };

See my reply on Patch [1/4]

Regards
Dong Aisheng

> +
> pd: imx8qx-pd {
> compatible = "fsl,imx8qxp-scu-pd";
> #power-domain-cells = <1>;
> --
> 2.16.4

2019-05-06 08:47:19

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH 2/4] nvmem: imx: add i.MX8 nvmem driver

Hi Aisheng,

> Subject: RE: [PATCH 2/4] nvmem: imx: add i.MX8 nvmem driver
>
> > From: Peng Fan
> > Sent: Sunday, May 5, 2019 9:28 PM
> > Subject: [PATCH 2/4] nvmem: imx: add i.MX8 nvmem driver
> >
> > This patch adds i.MX8 nvmem ocotp driver to access fuse via RPC to
> > i.MX8 system controller.
> >
> > Signed-off-by: Peng Fan <[email protected]>
>
> Only a few minor comments.
> Otherwise, this patch looks good to me.
>
> First, the patch title probably better to be:
> nvmem: imx: add i.MX8 SCU based ocotp driver support

Fix in V2.

>
> > Cc: Srinivas Kandagatla <[email protected]>
> > Cc: Shawn Guo <[email protected]>
> > Cc: Sascha Hauer <[email protected]>
> > Cc: Pengutronix Kernel Team <[email protected]>
> > Cc: Fabio Estevam <[email protected]>
> > Cc: NXP Linux Team <[email protected]>
> > Cc: [email protected]
> > ---
> > drivers/nvmem/Kconfig | 7 +++
> > drivers/nvmem/Makefile | 2 +
> > drivers/nvmem/imx-ocotp-scu.c | 135
> > ++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 144 insertions(+)
> > create mode 100644 drivers/nvmem/imx-ocotp-scu.c
> >
> > diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index
> > 530d570724c9..0e705c04bd8c 100644
> > --- a/drivers/nvmem/Kconfig
> > +++ b/drivers/nvmem/Kconfig
> > @@ -36,6 +36,13 @@ config NVMEM_IMX_OCOTP
> > This driver can also be built as a module. If so, the module
> > will be called nvmem-imx-ocotp.
> >
> > +config NVMEM_IMX_OCOTP_SCU
> > + tristate "i.MX8 On-Chip OTP Controller support"
>
> i.MX8 SCU On-Chip OTP Controller support
Fix in V2
>
> > + depends on IMX_SCU
> > + help
> > + This is a driver for the On-Chip OTP Controller (OCOTP)
>
> SCU On-Chip OTP
Fix in V2.
>
> > + available on i.MX8 SoCs.
> > +
[.....]

> > +
> > +static int imx_scu_ocotp_read(void *context, unsigned int offset,
> > + void *val, size_t bytes)
> > +{
> > + struct ocotp_priv *priv = context;
> > + u32 count, index, num_bytes;
> > + u8 *buf, *p;
>
> It seems buf has never been used as u8.
> So probably a better way is:
> U32 *buf;
> Void *p.
> Then we can save all the explicit conversion of u32.

Fix in V2.

>
> > + int i, ret;
> > +
> > + index = offset >> 2;
> > + num_bytes = round_up((offset % 4) + bytes, 4);
> > + count = num_bytes >> 2;
> > +
> > + if (count > (priv->data->nregs - index))
> > + count = priv->data->nregs - index;
> > +
> > + p = kzalloc(num_bytes, GFP_KERNEL);
> > + if (!p)
> > + return -ENOMEM;
> > +
> > + buf = p;
> > +
> > + for (i = index; i < (index + count); i++) {
> > + if (priv->data->devtype == IMX8QXP) {
> > + if ((i > 271) && (i < 544)) {
> > + *(u32 *)buf = 0;
> > + buf += 4;
> > + continue;
> > + }
> > + }
> > +
> > + ret = imx_sc_misc_otp_fuse_read(priv->nvmem_ipc, i,
> > + (u32 *)buf);
>
> Is this API already in kernel?

Ah. I forgot to post out that API in this patchset. Will add that in V2.

[....]
> > +
> > +MODULE_AUTHOR("Peng Fan <[email protected]>");
> > +MODULE_DESCRIPTION("i.MX8QM OCOTP fuse box driver");
>
> i.MX8 SCU OCOTP fuse box driver

Fix in V2.

Thanks,
Peng.

>
> Regards
> Dong Aisheng
>
> > +MODULE_LICENSE("GPL v2");
> > --
> > 2.16.4

2019-05-06 08:52:23

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH 1/4] dt-bindings: fsl: scu: add ocotp binding

Hi Aisheng,

> Subject: RE: [PATCH 1/4] dt-bindings: fsl: scu: add ocotp binding
>
> > From: Peng Fan
> > Sent: Sunday, May 5, 2019 9:28 PM
> >
> > NXP i.MX8QXP is an ARMv8 SoC with a Cortex-M4 core inside as system
> > controller(SCU), the ocotp controller is being controlled by the SCU,
> > so Linux need use RPC to SCU for ocotp handling. This patch adds
> > binding doc for i.MX8 SCU OCOTP driver.
> >
> > Signed-off-by: Peng Fan <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Cc: Mark Rutland <[email protected]>
> > Cc: Aisheng Dong <[email protected]>
> > Cc: Shawn Guo <[email protected]>
> > Cc: Ulf Hansson <[email protected]>
> > Cc: Stephen Boyd <[email protected]>
> > Cc: Anson Huang <[email protected]>
> > Cc: [email protected]
> > ---
> > Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt | 13
> > +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > index 5d7dbabbb784..9cb7d52bdf26 100644
> > --- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > @@ -100,6 +100,13 @@ ID in its "clocks" phandle cell.
> > See the full list of clock IDs from:
> > include/dt-bindings/clock/imx8qxp-clock.h
> >
> > +OCOTP bindings based on SCU Message Protocol
> > +------------------------------------------------------------
> > +Required properties:
> > +- compatible: Should be "fsl,imx8qxp-ocotp"
> > +- #address-cells: Must be 1. Contains byte index
> > +- #size-cells: Must be 1. Contains byte length
> > +
>
> Please put this unimportant one to the last.

ok. I just followed alphabetical order as OCOTP, Pinctrl, RTC sequence.
Will move it to last in V2.

> And it's better to mention the optional child nodes for data cells as Above
> #address-cells and #size-cells are used for it.
> Just like:
> Documentation/devicetree/bindings/nvmem/imx-ocotp.txt
>
> > Pinctrl bindings based on SCU Message Protocol
> > ------------------------------------------------------------
> >
> > @@ -177,6 +184,12 @@ firmware {
> > ...
> > };
> >
> > + ocotp: imx8qx-ocotp {
> > + #address-cells = <1>;
> > + #size-cells = <1>;
>
> Not sure if it's a free choice, but AFAIK we usually write #address-cells and
> #size-cells under compatible string according to the example in DT spec and
> doc.

Fix in V2.

Thanks,
Peng

> https://elinux.org/Device_Tree_Usage
> https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.
> 2
>
> Maybe Rob can comment to avoid confusing.
>
> Otherwise, this patch seems good to me.
>
> Regards
> Dong Aisheng
>
> > + compatible = "fsl,imx8qxp-ocotp";
> > + };
> > +
> > pd: imx8qx-pd {
> > compatible = "fsl,imx8qxp-scu-pd", "fsl,scu-pd";
> > #power-domain-cells = <1>;
> > --
> > 2.16.4