From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Roese <sr@denx.de>
Date: Sat, 7 Oct 2006 12:46:00 +0200
Subject: [U-Boot-Users] [PATCH] Indirect register access through PPC440
	DCR.
In-Reply-To: <406A31B117F2734987636D6CCC93EE3C35D033@ehost011-3.exch011.intermedia.net>
References: <406A31B117F2734987636D6CCC93EE3C35D033@ehost011-3.exch011.intermedia.net>
Message-ID: <200610071246.00685.sr@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Leonid,

On Friday 06 October 2006 01:17, Leonid wrote:
> * Add monitor functions for indirect access to PPC440 registers
>   via Data Control Register (DCR).
>
> PATCH: (diff file ppc440dcr.txt attached).

Please find some comments below...

> ==== //depot/Alchemy/ppc/uboot/u-boot-1.1.4/common/cmd_dcr.c#1 - /home/leonid/pd/ppc/uboot/u-boot-1.1.4/common/cmd_dcr.c ====
> @@ -104,10 +104,126 @@

Please generate the patch as described in the README:

diff -purN OLD NEW

Or create a git patch.

>  	} while (nbytes);
>
>  	return 0;
>  }
>
> +/* =======================================================================
> + * Interpreter command to retrieve an register value through AMCC PPC 4xx
> + * Device Control Register inderect addressing.
> + * =======================================================================
> + */
> +int do_getidcr ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[] )
> +{
> +	unsigned long get_dcr (unsigned short);
> +	unsigned long set_dcr (unsigned short, unsigned long);
> +	unsigned short   adr_dcrn;	/* Device Control Register Num for Address */
> +	unsigned short   dat_dcrn;	/* Device Control Register Num for Data */
> +        unsigned short     offset;      /* Register's offset */
> +	unsigned long     value;	/* register's value */
> +        unsigned char     * ptr=NULL, buf[80];

Only tabs for indentation.

> +
> +	/* Validate arguments */
> +	if (argc < 3) {
> +		printf ("Usage:\n%s\n", cmdtp->usage);
> +		return 1;
> +	}
> +
> +	/* Find out whether ther is '.' (dot) symbol in the first parameter. */
> +        strncpy(buf, argv[1], sizeof(buf)-1);

Indentation.

> +        buf[sizeof(buf)-1]=0; /* will guarantee zero-end string */
> +        ptr  = strchr(buf, '.');
> +
> +	if(ptr != NULL)
> +	  {/* first parameter has fromat adr_dcrn.dat_dcrn */

Opening brace in the line of the if statement please.

> +	    * ptr = 0; /* erase '.', create zero-end string */
> +	    ptr++; /* will point to dat_dcr */
> +	   adr_dcrn = (unsigned short) simple_strtoul (buf, NULL, 16);
> +	   dat_dcrn = (unsigned short) simple_strtoul (ptr, NULL, 16);
> +	  }
> +        else
> +          { /* first parameter has fromat adr_dcrn; dat_dcrn will be
> +               calculated as adr_dcrn+1. */

Please use:
	} else {

I'm stopping here. Please recheck you patch regarding the coding style.
You could of course use something like "Lindent" to help you here.

<snip>

Please fix the above mentioned issues and resubmit a new patch. Thanks.

Best regards,
Stefan