LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH]: pcmcia - spot slave decode flaws (for testing)
@ 2007-02-20 18:18 Alan
  2007-02-23 10:44 ` Jeff Garzik
  0 siblings, 1 reply; 5+ messages in thread
From: Alan @ 2007-02-20 18:18 UTC (permalink / raw)
  To: jgarzik, linux-kernel, linux-ide

If you've got a CF adapter or PCMCIA disc which shows up twice in libata
pata_pcmcia can you try this patch on top of the updates posted. It tries
to spot when the slave is a mirror of the master and to fix up problems
that causes.

Signed-off-by: Alan Cox <alan@redhat.com>

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.20-mm2/drivers/ata/pata_pcmcia.c linux-2.6.20-mm2/drivers/ata/pata_pcmcia.c
--- linux.vanilla-2.6.20-mm2/drivers/ata/pata_pcmcia.c	2007-02-20 13:37:58.000000000 +0000
+++ linux-2.6.20-mm2/drivers/ata/pata_pcmcia.c	2007-02-20 14:28:13.000000000 +0000
@@ -54,6 +54,39 @@
 	dev_node_t	node;
 };
 
+/**
+ *	pcmcia_set_mode	-	PCMCIA specific mode setup
+ *	@ap: Port
+ *	@r_failed_dev: Return pointer for failed device
+ *
+ *	Perform the tuning and setup of the devices and timings, which
+ *	for PCMCIA is the same as any other controller. We wrap it however
+ *	as we need to spot hardware with incorrect or missing master/slave
+ *	decode, which alas is embarrassingly common in the PC world
+ */
+ 
+static int pcmcia_set_mode(struct ata_port *ap, struct ata_device **r_failed_dev)
+{
+	struct ata_device *master = &ap->device[0];
+	struct ata_device *slave = &ap->device[1];
+	
+	if (!ata_dev_enabled(master) || !ata_dev_enabled(slave))
+		return ata_do_set_mode(ap, r_failed_dev);
+		
+	if (memcmp(master->id + ATA_ID_FW_REV,  slave->id + ATA_ID_FW_REV,
+			   ATA_ID_FW_REV_LEN + ATA_ID_PROD_LEN) == 0)
+	{
+		/* Suspicious match, but could be two cards from
+		   the same vendor - check serial */
+		if (memcmp(master->id + ATA_ID_SERNO, slave->id + ATA_ID_SERNO,
+			   ATA_ID_SERNO_LEN) == 0 && master->id[ATA_ID_SERNO] >> 8) {
+			ata_dev_printk(slave, KERN_WARNING, "is a ghost device, ignoring.\n");
+			ata_dev_disable(slave);
+		}	 
+	}
+	return ata_do_set_mode(ap, r_failed_dev);
+}
+
 static struct scsi_host_template pcmcia_sht = {
 	.module			= THIS_MODULE,
 	.name			= DRV_NAME,
@@ -73,6 +106,7 @@
 };
 
 static struct ata_port_operations pcmcia_port_ops = {
+	.set_mode	= pcmcia_set_mode,
 	.port_disable	= ata_port_disable,
 	.tf_load	= ata_tf_load,
 	.tf_read	= ata_tf_read,

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH]: pcmcia - spot slave decode flaws (for testing)
  2007-02-20 18:18 [PATCH]: pcmcia - spot slave decode flaws (for testing) Alan
@ 2007-02-23 10:44 ` Jeff Garzik
  2007-02-23 12:08   ` Alan
  0 siblings, 1 reply; 5+ messages in thread
From: Jeff Garzik @ 2007-02-23 10:44 UTC (permalink / raw)
  To: Alan; +Cc: linux-kernel, linux-ide

Alan wrote:
> If you've got a CF adapter or PCMCIA disc which shows up twice in libata
> pata_pcmcia can you try this patch on top of the updates posted. It tries
> to spot when the slave is a mirror of the master and to fix up problems
> that causes.
> 
> Signed-off-by: Alan Cox <alan@redhat.com>
> 
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.20-mm2/drivers/ata/pata_pcmcia.c linux-2.6.20-mm2/drivers/ata/pata_pcmcia.c
> --- linux.vanilla-2.6.20-mm2/drivers/ata/pata_pcmcia.c	2007-02-20 13:37:58.000000000 +0000
> +++ linux-2.6.20-mm2/drivers/ata/pata_pcmcia.c	2007-02-20 14:28:13.000000000 +0000
> @@ -54,6 +54,39 @@
>  	dev_node_t	node;
>  };
>  
> +/**
> + *	pcmcia_set_mode	-	PCMCIA specific mode setup
> + *	@ap: Port
> + *	@r_failed_dev: Return pointer for failed device
> + *
> + *	Perform the tuning and setup of the devices and timings, which
> + *	for PCMCIA is the same as any other controller. We wrap it however
> + *	as we need to spot hardware with incorrect or missing master/slave
> + *	decode, which alas is embarrassingly common in the PC world
> + */
> + 
> +static int pcmcia_set_mode(struct ata_port *ap, struct ata_device **r_failed_dev)
> +{
> +	struct ata_device *master = &ap->device[0];
> +	struct ata_device *slave = &ap->device[1];
> +	
> +	if (!ata_dev_enabled(master) || !ata_dev_enabled(slave))
> +		return ata_do_set_mode(ap, r_failed_dev);
> +		
> +	if (memcmp(master->id + ATA_ID_FW_REV,  slave->id + ATA_ID_FW_REV,
> +			   ATA_ID_FW_REV_LEN + ATA_ID_PROD_LEN) == 0)
> +	{
> +		/* Suspicious match, but could be two cards from
> +		   the same vendor - check serial */
> +		if (memcmp(master->id + ATA_ID_SERNO, slave->id + ATA_ID_SERNO,
> +			   ATA_ID_SERNO_LEN) == 0 && master->id[ATA_ID_SERNO] >> 8) {
> +			ata_dev_printk(slave, KERN_WARNING, "is a ghost device, ignoring.\n");
> +			ata_dev_disable(slave);
> +		}	 
> +	}
> +	return ata_do_set_mode(ap, r_failed_dev);

Code looks OK. Not applied due to "for testing" note.

General comment:  it might be nice to do this in the core, just as a 
sanity check for a variety of problems, past, present and future.



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH]: pcmcia - spot slave decode flaws (for testing)
  2007-02-23 10:44 ` Jeff Garzik
