[U-Boot] [PATCH] powerpc/85xx: compare actual device addresses with the device tree

Wolfgang Denk
2010-11-10T20:34:51Z

Dear Timur Tabi,

In message <4CDAFC37.40309@freescale.com> you wrote:
>
> It's more than that. Any mismatch in the CCSR base address, serial device
> offsets, or PCI addresses will be caught. For instance, if you put the PCIE1
> memory range at ff800000 in the device tree, but ff6000000 in U-Boot, it will
> catch that.

Would that hurt? I though Linux does it's own initialization of the
PCI system anyway, so does it matter what we did in U-Boot?

> Because the problem shows up when you least expect it. It's designed to
> eliminate debug problems. If we make this enabled only when people define a
> macro that isn't normally defined, then people won't know about it. We have so
> many problems with customers and other developers getting the device tree wrong,
> and immediately contacting us for help without doing even the slightest
> debugging. I'm sure you're familiar with that.

Hey, you are not supposed to bring us out of work. Rather help to make
the code complex and difficult to understand ;-)

> I can add a macro that needs to be defined in order to *disable* it, so that on
> finalized systems where the device tree is known to be correct, this check can
> be skipped. But I really would prefer to have this feature enabled by default.

I understand what you want to do, and why you want to do it, but then
I also feel thatit is inherently wrong. It cannot be U-Boot's taks to
validate the correctness of the device tree on every boot.

If we agree that this is adebug help, then please provide a separate
command to perform this operation. Make this command optional (feel
free to add it to the default list, but it must be possible to disable
it if wanted). Then users who want this feature can add it to their
boot command sequence, and others, who are interested in fast boot
times can omit it.

> So you're saying you want to see this:
>
> if (parent_address_cells == 1)
> dt_addr = be32_to_cpup(ranges + address_cells);
> else {

Yes, at least. Actually I prefer to use braces for the "then" branch
as well if the "else" branch needs them.

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
: I've tried (in vi) "g/[a-z]\n[a-z]/s//_/"...but that doesn't
: cut it. Any ideas? (I take it that it may be a two-pass sort of solution).
In the first pass, install perl. :-) Larry Wall <6849@jpl-devvax.JPL.NASA.GOV>