[PATCH v1 0/4] change drivers/soc/imx/soc-imx8m.c to use platform driver

[PATCH v1 0/4] change drivers/soc/imx/soc-imx8m.c to use platform driver

[Alice Guo]

Directly reading ocotp register relies on bootloader to enable the ocotp clock,
which is not always true, so we need to use nvmem API. Using nvmem API means
that driver defer probe needs to be supported.

Alice Guo (4):
  LF-2571-1: dt-bindings: soc: imx8m: add DT Binding doc for soc unique
    ID
  LF-2571-2 arm64: dts: imx8m: add compatible string to .dtsi file
  LF-2571-3 arm64: dts: imx8m: add nvmem-cell related stuff
  LF-2571-4 soc: imx8m: change to use platform driver

 .../bindings/soc/imx/imx8m-unique-id.yaml     | 32 ++++++++
 arch/arm64/boot/dts/freescale/imx8mm.dtsi     |  8 +-
 arch/arm64/boot/dts/freescale/imx8mn.dtsi     |  8 +-
 arch/arm64/boot/dts/freescale/imx8mp.dtsi     |  8 +-
 arch/arm64/boot/dts/freescale/imx8mq.dtsi     |  8 +-
 drivers/soc/imx/soc-imx8m.c                   | 75 +++++++++++--------
 6 files changed, 102 insertions(+), 37 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml

--
2.17.1

[PATCH v1 1/4] LF-2571-1: dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID

[Alice Guo]

Add DT Binding doc for the Unique ID of i.MX 8M series.

Signed-off-by: Alice Guo <alice.guo@nxp.com>
---
 .../bindings/soc/imx/imx8m-unique-id.yaml     | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml

diff --git a/Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml b/Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
new file mode 100644
index 000000000000..f1e45458cec7
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
@@ -0,0 +1,32 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/nxp/imx8m-unique-id.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX8M Platforms Device Tree Bindings
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - fsl,imx8mm-soc
+              - fsl,imx8mn-soc
+              - fsl,imx8mp-soc
+              - fsl,imx8mq-soc
+          - const: simple-bus
+
+  nvmem-cells:
+    maxItems: 1
+    description:
+      Reference to an nvmem node for the SOC Unique ID.
+
+  nvmem-cells-names:
+    const: soc_unique_id
+
+required:
+  - compatible
+  - nvmem-cells
+  - nvmem-cell-names
+...
--
2.17.1

[PATCH v1 2/4] LF-2571-2 arm64: dts: imx8m: add compatible string to .dtsi file

[Alice Guo]

Add compatible string to .dtsi file for binding of imx8_soc_init_driver
and device.

