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

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

[Richard Ršöjfors]

There is sometimes a need for the ocores driver to add devices to the bus when installed.

i2c_register_board_info can not always be used, because the I2C devices are not known at an early state,
they could for instance be connected on a I2C bus on a PCI device which has the Open Cores IP.

i2c_new_device can not be used in all cases either since the resulting bus nummer might be unknown.

The solution is the pass a list of I2C devices in the platform data to the Open Cores driver. Is
useful for MFD drivers for instance.


Signed-off-by: Richard Röjfors <richard.rojfors.ext-l7gf1WXxx3ujphq+ikeySQ@public.gmane.orgm>
---
Index: linux-2.6.30-rc7/drivers/i2c/busses/i2c-ocores.c
===================================================================
--- linux-2.6.30-rc7/drivers/i2c/busses/i2c-ocores.c	(revision 861)
+++ linux-2.6.30-rc7/drivers/i2c/busses/i2c-ocores.c	(revision 862)
@@ -216,6 +216,7 @@
 	struct ocores_i2c_platform_data *pdata;
 	struct resource *res, *res2;
 	int ret;
+	u8 i;

 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res)
@@ -271,6 +272,10 @@
 		goto add_adapter_failed;
 	}

+	/* add in known devices to the bus */
+	for (i = 0; i < pdata->num_devices; i++)
+		i2c_new_device(&i2c->adap, pdata->devices + i);
+
 	return 0;

 add_adapter_failed:
Index: linux-2.6.30-rc7/include/linux/i2c-ocores.h
===================================================================
--- linux-2.6.30-rc7/include/linux/i2c-ocores.h	(revision 861)
+++ linux-2.6.30-rc7/include/linux/i2c-ocores.h	(revision 862)
@@ -14,6 +14,8 @@
 struct ocores_i2c_platform_data {
 	u32 regstep;   /* distance between registers */
 	u32 clock_khz; /* input clock in kHz */
+	u8 num_devices; /* number of devices in the devices list */
+	struct i2c_board_info const *devices; /* devices connected to the bus */
 };

 #endif /* _LINUX_I2C_OCORES_H */

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

[Ben Dooks]

On Tue, Jun 02, 2009 at 07:52:40PM +0200, Richard R????jfors wrote:
> There is sometimes a need for the ocores driver to add devices to the bus when installed.

please wrap to 72 characters per line. 
 
> i2c_register_board_info can not always be used, because the I2C devices are not known at an early state,
> they could for instance be connected on a I2C bus on a PCI device which has the Open Cores IP.

Maybe i2c_register_board_info() needs to check if the bus is extant
or not. Anyone else have any feedback on this suggestion?
 
> i2c_new_device can not be used in all cases either since the resulting bus nummer might be unknown.
> 
> The solution is the pass a list of I2C devices in the platform data to the Open Cores driver. Is
> useful for MFD drivers for instance.

Possibly, but most systems I see know what devices they have before
instantiation so can hard-fix the bus numbering when supplying the MFD
drivers with their data.
 
> Signed-off-by: Richard R??jfors <richard.rojfors.ext-l7gf1WXxx3uGw+nKnLezzg@public.gmane.org>
> ---
> Index: linux-2.6.30-rc7/drivers/i2c/busses/i2c-ocores.c
> ===================================================================
> --- linux-2.6.30-rc7/drivers/i2c/busses/i2c-ocores.c	(revision 861)
> +++ linux-2.6.30-rc7/drivers/i2c/busses/i2c-ocores.c	(revision 862)
> @@ -216,6 +216,7 @@
>  	struct ocores_i2c_platform_data *pdata;
>  	struct resource *res, *res2;
>  	int ret;
> +	u8 i;

really, an int will do here, you might find compilers on !x86 archs
will spend time contracting this variable to an u8.
 
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!res)
> @@ -271,6 +272,10 @@
>  		goto add_adapter_failed;
>  	}
> 
> +	/* add in known devices to the bus */
> +	for (i = 0; i < pdata->num_devices; i++)
> +		i2c_new_device(&i2c->adap, pdata->devices + i);
> +
>  	return 0;
> 
>  add_adapter_failed:
> Index: linux-2.6.30-rc7/include/linux/i2c-ocores.h
> ===================================================================
> --- linux-2.6.30-rc7/include/linux/i2c-ocores.h	(revision 861)
> +++ linux-2.6.30-rc7/include/linux/i2c-ocores.h	(revision 862)
> @@ -14,6 +14,8 @@
>  struct ocores_i2c_platform_data {
>  	u32 regstep;   /* distance between registers */
>  	u32 clock_khz; /* input clock in kHz */
> +	u8 num_devices; /* number of devices in the devices list */
> +	struct i2c_board_info const *devices; /* devices connected to the bus */
>  };

anyone want to kerneldoc this?
 
>  #endif /* _LINUX_I2C_OCORES_H */
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

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

[Richard R?öjfors]

Ben Dooks wrote:
> On Tue, Jun 02, 2009 at 07:52:40PM +0200, Richard R????jfors wrote:
>> There is sometimes a need for the ocores driver to add devices to the bus when installed.
> 
> please wrap to 72 characters per line.
>  
>> i2c_register_board_info can not always be used, because the I2C devices are not known at an early state,
>> they could for instance be connected on a I2C bus on a PCI device which has the Open Cores IP.
> 
> Maybe i2c_register_board_info() needs to check if the bus is extant
> or not. Anyone else have any feedback on this suggestion?
>  
>> i2c_new_device can not be used in all cases either since the resulting bus nummer might be unknown.
>>
>> The solution is the pass a list of I2C devices in the platform data to the Open Cores driver. Is
>> useful for MFD drivers for instance.
> 
> Possibly, but most systems I see know what devices they have before
> instantiation so can hard-fix the bus numbering when supplying the MFD
> drivers with their data.

I agree, but we have a case here where PCI device(s) are plugged in to a
standard system so we really don't know the bus numbering.

>  
>> Signed-off-by: Richard R??jfors <richard.rojfors.ext-l7gf1WXxx3uGw+nKnLezzg@public.gmane.org>
>> ---
>> Index: linux-2.6.30-rc7/drivers/i2c/busses/i2c-ocores.c
>> ===================================================================
>> --- linux-2.6.30-rc7/drivers/i2c/busses/i2c-ocores.c	(revision 861)
>> +++ linux-2.6.30-rc7/drivers/i2c/busses/i2c-ocores.c	(revision 862)
>> @@ -216,6 +216,7 @@
>>  	struct ocores_i2c_platform_data *pdata;
>>  	struct resource *res, *res2;
>>  	int ret;
>> +	u8 i;
> 
> really, an int will do here, you might find compilers on !x86 archs
> will spend time contracting this variable to an u8.

Agreed.

>  
>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>  	if (!res)
>> @@ -271,6 +272,10 @@
>>  		goto add_adapter_failed;
>>  	}
>>
>> +	/* add in known devices to the bus */
>> +	for (i = 0; i < pdata->num_devices; i++)
>> +		i2c_new_device(&i2c->adap, pdata->devices + i);
>> +
>>  	return 0;
>>
>>  add_adapter_failed:
>> Index: linux-2.6.30-rc7/include/linux/i2c-ocores.h
>> ===================================================================
>> --- linux-2.6.30-rc7/include/linux/i2c-ocores.h	(revision 861)
>> +++ linux-2.6.30-rc7/include/linux/i2c-ocores.h	(revision 862)
>> @@ -14,6 +14,8 @@
>>  struct ocores_i2c_platform_data {
>>  	u32 regstep;   /* distance between registers */
>>  	u32 clock_khz; /* input clock in kHz */
>> +	u8 num_devices; /* number of devices in the devices list */
>> +	struct i2c_board_info const *devices; /* devices connected to the bus */
>>  };
> 
> anyone want to kerneldoc this?

If you accept the patch, I will send an updated one with updated docs.

>  
>>  #endif /* _LINUX_I2C_OCORES_H */
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[Jean Delvare]

On Tue, 2 Jun 2009 23:48:22 +0100, Ben Dooks wrote:
> On Tue, Jun 02, 2009 at 07:52:40PM +0200, Richard R????jfors wrote:
> > There is sometimes a need for the ocores driver to add devices to the bus when installed.
> 
> please wrap to 72 characters per line. 
>  
> > i2c_register_board_info can not always be used, because the I2C devices are not known at an early state,
> > they could for instance be connected on a I2C bus on a PCI device which has the Open Cores IP.
> 
> Maybe i2c_register_board_info() needs to check if the bus is extant
> or not. Anyone else have any feedback on this suggestion?

I don't understand the question ^^ Please clarify.

> > i2c_new_device can not be used in all cases either since the resulting bus nummer might be unknown.
> > 
> > The solution is the pass a list of I2C devices in the platform data to the Open Cores driver. Is
> > useful for MFD drivers for instance.
> 
> Possibly, but most systems I see know what devices they have before
> instantiation so can hard-fix the bus numbering when supplying the MFD
> drivers with their data.

