Re: [PATCH v9 0/8] i2c-atr and FPDLink

[Andy Shevchenko]

+Cc: Saravana (for devlink issues)

On Thu, Mar 02, 2023 at 05:52:24PM +0200, Tomi Valkeinen wrote:
> On 17/02/2023 15:45, Andy Shevchenko wrote:
> > On Fri, Feb 17, 2023 at 02:57:02PM +0200, Tomi Valkeinen wrote:
> > > On 17/02/2023 13:24, Andy Shevchenko wrote:
> > > > On Fri, Feb 17, 2023 at 08:57:32AM +0200, Tomi Valkeinen wrote:
> > > > > On 16/02/2023 17:53, Andy Shevchenko wrote:
> > > > > > On Thu, Feb 16, 2023 at 04:07:39PM +0200, Tomi Valkeinen wrote:

...

> > > > > > >     	struct i2c_board_info ser_info = {
> > > > > > > -		.of_node = to_of_node(rxport->remote_fwnode),
> > > > > > > -		.fwnode = rxport->remote_fwnode,
> > > > > > 
> > > > > > > +		.of_node = to_of_node(rxport->ser.fwnode),
> > > > > > > +		.fwnode = rxport->ser.fwnode,
> > > > > > 
> > > > > > Why do you need to have both?!
> > > > > 
> > > > > I didn't debug it, but having only fwnode there will break the probing (no
> > > > > match).
> > > > 
> > > > This needs to be investigated. The whole fwnode approach, when we have both
> > > > fwnode and legacy of_node fields in the same data structure, is that fwnode
> > > > _OR_ of_node initialization is enough, when both are defined the fwnode
> > > > should take precedence.
> > > > 
> > > > If your testing is correct (and I have no doubts) it means we have a serious
> > > > bug lurking somewhere.
> > > 
> > > Having both defined or only of_node defined works for me.
> > 
> > But of_node is _legacy_ stuff. We should not really consider this option in the
> > new code.
> > 
> > > Perhaps the issue is that these drivers only add of_match_table, and thus
> > > having only .fwnode above is not enough.
> > 
> > No, the code should work with fwnode that carrying DT node or another.
> > The matching table shouldn't affect this either.
> > 
> > > Looking at i2c_device_match(), i2c_of_match_device() only uses of_node, so
> > > perhaps I would need CONFIG_ACPI for acpi_driver_match_device to do matching
> > > with of_node? Although I don't see the acpi code using fwnode, just of_node.
> > > Well, I have to say I have no idea without spending more time on this.
> > 
> > Again, there is a bug and that bug seems nasty one as it would allow to
> > work the device in one environment and not in another.
> > 
> > Since it's about I²C board files, I believe that an issue is in I²C core.
> 
> I don't know if this is related in any way, but I see these when probing:

I believe this is for devlink (Saravana Cc'ed).

> [   36.952697] i2c 4-0044: Fixed dependency cycle(s) with /ocp/interconnect@48000000/segment@0/target-module@7c000/i2c@0/deser@3d/links/link@0/serializer/i2c/sensor@21/port/endpoint
> [   36.969268] i2c 4-0044: Fixed dependency cycle(s) with /ocp/interconnect@48000000/segment@0/target-module@7c000/i2c@0/deser@3d/ports/port@0/endpoint
> [   36.983001] i2c 4-0044: Failed to create device link with 4-0044
> [   36.992828] ds90ub953 4-0044: Found ub953 rev/mask 0x20
> [   37.017761] i2c 5-0021: Fixed dependency cycle(s) with /ocp/interconnect@48000000/segment@0/target-module@7c000/i2c@0/deser@3d/links/link@0/serializer/ports/port@0/endpoint
> [   37.033843] i2c 5-0021: Fixed dependency cycle(s) with /ocp/interconnect@48000000/segment@0/target-module@7c000/i2c@0/deser@3d/links/link@0/serializer
> [   37.117492] i2c 4-0045: Fixed dependency cycle(s) with /ocp/interconnect@48000000/segment@0/target-module@7c000/i2c@0/deser@3d/links/link@1/serializer/i2c/sensor@21/port/endpoint
> [   37.134033] i2c 4-0045: Fixed dependency cycle(s) with /ocp/interconnect@48000000/segment@0/target-module@7c000/i2c@0/deser@3d/ports/port@1/endpoint
> [   37.147735] i2c 4-0045: Failed to create device link with 4-0045
> [   37.156097] ds90ub953 4-0045: Found ub953 rev/mask 0x20
> [   37.186584] i2c 6-0021: Fixed dependency cycle(s) with /ocp/interconnect@48000000/segment@0/target-module@7c000/i2c@0/deser@3d/links/link@1/serializer/ports/port@0/endpoint
> [   37.202636] i2c 6-0021: Fixed dependency cycle(s) with /ocp/interconnect@48000000/segment@0/target-module@7c000/i2c@0/deser@3d/links/link@1/serializer
> 
> Then again, I see similar warnings/errors for some other devices too, when booting up (TI's DRA76 EVM).

-- 
With Best Regards,
Andy Shevchenko