diff mbox

[v6,1/1] video: drm: exynos: Add display-timing node parsing using video helper function

Message ID 1360910587-25548-2-git-send-email-vikas.sajjan@linaro.org
State New
Headers show

Commit Message

Vikas C Sajjan Feb. 15, 2013, 6:43 a.m. UTC
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 |   37 ++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

Comments

대인기/Tizen Platform Lab(SR)/삼성전자 Feb. 20, 2013, 11:15 a.m. UTC | #1
> -----Original Message-----
> From: Vikas Sajjan [mailto:vikas.sajjan@linaro.org]
> Sent: Friday, February 15, 2013 3:43 PM
> To: dri-devel@lists.freedesktop.org
> Cc: linux-media@vger.kernel.org; kgene.kim@samsung.com;
> inki.dae@samsung.com; l.krishna@samsung.com; patches@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.krishna@samsung.com>
> Signed-off-by: Vikas Sajjan <vikas.sajjan@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
Vikas C Sajjan Feb. 21, 2013, 4:44 a.m. UTC | #2
Hi Mr. Inki Dae,

On 20 February 2013 16:45, Inki Dae <inki.dae@samsung.com> wrote:
>
>
>> -----Original Message-----
>> From: Vikas Sajjan [mailto:vikas.sajjan@linaro.org]
>> Sent: Friday, February 15, 2013 3:43 PM
>> To: dri-devel@lists.freedesktop.org
>> Cc: linux-media@vger.kernel.org; kgene.kim@samsung.com;
>> inki.dae@samsung.com; l.krishna@samsung.com; patches@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.krishna@samsung.com>
>> Signed-off-by: Vikas Sajjan <vikas.sajjan@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
>
Joonyoung Shim Feb. 21, 2013, 6:55 a.m. UTC | #3
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.krishna@samsung.com>
> Signed-off-by: Vikas Sajjan <vikas.sajjan@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.
Vikas C Sajjan Feb. 21, 2013, 7:12 a.m. UTC | #4
Hi,

On 21 February 2013 12:25, Joonyoung Shim <jy0922.shim@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.krishna@samsung.com>
>> Signed-off-by: Vikas Sajjan <vikas.sajjan@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.
Joonyoung Shim Feb. 21, 2013, 7:18 a.m. UTC | #5
On 02/21/2013 04:12 PM, Vikas Sajjan wrote:
> Hi,
>
> On 21 February 2013 12:25, Joonyoung Shim <jy0922.shim@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.krishna@samsung.com>
>>> Signed-off-by: Vikas Sajjan <vikas.sajjan@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.
>
>
Joonyoung Shim Feb. 28, 2013, 1:51 a.m. UTC | #6
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.shim@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.krishna@samsung.com>
>>>> Signed-off-by: Vikas Sajjan <vikas.sajjan@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-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Linus Walleij Feb. 28, 2013, 11:56 p.m. UTC | #7
On Thu, Feb 28, 2013 at 2:51 AM, Joonyoung Shim <jy0922.shim@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
diff mbox

Patch

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);
+		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;