Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 9 Aug 2023 21:13:10 +0200
From:   Andi Shyti <andi.shyti@kernel.org>
To:     Nick Desaulniers <ndesaulniers@google.com>
Cc:     Jean Delvare <jdelvare@suse.com>,
        Nathan Chancellor <nathan@kernel.org>,
        Tom Rix <trix@redhat.com>, linux-kernel@vger.kernel.org,
        linux-i2c@vger.kernel.org, llvm@lists.linux.dev,
        kernel test robot <lkp@intel.com>
Subject: Re: [PATCH] i2c/busses: fix -Wmissing-variable-declarations
Message-ID: <20230809191310.atpqztlpgdbdvc5m@intel.intel>
References: <20230808-i2c-amd_static-v1-1-1902f608bba1@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230808-i2c-amd_static-v1-1-1902f608bba1@google.com>
Precedence: bulk

Hi Nick,

On Tue, Aug 08, 2023 at 09:56:16AM -0700, Nick Desaulniers wrote:
> I'm looking to enable -Wmissing-variable-declarations behind W=1. 0day
> bot spotted the following instance:
> 
>   drivers/i2c/busses/i2c-amd756.c:286:20: warning: no previous extern
>   declaration for non-static variable 'amd756_smbus'
>   [-Wmissing-variable-declarations]
>   286 | struct i2c_adapter amd756_smbus = {
>       |                    ^
>   drivers/i2c/busses/i2c-amd756.c:286:1: note: declare 'static' if the
>   variable is not intended to be used outside of this translation unit
>   286 | struct i2c_adapter amd756_smbus = {
>       | ^
> 
> This symbol is referenced by more than one translation unit, so create
> then include the correct header for their declarations.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/llvm/202308081000.tTL1ElTr-lkp@intel.com/
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

You might want to have a Fixes tag here and

Cc: Jean Delvare <jdelvare@suse.com>

[...]

> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-amd756.h
> @@ -0,0 +1,3 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

Please, leave a space here.

And you might also want to have something like:

#ifndef __I2C_AMD_756_H__
#define __I2C_AMD_756_H__

> +#include <linux/i2c.h>

space here.

> +extern struct i2c_adapter amd756_smbus;

#endif /* __I2C_AMD_756_H__ */

Jean, any opinion on this patch, I don't really know this driver,
but is there a way to avoid this extern declaration.

Thanks,
Andi