[PATCH v4 1/1] video: drm: exynos: Adds display-timing node parsing using video helper function

Paul Menzel paulepanter at users.sourceforge.net
Tue Feb 5 01:38:33 PST 2013


Dear Vikas,


thank you for the patch. Please send a fifth iteration with the
following changes to the commit message.

Am Dienstag, den 05.02.2013, 11:02 +0530 schrieb Vikas Sajjan:

The summary should not implicitly assume »patch« written before it. So
do not add third person s to »Add«.

        video: drm: exynos: Add display-timing node parsing using video helper function

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

As this is the same as the summary you should leave it out. Also it is
good style not to use »This/The patch« in the commit message.

Please use the commit message to explain your change. For example the if
statement. Why is the original code put into the else branch and is not
needed if the first condition is true?

> Signed-off-by: Leela Krishna Amudala <l.krishna at samsung.com>
> Signed-off-by: Vikas Sajjan <vikas.sajjan at linaro.org>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c |   41 +++++++++++++++++++++++++++---
>  1 file changed, 37 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..978e866 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,48 @@ 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: of_get_fb_videomode() :"
> +				"return value: %d\n", ret);
> +			return ret;
> +		}
> +		pdata->panel.timing = (struct fb_videomode) *fbmode;
> +
> +		pctrl = devm_pinctrl_get_select_default(dev);
> +		if (IS_ERR_OR_NULL(pctrl)) {
> +			DRM_ERROR("failed: devm_pinctrl_get_select_default()"
> +				"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;


Thanks,

Paul
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130205/2e7004c1/attachment.pgp>


More information about the dri-devel mailing list