[PATCH 11/13] libata: remove unused functions

[Tejun Heo]

Now that all LLDs are converted to use devres, default stop callbacks
are unused.  Remove them.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/libata-core.c |   81 +++-
 include/linux/libata.h|4 --
 2 files changed, 6 insertions(+), 79 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index e476574..b5538dd 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5515,31 +5515,6 @@ int ata_port_start(struct ata_port *ap)
 }
 
 /**
- * ata_port_stop - Undo ata_port_start()
- * @ap: Port to shut down
- *
- * Frees the PRD table.
- *
- * May be used as the port_stop() entry in ata_port_operations.
- *
- * LOCKING:
- * Inherited from caller.
- */
-void ata_port_stop (struct ata_port *ap)
-{
-   struct device *dev = ap->dev;
-
-   dmam_free_coherent(dev, ATA_PRD_TBL_SZ, ap->prd, ap->prd_dma);
-   ata_pad_free(ap, dev);
-}
-
-void ata_host_stop (struct ata_host *host)
-{
-   if (host->mmio_base)
-   iounmap(host->mmio_base);
-}
-
-/**
  * ata_dev_init - Initialize an ata_device structure
  * @dev: Device structure to initialize
  *
@@ -5878,7 +5853,7 @@ int ata_device_add(const struct ata_probe_ent *ent)
}
 
/* resource acquisition complete */
-   devres_close_group(dev, ata_device_add);
+   devres_remove_group(dev, ata_device_add);
 
/* perform each probe synchronously */
DPRINTK("probe begin\n");
@@ -6033,22 +6008,6 @@ void ata_host_detach(struct ata_host *host)
ata_port_detach(host->ports[i]);
 }
 
-/**
- * ata_host_remove - PCI layer callback for device removal
- * @host: ATA host set that was removed
- *
- * Unregister all objects associated with this host set. Free those
- * objects.
- *
- * LOCKING:
- * Inherited from calling layer (may sleep).
- */
-void ata_host_remove(struct ata_host *host)
-{
-   ata_host_detach(host);
-   devres_release_group(host->dev, ata_device_add);
-}
-
 struct ata_probe_ent *
 ata_probe_ent_alloc(struct device *dev, const struct ata_port_info *port)
 {
@@ -6108,26 +6067,13 @@ void ata_std_ports(struct ata_ioports *ioaddr)
 
 #ifdef CONFIG_PCI
 
-void ata_pci_host_stop (struct ata_host *host)
-{
-   struct pci_dev *pdev = to_pci_dev(host->dev);
-
-   /* XXX - the following if can go away once all LLDs are managed */
-   if (!list_empty(&host->dev->devres_head))
-   pcim_iounmap(pdev, host->mmio_base);
-   else
-   pci_iounmap(pdev, host->mmio_base);
-}
-
 /**
  * ata_pci_remove_one - PCI layer callback for device removal
  * @pdev: PCI device that was removed
  *
- * PCI layer indicates to libata via this hook that
- * hot-unplug or module unload event has occurred.
- * Handle this by unregistering all objects associated
- * with this PCI device.  Free those objects.  Then finally
- * release PCI resources and disable device.
+ * PCI layer indicates to libata via this hook that hot-unplug or
+ * module unload event has occurred.  Detach all ports.  Resource
+ * release is handled via devres.
  *
  * LOCKING:
  * Inherited from PCI layer (may sleep).
@@ -6137,14 +6083,7 @@ void ata_pci_remove_one(struct pci_dev *pdev)
struct device *dev = pci_dev_to_dev(pdev);
struct ata_host *host = dev_get_drvdata(dev);
 
-   /* XXX - the following if can go away once all LLDs are managed */
-   if (!list_empty(&host->dev->devres_head)) {
-   ata_host_remove(host);
-   pci_release_regions(pdev);
-   pci_disable_device(pdev);
-   dev_set_drvdata(dev, NULL);
-   } else
-   ata_host_detach(host);
+   ata_host_detach(host);
 }
 
 /* move to PCI subsystem */
@@ -6198,11 +6137,7 @@ int ata_pci_device_do_resume(struct pci_dev *pdev)
pci_set_power_state(pdev, PCI_D0);
pci_restore_state(pdev);
 
-   /* XXX - the following if can go away once all LLDs are managed */
-   if (!list_empty(&pdev->dev.devres_head))
-   rc = pcim_enable_device(pdev);
-   else
-   rc = pci_enable_device(pdev);
+   rc = pcim_enable_device(pdev);
if (rc) {
dev_printk(KERN_ERR, &pdev->dev,
   "failed to enable device after resume (%d)\n", rc);
@@ -6382,7 +6317,6 @@ EXPORT_SYMBOL_GPL(ata_std_ports);
 EXPORT_SYMBOL_GPL(ata_host_init);
 EXPORT_SYMBOL_GPL(ata_device_add);
 EXPORT_SYMBOL_GPL(ata_host_detach);
-EXPORT_SYMBOL_GPL(ata_host_remove);
 EXPORT_SYMBOL_GPL(ata_sg_init);
 EXPORT_SYMBOL_GPL(ata_sg_init_one);
 EXPORT_SYMBOL_GPL(ata_hsm_move);
@@ -6399,8 +6333,6 @@ EXPORT_SYMBOL_GPL(ata_check_status);
 EXPORT_SYMBOL_GPL(ata_altstatus);
 EXPORT_SYMBOL_GPL(ata_exec_command);
 EXPORT_SYMBOL_GPL(ata_port_start)

Re: [2.6.18,19] SATA boot problems (ICH6/ICH6W)

[Tejun Heo]

Kovid Goyal wrote:
> ata3: SATA max UDMA/133 cmd 0xFE00 ctl 0xFE12 bmdma 0xFEA0 irq 17
> ata4: SATA max UDMA/133 cmd 0xFE20 ctl 0xFE32 bmdma 0xFEA8 irq 17
> scsi2 : ata_piix
> scsi3 : ata_piix

Hmmm... ata3.00 was /dev/sda1.  Presence testing seems to have failed.
Please give a shot at 2.6.20-rc4.  We dropped whole presence detection
thing and went back to polling IDENTIFY and that fixed a lot of
detection bugs.  I think your problem is one of those.

Thanks.

-- 
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: ICH6-M libata disk timeouts 2.6.18 -> 2.6.19

[Tejun Heo]

Mike Accetta wrote:
> Here's a bit more information on these timeouts.  I noticed a mention
> of changing the command queue depth in a recent lkml post and decided
> to give that a whirl.
> 
> This problem seems to be related to the depth of the queue.  When I
> set the value for /sys/block/sdb/device/queue_depth to 4 the raid1
> re-sync completes without incident.  When set to 5 it eventually gives
> the timeout error message (although at well past 50% of the re-sync).
> The problem was originally observed with the default setting (31).

Please post the result of 'hdparm -I /dev/sdb'.  Thanks.

-- 
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: ahci_softreset prevents acpi_power_off

[Tejun Heo]

Hello,

Faik Uygur wrote:
> We have a Sony PCG-6H1M laptop. It started failing to poweroff with our 
> switch 
> from 2.6.16 stable series kernels to 2.6.18 stable series. Rebooting works.
> 
> While searching for the cause, I have found these reported bug reports in the 
> kernel bugzilla which may be related to this bug:
> 
> http://bugzilla.kernel.org/show_bug.cgi?id=6982
> http://bugzilla.kernel.org/show_bug.cgi?id=7447

Seems mostly unrelated.

> According to git bisect, this is the first bad commit:
> 
> 4658f79bec0b51222e769e328c2923f39f3bda77 is first bad commit
> commit 4658f79bec0b51222e769e328c2923f39f3bda77
> Author: Tejun Heo <[EMAIL PROTECTED]>
> Date:   Wed Mar 22 21:07:03 2006 +0900
> 
> [PATCH] ahci: add softreset
> 
> Now that libata is smart enought to handle both soft and hard resets,
> add softreset method.
> 
> Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
> Signed-off-by: Jeff Garzik <[EMAIL PROTECTED]>
> 
> :04 04 ba0a16d0ef82b6577bb61cfb18e6d9df9ee0984e 
> d0fc78d8f9bbe238f98ac8964562a33e64b30605 M  drivers
> 
> With v2.6.20-rc4 from git, it is still failing to poweroff. By not compiling 
> CONFIG_SCSI_SATA_AHCI, it successfully powers off.
> 
> Also with CONFIG_SCSI_SATA_AHCI, reverting this patch manually by setting 
> softreset to NULL in ata_do_eh calls in ahci.c makes the machine poweroff.

Wow, this is one of the most amazing error report.  ahci softreset
preventing system halt?

> I have attached the dmesg output with defined ATA_DEBUG, ATA_VERBOSE_DEBUG
> if it helps. Also you may find lspci output attached. 
> 
> Please let me know if anything else is needed.

Does everything else work okay?  Can you access devices attached to
ahci?  What happens when you try to shutdown?  If possible, please post
dmesg of shutting down.  You can store it easily using netconsole
(Documentation/networking/netconsole.txt).

Thanks.

-- 
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: Proposed changes for libata speed handling

[Tejun Heo]

Alan wrote:
> I'm currently hacking on the speed handling code a bit
> 
> I'd like to do the following unless anyone has any objections
> 
> - Remove post_set_mode and make drivers wrap the guts of the existing
> set_mode() function. This allows a driver to wrap and see success/failure
> while removing a callback, and also to add pre-mode code. (ie you'd do
> 
> foo_set_mode() {
> ata_default_set_mode()
> my_fiddling();
> }
> 
> - Fix the ->set_mode method FIXMEs in the current tree [DONE]
> 
> - Add set_specific_mode, with a default behaviour that works for most
> controllers. Those using a private ->set_mode might need a private
> ->set_specific_mode, in some cases like it8212 simply to error the request
> 
> - Hook set_specific_mode to the ata command parser so that instead of
> erroring set_features commands we snoop them and force the mode change
> desired on the controller (if valid)
> 
> - Send the command to set the speed before setting the controller speed,
> so that we send them at the right rate.
> 
> Any comments ?

Wouldn't it be better to have ->determine_xfer_mask() and
->set_specific_mode() than having two somewhat overlapping callbacks?
Or is there some problem that can't be handled that way?

Thanks.

-- 
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: Proposed changes for libata speed handling

[Tejun Heo]

Alan wrote:
> O> Wouldn't it be better to have ->determine_xfer_mask() and
>> ->set_specific_mode() than having two somewhat overlapping callbacks?
>> Or is there some problem that can't be handled that way?
> 
> I'm not sure I follow what you are suggesting - can you explain further.
> 
> Right now ->set_mode does all the policy management with regards to
> picking the right modes which is sometimes done by the usual ATA rules
> and sometimes by card specific code.
> 
> ->set_specific_mode does no policy work but merely sets up a mode.
> 
> The default behaviour of ->set_mode() is the ATA mode selection by best
> mode available, and this function is normally not provided by a driver.
> 
> The default behaviour of ->set_specific_mode() is to verify the mode is
> valid then issue ->set_pio|dma_mode() (for both devices in case a timing
> change on both is triggered). This function is overridable because of
> things like IT821x where the IDE mode is imaginary.

What I was thinking about was something like the following.

* ops

unsigned int (*determine_xfer_mask)(struct ata_device *dev);
int (*set_specific_mode)(struct ata_device *dev,
unsigned int xfer_mode);

* during init and EH

if (init) {
ap->xfer_mask &= ops->determine_xfer_mask(dev);
DETERMINE best_mode;
}

if (ap->ehi.target_mode && valid)
mode = ap->ehi.target_mode;
else
mode = best_mode;

rc = ops->set_specific_mode(dev, mode);

* when the user issues SET_XFERMODE, in the issue path

if (command is SET_XFERMODE) {
if (mode is invalid)
fail;
ap->ehi.target_mode = user_specified_mode;
ata_port_schedule_eh(ap);
done;
}

To sum up,

1. separate supported mode detection and mode programming such that we
can use the same programming path for both init and handling user-issued
SET_XFERMODE.

2. group all mode programming callbacks (->set_piomode, ->set_dmamode
and ->post_set_mode into ->set_specific_mode) into ->set_specific_mode
to allow more flexibility and replace ->set_mode.

3. make sure all xfer mode programming is done in EH.  this will ease
support for weird controllers (e.g. cross-port synchronization).

Thanks.

-- 
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: Norco DS-1220 (sil3726+sil3124) / libata-tj bug report

[Tejun Heo]

Brad Fitzpatrick wrote:
> On Sat, 13 Jan 2007, Brad Fitzpatrick wrote:
> 
>> I tried another disk (but of the same type), and got the same results both
>> direct and via PMP.
>>
>> Are these disks too old to be hot-plugged?  (but I recall the
>> identical errors happening on boot after I power-cycled the enclosure
>> and the host...)
> 
> Conclusion to this story, for posterity:
> 
> Seems to be the disk type.
> 
> I have six Western Digital WD2500JD-00GBB0.  Tried two both direct and
> PMP, no success.
> 
> Went and bought a new Segate ST3500630AS 500GB and it hotplugged both
> direct and PMP without problems.
> 
> So I guess the old WD2500JD-00GBB0 aren't compatible?

I'll have to ask other people about this.  Don't have that particular drive.

-- 
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: ICH7m problem using libata

[Tejun Heo]

Matthew Stapleton wrote:
> Tejun Heo wrote:
>> Can you try the attached patch over 2.6.19 and report full dmesg?  Thanks.
>>
>> -- 
>> tejun
>>
> 
> Attached is my full dmesg.

Does the problem still persist?

-- 
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

PATA ATAPI detection debug

[Tejun Heo]

Hello, all.

Many people have been reporting libata PATA ATAPI detection problem.  In
many but not all cases, the ATAPI device was occupying the slave slot
while a disk drive occupies the master slot.  Based on that and J.
Taimr's nullify freeze on via fix, I made a cocktail patch which
contained four different fixes and it seemed to have fixed the problem
for (at least) several people, but the reports are not all consistent.

The attached patches contain the same four fixes but has a selector
parameter to enable each fix separately.  Both are equivalent but using
2.6.20-rc5 is recommended to rule out detection problems fixed by
polling IDENTIFY.

If your libata driver is compiled into the kernel, add
'libata.debug_cocktail=N' to your kernel parameter.  If you compile
libata.ko as module, add 'debug_cocktail=N' module parameter to the
module parameter.  e.g. 'modprobe libata.ko debug_cocktail=1'.

Please test

0: nothing
1: common PIO mask between devices sharing a channel
2: force PIO0 (DMA mode is unaffected)
4: clear NIEN on both devices
8: make ata_bmdma_freeze() nill

If none of above works, try 5, 9, then 6, 10.

Please test each option and report the result.  It's best if you can
include dmesg's for each value but if you can't get the dmesg because
boot doesn't complete, just reporting the option doesn't work suffices.
 Also, please don't forget to attach the result of 'lspci -nnvvv'.

Thanks for your patience.

-- 
tejun
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 915a55a..0eea6bd 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -90,6 +90,9 @@ static int ata_probe_timeout = ATA_TMOUT_INTERNAL / HZ;
 module_param(ata_probe_timeout, int, 0444);
 MODULE_PARM_DESC(ata_probe_timeout, "Set ATA probing timeout (seconds)");
 
+int ata_debug_cocktail = 0;
+module_param_named(debug_cocktail, ata_debug_cocktail, int, 0444);
+
 MODULE_AUTHOR("Jeff Garzik");
 MODULE_DESCRIPTION("Library module for ATA devices");
 MODULE_LICENSE("GPL");
@@ -2227,6 +2230,10 @@ int ata_set_mode(struct ata_port *ap, struct ata_device **r_failed_dev)
 		pio_mask = ata_pack_xfermask(dev->pio_mask, 0, 0);
 		dma_mask = ata_pack_xfermask(0, dev->mwdma_mask, dev->udma_mask);
 		dev->pio_mode = ata_xfer_mask2mode(pio_mask);
+		if (dev->pio_mode && ata_debug_cocktail & (1 << 1)) {
+			ata_port_printk(ap, KERN_INFO, "XXX force PIO0\n");
+			dev->pio_mode = XFER_PIO_0;
+		}
 		dev->dma_mode = ata_xfer_mask2mode(dma_mask);
 
 		found = 1;
@@ -3124,6 +3131,29 @@ static void ata_dev_xfermask(struct ata_device *dev)
    dev->mwdma_mask, dev->udma_mask);
 	xfer_mask &= ata_id_xfermask(dev->id);
 
+	if (ata_debug_cocktail & (1 << 0)) {
+		int i;
+
+		/* PIO xfermask limits are shared by all devices on the same
+		 * channel to avoid violating device selection timing.
+		 */
+		ata_dev_printk(dev, KERN_INFO, "XXX common PIO mode: pre: %lx\n",
+			   xfer_mask);
+		for (i = 0; i < ATA_MAX_DEVICES; i++) {
+			struct ata_device *d = &ap->device[i];
+			unsigned int pio_mask;
+
+			if (ata_dev_absent(d))
+continue;
+
+			ata_unpack_xfermask(ata_id_xfermask(d->id),
+	&pio_mask, NULL, NULL);
+			pio_mask &= d->pio_mask;
+			xfer_mask &= ata_pack_xfermask(pio_mask, UINT_MAX, UINT_MAX);
+		}
+		ata_dev_printk(dev, KERN_INFO, "XXX common PIO mode: post: %lx\n",
+			   xfer_mask);
+	}
 	/*
 	 *	CFA Advanced TrueIDE timings are not allowed on a shared
 	 *	cable
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 7645f2b..5bb5bd4 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -39,6 +39,51 @@
 #include "libata.h"
 
 /**
+ *	ata_irq_on - Enable interrupts on a port.
+ *	@ap: Port on which interrupts are enabled.
+ *
+ *	Enable interrupts on a legacy IDE device using MMIO or PIO,
+ *	wait for idle, clear any pending interrupts.
+ *
+ *	LOCKING:
+ *	Inherited from caller.
+ */
+
+u8 ata_irq_on(struct ata_port *ap)
+{
+	struct ata_ioports *ioaddr = &ap->ioaddr;
+	u8 tmp;
+
+	ap->ctl &= ~ATA_NIEN;
+	ap->last_ctl = ap->ctl;
+
+	if (ata_debug_cocktail & (1 << 2)) {
+		ata_port_printk(ap, KERN_INFO, "XXX clear NIEN on both devices\n");
+		ap->ops->dev_select(ap, 1);
+	}
+	if (ap->flags & ATA_FLAG_MMIO)
+		writeb(ap->ctl, (void __iomem *) ioaddr->ctl_addr);
+	else
+		outb(ap->ctl, ioaddr->ctl_addr);
+	tmp = ata_wait_idle(ap);
+
+	if (ata_debug_cocktail & (1 << 2)) {
+		ap->ops->dev_select(ap, 0);
+		if (ap->flags & ATA_FLAG_MMIO)
+			writeb(ap->ctl, (void __iomem *) ioaddr->ctl_addr);
+		else
+			outb(ap->ctl, ioaddr->ctl_addr);
+		tmp = ata_wait_idle(ap);
+	}
+
+	ap->ops->irq_clear(ap);
+
+	return tmp;
+}
+
+
+
+/**
  *	ata_tf_load_pio - send taskfile registers to host controller
  *	@ap: Port to which output is sent
  *	@tf: ATA taskfile register set
@@ -664,6 +709,10 @@ void ata_bmdma_freeze(struct ata_port *ap)
 {
 	struct ata_ioports *ioaddr = &ap->ioaddr;
 
+	if (ata_debug_cocktail & (1 << 3)) {
+		ata_port_printk(ap, KERN_INFO, "XXX skip

[PATCH] ahci: improve and limit spurious interrupt messages

[Tejun Heo]

We're still seeing a lot of issues with NCQ implementation in drive
firmwares.  Sprious FISes during NCQ command phase occur on many
drives and some of them seem potentially dangerous (at least to me).
Until we find the solution, spurious messages can give us more info.
Improve and limit them such that more info can be reported while not
disturbing users too much.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 5998f74..00c6bcc 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -75,6 +75,7 @@ enum {
AHCI_CMD_CLR_BUSY   = (1 << 10),
 
RX_FIS_D2H_REG  = 0x40, /* offset of D2H Register FIS data */
+   RX_FIS_SDB  = 0x58, /* offset of SDB FIS data */
RX_FIS_UNK  = 0x60, /* offset of Unknown FIS data */
 
board_ahci  = 0,
@@ -202,6 +203,10 @@ struct ahci_port_priv {
dma_addr_t  cmd_tbl_dma;
void*rx_fis;
dma_addr_t  rx_fis_dma;
+   /* for NCQ spurious interrupt analysis */
+   int ncq_saw_spurious_sdb_cnt;
+   unsigned intncq_saw_d2h:1;
+   unsigned intncq_saw_dmas:1;
 };
 
 static u32 ahci_scr_read (struct ata_port *ap, unsigned int sc_reg);
@@ -1126,6 +1131,7 @@ static void ahci_host_intr(struct ata_port *ap)
void __iomem *mmio = ap->host->mmio_base;
void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
struct ata_eh_info *ehi = &ap->eh_info;
+   struct ahci_port_priv *pp = ap->private_data;
u32 status, qc_active;
int rc;
 
@@ -1154,17 +1160,43 @@ static void ahci_host_intr(struct ata_port *ap)
 
/* hmmm... a spurious interupt */
 
-   /* some devices send D2H reg with I bit set during NCQ command phase */
-   if (ap->sactive && (status & PORT_IRQ_D2H_REG_FIS))
+   /* if !NCQ, ignore.  No modern ATA device has broken HSM
+* implementation for non-NCQ commands.
+*/
+   if (!ap->sactive)
return;
 
-   /* ignore interim PIO setup fis interrupts */
-   if (ata_tag_valid(ap->active_tag) && (status & PORT_IRQ_PIOS_FIS))
-   return;
+   if (status & PORT_IRQ_D2H_REG_FIS) {
+   if (!pp->ncq_saw_d2h)
+   ata_port_printk(ap, KERN_INFO,
+   "D2H reg with I during NCQ, "
+   "this message won't be printed again\n");
+   pp->ncq_saw_d2h = 1;
+   } else if (status & PORT_IRQ_DMAS_FIS) {
+   if (!pp->ncq_saw_dmas)
+   ata_port_printk(ap, KERN_INFO,
+   "DMAS FIS during NCQ, "
+   "this message won't be printed again\n");
+   pp->ncq_saw_dmas = 1;
+   } else if (status & PORT_IRQ_SDB_FIS &&
+  pp->ncq_saw_spurious_sdb_cnt < 10) {
+   /* SDB FIS containing spurious completions might be
+* dangerous, we need to know more about them.  Print
+* more of it.
+*/
+   const u32 *f = pp->rx_fis + RX_FIS_SDB;
+
+   pp->ncq_saw_spurious_sdb_cnt++;
 
-   if (ata_ratelimit())
+   ata_port_printk(ap, KERN_INFO, "Spurious SDB FIS during NCQ "
+   "issue=0x%x SAct=0x%x FIS=%08x:%08x%s\n",
+   readl(port_mmio + PORT_CMD_ISSUE),
+   readl(port_mmio + PORT_SCR_ACT), f[0], f[1],
+   pp->ncq_saw_spurious_sdb_cnt < 10 ?
+   "" : ", shutting up");
+   } else
ata_port_printk(ap, KERN_INFO, "spurious interrupt "
-   "(irq_stat 0x%x active_tag %d sactive 0x%x)\n",
+   "(irq_stat 0x%x active_tag 0x%x sactive 
0x%x)\n",
status, ap->active_tag, ap->sactive);
 }
 
-
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: ahci problems with sata disk.

[Tejun Heo]

kenneth johansson wrote:
> I changed my bios setting for SATA from IDE to AHCI.
> 
> This resulted in some "interesting" read throughput. 
> 
> plots can be found at http://kenjo.org/~ken/sata/
> The plots was done on a live disk so some noise is expected but in the
> ahci mode the throughput get stuck at 17 MB way to much.

It's probably not an ahci problem but more of NCQ implementation problem
in the drive firmware.  Please report the result of 'hdparm -I /dev/sdX'
and try adjust queue depth and see what happens.

http://linux-ata.org/faq.html

-- 
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: ahci problems with sata disk.

[Tejun Heo]

kenneth johansson wrote:
> On Mon, 2007-01-15 at 18:13 +0900, Tejun Heo wrote:
>> kenneth johansson wrote:
>>> I changed my bios setting for SATA from IDE to AHCI.
>>>
>>> This resulted in some "interesting" read throughput. 
>>>
>>> plots can be found at http://kenjo.org/~ken/sata/
>>> The plots was done on a live disk so some noise is expected but in the
>>> ahci mode the throughput get stuck at 17 MB way to much.
>> It's probably not an ahci problem but more of NCQ implementation problem
>> in the drive firmware.  Please report the result of 'hdparm -I /dev/sdX'
>> and try adjust queue depth and see what happens.
>>
>> http://linux-ata.org/faq.html
>>
> 
> It was, when I turn of NCQ with "echo 1
>> /sys/block/sda/device/queue_depth" I get the same performance as when
> the BIOS is set to IDE.

Can you play with queue depth a bit?  e.g. Benchmark queue depth of 4, 8
and 16.

> I though that NCQ was intended to increase performance ??

Supposedly.

> also the disk is a Westen Digital raptor and it's probably the most
> benchmarked drive one could get so I was not expecting a problem with
> the drive. 

Most benchmarked doesn't make the firmware any better, it seems.  The
raptor Alan talked about, reportedly, locks up after hours of NCQ load too.

-- 
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: pata_sis:sg_write errors

[Tejun Heo]

Jordan Neumeyer wrote:
> Well recently I've been using libata since my my distribution offered it when
> they switched to 2.6.19( maybe? 18) in the initramfs image.  I have a sis 5513
> controller, which after a couple of days started acting up and coming up with
> the following error:
> 
> sg_write: data in/out 30576/30576 bytes for SCSI command 0xbe--guessing data 
> i$;
>  program grip not setting count and/or reply_len properly
> printk: 319 messages suppressed.
> sg_write: data in/out 30576/30576 bytes for SCSI command 0xbe--guessing data 
> $n;
>  program grip not setting count and/or reply_len properly
> printk: 321 messages suppressed.
> sg_write: data in/out 16464/16464 bytes for SCSI command 0xbe--guessing data 
> $n;
>  program grip not setting count and/or reply_len properly
> printk: 323 messages suppressed.
> sg_write: data in/out 16464/16464 bytes for SCSI command 0xbe--guessing data 
> $n;
>  program grip not setting count and/or reply_len properly
> printk: 323 messages suppressed.
> sg_write: data in/out 16464/16464 bytes for SCSI command 0xbe--guessing data 
> $n;
>  program grip not setting count and/or reply_len properly
> printk: 324 messages suppressed.
> 
> It's repeated over, and over; differing amounts of bytes.  I'm unsure what
> invoked such errors, because it only started to happen a few days after use of
> the kernel.  Which was 2.6.19-beyond kernel.  I don't believe any additions 
> made
> by the beyond kernel affected the libata system.
> 
> Has this been fixed in the 2.6.20-rcXs?  

This is not a kernel bug.  The SCSI midlayer is bitching that grip
hasn't set certain parameter while using the sg interface (which
previous kernels have ignored).  grip should be updated to use proper
parameter.  So, no, 2.6.20-rcX won't fix it.  You need to update grip.

-- 
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

[PATCH] sata_via: add PCI ID 0x5337

[Tejun Heo]

From: Luca Pedrielli <[EMAIL PROTECTED]>

Add PCI ID 0x5337 to supported PCI ID.  This is VT8237 in IDE mode.

Signed-off-by: Luca Pedrielli <[EMAIL PROTECTED]>
Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---

Luca, I formatted the patch in the form Jeff can take.  Please format
patches like this next time.

This was verified by another bug reporter too.

diff --git a/drivers/ata/sata_via.c b/drivers/ata/sata_via.c
index 1f1d71e..4b24354 100644
--- a/drivers/ata/sata_via.c
+++ b/drivers/ata/sata_via.c
@@ -85,6 +85,7 @@ static void vt6421_set_dma_mode(struct ata_port *ap, struct 
ata_device *adev);
 static int vt6421_port_start(struct ata_port *ap);
 
 static const struct pci_device_id svia_pci_tbl[] = {
+   { PCI_VDEVICE(VIA, 0x5337), vt6420 },
{ PCI_VDEVICE(VIA, 0x0591), vt6420 },
{ PCI_VDEVICE(VIA, 0x3149), vt6420 },
{ PCI_VDEVICE(VIA, 0x3249), vt6421 },
-
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

[PATCH] sata_uli: ignore SIMPLEX

[Tejun Heo]

Some uli controllers have stuck SIMPLEX bit which can't be cleared
with ata_pci_clear_simplex(), but the controller is capable of doing
DMAs on both channels simultaneously.  Ignore it.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>

diff --git a/drivers/ata/sata_uli.c b/drivers/ata/sata_uli.c
index 5c603ca..62b9269 100644
--- a/drivers/ata/sata_uli.c
+++ b/drivers/ata/sata_uli.c
@@ -226,6 +226,13 @@ static int uli_init_one (struct pci_dev *pdev, const 
struct pci_device_id *ent)
 
probe_ent->private_data = hpriv;
 
+   /* these chips have stuck dummy simplex bit, ignore it */
+   if (probe_ent->_host_flags & ATA_HOST_SIMPLEX) {
+   dev_printk(KERN_INFO, &pdev->dev,
+  "BMDMA simplex set, ignored\n");
+   probe_ent->_host_flags &= ~ATA_HOST_SIMPLEX;
+   }
+
switch (board_idx) {
case uli_5287:
hpriv->scr_cfg_addr[0] = ULI5287_BASE;
-
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

[PATCH] ahci: improve and limit spurious interrupt messages, take#2

[Tejun Heo]

We're still seeing a lot of issues with NCQ implementation in drive
firmwares.  Sprious FISes during NCQ command phase occur on many
drives and some of them seem potentially dangerous (at least to me).
Until we find the solution, spurious messages can give us more info.
Improve and limit them such that more info can be reported while not
disturbing users too much.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
Updated to not use bitfields as requested.  Thanks.

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 5998f74..d5ea1c3 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -75,6 +75,7 @@ enum {
AHCI_CMD_CLR_BUSY   = (1 << 10),
 
RX_FIS_D2H_REG  = 0x40, /* offset of D2H Register FIS data */
+   RX_FIS_SDB  = 0x58, /* offset of SDB FIS data */
RX_FIS_UNK  = 0x60, /* offset of Unknown FIS data */
 
board_ahci  = 0,
@@ -202,6 +203,9 @@ struct ahci_port_priv {
dma_addr_t  cmd_tbl_dma;
void*rx_fis;
dma_addr_t  rx_fis_dma;
+   /* for NCQ spurious interrupt analysis */
+   int spurious_sdb_cnt;
+   u32 seen_status;
 };
 
 static u32 ahci_scr_read (struct ata_port *ap, unsigned int sc_reg);
@@ -1126,6 +1130,7 @@ static void ahci_host_intr(struct ata_port *ap)
void __iomem *mmio = ap->host->mmio_base;
void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
struct ata_eh_info *ehi = &ap->eh_info;
+   struct ahci_port_priv *pp = ap->private_data;
u32 status, qc_active;
int rc;
 
@@ -1154,17 +1159,40 @@ static void ahci_host_intr(struct ata_port *ap)
 
/* hmmm... a spurious interupt */
 
-   /* some devices send D2H reg with I bit set during NCQ command phase */
-   if (ap->sactive && (status & PORT_IRQ_D2H_REG_FIS))
+   /* if !NCQ, ignore.  No modern ATA device has broken HSM
+* implementation for non-NCQ commands.
+*/
+   if (!ap->sactive)
return;
 
-   /* ignore interim PIO setup fis interrupts */
-   if (ata_tag_valid(ap->active_tag) && (status & PORT_IRQ_PIOS_FIS))
-   return;
+   if ((status & PORT_IRQ_D2H_REG_FIS) &&
+   !(pp->seen_status & PORT_IRQ_D2H_REG_FIS)) {
+   ata_port_printk(ap, KERN_INFO, "D2H reg with I during NCQ, "
+   "this message won't be printed again\n");
+   pp->seen_status |= PORT_IRQ_D2H_REG_FIS;
+   } else if ((status & PORT_IRQ_DMAS_FIS) &&
+  !(pp->seen_status & PORT_IRQ_DMAS_FIS)) {
+   ata_port_printk(ap, KERN_INFO, "DMAS FIS during NCQ, "
+   "this message won't be printed again\n");
+   pp->seen_status |= PORT_IRQ_DMAS_FIS;
+   } else if (status & PORT_IRQ_SDB_FIS && pp->spurious_sdb_cnt < 10) {
+   /* SDB FIS containing spurious completions might be
+* dangerous, we need to know more about them.  Print
+* more of it.
+*/
+   const u32 *f = pp->rx_fis + RX_FIS_SDB;
+
+   pp->spurious_sdb_cnt++;
 
-   if (ata_ratelimit())
+   ata_port_printk(ap, KERN_INFO, "Spurious SDB FIS during NCQ "
+   "issue=0x%x SAct=0x%x FIS=%08x:%08x%s\n",
+   readl(port_mmio + PORT_CMD_ISSUE),
+   readl(port_mmio + PORT_SCR_ACT), f[0], f[1],
+   pp->spurious_sdb_cnt < 10 ?
+   "" : ", shutting up");
+   } else
ata_port_printk(ap, KERN_INFO, "spurious interrupt "
-   "(irq_stat 0x%x active_tag %d sactive 0x%x)\n",
+   "(irq_stat 0x%x active_tag 0x%x sactive 
0x%x)\n",
status, ap->active_tag, ap->sactive);
 }
 
-
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: ICH7m problem using libata

[Tejun Heo]

Matthew Stapleton wrote:
> Tejun Heo wrote:
>> Does the problem still persist?
>>
> With that kernel and the previous patches it does.  I'll try kernel 
> 2.6.20-rc5 
> and the which-cocktail-2.6.19.patch

I got confused between your report and Jan's.  Yours (not so sure about
Jan's) seems to be drive firmware bug which hald was successful to
undiscover.  Does hald clear CDO_USE_FFLAGS using ioctl
CDROM_CLEAR_OPTIONS?  The failed commands seem to be from cdrom
open_for_data().  Considering general quality of ATAPI devices, I
wouldn't be too surprised if some device fails after hours of repeated
poll sequence containing READ_TOC and ALLOW_MEDIUM_REMOVAL.

I think it's best to contact hal people about your issue.  If/when you
contact hal people, please cc me.

Thanks.

-- 
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: ICH7m problem using libata

[Tejun Heo]

Jan Gutter wrote:
> On Tue, 2007-01-16 at 17:56 +0900, Tejun Heo wrote:
>> Matthew Stapleton wrote:
>>> Tejun Heo wrote:
>>>> Does the problem still persist?
> 
> Sorry for the delayed reply: Holidays attacked before I could apply the
> patch. 
> 
>> I got confused between your report and Jan's.  Yours (not so sure about
>> Jan's) seems to be drive firmware bug which hald was successful to
>> undiscover.  Does hald clear CDO_USE_FFLAGS using ioctl
>> CDROM_CLEAR_OPTIONS?  The failed commands seem to be from cdrom
>> open_for_data().  Considering general quality of ATAPI devices, I
>> wouldn't be too surprised if some device fails after hours of repeated
>> poll sequence containing READ_TOC and ALLOW_MEDIUM_REMOVAL.
> 
> Hmmm. I think I might have slightly different symptoms: Heavy disk use
> definitely causes the bug to appear more frequently. Compiling often
> causes it to kick in after about 15 minutes. I also get slightly
> different error messages.
> 
> With the latest patch set (2.6.19-gentoo-r4 + cocktail) the resets still
> occur, but at least the drive doesn't go down to PIO so soon anymore.
> I'll keep watching it for the rest of the day. Attached dmesg of the
> first hour or so. (If Evolution borks this mail, I'll resort to telnet and 
> SMTP ;-)
> 
> Thanks for the feedback, and hope the sucker gets nailed.

Can you try 2.6.20-rc5?  It has better error reporting and will tell us
which SCSI command is timing out.

-- 
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: pata_sis:sg_write errors

[Tejun Heo]

jnengland77 wrote:
> Hi
> Thank you for your help.  So all I need to do is update the sg3_utils?
> It's the only package I can find dealing with sg and scsi? And do I
> have to set this parameter or is it automatically set?  I've never
> dealt with this scsi system, because all I have are pata disks.  No
> sata nor scsi.

You need to update grip, cd ripper.  And, it's not something a user can
change.  The program itself needs to be updated.  So, bug the grip
developers.  The messages are just notices to let people know that those
programs need to be updated and can be safely ignored for the time being.

-- 
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

[PATCH] libata: initialize qc->dma_dir to DMA_NONE

[Tejun Heo]

libata didn't used to init qc->dma_dir to any specific value on qc
initialization and command translation path didn't set qc->dma_dir if
the command doesn't need data transfer.  This made non-data commands
to have random qc->dma_dir.

This usually doesn't cause problem because LLDs usually check
qc->protocol first and look at qc->dma_dir iff the command needs data
transfer but this doesn't hold for all LLDs.

It might be worthwhile to rename qc->dma_dir to qc->data_dir as we use
the field to tag data direction for both PIO and DMA protocols.

This problem has been spotted by James Bottomley.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
Cc: James Bottomley <[EMAIL PROTECTED]>

diff --git a/include/linux/libata.h b/include/linux/libata.h
index 7cfc18f..925ad7f 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1139,6 +1139,7 @@ static inline void ata_tf_init(struct ata_device *dev, 
struct ata_taskfile *tf)
 
 static inline void ata_qc_reinit(struct ata_queued_cmd *qc)
 {
+   qc->dma_dir = DMA_NONE;
qc->__sg = NULL;
qc->flags = 0;
qc->cursect = qc->cursg = qc->cursg_ofs = 0;
-
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: ICH7m problem using libata

[Tejun Heo]

Jan Gutter wrote:
> ata1.01: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x2 frozen
> ata1.01: cmd a0/01:00:00:00:00/00:00:00:00:00/b0 tag 0 cdb 0x43 data 12 in
>  res 40/00:03:00:00:00/00:00:00:00:00/b0 Emask 0x4 (timeout)

That's READ TOC.  Does the problem happen if you boot into single mode
(no hald)?  Can you access cd/dvd in single mode?

-- 
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: ICH7m problem using libata

[Tejun Heo]

Jan Gutter wrote:
> If it's a firmware bug and implementing a workaround in hald helps it,
> would other userspace apps (k3b, cdrecord, etc.) also be affected?

Quite some number of ODDs don't seem to like the way hald polls them.  I
don't think other apps will be affected by this.  I dunno much about
hald and don't have enough time to chase it down myself but feel free to
cc me when reporting this to hald people.  I'll try to help as much as I
can.

Thanks.

-- 
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 2/3] libata: Initialize nbytes for internal sg commands

[Tejun Heo]

Brian King wrote:
> Some LLDDs, like ipr, use nbytes and pad_len to determine
> the total data transfer length of a command. Make sure
> nbytes gets initialized for internally generated commands.

I think it's better to apply the following patch instead of this one.

http://article.gmane.org/gmane.linux.ide/14792

-- 
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] libata: initialize qc->dma_dir to DMA_NONE

[Tejun Heo]

James Bottomley wrote:
> This looks perfectly fine as a possible solution.  Is there any reason
> not to initialise qc->dma_dir unconditionally from the SCSI command?

That should work too.  I did what I did because it was more in line with
what the current code assumed and initializing the field on qc alloc
seemed like a good idea with or without unconditional qc->dma_dir =
scmd->sc_data_direction because not all commands are translated from
scsi command.

> The only potential problem is DMA_BIDIRECTIONAL, which we don't use
> (yet) ... but if it ever did come down libata will do the wrong thing
> anyway.

If that ever happens, libata probably should emulate it using multiple
commands, I guess.

Thanks.

-- 
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

[PATCH] ide: unregister idepnp driver on unload

[Tejun Heo]

idepnp driver is registered as a pnp driver on ide init but doesn't
get unregistered on ide unload causing driver list corruption and
eventually oops.  Fix it.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
diff --git a/drivers/ide/ide-pnp.c b/drivers/ide/ide-pnp.c
index df7d150..98410ca 100644
--- a/drivers/ide/ide-pnp.c
+++ b/drivers/ide/ide-pnp.c
@@ -73,3 +73,8 @@ void __init pnpide_init(void)
 {
pnp_register_driver(&idepnp_driver);
 }
+
+void __exit pnpide_exit(void)
+{
+   pnp_unregister_driver(&idepnp_driver);
+}
diff --git a/drivers/ide/ide.c b/drivers/ide/ide.c
index 1689076..3b334af 100644
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -1782,6 +1782,7 @@ done:
 }
 
 extern void pnpide_init(void);
+extern void pnpide_exit(void);
 extern void h8300_ide_init(void);
 
 /*
@@ -2094,6 +2095,10 @@ void cleanup_module (void)
for (index = 0; index < MAX_HWIFS; ++index)
ide_unregister(index);
 
+#ifdef CONFIG_BLK_DEV_IDEPNP
+   pnpide_exit();
+#endif
+
 #ifdef CONFIG_PROC_FS
proc_ide_destroy();
 #endif
-
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

[PATCH] libata: fix handling of port actions in per-dev action mask

[Tejun Heo]

libata EH ignores port-wide actions in per-dev action mask.  However,
device resume requests EH_SOFTRESET using per-dev action mask.  Under
certain circumstances, this results in not resetting frozen port after
resuming which causes failure of all commands.

This patch allows port-wide actions to be requested in per-dev action
mask.  Before EH recovery starts, port-wide actions will be collected.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
After being tested, this one should go into -stable too.

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 08ad44b..56cf59b 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1979,6 +1979,10 @@ static int ata_eh_recover(struct ata_port *ap, 
ata_prereset_fn_t prereset,
 
ehc->tries[dev->devno] = ATA_EH_DEV_TRIES;
 
+   /* collect port action mask recorded in dev actions */
+   ehc->i.action |= ehc->i.dev_action[i] & ~ATA_EH_PERDEV_MASK;
+   ehc->i.dev_action[i] &= ATA_EH_PERDEV_MASK;
+
/* process hotplug request */
if (dev->flags & ATA_DFLAG_DETACH)
ata_eh_detach_dev(dev);
-
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

[PATCH] sd: implement stop_on_shutdown

[Tejun Heo]

sd doesn't stop (unload head) on shutdown.  This behavior is necessary
for multi initiator cases.  Unloading head by powering off stresses
the drive and sometimes produces distinct clunking noise which
apparently disturbs users considering multiple reports on different
distributions.  halt(8) usually puts the drives to sleep prior to
shutdown but the implementation is fragile and it doesn't work with
sleep-to-disk.

This patch implements sd attribute stop_on_shutdown.  If set to 1, sd
stops the drive on non-restarting shutdown.  stop_on_shutdown is
initialized from sd parameter stop_on_shutdown_default which defaults
to 0.  So, this patch does not change the default behavior.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
Acked-by: Henrique de Moraes Holschuh <[EMAIL PROTECTED]>

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 978bfc1..f21e5fe 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -90,6 +90,12 @@ MODULE_ALIAS_BLOCKDEV_MAJOR(SCSI_DISK13_MAJOR);
 MODULE_ALIAS_BLOCKDEV_MAJOR(SCSI_DISK14_MAJOR);
 MODULE_ALIAS_BLOCKDEV_MAJOR(SCSI_DISK15_MAJOR);
 
+static int sd_stop_on_shutdown_dfl = 0;
+module_param_named(stop_on_shutdown_default, sd_stop_on_shutdown_dfl,
+  bool, 0644);
+MODULE_PARM_DESC(stop_on_shutdown_default, "Default setting for stopping "
+"disk on shutdown (0=disable, 1=enable)");
+
 /*
  * This is limited by the naming scheme enforced in sd_probe,
  * add another character to it if you really need more disks.
@@ -126,6 +132,7 @@ struct scsi_disk {
unsignedWCE : 1;/* state of disk WCE bit */
unsignedRCD : 1;/* state of disk RCD bit, unused */
unsignedDPOFUA : 1; /* state of disk DPOFUA bit */
+   unsignedstop_on_shutdown : 1; /* stop on device shutdown */
 };
 #define to_scsi_disk(obj) container_of(obj,struct scsi_disk,cdev)
 
@@ -207,6 +214,19 @@ static ssize_t sd_store_cache_type(struct class_device 
*cdev, const char *buf,
return count;
 }
 
+static ssize_t sd_store_stop_on_shutdown(struct class_device *cdev,
+const char *buf, size_t count)
+{
+   struct scsi_disk *sdkp = to_scsi_disk(cdev);
+
+   if (!capable(CAP_SYS_ADMIN))
+   return -EACCES;
+
+   sdkp->stop_on_shutdown = simple_strtoul(buf, NULL, 10);
+
+   return count;
+}
+
 static ssize_t sd_store_allow_restart(struct class_device *cdev, const char 
*buf,
  size_t count)
 {
@@ -239,6 +259,13 @@ static ssize_t sd_show_fua(struct class_device *cdev, char 
*buf)
return snprintf(buf, 20, "%u\n", sdkp->DPOFUA);
 }
 
+static ssize_t sd_show_stop_on_shutdown(struct class_device *cdev, char *buf)
+{
+   struct scsi_disk *sdkp = to_scsi_disk(cdev);
+
+   return snprintf(buf, 20, "%u\n", sdkp->stop_on_shutdown);
+}
+
 static ssize_t sd_show_allow_restart(struct class_device *cdev, char *buf)
 {
struct scsi_disk *sdkp = to_scsi_disk(cdev);
@@ -252,6 +279,8 @@ static struct class_device_attribute sd_disk_attrs[] = {
__ATTR(FUA, S_IRUGO, sd_show_fua, NULL),
__ATTR(allow_restart, S_IRUGO|S_IWUSR, sd_show_allow_restart,
   sd_store_allow_restart),
+   __ATTR(stop_on_shutdown, S_IRUGO|S_IWUSR, sd_show_stop_on_shutdown,
+  sd_store_stop_on_shutdown),
__ATTR_NULL,
 };
 
@@ -1662,6 +1691,7 @@ static int sd_probe(struct device *dev)
sdkp->disk = gd;
sdkp->index = index;
sdkp->openers = 0;
+   sdkp->stop_on_shutdown = sd_stop_on_shutdown_dfl;
 
if (!sdp->timeout) {
if (sdp->type != TYPE_MOD)
@@ -1766,6 +1796,29 @@ static void scsi_disk_release(struct class_device *cdev)
kfree(sdkp);
 }
 
+static int sd_stop_device(struct scsi_device *sdp)
+{
+   unsigned char cmd[6] = { START_STOP }; /* START_VALID and START == 0 */
+   struct scsi_sense_hdr sshdr;
+   int res;
+
+   if (!scsi_device_online(sdp))
+   return -ENODEV;
+
+   res = scsi_execute_req(sdp, cmd, DMA_NONE, NULL, 0, &sshdr,
+  SD_TIMEOUT, SD_MAX_RETRIES);
+   if (res) {
+   printk(KERN_WARNING "FAILED\n  status = %x, message = %02x, "
+  "host = %d, driver = %02x\n  ",
+  status_byte(res), msg_byte(res),
+  host_byte(res), driver_byte(res));
+   if (driver_byte(res) & DRIVER_SENSE)
+   scsi_print_sense_hdr("sd", &sshdr);
+   }
+
+   return res;
+}
+
 /*
  * Send a SYNCHRONIZE CACHE instruction down to the device through
  * the normal SCSI command structure.  Wait for the command to
@@ -1784,6 +1837,13 @@ static void sd_shutdown(struct device *dev)
sdkp->disk->disk_name);
 

Re: [PATCH] sd: implement stop_on_shutdown

[Tejun Heo]

Hello, Douglas.

Douglas Gilbert wrote:
>> This patch implements sd attribute stop_on_shutdown.  If set to 1, sd
>> stops the drive on non-restarting shutdown.  stop_on_shutdown is
>> initialized from sd parameter stop_on_shutdown_default which defaults
>> to 0.  So, this patch does not change the default behavior.
> 
> Tejun,
> The IMMED bit in the START STOP UNIT cdb is not being set
> when your patch stops a drive:
> 
> Advantage:
>   - the power won't be dropped immediately after sending
> the command to the drive (assuming the drive gets
> its power from the same power supply that shutdown
> turns off)
> 
> Disadvantage:
>   - it will delay shutdown proportional to the number of
> drives with the stop_on_shutdown attribute set. Say
> 5 seconds per disk.

We pretty much need the IMMED bit to be cleared.  The goal of this patch
is to allow drives unload their heads before power is cut off and with
IMMED set, we really don't know when is safe to power off the machine.

> Disadvantage (with or without IMMED bit set):
>   - if another initiator (e.g. on another machine) was
> using a different partition on that disk, then it might
> get upset (especially if it was running Linux).
> [I'm not sure why you say this patch is necessary
>  in this case.]

Yeap, I agree with you.  I don't think the behavior introduced by this
patch is necessary in that case (did I say otherwise?) and that's
precisely why the default behavior hasn't been changed.  This is just
for small little machines running SATA and possibly USB disks (dunno
their SAT implement START_STOP tho) which are currently having problems
shutting down its disks on sleep to disk and sometimes poweroff.

> BTW SCSI disks typically have a lower start-stop lifetime
> rating than ATA disks. This reflects that SCSI disks are
> designed to be on 168 hours per week.

Yeap, not as useful as for PC stuff but I think having and enabling this
still would help in single initiator cases.  As you just said, they are
rated for less load-unload lifetime to begin with.

Thanks.

-- 
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] sd: implement stop_on_shutdown

[Tejun Heo]

Darrick J. Wong wrote:
> Tejun Heo wrote:
>> sd doesn't stop (unload head) on shutdown.  This behavior is necessary
>> for multi initiator cases.  Unloading head by powering off stresses
>> the drive and sometimes produces distinct clunking noise which
>> apparently disturbs users considering multiple reports on different
>> distributions.  halt(8) usually puts the drives to sleep prior to
>> shutdown but the implementation is fragile and it doesn't work with
>> sleep-to-disk.
> 
> I wonder if this sort of thing (cache flush + spin down) is the sort of
> thing that ought to be done to near-line storage at suspend time too,
> though one would want allow_restart = 1 before doing such a thing.

For ATA, it's currently being done inside libata proper (a bit ugly).
It would be nice to have those implemented at sd layer but I wonder how
useful it's going to be for actual SCSI devices.  Do people actually
suspend using SCSI?  If it's useful at the SCSI layer, I can implement
and test it with SATA devices here.

Thanks.

-- 
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] sata_uli: ignore SIMPLEX

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> Some uli controllers have stuck SIMPLEX bit which can't be cleared
>> with ata_pci_clear_simplex(), but the controller is capable of doing
>> DMAs on both channels simultaneously.  Ignore it.
>>
>> Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
> 
> This is going to be the common case going forward.  Please create an
> 'ignore simplex bit' flag, and set it in sata_uli

I'm a little bit hesitant to do that.

1. I don't think we're gonna have a lot of drivers which need this.
Just a few.  We've just found one now.

2. I think libata core layer (sff included) is burdened with too much
already.  Also, now that devres is acked, we can easily transition to
alloc - init - register model which gives LLDs much more flexibility and
this kind of stuff can easily be done in init_one()'s.  It just fits there.

But, it's your call.

Thanks.

-- 
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

[PATCH] sata_promise: kill qc->nsect

[Tejun Heo]

Merge order left qc->nsect usage in sata_promise dangling.  Kill it.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>

diff --git a/drivers/ata/sata_promise.c b/drivers/ata/sata_promise.c
index 551644a..32ae03e 100644
--- a/drivers/ata/sata_promise.c
+++ b/drivers/ata/sata_promise.c
@@ -502,8 +502,6 @@ static void pdc_atapi_pkt(struct ata_queued_cmd *qc)
feature = PDC_FEATURE_ATAPI_PIO;
/* set byte counter register to real transfer byte count */
nbytes = qc->nbytes;
-   if (!nbytes)
-   nbytes = qc->nsect << 9;
if (nbytes > 0x)
nbytes = 0x;
} else {
-
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

[PATCH] libata: fix compile warning caused by ignoring __must_check results

[Tejun Heo]

Fix compile warnings in pata_cs5530, sata_inic162x and sata_nv which
are caused by throwing away return values marked with __must_check.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/pata_cs5530.c   |6 +-
 drivers/ata/sata_inic162x.c |4 +++-
 drivers/ata/sata_nv.c   |5 -
 3 files changed, 12 insertions(+), 3 deletions(-)

Index: work/drivers/ata/pata_cs5530.c
===
--- work.orig/drivers/ata/pata_cs5530.c
+++ work/drivers/ata/pata_cs5530.c
@@ -251,6 +251,7 @@ static int cs5530_is_palmax(void)
 static int cs5530_init_chip(void)
 {
struct pci_dev *master_0 = NULL, *cs5530_0 = NULL, *dev = NULL;
+   int rc;
 
while ((dev = pci_get_device(PCI_VENDOR_ID_CYRIX, PCI_ANY_ID, dev)) != 
NULL) {
switch (dev->device) {
@@ -272,7 +273,10 @@ static int cs5530_init_chip(void)
}
 
pci_set_master(cs5530_0);
-   pci_set_mwi(cs5530_0);
+   rc = pci_set_mwi(cs5530_0);
+   if (rc)
+   dev_printk(KERN_WARNING, &cs5530_0->dev,
+  "WARNING: failed to set MWI\n");
 
/*
 * Set PCI CacheLineSize to 16-bytes:
Index: work/drivers/ata/sata_inic162x.c
===
--- work.orig/drivers/ata/sata_inic162x.c
+++ work/drivers/ata/sata_inic162x.c
@@ -649,7 +649,9 @@ static int inic_pci_device_resume(struct
void __iomem *mmio_base = host->mmio_base;
int rc;
 
-   ata_pci_device_do_resume(pdev);
+   rc = ata_pci_device_do_resume(pdev);
+   if (rc)
+   return rc;
 
if (pdev->dev.power.power_state.event == PM_EVENT_SUSPEND) {
printk("XXX\n");
Index: work/drivers/ata/sata_nv.c
===
--- work.orig/drivers/ata/sata_nv.c
+++ work/drivers/ata/sata_nv.c
@@ -1554,8 +1554,11 @@ static int nv_pci_device_resume(struct p
 {
struct ata_host *host = dev_get_drvdata(&pdev->dev);
struct nv_host_priv *hpriv = host->private_data;
+   int rc;
 
-   ata_pci_device_do_resume(pdev);
+   rc = ata_pci_device_do_resume(pdev);
+   if (rc)
+   return rc;
 
if (pdev->dev.power.power_state.event == PM_EVENT_SUSPEND) {
if(hpriv->type >= CK804) {
-
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

[PATCH] libata: fix compile warning caused by ignoring __must_check results

[Tejun Heo]

Fix compile warnings in pata_cs5530, sata_inic162x and sata_nv which
are caused by throwing away return values marked with __must_check.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/libata-sff.c|   10 +++---
 drivers/ata/pata_cs5530.c   |6 +-
 drivers/ata/sata_inic162x.c |4 +++-
 drivers/ata/sata_nv.c   |5 -
 4 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 69a5910..c7acbe9 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -1096,12 +1096,16 @@ int ata_pci_init_one (struct pci_dev *pdev, struct 
ata_port_info **port_info,
} else
legacy_mode |= ATA_PORT_SECONDARY;
 
+   /* For backward-compatibility reasons, we don't care
+* whether the followings succeed or fail.  Throw away
+* return value explicitly to override __must_check.
+*/
if (legacy_mode & ATA_PORT_PRIMARY)
-   pci_request_region(pdev, 1, DRV_NAME);
+   rc = pci_request_region(pdev, 1, DRV_NAME);
if (legacy_mode & ATA_PORT_SECONDARY)
-   pci_request_region(pdev, 3, DRV_NAME);
+   rc = pci_request_region(pdev, 3, DRV_NAME);
/* If there is a DMA resource, allocate it */
-   pci_request_region(pdev, 4, DRV_NAME);
+   rc = pci_request_region(pdev, 4, DRV_NAME);
}
 
/* we have legacy mode, but all ports are unavailable */
diff --git a/drivers/ata/pata_cs5530.c b/drivers/ata/pata_cs5530.c
index 611d90f..d0e542c 100644
--- a/drivers/ata/pata_cs5530.c
+++ b/drivers/ata/pata_cs5530.c
@@ -251,6 +251,7 @@ static int cs5530_is_palmax(void)
 static int cs5530_init_chip(void)
 {
struct pci_dev *master_0 = NULL, *cs5530_0 = NULL, *dev = NULL;
+   int rc;
 
while ((dev = pci_get_device(PCI_VENDOR_ID_CYRIX, PCI_ANY_ID, dev)) != 
NULL) {
switch (dev->device) {
@@ -272,7 +273,10 @@ static int cs5530_init_chip(void)
}
 
pci_set_master(cs5530_0);
-   pci_set_mwi(cs5530_0);
+   rc = pci_set_mwi(cs5530_0);
+   if (rc)
+   dev_printk(KERN_WARNING, &cs5530_0->dev,
+  "WARNING: failed to set MWI\n");
 
/*
 * Set PCI CacheLineSize to 16-bytes:
diff --git a/drivers/ata/sata_inic162x.c b/drivers/ata/sata_inic162x.c
index b67817e..6cd1fe9 100644
--- a/drivers/ata/sata_inic162x.c
+++ b/drivers/ata/sata_inic162x.c
@@ -649,7 +649,9 @@ static int inic_pci_device_resume(struct pci_dev *pdev)
void __iomem *mmio_base = host->mmio_base;
int rc;
 
-   ata_pci_device_do_resume(pdev);
+   rc = ata_pci_device_do_resume(pdev);
+   if (rc)
+   return rc;
 
if (pdev->dev.power.power_state.event == PM_EVENT_SUSPEND) {
printk("XXX\n");
diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c
index cc10e43..9d8db97 100644
--- a/drivers/ata/sata_nv.c
+++ b/drivers/ata/sata_nv.c
@@ -1554,8 +1554,11 @@ static int nv_pci_device_resume(struct pci_dev *pdev)
 {
struct ata_host *host = dev_get_drvdata(&pdev->dev);
struct nv_host_priv *hpriv = host->private_data;
+   int rc;
 
-   ata_pci_device_do_resume(pdev);
+   rc = ata_pci_device_do_resume(pdev);
+   if (rc)
+   return rc;
 
if (pdev->dev.power.power_state.event == PM_EVENT_SUSPEND) {
if(hpriv->type >= CK804) {
-- 
1.4.4.3


-
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

[PATCH RESEND] libata: fix compile warning caused by ignoring __must_check results

[Tejun Heo]

* Fix compile warnings in pata_cs5530, sata_inic162x and sata_nv which
  are caused by throwing away return values marked with __must_check.

* pci_request_region() is marked with __must_check.
  ata_pci_init_one() didn't check return value for legacy devices
  triggering warning message.  This patch adds dummy assignment to rc
  to remove the warnings.  Ignoring the return value is intentional.
  Acquiring regions for legacy ctl and bmdma areas is recently added
  and we don't wanna fail initialization even if it fails (at least
  not yet).

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
Sorry, forgot to update description after adding sff changes.
Otherwise, the patch is identical.

 drivers/ata/libata-sff.c|   10 +++---
 drivers/ata/pata_cs5530.c   |6 +-
 drivers/ata/sata_inic162x.c |4 +++-
 drivers/ata/sata_nv.c   |5 -
 4 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 69a5910..c7acbe9 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -1096,12 +1096,16 @@ int ata_pci_init_one (struct pci_dev *pdev, struct 
ata_port_info **port_info,
} else
legacy_mode |= ATA_PORT_SECONDARY;
 
+   /* For backward-compatibility reasons, we don't care
+* whether the followings succeed or fail.  Throw away
+* return value explicitly to override __must_check.
+*/
if (legacy_mode & ATA_PORT_PRIMARY)
-   pci_request_region(pdev, 1, DRV_NAME);
+   rc = pci_request_region(pdev, 1, DRV_NAME);
if (legacy_mode & ATA_PORT_SECONDARY)
-   pci_request_region(pdev, 3, DRV_NAME);
+   rc = pci_request_region(pdev, 3, DRV_NAME);
/* If there is a DMA resource, allocate it */
-   pci_request_region(pdev, 4, DRV_NAME);
+   rc = pci_request_region(pdev, 4, DRV_NAME);
}
 
/* we have legacy mode, but all ports are unavailable */
diff --git a/drivers/ata/pata_cs5530.c b/drivers/ata/pata_cs5530.c
index 611d90f..d0e542c 100644
--- a/drivers/ata/pata_cs5530.c
+++ b/drivers/ata/pata_cs5530.c
@@ -251,6 +251,7 @@ static int cs5530_is_palmax(void)
 static int cs5530_init_chip(void)
 {
struct pci_dev *master_0 = NULL, *cs5530_0 = NULL, *dev = NULL;
+   int rc;
 
while ((dev = pci_get_device(PCI_VENDOR_ID_CYRIX, PCI_ANY_ID, dev)) != 
NULL) {
switch (dev->device) {
@@ -272,7 +273,10 @@ static int cs5530_init_chip(void)
}
 
pci_set_master(cs5530_0);
-   pci_set_mwi(cs5530_0);
+   rc = pci_set_mwi(cs5530_0);
+   if (rc)
+   dev_printk(KERN_WARNING, &cs5530_0->dev,
+  "WARNING: failed to set MWI\n");
 
/*
 * Set PCI CacheLineSize to 16-bytes:
diff --git a/drivers/ata/sata_inic162x.c b/drivers/ata/sata_inic162x.c
index b67817e..6cd1fe9 100644
--- a/drivers/ata/sata_inic162x.c
+++ b/drivers/ata/sata_inic162x.c
@@ -649,7 +649,9 @@ static int inic_pci_device_resume(struct pci_dev *pdev)
void __iomem *mmio_base = host->mmio_base;
int rc;
 
-   ata_pci_device_do_resume(pdev);
+   rc = ata_pci_device_do_resume(pdev);
+   if (rc)
+   return rc;
 
if (pdev->dev.power.power_state.event == PM_EVENT_SUSPEND) {
printk("XXX\n");
diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c
index cc10e43..9d8db97 100644
--- a/drivers/ata/sata_nv.c
+++ b/drivers/ata/sata_nv.c
@@ -1554,8 +1554,11 @@ static int nv_pci_device_resume(struct pci_dev *pdev)
 {
struct ata_host *host = dev_get_drvdata(&pdev->dev);
struct nv_host_priv *hpriv = host->private_data;
+   int rc;
 
-   ata_pci_device_do_resume(pdev);
+   rc = ata_pci_device_do_resume(pdev);
+   if (rc)
+   return rc;
 
if (pdev->dev.power.power_state.event == PM_EVENT_SUSPEND) {
if(hpriv->type >= CK804) {
-- 
1.4.4.3

-
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

[PATCHSET] Managed device resources, take #3

[Tejun Heo]

Hello,

This is the third iteration of devres patchset.  Changes from the last
take[L] are...

* devres patches collapsed into one as Jeff requested

* updates for sata_inic162x added

* rebased to the current upstream

Git tree is available at the following URLs.

  http://htj.dyndns.org/git/?p=libata-tj.git;a=shortlog;h=devres
  git://htj.dyndns.org/libata-tj devres

For detailed info, please read Documentation/driver-model/devres.txt.
Similar info can also be found from the initial take[I].

This patchset is against

[U]   libata-dev#upstream
[1] + sata_promise-kill-qc-nsect
[2] + lbiata-fix-compile-warning-caused-by-ignoring-__must_check results

The git tree contains both previous patches, but this patchset apply
okay with either one of or both patches omitted.

Thanks.

--
tejun

[L] http://thread.gmane.org/gmane.linux.kernel/482455
[I] http://thread.gmane.org/gmane.linux.ide/14690
[U] 8ed22df2570dbb7df2bd161bb9381a6fd17de3d3
[1] http://article.gmane.org/gmane.linux.ide/15189
[2] http://article.gmane.org/gmane.linux.ide/15192


-
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

[PATCH 2/7] libata: implement ata_host_detach()

[Tejun Heo]

Implement ata_host_detach() which calls ata_port_detach() for each
port in the host and export it.  ata_port_detach() is now internal and
thus un-exported.  ata_host_detach() will be used as the 'deregister
from libata layer' function after devres conversion.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/ahci.c|3 +--
 drivers/ata/libata-core.c |   22 +++---
 include/linux/libata.h|2 +-
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 5998f74..d089217 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1767,8 +1767,7 @@ static void ahci_remove_one (struct pci_dev *pdev)
unsigned int i;
int have_msi;
 
-   for (i = 0; i < host->n_ports; i++)
-   ata_port_detach(host->ports[i]);
+   ata_host_detach(host);
 
have_msi = hpriv->flags & AHCI_FLAG_MSI;
free_irq(host->irq, host);
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 8315ee6..20a943f 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6002,6 +6002,23 @@ void ata_port_detach(struct ata_port *ap)
 }
 
 /**
+ * ata_host_detach - Detach all ports of an ATA host
+ * @host: Host to detach
+ *
+ * Detach all ports of @host.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep).
+ */
+void ata_host_detach(struct ata_host *host)
+{
+   int i;
+
+   for (i = 0; i < host->n_ports; i++)
+   ata_port_detach(host->ports[i]);
+}
+
+/**
  * ata_host_remove - PCI layer callback for device removal
  * @host: ATA host set that was removed
  *
@@ -6016,8 +6033,7 @@ void ata_host_remove(struct ata_host *host)
 {
unsigned int i;
 
-   for (i = 0; i < host->n_ports; i++)
-   ata_port_detach(host->ports[i]);
+   ata_host_detach(host);
 
free_irq(host->irq, host);
if (host->irq2)
@@ -6392,7 +6408,7 @@ EXPORT_SYMBOL_GPL(ata_std_bios_param);
 EXPORT_SYMBOL_GPL(ata_std_ports);
 EXPORT_SYMBOL_GPL(ata_host_init);
 EXPORT_SYMBOL_GPL(ata_device_add);
-EXPORT_SYMBOL_GPL(ata_port_detach);
+EXPORT_SYMBOL_GPL(ata_host_detach);
 EXPORT_SYMBOL_GPL(ata_host_remove);
 EXPORT_SYMBOL_GPL(ata_sg_init);
 EXPORT_SYMBOL_GPL(ata_sg_init_one);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 09c110a..8bad682 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -718,7 +718,7 @@ extern int ata_pci_device_resume(struct pci_dev *pdev);
 extern int ata_pci_clear_simplex(struct pci_dev *pdev);
 #endif /* CONFIG_PCI */
 extern int ata_device_add(const struct ata_probe_ent *ent);
-extern void ata_port_detach(struct ata_port *ap);
+extern void ata_host_detach(struct ata_host *host);
 extern void ata_host_init(struct ata_host *, struct device *,
  unsigned long, const struct ata_port_operations *);
 extern void ata_host_remove(struct ata_host *host);
-- 
1.4.4.4


-
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

[PATCH 3/7] libata: update libata core layer to use devres

[Tejun Heo]

Update libata core layer to use devres.

* ata_device_add() acquires all resources in managed mode.

* ata_host is allocated as devres associated with ata_host_release.

* Port attached status is handled as devres associated with
  ata_host_attach_release().

* Initialization failure and host removal is handedl by releasing
  devres group.

* Except for ata_scsi_release() removal, LLD interface remains the
  same.  Some functions use hacky is_managed test to support both
  managed and unmanaged devices.  These will go away once all LLDs are
  updated to use devres.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/ahci.c|   21 +-
 drivers/ata/libata-core.c |  177 +++--
 drivers/ata/libata-scsi.c |3 +-
 drivers/ata/libata-sff.c  |   56 ++-
 include/linux/libata.h|9 +--
 5 files changed, 106 insertions(+), 160 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index d089217..7abe138 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1759,37 +1759,24 @@ err_out:
return rc;
 }
 
-static void ahci_remove_one (struct pci_dev *pdev)
+static void ahci_remove_one(struct pci_dev *pdev)
 {
struct device *dev = pci_dev_to_dev(pdev);
struct ata_host *host = dev_get_drvdata(dev);
struct ahci_host_priv *hpriv = host->private_data;
-   unsigned int i;
-   int have_msi;
 
-   ata_host_detach(host);
+   ata_host_remove(host);
 
-   have_msi = hpriv->flags & AHCI_FLAG_MSI;
-   free_irq(host->irq, host);
-
-   for (i = 0; i < host->n_ports; i++) {
-   struct ata_port *ap = host->ports[i];
-
-   ata_scsi_release(ap->scsi_host);
-   scsi_host_put(ap->scsi_host);
-   }
-
-   kfree(hpriv);
pci_iounmap(pdev, host->mmio_base);
-   kfree(host);
 
-   if (have_msi)
+   if (hpriv->flags & AHCI_FLAG_MSI)
pci_disable_msi(pdev);
else
pci_intx(pdev, 0);
pci_release_regions(pdev);
pci_disable_device(pdev);
dev_set_drvdata(dev, NULL);
+   kfree(hpriv);
 }
 
 static int __init ahci_init(void)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 20a943f..a9a4c67 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5496,28 +5496,25 @@ void ata_host_resume(struct ata_host *host)
  * LOCKING:
  * Inherited from caller.
  */
-
-int ata_port_start (struct ata_port *ap)
+int ata_port_start(struct ata_port *ap)
 {
struct device *dev = ap->dev;
int rc;
 
-   ap->prd = dma_alloc_coherent(dev, ATA_PRD_TBL_SZ, &ap->prd_dma, 
GFP_KERNEL);
+   ap->prd = dmam_alloc_coherent(dev, ATA_PRD_TBL_SZ, &ap->prd_dma,
+ GFP_KERNEL);
if (!ap->prd)
return -ENOMEM;
 
rc = ata_pad_alloc(ap, dev);
-   if (rc) {
-   dma_free_coherent(dev, ATA_PRD_TBL_SZ, ap->prd, ap->prd_dma);
+   if (rc)
return rc;
-   }
-
-   DPRINTK("prd alloc, virt %p, dma %llx\n", ap->prd, (unsigned long long) 
ap->prd_dma);
 
+   DPRINTK("prd alloc, virt %p, dma %llx\n", ap->prd,
+   (unsigned long long)ap->prd_dma);
return 0;
 }
 
-
 /**
  * ata_port_stop - Undo ata_port_start()
  * @ap: Port to shut down
@@ -5529,12 +5526,11 @@ int ata_port_start (struct ata_port *ap)
  * LOCKING:
  * Inherited from caller.
  */
-
 void ata_port_stop (struct ata_port *ap)
 {
struct device *dev = ap->dev;
 
-   dma_free_coherent(dev, ATA_PRD_TBL_SZ, ap->prd, ap->prd_dma);
+   dmam_free_coherent(dev, ATA_PRD_TBL_SZ, ap->prd, ap->prd_dma);
ata_pad_free(ap, dev);
 }
 
@@ -5717,6 +5713,27 @@ static struct ata_port * ata_port_add(const struct 
ata_probe_ent *ent,
return ap;
 }
 
+static void ata_host_release(struct device *gendev, void *res)
+{
+   struct ata_host *host = dev_get_drvdata(gendev);
+   int i;
+
+   for (i = 0; i < host->n_ports; i++) {
+   struct ata_port *ap = host->ports[i];
+
+   if (!ap)
+   continue;
+
+   if (ap->ops->port_stop)
+   ap->ops->port_stop(ap);
+
+   scsi_host_put(ap->scsi_host);
+   }
+
+   if (host->ops->host_stop)
+   host->ops->host_stop(host);
+}
+
 /**
  * ata_sas_host_init - Initialize a host struct
  * @host:  host to initialize
@@ -5769,11 +5786,17 @@ int ata_device_add(const struct ata_probe_ent *ent)
dev_printk(KERN_ERR, dev, "is not available: No interrupt 
assigned.\n");
return 0;
}
+
+   if (!devres_open_group(dev, ata_device_add, GFP_KERNEL))
+   return 0;
+
/* a

[PATCH 5/7] libata: remove unused functions

[Tejun Heo]

Now that all LLDs are converted to use devres, default stop callbacks
are unused.  Remove them.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/libata-core.c |   81 +++-
 include/linux/libata.h|4 --
 2 files changed, 6 insertions(+), 79 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index a9a4c67..34e7f18 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5516,31 +5516,6 @@ int ata_port_start(struct ata_port *ap)
 }
 
 /**
- * ata_port_stop - Undo ata_port_start()
- * @ap: Port to shut down
- *
- * Frees the PRD table.
- *
- * May be used as the port_stop() entry in ata_port_operations.
- *
- * LOCKING:
- * Inherited from caller.
- */
-void ata_port_stop (struct ata_port *ap)
-{
-   struct device *dev = ap->dev;
-
-   dmam_free_coherent(dev, ATA_PRD_TBL_SZ, ap->prd, ap->prd_dma);
-   ata_pad_free(ap, dev);
-}
-
-void ata_host_stop (struct ata_host *host)
-{
-   if (host->mmio_base)
-   iounmap(host->mmio_base);
-}
-
-/**
  * ata_dev_init - Initialize an ata_device structure
  * @dev: Device structure to initialize
  *
@@ -5879,7 +5854,7 @@ int ata_device_add(const struct ata_probe_ent *ent)
}
 
/* resource acquisition complete */
-   devres_close_group(dev, ata_device_add);
+   devres_remove_group(dev, ata_device_add);
 
/* perform each probe synchronously */
DPRINTK("probe begin\n");
@@ -6034,22 +6009,6 @@ void ata_host_detach(struct ata_host *host)
ata_port_detach(host->ports[i]);
 }
 
-/**
- * ata_host_remove - PCI layer callback for device removal
- * @host: ATA host set that was removed
- *
- * Unregister all objects associated with this host set. Free those
- * objects.
- *
- * LOCKING:
- * Inherited from calling layer (may sleep).
- */
-void ata_host_remove(struct ata_host *host)
-{
-   ata_host_detach(host);
-   devres_release_group(host->dev, ata_device_add);
-}
-
 struct ata_probe_ent *
 ata_probe_ent_alloc(struct device *dev, const struct ata_port_info *port)
 {
@@ -6109,26 +6068,13 @@ void ata_std_ports(struct ata_ioports *ioaddr)
 
 #ifdef CONFIG_PCI
 
-void ata_pci_host_stop (struct ata_host *host)
-{
-   struct pci_dev *pdev = to_pci_dev(host->dev);
-
-   /* XXX - the following if can go away once all LLDs are managed */
-   if (!list_empty(&host->dev->devres_head))
-   pcim_iounmap(pdev, host->mmio_base);
-   else
-   pci_iounmap(pdev, host->mmio_base);
-}
-
 /**
  * ata_pci_remove_one - PCI layer callback for device removal
  * @pdev: PCI device that was removed
  *
- * PCI layer indicates to libata via this hook that
- * hot-unplug or module unload event has occurred.
- * Handle this by unregistering all objects associated
- * with this PCI device.  Free those objects.  Then finally
- * release PCI resources and disable device.
+ * PCI layer indicates to libata via this hook that hot-unplug or
+ * module unload event has occurred.  Detach all ports.  Resource
+ * release is handled via devres.
  *
  * LOCKING:
  * Inherited from PCI layer (may sleep).
@@ -6138,14 +6084,7 @@ void ata_pci_remove_one(struct pci_dev *pdev)
struct device *dev = pci_dev_to_dev(pdev);
struct ata_host *host = dev_get_drvdata(dev);
 
-   /* XXX - the following if can go away once all LLDs are managed */
-   if (!list_empty(&host->dev->devres_head)) {
-   ata_host_remove(host);
-   pci_release_regions(pdev);
-   pci_disable_device(pdev);
-   dev_set_drvdata(dev, NULL);
-   } else
-   ata_host_detach(host);
+   ata_host_detach(host);
 }
 
 /* move to PCI subsystem */
@@ -6199,11 +6138,7 @@ int ata_pci_device_do_resume(struct pci_dev *pdev)
pci_set_power_state(pdev, PCI_D0);
pci_restore_state(pdev);
 
-   /* XXX - the following if can go away once all LLDs are managed */
-   if (!list_empty(&pdev->dev.devres_head))
-   rc = pcim_enable_device(pdev);
-   else
-   rc = pci_enable_device(pdev);
+   rc = pcim_enable_device(pdev);
if (rc) {
dev_printk(KERN_ERR, &pdev->dev,
   "failed to enable device after resume (%d)\n", rc);
@@ -6383,7 +6318,6 @@ EXPORT_SYMBOL_GPL(ata_std_ports);
 EXPORT_SYMBOL_GPL(ata_host_init);
 EXPORT_SYMBOL_GPL(ata_device_add);
 EXPORT_SYMBOL_GPL(ata_host_detach);
-EXPORT_SYMBOL_GPL(ata_host_remove);
 EXPORT_SYMBOL_GPL(ata_sg_init);
 EXPORT_SYMBOL_GPL(ata_sg_init_one);
 EXPORT_SYMBOL_GPL(ata_hsm_move);
@@ -6400,8 +6334,6 @@ EXPORT_SYMBOL_GPL(ata_check_status);
 EXPORT_SYMBOL_GPL(ata_altstatus);
 EXPORT_SYMBOL_GPL(ata_exec_command);
 EXPORT_SYMBOL_GPL(ata_port_start)

[PATCH 6/7] devres: implement pcim_iomap_regions()

[Tejun Heo]

Implement pcim_iomap_regions().  This function takes mask of BARs to
request and iomap.  No BAR should have length of zero.  BARs are
iomapped using pcim_iomap_table().

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 include/linux/io.h |2 +
 lib/iomap.c|   53 
 2 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/include/linux/io.h b/include/linux/io.h
index f5edf9c..45a9c94 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -45,6 +45,8 @@ void __iomem * pcim_iomap(struct pci_dev *pdev, int bar, 
unsigned long maxlen);
 void pcim_iounmap(struct pci_dev *pdev, void __iomem *addr);
 void __iomem * const * pcim_iomap_table(struct pci_dev *pdev);
 
+int pcim_iomap_regions(struct pci_dev *pdev, u16 mask, const char *name);
+
 /**
  * check_signature -   find BIOS signatures
  * @io_addr: mmio address to check
diff --git a/lib/iomap.c b/lib/iomap.c
index 3214028..4990c73 100644
--- a/lib/iomap.c
+++ b/lib/iomap.c
@@ -498,3 +498,56 @@ void pcim_iounmap(struct pci_dev *pdev, void __iomem *addr)
WARN_ON(1);
 }
 EXPORT_SYMBOL(pcim_iounmap);
+
+/**
+ * pcim_iomap_regions - Request and iomap PCI BARs
+ * @pdev: PCI device to map IO resources for
+ * @mask: Mask of BARs to request and iomap
+ * @name: Name used when requesting regions
+ *
+ * Request and iomap regions specified by @mask.
+ */
+int pcim_iomap_regions(struct pci_dev *pdev, u16 mask, const char *name)
+{
+   void __iomem * const *iomap;
+   int i, rc;
+
+   iomap = pcim_iomap_table(pdev);
+   if (!iomap)
+   return -ENOMEM;
+
+   for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
+   unsigned long len;
+
+   if (!(mask & (1 << i)))
+   continue;
+
+   rc = -EINVAL;
+   len = pci_resource_len(pdev, i);
+   if (!len)
+   goto err_inval;
+
+   rc = pci_request_region(pdev, i, name);
+   if (rc)
+   goto err_region;
+
+   rc = -ENOMEM;
+   if (!pcim_iomap(pdev, i, 0))
+   goto err_iomap;
+   }
+
+   return 0;
+
+ err_iomap:
+   pcim_iounmap(pdev, iomap[i]);
+ err_region:
+   pci_release_region(pdev, i);
+ err_inval:
+   while (--i >= 0) {
+   pcim_iounmap(pdev, iomap[i]);
+   pci_release_region(pdev, i);
+   }
+
+   return rc;
+}
+EXPORT_SYMBOL(pcim_iomap_regions);
-- 
1.4.4.4


-
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] sd: implement stop_on_shutdown

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> For ATA, it's currently being done inside libata proper (a bit ugly).
>> It would be nice to have those implemented at sd layer but I wonder how
>> useful it's going to be for actual SCSI devices.  Do people actually
>> suspend using SCSI?  If it's useful at the SCSI layer, I can implement
>> and test it with SATA devices here.
> 
> There is always the open question for multi-initiator SCSI devices, as
> to who "owns" the SCSI device.  Some devices should be suspended/stopped
> when the machine is suspended/stopped, others not.

I think that can be handled similarly as stop_on_shutdown on kernel side
and let udev and friends deal with the specifics.

Thanks.

-- 
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

[PATCH] ahci: don't enter slumber on power down

[Tejun Heo]

Some ATA/ATAPI devices act weirdly after the link is put into slumber
mode.  Some hang completely requiring physical power removal while
others fail to wake up till the link is hardreset a couple of times.

The addition of slumber on power down was never driven by real need.
It just followed what ahci spec said literally.  The spec itself seems
faulty in that it doesn't consider devices (not controllers) which
don't support link powersaving mode.

Theory never matches reality when it comes to dark allys of cheap
ATA/ATAPI world.  It's just unrealistic to expect vendors to test
rarely used link powersaving feature rigorously.  This patch makes
ahci more friendly to the coldness of reality.

This shouldn't have any negative effect - when suspend operation
succeeds, we power off the whole machine; otherwise, we wake up
everything.  I can't see any reason to be so elaborate with powering
down the link in the first place.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/ahci.c |   37 ++---
 1 file changed, 10 insertions(+), 27 deletions(-)

Index: work/drivers/ata/ahci.c
===
--- work.orig/drivers/ata/ahci.c
+++ work/drivers/ata/ahci.c
@@ -586,35 +586,18 @@ static void ahci_power_down(void __iomem
 {
u32 cmd, scontrol;
 
-   cmd = readl(port_mmio + PORT_CMD) & ~PORT_CMD_ICC_MASK;
-
-   if (cap & HOST_CAP_SSC) {
-   /* enable transitions to slumber mode */
-   scontrol = readl(port_mmio + PORT_SCR_CTL);
-   if ((scontrol & 0x0f00) > 0x100) {
-   scontrol &= ~0xf00;
-   writel(scontrol, port_mmio + PORT_SCR_CTL);
-   }
+   if (!(cap & HOST_CAP_SSS))
+   return;
 
-   /* put device into slumber mode */
-   writel(cmd | PORT_CMD_ICC_SLUMBER, port_mmio + PORT_CMD);
+   /* put device into listen mode, first set PxSCTL.DET to 0 */
+   scontrol = readl(port_mmio + PORT_SCR_CTL);
+   scontrol &= ~0xf;
+   writel(scontrol, port_mmio + PORT_SCR_CTL);
 
-   /* wait for the transition to complete */
-   ata_wait_register(port_mmio + PORT_CMD, PORT_CMD_ICC_SLUMBER,
- PORT_CMD_ICC_SLUMBER, 1, 50);
-   }
-
-   /* put device into listen mode */
-   if (cap & HOST_CAP_SSS) {
-   /* first set PxSCTL.DET to 0 */
-   scontrol = readl(port_mmio + PORT_SCR_CTL);
-   scontrol &= ~0xf;
-   writel(scontrol, port_mmio + PORT_SCR_CTL);
-
-   /* then set PxCMD.SUD to 0 */
-   cmd &= ~PORT_CMD_SPIN_UP;
-   writel(cmd, port_mmio + PORT_CMD);
-   }
+   /* then set PxCMD.SUD to 0 */
+   cmd = readl(port_mmio + PORT_CMD) & ~PORT_CMD_ICC_MASK;
+   cmd &= ~PORT_CMD_SPIN_UP;
+   writel(cmd, port_mmio + PORT_CMD);
 }
 
 static void ahci_init_port(void __iomem *port_mmio, u32 cap,
-
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: ICH6-M libata disk timeouts in 2.6.19 (except with small queue depth)

[Tejun Heo]

Mike Accetta wrote:
> The distinguishing factor appears to be the queue depth (4 works, 5
> and various values up to and including 31 fail) not the kernel version.
> I am going to try running with the queue depth clamped at 4 to see if
> this consistently masks the problem.  I may also try some more experiments
> if I have the time, like instrumenting what command was issued right
> before the group that all time out or increasing the SCSI timeout,
> in order to get some more insight into what is going on at the time of
> the failure.

Please report the result of 'hdparm -I /dev/sdX' where sdX is the
problematic disk.  Quite a few drives have problem with NCQ and we have
to blacklist them.

Thanks.

-- 
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: Reg. SATA II NCQ

[Tejun Heo]

Jaychander T wrote:

I understand from the SATA II NCQ specs that for NCQ commands(FPDMA)
we need not have to setup the DMA (PRD) for the commands issued but
has to do


That's different thing.  NCQ protocol doesn't require DMA SETUP FIS 
which is required for normal DMA commands but the controller still needs 
to be prepped with DMA table to know from/to where transfer data.



so only on reception of DMA setup FIS from the drive(throught ISR).
But in libata/ ata_qc_issue does qc prep and issue irrespective of the
protocol.


Yeap, DMA table needs to be prepped whether NCQ is used or not.


Even i glanced thro' the sil24 driver and seems to be the same.


The same for sil24.

--
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] libata: fix compile warning caused by ignoring __must_check results

[Tejun Heo]

Alan wrote:

On Sat, 20 Jan 2007 14:23:29 +0900
Tejun Heo <[EMAIL PROTECTED]> wrote:


Fix compile warnings in pata_cs5530, sata_inic162x and sata_nv which
are caused by throwing away return values marked with __must_check.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>


NAK NAK NAK NAK NAK NAK

As was previously discussed the bug is the bogus __must_check on
pci_set_mwi. That __must_check should be removed and that was also the
decision before but nothing happened. Please therefore get the pci header
fixed.


That works for me too.  I'll drop pci_set_mwi() part.

Thanks.

--
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

[PATCH] ahci: make ULi M5288 ignore interface fatal error bit

[Tejun Heo]

As with JMicron controllers, ULi M5288 sets interface fatal error bit
on device error including ATAPI CC.  This makes libata hardreset the
port on ATAPI CC thus making it impossible to use.  Ignore interface
fatal error bit on ULi M5288.  This fixes bugzilla bug #7837.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
Jeff, this is a regression.  Please commit to both #upstream and
#upstream-fixes.  Thanks.

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 5998f74..4fb41a1 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -361,7 +361,7 @@ static const struct pci_device_id ahci_pci_tbl[] = {
{ PCI_VDEVICE(INTEL, 0x27c1), board_ahci }, /* ICH7 */
{ PCI_VDEVICE(INTEL, 0x27c5), board_ahci }, /* ICH7M */
{ PCI_VDEVICE(INTEL, 0x27c3), board_ahci }, /* ICH7R */
-   { PCI_VDEVICE(AL, 0x5288), board_ahci }, /* ULi M5288 */
+   { PCI_VDEVICE(AL, 0x5288), board_ahci_ign_iferr }, /* ULi M5288 */
{ PCI_VDEVICE(INTEL, 0x2681), board_ahci }, /* ESB2 */
{ PCI_VDEVICE(INTEL, 0x2682), board_ahci }, /* ESB2 */
{ PCI_VDEVICE(INTEL, 0x2683), board_ahci }, /* ESB2 */
-
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: PATA ATAPI detection debug

[Tejun Heo]

Tejun Heo wrote:
> Hello, all.
> 
> Many people have been reporting libata PATA ATAPI detection problem.  In
> many but not all cases, the ATAPI device was occupying the slave slot
> while a disk drive occupies the master slot.  Based on that and J.
> Taimr's nullify freeze on via fix, I made a cocktail patch which
> contained four different fixes and it seemed to have fixed the problem
> for (at least) several people, but the reports are not all consistent.
> 
> The attached patches contain the same four fixes but has a selector
> parameter to enable each fix separately.  Both are equivalent but using
> 2.6.20-rc5 is recommended to rule out detection problems fixed by
> polling IDENTIFY.
> 
> If your libata driver is compiled into the kernel, add
> 'libata.debug_cocktail=N' to your kernel parameter.  If you compile
> libata.ko as module, add 'debug_cocktail=N' module parameter to the
> module parameter.  e.g. 'modprobe libata.ko debug_cocktail=1'.

Daniel Fraga and Sero's cases are different issue while I can't
determine whether J. Taimr's case is limted on via chipset.  So, I need
more input to proceed.

*Please* test and report the results.

Thanks.

-- 
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] ahci: improve and limit spurious interrupt messages, take#2

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> -/* ignore interim PIO setup fis interrupts */
>> -if (ata_tag_valid(ap->active_tag) && (status & PORT_IRQ_PIOS_FIS))
>> -return;
>> +if ((status & PORT_IRQ_D2H_REG_FIS) &&
>> +!(pp->seen_status & PORT_IRQ_D2H_REG_FIS)) {
>> +ata_port_printk(ap, KERN_INFO, "D2H reg with I during NCQ, "
>> +"this message won't be printed again\n");
>> +pp->seen_status |= PORT_IRQ_D2H_REG_FIS;
>> +} else if ((status & PORT_IRQ_DMAS_FIS) &&
>> +   !(pp->seen_status & PORT_IRQ_DMAS_FIS)) {
>> +ata_port_printk(ap, KERN_INFO, "DMAS FIS during NCQ, "
>> +"this message won't be printed again\n");
>> +pp->seen_status |= PORT_IRQ_DMAS_FIS;
>> +} else if (status & PORT_IRQ_SDB_FIS && pp->spurious_sdb_cnt < 10) {
>> +/* SDB FIS containing spurious completions might be
>> + * dangerous, we need to know more about them.  Print
>> + * more of it.
>> + */
>> +const u32 *f = pp->rx_fis + RX_FIS_SDB;
> 
> 
> This if/else/else tree does not take into account the possiblity that
> more than one bit may be set.

Thought it wouldn't really matter.  Will fix.

-- 
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

ahci: improve and limit spurious interrupt messages, take#3

[Tejun Heo]

We're still seeing a lot of issues with NCQ implementation in drive
firmwares.  Sprious FISes during NCQ command phase occur on many
drives and some of them seem potentially dangerous (at least to me).
Until we find the solution, spurious messages can give us more info.
Improve and limit them such that more info can be reported while not
disturbing users too much.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
Fixed as advised.  Thanks.

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 5998f74..659949a 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -75,6 +75,7 @@ enum {
AHCI_CMD_CLR_BUSY   = (1 << 10),
 
RX_FIS_D2H_REG  = 0x40, /* offset of D2H Register FIS data */
+   RX_FIS_SDB  = 0x58, /* offset of SDB FIS data */
RX_FIS_UNK  = 0x60, /* offset of Unknown FIS data */
 
board_ahci  = 0,
@@ -202,6 +203,10 @@ struct ahci_port_priv {
dma_addr_t  cmd_tbl_dma;
void*rx_fis;
dma_addr_t  rx_fis_dma;
+   /* for NCQ spurious interrupt analysis */
+   int ncq_saw_spurious_sdb_cnt;
+   unsigned intncq_saw_d2h:1;
+   unsigned intncq_saw_dmas:1;
 };
 
 static u32 ahci_scr_read (struct ata_port *ap, unsigned int sc_reg);
@@ -1126,8 +1131,9 @@ static void ahci_host_intr(struct ata_port *ap)
void __iomem *mmio = ap->host->mmio_base;
void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
struct ata_eh_info *ehi = &ap->eh_info;
+   struct ahci_port_priv *pp = ap->private_data;
u32 status, qc_active;
-   int rc;
+   int rc, known_irq = 0;
 
status = readl(port_mmio + PORT_IRQ_STAT);
writel(status, port_mmio + PORT_IRQ_STAT);
@@ -1154,17 +1160,52 @@ static void ahci_host_intr(struct ata_port *ap)
 
/* hmmm... a spurious interupt */
 
-   /* some devices send D2H reg with I bit set during NCQ command phase */
-   if (ap->sactive && (status & PORT_IRQ_D2H_REG_FIS))
+   /* if !NCQ, ignore.  No modern ATA device has broken HSM
+* implementation for non-NCQ commands.
+*/
+   if (!ap->sactive)
return;
 
-   /* ignore interim PIO setup fis interrupts */
-   if (ata_tag_valid(ap->active_tag) && (status & PORT_IRQ_PIOS_FIS))
-   return;
+   if (status & PORT_IRQ_D2H_REG_FIS) {
+   if (!pp->ncq_saw_d2h)
+   ata_port_printk(ap, KERN_INFO,
+   "D2H reg with I during NCQ, "
+   "this message won't be printed again\n");
+   pp->ncq_saw_d2h = 1;
+   known_irq = 1;
+   }
+
+   if (status & PORT_IRQ_DMAS_FIS) {
+   if (!pp->ncq_saw_dmas)
+   ata_port_printk(ap, KERN_INFO,
+   "DMAS FIS during NCQ, "
+   "this message won't be printed again\n");
+   pp->ncq_saw_dmas = 1;
+   known_irq = 1;
+   }
+
+   if (status & PORT_IRQ_SDB_FIS &&
+  pp->ncq_saw_spurious_sdb_cnt < 10) {
+   /* SDB FIS containing spurious completions might be
+* dangerous, we need to know more about them.  Print
+* more of it.
+*/
+   const u32 *f = pp->rx_fis + RX_FIS_SDB;
+
+   ata_port_printk(ap, KERN_INFO, "Spurious SDB FIS during NCQ "
+   "issue=0x%x SAct=0x%x FIS=%08x:%08x%s\n",
+   readl(port_mmio + PORT_CMD_ISSUE),
+   readl(port_mmio + PORT_SCR_ACT), f[0], f[1],
+   pp->ncq_saw_spurious_sdb_cnt < 10 ?
+   "" : ", shutting up");
+
+   pp->ncq_saw_spurious_sdb_cnt++;
+   known_irq = 1;
+   }
 
-   if (ata_ratelimit())
+   if (!known_irq)
ata_port_printk(ap, KERN_INFO, "spurious interrupt "
-   "(irq_stat 0x%x active_tag %d sactive 0x%x)\n",
+   "(irq_stat 0x%x active_tag 0x%x sactive 
0x%x)\n",
status, ap->active_tag, ap->sactive);
 }
 
-
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

[PATCH] libata: implement ATA_FLAG_IGN_SIMPLEX and use it in sata_uli

[Tejun Heo]

Some uli controllers have stuck SIMPLEX bit which can't be cleared
with ata_pci_clear_simplex(), but the controller is capable of doing
DMAs on both channels simultaneously.  Implement ATA_FLAG_IGN_SIMPLEX
which makes libata ignore the simplex bit and use it in sata_uli.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
Jeff, implemented as ATA_FLAG_* as requested.  The patch is against
#upstream-fixes and should probably included in 2.6.20.  Thanks.

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 623cec9..71e6ddd 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -870,7 +870,8 @@ ata_pci_init_native_mode(struct pci_dev *pdev, struct 
ata_port_info **port, int
pci_resource_start(pdev, 1) | ATA_PCI_CTL_OFS;
bmdma = pci_resource_start(pdev, 4);
if (bmdma) {
-   if (inb(bmdma + 2) & 0x80)
+   if (!(port[p]->flags & ATA_FLAG_IGN_SIMPLEX) &&
+   inb(bmdma + 2) & 0x80)
probe_ent->_host_flags |= ATA_HOST_SIMPLEX;
probe_ent->port[p].bmdma_addr = bmdma;
}
@@ -886,7 +887,8 @@ ata_pci_init_native_mode(struct pci_dev *pdev, struct 
ata_port_info **port, int
bmdma = pci_resource_start(pdev, 4);
if (bmdma) {
bmdma += 8;
-   if(inb(bmdma + 2) & 0x80)
+   if (!(port[p]->flags & ATA_FLAG_IGN_SIMPLEX) &&
+   inb(bmdma + 2) & 0x80)
probe_ent->_host_flags |= ATA_HOST_SIMPLEX;
probe_ent->port[p].bmdma_addr = bmdma;
}
@@ -920,7 +922,8 @@ static struct ata_probe_ent 
*ata_pci_init_legacy_port(struct pci_dev *pdev,
probe_ent->port[0].ctl_addr = ATA_PRIMARY_CTL;
if (bmdma) {
probe_ent->port[0].bmdma_addr = bmdma;
-   if (inb(bmdma + 2) & 0x80)
+   if (!(port[0]->flags & ATA_FLAG_IGN_SIMPLEX) &&
+   inb(bmdma + 2) & 0x80)
probe_ent->_host_flags |= ATA_HOST_SIMPLEX;
}
ata_std_ports(&probe_ent->port[0]);
@@ -937,7 +940,8 @@ static struct ata_probe_ent 
*ata_pci_init_legacy_port(struct pci_dev *pdev,
probe_ent->port[1].ctl_addr = ATA_SECONDARY_CTL;
if (bmdma) {
probe_ent->port[1].bmdma_addr = bmdma + 8;
-   if (inb(bmdma + 10) & 0x80)
+   if (!(port[1]->flags & ATA_FLAG_IGN_SIMPLEX) &&
+   inb(bmdma + 10) & 0x80)
probe_ent->_host_flags |= ATA_HOST_SIMPLEX;
}
ata_std_ports(&probe_ent->port[1]);
diff --git a/drivers/ata/sata_uli.c b/drivers/ata/sata_uli.c
index 5c603ca..a43aec6 100644
--- a/drivers/ata/sata_uli.c
+++ b/drivers/ata/sata_uli.c
@@ -128,7 +128,8 @@ static const struct ata_port_operations uli_ops = {
 
 static struct ata_port_info uli_port_info = {
.sht= &uli_sht,
-   .flags  = ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
+   .flags  = ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+ ATA_FLAG_IGN_SIMPLEX,
.pio_mask   = 0x1f, /* pio0-4 */
.udma_mask  = 0x7f, /* udma0-6 */
.port_ops   = &uli_ops,
diff --git a/include/linux/libata.h b/include/linux/libata.h
index df76fc4..22aa69e 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -177,6 +177,7 @@ enum {
  * Register FIS clearing BSY */
ATA_FLAG_DEBUGMSG   = (1 << 13),
ATA_FLAG_SETXFER_POLLING= (1 << 14), /* use polling for SETXFER */
+   ATA_FLAG_IGN_SIMPLEX= (1 << 15), /* ignore SIMPLEX */
 
/* The following flag belongs to ap->pflags but is kept in
 * ap->flags because it's referenced in many LLDs and will be
-
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

[PATCH] libata: share PIO limits among devices sharing a channel

[Tejun Heo]

PIO xfermask limits should be shared by all devices on the same
channel to avoid violating device selection timing.  libata used to
guarantee this at core level but I mistakenly dropped the code during
conversion to new EH.  This patch revives that guarantee.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
Jeff, IMHO, this is a regression and should go into #upstream-fixes.
Thanks.

 drivers/ata/libata-core.c |   17 +
 1 file changed, 17 insertions(+)

Index: work/drivers/ata/libata-core.c
===
--- work.orig/drivers/ata/libata-core.c
+++ work/drivers/ata/libata-core.c
@@ -3390,6 +3390,7 @@ static void ata_dev_xfermask(struct ata_
struct ata_port *ap = dev->ap;
struct ata_host *host = ap->host;
unsigned long xfer_mask;
+   int i;
 
/* controller modes available */
xfer_mask = ata_pack_xfermask(ap->pio_mask,
@@ -3412,6 +3413,22 @@ static void ata_dev_xfermask(struct ata_
   dev->mwdma_mask, dev->udma_mask);
xfer_mask &= ata_id_xfermask(dev->id);
 
+   /* PIO xfermask limits are shared by all devices on the same
+* channel to avoid violating device selection timing.
+*/
+   for (i = 0; i < ATA_MAX_DEVICES; i++) {
+   struct ata_device *d = &ap->device[i];
+   unsigned int pio_mask;
+
+   if (ata_dev_absent(d))
+   continue;
+
+   ata_unpack_xfermask(ata_id_xfermask(d->id),
+   &pio_mask, NULL, NULL);
+   pio_mask &= d->pio_mask;
+   xfer_mask &= ata_pack_xfermask(pio_mask, UINT_MAX, UINT_MAX);
+   }
+
/*
 *  CFA Advanced TrueIDE timings are not allowed on a shared
 *  cable
-
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

[PATCH] sata_via: don't diddle with ATA_NIEN in ->freeze

[Tejun Heo]

vt6420 completely loses its ability to raise IRQ for ATAPI devices if
ATA_NIEN is diddled with in ->freeze.  Further investigation is
necessary to determine whether this problem is shared on other
controllers but it doesn't seem to be at this point.

Make vt6420's ->freeze only clear IRQ to fix this problem.  This makes
vt6420 relatively more prone to IRQ storms but the controller is way
too braindamaged to worry about that anyway.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
Jeff, this fix is verified to fix J. Taimr's case and a few other
reported that 2.6.20-rc5 fixed their ATAPI detection problems too.  We
still have several case to hunt down but getting verified fixes in
early helps separating different causes.  Please include this in
#upstream-fixes.  Thanks.

 drivers/ata/sata_via.c |   12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Index: work/drivers/ata/sata_via.c
===
--- work.orig/drivers/ata/sata_via.c
+++ work/drivers/ata/sata_via.c
@@ -74,6 +74,7 @@ enum {
 static int svia_init_one (struct pci_dev *pdev, const struct pci_device_id 
*ent);
 static u32 svia_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void svia_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
+static void svia_noop_freeze(struct ata_port *ap);
 static void vt6420_error_handler(struct ata_port *ap);
 
 static const struct pci_device_id svia_pci_tbl[] = {
@@ -128,7 +129,7 @@ static const struct ata_port_operations 
.qc_issue   = ata_qc_issue_prot,
.data_xfer  = ata_pio_data_xfer,
 
-   .freeze = ata_bmdma_freeze,
+   .freeze = svia_noop_freeze,
.thaw   = ata_bmdma_thaw,
.error_handler  = vt6420_error_handler,
.post_internal_cmd  = ata_bmdma_post_internal_cmd,
@@ -204,6 +205,15 @@ static void svia_scr_write (struct ata_p
outl(val, ap->ioaddr.scr_addr + (4 * sc_reg));
 }
 
+static void svia_noop_freeze(struct ata_port *ap)
+{
+   /* Some VIA controllers choke if ATA_NIEN is manipulated in
+* certain way.  Leave it alone and just clear pending IRQ.
+*/
+   ata_chk_status(ap);
+   ap->ops->irq_clear(ap);
+}
+
 /**
  * vt6420_prereset - prereset for vt6420
  * @ap: target ATA port
-
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: Linux 2.6 SATA NCQ support

[Tejun Heo]

V VS wrote:
> Can you  please give us your inputs on why the NCQ depth is not getting
> set correctly? Should we be looking at any other place for fixing this
> problem?

Please give a shot at 2.6.20-rc5.  There was a related bug fix for ahci.
 Thanks.

-- 
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: Linux 2.6 SATA NCQ support

[Tejun Heo]

V VS wrote:
> Thanks for your immediate response. Really appreciate it very much.
> 
> With the kind of output that 2.16.19.1  has displayed,
> should we be suspecting lack of NCQ support in the on-board SATA port or
> the hard disk or anywhere else?

Nope, just driver bug, I think.

> Have started the download of 2.6.20 now...

Good luck.

-- 
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] libata: share PIO limits among devices sharing a channel

[Tejun Heo]

Jeff Garzik wrote:
> Alan wrote:
>> On Thu, 25 Jan 2007 20:29:47 +0900
>> Tejun Heo <[EMAIL PROTECTED]> wrote:
>>
>>> PIO xfermask limits should be shared by all devices on the same
>>> channel to avoid violating device selection timing.  libata used to
>>
>> NAK, this is totally wrong

Eek.. You actually said this is/was documented and relied upon all over.

http://thread.gmane.org/gmane.linux.ide/13184/focus=14486

(searching mailboxes more...)  Oh, and the dropping of common PIO mode
selection was agreed upon by Jeff and Alan.

One way or the other, this only affects PATA devices and as long as all
PATA LLDs guarantee that selection timing isn't violated, this can go.
I thought PATA drivers still relied on this common denominator PIO mode
configuration.  I think it's Alan's call.

> I'm curious what the motivation of this patch was?

Hope my intentions are explained.

Thanks.

-- 
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] libata: share PIO limits among devices sharing a channel

[Tejun Heo]

Alan wrote:
 NAK, this is totally wrong
>> Eek.. You actually said this is/was documented and relied upon all over.
>>
>> http://thread.gmane.org/gmane.linux.ide/13184/focus=14486
> 
> What I said (well tried to say since it was apparently vague and caused
> confusion) was relied upon was the hardware being in PIO0 during probe
> (because we don't call the PIO setup methods during probe/reset) and that
> we decide which modes both devices are using *before* we set either of
> them (which the current code appears to do correctly). This is because
> the device may need to know both to apply limits - such as merging
> address setup time.

I see.

>>> I'm curious what the motivation of this patch was?
>> Hope my intentions are explained.
> 
> Yep. I think it's a simple case of confusion about the meaning of an
> email.

Okay, then.  We can just ignore this whole thread.  Thanks for clarifying.

-- 
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

[PATCH] ahci: fix endianness in spurious interrupt message

[Tejun Heo]

Fix endianness in spurious interrupt message.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 2fe5a58..d8f0ce9 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1178,7 +1178,8 @@ static void ahci_host_intr(struct ata_port *ap)
ata_port_printk(ap, KERN_INFO, "Spurious SDB FIS during NCQ "
"issue=0x%x SAct=0x%x FIS=%08x:%08x%s\n",
readl(port_mmio + PORT_CMD_ISSUE),
-   readl(port_mmio + PORT_SCR_ACT), f[0], f[1],
+   readl(port_mmio + PORT_SCR_ACT),
+   le32_to_cpu(f[0]), le32_to_cpu(f[1]),
pp->ncq_saw_spurious_sdb_cnt < 10 ?
"" : ", shutting up");
 
-
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

[PATCH] sata_via: style clean up, no indirect method call in LLD

[Tejun Heo]

Call ata_bmdma_irq_clear() directly instead of through
ap->ops->irq_clear() according to libata style guideline.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>

diff --git a/drivers/ata/sata_via.c b/drivers/ata/sata_via.c
index 55b0123..d3d5c0d 100644
--- a/drivers/ata/sata_via.c
+++ b/drivers/ata/sata_via.c
@@ -211,7 +211,7 @@ static void svia_noop_freeze(struct ata_port *ap)
 * certain way.  Leave it alone and just clear pending IRQ.
 */
ata_chk_status(ap);
-   ap->ops->irq_clear(ap);
+   ata_bmdma_irq_clear(ap);
 }
 
 /**
-
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

[PATCH] ahci: use 0x80 as wait stat value instead of 0xff

[Tejun Heo]

Before hardreset, ahci initialized stat part of received FIS area to
0xff to wait for the first D2H Reg FIS which would change the value to
device ready state.  This used to work but now libata considers status
value of 0xff as device not present making this wait prone to failure.

This patch makes ahci use 0x80 for the wait stat value instead of
0xff to fix the above problem.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>

 drivers/ata/ahci.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: work/drivers/ata/ahci.c
===
--- work.orig/drivers/ata/ahci.c
+++ work/drivers/ata/ahci.c
@@ -915,7 +915,7 @@ static int ahci_hardreset(struct ata_por
 
/* clear D2H reception area to properly wait for D2H FIS */
ata_tf_init(ap->device, &tf);
-   tf.command = 0xff;
+   tf.command = 0x80;
ata_tf_to_fis(&tf, d2h_fis, 0);
 
rc = sata_std_hardreset(ap, class);
-
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

[PATCH] libata: fix ata_eh_suspend() return value

[Tejun Heo]

ata_eh_suspend() was returning 0 regardless of failure.  This bug has
potential to lose data on suspend.  Fix it.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
Jeff, this one is definitely for 2.6.20.  Thanks.

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index eee3525..52c85af 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1791,7 +1791,7 @@ static int ata_eh_suspend(struct ata_port *ap, struct 
ata_device **r_failed_dev)
*r_failed_dev = dev;
 
DPRINTK("EXIT\n");
-   return 0;
+   return rc;
 }
 
 /**
-
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] ahci: use 0x80 as wait stat value instead of 0xff

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> Before hardreset, ahci initialized stat part of received FIS area to
>> 0xff to wait for the first D2H Reg FIS which would change the value to
>> device ready state.  This used to work but now libata considers status
>> value of 0xff as device not present making this wait prone to failure.
>>
>> This patch makes ahci use 0x80 for the wait stat value instead of
>> 0xff to fix the above problem.
>>
>> Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
>>
>>  drivers/ata/ahci.c |2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> For the future, please make sure to include the "---" terminator prior
> to the diffstat, otherwise it will get copied into the kernel changelog.

Argh... sure.  My mistake.  Sorry.

-- 
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

[PATCH] ahci: port_no should be used when clearing IRQ in ahci_thaw()

[Tejun Heo]

ap->id is logcial port ID which is unique among all ATA ports and
doesn't have anything to do with hardware port index.  ap->port_no is
the hardware port index and thus should be used when clearing IRQ mask
in ahci_thaw().

This problem has been spotted by Jeff Garzik <[EMAIL PROTECTED]>.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index ab73b0a..7f1bf85 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1281,7 +1281,7 @@ static void ahci_thaw(struct ata_port *ap)
/* clear IRQ */
tmp = readl(port_mmio + PORT_IRQ_STAT);
writel(tmp, port_mmio + PORT_IRQ_STAT);
-   writel(1 << ap->id, mmio + HOST_IRQ_STAT);
+   writel(1 << ap->port_no, mmio + HOST_IRQ_STAT);
 
/* turn IRQ back on */
writel(DEF_PORT_IRQ, port_mmio + PORT_IRQ_MASK);
-
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: sata_promise TX2/TX4 cards rejecting smart auto-offline & auto-save ata cmds?

[Tejun Heo]

Jim Paris wrote:
> Tejun Heo wrote:
>> Andrew Paprocki wrote:
>>> On 10/30/06, Tejun Heo <[EMAIL PROTECTED]> wrote:
>>>> Please read the following thread.
>>>>
>>>> http://thread.gmane.org/gmane.linux.ide/13222/focus=13235
>>> Worked like a charm! Just patched up smartmontools now and I don't get
>>> the failed cmds anymore. Hope Bruce takes that patch soon.
>> Bruce took the patch and it will be included in the next version of 
>> smartmontools.  :-)
> 
> Hello Tejun, Bruce,
> 
> Sorry to bring up an old thread, but it looks like this patch did not
> actually make it into smartmontools 5.37 or CVS.  Tejun, could you
> please take another look and update the patch?  There's a new user of
> STRANGE_BUFFER_LENGTH and I don't know enough about the code to fix
> it properly.

I definitely can do that but I guess we need Bruce's confirmation about
the patch first.  Bruce?

-- 
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: sil_24 PMP performance issue.

[Tejun Heo]

Richard Scobie wrote:
> 
> Hi Tejun,
> 
> Did you get any response back from Silicon Image?
> 
> 
> Reference this thread:
> 
> http://marc.theaimsgroup.com/?l=linux-ide&m=116727812518914&w=2

Haven't found the time to work on PMP yet.  Sorry but you need to me
more patient.  :-)

-- 
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: sata driver fails to work after resume from S3

[Tejun Heo]

Zhang Rui wrote:
> But how to make SATA drive S3 work properly in 2.6.16.33?

There is no simple patch which can fix S3 support in 2.6.16.  You need
to backport whole libata EH including PM support.  If you absolutely
have to keep using 2.6.16.33, consider backporting libata as a whole
from 2.6.19 (it wouldn't be easy unless you're familiar with libata and
SCSI).  Otherwise, just use kernel >= 2.6.18.

-- 
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: [2.6.18,19] SATA boot problems (ICH6/ICH6W)

[Tejun Heo]

Hello, Gary.

Gary Hade wrote:
>>> If they verify your fix (ie,
>>> GoVault sometimes take more than 150ms to transmit the first D2H Reg FIs
>>> after SRST), I'll push similar patch upstream.
>> Thanks.  If you think that changes to increase the delays are
>> the way to go (at least until we can find a better solution)
>> I can provide patches.
> 
> Tejun, 
> I haven't heard anything from you on this so I'm including a delay
> increase patch against 2.6.20-rc6 for the 'ata-piix' case below.  
> I hope that you, Jeff, and others find this acceptable.

Sorry about being unresponsive.  The thing is that the change adds
unnecessary 2 secs of delay to a lot of other normal device-not-present
cases, so I was hesitant to ack the patch.  I'll give it more thoughts
(and respond timely this time :-)

> With respect to the 'ahci' case w/2.6.20-rc6 the GoVault device is 
> useable following boot although the below messages are being logged 
> during initialization.  Please let me know if you have any thoughts 
> on this.  
>   scsi1 : ahci
>   ata2: softreset failed (port busy but CLO unavailable)
>   ata2: softreset failed, retrying in 5 secs
>   ata2: port is slow to respond, please be patient (Status 0x80)
>   ata2: port failed to respond (30 secs, Status 0x80)
>   ata2: COMRESET failed (device not ready)
>   ata2: hardreset failed, retrying in 5 secs
>   ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>   ata2.00: ATAPI, max UDMA/66
>   ata2.00: configured for UDMA/66

The above should have been fixed in 2.6.20-rc6.  Please test it.  It was
caused by the ahci driver incorrectly clearing ahci CAP register and
fixed recently.

Thanks.

-- 
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: mm snapshot broken-out-2007-01-26-00-36.tar.gz uploaded

[Tejun Heo]

Acked-by: Tejun Heo <[EMAIL PROTECTED]>

Please repost with proper subject.

http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt

Thanks.

-- 
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 1/1] libata: rearrange dmesg info to add full ATA revision

[Tejun Heo]

Hello, Eric.

Eric D. Mudama wrote:
> Per Jeff's suggestion, this patch rearranges the info printed for ATA
> drives into dmesg to add the full ATA firmware revision and model
> information, while keeping the output to 2 lines.
> 
> Signed-off-by: Eric D. Mudama <[EMAIL PROTECTED]>

The patch is formatted and applies perfectly.  I'm glad to see this
change.  Just a few nits below.

>   char revbuf[7]; /* XYZ-99\0 */
> + char fwrevbuf[9];
> + char modelbuf[41];

Please use ATA_ID_FW_REV_LEN + 1 and ATA_ID_PROD_LEN + 1.

> @@ -1680,13 +1692,18 @@ int ata_dev_configure(struct ata_device *dev)
>   ata_dev_config_ncq(dev, ncq_desc, sizeof(ncq_desc));
>  
>   /* print device info to dmesg */
> - if (ata_msg_drv(ap) && print_info)
> - ata_dev_printk(dev, KERN_INFO, "%s, "
> - "max %s, %Lu sectors: %s %s\n",
> + if (ata_msg_drv(ap) && print_info) {
> + ata_dev_printk(dev, KERN_INFO,
> + "%s: %s, %s, max %s\n",
>   revbuf,
> - ata_mode_string(xfer_mask),
> + modelbuf, fwrevbuf,

Please merge the above line with the line above it.

> + ata_mode_string(xfer_mask));
> + ata_dev_printk(dev, KERN_INFO,
> + "%Lu sectors, multi %u: %s %s\n",
>   (unsigned long long)dev->n_sectors,
> + dev->multi_count,
>   lba_desc, ncq_desc);

Ditto.

> + }
>   } else {
>   /* CHS */
>  
> @@ -1703,22 +1720,19 @@ int ata_dev_configure(struct ata_device *dev)
>   }
>  
>   /* print device info to dmesg */
> - if (ata_msg_drv(ap) && print_info)
> - ata_dev_printk(dev, KERN_INFO, "%s, "
> - "max %s, %Lu sectors: CHS %u/%u/%u\n",
> + if (ata_msg_drv(ap) && print_info) {
> + ata_dev_printk(dev, KERN_INFO,
> + "%s: %s, %s, max %s\n",
>   revbuf,
> - ata_mode_string(xfer_mask),
> + modelbuf, fwrevbuf,

Ditto.

> + ata_mode_string(xfer_mask));
> + ata_dev_printk(dev, KERN_INFO, 
> + "%Lu sectors, multi %u, CHS %u/%u/%u\n",
>   (unsigned long long)dev->n_sectors,
> +     dev->multi_count,
>   dev->cylinders, dev->heads,
>   dev->sectors);

I would prefer

dev->multi_count, dev->cylinders,
dev->heads, dev->sectors

Other than these,

Acked-by: Tejun Heo <[EMAIL PROTECTED]>

-- 
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 1/1] libata: rearrange dmesg info to add full ATA revision

[Tejun Heo]

[The previous replay mysteriously didn't include Eric in To:, sorry,
quoting whole message here.]

Tejun Heo wrote:
> Hello, Eric.
> 
> Eric D. Mudama wrote:
>> Per Jeff's suggestion, this patch rearranges the info printed for ATA
>> drives into dmesg to add the full ATA firmware revision and model
>> information, while keeping the output to 2 lines.
>>
>> Signed-off-by: Eric D. Mudama <[EMAIL PROTECTED]>
> 
> The patch is formatted and applies perfectly.  I'm glad to see this
> change.  Just a few nits below.
> 
>>  char revbuf[7]; /* XYZ-99\0 */
>> +char fwrevbuf[9];
>> +char modelbuf[41];
> 
> Please use ATA_ID_FW_REV_LEN + 1 and ATA_ID_PROD_LEN + 1.

This depends on to which version this patch applies.  For
#upstream-fixes (2.6.20-rc6), you have to use raw numbers as you did.
For #upstream, there are above two constants to use.  I think this patch
is good for 2.6.20 as it's safe && will help us analyzing bug reports
for 2.6.20.  So, ignore this part of the comment.

Jeff, if it gets merged into #upstream through #upstream-fixes, please
don't forget to change above constants.

>> @@ -1680,13 +1692,18 @@ int ata_dev_configure(struct ata_device *dev)
>>  ata_dev_config_ncq(dev, ncq_desc, sizeof(ncq_desc));
>>  
>>  /* print device info to dmesg */
>> -if (ata_msg_drv(ap) && print_info)
>> -ata_dev_printk(dev, KERN_INFO, "%s, "
>> -"max %s, %Lu sectors: %s %s\n",
>> +if (ata_msg_drv(ap) && print_info) {
>> +ata_dev_printk(dev, KERN_INFO,
>> +"%s: %s, %s, max %s\n",
>>  revbuf,
>> -ata_mode_string(xfer_mask),
>> +modelbuf, fwrevbuf,
> 
> Please merge the above line with the line above it.
> 
>> +ata_mode_string(xfer_mask));
>> +ata_dev_printk(dev, KERN_INFO,
>> +"%Lu sectors, multi %u: %s %s\n",
>>  (unsigned long long)dev->n_sectors,
>> +dev->multi_count,
>>  lba_desc, ncq_desc);
> 
> Ditto.
> 
>> +}
>>  } else {
>>  /* CHS */
>>  
>> @@ -1703,22 +1720,19 @@ int ata_dev_configure(struct ata_device *dev)
>>  }
>>  
>>  /* print device info to dmesg */
>> -if (ata_msg_drv(ap) && print_info)
>> -ata_dev_printk(dev, KERN_INFO, "%s, "
>> -"max %s, %Lu sectors: CHS %u/%u/%u\n",
>> +if (ata_msg_drv(ap) && print_info) {
>> +ata_dev_printk(dev, KERN_INFO,
>> +"%s: %s, %s, max %s\n",
>>  revbuf,
>> -ata_mode_string(xfer_mask),
>> +modelbuf, fwrevbuf,
> 
> Ditto.
> 
>> +ata_mode_string(xfer_mask));
>> +ata_dev_printk(dev, KERN_INFO, 
>> +"%Lu sectors, multi %u, CHS %u/%u/%u\n",
>>  (unsigned long long)dev->n_sectors,
>> +dev->multi_count,
>>  dev->cylinders, dev->heads,
>>  dev->sectors);
> 
> I would prefer
> 
> dev->multi_count, dev->cylinders,
> dev->heads, dev->sectors
> 
> Other than these,
> 
> Acked-by: Tejun Heo <[EMAIL PROTECTED]>
> 

-- 
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: [2.6.18,19] SATA boot problems (ICH6/ICH6W)

[Tejun Heo]

Gary Hade wrote:
> Some of my random thoughts:
> There does appear to be this invalid assumption that 0xFF status 
> always implies device-not-present.  The status register access 
> restrictions in ATA/ATAPI-7 V1 5.14.2 include the statement "The 
> contents of this register, except for BSY, shall be ignored when 
> BSY is set to one." which the code does not honor.  There is apparently 
> past experience that 0xFF status implies device-not-present for some
> controllers (the odd clowns :) but I have no idea how common these are.

The 0xff is the value we get when there is no device present and the
motherboard manufacturer forgot to pull down the ATA bus.  It's not very
uncommon in cheap PATA world and, following the weird tradition, some
SATA controllers choose to emulate 0xff if there is no device attached
(link not established).  Not sure how many of them does it but intel's
SATA chipset is one of them, so we're pretty much stuck with it.

ie. In many P/SATA setups, your patch would add 2 extra secs of waiting
for empty ports.

> We obviously can't get rid of the check but since we cannot clear
> the read-only status register and there appears to be no specification 
> dictated upper limit on how long it should take for a software reset to 
> complete it just seems like we need to wait long enough to support the 
> slowest known device which may be the GoVault.

Agreed but still hesitant to ack the patch.  :-)

I'm gonna work on parallel probing for libata.  I think we can easily
hide extra 2 secs of waiting with parallel probing.  It will take some
time but that seems to be the 'right' thing to do especially considering
the fact that 150ms sleep has been enough for gazillions of ATA devices
during last decade except for this GoVault drive.

I'll leave this thread in my to-do folder and apply your patch after
parallel probing is in place (optimistic ETA 1 month).  How does that sound?

Thanks.

-- 
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: [2.6.18,19] SATA boot problems (ICH6/ICH6W)

[Tejun Heo]

Jeff Garzik wrote:
> SRST is specified to take no longer than 31 seconds from the clearing to
> the SRST bit to the clearing of the BSY bit.
> 
> Look through the software reset protocol documentation (ATA/ATAPI-7
> volume 2), mainly the device state machines.

Jeff, this is different delay.  We're short circuiting the 31sec wait
when the reported status is 0xff to avoid wasting half a minute on port
which has no device but is showing 0xff because bus datalines are not
pulled down (PATA) or the specific controller is made to emulate 0xff
when link isn't established yet (SATA).

This never matters for PATA devices because no sane device reports 0xff
as the status register value regardless of its state - 0xff is the
special value indicating no device attached and data bus is floating,
and 150ms is more than enough to allow the device to report its status
register value (the spec says >= 2ms).

Some SATA controllers use 0xff to indicate empty port.  This seldomly
matters as we have the almighty SStatus register to check device
presence (there is a bug regarding this, patch pending).

This GoVault drive fails because ata_piix doesn't have SCR while using
0xff to indicate port not ready (dunno exact which state causes 0xff
status tho) while the GoVault drive fails to clear that state in 150ms
(not 30s).  The libata sees 0xff after SRST if GoVault drive is attached
and thinks that the port is empty.  So, I'm afraid there is no easy way
out here but to wait longer for 0xff to clear.  As Kovid suggested just
a few secs should be enough.

Thanks.

-- 
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: [2.6.18,19] SATA boot problems (ICH6/ICH6W)

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> I'm gonna work on parallel probing for libata.  I think we can easily
>> hide extra 2 secs of waiting with parallel probing.  It will take some
>> time but that seems to be the 'right' thing to do especially considering
>> the fact that 150ms sleep has been enough for gazillions of ATA devices
>> during last decade except for this GoVault drive.
> 
> 
> FWIW I don't mind polling for a /maximum/ of $N seconds, but I want to
> stress that any patch that blatantly adds stuff like
> 
> if (status == 0xff)
> mdelay(2000)
> 
> will be rejected.

Aye aye captain.  I wouldn't do that myself either.

-- 
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: [2.6.18,19] SATA boot problems (ICH6/ICH6W)

[Tejun Heo]

Alan wrote:
>> Some SATA controllers use 0xff to indicate empty port.  This seldomly
>> matters as we have the almighty SStatus register to check device
>> presence (there is a bug regarding this, patch pending).
>>
>> This GoVault drive fails because ata_piix doesn't have SCR while using
>> 0xff to indicate port not ready (dunno exact which state causes 0xff
>> status tho) while the GoVault drive fails to clear that state in 150ms
>> (not 30s).  The libata sees 0xff after SRST if GoVault drive is attached
> 
> So we can also cut this down by only doing the extra polling on a device
> which is SATA and lacks SCR ?

That's true but the offending one is ata_piix, so the cutting down is
not as effective.  If we can live with the extra two secs per empty port
for some of ata_piix for the time being (maybe two or three releases),
the delay can be added now.  One more thing to consider is GoVault is
the only known device to show this behavior till now.

Hmm... What do you (Alan and Jeff) think?

-- 
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

[PATCH] iomap: iomap should be in obj-y not in lib-y

[Tejun Heo]

devres change moved iomap.o from obj-$(CONFIG_GENERIC_IOMAP) to lib-y
making it not linked if no in-kernel driver uses it.  Fix it.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>

diff --git a/lib/Makefile b/lib/Makefile
index 29b2e99..3b605da 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -5,14 +5,14 @@
 lib-y := ctype.o string.o vsprintf.o cmdline.o \
 bust_spinlocks.o rbtree.o radix-tree.o dump_stack.o \
 idr.o div64.o int_sqrt.o bitmap.o extable.o prio_tree.o \
-sha1.o irq_regs.o reciprocal_div.o iomap.o
+sha1.o irq_regs.o reciprocal_div.o
 
 lib-$(CONFIG_MMU) += ioremap.o
 lib-$(CONFIG_SMP) += cpumask.o
 
 lib-y  += kobject.o kref.o kobject_uevent.o klist.o
 
-obj-y += sort.o parser.o halfmd4.o debug_locks.o random32.o
+obj-y += sort.o parser.o halfmd4.o debug_locks.o random32.o iomap.o
 
 ifeq ($(CONFIG_DEBUG_KOBJECT),y)
 CFLAGS_kobject.o += -DDEBUG
-
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] iomap: iomap should be in obj-y not in lib-y

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> devres change moved iomap.o from obj-$(CONFIG_GENERIC_IOMAP) to lib-y
>> making it not linked if no in-kernel driver uses it.  Fix it.
>>
>> Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
> 
> applied
> 
> BTW can you update your scripts to email [EMAIL PROTECTED] ?  That's the
> primary email address I signoff with, etc.

Okay, done.

> [EMAIL PROTECTED] will continue to work for all eternity, but it makes
> Mozilla Thunderbird stupidly add an unneeded CC.

Also, you can avoid that by configuring aliases to the thunderbird
account.  It's in account setting -> Manage Identities.  Thungerbird is
pretty neat.  :-)

-- 
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] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Tejun Heo]

Mark Lord wrote:
> In an ideal world, we would use the existing ATA_12 opcode
> to issue 12-byte ATA passthrough commands for libata ATAPI drives
> from userspace.
> 
> But ATA_12 happens to have the same SCSI opcode value as the older
> CD/RW "BLANK" command, widely used by cdrecord and friends.
> 
> So, to achieve ATA passthru capability for libata ATAPI,
> we have to instead use the ATA_16 opcode: a 16-byte command.
> 
> SCSI normally disallows issuing 16-byte commands to 12-byte devices,
> so special support has to be added for this.
> 
> Introduce an "allow_ata_16" boolean to the scsi_host struct.
> This provides a means for libata to signal that 16-byte ATA_16
> commands should be permitted even for 12-byte ATAPI devices.
> 
> There are companion patches submitted separately
> to add ATAPI ATA_16 capability to libata.
> 
> Signed-off-by:  Mark Lord <[EMAIL PROTECTED]>

I might have missed the discussion but can't we just set
host->max_cmd_len to 16 unconditionally?  libata is emulating a SCSI
device anyway and, from SCSI's POV, the situation is that any libata
ATAPI device can do both 12 and 16 byte commands while some of them only
allow allow ATA_16 for 16 byte command.

Also, it's not like host->max_cmd_len gives any guaranteed protection
against CDB length.  Being a 'host' limit, it's set to the highest
number in the host.  In that respect, it should be set to 16 too.  All
ATA hosts can do 16 byte CDBs.

Thanks.

-- 
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] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Tejun Heo]

Mark Lord wrote:
> Tejun Heo wrote:
>> Mark Lord wrote:
>> ..
>>> So, to achieve ATA passthru capability for libata ATAPI,
>>> we have to instead use the ATA_16 opcode: a 16-byte command.
>>>
>>> SCSI normally disallows issuing 16-byte commands to 12-byte devices,
>>> so special support has to be added for this.
> 
>> I might have missed the discussion but can't we just set
>> host->max_cmd_len to 16 unconditionally?
> 
> Sure thing, if you and Jeff are happy with that, then lets do it.
> 
> I just kind of assumed that the complexity in ata_set_port_max_cmd_len()
> was there for some kind of reason.
> 
> For example, I think all existing ATAPI drives only speak 12-byte packet
> protocols, and so if we tell SCSI we're good for 16-byte, then won't the
> SCSI layer suddenly start sending us READ_16 and the like?

SCSI always uses the smallest command it can use, so we're safe.  Most
other commands are issued directly from the userland and it's their
responsibility not to feed disallowed commands to a device (or we need
more advanced filter).  Anyways, this has never been guaranteed because
the limit is host wide.

So, I'm for setting it to 16.  Jeff, what do you think?

-- 
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: VIA AHCI NCQ [was: Re: Recent AHCI drivers do not work with VIA VT8251 controllers (regression in 2.6.19 from 2.6.18)]

[Tejun Heo]

Simon Farnsworth wrote:
> On Tuesday 30 January 2007 15:53, Simon Farnsworth wrote:
>> Hello,
>>
>> I've got a VIA VT8251 AHCI controller, driving two Maxtor SATA disks.
>> Under Linux 2.6.18.5, it works; using 2.6.19.1, it fails to work (bug
>> report at http://bugzilla.kernel.org/show_bug.cgi?id=7589 - note that
>> pci=nomsi does not fix things for me, as I still get no interrupts).
>>
> I've been going through 2.6.20-rc releases this evening. 2.6.20-rc7 resolves 
> my AHCI fault (although not the MSI problem); it looks like Tejun Heo's fix 
> to ahci_thaw() in a718728f9e40ec79c0879ec6509a54fee214f5b2 fixes things for 
> my setup.

Yeah, that was my mistake spotted by Jeff.  IRQ pending was never
cleared properly so the controller couldn't raise it.

> Of course, I'm now curious as to why NCQ is disabled on VIA AHCI controllers. 
> Is there a known hardware bug I should be wary of, or is it just a case of 
> being cautious due to the other issues with the VT8251 (in which case I'll 
> try enabling it and reporting back)?

Yeap, there is a known NCQ problem with vt8251.  Can't find the original
bug report now tho.

commit 71f0737b2889b86f774a94afaf1342c2c0d61cb5
tree e983d3f4c3fccad0679aa8ae18a0c3ce91bfafe7
parent 8fa29b23d9e0ef976dc578aab98297d4f24f70da
author Tejun Heo <[EMAIL PROTECTED]> 1150899168 +0900
committer Jeff Garzik <[EMAIL PROTECTED]> 1151032959 -0400

[PATCH] ahci: disable NCQ support on vt8251

vt8251 chokes on NCQ commands.  Two different disks from different
vendors are showing the same symptom and it seems that the windows
driver from via doesn't support NCQ either.  Disable NCQ support on
this controller for the time being.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
Cc: Aalderd Bouwman <[EMAIL PROTECTED]>
Cc: Bastiaan Jacques <[EMAIL PROTECTED]>
Signed-off-by: Jeff Garzik <[EMAIL PROTECTED]>

-- 
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 1/1] libata: Initialize nbytes for internal sg commands

[Tejun Heo]

Brian King wrote:
> Some LLDDs, like ipr, use nbytes and pad_len to determine
> the total data transfer length of a command. Make sure
> nbytes gets initialized for internally generated commands.
> 
> Signed-off-by: Brian King <[EMAIL PROTECTED]>

Acked-by: Tejun Heo <[EMAIL PROTECTED]>

-- 
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] libata: fix translation for START STOP UNIT

[Tejun Heo]

Mark Lord wrote:
> Robert Hancock wrote:
>> I'd love to, but unfortunately nobody seems to have come up with a way
>> of doing this in Thunderbird that keeps it from mangling whitespace 
> 
> Yup, major nuisance.  I have to fire up Kmail whenever I'm posting patches,
> and then go back to Thunderbird again afterwards.

A much better solution with thunderbird is using external editor
extension.

http://globs.org/articles.php?lng=en&pg=2

Install the extension.  Configure it with your favorite editor.
Thunderbird won't do a thing about your message if you turn off
linewrap.

 For example, this line starts with a tab and a space and (double tab)  
is very long and I'm writing in emacs fired up from thunderbird. 
(double space)  

Cheers.

-- 
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

[PATCH 1/2] pata_platform: fix devres conversion

[Tejun Heo]

devres updates for pata_platform were dropped while merging devres
patches due to merge conflict.  This is the updated version.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/pata_platform.c |   31 +++
 1 file changed, 7 insertions(+), 24 deletions(-)

Index: work/drivers/ata/pata_platform.c
===
--- work.orig/drivers/ata/pata_platform.c
+++ work/drivers/ata/pata_platform.c
@@ -47,23 +47,6 @@ static int pata_platform_set_mode(struct
return 0;
 }
 
-static void pata_platform_host_stop(struct ata_host *host)
-{
-   int i;
-
-   /*
-* Unmap the bases for MMIO
-*/
-   for (i = 0; i < host->n_ports; i++) {
-   struct ata_port *ap = host->ports[i];
-
-   if (ap->flags & ATA_FLAG_MMIO) {
-   iounmap((void __iomem *)ap->ioaddr.ctl_addr);
-   iounmap((void __iomem *)ap->ioaddr.cmd_addr);
-   }
-   }
-}
-
 static struct scsi_host_template pata_platform_sht = {
.module = THIS_MODULE,
.name   = DRV_NAME,
@@ -106,8 +89,6 @@ static struct ata_port_operations pata_p
.irq_clear  = ata_bmdma_irq_clear,
 
.port_start = ata_port_start,
-   .port_stop  = ata_port_stop,
-   .host_stop  = pata_platform_host_stop
 };
 
 static void pata_platform_setup_port(struct ata_ioports *ioaddr,
@@ -209,15 +190,17 @@ static int __devinit pata_platform_probe
if (mmio) {
ae.port_flags |= ATA_FLAG_MMIO;
 
-   ae.port[0].cmd_addr = (unsigned long)ioremap(io_res->start,
-   io_res->end - io_res->start + 1);
+   ae.port[0].cmd_addr = (unsigned long)
+   devm_ioremap(&pdev->dev, io_res->start,
+io_res->end - io_res->start + 1);
if (unlikely(!ae.port[0].cmd_addr)) {
dev_err(&pdev->dev, "failed to remap IO base\n");
return -ENXIO;
}
 
-   ae.port[0].ctl_addr = (unsigned long)ioremap(ctl_res->start,
-   ctl_res->end - ctl_res->start + 1);
+   ae.port[0].ctl_addr = (unsigned long)
+   devm_ioremap(&pdev->dev, ctl_res->start,
+ctl_res->end - ctl_res->start + 1);
if (unlikely(!ae.port[0].ctl_addr)) {
dev_err(&pdev->dev, "failed to remap CTL base\n");
ret = -ENXIO;
@@ -261,7 +244,7 @@ static int __devexit pata_platform_remov
struct device *dev = &pdev->dev;
struct ata_host *host = dev_get_drvdata(dev);
 
-   ata_host_remove(host);
+   ata_host_detach(host);
dev_set_drvdata(dev, NULL);
 
return 0;
-
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] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Tejun Heo]

Jeff Garzik wrote:
> Mark Lord wrote:
>> For example, I think all existing ATAPI drives only speak 12-byte packet
>> protocols, and so if we tell SCSI we're good for 16-byte, then won't the
>> SCSI layer suddenly start sending us READ_16 and the like?
> 
> Speaking strictly about the device, IDENTIFY PACKET DEVICE data page
> tells us whether the device supports 12-byte or 16-byte CDBs.  No need
> to guess.
> 
> Some host controllers only support 12-byte, but I think that most should
> support 16-byte.

Out of curiosity, does ATA controllers which don't allow 16byte CDB
actually exist?  It's transferred using PIO protocol anyway.

-- 
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] RESEND: SCSI, libata: add support for ATA_16 commands to libata ATAPI devices

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> SCSI always uses the smallest command it can use, so we're safe.  Most
>> other commands are issued directly from the userland and it's their
>> responsibility not to feed disallowed commands to a device (or we need
>> more advanced filter).  Anyways, this has never been guaranteed because
>> the limit is host wide.
>>
>> So, I'm for setting it to 16.  Jeff, what do you think?
> 
> 
> Like I just noted in another email, the limit is really on the /device/
> side.  In theory the user could plug in a 16-byte ATAPI device and a
> 12-byte ATAPI device to the same host.
> 
> We should be able to safely raise the limit to 16-byte for most host
> controllers.  Note I said "most".  The bitch will be figuring out which
> host controllers do not like 16-byte CDBs.

Well, it's not any worse than what we're currently doing.  We don't set
host cdb len limit according to the host.  We set it to the largest
value among the attached devices.  So, if there is a 12 byte only
controller out there and if you connect 16 byte ATAPI device to it,
you're screwed already and will continue to be screwed after the change.

Note that raising host cdb limit to 16 doesn't make anybody issue 16
byte cdb to the device.  The only unconditionally allowed 16 byte cdb is
ATA_16 which is executed by libata SAT and thus doesn't pass through the
host.

-- 
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: Serious bug in sata_sil module in 2.6.19.2?

[Tejun Heo]

Florian Effenberger wrote:
> Hi there,
> 
> I am not sure if the sata_sil-Module is the culprit, but I think so. We
> ran a 2.6.15.1 kernel for months without any problems. Now I tried to
> switch to 2.6.19.2, and every night when our backup routine runs, the
> machine locks up. No log entry, no automatic reboot
> (/proc/sys/kernel/panic is set to 30 [sec]).
> 
> The display looks like if there was a video error, lots of bad and
> flickering characters, but it is pretty hard to read the error message.
> I have an image and a video of that, if you need it. The only thing I
> could read is something with "ata". After the reboot, the RAID devices
> are out of sync and need to resync again.
> 
> Booting up 2.6.15.1 on the same machine cures the problem, everything
> works just fine, no lockups.
> 
> Is there any chance of debugging that problem, of finding the culprit?
> 
> Any help would be greatly appreciated!

