[U-Boot-Users] [PATCH] Indirect register access through PPC440 DCR.

Stefan Roese
2006-10-07T10:46:00Z

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