Re: [PATCH v5 2/2] iio: adc: ti-ads1298: Add driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 16-02-2024 18:19, Jonathan Cameron wrote:
On Fri, 16 Feb 2024 17:07:49 +0100
Mike Looijmans <mike.looijmans@xxxxxxxx> wrote:

On 16-02-2024 16:53, Andy Shevchenko wrote:

...

+       if (reset_gpio) {
+               /*
+                * Deassert reset now that clock and power are active.
+                * Minimum reset pulsewidth is 2 clock cycles.
+                */
+               udelay(ADS1298_CLOCKS_TO_USECS(2));

This is sleeping context and you are calling unsleeping function. I haven't
checked the macro implementation and I have no idea what is the maximum it may
give, but making code robust just use fsleep() call.

It'll actually delay for 1 us (the "clock" is ~2MHz). So fsleep will compile to udelay anyway, which is fine, fsleep might get smarter in future and this would then profit.



+               gpiod_set_value_cansleep(reset_gpio, 0);
+       } else {
+               ret = ads1298_write_cmd(priv, ADS1298_CMD_RESET);
+               if (ret)
+                       return dev_err_probe(dev, ret, "RESET failed\n");
+       }
+       /* Wait 18 clock cycles for reset command to complete */
+       udelay(ADS1298_CLOCKS_TO_USECS(18));

Ditto.

...


If it's the only issue I think Jonathan can modify when applying
(no new patch version would be needed).

That'd be nice.
ok.  As this is still the top of my tree I'll just tweak it.

Does anyone else read fsleep as femtosecond sleep every time? :)
Maybe computers will go that fast one day.

Jonathan

Noticed this triggered a kernel test robot report. Sent a v6 that fixes the bug.

Can also make it a separate patch if you prefer, in that case ignore v6.


--
Mike Looijmans
System Expert

TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@xxxxxxxx
W: www.topic.nl








[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux