On 01.11.2017 22:05, Mauro Carvalho Chehab wrote:
> As warned by smatch:
> drivers/media/i2c/s5c73m3/s5c73m3-core.c:268 s5c73m3_check_status()
> error: uninitialized symbol 'status'.
>
> if s5c73m3_check_status() is called too late, time_is_after_jiffies(end)
> will return 0, causing the while to abort before reading status.
>
> The current code will do the wrong thing here, as it will still
> check if status != value. The right fix here is to just change
> the initial state of ret to -ETIMEDOUT. This way, if the
> routine is called too late, it will skip the flawed check
> and return that a timeout happened.
First of all it is rather uncommon situation, scenario in which two
consecutive lines of simple code takes more than 2 seconds is rather rare.
Of course it is possible but in such case proposed solution does not
look like as a proper fix, it reports timeout on the sensor without even
touching it.
I think the right fix would be to re-factor the loop to read the status
first and check timeout later.
Regards
Andrzej
>
> Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
> ---
> drivers/media/i2c/s5c73m3/s5c73m3-core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> index cdc4f2392ef9..45345f8b27a5 100644
> --- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> +++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> @@ -248,7 +248,7 @@ static int s5c73m3_check_status(struct s5c73m3 *state,
> unsigned int value)
> {
> unsigned long start = jiffies;
> unsigned long end = start + msecs_to_jiffies(2000);
> - int ret = 0;
> + int ret = -ETIMEDOUT;
> u16 status;
> int count = 0;
>
