Re: [PATCH v6 1/1] video: drm: exynos: Add display-timing node parsing using video helper function
On Thu, Feb 28, 2013 at 2:51 AM, Joonyoung Shim jy0922.s...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/1] video: drm: exynos: Add display-timing node parsing using video helper function
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.s...@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.kris...@samsung.com Signed-off-by: Vikas Sajjan vikas.saj...@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-de...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v6 1/1] video: drm: exynos: Add display-timing node parsing using video helper function
-Original Message- From: Vikas Sajjan [mailto:vikas.saj...@linaro.org] Sent: Friday, February 15, 2013 3:43 PM To: dri-de...@lists.freedesktop.org Cc: linux-media@vger.kernel.org; kgene@samsung.com; inki@samsung.com; l.kris...@samsung.com; patc...@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.kris...@samsung.com Signed-off-by: Vikas Sajjan vikas.saj...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/1] video: drm: exynos: Add display-timing node parsing using video helper function
Hi Mr. Inki Dae, On 20 February 2013 16:45, Inki Dae inki@samsung.com wrote: -Original Message- From: Vikas Sajjan [mailto:vikas.saj...@linaro.org] Sent: Friday, February 15, 2013 3:43 PM To: dri-de...@lists.freedesktop.org Cc: linux-media@vger.kernel.org; kgene@samsung.com; inki@samsung.com; l.kris...@samsung.com; patc...@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.kris...@samsung.com Signed-off-by: Vikas Sajjan vikas.saj...@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 -- Thanks and Regards Vikas Sajjan -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/1] video: drm: exynos: Add display-timing node parsing using video helper function
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.kris...@samsung.com Signed-off-by: Vikas Sajjan vikas.saj...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/1] video: drm: exynos: Add display-timing node parsing using video helper function
Hi, On 21 February 2013 12:25, Joonyoung Shim jy0922.s...@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.kris...@samsung.com Signed-off-by: Vikas Sajjan vikas.saj...@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. -- Thanks and Regards Vikas Sajjan -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/1] video: drm: exynos: Add display-timing node parsing using video helper function
On 02/21/2013 04:12 PM, Vikas Sajjan wrote: Hi, On 21 February 2013 12:25, Joonyoung Shim jy0922.s...@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.kris...@samsung.com Signed-off-by: Vikas Sajjan vikas.saj...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html