Re: [SAA713X TESTERS WANTED] Fix i2c quirk, may affect saa713x + mt352 combo

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

 



On Thu, May 21, 2009 at 6:30 PM, hermann pitton <hermann-pitton@xxxxxxxx> wrote:
> Hi,
>
> Am Donnerstag, den 21.05.2009, 08:53 -0400 schrieb Michael Krufky:
>> On Thu, May 21, 2009 at 8:51 AM, Michael Krufky <mkrufky@xxxxxxxxxxxxxx> wrote:
>> > On Thu, May 21, 2009 at 2:44 AM, Oldrich Jedlicka <oldium.pro@xxxxxxxxx>
>> > wrote:
>> >>
>> >> Hi Mike,
>> >>
>> >> On Wednesday 20 of May 2009 at 21:57:15, Michael Krufky wrote:
>> >> > I have discovered a bug in the saa7134 driver inside the function,
>> >> > "saa7134_i2c_xfer"
>> >> >
>> >> > In order to communicate with certain i2c clients on the saa713x i2c
>> >> > bus, a quirk was implemented to prevent failures during read
>> >> > transactions.
>> >> >
>> >> > The quirk forces an i2c write/read to a bogus address that is unlikely
>> >> > to be used by any i2c client.
>> >> >
>> >> > However, this quirk is not functioning properly.  The reason for the
>> >> > malfunction is that the i2c address chosen to use as the quirk address
>> >> > was 0xfd.
>> >> >
>> >> > The address 0xfd is indeed an i2c address unlikely to be used by any
>> >> > real i2c client, however, the address itself is invalid!  The address,
>> >> > 0xfd, has the read bit set -- this is problematic for the hardware,
>> >> > and causes the quirk workaround to fail.
>> >> >
>> >> > It's a wonder that nobody else has complained up to this point.
>> >>
>> >> I had a problem with 0xfd quirk already (the presence caused the device
>> >> not to
>> >> respond), this is why there is an exception
>> >>
>> >>        msgs[i].addr != 0x40
>> >>
>> >> I can check if it works with your version (0xfe), but the device behind
>> >> the
>> >> address 0x40 (remote control) is very stupid, so I don't think so.
>> >>
>> >> I think that better approach would be to use the quirk only for devices
>> >> (addresses) that really need it, not for all.
>> >>
>> >> Cheers,
>> >> Oldrich.
>> >>
>> >> > I am asking for testers, just to make sure that this doesn't cause any
>> >> > other strange errors to occur as a side effect.  I don't expect any
>> >> > new problems, but its always better to be safe than sorry :-)
>> >> >
>> >> > This change should not fix any of the other issues currently being
>> >> > discussed with the saa7134 driver -- all I am asking is for people to
>> >> > test and indicate that the change does not incur any NEW bugs or
>> >> > unwanted behavior.
>> >> >
>> >> > Please test the following repository, and send in your feedback as a
>> >> > reply to this thread.  Please remember to keep the mailing list in cc.
>> >> >
>> >> > http://kernellabs.com/hg/~mk/saa7134
>> >> >
>> >> > Thanks,
>> >> >
>> >> > Mike Krufky
>> >> > --
>> >> > 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
>> >>
>> >>
>> >
>> >
>> >
>> > Thanks for the feedback, guys.  Just to reiterate my original point:
>> >
>> > I am not questioning the i2c quirk itself -- this is already in the driver,
>> > and cards are working already as is.  I am just saying that there is a bug
>> > based on the address used.  Consider 7-bit i2c addresses --   0xfd is an
>> > 8-bit address, when converted to a 7-bit address and then back to an 8-bit
>> > address becomes 0xfc.  I don't know how to explain this properly, but it's
>> > invalid.  This is a bug in the saa7131 silicon that requires a write to a
>> > valid i2c address to occur before a read transaction, in order to prevent a
>> > situation where a read transaction results in no ack from the client.
>> >
>> > This is a known errata of the NXP silicon.
>> >
>> > The HVR1110r3 cannot perform successful i2c read transactions from the DVB-T
>> > demodulator without this i2c workaround in place, using a valid address.
>> >
>> > And yes, I am aware of David Wong's situation -- I have a changeset in one
>> > of my repositories that will allow us to disable this i2c quirk on a card by
>> > card basis, but so far I haven't heard of any card that needs this other
>> > than his board.
>> >
>> > http://linuxtv.org/hg/~mkrufky/dmbth/rev/781ffa6c43d3
>> >
>> > We can certainly merge that changeset if required.
>> >
>> > All I am asking is for you to test your boards against this tree and confirm
>> > that this changeset does not cause any *new* problems.
>> >
>> > Thanks go out to Hermann and Oldrich so far, as both of you seem to indicate
>> > that this tree did not cause any new problem on your hardware.
>> >
>> > Thanks again for the feedback.
>> >
>> > Regards,
>> >
>> > Mike
>> >
>>
>> (apologies for the double post -- first one got rejected by vger, again)
>>
>> Thanks for the feedback, guys.  Just to reiterate my original point:
>>
>> I am not questioning the i2c quirk itself -- this is already in the
>> driver, and cards are working already as is.  I am just saying that
>> there is a bug based on the address used.  Consider 7-bit i2c
>> addresses --   0xfd is an 8-bit address, when converted to a 7-bit
>> address and then back to an 8-bit address becomes 0xfc.  I don't know
>> how to explain this properly, but it's invalid.  This is a bug in the
>> saa7131 silicon that requires a write to a valid i2c address to occur
>> before a read transaction, in order to prevent a situation where a
>> read transaction results in no ack from the client.
>>
>> This is a known errata of the NXP silicon.
>>
>> The HVR1110r3 cannot perform successful i2c read transactions from the
>> DVB-T demodulator without this i2c workaround in place, using a valid
>> address.
>>
>> And yes, I am aware of David Wong's situation -- I have a changeset in
>> one of my repositories that will allow us to disable this i2c quirk on
>> a card by card basis, but so far I haven't heard of any card that
>> needs this other than his board.
>>
>>
>> http://linuxtv.org/hg/~mkrufky/dmbth/rev/781ffa6c43d3
>>
>> We can certainly merge that changeset if required.
>>
>> All I am asking is for you to test your boards against this tree and
>> confirm that this changeset does not cause any *new* problems.
>>
>> Thanks go out to Hermann and Oldrich so far, as both of you seem to
>> indicate that this tree did not cause any new problem on your
>> hardware.
>>
>> Thanks again for the feedback.
>>
>> Regards,
>>
>> Mike
>
> previously I reported only that i did test on my stuff with disabled
> quirk, like it was before the 300i appeared and also my recent cards did
> still work.
>
> Now, bad weather came up ;)
> and I tested also your repo with the 0xfe change on three different
> machines.
>
> With cards like two Medion Quad, CTX948 triple and CTX953. These fail on
> 2.6.29 for DVB-T without visible i2c errors and there was a report for a
> Tiger Hybrid LNA config 2 too. All fine like on anything else except
> 2.6.29.
>
> Also the Asus Tiger 3in1 LNA config 2, no problems and it had no
> problems on 2.6.29 with it. LNA is set to high gain with 0x22, 0x01 to
> 0x96 and for sure works.
>
> Also tested Asus P7131 Dual, Asus Tiger Rev:1.0.0 and some of the md7134
> hybrid devices with FMD1216ME MK3 like CTX917 and CTX946.
>
> As far I know they all follow Philips/NXP reference designs and use
> Philips drivers under windows.
>
> For the 300i reports exist, that it works better when it is hot
> enough :) You likely need feedback from such problematic cards.
>
> But we of course honor the pioneers in hardware and software.
>
> Strange, that you see now an even improved quirk is needed for
> Philips/NXP chips under each other. If NXP confirms the problem, nothing
> to discuss, else also not I think.
>
> Cheers,
> Hermann
>
> (please try LNA config 1 if you get some time)


Hermann,

So, in summary, you tested the 0xfe patch and it did not introduce any
*new* errors.  Correct?  Please confirm, just to avoid any possible
confusion.

I already tested on a Pinnacle 300i myself, and the 0xfe did not
create any new problems.  ( as a side note, I see unrelated problems
on the 300i that I intend to look at in the future.  This has nothing
to do with the i2c quirk )

Thanks for the testing.

Regards,

Mike
--
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