Message ID | 1360910587-25548-2-git-send-email-vikas.sajjan@linaro.org (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from <linux-media-owner@vger.kernel.org>) id 1U6F1F-0000Is-CQ; Fri, 15 Feb 2013 07:43:37 +0100 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.75/mailfrontend-4) with esmtp id 1U6F1E-0000AZ-Bw; Fri, 15 Feb 2013 07:43:37 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161125Ab3BOGnY (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Fri, 15 Feb 2013 01:43:24 -0500 Received: from mail-pa0-f54.google.com ([209.85.220.54]:41789 "EHLO mail-pa0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1161118Ab3BOGnW (ORCPT <rfc822;linux-media@vger.kernel.org>); Fri, 15 Feb 2013 01:43:22 -0500 Received: by mail-pa0-f54.google.com with SMTP id fa10so1639222pad.27 for <linux-media@vger.kernel.org>; Thu, 14 Feb 2013 22:43:22 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references:x-gm-message-state; bh=Zrj2R/oTJwHUy8d1iT83Ds9R5KYLKRe57P6Gd6DEOXk=; b=J67NxSXU4alNlveR8EHh7OLOjyPwcxicA3eUkOyy772Xdnb2WjAqcR/nRlwCyN3y5+ 7UN0fnHQQF/8AHm+G0nME/vaOOVg5jck5/1e+npqLpUrc4JMuQSL1SyGw5JZ3hteVPe/ Fjuk3fE7TJTyCHV5zWSJaliREebz0EOSEZEKtCLZvcmW9T03SztPfyUrbpRUd4vbG03F D0Snh1sBjq6nG8zs8BoRtjbuUYQIRaZoYQ8GsuO1UKxF9aOQsUq6x4RCQPs4FIicFfyA ydwJzT9IpGHvdnTbYgNLYpVSXQcjVGx7FB04RJj+rEk7tjOmMingU/1vpPx7KRfBNsDz WUOw== X-Received: by 10.66.81.199 with SMTP id c7mr10552577pay.39.1360910602124; Thu, 14 Feb 2013 22:43:22 -0800 (PST) Received: from localhost.localdomain ([115.113.119.130]) by mx.google.com with ESMTPS id kc15sm3776317pbb.5.2013.02.14.22.43.18 (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 14 Feb 2013 22:43:21 -0800 (PST) From: Vikas Sajjan <vikas.sajjan@linaro.org> To: dri-devel@lists.freedesktop.org Cc: linux-media@vger.kernel.org, kgene.kim@samsung.com, inki.dae@samsung.com, l.krishna@samsung.com, patches@linaro.org Subject: [PATCH v6 1/1] video: drm: exynos: Add display-timing node parsing using video helper function Date: Fri, 15 Feb 2013 12:13:07 +0530 Message-Id: <1360910587-25548-2-git-send-email-vikas.sajjan@linaro.org> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1360910587-25548-1-git-send-email-vikas.sajjan@linaro.org> References: <1360910587-25548-1-git-send-email-vikas.sajjan@linaro.org> X-Gm-Message-State: ALoCoQlcnKvwc1ICIeMmEAfSor3ZShlr1vXfO2MyiECaIGNAfG1DOlJktPPggXtoXl6/dv2FSSXq Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 5.6.1.2065439, Antispam-Engine: 2.7.2.376379, Antispam-Data: 2013.2.15.63626 X-PMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_2000_2999 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __CP_MEDIA_BODY 0, __CP_URI_IN_BODY 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __IN_REP_TO 0, __MIME_TEXT_ONLY 0, __MULTIPLE_RCPTS_CC_X2 0, __SANE_MSGID 0, __SUBJ_ALPHA_END 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_NO_WWW 0, __URI_NS , __YOUTUBE_RCVD 0' |
Commit Message
Vikas Sajjan
Feb. 15, 2013, 6:43 a.m. UTC
Add support for parsing the display-timing node using video helper function. The DT node parsing and pinctrl selection is done only if 'dev.of_node' exists and the NON-DT logic is still maintained under the 'else' part. 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 | 37 ++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-)
Comments
> -----Original Message----- > From: Vikas Sajjan [mailto:vikas.sajjan@linaro.org] > Sent: Friday, February 15, 2013 3:43 PM > To: dri-devel@lists.freedesktop.org > Cc: linux-media@vger.kernel.org; kgene.kim@samsung.com; > inki.dae@samsung.com; l.krishna@samsung.com; patches@linaro.org > Subject: [PATCH v6 1/1] video: drm: exynos: Add display-timing node > parsing using video helper function > > Add support for parsing the display-timing node using video helper > function. > > The DT node parsing and pinctrl selection is done only if 'dev.of_node' > exists and the NON-DT logic is still maintained under the 'else' part. > > 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 | 37 > ++++++++++++++++++++++++++---- > 1 file changed, 33 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > index 9537761..8b2c0ff 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > @@ -19,7 +19,9 @@ > #include <linux/clk.h> > #include <linux/of_device.h> > #include <linux/pm_runtime.h> > +#include <linux/pinctrl/consumer.h> > > +#include <video/of_display_timing.h> > #include <video/samsung_fimd.h> > #include <drm/exynos_drm.h> > > @@ -877,16 +879,43 @@ static int 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 = &pdata->panel.timing; > + ret = of_get_fb_videomode(dev->of_node, fbmode, > + OF_USE_NATIVE_MODE); > + if (ret) { > + DRM_ERROR("failed: of_get_fb_videomode()\n" > + "with return value: %d\n", ret); > + return ret; > + } > + > + pctrl = devm_pinctrl_get_select_default(dev); Why does it need pinctrl? and even though needed, I think this should be separated into another one. Thanks, Inki Dae > + if (IS_ERR_OR_NULL(pctrl)) { > + DRM_ERROR("failed: > devm_pinctrl_get_select_default()\n" > + "with return value: %d\n", PTR_RET(pctrl)); > + return PTR_RET(pctrl); > + } > + > + } else { > + pdata = pdev->dev.platform_data; > + if (!pdata) { > + DRM_ERROR("no platform data specified\n"); > + return -EINVAL; > + } > } > > panel = &pdata->panel; > -- > 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Mr. Inki Dae, On 20 February 2013 16:45, Inki Dae <inki.dae@samsung.com> wrote: > > >> -----Original Message----- >> From: Vikas Sajjan [mailto:vikas.sajjan@linaro.org] >> Sent: Friday, February 15, 2013 3:43 PM >> To: dri-devel@lists.freedesktop.org >> Cc: linux-media@vger.kernel.org; kgene.kim@samsung.com; >> inki.dae@samsung.com; l.krishna@samsung.com; patches@linaro.org >> Subject: [PATCH v6 1/1] video: drm: exynos: Add display-timing node >> parsing using video helper function >> >> Add support for parsing the display-timing node using video helper >> function. >> >> The DT node parsing and pinctrl selection is done only if 'dev.of_node' >> exists and the NON-DT logic is still maintained under the 'else' part. >> >> 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 | 37 >> ++++++++++++++++++++++++++---- >> 1 file changed, 33 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> index 9537761..8b2c0ff 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> @@ -19,7 +19,9 @@ >> #include <linux/clk.h> >> #include <linux/of_device.h> >> #include <linux/pm_runtime.h> >> +#include <linux/pinctrl/consumer.h> >> >> +#include <video/of_display_timing.h> >> #include <video/samsung_fimd.h> >> #include <drm/exynos_drm.h> >> >> @@ -877,16 +879,43 @@ static int 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 = &pdata->panel.timing; >> + ret = of_get_fb_videomode(dev->of_node, fbmode, >> + OF_USE_NATIVE_MODE); >> + if (ret) { >> + DRM_ERROR("failed: of_get_fb_videomode()\n" >> + "with return value: %d\n", ret); >> + return ret; >> + } >> + >> + pctrl = devm_pinctrl_get_select_default(dev); > > Why does it need pinctrl? and even though needed, I think this should be > separated into another one. > Will separate it out and send it in a separate patch. > Thanks, > Inki Dae > >> + if (IS_ERR_OR_NULL(pctrl)) { >> + DRM_ERROR("failed: >> devm_pinctrl_get_select_default()\n" >> + "with return value: %d\n", PTR_RET(pctrl)); >> + return PTR_RET(pctrl); >> + } >> + >> + } else { >> + pdata = pdev->dev.platform_data; >> + if (!pdata) { >> + DRM_ERROR("no platform data specified\n"); >> + return -EINVAL; >> + } >> } >> >> panel = &pdata->panel; >> -- >> 1.7.9.5 >
Hi, On 02/15/2013 03:43 PM, Vikas Sajjan wrote: > Add support for parsing the display-timing node using video helper > function. > > The DT node parsing and pinctrl selection is done only if 'dev.of_node' > exists and the NON-DT logic is still maintained under the 'else' part. > > 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 | 37 ++++++++++++++++++++++++++---- > 1 file changed, 33 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > index 9537761..8b2c0ff 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > @@ -19,7 +19,9 @@ > #include <linux/clk.h> > #include <linux/of_device.h> > #include <linux/pm_runtime.h> > +#include <linux/pinctrl/consumer.h> > > +#include <video/of_display_timing.h> > #include <video/samsung_fimd.h> > #include <drm/exynos_drm.h> > > @@ -877,16 +879,43 @@ static int 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 = &pdata->panel.timing; > + ret = of_get_fb_videomode(dev->of_node, fbmode, > + OF_USE_NATIVE_MODE); fbmode variable can be substituted to &pdata->panel.timing directly then can remove fbmode variable. > + if (ret) { > + DRM_ERROR("failed: of_get_fb_videomode()\n" > + "with return value: %d\n", ret); > + return ret; > + } > + > + pctrl = devm_pinctrl_get_select_default(dev); > + if (IS_ERR_OR_NULL(pctrl)) { It's enough to "if (IS_ERR(pctrl)) {". > + DRM_ERROR("failed: devm_pinctrl_get_select_default()\n" > + "with return value: %d\n", PTR_RET(pctrl)); > + return PTR_RET(pctrl); It's enough to "return PTR_ERR(pctrl);" > + } If this needs, parts for pinctrl should go to another patch. > + > + } else { > + pdata = pdev->dev.platform_data; > + if (!pdata) { > + DRM_ERROR("no platform data specified\n"); > + return -EINVAL; > + } > } > > panel = &pdata->panel; Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 21 February 2013 12:25, Joonyoung Shim <jy0922.shim@samsung.com> wrote: > Hi, > > > On 02/15/2013 03:43 PM, Vikas Sajjan wrote: >> >> Add support for parsing the display-timing node using video helper >> function. >> >> The DT node parsing and pinctrl selection is done only if 'dev.of_node' >> exists and the NON-DT logic is still maintained under the 'else' part. >> >> 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 | 37 >> ++++++++++++++++++++++++++---- >> 1 file changed, 33 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> index 9537761..8b2c0ff 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> @@ -19,7 +19,9 @@ >> #include <linux/clk.h> >> #include <linux/of_device.h> >> #include <linux/pm_runtime.h> >> +#include <linux/pinctrl/consumer.h> >> +#include <video/of_display_timing.h> >> #include <video/samsung_fimd.h> >> #include <drm/exynos_drm.h> >> @@ -877,16 +879,43 @@ static int 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 = &pdata->panel.timing; >> + ret = of_get_fb_videomode(dev->of_node, fbmode, >> + OF_USE_NATIVE_MODE); > > > fbmode variable can be substituted to &pdata->panel.timing directly then can > remove fbmode variable. > this is can be done. > >> + if (ret) { >> + DRM_ERROR("failed: of_get_fb_videomode()\n" >> + "with return value: %d\n", ret); >> + return ret; >> + } >> + >> + pctrl = devm_pinctrl_get_select_default(dev); >> + if (IS_ERR_OR_NULL(pctrl)) { > > > It's enough to "if (IS_ERR(pctrl)) {". > what if it returns NULL? > >> + DRM_ERROR("failed: >> devm_pinctrl_get_select_default()\n" >> + "with return value: %d\n", >> PTR_RET(pctrl)); >> + return PTR_RET(pctrl); > > > It's enough to "return PTR_ERR(pctrl);" > ok. >> + } > > > If this needs, parts for pinctrl should go to another patch. > I have it as a separate patch already. [PATCH v7 2/2] video: drm: exynos: Add pinctrl support to fimd > >> + >> + } else { >> + pdata = pdev->dev.platform_data; >> + if (!pdata) { >> + DRM_ERROR("no platform data specified\n"); >> + return -EINVAL; >> + } >> } >> panel = &pdata->panel; > > > Thanks.
On 02/21/2013 04:12 PM, Vikas Sajjan wrote: > Hi, > > On 21 February 2013 12:25, Joonyoung Shim <jy0922.shim@samsung.com> wrote: >> Hi, >> >> >> On 02/15/2013 03:43 PM, Vikas Sajjan wrote: >>> Add support for parsing the display-timing node using video helper >>> function. >>> >>> The DT node parsing and pinctrl selection is done only if 'dev.of_node' >>> exists and the NON-DT logic is still maintained under the 'else' part. >>> >>> 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 | 37 >>> ++++++++++++++++++++++++++---- >>> 1 file changed, 33 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>> index 9537761..8b2c0ff 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>> @@ -19,7 +19,9 @@ >>> #include <linux/clk.h> >>> #include <linux/of_device.h> >>> #include <linux/pm_runtime.h> >>> +#include <linux/pinctrl/consumer.h> >>> +#include <video/of_display_timing.h> >>> #include <video/samsung_fimd.h> >>> #include <drm/exynos_drm.h> >>> @@ -877,16 +879,43 @@ static int 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 = &pdata->panel.timing; >>> + ret = of_get_fb_videomode(dev->of_node, fbmode, >>> + OF_USE_NATIVE_MODE); >> >> fbmode variable can be substituted to &pdata->panel.timing directly then can >> remove fbmode variable. >> > this is can be done. >>> + if (ret) { >>> + DRM_ERROR("failed: of_get_fb_videomode()\n" >>> + "with return value: %d\n", ret); >>> + return ret; >>> + } >>> + >>> + pctrl = devm_pinctrl_get_select_default(dev); >>> + if (IS_ERR_OR_NULL(pctrl)) { >> >> It's enough to "if (IS_ERR(pctrl)) {". >> > what if it returns NULL? devm_pinctrl_get_select_default() never return NULL. >>> + DRM_ERROR("failed: >>> devm_pinctrl_get_select_default()\n" >>> + "with return value: %d\n", >>> PTR_RET(pctrl)); >>> + return PTR_RET(pctrl); >> >> It's enough to "return PTR_ERR(pctrl);" >> > ok. > >>> + } >> >> If this needs, parts for pinctrl should go to another patch. >> > I have it as a separate patch already. [PATCH v7 2/2] video: drm: > exynos: Add pinctrl support to fimd >>> + >>> + } else { >>> + pdata = pdev->dev.platform_data; >>> + if (!pdata) { >>> + DRM_ERROR("no platform data specified\n"); >>> + return -EINVAL; >>> + } >>> } >>> panel = &pdata->panel; >> >> Thanks. > > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 02/21/2013 04:18 PM, Joonyoung Shim wrote: > On 02/21/2013 04:12 PM, Vikas Sajjan wrote: >> Hi, >> >> On 21 February 2013 12:25, Joonyoung Shim <jy0922.shim@samsung.com> >> wrote: >>> Hi, >>> >>> >>> On 02/15/2013 03:43 PM, Vikas Sajjan wrote: >>>> Add support for parsing the display-timing node using video helper >>>> function. >>>> >>>> The DT node parsing and pinctrl selection is done only if >>>> 'dev.of_node' >>>> exists and the NON-DT logic is still maintained under the 'else' part. >>>> >>>> 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 | 37 >>>> ++++++++++++++++++++++++++---- >>>> 1 file changed, 33 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>>> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>>> index 9537761..8b2c0ff 100644 >>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>>> @@ -19,7 +19,9 @@ >>>> #include <linux/clk.h> >>>> #include <linux/of_device.h> >>>> #include <linux/pm_runtime.h> >>>> +#include <linux/pinctrl/consumer.h> >>>> +#include <video/of_display_timing.h> >>>> #include <video/samsung_fimd.h> >>>> #include <drm/exynos_drm.h> >>>> @@ -877,16 +879,43 @@ static int 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 = &pdata->panel.timing; >>>> + ret = of_get_fb_videomode(dev->of_node, fbmode, >>>> + OF_USE_NATIVE_MODE); >>> >>> fbmode variable can be substituted to &pdata->panel.timing directly >>> then can >>> remove fbmode variable. >>> >> this is can be done. >>>> + if (ret) { >>>> + DRM_ERROR("failed: of_get_fb_videomode()\n" >>>> + "with return value: %d\n", ret); >>>> + return ret; >>>> + } >>>> + >>>> + pctrl = devm_pinctrl_get_select_default(dev); >>>> + if (IS_ERR_OR_NULL(pctrl)) { >>> >>> It's enough to "if (IS_ERR(pctrl)) {". >>> >> what if it returns NULL? > > devm_pinctrl_get_select_default() never return NULL. > My mistake. If CONFIG_PINCTRL is disabled, devm_pinctrl_get_select_default can return NULL. Linus, devm_pinctrl_get_select() and pinctrl_get_select() also need IS_ERR_OR_NULL instead of IS_ERR? And many drivers using above functions don't check NULL, right? Thanks. > >>>> + DRM_ERROR("failed: >>>> devm_pinctrl_get_select_default()\n" >>>> + "with return value: %d\n", >>>> PTR_RET(pctrl)); >>>> + return PTR_RET(pctrl); >>> >>> It's enough to "return PTR_ERR(pctrl);" >>> >> ok. >> >>>> + } >>> >>> If this needs, parts for pinctrl should go to another patch. >>> >> I have it as a separate patch already. [PATCH v7 2/2] video: drm: >> exynos: Add pinctrl support to fimd >>>> + >>>> + } else { >>>> + pdata = pdev->dev.platform_data; >>>> + if (!pdata) { >>>> + DRM_ERROR("no platform data specified\n"); >>>> + return -EINVAL; >>>> + } >>>> } >>>> panel = &pdata->panel; >>> >>> Thanks. >> >> > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Feb 28, 2013 at 2:51 AM, Joonyoung Shim <jy0922.shim@samsung.com> wrote: > My mistake. If CONFIG_PINCTRL is disabled, devm_pinctrl_get_select_default > can return NULL. Yes, and that is perfectly legal and *NOT* an error. > devm_pinctrl_get_select() and pinctrl_get_select() also need IS_ERR_OR_NULL > instead of IS_ERR? No. In fact, IS_ERR_OR_NULL() shall not be used at all. Check the LKML mailinglist for Russells recent struggle to purge it from the kernel. > And many drivers using above functions don't check NULL, right? No, and they should not. Stub pinctrl handles, just like stub clocks and stub regulators, are perfectly legal, just that they have no effect. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 9537761..8b2c0ff 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -19,7 +19,9 @@ #include <linux/clk.h> #include <linux/of_device.h> #include <linux/pm_runtime.h> +#include <linux/pinctrl/consumer.h> +#include <video/of_display_timing.h> #include <video/samsung_fimd.h> #include <drm/exynos_drm.h> @@ -877,16 +879,43 @@ static int 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 = &pdata->panel.timing; + ret = of_get_fb_videomode(dev->of_node, fbmode, + OF_USE_NATIVE_MODE); + if (ret) { + DRM_ERROR("failed: of_get_fb_videomode()\n" + "with return value: %d\n", ret); + return ret; + } + + pctrl = devm_pinctrl_get_select_default(dev); + if (IS_ERR_OR_NULL(pctrl)) { + DRM_ERROR("failed: devm_pinctrl_get_select_default()\n" + "with return value: %d\n", PTR_RET(pctrl)); + return PTR_RET(pctrl); + } + + } else { + pdata = pdev->dev.platform_data; + if (!pdata) { + DRM_ERROR("no platform data specified\n"); + return -EINVAL; + } } panel = &pdata->panel;