@ 2007-02-23 12:08   ` Alan
  2007-02-24 21:23     ` Eric D. Mudama
  0 siblings, 1 reply; 5+ messages in thread
From: Alan @ 2007-02-23 12:08 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: linux-kernel, linux-ide

> Code looks OK. Not applied due to "for testing" note.
> 
> General comment:  it might be nice to do this in the core, just as a 
> sanity check for a variety of problems, past, present and future.

We tried that with old IDE and all hell broke loose. Lots of virtual disk
stuff and raid volumes have non-unique serial numbers. We even found a
case of identically serial numbered Maxtor drives.

It needs to stay in pcmcia, which is the only place we've seen the
duplication.



Alan

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH]: pcmcia - spot slave decode flaws (for testing)
  2007-02-23 12:08   ` Alan
@ 2007-02-24 21:23     ` Eric D. Mudama
  0 siblings, 0 replies; 5+ messages in thread
From: Eric D. Mudama @ 2007-02-24 21:23 UTC (permalink / raw)
  To: Alan; +Cc: Jeff Garzik, linux-kernel, linux-ide

On 2/23/07, Alan <alan@lxorguk.ukuu.org.uk> wrote:
> > Code looks OK. Not applied due to "for testing" note.
> >
> > General comment:  it might be nice to do this in the core, just as a
> > sanity check for a variety of problems, past, present and future.
>
> We tried that with old IDE and all hell broke loose. Lots of virtual disk
> stuff and raid volumes have non-unique serial numbers. We even found a
> case of identically serial numbered Maxtor drives.
>
> It needs to stay in pcmcia, which is the only place we've seen the
> duplication.

I don't think the maxtor drives actually had duplicate serial numbers.
 They were coming back as "M0000000000000000000" or something like
that to the limit of the strlen.  It looked more like buffer
corruption or something, and only happened with one or two people...

--eric

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH]: pcmcia - spot slave decode flaws (for testing)
  2007-02-21 15:37         ` Alan
@ 2007-03-21  1:56           ` Komuro
  0 siblings, 0 replies; 5+ messages in thread
From: Komuro @ 2007-03-21  1:56 UTC (permalink / raw)
  To: Alan; +Cc: linux-kernel


Hi,

"[PATCH]: pcmcia - spot slave decode flaws (for testing)" works properly.
(kernel 2.6.21-rc4-mm1)

pccard: PCMCIA card inserted into slot 1
pcmcia: registering new device pcmcia1.0
ata3: PATA max PIO0 cmd 0x00010100 ctl 0x0001010e bmdma 0x00000000 irq 3
scsi2 : pata_pcmcia
ata3.00: CFA: SunDisk SDP5-10, Rev 3.70, max PIO0
ata3.00: 20480 sectors, multi 0: LBA 
ata3.01: CFA: SunDisk SDP5-10, Rev 3.70, max PIO0
ata3.01: 20480 sectors, multi 0: LBA 
ata3.01: is a ghost device, ignoring.
ata3.01: disabled
ata3.00: configured for PIO0
scsi 2:0:0:0: Direct-Access     ATA      SunDisk SDP5-10  Rev  PQ: 0 ANSI: 5
sd 2:0:0:0: [sda] 20480 512-byte hardware sectors (10 MB)
sd 2:0:0:0: [sda] Write Protect is off
sd 2:0:0:0: [sda] Mode Sense: 00 3a 00 00
sd 2:0:0:0: [sda] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA
sd 2:0:0:0: [sda] 20480 512-byte hardware sectors (10 MB)
sd 2:0:0:0: [sda] Write Protect is off
sd 2:0:0:0: [sda] Mode Sense: 00 3a 00 00
sd 2:0:0:0: [sda] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA
 sda: sda1
