[PATCH v9 2/2] video: drm: exynos: Add pinctrl support to fimd
Hi Linus, On Fri, Mar 1, 2013 at 2:49 PM, Linus Walleij wrote: > On Thu, Feb 28, 2013 at 5:12 AM, Vikas Sajjan > wrote: > >> Adds support for pinctrl to drm fimd >> >> Signed-off-by: Leela Krishna Amudala >> Signed-off-by: Vikas Sajjan > (...) >> + pctrl = devm_pinctrl_get_select_default(dev); > > NAK. > > The device core will do this for you as of commit > ab78029ecc347debbd737f06688d788bd9d60c1d > "drivers/pinctrl: grab default handles from device core" > OK. Will abandon the patch. > Yours, > Linus Walleij > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 2/2] video: drm: exynos: Add pinctrl support to fimd
On Thu, Feb 28, 2013 at 5:12 AM, Vikas Sajjan wrote: > Adds support for pinctrl to drm fimd > > Signed-off-by: Leela Krishna Amudala > Signed-off-by: Vikas Sajjan (...) > + pctrl = devm_pinctrl_get_select_default(dev); NAK. The device core will do this for you as of commit ab78029ecc347debbd737f06688d788bd9d60c1d "drivers/pinctrl: grab default handles from device core" Yours, Linus Walleij
Re: [PATCH v9 2/2] video: drm: exynos: Add pinctrl support to fimd
On Thu, Feb 28, 2013 at 5:12 AM, Vikas Sajjan vikas.saj...@linaro.org wrote: Adds support for pinctrl to drm fimd Signed-off-by: Leela Krishna Amudala l.kris...@samsung.com Signed-off-by: Vikas Sajjan vikas.saj...@linaro.org (...) + pctrl = devm_pinctrl_get_select_default(dev); NAK. The device core will do this for you as of commit ab78029ecc347debbd737f06688d788bd9d60c1d drivers/pinctrl: grab default handles from device core Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v9 2/2] video: drm: exynos: Add pinctrl support to fimd
Hi Linus, On Fri, Mar 1, 2013 at 2:49 PM, Linus Walleij linus.wall...@linaro.org wrote: On Thu, Feb 28, 2013 at 5:12 AM, Vikas Sajjan vikas.saj...@linaro.org wrote: Adds support for pinctrl to drm fimd Signed-off-by: Leela Krishna Amudala l.kris...@samsung.com Signed-off-by: Vikas Sajjan vikas.saj...@linaro.org (...) + pctrl = devm_pinctrl_get_select_default(dev); NAK. The device core will do this for you as of commit ab78029ecc347debbd737f06688d788bd9d60c1d drivers/pinctrl: grab default handles from device core OK. Will abandon the patch. Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v9 2/2] video: drm: exynos: Add pinctrl support to fimd
On 02/28/2013 05:12 AM, Vikas Sajjan wrote: Adds support for pinctrl to drm fimd Signed-off-by: Leela Krishna Amudalal.kris...@samsung.com Signed-off-by: Vikas Sajjanvikas.saj...@linaro.org --- drivers/gpu/drm/exynos/exynos_drm_fimd.c |9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index e323cf9..21ada8d 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -19,6 +19,7 @@ #includelinux/clk.h #includelinux/of_device.h #includelinux/pm_runtime.h +#includelinux/pinctrl/consumer.h #includevideo/of_display_timing.h #includevideo/samsung_fimd.h @@ -879,6 +880,7 @@ static int fimd_probe(struct platform_device *pdev) struct exynos_drm_fimd_pdata *pdata; struct exynos_drm_panel_info *panel; struct resource *res; + struct pinctrl *pctrl; int win; int ret = -EINVAL; @@ -897,6 +899,13 @@ static int fimd_probe(struct platform_device *pdev) DRM_ERROR(failed: of_get_fb_videomode() : %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(): + %d\n, PTR_RET(pctrl)); + return PTR_ERR(pctrl); In situations like this I really side attempts to remove IS_ERR_OR_NULL() macro from the kernel completely ([1], [2]). What is the value returned from fimd_probe() when devm_pinctrl_get_select_default() returns NULL ? What header file have you added to use struct pinctrl in this driver ? Is this data structure fully declared there ? Are drivers supposed to dereference struct pinctrl at all ? I believe original intention was to have the pinctrl handle as an opaque cookie, and as long as it is used with the pinctrl API only and tested for errors with *IS_ERR()*, everything should be fine. The pinctrl API should handle any NULL pointer as it returned it to a driver in the first place. Please just use IS_ERR(), let's stop this IS_ERR_OR_NULL() insanity. + } + } else { pdata = pdev-dev.platform_data; if (!pdata) { [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/140543.html [2] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg78030.html ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v9 2/2] video: drm: exynos: Add pinctrl support to fimd
On Thu, Feb 28, 2013 at 11:03:57PM +0100, Sylwester Nawrocki wrote: Please just use IS_ERR(), let's stop this IS_ERR_OR_NULL() insanity. Yes, indeed. On that topic (and off-topic for this thread, sorry) I've committed a set of patches to remove most users of IS_ERR_OR_NULL() from arch/arm. These are the last remaining ones there - and I don't want to see any more appearing: arch/arm/plat-samsung/clock.c: if (IS_ERR_OR_NULL(clk)) arch/arm/plat-samsung/clock.c: if (!IS_ERR_OR_NULL(clk) clk-ops clk-ops-round_rate) arch/arm/plat-samsung/clock.c: if (IS_ERR_OR_NULL(clk)) arch/arm/plat-samsung/clock.c: if (IS_ERR_OR_NULL(clk) || IS_ERR_OR_NULL(parent)) arch/arm/mach-imx/devices/platform-ipu-core.c: if (IS_ERR_OR_NULL(imx_ipu_coredev)) arch/arm/mach-imx/devices/platform-ipu-core.c: if (IS_ERR_OR_NULL(imx_ipu_coredev)) arch/arm/kernel/smp_twd.c: * We use IS_ERR_OR_NULL() here, because if the clock stubs arch/arm/kernel/smp_twd.c: if (!IS_ERR_OR_NULL(twd_clk)) They currently all legal uses of it - though I'm sure that the samsung clock uses can be reduced to just IS_ERR(). The IMX use looks valid in that imx_ipu_coredev really can be an error pointer (on failure) or NULL if the platform device hasn't yet been created. The TWD one is explained in the comments in the code (if people had to write explanations for using IS_ERR_OR_NULL(), we'd probably have it used correctly!) ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v9 2/2] video: drm: exynos: Add pinctrl support to fimd
On 02/28/2013 05:12 AM, Vikas Sajjan wrote: > Adds support for pinctrl to drm fimd > > Signed-off-by: Leela Krishna Amudala > Signed-off-by: Vikas Sajjan > --- > drivers/gpu/drm/exynos/exynos_drm_fimd.c |9 + > 1 file changed, 9 insertions(+) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > index e323cf9..21ada8d 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -879,6 +880,7 @@ static int fimd_probe(struct platform_device *pdev) > struct exynos_drm_fimd_pdata *pdata; > struct exynos_drm_panel_info *panel; > struct resource *res; > + struct pinctrl *pctrl; > int win; > int ret = -EINVAL; > > @@ -897,6 +899,13 @@ static int fimd_probe(struct platform_device *pdev) > DRM_ERROR("failed: of_get_fb_videomode() : %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():" > + "%d\n", PTR_RET(pctrl)); > + return PTR_ERR(pctrl); In situations like this I really side attempts to remove IS_ERR_OR_NULL() macro from the kernel completely ([1], [2]). What is the value returned from fimd_probe() when devm_pinctrl_get_select_default() returns NULL ? What header file have you added to use struct pinctrl in this driver ? Is this data structure fully declared there ? Are drivers supposed to dereference struct pinctrl at all ? I believe original intention was to have the pinctrl handle as an opaque cookie, and as long as it is used with the pinctrl API only and tested for errors with *IS_ERR()*, everything should be fine. The pinctrl API should handle any NULL pointer as it returned it to a driver in the first place. Please just use IS_ERR(), let's stop this IS_ERR_OR_NULL() insanity. > + } > + > } else { > pdata = pdev->dev.platform_data; > if (!pdata) { [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/140543.html [2] http://www.mail-archive.com/linux-omap at vger.kernel.org/msg78030.html
[PATCH v9 2/2] video: drm: exynos: Add pinctrl support to fimd
On Thu, Feb 28, 2013 at 11:03:57PM +0100, Sylwester Nawrocki wrote: > Please just use IS_ERR(), let's stop this IS_ERR_OR_NULL() insanity. Yes, indeed. On that topic (and off-topic for this thread, sorry) I've committed a set of patches to remove most users of IS_ERR_OR_NULL() from arch/arm. These are the last remaining ones there - and I don't want to see any more appearing: arch/arm/plat-samsung/clock.c: if (IS_ERR_OR_NULL(clk)) arch/arm/plat-samsung/clock.c: if (!IS_ERR_OR_NULL(clk) && clk->ops && clk->ops->round_rate) arch/arm/plat-samsung/clock.c: if (IS_ERR_OR_NULL(clk)) arch/arm/plat-samsung/clock.c: if (IS_ERR_OR_NULL(clk) || IS_ERR_OR_NULL(parent)) arch/arm/mach-imx/devices/platform-ipu-core.c: if (IS_ERR_OR_NULL(imx_ipu_coredev)) arch/arm/mach-imx/devices/platform-ipu-core.c: if (IS_ERR_OR_NULL(imx_ipu_coredev)) arch/arm/kernel/smp_twd.c: * We use IS_ERR_OR_NULL() here, because if the clock stubs arch/arm/kernel/smp_twd.c: if (!IS_ERR_OR_NULL(twd_clk)) They currently all legal uses of it - though I'm sure that the samsung clock uses can be reduced to just IS_ERR(). The IMX use looks "valid" in that imx_ipu_coredev really can be an error pointer (on failure) or NULL if the platform device hasn't yet been created. The TWD one is explained in the comments in the code (if people had to write explanations for using IS_ERR_OR_NULL(), we'd probably have it used correctly!)
[PATCH v9 2/2] video: drm: exynos: Add pinctrl support to fimd
Adds support for pinctrl to drm fimd Signed-off-by: Leela Krishna Amudala Signed-off-by: Vikas Sajjan --- drivers/gpu/drm/exynos/exynos_drm_fimd.c |9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index e323cf9..21ada8d 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -879,6 +880,7 @@ static int fimd_probe(struct platform_device *pdev) struct exynos_drm_fimd_pdata *pdata; struct exynos_drm_panel_info *panel; struct resource *res; + struct pinctrl *pctrl; int win; int ret = -EINVAL; @@ -897,6 +899,13 @@ static int fimd_probe(struct platform_device *pdev) DRM_ERROR("failed: of_get_fb_videomode() : %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():" + "%d\n", PTR_RET(pctrl)); + return PTR_ERR(pctrl); + } + } else { pdata = pdev->dev.platform_data; if (!pdata) { -- 1.7.9.5