Tejun Heo wrote:
On Mon, Mar 13, 2006 at 04:56:14AM -0500, Jeff Garzik wrote:
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...
AFAICS, ata_piix doesn't call ata_port_disable() directly, but
sata_mv() does, through mv_host_intr() -> mv_err_intr()
->mv_stop_and_reset() -> __mv_phy_reset(). I'll check the code path
such that disabled ports are handled properly. Thanks for pointing
out.
Yeah, sorry, for ata_piix I was looking at vanilla linux-2.6.git rather
than libata-dev.git#upstream... :)
Jeff
-
: 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
