On Thu, 5 Apr 2007 12:13:02 +0100 Alan Cox <alan@xxxxxxxxxxxxxxxxxxx> wrote: > On Wed, 4 Apr 2007 21:37:04 -0700 > Randy Dunlap <randy.dunlap@xxxxxxxxxx> wrote: > > > From: Randy Dunlap <randy.dunlap@xxxxxxxxxx> > > > > Check PCI interface function results for errors in drivers/ide/pci. > > > > drivers/ide/pci/cs5530.c:244: warning: ignoring return value of 'pci_set_mwi', declared with attribute warn_unused_result > > NAK (again) Disagree. > The bug is in pci_set_mwi, it should not force return value checking. > Please fix that instead. You don't describe your reasoning, but I will assume that it is as follows: 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. 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. The changes we should make are a) rename pci_set_pwi() to pci_try_set_mwi() b) make pci_try_set_mwi() return a positive, non-zero code when the attempt to set the cacheline size failed. And make it return a negative error code when fatal errors occur. This way a) drivers which _do_ care about whether or not the cacheline size was set successfully can test for it or generate warnings and b) we can tell when pci_try_set_mwi() encountered a fatal error for other reasons. Now this may all sound excessively anal, but it is simply the correct design. And one of the reasons for adding the __must_check annotation is to weed out design errors. Simply turning the thing off is a step backwards. 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. - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
