Re: [PATCH] reset: gpio: Add self-deasserting reset callback

[Krzysztof Kozlowski <krzk@kernel.org>]

On 30/01/2025 22:53, Shenwei Wang wrote:
> During the driver probe phase, many drivers leverage convenience
> wrapper APIs such as device_reset and device_reset_optional provided
> by the reset core driver. However, both of these APIs depend on the
> presence of a .reset callback within the reset controller's operations
> structure.
> 
> Introducing the self-deasserting reset callback enhances flexibility for
> users and enables a more simple reset process during device initialization.


The reset callback was not added on purpose and you totally ignored the
reasons here. See below.

> 
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
>  drivers/reset/reset-gpio.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/reset/reset-gpio.c b/drivers/reset/reset-gpio.c
> index 2290b25b6703..614f9e261a13 100644
> --- a/drivers/reset/reset-gpio.c
> +++ b/drivers/reset/reset-gpio.c
> @@ -1,5 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  
> +#include <linux/delay.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
> @@ -37,6 +38,17 @@ static int reset_gpio_deassert(struct reset_controller_dev *rc,
>  	return 0;
>  }
>  
> +static int reset_gpio_reset(struct reset_controller_dev *rc, unsigned long id)
> +{
> +	struct reset_gpio_priv *priv = rc_to_reset_gpio(rc);
> +
> +	gpiod_set_value_cansleep(priv->reset, 1);
> +	usleep_range(10, 20);


No, because this is some arbitrary value which might or might not work.
If this gets accepted, next person will change it to their own need.
Then next person will revert previous change... and so on.


Best regards,
Krzysztof