diff mbox series

gpiolib: fix sysfs when cdev is not selected

Message ID 20201105104049.91077-1-warthog618@gmail.com
State Accepted
Commit 1f5eb8b17f02d216703ee56e4c3115f592b060fb
Headers show
Series gpiolib: fix sysfs when cdev is not selected | expand

Commit Message

Kent Gibson Nov. 5, 2020, 10:40 a.m. UTC
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 <nschichan@freebox.fr>
Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 drivers/gpio/gpiolib-cdev.h | 15 ---------------
 drivers/gpio/gpiolib.c      | 18 +++++++++++++++---
 2 files changed, 15 insertions(+), 18 deletions(-)

Comments

Nicolas Schichan Nov. 5, 2020, 12:01 p.m. UTC | #1
Hello Kent,

On Thu, Nov 5, 2020 at 11:41 AM Kent Gibson <warthog618@gmail.com> 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 <nschichan@freebox.fr>

> Signed-off-by: Kent Gibson <warthog618@gmail.com>

> ---

>  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 <nschichan@freebox.fr>


Regards,

-- 
Nicolas Schichan
Freebox SAS
Bartosz Golaszewski Nov. 5, 2020, 2:37 p.m. UTC | #2
On Thu, Nov 5, 2020 at 11:41 AM Kent Gibson <warthog618@gmail.com> 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 <nschichan@freebox.fr>
> Signed-off-by: Kent Gibson <warthog618@gmail.com>

Applied for fixes with Nicolas' tag.

Bartosz
diff mbox series

Patch

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);