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

[Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>]

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:

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

  Tomi