[U-Boot] [PATCH] [v2] powerpc/85xx: introduce 'fdt verify' command

[Wolfgang Denk]

Dear Timur Tabi,

In message <1290015805-18791-1-git-send-email-timur at freescale.com> you wrote:
> Introduce the 'fdt verify' command, which verifies some of the physical
> addresses in the device tree.

> +		addr = fdt_translate_address(blob, off, reg);
> +		if (!addr)
> +			/* We can't determine the base address, so skip it */
> +			continue;

Braces needed for multiline statements.

> +/*
> + * Verify the device addresses in the device tree
> + *
> + * This function compares several CONFIG_xxx macros that contain physical
> + * addresses with the corresponding nodes in the device tree, to see if the
> + * physical addresses are all correct.  For example, if CONFIG_SYS_NS16550_COM1
> + * is defined, then it contains the virtual address of the first UART.  We
> + * convert this to a physical address and compare that with the physical
> + * address of the first ns16550-compatible node in the device tree.  If they
> + * don't match, then we display a warning.
> + */

LIne toolong. Please check and fix globally.

> +	if (ccsr != CONFIG_SYS_CCSRBAR_PHYS)
> +		printf("Warning: U-Boot configured CCSR at address %llx, " \
> +		       "but the device tree has it at %llx\n",
> +		       (uint64_t) CONFIG_SYS_CCSRBAR_PHYS, ccsr);

Braces needed for multiline statements. Please fix globally.

No backslash needed at end of line, please remove.

>  struct fdt_header *working_fdt;
> @@ -436,6 +450,10 @@ int do_fdt (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
>  	else if (strncmp(argv[1], "re", 2) == 0) {
>  		fdt_resize(working_fdt);
>  	}
> +	/* verify the addresses in the fdt */
> +	else if (argv[1][0] == 'v') {
> +		fdt_verify_addresses(working_fdt);
> +	}
>  	else {

2x incorrect coding style - the "else" goes on the saame line with the
'}' and the '{'

> +	if (addr != dt_addr)
> +		printf("Warning: U-Boot configured device %s at address %llx,\n"
> +		       "but the device tree has it address %llx.\n",
> +		       alias, (u64)addr, dt_addr);

Braces needed.

> +		if (parent_address_cells == 1)
> +			dt_addr = be32_to_cpup(ranges + address_cells);
> +		else
> +			/* parent_address_cells == 2 */
> +			dt_addr = be64_to_cpup(ranges + address_cells);

and again.

> +		if (size_cells == 1)
> +			dt_size = be32_to_cpup(ranges + address_cells +
> +					       parent_address_cells);

and again.

> +		else
> +			/* size_cells == 2 */
> +			dt_size = be64_to_cpup(ranges + address_cells +
> +					      parent_address_cells);

and again. etc. etc.

> +
> +		/*
> +		 * Check for matches.  If the address matches but is the wrong
> +		 * type or wrong size, then return an error.
> +		 */
> +		if ((attr & PCI_CELL0_SS_MASK) == PCI_CELL0_SS_IO) {
> +			if (is_io && (dt_addr == addr)) {
> +				if (dt_size == size)
> +					return;
> +				else
> +					goto wrong_size;
> +			}
> +			if (!is_io && (dt_addr == addr))
> +				goto wrong_type;
> +		} else {
> +			if (!is_io && (dt_addr == addr)) {
> +				if (dt_size == size)
> +					return;
> +				else
> +					goto wrong_size;
> +			}
> +			if (is_io && (dt_addr == addr))
> +				goto wrong_type;
> +		}

Factor out the "(dt_addr == addr)" from all 4 "if"s.

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
Human beings were created by water to transport it uphill.