Re: [RFC] i2c: designware: Avoid initcall and initialize the driver like a regular one

From: Wolfram Sang
Date: Tue Dec 23 2014 - 10:26:44 EST



> >> This guarantees it will probe after GPIOs drivers.

BTW this is not true to the best of my knowledge. It will make that
"very likely" but not "guarantee" anything. So, the race window is
smaller but it is still there. You need a proper fix anyhow.

> >> Platforms based on devicetree won't be affected by this change.
> >
> > Huh, why is that?
> >
>
> Unless I'm missing something here, our beloved DeviceTree guarantees to
> model the dependency between I2C slaves devices and the I2C master their
> connected to.

Frankly, you are missing quite some things here. The I2C core registers
the clients when a master gets registered. No difference between
platform and DT here.

> So, a machine fully-based on DeviceTree would never attempt to use the I2C
> bus without first registering the master, right?

Neither would platform, that would be quite a bug.

> This means there won't be any early users of the I2C platform driver in this
> scenario.

There won't be with platform as well. But I think you are missing the
point. We are a *consumer* of GPIOs here. All of the above has nothing
to do with GPIO controllers being already available.

> >> Legacy platforms, relying on the I2C being available early, might need
> >> to implement proper defered mechanisms to overcome potential problems.
> >
> > NAK. We can't say "Let's cause a regression to force people to fix
> > things that used to work" IMO. You exactly pointed out the problem that using
> > subsys_initcall() creates.
> >
> > What about fixing the drivers you use to support deferred probing when
> > acquitin the irq?
> >
>
> Maybe we can fix the legacy ones instead. However, a quick look shows there
> aren't any!
>
> $ git grep i2c_designware
> drivers/i2c/busses/i2c-designware-pcidrv.c:MODULE_ALIAS("i2c_designware-pci");
> drivers/i2c/busses/i2c-designware-platdrv.c:MODULE_ALIAS("platform:i2c_designware");
> drivers/i2c/busses/i2c-designware-platdrv.c: .name = "i2c_designware",
>
> Looks like this patch is pretty harmless.

In-tree you are right. Out-of-tree, you probably aren't. I wouldn't care
about the latter if that would block a real bugfix. But since the above
patch is not the proper fix IMO, I prefer being stable here.

Regards,

Wolfram

Attachment: signature.asc
Description: Digital signature