diff mbox

drm/exynos: dsi: do not try to find bridge

Message ID 1497599415-4327-1-git-send-email-inki.dae@samsung.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Inki Dae June 16, 2017, 7:50 a.m. UTC
It doesn't need to try to find a bridge if bridge node doesn't exist.

Signed-off-by: Inki Dae <inki.dae@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Shuah Khan June 16, 2017, 2:16 p.m. UTC | #1
Hi Inki,

On Fri, Jun 16, 2017 at 1:50 AM, Inki Dae <inki.dae@samsung.com> wrote:
> It doesn't need to try to find a bridge if bridge node doesn't exist.
>
> Signed-off-by: Inki Dae <inki.dae@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index d404de8..e337cd2 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1687,9 +1687,11 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
>                 return ret;
>         }
>
> -       bridge = of_drm_find_bridge(dsi->bridge_node);

This is more of question than comment. I am seeing in some places,
such as mtk_dpi_probe(), of_node_put() is called right after
of_drm_find_bridge() whether or not bridge_node is found.

In this case, of_node_put() is done from exynos_dsi_remove() which looks
correct to me.

However, there is the discrepancy. One of these is incorrect perhaps?
When is the right time to call of_node_put()?

> -       if (bridge)
> -               drm_bridge_attach(encoder, bridge, NULL);
> +       if (dsi->bridge_node) {
> +               bridge = of_drm_find_bridge(dsi->bridge_node);
> +               if (bridge)
> +                       drm_bridge_attach(encoder, bridge, NULL);
> +       }
>
>         return mipi_dsi_host_register(&dsi->dsi_host);
>  }
> --
> 1.9.1

Looks good to me.

Reviewed-by: Shuah Khan <shuahkh@osg.samsung.com>

thanks,
-- Shuah
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shuah Khan June 16, 2017, 8:16 p.m. UTC | #2
On 06/16/2017 08:16 AM, Shuah Khan wrote:
> Hi Inki,
> 
> On Fri, Jun 16, 2017 at 1:50 AM, Inki Dae <inki.dae@samsung.com> wrote:
>> It doesn't need to try to find a bridge if bridge node doesn't exist.
>>
>> Signed-off-by: Inki Dae <inki.dae@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> index d404de8..e337cd2 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> @@ -1687,9 +1687,11 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
>>                 return ret;
>>         }
>>
>> -       bridge = of_drm_find_bridge(dsi->bridge_node);
> 
> This is more of question than comment. I am seeing in some places,
> such as mtk_dpi_probe(), of_node_put() is called right after
> of_drm_find_bridge() whether or not bridge_node is found.
> 
> In this case, of_node_put() is done from exynos_dsi_remove() which looks
> correct to me.
> 
> However, there is the discrepancy. One of these is incorrect perhaps?
> When is the right time to call of_node_put()?
> 
>> -       if (bridge)
>> -               drm_bridge_attach(encoder, bridge, NULL);
>> +       if (dsi->bridge_node) {
>> +               bridge = of_drm_find_bridge(dsi->bridge_node);
>> +               if (bridge)
>> +                       drm_bridge_attach(encoder, bridge, NULL);
>> +       }
>>
>>         return mipi_dsi_host_register(&dsi->dsi_host);
>>  }
>> --
>> 1.9.1
> 
> Looks good to me.
> 
> Reviewed-by: Shuah Khan <shuahkh@osg.samsung.com>
> 

I tested this on odroid-xu4

Tested-by: Shuah Khan <shuahkh@osg.samsung.com>

thanks,
-- Shuah

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Inki Dae June 19, 2017, 2:12 a.m. UTC | #3
Hi Shuah,

2017년 06월 17일 05:16에 Shuah Khan 이(가) 쓴 글:
> On 06/16/2017 08:16 AM, Shuah Khan wrote:
>> Hi Inki,
>>
>> On Fri, Jun 16, 2017 at 1:50 AM, Inki Dae <inki.dae@samsung.com> wrote:
>>> It doesn't need to try to find a bridge if bridge node doesn't exist.
>>>
>>> Signed-off-by: Inki Dae <inki.dae@samsung.com>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8 +++++---
>>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> index d404de8..e337cd2 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> @@ -1687,9 +1687,11 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
>>>                 return ret;
>>>         }
>>>
>>> -       bridge = of_drm_find_bridge(dsi->bridge_node);
>>
>> This is more of question than comment. I am seeing in some places,
>> such as mtk_dpi_probe(), of_node_put() is called right after
>> of_drm_find_bridge() whether or not bridge_node is found.
>>
>> In this case, of_node_put() is done from exynos_dsi_remove() which looks
>> correct to me.
>>
>> However, there is the discrepancy. One of these is incorrect perhaps?
>> When is the right time to call of_node_put()?

