libata-core and pccard CF card reader

[Mathieu Chouquet-Stringer <mchouque@free.fr>]

	Hello,

During the last couple of month I've been using extensively Mark Lord's
driver for my flash card reader (downloadable at [1]).

Not too long ago he posted an skeleton [2] that was close to work with
the lastest libata release...

In a private email, he made me aware that because of time constraint, he
wasn't sure when he would have the time to fix it.

So I started hacking and made some progress, I fixed a bunch of
functions whose prototypes had changed, add some more bits here and
there and was finally able to compile and to load my kernel module.


The host now registers itself with libata (using standard function
ata_device_add), I see it in dmesg but it fails to register the CF card:

ata1: dev 0 not supported, ignoring


Browsing the source, here's what happens:
- the code call ata_device_add (I guess it was originally coded like
  this because ata_pci_init_one which calls ata_pci_init_native_mode or
  ata_pci_init_native_mode would overwrite the special cmd_addr,
  altstatus_addr and ctl_addr)
- this function does some processing then calls ata_bus_probe
- which itself calls ata_dev_identify

I put some debug statements and found my 'problem' in
drivers/scsi/libata-core.c, function ata_dev_identify, around line 1329
(of the latest git). Here's a snippet of code:
=========================================================================
	/* ATA-specific feature tests */
	if (dev->class == ATA_DEV_ATA) {
		if (!ata_id_is_ata(dev->id)) /* sanity check */
			goto err_out_nosup;
=========================================================================

Problem is my device sets bit 15 of word 0 when you send it a "DEVICE
IDENTIFY" command; the ata_id_is_ata macro expanding to:
(((id)[0] & (1 << 15)) == 0)

I then proceeded by removing this check and if you abstract the debug
messages I put here and there in the code, here's what I got in the
logs:
=========================================================================
ata1: dev 0 cfg 49:0300 82:0000 83:0000 84:0000 85:0000 86:0000 87:0000 88:0000
class is ATA_DEV_ATA
ata id isn't ata, 0x848a
ata1: dev 0 not supported, ignoring
scsi20 : delkin_cb
libata version 1.20 loaded.
delkin_cb version 0.01
ACPI: PCI Interrupt 0000:03:00.0[A] -> Link [LNKA] -> GSI 11 (level, low) -> IRQ 11
ata1: PATA max PIO4 cmd 0x4010 ctl 0x401E bmdma 0xD2000000 irq 11
delkin_cb: id hasn't dma
delkin_cb: phy_reset, c50b2310
ata1: dev 0 cfg 49:0300 82:0000 83:0000 84:0000 85:0000 86:0000 87:0000 88:0000
class is ATA_DEV_ATA
ata id isn't ata, 0x848a
ata1: dev 0 ATA-10, max MWDMA2, 4001760 sectors: LBA
ata1: dev 0 configured for PIO4
scsi21 : delkin_cb
  Vendor: ATA       Model: SanDisk SDCFH-20  Rev: HDX 
  Type:   Direct-Access                      ANSI SCSI revision: 05
SCSI device sda: 4001760 512-byte hdwr sectors (2049 MB)
sda: Write Protect is off
sda: Mode Sense: 00 3a 00 00
SCSI device sda: drive cache: write through
SCSI device sda: 4001760 512-byte hdwr sectors (2049 MB)
sda: Write Protect is off
sda: Mode Sense: 00 3a 00 00
SCSI device sda: drive cache: write through
 sda: sda1
sd 21:0:0:0: Attached scsi removable disk sda
=========================================================================

So PIO modes appear to be working correctly (forget the bmdma value,
it's totally bogus...). I mounted the device and started reading stuff
(mainly comparing sha1sums of files I also had on disk): it worked.

But now, I wanted to know more about this bit 15 so I went googling.

On t13.org, I found this old description for bit 15 under "IDENTIFY
DRIVE" command:
15      0 reserved for non-magnetic drive


Then in a revision made in 95 (http://www.t13.org/ballots/d96112r0.pdf),
the bit meaning was changed to:
- Word 0 - Bit 15 set in ATA-2 meant non-magnetic media. Now means
  ATAPI. This breaks use with PCMCIA Flash ATA devices which set this
  bit.

  This was done to be compatible with ATAPI devices as currently
  specified in SFF8020. Since ATA-3 devices and ATAPI SFF8020 devices
  must coexist on the same physical cable with different device drivers,
  this compatibility is required. In the case of most if not all PCMCIA
  PC Card ATA devices today, a unique PCMCIA PC Card ATA aware device
  driver is used and this PCMCIA PC Card ATA aware device driver will
  never encounter an ATAPI device.


And now in the latest draft for ATA-8, they say:
15      0 = ATA Device
with a comment reading:
Devices that conform to this standard shall clear bit 15 to zero.

(note the "shall" here)


In the logs, I saw my device returned 0x848a. After reading some more
ATA documents, I found it's in fact a known ID and is defined as:
"CompactFlash™Association (CFA) feature set" (found in a bunch of
different working drafts of ATA-x).

So now the questions are:
- is this test in libata-core.c/ata_dev_identify correct?
- if it isn't, should we OR it with something like
  || dev->id[0] == 0x848a
- or should we just test bit 7 of word 0 ("removable media device")?
- or did I just open a new can of worms that will lead to adding a new
  subclass of device (CFA feature set) in ata_dev_identify?


Any pointers or ideas would be great, I'd like to get this thing merged
eventually...

Thanks.

[1] http://rtr.ca/delkin_cb-2.6.11.patch
[2] http://lkml.org/lkml/2006/1/19/150

-- 
Mathieu Chouquet-Stringer                         mchouque@free.fr
    "Le disparu, si l'on vénère sa mémoire, est plus présent et
                 plus puissant que le vivant".
           -- Antoine de Saint-Exupéry, Citadelle --