diff mbox

[U-Boot,V3,1/5] pxa: move i2c driver to the common place

Message ID 1300344306-26608-2-git-send-email-leiwen@marvell.com
State Superseded
Commit 68432c27438360aa3bc27c0c73bf3c12a8c6ffa7
Headers show

Commit Message

Lei Wen March 17, 2011, 6:45 a.m. UTC
For better sharing with other platform other than pxa's,
it is more convenient to put the driver to the common place.

Signed-off-by: Lei Wen <leiwen@marvell.com>
---
Changelog:
v2: rename previous pxa_i2c to mvi2c.

V3: change previous name from pxa_i2c to mv_i2c
    clean code style issue exist in original code

 arch/arm/cpu/pxa/Makefile |    1 -
 arch/arm/cpu/pxa/i2c.c    |  469 ---------------------------------------------
 drivers/i2c/Makefile      |    1 +
 drivers/i2c/mv_i2c.c      |  452 +++++++++++++++++++++++++++++++++++++++++++
 include/configs/innokom.h |    1 +
 include/configs/xm250.h   |    1 +
 6 files changed, 455 insertions(+), 470 deletions(-)
 delete mode 100644 arch/arm/cpu/pxa/i2c.c
 create mode 100644 drivers/i2c/mv_i2c.c

Comments

Prafulla Wadaskar March 22, 2011, 11:42 a.m. UTC | #1
> -----Original Message-----
> From: Lei Wen [mailto:leiwen@marvell.com]
> Sent: Thursday, March 17, 2011 12:15 PM
> To: Heiko Schocher; Wolfgang Denk; Prafulla Wadaskar; u-
> boot@lists.denx.de; Marek Vasut; Ashish Karkare; Prabhanjan Sarnaik;
> adrian.wenl@gmail.com
> Subject: [PATCH V3 1/5] pxa: move i2c driver to the common place
> 
> For better sharing with other platform other than pxa's,
> it is more convenient to put the driver to the common place.
> 
> Signed-off-by: Lei Wen <leiwen@marvell.com>
> ---
> Changelog:
> v2: rename previous pxa_i2c to mvi2c.
> 
> V3: change previous name from pxa_i2c to mv_i2c
>     clean code style issue exist in original code
> 
>  arch/arm/cpu/pxa/Makefile |    1 -
>  arch/arm/cpu/pxa/i2c.c    |  469 --------------------------------------
> -------
>  drivers/i2c/Makefile      |    1 +
>  drivers/i2c/mv_i2c.c      |  452
> +++++++++++++++++++++++++++++++++++++++++++
>  include/configs/innokom.h |    1 +
>  include/configs/xm250.h   |    1 +
>  6 files changed, 455 insertions(+), 470 deletions(-)
>  delete mode 100644 arch/arm/cpu/pxa/i2c.c
>  create mode 100644 drivers/i2c/mv_i2c.c


...snip...

> -#endif	/* CONFIG_HARD_I2C */
> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> index 052fe36..00a12cc 100644
> --- a/drivers/i2c/Makefile
> +++ b/drivers/i2c/Makefile
> @@ -29,6 +29,7 @@ COBJS-$(CONFIG_BFIN_TWI_I2C) += bfin-twi_i2c.o
>  COBJS-$(CONFIG_DRIVER_DAVINCI_I2C) += davinci_i2c.o
>  COBJS-$(CONFIG_FSL_I2C) += fsl_i2c.o
>  COBJS-$(CONFIG_I2C_MVTWSI) += mvtwsi.o
> +COBJS-$(CONFIG_I2C_MV) += mv_i2c.o

Mvtwsi and mv_i2c are two i2c drivers for Marvell.
Can you merge these two?

Regards..
Prafulla . .
Lei Wen March 22, 2011, 12:43 p.m. UTC | #2
Hi Prafulla,

On Tue, Mar 22, 2011 at 7:42 PM, Prafulla Wadaskar <prafulla@marvell.com> wrote:
>
>
>> -----Original Message-----
>> From: Lei Wen [mailto:leiwen@marvell.com]
>> Sent: Thursday, March 17, 2011 12:15 PM
>> To: Heiko Schocher; Wolfgang Denk; Prafulla Wadaskar; u-
>> boot@lists.denx.de; Marek Vasut; Ashish Karkare; Prabhanjan Sarnaik;
>> adrian.wenl@gmail.com
>> Subject: [PATCH V3 1/5] pxa: move i2c driver to the common place
>>
>> For better sharing with other platform other than pxa's,
>> it is more convenient to put the driver to the common place.
>>
>> Signed-off-by: Lei Wen <leiwen@marvell.com>
>> ---
>> Changelog:
>> v2: rename previous pxa_i2c to mvi2c.
>>
>> V3: change previous name from pxa_i2c to mv_i2c
>>     clean code style issue exist in original code
>>
>>  arch/arm/cpu/pxa/Makefile |    1 -
>>  arch/arm/cpu/pxa/i2c.c    |  469 --------------------------------------
>> -------
>>  drivers/i2c/Makefile      |    1 +
>>  drivers/i2c/mv_i2c.c      |  452
>> +++++++++++++++++++++++++++++++++++++++++++
>>  include/configs/innokom.h |    1 +
>>  include/configs/xm250.h   |    1 +
>>  6 files changed, 455 insertions(+), 470 deletions(-)
>>  delete mode 100644 arch/arm/cpu/pxa/i2c.c
>>  create mode 100644 drivers/i2c/mv_i2c.c
>
>
> ...snip...
>
>> -#endif       /* CONFIG_HARD_I2C */
>> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
>> index 052fe36..00a12cc 100644
>> --- a/drivers/i2c/Makefile
>> +++ b/drivers/i2c/Makefile
>> @@ -29,6 +29,7 @@ COBJS-$(CONFIG_BFIN_TWI_I2C) += bfin-twi_i2c.o
>>  COBJS-$(CONFIG_DRIVER_DAVINCI_I2C) += davinci_i2c.o
>>  COBJS-$(CONFIG_FSL_I2C) += fsl_i2c.o
>>  COBJS-$(CONFIG_I2C_MVTWSI) += mvtwsi.o
>> +COBJS-$(CONFIG_I2C_MV) += mv_i2c.o
>
> Mvtwsi and mv_i2c are two i2c drivers for Marvell.
> Can you merge these two?

As I explain to you before. Although kirkwood and pxa series are both
the product
of Marvell, but it don't necessary means that they must have the same controller
for both product line. For the i2c part, they just use two different controller.
So why you keep request merge those two? Do you mean you want to
create a unique I2C
framework for whole i2c drivers in drivers/i2c?

Best regards,
Lei
Prafulla Wadaskar March 23, 2011, 7:38 a.m. UTC | #3
> -----Original Message-----
> From: Lei Wen [mailto:adrian.wenl@gmail.com]
> Sent: Tuesday, March 22, 2011 6:14 PM
> To: Prafulla Wadaskar
> Cc: Lei Wen; Heiko Schocher; Wolfgang Denk; u-boot@lists.denx.de; Marek
> Vasut; Ashish Karkare; Prabhanjan Sarnaik; Yu Tang
> Subject: Re: [PATCH V3 1/5] pxa: move i2c driver to the common place
> 
...snip...
> >>  drivers/i2c/Makefile      |    1 +
> >>  drivers/i2c/mv_i2c.c      |  452
> >> +++++++++++++++++++++++++++++++++++++++++++
> >>  include/configs/innokom.h |    1 +
> >>  include/configs/xm250.h   |    1 +
> >>  6 files changed, 455 insertions(+), 470 deletions(-)
> >>  delete mode 100644 arch/arm/cpu/pxa/i2c.c
> >>  create mode 100644 drivers/i2c/mv_i2c.c
> >
> >
> > ...snip...
> >
> >> -#endif       /* CONFIG_HARD_I2C */
> >> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> >> index 052fe36..00a12cc 100644
> >> --- a/drivers/i2c/Makefile
> >> +++ b/drivers/i2c/Makefile
> >> @@ -29,6 +29,7 @@ COBJS-$(CONFIG_BFIN_TWI_I2C) += bfin-twi_i2c.o
> >>  COBJS-$(CONFIG_DRIVER_DAVINCI_I2C) += davinci_i2c.o
> >>  COBJS-$(CONFIG_FSL_I2C) += fsl_i2c.o
> >>  COBJS-$(CONFIG_I2C_MVTWSI) += mvtwsi.o
> >> +COBJS-$(CONFIG_I2C_MV) += mv_i2c.o
> >
> > Mvtwsi and mv_i2c are two i2c drivers for Marvell.
> > Can you merge these two?
> 
> As I explain to you before. Although kirkwood and pxa series are both
> the product
> of Marvell, but it don't necessary means that they must have the same
> controller
> for both product line. For the i2c part, they just use two different
> controller.
> So why you keep request merge those two? Do you mean you want to
> create a unique I2C
> framework for whole i2c drivers in drivers/i2c?

Hi Lei

1. Most of i2c drivers supported in u-boot are either in SoC specific folder or in drivers/i2c folder, there is no as such thumb rule here.
2. Secondly all these drivers have some common code, mostly i2c_read, i2c_write, i2c_probe, etc.. 
3. Specific to Marvell, we already have mvtwsi.c that supports Kirkwood and Orion5X SoCs. Whereas you are adding new mvi2c.c that will support armada100, pantheon apart from pxa.
4. What about if we need to support some new Marvell SoC with different i2C controller? Do we add one more driver?

I would love if some one creates drivers/i2c/i2c_core.c??? not necessarily you ;-)

