On Thu, Dec 09, 2021 at 09:44:49AM +0100, Wolfram Sang wrote: > > > client->dev.parent = &client->adapter->dev; > > client->dev.bus = &i2c_bus_type; > > client->dev.type = &i2c_client_type; > > - client->dev.of_node = of_node_get(info->of_node); > > - client->dev.fwnode = info->fwnode; > > > > device_enable_async_suspend(&client->dev); > > i2c_dev_set_name(adap, client, info); > > > > + device_set_node(&client->dev, info->fwnode); > > + client->dev.of_node = of_node_get(info->of_node); > > + > > I am basically OK with this change. I'd just move the code block a > little to have the same behaviour as before. Something like this > (hand-edited preview version): And this is exactly what have been done. > > client->dev.bus = &i2c_bus_type; > > client->dev.type = &i2c_client_type; > > client->dev.of_node = of_node_get(info->of_node); > > - client->dev.fwnode = info->fwnode; > > > > + device_set_node(&client->dev, info->fwnode); > > device_enable_async_suspend(&client->dev); > > i2c_dev_set_name(adap, client, info); > > Are you okay with that? It will be broken in this case. There is subtle detail about device_set_node(), it sets both fwnode and of_node at once. More thinking about this, I admit that the best strategy now is to drop this change for now. -- With Best Regards, Andy Shevchenko