[U-Boot] [PATCH 1/2][for v2008.10] Revert "85xx: Using proper I2C source clock divider for MPC8544"
This reverts commit dffd2446fb041f38ef034b0fcf41e51e5e489159.
The fix introduced by this patch is not correct. The problem is that the documentation is not correct for the MPC8544 with regards to which bit in PORDEVSR2 is for the SEC_CFG.
Signed-off-by: Kumar Gala galak@kernel.crashing.org --- cpu/mpc85xx/speed.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/cpu/mpc85xx/speed.c b/cpu/mpc85xx/speed.c index 70dfad0..485ba20 100644 --- a/cpu/mpc85xx/speed.c +++ b/cpu/mpc85xx/speed.c @@ -102,9 +102,9 @@ int get_clocks (void) * PORDEVSR2_SEC_CFG bit is 0 on all 85xx boards that are not an 8544. */ if (gur->pordevsr2 & MPC85xx_PORDEVSR2_SEC_CFG) - gd->i2c1_clk = sys_info.freqSystemBus / 2; - else gd->i2c1_clk = sys_info.freqSystemBus / 3; + else + gd->i2c1_clk = sys_info.freqSystemBus / 2; #else /* Most 85xx SOCs use CCB/2, so this is the default behavior. */ gd->i2c1_clk = sys_info.freqSystemBus / 2;
The MPC8544 RM incorrect shows the SEC_CFG bit in PORDEVSR2 as being bit 26, instead it should be bit 28. This caused in incorrect interpretation of the i2c_clk which is the same as the SEC clk on MPC8544. The SEC clk is controlled by cfg_sec_freq that is reported in PORDEVSR2.
Signed-off-by: Kumar Gala galak@kernel.crashing.org --- include/asm-ppc/immap_85xx.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/asm-ppc/immap_85xx.h b/include/asm-ppc/immap_85xx.h index 50c9dde..6c81c39 100644 --- a/include/asm-ppc/immap_85xx.h +++ b/include/asm-ppc/immap_85xx.h @@ -1579,7 +1579,7 @@ typedef struct ccsr_gur { #define MPC85xx_PORDEVSR_RIO_DEV_ID 0x00000007 uint pordbgmsr; /* 0xe0010 - POR debug mode status register */ uint pordevsr2; /* 0xe0014 - POR I/O device status regsiter 2 */ -#define MPC85xx_PORDEVSR2_SEC_CFG 0x00000020 +#define MPC85xx_PORDEVSR2_SEC_CFG 0x00000080 char res1[8]; uint gpporcr; /* 0xe0020 - General-purpose POR configuration register */ char res2[12];
Dear Kumar Gala,
In message 1224212330-14491-2-git-send-email-galak@kernel.crashing.org you wrote:
The MPC8544 RM incorrect shows the SEC_CFG bit in PORDEVSR2 as being bit 26, instead it should be bit 28. This caused in incorrect interpretation of the i2c_clk which is the same as the SEC clk on MPC8544. The SEC clk is controlled by cfg_sec_freq that is reported in PORDEVSR2.
Signed-off-by: Kumar Gala galak@kernel.crashing.org
include/asm-ppc/immap_85xx.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
Hi Kumar,
Kumar Gala wrote:
The MPC8544 RM incorrect shows the SEC_CFG bit in PORDEVSR2 as being bit 26, instead it should be bit 28. This caused in incorrect interpretation of the i2c_clk which is the same as the SEC clk on MPC8544. The SEC clk is controlled by cfg_sec_freq that is reported in PORDEVSR2.
Signed-off-by: Kumar Gala galak@kernel.crashing.org
That makes sense now. I just got the confirmation from the customer, that LWE_B[0] is indeed pulled down via FPGA:
LWE_B[0] Pulldown SEC Frequency Ratio 2:1
Thanks for your investigations.
Wolfgang.
On Thu, Oct 16, 2008 at 9:58 PM, Kumar Gala galak@kernel.crashing.org wrote:
-#define MPC85xx_PORDEVSR2_SEC_CFG 0x00000020 +#define MPC85xx_PORDEVSR2_SEC_CFG 0x00000080
How about adding a comment that the RM is wrong? Any time the code disagrees with the RM, it *has* to be documented.
On Oct 17, 2008, at 1:54 PM, Timur Tabi wrote:
On Thu, Oct 16, 2008 at 9:58 PM, Kumar Gala galak@kernel.crashing.org wrote:
-#define MPC85xx_PORDEVSR2_SEC_CFG 0x00000020 +#define MPC85xx_PORDEVSR2_SEC_CFG 0x00000080
How about adding a comment that the RM is wrong? Any time the code disagrees with the RM, it *has* to be documented.
Its documented in the commit.
- k
Kumar Gala wrote:
How about adding a comment that the RM is wrong? Any time the code disagrees with the RM, it *has* to be documented.
Its documented in the commit.
Sorry, but that's just not good enough for me. I *hate* it when people say that it is.
When I'm looking at some code, and I don't understand what it does, the last thing I'm doing to do is scour through the git log trying to find some explanation.
Would it really have been so hard to add this line?
/* The MPC8544 RM says it's bit 26, but it's really bit 28 */
Why don't we take your reasoning to the next level, and just say that code comments are unnecessary because the git log contains everything we need to know?
Timur Tabi wrote:
Kumar Gala wrote:
How about adding a comment that the RM is wrong? Any time the code disagrees with the RM, it *has* to be documented.
Its documented in the commit.
Sorry, but that's just not good enough for me. I *hate* it when people say that it is.
When I'm looking at some code, and I don't understand what it does, the last thing I'm doing to do is scour through the git log trying to find some explanation.
Would it really have been so hard to add this line?
/* The MPC8544 RM says it's bit 26, but it's really bit 28 */
I have to agree with Timur, we *need* a comment in the source code for future reference. The git log is good, but not sufficient.
Thanks, gvb
Dear Jerry Van Baren,
In message 48FC815A.90501@ge.com you wrote:
Would it really have been so hard to add this line?
/* The MPC8544 RM says it's bit 26, but it's really bit 28 */
I have to agree with Timur, we *need* a comment in the source code for future reference. The git log is good, but not sufficient.
Agreed, but in the first place we should figure out if it's *really* bit 28 (then the new code is wrong), or bit 24, then the doc is wrong again.
Best regards,
Wolfgang Denk
On Oct 20, 2008, at 2:22 PM, Wolfgang Denk wrote:
Dear Jerry Van Baren,
In message 48FC815A.90501@ge.com you wrote:
Would it really have been so hard to add this line?
/* The MPC8544 RM says it's bit 26, but it's really bit 28 */
I have to agree with Timur, we *need* a comment in the source code for future reference. The git log is good, but not sufficient.
Agreed, but in the first place we should figure out if it's *really* bit 28 (then the new code is wrong), or bit 24, then the doc is wrong again.
Its bit 24, my commit message was incorrect.
- k
Dear Kumar Gala,
In message 1224212330-14491-1-git-send-email-galak@kernel.crashing.org you wrote:
This reverts commit dffd2446fb041f38ef034b0fcf41e51e5e489159.
The fix introduced by this patch is not correct. The problem is that the documentation is not correct for the MPC8544 with regards to which bit in PORDEVSR2 is for the SEC_CFG.
Signed-off-by: Kumar Gala galak@kernel.crashing.org
cpu/mpc85xx/speed.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
participants (5)
-
Jerry Van Baren -
Kumar Gala -
Timur Tabi -
Wolfgang Denk -
Wolfgang Grandegger