[PATCH v1] gpio: gpiolib: clear the array_info's memory space

[PATCH v1] gpio: gpiolib: clear the array_info's memory space

[Yan Wang]

if hardware number different to array index,it needs to clear to points
memory space if the array_info have been assigned a value.

Signed-off-by: Yan Wang <rk.code@outlook.com>
---
 drivers/gpio/gpiolib.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 04fb05df805b..cdaffcdd45b2 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4340,8 +4340,11 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 		}
 
 		/* If there is no cache for fast bitmap processing path, continue */
-		if (!array_info)
+		if (!array_info) {
+			/*clear descs->info*/
+			memset(array_info, 0, sizeof(struct gpio_array));
 			continue;
+		}
 
 		/* Unmark array members which don't belong to the 'fast' chip */
 		if (array_info->chip != gc) {
-- 
2.17.1

Re: [PATCH v1] gpio: gpiolib: clear the array_info's memory space

[kernel test robot]

Hi Yan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on brgl/gpio/for-next]
[also build test WARNING on next-20230421]
[cannot apply to linus/master v6.3-rc7]
[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/Yan-Wang/gpio-gpiolib-clear-the-array_info-s-memory-space/20230423-190500
base:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link:    https://lore.kernel.org/r/KL1PR01MB54489B7A3D9D02D242B4BDA1E6669%40KL1PR01MB5448.apcprd01.prod.exchangelabs.com
patch subject: [PATCH v1] gpio: gpiolib: clear the array_info's memory space
config: arm-randconfig-r046-20230423 (https://download.01.org/0day-ci/archive/20230423/202304232025.QmaYH1Ov-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/061c9f6937fab64a9c1d051252fcd3236a35381f
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Yan-Wang/gpio-gpiolib-clear-the-array_info-s-memory-space/20230423-190500
        git checkout 061c9f6937fab64a9c1d051252fcd3236a35381f
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/gpio/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304232025.QmaYH1Ov-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/string.h:254,
                    from include/linux/bitmap.h:11,
                    from include/linux/cpumask.h:12,
                    from include/linux/smp.h:13,
                    from include/linux/lockdep.h:14,
                    from include/linux/mutex.h:17,
                    from include/linux/kernfs.h:11,
                    from include/linux/sysfs.h:16,
                    from include/linux/kobject.h:20,
                    from include/linux/of.h:17,
                    from include/linux/irqdomain.h:36,
                    from include/linux/acpi.h:13,
                    from drivers/gpio/gpiolib.c:3:
   drivers/gpio/gpiolib.c: In function 'gpiod_get_array':
>> include/linux/fortify-string.h:59:33: warning: argument 1 null where non-null expected [-Wnonnull]
      59 | #define __underlying_memset     __builtin_memset
         |                                 ^
   include/linux/fortify-string.h:453:9: note: in expansion of macro '__underlying_memset'
     453 |         __underlying_memset(p, c, __fortify_size);                      \
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/fortify-string.h:461:25: note: in expansion of macro '__fortify_memset_chk'
     461 | #define memset(p, c, s) __fortify_memset_chk(p, c, s,                   \
         |                         ^~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpiolib.c:4345:25: note: in expansion of macro 'memset'
    4345 |                         memset(array_info, 0, sizeof(struct gpio_array));
         |                         ^~~~~~
   include/linux/fortify-string.h:59:33: note: in a call to built-in function '__builtin_memset'
      59 | #define __underlying_memset     __builtin_memset
         |                                 ^
   include/linux/fortify-string.h:453:9: note: in expansion of macro '__underlying_memset'
     453 |         __underlying_memset(p, c, __fortify_size);                      \
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/fortify-string.h:461:25: note: in expansion of macro '__fortify_memset_chk'
     461 | #define memset(p, c, s) __fortify_memset_chk(p, c, s,                   \
         |                         ^~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpiolib.c:4345:25: note: in expansion of macro 'memset'
    4345 |                         memset(array_info, 0, sizeof(struct gpio_array));
         |                         ^~~~~~


vim +59 include/linux/fortify-string.h

78a498c3a227f2 Alexander Potapenko 2022-10-24  46  
78a498c3a227f2 Alexander Potapenko 2022-10-24  47  #if defined(__SANITIZE_MEMORY__)
78a498c3a227f2 Alexander Potapenko 2022-10-24  48  /*
78a498c3a227f2 Alexander Potapenko 2022-10-24  49   * For KMSAN builds all memcpy/memset/memmove calls should be replaced by the
78a498c3a227f2 Alexander Potapenko 2022-10-24  50   * corresponding __msan_XXX functions.
78a498c3a227f2 Alexander Potapenko 2022-10-24  51   */
78a498c3a227f2 Alexander Potapenko 2022-10-24  52  #include <linux/kmsan_string.h>
78a498c3a227f2 Alexander Potapenko 2022-10-24  53  #define __underlying_memcpy	__msan_memcpy
78a498c3a227f2 Alexander Potapenko 2022-10-24  54  #define __underlying_memmove	__msan_memmove
78a498c3a227f2 Alexander Potapenko 2022-10-24  55  #define __underlying_memset	__msan_memset
78a498c3a227f2 Alexander Potapenko 2022-10-24  56  #else
a28a6e860c6cf2 Francis Laniel      2021-02-25  57  #define __underlying_memcpy	__builtin_memcpy
a28a6e860c6cf2 Francis Laniel      2021-02-25  58  #define __underlying_memmove	__builtin_memmove
a28a6e860c6cf2 Francis Laniel      2021-02-25 @59  #define __underlying_memset	__builtin_memset
78a498c3a227f2 Alexander Potapenko 2022-10-24  60  #endif
78a498c3a227f2 Alexander Potapenko 2022-10-24  61  

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

Re: [PATCH v1] gpio: gpiolib: clear the array_info's memory space

[kernel test robot]

Hi Yan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on brgl/gpio/for-next]
[also build test WARNING on next-20230421]
[cannot apply to linus/master v6.3-rc7]
[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/Yan-Wang/gpio-gpiolib-clear-the-array_info-s-memory-space/20230423-190500
base:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link:    https://lore.kernel.org/r/KL1PR01MB54489B7A3D9D02D242B4BDA1E6669%40KL1PR01MB5448.apcprd01.prod.exchangelabs.com
patch subject: [PATCH v1] gpio: gpiolib: clear the array_info's memory space
config: microblaze-buildonly-randconfig-r006-20230423 (https://download.01.org/0day-ci/archive/20230423/202304232146.7M89pwCz-lkp@intel.com/config)
compiler: microblaze-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/061c9f6937fab64a9c1d051252fcd3236a35381f
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Yan-Wang/gpio-gpiolib-clear-the-array_info-s-memory-space/20230423-190500
        git checkout 061c9f6937fab64a9c1d051252fcd3236a35381f
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=microblaze olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=microblaze SHELL=/bin/bash drivers/gpio/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304232146.7M89pwCz-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/gpio/gpiolib.c: In function 'gpiod_get_array':
>> drivers/gpio/gpiolib.c:4345:25: warning: argument 1 null where non-null expected [-Wnonnull]
    4345 |                         memset(array_info, 0, sizeof(struct gpio_array));
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/string.h:20,
                    from include/linux/bitmap.h:11,
                    from include/linux/cpumask.h:12,
                    from include/linux/smp.h:13,
                    from include/linux/lockdep.h:14,
                    from include/linux/mutex.h:17,
                    from include/linux/kernfs.h:11,
                    from include/linux/sysfs.h:16,
                    from include/linux/kobject.h:20,
                    from include/linux/of.h:17,
                    from include/linux/irqdomain.h:36,
                    from include/linux/acpi.h:13,
                    from drivers/gpio/gpiolib.c:3:
   arch/microblaze/include/asm/string.h:16:14: note: in a call to function 'memset' declared 'nonnull'
      16 | extern void *memset(void *, int, __kernel_size_t);
         |              ^~~~~~


vim +4345 drivers/gpio/gpiolib.c

  4263	
  4264	/**
  4265	 * gpiod_get_array - obtain multiple GPIOs from a multi-index GPIO function
  4266	 * @dev:	GPIO consumer, can be NULL for system-global GPIOs
  4267	 * @con_id:	function within the GPIO consumer
  4268	 * @flags:	optional GPIO initialization flags
  4269	 *
  4270	 * This function acquires all the GPIOs defined under a given function.
  4271	 *
  4272	 * Return a struct gpio_descs containing an array of descriptors, -ENOENT if
  4273	 * no GPIO has been assigned to the requested function, or another IS_ERR()
  4274	 * code if an error occurred while trying to acquire the GPIOs.
  4275	 */
  4276	struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
  4277							const char *con_id,
  4278							enum gpiod_flags flags)
  4279	{
  4280		struct gpio_desc *desc;
  4281		struct gpio_descs *descs;
  4282		struct gpio_array *array_info = NULL;
  4283		struct gpio_chip *gc;
  4284		int count, bitmap_size;
  4285		size_t descs_size;
  4286	
  4287		count = gpiod_count(dev, con_id);
  4288		if (count < 0)
  4289			return ERR_PTR(count);
  4290	
  4291		descs_size = struct_size(descs, desc, count);
  4292		descs = kzalloc(descs_size, GFP_KERNEL);
  4293		if (!descs)
  4294			return ERR_PTR(-ENOMEM);
  4295	
  4296		for (descs->ndescs = 0; descs->ndescs < count; descs->ndescs++) {
  4297			desc = gpiod_get_index(dev, con_id, descs->ndescs, flags);
  4298			if (IS_ERR(desc)) {
  4299				gpiod_put_array(descs);
  4300				return ERR_CAST(desc);
  4301			}
  4302	
  4303			descs->desc[descs->ndescs] = desc;
  4304	
  4305			gc = gpiod_to_chip(desc);
  4306			/*
  4307			 * If pin hardware number of array member 0 is also 0, select
  4308			 * its chip as a candidate for fast bitmap processing path.
  4309			 */
  4310			if (descs->ndescs == 0 && gpio_chip_hwgpio(desc) == 0) {
  4311				struct gpio_descs *array;
  4312	
  4313				bitmap_size = BITS_TO_LONGS(gc->ngpio > count ?
  4314							    gc->ngpio : count);
  4315	
  4316				array = krealloc(descs, descs_size +
  4317						 struct_size(array_info, invert_mask, 3 * bitmap_size),
  4318						 GFP_KERNEL | __GFP_ZERO);
  4319				if (!array) {
  4320					gpiod_put_array(descs);
  4321					return ERR_PTR(-ENOMEM);
  4322				}
  4323	
  4324				descs = array;
  4325	
  4326				array_info = (void *)descs + descs_size;
  4327				array_info->get_mask = array_info->invert_mask +
  4328							  bitmap_size;
  4329				array_info->set_mask = array_info->get_mask +
  4330							  bitmap_size;
  4331	
  4332				array_info->desc = descs->desc;
  4333				array_info->size = count;
  4334				array_info->chip = gc;
  4335				bitmap_set(array_info->get_mask, descs->ndescs,
  4336					   count - descs->ndescs);
  4337				bitmap_set(array_info->set_mask, descs->ndescs,
  4338					   count - descs->ndescs);
  4339				descs->info = array_info;
  4340			}
  4341	
  4342			/* If there is no cache for fast bitmap processing path, continue */
  4343			if (!array_info) {
  4344				/*clear descs->info*/
> 4345				memset(array_info, 0, sizeof(struct gpio_array));
  4346				continue;
  4347			}
  4348	
  4349			/* Unmark array members which don't belong to the 'fast' chip */
  4350			if (array_info->chip != gc) {
  4351				__clear_bit(descs->ndescs, array_info->get_mask);
  4352				__clear_bit(descs->ndescs, array_info->set_mask);
  4353			}
  4354			/*
  4355			 * Detect array members which belong to the 'fast' chip
  4356			 * but their pins are not in hardware order.
  4357			 */
  4358			else if (gpio_chip_hwgpio(desc) != descs->ndescs) {
  4359				/*
  4360				 * Don't use fast path if all array members processed so
  4361				 * far belong to the same chip as this one but its pin
  4362				 * hardware number is different from its array index.
  4363				 */
  4364				if (bitmap_full(array_info->get_mask, descs->ndescs)) {
  4365					array_info = NULL;
  4366				} else {
  4367					__clear_bit(descs->ndescs,
  4368						    array_info->get_mask);
  4369					__clear_bit(descs->ndescs,
  4370						    array_info->set_mask);
  4371				}
  4372			} else {
  4373				/* Exclude open drain or open source from fast output */
  4374				if (gpiochip_line_is_open_drain(gc, descs->ndescs) ||
  4375				    gpiochip_line_is_open_source(gc, descs->ndescs))
  4376					__clear_bit(descs->ndescs,
  4377						    array_info->set_mask);
  4378				/* Identify 'fast' pins which require invertion */
  4379				if (gpiod_is_active_low(desc))
  4380					__set_bit(descs->ndescs,
  4381						  array_info->invert_mask);
  4382			}
  4383		}
  4384		if (array_info)
  4385			dev_dbg(dev,
  4386				"GPIO array info: chip=%s, size=%d, get_mask=%lx, set_mask=%lx, invert_mask=%lx\n",
  4387				array_info->chip->label, array_info->size,
  4388				*array_info->get_mask, *array_info->set_mask,
  4389				*array_info->invert_mask);
  4390		return descs;
  4391	}
  4392	EXPORT_SYMBOL_GPL(gpiod_get_array);
  4393	

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

[PATCH v2] gpio: gpiolib: clear the array_info's memory space

[Yan Wang]

if hardware number different to array index,it needs to clear to points
memory space if the array_info have been assigned a value.

Reported-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/oe-kbuild-all/202304232146.7M89pwCz-lkp@intel.com/
Signed-off-by: Yan Wang <rk.code@outlook.com>
---
v1->v2: fixed building warning
---
 drivers/gpio/gpiolib.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 04fb05df805b..8b2a8db44b54 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4359,6 +4359,8 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 			 * hardware number is different from its array index.
 			 */
 			if (bitmap_full(array_info->get_mask, descs->ndescs)) {
+				/*clear descs->info*/
+				memset(array_info, 0, sizeof(struct gpio_array));
 				array_info = NULL;
 			} else {
 				__clear_bit(descs->ndescs,
-- 
2.17.1

Re: [PATCH v2] gpio: gpiolib: clear the array_info's memory space

[andy.shevchenko]

Sun, Apr 23, 2023 at 09:59:43PM +0800, Yan Wang kirjoitti:
> if hardware number different to array index,it needs to clear to points
> memory space if the array_info have been assigned a value.

Can you explain a bit more what's going on there?

...

>  			if (bitmap_full(array_info->get_mask, descs->ndescs)) {
> +				/*clear descs->info*/
> +				memset(array_info, 0, sizeof(struct gpio_array));
>  				array_info = NULL;

...

>  			}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] gpio: gpiolib: clear the array_info's memory space

[Yan Wang]

On 4/26/2023 10:42 PM, andy.shevchenko@gmail.com wrote:
> Sun, Apr 23, 2023 at 09:59:43PM +0800, Yan Wang kirjoitti:
>> if hardware number different to array index,it needs to clear to points
>> memory space if the array_info have been assigned a value.
> Can you explain a bit more what's going on there?
>
> ...
Hi Andy,

I use gpiod_get_array() to get a gpio array form the node of DTS.

the node is as follows:
...
gpios = <&gpio1 0 0>, <&gpio1 10 0>;
...
first scan 0 pin of gpio1, its index and hardware number are 0,
the (descs->ndescs == 0 && gpio_chip_hwgpio(desc) == 0) is true and 
descs->info = array_info. then scan 10 pin , its index is 1 ,but 
hardware number is 10 , the  (gpio_chip_hwgpio(desc) != descs->ndescs) 
is true. array_info = NULL, Just make array_info point to NULL, Did't 
clean descs->info memory or point it to NULL. Should the array_info 
point to memory be cleared ? if not cleared ,I use 
gpiod_set_array_value_cansleep() setting pin 10 of gpio1 is invalid. I 
found that the set_mask and get_mask vlaues of descs->info are seted 
0x03 in gpiod_get_array(), I think 0x401 is their correct value. Thank 
you review
>>   			if (bitmap_full(array_info->get_mask, descs->ndescs)) {
>> +				/*clear descs->info*/
>> +				memset(array_info, 0, sizeof(struct gpio_array));
>>   				array_info = NULL;
> ...
>
>>   			}

Re: [PATCH v2] gpio: gpiolib: clear the array_info's memory space

[Yan Wang]

On 4/26/2023 10:42 PM, andy.shevchenko@gmail.com wrote:
> Sun, Apr 23, 2023 at 09:59:43PM +0800, Yan Wang kirjoitti:
>> if hardware number different to array index,it needs to clear to points
>> memory space if the array_info have been assigned a value.
> Can you explain a bit more what's going on there?
>
> ...
Hi Andy,

I use gpiod_get_array() to get a gpio array form the node of DTS.

the node is as follows:
...
gpios = <&gpio1 0 0>, <&gpio1 10 0>;
...

First scan pin-0 of gpio1,its index and hardware number are 0,

if (descs->ndescs == 0 && gpio_chip_hwgpio(desc) == 0) {
     ...
     descs->info = array_info.
}

Then scan pin-10 , its index is 1 ,but hardware number is 10 .

if (gpio_chip_hwgpio(desc) != descs->ndescs) {
     array_info = NULL;
}
just set array_info = NULL, Should the array_info point to memory be 
cleared ?

if not cleared ,I use the gpiod_set_array_value_cansleep() to pull up or 
down pin-10 is invalid.

I found that the set_mask and get_mask vlaues of descs->info are seted 
0x03 in gpiod_get_array(),
I think 0x401 is their correct value.

Thank you for review.
>> if (bitmap_full(array_info->get_mask, descs->ndescs)) {
>> + /*clear descs->info*/
>> + memset(array_info, 0, sizeof(struct gpio_array));
>> array_info = NULL;
> ...
>
>> }

Re: [PATCH v2] gpio: gpiolib: clear the array_info's memory space

[Andy Shevchenko]

On Thu, May 4, 2023 at 12:38 PM Yan Wang <rk.code@outlook.com> wrote:
> On 4/26/2023 10:42 PM, andy.shevchenko@gmail.com wrote:
> > Sun, Apr 23, 2023 at 09:59:43PM +0800, Yan Wang kirjoitti:
> >> if hardware number different to array index,it needs to clear to points
> >> memory space if the array_info have been assigned a value.
> > Can you explain a bit more what's going on there?

...

> I use gpiod_get_array() to get a gpio array form the node of DTS.
>
> the node is as follows:
> ...
> gpios = <&gpio1 0 0>, <&gpio1 10 0>;
> ...
>
> First scan pin-0 of gpio1,its index and hardware number are 0,
>
> if (descs->ndescs == 0 && gpio_chip_hwgpio(desc) == 0) {
>      ...
>      descs->info = array_info.
> }
>
> Then scan pin-10 , its index is 1 ,but hardware number is 10 .
>
> if (gpio_chip_hwgpio(desc) != descs->ndescs) {
>      array_info = NULL;
> }
> just set array_info = NULL, Should the array_info point to memory be
> cleared ?

This is a good question. The entire algorithm is a bit difficult to
understand from the first glance. I need some time to check it myself.

> if not cleared ,I use the gpiod_set_array_value_cansleep() to pull up or
> down pin-10 is invalid.

I'm not sure I follow. The array operations are against the given
array of the descriptors. If you ask to have that operation done, the
all descriptors in the array should be considered.

> I found that the set_mask and get_mask vlaues of descs->info are seted
> 0x03 in gpiod_get_array(),

Yes, this mask is for the argument. The 0x03 is the correct one.

> I think 0x401 is their correct value.

No. You have an array of two elements, and not 11.

> >> if (bitmap_full(array_info->get_mask, descs->ndescs)) {
> >> + /*clear descs->info*/
> >> + memset(array_info, 0, sizeof(struct gpio_array));
> >> array_info = NULL;
> >> }


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] gpio: gpiolib: clear the array_info's memory space

[Yan Wang]

> On May 4, 2023, at 19:36, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> 
> On Thu, May 4, 2023 at 12:38 PM Yan Wang <rk.code@outlook.com> wrote:
>> On 4/26/2023 10:42 PM, andy.shevchenko@gmail.com wrote:
>>> Sun, Apr 23, 2023 at 09:59:43PM +0800, Yan Wang kirjoitti:
>>>> if hardware number different to array index,it needs to clear to points
>>>> memory space if the array_info have been assigned a value.
>>> Can you explain a bit more what's going on there?
> 
> ...
> 
>> I use gpiod_get_array() to get a gpio array form the node of DTS.
>> 
>> the node is as follows:
>> ...
>> gpios = <&gpio1 0 0>, <&gpio1 10 0>;
>> ...
>> 
>> First scan pin-0 of gpio1,its index and hardware number are 0,
>> 
>> if (descs->ndescs == 0 && gpio_chip_hwgpio(desc) == 0) {
>> ...
>> descs->info = array_info.
>> }
>> 
>> Then scan pin-10 , its index is 1 ,but hardware number is 10 .
>> 
>> if (gpio_chip_hwgpio(desc) != descs->ndescs) {
>> array_info = NULL;
>> }
>> just set array_info = NULL, Should the array_info point to memory be
>> cleared ?
> 
> This is a good question. The entire algorithm is a bit difficult to
> understand from the first glance. I need some time to check it myself.
Looking forward to your test results.
> 
>> if not cleared ,I use the gpiod_set_array_value_cansleep() to pull up or
>> down pin-10 is invalid.
> 
> I'm not sure I follow. The array operations are against the given
> array of the descriptors. If you ask to have that operation done, the
> all descriptors in the array should be considered.
> 
>> I found that the set_mask and get_mask vlaues of descs->info are seted
>> 0x03 in gpiod_get_array(),
> 
> Yes, this mask is for the argument. The 0x03 is the correct one.
> 
>> I think 0x401 is their correct value.
> 
> No. You have an array of two elements, and not 11.

Due to hardware number are 10 and 0 , so this mask is 0x401（bit10 and bit0 are 1）.

> 
>>>> if (bitmap_full(array_info->get_mask, descs->ndescs)) {
>>>> + /*clear descs->info*/
>>>> + memset(array_info, 0, sizeof(struct gpio_array));
>>>> array_info = NULL;
>>>> }
> 
> 
> -- 
> With Best Regards,
> Andy Shevchenko

Re: [PATCH v2] gpio: gpiolib: clear the array_info's memory space

[Yan Wang]

Polite ping

On 5/4/2023 10:15 PM, Yan Wang wrote:
>
>> On May 4, 2023, at 19:36, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>>
>> On Thu, May 4, 2023 at 12:38 PM Yan Wang <rk.code@outlook.com> wrote:
>>> On 4/26/2023 10:42 PM, andy.shevchenko@gmail.com wrote:
>>>> Sun, Apr 23, 2023 at 09:59:43PM +0800, Yan Wang kirjoitti:
>>>>> if hardware number different to array index,it needs to clear to points
>>>>> memory space if the array_info have been assigned a value.
>>>> Can you explain a bit more what's going on there?
>> ...
>>
>>> I use gpiod_get_array() to get a gpio array form the node of DTS.
>>>
>>> the node is as follows:
>>> ...
>>> gpios = <&gpio1 0 0>, <&gpio1 10 0>;
>>> ...
>>>
>>> First scan pin-0 of gpio1,its index and hardware number are 0,
>>>
>>> if (descs->ndescs == 0 && gpio_chip_hwgpio(desc) == 0) {
>>> ...
>>> descs->info = array_info.
>>> }
>>>
>>> Then scan pin-10 , its index is 1 ,but hardware number is 10 .
>>>
>>> if (gpio_chip_hwgpio(desc) != descs->ndescs) {
>>> array_info = NULL;
>>> }
>>> just set array_info = NULL, Should the array_info point to memory be
>>> cleared ?
>> This is a good question. The entire algorithm is a bit difficult to
>> understand from the first glance. I need some time to check it myself.
> Looking forward to your test results.
>>> if not cleared ,I use the gpiod_set_array_value_cansleep() to pull up or
>>> down pin-10 is invalid.
>> I'm not sure I follow. The array operations are against the given
>> array of the descriptors. If you ask to have that operation done, the
>> all descriptors in the array should be considered.
>>
>>> I found that the set_mask and get_mask vlaues of descs->info are seted
>>> 0x03 in gpiod_get_array(),
>> Yes, this mask is for the argument. The 0x03 is the correct one.
>>
>>> I think 0x401 is their correct value.
>> No. You have an array of two elements, and not 11.
> Due to hardware number are 10 and 0 , so this mask is 0x401（bit10 and bit0 are 1）.
>
>>>>> if (bitmap_full(array_info->get_mask, descs->ndescs)) {
>>>>> + /*clear descs->info*/
>>>>> + memset(array_info, 0, sizeof(struct gpio_array));
>>>>> array_info = NULL;
>>>>> }
>>
>> -- 
>> With Best Regards,
>> Andy Shevchenko

Re: [PATCH v2] gpio: gpiolib: clear the array_info's memory space

[Linus Walleij]

> @@ -4359,6 +4359,8 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,

>                          * hardware number is different from its array index.
>                          */
>                         if (bitmap_full(array_info->get_mask, descs->ndescs)) {
> +                               /*clear descs->info*/
> +                               memset(array_info, 0, sizeof(struct gpio_array));
>                                 array_info = NULL;

This is not the right solution.

The array_info points beyond descs and descs have be krealloc:ed
to fit the array info.

The right solution is not to fill that memory with zeroes, but to krealloc
back to the size that descs had before we did this resizing to begin
with.

Possibly the condition should be detected *before* we start to krealloc()
so we can avoid all the krealloc():ing.

If the actual issue cannot be fixed I think it is no better or worse to just
leave the code as it is, we are just zeroing some unused memory.

Yours,
Linus Walleij