On Fri, Feb 1, 2013 at 6:59 AM, Vikas Sajjan vikas.sajjan@linaro.org wrote:

This patch adds display-timing node parsing using video helper function

Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Signed-off-by: Vikas Sajjan vikas.sajjan@linaro.org

---

drivers/gpu/drm/exynos/exynos_drm_fimd.c | 39 +++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index bf0d9ba..8eee13f 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -19,6 +19,7 @@ #include <linux/clk.h> #include <linux/of_device.h> #include <linux/pm_runtime.h> +#include <linux/pinctrl/consumer.h>

#include <video/samsung_fimd.h> #include <drm/exynos_drm.h> @@ -905,16 +906,46 @@ static int __devinit fimd_probe(struct platform_device *pdev) struct exynos_drm_subdrv *subdrv; struct exynos_drm_fimd_pdata *pdata; struct exynos_drm_panel_info *panel;

•   struct fb_videomode *fbmode;
  

•   struct pinctrl *pctrl;
    struct resource *res;
    int win;
    int ret = -EINVAL;
  
    DRM_DEBUG_KMS("%s\n", __FILE__);
  
  

•   pdata = pdev->dev.platform_data;
  

•   if (!pdata) {
  

•           dev_err(dev, "no platform data specified\n");
  

•           return -EINVAL;
  

•   if (pdev->dev.of_node) {
  

•           pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
  

•           if (!pdata) {
  

•                   DRM_ERROR("memory allocation for pdata failed\n");
  

•                   return -ENOMEM;
  

•           }
  

•           fbmode = devm_kzalloc(dev, sizeof(*fbmode), GFP_KERNEL);
  

•           if (!fbmode) {
  

•                   DRM_ERROR("memory allocation for fbmode failed\n");
  

•                   return -ENOMEM;
  

•           }
  

•           ret = of_get_fb_videomode(dev->of_node, fbmode, -1);
  

•           if (ret) {
  

•                   DRM_ERROR("failed to get fb_videomode\n");
  

•                   return -EINVAL;
  

This should probably return ret instead of -EINVAL. It would also be useful to print ret in the error msg.

•           }
  

•           pdata->panel.timing = (struct fb_videomode) *fbmode;
  

•           pctrl = devm_pinctrl_get_select_default(dev);
  

•           if (IS_ERR(pctrl)) {
  

•                   DRM_ERROR("no pinctrl data provided.\n");
  

•                   return -EINVAL;
  

I think the error message here is misleading. If there's no pinctrl provided, doesn't get_select_default return NULL? In which case, this error would never be printed?

I think this behavior is actually desired since there is no pinctrl for exynos5 and we don't want it to break. However I think your error should be more along the lines of "pinctrl_get_select_default failed". You should also print and return PTR_RET(pctrl) instead of EINVAL.

Sean

•           }
  

•   } else {
  

•           pdata = pdev->dev.platform_data;
  

•           if (!pdata) {
  

•                   DRM_ERROR("no platform data specified\n");
  

•                   return -EINVAL;
  

•           }
    }
  
    panel = &pdata->panel;
  

-- 1.7.9.5

---

dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel