[U-Boot] [PATCH] common/cmd_ide.c: Coding style cleanup (Lindent)

[Wolfgang Denk]

Dear Stefan Roese,

In message <1242207497-21212-1-git-send-email-sr at denx.de> you wrote:
> While trying to fix a problem in the IDE detection of the CPCI750
> I noticed the extreme bad coding style in the cmd_ide.c file. Before
> fixing the real problem I ran Lindent on this file. I also removed
> some "#if 0" parts.
> 
> Not sure if this patch will be accepted. It's probably not so easy to
> understand the history of changes in this file with this coding style
> cleanup. But the current condition is so bad that I think it's worth
> the change.

Another argument against the patch is that it makes the code in some
places more ugly...

> -const static pio_config_t pio_config_ns [IDE_MAX_PIO_MODE+1] =
> -{
> -    /*	Setup  Length  Hold  */
> -	{ 70,	165,	30 },		/* PIO-Mode 0, [ns]	*/
> -	{ 50,	125,	20 },		/* PIO-Mode 1, [ns]	*/
> -	{ 30,	101,	15 },		/* PIO-Mode 2, [ns]	*/
> -	{ 30,	 80,	10 },		/* PIO-Mode 3, [ns]	*/
> -	{ 25,	 70,	10 },		/* PIO-Mode 4, [ns]	*/
> +const static pio_config_t pio_config_ns[IDE_MAX_PIO_MODE + 1] = {
> +	/*  Setup  Length  Hold  */
> +	{70, 165, 30},		/* PIO-Mode 0, [ns]     */
> +	{50, 125, 20},		/* PIO-Mode 1, [ns]     */
> +	{30, 101, 15},		/* PIO-Mode 2, [ns]     */
> +	{30, 80, 10},		/* PIO-Mode 3, [ns]     */
> +	{25, 70, 10},		/* PIO-Mode 4, [ns]     */
>  };

Instead of nice vertically aligned columns the new code is less
readable.

> -static void  ide_ident (block_dev_desc_t *dev_desc);
> -static uchar ide_wait  (int dev, ulong t);
> +static void ide_ident(block_dev_desc_t * dev_desc);

And I really hate the space after the '*' in lines like these,
especially as it's done more or lessa randomly:

> -static void	atapi_inquiry(block_dev_desc_t *dev_desc);
> -ulong atapi_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer);
> +static void atapi_inquiry(block_dev_desc_t * dev_desc);
> +ulong atapi_read(int device, lbaint_t blknr, ulong blkcnt, void *buffer);

We have "* dev_desc", but "*buffer" ? That's just bogus.

> -	printf ("\nLoading from IDE device %d, partition %d: "
> -		"Name: %.32s  Type: %.32s\n",
> -		dev, part, info.name, info.type);
> +	printf("\nLoading from IDE device %d, partition %d: "
> +	       "Name: %.32s  Type: %.32s\n", dev, part, info.name, info.type);

Also, reordering the lines here makes the code more difficult to read.

> -	if (ide_dev_desc[dev].block_read (dev, info.start, 1, (ulong *)addr) != 1) {
> -		printf ("** Read error on %d:%d\n", dev, part);
> -		show_boot_progress (-48);
> +	if (ide_dev_desc[dev].block_read(dev, info.start, 1, (ulong *) addr) != 1) {

Why is there a space after the cast?

> -	switch (genimg_get_format ((void *)addr)) {
> +	switch (genimg_get_format((void *)addr)) {

But not so here?

>  	case IMAGE_FORMAT_LEGACY:
> -		hdr = (image_header_t *)addr;
> +		hdr = (image_header_t *) addr;

But here again?

> +			s = getenv("ide_doreset");
> +			if (s && strcmp(s, "on") == 0)
> +#endif
> +			{
> +				/* Need to soft reset the device in case it's an ATAPI...  */

Resulting line too long...

> -		pio_mode = 0; /* Force it to dead slow, and hope for the best... */
> +		pio_mode = 0;	/* Force it to dead slow, and hope for the best... */

Ditto.

> +		if (pwrsave) {
> +			c = ide_wait(device, IDE_SPIN_UP_TIME_OUT);	/* may take up to 4 sec */

And again...

> +U_BOOT_CMD(ide, 5, 1, do_ide,
> +	   "IDE sub-system",
> +	   "reset - reset IDE controller\n"
> +	   "ide info  - show available IDE devices\n"
> +	   "ide device [dev] - show or set current device\n"
> +	   "ide part [dev] - print partition table of one or all IDE devices\n"
> +	   "ide read  addr blk# cnt\n"
> +	   "ide write addr blk# cnt - read/write `cnt'"
> +	   " blocks starting at block `blk#'\n"
> +	   "    to/from memory address `addr'\n");
> +
> +U_BOOT_CMD(diskboot, 3, 1, do_diskboot,
> +	   "boot from IDE device", "loadAddr dev:part\n");

And indentation should be done by TAB - I see no reason to add more
space here.


I agree that this file badly needs to be cleaned up, and I even agree
to a patch like above - but it has to be done more intelligently. The
resulting file should adhere to the Coding Style requirements, and be
consistent in style.

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
The evolution of the human race will not be accomplished in  the  ten
thousand  years  of  tame  animals,  but in the million years of wild
animals, because man is and will always be a wild animal.
                                              - Charles Galton Darwin