Re: [PATCH] IDE: fix PCI must_checks

[Andrew Morton <akpm@linux-foundation.org>]

On Thu, 5 Apr 2007 21:16:09 +0100
Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > pci_set_mwi() is an advisory thing, and on certain platforms it might fail
> > to set the cacheline size to the desired number.  This is not a fatal error
> > and the driver can successfully run at a lesser performance level.
> 
> Correct.
> 
> > If that description is accurate then I'd contend that pci_set_mwi() is
> > misdesigned.  It should not be returning a negative error code for
> > something which is not an error.
> 
> It is an error to *some* drivers but not all. Kind of like setting some
> of the other features may be essential for some chips and not others.
> 
> > And we *need* to be excessively anal in the PCI setup code.  We have metric
> > shitloads of bugs due to problems in that area, and the more formality and
> > error handling and error reporting we can get in there the better off we
> > will be.
> 
> No argument there
> 
> If we want to deal with some of the mess we should also remove all direct
> writing of PCI latency timers and replace them with a function to stop
> drivers setting unsafe values and ignoring chip errata the core knows
> about but they dont

hm. Well, what to do?

How about we prevail upon Randy to:

- rename pci_set_mwi() to pci_try_set_mwi()

- make it return 0 on success, 1 if the "try" failed

- make it return -EFOO on error (presently unimplemented)

- review callers

- remove __must_check

?



I have a feeling that this is "still wrong": even things like
pci_read_config_foo() can get errors, and there are various PCI-bus error
handling proposals floating about which might require that callers be more
careful in what they accept.