sd 2:0:0:0: [sda] Attached SCSI removable disk
sd 2:0:0:0: Attached scsi generic sg0 type 0

Best Regards
Komuro


>If you've got a CF adapter or PCMCIA disc which shows up twice in libata
>pata_pcmcia can you try this patch on top of the updates posted. It tries
>to spot when the slave is a mirror of the master and to fix up problems
>that causes.
>
>Signed-off-by: Alan Cox <alan@redhat.com>
>
>diff -u --new-file --recursive --exclude-from /usr/src/exclude \
>linux.vanilla-2.6.20-mm2/drivers/ata/pata_pcmcia.c \
>                linux-2.6.20-mm2/drivers/ata/pata_pcmcia.c
>--- linux.vanilla-2.6.20-mm2/drivers/ata/pata_pcmcia.c	2007-02-20 13:37:58.000000000 \
>                +0000
>+++ linux-2.6.20-mm2/drivers/ata/pata_pcmcia.c	2007-02-20 14:28:13.000000000 +0000
>@@ -54,6 +54,39 @@
> 	dev_node_t	node;
> };
> 
>+/**
>+ *	pcmcia_set_mode	-	PCMCIA specific mode setup
>+ *	@ap: Port
>+ *	@r_failed_dev: Return pointer for failed device
>+ *
>+ *	Perform the tuning and setup of the devices and timings, which
>+ *	for PCMCIA is the same as any other controller. We wrap it however
>+ *	as we need to spot hardware with incorrect or missing master/slave
>+ *	decode, which alas is embarrassingly common in the PC world
>+ */
>+ 
>+static int pcmcia_set_mode(struct ata_port *ap, struct ata_device **r_failed_dev)
>+{
>+	struct ata_device *master = &ap->device[0];
>+	struct ata_device *slave = &ap->device[1];
>+	
>+	if (!ata_dev_enabled(master) || !ata_dev_enabled(slave))
>+		return ata_do_set_mode(ap, r_failed_dev);
>+		
>+	if (memcmp(master->id + ATA_ID_FW_REV,  slave->id + ATA_ID_FW_REV,
>+			   ATA_ID_FW_REV_LEN + ATA_ID_PROD_LEN) == 0)
>+	{
>+		/* Suspicious match, but could be two cards from
>+		   the same vendor - check serial */
>+		if (memcmp(master->id + ATA_ID_SERNO, slave->id + ATA_ID_SERNO,
>+			   ATA_ID_SERNO_LEN) == 0 && master->id[ATA_ID_SERNO] >> 8) {
>+			ata_dev_printk(slave, KERN_WARNING, "is a ghost device, ignoring.\n");
>+			ata_dev_disable(slave);
>+		}	 
>+	}
>+	return ata_do_set_mode(ap, r_failed_dev);
>+}
>+
> static struct scsi_host_template pcmcia_sht = {
> 	.module			= THIS_MODULE,
> 	.name			= DRV_NAME,
>@@ -73,6 +106,7 @@
> };
> 
> static struct ata_port_operations pcmcia_port_ops = {
>+	.set_mode	= pcmcia_set_mode,
> 	.port_disable	= ata_port_disable,
> 	.tf_load	= ata_tf_load,
> 	.tf_read	= ata_tf_read,

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2007-03-21  1:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-02-20 18:18 [PATCH]: pcmcia - spot slave decode flaws (for testing) Alan
2007-02-23 10:44 ` Jeff Garzik
2007-02-23 12:08   ` Alan
2007-02-24 21:23     ` Eric D. Mudama
  -- strict thread matches above, loose matches on Subject: below --
2006-12-17 12:27 [BUG KERNEL 2.6.20-rc1] ftp: get or put stops during file-transfer Komuro
2006-12-17  4:02 ` Al Viro
     [not found]   ` <20070211101537.e40fa309.komurojun-mbn@nifty.com>
2007-02-11  2:40     ` [BUG] PATA_PCMCIA does not work Komuro
2007-02-18  2:09       ` Komuro
2007-02-21 15:37         ` Alan
2007-03-21  1:56           ` [PATCH]: pcmcia - spot slave decode flaws (for testing) Komuro

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).