Re: [PATCH 02/13] si2157: Check error status bit on cmd execute

[Antti Palosaari <crope@iki.fi>]

On 12/29/18 7:51 PM, Brad Love wrote:
> Check error status bit on command execute, if error bit is
> set return -EAGAIN. Ignore -EAGAIN in probe during device check.
> 
> Signed-off-by: Brad Love <brad@nextdimension.cc>
> ---
>   drivers/media/tuners/si2157.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index 4855448..3924c42 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -56,14 +56,20 @@ static int si2157_cmd_execute(struct i2c_client *client, struct si2157_cmd *cmd)
>   				break;
>   		}
>   
> -		dev_dbg(&client->dev, "cmd execution took %d ms\n",
> +		dev_dbg(&client->dev, "cmd execution took %d ms, status=%x\n",
>   				jiffies_to_msecs(jiffies) -
> -				(jiffies_to_msecs(timeout) - TIMEOUT));
> +				(jiffies_to_msecs(timeout) - TIMEOUT),
> +				cmd->args[0]);
>   
>   		if (!((cmd->args[0] >> 7) & 0x01)) {
>   			ret = -ETIMEDOUT;
>   			goto err_mutex_unlock;
>   		}
> +		/* check error status bit */
> +		if (cmd->args[0] & 0x40) {
> +			ret = -EAGAIN;
> +			goto err_mutex_unlock;
> +		}
>   	}
>   
>   	mutex_unlock(&dev->i2c_mutex);
> @@ -477,7 +483,7 @@ static int si2157_probe(struct i2c_client *client,
>   	cmd.wlen = 0;
>   	cmd.rlen = 1;
>   	ret = si2157_cmd_execute(client, &cmd);
> -	if (ret)
> +	if (ret && (ret != -EAGAIN))
>   		goto err_kfree;
>   
>   	memcpy(&fe->ops.tuner_ops, &si2157_ops, sizeof(struct dvb_tuner_ops));
> 

So you added check if firmware returns error during command execution, 
but that error is still skipped during probe, which does not feel 
correct. Chip should work during probe and ideally driver should ensure 
it is correct chip. At least you should read some property value or 
execute some other command without failure.

regards
Antti

-- 
http://palosaari.fi/