Here is what I would like to suggest.
1. cmd_i2c mostly interfaced with i2c_probe, i2c_read, i2c_write, i2c_get_bun_num, i2c_set_bus_num, those should go in drivers/i2c_core.c
2. APIs like i2c_start, i2c_stop, i2c_send, i2c_recv, i2c_reset are more I2C controller specific and those will be different implementation on different SoCs, those can go in SoC specific i2c driver file.
3. all I2C driver files should be in drivers/i2c/
4. i2c_read/write API need to be redefined since those are not generic to be used to access any I2C peripheral( most of the device don't need address to be programmed)
5. Flags must be provided for i2c_read/write APIs to have precise control to execute I2C_START/I2C_STOP sequence in the call.

Since you are the one starting with re-using pxa driver for armada100 and Pantheon SoC, why don't you split it into i2c_core.c and i2c_pxa.c? then add i2c_armada100.c and i2c_pantheon.c?
Others can migrate in the similar way. (even mvtwsi,c)

Hi Heiko
What do you think on this?

Regards..
Prafulla . .
Heiko Schocher March 23, 2011, 8:22 a.m. UTC | #4
Hello Prafulla,

Prafulla Wadaskar wrote:
>> -----Original Message-----
>> From: Lei Wen [mailto:adrian.wenl@gmail.com]
>> Sent: Tuesday, March 22, 2011 6:14 PM
>> To: Prafulla Wadaskar
>> Cc: Lei Wen; Heiko Schocher; Wolfgang Denk; u-boot@lists.denx.de; Marek
>> Vasut; Ashish Karkare; Prabhanjan Sarnaik; Yu Tang
>> Subject: Re: [PATCH V3 1/5] pxa: move i2c driver to the common place
>>
> ...snip...
>>>>  drivers/i2c/Makefile      |    1 +
>>>>  drivers/i2c/mv_i2c.c      |  452
>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>>  include/configs/innokom.h |    1 +
>>>>  include/configs/xm250.h   |    1 +
>>>>  6 files changed, 455 insertions(+), 470 deletions(-)
>>>>  delete mode 100644 arch/arm/cpu/pxa/i2c.c
>>>>  create mode 100644 drivers/i2c/mv_i2c.c
>>>
>>> ...snip...
>>>
>>>> -#endif       /* CONFIG_HARD_I2C */
>>>> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
>>>> index 052fe36..00a12cc 100644
>>>> --- a/drivers/i2c/Makefile
>>>> +++ b/drivers/i2c/Makefile
>>>> @@ -29,6 +29,7 @@ COBJS-$(CONFIG_BFIN_TWI_I2C) += bfin-twi_i2c.o
>>>>  COBJS-$(CONFIG_DRIVER_DAVINCI_I2C) += davinci_i2c.o
>>>>  COBJS-$(CONFIG_FSL_I2C) += fsl_i2c.o
>>>>  COBJS-$(CONFIG_I2C_MVTWSI) += mvtwsi.o
>>>> +COBJS-$(CONFIG_I2C_MV) += mv_i2c.o
>>> Mvtwsi and mv_i2c are two i2c drivers for Marvell.
>>> Can you merge these two?
>> As I explain to you before. Although kirkwood and pxa series are both
>> the product
>> of Marvell, but it don't necessary means that they must have the same
>> controller
>> for both product line. For the i2c part, they just use two different
>> controller.
>> So why you keep request merge those two? Do you mean you want to
>> create a unique I2C
>> framework for whole i2c drivers in drivers/i2c?
> 
> Hi Lei
> 
> 1. Most of i2c drivers supported in u-boot are either in SoC specific folder or in drivers/i2c folder, there is no as such thumb rule here.

New drivers should go to drivers/i2c !
The existing (old) drivers are just not moved ...
patches welcome!

> 2. Secondly all these drivers have some common code, mostly i2c_read, i2c_write, i2c_probe, etc.. 
> 3. Specific to Marvell, we already have mvtwsi.c that supports Kirkwood and Orion5X SoCs. Whereas you are adding new mvi2c.c that will support armada100, pantheon apart from pxa.
> 4. What about if we need to support some new Marvell SoC with different i2C controller? Do we add one more driver?
> 
> I would love if some one creates drivers/i2c/i2c_core.c??? not necessarily you ;-)
> 
> Here is what I would like to suggest.
> 1. cmd_i2c mostly interfaced with i2c_probe, i2c_read, i2c_write, i2c_get_bun_num, i2c_set_bus_num, those should go in drivers/i2c_core.c

Yep, but see below comment.

> 2. APIs like i2c_start, i2c_stop, i2c_send, i2c_recv, i2c_reset are more I2C controller specific and those will be different implementation on different SoCs, those can go in SoC specific i2c driver file.

Yep.

> 3. all I2C driver files should be in drivers/i2c/

Yep.

> 4. i2c_read/write API need to be redefined since those are not generic to be used to access any I2C peripheral( most of the device don't need address to be programmed)

With which devices do you have problems? You can set with
"i2c mw chip address.0 ..." an addresslen = 0 ... or?

> 5. Flags must be provided for i2c_read/write APIs to have precise control to execute I2C_START/I2C_STOP sequence in the call.

If needed, yes.

> Since you are the one starting with re-using pxa driver for armada100 and Pantheon SoC, why don't you split it into i2c_core.c and i2c_pxa.c? then add i2c_armada100.c and i2c_pantheon.c?
> Others can migrate in the similar way. (even mvtwsi,c)
> 
> Hi Heiko
> What do you think on this?

I made such a i2c_core.c file in the multibus/multiadapter branch
for the i2c subsystem, see here:

http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=shortlog;h=refs/heads/multibus_v2

(also you can grep in u-boot ML for discussions about)

Actual state:
- arm boards: i2c driver tested on suen3 (kirkwood based board)
- powerpc boards: i2c driver tested on mpc82xx, 83xx, 8xx boards
- all others just coded not tested ...
  (A result of lacking hw and/or time)

ToDo:
- rebase against current head
  (Sorry, didn;t found time to rebase it since Oktober 2010)
- Update README
- porting arrived new i2c drivers, boards since Oktober 2010
  to this new i2c approach
- testing, testing, testing ... Testers welcome!

I prefer to integrate this to mainline, before we do above steps
(4?) and 5. As Lei mentioned, if a soc/board has different i2c
controllers and more than one bus we *need* this approach,
so it is not worthwhile to introduce a i2c_core file only ...
instead we should forwarding this branch to mainline?

Patches are welcome ;-)

I am afraid, this would get such a big cut as the arm relocation
changes ... and it affects all archs.

bye,
Heiko
Lei Wen March 23, 2011, 8:43 a.m. UTC | #5
Hi Heiko,

On Wed, Mar 23, 2011 at 4:22 PM, Heiko Schocher <hs@denx.de> wrote:
> Hello Prafulla,
>
> Prafulla Wadaskar wrote:
>>> -----Original Message-----
>>> From: Lei Wen [mailto:adrian.wenl@gmail.com]
>>> Sent: Tuesday, March 22, 2011 6:14 PM
>>> To: Prafulla Wadaskar
>>> Cc: Lei Wen; Heiko Schocher; Wolfgang Denk; u-boot@lists.denx.de; Marek
>>> Vasut; Ashish Karkare; Prabhanjan Sarnaik; Yu Tang
>>> Subject: Re: [PATCH V3 1/5] pxa: move i2c driver to the common place
>>>
>> ...snip...
>>>>>  drivers/i2c/Makefile      |    1 +
>>>>>  drivers/i2c/mv_i2c.c      |  452
>>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>>>  include/configs/innokom.h |    1 +
>>>>>  include/configs/xm250.h   |    1 +
>>>>>  6 files changed, 455 insertions(+), 470 deletions(-)
>>>>>  delete mode 100644 arch/arm/cpu/pxa/i2c.c
>>>>>  create mode 100644 drivers/i2c/mv_i2c.c
>>>>
>>>> ...snip...
>>>>
>>>>> -#endif       /* CONFIG_HARD_I2C */
>>>>> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
>>>>> index 052fe36..00a12cc 100644
>>>>> --- a/drivers/i2c/Makefile
>>>>> +++ b/drivers/i2c/Makefile
>>>>> @@ -29,6 +29,7 @@ COBJS-$(CONFIG_BFIN_TWI_I2C) += bfin-twi_i2c.o
>>>>>  COBJS-$(CONFIG_DRIVER_DAVINCI_I2C) += davinci_i2c.o
>>>>>  COBJS-$(CONFIG_FSL_I2C) += fsl_i2c.o
>>>>>  COBJS-$(CONFIG_I2C_MVTWSI) += mvtwsi.o
>>>>> +COBJS-$(CONFIG_I2C_MV) += mv_i2c.o
>>>> Mvtwsi and mv_i2c are two i2c drivers for Marvell.
>>>> Can you merge these two?
>>> As I explain to you before. Although kirkwood and pxa series are both
>>> the product
>>> of Marvell, but it don't necessary means that they must have the same
>>> controller
>>> for both product line. For the i2c part, they just use two different
>>> controller.
>>> So why you keep request merge those two? Do you mean you want to
>>> create a unique I2C
>>> framework for whole i2c drivers in drivers/i2c?
>>
>> Hi Lei
>>
>> 1. Most of i2c drivers supported in u-boot are either in SoC specific folder or in drivers/i2c folder, there is no as such thumb rule here.
>
> New drivers should go to drivers/i2c !
> The existing (old) drivers are just not moved ...
> patches welcome!
>
>> 2. Secondly all these drivers have some common code, mostly i2c_read, i2c_write, i2c_probe, etc..
>> 3. Specific to Marvell, we already have mvtwsi.c that supports Kirkwood and Orion5X SoCs. Whereas you are adding new mvi2c.c that will support armada100, pantheon apart from pxa.
>> 4. What about if we need to support some new Marvell SoC with different i2C controller? Do we add one more driver?
>>
>> I would love if some one creates drivers/i2c/i2c_core.c??? not necessarily you ;-)
>>
>> Here is what I would like to suggest.
>> 1. cmd_i2c mostly interfaced with i2c_probe, i2c_read, i2c_write, i2c_get_bun_num, i2c_set_bus_num, those should go in drivers/i2c_core.c
>
> Yep, but see below comment.
>
>> 2. APIs like i2c_start, i2c_stop, i2c_send, i2c_recv, i2c_reset are more I2C controller specific and those will be different implementation on different SoCs, those can go in SoC specific i2c driver file.
>
> Yep.
>
>> 3. all I2C driver files should be in drivers/i2c/
>
> Yep.
>
>> 4. i2c_read/write API need to be redefined since those are not generic to be used to access any I2C peripheral( most of the device don't need address to be programmed)
>
> With which devices do you have problems? You can set with
> "i2c mw chip address.0 ..." an addresslen = 0 ... or?
>
>> 5. Flags must be provided for i2c_read/write APIs to have precise control to execute I2C_START/I2C_STOP sequence in the call.
>
> If needed, yes.
>
>> Since you are the one starting with re-using pxa driver for armada100 and Pantheon SoC, why don't you split it into i2c_core.c and i2c_pxa.c? then add i2c_armada100.c and i2c_pantheon.c?
>> Others can migrate in the similar way. (even mvtwsi,c)
>>
>> Hi Heiko
>> What do you think on this?
>
> I made such a i2c_core.c file in the multibus/multiadapter branch
> for the i2c subsystem, see here:
>
> http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=shortlog;h=refs/heads/multibus_v2
>
> (also you can grep in u-boot ML for discussions about)
>
> Actual state:
> - arm boards: i2c driver tested on suen3 (kirkwood based board)
> - powerpc boards: i2c driver tested on mpc82xx, 83xx, 8xx boards
> - all others just coded not tested ...
>  (A result of lacking hw and/or time)
>
> ToDo:
> - rebase against current head
>  (Sorry, didn;t found time to rebase it since Oktober 2010)
> - Update README
> - porting arrived new i2c drivers, boards since Oktober 2010
>  to this new i2c approach
> - testing, testing, testing ... Testers welcome!
>
> I prefer to integrate this to mainline, before we do above steps
> (4?) and 5. As Lei mentioned, if a soc/board has different i2c
> controllers and more than one bus we *need* this approach,
> so it is not worthwhile to introduce a i2c_core file only ...
> instead we should forwarding this branch to mainline?
>
> Patches are welcome ;-)
>
> I am afraid, this would get such a big cut as the arm relocation
> changes ... and it affects all archs.

It is certainly a big change for introduce the i2c-core framework. :)

Also my incoming mmc/sd enabling patch for pantheon and armada100
is also based on this i2c enabling patch, as I need the i2c to turn on the
repsonding pmic power connection.

While we could get a i2c working pantheon, armada100,  and other pxa
series platform now with this patch set. So what about Could we merge
this first, and
gradually change to the i2c framework, test and make it mature.

Prafulla,
What do you think for this proposal?

Thanks,
Lei
Heiko Schocher March 23, 2011, 8:53 a.m. UTC | #6
Hello Lei,

Lei Wen wrote:
> Hi Heiko,
> 
> On Wed, Mar 23, 2011 at 4:22 PM, Heiko Schocher <hs@denx.de> wrote:
>> Hello Prafulla,
>>
>> Prafulla Wadaskar wrote:
>>>> -----Original Message-----
>>>> From: Lei Wen [mailto:adrian.wenl@gmail.com]
>>>> Sent: Tuesday, March 22, 2011 6:14 PM
>>>> To: Prafulla Wadaskar
>>>> Cc: Lei Wen; Heiko Schocher; Wolfgang Denk; u-boot@lists.denx.de; Marek
>>>> Vasut; Ashish Karkare; Prabhanjan Sarnaik; Yu Tang
>>>> Subject: Re: [PATCH V3 1/5] pxa: move i2c driver to the common place
>>>>
>>> ...snip...
>>>>>>  drivers/i2c/Makefile      |    1 +
>>>>>>  drivers/i2c/mv_i2c.c      |  452
>>>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>>>>  include/configs/innokom.h |    1 +
>>>>>>  include/configs/xm250.h   |    1 +
>>>>>>  6 files changed, 455 insertions(+), 470 deletions(-)
>>>>>>  delete mode 100644 arch/arm/cpu/pxa/i2c.c
>>>>>>  create mode 100644 drivers/i2c/mv_i2c.c
>>>>> ...snip...
>>>>>
>>>>>> -#endif       /* CONFIG_HARD_I2C */
>>>>>> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
>>>>>> index 052fe36..00a12cc 100644
>>>>>> --- a/drivers/i2c/Makefile
>>>>>> +++ b/drivers/i2c/Makefile
>>>>>> @@ -29,6 +29,7 @@ COBJS-$(CONFIG_BFIN_TWI_I2C) += bfin-twi_i2c.o
>>>>>>  COBJS-$(CONFIG_DRIVER_DAVINCI_I2C) += davinci_i2c.o
>>>>>>  COBJS-$(CONFIG_FSL_I2C) += fsl_i2c.o
>>>>>>  COBJS-$(CONFIG_I2C_MVTWSI) += mvtwsi.o
>>>>>> +COBJS-$(CONFIG_I2C_MV) += mv_i2c.o
>>>>> Mvtwsi and mv_i2c are two i2c drivers for Marvell.
>>>>> Can you merge these two?
>>>> As I explain to you before. Although kirkwood and pxa series are both
>>>> the product
>>>> of Marvell, but it don't necessary means that they must have the same
>>>> controller
>>>> for both product line. For the i2c part, they just use two different
>>>> controller.
>>>> So why you keep request merge those two? Do you mean you want to
>>>> create a unique I2C
>>>> framework for whole i2c drivers in drivers/i2c?
>>> Hi Lei
>>>
>>> 1. Most of i2c drivers supported in u-boot are either in SoC specific folder or in drivers/i2c folder, there is no as such thumb rule here.
>> New drivers should go to drivers/i2c !
>> The existing (old) drivers are just not moved ...
>> patches welcome!
>>
>>> 2. Secondly all these drivers have some common code, mostly i2c_read, i2c_write, i2c_probe, etc..
>>> 3. Specific to Marvell, we already have mvtwsi.c that supports Kirkwood and Orion5X SoCs. Whereas you are adding new mvi2c.c that will support armada100, pantheon apart from pxa.
>>> 4. What about if we need to support some new Marvell SoC with different i2C controller? Do we add one more driver?
>>>
>>> I would love if some one creates drivers/i2c/i2c_core.c??? not necessarily you ;-)
>>>
>>> Here is what I would like to suggest.
>>> 1. cmd_i2c mostly interfaced with i2c_probe, i2c_read, i2c_write, i2c_get_bun_num, i2c_set_bus_num, those should go in drivers/i2c_core.c
>> Yep, but see below comment.
>>
>>> 2. APIs like i2c_start, i2c_stop, i2c_send, i2c_recv, i2c_reset are more I2C controller specific and those will be different implementation on different SoCs, those can go in SoC specific i2c driver file.
>> Yep.
>>
>>> 3. all I2C driver files should be in drivers/i2c/
>> Yep.
>>
>>> 4. i2c_read/write API need to be redefined since those are not generic to be used to access any I2C peripheral( most of the device don't need address to be programmed)
>> With which devices do you have problems? You can set with
>> "i2c mw chip address.0 ..." an addresslen = 0 ... or?
>>
>>> 5. Flags must be provided for i2c_read/write APIs to have precise control to execute I2C_START/I2C_STOP sequence in the call.
>> If needed, yes.
>>
>>> Since you are the one starting with re-using pxa driver for armada100 and Pantheon SoC, why don't you split it into i2c_core.c and i2c_pxa.c? then add i2c_armada100.c and i2c_pantheon.c?
>>> Others can migrate in the similar way. (even mvtwsi,c)
>>>
>>> Hi Heiko
>>> What do you think on this?
>> I made such a i2c_core.c file in the multibus/multiadapter branch
>> for the i2c subsystem, see here:
>>
>> http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=shortlog;h=refs/heads/multibus_v2
>>
>> (also you can grep in u-boot ML for discussions about)
>>
>> Actual state:
>> - arm boards: i2c driver tested on suen3 (kirkwood based board)
>> - powerpc boards: i2c driver tested on mpc82xx, 83xx, 8xx boards
>> - all others just coded not tested ...
>>  (A result of lacking hw and/or time)
>>
>> ToDo:
>> - rebase against current head
>>  (Sorry, didn;t found time to rebase it since Oktober 2010)
>> - Update README
>> - porting arrived new i2c drivers, boards since Oktober 2010
>>  to this new i2c approach
>> - testing, testing, testing ... Testers welcome!
>>
>> I prefer to integrate this to mainline, before we do above steps
>> (4?) and 5. As Lei mentioned, if a soc/board has different i2c
>> controllers and more than one bus we *need* this approach,
>> so it is not worthwhile to introduce a i2c_core file only ...
>> instead we should forwarding this branch to mainline?
>>
>> Patches are welcome ;-)
>>
>> I am afraid, this would get such a big cut as the arm relocation
>> changes ... and it affects all archs.
> 
> It is certainly a big change for introduce the i2c-core framework. :)
> 
> Also my incoming mmc/sd enabling patch for pantheon and armada100
> is also based on this i2c enabling patch, as I need the i2c to turn on the
> repsonding pmic power connection.
> 
> While we could get a i2c working pantheon, armada100,  and other pxa
> series platform now with this patch set. So what about Could we merge
> this first, and
> gradually change to the i2c framework, test and make it mature.

I am fine with that, but please address the other comments from
Prafulla and Wolfgang (rename defines, use standard accessors).

If you plan to investigate time for the multibus/multiadapter
i2c branch, let me know, maybe I can rebase this branch before
you use it.

> Prafulla,
> What do you think for this proposal?
> 
> Thanks,
> Lei

bye,
Heiko
Lei Wen March 23, 2011, 8:56 a.m. UTC | #7
Hi Heiko,

