Dear Timur Tabi,

In message <1289343709-11793-1-git-send-email-ti...@freescale.com> you wrote:
> U-Boot uses macros to determine where devices should be located in physical
> memory, and Linux uses the device tree to determine where the devices are
> actually located.  However, U-Boot does not update the device tree with those
> addresses when it boots the operating system.  This means that it's possible
> to use a device tree with the wrong addresses, and U-Boot won't complain.
> This frequently happens when U-Boot is compiled for 32-bit physical addresses,
> but the device tree uses 36-bit addresses.

I understand that you can reasonably check these addresses only in
this specific case (i. e. a 36 bit DT and a 32 bit U-Boot) ?

What is the actualk woth of such a check when it covers only one out
of 4 possible combinations?

And should that not be made optional?  There are enough 85xx boards
which don;t even have 36 bit configurations in Linux, so why waste
memory and runtime on these?

> +     for (i = 0; i < count; i++) {
> +             /* Parse one line of the 'ranges' property */
> +             attr = *ranges;
> +             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);
> +             if (size_cells == 1)
> +                     dt_size = be32_to_cpup(ranges + address_cells +
> +                                            parent_address_cells);
> +             else
> +                     /* size_cells == 2 */
> +                     dt_size = be64_to_cpup(ranges + address_cells +
> +                                           parent_address_cells);

Braces needed for multiline statements. Please fix globally.

> +             /*
> +              * 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;
> +             }
> +             ranges += address_cells + parent_address_cells + size_cells;
> +     }
> +
> +     printf("Warning: node %s has a missing or incorrect %s region at\n"
> +            "address=%llx size=%llx\n",
> +            fdt_get_name(blob, node, NULL), is_io ? "an I/O" : "a memory",
> +            (u64)addr, (u64)size);
> +     return;
> +
> +wrong_type:
> +     printf("Warning: node %s has 'ranges' property for address %llx and\n"
> +            "size %llx, but of the wrong type\n",
> +            fdt_get_name(blob, node, NULL), (u64)dt_addr, (u64)dt_size);
> +     return;

wrong_type is referenced only once, so please move the code and get
rid of the "goto".


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: w...@denx.de
Perl already has an IDE.  It's called Unix.
                      -- Tom Christiansen in 375bd...@cs.colorado.edu
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to