sata_sil hasn't seen as much change as other drivers and is one of the
more stable ones.  I'm definitely interested.  Please post the pics.  If
the video contains useful info, can you host it somewhere?

-- 
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

[PATCH libata-dev#upstream-fixes] ahci/pata_jmicron: fix JMicron quirk

[Tejun Heo]

For all JMicrons except for 361 and 368, AHCI mode enable bits in the
Control(1) should be set.  This used to be done in both ahci and
pata_jmicron but while moving programming to PCI quirk, it was removed
from ahci part while still left in pata_jmicron.

The implemented JMicron PCI quirk was incorrect in that it didn't
program AHCI mode enable bits.  If pata_jmicron is loaded first and
programs those bits, the ahci ports work; otherwise, ahci device
detection fails miserably.

This patch makes JMicron PCI quirk clear SATA IDE mode bits and set
AHCI mode bits and remove the respective part from pata_jmicron.
Tested on JMB361, 363 and 368.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
Jeff, gotta be included in 2.6.20.
Justin, is this the problem you were seeing on suse?

diff --git a/drivers/ata/pata_jmicron.c b/drivers/ata/pata_jmicron.c
index 2d406a7..26365c1 100644
--- a/drivers/ata/pata_jmicron.c
+++ b/drivers/ata/pata_jmicron.c
@@ -202,20 +202,12 @@ static int jmicron_init_one (struct pci_dev *pdev, const 
struct pci_device_id *i
 
u32 reg;
 
-   if (id->driver_data != 368) {
-   /* Put the controller into AHCI mode in case the AHCI driver
-  has not yet been loaded. This can be done with either
-  function present */
+   /* PATA controller is fn 1, AHCI is fn 0 */
+   if (id->driver_data != 368 && PCI_FUNC(pdev->devfn) != 1)
+   return -ENODEV;
 
-   /* FIXME: We may want a way to override this in future */
-   pci_write_config_byte(pdev, 0x41, 0xa1);
-
-   /* PATA controller is fn 1, AHCI is fn 0 */
-   if (PCI_FUNC(pdev->devfn) != 1)
-   return -ENODEV;
-   }
-   if ( id->driver_data == 365 || id->driver_data == 366) {
-   /* The 365/66 have two PATA channels, redirect the second */
+   /* The 365/66 have two PATA channels, redirect the second */
+   if (id->driver_data == 365 || id->driver_data == 366) {
pci_read_config_dword(pdev, 0x80, ®);
reg |= (1 << 24);   /* IDE1 to PATA IDE secondary */
pci_write_config_dword(pdev, 0x80, reg);
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 5c34379..6f3e1bf 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1260,8 +1260,8 @@ static void quirk_jmicron_dualfn(struct pci_dev *pdev)
pci_read_config_dword(pdev, 0x40, &conf);
/* Enable dual function mode, AHCI on fn 0, IDE fn1 */
/* Set the class codes correctly and then direct IDE 0 
*/
-   conf &= ~0x000F0200;/* Clear bit 9 and 16-19 */
-   conf |=  0x00C20002;/* Set bit 1, 17, 22, 23 */
+   conf &= ~0x000FF200; /* Clear bit 9 and 12-19 */
+   conf |=  0x00C2A102; /* Set 1, 8, 13, 15, 17, 22, 23 */
pci_write_config_dword(pdev, 0x40, conf);
 
/* Reconfigure so that the PCI scanner discovers the
-
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

[PATCH] libata: add 150ms between completion of hardreset and status checking

[Tejun Heo]

Follow the old SRST rule and delay 150ms between completion of
hardreset and status checking.  Debouncing delay should usually cover
this but debounce duration could be shorter than 150ms under certain
circumstances.

Usefulness depends on host controller implementation but it can't hurt
and serves as a reminder that 2s delay for GoVault should also be
added here.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/libata-core.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index f210dbd..582e44d 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3014,6 +3014,9 @@ int sata_std_hardreset(struct ata_port *ap, unsigned int 
*class)
return 0;
}
 
+   /* wait a while before checking status, see SRST for more info */
+   msleep(150);
+
if (ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT)) {
ata_port_printk(ap, KERN_ERR,
"COMRESET failed (device not ready)\n");
-- 
1.4.4.4

-
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

[PATCH] sata_inic162x: fix a few glitches in hardreset

[Tejun Heo]

* Hardreset must not exit without actually performing reset regardless
  of link status.  We're resetting the link after all.

* Minor message update.

* 150ms delay is meaningful iff link is online after reset is
  complete.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/sata_inic162x.c |   12 
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/ata/sata_inic162x.c b/drivers/ata/sata_inic162x.c
index b2a6f77..fad2bd3 100644
--- a/drivers/ata/sata_inic162x.c
+++ b/drivers/ata/sata_inic162x.c
@@ -429,11 +429,6 @@ static int inic_hardreset(struct ata_port *ap, unsigned 
int *class)
/* hammer it into sane state */
inic_reset_port(port_base);
 
-   if (ata_port_offline(ap)) {
-   *class = ATA_DEV_NONE;
-   return 0;
-   }
-
val = readw(idma_ctl);
writew(val | IDMA_CTL_RST_ATA, idma_ctl);
readw(idma_ctl);/* flush */
@@ -443,16 +438,17 @@ static int inic_hardreset(struct ata_port *ap, unsigned 
int *class)
rc = sata_phy_resume(ap, timing);
if (rc) {
ata_port_printk(ap, KERN_WARNING, "failed to resume "
-   "link for reset (errno=%d)\n", rc);
+   "link after reset (errno=%d)\n", rc);
return rc;
}
 
-   msleep(150);
-
*class = ATA_DEV_NONE;
if (ata_port_online(ap)) {
struct ata_taskfile tf;
 
+   /* wait a while before checking status */
+   msleep(150);
+
if (ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT)) {
ata_port_printk(ap, KERN_WARNING,
"device busy after hardreset\n");
-- 
1.4.4.4

-
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

[PATCH] libata: reduce ATA command timeout to 7secs

[Tejun Heo]

Both ATA and ATAPI devices used the default timeouts defined by SCSI
high level driver.  For both disks and ODDs, it was 30secs, which was
way too long for disks.  This patch makes most ATA commands time out
after 7secs - the de facto ATA command timeout, while leaving ATAPI
timeout at 30secs.

Note that both normal commands and EH commands timeouts are adjusted
to 7 secs, but cache flushses still have 30sec timeout.  This is a
side effect of the way sd makes use of sdev->timeout, but this is a
good side effect in that ATA spec requires cache flushes are given
longer timeout.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
I think we've waited enough.  Combined with EH updates which will soon
be posted, this should make life much easier (well, at least
responsive) when something goes wrong with ATA.

 drivers/ata/libata-core.c |2 +-
 drivers/ata/libata-scsi.c |6 ++
 include/linux/libata.h|4 ++--
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 582e44d..a441c75 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -89,7 +89,7 @@ int libata_fua = 0;
 module_param_named(fua, libata_fua, int, 0444);
 MODULE_PARM_DESC(fua, "FUA support (0=off, 1=on)");
 
-static int ata_probe_timeout = ATA_TMOUT_INTERNAL / HZ;
+static int ata_probe_timeout = ATA_TMOUT_CMD / HZ;
 module_param(ata_probe_timeout, int, 0444);
 MODULE_PARM_DESC(ata_probe_timeout, "Set ATA probing timeout (seconds)");
 
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 0009818..dc2157c 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -903,6 +903,12 @@ static void ata_scsi_dev_config(struct scsi_device *sdev,
depth = min(ATA_MAX_QUEUE - 1, depth);
scsi_adjust_queue_depth(sdev, MSG_SIMPLE_TAG, depth);
}
+
+   /* setup command timeout */
+   if (dev->class == ATA_DEV_ATA)
+   sdev->timeout = ATA_TMOUT_CMD;
+   else
+   sdev->timeout = ATAPI_TMOUT_CMD;
 }
 
 /**
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 305e95f..4c3ed59 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -214,8 +214,8 @@ enum {
/* various lengths of time */
ATA_TMOUT_BOOT  = 30 * HZ,  /* heuristic */
ATA_TMOUT_BOOT_QUICK= 7 * HZ,   /* heuristic */
-   ATA_TMOUT_INTERNAL  = 30 * HZ,
-   ATA_TMOUT_INTERNAL_QUICK = 5 * HZ,
+   ATA_TMOUT_CMD   = 7 * HZ,   /* de facto ATA cmd timeout */
+   ATAPI_TMOUT_CMD = 30 * HZ,
 
/* ATA bus states */
BUS_UNKNOWN = 0,
-- 
1.4.4.4

-
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

[PATCH 1/5] libata: improve ata_down_xfermask_limit()

[Tejun Heo]

Make ata_down_xfermask_limit() accept @sel instead of @force_pio0.
@sel selects how the xfermask limit will be adjusted.  The following
selectors are defined.

* ATA_DNXFER_PIO: only speed down PIO
* ATA_DNXFER_DMA: only speed down DMA, don't cause transfer mode change
* ATA_DNXFER_40C: apply 40c cable limit
* ATA_DNXFER_FORCE_PIO  : force PIO
* ATA_DNXFER_FORCE_PIO0 : force PIO0 (same as original with @force_pio0 == 1)
* ATA_DNXFER_ANY: same as original with @force_pio0 == 0

Currently, only ANY and FORCE_PIO0 are used to maintain the original
behavior.  Other selectors will be used later to improve EH speed down
sequence.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/libata-core.c |  105 ++---
 drivers/ata/libata-eh.c   |9 +++-
 drivers/ata/libata.h  |   12 +-
 3 files changed, 96 insertions(+), 30 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index a441c75..417394b 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1747,6 +1747,7 @@ int ata_bus_probe(struct ata_port *ap)
int tries[ATA_MAX_DEVICES];
int i, rc, down_xfermask;
struct ata_device *dev;
+   int dnxfer_sel;
 
ata_port_probe(ap);
 
@@ -1828,13 +1829,15 @@ int ata_bus_probe(struct ata_port *ap)
/* fall through */
default:
tries[dev->devno]--;
-   if (down_xfermask &&
-   ata_down_xfermask_limit(dev, tries[dev->devno] == 1))
+   dnxfer_sel = ATA_DNXFER_ANY;
+   if (tries[dev->devno] == 1)
+   dnxfer_sel = ATA_DNXFER_FORCE_PIO0;
+   if (down_xfermask && ata_down_xfermask_limit(dev, dnxfer_sel))
tries[dev->devno] = 0;
}
 
if (!tries[dev->devno]) {
-   ata_down_xfermask_limit(dev, 1);
+   ata_down_xfermask_limit(dev, ATA_DNXFER_FORCE_PIO0);
ata_dev_disable(dev);
}
 
@@ -2267,7 +2270,7 @@ int ata_timing_compute(struct ata_device *adev, unsigned 
short speed,
 /**
  * ata_down_xfermask_limit - adjust dev xfer masks downward
  * @dev: Device to adjust xfer masks
- * @force_pio0: Force PIO0
+ * @sel: ATA_DNXFER_* selector
  *
  * Adjust xfer masks of @dev downward.  Note that this function
  * does not apply the change.  Invoking ata_set_mode() afterwards
@@ -2279,37 +2282,87 @@ int ata_timing_compute(struct ata_device *adev, 
unsigned short speed,
  * RETURNS:
  * 0 on success, negative errno on failure
  */
-int ata_down_xfermask_limit(struct ata_device *dev, int force_pio0)
+int ata_down_xfermask_limit(struct ata_device *dev, unsigned int sel)
 {
-   unsigned long xfer_mask;
-   int highbit;
+   char buf[32];
+   unsigned int orig_mask, xfer_mask;
+   unsigned int pio_mask, mwdma_mask, udma_mask;
+   int quiet, highbit;
 
-   xfer_mask = ata_pack_xfermask(dev->pio_mask, dev->mwdma_mask,
- dev->udma_mask);
+   quiet = !!(sel & ATA_DNXFER_QUIET);
+   sel &= ~ATA_DNXFER_QUIET;
 
-   if (!xfer_mask)
-   goto fail;
-   /* don't gear down to MWDMA from UDMA, go directly to PIO */
-   if (xfer_mask & ATA_MASK_UDMA)
-   xfer_mask &= ~ATA_MASK_MWDMA;
+   xfer_mask = orig_mask = ata_pack_xfermask(dev->pio_mask,
+ dev->mwdma_mask,
+ dev->udma_mask);
+   ata_unpack_xfermask(xfer_mask, &pio_mask, &mwdma_mask, &udma_mask);
 
-   highbit = fls(xfer_mask) - 1;
-   xfer_mask &= ~(1 << highbit);
-   if (force_pio0)
-   xfer_mask &= 1 << ATA_SHIFT_PIO;
-   if (!xfer_mask)
-   goto fail;
+   switch (sel) {
+   case ATA_DNXFER_PIO:
+   highbit = fls(pio_mask) - 1;
+   pio_mask &= ~(1 << highbit);
+   break;
+
+   case ATA_DNXFER_DMA:
+   if (udma_mask) {
+   highbit = fls(udma_mask) - 1;
+   udma_mask &= ~(1 << highbit);
+   if (!udma_mask)
+   return -ENOENT;
+   } else if (mwdma_mask) {
+   highbit = fls(mwdma_mask) - 1;
+   mwdma_mask &= ~(1 << highbit);
+   if (!mwdma_mask)
+   return -ENOENT;
+   }
+   break;
+
+   case ATA_DNXFER_40C:
+   udma_mask &= ATA_UDMA_MASK_40C;
+   break;
+
+   case ATA_DNXFER_FORCE_PIO0:
+   pio_mask &= 1;
+   case ATA_DNXFER_FORCE_PIO:
+   mwdma_mask = 0;
+   udma_mask = 0

[PATCH 2/5] libata: improve probe failure handling

[Tejun Heo]

* Move forcing device to PIO0 on device disable into
  ata_dev_disable().  This makes both old and new EHs act the same
  way.

* Speed down only PIO mode on probe failure.  All commands used during
  probing are PIO commands.  There's no point in speeding down DMA.

* Retry at least once after -ENODEV.  Some devices report garbled
  IDENTIFY data after certain events.  This shouldn't cause device
  detach and re-attach.

* Rearrange EH failure path for simplicity.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/libata-core.c |   39 +++
 drivers/ata/libata-eh.c   |   35 ++-
 2 files changed, 37 insertions(+), 37 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 417394b..cbd4bf1 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -596,6 +596,8 @@ void ata_dev_disable(struct ata_device *dev)
 {
if (ata_dev_enabled(dev) && ata_msg_drv(dev->ap)) {
ata_dev_printk(dev, KERN_WARNING, "disabled\n");
+   ata_down_xfermask_limit(dev, ATA_DNXFER_FORCE_PIO0 |
+ATA_DNXFER_QUIET);
dev->class++;
}
 }
@@ -1745,9 +1747,8 @@ int ata_bus_probe(struct ata_port *ap)
 {
unsigned int classes[ATA_MAX_DEVICES];
int tries[ATA_MAX_DEVICES];
-   int i, rc, down_xfermask;
+   int i, rc;
struct ata_device *dev;
-   int dnxfer_sel;
 
ata_port_probe(ap);
 
@@ -1755,8 +1756,6 @@ int ata_bus_probe(struct ata_port *ap)
tries[i] = ATA_PROBE_MAX_TRIES;
 
  retry:
-   down_xfermask = 0;
-
/* reset and determine device classes */
ap->ops->phy_reset(ap);
 
@@ -1804,10 +1803,8 @@ int ata_bus_probe(struct ata_port *ap)
 
/* configure transfer mode */
rc = ata_set_mode(ap, &dev);
-   if (rc) {
-   down_xfermask = 1;
+   if (rc)
goto fail;
-   }
 
for (i = 0; i < ATA_MAX_DEVICES; i++)
if (ata_dev_enabled(&ap->device[i]))
@@ -1819,27 +1816,29 @@ int ata_bus_probe(struct ata_port *ap)
return -ENODEV;
 
  fail:
+   tries[dev->devno]--;
+
switch (rc) {
case -EINVAL:
-   case -ENODEV:
+   /* eeek, something went very wrong, give up */
tries[dev->devno] = 0;
break;
+
+   case -ENODEV:
+   /* give it just one more chance */
+   tries[dev->devno] = min(tries[dev->devno], 1);
case -EIO:
-   sata_down_spd_limit(ap);
-   /* fall through */
-   default:
-   tries[dev->devno]--;
-   dnxfer_sel = ATA_DNXFER_ANY;
-   if (tries[dev->devno] == 1)
-   dnxfer_sel = ATA_DNXFER_FORCE_PIO0;
-   if (down_xfermask && ata_down_xfermask_limit(dev, dnxfer_sel))
-   tries[dev->devno] = 0;
+   if (tries[dev->devno] == 1) {
+   /* This is the last chance, better to slow
+* down than lose it.
+*/
+   sata_down_spd_limit(ap);
+   ata_down_xfermask_limit(dev, ATA_DNXFER_PIO);
+   }
}
 
-   if (!tries[dev->devno]) {
-   ata_down_xfermask_limit(dev, ATA_DNXFER_FORCE_PIO0);
+   if (!tries[dev->devno])
ata_dev_disable(dev);
-   }
 
goto retry;
 }
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 7b61562..1abfdba 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1964,8 +1964,7 @@ static int ata_eh_recover(struct ata_port *ap, 
ata_prereset_fn_t prereset,
 {
struct ata_eh_context *ehc = &ap->eh_context;
struct ata_device *dev;
-   int down_xfermask, i, rc;
-   int dnxfer_sel;
+   int i, rc;
 
DPRINTK("ENTER\n");
 
@@ -1994,7 +1993,6 @@ static int ata_eh_recover(struct ata_port *ap, 
ata_prereset_fn_t prereset,
}
 
  retry:
-   down_xfermask = 0;
rc = 0;
 
/* if UNLOADING, finish immediately */
@@ -2039,10 +2037,8 @@ static int ata_eh_recover(struct ata_port *ap, 
ata_prereset_fn_t prereset,
/* configure transfer mode if necessary */
if (ehc->i.flags & ATA_EHI_SETMODE) {
rc = ata_set_mode(ap, &dev);
-   if (rc) {
-   down_xfermask = 1;
+   if (rc)
goto dev_fail;
-   }
ehc->i.flags &= ~ATA_EHI_SETMODE;
}
 
@@ -2054,22 +2050,27 @@ static int ata_eh_recover(struct ata_port *ap, 
ata_prereset_fn_t prereset,
goto out;
 
  dev_fail:
+   ehc->tries[dev->devno]--;
+
switch (rc) {
-   case -

[PATCH 3/5] libata: put some intelligence into EH speed down sequence

[Tejun Heo]

The current EH speed down code is more of a proof that the EH
framework is capable of adjusting transfer speed in response to error.
This patch puts some intelligence into EH speed down sequence.  The
rules are..

* If there have been more than three timeout, HSM violation or
  unclassified DEV errors for known supported commands during last 10
  mins, NCQ is turned off.

* If there have been more than three timeout or HSM violation for known
  supported command, transfer mode is slowed down.  If DMA is active,
  it is first slowered by one grade (e.g. UDMA133->100).  If that
  doesn't help, it's slowered to 40c limit (UDMA33).  If PIO is
  active, it's slowered by one grade first.  If that doesn't help,
  PIO0 is forced.  Note that this rule does not change transfer mode.
  DMA is never degraded into PIO by this rule.

* If there have been more than ten ATA bus, timeout, HSM violation or
  unclassified device errors for known supported commands && speeding
  down DMA mode didn't help, the device is forced into PIO mode.  Note
  that this rule is considered only for PATA devices and is pretty
  difficult to trigger.

One error can only trigger one rule at a time.  After a rule is
triggered, error history is cleared such that the next speed down
happens only after some number of errors are accumulated.  This makes
sense because now speed down is done in bigger stride.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/libata-eh.c |  184 +++---
 include/linux/libata.h  |1 +
 2 files changed, 125 insertions(+), 60 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 1abfdba..3173862 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -44,6 +44,12 @@
 
 #include "libata.h"
 
+enum {
+   ATA_EH_SPDN_NCQ_OFF = (1 << 0),
+   ATA_EH_SPDN_SPEED_DOWN  = (1 << 1),
+   ATA_EH_SPDN_FALLBACK_TO_PIO = (1 << 2),
+};
+
 static void __ata_port_freeze(struct ata_port *ap);
 static void ata_eh_finish(struct ata_port *ap);
 static void ata_eh_handle_port_suspend(struct ata_port *ap);
@@ -65,12 +71,9 @@ static void ata_ering_record(struct ata_ering *ering, int 
is_io,
ent->timestamp = get_jiffies_64();
 }
 
-static struct ata_ering_entry * ata_ering_top(struct ata_ering *ering)
+static void ata_ering_clear(struct ata_ering *ering)
 {
-   struct ata_ering_entry *ent = &ering->ring[ering->cursor];
-   if (!ent->err_mask)
-   return NULL;
-   return ent;
+   memset(ering, 0, sizeof(*ering));
 }
 
 static int ata_ering_map(struct ata_ering *ering,
@@ -1159,87 +1162,99 @@ static unsigned int ata_eh_analyze_tf(struct 
ata_queued_cmd *qc,
return action;
 }
 
-static int ata_eh_categorize_ering_entry(struct ata_ering_entry *ent)
+static int ata_eh_categorize_error(int is_io, unsigned int err_mask)
 {
-   if (ent->err_mask & (AC_ERR_ATA_BUS | AC_ERR_TIMEOUT))
+   if (err_mask & AC_ERR_ATA_BUS)
return 1;
 
-   if (ent->is_io) {
-   if (ent->err_mask & AC_ERR_HSM)
-   return 1;
-   if ((ent->err_mask &
-(AC_ERR_DEV|AC_ERR_MEDIA|AC_ERR_INVALID)) == AC_ERR_DEV)
+   if (err_mask & AC_ERR_TIMEOUT)
+   return 2;
+
+   if (is_io) {
+   if (err_mask & AC_ERR_HSM)
return 2;
+   if ((err_mask &
+(AC_ERR_DEV|AC_ERR_MEDIA|AC_ERR_INVALID)) == AC_ERR_DEV)
+   return 3;
}
 
return 0;
 }
 
-struct speed_down_needed_arg {
+struct speed_down_verdict_arg {
u64 since;
-   int nr_errors[3];
+   int nr_errors[4];
 };
 
-static int speed_down_needed_cb(struct ata_ering_entry *ent, void *void_arg)
+static int speed_down_verdict_cb(struct ata_ering_entry *ent, void *void_arg)
 {
-   struct speed_down_needed_arg *arg = void_arg;
+   struct speed_down_verdict_arg *arg = void_arg;
+   int cat = ata_eh_categorize_error(ent->is_io, ent->err_mask);
 
if (ent->timestamp < arg->since)
return -1;
 
-   arg->nr_errors[ata_eh_categorize_ering_entry(ent)]++;
+   arg->nr_errors[cat]++;
return 0;
 }
 
 /**
- * ata_eh_speed_down_needed - Determine wheter speed down is necessary
+ * ata_eh_speed_down_verdict - Determine speed down verdict
  * @dev: Device of interest
  *
  * This function examines error ring of @dev and determines
- * whether speed down is necessary.  Speed down is necessary if
- * there have been more than 3 of Cat-1 errors or 10 of Cat-2
- * errors during last 15 minutes.
+ * whether NCQ needs to be turned off, transfer speed should be
+ * stepped down, or falling back to PIO is necessary.
+ *
+ * Cat-1 is 

[PATCHSET] ata_piix: improve combined mode handling

[Tejun Heo]

Subject: [PATCHSET] put some intelligence into speed down sequence

Hello,

The current EH speed down code is more of a feature demonstration and
goes through rdiculously many meaningless steps when condition is met.
This patchset tries to put some intelligence into speed down sequence.
The goal is to achieve reasonable number of speed down steps
reasonably spaced from one another and consider NCQ, cable type and
the current protocol when determining speed down steps, while not
bloating the code too much with nitty gritty details.

Roughly, the rules are...

1. If NCQ and protocol/timeout/unknown dev errors occur, turn off NCQ

2. If excessive transfer errors occur, speed down within the current
   transfer mode (UDMA/MWDMA/PIO).  If UDMA, it's first adjusted down
   a step, if error conditions persist, 40c limit is applied.  Speed
   down is done only twice.

3. If PATA && used up all DMA speed down steps && a LOT of
   transmission/unknown errors occur, switch to PIO.  So, we never
   automatically step down to PIO on SATA.  This is intended.  Some
   SATA hdd even seems to have problem with PIO data transfer
   commands.

The last patch makes ahci report HSM violation error on spurious
completion of NCQ commands, thus causing NCQ off after several such
incidents.  These drives should be blacklisted for DMA eventually.

This patchset is against...

  upstream (eb0e63cca36a3389f0ccab4584f6d479b983fad5)
+ [1] pata_platform-fix-devres-conversion
+ [2] libata-convert-to-iomap

Ric, I guess this resolves the to-do item from you which has been
sitting in my mailbox for way too long.  What do you think about the
rules?

Thanks.

--
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

[PATCH 4/5] libata: kill ATA_DNXFER_ANY

[Tejun Heo]

ATA_DNXFER_ANY isn't used anymore.  Kill it.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/libata-core.c |9 -
 drivers/ata/libata.h  |1 -
 2 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index cbd4bf1..651ab4b 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2327,15 +2327,6 @@ int ata_down_xfermask_limit(struct ata_device *dev, 
unsigned int sel)
udma_mask = 0;
break;
 
-   case ATA_DNXFER_ANY:
-   /* don't gear down to MWDMA from UDMA, go directly to PIO */
-   if (xfer_mask & ATA_MASK_UDMA)
-   xfer_mask &= ~ATA_MASK_MWDMA;
-
-   highbit = fls(xfer_mask) - 1;
-   xfer_mask &= ~(1 << highbit);
-   break;
-
default:
BUG();
}
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 36e08d2..6fc6260 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -48,7 +48,6 @@ enum {
ATA_DNXFER_40C  = 2,/* apply 40c cable limit */
ATA_DNXFER_FORCE_PIO= 3,/* force PIO */
ATA_DNXFER_FORCE_PIO0   = 4,/* force PIO0 */
-   ATA_DNXFER_ANY  = 5,/* speed down any */
 
ATA_DNXFER_QUIET= (1 << 31),
 };
-- 
1.4.4.4


-
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

[PATCH 5/5] ahci: consider SDB FIS containing spurious NCQ completions HSM violation

[Tejun Heo]

SDB FIS containing spurious NCQ completions is a clear protocol
violation.  Currently, only some Maxtors with early firmware revisions
are showing this problem.  Those firmwares have other NCQ related
problems including buggy NCQ error reporting and occasional lock up
after NCQ errors.

Consider spurious NCQ completions HSM violation and freeze the port
after it.  EH will turn off NCQ after this happens several times.
Eventually drives which show this behavior should be blacklisted for
NCQ.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/ahci.c |   26 +-
 1 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 8f1d753..1d8d377 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -199,7 +199,6 @@ struct ahci_port_priv {
void*rx_fis;
dma_addr_t  rx_fis_dma;
/* for NCQ spurious interrupt analysis */
-   int ncq_saw_spurious_sdb_cnt;
unsigned intncq_saw_d2h:1;
unsigned intncq_saw_dmas:1;
 };
@@ -1157,23 +1156,24 @@ static void ahci_host_intr(struct ata_port *ap)
known_irq = 1;
}
 
-   if (status & PORT_IRQ_SDB_FIS &&
-  pp->ncq_saw_spurious_sdb_cnt < 10) {
+   if (status & PORT_IRQ_SDB_FIS) {
/* SDB FIS containing spurious completions might be
-* dangerous, we need to know more about them.  Print
-* more of it.
+* dangerous, whine and fail commands with HSM
+* violation.  EH will turn off NCQ after several such
+* failures.
 */
const u32 *f = pp->rx_fis + RX_FIS_SDB;
 
-   ata_port_printk(ap, KERN_INFO, "Spurious SDB FIS during NCQ "
-   "issue=0x%x SAct=0x%x FIS=%08x:%08x%s\n",
-   readl(port_mmio + PORT_CMD_ISSUE),
-   readl(port_mmio + PORT_SCR_ACT),
-   le32_to_cpu(f[0]), le32_to_cpu(f[1]),
-   pp->ncq_saw_spurious_sdb_cnt < 10 ?
-   "" : ", shutting up");
+   ata_ehi_push_desc(ehi, "spurious completion during NCQ "
+ "issue=0x%x SAct=0x%x FIS=%08x:%08x",
+ readl(port_mmio + PORT_CMD_ISSUE),
+ readl(port_mmio + PORT_SCR_ACT),
+ le32_to_cpu(f[0]), le32_to_cpu(f[1]));
+
+   ehi->err_mask |= AC_ERR_HSM;
+   ehi->action |= ATA_EH_SOFTRESET;
+   ata_port_freeze(ap);
 
-   pp->ncq_saw_spurious_sdb_cnt++;
known_irq = 1;
}
 
-- 
1.4.4.4


-
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

Subject should be [PATCHSET] put some intelligence into speed down sequence

[Tejun Heo]

Eek, sorry.

-- 
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

Please ignore this thread. I'll resend.

[Tejun Heo]

Tejun Heo wrote:
> Eek, sorry.
> 


-- 
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

[PATCH 5/5] ahci: consider SDB FIS containing spurious NCQ completions HSM violation

[Tejun Heo]

SDB FIS containing spurious NCQ completions is a clear protocol
violation.  Currently, only some Maxtors with early firmware revisions
are showing this problem.  Those firmwares have other NCQ related
problems including buggy NCQ error reporting and occasional lock up
after NCQ errors.

Consider spurious NCQ completions HSM violation and freeze the port
after it.  EH will turn off NCQ after this happens several times.
Eventually drives which show this behavior should be blacklisted for
NCQ.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/ahci.c |   26 +-
 1 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 8f1d753..1d8d377 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -199,7 +199,6 @@ struct ahci_port_priv {
void*rx_fis;
dma_addr_t  rx_fis_dma;
/* for NCQ spurious interrupt analysis */
-   int ncq_saw_spurious_sdb_cnt;
unsigned intncq_saw_d2h:1;
unsigned intncq_saw_dmas:1;
 };
@@ -1157,23 +1156,24 @@ static void ahci_host_intr(struct ata_port *ap)
known_irq = 1;
}
 
-   if (status & PORT_IRQ_SDB_FIS &&
-  pp->ncq_saw_spurious_sdb_cnt < 10) {
+   if (status & PORT_IRQ_SDB_FIS) {
/* SDB FIS containing spurious completions might be
-* dangerous, we need to know more about them.  Print
-* more of it.
+* dangerous, whine and fail commands with HSM
+* violation.  EH will turn off NCQ after several such
+* failures.
 */
const u32 *f = pp->rx_fis + RX_FIS_SDB;
 
-   ata_port_printk(ap, KERN_INFO, "Spurious SDB FIS during NCQ "
-   "issue=0x%x SAct=0x%x FIS=%08x:%08x%s\n",
-   readl(port_mmio + PORT_CMD_ISSUE),
-   readl(port_mmio + PORT_SCR_ACT),
-   le32_to_cpu(f[0]), le32_to_cpu(f[1]),
-   pp->ncq_saw_spurious_sdb_cnt < 10 ?
-   "" : ", shutting up");
+   ata_ehi_push_desc(ehi, "spurious completion during NCQ "
+ "issue=0x%x SAct=0x%x FIS=%08x:%08x",
+ readl(port_mmio + PORT_CMD_ISSUE),
+ readl(port_mmio + PORT_SCR_ACT),
+ le32_to_cpu(f[0]), le32_to_cpu(f[1]));
+
+   ehi->err_mask |= AC_ERR_HSM;
+   ehi->action |= ATA_EH_SOFTRESET;
+   ata_port_freeze(ap);
 
-   pp->ncq_saw_spurious_sdb_cnt++;
known_irq = 1;
}
 
-- 
1.4.4.4


-
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

[PATCH 2/5] libata: improve probe failure handling

[Tejun Heo]

* Move forcing device to PIO0 on device disable into
  ata_dev_disable().  This makes both old and new EHs act the same
  way.

* Speed down only PIO mode on probe failure.  All commands used during
  probing are PIO commands.  There's no point in speeding down DMA.

* Retry at least once after -ENODEV.  Some devices report garbled
  IDENTIFY data after certain events.  This shouldn't cause device
  detach and re-attach.

* Rearrange EH failure path for simplicity.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/libata-core.c |   39 +++
 drivers/ata/libata-eh.c   |   35 ++-
 2 files changed, 37 insertions(+), 37 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 417394b..cbd4bf1 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -596,6 +596,8 @@ void ata_dev_disable(struct ata_device *dev)
 {
if (ata_dev_enabled(dev) && ata_msg_drv(dev->ap)) {
ata_dev_printk(dev, KERN_WARNING, "disabled\n");
+   ata_down_xfermask_limit(dev, ATA_DNXFER_FORCE_PIO0 |
+ATA_DNXFER_QUIET);
dev->class++;
}
 }
@@ -1745,9 +1747,8 @@ int ata_bus_probe(struct ata_port *ap)
 {
unsigned int classes[ATA_MAX_DEVICES];
int tries[ATA_MAX_DEVICES];
-   int i, rc, down_xfermask;
+   int i, rc;
struct ata_device *dev;
-   int dnxfer_sel;
 
ata_port_probe(ap);
 
@@ -1755,8 +1756,6 @@ int ata_bus_probe(struct ata_port *ap)
tries[i] = ATA_PROBE_MAX_TRIES;
 
  retry:
-   down_xfermask = 0;
-
/* reset and determine device classes */
ap->ops->phy_reset(ap);
 
@@ -1804,10 +1803,8 @@ int ata_bus_probe(struct ata_port *ap)
 
/* configure transfer mode */
rc = ata_set_mode(ap, &dev);
-   if (rc) {
-   down_xfermask = 1;
+   if (rc)
goto fail;
-   }
 
for (i = 0; i < ATA_MAX_DEVICES; i++)
if (ata_dev_enabled(&ap->device[i]))
@@ -1819,27 +1816,29 @@ int ata_bus_probe(struct ata_port *ap)
return -ENODEV;
 
  fail:
+   tries[dev->devno]--;
+
switch (rc) {
case -EINVAL:
-   case -ENODEV:
+   /* eeek, something went very wrong, give up */
tries[dev->devno] = 0;
break;
+
+   case -ENODEV:
+   /* give it just one more chance */
+   tries[dev->devno] = min(tries[dev->devno], 1);
case -EIO:
-   sata_down_spd_limit(ap);
-   /* fall through */
-   default:
-   tries[dev->devno]--;
-   dnxfer_sel = ATA_DNXFER_ANY;
-   if (tries[dev->devno] == 1)
-   dnxfer_sel = ATA_DNXFER_FORCE_PIO0;
-   if (down_xfermask && ata_down_xfermask_limit(dev, dnxfer_sel))
-   tries[dev->devno] = 0;
+   if (tries[dev->devno] == 1) {
+   /* This is the last chance, better to slow
+* down than lose it.
+*/
+   sata_down_spd_limit(ap);
+   ata_down_xfermask_limit(dev, ATA_DNXFER_PIO);
+   }
}
 
-   if (!tries[dev->devno]) {
-   ata_down_xfermask_limit(dev, ATA_DNXFER_FORCE_PIO0);
+   if (!tries[dev->devno])
ata_dev_disable(dev);
-   }
 
goto retry;
 }
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 7b61562..1abfdba 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1964,8 +1964,7 @@ static int ata_eh_recover(struct ata_port *ap, 
ata_prereset_fn_t prereset,
 {
struct ata_eh_context *ehc = &ap->eh_context;
struct ata_device *dev;
-   int down_xfermask, i, rc;
-   int dnxfer_sel;
+   int i, rc;
 
DPRINTK("ENTER\n");
 
@@ -1994,7 +1993,6 @@ static int ata_eh_recover(struct ata_port *ap, 
ata_prereset_fn_t prereset,
}
 
  retry:
-   down_xfermask = 0;
rc = 0;
 
/* if UNLOADING, finish immediately */
@@ -2039,10 +2037,8 @@ static int ata_eh_recover(struct ata_port *ap, 
ata_prereset_fn_t prereset,
/* configure transfer mode if necessary */
if (ehc->i.flags & ATA_EHI_SETMODE) {
rc = ata_set_mode(ap, &dev);
-   if (rc) {
-   down_xfermask = 1;
+   if (rc)
goto dev_fail;
-   }
ehc->i.flags &= ~ATA_EHI_SETMODE;
}
 
@@ -2054,22 +2050,27 @@ static int ata_eh_recover(struct ata_port *ap, 
ata_prereset_fn_t prereset,
goto out;
 
  dev_fail:
+   ehc->tries[dev->devno]--;
+
switch (rc) {
-   case -

[PATCH 1/5] libata: improve ata_down_xfermask_limit()

[Tejun Heo]

Make ata_down_xfermask_limit() accept @sel instead of @force_pio0.
@sel selects how the xfermask limit will be adjusted.  The following
selectors are defined.

* ATA_DNXFER_PIO: only speed down PIO
* ATA_DNXFER_DMA: only speed down DMA, don't cause transfer mode change
* ATA_DNXFER_40C: apply 40c cable limit
* ATA_DNXFER_FORCE_PIO  : force PIO
* ATA_DNXFER_FORCE_PIO0 : force PIO0 (same as original with @force_pio0 == 1)
* ATA_DNXFER_ANY: same as original with @force_pio0 == 0

Currently, only ANY and FORCE_PIO0 are used to maintain the original
behavior.  Other selectors will be used later to improve EH speed down
sequence.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/libata-core.c |  105 ++---
 drivers/ata/libata-eh.c   |9 +++-
 drivers/ata/libata.h  |   12 +-
 3 files changed, 96 insertions(+), 30 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index a441c75..417394b 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1747,6 +1747,7 @@ int ata_bus_probe(struct ata_port *ap)
int tries[ATA_MAX_DEVICES];
int i, rc, down_xfermask;
struct ata_device *dev;
+   int dnxfer_sel;
 
ata_port_probe(ap);
 
@@ -1828,13 +1829,15 @@ int ata_bus_probe(struct ata_port *ap)
/* fall through */
default:
tries[dev->devno]--;
-   if (down_xfermask &&
-   ata_down_xfermask_limit(dev, tries[dev->devno] == 1))
+   dnxfer_sel = ATA_DNXFER_ANY;
+   if (tries[dev->devno] == 1)
+   dnxfer_sel = ATA_DNXFER_FORCE_PIO0;
+   if (down_xfermask && ata_down_xfermask_limit(dev, dnxfer_sel))
tries[dev->devno] = 0;
}
 
if (!tries[dev->devno]) {
-   ata_down_xfermask_limit(dev, 1);
+   ata_down_xfermask_limit(dev, ATA_DNXFER_FORCE_PIO0);
ata_dev_disable(dev);
}
 
@@ -2267,7 +2270,7 @@ int ata_timing_compute(struct ata_device *adev, unsigned 
short speed,
 /**
  * ata_down_xfermask_limit - adjust dev xfer masks downward
  * @dev: Device to adjust xfer masks
- * @force_pio0: Force PIO0
+ * @sel: ATA_DNXFER_* selector
  *
  * Adjust xfer masks of @dev downward.  Note that this function
  * does not apply the change.  Invoking ata_set_mode() afterwards
@@ -2279,37 +2282,87 @@ int ata_timing_compute(struct ata_device *adev, 
unsigned short speed,
  * RETURNS:
  * 0 on success, negative errno on failure
  */
-int ata_down_xfermask_limit(struct ata_device *dev, int force_pio0)
+int ata_down_xfermask_limit(struct ata_device *dev, unsigned int sel)
 {
-   unsigned long xfer_mask;
-   int highbit;
+   char buf[32];
+   unsigned int orig_mask, xfer_mask;
+   unsigned int pio_mask, mwdma_mask, udma_mask;
+   int quiet, highbit;
 
-   xfer_mask = ata_pack_xfermask(dev->pio_mask, dev->mwdma_mask,
- dev->udma_mask);
+   quiet = !!(sel & ATA_DNXFER_QUIET);
+   sel &= ~ATA_DNXFER_QUIET;
 
-   if (!xfer_mask)
-   goto fail;
-   /* don't gear down to MWDMA from UDMA, go directly to PIO */
-   if (xfer_mask & ATA_MASK_UDMA)
-   xfer_mask &= ~ATA_MASK_MWDMA;
+   xfer_mask = orig_mask = ata_pack_xfermask(dev->pio_mask,
+ dev->mwdma_mask,
+ dev->udma_mask);
+   ata_unpack_xfermask(xfer_mask, &pio_mask, &mwdma_mask, &udma_mask);
 
-   highbit = fls(xfer_mask) - 1;
-   xfer_mask &= ~(1 << highbit);
-   if (force_pio0)
-   xfer_mask &= 1 << ATA_SHIFT_PIO;
-   if (!xfer_mask)
-   goto fail;
+   switch (sel) {
+   case ATA_DNXFER_PIO:
+   highbit = fls(pio_mask) - 1;
+   pio_mask &= ~(1 << highbit);
+   break;
+
+   case ATA_DNXFER_DMA:
+   if (udma_mask) {
+   highbit = fls(udma_mask) - 1;
+   udma_mask &= ~(1 << highbit);
+   if (!udma_mask)
+   return -ENOENT;
+   } else if (mwdma_mask) {
+   highbit = fls(mwdma_mask) - 1;
+   mwdma_mask &= ~(1 << highbit);
+   if (!mwdma_mask)
+   return -ENOENT;
+   }
+   break;
+
+   case ATA_DNXFER_40C:
+   udma_mask &= ATA_UDMA_MASK_40C;
+   break;
+
+   case ATA_DNXFER_FORCE_PIO0:
+   pio_mask &= 1;
+   case ATA_DNXFER_FORCE_PIO:
+   mwdma_mask = 0;
+   udma_mask = 0

asdf

[Tejun Heo]

Hello,

The current EH speed down code is more of a feature demonstration and
goes through rdiculously many meaningless steps when condition is met.
This patchset tries to put some intelligence into speed down sequence.
The goal is to achieve reasonable number of speed down steps
reasonably spaced from one another and consider NCQ, cable type and
the current protocol when determining speed down steps, while not
bloating the code too much with nitty gritty details.

Roughly, the rules are...

1. If NCQ and protocol/timeout/unknown dev errors occur, turn off NCQ

2. If excessive transfer errors occur, speed down within the current
   transfer mode (UDMA/MWDMA/PIO).  If UDMA, it's first adjusted down
   a step, if error conditions persist, 40c limit is applied.  Speed
   down is done only twice.

3. If PATA && used up all DMA speed down steps && a LOT of
   transmission/unknown errors occur, switch to PIO.  So, we never
   automatically step down to PIO on SATA.  This is intended.  Some
   SATA hdd even seems to have problem with PIO data transfer
   commands.

The last patch makes ahci report HSM violation error on spurious
completion of NCQ commands, thus causing NCQ off after several such
incidents.  These drives should be blacklisted for DMA eventually.

This patchset is against...

  upstream (eb0e63cca36a3389f0ccab4584f6d479b983fad5)
+ [1] pata_platform-fix-devres-conversion
+ [2] libata-convert-to-iomap

Ric, I guess this resolves the to-do item from you which has been
sitting in my mailbox for way too long.  What do you think about the
rules?

Thanks.

--
tejun

[1] http://article.gmane.org/gmane.linux.kernel/488509
[2] hmm.. can't find it any archive.  Might have been ignored due to
size.  Jeff, did you receive it?


-
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

[PATCH 4/5] libata: kill ATA_DNXFER_ANY

[Tejun Heo]

ATA_DNXFER_ANY isn't used anymore.  Kill it.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
---
 drivers/ata/libata-core.c |9 -
 drivers/ata/libata.h  |1 -
 2 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index cbd4bf1..651ab4b 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2327,15 +2327,6 @@ int ata_down_xfermask_limit(struct ata_device *dev, 
unsigned int sel)
udma_mask = 0;
break;
 
-   case ATA_DNXFER_ANY:
-   /* don't gear down to MWDMA from UDMA, go directly to PIO */
-   if (xfer_mask & ATA_MASK_UDMA)
-   xfer_mask &= ~ATA_MASK_MWDMA;
-
-   highbit = fls(xfer_mask) - 1;
-   xfer_mask &= ~(1 << highbit);
-   break;
-
default:
BUG();
}
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 36e08d2..6fc6260 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -48,7 +48,6 @@ enum {
ATA_DNXFER_40C  = 2,/* apply 40c cable limit */
ATA_DNXFER_FORCE_PIO= 3,/* force PIO */
ATA_DNXFER_FORCE_PIO0   = 4,/* force PIO0 */
-   ATA_DNXFER_ANY  = 5,/* speed down any */
 
ATA_DNXFER_QUIET= (1 << 31),
 };
-- 
1.4.4.4


-
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