On Wed, Mar 23, 2011 at 4:53 PM, Heiko Schocher <hs@denx.de> wrote:
> Hello Lei,
>
> Lei Wen wrote:
>> Hi Heiko,
>>
>> On Wed, Mar 23, 2011 at 4:22 PM, Heiko Schocher <hs@denx.de> wrote:
>>> Hello Prafulla,
>>>
>>> Prafulla Wadaskar wrote:
>>>>> -----Original Message-----
>>>>> From: Lei Wen [mailto:adrian.wenl@gmail.com]
>>>>> Sent: Tuesday, March 22, 2011 6:14 PM
>>>>> To: Prafulla Wadaskar
>>>>> Cc: Lei Wen; Heiko Schocher; Wolfgang Denk; u-boot@lists.denx.de; Marek
>>>>> Vasut; Ashish Karkare; Prabhanjan Sarnaik; Yu Tang
>>>>> Subject: Re: [PATCH V3 1/5] pxa: move i2c driver to the common place
>>>>>
>>>> ...snip...
>>>>>>>  drivers/i2c/Makefile      |    1 +
>>>>>>>  drivers/i2c/mv_i2c.c      |  452
>>>>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>>>>>  include/configs/innokom.h |    1 +
>>>>>>>  include/configs/xm250.h   |    1 +
>>>>>>>  6 files changed, 455 insertions(+), 470 deletions(-)
>>>>>>>  delete mode 100644 arch/arm/cpu/pxa/i2c.c
>>>>>>>  create mode 100644 drivers/i2c/mv_i2c.c
>>>>>> ...snip...
>>>>>>
>>>>>>> -#endif       /* CONFIG_HARD_I2C */
>>>>>>> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
>>>>>>> index 052fe36..00a12cc 100644
>>>>>>> --- a/drivers/i2c/Makefile
>>>>>>> +++ b/drivers/i2c/Makefile
>>>>>>> @@ -29,6 +29,7 @@ COBJS-$(CONFIG_BFIN_TWI_I2C) += bfin-twi_i2c.o
>>>>>>>  COBJS-$(CONFIG_DRIVER_DAVINCI_I2C) += davinci_i2c.o
>>>>>>>  COBJS-$(CONFIG_FSL_I2C) += fsl_i2c.o
>>>>>>>  COBJS-$(CONFIG_I2C_MVTWSI) += mvtwsi.o
>>>>>>> +COBJS-$(CONFIG_I2C_MV) += mv_i2c.o
>>>>>> Mvtwsi and mv_i2c are two i2c drivers for Marvell.
>>>>>> Can you merge these two?
>>>>> As I explain to you before. Although kirkwood and pxa series are both
>>>>> the product
>>>>> of Marvell, but it don't necessary means that they must have the same
>>>>> controller
>>>>> for both product line. For the i2c part, they just use two different
>>>>> controller.
>>>>> So why you keep request merge those two? Do you mean you want to
>>>>> create a unique I2C
>>>>> framework for whole i2c drivers in drivers/i2c?
>>>> Hi Lei
>>>>
>>>> 1. Most of i2c drivers supported in u-boot are either in SoC specific folder or in drivers/i2c folder, there is no as such thumb rule here.
>>> New drivers should go to drivers/i2c !
>>> The existing (old) drivers are just not moved ...
>>> patches welcome!
>>>
>>>> 2. Secondly all these drivers have some common code, mostly i2c_read, i2c_write, i2c_probe, etc..
>>>> 3. Specific to Marvell, we already have mvtwsi.c that supports Kirkwood and Orion5X SoCs. Whereas you are adding new mvi2c.c that will support armada100, pantheon apart from pxa.
>>>> 4. What about if we need to support some new Marvell SoC with different i2C controller? Do we add one more driver?
>>>>
>>>> I would love if some one creates drivers/i2c/i2c_core.c??? not necessarily you ;-)
>>>>
>>>> Here is what I would like to suggest.
>>>> 1. cmd_i2c mostly interfaced with i2c_probe, i2c_read, i2c_write, i2c_get_bun_num, i2c_set_bus_num, those should go in drivers/i2c_core.c
>>> Yep, but see below comment.
>>>
>>>> 2. APIs like i2c_start, i2c_stop, i2c_send, i2c_recv, i2c_reset are more I2C controller specific and those will be different implementation on different SoCs, those can go in SoC specific i2c driver file.
>>> Yep.
>>>
>>>> 3. all I2C driver files should be in drivers/i2c/
>>> Yep.
>>>
>>>> 4. i2c_read/write API need to be redefined since those are not generic to be used to access any I2C peripheral( most of the device don't need address to be programmed)
>>> With which devices do you have problems? You can set with
>>> "i2c mw chip address.0 ..." an addresslen = 0 ... or?
>>>
>>>> 5. Flags must be provided for i2c_read/write APIs to have precise control to execute I2C_START/I2C_STOP sequence in the call.
>>> If needed, yes.
>>>
>>>> Since you are the one starting with re-using pxa driver for armada100 and Pantheon SoC, why don't you split it into i2c_core.c and i2c_pxa.c? then add i2c_armada100.c and i2c_pantheon.c?
>>>> Others can migrate in the similar way. (even mvtwsi,c)
>>>>
>>>> Hi Heiko
>>>> What do you think on this?
>>> I made such a i2c_core.c file in the multibus/multiadapter branch
>>> for the i2c subsystem, see here:
>>>
>>> http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=shortlog;h=refs/heads/multibus_v2
>>>
>>> (also you can grep in u-boot ML for discussions about)
>>>
>>> Actual state:
>>> - arm boards: i2c driver tested on suen3 (kirkwood based board)
>>> - powerpc boards: i2c driver tested on mpc82xx, 83xx, 8xx boards
>>> - all others just coded not tested ...
>>>  (A result of lacking hw and/or time)
>>>
>>> ToDo:
>>> - rebase against current head
>>>  (Sorry, didn;t found time to rebase it since Oktober 2010)
>>> - Update README
>>> - porting arrived new i2c drivers, boards since Oktober 2010
>>>  to this new i2c approach
>>> - testing, testing, testing ... Testers welcome!
>>>
>>> I prefer to integrate this to mainline, before we do above steps
>>> (4?) and 5. As Lei mentioned, if a soc/board has different i2c
>>> controllers and more than one bus we *need* this approach,
>>> so it is not worthwhile to introduce a i2c_core file only ...
>>> instead we should forwarding this branch to mainline?
>>>
>>> Patches are welcome ;-)
>>>
>>> I am afraid, this would get such a big cut as the arm relocation
>>> changes ... and it affects all archs.
>>
>> It is certainly a big change for introduce the i2c-core framework. :)
>>
>> Also my incoming mmc/sd enabling patch for pantheon and armada100
>> is also based on this i2c enabling patch, as I need the i2c to turn on the
>> repsonding pmic power connection.
>>
>> While we could get a i2c working pantheon, armada100,  and other pxa
>> series platform now with this patch set. So what about Could we merge
>> this first, and
>> gradually change to the i2c framework, test and make it mature.
>
> I am fine with that, but please address the other comments from
> Prafulla and Wolfgang (rename defines, use standard accessors).

Ok, I would post another round of patch to fix those issue Prafulla and Wolfgang
metioned.

>
> If you plan to investigate time for the multibus/multiadapter
> i2c branch, let me know, maybe I can rebase this branch before
> you use it.

Currently, I have no bandwidth to do this... :(
Maybe I would try to investigate when I have time.

Best regards,
Lei
Heiko Schocher March 23, 2011, 9:07 a.m. UTC | #8
Hello Lei,

Lei Wen wrote:
> Hi Heiko,
> 
> On Wed, Mar 23, 2011 at 4:53 PM, Heiko Schocher <hs@denx.de> wrote:
>> Hello Lei,
>>
>> Lei Wen wrote:
>>> Hi Heiko,
>>>
>>> On Wed, Mar 23, 2011 at 4:22 PM, Heiko Schocher <hs@denx.de> wrote:
>>>> Hello Prafulla,
>>>>
>>>> Prafulla Wadaskar wrote:
>>>>>> -----Original Message-----
>>>>>> From: Lei Wen [mailto:adrian.wenl@gmail.com]
>>>>>> Sent: Tuesday, March 22, 2011 6:14 PM
>>>>>> To: Prafulla Wadaskar
>>>>>> Cc: Lei Wen; Heiko Schocher; Wolfgang Denk; u-boot@lists.denx.de; Marek
>>>>>> Vasut; Ashish Karkare; Prabhanjan Sarnaik; Yu Tang
>>>>>> Subject: Re: [PATCH V3 1/5] pxa: move i2c driver to the common place
>>>>>>
[...]
>>> Also my incoming mmc/sd enabling patch for pantheon and armada100
>>> is also based on this i2c enabling patch, as I need the i2c to turn on the
>>> repsonding pmic power connection.
>>>
>>> While we could get a i2c working pantheon, armada100,  and other pxa
>>> series platform now with this patch set. So what about Could we merge
>>> this first, and
>>> gradually change to the i2c framework, test and make it mature.
>> I am fine with that, but please address the other comments from
>> Prafulla and Wolfgang (rename defines, use standard accessors).
> 
> Ok, I would post another round of patch to fix those issue Prafulla and Wolfgang
> metioned.

Ok, thanks!

>> If you plan to investigate time for the multibus/multiadapter
>> i2c branch, let me know, maybe I can rebase this branch before
>> you use it.
> 
> Currently, I have no bandwidth to do this... :(
> Maybe I would try to investigate when I have time.

:-(

Thats also my state ... please give me a notice, if you find some time,
so we can sync us ...

bye,
Heiko
Prafulla Wadaskar March 23, 2011, 9:12 a.m. UTC | #9
> -----Original Message-----
> From: Heiko Schocher [mailto:hs@denx.de]
> Sent: Wednesday, March 23, 2011 2:23 PM
> To: Lei Wen
> Cc: Prafulla Wadaskar; Lei Wen; Wolfgang Denk; u-boot@lists.denx.de;
> Marek Vasut; Ashish Karkare; Prabhanjan Sarnaik; Yu Tang
> Subject: Re: [PATCH V3 1/5] pxa: move i2c driver to the common place
> 
> Hello Lei,
> 
> Lei Wen wrote:
> > Hi Heiko,
> >
> > On Wed, Mar 23, 2011 at 4:22 PM, Heiko Schocher <hs@denx.de> wrote:
> >> Hello Prafulla,
> >>
> >> Prafulla Wadaskar wrote:
> >>>> -----Original Message-----
> >>>> From: Lei Wen [mailto:adrian.wenl@gmail.com]
> >>>> Sent: Tuesday, March 22, 2011 6:14 PM
> >>>> To: Prafulla Wadaskar
> >>>> Cc: Lei Wen; Heiko Schocher; Wolfgang Denk; u-boot@lists.denx.de;
> Marek
> >>>> Vasut; Ashish Karkare; Prabhanjan Sarnaik; Yu Tang
> >>>> Subject: Re: [PATCH V3 1/5] pxa: move i2c driver to the common
> place
> >>>>
> >>> ...snip...
> >>>>>>  drivers/i2c/Makefile      |    1 +
> >>>>>>  drivers/i2c/mv_i2c.c      |  452
> >>>>>> +++++++++++++++++++++++++++++++++++++++++++
> >>>>>>  include/configs/innokom.h |    1 +
> >>>>>>  include/configs/xm250.h   |    1 +
> >>>>>>  6 files changed, 455 insertions(+), 470 deletions(-)
> >>>>>>  delete mode 100644 arch/arm/cpu/pxa/i2c.c
> >>>>>>  create mode 100644 drivers/i2c/mv_i2c.c
> >>>>> ...snip...
> >>>>>
> >>>>>> -#endif       /* CONFIG_HARD_I2C */
> >>>>>> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> >>>>>> index 052fe36..00a12cc 100644
> >>>>>> --- a/drivers/i2c/Makefile
> >>>>>> +++ b/drivers/i2c/Makefile
> >>>>>> @@ -29,6 +29,7 @@ COBJS-$(CONFIG_BFIN_TWI_I2C) += bfin-twi_i2c.o
> >>>>>>  COBJS-$(CONFIG_DRIVER_DAVINCI_I2C) += davinci_i2c.o
> >>>>>>  COBJS-$(CONFIG_FSL_I2C) += fsl_i2c.o
> >>>>>>  COBJS-$(CONFIG_I2C_MVTWSI) += mvtwsi.o
> >>>>>> +COBJS-$(CONFIG_I2C_MV) += mv_i2c.o
> >>>>> Mvtwsi and mv_i2c are two i2c drivers for Marvell.
> >>>>> Can you merge these two?
> >>>> As I explain to you before. Although kirkwood and pxa series are
> both
> >>>> the product
> >>>> of Marvell, but it don't necessary means that they must have the
> same
> >>>> controller
> >>>> for both product line. For the i2c part, they just use two
> different
> >>>> controller.
> >>>> So why you keep request merge those two? Do you mean you want to
> >>>> create a unique I2C
> >>>> framework for whole i2c drivers in drivers/i2c?
> >>> Hi Lei
> >>>
> >>> 1. Most of i2c drivers supported in u-boot are either in SoC
> specific folder or in drivers/i2c folder, there is no as such thumb rule
> here.
> >> New drivers should go to drivers/i2c !
> >> The existing (old) drivers are just not moved ...
> >> patches welcome!
> >>
> >>> 2. Secondly all these drivers have some common code, mostly
> i2c_read, i2c_write, i2c_probe, etc..
> >>> 3. Specific to Marvell, we already have mvtwsi.c that supports
> Kirkwood and Orion5X SoCs. Whereas you are adding new mvi2c.c that will
> support armada100, pantheon apart from pxa.
> >>> 4. What about if we need to support some new Marvell SoC with
> different i2C controller? Do we add one more driver?
> >>>
> >>> I would love if some one creates drivers/i2c/i2c_core.c??? not
> necessarily you ;-)
> >>>
> >>> Here is what I would like to suggest.
> >>> 1. cmd_i2c mostly interfaced with i2c_probe, i2c_read, i2c_write,
> i2c_get_bun_num, i2c_set_bus_num, those should go in drivers/i2c_core.c
> >> Yep, but see below comment.
> >>
> >>> 2. APIs like i2c_start, i2c_stop, i2c_send, i2c_recv, i2c_reset are
> more I2C controller specific and those will be different implementation
> on different SoCs, those can go in SoC specific i2c driver file.
> >> Yep.
> >>
> >>> 3. all I2C driver files should be in drivers/i2c/
> >> Yep.
> >>
> >>> 4. i2c_read/write API need to be redefined since those are not
> generic to be used to access any I2C peripheral( most of the device
> don't need address to be programmed)
> >> With which devices do you have problems? You can set with
> >> "i2c mw chip address.0 ..." an addresslen = 0 ... or?
> >>
> >>> 5. Flags must be provided for i2c_read/write APIs to have precise
> control to execute I2C_START/I2C_STOP sequence in the call.
> >> If needed, yes.
> >>
> >>> Since you are the one starting with re-using pxa driver for
> armada100 and Pantheon SoC, why don't you split it into i2c_core.c and
> i2c_pxa.c? then add i2c_armada100.c and i2c_pantheon.c?
> >>> Others can migrate in the similar way. (even mvtwsi,c)
> >>>
> >>> Hi Heiko
> >>> What do you think on this?
> >> I made such a i2c_core.c file in the multibus/multiadapter branch
> >> for the i2c subsystem, see here:
> >>
> >> http://git.denx.de/?p=u-boot/u-boot-
> i2c.git;a=shortlog;h=refs/heads/multibus_v2
> >>
> >> (also you can grep in u-boot ML for discussions about)
> >>
> >> Actual state:
> >> - arm boards: i2c driver tested on suen3 (kirkwood based board)
> >> - powerpc boards: i2c driver tested on mpc82xx, 83xx, 8xx boards
> >> - all others just coded not tested ...
> >>  (A result of lacking hw and/or time)
> >>
> >> ToDo:
> >> - rebase against current head
> >>  (Sorry, didn;t found time to rebase it since Oktober 2010)
> >> - Update README
> >> - porting arrived new i2c drivers, boards since Oktober 2010
> >>  to this new i2c approach
> >> - testing, testing, testing ... Testers welcome!
> >>
> >> I prefer to integrate this to mainline, before we do above steps
> >> (4?) and 5. As Lei mentioned, if a soc/board has different i2c
> >> controllers and more than one bus we *need* this approach,
> >> so it is not worthwhile to introduce a i2c_core file only ...
> >> instead we should forwarding this branch to mainline?
> >>
> >> Patches are welcome ;-)
> >>
> >> I am afraid, this would get such a big cut as the arm relocation
> >> changes ... and it affects all archs.
> >
> > It is certainly a big change for introduce the i2c-core framework. :)
> >
> > Also my incoming mmc/sd enabling patch for pantheon and armada100
> > is also based on this i2c enabling patch, as I need the i2c to turn on
> the
> > repsonding pmic power connection.
> >
> > While we could get a i2c working pantheon, armada100,  and other pxa
> > series platform now with this patch set. So what about Could we merge
> > this first, and
> > gradually change to the i2c framework, test and make it mature.
> 
> I am fine with that, but please address the other comments from
> Prafulla and Wolfgang (rename defines, use standard accessors).

Hi Lei
I can understand your dependency.
Even I am fine with that, my intention was if we can make it better why not :-)

