Message ID | 1592216485-20574-1-git-send-email-peng.fan@nxp.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | soc: imx: Kconfig: force using OF when COMPILE_TEST | expand |
> From: Peng Fan <peng.fan@nxp.com> > Sent: Monday, June 15, 2020 6:21 PM > > Fix the build warning with x86_64-randconfig > >> drivers/soc/imx/soc-imx8m.c:150:34: warning: unused variable > >> 'imx8_soc_match' [-Wunused-const-variable] > static const struct of_device_id imx8_soc_match[] = { ^ > > Fixes: fc40200ebf82 ("soc: imx: increase build coverage for imx8m soc driver") > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Peng Fan <peng.fan@nxp.com> Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com> Regards Aisheng
On Mon, Jun 15, 2020 at 06:21:25PM +0800, peng.fan@nxp.com wrote: > From: Peng Fan <peng.fan@nxp.com> > > Fix the build warning with x86_64-randconfig > >> drivers/soc/imx/soc-imx8m.c:150:34: warning: unused variable > >> 'imx8_soc_match' [-Wunused-const-variable] > static const struct of_device_id imx8_soc_match[] = { ^ > > Fixes: fc40200ebf82 ("soc: imx: increase build coverage for imx8m soc driver") > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > drivers/soc/imx/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/soc/imx/Kconfig b/drivers/soc/imx/Kconfig > index d515d2cc20ed..aadf13c9d396 100644 > --- a/drivers/soc/imx/Kconfig > +++ b/drivers/soc/imx/Kconfig > @@ -19,7 +19,7 @@ config IMX_SCU_SOC > > config SOC_IMX8M > bool "i.MX8M SoC family support" > - depends on ARCH_MXC || COMPILE_TEST > + depends on ARCH_MXC || (COMPILE_TEST && OF) A bit prettier (IMHO) would be: config SOC_IMX8M bool "i.MX8M SoC family support" depends on ARCH_MXC || COMPILE_TEST + depends on OF default ARCH_MXC && ARM64 select SOC_BUS help Best regards Uwe
On 2020-06-16 16:59, Uwe Kleine-König wrote: > On Mon, Jun 15, 2020 at 06:21:25PM +0800, peng.fan@nxp.com wrote: >> From: Peng Fan <peng.fan@nxp.com> >> >> Fix the build warning with x86_64-randconfig >>>> drivers/soc/imx/soc-imx8m.c:150:34: warning: unused variable >>>> 'imx8_soc_match' [-Wunused-const-variable] >> static const struct of_device_id imx8_soc_match[] = { ^ >> >> Fixes: fc40200ebf82 ("soc: imx: increase build coverage for imx8m soc driver") >> Reported-by: kernel test robot <lkp@intel.com> >> Signed-off-by: Peng Fan <peng.fan@nxp.com> >> --- >> drivers/soc/imx/Kconfig | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/soc/imx/Kconfig b/drivers/soc/imx/Kconfig >> index d515d2cc20ed..aadf13c9d396 100644 >> --- a/drivers/soc/imx/Kconfig >> +++ b/drivers/soc/imx/Kconfig >> @@ -19,7 +19,7 @@ config IMX_SCU_SOC >> >> config SOC_IMX8M >> bool "i.MX8M SoC family support" >> - depends on ARCH_MXC || COMPILE_TEST >> + depends on ARCH_MXC || (COMPILE_TEST && OF) > > A bit prettier (IMHO) would be: > > config SOC_IMX8M > bool "i.MX8M SoC family support" > depends on ARCH_MXC || COMPILE_TEST > + depends on OF > default ARCH_MXC && ARM64 > select SOC_BUS > help That's not just prettier, it's logically correct. If some code needs OF to build correctly, then it depends on OF, regardless of *why* it's being built at any given time. That said, if the only issue in this particular case is the warning above, then it's hardly a real dependency; simply marking the of_device_id data as __maybe_unused (like various other drivers do) should suffice. Limiting COMPILE_TEST coverage instead of actually fixing simple issues that it exposes seems a bit backwards... Robin.
> Subject: Re: [PATCH] soc: imx: Kconfig: force using OF when COMPILE_TEST > > On 2020-06-16 16:59, Uwe Kleine-König wrote: > > On Mon, Jun 15, 2020 at 06:21:25PM +0800, peng.fan@nxp.com wrote: > >> From: Peng Fan <peng.fan@nxp.com> > >> > >> Fix the build warning with x86_64-randconfig > >>>> drivers/soc/imx/soc-imx8m.c:150:34: warning: unused variable > >>>> 'imx8_soc_match' [-Wunused-const-variable] > >> static const struct of_device_id imx8_soc_match[] = { ^ > >> > >> Fixes: fc40200ebf82 ("soc: imx: increase build coverage for imx8m soc > >> driver") > >> Reported-by: kernel test robot <lkp@intel.com> > >> Signed-off-by: Peng Fan <peng.fan@nxp.com> > >> --- > >> drivers/soc/imx/Kconfig | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/soc/imx/Kconfig b/drivers/soc/imx/Kconfig index > >> d515d2cc20ed..aadf13c9d396 100644 > >> --- a/drivers/soc/imx/Kconfig > >> +++ b/drivers/soc/imx/Kconfig > >> @@ -19,7 +19,7 @@ config IMX_SCU_SOC > >> > >> config SOC_IMX8M > >> bool "i.MX8M SoC family support" > >> - depends on ARCH_MXC || COMPILE_TEST > >> + depends on ARCH_MXC || (COMPILE_TEST && OF) > > > > A bit prettier (IMHO) would be: > > > > config SOC_IMX8M > > bool "i.MX8M SoC family support" > > depends on ARCH_MXC || COMPILE_TEST > > + depends on OF > > default ARCH_MXC && ARM64 > > select SOC_BUS > > help > > That's not just prettier, it's logically correct. If some code needs OF to build > correctly, then it depends on OF, regardless of *why* it's being built at any > given time. > > That said, if the only issue in this particular case is the warning above, then it's > hardly a real dependency; simply marking the of_device_id data as > __maybe_unused (like various other drivers do) should suffice. Limiting > COMPILE_TEST coverage instead of actually fixing simple issues that it > exposes seems a bit backwards... ok, I'll use __maybe_unused in v2, but it actually not make sense to compile this file for x86_64. Thanks, Peng. > > Robin.
On Wed, Jun 17, 2020 at 02:05:33AM +0000, Peng Fan wrote: > > Subject: Re: [PATCH] soc: imx: Kconfig: force using OF when COMPILE_TEST > > > > On 2020-06-16 16:59, Uwe Kleine-König wrote: > > > On Mon, Jun 15, 2020 at 06:21:25PM +0800, peng.fan@nxp.com wrote: > > >> From: Peng Fan <peng.fan@nxp.com> > > >> > > >> Fix the build warning with x86_64-randconfig > > >>>> drivers/soc/imx/soc-imx8m.c:150:34: warning: unused variable > > >>>> 'imx8_soc_match' [-Wunused-const-variable] > > >> static const struct of_device_id imx8_soc_match[] = { ^ > > >> > > >> Fixes: fc40200ebf82 ("soc: imx: increase build coverage for imx8m soc > > >> driver") > > >> Reported-by: kernel test robot <lkp@intel.com> > > >> Signed-off-by: Peng Fan <peng.fan@nxp.com> > > >> --- > > >> drivers/soc/imx/Kconfig | 2 +- > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > >> > > >> diff --git a/drivers/soc/imx/Kconfig b/drivers/soc/imx/Kconfig index > > >> d515d2cc20ed..aadf13c9d396 100644 > > >> --- a/drivers/soc/imx/Kconfig > > >> +++ b/drivers/soc/imx/Kconfig > > >> @@ -19,7 +19,7 @@ config IMX_SCU_SOC > > >> > > >> config SOC_IMX8M > > >> bool "i.MX8M SoC family support" > > >> - depends on ARCH_MXC || COMPILE_TEST > > >> + depends on ARCH_MXC || (COMPILE_TEST && OF) > > > > > > A bit prettier (IMHO) would be: > > > > > > config SOC_IMX8M > > > bool "i.MX8M SoC family support" > > > depends on ARCH_MXC || COMPILE_TEST > > > + depends on OF > > > default ARCH_MXC && ARM64 > > > select SOC_BUS > > > help > > > > That's not just prettier, it's logically correct. If some code needs OF to build > > correctly, then it depends on OF, regardless of *why* it's being built at any > > given time. > > > > That said, if the only issue in this particular case is the warning above, then it's > > hardly a real dependency; simply marking the of_device_id data as > > __maybe_unused (like various other drivers do) should suffice. Limiting > > COMPILE_TEST coverage instead of actually fixing simple issues that it > > exposes seems a bit backwards... > > ok, I'll use __maybe_unused in v2, but it actually not make sense to compile > this file for x86_64. This is wrong in a subtle way. It makes sense to compile the driver on x86_64 (and other archs) for QA reasons. It doesn't make sense to use a kernel on an x86 machine with this driver enabled. That's why we have COMPILE_TEST which you disable if you want to create a kernel to actually run it. Best regards Uwe
On Wed, Jun 17, 2020 at 02:05:33AM +0000, Peng Fan wrote: > > Subject: Re: [PATCH] soc: imx: Kconfig: force using OF when COMPILE_TEST > > > > On 2020-06-16 16:59, Uwe Kleine-König wrote: > > > On Mon, Jun 15, 2020 at 06:21:25PM +0800, peng.fan@nxp.com wrote: > > >> From: Peng Fan <peng.fan@nxp.com> > > >> > > >> Fix the build warning with x86_64-randconfig > > >>>> drivers/soc/imx/soc-imx8m.c:150:34: warning: unused variable > > >>>> 'imx8_soc_match' [-Wunused-const-variable] > > >> static const struct of_device_id imx8_soc_match[] = { ^ > > >> > > >> Fixes: fc40200ebf82 ("soc: imx: increase build coverage for imx8m soc > > >> driver") > > >> Reported-by: kernel test robot <lkp@intel.com> > > >> Signed-off-by: Peng Fan <peng.fan@nxp.com> > > >> --- > > >> drivers/soc/imx/Kconfig | 2 +- > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > >> > > >> diff --git a/drivers/soc/imx/Kconfig b/drivers/soc/imx/Kconfig index > > >> d515d2cc20ed..aadf13c9d396 100644 > > >> --- a/drivers/soc/imx/Kconfig > > >> +++ b/drivers/soc/imx/Kconfig > > >> @@ -19,7 +19,7 @@ config IMX_SCU_SOC > > >> > > >> config SOC_IMX8M > > >> bool "i.MX8M SoC family support" > > >> - depends on ARCH_MXC || COMPILE_TEST > > >> + depends on ARCH_MXC || (COMPILE_TEST && OF) > > > > > > A bit prettier (IMHO) would be: > > > > > > config SOC_IMX8M > > > bool "i.MX8M SoC family support" > > > depends on ARCH_MXC || COMPILE_TEST > > > + depends on OF > > > default ARCH_MXC && ARM64 > > > select SOC_BUS > > > help > > > > That's not just prettier, it's logically correct. If some code needs OF to build > > correctly, then it depends on OF, regardless of *why* it's being built at any > > given time. > > > > That said, if the only issue in this particular case is the warning above, then it's > > hardly a real dependency; simply marking the of_device_id data as > > __maybe_unused (like various other drivers do) should suffice. Limiting > > COMPILE_TEST coverage instead of actually fixing simple issues that it > > exposes seems a bit backwards... > > ok, I'll use __maybe_unused in v2, Has v2 been posted, or am I missing it? Shawn > but it actually not make sense to compile > this file for x86_64.
> Subject: Re: [PATCH] soc: imx: Kconfig: force using OF when COMPILE_TEST > > On Wed, Jun 17, 2020 at 02:05:33AM +0000, Peng Fan wrote: > > > Subject: Re: [PATCH] soc: imx: Kconfig: force using OF when > > > COMPILE_TEST > > > > > > On 2020-06-16 16:59, Uwe Kleine-König wrote: > > > > On Mon, Jun 15, 2020 at 06:21:25PM +0800, peng.fan@nxp.com wrote: > > > >> From: Peng Fan <peng.fan@nxp.com> > > > >> > > > >> Fix the build warning with x86_64-randconfig > > > >>>> drivers/soc/imx/soc-imx8m.c:150:34: warning: unused variable > > > >>>> 'imx8_soc_match' [-Wunused-const-variable] > > > >> static const struct of_device_id imx8_soc_match[] = { ^ > > > >> > > > >> Fixes: fc40200ebf82 ("soc: imx: increase build coverage for imx8m > > > >> soc > > > >> driver") > > > >> Reported-by: kernel test robot <lkp@intel.com> > > > >> Signed-off-by: Peng Fan <peng.fan@nxp.com> > > > >> --- > > > >> drivers/soc/imx/Kconfig | 2 +- > > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > > >> > > > >> diff --git a/drivers/soc/imx/Kconfig b/drivers/soc/imx/Kconfig > > > >> index > > > >> d515d2cc20ed..aadf13c9d396 100644 > > > >> --- a/drivers/soc/imx/Kconfig > > > >> +++ b/drivers/soc/imx/Kconfig > > > >> @@ -19,7 +19,7 @@ config IMX_SCU_SOC > > > >> > > > >> config SOC_IMX8M > > > >> bool "i.MX8M SoC family support" > > > >> - depends on ARCH_MXC || COMPILE_TEST > > > >> + depends on ARCH_MXC || (COMPILE_TEST && OF) > > > > > > > > A bit prettier (IMHO) would be: > > > > > > > > config SOC_IMX8M > > > > bool "i.MX8M SoC family support" > > > > depends on ARCH_MXC || COMPILE_TEST > > > > + depends on OF > > > > default ARCH_MXC && ARM64 > > > > select SOC_BUS > > > > help > > > > > > That's not just prettier, it's logically correct. If some code needs > > > OF to build correctly, then it depends on OF, regardless of *why* > > > it's being built at any given time. > > > > > > That said, if the only issue in this particular case is the warning > > > above, then it's hardly a real dependency; simply marking the > > > of_device_id data as __maybe_unused (like various other drivers do) > > > should suffice. Limiting COMPILE_TEST coverage instead of actually > > > fixing simple issues that it exposes seems a bit backwards... > > > > ok, I'll use __maybe_unused in v2, but it actually not make sense to > > compile this file for x86_64. > > This is wrong in a subtle way. It makes sense to compile the driver on > x86_64 (and other archs) for QA reasons. It doesn't make sense to use a > kernel on an x86 machine with this driver enabled. That's why we have > COMPILE_TEST which you disable if you want to create a kernel to actually > run it. Shawn, What's your idea? I am not very clear on which path to go. Thanks, Peng. > > Best regards > Uwe > > > -- > Pengutronix e.K. | Uwe Kleine-König > | > Industrial Linux Solutions | https://www.pengutronix.de/ > |
On Wed, Jun 24, 2020 at 01:54:42AM +0000, Peng Fan wrote: > > > ok, I'll use __maybe_unused in v2, but it actually not make sense to > > > compile this file for x86_64. > > > > This is wrong in a subtle way. It makes sense to compile the driver on > > x86_64 (and other archs) for QA reasons. It doesn't make sense to use a > > kernel on an x86 machine with this driver enabled. That's why we have > > COMPILE_TEST which you disable if you want to create a kernel to actually > > run it. > > Shawn, > > What's your idea? I am not very clear on which path to go. I think we agreed on __maybe_unused. What Uwe was questioning is the second part of your statement - "it actually not make sense to compile this file for x86_64." Shawn
diff --git a/drivers/soc/imx/Kconfig b/drivers/soc/imx/Kconfig index d515d2cc20ed..aadf13c9d396 100644 --- a/drivers/soc/imx/Kconfig +++ b/drivers/soc/imx/Kconfig @@ -19,7 +19,7 @@ config IMX_SCU_SOC config SOC_IMX8M bool "i.MX8M SoC family support" - depends on ARCH_MXC || COMPILE_TEST + depends on ARCH_MXC || (COMPILE_TEST && OF) default ARCH_MXC && ARM64 select SOC_BUS help