Re: [PATCH 1/5] mxl5007t: fix buggy register read

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/10/2013 03:20 AM, Devin Heitmueller wrote:
On Tue, Apr 9, 2013 at 7:53 PM, Antti Palosaari <crope@xxxxxx> 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@xxxxxx>

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/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux