DA8xx: fix LPSC numbering

Hi,

I'm in the process of porting u-boot to a new board we have developed using the OMAP-L138 (or DA850). While I'm a long way from finishing the port, I have noticed that the definitions of some of the local power and sleep controller (LPSC) register module numbers are incorrect.

As a reply to this mail I've attached my first attempt at a patch for u-boot, which addresses this issue.

Any feedback (esp. wrt formatting and use of the git tools etc.) is gratefully received.

Laurence Withers (1):
DA8xx: fix LPSC numbering

 arch/arm/include/asm/arch-davinci/hardware.h |   29 +++++++++++++++++---------
 1 files changed, 19 insertions(+), 10 deletions(-)

Laurence Withers, <[email protected]>                http://www.guralp.com/
Direct tel:+447753988197 or tel:+443333408643               Software Engineer
General support queries: <[email protected]>         CMG-DCM CMG-EAM CMG-NAM

The DA8xx chips have two modules PSC0 and PSC1 for the local power and sleep controllers (LPSC). Each LPSC has up to 32 submodules over which it has control, which are enumerated by the DAVINCI_LPSC_* symbols.

This commit fixes the definitions of a number of symbols to be consistent with both the OMAP-L137 and OMAP-L138 data sheets (TI documents SPRS563D and SPRS586A respectively): some minor renaming to reflect actual functionality and some reordering of modules in PSC1 to be correct.

None of the affected symbols were actually used anywhere in the code, so there are no related code changes.

---
 arch/arm/include/asm/arch-davinci/hardware.h |   29 +++++++++++++++++---------
 1 files changed, 19 insertions(+), 10 deletions(-)

