| Message ID | 20211008081739.26807-1-brgl@bgdev.pl |
|---|---|
| Headers | show |
| Series | gpio: implement the configfs testing module | expand |
On Fri, Oct 8, 2021 at 10:17 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > I'm respinning this series now because I noticed that I need to start writing > tests for my work on the new libgpiod v2 code to make sense (it's just becoming > too complicated to make even remotely functional without test coverage). At the > same time I don't want to rewrite the tests using gpio-mockup if the goal is to > replace it with gpio-sim anyway. > > I fixed issues pointed out by Al Viro and made sure that references are > correctly counted (including error paths) and that memory allocated for the > pending and live groups gets freed. > > === > > Cc'ing Viresh too. > > Viresh: while there's still a long way to go before the libgpio v2.0 release, > in order to merge the Rust bindings, we'll need a test-suite similar to what > we have now for C++ and Python bindings, except that it will have to be based > on the gpio-sim module when it makes its way into mainline. > > === > > This series adds a new GPIO testing module based on configfs committable items > and sysfs. The goal is to provide a testing driver that will be configurable > at runtime (won't need module reload) and easily extensible. The control over > the attributes is also much more fine-grained than in gpio-mockup. > > This series also contains a respin of the patches I sent separately to the > configfs maintainers - these patches implement the concept of committable > items that was well defined for a long time but never actually completed. > > Apart from the new driver itself, its selftests and the configfs patches, this > series contains some changes to the bitmap API - most importantly: it adds > devres managed variants of bitmap_alloc() and bitmap_zalloc(). > > v1 -> v2: > - add selftests for gpio-sim > - add helper programs for selftests > - update the configfs rename callback to work with the new API introduced in > v5.11 > - fix a missing quote in the documentation > - use !! whenever using bits operation that are required to return 0 or 1 > - use provided bitmap API instead of reimplementing copy or fill operations > - fix a deadlock in gpio_sim_direction_output() > - add new read-only configfs attributes for mapping of configfs items to GPIO > device names > - and address other minor issues pointed out in reviews of v1 > > v2 -> v3: > - use devm_bitmap_alloc() instead of the zalloc variant if we're initializing > the bitmap with 1s > - drop the patch exporting device_is_bound() > - don't return -ENODEV from dev_nam and chip_name configfs attributes, return > a string indicating that the device is not available yet ('n/a') > - fix indentation where it makes sense > - don't protect IDA functions which use their own locking and where it's not > needed > - use kmemdup() instead of kzalloc() + memcpy() > - collected review tags > - minor coding style fixes > > v3 -> v4: > - return 'none' instead of 'n/a' from dev_name and chip_name before the device > is registered > - use sysfs_emit() instead of s*printf() > - drop GPIO_SIM_MAX_PROP as it's only used in an array's definition where it's > fine to hardcode the value > > v4 -> v5: > - drop lib patches that are already upstream > - use BIT() instead of (1UL << bit) for flags > - fix refcounting for the configfs_dirent in rename() > - drop d_move() from the rename() callback > - free memory allocated for the live and pending groups in configfs_d_iput() > and not in detach_groups() > - make sure that if a group of some name is in the live directory, a new group > with the same name cannot be created in the pending directory > > v5 -> v6: > - go back to using (1UL << bit) instead of BIT() > - if the live group dentry doesn't exist for whatever reason at the time when > mkdir() in the pending group is called (would be a BUG()), return -ENOENT > instead of -EEXIST which should only be returned if given subsystem already > exists in either live or pending group > > v6 -> v7: > - as detailed by Andy in commit 6fda593f3082 ("gpio: mockup: Convert to use > software nodes") removing device properties after the platform device is > removed but before the GPIO device gets dropped can lead to a use-after-free > bug - use software nodes to manually control the freeing of the properties > > Bartosz Golaszewski (8): > configfs: increase the item name length > configfs: use (1UL << bit) for internal flags > configfs: implement committable items > samples: configfs: add a committable group > gpio: sim: new testing module > selftests: gpio: provide a helper for reading chip info > selftests: gpio: add a helper for reading GPIO line names > selftests: gpio: add test cases for gpio-sim > > Documentation/admin-guide/gpio/gpio-sim.rst | 72 ++ > Documentation/filesystems/configfs.rst | 6 +- > drivers/gpio/Kconfig | 8 + > drivers/gpio/Makefile | 1 + > drivers/gpio/gpio-sim.c | 886 ++++++++++++++++++ > fs/configfs/configfs_internal.h | 22 +- > fs/configfs/dir.c | 276 +++++- > include/linux/configfs.h | 3 +- > samples/configfs/configfs_sample.c | 153 +++ > tools/testing/selftests/gpio/.gitignore | 2 + > tools/testing/selftests/gpio/Makefile | 4 +- > tools/testing/selftests/gpio/config | 1 + > tools/testing/selftests/gpio/gpio-chip-info.c | 57 ++ > tools/testing/selftests/gpio/gpio-line-name.c | 55 ++ > tools/testing/selftests/gpio/gpio-sim.sh | 229 +++++ > 15 files changed, 1752 insertions(+), 23 deletions(-) > create mode 100644 Documentation/admin-guide/gpio/gpio-sim.rst > create mode 100644 drivers/gpio/gpio-sim.c > create mode 100644 tools/testing/selftests/gpio/gpio-chip-info.c > create mode 100644 tools/testing/selftests/gpio/gpio-line-name.c > create mode 100755 tools/testing/selftests/gpio/gpio-sim.sh > > -- > 2.30.1 > Another ping... Bart
On Fri, Oct 15, 2021 at 4:13 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> Another ping...
If it's hard to get attention I would simply queue these on a immutable branch
in the GPIO tree, then merge it into what you send to linux-next and offer
the interested parties to pull it in.
If noone complains as we get really close to the merge window, just offer
this branch to Torvalds in a separate pull request.
Yours,
Linus Walleij
I'm respinning this series now because I noticed that I need to start writing tests for my work on the new libgpiod v2 code to make sense (it's just becoming too complicated to make even remotely functional without test coverage). At the same time I don't want to rewrite the tests using gpio-mockup if the goal is to replace it with gpio-sim anyway. I fixed issues pointed out by Al Viro and made sure that references are correctly counted (including error paths) and that memory allocated for the pending and live groups gets freed. === Cc'ing Viresh too. Viresh: while there's still a long way to go before the libgpio v2.0 release, in order to merge the Rust bindings, we'll need a test-suite similar to what we have now for C++ and Python bindings, except that it will have to be based on the gpio-sim module when it makes its way into mainline. === This series adds a new GPIO testing module based on configfs committable items and sysfs. The goal is to provide a testing driver that will be configurable at runtime (won't need module reload) and easily extensible. The control over the attributes is also much more fine-grained than in gpio-mockup. This series also contains a respin of the patches I sent separately to the configfs maintainers - these patches implement the concept of committable items that was well defined for a long time but never actually completed. Apart from the new driver itself, its selftests and the configfs patches, this series contains some changes to the bitmap API - most importantly: it adds devres managed variants of bitmap_alloc() and bitmap_zalloc(). v1 -> v2: - add selftests for gpio-sim - add helper programs for selftests - update the configfs rename callback to work with the new API introduced in v5.11 - fix a missing quote in the documentation - use !! whenever using bits operation that are required to return 0 or 1 - use provided bitmap API instead of reimplementing copy or fill operations - fix a deadlock in gpio_sim_direction_output() - add new read-only configfs attributes for mapping of configfs items to GPIO device names - and address other minor issues pointed out in reviews of v1 v2 -> v3: - use devm_bitmap_alloc() instead of the zalloc variant if we're initializing the bitmap with 1s - drop the patch exporting device_is_bound() - don't return -ENODEV from dev_nam and chip_name configfs attributes, return a string indicating that the device is not available yet ('n/a') - fix indentation where it makes sense - don't protect IDA functions which use their own locking and where it's not needed - use kmemdup() instead of kzalloc() + memcpy() - collected review tags - minor coding style fixes v3 -> v4: - return 'none' instead of 'n/a' from dev_name and chip_name before the device is registered - use sysfs_emit() instead of s*printf() - drop GPIO_SIM_MAX_PROP as it's only used in an array's definition where it's fine to hardcode the value v4 -> v5: - drop lib patches that are already upstream - use BIT() instead of (1UL << bit) for flags - fix refcounting for the configfs_dirent in rename() - drop d_move() from the rename() callback - free memory allocated for the live and pending groups in configfs_d_iput() and not in detach_groups() - make sure that if a group of some name is in the live directory, a new group with the same name cannot be created in the pending directory v5 -> v6: - go back to using (1UL << bit) instead of BIT() - if the live group dentry doesn't exist for whatever reason at the time when mkdir() in the pending group is called (would be a BUG()), return -ENOENT instead of -EEXIST which should only be returned if given subsystem already exists in either live or pending group v6 -> v7: - as detailed by Andy in commit 6fda593f3082 ("gpio: mockup: Convert to use software nodes") removing device properties after the platform device is removed but before the GPIO device gets dropped can lead to a use-after-free bug - use software nodes to manually control the freeing of the properties Bartosz Golaszewski (8): configfs: increase the item name length configfs: use (1UL << bit) for internal flags configfs: implement committable items samples: configfs: add a committable group gpio: sim: new testing module selftests: gpio: provide a helper for reading chip info selftests: gpio: add a helper for reading GPIO line names selftests: gpio: add test cases for gpio-sim Documentation/admin-guide/gpio/gpio-sim.rst | 72 ++ Documentation/filesystems/configfs.rst | 6 +- drivers/gpio/Kconfig | 8 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-sim.c | 886 ++++++++++++++++++ fs/configfs/configfs_internal.h | 22 +- fs/configfs/dir.c | 276 +++++- include/linux/configfs.h | 3 +- samples/configfs/configfs_sample.c | 153 +++ tools/testing/selftests/gpio/.gitignore | 2 + tools/testing/selftests/gpio/Makefile | 4 +- tools/testing/selftests/gpio/config | 1 + tools/testing/selftests/gpio/gpio-chip-info.c | 57 ++ tools/testing/selftests/gpio/gpio-line-name.c | 55 ++ tools/testing/selftests/gpio/gpio-sim.sh | 229 +++++ 15 files changed, 1752 insertions(+), 23 deletions(-) create mode 100644 Documentation/admin-guide/gpio/gpio-sim.rst create mode 100644 drivers/gpio/gpio-sim.c create mode 100644 tools/testing/selftests/gpio/gpio-chip-info.c create mode 100644 tools/testing/selftests/gpio/gpio-line-name.c create mode 100755 tools/testing/selftests/gpio/gpio-sim.sh