2020-07-09 16:51:15

by kernel test robot

[permalink] [raw]
Subject: sound/soc/codecs/zl38060.c:614:34: warning: unused variable 'zl38_dt_ids'

Hi Sven,

FYI, the error/warning still remains.

tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 0bddd227f3dc55975e2b8dfa7fc6f959b062a2c7
commit: 52e8a94baf9026276fcdc9ff21a50dc2ca0bc94b ASoC: Add initial ZL38060 driver
date: 3 months ago
config: x86_64-randconfig-r004-20200709 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 02946de3802d3bc65bc9f2eb9b8d4969b5a7add8)
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 x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
git checkout 52e8a94baf9026276fcdc9ff21a50dc2ca0bc94b
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> sound/soc/codecs/zl38060.c:614:34: warning: unused variable 'zl38_dt_ids' [-Wunused-const-variable]
static const struct of_device_id zl38_dt_ids[] = {
^
1 warning generated.

vim +/zl38_dt_ids +614 sound/soc/codecs/zl38060.c

613
> 614 static const struct of_device_id zl38_dt_ids[] = {
615 { .compatible = "mscc,zl38060", },
616 { /* sentinel */ }
617 };
618 MODULE_DEVICE_TABLE(of, zl38_dt_ids);
619

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (1.68 kB)
.config.gz (33.92 kB)
Download all attachments

2020-07-10 02:44:42

by Nathan Chancellor

[permalink] [raw]
Subject: Re: sound/soc/codecs/zl38060.c:614:34: warning: unused variable 'zl38_dt_ids'

On Fri, Jul 10, 2020 at 12:47:31AM +0800, kernel test robot wrote:
> Hi Sven,
>
> FYI, the error/warning still remains.
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 0bddd227f3dc55975e2b8dfa7fc6f959b062a2c7
> commit: 52e8a94baf9026276fcdc9ff21a50dc2ca0bc94b ASoC: Add initial ZL38060 driver
> date: 3 months ago
> config: x86_64-randconfig-r004-20200709 (attached as .config)
> compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 02946de3802d3bc65bc9f2eb9b8d4969b5a7add8)
> 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 x86_64 cross compiling tool for clang build
> # apt-get install binutils-x86-64-linux-gnu
> git checkout 52e8a94baf9026276fcdc9ff21a50dc2ca0bc94b
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> All warnings (new ones prefixed by >>):
>
> >> sound/soc/codecs/zl38060.c:614:34: warning: unused variable 'zl38_dt_ids' [-Wunused-const-variable]
> static const struct of_device_id zl38_dt_ids[] = {
> ^
> 1 warning generated.
>
> vim +/zl38_dt_ids +614 sound/soc/codecs/zl38060.c
>
> 613
> > 614 static const struct of_device_id zl38_dt_ids[] = {
> 615 { .compatible = "mscc,zl38060", },
> 616 { /* sentinel */ }
> 617 };
> 618 MODULE_DEVICE_TABLE(of, zl38_dt_ids);
> 619
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/[email protected]
>

When CONFIG_SND_SOC_ZL38060 is y, MODULE_DEVICE_TABLE expands to nothing
so zl38_dt_ids will be unused. This is a pretty common construct in the
kernel and the only way I can think of to resolve this through the code
is by adding __used annotations to all of these variables, which I think
is overkill for this.

Personally, I think this warning should be downgraded to W=2, thoughts?

Cheers,
Nathan

2020-07-10 12:26:13

by Mark Brown

[permalink] [raw]
Subject: Re: sound/soc/codecs/zl38060.c:614:34: warning: unused variable 'zl38_dt_ids'

On Thu, Jul 09, 2020 at 07:41:00PM -0700, Nathan Chancellor wrote:

> When CONFIG_SND_SOC_ZL38060 is y, MODULE_DEVICE_TABLE expands to nothing
> so zl38_dt_ids will be unused. This is a pretty common construct in the
> kernel and the only way I can think of to resolve this through the code
> is by adding __used annotations to all of these variables, which I think
> is overkill for this.

> Personally, I think this warning should be downgraded to W=2, thoughts?

We've had that warning available for ever, we shouldn't need to disable
it now. I had thought there was supposed to be magic which caused
of_match_ptr() to make things look referenced when !OF but don't seem to
actually see any sign of it. The other thing is to just have ifdefs
around the table.


Attachments:
(No filename) (780.00 B)
signature.asc (499.00 B)
Download all attachments

2020-07-11 03:32:14

by Nathan Chancellor

[permalink] [raw]
Subject: Re: sound/soc/codecs/zl38060.c:614:34: warning: unused variable 'zl38_dt_ids'

On Fri, Jul 10, 2020 at 01:24:59PM +0100, Mark Brown wrote:
> On Thu, Jul 09, 2020 at 07:41:00PM -0700, Nathan Chancellor wrote:
>
> > When CONFIG_SND_SOC_ZL38060 is y, MODULE_DEVICE_TABLE expands to nothing
> > so zl38_dt_ids will be unused. This is a pretty common construct in the
> > kernel and the only way I can think of to resolve this through the code
> > is by adding __used annotations to all of these variables, which I think
> > is overkill for this.
>
> > Personally, I think this warning should be downgraded to W=2, thoughts?
>
> We've had that warning available for ever, we shouldn't need to disable
> it now. I had thought there was supposed to be magic which caused
> of_match_ptr() to make things look referenced when !OF but don't seem to
> actually see any sign of it. The other thing is to just have ifdefs
> around the table.

While it has been available, it's been hidden behind W=1, which is now
default on for 0day.

Sure, you could hide it behind an ifdef for either CONFIG_OF or MODULE
(since you could build this as a module with CONFIG_OF disabled).

I just figured this would be something frowned upon but if that is how
you would prefer it to be fixed, then I have no objections to it.

Cheers,
Nathan

2020-07-11 15:09:08

by Sven Van Asbroeck

[permalink] [raw]
Subject: Re: sound/soc/codecs/zl38060.c:614:34: warning: unused variable 'zl38_dt_ids'

Hi Nathan and Mark,

On Fri, Jul 10, 2020 at 11:30 PM Nathan Chancellor
<[email protected]> wrote:
>
> Sure, you could hide it behind an ifdef for either CONFIG_OF or MODULE
> (since you could build this as a module with CONFIG_OF disabled).
>
> I just figured this would be something frowned upon but if that is how
> you would prefer it to be fixed, then I have no objections to it.
>

That's how things used to work in the past: we had to #ifdef CONFIG_OF
all the driver of_XXX code, so it could be built on !CONFIG_OF.

I remember problems with this approach: it generated a lot of
visual noise, it was fragile in both directions: including too much,
and excluding too much. Last but not least, it required us ARM people
to test build each patch on !CONFIG_OF, which many conveniently forgot
to do :)

My "vote" would be to fix the warning using compiler magic. For
example:

#if !CONFIG_OF
// #define of_match_ptr(x) NULL
#define of_match_ptr(x) ((x) == NULL ? NULL : NULL)
#endif

That seems to eliminate the warning on my gcc version 7.5.0, but of
course as compilers get more clever, this could eventually
generate a warning (if it doesn't already on clang).

So perhaps use compiler attributes instead?

Stand-alone testable code snippet below.
======================================================

// gcc -Wall unused.c
// results in: match_table is NULL
// gcc -Wall -DCONFIG_OF unused.c
// results in: match_table[0] = 5

#include <stdio.h>

#ifdef CONFIG_OF
#define of_match_ptr(x) x
#else
#define of_match_ptr(x) ((x) == NULL ? NULL : NULL)
#endif

struct of_device_id {
int id;
};

static const struct of_device_id some_ids[] = {
{ .id = 5, },
{ /* sentinel */ }
};

struct driver {
const struct of_device_id *of_match_table;
};

static const struct driver my_driver = {
.of_match_table = of_match_ptr(some_ids),
};

int main()
{
if (my_driver.of_match_table)
printf("match_table[0] = %d\n", my_driver.of_match_table[0].id);
else
printf("match_table is NULL\n");
}