| Message ID | 1b1f706b60e4c571c4f17d53ac640e8bd8384856.1613134924.git.syednwaris@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce the for_each_set_clump macro | expand |
On Fri, Feb 12, 2021 at 06:52:00PM +0530, Syed Nayyar Waris wrote: > This patch reimplements the xgpio_set_multiple() function in > drivers/gpio/gpio-xilinx.c to use the new generic functions: > bitmap_get_value() and bitmap_set_value(). The code is now simpler > to read and understand. Moreover, instead of looping for each bit > in xgpio_set_multiple() function, now we can check each channel at > a time and save cycles. > > Cc: William Breathitt Gray <vilhelm.gray@gmail.com> > Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com> > Cc: Michal Simek <michal.simek@xilinx.com> > Signed-off-by: Syed Nayyar Waris <syednwaris@gmail.com> Acked-by: William Breathitt Gray <vilhelm.gray@gmail.com> > --- > drivers/gpio/gpio-xilinx.c | 63 +++++++++++++++++++------------------- > 1 file changed, 32 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c > index be539381fd82..8445e69cf37b 100644 > --- a/drivers/gpio/gpio-xilinx.c > +++ b/drivers/gpio/gpio-xilinx.c > @@ -15,6 +15,7 @@ > #include <linux/of_device.h> > #include <linux/of_platform.h> > #include <linux/slab.h> > +#include "gpiolib.h" > > /* Register Offset Definitions */ > #define XGPIO_DATA_OFFSET (0x0) /* Data register */ > @@ -141,37 +142,37 @@ static void xgpio_set_multiple(struct gpio_chip *gc, unsigned long *mask, > { > unsigned long flags; > struct xgpio_instance *chip = gpiochip_get_data(gc); > - int index = xgpio_index(chip, 0); > - int offset, i; > - > - spin_lock_irqsave(&chip->gpio_lock[index], flags); > - > - /* Write to GPIO signals */ > - for (i = 0; i < gc->ngpio; i++) { > - if (*mask == 0) > - break; > - /* Once finished with an index write it out to the register */ > - if (index != xgpio_index(chip, i)) { > - xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET + > - index * XGPIO_CHANNEL_OFFSET, > - chip->gpio_state[index]); > - spin_unlock_irqrestore(&chip->gpio_lock[index], flags); > - index = xgpio_index(chip, i); > - spin_lock_irqsave(&chip->gpio_lock[index], flags); > - } > - if (__test_and_clear_bit(i, mask)) { > - offset = xgpio_offset(chip, i); > - if (test_bit(i, bits)) > - chip->gpio_state[index] |= BIT(offset); > - else > - chip->gpio_state[index] &= ~BIT(offset); > - } > - } > - > - xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET + > - index * XGPIO_CHANNEL_OFFSET, chip->gpio_state[index]); > - > - spin_unlock_irqrestore(&chip->gpio_lock[index], flags); > + u32 *const state = chip->gpio_state; > + unsigned int *const width = chip->gpio_width; > + > + DECLARE_BITMAP(old, 64); > + DECLARE_BITMAP(new, 64); > + DECLARE_BITMAP(changed, 64); > + > + spin_lock_irqsave(&chip->gpio_lock[0], flags); > + spin_lock(&chip->gpio_lock[1]); > + > + bitmap_set_value(old, 64, state[0], width[0], 0); > + bitmap_set_value(old, 64, state[1], width[1], width[0]); > + bitmap_replace(new, old, bits, mask, gc->ngpio); > + > + bitmap_set_value(old, 64, state[0], 32, 0); > + bitmap_set_value(old, 64, state[1], 32, 32); > + state[0] = bitmap_get_value(new, 0, width[0]); > + state[1] = bitmap_get_value(new, width[0], width[1]); > + bitmap_set_value(new, 64, state[0], 32, 0); > + bitmap_set_value(new, 64, state[1], 32, 32); > + bitmap_xor(changed, old, new, 64); > + > + if (((u32 *)changed)[0]) > + xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET, > + state[0]); > + if (((u32 *)changed)[1]) > + xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET + > + XGPIO_CHANNEL_OFFSET, state[1]); > + > + spin_unlock(&chip->gpio_lock[1]); > + spin_unlock_irqrestore(&chip->gpio_lock[0], flags); > } > > /** > -- > 2.29.0 >
On Sun, Feb 14, 2021 at 10:11 AM kernel test robot <lkp@intel.com> wrote: > > Hi Syed, > > I love your patch! Yet something to improve: > > [auto build test ERROR on e71ba9452f0b5b2e8dc8aa5445198cd9214a6a62] > > url: https://github.com/0day-ci/linux/commits/Syed-Nayyar-Waris/Introduce-the-for_each_set_clump-macro/20210212-213005 > base: e71ba9452f0b5b2e8dc8aa5445198cd9214a6a62 > config: i386-randconfig-d002-20200329 (attached as .config) > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 > reproduce (this is a W=1 build): > # https://github.com/0day-ci/linux/commit/d83196ca7a23f614773c049b69ce3896679cec61 > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review Syed-Nayyar-Waris/Introduce-the-for_each_set_clump-macro/20210212-213005 > git checkout d83196ca7a23f614773c049b69ce3896679cec61 > # save the attached .config to linux build tree > make W=1 ARCH=i386 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > > All errors (new ones prefixed by >>, old ones prefixed by <<): > > >> ERROR: modpost: "bitmap_get_value" [drivers/gpio/gpio-xilinx.ko] undefined! > >> ERROR: modpost: "bitmap_set_value" [drivers/gpio/gpio-xilinx.ko] undefined! > Dear All, I was able to reproduce the above 2 build errors with the config file provided and the build command mentioned in the above mail. But I need help to fix the build errors. Scenario: Function 'bitmap_get_value()' is declared in 'gpiolib.h'. It is defined in 'gpiolib.c'. Function 'bitmap_get_value()' is then being used in gpio-xilinx.c and the build error is thrown. Similar situation holds for the other function 'bitmap_set_value'. How do I resolve the above build error?. Kindly illuminate. The full patchset can be found at: https://lore.kernel.org/patchwork/cover/1380056/ Regards Syed Nayyar Waris
On Mon, Feb 15, 2021 at 06:56:10PM +0530, Syed Nayyar Waris wrote: > On Sun, Feb 14, 2021 at 10:11 AM kernel test robot <lkp@intel.com> wrote: > > > > Hi Syed, > > > > I love your patch! Yet something to improve: > > > > [auto build test ERROR on e71ba9452f0b5b2e8dc8aa5445198cd9214a6a62] > > > > url: https://github.com/0day-ci/linux/commits/Syed-Nayyar-Waris/Introduce-the-for_each_set_clump-macro/20210212-213005 > > base: e71ba9452f0b5b2e8dc8aa5445198cd9214a6a62 > > config: i386-randconfig-d002-20200329 (attached as .config) > > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 > > reproduce (this is a W=1 build): > > # https://github.com/0day-ci/linux/commit/d83196ca7a23f614773c049b69ce3896679cec61 > > git remote add linux-review https://github.com/0day-ci/linux > > git fetch --no-tags linux-review Syed-Nayyar-Waris/Introduce-the-for_each_set_clump-macro/20210212-213005 > > git checkout d83196ca7a23f614773c049b69ce3896679cec61 > > # save the attached .config to linux build tree > > make W=1 ARCH=i386 > > > > If you fix the issue, kindly add following tag as appropriate > > Reported-by: kernel test robot <lkp@intel.com> > > > > All errors (new ones prefixed by >>, old ones prefixed by <<): > > > > >> ERROR: modpost: "bitmap_get_value" [drivers/gpio/gpio-xilinx.ko] undefined! > > >> ERROR: modpost: "bitmap_set_value" [drivers/gpio/gpio-xilinx.ko] undefined! > > > > Dear All, > > I was able to reproduce the above 2 build errors with the config file > provided and the build command mentioned in the above mail. But I need > help to fix the build errors. > > Scenario: > Function 'bitmap_get_value()' is declared in 'gpiolib.h'. It is > defined in 'gpiolib.c'. Function 'bitmap_get_value()' is then being > used in gpio-xilinx.c and the build error is thrown. Similar situation > holds for the other function 'bitmap_set_value'. > > How do I resolve the above build error?. Kindly illuminate. The full > patchset can be found at: > https://lore.kernel.org/patchwork/cover/1380056/ Missed EXPORT_SYMBOL_GPL() or so?
On Mon, Feb 15, 2021 at 7:16 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Mon, Feb 15, 2021 at 06:56:10PM +0530, Syed Nayyar Waris wrote: > > On Sun, Feb 14, 2021 at 10:11 AM kernel test robot <lkp@intel.com> wrote: > > > > > > Hi Syed, > > > > > > I love your patch! Yet something to improve: > > > > > > [auto build test ERROR on e71ba9452f0b5b2e8dc8aa5445198cd9214a6a62] > > > > > > url: https://github.com/0day-ci/linux/commits/Syed-Nayyar-Waris/Introduce-the-for_each_set_clump-macro/20210212-213005 > > > base: e71ba9452f0b5b2e8dc8aa5445198cd9214a6a62 > > > config: i386-randconfig-d002-20200329 (attached as .config) > > > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 > > > reproduce (this is a W=1 build): > > > # https://github.com/0day-ci/linux/commit/d83196ca7a23f614773c049b69ce3896679cec61 > > > git remote add linux-review https://github.com/0day-ci/linux > > > git fetch --no-tags linux-review Syed-Nayyar-Waris/Introduce-the-for_each_set_clump-macro/20210212-213005 > > > git checkout d83196ca7a23f614773c049b69ce3896679cec61 > > > # save the attached .config to linux build tree > > > make W=1 ARCH=i386 > > > > > > If you fix the issue, kindly add following tag as appropriate > > > Reported-by: kernel test robot <lkp@intel.com> > > > > > > All errors (new ones prefixed by >>, old ones prefixed by <<): > > > > > > >> ERROR: modpost: "bitmap_get_value" [drivers/gpio/gpio-xilinx.ko] undefined! > > > >> ERROR: modpost: "bitmap_set_value" [drivers/gpio/gpio-xilinx.ko] undefined! > > > > > > > Dear All, > > > > I was able to reproduce the above 2 build errors with the config file > > provided and the build command mentioned in the above mail. But I need > > help to fix the build errors. > > > > Scenario: > > Function 'bitmap_get_value()' is declared in 'gpiolib.h'. It is > > defined in 'gpiolib.c'. Function 'bitmap_get_value()' is then being > > used in gpio-xilinx.c and the build error is thrown. Similar situation > > holds for the other function 'bitmap_set_value'. > > > > How do I resolve the above build error?. Kindly illuminate. The full > > patchset can be found at: > > https://lore.kernel.org/patchwork/cover/1380056/ > > Missed EXPORT_SYMBOL_GPL() or so? It has EXPORT_SYMBOL_GPL(). But the build errors still persist. Thanks Syed > > > -- > With Best Regards, > Andy Shevchenko > >
On Mon, Feb 15, 2021 at 07:23:10PM +0530, Syed Nayyar Waris wrote: > On Mon, Feb 15, 2021 at 7:16 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Mon, Feb 15, 2021 at 06:56:10PM +0530, Syed Nayyar Waris wrote: > > > On Sun, Feb 14, 2021 at 10:11 AM kernel test robot <lkp@intel.com> wrote: > > > > >> ERROR: modpost: "bitmap_get_value" [drivers/gpio/gpio-xilinx.ko] undefined! > > > > >> ERROR: modpost: "bitmap_set_value" [drivers/gpio/gpio-xilinx.ko] undefined! > > > > > > > > > > Dear All, > > > > > > I was able to reproduce the above 2 build errors with the config file > > > provided and the build command mentioned in the above mail. But I need > > > help to fix the build errors. > > > > > > Scenario: > > > Function 'bitmap_get_value()' is declared in 'gpiolib.h'. It is > > > defined in 'gpiolib.c'. Function 'bitmap_get_value()' is then being > > > used in gpio-xilinx.c and the build error is thrown. Similar situation > > > holds for the other function 'bitmap_set_value'. > > > > > > How do I resolve the above build error?. Kindly illuminate. The full > > > patchset can be found at: > > > https://lore.kernel.org/patchwork/cover/1380056/ > > > > Missed EXPORT_SYMBOL_GPL() or so? > > It has EXPORT_SYMBOL_GPL(). But the build errors still persist. Okay, I don't see an email with configuration file, so you need to dig it up to understand what combination of options brought this.
diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c index be539381fd82..8445e69cf37b 100644 --- a/drivers/gpio/gpio-xilinx.c +++ b/drivers/gpio/gpio-xilinx.c @@ -15,6 +15,7 @@ #include <linux/of_device.h> #include <linux/of_platform.h> #include <linux/slab.h> +#include "gpiolib.h" /* Register Offset Definitions */ #define XGPIO_DATA_OFFSET (0x0) /* Data register */ @@ -141,37 +142,37 @@ static void xgpio_set_multiple(struct gpio_chip *gc, unsigned long *mask, { unsigned long flags; struct xgpio_instance *chip = gpiochip_get_data(gc); - int index = xgpio_index(chip, 0); - int offset, i; - - spin_lock_irqsave(&chip->gpio_lock[index], flags); - - /* Write to GPIO signals */ - for (i = 0; i < gc->ngpio; i++) { - if (*mask == 0) - break; - /* Once finished with an index write it out to the register */ - if (index != xgpio_index(chip, i)) { - xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET + - index * XGPIO_CHANNEL_OFFSET, - chip->gpio_state[index]); - spin_unlock_irqrestore(&chip->gpio_lock[index], flags); - index = xgpio_index(chip, i); - spin_lock_irqsave(&chip->gpio_lock[index], flags); - } - if (__test_and_clear_bit(i, mask)) { - offset = xgpio_offset(chip, i); - if (test_bit(i, bits)) - chip->gpio_state[index] |= BIT(offset); - else - chip->gpio_state[index] &= ~BIT(offset); - } - } - - xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET + - index * XGPIO_CHANNEL_OFFSET, chip->gpio_state[index]); - - spin_unlock_irqrestore(&chip->gpio_lock[index], flags); + u32 *const state = chip->gpio_state; + unsigned int *const width = chip->gpio_width; + + DECLARE_BITMAP(old, 64); + DECLARE_BITMAP(new, 64); + DECLARE_BITMAP(changed, 64); + + spin_lock_irqsave(&chip->gpio_lock[0], flags); + spin_lock(&chip->gpio_lock[1]); + + bitmap_set_value(old, 64, state[0], width[0], 0); + bitmap_set_value(old, 64, state[1], width[1], width[0]); + bitmap_replace(new, old, bits, mask, gc->ngpio); + + bitmap_set_value(old, 64, state[0], 32, 0); + bitmap_set_value(old, 64, state[1], 32, 32); + state[0] = bitmap_get_value(new, 0, width[0]); + state[1] = bitmap_get_value(new, width[0], width[1]); + bitmap_set_value(new, 64, state[0], 32, 0); + bitmap_set_value(new, 64, state[1], 32, 32); + bitmap_xor(changed, old, new, 64); + + if (((u32 *)changed)[0]) + xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET, + state[0]); + if (((u32 *)changed)[1]) + xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET + + XGPIO_CHANNEL_OFFSET, state[1]); + + spin_unlock(&chip->gpio_lock[1]); + spin_unlock_irqrestore(&chip->gpio_lock[0], flags); } /**
This patch reimplements the xgpio_set_multiple() function in drivers/gpio/gpio-xilinx.c to use the new generic functions: bitmap_get_value() and bitmap_set_value(). The code is now simpler to read and understand. Moreover, instead of looping for each bit in xgpio_set_multiple() function, now we can check each channel at a time and save cycles. Cc: William Breathitt Gray <vilhelm.gray@gmail.com> Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com> Cc: Michal Simek <michal.simek@xilinx.com> Signed-off-by: Syed Nayyar Waris <syednwaris@gmail.com> --- drivers/gpio/gpio-xilinx.c | 63 +++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 31 deletions(-)