Re: [PATCH v2 4/5] gpiolib: don't overwrite the entire flags field in gpiod_free_commit()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] gpiolib: commit message and indentation reference
  gpiod_update_flags()
--

commit 8c916355e1aeab892f0c8dc4d52c44c7d5dcf4e9
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

gpiolib: don't overwrite the entire flags field in gpiod_free_commit()

This commit replaces the non-atomic clear_bit() sequence and WRITE_ONCE()
on desc->flags in gpiod_free_commit() with an atomic set_mask_bits() call.

> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index af2d67d977f49..4c9c524389ca1 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c

[ ... ]

> @@ -2696,24 +2697,23 @@ void gpiod_free_commit(struct gpio_desc *desc)
>  #ifdef CONFIG_GPIO_CDEV
>  		WRITE_ONCE(desc->debounce_period_us, 0);
>  #endif
> -
> -		clear_bit(GPIOD_FLAG_ACTIVE_LOW, &flags);
> -		clear_bit(GPIOD_FLAG_REQUESTED, &flags);
> -		clear_bit(GPIOD_FLAG_OPEN_DRAIN, &flags);
> -		clear_bit(GPIOD_FLAG_OPEN_SOURCE, &flags);
> -		clear_bit(GPIOD_FLAG_PULL_UP, &flags);
> -		clear_bit(GPIOD_FLAG_PULL_DOWN, &flags);
> -		clear_bit(GPIOD_FLAG_BIAS_DISABLE, &flags);
> -		clear_bit(GPIOD_FLAG_EDGE_RISING, &flags);
> -		clear_bit(GPIOD_FLAG_EDGE_FALLING, &flags);
> -		clear_bit(GPIOD_FLAG_EVENT_CLOCK_REALTIME, &flags);
> -		clear_bit(GPIOD_FLAG_EVENT_CLOCK_HTE, &flags);
> -		clear_bit(GPIOD_FLAG_IS_HOGGED, &flags);
>  #ifdef CONFIG_OF_DYNAMIC
>  		WRITE_ONCE(desc->hog, NULL);
>  #endif
>  		desc_set_label(desc, NULL);
> -		WRITE_ONCE(desc->flags, flags);
> +		set_mask_bits(&desc->flags,
> +				   BIT(GPIOD_FLAG_ACTIVE_LOW) |
> +				   BIT(GPIOD_FLAG_REQUESTED) |
> +				   BIT(GPIOD_FLAG_OPEN_DRAIN) |
> +				   BIT(GPIOD_FLAG_OPEN_SOURCE) |
> +				   BIT(GPIOD_FLAG_PULL_UP) |
> +				   BIT(GPIOD_FLAG_PULL_DOWN) |
> +				   BIT(GPIOD_FLAG_BIAS_DISABLE) |
> +				   BIT(GPIOD_FLAG_EDGE_RISING) |
> +				   BIT(GPIOD_FLAG_EDGE_FALLING) |
> +				   BIT(GPIOD_FLAG_EVENT_CLOCK_REALTIME) |
> +				   BIT(GPIOD_FLAG_EVENT_CLOCK_HTE) |
> +				   BIT(GPIOD_FLAG_IS_HOGGED), 0);
>  		gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_RELEASED);
>  	}
>  }

[Severity: Low]
This isn't a bug, but the commit message says gpiod_update_flags() is used
here, whereas the code calls set_mask_bits().  The BIT(GPIOD_FLAG_*)
continuation lines are also still indented to align with the opening
parenthesis of gpiod_update_flags() rather than set_mask_bits().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-gpio-desc-flags-atomic-v2-0-c4c54b476628@oss.qualcomm.com?part=4