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