index df3f549..d0a3036 100644
@@ -214,12 +214,12 @@ typedef volatile unsigned int *	dv_reg_p;
 #else /* CONFIG_SOC_DA8XX */
 
 enum davinci_lpsc_ids {
-	DAVINCI_LPSC_TPCC = 0,
+	DAVINCI_LPSC_TPCC0 = 0,
 	DAVINCI_LPSC_TPTC0,
 	DAVINCI_LPSC_TPTC1,
 	DAVINCI_LPSC_AEMIF,
 	DAVINCI_LPSC_SPI0,
-	DAVINCI_LPSC_MMC_SD,
+	DAVINCI_LPSC_MMC_SD0,
 	DAVINCI_LPSC_AINTC,
 	DAVINCI_LPSC_ARM_RAM_ROM,
 	DAVINCI_LPSC_SECCTL_KEYMGR,

@@ -227,31 +227,40 @@ enum davinci_lpsc_ids {
 	DAVINCI_LPSC_SCR0,
 	DAVINCI_LPSC_SCR1,
 	DAVINCI_LPSC_SCR2,
-	DAVINCI_LPSC_DMAX,
+	DAVINCI_LPSC_PRUSS,
 	DAVINCI_LPSC_ARM,
 	DAVINCI_LPSC_GEM,
 	/* for LPSCs in PSC1, offset from 32 for differentiation */
 	DAVINCI_LPSC_PSC1_BASE = 32,
-	DAVINCI_LPSC_USB11,
+	DAVINCI_LPSC_TPCC1 = 32,
 	DAVINCI_LPSC_USB20,
+	DAVINCI_LPSC_USB11,
 	DAVINCI_LPSC_GPIO,
 	DAVINCI_LPSC_UHPI,
 	DAVINCI_LPSC_EMAC,
 	DAVINCI_LPSC_DDR_EMIF,
 	DAVINCI_LPSC_McASP0,
-	DAVINCI_LPSC_McASP1,
-	DAVINCI_LPSC_McASP2,
+	DAVINCI_LPSC_McASP1_SATA,
+	DAVINCI_LPSC_McASP2_VPIF,
 	DAVINCI_LPSC_SPI1,
 	DAVINCI_LPSC_I2C1,
 	DAVINCI_LPSC_UART1,
 	DAVINCI_LPSC_UART2,
+	DAVINCI_LPSC_McBSP0,
+	DAVINCI_LPSC_McBSP1,
 	DAVINCI_LPSC_LCDC,
 	DAVINCI_LPSC_ePWM,
+	DAVINCI_LPSC_MMC_SD1,
+	DAVINCI_LPSC_uPP,
 	DAVINCI_LPSC_eCAP,
-	DAVINCI_LPSC_eQEP,
-	DAVINCI_LPSC_SCR_P0,
-	DAVINCI_LPSC_SCR_P1,
-	DAVINCI_LPSC_CR_P3,
+	DAVINCI_LPSC_eQEP_TPTC2,
+	DAVINCI_LPSC_SCR_F0 = 56,
+	DAVINCI_LPSC_SCR_F1,
+	DAVINCI_LPSC_SCR_F2,
+	DAVINCI_LPSC_SCR_F6,
+	DAVINCI_LPSC_SCR_F7,
+	DAVINCI_LPSC_SCR_F8,
+	DAVINCI_LPSC_BR_F7,
 	DAVINCI_LPSC_L3_CBA_RAM
 };
 

1.7.2.5

Dear Laurence Withers,

SoB missing.

Lines in commit message too long.

Please fix also all other places where these identifiers are used in the code.

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
Genius doesn't work on an assembly line basis.  You can't simply say,
"Today I will be brilliant."
	-- Kirk, "The Ultimate Computer", stardate 4731.3

Thanks; I have fixed and will resend.

The identifiers changed in the patch were not used anywhere in the code. I also verified that a "./MAKEALL -s davinci" gave the same results before and after.

Many thanks for the feedback, and bye for now,

Laurence Withers, <[email protected]>                http://www.guralp.com/
Direct tel:+447753988197 or tel:+443333408643               Software Engineer
General support queries: <[email protected]>         CMG-DCM CMG-EAM CMG-NAM

Dear Laurence Withers,

I cannot confirm this; for example, you change:

- DAVINCI_LPSC_TPCC = 0,
+ DAVINCI_LPSC_TPCC0 = 0,

I see:

-> grep -R DAVINCI_LPSC_TPCC *
arch/arm/include/asm/arch-davinci/hardware.h:#define DAVINCI_LPSC_TPCC 2
arch/arm/include/asm/arch-davinci/hardware.h: DAVINCI_LPSC_TPCC = 0,
board/davinci/dm355evm/dm355evm.c: lpsc_on(DAVINCI_LPSC_TPCC);

you change:

- DAVINCI_LPSC_MMC_SD,
+ DAVINCI_LPSC_MMC_SD0,

I see:

-> grep -R DAVINCI_LPSC_MMC_SD *
arch/arm/cpu/arm926ejs/davinci/psc.c: case DAVINCI_LPSC_MMC_SD:
arch/arm/include/asm/arch-davinci/hardware.h:#define DAVINCI_LPSC_MMC_SD 15
arch/arm/include/asm/arch-davinci/hardware.h: DAVINCI_LPSC_MMC_SD,

Seems your change would break a few boards...

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 story about the person who wished his computer were as
easy to use as his telephone. That wish has come  true,  since  I  no
longer know how to use my telephone.

There are several chips in the Davinci family; my commit changes the LPSC definitions for the DA8xx (the DA830/DA850) processors only, leaving the other processors alone (they are quite different).

Looking at the hardware.h file that I changed, just above the enum { } block where my changes reside are a set of #defines for the same symbols. The #defines are used when CONFIG_SOC_DA8XX is not defined; the enum is used when it is.

The existing uses in the code are special cases for the non-DA8xx processors; DAVINCI_LPSC_TPCC is used in the DM355 EVM kit board code, and while DAVINCI_LPSC_MMC_SD is used in the processor generic C code it is wrapped in a #ifdef CONFIG_SOC_DM644X (another chip) with a comment about special treatment.

So, I can change the non-DA8xx names to match. But that changes existing, working code and the names would no longer match the datasheets.

Or, I can leave the old names be, and then have the new names with suffices, e.g.:

DAVINCI_LPSC_MMC_SD, /* actually MMC_SD0 on DA8xx */
DAVINCI_LPSC_MMC_SD1,

It seems a little inconsistent to me, but I'm happy to do it if preferred. Or, I can do what I have done, and change only the DA8xx names.

Which is the preferred option? Or something else entirely?

Many thanks, and bye for now,

Laurence Withers, <[email protected]>                http://www.guralp.com/
Direct tel:+447753988197 or tel:+443333408643               Software Engineer
General support queries: <[email protected]>         CMG-DCM CMG-EAM CMG-NAM

Dear Laurence Withers,

Argh. Can we please clean up such a mess, then?

We should agree on a common way to implement this - either #define _or_ enum, but not a mix of both.

Having a closer look, it turns out that all these "indices" are actually register names, and functions like lpsc_on() use horrible code like this:

	mdstat = REG_P(PSC_MDSTAT_BASE + (id * 4));

to perform address magic, even worse, perform device I/O without using any I/O acessors (as in dsp_on()).

I am aware that this is unrelated to your patch, but this needs to be cleaned up ASAP!

As for your patch - maybe we can at least unify the #define versus enum mixture?

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
"Beauty is transitory." "Beauty survives."
	-- Spock and Kirk, "That Which Survives", stardate unknown

So, as a rough way forward, does the following sound sane?

- drop REG_P() and REG() macros, use readl()/writel()/etc. instead.
- drop address magic, use structures/arrays instead.
- choose either enum or #define (I'd say enum as it's mostly a set of incrementing integers, but it depends on what the result looks like).
- try to unify the various Davinci LPSC modules as much as possible.

I'm willing to give this a go, but I am unable to physically test the results on any of the Davinci family except the DA850.

Bye for now,

Laurence Withers, <[email protected]>                http://www.guralp.com/
Direct tel:+447753988197 or tel:+443333408643               Software Engineer
General support queries: <[email protected]>         CMG-DCM CMG-EAM CMG-NAM

Dear Laurence Withers,

Why do you falg a message that does not contail a patch with a '[PATCH]' subject? Please don't!!

This is crap. Don't do such things.

Please read the documentation _before_ posting.

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
I am more bored than you could ever possibly be.  Go back to work.