diff mbox series

[v6,1/4] dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID

Message ID 20201124015949.29262-1-alice.guo@nxp.com
State Changes Requested
Headers show
Series [v6,1/4] dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID | expand

Checks

Context Check Description
robh/dt-meta-schema success
robh/checkpatch success

Commit Message

Alice Guo Nov. 24, 2020, 1:59 a.m. UTC
Add DT Binding doc for the Unique ID of i.MX 8M series.

Signed-off-by: Alice Guo <alice.guo@nxp.com>
---

v2: remove the subject prefix "LF-2571-1"
v3: put it into Documentation/devicetree/bindings/arm/fsl.yaml
    modify the description of nvmem-cells
    use "make ARCH=arm64 dtbs_check" to test it and fix errors
v4: use allOf to limit new version DTS files for i.MX8M to include
    "fsl,imx8mm/n/p/q-soc", nvmem-cells and nvmem-cells-names
v5: correct the error of using allOf
v6: none

 .../devicetree/bindings/arm/fsl.yaml          | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)

--
2.17.1

Comments

Krzysztof Kozlowski Nov. 24, 2020, 9:46 a.m. UTC | #1
On Tue, Nov 24, 2020 at 09:59:46AM +0800, Alice Guo wrote:
> Add DT Binding doc for the Unique ID of i.MX 8M series.
> 
> Signed-off-by: Alice Guo <alice.guo@nxp.com>

I already reviewed it.

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof


> ---
> 
> v2: remove the subject prefix "LF-2571-1"
> v3: put it into Documentation/devicetree/bindings/arm/fsl.yaml
>     modify the description of nvmem-cells
>     use "make ARCH=arm64 dtbs_check" to test it and fix errors
> v4: use allOf to limit new version DTS files for i.MX8M to include
>     "fsl,imx8mm/n/p/q-soc", nvmem-cells and nvmem-cells-names
> v5: correct the error of using allOf
> v6: none
> 
>  .../devicetree/bindings/arm/fsl.yaml          | 47 +++++++++++++++++++
>  1 file changed, 47 insertions(+)
Krzysztof Kozlowski Nov. 24, 2020, 9:47 a.m. UTC | #2
On Tue, Nov 24, 2020 at 09:59:48AM +0800, Alice Guo wrote:
> In order to be able to use NVMEM APIs to read soc unique ID, add the
> nvmem data cell and name for nvmem-cells to the "soc" node, and add a
> nvmem node which provides soc unique ID to efuse@30350000.
> 
> Signed-off-by: Alice Guo <alice.guo@nxp.com>

I already reviewed it. Do not ignore received tags.

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

> ---
> 
> v2: remove the subject prefix "LF-2571-3"
> v3: convert register addresses and sizes to hex
> v4: delete "stuff" in subject and commit message, add detailed
>     description
> v5: change underscore of device node to hyphen
> v6: none
Krzysztof Kozlowski Nov. 24, 2020, 9:48 a.m. UTC | #3
On Tue, Nov 24, 2020 at 09:59:49AM +0800, Alice Guo wrote:
> 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>

I already reviewed it. You skipped all my review tags from v5.

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

> ---
> 
> v2: remove the subject prefix "LF-2571-4"
> v3: Keep the original way which uses device_initcall to read soc unique
>     ID, and add the other way which uses module_platform_driver and
>     nvmem API, so that it will not break the old version DTBs.
> v4: delete "__maybe_unused"
>     delete MODULE_DEVICE_TABLE(of, imx8m_soc_match);
>     rename match table, "fsl,imx8mm/n/q/p" is actually a machine
> compabile and "fsl,imx8mm/n/q/p-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()
> v5: add cleanup part "of_node_put"
>     add note to explain that why device_initcall still exists
> v6: none
Adam Ford Nov. 25, 2020, 12:44 a.m. UTC | #4
On Mon, Nov 23, 2020 at 8:04 PM Alice Guo <alice.guo@nxp.com> wrote:
>
> 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>
> ---
>
The patch reads V6, but the change log only shows V2.  Can you
elaborate on what has changed between V2 and V6?

adam

> v2: remove the subject prefix "LF-2571-4"
> v3: Keep the original way which uses device_initcall to read soc unique
>     ID, and add the other way which uses module_platform_driver and
>     nvmem API, so that it will not break the old version DTBs.
> v4: delete "__maybe_unused"
>     delete MODULE_DEVICE_TABLE(of, imx8m_soc_match);
>     rename match table, "fsl,imx8mm/n/q/p" is actually a machine
> compabile and "fsl,imx8mm/n/q/p-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()
> v5: add cleanup part "of_node_put"
>     add note to explain that why device_initcall still exists
> v6: none
>
>  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..250530177920 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,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 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 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_data, },
> +       { }
> +};
> +
>  #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_info(struct platform_device *pdev)
>  {
>         struct soc_device_attribute *soc_dev_attr;
>         struct soc_device *soc_dev;
> @@ -182,7 +211,10 @@ static int __init imx8_soc_init(void)
>         if (ret)
>                 goto free_soc;
>
> -       id = of_match_node(imx8_soc_match, of_root);
> +       if (pdev)
> +               id = of_match_node(imx8_soc_match, pdev->dev.of_node);
> +       else
> +               id = of_match_node(imx8_machine_match, of_root);
>         if (!id) {
>                 ret = -ENODEV;
>                 goto free_soc;
> @@ -191,8 +223,16 @@ static int __init imx8_soc_init(void)
>         data = id->data;
>         if (data) {
>                 soc_dev_attr->soc_id = data->name;
> -               if (data->soc_revision)
> -                       soc_rev = data->soc_revision();
> +               if (data->soc_revision) {
> +                       if (pdev) {
> +                               soc_rev = data->soc_revision(&pdev->dev);
> +                               ret = soc_rev;
> +                               if (ret < 0)
> +                                       goto free_soc;
> +                       } else {
> +                               soc_rev = data->soc_revision(NULL);
> +                       }
> +               }
>         }
>
>         soc_dev_attr->revision = imx8_revision(soc_rev);
> @@ -230,4 +270,27 @@ static int __init imx8_soc_init(void)
>         kfree(soc_dev_attr);
>         return ret;
>  }
> +
> +/* Retain device_initcall is for backward compatibility with DTS. */
> +static int __init imx8_soc_init(void)
> +{
> +       int ret = 0;
> +
> +       if (of_find_matching_node_and_match(NULL, imx8_soc_match, NULL))
> +               return 0;
> +
> +       ret = imx8_soc_info(NULL);
> +       return ret;
> +}
>  device_initcall(imx8_soc_init);
> +
> +static struct platform_driver imx8_soc_info_driver = {
> +       .probe = imx8_soc_info,
> +       .driver = {
> +               .name = "imx8_soc_info",
> +               .of_match_table = imx8_soc_match,
> +       },
> +};
> +
> +module_platform_driver(imx8_soc_info_driver);
> +MODULE_LICENSE("GPL v2");
> --
> 2.17.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Krzysztof Kozlowski Nov. 25, 2020, 7:33 a.m. UTC | #5
On Wed, 25 Nov 2020 at 01:44, Adam Ford <aford173@gmail.com> wrote:
>
> On Mon, Nov 23, 2020 at 8:04 PM Alice Guo <alice.guo@nxp.com> wrote:
> >
> > 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>
> > ---
> >
> The patch reads V6, but the change log only shows V2.  Can you
> elaborate on what has changed between V2 and V6?
>
> adam
>
> > v2: remove the subject prefix "LF-2571-4"
> > v3: Keep the original way which uses device_initcall to read soc unique
> >     ID, and add the other way which uses module_platform_driver and
> >     nvmem API, so that it will not break the old version DTBs.
> > v4: delete "__maybe_unused"
> >     delete MODULE_DEVICE_TABLE(of, imx8m_soc_match);
> >     rename match table, "fsl,imx8mm/n/q/p" is actually a machine
> > compabile and "fsl,imx8mm/n/q/p-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()
> > v5: add cleanup part "of_node_put"
> >     add note to explain that why device_initcall still exists
> > v6: none

Hi Adam,

It says up to v6, just in unnatural order... I was also surprised.

Best regards,
Krzysztof
Alice Guo Nov. 26, 2020, 2:15 a.m. UTC | #6
> -----Original Message-----
> From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> On
> Behalf Of Adam Ford
> Sent: 2020年11月25日 8:45
> To: Alice Guo <alice.guo@nxp.com>
> Cc: devicetree <devicetree@vger.kernel.org>; Peng Fan <peng.fan@nxp.com>;
> Sascha Hauer <s.hauer@pengutronix.de>; Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>; Rob
> Herring <robh+dt@kernel.org>; dl-linux-imx <linux-imx@nxp.com>; Shawn Guo
> <shawnguo@kernel.org>; arm-soc <linux-arm-kernel@lists.infradead.org>
> Subject: Re: [PATCH v6 4/4] soc: imx8m: change to use platform driver
> 
> On Mon, Nov 23, 2020 at 8:04 PM Alice Guo <alice.guo@nxp.com> wrote:
> >
> > 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>
> > ---
> >
> The patch reads V6, but the change log only shows V2.  Can you elaborate on
> what has changed between V2 and V6?
> 
> adam

Hi,

Sorry. The order of changlog is reversed. Thank Adam for pointing out the problem, and thank Krzysztof for reviewing my patch.
Do I need to resend the patchset with the correct changelog order?

Best regards,
Alice

> > v2: remove the subject prefix "LF-2571-4"
> > v3: Keep the original way which uses device_initcall to read soc unique
> >     ID, and add the other way which uses module_platform_driver and
> >     nvmem API, so that it will not break the old version DTBs.
> > v4: delete "__maybe_unused"
> >     delete MODULE_DEVICE_TABLE(of, imx8m_soc_match);
> >     rename match table, "fsl,imx8mm/n/q/p" is actually a machine
> > compabile and "fsl,imx8mm/n/q/p-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()
> > v5: add cleanup part "of_node_put"
> >     add note to explain that why device_initcall still exists
> > v6: none
> >
> >  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..250530177920 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,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 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 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_data, },
> > +       { }
> > +};
> > +
> >  #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_info(struct platform_device *pdev)
> >  {
> >         struct soc_device_attribute *soc_dev_attr;
> >         struct soc_device *soc_dev;
> > @@ -182,7 +211,10 @@ static int __init imx8_soc_init(void)
> >         if (ret)
> >                 goto free_soc;
> >
> > -       id = of_match_node(imx8_soc_match, of_root);
> > +       if (pdev)
> > +               id = of_match_node(imx8_soc_match,
> pdev->dev.of_node);
> > +       else
> > +               id = of_match_node(imx8_machine_match, of_root);
> >         if (!id) {
> >                 ret = -ENODEV;
> >                 goto free_soc;
> > @@ -191,8 +223,16 @@ static int __init imx8_soc_init(void)
> >         data = id->data;
> >         if (data) {
> >                 soc_dev_attr->soc_id = data->name;
> > -               if (data->soc_revision)
> > -                       soc_rev = data->soc_revision();
> > +               if (data->soc_revision) {
> > +                       if (pdev) {
> > +                               soc_rev =
> data->soc_revision(&pdev->dev);
> > +                               ret = soc_rev;
> > +                               if (ret < 0)
> > +                                       goto free_soc;
> > +                       } else {
> > +                               soc_rev = data->soc_revision(NULL);
> > +                       }
> > +               }
> >         }
> >
> >         soc_dev_attr->revision = imx8_revision(soc_rev); @@ -230,4
> > +270,27 @@ static int __init imx8_soc_init(void)
> >         kfree(soc_dev_attr);
> >         return ret;
> >  }
> > +
> > +/* Retain device_initcall is for backward compatibility with DTS. */
> > +static int __init imx8_soc_init(void) {
> > +       int ret = 0;
> > +
> > +       if (of_find_matching_node_and_match(NULL, imx8_soc_match,
> NULL))
> > +               return 0;
> > +
> > +       ret = imx8_soc_info(NULL);
> > +       return ret;
> > +}
> >  device_initcall(imx8_soc_init);
> > +
> > +static struct platform_driver imx8_soc_info_driver = {
> > +       .probe = imx8_soc_info,
> > +       .driver = {
> > +               .name = "imx8_soc_info",
> > +               .of_match_table = imx8_soc_match,
> > +       },
> > +};
> > +
> > +module_platform_driver(imx8_soc_info_driver);
> > +MODULE_LICENSE("GPL v2");
> > --
> > 2.17.1
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Krzysztof Kozlowski Nov. 26, 2020, 8:07 a.m. UTC | #7
On Thu, Nov 26, 2020 at 02:15:35AM +0000, Alice Guo wrote:
> 
> 
> > -----Original Message-----
> > From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> On
> > Behalf Of Adam Ford
> > Sent: 2020年11月25日 8:45
> > To: Alice Guo <alice.guo@nxp.com>
> > Cc: devicetree <devicetree@vger.kernel.org>; Peng Fan <peng.fan@nxp.com>;
> > Sascha Hauer <s.hauer@pengutronix.de>; Linux Kernel Mailing List
> > <linux-kernel@vger.kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>; Rob
> > Herring <robh+dt@kernel.org>; dl-linux-imx <linux-imx@nxp.com>; Shawn Guo
> > <shawnguo@kernel.org>; arm-soc <linux-arm-kernel@lists.infradead.org>
> > Subject: Re: [PATCH v6 4/4] soc: imx8m: change to use platform driver
> > 
> > On Mon, Nov 23, 2020 at 8:04 PM Alice Guo <alice.guo@nxp.com> wrote:
> > >
> > > 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>
> > > ---
> > >
> > The patch reads V6, but the change log only shows V2.  Can you elaborate on
> > what has changed between V2 and V6?
> > 
> > adam
> 
> Hi,
> 
> Sorry. The order of changlog is reversed. Thank Adam for pointing out the problem, and thank Krzysztof for reviewing my patch.
> Do I need to resend the patchset with the correct changelog order?

No, no need.

Best regards,
Krzysztof
Rob Herring Nov. 30, 2020, 9:57 p.m. UTC | #8
On Tue, Nov 24, 2020 at 09:59:46AM +0800, Alice Guo wrote:
> Add DT Binding doc for the Unique ID of i.MX 8M series.
> 
> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> ---
> 
> v2: remove the subject prefix "LF-2571-1"
> v3: put it into Documentation/devicetree/bindings/arm/fsl.yaml

No, I prefer this be a separate schema file and not clutter board/soc 
schemas with child nodes.

>     modify the description of nvmem-cells
>     use "make ARCH=arm64 dtbs_check" to test it and fix errors
> v4: use allOf to limit new version DTS files for i.MX8M to include
>     "fsl,imx8mm/n/p/q-soc", nvmem-cells and nvmem-cells-names
> v5: correct the error of using allOf
> v6: none
> 
>  .../devicetree/bindings/arm/fsl.yaml          | 47 +++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index 67980dcef66d..7132ffd41abb 100644
> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> @@ -918,6 +918,53 @@ properties:
>                - fsl,s32v234-evb           # S32V234-EVB2 Customer Evaluation Board
>            - const: fsl,s32v234
> 
> +  soc:
> +    type: object
> +    properties:
> +      compatible:
> +        oneOf:
> +          - description: new version compatible for i.MX8M SoCs
> +            items:
> +              - enum:
> +                  - fsl,imx8mm-soc
> +                  - fsl,imx8mn-soc
> +                  - fsl,imx8mp-soc
> +                  - fsl,imx8mq-soc
> +              - const: simple-bus
> +
> +          - description: old version compatible for i.MX8M SoCs
> +            items:
> +              - const: simple-bus

Fix your dts files and drop this.

> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - fsl,imx8mm
> +              - fsl,imx8mn
> +              - fsl,imx8mp
> +              - fsl,imx8mq
> +
> +    then:
> +      patternProperties:
> +        "^soc@[0-9a-f]+$":

And this is just wrong. First you say the node is 'soc' and then here it 
has a unit address.

> +          properties:
> +            compatible:
> +              items:
> +                - enum:
> +                    - fsl,imx8mm-soc
> +                    - fsl,imx8mn-soc
> +                    - fsl,imx8mp-soc
> +                    - fsl,imx8mq-soc
> +                - const: simple-bus
> +
> +          required:
> +            - compatible
> +            - nvmem-cells
> +            - nvmem-cell-names
> +
>  additionalProperties: true
> 
>  ...
> --
> 2.17.1
>
Alice Guo (OSS) Dec. 1, 2020, 3:31 a.m. UTC | #9
> -----Original Message-----
> From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> On
> Behalf Of Rob Herring
> Sent: 2020年12月1日 5:57
> To: Alice Guo <alice.guo@nxp.com>
> Cc: devicetree@vger.kernel.org; Peng Fan <peng.fan@nxp.com>;
> s.hauer@pengutronix.de; linux-kernel@vger.kernel.org; krzk@kernel.org;
> dl-linux-imx <linux-imx@nxp.com>; shawnguo@kernel.org;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH v6 1/4] dt-bindings: soc: imx8m: add DT Binding doc for soc
> unique ID
> 
> On Tue, Nov 24, 2020 at 09:59:46AM +0800, Alice Guo wrote:
> > Add DT Binding doc for the Unique ID of i.MX 8M series.
> >
> > Signed-off-by: Alice Guo <alice.guo@nxp.com>
> > ---
> >
> > v2: remove the subject prefix "LF-2571-1"
> > v3: put it into Documentation/devicetree/bindings/arm/fsl.yaml
> 
> No, I prefer this be a separate schema file and not clutter board/soc schemas
> with child nodes.

Hi,
Thank you for your comments. I read "Documentation/devicetree/bindings/arm/arm,realview.yaml"
in which there is a "soc". So I added my "soc" to this current file. Can I keep it in Documentation/devicetree/bindings/arm/fsl.yaml?

> >     modify the description of nvmem-cells
> >     use "make ARCH=arm64 dtbs_check" to test it and fix errors
> > v4: use allOf to limit new version DTS files for i.MX8M to include
> >     "fsl,imx8mm/n/p/q-soc", nvmem-cells and nvmem-cells-names
> > v5: correct the error of using allOf
> > v6: none
> >
> >  .../devicetree/bindings/arm/fsl.yaml          | 47
> +++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml
> > b/Documentation/devicetree/bindings/arm/fsl.yaml
> > index 67980dcef66d..7132ffd41abb 100644
> > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > @@ -918,6 +918,53 @@ properties:
> >                - fsl,s32v234-evb           # S32V234-EVB2 Customer
> Evaluation Board
> >            - const: fsl,s32v234
> >
> > +  soc:
> > +    type: object
> > +    properties:
> > +      compatible:
> > +        oneOf:
> > +          - description: new version compatible for i.MX8M SoCs
> > +            items:
> > +              - enum:
> > +                  - fsl,imx8mm-soc
> > +                  - fsl,imx8mn-soc
> > +                  - fsl,imx8mp-soc
> > +                  - fsl,imx8mq-soc
> > +              - const: simple-bus
> > +
> > +          - description: old version compatible for i.MX8M SoCs
> > +            items:
> > +              - const: simple-bus
> 
> Fix your dts files and drop this.

