[U-Boot] [PATCH V3 03/12] video:exynos_fb:fdt: add additional fdt data

[Ajay kumar]

Hi Piotr,
Find my comments inline.


On Tue, Feb 25, 2014 at 11:33 PM, Piotr Wilczek <p.wilczek at samsung.com>wrote:

> This patch adds additional data parsing from DTB and adds the new
> exynos_lcd_panel_init() function for panel specific initialisation
> from the board file.
>
> Signed-off-by: Piotr Wilczek <p.wilczek at samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
> Cc: Minkyu Kang <mk7.kang at samsung.com>
> ---
> Changes for v3:
>  - none
>
> Changes for v2:
>  - removed duplicate DTB node parsing for panel_info.logo_on
>  - added (weak) exynos_lcd_panel_init function for panel specific
>         initialisation from board file
>
>  drivers/video/exynos_fb.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/drivers/video/exynos_fb.c b/drivers/video/exynos_fb.c
> index 00a0a11..88d9037 100644
> --- a/drivers/video/exynos_fb.c
> +++ b/drivers/video/exynos_fb.c
> @@ -104,6 +104,13 @@ void __exynos_backlight_reset(void)
>  void exynos_backlight_reset(void)
>         __attribute__((weak, alias("__exynos_backlight_reset")));
>
> +int __exynos_lcd_panel_init(vidinfo_t *vid)
> +{
> +       return 0;
> +}
> +int exynos_lcd_panel_init(vidinfo_t *vid)
> +       __attribute__((weak, alias("__exynos_lcd_panel_init")));
> +
>
This is redundant! We already have exynos_cfg_lcd_gpio, exynos_lcd_power_on
and other similar functions to support "panel init".
Please check board/samsung/smdk5250.c

>  static void lcd_panel_on(vidinfo_t *vid)
>  {
>         udelay(vid->init_delay);
> @@ -269,6 +276,15 @@ int exynos_fimd_parse_dt(const void *blob)
>         panel_info.dual_lcd_enabled = fdtdec_get_int(blob, node,
>
> "samsung,dual-lcd-enabled", 0);
>
> +       panel_info.resolution = fdtdec_get_int(blob, node,
> +                                               "samsung,resolution", 0);
> +
> +       panel_info.rgb_mode = fdtdec_get_int(blob, node,
> +                                               "samsung,rgb-mode", 0);
> +
> +       panel_info.power_on_delay = fdtdec_get_int(blob, node,
> +                                               "samsung,power-on-delay",
> 0);
> +
>
All the above DT properties are already present in the same file!
This are definitely duplicate entries.
For passing resolution, please use "samsung,vl-col" and "samsung,vl-row"

>         return 0;
>  }
>  #endif
> @@ -281,10 +297,15 @@ void lcd_ctrl_init(void *lcdbase)
>  #ifdef CONFIG_OF_CONTROL
>         if (exynos_fimd_parse_dt(gd->fdt_blob))
>                 debug("Can't get proper panel info\n");
> +#ifdef CONFIG_EXYNOS_MIPI_DSIM
> +       exynos_init_dsim_platform_data(&panel_info);
> +#endif
> +       exynos_lcd_panel_init(&panel_info);
>
This is already present as part of lcd_enable in same file!
Please check it.

>  #else
>         /* initialize parameters which is specific to panel. */
>         init_panel_info(&panel_info);
>  #endif
> +
>         panel_width = panel_info.vl_width;
>         panel_height = panel_info.vl_height;
>
> --
> 1.8.3.2
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>


Regards,
Ajay Kumar