Re: [PATCH 4/4] devicetree: bindings: spi: Introduce SPI bus extensions

[Luca Ceresoli <luca.ceresoli@bootlin.com>]

On Tue, 29 Jul 2025 14:52:00 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On 29/07/2025 11:51, Ayush Singh wrote:
> > An SPI bus can be wired to the connector and allows an add-on board to
> > connect additional SPI devices to this bus.
> >   
> 
> ... so I found the binding. Not marked by my filters due to non-standard
> subject.
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> 
> > Those additional SPI devices could be described as sub-nodes of the SPI
> > bus controller node 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 ---
> > 
> >   spi1: spi@abcd0000 {
> >       compatible = "xyz,foo";
> >       spi-bus-extension@0 {
> >           spi-bus = <&spi_ctrl>;
> >       };
> >       ...
> >   };
> > 
> >   spi5: spi@cafe0000 {
> >       compatible = "xyz,bar";
> >       spi-bus-extension@0 {
> >           spi-bus = <&spi_sensors>;
> >       };
> >       ...
> >   };
> > 
> >   connector {
> >       spi_ctrl: spi-ctrl {
> >           spi-parent = <&spi1>;
> >           #address-cells = <1>;
> >           #size-cells = <0>;
> >       };
> > 
> >       spi_sensors: spi-sensors {
> >           spi-parent = <&spi5>;
> >           #address-cells = <1>;
> >           #size-cells = <0>;
> >       };
> >   };  
> 
> It looks like you are re-doing I2C work. Please wait till I2C discussion
> finishes, so we won't have to comment on the same in multiple places.
> 
> > 
> >   --- device tree overlay ---
> > 
> >   ...
> >   // This node will overlay on the spi-ctrl node of the base tree
> >   spi-ctrl {
> >       eeprom@50 { compatible = "atmel,24c64"; ... };
> >   };
> >   ...
> > 
> >   --- resulting device tree ---
> > 
> >   spi1: spi@abcd0000 {
> >       compatible = "xyz,foo";
> >       spi-bus-extension@0 {
> >           spi-bus = <&spi_ctrl>;
> >       };
> >       ...
> >   };
> > 
> >   spi5: spi@cafe0000 {
> >       compatible = "xyz,bar";
> >       spi-bus-extension@0 {
> >           spi-bus = <&spi_sensors>;
> >       };
> >       ...
> >   };
> > 
> >   connector {
> >       spi_ctrl: spi-ctrl {
> >           spi-parent = <&spi1>;
> >           #address-cells = <1>;
> >           #size-cells = <0>;
> > 
> >           device@1 { compatible = "xyz,foo"; ... };
> >       };
> > 
> >       spi_sensors: spi-sensors {
> >           spi-parent = <&spi5>;
> >           #address-cells = <1>;
> >           #size-cells = <0>;
> >       };
> >   };
> > 
> > Here spi-ctrl (same goes for spi-sensors) represent the part of SPI bus
> > that is on the hot-pluggable add-on. On hot-plugging it will physically
> > connect to the SPI adapter on the base board. Let's call the 'spi-ctrl'
> > node an "extension node".
> > 
> > In order to decouple the overlay from the base tree, the SPI adapter
> > (spi@abcd0000) and the extension node (spi-ctrl) are separate nodes.
> > 
> > The extension node is linked to the SPI bus controller in two ways. The
> > first one with the spi-bus-extension available in SPI controller
> > sub-node and the second one with the spi-parent property available in
> > the extension node itself.
> > 
> > The purpose of those two links is to provide the link in both direction
> > from the SPI controller to the SPI extension and from the SPI extension
> > to the SPI controller.
> > 
> > Signed-off-by: Ayush Singh <ayush@beagleboard.org>
> > ---
> >  .../devicetree/bindings/spi/spi-controller.yaml    | 66 +++++++++++++++++++++-
> >  1 file changed, 65 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> > index 82d051f7bd6e09dab9809c85ff13475d2b118efd..9b44ce4542f9552c94cb0658ffe3f6d3f29bc434 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml
> > +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> > @@ -25,6 +25,13 @@ properties:
> >    "#size-cells":
> >      const: 0
> >  
> > +  spi-parent:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description:
> > +      In case of an SPI bus extension, reference to the SPI bus controller
> > +      this extension is connected to. In other word, reference the SPI bus
> > +      controller on the fixed side that drives the bus extension.
> > +
> >    cs-gpios:
> >      description: |
> >        GPIOs used as chip selects.
> > @@ -111,7 +118,26 @@ properties:
> >        - compatible
> >  
> >  patternProperties:
> > -  "^.*@[0-9a-f]+$":
> > +  'spi-bus-extension@[0-9a-f]+$':
> > +    type: object
> > +    description:
> > +      An SPI bus extension connected to an SPI bus. Those extensions allow to
> > +      decouple SPI busses when they are wired to connectors.  
> 
> I really do not get why you need separate two-way phandles for marking
> parent child relationship. IOW, if you need two way, then why not graphs?
> 
> Or why not just making the device@2 a child of SPI, since it is coming
> from overlay.

For the same reason as in I2C (and the proposed solution is the same).

As you wrote above, Ayush could wait for the I2C discussion to finish.
Problem is, the I2C discussion is not moving forward: Hervé is sending
proposals but there is no feedback on the core of the proposal, and no
feedback at all on the latest iteration. In case your filters missed it:
https://lore.kernel.org/all/20250618082313.549140-1-herve.codina@bootlin.com/

I think Ayush's series is useful anyway in that it shows the approach
Hervé proposed works unmodified for another subsystem and is useful for
a different use case.

Luca

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