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

From: Boris Brezillon
Date: Tue Aug 01 2017 - 09:58:32 EST


On Tue, 1 Aug 2017 15:34:14 +0200
Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx> wrote:

> 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?

I just realized I forgot to add a "depends on I2C" in the I3C Kconfig
entry. Indeed, I'm unconditionally calling functions provided by the
I2C framework which have no dummy wrapper when I2C support is disabled.
I could of course conditionally compile some portion of the I3C
framework so that it still builds when I2C is disabled but I'm not sure
it's worth the trouble.

This "depends on I2C" should also solve the I2C+I3C driver issue, since
I2C is necessarily enabled when I3C is.

Am I missing something?