| Message ID | 1361965796-16117-2-git-send-email-vikas.sajjan@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Feb 27, 2013 at 3:49 AM, Vikas Sajjan <vikas.sajjan@linaro.org> 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 | 25 +++++++++++++++++++++---- > 1 file changed, 21 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..7932dc2 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > @@ -20,6 +20,7 @@ > #include <linux/of_device.h> > #include <linux/pm_runtime.h> > > +#include <video/of_display_timing.h> > #include <video/samsung_fimd.h> > #include <drm/exynos_drm.h> > > @@ -883,10 +884,26 @@ static int fimd_probe(struct platform_device *pdev) > > 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; > + } > + > + ret = of_get_fb_videomode(dev->of_node, &pdata->panel.timing, > + OF_USE_NATIVE_MODE); > + if (ret) { > + DRM_ERROR("failed: of_get_fb_videomode()\n" > + "with return value: %d\n", ret); > + return ret; Here I think you leak pdata in the error path. Stéphane > + } > + } 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
On Wed, Feb 27, 2013 at 11:21 AM, Stéphane Marchesin <stephane.marchesin@gmail.com> wrote: > On Wed, Feb 27, 2013 at 3:49 AM, Vikas Sajjan <vikas.sajjan@linaro.org> 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 | 25 +++++++++++++++++++++---- >> 1 file changed, 21 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..7932dc2 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> @@ -20,6 +20,7 @@ >> #include <linux/of_device.h> >> #include <linux/pm_runtime.h> >> >> +#include <video/of_display_timing.h> >> #include <video/samsung_fimd.h> >> #include <drm/exynos_drm.h> >> >> @@ -883,10 +884,26 @@ static int fimd_probe(struct platform_device *pdev) >> >> 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; >> + } >> + >> + ret = of_get_fb_videomode(dev->of_node, &pdata->panel.timing, >> + OF_USE_NATIVE_MODE); >> + if (ret) { >> + DRM_ERROR("failed: of_get_fb_videomode()\n" >> + "with return value: %d\n", ret); >> + return ret; > > Here I think you leak pdata in the error path. > Hmm nevermind it goes away with the dev. Stéphane > Stéphane > >> + } >> + } 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
On 02/27/2013 08:49 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 | 25 +++++++++++++++++++++---- > 1 file changed, 21 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..7932dc2 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > @@ -20,6 +20,7 @@ > #include <linux/of_device.h> > #include <linux/pm_runtime.h> > > +#include <video/of_display_timing.h> > #include <video/samsung_fimd.h> > #include <drm/exynos_drm.h> > > @@ -883,10 +884,26 @@ static int fimd_probe(struct platform_device *pdev) > > 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; > + } > + > + ret = of_get_fb_videomode(dev->of_node, &pdata->panel.timing, > + OF_USE_NATIVE_MODE); > + if (ret) { > + DRM_ERROR("failed: of_get_fb_videomode()\n" > + "with return value: %d\n", ret); Could you make this error log to one line? except this, Acked-by: Joonyoung Shim <jy0922.shim@samsung.com> > + return ret; > + } > + } else { > + pdata = pdev->dev.platform_data; > + if (!pdata) { > + DRM_ERROR("no platform data specified\n"); > + return -EINVAL; > + } > } > > panel = &pdata->panel;
Hi, On 28 February 2013 08:07, Joonyoung Shim <jy0922.shim@samsung.com> wrote: > On 02/27/2013 08:49 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 | 25 >> +++++++++++++++++++++---- >> 1 file changed, 21 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..7932dc2 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> @@ -20,6 +20,7 @@ >> #include <linux/of_device.h> >> #include <linux/pm_runtime.h> >> +#include <video/of_display_timing.h> >> #include <video/samsung_fimd.h> >> #include <drm/exynos_drm.h> >> @@ -883,10 +884,26 @@ static int fimd_probe(struct platform_device >> *pdev) >> 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; >> + } >> + >> + ret = of_get_fb_videomode(dev->of_node, >> &pdata->panel.timing, >> + OF_USE_NATIVE_MODE); >> + if (ret) { >> + DRM_ERROR("failed: of_get_fb_videomode()\n" >> + "with return value: %d\n", ret); > > > Could you make this error log to one line? > The Line was going beyond 80 line marks, hence I had to split it. > except this, > Acked-by: Joonyoung Shim <jy0922.shim@samsung.com> > > >> + return ret; >> + } >> + } else { >> + pdata = pdev->dev.platform_data; >> + if (!pdata) { >> + DRM_ERROR("no platform data specified\n"); >> + return -EINVAL; >> + } >> } >> panel = &pdata->panel; > >
On 02/28/2013 11:45 AM, Vikas Sajjan wrote: > Hi, > > On 28 February 2013 08:07, Joonyoung Shim <jy0922.shim@samsung.com> wrote: >> On 02/27/2013 08:49 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 | 25 >>> +++++++++++++++++++++---- >>> 1 file changed, 21 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..7932dc2 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>> @@ -20,6 +20,7 @@ >>> #include <linux/of_device.h> >>> #include <linux/pm_runtime.h> >>> +#include <video/of_display_timing.h> >>> #include <video/samsung_fimd.h> >>> #include <drm/exynos_drm.h> >>> @@ -883,10 +884,26 @@ static int fimd_probe(struct platform_device >>> *pdev) >>> 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; >>> + } >>> + >>> + ret = of_get_fb_videomode(dev->of_node, >>> &pdata->panel.timing, >>> + OF_USE_NATIVE_MODE); >>> + if (ret) { >>> + DRM_ERROR("failed: of_get_fb_videomode()\n" >>> + "with return value: %d\n", ret); >> >> Could you make this error log to one line? >> > The Line was going beyond 80 line marks, hence I had to split it. So remove or contract some log messages, e.g. "with return value" I think that is unnecessary. >> except this, >> Acked-by: Joonyoung Shim <jy0922.shim@samsung.com> >> >> >>> + return ret; >>> + } >>> + } else { >>> + pdata = pdev->dev.platform_data; >>> + if (!pdata) { >>> + DRM_ERROR("no platform data specified\n"); >>> + return -EINVAL; >>> + } >>> } >>> panel = &pdata->panel; >> > >
On Thu, Feb 28, 2013 at 8:21 AM, Joonyoung Shim <jy0922.shim@samsung.com> wrote: > On 02/28/2013 11:45 AM, Vikas Sajjan wrote: >> >> Hi, >> >> On 28 February 2013 08:07, Joonyoung Shim <jy0922.shim@samsung.com> wrote: >>> >>> On 02/27/2013 08:49 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 | 25 >>>> +++++++++++++++++++++---- >>>> 1 file changed, 21 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..7932dc2 100644 >>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >>>> @@ -20,6 +20,7 @@ >>>> #include <linux/of_device.h> >>>> #include <linux/pm_runtime.h> >>>> +#include <video/of_display_timing.h> >>>> #include <video/samsung_fimd.h> >>>> #include <drm/exynos_drm.h> >>>> @@ -883,10 +884,26 @@ static int fimd_probe(struct platform_device >>>> *pdev) >>>> 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; >>>> + } >>>> + >>>> + ret = of_get_fb_videomode(dev->of_node, >>>> &pdata->panel.timing, >>>> + OF_USE_NATIVE_MODE); >>>> + if (ret) { >>>> + DRM_ERROR("failed: of_get_fb_videomode()\n" >>>> + "with return value: %d\n", ret); >>> >>> >>> Could you make this error log to one line? >>> >> The Line was going beyond 80 line marks, hence I had to split it. > > > So remove or contract some log messages, e.g. "with return value" > I think that is unnecessary. > Will do and resend. > >>> except this, >>> Acked-by: Joonyoung Shim <jy0922.shim@samsung.com> >>> >>> >>>> + return ret; >>>> + } >>>> + } else { >>>> + pdata = pdev->dev.platform_data; >>>> + if (!pdata) { >>>> + DRM_ERROR("no platform data specified\n"); >>>> + return -EINVAL; >>>> + } >>>> } >>>> panel = &pdata->panel; >>> >>> >> >> > > -- > 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..7932dc2 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -20,6 +20,7 @@ #include <linux/of_device.h> #include <linux/pm_runtime.h> +#include <video/of_display_timing.h> #include <video/samsung_fimd.h> #include <drm/exynos_drm.h> @@ -883,10 +884,26 @@ static int fimd_probe(struct platform_device *pdev) 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; + } + + ret = of_get_fb_videomode(dev->of_node, &pdata->panel.timing, + OF_USE_NATIVE_MODE); + if (ret) { + DRM_ERROR("failed: of_get_fb_videomode()\n" + "with return value: %d\n", ret); + return ret; + } + } else { + pdata = pdev->dev.platform_data; + if (!pdata) { + DRM_ERROR("no platform data specified\n"); + return -EINVAL; + } } panel = &pdata->panel;