On Wed, Feb 06, 2019 at 10:49:12AM +0100, Wolfram Sang wrote: > > > And there is a regression! Good that I didn't push out before > > double-checking. No one noticed that this breaks registering child > > devices because of_i2c_register_devices() doesn't have a pointer to work > > with anymore? > > Well, sorry, I forgot an important detail. There is no regression > because most drivers still populate adap->dev.of_data with the node > pointer of their parent. I experimentally removed this from my driver > under test motivated by this comment from the commit in the Fixes: tag: > > "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," > > But removing this bad practice from I2C core is more work. I wonder now > if we are in some inconsistent in-between state if I apply this patch as > is? I think this patch would serve as preparatory work to remove the sharing of device nodes. There shouldn't be any regressions here because we only fall back to the parent's ->of_node if the I2C adapter's ->of_node does not match. Since the I2C adapter's ->of_node match is what we currently do, the only thing that this patch does is add a fallback for the cases where the I2C adapter's ->of_node is not set. As far as I can tell, the only code where this should matter is the drm_dp_aux helpers where the I2C adapter's ->of_node is no longer being set because of the commit that introduced the regression for Tegra124 Nyan (and Venice2) boards. So I think this patch is safe to apply and as you suggested this can be used as the baseline for cleaning up all the cases where we reuse the parent's ->of_node for the I2C adapter's ->of_node. So I guess you could say we're in some in-between state, but I don't think it's inconsistent. It just allows us to do this step by step, which I think is good. Thierry
Attachment:
signature.asc
Description: PGP signature