My changes are below.

> 
> > +
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - fsl,imx8mm
> > +              - fsl,imx8mn
> > +              - fsl,imx8mp
> > +              - fsl,imx8mq
> > +
> > +    then:
> > +      patternProperties:
> > +        "^soc@[0-9a-f]+$":
> 
> And this is just wrong. First you say the node is 'soc' and then here it has a unit
> address.

Here are my changes. I deleted the section from "soc:" to "- const: simple bus". Please help me to see if they are correct and workable. Thank you.
allOf:
  - if:
      properties:
        compatible:
          contains:
            enum:
              - fsl,imx8mm
              - fsl,imx8mn
              - fsl,imx8mp
              - fsl,imx8mq

    then:
      patternProperties:
        "^soc@[0-9a-f]+$":
          properties:
            compatible:
              items:
                - enum:
                    - fsl,imx8mm-soc
                    - fsl,imx8mn-soc
                    - fsl,imx8mp-soc
                    - fsl,imx8mq-soc
                - const: simple-bus

          required:
            - compatible
            - nvmem-cells
            - nvmem-cell-names

Best Regards,
Alice Guo


> 
> > +          properties:
> > +            compatible:
> > +              items:
> > +                - enum:
> > +                    - fsl,imx8mm-soc
> > +                    - fsl,imx8mn-soc
> > +                    - fsl,imx8mp-soc
> > +                    - fsl,imx8mq-soc
> > +                - const: simple-bus
> > +
> > +          required:
> > +            - compatible
> > +            - nvmem-cells
> > +            - nvmem-cell-names
> > +
> >  additionalProperties: true
> >
> >  ...
> > --
> > 2.17.1
> >
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Alice Guo (OSS) Dec. 9, 2020, 2:30 a.m. UTC | #10
Gentle ping..  and Krzysztof Kozlowski, do you agree?

Best Regards,
Alice Guo

> -----Original Message-----
> From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> On
> Behalf Of Alice Guo (OSS)
> Sent: 2020年12月1日 11:31
> To: Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>;
> shawnguo@kernel.org
> Cc: devicetree@vger.kernel.org; Peng Fan <peng.fan@nxp.com>;
> s.hauer@pengutronix.de; linux-kernel@vger.kernel.org; krzk@kernel.org;
> dl-linux-imx <linux-imx@nxp.com>; linux-arm-kernel@lists.infradead.org
> Subject: RE: [PATCH v6 1/4] dt-bindings: soc: imx8m: add DT Binding doc for soc
> unique ID
> 
> 
> 
> > -----Original Message-----
> > From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org>
> > On Behalf Of Rob Herring
> > Sent: 2020年12月1日 5:57
> > To: Alice Guo <alice.guo@nxp.com>
> > Cc: devicetree@vger.kernel.org; Peng Fan <peng.fan@nxp.com>;
> > s.hauer@pengutronix.de; linux-kernel@vger.kernel.org; krzk@kernel.org;
> > dl-linux-imx <linux-imx@nxp.com>; shawnguo@kernel.org;
> > linux-arm-kernel@lists.infradead.org
> > Subject: Re: [PATCH v6 1/4] dt-bindings: soc: imx8m: add DT Binding
> > doc for soc unique ID
> >
> > On Tue, Nov 24, 2020 at 09:59:46AM +0800, Alice Guo wrote:
> > > Add DT Binding doc for the Unique ID of i.MX 8M series.
> > >
> > > Signed-off-by: Alice Guo <alice.guo@nxp.com>
> > > ---
> > >
> > > v2: remove the subject prefix "LF-2571-1"
> > > v3: put it into Documentation/devicetree/bindings/arm/fsl.yaml
> >
> > No, I prefer this be a separate schema file and not clutter board/soc
> > schemas with child nodes.
> 
> Hi,
> Thank you for your comments. I read
> "Documentation/devicetree/bindings/arm/arm,realview.yaml"
> in which there is a "soc". So I added my "soc" to this current file. Can I keep it in
> Documentation/devicetree/bindings/arm/fsl.yaml?
> 
> > >     modify the description of nvmem-cells
> > >     use "make ARCH=arm64 dtbs_check" to test it and fix errors
> > > v4: use allOf to limit new version DTS files for i.MX8M to include
> > >     "fsl,imx8mm/n/p/q-soc", nvmem-cells and nvmem-cells-names
> > > v5: correct the error of using allOf
> > > v6: none
> > >
> > >  .../devicetree/bindings/arm/fsl.yaml          | 47
> > +++++++++++++++++++
> > >  1 file changed, 47 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml
> > > b/Documentation/devicetree/bindings/arm/fsl.yaml
> > > index 67980dcef66d..7132ffd41abb 100644
> > > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > > @@ -918,6 +918,53 @@ properties:
> > >                - fsl,s32v234-evb           # S32V234-EVB2 Customer
> > Evaluation Board
> > >            - const: fsl,s32v234
> > >
> > > +  soc:
> > > +    type: object
> > > +    properties:
> > > +      compatible:
> > > +        oneOf:
> > > +          - description: new version compatible for i.MX8M SoCs
> > > +            items:
> > > +              - enum:
> > > +                  - fsl,imx8mm-soc
> > > +                  - fsl,imx8mn-soc
> > > +                  - fsl,imx8mp-soc
> > > +                  - fsl,imx8mq-soc
> > > +              - const: simple-bus
> > > +
> > > +          - description: old version compatible for i.MX8M SoCs
> > > +            items:
> > > +              - const: simple-bus
> >
> > Fix your dts files and drop this.
> 
> My changes are below.
> 
> >
> > > +
> > > +allOf:
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            enum:
> > > +              - fsl,imx8mm
> > > +              - fsl,imx8mn
> > > +              - fsl,imx8mp
> > > +              - fsl,imx8mq
> > > +
> > > +    then:
> > > +      patternProperties:
> > > +        "^soc@[0-9a-f]+$":
> >
> > And this is just wrong. First you say the node is 'soc' and then here
> > it has a unit address.
> 
> Here are my changes. I deleted the section from "soc:" to "- const: simple bus".
> Please help me to see if they are correct and workable. Thank you.
> allOf:
>   - if:
>       properties:
>         compatible:
>           contains:
>             enum:
>               - fsl,imx8mm
>               - fsl,imx8mn
>               - fsl,imx8mp
>               - fsl,imx8mq
> 
>     then:
>       patternProperties:
>         "^soc@[0-9a-f]+$":
>           properties:
>             compatible:
>               items:
>                 - enum:
>                     - fsl,imx8mm-soc
>                     - fsl,imx8mn-soc
>                     - fsl,imx8mp-soc
>                     - fsl,imx8mq-soc
>                 - const: simple-bus
> 
>           required:
>             - compatible
>             - nvmem-cells
>             - nvmem-cell-names
> 
> Best Regards,
> Alice Guo
> 
> 
> >
> > > +          properties:
> > > +            compatible:
> > > +              items:
> > > +                - enum:
> > > +                    - fsl,imx8mm-soc
> > > +                    - fsl,imx8mn-soc
> > > +                    - fsl,imx8mp-soc
> > > +                    - fsl,imx8mq-soc
> > > +                - const: simple-bus
> > > +
> > > +          required:
> > > +            - compatible
> > > +            - nvmem-cells
> > > +            - nvmem-cell-names
> > > +
> > >  additionalProperties: true
> > >
> > >  ...
> > > --
> > > 2.17.1
> > >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Krzysztof Kozlowski Dec. 9, 2020, 7:43 a.m. UTC | #11
On Wed, Dec 09, 2020 at 02:30:22AM +0000, Alice Guo (OSS) wrote:
> Gentle ping..  and Krzysztof Kozlowski, do you agree?

