Re: [PATCH v10 2/5] gpio: sim: new testing module
From: Kent Gibson <warthog618@gmail.com>
Date: 2021-11-26 02:27:27
Also in:
lkml
On Thu, Nov 25, 2021 at 02:14:20PM +0100, Bartosz Golaszewski wrote:
On Wed, Nov 24, 2021 at 12:43 PM Bartosz Golaszewski [off-list ref] wrote:quoted
Implement a new, modern GPIO testing module controlled by configfs attributes instead of module parameters. The goal of this driver is to provide a replacement for gpio-mockup that will be easily extensible with new features and doesn't require reloading the module to change the setup. Signed-off-by: Bartosz Golaszewski <redacted> --- Documentation/admin-guide/gpio/gpio-sim.rst | 80 ++ drivers/gpio/Kconfig | 8 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-sim.c | 1370 +++++++++++++++++++ 4 files changed, 1459 insertions(+) create mode 100644 Documentation/admin-guide/gpio/gpio-sim.rst create mode 100644 drivers/gpio/gpio-sim.cHi guys! I'd like to get your opinion on some parts of the interface. Should we allow creating multiple gpiochips per platform device like some drivers do? And if so - should the sysfs groups be created for each gpiochip device kobject and not the parent? Currently we do this: # Create the chip (platform device + single gpiochip): mkdir /sys/kernel/config/gpio-sim/my-chip # Configure it echo 8 > /sys/kernel/config/gpio-sim/my-chip/num_lines # Enable it echo 1 > /sys/kernel/config/gpio-sim/my-chip/live What I mean above would make it look like this: # Create the platform device mkdir /sys/kernel/config/gpio-sim/my-gpio-device # what's inside? ls /sys/kernel/config/gpio-sim/my-gpio-device live # Create GPIO chips mkdir /sys/kernel/config/gpio-sim/my-gpio-device/chip0 mkdir /sys/kernel/config/gpio-sim/my-gpio-device/chip1 # Configure chips echo 8 > /sys/kernel/config/gpio-sim/my-gpio-device/chip0/num_lines echo 4 > /sys/kernel/config/gpio-sim/my-gpio-device/chip1/num_lines echo foobar > /sys/kernel/config/gpio-sim/my-gpio-device/chip1/label # Enable both chips echo 1 > /sys/kernel/config/gpio-sim/my-gpio-device/live And in sysfs instead of current: echo pull-up > /sys/devices/platform/gpio-sim.0/sim_line0/pull We'd have to do: echo pull-up > /sys/devices/platform/gpio-sim.0/gpiochip1/sim_line0/pull While I don't see any usefulness of that at this time, if we don't do it now, then it'll be hard to extend this module later. What are your thoughts?
I might be missing something, but I don't see the platform abstraction adding anything that can't be easily emulated in userspace using multiple chips, and it complicates the minimal case as you now have to create a platform as well as the chip. So I'd keep it simple and stick with the chip level abstraction. Cheers, Kent.