Re: [PATCH 2/6] libata: add xfer_mask handling functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Albert Lee wrote:
Tejun Heo wrote:

Add ata_pack_xfermask(), ata_xfer_mask2mode(), ata_xfer_mode2mask(),
ata_xfer_mode2shift() and ata_id_xfermask().  These functions will be
used by following patches to simplify xfer_mask handling.

Signed-off-by: Tejun Heo <htejun@xxxxxxxxx>

(snip)
+/**
+ *	ata_id_xfermask - Compute xfermask from the given IDENTIFY data
+ *	@id: IDENTIFY data to compute xfer mask from
+ *
+ *	Compute the xfermask for this device. This is not as trivial
+ *	as it seems if we must consider early devices correctly.
+ *
+ *	FIXME: pre IDE drive timing (do we care ?).
+ *
+ *	LOCKING:
+ *	None.
+ *
+ *	RETURNS:
+ *	Computed xfermask
+ */
+static unsigned int ata_id_xfermask(const u16 *id)
+{
+	unsigned int pio_mask, mwdma_mask, udma_mask;
+
+	/* Usual case. Word 53 indicates word 64 is valid */
+	if (id[ATA_ID_FIELD_VALID] & (1 << 1)) {
+		pio_mask = id[ATA_ID_PIO_MODES] & 0x03;
+		pio_mask <<= 3;
+		pio_mask |= 0x7;
+	} else {
+		/* If word 64 isn't valid then Word 51 high byte holds
+		 * the PIO timing number for the maximum. Turn it into
+		 * a mask.
+		 */
+		pio_mask = (2 << (id[ATA_ID_OLD_PIO_MODES] & 0xFF)) - 1 ;
+
+		/* But wait.. there's more. Design your standards by
+		 * committee and you too can get a free iordy field to
+		 * process. However its the speeds not the modes that
+		 * are supported... Note drivers using the timing API
+		 * will get this right anyway
+		 */
+	}
+
+	mwdma_mask = id[ATA_ID_MWDMA_MODES] & 0x07;
+	udma_mask = id[ATA_ID_UDMA_MODES] & 0xff;
+
+	return ata_pack_xfermask(pio_mask, mwdma_mask, udma_mask);
+}
+
/*


We have ap->pio_mask, ap->mwdma_mask and ap->udma_mask.
Just thinking what if we have these masks in ata_device?

We need those masks in both ap and dev. ap capability should be stored in ap->*_mask and dev capabilities in dev->*_mask. e.g. speed limits due to controller quirks should be done by limiting ap->*_mask in ->dev_config() while device speeding down due to transmission errors should be done by limiting dev->*_mask.

Maybe we can save some bitwise operations and make the code more intuitive to read?

Ex. ata_id_xfermask() can store the calculated masks to dev->pio_mask, dev->mwdma_mask, etc.

This will be done when adding dev->*_mask.

Ex. ata_mode_string() can take mode (XFER_UDMA_7..) as parameter and translate the given mode to string.

Hmmm... I don't know. My plan is to unify transfer mode handling to unsigned int xfer_mask in libata core layer proper. It's easier to pass around, mask, manipulate, etc... That's why I made ata_mode_string() take xfer_mask instead of mode constants. But I'm okay either way. Converting back and forth between xfermask and mode constant isn't difficult.

Ex. to print out the max mode support by the device, 1. ata_id_xfermask() calculates and saves the masks to dev->pio_mask, etc.
    2. Another function, say, ata_dev_max_mode() takes dev as parameter,
       packs the mode by ata_pack_xfermask() internally,
       find the max mode supported by the drive,
       then use ata_xfer_mask2mode() to return the mode.
    3. ata_mode_string() translates the mode, say XFER_UDMA_7, to string literal.

Hmm.. 1, 2 and 3 can just be done like the following.

printk("max speed=%s\n", ata_mode_string(ata_id_xfermask(id));

I think it's simpler this way. No?

--
tejun
-
: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux