On Sun, 31 Oct 1999, Alan Cox wrote:

> > The initialisation of PCI devices has nothing to do with theory. This job=
> > must be done prior to attaching drivers to PCI devices. If the BIOS didn't=
> > do the work, the right place for doing the work is certainly _not_ PCI
> > device drivers.
> 
> Only the drivers know about some of the issues. Worse yet we have to deal
> with other horrible problems when we do set it because the drivers don't know
> about the motherboard/chipset bugs. 

Indeed. And some bugs are even desired behaviour or lack of understanding
of the specifications: About half the number of existing host bridges for
example are broken by design regarding PCI ordering rules. 
 
> > trade-off that takes into account the MIN_GNT and MAX_LAT of all PCI
> > devices. The Linux kernel design is _quite_ fine for implementing the
> > right thing in the right place regarding PCI devices initialisation.=20
> 
> They are only part of the value. Some chipsets have latency constraints that
> are imposed. for example the SIS5598 which cannot handle high latency settings.
> I'd prefer to see it in drivers/pci as well.
> 
> > latency timer.. The system software that deals with latency timer values
> > must choose some compromise between desires of _all_ PCI device drivers
> 
> and all non PCI devices, and bugs, and ISA interactions. Then add devices
> that misreport their needs. So yes it does want to be in drivers/pci

Devices that misreport their needs should be quirked, assuming that most 
are reporting resonnably their needs.
 
> > Therefore, tampering latency timers from PCI device drivers can only be a
> > _wrong_ approach, unless it is intended to fix some hardware bug that
> > addresses this device in particular.
> 
> Anything which isn't a paticular device quirk should be in drivers/pci. Im not
> sure that is true with 2.2.x even if it is becoming so with 2.3.x.
> 
> If you had people hitting the latency 0 problem then the patch for 2.2.x needs
> to both remove it from the driver and put it where it belongs in drivers/pci.

The drivers (both sym and ncr) care about latency timers since years.

By the way, the change I have removed based the latency timer value on 
MIN_GNT*8. As you know MIN_GNT is in 0.25 micro-second unit.

This looks fine in theory, but this is not so, for example:

The 895 revision 1 has MIN_GNT=30 which is indeed a bogus value, but the
896 (all revisions) has MIN_GNT=17 which is fine. 

The driver has far better knowledge about the need in latency for the
devices, since it knows about the max burst size it will configure.
So, the driver want to use something like the following code for a good
majorant of latency timer value:

        /*
         * Tune PCI LATENCY TIMER according to burst max length transfer.
         * (latency timer >= burst length + 6, we add 10 to be quite sure)
         * If current value is zero, the device has probably been configured 
         * for no bursting due to some broken hardware.
         */

        if (latency_timer == 0 && chip->burst_max)
                printk("ncr53c8xx: PCI_LATENCY_TIMER=0, bursting should'nt be 
allowed.\n");

        if ((driver_setup.pci_fix_up & 4) && chip->burst_max) {
                uchar lt = (1 << chip->burst_max) + 6 + 10;
                if (latency_timer < lt) {
                        latency_timer = lt;
                        if (initverbose)
                                printk("ncr53c8xx: setting PCI_LATENCY_TIMER to %d bus 
clocks (fix-up).\n", latency_timer);
                         (void) pcibios_write_config_byte(bus, device_fn,
                                        PCI_LATENCY_TIMER, latency_timer);
                }
        }

        /*
         * Fix up for recent chips that support CACHE LINE SIZE.
         * If PCI config space is not OK, remove features that shall not be 
         * used by the chip. No need to trigger possible chip bugs.
         */

        if ((chip->features & FE_CLSE) && cache_line_size == 0) {
                chip->features &= ~FE_CACHE_SET;
                printk("ncr53c8xx: PCI_CACHE_LINE_SIZE not set, features based on 
CACHE LINE SIZE not used.\n");
        }

This code if for _years_ in the ncr driver and was there in the first
version of the sym driver.
 
> It could also do with DaveM and Rth checking that isnt needed on sparc/alpha
> although I would hope there is ifdef(__sparc__) around such cases.

The ifdef(__sparc__) does not give permission to hack everywhere.
 
> The last two were the points I was trying to make. The rest definitely belongs
> elsewhere. If you stick a 3c590 into a SIS5598 board and run X11 right now
> you get display flicker because the 3c590 doesnt know that the latency fix
> it does is worse than the disease in this case.

May-be this one was relevant, but the hack I removed was not (see above).

G�rard.


-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]

Reply via email to