| Message ID | 20250312174123.4111661-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/1] media: i2c: ds90ub960: Remove of_node assignment | expand |
Hi, On 12/03/2025 19:41, Andy Shevchenko wrote: > Remove of_node assignment which duplicates fwnode in struct i2c_board_info. > In general drivers must not set both, it's quite confusing. The I²C core > will consider fwnode with a priority and of_node is subject to remove from > above mentioned data structure. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/media/i2c/ds90ub960.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c > index 5dde8452739b..5afdbbad9ff4 100644 > --- a/drivers/media/i2c/ds90ub960.c > +++ b/drivers/media/i2c/ds90ub960.c > @@ -1682,7 +1682,6 @@ static int ub960_rxport_add_serializer(struct ub960_data *priv, u8 nport) > struct device *dev = &priv->client->dev; > struct ds90ub9xx_platform_data *ser_pdata = &rxport->ser.pdata; > struct i2c_board_info ser_info = { > - .of_node = to_of_node(rxport->ser.fwnode), > .fwnode = rxport->ser.fwnode, > .platform_data = ser_pdata, > }; This sounds logical, but breaks the driver for me. I also couldn't find this documented and didn't immediately find this from the i2c core implementation side. Or am I missing some patch (running on v6.14)? Tomi
On Thu, Apr 03, 2025 at 02:01:40PM +0300, Tomi Valkeinen wrote: > On 12/03/2025 19:41, Andy Shevchenko wrote: > > Remove of_node assignment which duplicates fwnode in struct i2c_board_info. > > In general drivers must not set both, it's quite confusing. The I²C core > > will consider fwnode with a priority and of_node is subject to remove from > > above mentioned data structure. ... > > struct device *dev = &priv->client->dev; > > struct ds90ub9xx_platform_data *ser_pdata = &rxport->ser.pdata; > > struct i2c_board_info ser_info = { > > - .of_node = to_of_node(rxport->ser.fwnode), > > .fwnode = rxport->ser.fwnode, > > .platform_data = ser_pdata, > > }; > > This sounds logical, but breaks the driver for me. The above assignments are still confusing, but I think I understand what's going on. You run on OF based environment and some code uses of_*() APIs instead of fwnode ones. That's why the current implementation in the I²C core doesn't work. > I also couldn't find this documented and didn't immediately find this from > the i2c core implementation side. The current code does this: client->dev.of_node = of_node_get(info->of_node); client->dev.fwnode = info->fwnode; when it should do something like device_set_node() instead. > Or am I missing some patch (running on v6.14)? Yeah, there is a series in the mailing list [1], but it has no comments and no move so far... TL;DR: it seems in this change is incomplete. [1]: https://lore.kernel.org/r/20250312185137.4154173-1-andriy.shevchenko@linux.intel.com
On Thu, Apr 03, 2025 at 02:12:36PM +0300, Andy Shevchenko wrote: > On Thu, Apr 03, 2025 at 02:01:40PM +0300, Tomi Valkeinen wrote: > > On 12/03/2025 19:41, Andy Shevchenko wrote: > > > Remove of_node assignment which duplicates fwnode in struct i2c_board_info. > > > In general drivers must not set both, it's quite confusing. The I²C core > > > will consider fwnode with a priority and of_node is subject to remove from > > > above mentioned data structure. FWIW, this patch had been send as part of the i2c conversion series. Please, do not apply it standalone (at least without that i2c stuff being accepted and applied first).
diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c index 5dde8452739b..5afdbbad9ff4 100644 --- a/drivers/media/i2c/ds90ub960.c +++ b/drivers/media/i2c/ds90ub960.c @@ -1682,7 +1682,6 @@ static int ub960_rxport_add_serializer(struct ub960_data *priv, u8 nport) struct device *dev = &priv->client->dev; struct ds90ub9xx_platform_data *ser_pdata = &rxport->ser.pdata; struct i2c_board_info ser_info = { - .of_node = to_of_node(rxport->ser.fwnode), .fwnode = rxport->ser.fwnode, .platform_data = ser_pdata, };
Remove of_node assignment which duplicates fwnode in struct i2c_board_info. In general drivers must not set both, it's quite confusing. The I²C core will consider fwnode with a priority and of_node is subject to remove from above mentioned data structure. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/media/i2c/ds90ub960.c | 1 - 1 file changed, 1 deletion(-)