Re: [PATCH 04/14] bus: mvebu-mbus: Add static window allocation to the DT binding

[Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]

On Tue, Jun 11, 2013 at 05:26:47PM +0200, Arnd Bergmann wrote:

> That looks ok to me, yes. If you have just one device under some of the nodes
> however, I think it's easier use an empty ranges property and do
> 
>  	devbus-bootcs {
>  		ranges;
>  		nor {
>  			reg = <0x012f0000 0 0x1000000>;
>  		};
> 
> The 'ranges' here is most useful if you have multiple devices
> add different offsets. I would also predefine those ranges to
> be as large as possible so you don't have to adapt them when
> the child device grows beyond it.

It isn't super critical, but the ranges does keep the 0x012f0000 value
in the SOC .dtsi and the board level doesn't have to be exposed to the
value, it just uses 0 to setup the NOR.

Also, it makes it much simpler for the mbus driver to detect which
target id's are required so they can be allocated/setup - the rule
would be every child stanza should have a ranges with the target(s) it
needs.

The full version probably looks like this:

mbus {
    ranges ....

                        devbus-bootcs@10400 {
                                compatible = "marvell,mvebu-devbus";
                                reg = <INTERNAL_REGS + 0x10400 0x8>;
                                ranges = <0 DEVBUS_BOOTCS 0x1000000>;
                                #address-cells = <1>;
                                #size-cells = <1>;
                                clocks = <&coreclk 0>;

				// In board .dts:
                                nor@0 {
                                        compatible = "cfi-flash";
                                        reg = <0 0x1000000>;
                                        bank-width = <2>;
                                };
                        };
};

And notice here that we need to offset INTERNAL_REGS to make this
work. To me, this is the primary reason why the 2nd cell of the mbus
address format *must* be the address offset within the target, and not
anything else.

> > 2. There's no need to use an algorithm to 'find' a base address for decoding
> >    windows (such as first-fit as Arnd suggested in another mail).
> >    The base address is already there in the ranges property, so the mbus can
> >    simply allocate the window on that base address.
> > 
> > So, am I right or completely lost?
> 
> Since you already need code to update the ranges property, I think
> it's best to discard the ranges at boot time and create a new mapping
> that just maps the devices you actually want to use. That would make
> it much easier to cope with boot loaders that don't set up the
> mappings in the same way that is listed in the device tree. Just
> put the one entry for the internal-regs in the ranges, since that
> is needed to map do the mbus setup.

So, I think we can/have agree that the ranges in the FDT should
reflect the bootloader's settings, and if the ranges is missing an
element it means the bootloader didn't set it up.

A compatible bootloader should dump the entire mbus register table
into the ranges. The address encoding is such that we can represent
every mbus window entry in ranges. If no bootloader support is present
then the ranges value is forced into the mbus windows anyhow.

If there isn't a ranges entry, but there is a child that requires it
(how do we figure this out?) then dynamic assignment of that window
makes sense to me. (however, this also looks a bit tricky, how do you
avoid hitting the PCI-E window reservations, for instance?)

Unconditional dynamic assignment is a bit more tricky, for instance
the boot rom has to be located at a specific address since it is used
as the SMP secondary startup trampoline. (Ezequiel: IMHO, you should
check for this and bail secondary startup if it is not setup)

I'm not sure there is a good reason to reject the address map in the
DT?

Jason