Moving I2C2 init to plat-omap/devices.c ?

Moving I2C2 init to plat-omap/devices.c ?

[Syed Mohammed, Khasim]

Hello,

I would like to enable I2C support for 2430. I see I2C1 device
initialization defined in arch/arm/plat-omap/devices.c and I2C2 in
arch/arm/mach-omap2/devices.c. I believe the reason for this might be to
move the common part (I2C1) to plat-omap. 

I was thinking of moving the I2C2 initialization from
mach-omap2/devices.c to plat-omap/devices.c. The reason is OMAP1 has 1
I2C block, OMAP2 has 2 and OMAP3 has 4 (1 dedicated). The I2C register
base addresses are same for 2420, 2430 and 3430. So, adding same code in
two directories looks redundant. 

Let me know if you see any issues with this approach.

Thanks & Regards,
Khasim

Re: Moving I2C2 init to plat-omap/devices.c ?

[Tony Lindgren]

* Syed Mohammed, Khasim <x0khasim@ti.com> [061115 22:28]:
> Hello,
> 
> I would like to enable I2C support for 2430. I see I2C1 device
> initialization defined in arch/arm/plat-omap/devices.c and I2C2 in
> arch/arm/mach-omap2/devices.c. I believe the reason for this might be to
> move the common part (I2C1) to plat-omap. 
> 
> I was thinking of moving the I2C2 initialization from
> mach-omap2/devices.c to plat-omap/devices.c. The reason is OMAP1 has 1
> I2C block, OMAP2 has 2 and OMAP3 has 4 (1 dedicated). The I2C register
> base addresses are same for 2420, 2430 and 3430. So, adding same code in
> two directories looks redundant. 
> 
> Let me know if you see any issues with this approach.

Well most of that code is just data for the ports. And it sounds like
you're assuming that omap3 is a separate directory and "completely different"
from omap2.. Until we've seen some patches and docs, I don't think we
can make that assumption.

Regards,

Tony

RE: Moving I2C2 init to plat-omap/devices.c ?

[Syed Mohammed, Khasim]

Tony,

Agreed, it might be early to talk on 3430. I was just giving examples of
I2C blocks across OMAPs. 

For my work now, I will follow the existing structure. This can be
discussed later along with 3430 code placement :)

Regards,
Khasim
-----Original Message-----
From: Tony Lindgren [mailto:tony@atomide.com] 
Sent: Wednesday, November 15, 2006 7:02 PM
To: Syed Mohammed, Khasim
Cc: linux-omap-open-source@linux.omap.com
Subject: Re: Moving I2C2 init to plat-omap/devices.c ?

* Syed Mohammed, Khasim <x0khasim@ti.com> [061115 22:28]:
> Hello,
> 
> I would like to enable I2C support for 2430. I see I2C1 device
> initialization defined in arch/arm/plat-omap/devices.c and I2C2 in
> arch/arm/mach-omap2/devices.c. I believe the reason for this might be
to
> move the common part (I2C1) to plat-omap. 
> 
> I was thinking of moving the I2C2 initialization from
> mach-omap2/devices.c to plat-omap/devices.c. The reason is OMAP1 has 1
> I2C block, OMAP2 has 2 and OMAP3 has 4 (1 dedicated). The I2C register
> base addresses are same for 2420, 2430 and 3430. So, adding same code
in
> two directories looks redundant. 
> 
> Let me know if you see any issues with this approach.

Well most of that code is just data for the ports. And it sounds like
you're assuming that omap3 is a separate directory and "completely
different"
from omap2.. Until we've seen some patches and docs, I don't think we
can make that assumption.

Regards,

Tony

Re: Moving I2C2 init to plat-omap/devices.c ?

[Tony Lindgren]

* Syed Mohammed, Khasim <x0khasim@ti.com> [061116 03:22]:
> Tony,
> 
> Agreed, it might be early to talk on 3430. I was just giving examples of
> I2C blocks across OMAPs. 
> 
> For my work now, I will follow the existing structure. This can be
> discussed later along with 3430 code placement :)

Yeah, and we may want to add some common init function where you just
pass the port offsets or something like that.

Regards,

Tony

RE: Moving I2C2 init to plat-omap/devices.c ?

[Woodruff, Richard]

- OMAP1 and OMAP2420 use the same IP block for I2c.

- OMAP2420 has 2 instances of the same IP.
	-- We just added code in the single driver to allow 2 ports.

- OMAP2430 & OMAP3430 extended the functionality of the I2C IP.  It did
this in a mostly compatible fashion by defining previously reserved
bits.  The new function adds the ability to do high speed operation.
New interrupts, clock combinations, deeper fifos, and DMA options are
the changes.
	-- High speed allows quick power control.

- OMAP2430 has 2 IP instances of these high speed capable ports.

