| Message ID | 1430658023-17579-3-git-send-email-olli.salonen@iki.fi (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Moikka! I didn't make any tests, but I could guess that error flag is set by firmware when some unsupported / invalid parameter is given as a firmware command request. Anyhow, I am not sure which is proper error code to return. Could you please study, check from API docs and so, which are suitable error codes. EINVAL sounds proper code, but imho it is not good as generally it is returned by driver when invalid parameters are requested and I would like to see some other code to make difference (between driver and firmware error). regards Antti On 05/03/2015 04:00 PM, Olli Salonen wrote: > Return error from si2168_cmd_execute in case the demodulator returns an > error. > > Signed-off-by: Olli Salonen <olli.salonen@iki.fi> > --- > drivers/media/dvb-frontends/si2168.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c > index 29a5936..b68ab34 100644 > --- a/drivers/media/dvb-frontends/si2168.c > +++ b/drivers/media/dvb-frontends/si2168.c > @@ -60,6 +60,12 @@ static int si2168_cmd_execute(struct i2c_client *client, struct si2168_cmd *cmd) > jiffies_to_msecs(jiffies) - > (jiffies_to_msecs(timeout) - TIMEOUT)); > > + /* error bit set? */ > + if ((cmd->args[0] >> 6) & 0x01) { > + ret = -EREMOTEIO; > + goto err_mutex_unlock; > + } > + > if (!((cmd->args[0] >> 7) & 0x01)) { > ret = -ETIMEDOUT; > goto err_mutex_unlock; >
Hi Antti,
I basically used the same code that was used earlier in the same
function in case i2c_master_send/recv returns an unexpected result.
When working with the RTL2832P based Si2168 device I noticed that at
some point, often but not always during firmware loading, the Si2168
started to return error. When that happened any subsequent firmware
commands would fail as well. So it made sense to interrupt the loading
of firmware at the point when the first error is returned by the
demod. The reason for these I2C problems is not known - two hacks (rc
query interval increase and usage of the new I2C write method) were
used to mitigate the issue.
Based on the descriptions in
https://www.kernel.org/doc/Documentation/i2c/fault-codes EIO could be
applicable, though very loosely specified (which isn't necessary
wrong, as I don't have exact knowledge on which cases the error bit is
set):
EIO
This rather vague error means something went wrong when
performing an I/O operation. Use a more specific fault
code when you can.
The description for EINVAL doesn't match when it comes to the "before
any I/O operation was started", but other than that it might be
usable:
EINVAL
This rather vague error means an invalid parameter has been
detected before any I/O operation was started. Use a more
specific fault code when you can.
Looking at other DVB drivers, it seems it's mainly EREMOTEIO or EINVAL
that's used, depending on the case.
Cheers,
-olli
On 7 May 2015 at 22:49, Antti Palosaari <crope@iki.fi> wrote:
> Moikka!
> I didn't make any tests, but I could guess that error flag is set by
> firmware when some unsupported / invalid parameter is given as a firmware
> command request.
>
> Anyhow, I am not sure which is proper error code to return. Could you please
> study, check from API docs and so, which are suitable error codes. EINVAL
> sounds proper code, but imho it is not good as generally it is returned by
> driver when invalid parameters are requested and I would like to see some
> other code to make difference (between driver and firmware error).
>
> regards
> Antti
>
>
> On 05/03/2015 04:00 PM, Olli Salonen wrote:
>>
>> Return error from si2168_cmd_execute in case the demodulator returns an
>> error.
>>
>> Signed-off-by: Olli Salonen <olli.salonen@iki.fi>
>> ---
>> drivers/media/dvb-frontends/si2168.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/media/dvb-frontends/si2168.c
>> b/drivers/media/dvb-frontends/si2168.c
>> index 29a5936..b68ab34 100644
>> --- a/drivers/media/dvb-frontends/si2168.c
>> +++ b/drivers/media/dvb-frontends/si2168.c
>> @@ -60,6 +60,12 @@ static int si2168_cmd_execute(struct i2c_client
>> *client, struct si2168_cmd *cmd)
>> jiffies_to_msecs(jiffies) -
>> (jiffies_to_msecs(timeout) - TIMEOUT));
>>
>> + /* error bit set? */
>> + if ((cmd->args[0] >> 6) & 0x01) {
>> + ret = -EREMOTEIO;
>> + goto err_mutex_unlock;
>> + }
>> +
>> if (!((cmd->args[0] >> 7) & 0x01)) {
>> ret = -ETIMEDOUT;
>> goto err_mutex_unlock;
>>
>
> --
> http://palosaari.fi/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c index 29a5936..b68ab34 100644 --- a/drivers/media/dvb-frontends/si2168.c +++ b/drivers/media/dvb-frontends/si2168.c @@ -60,6 +60,12 @@ static int si2168_cmd_execute(struct i2c_client *client, struct si2168_cmd *cmd) jiffies_to_msecs(jiffies) - (jiffies_to_msecs(timeout) - TIMEOUT)); + /* error bit set? */ + if ((cmd->args[0] >> 6) & 0x01) { + ret = -EREMOTEIO; + goto err_mutex_unlock; + } + if (!((cmd->args[0] >> 7) & 0x01)) { ret = -ETIMEDOUT; goto err_mutex_unlock;
Return error from si2168_cmd_execute in case the demodulator returns an error. Signed-off-by: Olli Salonen <olli.salonen@iki.fi> --- drivers/media/dvb-frontends/si2168.c | 6 ++++++ 1 file changed, 6 insertions(+)