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:   Thu, 13 Apr 2023 21:27:48 +0200
From:   Wolfram Sang <wsa@kernel.org>
To:     Arnd Bergmann <arnd@kernel.org>
Cc:     Jean-Marie Verdun <verdun@hpe.com>,
        Nick Hawkins <nick.hawkins@hpe.com>,
        Joel Stanley <joel@jms.id.au>, Arnd Bergmann <arnd@arndb.de>,
        Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= 
        <u.kleine-koenig@pengutronix.de>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] i2c: gxp: fix build failure without CONFIG_I2C_SLAVE
Message-ID: <ZDhXtDLiTtm2iXGW@sai>
Mail-Followup-To: Wolfram Sang <wsa@kernel.org>,
        Arnd Bergmann <arnd@kernel.org>, Jean-Marie Verdun <verdun@hpe.com>,
        Nick Hawkins <nick.hawkins@hpe.com>, Joel Stanley <joel@jms.id.au>,
        Arnd Bergmann <arnd@arndb.de>,
        Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20230403074939.3785593-1-arnd@kernel.org>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="YSLKM2vUjZb+N5Xo"
Content-Disposition: inline
In-Reply-To: <20230403074939.3785593-1-arnd@kernel.org>
Precedence: bulk


--YSLKM2vUjZb+N5Xo
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi Arnd,

> The gxp_i2c_slave_irq_handler() is hidden in an #ifdef, but the
> caller uses an IS_ENABLED() check:
>=20
> drivers/i2c/busses/i2c-gxp.c: In function 'gxp_i2c_irq_handler':
> drivers/i2c/busses/i2c-gxp.c:467:29: error: implicit declaration of funct=
ion 'gxp_i2c_slave_irq_handler'; did you mean 'gxp_i2c_irq_handler'? [-Werr=
or=3Dimplicit-function-declaration]
>=20
> It has to consistently use one method or the other to avoid warnings,
> so move to IS_ENABLED() here for readability and build coverage, and
> move the #ifdef in linux/i2c.h to allow building it as dead code.

Can't we have a solution which modifies this driver only (maybe by
defining an empty irq handler for the non-IS_ENABLED part?)? Doesn't
feel good to touch i2c.h only because of this...

> -#if IS_ENABLED(CONFIG_I2C_SLAVE)
>  enum i2c_slave_event {
>  	I2C_SLAVE_READ_REQUESTED,
>  	I2C_SLAVE_WRITE_REQUESTED,
> @@ -396,9 +395,10 @@ enum i2c_slave_event {
> =20
>  int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_c=
b);
>  int i2c_slave_unregister(struct i2c_client *client);

=2E.. especially with moving these two prototypes out of the protected
block. The functions themselves are also protected by the same symbol
via the Makefile. I'd rather get a build error right away than a linker
error later if a driver misses to select I2C_SLAVE. Or do I miss
something?

> -bool i2c_detect_slave_mode(struct device *dev);
>  int i2c_slave_event(struct i2c_client *client,
>  		    enum i2c_slave_event event, u8 *val);
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +bool i2c_detect_slave_mode(struct device *dev);
>  #else
>  static inline bool i2c_detect_slave_mode(struct device *dev) { return fa=
lse; }
>  #endif

All the best,

   Wolfram


--YSLKM2vUjZb+N5Xo
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEOZGx6rniZ1Gk92RdFA3kzBSgKbYFAmQ4V68ACgkQFA3kzBSg
KbbCbg/+KMqGcmARpv4WTOLrw8dNeL7kGcYB8y8JBS5rZNf6WFS/suaqj5KGTon6
+4x9oSnNvDzcZIdcgaTuggayelF33q90mKiuctnXd3uRc4bEzfS2sEgdHsOOoDkj
Qazub13ObNSHM3nzfquGyCOde/wPWNazwdurQYU/EDmCz1uhe32vjZJx8YlW1RwV
u3VtH3ur2k5g2p50oY677jnTUsw23DY0AhFFRNRFHcTk0SKnb+2we8NzcvRYX0J7
CgXzjm7xvkJ2vwdwjzbTIcTxbHs9y1pPhwpQVB+xE06t+jk6aYkXPCKx2KWQo5in
LZD2prPO/L0sqRYZci9AfQSyfe1j4ctihTjRPps/xO0Tdfbh3d7mavL5FtpU/hdH
zrYyUGJ66kB1hp5NzHXMLyLEAroEPsIIy666z0R3JwAKy0T5/gbEASN3r60io/l7
l/35pXnj6E4mBQRc7V7OShi+jloZAUhDRBWpq5vnZAXm799hbIMpoYUuqBdt6OhO
K4ny9xlEczj+6MPJOwTYt5p8bZa5trRTsupm7J0eQifiIiR0IS0ABGkHydq4BrdJ
3/PDgNcGT6mHVi4vcFOWoFuqciGBUpwiSAAp/S0lKwcjAj1dKYsTEjh1F6cHslCC
tBBikemwBHscnDmJBQ4Ob4peTNE/EIUtY7P2gCI5NBENg+U8cT8=
=1Kzt
-----END PGP SIGNATURE-----

--YSLKM2vUjZb+N5Xo--