public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Antti Palosaari <crope@iki.fi>
To: "Frank Schäfer" <fschaefer.oss@googlemail.com>, m.chehab@samsung.com
Cc: linux-media@vger.kernel.org
Subject: Re: [PATCH 3/4] em28xx-i2c: do not map -ENXIO errors to -ENODEV for empty i2c transfers
Date: Wed, 12 Feb 2014 01:51:10 +0200	[thread overview]
Message-ID: <52FAB76E.3040001@iki.fi> (raw)
In-Reply-To: <52F7CA40.2010106@googlemail.com>

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

-- 
http://palosaari.fi/

  reply	other threads:[~2014-02-11 23:51 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-19 21:48 [PATCH 0/4] em28xx: resolve the remaining issues with the i2c code Frank Schäfer
2014-01-19 21:48 ` [PATCH 1/4] em28xx-i2c: fix the i2c error description strings for -ENXIO Frank Schäfer
2014-01-19 21:48 ` [PATCH 2/4] em28xx-i2c: fix the error code for unknown errors Frank Schäfer
2014-01-19 21:48 ` [PATCH 3/4] em28xx-i2c: do not map -ENXIO errors to -ENODEV for empty i2c transfers Frank Schäfer
2014-02-04 18:47   ` Mauro Carvalho Chehab
2014-02-09 18:34     ` Frank Schäfer
2014-02-11 23:51       ` Antti Palosaari [this message]
2014-01-19 21:48 ` [PATCH 4/4] em28xx-i2c: remove duplicate error printing code from em28xx_i2c_xfer() Frank Schäfer
2014-02-04 18:50   ` Mauro Carvalho Chehab
2014-02-09 18:38     ` Frank Schäfer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52FAB76E.3040001@iki.fi \
    --to=crope@iki.fi \
    --cc=fschaefer.oss@googlemail.com \
    --cc=linux-media@vger.kernel.org \
    --cc=m.chehab@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox