Re: [PATCHv3 2/4] arm: dts: Add a system manager compatible property

[Mark Rutland <mark.rutland@arm.com>]

On Wed, Dec 04, 2013 at 10:52:54PM +0000, dinguyen@altera.com wrote:
> From: Dinh Nguyen <dinguyen@altera.com>
> 
> The "altr,sysmgr-sdmmc-sdr" compatible property is used for the SOCFPGA
> clk-sysmgr driver. This property represents the register inside the
> system manager that controls the clock phase of the SD/MMC driver.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
> ---
> v3: Cannot use the syscon driver along with the clock because as of v3.13-rc1,
> the syscon driver is loaded after the clocks.
> v2: Add syscon
> ---
>  .../bindings/arm/altera/socfpga-system.txt         |   10 ++++++++++
>  arch/arm/boot/dts/socfpga.dtsi                     |   14 +++++++++++---
>  2 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-system.txt b/Documentation/devicetree/bindings/arm/altera/socfpga-system.txt
> index f4d04a0..7a6c7ed 100644
> --- a/Documentation/devicetree/bindings/arm/altera/socfpga-system.txt
> +++ b/Documentation/devicetree/bindings/arm/altera/socfpga-system.txt
> @@ -5,9 +5,19 @@ Required properties:
>  - reg : Should contain 1 register ranges(address and length)
>  - cpu1-start-addr : CPU1 start address in hex.
>  
> +Optional properties:
> +- compatible = "altr,sysmgr-sdmmc-sdr". This compatible property is used
> +to represent the clock phase settings for the SD/MMC IP.
> +

This makes no sense with the example below. This is _not_ an optional
property of the sysmgr node, this is a poor description of a child node.

>  Example:
>  	 sysmgr@ffd08000 {
>  		compatible = "altr,sys-mgr";
>  		reg = <0xffd08000 0x1000>;
>  		cpu1-start-addr = <0xffd080c4>;
> +
> +		sysmgr_sdr_mmc: sysmgr_sdr_mmc {
> +			#clock-cells = <0>;
> +			compatible = "altr,sysmgr-sdmmc-sdr";
> +			reg = <0x108 1>;

What's this reg?

Is # clock-cells required?

Neither were described in the binding.

Mark.