mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ayush Singh <ayush@beagleboard.org>
To: Herve Codina <herve.codina@bootlin.com>
Cc: xypron.glpk@gmx.de, Jason Kridner <jkridner@beagleboard.org>,
	Deepak Khatri <lorforlinux@beagleboard.org>,
	Dhruva Gole <d-gole@ti.com>,
	Robert Nelson <robertcnelson@beagleboard.org>,
	Andrew Davis <afd@ti.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	David Gibson <david@gibson.dropbear.id.au>,
	Luca Ceresoli <luca.ceresoli@bootlin.com>,
	Pantelis Antoniou <pantelis.antoniou@gmail.com>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
	<devicetree@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [Discussion] Global vs Local devicetree overlays for addon board + connector setups
Date: Thu, 1 May 2025 11:35:29 +0530	[thread overview]
Message-ID: <f22811ea-c44c-4900-a6c7-11bec12a7e8a@beagleboard.org> (raw)
In-Reply-To: <20250430160944.7740d5e9@bootlin.com>

On 4/30/25 19:39, Herve Codina wrote:

> Hi Ayush,
>
> Thanks for this discussion initiative!
>
>
> On Wed, 30 Apr 2025 17:37:33 +0530
> Ayush Singh <ayush@beagleboard.org> wrote:
>
> ...
>
>> 1. __symbols__ based approach [3]
>>
>>
>> This was originally proposed by Andre Davis [3]. It defines an overlay
>> with just special names in `__symbols__`, which is used along with an
>> overlay for the addon-board, which makes use of the node names defined
>> in the connector `__symbols__` overlay. Please take a look at the
>> original patch series since it provides a working example of how it can
>> be used [3].
>>
> The __symbols__ based approach needs 2 overlays to handle the case where
> 2 connectors (A and B) are present an you want to connect a board described
> by a single overlay.
>
> The first overlay applied "adapts" the __symbols__ node for the connector
> where the board is connected (for instance connector A) in order to have
> the symbols used by the overlay describing the board resolved to the
> correct symbols.
>
> I think this open race conditions when the overlay is applied by the kernel
> itself. Indeed, we need to perform 2 steps in an atomic way:
>    1) Adapt symbols
>    2) Applied board overlay
>
> Also, a 3rd step should be added to restore symbols modified to their
> original value after the overlay is applied. This should be done to avoid
> any symbols collision.
>
> An other negative point is that properties in __symbols__ node are not
> described by device-tree bindings.
> How can we ensure interoperability between different base board.
>
> The export-symbols node is a node fully described in the DT and it is
> a sub-node of a connector node. This connector node has compatible string
> and a binding describing its own property and sub-nodes. Among them, the
> export-symbols node is described and can be checks by dt_binding_check.
>
> This implies that whatever the base board, for a given connector type
> (compatible string) the properties inside the export-symbols node have the
> exact same name. Any addon board overlay designed for this connector type
> will apply whatever the system where this connector is soldered.
>
>> It has a few nice benefits such as it works pretty well with existing
>> infrastructure, and does not need much in the way of new functionality.
> With existing infrastructure in the kernel, it leads to memory leaks if you
> add or change a property in an existing node.
>
> In other word, each time you update a symbol in the __symbols__ node, you
> have a memory leak.
>
>> However, for this discussion thread, I want to consolidate the
>> discussion regarding how this approach directly adds the devices to the
>> appropriate nodes, Eg. An SPI device in addon board will be added to the
>> appropriate SPI controller, etc. This means the changes are made to the
>> global tree.
>>
>>
> ...
>
>> Basic Requirements
>>
>> *********************
>>
>>
>> Here are some basic functionality that the chosen approach can do for it
>> to be viable for the connector + addon board setups:
>>
>>
>> 1. Dynamic device addition/removal from userspace
>>
>>
>> A lot of connectors + addon board setups do not have any dynamic
>> discovery addition. This is compounded when talking about treating the
>> whole header in SBCs like PocketBeagle 2 as a connector, since people
>> would want to wire LEDs and stuff to individual pins. So a mechanism
>> using sysfs or configfs is required
> request_firmware() or the firmware upload feature (CONFIG_FW_UPLOAD) could
> also be used if the connector is seen as a specific device and has a driver.
>    https://elixir.bootlin.com/linux/v6.15-rc3/source/Documentation/driver-api/firmware/fw_upload.rst

Yup, my current plan is to use request_firmware. If it is good enough 
security wise for remoteproc, it should be fine here as well.


>
>>
>> 2. Dynamic device addition/removal by driver using EEPROM or something else
>>
>>
>> Some setups (MikroBUS boards with 1-wire EEPROM, Beagle capes) have
>> EEPROMs that contain board information which can be used to detect which
>> overlay should be applied.
>>
>>
>> Main Discussion
>>
>> *****************
>>
>> The main topic I wish to discuss if global devicetree overlays are okay
>> for addon-board setups. Let me outline some reasons for I prefer the
>> local devicetree overlays approach:
>>
>>
>> 1. Addon board removal on multiple connector setups
>>
>>
>> Say connector A added an I2C device to the controller, then connector B
>> added an I2C device to the same controller. I am not sure how well
>> removing overlays out-of-order works.
>>
>>
>> 2. Who owns the device
>>
>>
>> Say there are 2 connectors A and B. Both connectors share an I2C
>> controller. Let both connectors have the same device attached. In case
>> of `__symbols__` based approach, both connectors would technically be
>> successful in applying the overlays, rather than one of the overlays
>> failing.
>>
>>
>> 3. How to register the newly added devices
>>
>>
>> I am a bit unsure about this one since I will have to check if the
>> kernel tries to register new devices automatically after applying the
>> overlay. For local setups, I was using `devm_of_platform_populate` on
>> the connector device.
> It depends on the bus the device is added.
> when an overlay is applied by the kernel, OF_RECONFIG_* events are
> triggered. Some buses handle them:
>
> 	$ git grep OF_RECONFIG_CHANGE
> 	drivers/bus/imx-weim.c: case OF_RECONFIG_CHANGE_ADD:
> 	drivers/bus/imx-weim.c: case OF_RECONFIG_CHANGE_REMOVE:
> 	drivers/gpio/gpiolib-of.c:      case OF_RECONFIG_CHANGE_ADD:
> 	drivers/gpio/gpiolib-of.c:      case OF_RECONFIG_CHANGE_REMOVE:
> 	drivers/i2c/i2c-core-of.c:      case OF_RECONFIG_CHANGE_ADD:
> 	drivers/i2c/i2c-core-of.c:      case OF_RECONFIG_CHANGE_REMOVE:
> 	drivers/of/dynamic.c: * Return: OF_RECONFIG_CHANGE_REMOVE on device going from enabled to
> 	drivers/of/dynamic.c: * disabled, OF_RECONFIG_CHANGE_ADD on device going from disabled to
> 	drivers/of/dynamic.c:   return new_state ? OF_RECONFIG_CHANGE_ADD : OF_RECONFIG_CHANGE_REMOVE;
> 	drivers/of/platform.c:  case OF_RECONFIG_CHANGE_ADD:
> 	drivers/of/platform.c:  case OF_RECONFIG_CHANGE_REMOVE:
> 	drivers/spi/spi.c:      case OF_RECONFIG_CHANGE_ADD:
> 	drivers/spi/spi.c:      case OF_RECONFIG_CHANGE_REMOVE:
> 	include/linux/of.h:     OF_RECONFIG_CHANGE_ADD,
> 	include/linux/of.h:     OF_RECONFIG_CHANGE_REMOVE,
>
>>
>> 4. Security
>>
>>
>> I think local modification is more secure than global tree modification.
>> A completely local solution should be as secure from devicetree
>> perspective as USB. But I am not an expert.
>>
>>
>> Drawbacks of local setups
>>
>> ***************************
>>
>>
>> 1. Needs a lot of surrounding work.
>>
>>
>> I2C bus extension is needed for I2C devices to work, something similar
>> for SPI. At least ADC, PWM and GPIO should be covered with just nexus nodes.
> I wouldn't say 'a lot'.
> I already did the work for I2C bus extension [0] and the implementation was
> not so complex.
>
> [0] https://lore.kernel.org/all/20250401081041.114333-1-herve.codina@bootlin.com/

Well, when I say a lot of work, I don't mean codewise. That will be 
pretty simple actually. The problem I foresee is convincing everything 
in different subsystems. Basically, a lot of discussions.

Still, I do think figuring these things out now would be best in the 
long run. After all, while I cannot say for everyone else, I want to use 
this setup for at least MikroBUS, Grove, PocketBeagle 2 connector, 
BeagleBone Black connector, and RPI (for BeagleY-AI) connector.

>>
>> Closing Thoughts
>>
>> ******************
>>
>>
>> I would really like to reach consensus regarding weather the addon-board
>> overlays should be global or local. This will help to give a direction
>> regarding what should be improved, and hopefully make future development
>> move faster. Once a bit of consensus has been reached, we can discuss
>> specific implementations.
>>
>
> Again, thanks again for initiating this discussion.
> Hope this will help to move forward on this topic!
>
> Best regards,
> Hervé


Ayush Singh


  parent reply	other threads:[~2025-05-01  6:05 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-30 12:07 Ayush Singh
2025-04-30 14:09 ` Herve Codina
2025-04-30 14:31   ` Geert Uytterhoeven
2025-04-30 14:58     ` Herve Codina
2025-05-01  6:05   ` Ayush Singh [this message]
2025-05-04 12:33   ` Ayush Singh
2025-05-04 12:50     ` Greg Kroah-Hartman
2025-05-04 13:00       ` Ayush Singh
2025-05-04 13:13         ` Greg Kroah-Hartman
2025-05-05  6:43           ` Geert Uytterhoeven
2025-05-05 20:56             ` Rob Herring
2025-06-04 19:03 ` Andrew Davis
2025-06-05  7:22   ` Ayush Singh
2025-06-09 14:13     ` Luca Ceresoli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f22811ea-c44c-4900-a6c7-11bec12a7e8a@beagleboard.org \
    --to=ayush@beagleboard.org \
    --cc=afd@ti.com \
    --cc=d-gole@ti.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=devicetree@vger.kernel.org \
    --cc=geert@linux-m68k.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=herve.codina@bootlin.com \
    --cc=jkridner@beagleboard.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lorforlinux@beagleboard.org \
    --cc=luca.ceresoli@bootlin.com \
    --cc=pantelis.antoniou@gmail.com \
    --cc=robertcnelson@beagleboard.org \
    --cc=xypron.glpk@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®