Revert "85xx: Using proper I2C source clock divider for MPC8544"
PATCH 1/2 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.
--- cpu/mpc85xx/speed.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
cpu/mpc85xx/speed.c +2 -2
@@ -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 ;
Signature · 1 line
1.5.5.1
PATCH 2/2 85xx: Using proper I2C source clock divider for MPC8544
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.
--- include/asm-ppc/immap_85xx.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
include/asm-ppc/immap_85xx.h +1 -1
@@ -1579,7 +1579,7 @@ typedef struct ccsr_gur {#define MPC85xx_PORDEVSR_RIO_DEV_ID 0x00000007uint 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 0x00000080char res1[8 ];uint gpporcr;/* 0xe0020 - General-purpose POR configuration register */ char res2[12 ];
Signature · 1 line
1.5.5.1
Wolfgang Denk
AppliedPATCH 2/2 85xx: Using proper I2C source clock divider for MPC8544
Dear Kumar Gala,
Quoted 10 lines · In message <[email protected]> you…
The MPC8544 RM incorrect shows the
SEC_CFGbit in PORDEVSR2 as being bit 26, instead it should be bit 28. This caused in incorrect interpretation of thei2c_clkwhich is the same as the SEC clk on MPC8544. The SEC clk is controlled bycfg_sec_freqthat is reported in PORDEVSR2.Signed-off-by Kumar Gala <[email protected]>--- include/asm-ppc/immap_85xx.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
Signature · 5 lines
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 The explanation requiring the fewest assumptions is the most likely to be correct. -- William of Occam
PATCH 2/2 85xx: Using proper I2C source clock divider for MPC8544
In reply to Kumar GalaHi Kumar,
Quoted 7 lines · Kumar Gala
The MPC8544 RM incorrect shows the
SEC_CFGbit in PORDEVSR2 as being bit 26, instead it should be bit 28. This caused in incorrect interpretation of thei2c_clkwhich is the same as the SEC clk on MPC8544. The SEC clk is controlled bycfg_sec_freqthat is reported in PORDEVSR2.Signed-off-by Kumar Gala <[email protected]>
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.
PATCH 2/2 85xx: Using proper I2C source clock divider for MPC8544
In reply to Kumar Galainclude/asm-ppc/immap_85xx.h typedef struct ccsr_gur { · line 1582
Quoted 2 lines · Kumar Gala
include/asm-ppc/immap_85xx.h +1
-1-#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.
Signature · 2 lines
Timur Tabi Linux kernel developer at Freescale
PATCH 2/2 85xx: Using proper I2C source clock divider for MPC8544
Quoted 8 lines · Timur Tabi
include/asm-ppc/immap_85xx.h typedef struct ccsr_gur { · line 1582
Quoted 2 lines · Kumar Gala
include/asm-ppc/immap_85xx.h +1
-1-#define MPC85xx_PORDEVSR2_SEC_CFG 0x00000020 +#define MPC85xx_PORDEVSR2_SEC_CFG 0x00000080How 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
PATCH 2/2 85xx: Using proper I2C source clock divider for MPC8544
Quoted 4 lines · Kumar Gala
Quoted 2 lines
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?
Signature · 2 lines
Timur Tabi Linux kernel developer at Freescale
PATCH 2/2 85xx: Using proper I2C source clock divider for MPC8544
Quoted 9 lines · Timur Tabi
Quoted 3 lines · Kumar Gala
Quoted 2 lines
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.
Quoted 7 lines
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
PATCH 2/2 85xx: Using proper I2C source clock divider for MPC8544
Dear Jerry Van Baren,
Quoted 7 lines · In message <[email protected]> you wrote:
Quoted 4 lines
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
Signature · 4 lines
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 This is an unauthorized cybernetic announcement.
PATCH 2/2 85xx: Using proper I2C source clock divider for MPC8544
Quoted 8 lines · Wolfgang Denk
Dear Jerry Van Baren,
Quoted 5 lines · In message <[email protected]> you wrote:
Quoted 4 lines
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 */
Quoted 7 lines
Quoted 3 lines
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
Wolfgang Denk
AppliedPATCH 1/2 Revert "85xx: Using proper I2C source clock divider for MPC8544"
In reply to Kumar GalaDear Kumar Gala,
Quoted 10 lines · In message <[email protected]> you…
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 <[email protected]>--- cpu/mpc85xx/speed.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
Signature · 5 lines
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
Honest error is to be pitied, not ridiculed.
-- Philip Earl of Chesterfield