[U-Boot] [PATCH] inka4x0: Add hardware diagnosis functions for inka4x0

[Wolfgang Denk]

Dear Detlev & Kumar,

In message <m2ljqtr68m.fsf at ohwell.denx.de> you wrote:
> 
...
> >> +	/* reset PSC */
> >> +	psc->command = PSC_SEL_MODE_REG_1;
> >
> > Should we not use accessor function to access the device registers
> > (here and in the following code) ?
> 
> Indeed we should.  But here we go, I can understand, why this wasn't
> done the first time around.  The new code looks like this:
> 
> 	/* reset PSC */
> 	out_8(&psc->command, PSC_SEL_MODE_REG_1);
> 
> 	/* select clock sources */
> 
> 	out_be16(&psc->psc_clock_select, 0);
> 	baseclk = (gd->ipb_clk + 16) / 32;
> 
> 	/* switch to UART mode */
> 	out_be32(&psc->sicr, 0);
...
> Look at the wonderful mix of out_{8,be16,be32}.  What a heart refreshing
> break from the monotonic coding routine :(

Indeed, what a mess, especially given the fact that these are all 32
bit registers, or am I wrong?

> Actully this is due to to mpc5xxx.h:
> 
> struct mpc5xxx_psc {
> 	volatile u8	mode;		/* PSC + 0x00 */
> 	volatile u8	reserved0[3];
> 	union {				/* PSC + 0x04 */
> 		volatile u16	status;
> 		volatile u16	clock_select;
> 	} sr_csr;
> #define psc_status	sr_csr.status
> #define psc_clock_select sr_csr.clock_select
> 	volatile u16	reserved1;
> 	volatile u8	command;	/* PSC + 0x08 */
> 	volatile u8	reserved2[3];
> 	union {				/* PSC + 0x0c */
> 		volatile u8	buffer_8;
> 		volatile u16	buffer_16;
> 		volatile u32	buffer_32;
> 	} buffer;
> #define psc_buffer_8	buffer.buffer_8
> #define psc_buffer_16	buffer.buffer_16
> #define psc_buffer_32	buffer.buffer_32
> 	union {				/* PSC + 0x10 */
> 		volatile u8	ipcr;
> 		volatile u8	acr;
> 	} ipcr_acr;
> #define psc_ipcr	ipcr_acr.ipcr
> #define psc_acr		ipcr_acr.acr
> 	volatile u8	reserved3[3];
> 	union {				/* PSC + 0x14 */
> 		volatile u16	isr;
> 		volatile u16	imr;
> 	} isr_imr;
> 
> 
> Git-blame points the finger to someone called 'wd'.  Maybe he knows
> about the advantages of such structure definitions?  

Sorry, I don;t. I'm pretty sure that I didn't write that code.

> On second check the corresponding header file in Linux (mpc52xx_uart.h) has
> stuff like this introduced by Kumar Gala, maybe he can enlighten me?
> 
> To be honest, I wouldn't even dare accessing memory mapped registers in
> the non-native size, but maybe I'm missing something here.

Kumar, do you have any explanation for this strange design?

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
Die meisten Menschen pflegen im Kindesalter vom Zeigen auf Gegenstän-
de (Mausbewegung) und "ga" sagen  (Mausklick)  abzukommen,  zugunsten
eines  mächtigeren  und langwierig zu erlernenden Tools (Sprache).
                             -- Achim Linder in de.comp.os.linux.misc