From patchwork Mon Feb 5 16:11:30 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thierry Reding X-Patchwork-Id: 10200879 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id E4683601A1 for ; Mon, 5 Feb 2018 16:11:38 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5FCE028618 for ; Mon, 5 Feb 2018 16:11:38 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5413D28669; Mon, 5 Feb 2018 16:11:38 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 8F53928618 for ; Mon, 5 Feb 2018 16:11:37 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 044876E2C8; Mon, 5 Feb 2018 16:11:36 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-qk0-x243.google.com (mail-qk0-x243.google.com [IPv6:2607:f8b0:400d:c09::243]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0D5EE6E2C8 for ; Mon, 5 Feb 2018 16:11:33 +0000 (UTC) Received: by mail-qk0-x243.google.com with SMTP id d125so12694530qkg.13 for ; Mon, 05 Feb 2018 08:11:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=SRz4tWPIo3bVaDD+jeU+NaP2VtTTqa2mNMo0khAUftU=; b=DS4zjGDelUZ/I7d3knX1/guFDvP6mmzQDaTsg/HwyF7Ibu7JLGUM7SQhOb5gnHIr9i hiHNpDd19qJkgQsVn+0zZeFrvyTjPIrMsqBweONAGUQcciLVpeRFP1+zcIIoGjXPrIvk /0UduqMd91rV0z+DQKhfh6YtfbKrkWVfzOV+HdpxQ7Qq5oojbRDT5P91BJ7aTj97PmDM Uxi4+Kge17wWXmsJu0tccfS6bicH4Jb1iao+ULcPegHTbzh6KdJUNgjhS8qAe4+lZyhQ co6bJ90L05R8nr30Bhuy1lN3KPrcDa184t9//NQdWB+eWJsxLZ5dhUMmxZyqJHyCT+40 40EA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=SRz4tWPIo3bVaDD+jeU+NaP2VtTTqa2mNMo0khAUftU=; b=Ei0yMiq+TfYpiSh4JUSYA3ZagnU/Zq7eo96SzAwez+ybYT8yGgwrs+Qr9Gvn7tAjbO NeFN+GjabiQi6FzcwZHZ11hnLQD4NMuXMZRHfGIExtjZaiocHL9olQNUoIszUdwOLkWa 7dIjx9x5c+jxeFYTO/Tp8SdiA3Wk5w+x1Dbp6z0J/+4Nm1U5N+CrGml+/F6jNa8Fl2uo HySwAXNCYkNU0uvfl7dZrtjaQt/qH/OnnuitsgVUKShCRQTWodE1tkDqDfcGARF7Hqip duAs3rHUg/AsKYRXCOIrkerdussKKIE2kl2rlkelN0mPV02w0vqUPxrkmN6ZW1KW9tXY +Qeg== X-Gm-Message-State: AKwxytfNYqrstqU+oE9NPF9CJHuV4Z6EFf0WyuGj03KN83oAS23o6cLk gO/U1XzhQ+QJnTthjCM3T2M= X-Google-Smtp-Source: AH8x224of/wuiguJnrAsRVqCZhUmkfd1IFfofnPUaZ5GrU9QrRjsx7ztiwUVesrQhJafVwau34q6fg== X-Received: by 10.55.71.149 with SMTP id u143mr77995001qka.240.1517847092742; Mon, 05 Feb 2018 08:11:32 -0800 (PST) Received: from localhost (p200300E41F41B00009B231EAB1BE76D4.dip0.t-ipconnect.de. [2003:e4:1f41:b000:9b2:31ea:b1be:76d4]) by smtp.gmail.com with ESMTPSA id v27sm2820684qtb.53.2018.02.05.08.11.31 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 05 Feb 2018 08:11:32 -0800 (PST) Date: Mon, 5 Feb 2018 17:11:30 +0100 From: Thierry Reding To: Lucas Stach Subject: Re: [PATCH] drm: don't link DP aux i2c adapter to the hardware device node Message-ID: <20180205161130.GA3493@ulmo> References: <20170113173630.22138-1-l.stach@pengutronix.de> <20170123163347.GC2043@ulmo.ba.sec> <1491382352.2904.9.camel@pengutronix.de> <20170405120431.GA9162@ulmo.ba.sec> MIME-Version: 1.0 In-Reply-To: <20170405120431.GA9162@ulmo.ba.sec> User-Agent: Mutt/1.9.1 (2017-09-22) X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Wolfram Sang , dri-devel , patchwork-lst@pengutronix.de, linux-i2c@vger.kernel.org, "kernel@pengutronix.de" , Daniel Vetter Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Apr 05, 2017 at 02:04:31PM +0200, Thierry Reding wrote: > On Wed, Apr 05, 2017 at 10:52:32AM +0200, Lucas Stach wrote: > > Hi Rob, > > > > Am Mittwoch, den 29.03.2017, 08:56 -0500 schrieb Rob Herring: > > > On Mon, Jan 23, 2017 at 10:33 AM, Thierry Reding > > > wrote: > > > > On Fri, Jan 13, 2017 at 06:36:30PM +0100, Lucas Stach wrote: > > > >> The i2c adapter on DP AUX is purely a software construct. Linking > > > >> it to the device node of the parent device is wrong, as it leads to > > > >> 2 devices sharing the same device node, which is bad practice, as > > > > > > > > Who says that two devices can't share the same device node? It's done > > > > all the time. > > > > > > It's done *some of the time* and I would not consider it best practice. > > > > > > >> well as the i2c trying to populate children of the i2c adapter by > > > >> looking at the child device nodes of the parent device. > > > > > > > > A set of patches landed in v4.9 to work around this issue in a better > > > > way. See: > > > > > > > > 98b00488459e dt-bindings: i2c: Add support for 'i2c-bus' subnode > > > > 7e4c224abfe8 i2c: core: Add support for 'i2c-bus' subnode > > > > > > What does this buy us? I don't see why this needs to be in DT either. > > > Contrary to popular belief, DT is not the only way to instantiate > > > devices, C code can still do it. > > > > > > Also, if this one line removal has no side effects, then how was it > > > even needed? We can always add it back if there's some argument for > > > why it is needed. > > > > Okay, so I take this as you mostly agreeing with the rationale of this > > patch. > > For some general background on this: I was originally using this for DP > support on Tegra (though that ended up never getting merged because of a > particularily frustrating episode of trying to get better link training > support into the core helpers) and use it as a means to obtain the I2C > controller used for DDC. On Tegra, and I suspect other devices as well, > the DP AUX controller is separate from the encoder, so the idea was to > link them together using a standard ddc-i2c-bus phandle. > > I ended up not needing that because the encoder and DP AUX controller > are so tightly linked on Tegra that I need direct access to the DP AUX > anyway and can therefore directly get the I2C controller from that. > > If there aren't any other users of this, I suppose we could simply > remove the line. Should someone turn up in the future and require the > I2C controller to be looked up from a phandle we could add it again, > at which point we'd have to investigate again how to get rid of the > errors. > > Acked-by: Thierry Reding I'm going to have to retract that: I just noticed that this patch breaks eDP for Venice2 (and presumably all Tegra124 Nyan boards as well, though I don't have those to test with). My description above isn't quite correct. For eDP device we do use the ddc-i2c-bus property in DT to denote which I2C bus to use for probing the EDID. So the reason why eDP now breaks is because the simple-panel driver will look for the I2C adapter, not find a matching one and defer probe (indefinitely). A, perhaps nicer, alternative I found to make it work is the below patch. Would that be more reasonable? Looping in Wolfram. Thierry --- >8 --- diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c index 8d474bb1dc15..f88527a61cd1 100644 --- a/drivers/i2c/i2c-core-of.c +++ b/drivers/i2c/i2c-core-of.c @@ -118,6 +118,14 @@ static int of_dev_node_match(struct device *dev, void *data) return dev->of_node == data; } +static int of_parent_node_match(struct device *dev, void *data) +{ + if (dev->parent) + return dev->parent->of_node == data; + + return 0; +} + /* must call put_device() when done with returned i2c_client device */ struct i2c_client *of_find_i2c_device_by_node(struct device_node *node) { @@ -143,6 +151,9 @@ struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node) struct i2c_adapter *adapter; dev = bus_find_device(&i2c_bus_type, NULL, node, of_dev_node_match); + if (!dev) + dev = bus_find_device(&i2c_bus_type, NULL, node, of_parent_node_match); + if (!dev) return NULL;