Signed-off-by: Alice Guo <alice.guo@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 2 +-
 arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 2 +-
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index ee486597afc0..ec71a5e8a062 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -257,7 +257,7 @@
 	};

 	soc@0 {
-		compatible = "simple-bus";
+		compatible = "fsl,imx8mm-soc", "simple-bus";
 		#address-cells = <1>;
 		#size-cells = <1>;
 		ranges = <0x0 0x0 0x0 0x3e000000>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index a06d2a6268e6..6d3a809a00fd 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -241,7 +241,7 @@
 	};

 	soc@0 {
-		compatible = "simple-bus";
+		compatible = "fsl,imx8mn-soc", "simple-bus";
 		#address-cells = <1>;
 		#size-cells = <1>;
 		ranges = <0x0 0x0 0x0 0x3e000000>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 479312293036..9cd218f5f004 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -218,7 +218,7 @@
 	};

 	soc@0 {
-		compatible = "simple-bus";
+		compatible = "fsl,imx8mp-soc", "simple-bus";
 		#address-cells = <1>;
 		#size-cells = <1>;
 		ranges = <0x0 0x0 0x0 0x3e000000>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 5e0e7d0f1bc4..a3c76bb880db 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -286,7 +286,7 @@
 	};

 	soc@0 {
-		compatible = "simple-bus";
+		compatible = "fsl,imx8mq-soc", "simple-bus";
 		#address-cells = <1>;
 		#size-cells = <1>;
 		ranges = <0x0 0x0 0x0 0x3e000000>;
--
2.17.1

[PATCH v1 3/4] LF-2571-3 arm64: dts: imx8m: add nvmem-cell related stuff

[Alice Guo]

Add nvmem-cell related stuff for the soc unique ID.

Signed-off-by: Alice Guo <alice.guo@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 6 ++++++
 arch/arm64/boot/dts/freescale/imx8mn.dtsi | 6 ++++++
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 ++++++
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 6 ++++++
 4 files changed, 24 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index ec71a5e8a062..b45dfe133ec7 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -261,6 +261,8 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 		ranges = <0x0 0x0 0x0 0x3e000000>;
+		nvmem-cells = <&imx8mm_uid>;
+		nvmem-cell-names = "soc_unique_id";

 		aips1: bus@30000000 {
 			compatible = "fsl,aips-bus", "simple-bus";
@@ -475,6 +477,10 @@
 				#address-cells = <1>;
 				#size-cells = <1>;

+				imx8mm_uid: unique_id@410 {
+					reg = <4 8>;
+				};
+
 				cpu_speed_grade: speed-grade@10 {
 					reg = <0x10 4>;
 				};
diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index 6d3a809a00fd..7418c886fdab 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -245,6 +245,8 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 		ranges = <0x0 0x0 0x0 0x3e000000>;
+		nvmem-cells = <&imx8mn_uid>;
+		nvmem-cell-names = "soc_unique_id";

 		aips1: bus@30000000 {
 			compatible = "fsl,aips-bus", "simple-bus";
@@ -388,6 +390,10 @@
 				#address-cells = <1>;
 				#size-cells = <1>;

+				imx8mn_uid: unique_id@410 {
+					reg = <4 8>;
+				};
+
 				cpu_speed_grade: speed-grade@10 {
 					reg = <0x10 4>;
 				};
diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 9cd218f5f004..e0f1d5a0dc21 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -222,6 +222,8 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 		ranges = <0x0 0x0 0x0 0x3e000000>;
+		nvmem-cells = <&imx8mp_uid>;
+		nvmem-cell-names = "soc_unique_id";

 		aips1: bus@30000000 {
 			compatible = "fsl,aips-bus", "simple-bus";
@@ -328,6 +330,10 @@
 				#address-cells = <1>;
 				#size-cells = <1>;

+				imx8mp_uid: unique_id@420 {
+					reg = <8 8>;
+				};
+
 				cpu_speed_grade: speed-grade@10 {
 					reg = <0x10 4>;
 				};
diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index a3c76bb880db..8fad6730db3d 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -291,6 +291,8 @@
 		#size-cells = <1>;
 		ranges = <0x0 0x0 0x0 0x3e000000>;
 		dma-ranges = <0x40000000 0x0 0x40000000 0xc0000000>;
+		nvmem-cells = <&imx8mq_uid>;
+		nvmem-cell-names = "soc_unique_id";

 		bus@30000000 { /* AIPS1 */
 			compatible = "fsl,aips-bus", "simple-bus";
@@ -555,6 +557,10 @@
 				#address-cells = <1>;
 				#size-cells = <1>;

+				imx8mq_uid: soc_uid@410 {
+					reg = <4 8>;
+				};
+
 				cpu_speed_grade: speed-grade@10 {
 					reg = <0x10 4>;
 				};
--
2.17.1

[PATCH v1 4/4] LF-2571-4 soc: imx8m: change to use platform driver

[Alice Guo]

Directly reading ocotp register depends on that bootloader enables ocotp
clk, which is not always effective, so change to use nvmem API. Using
nvmem API requires to support driver defer probe and thus change
soc-imx8m.c to use platform driver.

The other reason is that directly reading ocotp register causes kexec
kernel hang because the 1st kernel running will disable unused clks
after kernel boots up, and then ocotp clk will be disabled even if
bootloader enables it. When kexec kernel, ocotp clk needs to be enabled
before reading ocotp registers, and nvmem API with platform driver
supported can accomplish this.

Signed-off-by: Alice Guo <alice.guo@nxp.com>
---
 drivers/soc/imx/soc-imx8m.c | 75 +++++++++++++++++++++----------------
 1 file changed, 42 insertions(+), 33 deletions(-)

diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c
index cc57a384d74d..83f3297509be 100644
--- a/drivers/soc/imx/soc-imx8m.c
+++ b/drivers/soc/imx/soc-imx8m.c
@@ -5,6 +5,8 @@

 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
 #include <linux/of_address.h>
 #include <linux/slab.h>
 #include <linux/sys_soc.h>
@@ -29,7 +31,7 @@

 struct imx8_soc_data {
 	char *name;
-	u32 (*soc_revision)(void);
+	u32 (*soc_revision)(struct device *dev);
 };

 static u64 soc_uid;
@@ -50,12 +52,15 @@ static u32 imx8mq_soc_revision_from_atf(void)
 static inline u32 imx8mq_soc_revision_from_atf(void) { return 0; };
 #endif

-static u32 __init imx8mq_soc_revision(void)
+static u32 __init imx8mm_soc_uid(struct device *dev);
+
+static u32 __init imx8mq_soc_revision(struct device *dev)
 {
 	struct device_node *np;
 	void __iomem *ocotp_base;
 	u32 magic;
 	u32 rev;
+	int ret = 0;

 	np = of_find_compatible_node(NULL, NULL, "fsl,imx8mq-ocotp");
 	if (!np)
@@ -75,9 +80,9 @@ static u32 __init imx8mq_soc_revision(void)
 			rev = REV_B1;
 	}

-	soc_uid = readl_relaxed(ocotp_base + OCOTP_UID_HIGH);
-	soc_uid <<= 32;
-	soc_uid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW);
+	ret = imx8mm_soc_uid(dev);
+	if (ret)
+		return ret;

 	iounmap(ocotp_base);
 	of_node_put(np);
@@ -85,33 +90,20 @@ static u32 __init imx8mq_soc_revision(void)
 	return rev;
 }

-static void __init imx8mm_soc_uid(void)
+static u32 __init imx8mm_soc_uid(struct device *dev)
 {
-	void __iomem *ocotp_base;
-	struct device_node *np;
-	u32 offset = of_machine_is_compatible("fsl,imx8mp") ?
-		     IMX8MP_OCOTP_UID_OFFSET : 0;
-
-	np = of_find_compatible_node(NULL, NULL, "fsl,imx8mm-ocotp");
-	if (!np)
-		return;
-
-	ocotp_base = of_iomap(np, 0);
-	WARN_ON(!ocotp_base);
-
-	soc_uid = readl_relaxed(ocotp_base + OCOTP_UID_HIGH + offset);
-	soc_uid <<= 32;
-	soc_uid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW + offset);
+	int ret = 0;

-	iounmap(ocotp_base);
-	of_node_put(np);
+	ret = nvmem_cell_read_u64(dev, "soc_unique_id", &soc_uid);
+	return ret;
 }

-static u32 __init imx8mm_soc_revision(void)
+static u32 __init imx8mm_soc_revision(struct device *dev)
 {
 	struct device_node *np;
 	void __iomem *anatop_base;
 	u32 rev;
+	int ret;

 	np = of_find_compatible_node(NULL, NULL, "fsl,imx8mm-anatop");
 	if (!np)
@@ -125,7 +117,9 @@ static u32 __init imx8mm_soc_revision(void)
 	iounmap(anatop_base);
 	of_node_put(np);

-	imx8mm_soc_uid();
+	ret = imx8mm_soc_uid(dev);
+	if (ret)
+		return ret;

 	return rev;
 }
@@ -151,19 +145,20 @@ static const struct imx8_soc_data imx8mp_soc_data = {
 };

 static __maybe_unused const struct of_device_id imx8_soc_match[] = {
-	{ .compatible = "fsl,imx8mq", .data = &imx8mq_soc_data, },
-	{ .compatible = "fsl,imx8mm", .data = &imx8mm_soc_data, },
-	{ .compatible = "fsl,imx8mn", .data = &imx8mn_soc_data, },
-	{ .compatible = "fsl,imx8mp", .data = &imx8mp_soc_data, },
+	{ .compatible = "fsl,imx8mq-soc", .data = &imx8mq_soc_data, },
+	{ .compatible = "fsl,imx8mm-soc", .data = &imx8mm_soc_data, },
+	{ .compatible = "fsl,imx8mn-soc", .data = &imx8mn_soc_data, },
+	{ .compatible = "fsl,imx8mp-soc", .data = &imx8mp_soc_data, },
 	{ }
 };
+MODULE_DEVICE_TABLE(of, imx8_soc_match);

 #define imx8_revision(soc_rev) \
 	soc_rev ? \
 	kasprintf(GFP_KERNEL, "%d.%d", (soc_rev >> 4) & 0xf,  soc_rev & 0xf) : \
 	"unknown"

-static int __init imx8_soc_init(void)
+static int imx8_soc_init_probe(struct platform_device *pdev)
 {
 	struct soc_device_attribute *soc_dev_attr;
 	struct soc_device *soc_dev;
@@ -182,7 +177,7 @@ static int __init imx8_soc_init(void)
 	if (ret)
 		goto free_soc;

-	id = of_match_node(imx8_soc_match, of_root);
+	id = of_match_node(imx8_soc_match, pdev->dev.of_node);
 	if (!id) {
 		ret = -ENODEV;
 		goto free_soc;
@@ -192,7 +187,11 @@ static int __init imx8_soc_init(void)
 	if (data) {
 		soc_dev_attr->soc_id = data->name;
 		if (data->soc_revision)
-			soc_rev = data->soc_revision();
+			soc_rev = data->soc_revision(&pdev->dev);
+
+		ret = soc_rev;
+		if (ret < 0)
+			goto free_soc;
 	}

 	soc_dev_attr->revision = imx8_revision(soc_rev);
@@ -230,4 +229,14 @@ static int __init imx8_soc_init(void)
 	kfree(soc_dev_attr);
 	return ret;
 }
-device_initcall(imx8_soc_init);
+
+static struct platform_driver imx8_soc_init_driver = {
+	.probe = imx8_soc_init_probe,
+	.driver = {
+		.name = "imx8_soc_init",
+		.of_match_table = of_match_ptr(imx8_soc_match),
+	},
+};
+
+module_platform_driver(imx8_soc_init_driver);
+MODULE_LICENSE("GPL v2");
--
2.17.1

Re: [PATCH v1 0/4] change drivers/soc/imx/soc-imx8m.c to use platform driver

[Krzysztof Kozlowski]

On Wed, 4 Nov 2020 at 04:09, Alice Guo <alice.guo@nxp.com> wrote:
>
> Directly reading ocotp register relies on bootloader to enable the ocotp clock,
> which is not always true, so we need to use nvmem API. Using nvmem API means
> that driver defer probe needs to be supported.
>
> Alice Guo (4):
>   LF-2571-1: dt-bindings: soc: imx8m: add DT Binding doc for soc unique
>     ID

Judging by recipient address, you are sending to Linux kernel. In such
case, the subject prefix "LF" is wrong and please use "git log
--oneline" to get the list of prefixes for the given subsystem.

Best regards,
Krzysztof

Re: [PATCH v1 1/4] LF-2571-1: dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID

[Krzysztof Kozlowski]

On Wed, 4 Nov 2020 at 04:09, Alice Guo <alice.guo@nxp.com> wrote:
>
> Add DT Binding doc for the Unique ID of i.MX 8M series.
>
> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> ---
>  .../bindings/soc/imx/imx8m-unique-id.yaml     | 32 +++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
>
> diff --git a/Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml b/Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
> new file mode 100644
> index 000000000000..f1e45458cec7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
> @@ -0,0 +1,32 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/nxp/imx8m-unique-id.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP i.MX8M Platforms Device Tree Bindings

This is not a title for these bindings. Please describe the bindings
for this device. Based on description, this might could go to
bindings/nvmem directory.

> +
> +properties:
> +  compatible:
> +    oneOf:

No need for oneOf.

> +      - items:
> +          - enum:
> +              - fsl,imx8mm-soc
> +              - fsl,imx8mn-soc
> +              - fsl,imx8mp-soc
> +              - fsl,imx8mq-soc
> +          - const: simple-bus
> +
> +  nvmem-cells:
> +    maxItems: 1
> +    description:
> +      Reference to an nvmem node for the SOC Unique ID.

Misleading description - nvmem-cells do not contain a reference. Just
skip it, nvmem-cells should be obvious from the nvmem provider
bindings.

> +
> +  nvmem-cells-names:
> +    const: soc_unique_id

additionalProperties: false


Best regards,
Krzysztof

Re: [PATCH v1 1/4] LF-2571-1: dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID

[Krzysztof Kozlowski]

On Wed, 4 Nov 2020 at 08:58, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Wed, 4 Nov 2020 at 04:09, Alice Guo <alice.guo@nxp.com> wrote:
> >
> > Add DT Binding doc for the Unique ID of i.MX 8M series.
> >
> > Signed-off-by: Alice Guo <alice.guo@nxp.com>
> > ---
> >  .../bindings/soc/imx/imx8m-unique-id.yaml     | 32 +++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml b/Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
> > new file mode 100644
> > index 000000000000..f1e45458cec7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
> > @@ -0,0 +1,32 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/arm/nxp/imx8m-unique-id.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP i.MX8M Platforms Device Tree Bindings
>
> This is not a title for these bindings. Please describe the bindings
> for this device. Based on description, this might could go to
> bindings/nvmem directory.
>
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
>
> No need for oneOf.
>
> > +      - items:
> > +          - enum:
> > +              - fsl,imx8mm-soc
> > +              - fsl,imx8mn-soc
> > +              - fsl,imx8mp-soc
> > +              - fsl,imx8mq-soc
> > +          - const: simple-bus
> > +
> > +  nvmem-cells:
> > +    maxItems: 1
> > +    description:
> > +      Reference to an nvmem node for the SOC Unique ID.
>
> Misleading description - nvmem-cells do not contain a reference. Just
> skip it, nvmem-cells should be obvious from the nvmem provider
> bindings.
>
> > +
> > +  nvmem-cells-names:
> > +    const: soc_unique_id
>
> additionalProperties: false

... or what looks more appropriate - you should include nvmem-consumer
bindings and use unevaluatedProperties:false.

Best regards,
Krzysztof

RE: [PATCH v1 1/4] LF-2571-1: dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID

[Peng Fan]

> Subject: Re: [PATCH v1 1/4] LF-2571-1: dt-bindings: soc: imx8m: add DT
> Binding doc for soc unique ID
> 
> On Wed, 4 Nov 2020 at 04:09, Alice Guo <alice.guo@nxp.com> wrote:
> >
> > Add DT Binding doc for the Unique ID of i.MX 8M series.
> >
> > Signed-off-by: Alice Guo <alice.guo@nxp.com>
> > ---
> >  .../bindings/soc/imx/imx8m-unique-id.yaml     | 32
> +++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
> > b/Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
> > new file mode 100644
> > index 000000000000..f1e45458cec7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/imx/imx8m-unique-id.yaml
> > @@ -0,0 +1,32 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fschemas%2Farm%2Fnxp%2Fimx8m-unique-id.yaml%23&amp
> ;data=0
> >
> +4%7C01%7Cpeng.fan%40nxp.com%7Ccd23b4c8f61c4cea5fec08d8809774b7
> %7C686e
> >
> +a1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637400735282634119%7C
> Unknown%7
> >
> +CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW
> wiLCJX
> >
> +VCI6Mn0%3D%7C1000&amp;sdata=I8IcySWYAmFhlm7xNoUyptxR2cxMAfY
> APv6bf%2Bp
> > +cHG8%3D&amp;reserved=0
> > +$schema:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=04%7C01%7Cpe
> ng.fan%
> >
> +40nxp.com%7Ccd23b4c8f61c4cea5fec08d8809774b7%7C686ea1d3bc2b4c6
> fa92cd9
> >
> +9c5c301635%7C0%7C0%7C637400735282634119%7CUnknown%7CTWFpb
> GZsb3d8eyJWI
> >
> +joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C1000&
> >
> +amp;sdata=m8T4UAOIn3fDhO5OUrXygT%2BRmwwZXQW9dC5zFQaXl9Q%3
> D&amp;reserv
> > +ed=0
> > +
> > +title: NXP i.MX8M Platforms Device Tree Bindings
> 
> This is not a title for these bindings. Please describe the bindings for this
> device. Based on description, this might could go to bindings/nvmem
> directory.

It might be misleading here. I think the bindings is for SoC, just like
"arm,realview-eb-soc" in
Documentation/devicetree/bindings/arm/arm,realview.yaml

Reading the patch, it is to convert soc-imx8m.c to platform driver,
so need to add a compatible string for the soc device,
I think Documentation/devicetree/bindings/arm/fsl.yaml should be
a better place.

Regards,
Peng.

> 
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> 
> No need for oneOf.
> 
> > +      - items:
> > +          - enum:
> > +              - fsl,imx8mm-soc
> > +              - fsl,imx8mn-soc
> > +              - fsl,imx8mp-soc
> > +              - fsl,imx8mq-soc
> > +          - const: simple-bus
> > +
> > +  nvmem-cells:
> > +    maxItems: 1
> > +    description:
> > +      Reference to an nvmem node for the SOC Unique ID.
> 
> Misleading description - nvmem-cells do not contain a reference. Just skip it,
> nvmem-cells should be obvious from the nvmem provider bindings.
> 
> > +
> > +  nvmem-cells-names:
> > +    const: soc_unique_id
> 
> additionalProperties: false
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH v1 3/4] LF-2571-3 arm64: dts: imx8m: add nvmem-cell related stuff

[Krzysztof Kozlowski]

On Wed, 4 Nov 2020 at 04:12, Alice Guo <alice.guo@nxp.com> wrote:
>
> Add nvmem-cell related stuff for the soc unique ID.

Subject and commit msg: please do not add "stuff" but describe what
are you adding and why (what is the purpose, feature, benefit).
"Stuff" is too generic.

>
> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mm.dtsi | 6 ++++++
>  arch/arm64/boot/dts/freescale/imx8mn.dtsi | 6 ++++++
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 ++++++
>  arch/arm64/boot/dts/freescale/imx8mq.dtsi | 6 ++++++
>  4 files changed, 24 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> index ec71a5e8a062..b45dfe133ec7 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> @@ -261,6 +261,8 @@
>                 #address-cells = <1>;
>                 #size-cells = <1>;
>                 ranges = <0x0 0x0 0x0 0x3e000000>;
> +               nvmem-cells = <&imx8mm_uid>;
> +               nvmem-cell-names = "soc_unique_id";
>
>                 aips1: bus@30000000 {
>                         compatible = "fsl,aips-bus", "simple-bus";
> @@ -475,6 +477,10 @@
>                                 #address-cells = <1>;
>                                 #size-cells = <1>;
>
> +                               imx8mm_uid: unique_id@410 {
> +                                       reg = <4 8>;

Register addresses and sizes are by convention in hex.

Best regards,
Krzysztof

Re: [PATCH v1 1/4] LF-2571-1: dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID

[Krzysztof Kozlowski]

On Wed, 4 Nov 2020 at 09:04, Peng Fan <peng.fan@nxp.com> wrote:
> > > +ed=0
> > > +
> > > +title: NXP i.MX8M Platforms Device Tree Bindings
> >
> > This is not a title for these bindings. Please describe the bindings for this
> > device. Based on description, this might could go to bindings/nvmem
> > directory.
>
> It might be misleading here. I think the bindings is for SoC, just like
> "arm,realview-eb-soc" in
> Documentation/devicetree/bindings/arm/arm,realview.yaml
>
> Reading the patch, it is to convert soc-imx8m.c to platform driver,
> so need to add a compatible string for the soc device,
> I think Documentation/devicetree/bindings/arm/fsl.yaml should be
> a better place.

Thanks for clarification. Putting it with arm/fsl.yaml looks reasonable.

Best regards,
Krzysztof