Re: [PATCH v2 03/18] gpiolib: make cdev a build option
On Mon, Jul 27, 2020 at 7:57 AM Kent Gibson wrote: > I've gone with this: > > +config GPIO_CDEV > + bool > + prompt "Character device (/dev/gpiochipN) support" if EXPERT > + default y > > so the entry is always present in menuconfig, and GPIO_CDEV_V1 can still > depend on it, but GPIO_CDEV can only be disabled if EXPERT is set. This is perfect, thanks Kent! Yours, Linus Walleij
Re: [PATCH v2 03/18] gpiolib: make cdev a build option
On Mon, Jul 27, 2020 at 09:46:01AM +0800, Kent Gibson wrote: > On Mon, Jul 27, 2020 at 12:25:53AM +0200, Linus Walleij wrote: > > On Sat, Jul 25, 2020 at 6:21 AM Kent Gibson wrote: > > > > > +config GPIO_CDEV > > > + bool "/dev/gpiochipN (character device interface)" > > > + default y > > > > I don't want to make it too easy to do this, as I see it as a standard > > kernel feature. > > > > Can we add: > > > > depends on EXPERT > > > > as with other standard kernel features? > > > > Fair enough. > > But what of the GPIO_CDEV_V1 option to disable uAPI V1 added in patch 04, > and that depends on GPIO_CDEV? > That is equivalent to GPIO_SYSFS, which is not dependent on EXPERT, > so I'll need to restructure the dependencies so it doesn't > inherit the EXPERT dependency. > Unless you also want it to be dependent on EXPERT. > I've gone with this: +config GPIO_CDEV + bool + prompt "Character device (/dev/gpiochipN) support" if EXPERT + default y so the entry is always present in menuconfig, and GPIO_CDEV_V1 can still depend on it, but GPIO_CDEV can only be disabled if EXPERT is set. > Hmmm, and maybe patch 04 should be later in the series - after V2 is > fully implemented and V1 is deprecated - around patch 11. > Just ignore me - the earlier code patches need the define else the V1 will be compiled out. Cheers, Kent.
Re: [PATCH v2 03/18] gpiolib: make cdev a build option
On Mon, Jul 27, 2020 at 12:25:53AM +0200, Linus Walleij wrote: > On Sat, Jul 25, 2020 at 6:21 AM Kent Gibson wrote: > > > +config GPIO_CDEV > > + bool "/dev/gpiochipN (character device interface)" > > + default y > > I don't want to make it too easy to do this, as I see it as a standard > kernel feature. > > Can we add: > > depends on EXPERT > > as with other standard kernel features? > Fair enough. But what of the GPIO_CDEV_V1 option to disable uAPI V1 added in patch 04, and that depends on GPIO_CDEV? That is equivalent to GPIO_SYSFS, which is not dependent on EXPERT, so I'll need to restructure the dependencies so it doesn't inherit the EXPERT dependency. Unless you also want it to be dependent on EXPERT. Hmmm, and maybe patch 04 should be later in the series - after V2 is fully implemented and V1 is deprecated - around patch 11. Cheers, Kent.
Re: [PATCH v2 03/18] gpiolib: make cdev a build option
On Sat, Jul 25, 2020 at 6:21 AM Kent Gibson wrote: > +config GPIO_CDEV > + bool "/dev/gpiochipN (character device interface)" > + default y I don't want to make it too easy to do this, as I see it as a standard kernel feature. Can we add: depends on EXPERT as with other standard kernel features? Yours, Linus Walleij
Re: [PATCH v2 03/18] gpiolib: make cdev a build option
On Sat, Jul 25, 2020 at 7:21 AM Kent Gibson wrote:
>
> Make the gpiolib-cdev module a build option. This allows the CDEV
> interface to be removed from the kernel to reduce kernel size in
> applications where is it not required, and provides the parent for
> other other CDEV interface specific build options to follow.
>
> Suggested-by: Bartosz Golaszewski
> Signed-off-by: Kent Gibson
> ---
> drivers/gpio/Kconfig| 16 ++--
> drivers/gpio/Makefile | 2 +-
> drivers/gpio/gpiolib-cdev.h | 15 +++
> 3 files changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 8030fd91a3cc..b5bb9efc1092 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -66,8 +66,20 @@ config GPIO_SYSFS
>
> This ABI is deprecated. If you want to use GPIO from userspace,
> use the character device /dev/gpiochipN with the appropriate
> - ioctl() operations instead. The character device is always
> - available.
> + ioctl() operations instead.
> +
> +config GPIO_CDEV
> + bool "/dev/gpiochipN (character device interface)"
> + default y
> + help
> + Say Y here to add the character device /dev/gpiochipN interface
> + for GPIOs. The character device allows userspace to control GPIOs
> + using ioctl() operations.
> +
> + Only say N is you are sure that the GPIO character device is not
is -> if
> + required.
> +
> + If unsure, say Y.
>
> config GPIO_GENERIC
> depends on HAS_IOMEM # Only for IOMEM drivers
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 4f9abff4f2dc..7c24c8d77068 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -7,8 +7,8 @@ obj-$(CONFIG_GPIOLIB) += gpiolib.o
> obj-$(CONFIG_GPIOLIB) += gpiolib-devres.o
> obj-$(CONFIG_GPIOLIB) += gpiolib-legacy.o
> obj-$(CONFIG_GPIOLIB) += gpiolib-devprop.o
> -obj-$(CONFIG_GPIOLIB) += gpiolib-cdev.o
> obj-$(CONFIG_OF_GPIO) += gpiolib-of.o
> +obj-$(CONFIG_GPIO_CDEV)+= gpiolib-cdev.o
> obj-$(CONFIG_GPIO_SYSFS) += gpiolib-sysfs.o
> obj-$(CONFIG_GPIO_ACPI)+= gpiolib-acpi.o
>
> diff --git a/drivers/gpio/gpiolib-cdev.h b/drivers/gpio/gpiolib-cdev.h
> index 973578e7ad10..19a4e3d57120 100644
> --- a/drivers/gpio/gpiolib-cdev.h
> +++ b/drivers/gpio/gpiolib-cdev.h
> @@ -5,7 +5,22 @@
>
> #include
>
> +#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 */
> --
> 2.27.0
>
--
With Best Regards,
Andy Shevchenko
[PATCH v2 03/18] gpiolib: make cdev a build option
Make the gpiolib-cdev module a build option. This allows the CDEV
interface to be removed from the kernel to reduce kernel size in
applications where is it not required, and provides the parent for
other other CDEV interface specific build options to follow.
Suggested-by: Bartosz Golaszewski
Signed-off-by: Kent Gibson
---
drivers/gpio/Kconfig| 16 ++--
drivers/gpio/Makefile | 2 +-
drivers/gpio/gpiolib-cdev.h | 15 +++
3 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 8030fd91a3cc..b5bb9efc1092 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -66,8 +66,20 @@ config GPIO_SYSFS
This ABI is deprecated. If you want to use GPIO from userspace,
use the character device /dev/gpiochipN with the appropriate
- ioctl() operations instead. The character device is always
- available.
+ ioctl() operations instead.
+
+config GPIO_CDEV
+ bool "/dev/gpiochipN (character device interface)"
+ default y
+ help
+ Say Y here to add the character device /dev/gpiochipN interface
+ for GPIOs. The character device allows userspace to control GPIOs
+ using ioctl() operations.
+
+ Only say N is you are sure that the GPIO character device is not
+ required.
+
+ If unsure, say Y.
config GPIO_GENERIC
depends on HAS_IOMEM # Only for IOMEM drivers
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4f9abff4f2dc..7c24c8d77068 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -7,8 +7,8 @@ obj-$(CONFIG_GPIOLIB) += gpiolib.o
obj-$(CONFIG_GPIOLIB) += gpiolib-devres.o
obj-$(CONFIG_GPIOLIB) += gpiolib-legacy.o
obj-$(CONFIG_GPIOLIB) += gpiolib-devprop.o
-obj-$(CONFIG_GPIOLIB) += gpiolib-cdev.o
obj-$(CONFIG_OF_GPIO) += gpiolib-of.o
+obj-$(CONFIG_GPIO_CDEV)+= gpiolib-cdev.o
obj-$(CONFIG_GPIO_SYSFS) += gpiolib-sysfs.o
obj-$(CONFIG_GPIO_ACPI)+= gpiolib-acpi.o
diff --git a/drivers/gpio/gpiolib-cdev.h b/drivers/gpio/gpiolib-cdev.h
index 973578e7ad10..19a4e3d57120 100644
--- a/drivers/gpio/gpiolib-cdev.h
+++ b/drivers/gpio/gpiolib-cdev.h
@@ -5,7 +5,22 @@
#include
+#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 */
--
2.27.0