- OMAP3430 has 3 IP instance, driver controllable ports.  A 4th instance
is also on the chip but it is not driver controllable and is managed
directly by hardware for smart reflex protocol exchanges with the power
chip.
	-- 2430 generally uses a dedicated I2C channel to control the
power chip.
	-- 3430 uses 2 channels to the power chip, 1 control, 1
automatic.

For quicker time to functionality and isolation between release streams
we created 2 separate versions, one for 2420 and one for 2430/3430
versions.  A merged version is possible but would take a bit more time.

How things end up in this tree and the final tree is the course which
needs plotting. 

Regards,
Richard W.


> -----Original Message-----
> From: linux-omap-open-source-bounces@linux.omap.com
[mailto:linux-omap-
> open-source-bounces@linux.omap.com] On Behalf Of Tony Lindgren
> Sent: Wednesday, November 15, 2006 7:55 PM
> To: Syed Mohammed, Khasim
> Cc: linux-omap-open-source@linux.omap.com
> Subject: Re: Moving I2C2 init to plat-omap/devices.c ?
> 
> * Syed Mohammed, Khasim <x0khasim@ti.com> [061116 03:22]:
> > Tony,
> >
> > Agreed, it might be early to talk on 3430. I was just giving
examples of
> > I2C blocks across OMAPs.
> >
> > For my work now, I will follow the existing structure. This can be
> > discussed later along with 3430 code placement :)
> 
> Yeah, and we may want to add some common init function where you just
> pass the port offsets or something like that.
> 
> Regards,
> 
> Tony
> _______________________________________________
> Linux-omap-open-source mailing list
> Linux-omap-open-source@linux.omap.com
> http://linux.omap.com/mailman/listinfo/linux-omap-open-source

Re: Moving I2C2 init to plat-omap/devices.c ?

[Nishanth Menon]

Richard,
> For quicker time to functionality and isolation between release streams
> we created 2 separate versions, one for 2420 and one for 2430/3430
> versions.  A merged version is possible but would take a bit more time.
> 
I dont know if we want that to happen, the 16xx-2420 where pretty
similar compared to hiend 2430-3430 variant. mebbe it is high time we
have i2c and other ip drivers based on IP versions instead of omap
versions.. will save a hell lot of confusions. dma3.3 vs dma4, gpmc
variants, i2c variants... all of them require optimization in some
perspective, the new i2c has FIFO which the old 2420 does not have, the
code is entirely different for optimization reason.

Rest i leave to list discretion :)
Regards,
NM

RE: Moving I2C2 init to plat-omap/devices.c ?

[Woodruff, Richard]

> > For quicker time to functionality and isolation between release 
> > streams we created 2 separate versions, one for 2420 and one for 
> > 2430/3430 versions.  A merged version is possible but would 
> take a bit more time.
> > 
> I don't know if we want that to happen, the 16xx-2420 where 
> pretty similar compared to hiend 2430-3430 variant. mebbe it 
> is high time we have i2c and other ip drivers based on IP 
> versions instead of omap versions.. will save a hell lot of 
> confusions. dma3.3 vs dma4, gpmc variants, i2c variants... 
> all of them require optimization in some perspective, the new 
> i2c has FIFO which the old 2420 does not have, the code is 
> entirely different for optimization reason.

IP version would clarify some aspects around differences and errata.
You would still end up with both (chip & IP) as the integration of the
IP into a chip is an area where things are different also.  I find USB
client to be one which can be confusing.  From 1510-2420 there are many
bug fixes and many are not apparent by register differences.  This can
result in older ones being broke or newer ones not running to their
potential when sharing.

By adding enough layers of indirection/functional pointer tables you can
usually merge things.  However, this can make the code harder to follow.
If people wanted to take the time to merge I suspect that making the HS
driver work in a backward compatible manner might be easier than
upgrading the FS one.  Given what you know today would you agree with
that?  Getting any code into the I2C tree might take a bit of effort.
Tony floated an interesting idea about creating an embedded I2C using
Daivd's frame work.  This might allow faster integration of performance
related changes and the like with out fighting with SMB people.

Regards,
Richard W.

Re: Moving I2C2 init to plat-omap/devices.c ?

[Nishanth Menon]

Woodruff, Richard stated on 11/16/2006 8:28 PM:
> IP version would clarify some aspects around differences and errata.
> You would still end up with both (chip & IP) as the integration of the
> IP into a chip is an area where things are different also.  I find USB
> client to be one which can be confusing.  From 1510-2420 there are many
> bug fixes and many are not apparent by register differences.  This can
> result in older ones being broke or newer ones not running to their
> potential when sharing.
I think a V3.x driver, if defined to handle IP of range 3.0>= x <4.0,
then it can have backward compatibility based on subversion information.
I can think of more ways this will work than a omap-version based
approach.. this may not be the thread to discuss that however..


> 
> By adding enough layers of indirection/functional pointer tables you can
> usually merge things.  However, this can make the code harder to follow.
> If people wanted to take the time to merge I suspect that making the HS
> driver work in a backward compatible manner might be easier than
> upgrading the FS one.  Given what you know today would you agree with
> that?  Getting any code into the I2C tree might take a bit of effort.
I am pointing at the fact that using FIFO changes our bulk of our logic,
we need to be depending on a different set of interrupts, the transfer
logic is different etc.. makes 70-80% code look different I think.. yes
we can still have the same ARDY,NAK,timeout handling..
I am not sure to what extent..

> Tony floated an interesting idea about creating an embedded I2C using
> Daivd's frame work.  This might allow faster integration of performance
> related changes and the like with out fighting with SMB people.
> 
Err... I dont know.. is it more useful to get lm-sensor to look at that?
I dont think it makes sense to diverge off the lm-sensor line of thought
 since the client device support and framework in lmsensor is something
most folks like...but then... :) I am a wimp not ready to look at
something different :D

Regards,
Nishanth Menon

Re: Moving I2C2 init to plat-omap/devices.c ?

[David Brownell]

On Thursday 16 November 2006 6:58 am, Woodruff, Richard wrote:

> 	Getting any code into the I2C tree might take a bit of effort.
> Tony floated an interesting idea about creating an embedded I2C using
> David's frame work.  This might allow faster integration of performance
> related changes and the like with out fighting with SMB people.

I pinged the i2c list and got sort of mixed responses about that
notion of "embedded I2C" (i.e. rewrite) ... so I took a slightly
different and submitted patches aiming to add more driver model
compatibility to the current stack.

That means there'd still be a major functionality gap in the I2C
stack -- all the calls require a thread context, there are no
async callbacks -- but at least there's a solution for the setup
and configuration problems, potentially mergeable in 2.6.20 and
making the version of i2c-omap.c that's upstream become usable.

There may still be a need for a separate "embedded" stack though;
someone who can justify it should do so!

The patches are:

 http://lists.lm-sensors.org/pipermail/i2c/2006-November/000458.html
	... simple updates, some cleanup plus adding some missing
	driver model calls:  shutdown(), and suspend()/resume().
	Looks like this will be merged after minor tweaking.

 http://lists.lm-sensors.org/pipermail/i2c/2006-November/000469.html
	... not actually a patch; copied at the end of this message,
	this introduces the three interesting patches

 http://lists.lm-sensors.org/pipermail/i2c/2006-November/000470.html
	... first patch mentioned, adds new driver binding model
	that fully conforms to the Linux driver model

 http://lists.lm-sensors.org/pipermail/i2c/2006-November/000468.html
	... second patch mentioned, activates that new model

 http://lists.lm-sensors.org/pipermail/i2c/2006-November/000471.html
	... updates OSK support to use it, and tps65010 driver
	(but not aic23/ALSA, that's not upstream yet)

I figure getting those last three patches merged will "take a bit of
effort", so feel free to weigh in.  In terms of the original post on
this thread, it sort of assumes the OMAP I2C host would switch over
to a slightly different registration call, more or less passing the
platform_device.id ("2" etc) to the I2C layer and saying "use this
as the bus ID".

- Dave

Re: Moving I2C2 init to plat-omap/devices.c ?

[Tony Lindgren]

Hi,

* David Brownell <david-b@pacbell.net> [061116 22:21]:
> On Thursday 16 November 2006 6:58 am, Woodruff, Richard wrote:
> 
> > 	Getting any code into the I2C tree might take a bit of effort.
> > Tony floated an interesting idea about creating an embedded I2C using
> > David's frame work.  This might allow faster integration of performance
> > related changes and the like with out fighting with SMB people.
> 
> I pinged the i2c list and got sort of mixed responses about that
> notion of "embedded I2C" (i.e. rewrite) ... so I took a slightly
> different and submitted patches aiming to add more driver model
> compatibility to the current stack.
> 
> That means there'd still be a major functionality gap in the I2C
> stack -- all the calls require a thread context, there are no
> async callbacks -- but at least there's a solution for the setup
> and configuration problems, potentially mergeable in 2.6.20 and
> making the version of i2c-omap.c that's upstream become usable.
> 
> There may still be a need for a separate "embedded" stack though;
> someone who can justify it should do so!
> 
> The patches are:
> 
>  http://lists.lm-sensors.org/pipermail/i2c/2006-November/000458.html
> 	... simple updates, some cleanup plus adding some missing
> 	driver model calls:  shutdown(), and suspend()/resume().
> 	Looks like this will be merged after minor tweaking.
> 
>  http://lists.lm-sensors.org/pipermail/i2c/2006-November/000469.html
> 	... not actually a patch; copied at the end of this message,
> 	this introduces the three interesting patches
> 
>  http://lists.lm-sensors.org/pipermail/i2c/2006-November/000470.html
> 	... first patch mentioned, adds new driver binding model
> 	that fully conforms to the Linux driver model
> 
>  http://lists.lm-sensors.org/pipermail/i2c/2006-November/000468.html
> 	... second patch mentioned, activates that new model
> 
>  http://lists.lm-sensors.org/pipermail/i2c/2006-November/000471.html
> 	... updates OSK support to use it, and tps65010 driver
> 	(but not aic23/ALSA, that's not upstream yet)
> 
> I figure getting those last three patches merged will "take a bit of
> effort", so feel free to weigh in.  In terms of the original post on
> this thread, it sort of assumes the OMAP I2C host would switch over
> to a slightly different registration call, more or less passing the
> platform_device.id ("2" etc) to the I2C layer and saying "use this
> as the bus ID".

Wow, that's great, nice job! That should fix the zero length i2c
probe issue too :)

Tony

Re: Moving I2C2 init to plat-omap/devices.c ?

[Tony Lindgren]

* Nishanth Menon <menon.nishanth@gmail.com> [061116 22:12]:
> Woodruff, Richard stated on 11/16/2006 8:28 PM:
> > IP version would clarify some aspects around differences and errata.
> > You would still end up with both (chip & IP) as the integration of the
> > IP into a chip is an area where things are different also.  I find USB
> > client to be one which can be confusing.  From 1510-2420 there are many
> > bug fixes and many are not apparent by register differences.  This can
> > result in older ones being broke or newer ones not running to their
> > potential when sharing.
> I think a V3.x driver, if defined to handle IP of range 3.0>= x <4.0,
> then it can have backward compatibility based on subversion information.
> I can think of more ways this will work than a omap-version based
> approach.. this may not be the thread to discuss that however..
 
Doing different handlers in the drivers depending on the version number
is probably cleanest way to share the code that can be shared. And that
keeps the code clean of lots of if then else.
 
> > By adding enough layers of indirection/functional pointer tables you can
> > usually merge things.  However, this can make the code harder to follow.
> > If people wanted to take the time to merge I suspect that making the HS
> > driver work in a backward compatible manner might be easier than
> > upgrading the FS one.  Given what you know today would you agree with
> > that?  Getting any code into the I2C tree might take a bit of effort.
> I am pointing at the fact that using FIFO changes our bulk of our logic,
> we need to be depending on a different set of interrupts, the transfer
> logic is different etc.. makes 70-80% code look different I think.. yes
> we can still have the same ARDY,NAK,timeout handling..
> I am not sure to what extent..

We could have omap_i2c_v123_whatever() and omap_i2c_v234_whatever() type
functions for the parts that are different to keep the code clean.

Regards,

Tony

Re: Moving I2C2 init to plat-omap/devices.c ?

[David Brownell]

> >  http://lists.lm-sensors.org/pipermail/i2c/2006-November/000458.html
> > 	... simple updates, some cleanup plus adding some missing
> > 	driver model calls:  shutdown(), and suspend()/resume().
> > 	Looks like this will be merged after minor tweaking.
> > 
> >  http://lists.lm-sensors.org/pipermail/i2c/2006-November/000469.html
> > 	... not actually a patch; copied at the end of this message,
> > 	this introduces the three interesting patches
> > 
> >  http://lists.lm-sensors.org/pipermail/i2c/2006-November/000470.html
> > 	... first patch mentioned, adds new driver binding model
> > 	that fully conforms to the Linux driver model
> > 
> >  http://lists.lm-sensors.org/pipermail/i2c/2006-November/000468.html
> > 	... second patch mentioned, activates that new model
> > 
> >  http://lists.lm-sensors.org/pipermail/i2c/2006-November/000471.html
> > 	... updates OSK support to use it, and tps65010 driver
> > 	(but not aic23/ALSA, that's not upstream yet)
> > 
> > I figure getting those last three patches merged will "take a bit of
> > effort", so feel free to weigh in.  In terms of the original post on
> > this thread, it sort of assumes the OMAP I2C host would switch over
> > to a slightly different registration call, more or less passing the
> > platform_device.id ("2" etc) to the I2C layer and saying "use this
> > as the bus ID".
>
> Wow, that's great, nice job! That should fix the zero length i2c
> probe issue too :)

I find that I prefer to think of it as the "so-called 'I2C' stack
can't work without 'SMBUS_QUICK'" issue.  ;)

It does happen to be true that SMBUS_QUICK would be a zero-length
transaction, if I2C allowed those (and I'm told it doesn't) ... and
of course, anyone starting to use the I2C stack has to get over a
major wierd-out caused by the way that stack violates almost every
rule about how to write drivers in Linux.

- Dave