diff mbox series

soc: imx: Kconfig: force using OF when COMPILE_TEST

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

Commit Message

Peng Fan June 15, 2020, 10:21 a.m. UTC 
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(-)

Comments

Aisheng Dong June 16, 2020, 2:41 a.m. UTC | #1 
> 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
Uwe Kleine-König June 16, 2020, 3:59 p.m. UTC | #2 
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
Robin Murphy June 16, 2020, 4:40 p.m. UTC | #3 
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.
Peng Fan June 17, 2020, 2:05 a.m. UTC | #4 
> 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.
Uwe Kleine-König June 17, 2020, 6:05 a.m. UTC | #5 
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
Shawn Guo June 23, 2020, 12:16 p.m. UTC | #6 
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.
Peng Fan June 24, 2020, 1:54 a.m. UTC | #7 
> 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/
> |
Shawn Guo June 24, 2020, 3:27 a.m. UTC | #8 
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 mbox series

Patch

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