[PATCH 0/5] gpio: remove gpiod_toggle_active_low()

[PATCH 0/5] gpio: remove gpiod_toggle_active_low()

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

The semantics of gpiod_toggle_active_low() are just bad and in almost
all cases require users to check the current state anyway. Let's replace
it with something clearer and more useful.

For getting this upstream: I'm thinking that I should apply patch 1/5,
provide other subsystems with an immutable tag and then we can apply
patch 5/5 for the next release once first four are in master.

Bartosz Golaszewski (5):
  gpiolib: provide gpiod_set_active_[low/high]()
  mtd: rawnand: ingenic: use gpiod_set_active_high()
  mmc: slot-gpio: use gpiod_set_active_[low|high]()
  platform/x86: int3472/discrete: use gpiod_set_active_low()
  gpiolib: remove gpiod_toggle_active_low()

 drivers/gpio/gpiolib.c                        | 21 ++++++++++++++-----
 drivers/mmc/core/slot-gpio.c                  | 11 +++++-----
 .../mtd/nand/raw/ingenic/ingenic_nand_drv.c   |  5 ++---
 .../x86/intel/int3472/clk_and_regulator.c     |  2 +-
 drivers/platform/x86/intel/int3472/led.c      |  2 +-
 include/linux/gpio/consumer.h                 | 11 ++++++++--
 6 files changed, 34 insertions(+), 18 deletions(-)

-- 
2.39.2

[PATCH 1/5] gpiolib: provide gpiod_set_active_[low/high]()

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Out current interface for changing line polarity is quite cumbersome to
use as it only "toggles" the current state instead of deterministically
setting it. Because of that all but one user in the kernel first need
check the current state anyway. Let's provide two new functions that
allow users to set this value explicitly with the aim of removing the
existing function.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpiolib.c        | 22 ++++++++++++++++++++++
 include/linux/gpio/consumer.h | 14 ++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index edffa0d2acaa..131965814a7c 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2725,6 +2725,28 @@ void gpiod_toggle_active_low(struct gpio_desc *desc)
 }
 EXPORT_SYMBOL_GPL(gpiod_toggle_active_low);
 
+/**
+ * gpiod_set_active_low() - set the GPIO as active-low
+ * @desc: the GPIO descriptor to set the active-low setting for
+ */
+void gpiod_set_active_low(struct gpio_desc *desc)
+{
+	VALIDATE_DESC_VOID(desc);
+	set_bit(FLAG_ACTIVE_LOW, &desc->flags);
+}
+EXPORT_SYMBOL_GPL(gpiod_set_active_low);
+
+/**
+ * gpiod_set_active_high() - set the GPIO as active-high
+ * @desc: the GPIO descriptor to set the active-low setting for
+ */
+void gpiod_set_active_high(struct gpio_desc *desc)
+{
+	VALIDATE_DESC_VOID(desc);
+	clear_bit(FLAG_ACTIVE_LOW, &desc->flags);
+}
+EXPORT_SYMBOL_GPL(gpiod_set_active_high);
+
 static int gpio_chip_get_value(struct gpio_chip *gc, const struct gpio_desc *desc)
 {
 	return gc->get ? gc->get(gc, gpio_chip_hwgpio(desc)) : -EIO;
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 6cc345440a5b..ddbf0d8e4a75 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -160,6 +160,8 @@ int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
 int gpiod_set_config(struct gpio_desc *desc, unsigned long config);
 int gpiod_set_debounce(struct gpio_desc *desc, unsigned int debounce);
 void gpiod_toggle_active_low(struct gpio_desc *desc);
+void gpiod_set_active_low(struct gpio_desc *desc);
+void gpiod_set_active_high(struct gpio_desc *desc);
 
 int gpiod_is_active_low(const struct gpio_desc *desc);
 int gpiod_cansleep(const struct gpio_desc *desc);
@@ -499,6 +501,18 @@ static inline void gpiod_toggle_active_low(struct gpio_desc *desc)
 	WARN_ON(desc);
 }
 
+static inline void gpiod_set_active_low(struct gpio_desc *desc
+{
+	/* GPIO can never have been requested */
+	WARN_ON(desc);
+}
+
+static inline void gpiod_set_active_high(struct gpio_desc *desc)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(desc);
+}
+
 static inline int gpiod_is_active_low(const struct gpio_desc *desc)
 {
 	/* GPIO can never have been requested */
-- 
2.39.2

Re: [PATCH 1/5] gpiolib: provide gpiod_set_active_[low/high]()

[kernel test robot]

Hi Bartosz,

kernel test robot noticed the following build errors:

[auto build test ERROR on brgl/gpio/for-next]
[also build test ERROR on mtd/nand/next linus/master ulf-hansson-mmc-mirror/next v6.6-rc1 next-20230913]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/gpiolib-provide-gpiod_set_active_-low-high/20230913-195053
base:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link:    https://lore.kernel.org/r/20230913115001.23183-2-brgl%40bgdev.pl
patch subject: [PATCH 1/5] gpiolib: provide gpiod_set_active_[low/high]()
config: s390-allnoconfig (https://download.01.org/0day-ci/archive/20230913/202309132304.Uw3cYH9C-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230913/202309132304.Uw3cYH9C-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309132304.Uw3cYH9C-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/nvmem-provider.h:15,
                    from include/linux/rtc.h:18,
                    from include/linux/efi.h:20,
                    from block/partitions/efi.h:19,
                    from block/partitions/msdos.c:32:
>> include/linux/gpio/consumer.h:505:1: error: expected ';', ',' or ')' before '{' token
     505 | {
         | ^


vim +505 include/linux/gpio/consumer.h

   503	
   504	static inline void gpiod_set_active_low(struct gpio_desc *desc
 > 505	{
   506		/* GPIO can never have been requested */
   507		WARN_ON(desc);
   508	}
   509	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/5] gpiolib: provide gpiod_set_active_[low/high]()

[kernel test robot]

Hi Bartosz,

kernel test robot noticed the following build warnings:

[auto build test WARNING on brgl/gpio/for-next]
[also build test WARNING on mtd/nand/next linus/master ulf-hansson-mmc-mirror/next v6.6-rc1 next-20230913]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/gpiolib-provide-gpiod_set_active_-low-high/20230913-195053
base:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link:    https://lore.kernel.org/r/20230913115001.23183-2-brgl%40bgdev.pl
patch subject: [PATCH 1/5] gpiolib: provide gpiod_set_active_[low/high]()
config: alpha-defconfig (https://download.01.org/0day-ci/archive/20230914/202309140219.EH2Y6r52-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230914/202309140219.EH2Y6r52-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309140219.EH2Y6r52-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/nvmem-provider.h:15,
                    from include/linux/rtc.h:18,
                    from include/linux/mc146818rtc.h:15,
                    from drivers/block/floppy.c:183:
   include/linux/gpio/consumer.h:505:1: error: expected ';', ',' or ')' before '{' token
     505 | {
         | ^
>> drivers/block/floppy.c:165:12: warning: 'print_unex' defined but not used [-Wunused-variable]
     165 | static int print_unex = 1;
         |            ^~~~~~~~~~


vim +/print_unex +165 drivers/block/floppy.c

87f530d8f17336 Joe Perches            2010-03-10  163  
^1da177e4c3f41 Linus Torvalds         2005-04-16  164  /* do print messages for unexpected interrupts */
^1da177e4c3f41 Linus Torvalds         2005-04-16 @165  static int print_unex = 1;
^1da177e4c3f41 Linus Torvalds         2005-04-16  166  #include <linux/module.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16  167  #include <linux/sched.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16  168  #include <linux/fs.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16  169  #include <linux/kernel.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16  170  #include <linux/timer.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16  171  #include <linux/workqueue.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16  172  #include <linux/fdreg.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16  173  #include <linux/fd.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16  174  #include <linux/hdreg.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16  175  #include <linux/errno.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16  176  #include <linux/slab.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16  177  #include <linux/mm.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16  178  #include <linux/bio.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16  179  #include <linux/string.h>
50297cbf07427b Marcelo Feitoza Parisi 2006-03-28  180  #include <linux/jiffies.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16  181  #include <linux/fcntl.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16  182  #include <linux/delay.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16 @183  #include <linux/mc146818rtc.h>	/* CMOS defines */
^1da177e4c3f41 Linus Torvalds         2005-04-16  184  #include <linux/ioport.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16  185  #include <linux/interrupt.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16  186  #include <linux/init.h>
b81e0c2372e65e Christoph Hellwig      2021-09-20  187  #include <linux/major.h>
d052d1beff7069 Russell King           2005-10-29  188  #include <linux/platform_device.h>
83f9ef463bcb4b Scott James Remnant    2009-04-02  189  #include <linux/mod_devicetable.h>
b1c82b5c55851b Jes Sorensen           2006-03-23  190  #include <linux/mutex.h>
d49375434ec011 Joe Perches            2010-03-10  191  #include <linux/io.h>
d49375434ec011 Joe Perches            2010-03-10  192  #include <linux/uaccess.h>
0cc15d03bcccdf Andi Kleen             2012-07-02  193  #include <linux/async.h>
229b53c9bf4e11 Al Viro                2017-06-27  194  #include <linux/compat.h>
^1da177e4c3f41 Linus Torvalds         2005-04-16  195  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/5] gpiolib: provide gpiod_set_active_[low/high]()

[kernel test robot]

Hi Bartosz,

kernel test robot noticed the following build warnings:

[auto build test WARNING on brgl/gpio/for-next]
[also build test WARNING on mtd/nand/next linus/master ulf-hansson-mmc-mirror/next v6.6-rc1 next-20230913]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/gpiolib-provide-gpiod_set_active_-low-high/20230913-195053
base:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link:    https://lore.kernel.org/r/20230913115001.23183-2-brgl%40bgdev.pl
patch subject: [PATCH 1/5] gpiolib: provide gpiod_set_active_[low/high]()
config: m68k-defconfig (https://download.01.org/0day-ci/archive/20230914/202309140338.TPkz7l7g-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230914/202309140338.TPkz7l7g-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309140338.TPkz7l7g-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/net/ethernet/smsc/smc91x.c:60:
   include/linux/gpio/consumer.h:505:1: error: expected ';', ',' or ')' before '{' token
     505 | {
         | ^
>> drivers/net/ethernet/smsc/smc91x.c:47:19: warning: 'version' defined but not used [-Wunused-const-variable=]
      47 | static const char version[] =
         |                   ^~~~~~~


vim +/version +47 drivers/net/ethernet/smsc/smc91x.c

^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16 @47  static const char version[] =
6389aa458ed995 drivers/net/ethernet/smsc/smc91x.c Ben Boeckel     2013-11-01  48  	"smc91x.c: v1.1, sep 22 2004 by Nicolas Pitre <nico@fluxnic.net>";
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  49  
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  50  /* Debugging level */
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  51  #ifndef SMC_DEBUG
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  52  #define SMC_DEBUG		0
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  53  #endif
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  54  
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  55  
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  56  #include <linux/module.h>
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  57  #include <linux/kernel.h>
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  58  #include <linux/sched.h>
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  59  #include <linux/delay.h>
c0e906a953f03c drivers/net/ethernet/smsc/smc91x.c Andy Shevchenko 2023-03-16 @60  #include <linux/gpio/consumer.h>
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  61  #include <linux/interrupt.h>
476c32c47a84fc drivers/net/smc91x.c               David Howells   2010-10-07  62  #include <linux/irq.h>
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  63  #include <linux/errno.h>
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  64  #include <linux/ioport.h>
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  65  #include <linux/crc32.h>
d052d1beff7069 drivers/net/smc91x.c               Russell King    2005-10-29  66  #include <linux/platform_device.h>
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  67  #include <linux/spinlock.h>
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  68  #include <linux/ethtool.h>
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  69  #include <linux/mii.h>
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  70  #include <linux/workqueue.h>
682a1694115ec1 drivers/net/smc91x.c               Thomas Chou     2011-01-25  71  #include <linux/of.h>
3f823c15d53dc7 drivers/net/ethernet/smsc/smc91x.c Tony Lindgren   2013-12-11  72  #include <linux/of_device.h>
^1da177e4c3f41 drivers/net/smc91x.c               Linus Torvalds  2005-04-16  73  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 2/5] mtd: rawnand: ingenic: use gpiod_set_active_high()

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Use the new, less cumbersome interface for setting the GPIO as
active-high that doesn't require first checking the current state.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
index 6748226b8bd1..c055133c45fe 100644
--- a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
+++ b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
@@ -388,9 +388,8 @@ static int ingenic_nand_init_chip(struct platform_device *pdev,
 	 * here for older DTs so we can re-use the generic nand_gpio_waitrdy()
 	 * helper, and be consistent with what other drivers do.
 	 */
-	if (of_machine_is_compatible("qi,lb60") &&
-	    gpiod_is_active_low(nand->busy_gpio))
-		gpiod_toggle_active_low(nand->busy_gpio);
+	if (of_machine_is_compatible("qi,lb60"))
+		gpiod_set_active_high(nand->busy_gpio);
 
 	nand->wp_gpio = devm_gpiod_get_optional(dev, "wp", GPIOD_OUT_LOW);
 
-- 
2.39.2

Re: [PATCH 2/5] mtd: rawnand: ingenic: use gpiod_set_active_high()

[Paul Cercueil]

Hi,

Le mercredi 13 septembre 2023 à 13:49 +0200, Bartosz Golaszewski a
écrit :
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Use the new, less cumbersome interface for setting the GPIO as
> active-high that doesn't require first checking the current state.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Works for me.

Acked-by: Paul Cercueil <paul@crapouillou.net>

Cheers,
-Paul

> ---
>  drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
> b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
> index 6748226b8bd1..c055133c45fe 100644
> --- a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
> +++ b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
> @@ -388,9 +388,8 @@ static int ingenic_nand_init_chip(struct
> platform_device *pdev,
>          * here for older DTs so we can re-use the generic
> nand_gpio_waitrdy()
>          * helper, and be consistent with what other drivers do.
>          */
> -       if (of_machine_is_compatible("qi,lb60") &&
> -           gpiod_is_active_low(nand->busy_gpio))
> -               gpiod_toggle_active_low(nand->busy_gpio);
> +       if (of_machine_is_compatible("qi,lb60"))
> +               gpiod_set_active_high(nand->busy_gpio);
>  
>         nand->wp_gpio = devm_gpiod_get_optional(dev, "wp",
> GPIOD_OUT_LOW);
>  

Re: [PATCH 2/5] mtd: rawnand: ingenic: use gpiod_set_active_high()

[Miquel Raynal]

Hi Bartosz,

paul@crapouillou.net wrote on Wed, 13 Sep 2023 14:01:47 +0200:

> Hi,
> 
> Le mercredi 13 septembre 2023 à 13:49 +0200, Bartosz Golaszewski a
> écrit :
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > 
> > Use the new, less cumbersome interface for setting the GPIO as
> > active-high that doesn't require first checking the current state.
> > 
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>  
> 
> Works for me.
> 
> Acked-by: Paul Cercueil <paul@crapouillou.net>

For me as well, the new API looks much better.

I saw Hans is fine with you merging the platform/x86 patch. I am also
fine if all maintainers agree on that. In this case:

Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>

However, if you finally need to produce an immutable tag, let me know
and I will take the patch through our tree.

Thanks,
Miquèl

Re: [PATCH 2/5] mtd: rawnand: ingenic: use gpiod_set_active_high()

[Andy Shevchenko]

On Wed, Sep 13, 2023 at 2:50 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Use the new, less cumbersome interface for setting the GPIO as
> active-high that doesn't require first checking the current state.

...

>          * here for older DTs so we can re-use the generic nand_gpio_waitrdy()
>          * helper, and be consistent with what other drivers do.
>          */
> -       if (of_machine_is_compatible("qi,lb60") &&
> -           gpiod_is_active_low(nand->busy_gpio))
> -               gpiod_toggle_active_low(nand->busy_gpio);
> +       if (of_machine_is_compatible("qi,lb60"))
> +               gpiod_set_active_high(nand->busy_gpio);

Why not moving this quirk to gpiolib-of.c?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/5] mtd: rawnand: ingenic: use gpiod_set_active_high()

[Linus Walleij]

On Wed, Sep 13, 2023 at 10:05 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Sep 13, 2023 at 2:50 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Use the new, less cumbersome interface for setting the GPIO as
> > active-high that doesn't require first checking the current state.
>
> ...
>
> >          * here for older DTs so we can re-use the generic nand_gpio_waitrdy()
> >          * helper, and be consistent with what other drivers do.
> >          */
> > -       if (of_machine_is_compatible("qi,lb60") &&
> > -           gpiod_is_active_low(nand->busy_gpio))
> > -               gpiod_toggle_active_low(nand->busy_gpio);
> > +       if (of_machine_is_compatible("qi,lb60"))
> > +               gpiod_set_active_high(nand->busy_gpio);
>
> Why not moving this quirk to gpiolib-of.c?

That's a better idea here I think, it's clearly a quirk for a
buggy device tree.

Yours,
Linus Walleij

Re: [PATCH 2/5] mtd: rawnand: ingenic: use gpiod_set_active_high()

[Miquel Raynal]

Hi Andy,

linus.walleij@linaro.org wrote on Wed, 13 Sep 2023 22:12:40 +0200:

> On Wed, Sep 13, 2023 at 10:05 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Wed, Sep 13, 2023 at 2:50 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:  
> > >
> > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > >
> > > Use the new, less cumbersome interface for setting the GPIO as
> > > active-high that doesn't require first checking the current state.  
> >
> > ...
> >  
> > >          * here for older DTs so we can re-use the generic nand_gpio_waitrdy()
> > >          * helper, and be consistent with what other drivers do.
> > >          */
> > > -       if (of_machine_is_compatible("qi,lb60") &&
> > > -           gpiod_is_active_low(nand->busy_gpio))
> > > -               gpiod_toggle_active_low(nand->busy_gpio);
> > > +       if (of_machine_is_compatible("qi,lb60"))
> > > +               gpiod_set_active_high(nand->busy_gpio);  
> >
> > Why not moving this quirk to gpiolib-of.c?  
> 
> That's a better idea here I think, it's clearly a quirk for a
> buggy device tree.

Agreed, it's just for backward compatibility purposes in a single
driver. I believe it should stay here.

Thanks,
Miquèl

Re: [PATCH 2/5] mtd: rawnand: ingenic: use gpiod_set_active_high()

[Andy Shevchenko]

On Wed, Sep 13, 2023 at 11:23 PM Miquel Raynal
<miquel.raynal@bootlin.com> wrote:
> linus.walleij@linaro.org wrote on Wed, 13 Sep 2023 22:12:40 +0200:
> > On Wed, Sep 13, 2023 at 10:05 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Wed, Sep 13, 2023 at 2:50 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

...

> > > Why not moving this quirk to gpiolib-of.c?
> >
> > That's a better idea here I think, it's clearly a quirk for a
> > buggy device tree.
>
> Agreed, it's just for backward compatibility purposes in a single
> driver. I believe it should stay here.

I believe Linus was for moving.

gpiolib-of.c contains a lot of quirks, including this one. Calling
these new (or old) APIs for overriding polarity in many cases
shouldn't be needed if were no issues with DT or something like that.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/5] mtd: rawnand: ingenic: use gpiod_set_active_high()

[Andy Shevchenko]

On Thu, Sep 14, 2023 at 10:02 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Wed, Sep 13, 2023 at 11:23 PM Miquel Raynal
> <miquel.raynal@bootlin.com> wrote:
> > linus.walleij@linaro.org wrote on Wed, 13 Sep 2023 22:12:40 +0200:
> > > On Wed, Sep 13, 2023 at 10:05 PM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Wed, Sep 13, 2023 at 2:50 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> ...
>
> > > > Why not moving this quirk to gpiolib-of.c?
> > >
> > > That's a better idea here I think, it's clearly a quirk for a
> > > buggy device tree.
> >
> > Agreed, it's just for backward compatibility purposes in a single
> > driver. I believe it should stay here.
>
> I believe Linus was for moving.
>
> gpiolib-of.c contains a lot of quirks, including this one. Calling

To be clear:
"including one for the same issue"

> these new (or old) APIs for overriding polarity in many cases
> shouldn't be needed if there were no issues with DT or something like that.

To be clear:
The less we call these APIs from drivers the better. Ideally these
APIs shouldn't have existed.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/5] mtd: rawnand: ingenic: use gpiod_set_active_high()

[Paul Cercueil]

Hi,

Le jeudi 14 septembre 2023 à 10:02 +0300, Andy Shevchenko a écrit :
> On Wed, Sep 13, 2023 at 11:23 PM Miquel Raynal
> <miquel.raynal@bootlin.com> wrote:
> > linus.walleij@linaro.org wrote on Wed, 13 Sep 2023 22:12:40 +0200:
> > > On Wed, Sep 13, 2023 at 10:05 PM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Wed, Sep 13, 2023 at 2:50 PM Bartosz Golaszewski
> > > > <brgl@bgdev.pl> wrote:
> 
> ...
> 
> > > > Why not moving this quirk to gpiolib-of.c?
> > > 
> > > That's a better idea here I think, it's clearly a quirk for a
> > > buggy device tree.
> > 
> > Agreed, it's just for backward compatibility purposes in a single
> > driver. I believe it should stay here.
> 
> I believe Linus was for moving.

Which Linus? Because the one who's also the gpio maintainer just wrote
above that it was better to keep it in the driver.

Cheers,
-Paul

> 
> gpiolib-of.c contains a lot of quirks, including this one. Calling
> these new (or old) APIs for overriding polarity in many cases
> shouldn't be needed if were no issues with DT or something like that.
> 

Re: [PATCH 2/5] mtd: rawnand: ingenic: use gpiod_set_active_high()

[Bartosz Golaszewski]

On Thu, Sep 14, 2023 at 10:30 AM Paul Cercueil <paul@crapouillou.net> wrote:
>
> Hi,
>
> Le jeudi 14 septembre 2023 à 10:02 +0300, Andy Shevchenko a écrit :
> > On Wed, Sep 13, 2023 at 11:23 PM Miquel Raynal
> > <miquel.raynal@bootlin.com> wrote:
> > > linus.walleij@linaro.org wrote on Wed, 13 Sep 2023 22:12:40 +0200:
> > > > On Wed, Sep 13, 2023 at 10:05 PM Andy Shevchenko
> > > > <andy.shevchenko@gmail.com> wrote:
> > > > > On Wed, Sep 13, 2023 at 2:50 PM Bartosz Golaszewski
> > > > > <brgl@bgdev.pl> wrote:
> >
> > ...
> >
> > > > > Why not moving this quirk to gpiolib-of.c?
> > > >
> > > > That's a better idea here I think, it's clearly a quirk for a
> > > > buggy device tree.
> > >
> > > Agreed, it's just for backward compatibility purposes in a single
> > > driver. I believe it should stay here.
> >
> > I believe Linus was for moving.
>
> Which Linus? Because the one who's also the gpio maintainer just wrote
> above that it was better to keep it in the driver.
>

I'm also under the impression that Linus meant moving it to gpiolib-of.c. Let's

Linus: Could you clarify?

Bart

> Cheers,
> -Paul
>
> >
> > gpiolib-of.c contains a lot of quirks, including this one. Calling
> > these new (or old) APIs for overriding polarity in many cases
> > shouldn't be needed if were no issues with DT or something like that.
> >
>

Re: [PATCH 2/5] mtd: rawnand: ingenic: use gpiod_set_active_high()

[Linus Walleij]

On Thu, Sep 14, 2023 at 11:30 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> On Thu, Sep 14, 2023 at 10:30 AM Paul Cercueil <paul@crapouillou.net> wrote:

> > > I believe Linus was for moving.

Yes.

> > Which Linus? Because the one who's also the gpio maintainer just wrote
> > above that it was better to keep it in the driver.

What. No. I expressed myself unclearly:

> > Why not moving this quirk to gpiolib-of.c?
>
> That's a better idea here I think, it's clearly a quirk for a
> buggy device tree.

"That's a better idea here I think"

means

"That's a better idea [IN THIS CASE] I think"

i.e. in this case it is a better idea to move it into gpiolib-of.c

> I'm also under the impression that Linus meant moving it to gpiolib-of.c. Let's
>
> Linus: Could you clarify?

Yes.

I invented that thing so I'm a fan of it.

Yours,
Linus Walleij

[PATCH 3/5] mmc: slot-gpio: use gpiod_set_active_[low|high]()

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

We have new, less cumbersome and clearer interfaces for controlling GPIO
polarity. Use them in the MMC code.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/mmc/core/slot-gpio.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 2a2d949a9344..a6fea6559a5e 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -204,12 +204,11 @@ int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
 	}
 
 	/* override forces default (active-low) polarity ... */
-	if (override_active_level && !gpiod_is_active_low(desc))
-		gpiod_toggle_active_low(desc);
-
+	if (override_active_level)
+		gpiod_set_active_low(desc);
 	/* ... or active-high */
-	if (host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH)
-		gpiod_toggle_active_low(desc);
+	else if (host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH)
+		gpiod_set_active_high(desc);
 
 	ctx->cd_gpio = desc;
 
@@ -256,7 +255,7 @@ int mmc_gpiod_request_ro(struct mmc_host *host, const char *con_id,
 	}
 
 	if (host->caps2 & MMC_CAP2_RO_ACTIVE_HIGH)
-		gpiod_toggle_active_low(desc);
+		gpiod_set_active_high(desc);
 
 	ctx->ro_gpio = desc;
 
-- 
2.39.2

Re: [PATCH 3/5] mmc: slot-gpio: use gpiod_set_active_[low|high]()

[Linus Walleij]

On Wed, Sep 13, 2023 at 1:50 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> We have new, less cumbersome and clearer interfaces for controlling GPIO
> polarity. Use them in the MMC code.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

I like the looks of the code better, obviously but this looks like this for
a reason unfortunately.

See the following from
Documentation/devicetree/bindings/mmc/mmc-controller.yaml:

  # CD and WP lines can be implemented on the hardware in one of two
  # ways: as GPIOs, specified in cd-gpios and wp-gpios properties, or
  # as dedicated pins. Polarity of dedicated pins can be specified,
  # using *-inverted properties. GPIO polarity can also be specified
  # using the GPIO_ACTIVE_LOW flag. This creates an ambiguity in the
  # latter case. We choose to use the XOR logic for GPIO CD and WP
  # lines.  This means, the two properties are "superimposed," for
  # example leaving the GPIO_ACTIVE_LOW flag clear and specifying the
  # respective *-inverted property property results in a
  # double-inversion and actually means the "normal" line polarity is
  # in effect.

Will you still provide the desired "double inversion" after this patch?

Yours,
Linus Walleij

Re: [PATCH 3/5] mmc: slot-gpio: use gpiod_set_active_[low|high]()

[Bartosz Golaszewski]

On Wed, Sep 13, 2023 at 2:24 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Wed, Sep 13, 2023 at 1:50 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > We have new, less cumbersome and clearer interfaces for controlling GPIO
> > polarity. Use them in the MMC code.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> I like the looks of the code better, obviously but this looks like this for
> a reason unfortunately.
>
> See the following from
> Documentation/devicetree/bindings/mmc/mmc-controller.yaml:
>
>   # CD and WP lines can be implemented on the hardware in one of two
>   # ways: as GPIOs, specified in cd-gpios and wp-gpios properties, or
>   # as dedicated pins. Polarity of dedicated pins can be specified,
>   # using *-inverted properties. GPIO polarity can also be specified
>   # using the GPIO_ACTIVE_LOW flag. This creates an ambiguity in the
>   # latter case. We choose to use the XOR logic for GPIO CD and WP
>   # lines.  This means, the two properties are "superimposed," for
>   # example leaving the GPIO_ACTIVE_LOW flag clear and specifying the
>   # respective *-inverted property property results in a
>   # double-inversion and actually means the "normal" line polarity is
>   # in effect.
>

I hate it, thanks. :)

> Will you still provide the desired "double inversion" after this patch?
>

Not in the current form. Would it work to go:

if (override_active_level) {
    if (!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH))
        gpiod_set_active_high(desc);
    else
        gpiod_set_active_low(desc);
} else {
    if (host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH)
        gpiod_set_active_high(desc);
    else
        gpiod_set_active_low(desc);
}

?

Alternatively we could reimplement the toggle semantics locally in a
helper function in order to get rid of it from GPIOLIB.

Bart

Re: [PATCH 3/5] mmc: slot-gpio: use gpiod_set_active_[low|high]()

[Linus Walleij]

On Wed, Sep 13, 2023 at 2:39 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> On Wed, Sep 13, 2023 at 2:24 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > On Wed, Sep 13, 2023 at 1:50 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > >
> > > We have new, less cumbersome and clearer interfaces for controlling GPIO
> > > polarity. Use them in the MMC code.
> > >
> > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > I like the looks of the code better, obviously but this looks like this for
> > a reason unfortunately.
> >
> > See the following from
> > Documentation/devicetree/bindings/mmc/mmc-controller.yaml:
> >
> >   # CD and WP lines can be implemented on the hardware in one of two
> >   # ways: as GPIOs, specified in cd-gpios and wp-gpios properties, or
> >   # as dedicated pins. Polarity of dedicated pins can be specified,
> >   # using *-inverted properties. GPIO polarity can also be specified
> >   # using the GPIO_ACTIVE_LOW flag. This creates an ambiguity in the
> >   # latter case. We choose to use the XOR logic for GPIO CD and WP
> >   # lines.  This means, the two properties are "superimposed," for
> >   # example leaving the GPIO_ACTIVE_LOW flag clear and specifying the
> >   # respective *-inverted property property results in a
> >   # double-inversion and actually means the "normal" line polarity is
> >   # in effect.
> >
>
> I hate it, thanks. :)
>
> > Will you still provide the desired "double inversion" after this patch?
> >
>
> Not in the current form. Would it work to go:
>
> if (override_active_level) {
>     if (!(host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH))
>         gpiod_set_active_high(desc);
>     else
>         gpiod_set_active_low(desc);
> } else {
>     if (host->caps2 & MMC_CAP2_CD_ACTIVE_HIGH)
>         gpiod_set_active_high(desc);
>     else
>         gpiod_set_active_low(desc);
> }
>
> ?

I *think* so but my boolean parser i known to be flawed since I have
screwed up double inversions repeatedly over the years, so it should
not be trusted at all.

> Alternatively we could reimplement the toggle semantics locally in a
> helper function in order to get rid of it from GPIOLIB.

I don't know about that, the flag is inside gpio_desc so we cannot
access it (struct is private to gpiolib...)

Yours,
Linus Walleij

Re: [PATCH 3/5] mmc: slot-gpio: use gpiod_set_active_[low|high]()

[Linus Walleij]

On Thu, Sep 14, 2023 at 10:31 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Sep 13, 2023 at 2:39 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> > Alternatively we could reimplement the toggle semantics locally in a
> > helper function in order to get rid of it from GPIOLIB.
>
> I don't know about that, the flag is inside gpio_desc so we cannot
> access it (struct is private to gpiolib...)

Actually I think the way the toggle call came about was for this one
MMC usecase.

Then other subsystems have used it without asking the GPIO
maintainers or without implementing the more proper accessors
or patching drivers/gpio/gpiolib-of.c because why not, probably
thinking something like "hey weird that it is just toggle I guess they
are not so smart, but it works, ship it".

Yours,
Linus Walleij

[PATCH 4/5] platform/x86: int3472/discrete: use gpiod_set_active_low()

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Use the new polarity setter instead of the more cumbersome toggle
function.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/platform/x86/intel/int3472/clk_and_regulator.c | 2 +-
 drivers/platform/x86/intel/int3472/led.c               | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/clk_and_regulator.c b/drivers/platform/x86/intel/int3472/clk_and_regulator.c
index ef4b3141efcd..31e520838b95 100644
--- a/drivers/platform/x86/intel/int3472/clk_and_regulator.c
+++ b/drivers/platform/x86/intel/int3472/clk_and_regulator.c
@@ -183,7 +183,7 @@ int skl_int3472_register_gpio_clock(struct int3472_discrete_device *int3472,
 	}
 
 	if (polarity == GPIO_ACTIVE_LOW)
-		gpiod_toggle_active_low(int3472->clock.ena_gpio);
+		gpiod_set_active_low(int3472->clock.ena_gpio);
 
 	/* Ensure the pin is in output mode and non-active state */
 	gpiod_direction_output(int3472->clock.ena_gpio, 0);
diff --git a/drivers/platform/x86/intel/int3472/led.c b/drivers/platform/x86/intel/int3472/led.c
index bca1ce7d0d0c..46c9c569df5e 100644
--- a/drivers/platform/x86/intel/int3472/led.c
+++ b/drivers/platform/x86/intel/int3472/led.c
@@ -32,7 +32,7 @@ int skl_int3472_register_pled(struct int3472_discrete_device *int3472,
 				     "getting privacy LED GPIO\n");
 
 	if (polarity == GPIO_ACTIVE_LOW)
-		gpiod_toggle_active_low(int3472->pled.gpio);
+		gpiod_set_active_low(int3472->pled.gpio);
 
 	/* Ensure the pin is in output mode and non-active state */
 	gpiod_direction_output(int3472->pled.gpio, 0);
-- 
2.39.2

Re: [PATCH 4/5] platform/x86: int3472/discrete: use gpiod_set_active_low()

[Hans de Goede]

Hi,

On 9/13/23 13:50, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Use the new polarity setter instead of the more cumbersome toggle
> function.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  drivers/platform/x86/intel/int3472/clk_and_regulator.c | 2 +-
>  drivers/platform/x86/intel/int3472/led.c               | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/int3472/clk_and_regulator.c b/drivers/platform/x86/intel/int3472/clk_and_regulator.c
> index ef4b3141efcd..31e520838b95 100644
> --- a/drivers/platform/x86/intel/int3472/clk_and_regulator.c
> +++ b/drivers/platform/x86/intel/int3472/clk_and_regulator.c
> @@ -183,7 +183,7 @@ int skl_int3472_register_gpio_clock(struct int3472_discrete_device *int3472,
>  	}
>  
>  	if (polarity == GPIO_ACTIVE_LOW)
> -		gpiod_toggle_active_low(int3472->clock.ena_gpio);
> +		gpiod_set_active_low(int3472->clock.ena_gpio);
>  
>  	/* Ensure the pin is in output mode and non-active state */
>  	gpiod_direction_output(int3472->clock.ena_gpio, 0);
> diff --git a/drivers/platform/x86/intel/int3472/led.c b/drivers/platform/x86/intel/int3472/led.c
> index bca1ce7d0d0c..46c9c569df5e 100644
> --- a/drivers/platform/x86/intel/int3472/led.c
> +++ b/drivers/platform/x86/intel/int3472/led.c
> @@ -32,7 +32,7 @@ int skl_int3472_register_pled(struct int3472_discrete_device *int3472,
>  				     "getting privacy LED GPIO\n");
>  
>  	if (polarity == GPIO_ACTIVE_LOW)
> -		gpiod_toggle_active_low(int3472->pled.gpio);
> +		gpiod_set_active_low(int3472->pled.gpio);
>  
>  	/* Ensure the pin is in output mode and non-active state */
>  	gpiod_direction_output(int3472->pled.gpio, 0);


Thanks. I agree that the new API is much better:

Acked-by: Hans de Goede <hdegoede@redhat.com>

Feel free to merge this through the GPIO tree.

Regards,

Hans

[PATCH 5/5] gpiolib: remove gpiod_toggle_active_low()

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

With all users now having switched to gpiod_set_active_[low|high](), we
can now remove gpiod_toggle_active_low().

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpiolib.c        | 11 -----------
 include/linux/gpio/consumer.h |  7 -------
 2 files changed, 18 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 131965814a7c..14b84bad93ea 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2714,17 +2714,6 @@ int gpiod_is_active_low(const struct gpio_desc *desc)
 }
 EXPORT_SYMBOL_GPL(gpiod_is_active_low);
 
-/**
- * gpiod_toggle_active_low - toggle whether a GPIO is active-low or not
- * @desc: the gpio descriptor to change
- */
-void gpiod_toggle_active_low(struct gpio_desc *desc)
-{
-	VALIDATE_DESC_VOID(desc);
-	change_bit(FLAG_ACTIVE_LOW, &desc->flags);
-}
-EXPORT_SYMBOL_GPL(gpiod_toggle_active_low);
-
 /**
  * gpiod_set_active_low() - set the GPIO as active-low
  * @desc: the GPIO descriptor to set the active-low setting for
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index ddbf0d8e4a75..395e1a67c4c8 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -159,7 +159,6 @@ int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
 
 int gpiod_set_config(struct gpio_desc *desc, unsigned long config);
 int gpiod_set_debounce(struct gpio_desc *desc, unsigned int debounce);
-void gpiod_toggle_active_low(struct gpio_desc *desc);
 void gpiod_set_active_low(struct gpio_desc *desc);
 void gpiod_set_active_high(struct gpio_desc *desc);
 
@@ -495,12 +494,6 @@ static inline int gpiod_set_debounce(struct gpio_desc *desc, unsigned int deboun
 	return -ENOSYS;
 }
 
-static inline void gpiod_toggle_active_low(struct gpio_desc *desc)
-{
-	/* GPIO can never have been requested */
-	WARN_ON(desc);
-}
-
 static inline void gpiod_set_active_low(struct gpio_desc *desc
 {
 	/* GPIO can never have been requested */
-- 
2.39.2