No problem but seems more reasonable to call of_node_put() right after of_drm_find_bridge() because there is no reason to keep bridge_node after binding it.
Could you clean up this?

Thanks,
Inki Dae

>>
>>> -       if (bridge)
>>> -               drm_bridge_attach(encoder, bridge, NULL);
>>> +       if (dsi->bridge_node) {
>>> +               bridge = of_drm_find_bridge(dsi->bridge_node);
>>> +               if (bridge)
>>> +                       drm_bridge_attach(encoder, bridge, NULL);
>>> +       }
>>>
>>>         return mipi_dsi_host_register(&dsi->dsi_host);
>>>  }
>>> --
>>> 1.9.1
>>
>> Looks good to me.
>>
>> Reviewed-by: Shuah Khan <shuahkh@osg.samsung.com>
>>
> 
> I tested this on odroid-xu4
> 
> Tested-by: Shuah Khan <shuahkh@osg.samsung.com>
> 
> thanks,
> -- Shuah
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shuah Khan June 19, 2017, 1:52 p.m. UTC | #4
Hi Inki,

On Sun, Jun 18, 2017 at 8:12 PM, Inki Dae <inki.dae@samsung.com> wrote:
> Hi Shuah,
>
> 2017년 06월 17일 05:16에 Shuah Khan 이(가) 쓴 글:
>> On 06/16/2017 08:16 AM, Shuah Khan wrote:
>>> Hi Inki,
>>>
>>> On Fri, Jun 16, 2017 at 1:50 AM, Inki Dae <inki.dae@samsung.com> wrote:
>>>> It doesn't need to try to find a bridge if bridge node doesn't exist.
>>>>
>>>> Signed-off-by: Inki Dae <inki.dae@samsung.com>
>>>> ---
>>>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8 +++++---
>>>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>> index d404de8..e337cd2 100644
>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>> @@ -1687,9 +1687,11 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
>>>>                 return ret;
>>>>         }
>>>>
>>>> -       bridge = of_drm_find_bridge(dsi->bridge_node);
>>>
>>> This is more of question than comment. I am seeing in some places,
>>> such as mtk_dpi_probe(), of_node_put() is called right after
>>> of_drm_find_bridge() whether or not bridge_node is found.
>>>
>>> In this case, of_node_put() is done from exynos_dsi_remove() which looks
>>> correct to me.
>>>
>>> However, there is the discrepancy. One of these is incorrect perhaps?
>>> When is the right time to call of_node_put()?
>
> No problem but seems more reasonable to call of_node_put() right after of_drm_find_bridge() because there is no reason to keep bridge_node after binding it.
> Could you clean up this?
>
> Thanks,
> Inki Dae

Yes I will take care of cleaning this up.

thanks,
-- Shuah

>
>>>
>>>> -       if (bridge)
>>>> -               drm_bridge_attach(encoder, bridge, NULL);
>>>> +       if (dsi->bridge_node) {
>>>> +               bridge = of_drm_find_bridge(dsi->bridge_node);
>>>> +               if (bridge)
>>>> +                       drm_bridge_attach(encoder, bridge, NULL);
>>>> +       }
>>>>
>>>>         return mipi_dsi_host_register(&dsi->dsi_host);
>>>>  }
>>>> --
>>>> 1.9.1
>>>
>>> Looks good to me.
>>>
>>> Reviewed-by: Shuah Khan <shuahkh@osg.samsung.com>
>>>
>>
>> I tested this on odroid-xu4
>>
>> Tested-by: Shuah Khan <shuahkh@osg.samsung.com>
>>
>> thanks,
>> -- Shuah
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index d404de8..e337cd2 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1687,9 +1687,11 @@  static int exynos_dsi_bind(struct device *dev, struct device *master,
 		return ret;
 	}
 
-	bridge = of_drm_find_bridge(dsi->bridge_node);
-	if (bridge)
-		drm_bridge_attach(encoder, bridge, NULL);
+	if (dsi->bridge_node) {
+		bridge = of_drm_find_bridge(dsi->bridge_node);
+		if (bridge)
+			drm_bridge_attach(encoder, bridge, NULL);
+	}
 
 	return mipi_dsi_host_register(&dsi->dsi_host);
 }