Re: [PATCH v6 4/4] soc: imx8m: change to use platform driver
Hi Alice,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on v5.10-rc6 next-20201201]
[cannot apply to shawnguo/for-next krzk/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Alice-Guo/dt-bindings-soc-imx8m-add-DT-Binding-doc-for-soc-unique-ID/20201124-100402
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: mips-randconfig-r026-20201201 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project
2671fccf0381769276ca8246ec0499adcb9b0355)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install mips cross compiling tool for clang build
# apt-get install binutils-mips-linux-gnu
#
https://github.com/0day-ci/linux/commit/410e3191d62767ba3d347a48d38ad3bd00bee387
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Alice-Guo/dt-bindings-soc-imx8m-add-DT-Binding-doc-for-soc-unique-ID/20201124-100402
git checkout 410e3191d62767ba3d347a48d38ad3bd00bee387
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> drivers/soc/imx/soc-imx8m.c:174:34: warning: unused variable
>> 'imx8_machine_match' [-Wunused-const-variable]
static const struct of_device_id imx8_machine_match[] = {
^
1 warning generated.
vim +/imx8_machine_match +174 drivers/soc/imx/soc-imx8m.c
173
> 174 static const struct of_device_id imx8_machine_match[] = {
175 { .compatible = "fsl,imx8mq", .data = _soc_data, },
176 { .compatible = "fsl,imx8mm", .data = _soc_data, },
177 { .compatible = "fsl,imx8mn", .data = _soc_data, },
178 { .compatible = "fsl,imx8mp", .data = _soc_data, },
179 { }
180 };
181
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH v6 4/4] soc: imx8m: change to use platform driver
On Thu, Nov 26, 2020 at 02:15:35AM +, Alice Guo wrote: > > > > -Original Message- > > From: linux-arm-kernel On > > Behalf Of Adam Ford > > Sent: 2020年11月25日 8:45 > > To: Alice Guo > > Cc: devicetree ; Peng Fan ; > > Sascha Hauer ; Linux Kernel Mailing List > > ; Krzysztof Kozlowski ; Rob > > Herring ; dl-linux-imx ; Shawn Guo > > ; arm-soc > > Subject: Re: [PATCH v6 4/4] soc: imx8m: change to use platform driver > > > > On Mon, Nov 23, 2020 at 8:04 PM 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 > > > --- > > > > > 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
RE: [PATCH v6 4/4] soc: imx8m: change to use platform driver
> -Original Message-
> From: linux-arm-kernel On
> Behalf Of Adam Ford
> Sent: 2020年11月25日 8:45
> To: Alice Guo
> Cc: devicetree ; Peng Fan ;
> Sascha Hauer ; Linux Kernel Mailing List
> ; Krzysztof Kozlowski ; Rob
> Herring ; dl-linux-imx ; Shawn Guo
> ; arm-soc
> Subject: Re: [PATCH v6 4/4] soc: imx8m: change to use platform driver
>
> On Mon, Nov 23, 2020 at 8:04 PM 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
> > ---
> >
> 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
> > #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",
> _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_rev
Re: [PATCH v6 4/4] soc: imx8m: change to use platform driver
On Wed, 25 Nov 2020 at 01:44, Adam Ford wrote: > > On Mon, Nov 23, 2020 at 8:04 PM 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 > > --- > > > 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
Re: [PATCH v6 4/4] soc: imx8m: change to use platform driver
On Mon, Nov 23, 2020 at 8:04 PM 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
> ---
>
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
> #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", _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", _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 = _soc_data, },
> { .compatible = "fsl,imx8mm", .data = _soc_data, },
> { .compatible = "fsl,imx8mn", .data = _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 = _soc_data, },
> + { .compatible = "fsl,imx8mm-soc", .data = _soc_data, },
> + { .compatible = "fsl,imx8mn-soc", .data = _soc_data, },
> + { .compatible = "fsl,imx8mp-soc", .data = _soc_data, },
> + {
Re: [PATCH v6 4/4] soc: imx8m: change to use platform driver
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 I already reviewed it. You skipped all my review tags from v5. Reviewed-by: Krzysztof Kozlowski 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
[PATCH v6 4/4] soc: imx8m: change to use platform driver
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
---
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
#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", _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", _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 = _soc_data, },
{ .compatible = "fsl,imx8mm", .data = _soc_data, },
{ .compatible = "fsl,imx8mn", .data = _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 = _soc_data, },
+ { .compatible = "fsl,imx8mm-soc", .data = _soc_data, },
+ { .compatible = "fsl,imx8mn-soc", .data = _soc_data, },
+ { .compatible = "fsl,imx8mp-soc", .data = _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)
