[PATCH 4/4] powerpc: Call of_register_i2c_devices() for fsl-i2c.

[PATCH 4/4] powerpc: Call of_register_i2c_devices() for fsl-i2c.

[Scott Wood]

Call of_register_i2c_devices() to register I2C devices on
the device tree under an fsl-i2c node.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
This patch depends on David Brownell's patchset at
http://lists.lm-sensors.org/pipermail/i2c/2006-November/000516.html

 arch/powerpc/sysdev/fsl_soc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index ad31e56..a4d3c46 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -345,6 +345,8 @@ static int __init fsl_i2c_of_init(void)
 						    fsl_i2c_platform_data));
 		if (ret)
 			goto unreg;
+
+		of_register_i2c_devices(np, i);
 	}
 
 	return 0;
-- 
1.4.2.3

Re: [PATCH 4/4] powerpc: Call of_register_i2c_devices() for fsl-i2c.

[Arnd Bergmann]

On Thursday 07 December 2006 18:35, Scott Wood wrote:
> --- a/arch/powerpc/sysdev/fsl_soc.c
> +++ b/arch/powerpc/sysdev/fsl_soc.c
> @@ -345,6 +345,8 @@ static int __init fsl_i2c_of_init(void)
> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 =
=A0 =A0fsl_i2c_platform_data));
> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0if (ret)
> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0g=
oto unreg;
> +
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0of_register_i2c_devices(np,=
 i);
> =A0=A0=A0=A0=A0=A0=A0=A0}

I get the impression that this fsl_i2c_of_init function is doing the
probing in the opposite way it should. Instead of searching the
whole device tree for type=3D"i2c" devices to call=20
platform_device_register_simple, can't you use of_platform_bus_probe
to create the i2c device in the first place, and then have a
of_platform_driver that calls of_register_i2c_devices() in its probe
function?

	Arnd <><

Re: [PATCH 4/4] powerpc: Call of_register_i2c_devices() for fsl-i2c.

[Kumar Gala]

On Dec 7, 2006, at 1:35 PM, Arnd Bergmann wrote:

> On Thursday 07 December 2006 18:35, Scott Wood wrote:
>> --- a/arch/powerpc/sysdev/fsl_soc.c
>> +++ b/arch/powerpc/sysdev/fsl_soc.c
>> @@ -345,6 +345,8 @@ static int __init fsl_i2c_of_init(void)
>>                                                      
>> fsl_i2c_platform_data));
>>                 if (ret)
>>                         goto unreg;
>> +
>> +               of_register_i2c_devices(np, i);
>>         }
>
> I get the impression that this fsl_i2c_of_init function is doing the
> probing in the opposite way it should. Instead of searching the
> whole device tree for type="i2c" devices to call
> platform_device_register_simple, can't you use of_platform_bus_probe
> to create the i2c device in the first place, and then have a
> of_platform_driver that calls of_register_i2c_devices() in its probe
> function?

Some of this is historic because of arch/ppc legacy.  We explicitly  
spec'd devices as platform devices.  At some point we will move to  
having the drivers be proper of_platform drivers instead of platform  
drivers.

- k

Re: [PATCH 4/4] powerpc: Call of_register_i2c_devices() for fsl-i2c.

[Kumar Gala]

On Dec 7, 2006, at 11:35 AM, Scott Wood wrote:

> Call of_register_i2c_devices() to register I2C devices on
> the device tree under an fsl-i2c node.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
> This patch depends on David Brownell's patchset at
> http://lists.lm-sensors.org/pipermail/i2c/2006-November/000516.html
>
>  arch/powerpc/sysdev/fsl_soc.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/ 
> fsl_soc.c
> index ad31e56..a4d3c46 100644
> --- a/arch/powerpc/sysdev/fsl_soc.c
> +++ b/arch/powerpc/sysdev/fsl_soc.c
> @@ -345,6 +345,8 @@ static int __init fsl_i2c_of_init(void)
>  						    fsl_i2c_platform_data));
>  		if (ret)
>  			goto unreg;
> +
> +		of_register_i2c_devices(np, i);
>  	}

You are assuming the bus number is the same as our index. This may  
not be true.

>  	return 0;
> -- 
> 1.4.2.3
>
>
>
>
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 4/4] powerpc: Call of_register_i2c_devices() for fsl-i2c.

[Scott Wood]

Kumar Gala wrote:
> You are assuming the bus number is the same as our index. This may  not 
> be true.

http://ozlabs.org/pipermail/linuxppc-dev/2006-December/028924.html

The above patch sets the I2C ID to the index, so as long as there are no 
non-SOC i2c busses with static bus numbers, it'll work.

If there are non-SOC i2c busses (such as on a PCI card) that claim a 
conflicting static bus ID, there'll be problems -- but that problem 
isn't powerpc or device-tree specific.

A more robust solution would be to allow each i2c adapter driver to 
specify an i2c_board_info list when registering itself.  The adapter 
driver could then get this list from platform code (if appropriate; a 
PCI card that uses i2c internally would simply have its own list), 
without having to deal with system-wide bus IDs.

-Scott

Re: [PATCH 4/4] powerpc: Call of_register_i2c_devices() for fsl-i2c.

[Kumar Gala]

On Dec 8, 2006, at 12:38 PM, Scott Wood wrote:

> Kumar Gala wrote:
>> You are assuming the bus number is the same as our index. This  
>> may  not be true.
>
> http://ozlabs.org/pipermail/linuxppc-dev/2006-December/028924.html
>
> The above patch sets the I2C ID to the index, so as long as there  
> are no non-SOC i2c busses with static bus numbers, it'll work.
>
> If there are non-SOC i2c busses (such as on a PCI card) that claim  
> a conflicting static bus ID, there'll be problems -- but that  
> problem isn't powerpc or device-tree specific.

True, but I think we can do better.

> A more robust solution would be to allow each i2c adapter driver to  
> specify an i2c_board_info list when registering itself.  The  
> adapter driver could then get this list from platform code (if  
> appropriate; a PCI card that uses i2c internally would simply have  
> its own list), without having to deal with system-wide bus IDs.

What about doing the of_register_i2c_devices in the fsl-i2c driver  
itself.  Than it knows what bus number its been assigned.

- k

Re: [PATCH 4/4] powerpc: Call of_register_i2c_devices() for fsl-i2c.

[Scott Wood]

Kumar Gala wrote:
> What about doing the of_register_i2c_devices in the fsl-i2c driver  
> itself.  Than it knows what bus number its been assigned.

It won't know the dynamic bus number until after it registers the 
adapter, which is too late (even if the new-style device registration 
were modified to support registering devices after adapter registration, 
legacy probing drivers for devices on the same address might grab the 
device first, before the proper device gets registered).

The allocation of an i2c bus ID could be factored out into its own 
function that gets called before registering the i2c adapter.  This 
(plus calling of_register_i2c_devices in the adapter driver) isn't much 
different from passing the list in with the adapter registration, but it 
would avoid some list management.  OTOH, it means we'd have to continue 
having global numeric bus IDs even after the last legacy driver is 
converted.

-Scott