I don't like the idea much either, nor the implementation.

Firstly, I don't understand why this would be needed. I can understand
that in some cases you don't know the I2C bus number in advance, but
then some code must still instantiate the I2C bus, and the same code
should be able to call i2c_new_device() directly to instantiate the
devices on that bus. Richard, did you try to just do this? If it
doesn't work, please explain why.

Secondly, if (and only if) this is really needed, then I'd rather
implement the mechanism as part of i2c-core (basically extending
i2c_register_board_info) than in individual bus drivers.

-- 
Jean Delvare

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

[Peter Korsgaard]

>>>>> "Jean" == Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> writes:

Hi,

 Jean> I don't like the idea much either, nor the implementation.

 Jean> Firstly, I don't understand why this would be needed. I can understand
 Jean> that in some cases you don't know the I2C bus number in advance, but
 Jean> then some code must still instantiate the I2C bus, and the same code
 Jean> should be able to call i2c_new_device() directly to instantiate the
 Jean> devices on that bus. Richard, did you try to just do this? If it
 Jean> doesn't work, please explain why.

Indeed. Isn't it just a matter of using i2c_add_numbered_adapter -
E.G.:

--- linux-2.6/drivers/i2c/busses/i2c-ocores.c	2008-11-26 11:16:27.000000000 +0100
+++ linux-2.6-new/drivers/i2c/busses/i2c-ocores.c	2008-12-13 19:59:12.000000000 +0100
@@ -261,11 +261,12 @@
 	/* hook up driver to tree */
 	platform_set_drvdata(pdev, i2c);
 	i2c->adap = ocores_adapter;
+	i2c->adap.nr = pdev->id;
 	i2c_set_adapdata(&i2c->adap, i2c);
 	i2c->adap.dev.parent = &pdev->dev;
 
 	/* add i2c adapter to i2c tree */
-	ret = i2c_add_adapter(&i2c->adap);
+	ret = i2c_add_numbered_adapter(&i2c->adap);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to add adapter\n");
 		goto add_adapter_failed;

Or am I misunderstanding the issue?

-- 
Bye, Peter Korsgaard

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

[Jean Delvare]

On Wed, 03 Jun 2009 10:22:38 +0200, Peter Korsgaard wrote:
> >>>>> "Jean" == Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> writes:
> 
> Hi,
> 
>  Jean> I don't like the idea much either, nor the implementation.
> 
>  Jean> Firstly, I don't understand why this would be needed. I can understand
>  Jean> that in some cases you don't know the I2C bus number in advance, but
>  Jean> then some code must still instantiate the I2C bus, and the same code
>  Jean> should be able to call i2c_new_device() directly to instantiate the
>  Jean> devices on that bus. Richard, did you try to just do this? If it
>  Jean> doesn't work, please explain why.
> 
> Indeed. Isn't it just a matter of using i2c_add_numbered_adapter -
> E.G.:
> 
> --- linux-2.6/drivers/i2c/busses/i2c-ocores.c	2008-11-26 11:16:27.000000000 +0100
> +++ linux-2.6-new/drivers/i2c/busses/i2c-ocores.c	2008-12-13 19:59:12.000000000 +0100
> @@ -261,11 +261,12 @@
>  	/* hook up driver to tree */
>  	platform_set_drvdata(pdev, i2c);
>  	i2c->adap = ocores_adapter;
> +	i2c->adap.nr = pdev->id;
>  	i2c_set_adapdata(&i2c->adap, i2c);
>  	i2c->adap.dev.parent = &pdev->dev;
>  
>  	/* add i2c adapter to i2c tree */
> -	ret = i2c_add_adapter(&i2c->adap);
> +	ret = i2c_add_numbered_adapter(&i2c->adap);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Failed to add adapter\n");
>  		goto add_adapter_failed;
> 
> Or am I misunderstanding the issue?

Richard explicitly said that his I2C bus is on an add-on board and thus
the I2C bus number can't be decided in advance. So the above is not
usable in his case.

-- 
Jean Delvare

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

[Richard R?öjfors]

