Re: [PATCH v9 4/4] soc: imx8m: change to use platform driver

2020-12-22 Thread Fabio Estevam
Hi Alice,

On Tue, Dec 22, 2020 at 5:11 AM Alice Guo (OSS)  wrote:

> -   soc_uid = readl_relaxed(ocotp_base + OCOTP_UID_HIGH);
> -   soc_uid <<= 32;
> -   soc_uid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW);
> +   if (dev) {
> +   int ret = 0;

No need to initialize ret to 0.

> -   imx8mm_soc_uid();
> +   if (dev) {
> +   int ret = 0;

Ditto.


> +/* Retain device_initcall is for backward compatibility with DTS. */
> +static int __init imx8_soc_init(void)
> +{
> +   int ret = 0;

Ditto.

> +
> +   if (of_find_matching_node_and_match(NULL, imx8_soc_match, NULL))
> +   return 0;
> +
> +   ret = imx8_soc_info(NULL);
> +   return ret;

Or you could even get rid of 'ret' and return imx8_soc_info() directly.


[PATCH v9 4/4] soc: imx8m: change to use platform driver

2020-12-22 Thread Alice Guo (OSS)
From: 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.

Reviewed-by: Krzysztof Kozlowski 
Signed-off-by: Alice Guo 
---

Changes for v9:
 - re-add Reviewed-by because it was lost in v8
Changes for v8:
 - lost Reviewed-by carelessly
Changes for v7:
 - solve the problem "drivers/soc/imx/soc-imx8m.c:174:34: warning:
   unused variable"
Changes for v6:
 - leave only the changelog under '---'
Changes for v5:
 - add cleanup part "of_node_put"
 - add note to explain that why device_initcall still exists
Changes for v4:
 - delete "__maybe_unused"
 - delete MODULE_DEVICE_TABLE(of, imx8m_soc_match);
 - rename match table
   "fsl,imx8m*"is actually a machine compabile and "fsl,imx8m*-soc" is a
   compabile of soc@0
 - delete "flag" and change to determine whether the pointer is NULL
 - ues of_find_matching_node_and_match()
 - delete of_match_ptr()
Changes for v3:
 - keep the original way which uses device_initcall to read soc unique
   ID
 - add the other way which uses module_platform_driver and nvmem API, so
   that it will not break the old version DTBs
Changes for v2:
 - remove the subject prefix "LF-2571-4"

 drivers/soc/imx/soc-imx8m.c | 87 -
 1 file changed, 75 insertions(+), 12 deletions(-)

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

 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -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,7 +52,7 @@ 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 imx8mq_soc_revision(struct device *dev)
 {
struct device_node *np;
void __iomem *ocotp_base;
@@ -75,9 +77,20 @@ 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);
+   if (dev) {
+   int ret = 0;
+
+   ret = nvmem_cell_read_u64(dev, "soc_unique_id", &soc_uid);
+   if (ret) {
+   iounmap(ocotp_base);
+   of_node_put(np);
+   return ret;
+   }
+   } else {
+   soc_uid = readl_relaxed(ocotp_base + OCOTP_UID_HIGH);
+   soc_uid <<= 32;
+   soc_uid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW);
+   }

iounmap(ocotp_base);
of_node_put(np);
@@ -107,7 +120,7 @@ static void __init imx8mm_soc_uid(void)
of_node_put(np);
 }

-static u32 __init imx8mm_soc_revision(void)
+static u32 __init imx8mm_soc_revision(struct device *dev)
 {
struct device_node *np;
void __iomem *anatop_base;
@@ -125,7 +138,15 @@ static u32 __init imx8mm_soc_revision(void)
iounmap(anatop_base);
of_node_put(np);

-   imx8mm_soc_uid();
+   if (dev) {
+   int ret = 0;
+
+   ret = nvmem_cell_read_u64(dev, "soc_unique_id", &soc_uid);
+   if (ret)
+   return ret;
+   } else {
+   imx8mm_soc_uid();
+   }

return rev;
 }
@@ -150,7 +171,7 @@ static const struct imx8_soc_data imx8mp_soc_data = {
.soc_revision = imx8mm_soc_revision,
 };

-static __maybe_unused const struct of_device_id imx8_soc_match[] = {
+static __maybe_unused const struct of_device_id imx8_machine_match[] = {
{ .compatible = "fsl,imx8mq", .data = &imx8mq_soc_data, },
{ .compatible = "fsl,imx8mm", .data = &imx8mm_soc_data, },
{ .compatible = "fsl,imx8mn", .data = &imx8mn_soc_data, },
@@ -158,12 +179,20 @@ static __maybe_unused const struct of_device_id 
imx8_soc_match[] = {
{ }
 };

+static __maybe_unused const struct of_device_id imx8_soc_match[] = {
+   { .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_d