On 31.7.2022 19.00, Jonathan Cameron wrote: > On Sun, 24 Jul 2022 19:43:15 +0300 > Matti Lehtimäki <matti.lehtimaki@xxxxxxxxx> wrote: > >> Some sensors do not always start fast enough to read a valid ID from >> registers at first attempt. Let's retry at most 3 times with short sleep >> in between to fix random timing issues. >> >> Signed-off-by: Matti Lehtimäki <matti.lehtimaki@xxxxxxxxx> > Hi Matti, > > My gut feeling is this isn't in a fast path, so why not just wait > for whatever the documented power up time of the sensor is? > > I'd expect to see a sleep in st_sensors_power_enable() if one is > required. In the specification for the sensor (lis2hh12) I have on my device I found that the maximum boot time of the sensor (starting from Vdd power on) is defined as 20 ms. Not sure if the other sensors supported by the driver have different values but based on checking a couple of specifications I didn't find any bigger values so far. >> + msleep(20); > How do we know 60msecs is long enough for all sensors? Based on the specification for the sensor I have and also driver used in Android kernel for my device (it uses a 3 x 20 ms loop) I think 20 ms is a good value but to be sure a slightly longer might make sense. As suggested in the other review comment by changing the regmap_read to regmap_read_poll_timeout the function doesn't always need to wait at least 20 ms in case first read doesn't provide the correct value, if a suitable shorter poll interval is used (something like 1-10 ms). However testing on my device has shown that I still need to have a loop or at least a retry possibility because I have noticed a rare random read error (-6, happens after some time not at first read) when reading the id from the hardware. This could be due to for example internal init failure of the sensor chip causing an internal reset. Because of this read error regmap_read_poll_timeout returns with an error and without retrying to read the id the sensor probe fails. -Matti
