linux-kernel - Re: [PATCH v4 5/8] i2c: i2c-core-of: follow i2c-parent phandle to probe devices from added nodes

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]

Message-ID: <20241213122826.1c01a284@booty>
Date: Fri, 13 Dec 2024 12:28:26 +0100
From: Luca Ceresoli <luca.ceresoli@...tlin.com>
To: "Sverdlin, Alexander" <alexander.sverdlin@...mens.com>
Cc: "rafael@...nel.org" <rafael@...nel.org>, "jingoohan1@...il.com"
 <jingoohan1@...il.com>, "robh@...nel.org" <robh@...nel.org>,
 "rfoss@...nel.org" <rfoss@...nel.org>, "Laurent.pinchart@...asonboard.com"
 <Laurent.pinchart@...asonboard.com>, "mripard@...nel.org"
 <mripard@...nel.org>, "tzimmermann@...e.de" <tzimmermann@...e.de>,
 "daniel@...ll.ch" <daniel@...ll.ch>, "conor+dt@...nel.org"
 <conor+dt@...nel.org>, "maarten.lankhorst@...ux.intel.com"
 <maarten.lankhorst@...ux.intel.com>, "andrzej.hajda@...el.com"
 <andrzej.hajda@...el.com>, "daniel.thompson@...aro.org"
 <daniel.thompson@...aro.org>, "wsa+renesas@...g-engineering.com"
 <wsa+renesas@...g-engineering.com>, "lee@...nel.org" <lee@...nel.org>,
 "krzk+dt@...nel.org" <krzk+dt@...nel.org>, "arnd@...db.de" <arnd@...db.de>,
 "jonas@...boo.se" <jonas@...boo.se>, "saravanak@...gle.com"
 <saravanak@...gle.com>, "airlied@...il.com" <airlied@...il.com>,
 "dragan.cvetic@....com" <dragan.cvetic@....com>,
 "neil.armstrong@...aro.org" <neil.armstrong@...aro.org>,
 "derek.kiernan@....com" <derek.kiernan@....com>, "deller@....de"
 <deller@....de>, "jernej.skrabec@...il.com" <jernej.skrabec@...il.com>,
 "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
 "linux-fbdev@...r.kernel.org" <linux-fbdev@...r.kernel.org>,
 "thomas.petazzoni@...tlin.com" <thomas.petazzoni@...tlin.com>,
 "contact@...lk.fr" <contact@...lk.fr>, "herve.codina@...tlin.com"
 <herve.codina@...tlin.com>, "dri-devel@...ts.freedesktop.org"
 <dri-devel@...ts.freedesktop.org>, "devicetree@...r.kernel.org"
 <devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
 <linux-kernel@...r.kernel.org>, "paul.kocialkowski@...tlin.com"
 <paul.kocialkowski@...tlin.com>, "linux-i2c@...r.kernel.org"
 <linux-i2c@...r.kernel.org>
Subject: Re: [PATCH v4 5/8] i2c: i2c-core-of: follow i2c-parent phandle to
 probe devices from added nodes

Hello Alexander,

On Thu, 12 Dec 2024 19:12:02 +0000
"Sverdlin, Alexander" <alexander.sverdlin@...mens.com> wrote:

> Hi Luca!
> 
> On Tue, 2024-09-17 at 10:53 +0200, Luca Ceresoli wrote:
> > When device tree nodes are added, the I2C core tries to probe client
> > devices based on the classic DT structure:
> > 
> >   i2c@...d0000 {
> >       some-client@42 { compatible = "xyz,blah"; ... };
> >   };
> > 
> > However for hotplug connectors described via device tree overlays there is
> > additional level of indirection, which is needed to decouple the overlay
> > and the base tree:
> > 
> >   --- base device tree ---
> > 
> >   i2c1: i2c@...d0000 { compatible = "xyz,i2c-ctrl"; ... };
> >   i2c5: i2c@...e0000 { compatible = "xyz,i2c-ctrl"; ... };
> > 
> >   connector {
> >       i2c-ctrl {
> >           i2c-parent = <&i2c1>;
> >           #address-cells = <1>;
> >           #size-cells = <0>;
> >       };
> > 
> >       i2c-sensors {
> >           i2c-parent = <&i2c5>;
> >           #address-cells = <1>;
> >           #size-cells = <0>;
> >       };
> >   };
> > 
> >   --- device tree overlay ---
> > 
> >   ...
> >   // This node will overlay on the i2c-ctrl node of the base tree  
> 
> Why don't you overlay it right over &i2c1?
> It should have worked since commit ea7513bbc041
> ("i2c/of: Add OF_RECONFIG notifier handler").
> Doesn't it work for your use-case?

One reason is decoupling the base board and addon. A different base
board may wire the same connector pins to 'i2c4' instead of 'i2c1'. We
want a single overlay to describe the addon, independently of the base
board, so it has to mention only connector pins, not base board
hardware.

Another reason is that using phandles to labels in the base tree in the
overlay (such as &i2c1) would need properties added by the __symbols__
node, and overlays adding properties to nodes in the live tree are not
welcome. This is both for a conceptual reason (adding an overlay ==
adding hardware and not _changing_ hardware, so adding nodes should be
enough) and an implementation one (properties added to nodes in the
live tree become deadprops and thus leak memory.

This topic was discussed at the latest Linux Plumbers Conference last
September. Slides and video of the discussion are available here:
https://lpc.events/event/18/contributions/1696/

More info are in the cover letter. Discussion leading to this
implementation started after v2:
https://lore.kernel.org/all/20240510163625.GA336987-robh@kernel.org/

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com