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] [day] [month] [year] [list]
Message-ID: <ZADL62W6Mv6uNjKf@smile.fi.intel.com>
Date:   Thu, 2 Mar 2023 18:16:43 +0200
From:   Andy Shevchenko <andriy.shevchenko@...el.com>
To:     unlisted-recipients:; (no To-header on input)
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

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

-- 
With Best Regards,
Andy Shevchenko