Hello Prafulla, Prafulla Wadaskar wrote: >> -----Original Message----- >> From: Lei Wen [mailto:adrian.w...@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 -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
