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

[Detlev Zundel <dzu@denx.de>]

Hi Anatolij,

> Hi Detlev,
>
> Detlev Zundel wrote:
>> This patch adds advanced diagnosis functions for the inka4x0 board.
>> 
>> Signed-off-by: Andreas Pfefferle <ap@denx.de>
>> Signed-off-by: Detlev Zundel <ap@denx.de>
>
> Something went wrong here, duplicated e-mail address from
> Andreas in your SoB. Also please see some style comments below.

Yes indeed, something went wrong here ;)  Thanks for catching this.
Actually I *knew* there was a problem because the automatically
generated coverletter had Andreas the originator of one of the patches
which I believed was markes as mine.  I simply didn't see it.  Thanks
for catching this!

>> diff --git a/board/inka4x0/inkadiag.c b/board/inka4x0/inkadiag.c
>> new file mode 100644
>> index 0000000..bdbf652
>> --- /dev/null
>> +++ b/board/inka4x0/inkadiag.c
> ...
>> +
>> +static void inka_digio_set_output(unsigned int state, int which)
>> +{
>> +	if (which == 0) {
>> +		/* other */
>> +		CHECK_LED(0);
>> +		CHECK_LED(1);
>> +		CHECK_LED(2);
>> +		CHECK_LED(3);
>> +		CHECK_LED(4);
>> +		CHECK_LED(5);
>> +	} else {
>> +		struct mpc5xxx_gpio *gpio = (struct mpc5xxx_gpio *)MPC5XXX_GPIO;
>> +		if (which == 1) {
>
> maybe we should add blank line before 'if' statement here?

To be honest - I don't care. Fixed.

> ...
>> +static void ser_putc(volatile struct mpc5xxx_psc *psc, const char c)
>> +{
>> +	/* Wait 1 second for last character to go. */
>> +	int i = 0;
>> +	while (!(psc->psc_status & PSC_SR_TXEMP) && (i++ < 1000000/10))
>
> add blank line before while().

As you wish.

> ...
>> +static int ser_getc(volatile struct mpc5xxx_psc *psc)
>> +{
>> +	/* Wait for a character to arrive. */
>> +	int i = 0;
>> +	while (!(psc->psc_status & PSC_SR_RXRDY) && (i++ < 1000000/10))
>> +		udelay(10);
>
> add blank line before while().

Sure we do.

> ...
>> +U_BOOT_CMD(inkadiag, 6, 1, do_inkadiag,
>> +	   "inkadiag - inka diagnosis\n",
>> +	   "[inkadiag what ...]\n"
>> +	   "    - perform a diagnosis on inka hardware\n"
>> +	   "'inkadiag' performs hardware tests.\n\n");
>> +
>> +/* Relocate the command table functino pointers when running in RAM */
>
> Typo, s/functino/function/

Ah, thanks!

>> +int inkadiag_init_r (void) {
>> +	cmd_tbl_t *cmdtp;
>> +
>> +	for (cmdtp = &cmd_inkadiag_sub[0]; cmdtp !=
>> +		     &cmd_inkadiag_sub[ARRAY_SIZE(cmd_inkadiag_sub)]; cmdtp++) {
>> +		ulong addr;
>> +		addr = (ulong) (cmdtp->cmd) + gd->reloc_off;
>
> add blank line after declaration.

Fixed.

Thanks for taking the time to look over it!

Cheers
  Detlev

-- 
If you currently have a 32-bit UNIX system, you are advised to
trade it in for a 64-bit one sometime before the year 2106.
 -- Andrew S. Tanenbaum: Modern Operating Systems, 2nd Edition
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de