From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751280AbdGQE06 (ORCPT ); Mon, 17 Jul 2017 00:26:58 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:46500 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751093AbdGQE05 (ORCPT ); Mon, 17 Jul 2017 00:26:57 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 9679D6121C Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=architt@codeaurora.org Subject: Re: [PATCH 6/8] drm: Allow DSI devices to be registered before the host registers. To: Eric Anholt , dri-devel@lists.freedesktop.org, Andrzej Hajda , Laurent Pinchart , Thierry Reding Cc: linux-kernel@vger.kernel.org, Boris Brezillon References: <20170627195839.3338-1-eric@anholt.net> <20170627195839.3338-7-eric@anholt.net> <6fbaa797-1040-48e8-c361-dad4363cd30d@codeaurora.org> <87vamu7hi4.fsf@eliezer.anholt.net> From: Archit Taneja Message-ID: Date: Mon, 17 Jul 2017 09:56:49 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <87vamu7hi4.fsf@eliezer.anholt.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/15/2017 04:28 AM, Eric Anholt wrote: > Archit Taneja writes: > >> On 06/28/2017 01:28 AM, Eric Anholt wrote: >>> When a mipi_dsi_host is registered, the DT is walked to find any child >>> nodes with compatible strings. Those get registered as DSI devices, >>> and most DSI panel drivers are mipi_dsi_drivers that attach to those nodes. >>> >>> There is one special case currently, the adv7533 bridge, where the >>> bridge probes on I2C, and during the bridge attach step it looks up >>> the mipi_dsi_host and registers the mipi_dsi_device (for its own stub >>> mipi_dsi_driver). >>> >>> For the Raspberry Pi panel, though, we also need to attach on I2C (our >>> control bus), but don't have a bridge driver. The lack of a bridge's >>> attach() step like adv7533 uses means that we aren't able to delay the >>> mipi_dsi_device creation until the mipi_dsi_host is present. >>> >>> To fix this, we extend mipi_dsi_device_register_full() to allow being >>> called with a NULL host, which puts the device on a queue waiting for >>> a host to appear. When a new host is registered, we fill in the host >>> value and finish the device creation process. >> >> This is quite a nice idea. The only bothering thing is the info.of_node usage >> varies between child nodes (mipi_dsi_devs) and non-child nodes (i2c control >> bus). >> >> For DSI children expressed in DT, the of_node in info holds the DT node >> corresponding to the DSI child itself. For non-DT ones, this patch assumes >> that info.of_node stores the DSI host DT node. I think it should be okay as >> long as we mention the usage in a comment somewhere. The other option is to >> have a new info.host_node field to keep a track of the host DT node. > > I think maybe you misread the patch? We're using > of_get_parent(dsi->dev.node), which came from info->node, to compare to > host->dev->of_node(). I think I did misread it. Although, I'm not entirely clear what we should be setting info.node to. In patch #8, info.node is set by: endpoint = of_graph_get_next_endpoint(dev->of_node, NULL); info.node = of_graph_get_remote_port(endpoint); Looking at the dt bindings in patch #7, it looks like info.node is set to the 'port' device node in dsi@7e700000, is that right? I suppose 'port' here seems like a reasonable representation of dsi->dev.node, I wonder how it would work if the dsi host had multiple ports underneath it. I.e: dsi@7e700000 { ... ... ports { port@0 { ... dsi_out_port: endpoint { remote-endpoint = <&panel_dsi_port>; }; }; port@1 { ... ... }; }; }; Here, we would need to set info.node to the 'ports' node, so that of_get_parent(dsi->dev.of_node) equals host->dev->of_node. That doesn't seem correct. Ideally, a dev's 'of_node' should be left to NULL if we don't have a corresponding OF node. We're sort of overriding it here since we don't have any other place to store this information in the mipi_dsi_device struct. Maybe we could add a 'host_node' entry in mipi_dsi_device itself, which is exclusively used cases where the DSI device doesn't have a DT node. Our check in mipi_dsi_host_register() could then be something like: if (dsi->host_node) == host->dev->of_node) { ... ... } Since Thierry also reviews drm_mipi_dsi.c changes, it would nice to get some comments from him too. Thanks, Archit -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project From mboxrd@z Thu Jan 1 00:00:00 1970 From: Archit Taneja Subject: Re: [PATCH 6/8] drm: Allow DSI devices to be registered before the host registers. Date: Mon, 17 Jul 2017 09:56:49 +0530 Message-ID: References: <20170627195839.3338-1-eric@anholt.net> <20170627195839.3338-7-eric@anholt.net> <6fbaa797-1040-48e8-c361-dad4363cd30d@codeaurora.org> <87vamu7hi4.fsf@eliezer.anholt.net> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: Received: from smtp.codeaurora.org (smtp.codeaurora.org [198.145.29.96]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0AA3C6E1B8 for ; Mon, 17 Jul 2017 04:26:57 +0000 (UTC) In-Reply-To: <87vamu7hi4.fsf@eliezer.anholt.net> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Eric Anholt , dri-devel@lists.freedesktop.org, Andrzej Hajda , Laurent Pinchart , Thierry Reding Cc: Boris Brezillon , linux-kernel@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org CgpPbiAwNy8xNS8yMDE3IDA0OjI4IEFNLCBFcmljIEFuaG9sdCB3cm90ZToKPiBBcmNoaXQgVGFu ZWphIDxhcmNoaXR0QGNvZGVhdXJvcmEub3JnPiB3cml0ZXM6Cj4gCj4+IE9uIDA2LzI4LzIwMTcg MDE6MjggQU0sIEVyaWMgQW5ob2x0IHdyb3RlOgo+Pj4gV2hlbiBhIG1pcGlfZHNpX2hvc3QgaXMg cmVnaXN0ZXJlZCwgdGhlIERUIGlzIHdhbGtlZCB0byBmaW5kIGFueSBjaGlsZAo+Pj4gbm9kZXMg d2l0aCBjb21wYXRpYmxlIHN0cmluZ3MuICBUaG9zZSBnZXQgcmVnaXN0ZXJlZCBhcyBEU0kgZGV2 aWNlcywKPj4+IGFuZCBtb3N0IERTSSBwYW5lbCBkcml2ZXJzIGFyZSBtaXBpX2RzaV9kcml2ZXJz IHRoYXQgYXR0YWNoIHRvIHRob3NlIG5vZGVzLgo+Pj4KPj4+IFRoZXJlIGlzIG9uZSBzcGVjaWFs IGNhc2UgY3VycmVudGx5LCB0aGUgYWR2NzUzMyBicmlkZ2UsIHdoZXJlIHRoZQo+Pj4gYnJpZGdl IHByb2JlcyBvbiBJMkMsIGFuZCBkdXJpbmcgdGhlIGJyaWRnZSBhdHRhY2ggc3RlcCBpdCBsb29r cyB1cAo+Pj4gdGhlIG1pcGlfZHNpX2hvc3QgYW5kIHJlZ2lzdGVycyB0aGUgbWlwaV9kc2lfZGV2 aWNlIChmb3IgaXRzIG93biBzdHViCj4+PiBtaXBpX2RzaV9kcml2ZXIpLgo+Pj4KPj4+IEZvciB0 aGUgUmFzcGJlcnJ5IFBpIHBhbmVsLCB0aG91Z2gsIHdlIGFsc28gbmVlZCB0byBhdHRhY2ggb24g STJDIChvdXIKPj4+IGNvbnRyb2wgYnVzKSwgYnV0IGRvbid0IGhhdmUgYSBicmlkZ2UgZHJpdmVy LiAgVGhlIGxhY2sgb2YgYSBicmlkZ2Uncwo+Pj4gYXR0YWNoKCkgc3RlcCBsaWtlIGFkdjc1MzMg dXNlcyBtZWFucyB0aGF0IHdlIGFyZW4ndCBhYmxlIHRvIGRlbGF5IHRoZQo+Pj4gbWlwaV9kc2lf ZGV2aWNlIGNyZWF0aW9uIHVudGlsIHRoZSBtaXBpX2RzaV9ob3N0IGlzIHByZXNlbnQuCj4+Pgo+ Pj4gVG8gZml4IHRoaXMsIHdlIGV4dGVuZCBtaXBpX2RzaV9kZXZpY2VfcmVnaXN0ZXJfZnVsbCgp IHRvIGFsbG93IGJlaW5nCj4+PiBjYWxsZWQgd2l0aCBhIE5VTEwgaG9zdCwgd2hpY2ggcHV0cyB0 aGUgZGV2aWNlIG9uIGEgcXVldWUgd2FpdGluZyBmb3IKPj4+IGEgaG9zdCB0byBhcHBlYXIuICBX aGVuIGEgbmV3IGhvc3QgaXMgcmVnaXN0ZXJlZCwgd2UgZmlsbCBpbiB0aGUgaG9zdAo+Pj4gdmFs dWUgYW5kIGZpbmlzaCB0aGUgZGV2aWNlIGNyZWF0aW9uIHByb2Nlc3MuCj4+Cj4+IFRoaXMgaXMg cXVpdGUgYSBuaWNlIGlkZWEuIFRoZSBvbmx5IGJvdGhlcmluZyB0aGluZyBpcyB0aGUgaW5mby5v Zl9ub2RlIHVzYWdlCj4+IHZhcmllcyBiZXR3ZWVuIGNoaWxkIG5vZGVzIChtaXBpX2RzaV9kZXZz KSBhbmQgbm9uLWNoaWxkIG5vZGVzIChpMmMgY29udHJvbAo+PiBidXMpLgo+Pgo+PiBGb3IgRFNJ IGNoaWxkcmVuIGV4cHJlc3NlZCBpbiBEVCwgdGhlIG9mX25vZGUgaW4gaW5mbyBob2xkcyB0aGUg RFQgbm9kZQo+PiBjb3JyZXNwb25kaW5nIHRvIHRoZSBEU0kgY2hpbGQgaXRzZWxmLiBGb3Igbm9u LURUIG9uZXMsIHRoaXMgcGF0Y2ggYXNzdW1lcwo+PiB0aGF0IGluZm8ub2Zfbm9kZSBzdG9yZXMg dGhlIERTSSBob3N0IERUIG5vZGUuIEkgdGhpbmsgaXQgc2hvdWxkIGJlIG9rYXkgYXMKPj4gbG9u ZyBhcyB3ZSBtZW50aW9uIHRoZSB1c2FnZSBpbiBhIGNvbW1lbnQgc29tZXdoZXJlLiBUaGUgb3Ro ZXIgb3B0aW9uIGlzIHRvCj4+IGhhdmUgYSBuZXcgaW5mby5ob3N0X25vZGUgZmllbGQgdG8ga2Vl cCBhIHRyYWNrIG9mIHRoZSBob3N0IERUIG5vZGUuCj4gCj4gSSB0aGluayBtYXliZSB5b3UgbWlz cmVhZCB0aGUgcGF0Y2g/ICBXZSdyZSB1c2luZwo+IG9mX2dldF9wYXJlbnQoZHNpLT5kZXYubm9k ZSksIHdoaWNoIGNhbWUgZnJvbSBpbmZvLT5ub2RlLCB0byBjb21wYXJlIHRvCj4gaG9zdC0+ZGV2 LT5vZl9ub2RlKCkuCgpJIHRoaW5rIEkgZGlkIG1pc3JlYWQgaXQuCgpBbHRob3VnaCwgSSdtIG5v dCBlbnRpcmVseSBjbGVhciB3aGF0IHdlIHNob3VsZCBiZSBzZXR0aW5nIGluZm8ubm9kZSB0by4K SW4gcGF0Y2ggIzgsIGluZm8ubm9kZSBpcyBzZXQgYnk6CgoJZW5kcG9pbnQgPSBvZl9ncmFwaF9n ZXRfbmV4dF9lbmRwb2ludChkZXYtPm9mX25vZGUsIE5VTEwpOwoJaW5mby5ub2RlID0gb2ZfZ3Jh cGhfZ2V0X3JlbW90ZV9wb3J0KGVuZHBvaW50KTsKCkxvb2tpbmcgYXQgdGhlIGR0IGJpbmRpbmdz IGluIHBhdGNoICM3LCBpdCBsb29rcyBsaWtlIGluZm8ubm9kZSBpcyBzZXQKdG8gdGhlICdwb3J0 JyBkZXZpY2Ugbm9kZSBpbiBkc2lAN2U3MDAwMDAsIGlzIHRoYXQgcmlnaHQ/CgpJIHN1cHBvc2Ug J3BvcnQnIGhlcmUgc2VlbXMgbGlrZSBhIHJlYXNvbmFibGUgcmVwcmVzZW50YXRpb24gb2YKZHNp LT5kZXYubm9kZSwgSSB3b25kZXIgaG93IGl0IHdvdWxkIHdvcmsgaWYgdGhlIGRzaSBob3N0IGhh ZCBtdWx0aXBsZQpwb3J0cyB1bmRlcm5lYXRoIGl0LiBJLmU6Cgpkc2lAN2U3MDAwMDAgewoJLi4u CgkuLi4KCXBvcnRzIHsKCQlwb3J0QDAgewoJCQkuLi4KCQkJZHNpX291dF9wb3J0OiBlbmRwb2lu dCB7CgkJCQlyZW1vdGUtZW5kcG9pbnQgPSA8JnBhbmVsX2RzaV9wb3J0PjsKCQkJfTsKCQl9OwoJ CXBvcnRAMSB7CgkJCS4uLgoJCQkuLi4KCQl9OwoJfTsKfTsKCkhlcmUsIHdlIHdvdWxkIG5lZWQg dG8gc2V0IGluZm8ubm9kZSB0byB0aGUgJ3BvcnRzJyBub2RlLCBzbyB0aGF0Cm9mX2dldF9wYXJl bnQoZHNpLT5kZXYub2Zfbm9kZSkgZXF1YWxzIGhvc3QtPmRldi0+b2Zfbm9kZS4gVGhhdCBkb2Vz bid0CnNlZW0gY29ycmVjdC4KCklkZWFsbHksIGEgZGV2J3MgJ29mX25vZGUnIHNob3VsZCBiZSBs ZWZ0IHRvIE5VTEwgaWYgd2UgZG9uJ3QgaGF2ZSBhCmNvcnJlc3BvbmRpbmcgT0Ygbm9kZS4gV2Un cmUgc29ydCBvZiBvdmVycmlkaW5nIGl0IGhlcmUgc2luY2Ugd2UgZG9uJ3QKaGF2ZSBhbnkgb3Ro ZXIgcGxhY2UgdG8gc3RvcmUgdGhpcyBpbmZvcm1hdGlvbiBpbiB0aGUgbWlwaV9kc2lfZGV2aWNl CnN0cnVjdC4KCk1heWJlIHdlIGNvdWxkIGFkZCBhICdob3N0X25vZGUnIGVudHJ5IGluIG1pcGlf ZHNpX2RldmljZSBpdHNlbGYsIHdoaWNoCmlzIGV4Y2x1c2l2ZWx5IHVzZWQgY2FzZXMgd2hlcmUg dGhlIERTSSBkZXZpY2UgZG9lc24ndCBoYXZlIGEgRFQgbm9kZS4KT3VyIGNoZWNrIGluIG1pcGlf ZHNpX2hvc3RfcmVnaXN0ZXIoKSBjb3VsZCB0aGVuIGJlIHNvbWV0aGluZyBsaWtlOgoKCWlmIChk c2ktPmhvc3Rfbm9kZSkgPT0gaG9zdC0+ZGV2LT5vZl9ub2RlKSB7CgkJLi4uCgkJLi4uCgl9CgpT aW5jZSBUaGllcnJ5IGFsc28gcmV2aWV3cyBkcm1fbWlwaV9kc2kuYyBjaGFuZ2VzLCBpdCB3b3Vs ZCBuaWNlIHRvCmdldCBzb21lIGNvbW1lbnRzIGZyb20gaGltIHRvby4KClRoYW5rcywKQXJjaGl0 CgotLSAKUXVhbGNvbW0gSW5ub3ZhdGlvbiBDZW50ZXIsIEluYy4gaXMgYSBtZW1iZXIgb2YgQ29k ZSBBdXJvcmEgRm9ydW0sCmEgTGludXggRm91bmRhdGlvbiBDb2xsYWJvcmF0aXZlIFByb2plY3QK X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg==