Christoph Hellwig wrote:
> I think this needs a little restructuring. ahd_pci_suspend/ahd_pci_resume
> should be merged into their callers and use the normal Linux pci accessors,
> and ahd_suspend/ahd_resume are tiny enough to merged into the caller aswell.
> 
Yes, partially. ahc_pci_resume() has to stay there as it uses quite some
functions from aic7xxx_pci.c. And I'd rather keep the overall structure
(ie split between linux-specific and generic pci access) for now.
In the long run it should be restructured as we don't do cross-compilation
with ***BSD anymore.

>> +#ifdef CONFIG_PM
>> +	.suspend	= ahd_linux_pci_dev_suspend,
>> +	.resume		= ahd_linux_pci_dev_resume,
>> +#endif
>>  	.remove		= ahd_linux_pci_dev_remove,
>>  	.id_table	= ahd_linux_pci_id_table
>>  };
>>  
>> +static int
>> +ahd_linux_pci_dev_suspend(struct pci_dev *pdev, pm_message_t mesg)
> 
> I think this needsa #ifdef CONFIG_PM aswell.  Also any chance you
> could implement functions before their use so we can avoid forward
> declarations.
> 
Yes, no problem.

>> +{
>> +	struct ahd_softc *ahd = pci_get_drvdata(pdev);
>> +	int rc;
>> +
>> +	if ((rc = ahd_suspend(ahd)))
>> +		return rc;
> 
> 	rc = ahd_suspend(ahd)
> 	if (rc)
> 		return rc;
> 
> but as I mentioned above better just inline the content of ahd_suspend
> into this function. That would also catch that ahd_suspend returns
> positive errno values and we'd have to invert them here.
> 
Ok. Done.
Updated patch attached.
More to your liking?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)