2020-11-05 10:43:23

by Kent Gibson

[permalink] [raw]
Subject: [PATCH] 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(-)

diff --git a/drivers/gpio/gpiolib-cdev.h b/drivers/gpio/gpiolib-cdev.h
index cb41dd757338..b42644cbffb8 100644
--- a/drivers/gpio/gpiolib-cdev.h
+++ b/drivers/gpio/gpiolib-cdev.h
@@ -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 */
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 8e29a60c3697..c980ddcda833 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -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);
--
2.29.2


2020-11-05 12:04:33

by Nicolas Schichan

[permalink] [raw]
Subject: Re: [PATCH] gpiolib: fix sysfs when cdev is not selected

Hello Kent,

On Thu, Nov 5, 2020 at 11:41 AM Kent Gibson <[email protected]> wrote:
>
> 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(-)
>
> diff --git a/drivers/gpio/gpiolib-cdev.h b/drivers/gpio/gpiolib-cdev.h
> index cb41dd757338..b42644cbffb8 100644
> --- a/drivers/gpio/gpiolib-cdev.h
> +++ b/drivers/gpio/gpiolib-cdev.h
> @@ -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 */
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 8e29a60c3697..c980ddcda833 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -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);
> --
> 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,

--
Nicolas Schichan
Freebox SAS

2020-11-05 14:41:39

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH] gpiolib: fix sysfs when cdev is not selected

On Thu, Nov 5, 2020 at 11:41 AM Kent Gibson <[email protected]> wrote:
>
> 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]>

Applied for fixes with Nicolas' tag.

Bartosz