Hans Verkuil wrote:
> On Friday 18 April 2008 04:15:53 Andy Walls wrote:
> > Hans,
> >
> > In the following 4 messages I'm submitting I2C related patches for
> > the cx18 driver. Some of these patches are cleaned up versions of
> > ones I posted earlier to the list.
> >
> > The patches build upon each other and must be applied in order. They
> > were broken up to facilitate inspection of one changeset at time.
> >
> > Summary of patches:
> > Patch 1/4: Collapse per I2C bus callback functions into a common set
>
> Good one.
>
> > Patch 2/4: Force PCI MMIO posted writes to complete for time
> > sensitive i2c bus line manipulation
>
> I do not believe that this will make any difference. I have tried this
> in the past, but without any improvement.
That's surprising to hear.
I did notice this peculiar setting on my machine:
# lspci -vv
[...]
02:01.0 Multimedia video controller: Conexant Unknown device 5b7a
Subsystem: Hauppauge computer works Inc. Unknown device 7444
[...]
Latency: 64 (500ns min, 50000ns max), Cache Line Size: 32 bytes
[...]
Region 0: Memory at f4000000 (32-bit, non-prefetchable) [size=64M]
[...]
02:02.0 Multimedia video controller: Internext Compression Inc iTVC16 (CX23416)
MPEG-2 Encoder (rev 01)
Subsystem: Hauppauge computer works Inc. WinTV PVR 150
[...]
Latency: 64 (32000ns min, 2000ns max), Cache Line Size: 32 bytes
[...]
Region 0: Memory at dc000000 (32-bit, prefetchable) [size=64M]
[...]
The HVR-1600 CX23418's memory window is "non-prefetchable" as I would
expect. The PVR-150 CX23416's memory window is marked as
"prefetchable".
I'm wondering if in your past experience the write_reg_sync() did flush
posted writes out of a bridge, but subsequent reads of the registers to
get SCL and SDA states came from prefetched data? I'm not sure how one
would work around that, except for reading some other register more the
one cache line away, or constantly poling the registers like ivtv
newi2c=1 does.
> In particular I do not like the extra DEBUG_WARN code: this should
> really never happen and AFAIK can never happen. It's OK for people to
> test, but I do not agree with putting this in the final driver.
I don't like the DEBUG_WARN mess either. I was hoping to see if some
tester's hardware would trip it.
I'd be happy with just a single write_reg_sync() call there instead.
There is at least one case I can think of where it is possible for the
DEBUG_WARN to get tripped:
# lspci -tv
-[0000:00]-+-00.0 ATI Technologies Inc RS480 Host Bridge
+-01.0-[0000:01]----05.0 ATI Technologies Inc RS480 [Radeon Xpress
200G Series]
+-12.0 ATI Technologies Inc 4379 Serial ATA Controller
+-13.0 ATI Technologies Inc IXP SB400 USB Host Controller
+-13.1 ATI Technologies Inc IXP SB400 USB Host Controller
+-13.2 ATI Technologies Inc IXP SB400 USB2 Host Controller
+-14.0 ATI Technologies Inc IXP SB400 SMBus Controller
+-14.1 ATI Technologies Inc Standard Dual Channel PCI IDE Controller
+-14.3 ATI Technologies Inc IXP SB400 PCI-ISA Bridge
+-14.4-[0000:02]--+-00.0 RaLink RT2500 802.11g Cardbus/mini-PCI
| +-01.0 Conexant Unknown device 5b7a
| +-02.0 Internext Compression Inc iTVC16 (CX23416)
MPEG-2 Encoder
| +-03.0 Realtek Semiconductor Co., Ltd.
RTL-8139/8139C/8139C+
| \-04.0 VIA Technologies, Inc. IEEE 1394 Host
Controller
+-14.5 ATI Technologies Inc IXP SB400 AC'97 Audio Controller
+-18.0 Advanced Micro Devices [AMD] K8 [Athlon64/Opteron]
HyperTransport Technology Configuration
+-18.1 Advanced Micro Devices [AMD] K8 [Athlon64/Opteron] Address
Map
+-18.2 Advanced Micro Devices [AMD] K8 [Athlon64/Opteron] DRAM
Controller
\-18.3 Advanced Micro Devices [AMD] K8 [Athlon64/Opteron]
Miscellaneous Control
Write data from a CPU, destined for the HVR-1600 at 01.0, gets through
host bridge [0000:00] and gets posted into PCI-PCI bridge [0000:02].
The CPU thinks the write has completed. When data is flushed from
PCI-PCI bridge [0000:02] to 01.0, there is a PERR or SERR on the second
bus segment that prevents the write from being completed to 01.0.
How likely is this? I don't know. The comments in
linux/drivers/pci/quirks.c make me think it's not as rare as I would
have first guessed. There may be other failure scenarios as well.
What I should really do, to catch PCI errors, is write PCI error
callback routines for the cx18 driver as discussed in
linux/Documentation/pci-error-recovery.txt.
> > Patch 3/4: Perform more extensive I2C bus normalization and slave
> > reset
>
> This is interesting. I'd be very curious to hear whether this will fix
> things!
I'm not as hopeful as you are on these, but I wanted to cover all the
bases. (Aside from the PCI bus and I2C slaves, can you think of any
other potential problem sources?)
If these routines don't seem to help at first, there is some slight room
for improving them (i.e. watching for slaves pulling the SCL or SDA low
and implementing timeout loops). I just wanted to get the main
sequences implemented first.
The EEPROM reset sequence actually has an element (START followed by
STOP) that is illegal according to the official I2C bus spec. But that
same spec acknowledges that element is not a problem for most slaves.
> > Patch 4/4: Fix I2C timing constants
>
> No problem.
>
> > I have tested all of these and they don't introduce any problems for
> > me. My hope is that patches 2/4 and 3/4 fix the I2C problems
> HVR-1600
> > users have been experiencing, or at least shed some light on the
> > symptoms with additional debug messages.
>
> Everyone who has i2c problems: please try these patches and send
> feedback!
If feedback included "lscpi -tv" and "lspci -vvvnnxxx" information that
would be helpful. Since the problem seems to follow mainboards,
differential analysis of working mainboards vs problem mainboards may
help locate the root cause.
> BTW: it is likely that the cx18 driver will go into 2.6.26, even if
> there are still some outstanding issues. So that is good news.
Cool.
> Hans
Thanks for taking the time to review and comment.
Regards,
Andy
_______________________________________________
ivtv-devel mailing list
[email protected]
http://ivtvdriver.org/mailman/listinfo/ivtv-devel