[PATCH v3 03/12] bus: mvebu-mbus: Add static window allocation to the DT binding

Hi Arnd,

On Tue, Jun 18, 2013 at 11:45:26PM +0200, Arnd Bergmann wrote:

On Tuesday 18 June 2013, Ezequiel Garcia wrote:

quoted

On Tue, Jun 18, 2013 at 06:14:33PM +0200, Arnd Bergmann wrote:

quoted

On Tuesday 18 June 2013, Ezequiel Garcia wrote:

quoted

+Required properties:
+
+- compatible:	 Should be set to one of the following:
+		 marvell,armada370-mbus
+		 marvell,armadaxp-mbus
+
+- reg:		 Device's register space.
+		 Two entries are expected, see the examples below.
+		 The first one controls the devices decoding window and
+		 the second one controls the SDRAM decoding window.
+
+- address-cells: Must be '2'. The first cell for the MBus ID encoding,
+                 the second cell for the address offset within the window.
+
+- size-cells:    Must be '1'.
+
+- ranges:        Must be set up to provide a proper translation for each child.
+	         See the examples below.

You should explain here what the policy is regarding windows set up by the
boot loader. Are the ranges in here required to be the ones that the boot
loader has preconfigured, or are they the ones that the mbus driver is supposed
to set up?

Actually, I might be confused but the kernel MBus driver is completely
independent from the bootloader's -except in the internal register case,
(because we don't touch that from where the bootloader left it).

Right, then state that clearly in the binding.

quoted

What happens is that any decoding window that was setup by the bootloader,
is wiped and completely new windows are allocated using the translations
in the DT, as described by this binding.

This was the case from the start with the old MBus driver. FWIW, I think
it's actually the best choice that can be made: it makes the kernel
independent of the previous setting.

I know you've suggested differently in the past, but I'm not sure I
understand what's the benefit in keeping the bootloaders configuration.

The device tree /normally/ describes things that are either wired up
in hardware or set up by the boot loader. Describing things that the
boot loader may or may not have set up and that the kernel should
set up but may ignore if it wants to is a bit fishy, but it seems
that you have decided to do it that way. You should definitely
document the fact that all ranges except the "internal-regs" are just
suggestions and cannot be relied on to be present at boot time.

Hold on! I've just noticed this, and I want to clarify something, just
to avoid mis-interpretations. The binding is not saying "the windows
described through this ranges are present at boot time".

Rather, it is "this binding will guarantee that the windows described 
in it will be present after the mbus allocates them".

Does it sound too fishy?
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com