Re: [PATCH v2 2/2] serial: sc16is7xx: hard reset the chip if reset-gpios is defined in dt

[Andy Shevchenko <andy@kernel.org>]

On Tue, Jun 04, 2024 at 09:27:26PM +0800, Hui Wang wrote:
> Certain designs connect a gpio to the reset pin, and the reset pin

GPIO

> needs to be setup correctly before accessing the chip.
> 
> Here adding a function to handle the chip reset. If the reset-gpios is
> defined in the dt, do the hard reset through this gpio, othwerwise do

DT

> the soft reset as before.

...

> +static int sc16is7xx_reset(struct device *dev, struct regmap *regmaps[])
> +{
> +	struct gpio_desc *reset_gpiod;

> +	reset_gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> +	if (!reset_gpiod)
> +		/* soft reset device, purging any pending irq / data */
> +		regmap_write(regmaps[0], SC16IS7XX_IOCONTROL_REG,
> +			     SC16IS7XX_IOCONTROL_SRESET_BIT);
> +	else if (!IS_ERR(reset_gpiod)) {
> +		/* delay 5 us (at least 3 us) and deassert the gpio to exit the hard reset */
> +		udelay(5);
> +		gpiod_set_value_cansleep(reset_gpiod, 0);
> +	} else
> +		return PTR_ERR(reset_gpiod);

You can do better here.

	reset_gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
	if (IS_ERR(reset_gpiod))
		return PTR_ERR(reset_gpiod);

	if (reset_gpiod) {
		/* delay 5 us (at least 3 us) and deassert the GPIO to exit the hard reset */
		fsleep(5);
		gpiod_set_value_cansleep(reset_gpiod, 0);
	} else {
		/* soft reset device, purging any pending IRQ / data */
		regmap_write(regmaps[0], SC16IS7XX_IOCONTROL_REG,
			     SC16IS7XX_IOCONTROL_SRESET_BIT);
	}

> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko