Re: [PATCH v7 3/3] [POWERPC] MPC832x_RDB: update dts to use SPI1in QE, register mmc_spi stub

[Segher Boessenkool <segher@kernel.crashing.org>]

>> Not at all.  The device tree describe how the hardware _is_
>> set up (after firmware, bootloader etc.); now how it _should
>> be_ set up by the kernel.
>
> I agree with this general sentiment, but in the case of QE pin 
> configuration, then device tree, in a sense, does contain how the 
> hardware is set up.  The par_io section in the device tree describes 
> they layout of the wiring between the SOC and peripherals.  If the 
> par_io registers are not programmed correctly, the SOC won't be able 
> to communicate with the peripheral.
>
> Sure, the kernel currently reads the device tree and programs the 
> par_io registers accordingly, but that doesn't mean the information 
> *shouldn't* be in the device tree.

It is fine if the device tree describes how things are wired together;
that's part of the device tree's purpose, after all.

It is not fine if the device tree says _how to_ configure the hardware;
you are supposed to put your device drivers elsewhere, not in the
device tree!

>> It would make a lot of sense to do this work in the firmware
>> instead, but it doesn't make sense at all to put this stuff
>> into the device tree.
>
> 1) If the firmware does configure the pins, then the device tree 
> *will* describe how the hardware is set up.

Yes.  It won't show pario register contents though.

> 2) How would the firmware know how to do board configuration if it 
> doesn't have the instructions in the device tree?

This kind of thing is typically hardcoded into the firmware (just like
the device tree is) -- just put it somewhere _next to_ the device tree,
not _in_ it.  The device tree is not a config file for the firmware; it
is an interface between the firmware and its client (an OS, typically).

> Besides, every other board does it's par_io configuration based on the 
> device tree.  So if Anton is going to break that pattern, we should be 
> talking about moving all that code into U-boot, instead of just 
> putting in a one-time exception (especially since the patch contains 
> no explanation as to why these par_io pins are being configured 
> differently than every other board).

Yes, I definitely didn't mean to single out Anton's patch, sorry if it
sounded that way.


Segher