Em Mon, 7 Dec 2020 23:15:23 +0200
Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> escreveu:
> Verify the software reset has been completed until proceeding.
>
> The spec does not guarantee a delay but presumably 100 ms should be
> enough.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> ---
> drivers/media/i2c/ccs/ccs-core.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
> index fdf2e83eeac3..e1b3c5693e01 100644
> --- a/drivers/media/i2c/ccs/ccs-core.c
> +++ b/drivers/media/i2c/ccs/ccs-core.c
> @@ -1553,11 +1553,26 @@ static int ccs_power_on(struct device *dev)
> */
>
> if (!sensor->reset && !sensor->xshutdown) {
> + u8 retry = 100;
> + u32 reset;
> +
> rval = ccs_write(sensor, SOFTWARE_RESET, CCS_SOFTWARE_RESET_ON);
> if (rval < 0) {
> dev_err(dev, "software reset failed\n");
> goto out_cci_addr_fail;
> }
> +
> + do {
> + rval = ccs_read(sensor, SOFTWARE_RESET, &reset);
> + reset = !rval && reset == CCS_SOFTWARE_RESET_OFF;
> + if (reset)
> + break;
> +
> + usleep_range(1000, 2000);
> + } while (--retry);
Hmm... the way it is, the loop will wait for some time between
100ms and 200ms. Based on past experiences with non-deterministic
sleep times, this can be hard to debug if some device would require
to wait for a value between 100ms and 200ms.
So, I would, instead, make the retry time more deterministic, by
using time_is_after_jiffies(), e. g.:
end = jiffies + msecs_to_jiffies(retry);
do {
rval = ccs_read(sensor, SOFTWARE_RESET, &reset);
reset = !rval && reset == CCS_SOFTWARE_RESET_OFF;
if (reset)
break;
usleep_range(1000, 2000);
} while (time_is_after_jiffies(end));
In any case, I'm taking this patch, but it seems worth to change
to something like the above on a followup patch.
> +
> + if (!reset)
> + return -EIO;
> }
>
> if (sensor->hwcfg.i2c_addr_alt) {
Thanks,
Mauro
