[U-Boot] [PATCH 2/2] Powerpc/i2c: Force i2c to become bus master out of reset

[Huang Changming-R66093]

Thanks and Best Regards
Jerry Huang


> -----Original Message-----
> From: Heiko Schocher [mailto:hs at denx.de]
> Sent: Friday, October 28, 2011 2:34 PM
> To: Huang Changming-R66093
> Cc: Joakim Tjernlund; u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH 2/2] Powerpc/i2c: Force i2c to become bus
> master out of reset
> 
> Hello Huang Changming-R66093,
> 
> Huang Changming-R66093 wrote:
> >>> These tow part codes are doing the different thing due to the
> >> different reason:
> >>> 1. Kernel's code:
> >>> because Sometimes 9th clock pulse isn't generated, that code is to
> >> generate the 9th clock pulse.
> >>
> >> What code are you looking at? Are you just reading the comment?
> >> Look at http://git.kernel.org/?p=linux/kernel/git/next/linux-
> >> next.git;a=commit;h=0c2daaafcdec726e89cbccca61d576de8429c537
> >> to get a clue.
> >>
> >> Your code isn't complete, you cannot trust the manual to be the
> whole
> >> truth.
> >>
> > I am very confused, why do you always say my code isn't complete?
> 
> Because it is not complete?
I don't think it is not complete, because this patch resolves my issue.


> > I have described I want to do detailed in comment.
> > "when a system reset does not cause all I2C devices to be reset",
> 
> You wrote:
> "So, the kernel's patch and my patch is to resolve the different
> issue."
> 
> Yes! There are more "problems" on i2c bus, as also Joakim described!
> And if we try to fix them in the common driver (as you did), we should
> fix all known problems -> so your patch is incomplete.

Yes, we should fix all known problems, but we should not hope one patch includes everything.
it make sense for one patch to resolve one thing, instead of all.

> > My codes will force the I2c module to become bus master and drive SCL,
> > which force this i2c module to generate SCL so that the device
> driving SDA can finish its transaction.
> > These codes have been used on Emerson's P1022 board and resolved his
> > issue (board will hang when u-boot booting, with my codes, this issue
> > is resolved and board can boot well)
> 
> Then you maybe define CONFIG_SYS_I2C_BOARD_LATE_INIT for this board and
> do this in board code.

I have pointed out, this is one feature of FSL I2C module and almost cover all 85xx platform,
we provides one solution to resolve the i2c bus "reset" issue.
So I think it is not necessary to define one macro for one special board. 

> > This is one feature of FSL I2C almost cover all 85xx platform, and
> the code according to the RM has been verified, don't you think the
> manual can be trust?
> 
> See (for example on a mpc83xx based board)
> board/keymile/common/common.c functions i2c_write_start_seq() and
> i2c_make_abort() which solve also a deblocked i2c bus ... and they
> started with the procedure noted in the RM, but this didn't solved all
> issues for them.
> 
> > Below is the description from your link:
> > This patch improves the recovery of the MPC's I2C bus from errors
> like
> > bus hangs resulting in timeouts:
> > 1. make the bus timeout configurable, as it depends on the bus clock
> and
> >     the attached slave chip(s); default is still 1 second; 2. detect
> > any of the cases indicated by the CF, BB and RXAK MSR flags if a
> >     timeout occurs, and add a missing (required) MAL reset; 3. use a
> > more reliable method to fixup the bus if a hang has been detected.
> >     The sequence is sent 9 times which seems to be necessary if a
> slave
> >     "misses" more than one clock cycle.  For 400 kHz bus speed, the
> fixup is
> >     also ~70us (81us vs. 150us) faster.
> >
> > This patch is created because a slave miss more than one clock cycle
> and can resolve this issue.
> >
> > So, the kernel's patch and my patch is to resolve the different issue.
> 
> Yes, and if editing a common driver, we should find a solution which
> fit for all ... did you tried the linux driver code for your board?
> Maybe it solves your problem too? So we can adapt it to u-boot code?
> 
I can't try the Linux driver on my boards, because all boards on my hand has not these two i2c issue.
These codes is generated according to FSL Reference Manual, if you check the RM of FSL, you will find out all 85xx chips(now I know) support this feature.
And these code can be work on Emerson's p1022 board, if the code according to the RM can work, I don't  think I have any reason to ask customer to use the other codes.
And the solution RM provide can resolve the issue, why don't we trust the RM?