| Message ID | 20201124015949.29262-1-alice.guo@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v6,1/4] dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID | expand |
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
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
> -----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
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
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 >
> -----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
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
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 --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 ...
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