On Fri, Nov 26, 2021 at 3:23 AM Kent Gibson [off-list ref] wrote:

On Thu, Nov 25, 2021 at 02:14:20PM +0100, Bartosz Golaszewski wrote:

quoted

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.c

Hi 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.

I wouldn't stress about the level of complication of the user-space
interface. Normally users will wrap it in some kind of library anyway.

So I'd keep it simple and stick with the chip level abstraction.

Cheers,
Kent.

Yes, in user-space nothing would change as it only sees separate
gpiochips whether they're just banks of the same device or actual
devices of their own (except if you start digging into uevents'
contents, then you'll see it).

But this module is also aimed at doing some kernel subsystem testing
(even if triggered from user-space) and providing multiple banks of
GPIOs is a regularly used feature. Adding it allows us to test this
other level of hierarchy + multi-level device properties etc.

Another issue is logical correctness. In this version the line
attributes in sysfs are at the platform device level
(/sys/platform/devices/gpio-sim.X). Logically they really should be at
the gpio device level (/sys/platform/devices/gpio-sim.X/gpiochipY).

So I'm willing to give it a shot.