* [U-Boot] Moving i2c_board_init to after i2c_init operations @ 2010-04-09 17:22 Richard Retanubun 2010-04-12 6:11 ` Heiko Schocher 0 siblings, 1 reply; 11+ messages in thread From: Richard Retanubun @ 2010-04-09 17:22 UTC (permalink / raw) To: u-boot Hi Heiko, I am trying to do something similar to what you did in this commit: http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=commit;h=39df00d9aecfb465b9eec9af593f9b763fb5209a and I have a question, must i2c_board_init for fsl_i2c.c be called before the actual controller initialization? (i.e. setting the bus speed and the controller's slave address) if we call it at the end of i2c_init, we can take advantage of the setups done before, no? Most of the other implementation does it in the beginning because they can change the i2c pins to GPIO and 'bit-bang' the reset pattern, and then change it back to i2c pins. The alternative is of course to assume nothing in i2c_board_init, and configure everything (I kinda liked to use set_i2c_bus_speed, rather than hardcoding something). Since the kmeter1 board is the only user of i2c_board_init in fsl_i2c.c, would you be opposed if I submit a patch that moves the call for i2c_board_init to the end of i2c_init? Or, can I make another 'callpoint' (e.g. i2c_reset_bus) and call that at the end? Thanks for your time. - Richard Retanubun ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] Moving i2c_board_init to after i2c_init operations 2010-04-09 17:22 [U-Boot] Moving i2c_board_init to after i2c_init operations Richard Retanubun @ 2010-04-12 6:11 ` Heiko Schocher 2010-04-12 18:02 ` [U-Boot] [PATCH]fsl_i2c: Move " richardretanubun at ruggedcom.com 0 siblings, 1 reply; 11+ messages in thread From: Heiko Schocher @ 2010-04-12 6:11 UTC (permalink / raw) To: u-boot Hello Richard, Richard Retanubun wrote: > I am trying to do something similar to what you did in this commit: > > http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=commit;h=39df00d9aecfb465b9eec9af593f9b763fb5209a > > > and I have a question, must i2c_board_init for fsl_i2c.c be called before > the actual controller initialization? (i.e. setting the bus speed and > the controller's slave address) > > if we call it at the end of i2c_init, we can take advantage of the > setups done before, no? Yes, you are right. > Most of the other implementation does it in the beginning because they > can change the > i2c pins to GPIO and 'bit-bang' the reset pattern, and then change it > back to i2c pins. Yep. > The alternative is of course to assume nothing in i2c_board_init, and > configure everything > (I kinda liked to use set_i2c_bus_speed, rather than hardcoding something). > > Since the kmeter1 board is the only user of i2c_board_init in fsl_i2c.c, > would you be > opposed if I submit a patch that moves the call for i2c_board_init to > the end of i2c_init? No, this is OK, and looks cleaner to me. Thanks! bye Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH]fsl_i2c: Move i2c_board_init to after i2c_init operations 2010-04-12 6:11 ` Heiko Schocher @ 2010-04-12 18:02 ` richardretanubun at ruggedcom.com 2010-04-12 18:17 ` Wolfgang Denk 0 siblings, 1 reply; 11+ messages in thread From: richardretanubun at ruggedcom.com @ 2010-04-12 18:02 UTC (permalink / raw) To: u-boot ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH]fsl_i2c: Move i2c_board_init to after i2c_init operations 2010-04-12 18:02 ` [U-Boot] [PATCH]fsl_i2c: Move " richardretanubun at ruggedcom.com @ 2010-04-12 18:17 ` Wolfgang Denk 2010-04-12 18:22 ` Richard Retanubun 0 siblings, 1 reply; 11+ messages in thread From: Wolfgang Denk @ 2010-04-12 18:17 UTC (permalink / raw) To: u-boot Dear richardretanubun at ruggedcom.com, In message <20100412180215.GA11973@richardretanubun.eng.lan> you wrote: > From 00f84e4a9a2d13971c9328fc815825456b25f760 Mon Sep 17 00:00:00 2001 > From: Richard Retanubun <richardretanubun@ruggedcom.com> > Date: Mon, 12 Apr 2010 13:32:09 -0400 > Subject: [PATCH] fsl_i2c: Move the call for i2c_init_board to the end of i2c_init > > This patch moved the call to i2c_init_board to the end of i2c_init. > This allows the board fixup functions to take advantage of the > setups done by i2c_init (i.e. bus speed and slave address). > > On other boards, i2c_init_board is called before i2c_init operation > because the method of resetting i2c bus typically uses GPIOs > to bit-bang SCLK. For i2c controllers that is using fsl_i2c this is > unneccessary because there is a i2c register access sequence that > accomplish the same thing, and it is better to do the accesses > after the bus speed and slave address have been configured. I dislike such inconsitent behaviour. It would be better if we could rely on a specific initialization sequence, i. e. the function always be called at the beginning or always at the end. Having to deal with a situation where one or the other might happen, even on the same hardware, depending on if we use the HW or the SW I2C driver, sounds like a nightmare to me. If you need a late init function, then better add a new one. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "Have you lived in this village all your life?" "No, not yet." ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH]fsl_i2c: Move i2c_board_init to after i2c_init operations 2010-04-12 18:17 ` Wolfgang Denk @ 2010-04-12 18:22 ` Richard Retanubun 2010-04-12 18:38 ` Wolfgang Denk 0 siblings, 1 reply; 11+ messages in thread From: Richard Retanubun @ 2010-04-12 18:22 UTC (permalink / raw) To: u-boot Wolfgang Denk wrote: > > If you need a late init function, then better add a new one. Hi Wolfgang, That is one of the options I put forth, what would be a name you like for this callpoint? i2c_board_late_init ?? - Richard ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH]fsl_i2c: Move i2c_board_init to after i2c_init operations 2010-04-12 18:22 ` Richard Retanubun @ 2010-04-12 18:38 ` Wolfgang Denk 2010-04-12 19:17 ` [U-Boot] [PATCH]fsl_i2c: Add i2c_board_late_init richardretanubun at ruggedcom.com 0 siblings, 1 reply; 11+ messages in thread From: Wolfgang Denk @ 2010-04-12 18:38 UTC (permalink / raw) To: u-boot Dear Richard, In message <4BC364D1.8020306@RuggedCom.com> you wrote: > > That is one of the options I put forth, what would be a name you like for this callpoint? > > i2c_board_late_init ?? I think we agree that it's not exactly an elegant name, but given the fact that we already have board_late_init() it would be at least consistent. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Never underestimate the power of human stupidity when it comes to using technology they don't understand. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH]fsl_i2c: Add i2c_board_late_init 2010-04-12 18:38 ` Wolfgang Denk @ 2010-04-12 19:17 ` richardretanubun at ruggedcom.com 2010-04-12 19:28 ` Wolfgang Denk 2010-04-12 19:32 ` [U-Boot] [PATCH v2]fsl_i2c: " richardretanubun at ruggedcom.com 0 siblings, 2 replies; 11+ messages in thread From: richardretanubun at ruggedcom.com @ 2010-04-12 19:17 UTC (permalink / raw) To: u-boot ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH]fsl_i2c: Add i2c_board_late_init 2010-04-12 19:17 ` [U-Boot] [PATCH]fsl_i2c: Add i2c_board_late_init richardretanubun at ruggedcom.com @ 2010-04-12 19:28 ` Wolfgang Denk 2010-04-14 7:02 ` Heiko Schocher 2010-04-12 19:32 ` [U-Boot] [PATCH v2]fsl_i2c: " richardretanubun at ruggedcom.com 1 sibling, 1 reply; 11+ messages in thread From: Wolfgang Denk @ 2010-04-12 19:28 UTC (permalink / raw) To: u-boot Dear richardretanubun at ruggedcom.com, In message <20100412191703.GA30835@richardretanubun.eng.lan> you wrote: > From d0d9e0df99ce9035db43ebcf9d48601fa6f096d4 Mon Sep 17 00:00:00 2001 > From: Richard Retanubun <RichardRetanubun@RuggedCom.com> > Date: Mon, 12 Apr 2010 15:08:17 -0400 > Subject: [PATCH] fsl_i2c: Added a callpoint for i2c_board_late_init > > This patch adds a callpoint in i2c_init that allows board specific > i2c board initialization (typically for i2c bus reset) that is called > after i2c_init operations, allowing the i2c_board_late_init function > to use the pre-configured i2c bus speed and slave address. > --- > > Hi Wolfgang & Heiko, > > This is the patch that adds another callpoint. Thanks for > all the feedback. > > - Richard > > drivers/i2c/fsl_i2c.c | 8 ++++++++ > 1 files changed, 8 insertions(+), 0 deletions(-) > > diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c > index 2241990..a1a62fa 100644 > --- a/drivers/i2c/fsl_i2c.c > +++ b/drivers/i2c/fsl_i2c.c > @@ -249,6 +249,14 @@ i2c_init(int speed, int slaveadd) > writeb(0x0, &dev->sr); /* clear status register */ > writeb(I2C_CR_MEN, &dev->cr); /* start I2C controller */ > #endif > + > +#ifdef CONFIG_SYS_I2C_BOARD_LATE_INIT > + /* Call board specific i2c bus reset routine after the bus has been */ > + /* initialized. Use either this callpoint or i2c_init_board; which is */ > + /* called before i2c_init operations. */ > + /* For details about this problem see doc/I2C_Edge_Conditions. */ > + i2c_board_late_init(); > +#endif Incorrect multiline comment style. Instead of adding more #ifdef'ery we now tend to use weak symbols - but I'm not sure about this here in this context, though. Heiko, what do you think about this? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de There's an old proverb that says just about whatever you want it to. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH]fsl_i2c: Add i2c_board_late_init 2010-04-12 19:28 ` Wolfgang Denk @ 2010-04-14 7:02 ` Heiko Schocher 2010-04-14 15:48 ` [U-Boot] [PATCH v3]fsl_i2c: " richardretanubun at ruggedcom.com 0 siblings, 1 reply; 11+ messages in thread From: Heiko Schocher @ 2010-04-14 7:02 UTC (permalink / raw) To: u-boot Hello Wolfgang, Richard, Wolfgang Denk wrote: > Dear richardretanubun at ruggedcom.com, > > In message <20100412191703.GA30835@richardretanubun.eng.lan> you wrote: >> From d0d9e0df99ce9035db43ebcf9d48601fa6f096d4 Mon Sep 17 00:00:00 2001 >> From: Richard Retanubun <RichardRetanubun@RuggedCom.com> >> Date: Mon, 12 Apr 2010 15:08:17 -0400 >> Subject: [PATCH] fsl_i2c: Added a callpoint for i2c_board_late_init >> >> This patch adds a callpoint in i2c_init that allows board specific >> i2c board initialization (typically for i2c bus reset) that is called >> after i2c_init operations, allowing the i2c_board_late_init function >> to use the pre-configured i2c bus speed and slave address. >> --- >> >> Hi Wolfgang & Heiko, >> >> This is the patch that adds another callpoint. Thanks for >> all the feedback. >> >> - Richard >> >> drivers/i2c/fsl_i2c.c | 8 ++++++++ >> 1 files changed, 8 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c >> index 2241990..a1a62fa 100644 >> --- a/drivers/i2c/fsl_i2c.c >> +++ b/drivers/i2c/fsl_i2c.c >> @@ -249,6 +249,14 @@ i2c_init(int speed, int slaveadd) >> writeb(0x0, &dev->sr); /* clear status register */ >> writeb(I2C_CR_MEN, &dev->cr); /* start I2C controller */ >> #endif >> + >> +#ifdef CONFIG_SYS_I2C_BOARD_LATE_INIT >> + /* Call board specific i2c bus reset routine after the bus has been */ >> + /* initialized. Use either this callpoint or i2c_init_board; which is */ >> + /* called before i2c_init operations. */ >> + /* For details about this problem see doc/I2C_Edge_Conditions. */ >> + i2c_board_late_init(); >> +#endif > > Incorrect multiline comment style. Yep, Richard please fix this. Also it would be nice to have an entry in the README for your new callpoint. > Instead of adding more #ifdef'ery we now tend to use weak symbols - > but I'm not sure about this here in this context, though. Heiko, what > do you think about this? I would prefer here the define version, but common uboot style is, as you said, using weak symbols ... If using here weak symbols, I vote for changing the "#ifdef CONFIG_SYS_I2C_INIT_BOARD" case also to weak. Thanks. bye Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v3]fsl_i2c: Add i2c_board_late_init 2010-04-14 7:02 ` Heiko Schocher @ 2010-04-14 15:48 ` richardretanubun at ruggedcom.com 0 siblings, 0 replies; 11+ messages in thread From: richardretanubun at ruggedcom.com @ 2010-04-14 15:48 UTC (permalink / raw) To: u-boot ^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v2]fsl_i2c: Add i2c_board_late_init 2010-04-12 19:17 ` [U-Boot] [PATCH]fsl_i2c: Add i2c_board_late_init richardretanubun at ruggedcom.com 2010-04-12 19:28 ` Wolfgang Denk @ 2010-04-12 19:32 ` richardretanubun at ruggedcom.com 1 sibling, 0 replies; 11+ messages in thread From: richardretanubun at ruggedcom.com @ 2010-04-12 19:32 UTC (permalink / raw) To: u-boot ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-04-14 15:48 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-04-09 17:22 [U-Boot] Moving i2c_board_init to after i2c_init operations Richard Retanubun 2010-04-12 6:11 ` Heiko Schocher 2010-04-12 18:02 ` [U-Boot] [PATCH]fsl_i2c: Move " richardretanubun at ruggedcom.com 2010-04-12 18:17 ` Wolfgang Denk 2010-04-12 18:22 ` Richard Retanubun 2010-04-12 18:38 ` Wolfgang Denk 2010-04-12 19:17 ` [U-Boot] [PATCH]fsl_i2c: Add i2c_board_late_init richardretanubun at ruggedcom.com 2010-04-12 19:28 ` Wolfgang Denk 2010-04-14 7:02 ` Heiko Schocher 2010-04-14 15:48 ` [U-Boot] [PATCH v3]fsl_i2c: " richardretanubun at ruggedcom.com 2010-04-12 19:32 ` [U-Boot] [PATCH v2]fsl_i2c: " richardretanubun at ruggedcom.com
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.