[PATCH v1 1/1] media: i2c: ds90ub960: Remove of_node assignment

[PATCH v1 1/1] media: i2c: ds90ub960: Remove of_node assignment

[Andy Shevchenko]

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,
 	};
-- 
2.47.2

Re: [PATCH v1 1/1] media: i2c: ds90ub960: Remove of_node assignment

[Tomi Valkeinen]

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

Re: [PATCH v1 1/1] media: i2c: ds90ub960: Remove of_node assignment

[Andy Shevchenko]

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

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] media: i2c: ds90ub960: Remove of_node assignment

[Andy Shevchenko]

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).

-- 
With Best Regards,
Andy Shevchenko