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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx>
> Signed-off-by: Vikas Sajjan <vikas.sajjan@xxxxxxxxxx>
> ---
>  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

Attachment: signature.asc
Description: This is a digitally signed message part


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux