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_suppord0-t.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.
