From mboxrd@z Thu Jan 1 00:00:00 1970 From: Timur Tabi Date: Wed, 10 Nov 2010 14:54:30 -0600 Subject: [U-Boot] [PATCH] powerpc/85xx: compare actual device addresses with the device tree In-Reply-To: <20101110203451.550DD1522C0@gemini.denx.de> References: <1289343709-11793-1-git-send-email-timur@freescale.com> <20101110195113.3A7871522C0@gemini.denx.de> <4CDAFC37.40309@freescale.com> <20101110203451.550DD1522C0@gemini.denx.de> Message-ID: <4CDB0686.300@freescale.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Wolfgang Denk wrote: > 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? Well, it probably isn't an *error* technically, but it might be unexpected. But I will consider removing that code, if there's some kind of consensus that it's not worthwhile. >> 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 ;-) Heh. >> 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. Well, I really do think it should be done at every boot by default, but I would be willing to consider an "fdt verify" command. Would you be willing to buy me a beer every time someone forgets to run "fdt verify" and emails me instead? > 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. Would "fdt verify" be a good place? >> 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. Ok. -- Timur Tabi Linux kernel developer at Freescale