On 6/12/24 21:50, Hugo Villeneuve wrote:
On Wed, 12 Jun 2024 21:14:54 +0800
Hui Wang <hui.wang@xxxxxxxxxxxxx> wrote:
Hi Hui,
Some boards connect a GPIO to the reset pin, and the reset pin needs
to be setup correctly before accessing the chip.
Add a function to handle the chip reset. If the reset-gpios is defined
in the DT, do hardware reset through this GPIO, othwerwise do software
reset as before.
...
+ /* The minimum reset pulse width is 3 us. */
+ udelay(5);
+ gpiod_set_value_cansleep(reset_gpio, 0);
+ /* Deassert GPIO */
Move this comment after its corresponding statement:
gpiod_set_value_cansleep(reset_gpio, 0); /* Deassert GPIO */
Got it.
+ } else {
+ /* Software reset */
+ regmap_write(regmap, SC16IS7XX_IOCONTROL_REG,
+ SC16IS7XX_IOCONTROL_SRESET_BIT);
+ }
+
+ return 0;
+}
+
int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
struct regmap *regmaps[], int irq)
{
@@ -1527,6 +1552,10 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
s->devtype = devtype;
dev_set_drvdata(dev, s);
+ ret = sc16is7xx_reset(dev, regmaps[0]);
+ if (ret)
+ goto out_clk;
+
You could move this block where the original software reset was
located, unless you have a good reason to move it here? This will make
the review (diff) easier...
If I move it to the place where the original software reset was located,
I can't use "goto out_clk" anymore, it not only needs to release clk but
also needs to stop the kthread, so I have to add a new tag before the
kthread_stop(s->kworker_task) and goto that tag.
Thanks.
kthread_init_worker(&s->kworker);
s->kworker_task = kthread_run(kthread_worker_fn, &s->kworker,
"sc16is7xx");
@@ -1536,10 +1565,6 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
}
sched_set_fifo(s->kworker_task);
- /* reset device, purging any pending irq / data */
- regmap_write(regmaps[0], SC16IS7XX_IOCONTROL_REG,
- SC16IS7XX_IOCONTROL_SRESET_BIT);
-
/* Mark each port line and status as uninitialised. */
for (i = 0; i < devtype->nr_uart; ++i) {
s->p[i].port.line = SC16IS7XX_MAX_DEVS;
--
2.34.1
