From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH] drm: don't link DP aux i2c adapter to the hardware device node Date: Wed, 5 Apr 2017 14:04:31 +0200 Message-ID: <20170405120431.GA9162@ulmo.ba.sec> References: <20170113173630.22138-1-l.stach@pengutronix.de> <20170123163347.GC2043@ulmo.ba.sec> <1491382352.2904.9.camel@pengutronix.de> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1592174627==" Return-path: Received: from mail-wr0-x241.google.com (mail-wr0-x241.google.com [IPv6:2a00:1450:400c:c0c::241]) by gabe.freedesktop.org (Postfix) with ESMTPS id 86C256E7B3 for ; Wed, 5 Apr 2017 12:04:34 +0000 (UTC) Received: by mail-wr0-x241.google.com with SMTP id w43so2080279wrb.1 for ; Wed, 05 Apr 2017 05:04:34 -0700 (PDT) In-Reply-To: <1491382352.2904.9.camel@pengutronix.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Lucas Stach Cc: patchwork-lst@pengutronix.de, dri-devel , "kernel@pengutronix.de" , Daniel Vetter List-Id: dri-devel@lists.freedesktop.org --===============1592174627== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Q68bSM7Ycu6FN28Q" Content-Disposition: inline --Q68bSM7Ycu6FN28Q Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Apr 05, 2017 at 10:52:32AM +0200, Lucas Stach wrote: > Hi Rob, >=20 > 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. > >=20 > > It's done *some of the time* and I would not consider it best practice. > >=20 > > >> 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' subn= ode > > > 7e4c224abfe8 i2c: core: Add support for 'i2c-bus' subnode > >=20 > > 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. > >=20 > > 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. >=20 > 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 --Q68bSM7Ycu6FN28Q Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAljk3UsACgkQ3SOs138+ s6Fi+hAAvGQPM2To3R7z+CZU+NmiyCnoqvR0Vph7xp7mKIQA+LaK9IrfKawOVFWx qchKEWrMUwsMe09Hrf3xgOT79JyCRG+VYh8My2EDL4VMgmTeuPnXb9G0HKd+fT/8 V01kXnt/z7CkA0uUzJzqqpDFPGlP4bZeAIO3Xfr+keTD1cLywWzAJ/Mi9tEfVpFU Imgh833e3atRDVBECS5b94FdeG0CW4aCHb7dLlXcvxkTVEY1TSQgVLRr+/To8onz B23kjEj0J8KEv44xq282L4KNfOrhd8M3kqcl3cdTpyHe570YEXMYL3kCpzfPej/P oUpDmKOFii9Qo+PdRh2JejLHjxYebbGEFomKlg0h3+ets3/lHh+cO0ZVdDzawdJF g6GPtkJwIRfP7wD0EIgwTnRoerhtD0iBjPN/mrYCOmwiUl8Eo1yBSrxFt5jL8+vL 1w95N8eDAWZmoo5+ifDiroNhZup4ynXPn6OKRBOMDDaJBfZR3NwIu8W7XTgH7c28 4E4Nprv5cBv4WWNV0iNW/E+fuAAc3wVpA/OPlNLepI5VgyL3xBMOJ768xItNPnBH GOTtOw8Gj+qApW73Z9seZ2SbUF2W7vXW0lFlJthVck1YdU/TFIs/OMkDLeGkPKPT WaxJSNVXmWoKl/HCwd9f6Si7/kCrmjzptMIqQTZ6J2bV1FHGbqM= =HJfy -----END PGP SIGNATURE----- --Q68bSM7Ycu6FN28Q-- --===============1592174627== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1592174627==--