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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 2 Mar 2023 17:52:24 +0200
From:   Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To:     Andy Shevchenko <andriy.shevchenko@...el.com>
Cc:     linux-media@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Wolfram Sang <wsa@...nel.org>,
        Luca Ceresoli <luca.ceresoli@...tlin.com>,
        Matti Vaittinen <Matti.Vaittinen@...rohmeurope.com>,
        Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Peter Rosin <peda@...ntia.se>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Michael Tretter <m.tretter@...gutronix.de>,
        Shawn Tu <shawnx.tu@...el.com>,
        Hans Verkuil <hverkuil@...all.nl>,
        Mike Pagano <mpagano@...too.org>,
        Krzysztof Hałasa <khalasa@...p.pl>,
        Marek Vasut <marex@...x.de>,
        Satish Nagireddy <satish.nagireddy@...cruise.com>
Subject: Re: [PATCH v9 0/8] i2c-atr and FPDLink

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@...00000/segment@...arget-module@...00/i2c@...eser@...links/link@...erializer/i2c/sensor@...port/endpoint
[   36.969268] i2c 4-0044: Fixed dependency cycle(s) with /ocp/interconnect@...00000/segment@...arget-module@...00/i2c@...eser@...ports/port@...ndpoint
[   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@...00000/segment@...arget-module@...00/i2c@...eser@...links/link@...erializer/ports/port@...ndpoint
[   37.033843] i2c 5-0021: Fixed dependency cycle(s) with /ocp/interconnect@...00000/segment@...arget-module@...00/i2c@...eser@...links/link@...erializer
[   37.117492] i2c 4-0045: Fixed dependency cycle(s) with /ocp/interconnect@...00000/segment@...arget-module@...00/i2c@...eser@...links/link@...erializer/i2c/sensor@...port/endpoint
[   37.134033] i2c 4-0045: Fixed dependency cycle(s) with /ocp/interconnect@...00000/segment@...arget-module@...00/i2c@...eser@...ports/port@...ndpoint
[   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@...00000/segment@...arget-module@...00/i2c@...eser@...links/link@...erializer/ports/port@...ndpoint
[   37.202636] i2c 6-0021: Fixed dependency cycle(s) with /ocp/interconnect@...00000/segment@...arget-module@...00/i2c@...eser@...links/link@...erializer

Then again, I see similar warnings/errors for some other devices too, when booting up (TI's DRA76 EVM).

  Tomi