| Message ID | 1497599415-4327-1-git-send-email-inki.dae@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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 > > >
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 >> >> >>
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); }
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(-)