On Mon, 26 Jan 2015 17:30:13 +0100, Wolfram Sang wrote:
>
> > > Own module: Again, undecided. On the one hand it makes for a nice
> > > encapsulation, on the other hand there is overhead for having another
> > > module. I am very happy that the core code for slave support is so slim.
> >
> > I gave a try to the separate module approach and I have to agree that
> > it seems overkill given the small amount of code.
>
> OK, thanks for trying!
>
> > Something like this?
>
> Yes, pretty much what I had in mind. One issue, though:
>
> > +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> > enum i2c_slave_event {
> > I2C_SLAVE_REQ_READ_START,
> > I2C_SLAVE_REQ_READ_END,
> > @@ -263,6 +266,7 @@ static inline int i2c_slave_event(struct
> > {
> > return client->slave_cb(client, event, val);
> > }
> > +#endif
>
> This should fail because bus drivers need those enums for their slave
> backend. Try building i2c-sh_mobile which builds with an x86 toolchain
> as well.
Sorry I missed that, because there is currently no i2c bus driver
implementing slave support on x86-64.
> * Either we leave this included, so bus drivers don't need any ifdeffery
We can do that. The enum itself has no run-time cost so I don't mind.
> or
>
> * we mandate that bus drivers also use the ifedeffery. Then, we could
> also mask out the (un)reg_slave callbacks in struct i2c_adapter
>
> What do you think?
Oh, I admit I completely missed the (un)reg_slave callbacks in my first
patch.
While I am happy with a few ifdefs in i2c-core and i2c.h, I agree it
will become messy if these are required in device drivers as well.
Hmm, what about bus drivers with slave mode support must select
CONFIG_I2C_SLAVE? This solves my problem nicely, and makes no change
compared to the current situation for people using slave mode.
Thanks,
--
Jean Delvare
SUSE L3 Support
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