Peter Korsgaard wrote:
>>>>>> "Jean" == Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> writes:
> 
> Hi,
> 
>  Jean> I don't like the idea much either, nor the implementation.
> 
>  Jean> Firstly, I don't understand why this would be needed. I can understand
>  Jean> that in some cases you don't know the I2C bus number in advance, but
>  Jean> then some code must still instantiate the I2C bus, and the same code
>  Jean> should be able to call i2c_new_device() directly to instantiate the
>  Jean> devices on that bus. Richard, did you try to just do this? If it
>  Jean> doesn't work, please explain why.
> 
> Indeed. Isn't it just a matter of using i2c_add_numbered_adapter -
> E.G.:

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.
How should those drivers know which bus numbers that are free?
Let say there are 2 boards of one type and two of an other.

The MFD:s might register there platform devices before there are I2C bus drivers available.
So a call to i2c_get_adapter, won't return any adapter, so AFAIK there is no
way to check if a bus number is "free" for use(?) Should those drivers talk to each other
and try to coordinate the bus number usage?


But even if it was possible to figure out bus numbers to use, the bus drivers might
not be available when the MFD registers the platform device, so when should it call
i2c_new_device? Start a timer and call i2c_get_adapter until it returns something?


--Richard

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

[Richard Ršöjfors]

Hi,

Any ideas of how to progress on this "issue"?

Richard Röjfors wrote:
> Peter Korsgaard wrote:
>>>>>>> "Jean" == Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> writes:
>> Hi,
>>
>>  Jean> I don't like the idea much either, nor the implementation.
>>
>>  Jean> Firstly, I don't understand why this would be needed. I can understand
>>  Jean> that in some cases you don't know the I2C bus number in advance, but
>>  Jean> then some code must still instantiate the I2C bus, and the same code
>>  Jean> should be able to call i2c_new_device() directly to instantiate the
>>  Jean> devices on that bus. Richard, did you try to just do this? If it
>>  Jean> doesn't work, please explain why.
>>
>> Indeed. Isn't it just a matter of using i2c_add_numbered_adapter -
>> E.G.:
> 
> 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.
> How should those drivers know which bus numbers that are free?
> Let say there are 2 boards of one type and two of an other.
> 
> The MFD:s might register there platform devices before there are I2C bus drivers available.
> So a call to i2c_get_adapter, won't return any adapter, so AFAIK there is no
> way to check if a bus number is "free" for use(?) Should those drivers talk to each other
> and try to coordinate the bus number usage?
> 
> 
> But even if it was possible to figure out bus numbers to use, the bus drivers might
> not be available when the MFD registers the platform device, so when should it call
> i2c_new_device? Start a timer and call i2c_get_adapter until it returns something?


--Richard

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

[Jean Delvare]

On Wed, 03 Jun 2009 10:53:10 +0200, Richard R?öjfors wrote:
> Peter Korsgaard wrote:
> >>>>>> "Jean" == Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> writes:
> > 
> > Hi,
> > 
> >  Jean> I don't like the idea much either, nor the implementation.
> > 
> >  Jean> Firstly, I don't understand why this would be needed. I can understand
> >  Jean> that in some cases you don't know the I2C bus number in advance, but
> >  Jean> then some code must still instantiate the I2C bus, and the same code
> >  Jean> should be able to call i2c_new_device() directly to instantiate the
> >  Jean> devices on that bus. Richard, did you try to just do this? If it
> >  Jean> doesn't work, please explain why.
> > 
> > Indeed. Isn't it just a matter of using i2c_add_numbered_adapter -
> > E.G.:
> 
> 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.

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.

It might make sense to consider ocores an "I2C algorithm" and have
i2c-algo-ocores for the abstracted implementation.

> How should those drivers know which bus numbers that are free?
> Let say there are 2 boards of one type and two of an other.
> 
> The MFD:s might register there platform devices before there are I2C bus drivers available.
> So a call to i2c_get_adapter, won't return any adapter, so AFAIK there is no
> way to check if a bus number is "free" for use(?) Should those drivers talk to each other
> and try to coordinate the bus number usage?
>
> But even if it was possible to figure out bus numbers to use, the bus drivers might
> not be available when the MFD registers the platform device, so when should it call
> i2c_new_device? Start a timer and call i2c_get_adapter until it returns something?

I don't think you need to answer these questions any longer with the
model I proposed above.

-- 
Jean Delvare

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

[Richard Ršöjfors]

Jean Delvare wrote:
>>>
>>> Indeed. Isn't it just a matter of using i2c_add_numbered_adapter -
>>> E.G.:
>> 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.

