Re: [PATCH] I2C: ocores can add I2C devices to the bus

[Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>]

On Thu, 04 Jun 2009 16:24:30 +0200, Richard Ršöjfors wrote:

Please do all of us a favor and stop using non-ASCII characters in your
e-mail address.

> >> Let say there are several PCI boards which have I2C busses, connected in let say
> >> a standard PC. The PCI drivers could be MFD:s which exposes some platform
> >> devices for the I2C busses.
> >
> > This doesn't make any sense to me to start with. PCI boards have PCI
> > drivers, not platform drivers. If you have an I2C bus on a PCI board,
> > the PCI driver simply registers it using i2c_add_adapter(), there is no
> > platform driver or device involved.
> 
> We have a PCIe device, actually a FPGA with a lot of different IP:s in it.
> For instance an UART, I2C, SPI controller, ethernet controller,
> radio tuner, video grabber, SDHCI, etc etc. And we only get one single
> interrupt from the device.
> 
> The PCI driver for this device implements an MFD
> (multi function device, check drivers/mfd). The idea of MFD:s is to
> register platform devices for all cells. And to multiplex IRQ:s to the
> platform devices. And by doing this all existing platform drivers
> can be reused.

OK, I understand your design now.

> > If you happen to have a PCI device which implements an I2C adapter
> > compatible with i2c-ocores, then what you want is to abstract the I2C
> > controller logic to a separate module, and have the current i2c-ocores
> > driver (which would become a simple glue module, and may be renamed to
> > i2c-ocores-platform) depend on it. Then add your i2c-ocores-pci driver
> > for the PCI implementation, also using the abstracted logic module as
> > its backend.
> 
> Problem here is that our PCI device implements _a lot_ of stuff,
> not only I2C.
> That's what the MFD is all about, split it into multiple
> platform devices. So in my case the MFD driver has to call the algo
> directly.
> 
> I don't see the bad thing about my idea. I mean the MFD driver knows
> that it have an ocores I2C hardware, and that there are a couple of
> devices on the I2C bus.
> Why not pass a table of devices when registering the I2C platform device?
> I think that's nicer than having the MFD register a lot of platform
> devices, but when it comes I2C it has to implement it by itself.
> This more or less breaks the whole MFD idea :-(

I am now convinced your proposed implementation makes sense for your
specific need (which is relatively rare, which is why i2c-core doesn't
handle it.) And contrary to what I first wrote, this doesn't need to be
moved to i2c-core: this is specific enough that I'd rather let the code
live in the bus driver (i2c-ocores) for now, and only if at least two
other bus drivers need the same, consider moving it to i2c-core.

So if you fix the minor objection Ben had about your patch and resend
it, I think we can merge that. Oh, and I also believe your driver
should call i2c_unregister_device() on removal, for symmetry.

> Btw SPI does something like I do, some of the SPI drivers calls
> of_register_spi_devices, which registers devices to the newly created
> master.

We have of_register_i2c_devices() which works exactly the same, but I
can't see the relation between Open Firmware and the problem at hand.

-- 
Jean Delvare