From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751408AbdGQNjM (ORCPT ); Mon, 17 Jul 2017 09:39:12 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:53106 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751278AbdGQNjK (ORCPT ); Mon, 17 Jul 2017 09:39:10 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org C74EA602BC 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 , Andrzej Hajda , dri-devel@lists.freedesktop.org, 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> <37d3cc1b-a27a-9dba-4bec-2d06bea33858@codeaurora.org> <87shhy7hdt.fsf@eliezer.anholt.net> From: Archit Taneja Message-ID: <8d40ab2b-2d1d-5d6d-0bab-afa536bf6a32@codeaurora.org> Date: Mon, 17 Jul 2017 19:09:04 +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: <87shhy7hdt.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:31 AM, Eric Anholt wrote: > Archit Taneja writes: > >> On 06/29/2017 04:09 PM, Andrzej Hajda wrote: >>> On 29.06.2017 07:03, Archit Taneja wrote: >>>> >>>> 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. >>> >>> Field abuse is not a biggest issue. >>> >>> This patch changes totally semantic of mipi_dsi_device_register_full. >>> Currently semantic of *_device_register* function is to create and add >>> device to existing bus, ie after return we have device attached to bus, >>> so it can be instantly used. With this change function can return some >>> unattached device, without warranty it will be ever attached - kind of >>> hidden deferring. Such change looks for me quite dangerous, even if it >>> looks convenient in this case. >>> >>> As discussed in other thread more appealing solution for me would be: >>> 1. host creates dsi bus, but doesn't call component_add as it does not >>> have all required resources. >>> 2. host waits for all required dsi devs attached, gets registered panels >>> or bridges and calls component_add after that. >>> 3. in bind phase it has all it needs, hasn't it? >> >> This seems like it would work, but would require KMS drivers to restructure >> themselves around this approach. For KMS drivers that don't even use the >> component stuff, it might be asking too much. >> >> We could maybe consider Eric's patch as an intermediate solution, we should >> definitely put WARN_ON(!dsi->host) like checks for all the existing >> mipi_dsi_*() API. > > Could you clarify which entrypoints you'd like a warning on? Is it just > "everything that gets the host ops"? Sorry, all API was a bad suggestion. I think a warning and early bail in mipi_dsi_attach() should be sufficient. A panel/bridge DSI device shouldn't use any other API if it hasn't called mipi_dsi_attach(). Thanks, Archit > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project