On Tue, Nov 14, 2017 at 03:34:19PM +0100, Andrzej Hajda wrote: > On 05.04.2017 14:04, 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 > >>> <thierry.reding@xxxxxxxxx> 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 <treding@xxxxxxxxxx> > > It looks like the patch has been accepted but not merged. Who can pick > it up? or is it OK if I put it int drm-misc? I don't have the patch itself, but sounds good. Just go ahead and push. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