I did not know that you wait for something from my side.

> 
> Best Regards,
> Alice Guo
> 
> > -----Original Message-----
> > From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> On
> > Behalf Of Alice Guo (OSS)
> > Sent: 2020年12月1日 11:31
> > To: Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk@kernel.org>;
> > shawnguo@kernel.org
> > Cc: devicetree@vger.kernel.org; Peng Fan <peng.fan@nxp.com>;
> > s.hauer@pengutronix.de; linux-kernel@vger.kernel.org; krzk@kernel.org;
> > dl-linux-imx <linux-imx@nxp.com>; linux-arm-kernel@lists.infradead.org
> > Subject: RE: [PATCH v6 1/4] dt-bindings: soc: imx8m: add DT Binding doc for soc
> > unique ID
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org>
> > > On Behalf Of Rob Herring
> > > Sent: 2020年12月1日 5:57
> > > To: Alice Guo <alice.guo@nxp.com>
> > > Cc: devicetree@vger.kernel.org; Peng Fan <peng.fan@nxp.com>;
> > > s.hauer@pengutronix.de; linux-kernel@vger.kernel.org; krzk@kernel.org;
> > > dl-linux-imx <linux-imx@nxp.com>; shawnguo@kernel.org;
> > > linux-arm-kernel@lists.infradead.org
> > > Subject: Re: [PATCH v6 1/4] dt-bindings: soc: imx8m: add DT Binding
> > > doc for soc unique ID
> > >
> > > On Tue, Nov 24, 2020 at 09:59:46AM +0800, Alice Guo wrote:
> > > > Add DT Binding doc for the Unique ID of i.MX 8M series.
> > > >
> > > > Signed-off-by: Alice Guo <alice.guo@nxp.com>
> > > > ---
> > > >
> > > > v2: remove the subject prefix "LF-2571-1"
> > > > v3: put it into Documentation/devicetree/bindings/arm/fsl.yaml
> > >
> > > No, I prefer this be a separate schema file and not clutter board/soc
> > > schemas with child nodes.
> > 
> > Hi,
> > Thank you for your comments. I read
> > "Documentation/devicetree/bindings/arm/arm,realview.yaml"
> > in which there is a "soc". So I added my "soc" to this current file. Can I keep it in
> > Documentation/devicetree/bindings/arm/fsl.yaml?

Please go with Rob's suggestion.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
index 67980dcef66d..7132ffd41abb 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -918,6 +918,53 @@  properties:
               - fsl,s32v234-evb           # S32V234-EVB2 Customer Evaluation Board
           - const: fsl,s32v234

+  soc:
+    type: object
+    properties:
+      compatible:
+        oneOf:
+          - description: new version compatible for i.MX8M SoCs
+            items:
+              - enum:
+                  - fsl,imx8mm-soc
+                  - fsl,imx8mn-soc
+                  - fsl,imx8mp-soc
+                  - fsl,imx8mq-soc
+              - const: simple-bus
+
+          - description: old version compatible for i.MX8M SoCs
+            items:
+              - const: simple-bus
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - fsl,imx8mm
+              - fsl,imx8mn
+              - fsl,imx8mp
+              - fsl,imx8mq
+
+    then:
+      patternProperties:
+        "^soc@[0-9a-f]+$":
+          properties:
+            compatible:
+              items:
+                - enum:
+                    - fsl,imx8mm-soc
+                    - fsl,imx8mn-soc
+                    - fsl,imx8mp-soc
+                    - fsl,imx8mq-soc
+                - const: simple-bus
+
+          required:
+            - compatible
+            - nvmem-cells
+            - nvmem-cell-names
+
 additionalProperties: true

 ...