[U-Boot] [PATCH] dm: core: Add platform specific bus translation function

Simon Glass
2015-12-03T17:21:52Z

Hi Stefan,

On 3 December 2015 at 04:31, Stefan Roese <sr@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 <sr@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 <sr@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.

What about the device tree mailing list. Should I send an email there?

Regards,
Simon