> 
> If you plan to investigate time for the multibus/multiadapter
> i2c branch, let me know, maybe I can rebase this branch before
> you use it.

This is a good approach indeed.
If you (Lei) agree, I will pull the multibus/multiadapter patches on u-boot-marvell.git which will be a good starting reference for your work.
If not, you may continue as you requested.

BTW: in his patch, he has renamed and moved boards/Marvell/common/i2c.c to mv_i2c.c.
I will be happy to see and test if the patch series by Heiko gets mainlined.

Regards..
Prafulla . .
Prafulla Wadaskar March 23, 2011, 9:16 a.m. UTC | #10
> -----Original Message-----
> From: Heiko Schocher [mailto:hs@denx.de]
> Sent: Wednesday, March 23, 2011 2:37 PM
> To: Lei Wen
> Cc: Prafulla Wadaskar; Lei Wen; Wolfgang Denk; u-boot@lists.denx.de;
> Marek Vasut; Ashish Karkare; Prabhanjan Sarnaik; Yu Tang
> Subject: Re: [PATCH V3 1/5] pxa: move i2c driver to the common place
> 
> Hello Lei,
> 
> Lei Wen wrote:
> > Hi Heiko,
> >
> > On Wed, Mar 23, 2011 at 4:53 PM, Heiko Schocher <hs@denx.de> wrote:
> >> Hello Lei,
> >>
> >> Lei Wen wrote:
> >>> Hi Heiko,
> >>>
> >>> On Wed, Mar 23, 2011 at 4:22 PM, Heiko Schocher <hs@denx.de> wrote:
> >>>> Hello Prafulla,
> >>>>
> >>>> Prafulla Wadaskar wrote:
> >>>>>> -----Original Message-----
> >>>>>> From: Lei Wen [mailto:adrian.wenl@gmail.com]
> >>>>>> Sent: Tuesday, March 22, 2011 6:14 PM
> >>>>>> To: Prafulla Wadaskar
> >>>>>> Cc: Lei Wen; Heiko Schocher; Wolfgang Denk; u-boot@lists.denx.de;
> Marek
> >>>>>> Vasut; Ashish Karkare; Prabhanjan Sarnaik; Yu Tang
> >>>>>> Subject: Re: [PATCH V3 1/5] pxa: move i2c driver to the common
> place
> >>>>>>
> [...]
> >>> Also my incoming mmc/sd enabling patch for pantheon and armada100
> >>> is also based on this i2c enabling patch, as I need the i2c to turn
> on the
> >>> repsonding pmic power connection.
> >>>
> >>> While we could get a i2c working pantheon, armada100,  and other pxa
> >>> series platform now with this patch set. So what about Could we
> merge
> >>> this first, and
> >>> gradually change to the i2c framework, test and make it mature.
> >> I am fine with that, but please address the other comments from
> >> Prafulla and Wolfgang (rename defines, use standard accessors).
> >
> > Ok, I would post another round of patch to fix those issue Prafulla
> and Wolfgang
> > metioned.
> 
> Ok, thanks!
> 
> >> If you plan to investigate time for the multibus/multiadapter
> >> i2c branch, let me know, maybe I can rebase this branch before
> >> you use it.
> >
> > Currently, I have no bandwidth to do this... :(
> > Maybe I would try to investigate when I have time.
> 
> :-(
> 
> Thats also my state ... please give me a notice, if you find some time,
> so we can sync us ...

Sorry I posted earlier email before checking this email.
Even same the my state too :-)
Well Let's lei get his stuff, we will think of migration latter :-(

Regards..
Prafulla . .
diff mbox

Patch

diff --git a/arch/arm/cpu/pxa/Makefile b/arch/arm/cpu/pxa/Makefile
index 49a6ed3..e8b59a3 100644
--- a/arch/arm/cpu/pxa/Makefile
+++ b/arch/arm/cpu/pxa/Makefile
@@ -28,7 +28,6 @@  LIB	= $(obj)lib$(CPU).o
 START	= start.o
 
 COBJS	+= cpu.o
-COBJS	+= i2c.o
 COBJS	+= pxafb.o
 COBJS	+= timer.o
 COBJS	+= usb.o
diff --git a/arch/arm/cpu/pxa/i2c.c b/arch/arm/cpu/pxa/i2c.c
deleted file mode 100644
index 7aa49ae..0000000
--- a/arch/arm/cpu/pxa/i2c.c
+++ /dev/null
@@ -1,469 +0,0 @@ 
-/*
- * (C) Copyright 2000
- * Paolo Scaffardi, AIRVENT SAM s.p.a - RIMINI(ITALY), arsenio@tin.it
- *
- * (C) Copyright 2000 Sysgo Real-Time Solutions, GmbH <www.elinos.com>
- * Marius Groeger <mgroeger@sysgo.de>
- *
- * (C) Copyright 2003 Pengutronix e.K.
- * Robert Schwebel <r.schwebel@pengutronix.de>
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of
- * the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- *
- * Back ported to the 8xx platform (from the 8260 platform) by
- * Murray.Jensen@cmst.csiro.au, 27-Jan-01.
- */
-
-/* FIXME: this file is PXA255 specific! What about other XScales? */
-
-#include <common.h>
-#include <asm/io.h>
-
-#ifdef CONFIG_HARD_I2C
-
-/*
- *	- CONFIG_SYS_I2C_SPEED
- *	- I2C_PXA_SLAVE_ADDR
- */
-
-#include <asm/arch/hardware.h>
-#include <asm/arch/pxa-regs.h>
-#include <i2c.h>
-
-/*#define	DEBUG_I2C	1	/###* activate local debugging output  */
-#define I2C_PXA_SLAVE_ADDR	0x1	/* slave pxa unit address           */
-
-#if (CONFIG_SYS_I2C_SPEED == 400000)
-#define I2C_ICR_INIT	(ICR_FM | ICR_BEIE | ICR_IRFIE | ICR_ITEIE | ICR_GCD | ICR_SCLE)
-#else
-#define I2C_ICR_INIT	(ICR_BEIE | ICR_IRFIE | ICR_ITEIE | ICR_GCD | ICR_SCLE)
-#endif
-
-#define I2C_ISR_INIT		0x7FF
-
-#ifdef DEBUG_I2C
-#define PRINTD(x) printf x
-#else
-#define PRINTD(x)
-#endif
-
-
-/* Shall the current transfer have a start/stop condition? */
-#define I2C_COND_NORMAL		0
-#define I2C_COND_START		1
-#define I2C_COND_STOP		2
-
-/* Shall the current transfer be ack/nacked or being waited for it? */
-#define I2C_ACKNAK_WAITACK	1
-#define I2C_ACKNAK_SENDACK	2
-#define I2C_ACKNAK_SENDNAK	4
-
-/* Specify who shall transfer the data (master or slave) */
-#define I2C_READ		0
-#define I2C_WRITE		1
-
-/* All transfers are described by this data structure */
-struct i2c_msg {
-	u8 condition;
-	u8 acknack;
-	u8 direction;
-	u8 data;
-};
-
-
-/**
- * i2c_pxa_reset: - reset the host controller
- *
- */
-
-static void i2c_reset( void )
-{
-	writel(readl(ICR) & ~ICR_IUE, ICR);	/* disable unit */
-	writel(readl(ICR) | ICR_UR, ICR);	/* reset the unit */
-	udelay(100);
-	writel(readl(ICR) & ~ICR_IUE, ICR);	/* disable unit */
-#ifdef CONFIG_CPU_MONAHANS
-	/* | CKENB_1_PWM1 | CKENB_0_PWM0); */
-	writel(readl(CKENB) | (CKENB_4_I2C), CKENB);
-#else /* CONFIG_CPU_MONAHANS */
-	/* set the global I2C clock on */
-	writel(readl(CKEN) | CKEN14_I2C, CKEN);
-#endif
-	writel(I2C_PXA_SLAVE_ADDR, ISAR);	/* set our slave address */
-	writel(I2C_ICR_INIT, ICR);		/* set control reg values */
-	writel(I2C_ISR_INIT, ISR);		/* set clear interrupt bits */
-	writel(readl(ICR) | ICR_IUE, ICR);	/* enable unit */
-	udelay(100);
-}
-
-
-/**
- * i2c_isr_set_cleared: - wait until certain bits of the I2C status register
- *	                  are set and cleared
- *
- * @return: 1 in case of success, 0 means timeout (no match within 10 ms).
- */
-static int i2c_isr_set_cleared( unsigned long set_mask, unsigned long cleared_mask )
-{
-	int timeout = 10000;
-
-	while( ((ISR & set_mask)!=set_mask) || ((ISR & cleared_mask)!=0) ){
-		udelay( 10 );
-		if( timeout-- < 0 ) return 0;
-	}
-
-	return 1;
-}
-
-
-/**
- * i2c_transfer: - Transfer one byte over the i2c bus
- *
- * This function can tranfer a byte over the i2c bus in both directions.
- * It is used by the public API functions.
- *
- * @return:  0: transfer successful
- *          -1: message is empty
- *          -2: transmit timeout
- *          -3: ACK missing
- *          -4: receive timeout
- *          -5: illegal parameters
- *          -6: bus is busy and couldn't be aquired
- */
-int i2c_transfer(struct i2c_msg *msg)
-{
-	int ret;
-
-	if (!msg)
-		goto transfer_error_msg_empty;
-
-	switch(msg->direction) {
-
-	case I2C_WRITE:
-
-		/* check if bus is not busy */
-		if (!i2c_isr_set_cleared(0,ISR_IBB))
-			goto transfer_error_bus_busy;
-
-		/* start transmission */
-		writel(readl(ICR) & ~ICR_START, ICR);
-		writel(readl(ICR) & ~ICR_STOP, ICR);
-		writel(msg->data, IDBR);
-		if (msg->condition == I2C_COND_START)
-			writel(readl(ICR) | ICR_START, ICR);
-		if (msg->condition == I2C_COND_STOP)
-			writel(readl(ICR) | ICR_STOP, ICR);
-		if (msg->acknack == I2C_ACKNAK_SENDNAK)
-			writel(readl(ICR) | ICR_ACKNAK, ICR);
-		if (msg->acknack == I2C_ACKNAK_SENDACK)
-			writel(readl(ICR) & ~ICR_ACKNAK, ICR);
-		writel(readl(ICR) & ~ICR_ALDIE, ICR);
-		writel(readl(ICR) | ICR_TB, ICR);
-
-		/* transmit register empty? */
-		if (!i2c_isr_set_cleared(ISR_ITE,0))
-			goto transfer_error_transmit_timeout;
-
-		/* clear 'transmit empty' state */
-		writel(readl(ISR) | ISR_ITE, ISR);
-
-		/* wait for ACK from slave */
-		if (msg->acknack == I2C_ACKNAK_WAITACK)
-			if (!i2c_isr_set_cleared(0,ISR_ACKNAK))
-				goto transfer_error_ack_missing;
-		break;
-
-	case I2C_READ:
-
-		/* check if bus is not busy */
-		if (!i2c_isr_set_cleared(0,ISR_IBB))
-			goto transfer_error_bus_busy;
-
-		/* start receive */
-		writel(readl(ICR) & ~ICR_START, ICR);
-		writel(readl(ICR) & ~ICR_STOP, ICR);
-		if (msg->condition == I2C_COND_START)
-			writel(readl(ICR) | ICR_START, ICR);
-		if (msg->condition == I2C_COND_STOP)
-			writel(readl(ICR) | ICR_STOP, ICR);
-		if (msg->acknack == I2C_ACKNAK_SENDNAK)
-			writel(readl(ICR) | ICR_ACKNAK, ICR);
-		if (msg->acknack == I2C_ACKNAK_SENDACK)
-			writel(readl(ICR) & ~ICR_ACKNAK, ICR);
-		writel(readl(ICR) & ~ICR_ALDIE, ICR);
-		writel(readl(ICR) | ICR_TB, ICR);
-
-		/* receive register full? */
-		if (!i2c_isr_set_cleared(ISR_IRF,0))
-			goto transfer_error_receive_timeout;
-
-		msg->data = readl(IDBR);
-
-		/* clear 'receive empty' state */
-		writel(readl(ISR) | ISR_IRF, ISR);
-
-		break;
-
-	default:
-
-		goto transfer_error_illegal_param;
-
-	}
-
-	return 0;
-
-transfer_error_msg_empty:
-		PRINTD(("i2c_transfer: error: 'msg' is empty\n"));
-		ret = -1; goto i2c_transfer_finish;
-
-transfer_error_transmit_timeout:
-		PRINTD(("i2c_transfer: error: transmit timeout\n"));
-		ret = -2; goto i2c_transfer_finish;
-
-transfer_error_ack_missing:
-		PRINTD(("i2c_transfer: error: ACK missing\n"));
-		ret = -3; goto i2c_transfer_finish;
-
-transfer_error_receive_timeout:
-		PRINTD(("i2c_transfer: error: receive timeout\n"));
-		ret = -4; goto i2c_transfer_finish;
-
-transfer_error_illegal_param:
-		PRINTD(("i2c_transfer: error: illegal parameters\n"));
-		ret = -5; goto i2c_transfer_finish;
-
-transfer_error_bus_busy:
-		PRINTD(("i2c_transfer: error: bus is busy\n"));
-		ret = -6; goto i2c_transfer_finish;
-
-i2c_transfer_finish:
-		PRINTD(("i2c_transfer: ISR: 0x%04x\n",ISR));
-		i2c_reset();
-		return ret;
-
-}
-
-/* ------------------------------------------------------------------------ */
-/* API Functions                                                            */
-/* ------------------------------------------------------------------------ */
-
-void i2c_init(int speed, int slaveaddr)
-{
-#ifdef CONFIG_SYS_I2C_INIT_BOARD
-	/* call board specific i2c bus reset routine before accessing the   */
-	/* environment, which might be in a chip on that bus. For details   */
-	/* about this problem see doc/I2C_Edge_Conditions.                  */
-	i2c_init_board();
-#endif
-}
-
-
-/**
- * i2c_probe: - Test if a chip answers for a given i2c address
- *
- * @chip:	address of the chip which is searched for
- * @return:	0 if a chip was found, -1 otherwhise
- */
-
-int i2c_probe(uchar chip)
-{
-	struct i2c_msg msg;
-
-	i2c_reset();
-
-	msg.condition = I2C_COND_START;
-	msg.acknack   = I2C_ACKNAK_WAITACK;
-	msg.direction = I2C_WRITE;
-	msg.data      = (chip << 1) + 1;
-	if (i2c_transfer(&msg)) return -1;
-
-	msg.condition = I2C_COND_STOP;
-	msg.acknack   = I2C_ACKNAK_SENDNAK;
-	msg.direction = I2C_READ;
-	msg.data      = 0x00;
-	if (i2c_transfer(&msg)) return -1;
-
-	return 0;
-}
-
-
-/**
- * i2c_read: - Read multiple bytes from an i2c device
- *
- * The higher level routines take into account that this function is only
- * called with len < page length of the device (see configuration file)
- *
- * @chip:	address of the chip which is to be read
- * @addr:	i2c data address within the chip
- * @alen:	length of the i2c data address (1..2 bytes)
- * @buffer:	where to write the data
- * @len:	how much byte do we want to read
- * @return:	0 in case of success
- */
-
-int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
-{
-	struct i2c_msg msg;
-	u8 addr_bytes[3]; /* lowest...highest byte of data address */
-	int ret;
-
-	PRINTD(("i2c_read(chip=0x%02x, addr=0x%02x, alen=0x%02x, len=0x%02x)\n",chip,addr,alen,len));
-
-	i2c_reset();
-
-	/* dummy chip address write */
-	PRINTD(("i2c_read: dummy chip address write\n"));
-	msg.condition = I2C_COND_START;
-	msg.acknack   = I2C_ACKNAK_WAITACK;
-	msg.direction = I2C_WRITE;
-	msg.data      = (chip << 1);
-	msg.data     &= 0xFE;
-	if ((ret=i2c_transfer(&msg))) return -1;
-
-	/*
-	 * send memory address bytes;
-	 * alen defines how much bytes we have to send.
-	 */
-	/*addr &= ((1 << CONFIG_SYS_EEPROM_PAGE_WRITE_BITS)-1); */
-	addr_bytes[0] = (u8)((addr >>  0) & 0x000000FF);
-	addr_bytes[1] = (u8)((addr >>  8) & 0x000000FF);
-	addr_bytes[2] = (u8)((addr >> 16) & 0x000000FF);
-
-	while (--alen >= 0) {
-
-		PRINTD(("i2c_read: send memory word address byte %1d\n",alen));
-		msg.condition = I2C_COND_NORMAL;
-		msg.acknack   = I2C_ACKNAK_WAITACK;
-		msg.direction = I2C_WRITE;
-		msg.data      = addr_bytes[alen];
-		if ((ret=i2c_transfer(&msg))) return -1;
-	}
-
-
-	/* start read sequence */
-	PRINTD(("i2c_read: start read sequence\n"));
-	msg.condition = I2C_COND_START;
-	msg.acknack   = I2C_ACKNAK_WAITACK;
-	msg.direction = I2C_WRITE;
-	msg.data      = (chip << 1);
-	msg.data     |= 0x01;
-	if ((ret=i2c_transfer(&msg))) return -1;
-
-	/* read bytes; send NACK at last byte */
-	while (len--) {
-
-		if (len==0) {
-			msg.condition = I2C_COND_STOP;
-			msg.acknack   = I2C_ACKNAK_SENDNAK;
-		} else {
-			msg.condition = I2C_COND_NORMAL;
-			msg.acknack   = I2C_ACKNAK_SENDACK;
-		}
-
-		msg.direction = I2C_READ;
-		msg.data      = 0x00;
-		if ((ret=i2c_transfer(&msg))) return -1;
-
-		*buffer = msg.data;
-		PRINTD(("i2c_read: reading byte (0x%08x)=0x%02x\n",(unsigned int)buffer,*buffer));
-		buffer++;
-
-	}
-
-	i2c_reset();
-
-	return 0;
-}
-
-
-/**
- * i2c_write: -  Write multiple bytes to an i2c device
- *
- * The higher level routines take into account that this function is only
- * called with len < page length of the device (see configuration file)
- *
- * @chip:	address of the chip which is to be written
- * @addr:	i2c data address within the chip
- * @alen:	length of the i2c data address (1..2 bytes)
- * @buffer:	where to find the data to be written
- * @len:	how much byte do we want to read
- * @return:	0 in case of success
- */
-
-int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
-{
-	struct i2c_msg msg;
-	u8 addr_bytes[3]; /* lowest...highest byte of data address */
-
-	PRINTD(("i2c_write(chip=0x%02x, addr=0x%02x, alen=0x%02x, len=0x%02x)\n",chip,addr,alen,len));
-
-	i2c_reset();
-
-	/* chip address write */
-	PRINTD(("i2c_write: chip address write\n"));
-	msg.condition = I2C_COND_START;
-	msg.acknack   = I2C_ACKNAK_WAITACK;
-	msg.direction = I2C_WRITE;
-	msg.data      = (chip << 1);
-	msg.data     &= 0xFE;
-	if (i2c_transfer(&msg)) return -1;
-
-	/*
-	 * send memory address bytes;
-	 * alen defines how much bytes we have to send.
-	 */
-	addr_bytes[0] = (u8)((addr >>  0) & 0x000000FF);
-	addr_bytes[1] = (u8)((addr >>  8) & 0x000000FF);
-	addr_bytes[2] = (u8)((addr >> 16) & 0x000000FF);
-
-	while (--alen >= 0) {
-
-		PRINTD(("i2c_write: send memory word address\n"));
-		msg.condition = I2C_COND_NORMAL;
-		msg.acknack   = I2C_ACKNAK_WAITACK;
-		msg.direction = I2C_WRITE;
-		msg.data      = addr_bytes[alen];
-		if (i2c_transfer(&msg)) return -1;
-	}
-
-	/* write bytes; send NACK at last byte */
-	while (len--) {
-
-		PRINTD(("i2c_write: writing byte (0x%08x)=0x%02x\n",(unsigned int)buffer,*buffer));
-
-		if (len==0)
-			msg.condition = I2C_COND_STOP;
-		else
-			msg.condition = I2C_COND_NORMAL;
-
-		msg.acknack   = I2C_ACKNAK_WAITACK;
-		msg.direction = I2C_WRITE;
-		msg.data      = *(buffer++);
-
-		if (i2c_transfer(&msg)) return -1;
-
-	}
-
-	i2c_reset();
-
-	return 0;
-
-}
-
-#endif	/* CONFIG_HARD_I2C */
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index 052fe36..00a12cc 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -29,6 +29,7 @@  COBJS-$(CONFIG_BFIN_TWI_I2C) += bfin-twi_i2c.o
 COBJS-$(CONFIG_DRIVER_DAVINCI_I2C) += davinci_i2c.o
 COBJS-$(CONFIG_FSL_I2C) += fsl_i2c.o
 COBJS-$(CONFIG_I2C_MVTWSI) += mvtwsi.o
+COBJS-$(CONFIG_I2C_MV) += mv_i2c.o
 COBJS-$(CONFIG_I2C_MXC) += mxc_i2c.o
 COBJS-$(CONFIG_DRIVER_OMAP1510_I2C) += omap1510_i2c.o
 COBJS-$(CONFIG_DRIVER_OMAP24XX_I2C) += omap24xx_i2c.o
diff --git a/drivers/i2c/mv_i2c.c b/drivers/i2c/mv_i2c.c
new file mode 100644
index 0000000..09756a4
--- /dev/null
+++ b/drivers/i2c/mv_i2c.c
@@ -0,0 +1,452 @@ 
+/*
+ * (C) Copyright 2000
+ * Paolo Scaffardi, AIRVENT SAM s.p.a - RIMINI(ITALY), arsenio@tin.it
+ *
+ * (C) Copyright 2000 Sysgo Real-Time Solutions, GmbH <www.elinos.com>
+ * Marius Groeger <mgroeger@sysgo.de>
+ *
+ * (C) Copyright 2003 Pengutronix e.K.
+ * Robert Schwebel <r.schwebel@pengutronix.de>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ *
+ * Back ported to the 8xx platform (from the 8260 platform) by
+ * Murray.Jensen@cmst.csiro.au, 27-Jan-01.
+ */
+
+#include <common.h>
+#include <asm/io.h>
+
+#ifdef CONFIG_HARD_I2C
+
+/*
+ *	- CONFIG_SYS_I2C_SPEED
+ *	- I2C_PXA_SLAVE_ADDR
+ */
+
+#include <asm/arch/hardware.h>
+#include <asm/arch/pxa-regs.h>
+#include <i2c.h>
+
+#if (CONFIG_SYS_I2C_SPEED == 400000)
+#define I2C_ICR_INIT	(ICR_FM | ICR_BEIE | ICR_IRFIE | ICR_ITEIE | ICR_GCD \
+			| ICR_SCLE)
+#else
+#define I2C_ICR_INIT	(ICR_BEIE | ICR_IRFIE | ICR_ITEIE | ICR_GCD | ICR_SCLE)
+#endif
+
+#define I2C_ISR_INIT		0x7FF
+
+#ifdef DEBUG_I2C
+#define PRINTD(x) printf x
+#else
+#define PRINTD(x)
+#endif
+
+/* Shall the current transfer have a start/stop condition? */
+#define I2C_COND_NORMAL		0
+#define I2C_COND_START		1
+#define I2C_COND_STOP		2
+
+/* Shall the current transfer be ack/nacked or being waited for it? */
+#define I2C_ACKNAK_WAITACK	1
+#define I2C_ACKNAK_SENDACK	2
+#define I2C_ACKNAK_SENDNAK	4
+
+/* Specify who shall transfer the data (master or slave) */
+#define I2C_READ		0
+#define I2C_WRITE		1
+
+/* All transfers are described by this data structure */
+struct i2c_msg {
+	u8 condition;
+	u8 acknack;
+	u8 direction;
+	u8 data;
+};
+
+/*
+ * i2c_pxa_reset: - reset the host controller
+ *
+ */
+static void i2c_reset(void)
+{
+	writel(readl(ICR) & ~ICR_IUE, ICR);	/* disable unit */
+	writel(readl(ICR) | ICR_UR, ICR);	/* reset the unit */
+	udelay(100);
+	writel(readl(ICR) & ~ICR_IUE, ICR);	/* disable unit */
+#ifdef CONFIG_CPU_MONAHANS
+	/* | CKENB_1_PWM1 | CKENB_0_PWM0); */
+	writel(readl(CKENB) | (CKENB_4_I2C), CKENB);
+#else /* CONFIG_CPU_MONAHANS */
+	/* set the global I2C clock on */
+	writel(readl(CKEN) | CKEN14_I2C, CKEN);
+#endif
+	writel(I2C_PXA_SLAVE_ADDR, ISAR);	/* set our slave address */
+	writel(I2C_ICR_INIT, ICR);		/* set control reg values */
+	writel(I2C_ISR_INIT, ISR);		/* set clear interrupt bits */
+	writel(readl(ICR) | ICR_IUE, ICR);	/* enable unit */
+	udelay(100);
+}
+
+/*
+ * i2c_isr_set_cleared: - wait until certain bits of the I2C status register
+ *	                  are set and cleared
+ *
+ * @return: 1 in case of success, 0 means timeout (no match within 10 ms).
+ */
+static int i2c_isr_set_cleared(unsigned long set_mask,
+			       unsigned long cleared_mask)
+{
+	int timeout = 10000;
+
+	while (((ISR & set_mask) != set_mask) || ((ISR & cleared_mask) != 0)) {
+		udelay(10);
+		if (timeout-- < 0)
+			return 0;
+	}
+
+	return 1;
+}
+
+/*
+ * i2c_transfer: - Transfer one byte over the i2c bus
+ *
+ * This function can tranfer a byte over the i2c bus in both directions.
+ * It is used by the public API functions.
+ *
+ * @return:  0: transfer successful
+ *          -1: message is empty
+ *          -2: transmit timeout
+ *          -3: ACK missing
+ *          -4: receive timeout
+ *          -5: illegal parameters
+ *          -6: bus is busy and couldn't be aquired
+ */
+int i2c_transfer(struct i2c_msg *msg)
+{
+	int ret;
+
+	if (!msg)
+		goto transfer_error_msg_empty;
+
+	switch (msg->direction) {
+	case I2C_WRITE:
+		/* check if bus is not busy */
+		if (!i2c_isr_set_cleared(0, ISR_IBB))
+			goto transfer_error_bus_busy;
+
+		/* start transmission */
+		writel(readl(ICR) & ~ICR_START, ICR);
+		writel(readl(ICR) & ~ICR_STOP, ICR);
+		writel(msg->data, IDBR);
+		if (msg->condition == I2C_COND_START)
+			writel(readl(ICR) | ICR_START, ICR);
+		if (msg->condition == I2C_COND_STOP)
+			writel(readl(ICR) | ICR_STOP, ICR);
+		if (msg->acknack == I2C_ACKNAK_SENDNAK)
+			writel(readl(ICR) | ICR_ACKNAK, ICR);
+		if (msg->acknack == I2C_ACKNAK_SENDACK)
+			writel(readl(ICR) & ~ICR_ACKNAK, ICR);
+		writel(readl(ICR) & ~ICR_ALDIE, ICR);
+		writel(readl(ICR) | ICR_TB, ICR);
+
+		/* transmit register empty? */
+		if (!i2c_isr_set_cleared(ISR_ITE, 0))
+			goto transfer_error_transmit_timeout;
+
+		/* clear 'transmit empty' state */
+		writel(readl(ISR) | ISR_ITE, ISR);
+
+		/* wait for ACK from slave */
+		if (msg->acknack == I2C_ACKNAK_WAITACK)
+			if (!i2c_isr_set_cleared(0, ISR_ACKNAK))
+				goto transfer_error_ack_missing;
+		break;
+
+	case I2C_READ:
+
+		/* check if bus is not busy */
+		if (!i2c_isr_set_cleared(0, ISR_IBB))
+			goto transfer_error_bus_busy;
+
+		/* start receive */
+		writel(readl(ICR) & ~ICR_START, ICR);
+		writel(readl(ICR) & ~ICR_STOP, ICR);
+		if (msg->condition == I2C_COND_START)
+			writel(readl(ICR) | ICR_START, ICR);
+		if (msg->condition == I2C_COND_STOP)
+			writel(readl(ICR) | ICR_STOP, ICR);
+		if (msg->acknack == I2C_ACKNAK_SENDNAK)
+			writel(readl(ICR) | ICR_ACKNAK, ICR);
+		if (msg->acknack == I2C_ACKNAK_SENDACK)
+			writel(readl(ICR) & ~ICR_ACKNAK, ICR);
+		writel(readl(ICR) & ~ICR_ALDIE, ICR);
+		writel(readl(ICR) | ICR_TB, ICR);
+
+		/* receive register full? */
+		if (!i2c_isr_set_cleared(ISR_IRF, 0))
+			goto transfer_error_receive_timeout;
+
+		msg->data = readl(IDBR);
+
+		/* clear 'receive empty' state */
+		writel(readl(ISR) | ISR_IRF, ISR);
+
+		break;
+	default:
+		goto transfer_error_illegal_param;
+	}
+
+	return 0;
+
+transfer_error_msg_empty:
+		PRINTD(("i2c_transfer: error: 'msg' is empty\n"));
+		ret = -1; goto i2c_transfer_finish;
+
+transfer_error_transmit_timeout:
+		PRINTD(("i2c_transfer: error: transmit timeout\n"));
+		ret = -2; goto i2c_transfer_finish;
+
+transfer_error_ack_missing:
+		PRINTD(("i2c_transfer: error: ACK missing\n"));
+		ret = -3; goto i2c_transfer_finish;
+
+transfer_error_receive_timeout:
+		PRINTD(("i2c_transfer: error: receive timeout\n"));
+		ret = -4; goto i2c_transfer_finish;
+
+transfer_error_illegal_param:
+		PRINTD(("i2c_transfer: error: illegal parameters\n"));
+		ret = -5; goto i2c_transfer_finish;
+
+transfer_error_bus_busy:
+		PRINTD(("i2c_transfer: error: bus is busy\n"));
+		ret = -6; goto i2c_transfer_finish;
+
+i2c_transfer_finish:
+		PRINTD(("i2c_transfer: ISR: 0x%04x\n", ISR));
+		i2c_reset();
+		return ret;
+}
+
+/* ------------------------------------------------------------------------ */
+/* API Functions                                                            */
+/* ------------------------------------------------------------------------ */
+void i2c_init(int speed, int slaveaddr)
+{
+#ifdef CONFIG_SYS_I2C_INIT_BOARD
+	/* call board specific i2c bus reset routine before accessing the   */
+	/* environment, which might be in a chip on that bus. For details   */
+	/* about this problem see doc/I2C_Edge_Conditions.                  */
+	i2c_init_board();
+#endif
+}
+
+/*
+ * i2c_probe: - Test if a chip answers for a given i2c address
+ *
+ * @chip:	address of the chip which is searched for
+ * @return:	0 if a chip was found, -1 otherwhise
+ */
+int i2c_probe(uchar chip)
+{
+	struct i2c_msg msg;
+
+	i2c_reset();
+
+	msg.condition = I2C_COND_START;
+	msg.acknack   = I2C_ACKNAK_WAITACK;
+	msg.direction = I2C_WRITE;
+	msg.data      = (chip << 1) + 1;
+	if (i2c_transfer(&msg))
+		return -1;
+
+	msg.condition = I2C_COND_STOP;
+	msg.acknack   = I2C_ACKNAK_SENDNAK;
+	msg.direction = I2C_READ;
+	msg.data      = 0x00;
+	if (i2c_transfer(&msg))
+		return -1;
+
+	return 0;
+}
+
+/*
+ * i2c_read: - Read multiple bytes from an i2c device
+ *
+ * The higher level routines take into account that this function is only
+ * called with len < page length of the device (see configuration file)
+ *
+ * @chip:	address of the chip which is to be read
+ * @addr:	i2c data address within the chip
+ * @alen:	length of the i2c data address (1..2 bytes)
+ * @buffer:	where to write the data
+ * @len:	how much byte do we want to read
+ * @return:	0 in case of success
+ */
+int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
+{
+	struct i2c_msg msg;
+	u8 addr_bytes[3]; /* lowest...highest byte of data address */
+
+	PRINTD(("i2c_read(chip=0x%02x, addr=0x%02x, alen=0x%02x, "
+		"len=0x%02x)\n", chip, addr, alen, len));
+
+	i2c_reset();
+
+	/* dummy chip address write */
+	PRINTD(("i2c_read: dummy chip address write\n"));
+	msg.condition = I2C_COND_START;
+	msg.acknack   = I2C_ACKNAK_WAITACK;
+	msg.direction = I2C_WRITE;
+	msg.data = (chip << 1);
+	msg.data &= 0xFE;
+	if (i2c_transfer(&msg))
+		return -1;
+
+	/*
+	 * send memory address bytes;
+	 * alen defines how much bytes we have to send.
+	 */
+	/*addr &= ((1 << CONFIG_SYS_EEPROM_PAGE_WRITE_BITS)-1); */
+	addr_bytes[0] = (u8)((addr >>  0) & 0x000000FF);
+	addr_bytes[1] = (u8)((addr >>  8) & 0x000000FF);
+	addr_bytes[2] = (u8)((addr >> 16) & 0x000000FF);
+
+	while (--alen >= 0) {
+		PRINTD(("i2c_read: send memory word address byte %1d\n", alen));
+		msg.condition = I2C_COND_NORMAL;
+		msg.acknack   = I2C_ACKNAK_WAITACK;
+		msg.direction = I2C_WRITE;
+		msg.data      = addr_bytes[alen];
+		if (i2c_transfer(&msg))
+			return -1;
+	}
+
+	/* start read sequence */
+	PRINTD(("i2c_read: start read sequence\n"));
+	msg.condition = I2C_COND_START;
+	msg.acknack   = I2C_ACKNAK_WAITACK;
+	msg.direction = I2C_WRITE;
+	msg.data      = (chip << 1);
+	msg.data     |= 0x01;
+	if (i2c_transfer(&msg))
+		return -1;
+
+	/* read bytes; send NACK at last byte */
+	while (len--) {
+		if (len == 0) {
+			msg.condition = I2C_COND_STOP;
+			msg.acknack   = I2C_ACKNAK_SENDNAK;
+		} else {
+			msg.condition = I2C_COND_NORMAL;
+			msg.acknack   = I2C_ACKNAK_SENDACK;
+		}
+
+		msg.direction = I2C_READ;
+		msg.data      = 0x00;
+		if (i2c_transfer(&msg))
+			return -1;
+
+		*buffer = msg.data;
+		PRINTD(("i2c_read: reading byte (0x%08x)=0x%02x\n",
+			(unsigned int)buffer, *buffer));
+		buffer++;
+	}
+
+	i2c_reset();
+
+	return 0;
+}
+
+/*
+ * i2c_write: -  Write multiple bytes to an i2c device
+ *
+ * The higher level routines take into account that this function is only
+ * called with len < page length of the device (see configuration file)
+ *
+ * @chip:	address of the chip which is to be written
+ * @addr:	i2c data address within the chip
+ * @alen:	length of the i2c data address (1..2 bytes)
+ * @buffer:	where to find the data to be written
+ * @len:	how much byte do we want to read
+ * @return:	0 in case of success
+ */
+int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
+{
+	struct i2c_msg msg;
+	u8 addr_bytes[3]; /* lowest...highest byte of data address */
+
+	PRINTD(("i2c_write(chip=0x%02x, addr=0x%02x, alen=0x%02x, "
+		"len=0x%02x)\n", chip, addr, alen, len));
+
+	i2c_reset();
+
+	/* chip address write */
+	PRINTD(("i2c_write: chip address write\n"));
+	msg.condition = I2C_COND_START;
+	msg.acknack   = I2C_ACKNAK_WAITACK;
+	msg.direction = I2C_WRITE;
+	msg.data = (chip << 1);
+	msg.data &= 0xFE;
+	if (i2c_transfer(&msg))
+		return -1;
+
+	/*
+	 * send memory address bytes;
+	 * alen defines how much bytes we have to send.
+	 */
+	addr_bytes[0] = (u8)((addr >>  0) & 0x000000FF);
+	addr_bytes[1] = (u8)((addr >>  8) & 0x000000FF);
+	addr_bytes[2] = (u8)((addr >> 16) & 0x000000FF);
+
+	while (--alen >= 0) {
+		PRINTD(("i2c_write: send memory word address\n"));
+		msg.condition = I2C_COND_NORMAL;
+		msg.acknack   = I2C_ACKNAK_WAITACK;
+		msg.direction = I2C_WRITE;
+		msg.data      = addr_bytes[alen];
+		if (i2c_transfer(&msg))
+			return -1;
+	}
+
+	/* write bytes; send NACK at last byte */
+	while (len--) {
+		PRINTD(("i2c_write: writing byte (0x%08x)=0x%02x\n",
+			(unsigned int)buffer, *buffer));
+
+		if (len == 0)
+			msg.condition = I2C_COND_STOP;
+		else
+			msg.condition = I2C_COND_NORMAL;
+
+		msg.acknack   = I2C_ACKNAK_WAITACK;
+		msg.direction = I2C_WRITE;
+		msg.data      = *(buffer++);
+
+		if (i2c_transfer(&msg))
+			return -1;
+	}
+
+	i2c_reset();
+
+	return 0;
+}
+#endif	/* CONFIG_HARD_I2C */
diff --git a/include/configs/innokom.h b/include/configs/innokom.h
index d8fcbdb..0ea73c9 100644
--- a/include/configs/innokom.h
+++ b/include/configs/innokom.h
@@ -140,6 +140,7 @@ 
 /*
  * I2C bus
  */
+#define CONFIG_I2C_MV			1
 #define CONFIG_HARD_I2C			1
 #define CONFIG_SYS_I2C_SPEED			50000
 #define CONFIG_SYS_I2C_SLAVE			0xfe
diff --git a/include/configs/xm250.h b/include/configs/xm250.h
index 497cb91..b4b940a 100644
--- a/include/configs/xm250.h
+++ b/include/configs/xm250.h
@@ -61,6 +61,7 @@ 
 /*
  * I2C bus
  */
+#define CONFIG_I2C_MV			1
 #define CONFIG_HARD_I2C			1
 #define CONFIG_SYS_I2C_SPEED			50000
 #define CONFIG_SYS_I2C_SLAVE			0xfe