Message ID | 1430658023-17579-3-git-send-email-olli.salonen@iki.fi (mailing list archive) |
---|---|
State | Superseded, archived |
Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from <linux-media-owner@vger.kernel.org>) id 1YotVz-0006BT-Jr; Sun, 03 May 2015 15:00:59 +0200 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.76/mailfrontend-5) with esmtp id 1YotVx-0006oB-8o; Sun, 03 May 2015 15:00:59 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751906AbbECNAz (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Sun, 3 May 2015 09:00:55 -0400 Received: from mail-lb0-f182.google.com ([209.85.217.182]:33107 "EHLO mail-lb0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751340AbbECNAy (ORCPT <rfc822; linux-media@vger.kernel.org>); Sun, 3 May 2015 09:00:54 -0400 Received: by lbbzk7 with SMTP id zk7so89637900lbb.0 for <linux-media@vger.kernel.org>; Sun, 03 May 2015 06:00:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references; bh=gZagXGabbu6LGpGoOlIhtsIwDoyhMgAwn9UTScBBXko=; b=SJL4GqmLojraNYncI+O6nSR3qI8KGhmpMmITRxRzMAZFmz1/pIF9yeLIRBb+9ALJWk Z8OPhjEaIhLW0zvOcIgHt9OqNQ+eu9+7tTr05xc1QXN1auRVJ2p2vpCoks8h8sxjBWN1 pyJ8cTeNVcF9WyC9475mN8m89wIXuyANHkxyz2wetXPLKfGpUBceZtMmlpDdo3BGhKuG j8W441lg/LxrQ1SWNW5+F0FQYOD5okXv2J4EuQEznhr6ZV1A0BlvdgsMwpvjSNbHxIgr 2g9H0pkxDXyVqBh7LyrmG/4RnK28RcS7NdZORTnnlo0P9J/YrJDczV7+lpDzdqpRmj6P OXug== X-Gm-Message-State: ALoCoQmRGneIlifvyGo4J+kBKVVQYvyB3KDs5XJEL78EDhAdT51S7afDC7KAIJsr6PqxFOtoVrwR X-Received: by 10.112.181.68 with SMTP id du4mr15190392lbc.11.1430658053113; Sun, 03 May 2015 06:00:53 -0700 (PDT) Received: from dl160.lan (188-67-118-178.bb.dnainternet.fi. [188.67.118.178]) by mx.google.com with ESMTPSA id s8sm2673608las.29.2015.05.03.06.00.51 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Sun, 03 May 2015 06:00:52 -0700 (PDT) From: Olli Salonen <olli.salonen@iki.fi> To: linux-media@vger.kernel.org Cc: Olli Salonen <olli.salonen@iki.fi>, Antti Palosaari <crope@iki.fi> Subject: [PATCH v3 5/6] si2168: add I2C error handling Date: Sun, 3 May 2015 16:00:22 +0300 Message-Id: <1430658023-17579-3-git-send-email-olli.salonen@iki.fi> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1430658023-17579-1-git-send-email-olli.salonen@iki.fi> References: <1430658023-17579-1-git-send-email-olli.salonen@iki.fi> Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2015.5.3.125717 X-PMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_1000_1099 0, BODY_SIZE_2000_LESS 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, REFERENCES 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __CP_URI_IN_BODY 0, __DATE_TZ_RU 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __IN_REP_TO 0, __MIME_TEXT_ONLY 0, __MULTIPLE_RCPTS_CC_X2 0, __REFERENCES 0, __SANE_MSGID 0, __SUBJ_ALPHA_END 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_NO_WWW 0, __URI_NS , __YOUTUBE_RCVD 0' |
Commit Message
Olli Salonen
May 3, 2015, 1 p.m. UTC
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(+)
Comments
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;