Re: [RFC 2/5] i3c: Add core I3C infrastructure

From: Boris Brezillon
Date: Tue Aug 01 2017 - 09:34:24 EST


On Tue, 1 Aug 2017 15:11:44 +0200
Arnd Bergmann <arnd@xxxxxxxx> wrote:

> On Tue, Aug 1, 2017 at 2:29 PM, Boris Brezillon
> <boris.brezillon@xxxxxxxxxxxxxxxxxx> wrote:
> > On Tue, 1 Aug 2017 14:00:05 +0200
> > Arnd Bergmann <arnd@xxxxxxxx> wrote:
>
> >> Another argument for a combined bus would be devices that
> >> can be attached to either i2c and i3c, depending on the host
> >> capabilities.
> >
> > Hm, that's already the case, isn't it? And you'll anyway need to
> > develop specific code for both cases in the I2C/I3C device driver
> > because I2C and I3C transfers are different. So I don't see how it
> > would help to have a single bus here.
> >
> >> We have discussed whether i2c and spi should be
> >> merged into a single bus_type in the past, as a lot of devices
> >> can be attached to either of them.
> >
> > Oh, really? What's the rational behind that? I mean, I2C and SPI are
> > quite different, and even if some devices provide both interfaces, I
> > don't see why we should merge them. But you probably had good reasons
> > to do so.
>
> Well, we never changed it, so at least the work required to merge
> the two was considered too much to justify any advantages.
>
> The main problem with having one driver that can operate on
> different bus types (i2c plus either spi or i3c) is the handling for
> the various combinations in configurations (e.g. I2C=m, SPI=y).
>
> The easy case is having a module_init function that registers two
> device drivers, but that requires having a Kconfig dependency
> on both subsystems, and you can't use the module_i2c_driver()
> helper.
>
> The second way is to have a number of #ifdef and complex
> Kconfig dependencies for the driver to only register the
> device_driver objects for the buses that are enabled. This
> is also doable, but everyone gets the logic wrong the first time.

Hm, I understand now why you'd prefer to have a single bus. Can't we
solve this problem with a module_i3c_i2c_driver() macro that would hide
all this complexity from I2C/I3C drivers?

>
> What we end up doing to work around this for other drivers is
> to have the base driver in one library module, and separate
> modules for the bus-specific portions, which can then
> use module_i2c_driver again. There are many instances
> for combined i2c/spi drivers in the kernel, and it works fine,
> but it adds a fair bit of overhead compared to having one
> driver that would e.g. use regmap to abstract the differences
> in the probe() function and otherwise keeps everything in
> one place.
>
> Arnd