Hi Stefan, On 4 December 2015 at 00:45, Stefan Roese <s...@denx.de> wrote: > Hi Simon, > > On 03.12.2015 18:21, Simon Glass wrote: >> Hi Stefan, >> >> On 3 December 2015 at 04:31, Stefan Roese <s...@denx.de> wrote: >>> >>> Hi Simon, >>> >>> On 02.12.2015 18:45, Simon Glass wrote: >>>> Hi Stefan, >>>> >>>> On 2 December 2015 at 10:43, Stefan Roese <s...@denx.de> wrote: >>>>> Hi Simon, >>>>> >>>>> ( Last mail for tonight - a glass of quite nice red wine is >>>>> waiting for me ... ;) ) >>>>> >>>> >>>> That's the only sad thing about me being so many hours behind. Still I >>>> can do the same thing with people in Asia I suppose :-) >>> >>> Right. I'm not sure about the wine quality in Asia though... ;) >>> >>>>> >>>>> On 02.12.2015 17:53, Simon Glass wrote: >>>>>> >>>>>> Hi Stefan, >>>>>> >>>>>> On 2 December 2015 at 09:00, Stefan Roese <s...@denx.de> wrote: >>>>>>> >>>>>>> >>>>>>> Hi Simon, >>>>>>> >>>>>>> On 02.12.2015 16:50, Simon Glass wrote: >>>>>>> >>>>>>> <snip> >>>>>>> >>>>>>>>>> I think it would be better to make it depend on whether the bit is >>>>>>>>>> flipped, rather than whether you are in SPL or not. >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> You simply can't detect if this "bit is flipped". You just have >>>>>>>>> to know. This is a long lasting ugly thing on some Marvell >>>>>>>>> patforms. Here the comment from armada-xp-gp.dts: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Can you point me to the place in U-Boot where this bit is flipped? >>>>>>>> Something, somewhere has to make the change. So something has to know. >>>>>>>> Before it makes the change, the range works one way. Afterwards it >>>>>>>> works another way. >>>>>>> >>>>>>> >>>>>>> >>>>>>> Sure. I've mentioned this before. Its here: >>>>>>> >>>>>>> arch/arm/mach-mvebu/cpu.c: >>>>>>> >>>>>>> int arch_cpu_init(void) >>>>>>> { >>>>>>> ... >>>>>>> >>>>>>> /* Linux expects the internal registers to be at 0xf1000000 >>>>>>> */ >>>>>>> writel(SOC_REGS_PHY_BASE, INTREG_BASE_ADDR_REG); >>>>>>> >>>>>>> This is the line that changes the register base address. And >>>>>>> to change it back you need to write to the new address, as the >>>>>>> address holding this base address is also moved. Quite ugly! >>>>>>> >>>>>>> So its really right at the start of U-Boot proper. >>>>>> >>>>>> >>>>>> OK I see. So really we can determine which way the address 'switch' >>>>>> it. It's just a case of making the change when we are ready, and >>>>>> keeping a record of that. >>>>> >>>>> >>>>> Yes. But how is the "common code" in dev_get_addr() supposed to know >>>>> which version of U-Boot we are running on? This boils down to some >>>>> callback again, or not? Or even worse the ugly #ifdef. >>>> >>>> You would call a driver-model core function to select the ranges >>>> property to prefer. Then driver model will remember this setting and >>>> use it. >>> >>> Yes. This can be done. I've taken the time to implement such a >>> version. And attached a small patch in a hackish version, just as >>> an RFC. As you will see, I've added the "ranges-spl" property to >>> some of the DT nodes. And added the DM core functions to enable to >>> usage of a different, non-standard "ranges" property name. >>> >>> All this is not really "clean" and will definitely break non-DM >>> usage of fdt_support.c. Not sure where to go from here. I would >>> still prefer my first patch version, even though I know that >>> you don't like to add this hook / callback into the DM core code. >>> >>> Let me know what you think. >> >> Actually that looks pretty good to me. I think the root uclass needs >> to grow a private struct, where you store the ranges name. It is >> slightly odd to have fdtdec calling back into DM, but I don't see a >> big problem with it. The two are strongly coupled anyway. You can put >> an #ifdef CONFIG_DM in fdtdec to solve your problem I suppose. > > Its not only fdtdec.c but also fdt_support.c that needs this callback > into DM. And fdt_support.c is currently not coupled with DM at all. > Making this change generic, we really need to exchange all "ranges" > occurrences in the whole U-Boot source tree: > > $ git grep "\"ranges\"" > arch/powerpc/cpu/mpc85xx/portals.c: range = fdt_getprop_w(blob, > off, "ranges", &len); > arch/powerpc/cpu/mpc85xx/portals.c: fdt_setprop_inplace(blob, > off, "ranges", range, len); > arch/powerpc/cpu/ppc4xx/fdt.c: rc = fdt_find_and_setprop(blob, ebc_path, > "ranges", ranges, > arch/sparc/include/asm/prom.h:/* Element of the "ranges" vector */ > board/ifm/o2dnt2/o2dnt2.c: prop = fdt_get_property_w(blob, off, > "ranges", &len); > board/ifm/o2dnt2/o2dnt2.c: fdt_setprop(blob, off, "ranges", > reg2, len); > board/intercontrol/digsy_mtc/digsy_mtc.c: prop = > fdt_get_property_w(blob, off, "ranges", &len); > board/intercontrol/digsy_mtc/digsy_mtc.c: fdt_setprop(blob, > off, "ranges", reg2, len); > board/pdm360ng/pdm360ng.c: rc = fdt_find_and_setprop(blob, "/localbus", > "ranges", > board/socrates/socrates.c: rc = fdt_find_and_setprop(blob, "/localbus", > "ranges", > common/fdt_support.c: /* Normally, an absence of a "ranges" property means > we are > common/fdt_support.c: * /ht nodes with no "ranges" property and a lot of > perfectly > common/fdt_support.c: * "ranges" as equivalent to an empty "ranges" > property which means > common/fdt_support.c: return __of_translate_address(blob, node_offset, > in_addr, "ranges"); > common/fdt_support.c: prop = fdt_getprop(fdt, node, "ranges", &size); > common/fdt_support.c: * a number of the "ranges" property array. > common/fdt_support.c: * The "ranges" property is an array of > common/fdt_support.c: ranges = fdt_getprop(fdt, node, "ranges", > &ranges_len); > drivers/core/Kconfig: on some platforms (e.g. MVEBU) using complex > "ranges" > drivers/core/Kconfig: on some platforms (e.g. MVEBU) using complex > "ranges" > drivers/core/simple-bus.c: ret = fdtdec_get_int_array(gd->fdt_blob, > dev->of_offset, "ranges", > drivers/pci/pci-uclass.c: prop = fdt_getprop(blob, node, "ranges", > &len); > > So at least pci-class.c should get changes as well. This looks not > really promising to me. So yes, this works, but I think its quite > clumsy and generates much more code and necessary changes, > especially to the dts files, where all the ranges properties now > need to get duplicated. > >> What about the device tree mailing list. Should I send an email there? > > Sure. We could try to ask about their opinion as well.
What about the idea of setting up an offset in device core. Is it a simple offset? Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot