[PATCH] clk: s2mps11: Add used attribute to s2mps11_dt_match

[PATCH] clk: s2mps11: Add used attribute to s2mps11_dt_match

[Nathan Chancellor]

Clang warns after commit 8985167ecf57 ("clk: s2mps11: Fix matching when
built as module and DT node contains compatible"):

drivers/clk/clk-s2mps11.c:242:34: warning: variable 's2mps11_dt_match'
is not needed and will not be emitted [-Wunneeded-internal-declaration]
static const struct of_device_id s2mps11_dt_match[] = {
                                 ^
1 warning generated.

This warning happens when a variable is used in some construct that
doesn't require a reference to that variable to be emitted in the symbol
table; in this case, it's MODULE_DEVICE_TABLE, which only needs to hold
the data of the variable, not the variable itself.

$ nm -S drivers/clk/clk-s2mps11.o | rg s2mps11_dt_match
00000078 000003d4 R __mod_of__s2mps11_dt_match_device_table

Normally, with device ID table variables, it means that the variable
just needs to be tied to the device declaration at the bottom of the
file, like s2mps11_clk_id:

$ nm -S drivers/clk/clk-s2mps11.o | rg s2mps11_clk_id
00000000 00000078 R __mod_platform__s2mps11_clk_id_device_table
00000000 00000078 r s2mps11_clk_id

However, because the comment above this deliberately doesn't want this
variable added to .of_match_table, we need to mark s2mps11_dt_match as
__used to silence this warning. This makes it clear to Clang that the
variable is used for something, even if a reference to it isn't being
emitted.

Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 drivers/clk/clk-s2mps11.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-s2mps11.c b/drivers/clk/clk-s2mps11.c
index 9c2e6be392e3..5b419b82f7ca 100644
--- a/drivers/clk/clk-s2mps11.c
+++ b/drivers/clk/clk-s2mps11.c
@@ -239,7 +239,7 @@ MODULE_DEVICE_TABLE(platform, s2mps11_clk_id);
  * This requires of_device_id table.  In the same time this will not change the
  * actual *device* matching so do not add .of_match_table.
  */
-static const struct of_device_id s2mps11_dt_match[] = {
+static const struct of_device_id s2mps11_dt_match[] __used = {
 	{
 		.compatible = "samsung,s2mps11-clk",
 		.data = (void *)S2MPS11X,
-- 
2.19.1

Re: [PATCH] clk: s2mps11: Add used attribute to s2mps11_dt_match

[Stephen Boyd]

Quoting Nathan Chancellor (2018-10-18 12:13:40)
> Clang warns after commit 8985167ecf57 ("clk: s2mps11: Fix matching when
> built as module and DT node contains compatible"):
> 
> drivers/clk/clk-s2mps11.c:242:34: warning: variable 's2mps11_dt_match'
> is not needed and will not be emitted [-Wunneeded-internal-declaration]
> static const struct of_device_id s2mps11_dt_match[] = {
>                                  ^
> 1 warning generated.
> 
> This warning happens when a variable is used in some construct that
> doesn't require a reference to that variable to be emitted in the symbol
> table; in this case, it's MODULE_DEVICE_TABLE, which only needs to hold
> the data of the variable, not the variable itself.
> 
> $ nm -S drivers/clk/clk-s2mps11.o | rg s2mps11_dt_match
> 00000078 000003d4 R __mod_of__s2mps11_dt_match_device_table
> 
> Normally, with device ID table variables, it means that the variable
> just needs to be tied to the device declaration at the bottom of the
> file, like s2mps11_clk_id:
> 
> $ nm -S drivers/clk/clk-s2mps11.o | rg s2mps11_clk_id
> 00000000 00000078 R __mod_platform__s2mps11_clk_id_device_table
> 00000000 00000078 r s2mps11_clk_id
> 
> However, because the comment above this deliberately doesn't want this
> variable added to .of_match_table, we need to mark s2mps11_dt_match as
> __used to silence this warning. This makes it clear to Clang that the
> variable is used for something, even if a reference to it isn't being
> emitted.
> 
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---

Just curious if this is a common occurrence? Maybe we should make a new
MODULE_DEVICE_TABLE macro that can declare the table, struct type, and
add in __used to the declaration like is done here? That way this gotcha
goes away without us having to remember that the table is used or not
used somewhere.

Anyway, I'll apply this patch to clk-next.

Re: [PATCH] clk: s2mps11: Add used attribute to s2mps11_dt_match

[Nathan Chancellor]

On Thu, Oct 18, 2018 at 01:29:57PM -0700, Stephen Boyd wrote:
> Quoting Nathan Chancellor (2018-10-18 12:13:40)
> > Clang warns after commit 8985167ecf57 ("clk: s2mps11: Fix matching when
> > built as module and DT node contains compatible"):
> > 
> > drivers/clk/clk-s2mps11.c:242:34: warning: variable 's2mps11_dt_match'
> > is not needed and will not be emitted [-Wunneeded-internal-declaration]
> > static const struct of_device_id s2mps11_dt_match[] = {
> >                                  ^
> > 1 warning generated.
> > 
> > This warning happens when a variable is used in some construct that
> > doesn't require a reference to that variable to be emitted in the symbol
> > table; in this case, it's MODULE_DEVICE_TABLE, which only needs to hold
> > the data of the variable, not the variable itself.
> > 
> > $ nm -S drivers/clk/clk-s2mps11.o | rg s2mps11_dt_match
> > 00000078 000003d4 R __mod_of__s2mps11_dt_match_device_table
> > 
> > Normally, with device ID table variables, it means that the variable
> > just needs to be tied to the device declaration at the bottom of the
> > file, like s2mps11_clk_id:
> > 
> > $ nm -S drivers/clk/clk-s2mps11.o | rg s2mps11_clk_id
> > 00000000 00000078 R __mod_platform__s2mps11_clk_id_device_table
> > 00000000 00000078 r s2mps11_clk_id
> > 
> > However, because the comment above this deliberately doesn't want this
> > variable added to .of_match_table, we need to mark s2mps11_dt_match as
> > __used to silence this warning. This makes it clear to Clang that the
> > variable is used for something, even if a reference to it isn't being
> > emitted.
> > 
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> 
> Just curious if this is a common occurrence? Maybe we should make a new
> MODULE_DEVICE_TABLE macro that can declare the table, struct type, and
> add in __used to the declaration like is done here? That way this gotcha
> goes away without us having to remember that the table is used or not
> used somewhere.
> 

I've only had to fix two of these warnings prior to this one, running
arm64 and x86_64 defconfig/allyesconfig builds but as soon as options like
CONFIG_OF or CONFIG_ACPI get turned off, it can happen quite frequently
because things like of_match_ptr and ACPI_PTR turn into NULL, meaning the
variable doesn't have a reference in the symbol table anymore. It would
definitely be nice to avoid having to paper the warnings up.

> Anyway, I'll apply this patch to clk-next.
> 

Thanks!
Nathan