> 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 :-(


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.

--Richard

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

[Mark Brown]

On Thu, Jun 04, 2009 at 04:24:30PM +0200, Richard R????jfors wrote:

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

It's probably worth pointing out here that MFD uses platform devices to
represent the subdevices no matter what the underlying access method for
the device actually is.

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

[Jean Delvare]

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

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

[Richard Röjfors]

Jean Delvare wrote:
> 
> 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.

Will do.

> Oh, and I also believe your driver
> should call i2c_unregister_device() on removal, for symmetry.

Isn't better to leave that to i2c_del_adapter? Otherwise we need to store a list
of the I2C-clients returned by i2c_new_device.

--Richard

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

[Jean Delvare]

On Fri, 05 Jun 2009 09:12:57 +0200, Richard Röjfors wrote:
> Jean Delvare wrote:
> > 
> > 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.
> 
> Will do.
> 
> > Oh, and I also believe your driver
> > should call i2c_unregister_device() on removal, for symmetry.
> 
> Isn't better to leave that to i2c_del_adapter? Otherwise we need to store a list
> of the I2C-clients returned by i2c_new_device.

Err, you're right. I tend to forget that the parameters passed to
i2c_unregister_device() differ from those passed to i2c_new_device().
Maybe we'd need i2c_unregister_device_by_addr() or something. In the
meantime, yes, just leave it to i2c_del_adapter() to clean up the
remaining clients.

-- 
Jean Delvare

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

[Ben Dooks]

On Tue, Jun 02, 2009 at 07:52:40PM +0200, Richard R????jfors wrote:
> There is sometimes a need for the ocores driver to add devices to the bus when installed.
> 
> i2c_register_board_info can not always be used, because the I2C devices are not known at an early state,
> they could for instance be connected on a I2C bus on a PCI device which has the Open Cores IP.
> 
> i2c_new_device can not be used in all cases either since the resulting bus nummer might be unknown.
> 
> The solution is the pass a list of I2C devices in the platform data to the Open Cores driver. Is
> useful for MFD drivers for instance.

Hi, these two minor issues here I'd like to get at-least a reply
if not a fix for:
 
> Signed-off-by: Richard R??jfors <richard.rojfors.ext-l7gf1WXxx3uGw+nKnLezzg@public.gmane.org>
> ---
> Index: linux-2.6.30-rc7/drivers/i2c/busses/i2c-ocores.c
> ===================================================================
> --- linux-2.6.30-rc7/drivers/i2c/busses/i2c-ocores.c	(revision 861)
> +++ linux-2.6.30-rc7/drivers/i2c/busses/i2c-ocores.c	(revision 862)
> @@ -216,6 +216,7 @@
>  	struct ocores_i2c_platform_data *pdata;
>  	struct resource *res, *res2;
>  	int ret;
> +	u8 i;

Do you really need u8, doing the following would probably produce
better code:

	unsigned i

>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!res)
> @@ -271,6 +272,10 @@
>  		goto add_adapter_failed;
>  	}
> 
> +	/* add in known devices to the bus */
> +	for (i = 0; i < pdata->num_devices; i++)
> +		i2c_new_device(&i2c->adap, pdata->devices + i);
> +
>  	return 0;
> 
>  add_adapter_failed:
> Index: linux-2.6.30-rc7/include/linux/i2c-ocores.h
> ===================================================================
> --- linux-2.6.30-rc7/include/linux/i2c-ocores.h	(revision 861)
> +++ linux-2.6.30-rc7/include/linux/i2c-ocores.h	(revision 862)
> @@ -14,6 +14,8 @@
>  struct ocores_i2c_platform_data {
>  	u32 regstep;   /* distance between registers */
>  	u32 clock_khz; /* input clock in kHz */
> +	u8 num_devices; /* number of devices in the devices list */
> +	struct i2c_board_info const *devices; /* devices connected to the bus */
>  };

Any chance of kerneldoc for this structure?

-- 
Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

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

[Ben Dooks]

On Sat, Jun 13, 2009 at 10:38:30AM +0100, Ben Dooks wrote:
> On Tue, Jun 02, 2009 at 07:52:40PM +0200, Richard R????jfors wrote:
> > There is sometimes a need for the ocores driver to add devices to the bus when installed.
> > 
> > i2c_register_board_info can not always be used, because the I2C devices are not known at an early state,
> > they could for instance be connected on a I2C bus on a PCI device which has the Open Cores IP.
> > 
> > i2c_new_device can not be used in all cases either since the resulting bus nummer might be unknown.
> > 
> > The solution is the pass a list of I2C devices in the platform data to the Open Cores driver. Is
> > useful for MFD drivers for instance.
> 
> Hi, these two minor issues here I'd like to get at-least a reply
> if not a fix for:

sorry, just noticed a new patch. will go and merge that.

-- 
Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'