Hi Vikas,

Please see my comments inline.

On Monday 29 of July 2013 19:19:28 Vikas Sajjan wrote:
> Adds FIMD DT node to exynos5420 based SMDK. Also adds display-timimg
> information node.
> 
> Signed-off-by: Vikas Sajjan <[email protected]>
> ---
>  arch/arm/boot/dts/exynos5420-smdk5420.dts |   18 ++++++++++++++++++
>  arch/arm/boot/dts/exynos5420.dtsi         |    8 ++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts
> b/arch/arm/boot/dts/exynos5420-smdk5420.dts index 08607df..7c2f477 100644
> --- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
> +++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
> @@ -30,4 +30,22 @@
>                       clock-frequency = <24000000>;
>               };
>       };
> +
> +     fimd {
> +             display-timings {
> +                     native-mode = <&timing0>;
> +                     timing0: timing@0 {
> +                             clock-frequency = <50000>;
> +                             hactive = <2560>;
> +                             vactive = <1600>;
> +                             hfront-porch = <48>;
> +                             hback-porch = <80>;
> +                             hsync-len = <32>;
> +                             vback-porch = <16>;
> +                             vfront-porch = <8>;
> +                             vsync-len = <6>;
> +                     };
> +             };
> +     };
> +
>  };
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi
> b/arch/arm/boot/dts/exynos5420.dtsi index bd6b310..1f659f4 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -180,4 +180,12 @@
>               clocks = <&clock 260>, <&clock 131>;
>               clock-names = "uart", "clk_uart_baud0";
>       };
> +
> +     fimd {
> +             samsung,power-domain = <&disp_pd>;
> +             clocks = <&clock 147>, <&clock 421>;
> +             clock-names = "sclk_fimd", "fimd";
> +             status = "okay";

FIMD can't operate without display timings, so status = "okay" doesn't 
represent real device state here. Please move status override to board dts.

I know that dtsi files of Exynos 5 SoCs have "okay" status as default, but this 
is broken and needs to be fixed, because it moves the responsibility of knowing 
about all SoC hardware to board dts.

Generally, the ePAPR document, chapter 2.3.4, specifies following definition of 
status property:

        The status property indicates the operational status of a device. Valid 
        values are listed and defined in the following table.

The table specifies following values:

        “okay”
        Indicates the device is operational

        “disabled”
        Indicates that the device is not presently operational, but it might 
become 
        operational in the future (for example, something is not plugged in, or 
                
        switched off). Refer to the device binding for details on what disabled 
        means for a given device.

        “fail”
        Indicates that the device is not operational. A serious error was 
detected 
        in the device, and it is unlikely to become operational without repair.

        “fail-sss”
        Indicates that the device is not operational. A serious error was 
detected      
        in the device and it is unlikely to become operational without repair. 
The 
        sss portion of the value is specific to the device and indicates the 
error 
        condition detected.

Without all required properties the device will not be operational, so its 
status should be set to "disabled" and then overriden to "okay" when missing 
properties are provided (e.g. in board dts).

Best regards,
Tomasz

> +     };
> +
>  };
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to