Re: libata passthru: support PIO multi commands
Alan Cox wrote: >>ata_scsi_pass_thru() is not executed at ioctl submission time (block >>queue submission time), but rather immediately before it is issued to >>the drive. At that point you know the bus is idle, all other commands >>have finished executing, and dev->multi_count is fresh not stale. The >>code path goes from ata_scsi_pass_thru() to ata_qc_issue() without >>releasing the spinlock, even. > > > Think up to user space > > Poorusersapp set multicount to 4 > > Evilproprietarytuningdaemon set multicount to 8 > > Poorusersapp issue I/O > > at which point an error is indeed best. > > >>But the last point is true -- we should error rather than just warn >>there, AFAICS. > > > Definitely. We've been asked "please do something stupid" and not even in > a case where the requester may know better. > It looks like the ATA passthru commands contain more information than what libata needs to execute a command. e.g. protocol number: libata could possibly infer the protocol from the command opcode. e.g. multi_count: libata caches dev->multi_count. Passing multi_count along with each passthru command looks useless for libata. e.g. t_dir: libata could possible infer the direction from the command opcode or from the protocol number (e.g. 4: PIO_IN / 5: PIO_OUT). Due to the redundant info, there is possiblely inconsistency between the parameters. e.g. t_dir vs protocol. e.g. command vs protocol. It seems the "redundant" parameters are designed to allow stateless SATL implementation: The application/passthru command tells the stateless SATL implementation the protocol and the multi_count, etc. Then SATL just follows the instruction blindly, even if asked to do something stupid. Currently libata - uses the passthru protocol number blindly (even if the application issues a DMA command with wrong PIO protocol.) - checks and warns about multi_count - ignores t_dir, byte_block and so on. Maybe we need a strategy to deal with incorrect passed-thru commands? say, - check and reject if something wrong - mimimal check and warn/ignore, if it doesn't hurt command execution -- albert - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[-mm patch] drivers/ata/sata_nv.c: make 3 functions static
This patch makes 3 needlessly global functions static. Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]> --- drivers/ata/sata_nv.c |8 1 file changed, 4 insertions(+), 4 deletions(-) --- linux-2.6.22-rc4-mm2/drivers/ata/sata_nv.c.old 2007-06-13 00:02:18.0 +0200 +++ linux-2.6.22-rc4-mm2/drivers/ata/sata_nv.c 2007-06-13 00:03:12.0 +0200 @@ -314,7 +314,7 @@ static int nv_ncqintr_dmasetupfis(struct ata_port *ap); static void ncq_clear_singlefis(struct ata_port *ap, u32 val); static u32 ncq_ownfisintr_value (struct ata_port *ap); -void ncq_hotplug(struct ata_port *ap, u32 fis); +static void ncq_hotplug(struct ata_port *ap, u32 fis); static irqreturn_t nv_mcp55_interrupt(int irq, void *dev_instance); static int ncq_interrupt(struct ata_port *ap, u32 fis); static int nv_scsi_queuecmd(struct scsi_cmnd *cmd, @@ -1931,7 +1931,7 @@ ncq_clear(ap); } -int nv_std_prereset(struct ata_port *ap, unsigned long deadline) +static int nv_std_prereset(struct ata_port *ap, unsigned long deadline) { struct ata_eh_context *ehc = &ap->eh_context; @@ -2265,7 +2265,7 @@ return 0; } -u32 ncq_valid_dhfisflag(struct nv_port_priv *pp) +static u32 ncq_valid_dhfisflag(struct nv_port_priv *pp) { u32 valid = (pp->dhfis_flags == pp->qc_active); @@ -2332,7 +2332,7 @@ #endif -void ncq_hotplug(struct ata_port *ap, u32 fis) +static void ncq_hotplug(struct ata_port *ap, u32 fis) { u32 serror; struct ata_eh_info *ehi = &ap->eh_info; - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] Make the IDE DMA timeout modifiable
Bartlomiej Zolnierkiewicz wrote: On Feb 20, 2007, at 5:44 PM, Bartlomiej Zolnierkiewicz wrote: On Wednesday 21 February 2007 02:19, Suleiman Souhlal wrote: It can be changed via /proc/ide/hd?/settings. Why do we need to change IDE DMA timeout dynamically? I've used it to test error recovery (for example). Seems quite useable for developers but I would prefer not to expose it in production kernels for end users. It seems that I have counter example of a customer asking if this timeout can be done configurable. :-) BTW, why the timeout is so damn long? 2*WAIT_CMD is 20 secs, and if DMA is not complete or interrupt pending, it may wait 10 more secs... Thanks, Bart MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PROBLEM + PATCH] Sata port disabled by BIOS gets initialized
Tejun Heo wrote: > Hello, > >> If the user decides to disable the port through the BIOS, the driver >> needs to respect the user's wish to not use the port and carry on. >> Here the end result is a forceful reinitialization of the port by the >> driver against the user's wishes. > > Well, currently, the Linux driver policy is to exploit the hardware > capability to the maximum - e.g. we unlock HPA unconditionally and force > multi-mode controllers into its best possible mode. We try hard to > ignore BIOS imposed settings/limits. Isn't there a case for speeding up boot and not wasting resources by respecting BIOS settings in this regard? If you have an 8-port controller on a board and one disk, forcing all of them enabled regardless of BIOS settings is just 7 redundant port scans. It should at least be an option - the default being to open up all gunports, an option to respect BIOS settings and only use the ones requested and enabled.. (I can see an edge case where a user disables a disk in the BIOS to stop another OS from looking at/for it, but wants the Linux system to boot from it) -- Matt Sealey <[EMAIL PROTECTED]> Genesi, Manager, Developer Relations - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2b/3] Expose Power Management Policy option to users
Enable link power management for ata drivers libata drivers can define a function (enable_pm) that will perform hardware specific actions to enable whatever power management policy the user set up from the scsi sysfs interface if the driver supports it. This power management policy will be activated after all disks have been enumerated and intialized. Signed-off-by: Kristen Carlson Accardi <[EMAIL PROTECTED]> Index: 2.6-git/drivers/ata/libata-scsi.c === --- 2.6-git.orig/drivers/ata/libata-scsi.c +++ 2.6-git/drivers/ata/libata-scsi.c @@ -2890,6 +2890,51 @@ void ata_scsi_simulate(struct ata_device } } +int ata_scsi_set_link_pm_policy(struct Scsi_Host *shost, + enum scsi_host_link_pm policy) +{ + struct ata_port *ap = ata_shost_to_port(shost); + int rc = -EINVAL; + int i; + + /* +* make sure no broken devices are on this port, +* and that all devices support interface power +* management +*/ + for (i = 0; i < ATA_MAX_DEVICES; i++) { + struct ata_device *dev = &ap->device[i]; + + /* only check drives which exist */ + if (!ata_dev_enabled(dev)) + continue; + + /* +* do we need to handle the case where we've hotplugged +* a broken drive (since hotplug and ALPM are mutually +* exclusive) ? +* +* If so, if we detect a broken drive on a port with +* alpm already enabled, then we should reset the policy +* to off for the entire port. +*/ + if ((dev->horkage & ATA_HORKAGE_ALPM) || + !(dev->flags & ATA_DFLAG_IPM)) { + ata_dev_printk(dev, KERN_ERR, + "Unable to set Link PM policy\n"); + ap->pm_policy = SHOST_MAX_PERFORMANCE; + } + } + + if (ap->ops->enable_pm) + rc = ap->ops->enable_pm(ap, policy); + + if (!rc) + shost->shost_link_pm_policy = ap->pm_policy; + return rc; +} +EXPORT_SYMBOL_GPL(ata_scsi_set_link_pm_policy); + int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht) { int i, rc; @@ -2912,7 +2957,7 @@ int ata_scsi_add_hosts(struct ata_host * shost->max_lun = 1; shost->max_channel = 1; shost->max_cmd_len = 16; - + shost->shost_link_pm_policy = ap->pm_policy; rc = scsi_add_host(ap->scsi_host, ap->host->dev); if (rc) goto err_add; Index: 2.6-git/include/linux/libata.h === --- 2.6-git.orig/include/linux/libata.h +++ 2.6-git/include/linux/libata.h @@ -136,6 +136,7 @@ enum { ATA_DFLAG_CDB_INTR = (1 << 2), /* device asserts INTRQ when ready for CDB */ ATA_DFLAG_NCQ = (1 << 3), /* device supports NCQ */ ATA_DFLAG_FLUSH_EXT = (1 << 4), /* do FLUSH_EXT instead of FLUSH */ + ATA_DFLAG_IPM = (1 << 6), /* device supports interface PM */ ATA_DFLAG_CFG_MASK = (1 << 8) - 1, ATA_DFLAG_PIO = (1 << 8), /* device limited to PIO mode */ @@ -300,6 +301,7 @@ enum { ATA_HORKAGE_NONCQ = (1 << 2), /* Don't use NCQ */ ATA_HORKAGE_MAX_SEC_128 = (1 << 3), /* Limit max sects to 128 */ ATA_HORKAGE_DMA_RW_ONLY = (1 << 4), /* ATAPI DMA for RW only */ + ATA_HORKAGE_ALPM= (1 << 5), /* ALPM problems */ }; enum hsm_task_states { @@ -548,6 +550,7 @@ struct ata_port { pm_message_tpm_mesg; int *pm_result; + enum scsi_host_link_pm pm_policy; void*private_data; @@ -607,7 +610,7 @@ struct ata_port_operations { int (*port_suspend) (struct ata_port *ap, pm_message_t mesg); int (*port_resume) (struct ata_port *ap); - + int (*enable_pm) (struct ata_port *ap, enum scsi_host_link_pm policy); int (*port_start) (struct ata_port *ap); void (*port_stop) (struct ata_port *ap); @@ -812,7 +815,7 @@ extern int ata_cable_40wire(struct ata_p extern int ata_cable_80wire(struct ata_port *ap); extern int ata_cable_sata(struct ata_port *ap); extern int ata_cable_unknown(struct ata_port *ap); - +extern int ata_scsi_set_link_pm_policy(struct Scsi_Host *shost, enum scsi_host_link_pm); /* * Timing helpers */ Index: 2.6-git/drivers/ata/libata-core.c === --- 2.6-git.orig/drivers/ata/libata-core.c +++ 2.6-git/drivers/ata/libata-core.c @@ -2019,6 +2019,9 @@ int ata_dev_configure(struct ata_device if (dev->flags & ATA_DFLAG_LBA48) dev->max_sectors = ATA_MAX_SEC
Re: [patch 2a/3] Expose Power Management Policy option to users
Expose Power Management Policy option to users This patch will modify the scsi subsystem to allow users to set a power management policy for the link. The scsi subsystem will create a new sysfs file for each host in /sys/class/scsi_host called "link_power_management_policy". This file can have 3 possible values: Value Meaning --- min_power User wishes the link to conserve power as much as possible, even at the cost of some performance max_performance User wants priority to be on performance, not power savings medium_powerUser wants power savings, with less performance cost than min_power (but less power savings as well). Signed-off-by: Kristen Carlson Accardi <[EMAIL PROTECTED]> Index: 2.6-git/drivers/scsi/hosts.c === --- 2.6-git.orig/drivers/scsi/hosts.c +++ 2.6-git/drivers/scsi/hosts.c @@ -54,6 +54,25 @@ static struct class shost_class = { }; /** + * scsi_host_set_link_pm - Change the link power management policy + * @shost: scsi host to change the policy of. + * @policy:policy to change to. + * + * Returns zero if successful or an error if the requested + * transition is illegal. + **/ +int scsi_host_set_link_pm(struct Scsi_Host *shost, + enum scsi_host_link_pm policy) +{ + struct scsi_host_template *sht = shost->hostt; + if (sht->set_link_pm_policy) + sht->set_link_pm_policy(shost, policy); + + return 0; +} +EXPORT_SYMBOL_GPL(scsi_host_set_link_pm); + +/** * scsi_host_set_state - Take the given host through the host * state model. * @shost: scsi host to change the state of. Index: 2.6-git/drivers/scsi/scsi_sysfs.c === --- 2.6-git.orig/drivers/scsi/scsi_sysfs.c +++ 2.6-git/drivers/scsi/scsi_sysfs.c @@ -189,6 +189,74 @@ show_shost_state(struct class_device *cl static CLASS_DEVICE_ATTR(state, S_IRUGO | S_IWUSR, show_shost_state, store_shost_state); +static const struct { + enum scsi_host_link_pm value; + char*name; +} shost_link_pm_policy[] = { + { SHOST_NOT_AVAILABLE, "max_performance" }, + { SHOST_MIN_POWER, "min_power" }, + { SHOST_MAX_PERFORMANCE, "max_performance" }, + { SHOST_MEDIUM_POWER, "medium_power" }, +}; + +const char *scsi_host_link_pm_policy(enum scsi_host_link_pm policy) +{ + int i; + char *name = NULL; + + for (i = 0; i < ARRAY_SIZE(shost_link_pm_policy); i++) { + if (shost_link_pm_policy[i].value == policy) { + name = shost_link_pm_policy[i].name; + break; + } + } + return name; +} + +static ssize_t store_link_pm_policy(struct class_device *class_dev, + const char *buf, size_t count) +{ + struct Scsi_Host *shost = class_to_shost(class_dev); + enum scsi_host_link_pm policy = 0; + int i; + + /* +* we are skipping array location 0 on purpose - this +* is because a value of SHOST_NOT_AVAILABLE is displayed +* to the user as max_performance, but when the user +* writes "max_performance", they actually want the +* value to match SHOST_MAX_PERFORMANCE. +*/ + for (i = 1; i < ARRAY_SIZE(shost_link_pm_policy); i++) { + const int len = strlen(shost_link_pm_policy[i].name); + if (strncmp(shost_link_pm_policy[i].name, buf, len) == 0 && + buf[len] == '\n') { + policy = shost_link_pm_policy[i].value; + break; + } + } + if (!policy) + return -EINVAL; + + if (scsi_host_set_link_pm(shost, policy)) + return -EINVAL; + return count; +} +static ssize_t +show_link_pm_policy(struct class_device *class_dev, char *buf) +{ + struct Scsi_Host *shost = class_to_shost(class_dev); + const char *policy = + scsi_host_link_pm_policy(shost->shost_link_pm_policy); + + if (!policy) + return -EINVAL; + + return snprintf(buf, 23, "%s\n", policy); +} +static CLASS_DEVICE_ATTR(link_power_management_policy, S_IRUGO | S_IWUSR, + show_link_pm_policy, store_link_pm_policy); + shost_rd_attr(unique_id, "%u\n"); shost_rd_attr(host_busy, "%hu\n"); shost_rd_attr(cmd_per_lun, "%hd\n"); @@ -207,6 +275,7 @@ static struct class_device_attribute *sc &class_device_attr_proc_name, &class_device_attr_scan, &class_device_attr_state, + &class_device_attr_link_power_management_policy, NULL }; Index: 2.6-git/include/scsi/scsi_host.h === --- 2.6-git.orig/include/scsi/scsi_host.h +++ 2.6-git/include/scsi/scsi_host.h @
Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.
On Tue, 12 Jun 2007 11:46:56 -0400 Jeff Garzik <[EMAIL PROTECTED]> wrote: > Matthew Garrett wrote: > > On Tue, Jun 12, 2007 at 11:17:14AM -0300, Henrique de Moraes Holschuh wrote: > >> On Tue, 12 Jun 2007, Matthew Garrett wrote: > >>> Laptop bays are designed to deal with hotplugging PATA - I don't think > >>> this is too much of an issue :) > >> The new SATA ones use the SATA hardware hotplug ;-) Just like the pci-e > >> cards use usb2.0 and pci-e hotplug... > > > > Yes, but they'll also send an ACPI interrupt even if the SATA host > > controller doesn't - it's part of the spec for bays. > > Regardless, having a laptop does not imply having a docking bay. > > Jeff > For bay devices, we can use ACPI just like we do now. For non-bay devices, we can implement hotplug via polling when ALPM is enabled. In my experience most laptop vendors implement extra drive as either PATA in a dock station, USB in a dock station, or a bay device either on the dock station, or on the laptop itself. Kristen - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.
Matthew Garrett wrote: Excluding the corner case of an Expresscard SATA controller (where I suspect you'd want different policy), I doubt there are any cases where you have a laptop with hotplug capabilities without it being implemented as an ACPI bay. Cardbus card. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata passthru: support PIO multi commands
> ata_scsi_pass_thru() is not executed at ioctl submission time (block > queue submission time), but rather immediately before it is issued to > the drive. At that point you know the bus is idle, all other commands > have finished executing, and dev->multi_count is fresh not stale. The > code path goes from ata_scsi_pass_thru() to ata_qc_issue() without > releasing the spinlock, even. Think up to user space Poorusersappset multicount to 4 Evilproprietarytuningdaemon set multicount to 8 Poorusersappissue I/O at which point an error is indeed best. > But the last point is true -- we should error rather than just warn > there, AFAICS. Definitely. We've been asked "please do something stupid" and not even in a case where the requester may know better. Alan - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 0/3] AHCI Link Power Management
On Tue, 12 Jun 2007 13:40:15 +0900 Tejun Heo <[EMAIL PROTECTED]> wrote: > I don't think the end result will vary in any significant way. My > biggest argument for sw implementation is it can be used for other > controllers. What I had in mind when I created the new port operation "enable_pm" was that other controllers (besides the ahci controller) could define their own method of enabling power management. Maybe for non-ahci controllers this is a software based solution which uses generic SATA dipm/hipm stuff and polling. See patch 2/3 of this series for the implementation of this. Kristen - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.
On Tue, Jun 12, 2007 at 11:46:56AM -0400, Jeff Garzik wrote: > Matthew Garrett wrote: > >On Tue, Jun 12, 2007 at 11:17:14AM -0300, Henrique de Moraes Holschuh > >wrote: > >>On Tue, 12 Jun 2007, Matthew Garrett wrote: > >>>Laptop bays are designed to deal with hotplugging PATA - I don't think > >>>this is too much of an issue :) > >>The new SATA ones use the SATA hardware hotplug ;-) Just like the pci-e > >>cards use usb2.0 and pci-e hotplug... > > > >Yes, but they'll also send an ACPI interrupt even if the SATA host > >controller doesn't - it's part of the spec for bays. > > Regardless, having a laptop does not imply having a docking bay. Excluding the corner case of an Expresscard SATA controller (where I suspect you'd want different policy), I doubt there are any cases where you have a laptop with hotplug capabilities without it being implemented as an ACPI bay. -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.
On Wed, Jun 13, 2007 at 12:45:21AM +0900, Tejun Heo wrote: > Matthew Garrett wrote: > > Yes, but they'll also send an ACPI interrupt even if the SATA host > > controller doesn't - it's part of the spec for bays. > > Does the spec mandate that the ACPI interrupt shouldn't depend on SATA > phy status? I don't think vendors are likely to implement separate > mechanism when SATA phy status can do the job fine. I suspect that the spec allows them to do that, but think that it's unlikely to actually happen in most cases. Bear in mind that Windows doesn't tend to drive the hardware in AHCI mode, and that their implementation is likely to be very similar to the implementation they used for PATA devices. -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.
Matthew Garrett wrote: On Tue, Jun 12, 2007 at 11:17:14AM -0300, Henrique de Moraes Holschuh wrote: On Tue, 12 Jun 2007, Matthew Garrett wrote: Laptop bays are designed to deal with hotplugging PATA - I don't think this is too much of an issue :) The new SATA ones use the SATA hardware hotplug ;-) Just like the pci-e cards use usb2.0 and pci-e hotplug... Yes, but they'll also send an ACPI interrupt even if the SATA host controller doesn't - it's part of the spec for bays. Regardless, having a laptop does not imply having a docking bay. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 0/3] AHCI Link Power Management
On Tue, 12 Jun 2007 00:43:12 -0400 Jeff Garzik <[EMAIL PROTECTED]> wrote: > We will do AHCI link PM -- presuming that I can be convinced that it > does not repeatedly park the hard drive heads, or something similarly > annoying on PATA<->SATA bridges and similar setups. > > IF it works as advertised -- a big if considering all the AHCI silicon > implementations out there -- we definitely want to use it. > > Jeff > I understand that this is a concern of yours based on some experience you had with earlier controllers. In general, this behavior would be considered incorrect - link power management should not translate to disk parking, even on PATA->SATA brigdes, and if it does, then that's completely broken. That said, I would believe you if you said broken hardware exists, and when you get specific examples of it, you can add it to the blacklist for this feature. - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.
Matthew Garrett wrote: > On Tue, Jun 12, 2007 at 11:17:14AM -0300, Henrique de Moraes Holschuh wrote: >> On Tue, 12 Jun 2007, Matthew Garrett wrote: >>> Laptop bays are designed to deal with hotplugging PATA - I don't think >>> this is too much of an issue :) >> The new SATA ones use the SATA hardware hotplug ;-) Just like the pci-e >> cards use usb2.0 and pci-e hotplug... > > Yes, but they'll also send an ACPI interrupt even if the SATA host > controller doesn't - it's part of the spec for bays. Does the spec mandate that the ACPI interrupt shouldn't depend on SATA phy status? I don't think vendors are likely to implement separate mechanism when SATA phy status can do the job fine. -- tejun - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.
On Tue, Jun 12, 2007 at 11:17:14AM -0300, Henrique de Moraes Holschuh wrote: > On Tue, 12 Jun 2007, Matthew Garrett wrote: > > Laptop bays are designed to deal with hotplugging PATA - I don't think > > this is too much of an issue :) > > The new SATA ones use the SATA hardware hotplug ;-) Just like the pci-e > cards use usb2.0 and pci-e hotplug... Yes, but they'll also send an ACPI interrupt even if the SATA host controller doesn't - it's part of the spec for bays. -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata passthru: support PIO multi commands
Alan Cox wrote: + if (multi_count != dev->multi_count) + ata_dev_printk(dev, KERN_WARNING, + "invalid multi_count %u ignored\n", + multi_count); + } What limits log spamming here ? Intelligence of the user with privs? Not ever SG_IO call requires priviledges. All calls relevant to this thread require CAP_SYS_RAWIO. Issue a multi count sized write. Acidentally collide with another program which mucks up the multiwrite count (for once it probably won't be HAL making a mess ;)) and send data. The state of the disk at the point a short multiwrite terminates is undefined, it may have written some of the sectors, it may have blanked them all (Optical media) etc. Far better just to error the invalid command - especially as in some cases without Mark's fixes for DRQ cleanup on error it may even hang in one direction. ata_scsi_pass_thru() is not executed at ioctl submission time (block queue submission time), but rather immediately before it is issued to the drive. At that point you know the bus is idle, all other commands have finished executing, and dev->multi_count is fresh not stale. The code path goes from ata_scsi_pass_thru() to ata_qc_issue() without releasing the spinlock, even. But the last point is true -- we should error rather than just warn there, AFAICS. Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.
On Tue, 12 Jun 2007, Matthew Garrett wrote: > On Tue, Jun 12, 2007 at 09:18:19AM -0300, Henrique de Moraes Holschuh wrote: > > On Tue, 12 Jun 2007, Matthew Garrett wrote: > > > > > > On laptops, I suspect that we'll probably get an ACPI interrupt even if > > > the AHCI hotplug pathway can't manage. > > > > As long as we don't crash the drive or AHCI controller because we hotplugged > > it in a way it didn't like (like trying to hotplug a ICH5R would cause). > > Laptop bays are designed to deal with hotplugging PATA - I don't think > this is too much of an issue :) The new SATA ones use the SATA hardware hotplug ;-) Just like the pci-e cards use usb2.0 and pci-e hotplug... -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.
On Tue, Jun 12, 2007 at 09:18:19AM -0300, Henrique de Moraes Holschuh wrote: > On Tue, 12 Jun 2007, Matthew Garrett wrote: > > > > On laptops, I suspect that we'll probably get an ACPI interrupt even if > > the AHCI hotplug pathway can't manage. > > As long as we don't crash the drive or AHCI controller because we hotplugged > it in a way it didn't like (like trying to hotplug a ICH5R would cause). Laptop bays are designed to deal with hotplugging PATA - I don't think this is too much of an issue :) -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ide: convert ide_find_best_mode() users to use ide_max_dma_mode()
Bartlomiej Zolnierkiewicz wrote: ide-timing.h: * remove handling of DMA modes from ide_find_best_mode() and rename it to ide_find_best_pio_mode() * drop no longer needed "map" argument from ide_find_best_pio_mode() and delete needless ->id check * remove no longer needed XFER_SWDMA and XFER_UDMA* defines au1xxx-ide.c: * use ide_max_dma_mode() instead of ide_find_best_mode() * remove needless CONFIG_BLK_DEV_IDE_AU1XXX_MDMA2_DBDMA #ifdef amd74xx.c: * store UDMA masks in amd_ide_chip[] and while at it make "base" field to be u8 instead of unsigned long * convert the driver to use UDMA masks from amd_ide_chip[] * use ide_max_dma_mode() and ide_find_best_pio_mode() instead of ide_find_best_mode() * delete stale comment from amd74xx_ide_dma_check() * remove no longer needed AMD_UDMA* defines via82cxxx.c: * remove unused DISPLAY_VIA_TIMINGS define * store UDMA masks in via_isa_bridges[] and while at it make "flags" field to be u8 instead of u16 * convert the driver to use UDMA masks from via_isa_bridges[] * use ide_max_dma_mode() and ide_find_best_pio_mode() instead of ide_find_best_mode() * remove no longer needed VIA_UDMA* defines pmac.c: * use ide_max_dma_mode() instead of ide_find_best_mode() There should be no functionality changes caused by this patch. I'm afraid this hasn't been accomplished yet. ;-) Signed-off-by: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]> Index: b/drivers/ide/mips/au1xxx-ide.c === --- a/drivers/ide/mips/au1xxx-ide.c +++ b/drivers/ide/mips/au1xxx-ide.c @@ -381,9 +381,7 @@ static int auide_dma_setup(ide_drive_t * static int auide_dma_check(ide_drive_t *drive) { - u8 speed; - -#ifdef CONFIG_BLK_DEV_IDE_AU1XXX_MDMA2_DBDMA + u8 speed = ide_max_dma_mode(drive); if( dbdma_init_done == 0 ){ Ugh, coding style isnn't kosher here. :-) auide_hwif.white_list = ide_in_drive_list(drive->id, @@ -394,7 +392,6 @@ static int auide_dma_check(ide_drive_t * auide_ddma_init(&auide_hwif); dbdma_init_done = 1; } -#endif /* Is the drive in our DMA black list? */ @@ -409,8 +406,6 @@ static int auide_dma_check(ide_drive_t * else drive->using_dma = 1; - speed = ide_find_best_mode(drive, XFER_PIO | XFER_MWDMA); - if (drive->autodma && (speed & XFER_MODE) != XFER_PIO) ide_max_dma_mode() returns 0 if DMA not available, so that if should have looked this way: if (drive->autodma && speed) return 0; But is this really equivalent? Why there's no fallback call to ide_find_best_pio_mode() like in other cases? MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.
On Tue, 12 Jun 2007, Matthew Garrett wrote: > On Mon, Jun 11, 2007 at 08:59:46PM -0700, Arjan van de Ven wrote: > > that's a temporary shortcoming; even with these power savings you can > > do hotplug as long as you're willing to poll for it at a reasonable > > interval and are willing to wait the time between polls for a hotplug > > to take effect.. > > On laptops, I suspect that we'll probably get an ACPI interrupt even if > the AHCI hotplug pathway can't manage. As long as we don't crash the drive or AHCI controller because we hotplugged it in a way it didn't like (like trying to hotplug a ICH5R would cause). It is not like 1Hz would not be fast enough for laptop bays and docks, so the polling frequency would not be a problem. I still prefer when levels and on/off are kept separate, but if it won't stand in the way of hotplug, I certainly don't care enough to bother. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.
On Mon, Jun 11, 2007 at 08:59:46PM -0700, Arjan van de Ven wrote: > that's a temporary shortcoming; even with these power savings you can > do hotplug as long as you're willing to poll for it at a reasonable > interval and are willing to wait the time between polls for a hotplug > to take effect.. On laptops, I suspect that we'll probably get an ACPI interrupt even if the AHCI hotplug pathway can't manage. -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html