| Message ID | 1390168117-2925-4-git-send-email-fschaefer.oss@googlemail.com (mailing list archive) |
|---|---|
| State | Rejected, 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 1W50Dg-0007bz-Kx; Sun, 19 Jan 2014 22:47:52 +0100 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.72/mailfrontend-6) with esmtp id 1W50De-0004Jh-3q; Sun, 19 Jan 2014 22:47:52 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752250AbaASVrs (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Sun, 19 Jan 2014 16:47:48 -0500 Received: from mail-ea0-f182.google.com ([209.85.215.182]:61780 "EHLO mail-ea0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752055AbaASVrq (ORCPT <rfc822;linux-media@vger.kernel.org>); Sun, 19 Jan 2014 16:47:46 -0500 Received: by mail-ea0-f182.google.com with SMTP id r15so1909730ead.27 for <linux-media@vger.kernel.org>; Sun, 19 Jan 2014 13:47:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20120113; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; bh=UKhGSvPXeMm1gS8y3v/6Q5jVh1xtCrXD9UOHgeNNdVQ=; b=V0HW5aHMHhZ5ol6MD7r/8Qh7k0T7UE6ldSI5LzbzC2tPMYAqw2EY4Q/HDy2LsP1uHS N3yDHVlH/e/YFJe/R1J/4J3xvKN6XhwFKzZLAZUa1bi/LXDpU9WgDJcLzVaUe9ZsK5KR SDMZrcDpB5Nv3DJa7O8V4t9oGJ80BcsgUHU5VMqLvAg+kmVAqe5TY5QZ/i7r1YsNwtS7 PKK3xmCro04vGEauPWRkLiJ4S22lmGYsSWyRbQdhdZLoNN7/7f6+UGVohrUaIkHFP/93 TKKSbCoIPvtxfkUv2rkuqctmmIWhOXnAzm5twtj1nmInrxJKg4ln19FDZmGzXg4E1dAC z6Xg== X-Received: by 10.15.111.6 with SMTP id ci6mr14190155eeb.59.1390168065126; Sun, 19 Jan 2014 13:47:45 -0800 (PST) Received: from Athlon64X2-5000.site (ip-178-201-83-195.unitymediagroup.de. [178.201.83.195]) by mx.google.com with ESMTPSA id j46sm47007878eew.18.2014.01.19.13.47.44 for <multiple recipients> (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 19 Jan 2014 13:47:44 -0800 (PST) From: =?UTF-8?q?Frank=20Sch=C3=A4fer?= <fschaefer.oss@googlemail.com> To: m.chehab@samsung.com Cc: linux-media@vger.kernel.org, =?UTF-8?q?Frank=20Sch=C3=A4fer?= <fschaefer.oss@googlemail.com> Subject: [PATCH 3/4] em28xx-i2c: do not map -ENXIO errors to -ENODEV for empty i2c transfers Date: Sun, 19 Jan 2014 22:48:36 +0100 Message-Id: <1390168117-2925-4-git-send-email-fschaefer.oss@googlemail.com> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1390168117-2925-1-git-send-email-fschaefer.oss@googlemail.com> References: <1390168117-2925-1-git-send-email-fschaefer.oss@googlemail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: 2014.1.19.213915 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_1200_1299 0, BODY_SIZE_2000_LESS 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, CT_TEXT_PLAIN_UTF8_CAPS 0, DKIM_SIGNATURE 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __CP_URI_IN_BODY 0, __CT 0, __CTE 0, __CT_TEXT_PLAIN 0, __FRAUD_BODY_WEBMAIL 0, __FRAUD_WEBMAIL 0, __FRAUD_WEBMAIL_FROM 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __IN_REP_TO 0, __MIME_TEXT_ONLY 0, __MIME_VERSION 0, __MULTIPLE_RCPTS_CC_X2 0, __PHISH_SPEAR_STRUCTURE_1 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
Frank Schaefer
Jan. 19, 2014, 9:48 p.m. UTC
Commit e63b009d6e "" changed the error codes i2c ACK errors from -ENODEV to -ENXIO.
But it also introduced a line that maps -ENXIO back to -ENODEV in case of empty i2c
messages, which makes no sense, because
1.) an ACK error is an ACK error no matter what the i2c message content is
2.) -ENXIO is perfectly suited for probing, too
3.) we are loosing the ability to distinguish USB device disconnects
Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
drivers/media/usb/em28xx/em28xx-i2c.c | 1 -
1 Datei geändert, 1 Zeile entfernt(-)
Comments
Em Sun, 19 Jan 2014 22:48:36 +0100 Frank Schäfer <fschaefer.oss@googlemail.com> escreveu: > Commit e63b009d6e "" changed the error codes i2c ACK errors from -ENODEV to -ENXIO. > But it also introduced a line that maps -ENXIO back to -ENODEV in case of empty i2c > messages, which makes no sense, because > 1.) an ACK error is an ACK error no matter what the i2c message content is > 2.) -ENXIO is perfectly suited for probing, too I don't agree with this patch. 0-byte messages are only usin during device probe. > 3.) we are loosing the ability to distinguish USB device disconnects Huh? > > Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com> > --- > drivers/media/usb/em28xx/em28xx-i2c.c | 1 - > 1 Datei geändert, 1 Zeile entfernt(-) > > diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c > index ba6433c..a26d7d4 100644 > --- a/drivers/media/usb/em28xx/em28xx-i2c.c > +++ b/drivers/media/usb/em28xx/em28xx-i2c.c > @@ -539,7 +539,6 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap, > if (rc == -ENXIO) { > if (i2c_debug > 1) > printk(KERN_CONT " no device\n"); > - rc = -ENODEV; > } else { > if (i2c_debug > 1) > printk(KERN_CONT " ERROR: %i\n", rc);
Am 04.02.2014 19:47, schrieb Mauro Carvalho Chehab: > Em Sun, 19 Jan 2014 22:48:36 +0100 > Frank Schäfer <fschaefer.oss@googlemail.com> escreveu: > >> Commit e63b009d6e "" changed the error codes i2c ACK errors from -ENODEV to -ENXIO. >> But it also introduced a line that maps -ENXIO back to -ENODEV in case of empty i2c >> messages, which makes no sense, because >> 1.) an ACK error is an ACK error no matter what the i2c message content is >> 2.) -ENXIO is perfectly suited for probing, too > I don't agree with this patch. 0-byte messages are only usin during device > probe. ??? The error handling is inconsistent for no good reason. The old code always returned -ENODEV. Then you came to the conclusion that -ENODEV isn't good and we both agreed that -ENXIO is appropriate. But then you decided to keep -ENODEV for 0-Byte messages only. Why ? According to the i2c error code description, -ENXIO and -ENODEV are both suited for probing. AFAICS there are zero reasons for returning different error codes in case of the same i2c ack error. So please, either -ENODEV or -ENXIO instead of such inconsistencies. >> 3.) we are loosing the ability to distinguish USB device disconnects > Huh? Maybe (like me) you didn't notice that before. This is probably the most cogent argument for changing -ENODEV to -ENXIO for i2c ack errors in case of USB devices. ;-) Regards, Frank >> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com> >> --- >> drivers/media/usb/em28xx/em28xx-i2c.c | 1 - >> 1 Datei geändert, 1 Zeile entfernt(-) >> >> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c >> index ba6433c..a26d7d4 100644 >> --- a/drivers/media/usb/em28xx/em28xx-i2c.c >> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c >> @@ -539,7 +539,6 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap, >> if (rc == -ENXIO) { >> if (i2c_debug > 1) >> printk(KERN_CONT " no device\n"); >> - rc = -ENODEV; >> } else { >> if (i2c_debug > 1) >> printk(KERN_CONT " ERROR: %i\n", rc); -- 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
On 09.02.2014 20:34, Frank Schäfer wrote: > > Am 04.02.2014 19:47, schrieb Mauro Carvalho Chehab: >> Em Sun, 19 Jan 2014 22:48:36 +0100 >> Frank Schäfer <fschaefer.oss@googlemail.com> escreveu: >> >>> Commit e63b009d6e "" changed the error codes i2c ACK errors from -ENODEV to -ENXIO. >>> But it also introduced a line that maps -ENXIO back to -ENODEV in case of empty i2c >>> messages, which makes no sense, because >>> 1.) an ACK error is an ACK error no matter what the i2c message content is >>> 2.) -ENXIO is perfectly suited for probing, too >> I don't agree with this patch. 0-byte messages are only usin during device >> probe. > ??? > > The error handling is inconsistent for no good reason. > > The old code always returned -ENODEV. > Then you came to the conclusion that -ENODEV isn't good and we both > agreed that -ENXIO is appropriate. > But then you decided to keep -ENODEV for 0-Byte messages only. > Why ? > According to the i2c error code description, -ENXIO and -ENODEV are both > suited for probing. > AFAICS there are zero reasons for returning different error codes in > case of the same i2c ack error. > So please, either -ENODEV or -ENXIO instead of such inconsistencies. > >>> 3.) we are loosing the ability to distinguish USB device disconnects >> Huh? > Maybe (like me) you didn't notice that before. > This is probably the most cogent argument for changing -ENODEV to -ENXIO > for i2c ack errors in case of USB devices. ;-) I looked the I2C spec and there seems to be *not* restriction I2C message len, nor any mention zero len is not valid. If that is the case, adapter should just do the zero len xfer if requested and return success if it is success (slave answers). If adapter does not support zero len messages it should return EOPNOTSUPP according to kernel i2c fault codes document. I think em28xx supports, so that is not case here. I think Frank is correct. I2C spec is here: http://www.nxp.com/documents/user_manual/UM10204.pdf Hope this helps. regards Antti
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c index ba6433c..a26d7d4 100644 --- a/drivers/media/usb/em28xx/em28xx-i2c.c +++ b/drivers/media/usb/em28xx/em28xx-i2c.c @@ -539,7 +539,6 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap, if (rc == -ENXIO) { if (i2c_debug > 1) printk(KERN_CONT " no device\n"); - rc = -ENODEV; } else { if (i2c_debug > 1) printk(KERN_CONT " ERROR: %i\n", rc);