On Wed, Dec 04, 2019 at 10:53:48AM +0100, Sascha Hauer wrote:
> Always add the (un)reg_slave hooks to struct i2c_algorithm, even when
> I2C slave support is disabled. With the cost of some binary space I2C
> drivers with optional I2C slave support no longer have to #ifdef
> the hooks. For the same reason add a stub for i2c_slave_event and make
> enum i2c_slave_event present without I2C slave support.
>
> Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
This kind of reverts d5fd120e7860 ("i2c: Only include slave support if
selected"), so adding Jean here for more discussion.
I don't mind the additional bytes used in i2c_algorithm, so I am in
favor of this approach.
I do mind the extra bytes used in each and every i2c_client (which is
not affected in this patch). What we could do on top of this:
Because i2c-slave backends will be rare (and only those need it), it
might be worthwhile to introduce a struct i2c_slave_client and embed the
original i2c_client there. Maybe this way, we could get rid of the
I2C_SLAVE symbol entirely? The I2C core code is not a lot as well.
> ---
> include/linux/i2c.h | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index d2f786706657..74ebfcb43dd2 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -359,7 +359,6 @@ static inline void i2c_set_clientdata(struct i2c_client *dev, void *data)
>
> /* I2C slave support */
>
> -#if IS_ENABLED(CONFIG_I2C_SLAVE)
> enum i2c_slave_event {
> I2C_SLAVE_READ_REQUESTED,
> I2C_SLAVE_WRITE_REQUESTED,
> @@ -368,6 +367,7 @@ enum i2c_slave_event {
> I2C_SLAVE_STOP,
> }
>
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> extern int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb);
> extern int i2c_slave_unregister(struct i2c_client *client);
> extern bool i2c_detect_slave_mode(struct device *dev);
> @@ -379,6 +379,11 @@ static inline int i2c_slave_event(struct i2c_client *client,
> }
> #else
> static inline bool i2c_detect_slave_mode(struct device *dev) { return false; }
> +static inline int i2c_slave_event(struct i2c_client *client,
> + enum i2c_slave_event event, u8 *val)
> +{
> + return -EINVAL;
> +}
> #endif
>
> /**
> @@ -553,10 +558,8 @@ struct i2c_algorithm {
> /* To determine what the adapter supports */
> u32 (*functionality)(struct i2c_adapter *adap);
>
> -#if IS_ENABLED(CONFIG_I2C_SLAVE)
> int (*reg_slave)(struct i2c_client *client);
> int (*unreg_slave)(struct i2c_client *client);
> -#endif
> };
>
> /**
> --
> 2.24.0
>
Attachment:
signature.asc
Description: PGP signature
