Re: [PATCH] ARM: socfpga: dts: Add support for SD/MMC

Stephen Warren
2013-07-26T20:02:54Z

On 07/26/2013 01:33 PM, Dinh Nguyen wrote:
> On Fri, 2013-07-26 at 11:24 -0600, Stephen Warren wrote:
>> On 07/25/2013 04:04 PM, dinguyen@altera.com wrote:
>>> From: Dinh Nguyen <dinguyen@altera.com>
>>>
>>> Add bindings for SD/MMC for SOCFPGA.
>>> Add "syscon" to the "altr,sys-mgr" binding.

>>> diff --git a/Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt

>>> +Example:
>>> +
>>> + The MSHC controller node can be split into two portions, SoC specific and
>>> + board specific portions, as listed below.
>>
>> That doesn't sound like a good idea. There should be one DT node for
>> each logical block. The internal construction of the Linux drivers
>> (presumably you have entirely separate code to handle the two nodes in
>> Linux so far?) should not influence the DT construction at all.
>
> In the end, there is only 1 DT node for each logical block:

Oh right, I see you were intending to show the distinction between the
SoC .dtsi and board .dts file. I hadn't realized that. I don't think
it's common to do that in the examples, so I would recommend just
merging the whole example together myself.

> dwmmc0@ff704000 {
> compatible = "altr,socfpga-dw-mshc";

That should include the baseline synopsis compatible value too.

> reg = <0xff704000 0x00001000>;
> interrupts = <0x00000000 0x0000008b 0x00000004>;
> fifo-depth = <0x00000400>;
> #address-cells = <0x00000001>;
> #size-cells = <0x00000000>;
> clocks = <0x00000016 0x00000017>;
> clock-names = "biu", "ciu";
> num-slots = <0x00000001>;
> supports-highspeed;
> broken-cd;
> altr,dw-mshc-ciu-div = <0x00000003>;
> altr,dw-mshc-sdr-timing = <0x00000000 0x00000003>;
> slot@0 {
> reg = <0x00000000>;
> bus-width = <0x00000004>;
> };
> };