| Message ID | KL1PR01MB5448327326B6EDA8001AF714E6669@KL1PR01MB5448.apcprd01.prod.exchangelabs.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] gpio: gpiolib: clear the array_info's memory space | expand |
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; ... > }
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; > ... > >> }
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; > ... > >> }
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; > >> }
> 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
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
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,
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(+)