From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752550AbdF0KIe (ORCPT ); Tue, 27 Jun 2017 06:08:34 -0400 Received: from mailout4.w1.samsung.com ([210.118.77.14]:49074 "EHLO mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751632AbdF0KI0 (ORCPT ); Tue, 27 Jun 2017 06:08:26 -0400 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 X-AuditID: cbfec7f2-f797e6d000004438-18-59522e75d7ee Subject: Re: [PATCH] drm: exynos: dsi: release DSI_PORT_OUT node right after of_drm_find_bridge() To: Inki Dae , Shuah Khan , sw0312.kim@samsung.com, kyungmin.park@samsung.com, airlied@linux.ie, kgene@kernel.org, krzk@kernel.org, javier@osg.samsung.com Cc: linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org From: Andrzej Hajda Message-id: Date: Tue, 27 Jun 2017 12:07:48 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 In-reply-to: <5952219F.1030902@samsung.com> Content-transfer-encoding: 8bit Content-language: en-US X-Brightmail-Tracker: H4sIAAAAAAAAA01Sa0gUYRTtm5mdGTc3xtX0pr3YsLTIFEWGFMvox0L9KOuHCmmLDiq6Krur aBFKofiofK2li+WrLCwzVjFJMR+5mraulkj+8I2Wjy1fG26l5DoK/jv3O+fcc+/lo3HxZ4Ej HRWr4hSxshgJKSQadGbD6US3gCD3rALEPuj7hLGDpl8kmz+WS7ALxtcYmzM1j7MGw1uK1d9d oFjt1JCA/fq+hGSLDC0YW7i6SLBFBbPkeWuptjqTlL77PS6QjmV3YdK6ZynS+pwRSvqwvhpJ V7SHr1DBQt9wLiYqkVOc8bspjCyuGEDx605JebrgVLRkn4WsaGC84GN5GcZje+gfrSWzkJAW M88R1KvfEHyxgmC1qUOw41grM2+rqhCUtowSFkLE2MBaAY9xxhV+rOZvu2cQTM6bkYWwZcJA vZiKWQg7Ro9gonGRshQ4cx9BlW5kS0Vu2tfrhkm+rR9U/CvczKZpgnGG1he05Xk/EwgV+XmU BVsxp+DD0lOMTz4CbYPft6dwgHtpw1tTAGOkYEk7Qlr6AHMItK04Dy+CvtuV38wW5rrqKR4f hMyMNoy3ZiNYzumm+EKNYGOxCOdVPtDRNSDgw/ZBfsPj7aYiyEgX8xIpVK7kETz2h2ytWcBf ZRpBR0k/lYuOanZdT7Preppd+2h27VOGiGpkxyUo5RGc0tNNKZMrE2Ij3MLi5Fq0+cd6N7qW G5Gp+2w7YmgksRb9wa4GiQWyRGWyvB0BjUvsRH/xgCCxKFyWfItTxIUqEmI4ZTtyogmJg0jY MxQoZiJkKi6a4+I5xQ6L0VaOqahQWKpOusxU1cATD+9pd/ybi9n7xmzQeNMx/PoJeeXCuk+5 +kunpzbJZabZOGej897be80ls3MipN3o0GxSqU0/09DkOFurd1C49IX4puvtb/cbU46/POB1 zvSoRtRzZ9K5eNo/+Ui4IfESh/Y4Za7hKxeiQzUq7xIVnqF7JSGUkTKPk7hCKfsPJwvGcl8D AAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrJIsWRmVeSWpSXmKPExsVy+t/xy7qlekGRBjNnq1r0njvJZHHl63s2 i0n3J7BYvHm7hsmi//FrZovz5zewW5xtesNusenxNVaLy7vmsFnMOL+PyWLqlw8sFjMmv2Rz 4PHYtKqTzWP7twesHve7jzN5bF5S77Gl/y67R9+WVYwenzfJBbBHudlkpCampBYppOYl56dk 5qXbKoWGuOlaKCnkJeam2ipF6PqGBCkplCXmlAJ5RgZowME5wD1YSd8uwS1j5qKLjAV/pSsm HotqYPwo1sXIySEhYCLxY8FPNghbTOLCvfVANheHkMASRomJq86CJXgFBCV+TL7H0sXIwcEs oC4xZUouRM0zRon+oweYQWqEBZIlpnxoYAKxRQTOMkrMv+wNUsQs0MMo8XLiCiaIjueMEmf2 3AebyiagKfF3802oDXYSi/5MZQXZwCKgKnFgOQdIWFQgQmLX9QOsIDangLbE/o/zwBYwC8hL HLzynAXCFpdobr3JMoFRcBaSW2ch3DoLSccsJB0LGFlWMYqklhbnpucWG+oVJ+YWl+al6yXn 525iBEbztmM/N+9gvLQx+BCjAAejEg/vD6bASCHWxLLiytxDjBIczEoivL+ZgyKFeFMSK6tS i/Lji0pzUosPMZoCvTCRWUo0OR+YaPJK4g1NDM0tDY2MLSzMjYyUxHlLPlwJFxJITyxJzU5N LUgtgulj4uCUamAsWHp7nx4P+2nJRYXPK22cble4C+63W+C1mGHXN7UMy9MKkoLWIvtvsIV/ eMZofmu7ixrrk5tLm3d17LIT/iUqupJp7ieXtM7qppAAj5kNUUIfXnEdeh2SlX988uNvSg3f BX02nnXw+1Ox+vkHXZNJwct/2TpGpbw//9D53P/NRvW7Xd8m3/RTYinOSDTUYi4qTgQAK+QK M/wCAAA= X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20170627100749eucas1p10e7b079ab413f28dc9b1cbbb587e2639 X-Msg-Generator: CA X-Sender-IP: 182.198.249.179 X-Local-Sender: =?UTF-8?B?QW5kcnplaiBIYWpkYRtTUlBPTC1LZXJuZWwgKFRQKRvsgrw=?= =?UTF-8?B?7ISx7KCE7J6QG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Global-Sender: =?UTF-8?B?QW5kcnplaiBIYWpkYRtTUlBPTC1LZXJuZWwgKFRQKRtTYW1z?= =?UTF-8?B?dW5nIEVsZWN0cm9uaWNzG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDAyQ0QwMjczOTI=?= CMS-TYPE: 201P X-HopCount: 7 X-CMS-RootMailID: 20170626004946epcas4p40785020666ae6d43476822d067604baa X-RootMTR: 20170626004946epcas4p40785020666ae6d43476822d067604baa References: <20170624005628.5896-1-shuahkh@osg.samsung.com> <5d308e03-e92a-7bac-f65e-23c2ccbe161a@samsung.com> <5952219F.1030902@samsung.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 27.06.2017 11:13, Inki Dae wrote: > Hi Andrzej, > > 2017년 06월 26일 16:02에 Andrzej Hajda 이(가) 쓴 글: >> Hi Shuah, >> >> >> On 24.06.2017 02:56, Shuah Khan wrote: >>> Fix to call of_node_put() right after of_drm_find_bridge() instead of >>> holding on to it until exynos_dsi_remove(). >> I think the current implementation is OK, node is get in probe and put >> in remove. >> There could be many bind/unbind during lifetime of the bound driver. >> For example, there is possible sequence: >> 1. probe - >> 2. bind >> 3. unbind >> 4. bind >> >> With this patch on 2nd bind (point 4) driver will call >> of_drm_find_bridge on dsi->bridge_node which was put earlier (point 2.). >> > Right. This is a problem. > > How about moving of_drm_find_bridge function call to probe and keeping drm_bridge_attach call in bind for cleanup? > Seems it doesn't need to call of_drm_find_bridge function every time bind callback is called. I think the reason to call of_drm_find_bridge in bind is to avoid probe deferring, and with current code it will probably defer forever, explanation below: The only bridge connected to DSI is bridge provided by MIC driver, and MIC driver is componentized, but it registers his bridge in bind callback. So with drm_bridge_attach moved to probe of DSI initialization would never succeed because: 1. MIC->bind will be called only if all DRM components are present. 2. DSI driver will defer probing because the bridge is not present, thus DSI component will never be present. So the current code looks better to me, but there are two thing which could be improved: A. Maybe it would be good thing to move MIC bridge registration to probe, I see no reason to postpone it to bind phase, this way it should be safer if for some reason DSI->bind callback would be called before MIC->bind. B. Add error checking to of_drm_find_bridge in DSI driver, I know MIC is generally optional, but if the graph link is present (ie. dsi->bridge_node is valid) MIC bridge should be present, if it is not, it indicates some problem. Regards Andrzej > > Thanks, > Inki Dae > >> Regards >> Andrzej >> >> >>> Suggested-by: Inki Dae >>> Signed-off-by: Shuah Khan >>> --- >>> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 5 +---- >>> 1 file changed, 1 insertion(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c >>> index e337cd2..7513b88 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c >>> @@ -1689,6 +1689,7 @@ static int exynos_dsi_bind(struct device *dev, struct device *master, >>> >>> if (dsi->bridge_node) { >>> bridge = of_drm_find_bridge(dsi->bridge_node); >>> + of_node_put(dsi->bridge_node); >>> if (bridge) >>> drm_bridge_attach(encoder, bridge, NULL); >>> } >>> @@ -1807,10 +1808,6 @@ static int exynos_dsi_probe(struct platform_device *pdev) >>> >>> static int exynos_dsi_remove(struct platform_device *pdev) >>> { >>> - struct exynos_dsi *dsi = platform_get_drvdata(pdev); >>> - >>> - of_node_put(dsi->bridge_node); >>> - >>> pm_runtime_disable(&pdev->dev); >>> >>> component_del(&pdev->dev, &exynos_dsi_component_ops); >> >> -- >> 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 >> >> >> > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrzej Hajda Subject: Re: [PATCH] drm: exynos: dsi: release DSI_PORT_OUT node right after of_drm_find_bridge() Date: Tue, 27 Jun 2017 12:07:48 +0200 Message-ID: References: <20170624005628.5896-1-shuahkh@osg.samsung.com> <5d308e03-e92a-7bac-f65e-23c2ccbe161a@samsung.com> <5952219F.1030902@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-reply-to: <5952219F.1030902@samsung.com> Content-language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Inki Dae , Shuah Khan , sw0312.kim@samsung.com, kyungmin.park@samsung.com, airlied@linux.ie, kgene@kernel.org, krzk@kernel.org, javier@osg.samsung.com Cc: linux-samsung-soc@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org List-Id: linux-samsung-soc@vger.kernel.org T24gMjcuMDYuMjAxNyAxMToxMywgSW5raSBEYWUgd3JvdGU6Cj4gSGkgQW5kcnplaiwKPgo+IDIw MTfrhYQgMDbsm5QgMjbsnbwgMTY6MDLsl5AgQW5kcnplaiBIYWpkYSDsnbQo6rCAKSDsk7Qg6riA Ogo+PiBIaSBTaHVhaCwKPj4KPj4KPj4gT24gMjQuMDYuMjAxNyAwMjo1NiwgU2h1YWggS2hhbiB3 cm90ZToKPj4+IEZpeCB0byBjYWxsIG9mX25vZGVfcHV0KCkgcmlnaHQgYWZ0ZXIgb2ZfZHJtX2Zp bmRfYnJpZGdlKCkgaW5zdGVhZCBvZgo+Pj4gaG9sZGluZyBvbiB0byBpdCB1bnRpbCBleHlub3Nf ZHNpX3JlbW92ZSgpLgo+PiBJIHRoaW5rIHRoZSBjdXJyZW50IGltcGxlbWVudGF0aW9uIGlzIE9L LCBub2RlIGlzIGdldCBpbiBwcm9iZSBhbmQgcHV0Cj4+IGluIHJlbW92ZS4KPj4gVGhlcmUgY291 bGQgYmUgbWFueSBiaW5kL3VuYmluZCBkdXJpbmcgbGlmZXRpbWUgb2YgdGhlIGJvdW5kIGRyaXZl ci4KPj4gRm9yIGV4YW1wbGUsIHRoZXJlIGlzIHBvc3NpYmxlIHNlcXVlbmNlOgo+PiAxLiBwcm9i ZSAtCj4+IDIuIGJpbmQKPj4gMy4gdW5iaW5kCj4+IDQuIGJpbmQKPj4KPj4gV2l0aCB0aGlzIHBh dGNoIG9uIDJuZCBiaW5kIChwb2ludCA0KSBkcml2ZXIgd2lsbCBjYWxsCj4+IG9mX2RybV9maW5k X2JyaWRnZSBvbiBkc2ktPmJyaWRnZV9ub2RlIHdoaWNoIHdhcyBwdXQgZWFybGllciAocG9pbnQg Mi4pLgo+Pgo+IFJpZ2h0LiBUaGlzIGlzIGEgcHJvYmxlbS4KPgo+IEhvdyBhYm91dCBtb3Zpbmcg b2ZfZHJtX2ZpbmRfYnJpZGdlIGZ1bmN0aW9uIGNhbGwgdG8gcHJvYmUgYW5kIGtlZXBpbmcgZHJt X2JyaWRnZV9hdHRhY2ggY2FsbCBpbiBiaW5kIGZvciBjbGVhbnVwPwo+IFNlZW1zIGl0IGRvZXNu J3QgbmVlZCB0byBjYWxsIG9mX2RybV9maW5kX2JyaWRnZSBmdW5jdGlvbiBldmVyeSB0aW1lIGJp bmQgY2FsbGJhY2sgaXMgY2FsbGVkLgoKSSB0aGluayB0aGUgcmVhc29uIHRvIGNhbGwgb2ZfZHJt X2ZpbmRfYnJpZGdlIGluIGJpbmQgaXMgdG8gYXZvaWQgcHJvYmUKZGVmZXJyaW5nLCBhbmQgd2l0 aCBjdXJyZW50IGNvZGUgaXQgd2lsbCBwcm9iYWJseSBkZWZlciBmb3JldmVyLApleHBsYW5hdGlv biBiZWxvdzoKClRoZSBvbmx5IGJyaWRnZSBjb25uZWN0ZWQgdG8gRFNJIGlzIGJyaWRnZSBwcm92 aWRlZCBieSBNSUMgZHJpdmVyLCBhbmQKTUlDIGRyaXZlciBpcyBjb21wb25lbnRpemVkLCBidXQg aXQgcmVnaXN0ZXJzIGhpcyBicmlkZ2UgaW4gYmluZCBjYWxsYmFjay4KU28gd2l0aCBkcm1fYnJp ZGdlX2F0dGFjaCBtb3ZlZCB0byBwcm9iZSBvZiBEU0kgaW5pdGlhbGl6YXRpb24gd291bGQKbmV2 ZXIgc3VjY2VlZCBiZWNhdXNlOgoxLiBNSUMtPmJpbmQgd2lsbCBiZSBjYWxsZWQgb25seSBpZiBh bGwgRFJNIGNvbXBvbmVudHMgYXJlIHByZXNlbnQuCjIuIERTSSBkcml2ZXIgd2lsbCBkZWZlciBw cm9iaW5nIGJlY2F1c2UgdGhlIGJyaWRnZSBpcyBub3QgcHJlc2VudCwgdGh1cwpEU0kgY29tcG9u ZW50IHdpbGwgbmV2ZXIgYmUgcHJlc2VudC4KClNvIHRoZSBjdXJyZW50IGNvZGUgbG9va3MgYmV0 dGVyIHRvIG1lLCBidXQgdGhlcmUgYXJlIHR3byB0aGluZyB3aGljaApjb3VsZCBiZSBpbXByb3Zl ZDoKQS4gTWF5YmUgaXQgd291bGQgYmUgZ29vZCB0aGluZyB0byBtb3ZlIE1JQyBicmlkZ2UgcmVn aXN0cmF0aW9uIHRvCnByb2JlLCBJIHNlZSBubyByZWFzb24gdG8gcG9zdHBvbmUgaXQgdG8gYmlu ZCBwaGFzZSwgdGhpcyB3YXkgaXQgc2hvdWxkCmJlIHNhZmVyIGlmIGZvciBzb21lIHJlYXNvbiBE U0ktPmJpbmQgY2FsbGJhY2sgd291bGQgYmUgY2FsbGVkIGJlZm9yZQpNSUMtPmJpbmQuCkIuIEFk ZCBlcnJvciBjaGVja2luZyB0byBvZl9kcm1fZmluZF9icmlkZ2UgaW4gRFNJIGRyaXZlciwgSSBr bm93IE1JQyBpcwpnZW5lcmFsbHkgb3B0aW9uYWwsIGJ1dCBpZiB0aGUgZ3JhcGggbGluayBpcyBw cmVzZW50IChpZS4gCmRzaS0+YnJpZGdlX25vZGUgaXMgdmFsaWQpIE1JQyBicmlkZ2Ugc2hvdWxk IGJlIHByZXNlbnQsIGlmIGl0IGlzIG5vdCwKaXQgaW5kaWNhdGVzIHNvbWUgcHJvYmxlbS4KClJl Z2FyZHMKQW5kcnplagoKPgo+IFRoYW5rcywKPiBJbmtpIERhZQo+Cj4+IFJlZ2FyZHMKPj4gQW5k cnplago+Pgo+Pgo+Pj4gU3VnZ2VzdGVkLWJ5OiBJbmtpIERhZSA8aW5raS5kYWVAc2Ftc3VuZy5j b20+Cj4+PiBTaWduZWQtb2ZmLWJ5OiBTaHVhaCBLaGFuIDxzaHVhaGtoQG9zZy5zYW1zdW5nLmNv bT4KPj4+IC0tLQo+Pj4gIGRyaXZlcnMvZ3B1L2RybS9leHlub3MvZXh5bm9zX2RybV9kc2kuYyB8 IDUgKy0tLS0KPj4+ICAxIGZpbGUgY2hhbmdlZCwgMSBpbnNlcnRpb24oKyksIDQgZGVsZXRpb25z KC0pCj4+Pgo+Pj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9leHlub3MvZXh5bm9zX2Ry bV9kc2kuYyBiL2RyaXZlcnMvZ3B1L2RybS9leHlub3MvZXh5bm9zX2RybV9kc2kuYwo+Pj4gaW5k ZXggZTMzN2NkMi4uNzUxM2I4OCAxMDA2NDQKPj4+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9leHlu b3MvZXh5bm9zX2RybV9kc2kuYwo+Pj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2V4eW5vcy9leHlu b3NfZHJtX2RzaS5jCj4+PiBAQCAtMTY4OSw2ICsxNjg5LDcgQEAgc3RhdGljIGludCBleHlub3Nf ZHNpX2JpbmQoc3RydWN0IGRldmljZSAqZGV2LCBzdHJ1Y3QgZGV2aWNlICptYXN0ZXIsCj4+PiAg Cj4+PiAgCWlmIChkc2ktPmJyaWRnZV9ub2RlKSB7Cj4+PiAgCQlicmlkZ2UgPSBvZl9kcm1fZmlu ZF9icmlkZ2UoZHNpLT5icmlkZ2Vfbm9kZSk7Cj4+PiArCQlvZl9ub2RlX3B1dChkc2ktPmJyaWRn ZV9ub2RlKTsKPj4+ICAJCWlmIChicmlkZ2UpCj4+PiAgCQkJZHJtX2JyaWRnZV9hdHRhY2goZW5j b2RlciwgYnJpZGdlLCBOVUxMKTsKPj4+ICAJfQo+Pj4gQEAgLTE4MDcsMTAgKzE4MDgsNiBAQCBz dGF0aWMgaW50IGV4eW5vc19kc2lfcHJvYmUoc3RydWN0IHBsYXRmb3JtX2RldmljZSAqcGRldikK Pj4+ICAKPj4+ICBzdGF0aWMgaW50IGV4eW5vc19kc2lfcmVtb3ZlKHN0cnVjdCBwbGF0Zm9ybV9k ZXZpY2UgKnBkZXYpCj4+PiAgewo+Pj4gLQlzdHJ1Y3QgZXh5bm9zX2RzaSAqZHNpID0gcGxhdGZv cm1fZ2V0X2RydmRhdGEocGRldik7Cj4+PiAtCj4+PiAtCW9mX25vZGVfcHV0KGRzaS0+YnJpZGdl X25vZGUpOwo+Pj4gLQo+Pj4gIAlwbV9ydW50aW1lX2Rpc2FibGUoJnBkZXYtPmRldik7Cj4+PiAg Cj4+PiAgCWNvbXBvbmVudF9kZWwoJnBkZXYtPmRldiwgJmV4eW5vc19kc2lfY29tcG9uZW50X29w cyk7Cj4+Cj4+IC0tCj4+IFRvIHVuc3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBs aW5lICJ1bnN1YnNjcmliZSBsaW51eC1zYW1zdW5nLXNvYyIgaW4KPj4gdGhlIGJvZHkgb2YgYSBt ZXNzYWdlIHRvIG1ham9yZG9tb0B2Z2VyLmtlcm5lbC5vcmcKPj4gTW9yZSBtYWpvcmRvbW8gaW5m byBhdCAgaHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sCj4+Cj4+Cj4+ Cj4KCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpsaW51 eC1hcm0ta2VybmVsIG1haWxpbmcgbGlzdApsaW51eC1hcm0ta2VybmVsQGxpc3RzLmluZnJhZGVh ZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFpbG1hbi9saXN0aW5mby9saW51eC1h cm0ta2VybmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: a.hajda@samsung.com (Andrzej Hajda) Date: Tue, 27 Jun 2017 12:07:48 +0200 Subject: [PATCH] drm: exynos: dsi: release DSI_PORT_OUT node right after of_drm_find_bridge() In-Reply-To: <5952219F.1030902@samsung.com> References: <20170624005628.5896-1-shuahkh@osg.samsung.com> <5d308e03-e92a-7bac-f65e-23c2ccbe161a@samsung.com> <5952219F.1030902@samsung.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 27.06.2017 11:13, Inki Dae wrote: > Hi Andrzej, > > 2017? 06? 26? 16:02? Andrzej Hajda ?(?) ? ?: >> Hi Shuah, >> >> >> On 24.06.2017 02:56, Shuah Khan wrote: >>> Fix to call of_node_put() right after of_drm_find_bridge() instead of >>> holding on to it until exynos_dsi_remove(). >> I think the current implementation is OK, node is get in probe and put >> in remove. >> There could be many bind/unbind during lifetime of the bound driver. >> For example, there is possible sequence: >> 1. probe - >> 2. bind >> 3. unbind >> 4. bind >> >> With this patch on 2nd bind (point 4) driver will call >> of_drm_find_bridge on dsi->bridge_node which was put earlier (point 2.). >> > Right. This is a problem. > > How about moving of_drm_find_bridge function call to probe and keeping drm_bridge_attach call in bind for cleanup? > Seems it doesn't need to call of_drm_find_bridge function every time bind callback is called. I think the reason to call of_drm_find_bridge in bind is to avoid probe deferring, and with current code it will probably defer forever, explanation below: The only bridge connected to DSI is bridge provided by MIC driver, and MIC driver is componentized, but it registers his bridge in bind callback. So with drm_bridge_attach moved to probe of DSI initialization would never succeed because: 1. MIC->bind will be called only if all DRM components are present. 2. DSI driver will defer probing because the bridge is not present, thus DSI component will never be present. So the current code looks better to me, but there are two thing which could be improved: A. Maybe it would be good thing to move MIC bridge registration to probe, I see no reason to postpone it to bind phase, this way it should be safer if for some reason DSI->bind callback would be called before MIC->bind. B. Add error checking to of_drm_find_bridge in DSI driver, I know MIC is generally optional, but if the graph link is present (ie. dsi->bridge_node is valid) MIC bridge should be present, if it is not, it indicates some problem. Regards Andrzej > > Thanks, > Inki Dae > >> Regards >> Andrzej >> >> >>> Suggested-by: Inki Dae >>> Signed-off-by: Shuah Khan >>> --- >>> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 5 +---- >>> 1 file changed, 1 insertion(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c >>> index e337cd2..7513b88 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c >>> @@ -1689,6 +1689,7 @@ static int exynos_dsi_bind(struct device *dev, struct device *master, >>> >>> if (dsi->bridge_node) { >>> bridge = of_drm_find_bridge(dsi->bridge_node); >>> + of_node_put(dsi->bridge_node); >>> if (bridge) >>> drm_bridge_attach(encoder, bridge, NULL); >>> } >>> @@ -1807,10 +1808,6 @@ static int exynos_dsi_probe(struct platform_device *pdev) >>> >>> static int exynos_dsi_remove(struct platform_device *pdev) >>> { >>> - struct exynos_dsi *dsi = platform_get_drvdata(pdev); >>> - >>> - of_node_put(dsi->bridge_node); >>> - >>> pm_runtime_disable(&pdev->dev); >>> >>> component_del(&pdev->dev, &exynos_dsi_component_ops); >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in >> the body of a message to majordomo at vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> >> >