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

[Dinh Nguyen <dinguyen@altera.com>]

On Fri, 2013-07-26 at 14:02 -0600, Stephen Warren wrote:
> 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.

I'll merge it.

> 
> > dwmmc0@ff704000 {
> > 	compatible = "altr,socfpga-dw-mshc";
> 
> That should include the baseline synopsis compatible value too.

We don't need the baseline synopsis compatible because of
dw_mci_pltfm_register() call.

Thanks,
Dinh
> 
> > 	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>;
> > 		};
> > 	};
> 
>