Hi Francesco, On 13-Mar-25 3:43 PM, Francesco Dolcini wrote: > From: Francesco Dolcini <francesco.dolcini@xxxxxxxxxxx> > > This series adds support for the Toradex Embedded Controller, currently used > on Toradex SMARC iMX95 and iMX8MP boards, with more to come in the future. > > The EC provides board power-off, reset and GPIO expander functionalities. > > Sending it as an RFC to gather initial feedback on it before investing more > time in testing and adding the remaining functionalities, with that said both > the code and the binding are in condition to be wholly reviewed. > > Emanuele Ghidoli (2): > dt-bindings: firmware: add toradex,embedded-controller > platform: toradex: add preliminary support for Embedded Controller Thank you for your patches. 2 remarks, as Andy already hinted at drivers/platform/arm64/ likely is a better location for this. But as the commit first adding that directory indicates: https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=363c8aea25728604537b170a1cc24e2f46844896 The reason for having ARM EC drivers there is that these are for x86-pc-like laptops with all the typical laptops bells and whistles like EC handled battery charging limits / spk/mic mute-leds built into keys on the keyboards. Special key handling (like mute, kbd backlight) done by the EC etc. Since all the experience for dealing with those laptop-esque features and exporting them to userspace with a consistent userspace API is in hands of the maintainers of drivers/platform/x86 it was decided to add a new drivers/platform/arm64 directory maintained by the same folks. If this EC driver's only functionality is: "The EC provides board power-off, reset and GPIO expander functionalities." I'm not sure that drivers/platform/arm64 is the best place for this. Also you mention GPIO expander, but that does not seem to be supported yet? The GPIO functionality really should be in its own GPIO driver. So it seems to me that what would be a better fit here would be: 1. A drivers/mfd/ MFD driver with the regmap stuff, registering "board-reset" and "gpio" cells 2. A drivers/power/reset/ driver for the "board-reset" cell, quoting from the Kconfig help text for that dir: menuconfig POWER_RESET bool "Board level reset or power off" help Provides a number of drivers which either reset a complete board or shut it down, by manipulating the main power supply on the board. Say Y here to enable board reset and power off which seems to match exactly with your current EC driver functionality 3. A drivers/gpio/ driver to expose the GPIOs using the standard GPIO framework, this can be added later in a follow up patch. Regards, Hans
