Re: [KERNEL 2.6.26-rc4] bugreport : pata_pcmcia with Sandisk Extreme III 8GB

[Tejun Heo <htejun@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 994 bytes --]

Hello,

Komuro wrote:
> On Sat, 14 Jun 2008 03:20:41 -0400
> Jeff Garzik <jeff@garzik.org> wrote:
> 
> 
> After removing the 3-lines below in ata_eh_reset
> the pata_pcmcia works properly.
> 
> 
> -       spin_lock_irqsave(link->ap->lock, flags);
> -       link->eh_info.serror = 0;
> -       spin_unlock_irqrestore(link->ap->lock, flags);
> 
> Please fix this problem.

Thanks for diagnosing the problem but it doesn't make any sense at all.
 Those three lines just clear cached SError value.  pata_pcmcia being a
PATA driver, SError is not implemented and always zero.

Also the init_one error is -ENOMEM.  For the above change to make any
difference, EH should have been entered which is invoked deep into
ata_host_register() and once control reaches that point it never returns
error code.  I have difficult time imagining any way the above diff can
have anything to do with the reported failure.

Please apply the attached patch and report the log after probe failure.

Thanks.

-- 
tejun

[-- Attachment #2: pata_pcmcia-debug.patch --]
[-- Type: text/x-patch, Size: 1776 bytes --]

diff --git a/drivers/ata/pata_pcmcia.c b/drivers/ata/pata_pcmcia.c
index 3d39f9d..d5bcdc2 100644
--- a/drivers/ata/pata_pcmcia.c
+++ b/drivers/ata/pata_pcmcia.c
@@ -177,8 +177,10 @@ static int pcmcia_init_one(struct pcmcia_device *pdev)
 	struct ata_port_operations *ops = &pcmcia_port_ops;
 
 	info = kzalloc(sizeof(*info), GFP_KERNEL);
-	if (info == NULL)
+	if (info == NULL) {
+		printk("XXX info alloc failed\n");
 		return -ENOMEM;
+	}
 
 	/* Glue stuff together. FIXME: We may be able to get rid of info with care */
 	info->pdev = pdev;
@@ -196,8 +198,10 @@ static int pcmcia_init_one(struct pcmcia_device *pdev)
 	/* Allocate resoure probing structures */
 
 	stk = kzalloc(sizeof(*stk), GFP_KERNEL);
-	if (!stk)
+	if (!stk) {
+		printk("XXX stk alloc failed\n");
 		goto out1;
+	}
 
 	cfg = &stk->parse.cftable_entry;
 
@@ -290,8 +294,11 @@ next_entry:
 	ret = -ENOMEM;
 	io_addr = devm_ioport_map(&pdev->dev, io_base, 8);
 	ctl_addr = devm_ioport_map(&pdev->dev, ctl_base, 1);
-	if (!io_addr || !ctl_addr)
+	if (!io_addr || !ctl_addr) {
+		printk("XXX ioport_map failed io_addr=%p ctl_addr=%p\n",
+		       io_addr, ctl_addr);
 		goto failed;
+	}
 
 	/* Success. Disable the IRQ nIEN line, do quirks */
 	iowrite8(0x02, ctl_addr);
@@ -311,8 +318,10 @@ next_entry:
 	 */
 	ret = -ENOMEM;
 	host = ata_host_alloc(&pdev->dev, n_ports);
-	if (!host)
+	if (!host) {
+		printk("XXX host alloc failed\n");
 		goto failed;
+	}
 
 	for (p = 0; p < n_ports; p++) {
 		ap = host->ports[p];
@@ -331,8 +340,10 @@ next_entry:
 	/* activate */
 	ret = ata_host_activate(host, pdev->irq.AssignedIRQ, ata_sff_interrupt,
 				IRQF_SHARED, &pcmcia_sht);
-	if (ret)
+	if (ret) {
+		printk("XXX host activate failed ret=%d\n", ret);
 		goto failed;
+	}
 
 	info->ndev = 1;
 	kfree(stk);