Re: [PATCH] Add ata_piix's own resume function

[Jens Axboe <axboe@suse.de>]

On Fri, May 26 2006, Jeff Garzik wrote:
> Jens Axboe wrote:
> >On Fri, May 26 2006, zhao, forrest wrote:
> >>For ata_piix resume operation, it first waits for BUSY bit clear,
> >>then calls ata_device_resume().
> >
> >This has the problem that it introduces scsi specific knowledge into
> >ata_piix, something that is both a violation and a problem because we
> >are moving libata away from scsi. I think the best way to currently do
> >this is to introduce a ata_port_ops hook (pre_resume()?) that waits for
> >busy clear and gets called in ata_device_resume is probably the way to
> >go.
> 
> ata_device_resume() is fine as-is.  Modifying it to resurrect the bus is 
> a gross layering violation.  Resume must be done in this order:
> 
> 	controller -> bus -> device
> 
> Thus, the bus must be resurrected and brought to a known HSM state (bus 
> idle), and then ata_device_resume() will work just fine.
> 
> The proper solution is to modify the pci_driver::resume code path, to 
> resurrect not only the HBA but also the ATA bus.  Currently we have 
> ata_pci_device_{suspend,resume}, whose contents is wholly generic to any 
> random PCI device.
> 
> I would suggest creating pata_pci_device_resume(), which calls 
> ata_pci_device_resume(), and then waits for BSY to clear.

I thought about that, and I don't agree. Waiting for the BSY to clear is
not a pci property, at best I'd consider that even worse than defining a
scsi resume function in ata_piix.

-- 
Jens Axboe