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

[Hui Wang <hui.wang@xxxxxxxxxxxxx>]

On 6/4/24 21:42, Andy Shevchenko wrote:
> 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
Got it.
> > 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
Got it.
> > 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);
> 	}

OK, got it, will fix all comment in the v3.

Thanks.

> > +	return 0;
> > +}