Re: Some IDE issues with 2.6.28 on PC-Engines ALIX2

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

 



Hello.

Bartlomiej Zolnierkiewicz wrote:

2. The cs5535 ide driver doesn't seem to be able to recognize the
newer CS5536 controller for IDE.
No wonder, it's even impossible to determine CS5536 IDE controller's device ID from the datasheet; include/linux/pci_ids.h tells me that the device ID is 0x209A, so adding another ID to the 'cs5535' driver's ID table shouldn't be an issue -- if they are indeed compatible. Looking at the datasheets, they are not -- bad luck, we need a new driver... BTW, libata seems to already have support for this chipset.

Indeed...

Karl, care to give a try to the following patch (it is completely untested
so please backup your data first if necessary)?

From: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
Subject: [PATCH] ide: add CS5536 host driver

This is a port of libata's pata_cs5536.c (written by Martin K. Petersen)
to IDE subsystem.

Changes done while at it:

* Reprogram PIO/MWDMA timings if needed before and after DMA transfer
  (chipset uses shared PIO/MWDMA timings).
* Fix cable detection to report 80-wires cable if BIOS set it for any
  device on a port (IDE core will do drive-side cable detection later).

  The original pata_cs5536 cable detection is indeed somewhat bogus...

* CS5536 is used mostly for driving CF cards and this allows use of

I woulnd't be so sure -- but anyway, I mostly see the development boards... :-)

Cc: Martin K. Petersen <mkp@xxxxxxx>
Cc: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>
Cc: Karl Auerbach <karl@xxxxxxx>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>

  Looks good but needs some polishing...

Index: b/drivers/ide/Kconfig
===================================================================
--- a/drivers/ide/Kconfig
+++ b/drivers/ide/Kconfig
@@ -465,6 +465,16 @@ config BLK_DEV_CS5535
It is safe to say Y to this question. +config BLK_DEV_CS5536
+	tristate "CS5536 chipset support"
+	depends on X86 && !X86_64

  Why not just depend on X86_32?

Index: b/drivers/ide/cs5536.c
===================================================================
--- /dev/null
+++ b/drivers/ide/cs5536.c
@@ -0,0 +1,303 @@
[...]
+ * The IDE timing registers for the CS5536 live in the Geode Machine
+ * Specific Register file and not PCI config space.  Most BIOSes
+ * virtualize the PCI registers so the chip looks like a standard IDE
+ * controller.  Unfortunately not all implementations get this right.
+ * In particular some have problems with unaligned accesses to the

I'd say that people doing unaligned accesses are just looking for trouble... :-)

+enum {
+	MSR_IDE_CFG		= 0x51300010,
+	PCI_IDE_CFG		= 0x40,
+
+	CFG			= 0,
+	DTC			= 2,
+	CAST			= 3,
+	ETC			= 4,
+
+	IDE_CFG_CHANEN		= (1 << 1),
+	IDE_CFG_CABLE		= (1 << 17) | (1 << 16),
+
+	IDE_D0_SHIFT		= 24,
+	IDE_D1_SHIFT		= 16,
+	IDE_DRV_MASK		= 0xff,
+
+	IDE_CAST_D0_SHIFT	= 6,
+	IDE_CAST_D1_SHIFT	= 4,
+	IDE_CAST_DRV_MASK	= 0x3,
+
+	IDE_CAST_CMD_SHIFT	= 24,
+	IDE_CAST_CMD_MASK	= 0xff,
+};

Declaring a lot of semi-related constants is not what enum was intended for I think...

+static void cs5536_program_dtc(ide_drive_t *drive, u8 tim)
+{
+	struct pci_dev *pdev = to_pci_dev(drive->hwif->dev);
+	int dshift = (drive->dn & 1) ? IDE_D1_SHIFT : IDE_D0_SHIFT;

Masking with 1 is pointless (though harmless) for this single-channel controller.

+/**
+ *	cs5536_set_pio_mode		-	PIO timing setup
+ *	@drive: ATA device
+ *	@pio: PIO mode number
+ */
+
+static void cs5536_set_pio_mode(ide_drive_t *drive, const u8 pio)
+{
+	static const u8 drv_timings[5] = {
+		0x98, 0x55, 0x32, 0x21, 0x20,
+	};
+
+	static const u8 addr_timings[5] = {
+		0x2, 0x1, 0x0, 0x0, 0x0,
+	};
+
+	static const u8 cmd_timings[5] = {
+		0x99, 0x92, 0x90, 0x22, 0x20,
+	};
+
+	struct pci_dev *pdev = to_pci_dev(drive->hwif->dev);
+	ide_drive_t *pair = ide_get_pair_dev(drive);
+	int cshift = (drive->dn & 1) ? IDE_CAST_D1_SHIFT : IDE_CAST_D0_SHIFT;

  Same comment...

+	u32 cast;
+	u8 cmd_pio = pio;
+
+	if (pair)
+		cmd_pio = min(pio, ide_get_best_pio_mode(pair, 255, 4));
+
+	drive->drive_data &= 0xff00;

  IDE_DRV_MASK << 8?

+/**
+ *	cs5536_set_dma_mode		-	DMA timing setup
+ *	@drive: ATA device
+ *	@mode: DMA mode
+ */
+
+static void cs5536_set_dma_mode(ide_drive_t *drive, const u8 mode)
+{
+	static const u8 udma_timings[6] = {
+		0xc2, 0xc1, 0xc0, 0xc4, 0xc5, 0xc6,
+	};
+
+	static const u8 mwdma_timings[3] = {
+		0x67, 0x21, 0x20,
+	};
+
+	struct pci_dev *pdev = to_pci_dev(drive->hwif->dev);
+	int dshift = (drive->dn & 1) ? IDE_D1_SHIFT : IDE_D0_SHIFT;
+	u32 etc;
+
+	if (mode >= XFER_UDMA_0) {
+		cs5536_read(pdev, ETC, &etc);
+
+		etc &= ~(IDE_DRV_MASK << dshift);
+		etc |= udma_timings[mode - XFER_UDMA_0] << dshift;
+
+		cs5536_write(pdev, ETC, etc);
+	} else { /* MWDMA */
+		drive->drive_data &= 0xff;

  IDE_DRV_MASK?

+		drive->drive_data |= mwdma_timings[mode - XFER_MW_DMA_0] << 8;

How about disabling UltraDMA mode? While not an issue in the original driver with the set_{dma|pio}mode() method call order strictly determined and the latter method disabling UltraDMA, here this becames an issue...

+static void cs5536_dma_start(ide_drive_t *drive)
+{
+	if (drive->current_speed < XFER_UDMA_0)
+		cs5536_program_dtc(drive, drive->drive_data >> 8);

Worth comparing the values as PIO4 and MWDMA2 timings exactly correspond. Hm, PIO3 and MWDMA1 too...

+/**
+ *	cs5536_init_one
+ *	@dev: PCI device
+ *	@id: Entry in match table
+ */
+
+static int cs5536_init_one(struct pci_dev *dev, const struct pci_device_id *id)
+{
+	u32 cfg;
+
+	if (use_msr)
+		printk(KERN_ERR DRV_NAME ": Using MSR regs instead of PCI\n");

  Why KERN_ERR? :-O

+	cs5536_read(dev, CFG, &cfg);
+
+	if ((cfg & IDE_CFG_CHANEN) == 0) {
+		printk(KERN_ERR DRV_NAME ": disabled by BIOS\n");
+		return -ENODEV;

  Eh, why not do it via the usual .enablebits mechanism?

MBR, Sergei


--
To unsubscribe from this list: 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