From: Mauro Carvalho Chehab <mchehab@kernel.org>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: linux-media@vger.kernel.org
Subject: Re: [PATCH 17/24] ccs: Wait until software reset is done
Date: Tue, 12 Jan 2021 17:49:48 +0100 [thread overview]
Message-ID: <20210112174948.1434286d@coco.lan> (raw)
In-Reply-To: <20201207211530.21180-18-sakari.ailus@linux.intel.com>
Em Mon, 7 Dec 2020 23:15:23 +0200
Sakari Ailus <sakari.ailus@linux.intel.com> 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@linux.intel.com>
> ---
> 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
next prev parent reply other threads:[~2021-01-12 16:50 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-07 21:15 [PATCH 00/24] Additional CCS feature support Sakari Ailus
2020-12-07 21:15 ` [PATCH 01/24] ccs: Add digital gain support Sakari Ailus
2020-12-07 21:15 ` [PATCH 02/24] ccs: Add support for old-style SMIA digital gain Sakari Ailus
2020-12-07 21:15 ` [PATCH 03/24] ccs: Remove analogue gain field Sakari Ailus
2020-12-07 21:15 ` [PATCH 04/24] ccs: Only add analogue gain control if the device supports it Sakari Ailus
2020-12-07 21:15 ` [PATCH 05/24] v4l: uapi: Add user control base for CCS controls Sakari Ailus
2020-12-07 21:15 ` [PATCH 06/24] Documentation: ccs: Add user documentation for the CCS driver Sakari Ailus
2020-12-07 21:15 ` [PATCH 07/24] v4l: uapi: ccs: Add controls for analogue gain constants Sakari Ailus
2020-12-11 11:39 ` [PATCH v1.1 " Sakari Ailus
2020-12-07 21:15 ` [PATCH 08/24] ccs: Add support for analogue gain coefficient controls Sakari Ailus
2020-12-07 21:15 ` [PATCH 09/24] v4l: uapi: ccs: Add controls for CCS alternative analogue gain Sakari Ailus
2020-12-07 21:15 ` [PATCH 10/24] ccs: Add support for alternate analogue global gain Sakari Ailus
2020-12-07 21:15 ` [PATCH 11/24] ccs: Add debug prints for MSR registers Sakari Ailus
2020-12-07 21:15 ` [PATCH 12/24] v4l: uapi: ccs: Add CCS controls for shading correction Sakari Ailus
2020-12-07 21:15 ` [PATCH 13/24] ccs: Add shading correction and luminance correction level controls Sakari Ailus
2020-12-07 21:15 ` [PATCH 14/24] ccs: Get the endpoint by port rather than any next endpoint Sakari Ailus
2020-12-07 21:15 ` [PATCH 15/24] ccs: Don't change the I²C address just for software reset Sakari Ailus
2020-12-07 21:15 ` [PATCH 16/24] ccs: Only do software reset if we have no hardware reset Sakari Ailus
2020-12-07 21:15 ` [PATCH 17/24] ccs: Wait until software reset is done Sakari Ailus
2021-01-12 16:49 ` Mauro Carvalho Chehab [this message]
2021-01-12 17:12 ` Sakari Ailus
2020-12-07 21:15 ` [PATCH 18/24] ccs: Hardware requires a delay after starting the clock of lifting reset Sakari Ailus
2020-12-07 21:15 ` [PATCH 19/24] ccs: Add a sanity check for external clock frequency Sakari Ailus
2020-12-07 21:15 ` [PATCH 20/24] ccs: Support and default to auto PHY control Sakari Ailus
2020-12-07 21:15 ` [PATCH 21/24] Documentation: Include CCS PLL calculator to CCS driver documentation Sakari Ailus
2020-12-07 21:15 ` [PATCH 22/24] ccs-pll: Switch from standard integer types to kernel ones Sakari Ailus
2020-12-07 21:15 ` [PATCH 23/24] ccs: " Sakari Ailus
2020-12-07 21:15 ` [PATCH 24/24] Revert "media: ccs-pll: Fix MODULE_LICENSE" Sakari Ailus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210112174948.1434286d@coco.lan \
--to=mchehab@kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=sakari.ailus@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox