Re: [PATCH 3/4] libata: make ata_set_mode() responsible for failure handling

[Jeff Garzik <jgarzik@pobox.com>]

Tejun Heo wrote:
> On Mon, Mar 13, 2006 at 03:37:17AM -0500, Jeff Garzik wrote:
> 
>>I have the following concern with this patch (#3) however:
>>
>>
>>>-static void ata_dev_set_mode(struct ata_port *ap, struct ata_device *dev)
>>>+static int ata_dev_set_mode(struct ata_port *ap, struct ata_device *dev)
>>>{
>>>-	if (!ata_dev_present(dev) || (ap->flags & ATA_FLAG_PORT_DISABLED))
>>>-		return;
>>
>>I think you drop too many ATA_FLAG_PORT_DISABLED tests in this patch, 
>>leading the code to potentially miss a previously-flagged PORT_DISABLED 
>>(perhaps by an LLDD).
>>
> 
> 
> Hmmm... the plan is to disallow LLDD's take ports or devices offline
> from low level callbacks.  They should just let upper layer know by
> returning failure code.

Long term that plan is fine, but you still have to deal with the 
existing API one way or another.  ata_port_disable() is called directly 
by a bunch of Alan's PATA drivers, and by ata_piix and sata_mv.

Thus you would either need to keep the PORT_DISABLED checks or convert 
the drivers in question to a better API.

So just check those ata_port_disable() cases...

	Jeff