gpiolib: fix sysfs when cdev is not selected

PATCH In review Kent Gibson 3 participants 3 messages Nov 5, 2020 GPIO

Share & tools

Kent Gibson

author

2020-11-05 10:41 UTC #0 lore raw

PATCH 1/1 gpiolib: fix sysfs when cdev is not selected

In gpiochip_setup_dev() the call to gpiolib_cdev_register() indirectly calls device_add(). This is still required for the sysfs even when CONFIG_GPIO_CDEV is not selected in the build.

Replace the stubbed functions in gpiolib-cdev.h with macros in gpiolib.c that perform the required device_add() and device_del() when CONFIG_GPIO_CDEV is not selected.

Fixes d143493c01b7 (gpiolib: make cdev a build option)

Reported-by Nicolas Schichan <[email protected]>

Signed-off-by Kent Gibson <[email protected]>

---
 drivers/gpio/gpiolib-cdev.h | 15 ---------------
 drivers/gpio/gpiolib.c      | 18 +++++++++++++++---
 2 files changed, 15 insertions(+), 18 deletions(-)

drivers/gpio/gpiolib-cdev.h +0 ~~-15~~

index cb41dd757338..b42644cbffb8 100644
@@ -7,22 +7,7 @@
 
 struct gpio_device;
 
-#ifdef CONFIG_GPIO_CDEV
-
 int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt);
 void gpiolib_cdev_unregister(struct gpio_device *gdev);
 
-#else
-
-static inline int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
-{
-	return 0;
-}
-
-static inline void gpiolib_cdev_unregister(struct gpio_device *gdev)
-{
-}
-
-#endif /* CONFIG_GPIO_CDEV */
-
 #endif /* GPIOLIB_CDEV_H */

drivers/gpio/gpiolib.c +15 -3

index 8e29a60c3697..c980ddcda833 100644
@@ -480,11 +480,23 @@ static void gpiodevice_release(struct device *dev)
 	kfree(gdev);
 }
 
+#ifdef CONFIG_GPIO_CDEV
+#define gcdev_register(gdev, devt)	gpiolib_cdev_register((gdev), (devt))
+#define gcdev_unregister(gdev)		gpiolib_cdev_unregister((gdev))
+#else
+/*
+ * gpiolib_cdev_register() indirectly calls device_add(), which is still
+ * required even when cdev is not selected.
+ */
+#define gcdev_register(gdev, devt)	device_add(&(gdev)->dev)
+#define gcdev_unregister(gdev)		device_del(&(gdev)->dev)
+#endif
+
 static int gpiochip_setup_dev(struct gpio_device *gdev)
 {
 	int ret;
 
-	ret = gpiolib_cdev_register(gdev, gpio_devt);
+	ret = gcdev_register(gdev, gpio_devt);
 	if (ret)
 		return ret;

@@ -500,7 +512,7 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
 	return 0;
 
 err_remove_device:
-	gpiolib_cdev_unregister(gdev);
+	gcdev_unregister(gdev);
 	return ret;
 }

@@ -825,7 +837,7 @@ void gpiochip_remove(struct gpio_chip *gc)
 	 * be removed, else it will be dangling until the last user is
 	 * gone.
 	 */
-	gpiolib_cdev_unregister(gdev);
+	gcdev_unregister(gdev);
 	put_device(&gdev->dev);
 }
 EXPORT_SYMBOL_GPL(gpiochip_remove);

Signature · 1 line

2.29.2

Nicolas Schichan

Tested-by on v1 1/1

2020-11-05 12:02 UTC #1 lore raw

Hello Kent,

Quoted 92 lines · Kent Gibson

In gpiochip_setup_dev() the call to gpiolib_cdev_register() indirectly calls device_add(). This is still required for the sysfs even when CONFIG_GPIO_CDEV is not selected in the build.

Replace the stubbed functions in gpiolib-cdev.h with macros in gpiolib.c that perform the required device_add() and device_del() when CONFIG_GPIO_CDEV is not selected.

Fixes d143493c01b7 (gpiolib: make cdev a build option)

Reported-by Nicolas Schichan <[email protected]>

Signed-off-by Kent Gibson <[email protected]>

---
 drivers/gpio/gpiolib-cdev.h | 15 ---------------
 drivers/gpio/gpiolib.c      | 18 +++++++++++++++---
 2 files changed, 15 insertions(+), 18 deletions(-)

drivers/gpio/gpiolib-cdev.h +0 ~~-15~~

index cb41dd757338..b42644cbffb8 100644
@@ -7,22 +7,7 @@
 
 struct gpio_device;
 
-#ifdef CONFIG_GPIO_CDEV
-
 int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt);
 void gpiolib_cdev_unregister(struct gpio_device *gdev);
 
-#else
-
-static inline int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
-{
-       return 0;
-}
-
-static inline void gpiolib_cdev_unregister(struct gpio_device *gdev)
-{
-}
-
-#endif /* CONFIG_GPIO_CDEV */
-
 #endif /* GPIOLIB_CDEV_H */

drivers/gpio/gpiolib.c +15 -4

index 8e29a60c3697..c980ddcda833 100644
@@ -480,11 +480,23 @@ static void gpiodevice_release(struct device *dev)
        kfree(gdev);
 }
 
+#ifdef CONFIG_GPIO_CDEV
+#define gcdev_register(gdev, devt)     gpiolib_cdev_register((gdev), (devt))
+#define gcdev_unregister(gdev)         gpiolib_cdev_unregister((gdev))
+#else
+/*
+ * gpiolib_cdev_register() indirectly calls device_add(), which is still
+ * required even when cdev is not selected.
+ */
+#define gcdev_register(gdev, devt)     device_add(&(gdev)->dev)
+#define gcdev_unregister(gdev)         device_del(&(gdev)->dev)
+#endif
+
 static int gpiochip_setup_dev(struct gpio_device *gdev)
 {
        int ret;
 
-       ret = gpiolib_cdev_register(gdev, gpio_devt);
+       ret = gcdev_register(gdev, gpio_devt);
        if (ret)
                return ret;

@@ -500,7 +512,7 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
        return 0;
 
 err_remove_device:
-       gpiolib_cdev_unregister(gdev);
+       gcdev_unregister(gdev);
        return ret;
 }

@@ -825,7 +837,7 @@ void gpiochip_remove(struct gpio_chip *gc)
         * be removed, else it will be dangling until the last user is
         * gone.
         */
-       gpiolib_cdev_unregister(gdev);
+       gcdev_unregister(gdev);
        put_device(&gdev->dev);
 }
 EXPORT_SYMBOL_GPL(gpiochip_remove);
--

Quoted 2 lines

2.29.2

I have tested your patch and it works for both CONFIG_GPIO_CDEV set and unset.

Tested-by Nicolas Schichan <[email protected]>

Regards,

Signature · 2 lines

Nicolas Schichan
Freebox SAS

Bartosz Golaszewski

2020-11-05 14:37 UTC #2 lore raw

In reply to Kent Gibson

Quoted 5 lines · Kent Gibson

In gpiochip_setup_dev() the call to gpiolib_cdev_register() indirectly calls device_add(). This is still required for the sysfs even when CONFIG_GPIO_CDEV is not selected in the build.

Quoted 7 lines

Replace the stubbed functions in gpiolib-cdev.h with macros in gpiolib.c that perform the required device_add() and device_del() when CONFIG_GPIO_CDEV is not selected.
Fixes d143493c01b7 (gpiolib: make cdev a build option)
Reported-by Nicolas Schichan <[email protected]>
Signed-off-by Kent Gibson <[email protected]>

Applied for fixes with Nicolas' tag.

Bartosz