Subject: Re: [RFC][PATCH] MPC832x_RDB: update dts to use spi, register mmc_spi stub

From: Segher Boessenkool <[email-masked]>
Date: Wed, 1 Aug 2007 00:06:46 +0200

>>>  		spi@4c0 {
>>>  			device_type = "spi";
>>> +			device-id = <1>;
>>
>> Can we just use the reg value for bus_num in the kernel.
>
> Sure, technically nothing prevents this. But, QE specs names
> SPIs by these ids.

As a minimum the property name should start with "fsl," then.

> Plus, from the kernel side spi name will be
> not pretty, it will be spi1216.1.

What, the kernel cannot implement a counter itself?

>>> +			max-chipselect = <1>;
>>
>> I'm not sure how I feel about this in here, I'm thinking it should go.
>
> It's board-specific, i.e. how much chips connected to this SPI bus.

It is misnamed then.  It should be automatically derived from
the child nodes, though.

>>> +			mmc@01 {

@01 should be @1.  Except that it is wrong, since there is
no "reg" property.

>>> +				device_type = "mmc";

No device_type please.

>>> +				compatible = "mmc-spi";

Needs to be more specific.

>>> +				device-id = <1>;

Get rid of this.

>>> +				max-speed-hz = <bebc20>; /* 12500000 Hz */

Just max-speed.

>>> +				chip-select = <0>;

This should be named "reg".  And the parent needs #address-cells
and #size-cells properties.

>>> +				pio-handle = <&mmc1pio>;

What is this for?

>> we should do this in board code and not the device tree.
>
> Well, I've done this initially. But Vitaly hinted that this could
> be done in the DT instead, which made sense to me - mmc is the child
> device of SPI bus. Why do you think it shouldn't be in the DT? I'm
> not arguing, just want understand this.

The hardware should be described in the device tree.  This isn't
the same as simply copying all your Linux code into it ;-)

Segher