Re: [PATCH RFC/RFT 00/15] gpio: sysfs: add a per-chip export/unexport attribute pair

[Jan Lübbe]

Hi Bartosz,

On Tue, 2025-06-10 at 16:38 +0200, Bartosz Golaszewski wrote:
> Following our discussion[1], here's a proposal for extending the sysfs
> interface with attributes not referring to GPIO lines by their global
> numbers in a backward compatible way.
> 
> Long story short: there is now a new class device for each GPIO chip.
> It's called chipX where X is the ID of the device as per the driver
> model and it lives next to the old gpiochipABC where ABC is the GPIO
> base. Each new chip class device has a pair of export/unexport
> attributes which work similarly to the global ones under /sys/class/gpio
> but take hardware offsets within the chip as input, instead of the
> global numbers. Finally, each exported line appears at the same time as
> the global /sys/class/gpio/gpioABC as well as per-chip
> /sys/class/gpio/chipX/gpioY sysfs group.
> 
> First, there are some documentation updates, followed by a set of
> updates to the sysfs code that's useful even without the new
> functionality. Then the actual implementation of a parallel GPIO chip
> entry not containing the base GPIO number in the name and the
> corresponding sysfs attribute group for each exported line that lives
> under the new chip class device. Finally: also allow to compile out the
> legacy parts leaving only the new elements of the sysfs ABI.
> 
> This series passes the compatibility tests I wrote while working on the
> user-space compatibility layer for sysfs[2].
> 
> [1] https://lore.kernel.org/all/CAMRc=McUCeZcU6co1aN54rTudo+JfPjjForu4iKQ5npwXk6GXA@xxxxxxxxxxxxxx/
> [2] https://github.com/brgl/gpio-sysfs-compat-tests
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>

thanks for implementing this! I tried it on one of our boards and noticed a few
things.

After unexporting a GPIO from the chipX dir, the subdirectory is not removed:
 root@lxatac-00006:/sys/class/gpio/chip9# echo 1 > export 
 root@lxatac-00006:/sys/class/gpio/chip9# echo 1 > unexport 
 root@lxatac-00006:/sys/class/gpio/chip9# ls -l gpio1/
 total 0
 -rw-r--r-- 1 root root 4096 Jun 18 12:52 active_low
 -rw-r--r-- 1 root root 4096 Jun 18 12:52 direction
 -rw-r--r-- 1 root root 4096 Jun 18 12:52 edge
 -rw-r--r-- 1 root root 4096 Jun 18 12:52 value
Subsequent attempts to export it again fail.

The contents of /sys/kernel/debug/gpio don't really fit any more:
 gpiochip10: GPIOs 660-663, parent: i2c/0-0024, pca9570, can sleep:
  gpio-660 (DUT_PWR_EN          |tacd                ) out hi 
  gpio-661 (DUT_PWR_DISCH       |tacd                ) out lo 
  gpio-662 (DUT_PWR_ADCRST      |reset               ) out lo 
The header is inconsistent: it uses the 'gpiochip' prefix, but not the base as
the old class devices in /sys/class/gpio/. Perhaps something like this?
 chip10: GPIOs 0-2 (global IDs 660-663), parent: i2c/0-0024, pca9570, can sleep:
  gpio-0 (660) (DUT_PWR_EN          |tacd                ) out hi 
  gpio-1 (661) (DUT_PWR_DISCH       |tacd                ) out lo 
  gpio-2 (662) (DUT_PWR_ADCRST      |reset               ) out lo   
If GPIO_SYSFS_LEGACY is disabled, the global IDs could be hidden.

Unix permissions/ownership just works.


As far as I can see, this is basically everything I need to replace the old
global ID based GPIO access in labgrid. Thanks again! :)


Regards,
Jan
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |