From: Antti Palosaari <crope@iki.fi>
To: Devin Heitmueller <dheitmueller@kernellabs.com>
Cc: linux-media@vger.kernel.org, Michael Krufky <mkrufky@linuxtv.org>
Subject: Re: [PATCH 1/5] mxl5007t: fix buggy register read
Date: Wed, 10 Apr 2013 03:45:05 +0300 [thread overview]
Message-ID: <5164B611.2060500@iki.fi> (raw)
In-Reply-To: <CAGoCfiw_pyh5MchkU59Y9NJz+Rgf5B7Gvd92A1pF+e18DVWgKQ@mail.gmail.com>
On 04/10/2013 03:20 AM, Devin Heitmueller wrote:
> On Tue, Apr 9, 2013 at 7:53 PM, Antti Palosaari <crope@iki.fi> wrote:
>> Chip uses WRITE + STOP + READ + STOP sequence for I2C register read.
>> Driver was using REPEATED START condition which makes it failing if
>> I2C adapter was implemented correctly.
>>
>> Add use_broken_read_reg_intentionally option to keep old buggy
>> implantation as there is buggy I2C adapter implementation relying
>> that bug...
>>
>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>
> Hi Antti,
>
> The existing code actually looks fine. This is actually how most
> devices do register reads.
Yes, most devices do that, but not all!
MxL5007t has a special register for setting register to read. Look the
code and you could see it easily. It was over year ago I fixed it...
> Further, it *should* be done in a single call to i2c_transfer() or
> else you won't hold the lock and you will create a race condition.
No. That's why I added new lock. Single i2c_transfer() means all
messages are done using repeated START condition.
> This sounds more like it's a bug in the i2c master rather than the 5007 driver.
No.
> Do you have i2c bus traces that clearly show that this was the cause
> of the issue? If we need to define something as "broken" behavior, at
> first glance it looks like the way *you're* proposing is the broken
> behavior - presumably to work around a bug in the i2c master not
> properly supporting repeated start.
Yes and no. I made own Cypress FX2 firmware and saw initially that issue
then. Also, as you could see looking the following patches I ensured /
confirmed issue using two different I2C adapters (AF9015 and AF9035). So
I have totally 3 working adapters to prove it (which are broken without
that patch)!
> Also, any reason you didn't put Mike into the cc: for this (since he
> owns the driver)?
you added :)
> Devin
regards
Antti
--
http://palosaari.fi/
next prev parent reply other threads:[~2013-04-10 0:45 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-09 23:53 [PATCH 0/5] fix buggy mxl5007t register read Antti Palosaari
2013-04-09 23:53 ` [PATCH 1/5] mxl5007t: fix buggy " Antti Palosaari
2013-04-10 0:20 ` Devin Heitmueller
2013-04-10 0:45 ` Antti Palosaari [this message]
2013-04-10 1:38 ` Devin Heitmueller
2013-04-10 1:53 ` Antti Palosaari
2013-04-09 23:53 ` [PATCH 2/5] af9015: fix I2C adapter read (without REPEATED STOP) Antti Palosaari
2013-04-09 23:53 ` [PATCH 3/5] af9015: do not use buggy mxl5007t read reg Antti Palosaari
2013-04-09 23:53 ` [PATCH 4/5] af9035: implement I2C adapter read operation Antti Palosaari
2013-04-09 23:53 ` [PATCH 5/5] af9035: do not use buggy mxl5007t read reg Antti Palosaari
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=5164B611.2060500@iki.fi \
--to=crope@iki.fi \
--cc=dheitmueller@kernellabs.com \
--cc=linux-media@vger.kernel.org \
--cc=mkrufky@linuxtv.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.