Re: [PATCH 11/16] PATA driver for Celleb

Re: [PATCH 11/16] PATA driver for Celleb

[Benjamin Herrenschmidt]

> We use a drivers/ide driver because its design is more suitable for
> SCC IDE controller than libata driver. But we will try to implement
> the libata driver as needed. This work needs to override many 
> callbacks of ata_port_operations by modifying generic helpers.

Yup, that's basically the same problem I have with mpc52xx... all of the
port reset & error handling code in libata does direct IO access and
thus if you need special IO accessors, you need to re-implement the
whole thing, which isn't very nice.

I have ideas on how we can fix libata but I haven't had time to talk to
Jeff much about it yet.

> We use the PRD transfer end read function on SCC IDE controller
> to handle this issue.
> This function guarantees that issue of DMA End interrupt and
> setting the interrupt bit of status register are done after 
> the outstanding write data to memory was pushed. Pushing is 
> implemented by the dummy read of IDE controller, which reads 
> the same path as the path of the write data. 
> Therefore this function is same as the flush of outstanding DMAs.
> To use this function, we specify the PRD table address(dmatable_dma) 
> in PRD Transfer End Read Base Address Register(dma_base + 0x018) as
> the path. Of course it also requires to set a strong order in 
> the IOIF space.

I haven't seem the dummy read in your code.. where is it ?

> > - Regarding PCI and PCIe on SCC, I didn't see any code in your PCI
> >support code for handling that issue. Is it fixed ? If not, then I
> >suppose you'll have problem with most PCI device/drivers on the field as
> >they do rely on such ordering to be provided. There was also an issue
> >with prefetch on PCI, for which I currently disable the prefetch in the
> >workarounds for the cell blade. I don't know if you handle that at all
> >neither.
> 
> We've noticed the problem you mentioned, and been developing the support
> for dummy read(please refer to previous comments for IDE) and etc. to solve
> the problem, but we haven't finished yet.  We also currently  disable the
> prefetch by initial setup of SCC.  We are trying now so that we can post
> the fixed driver code in next patch.

Ok. Can you confirm that the prefetch disable is the same was what I'm
doing for Spider ? (see my patch about IO workarounds posted last week).

The patch I posted handles both the dummy read and the prefetch disable.
I've added a mecanism to hook on all MMIO accesses to make that
possible. You may want to do something very similar.

Cheers,
Ben.

Re: [PATCH 11/16] PATA driver for Celleb

[Akira Iguchi]

>> We use the PRD transfer end read function on SCC IDE controller
>> to handle this issue.
>> This function guarantees that issue of DMA End interrupt and
>> setting the interrupt bit of status register are done after 
>> the outstanding write data to memory was pushed. Pushing is 
>> implemented by the dummy read of IDE controller, which reads 
>> the same path as the path of the write data. 
>> Therefore this function is same as the flush of outstanding DMAs.
>> To use this function, we specify the PRD table address(dmatable_dma) 
>> in PRD Transfer End Read Base Address Register(dma_base + 0x018) as
>> the path. Of course it also requires to set a strong order in 
>> the IOIF space.
>
>I haven't seem the dummy read in your code.. where is it ?

The dummy read of this function is done autonomously by IDE controller
when memory write was performed. It differs from the dummy read function
in PCI module.
Therefore the corresponding code does not exist.

>
>> > - Regarding PCI and PCIe on SCC, I didn't see any code in your PCI
>> >support code for handling that issue. Is it fixed ? If not, then I
>> >suppose you'll have problem with most PCI device/drivers on the field as
>> >they do rely on such ordering to be provided. There was also an issue
>> >with prefetch on PCI, for which I currently disable the prefetch in the
>> >workarounds for the cell blade. I don't know if you handle that at all
>> >neither.
>> 
>> We've noticed the problem you mentioned, and been developing the support
>> for dummy read(please refer to previous comments for IDE) and etc. to solve
>> the problem, but we haven't finished yet.  We also currently  disable the
>> prefetch by initial setup of SCC.  We are trying now so that we can post
>> the fixed driver code in next patch.
>
>Ok. Can you confirm that the prefetch disable is the same was what I'm
>doing for Spider ? (see my patch about IO workarounds posted last week).
>
>The patch I posted handles both the dummy read and the prefetch disable.
>I've added a mecanism to hook on all MMIO accesses to make that
>possible. You may want to do something very similar.

We read your IO workaroud patch for spider. The method for disabling prefetch
is same as we suppose. And the method for dummy read may be available, 
or be a good reference for us.
We found a mistake in our patch at disabling prefetch. We set the prefetch
enable bit wrongly.  We'll correct it in the next patch.


Best regards,
Akira Iguchi
Toshiba

Re: [PATCH 11/16] PATA driver for Celleb

[Benjamin Herrenschmidt]

> We read your IO workaroud patch for spider. The method for disabling prefetch
> is same as we suppose. And the method for dummy read may be available, 
> or be a good reference for us.
> We found a mistake in our patch at disabling prefetch. We set the prefetch
> enable bit wrongly.  We'll correct it in the next patch.

Excellent, thanks !

Ben.

[PATCH 11/16] PATA driver for Celleb

[Ishizaki Kou]

This patch adds PATA driver for Celleb platform.

Signed-off-by: Kou Ishizaki <kou.ishizaki@toshiba.co.jp>
---

Index: linux-2.6.19/drivers/ide/Kconfig
diff -u linux-2.6.19/drivers/ide/Kconfig:1.1.1.1 linux-2.6.19/drivers/ide/Kconfig:1.2
--- linux-2.6.19/drivers/ide/Kconfig:1.1.1.1	Fri Oct  6 10:41:06 2006
+++ linux-2.6.19/drivers/ide/Kconfig	Fri Oct  6 12:26:35 2006
@@ -773,6 +773,14 @@
 	  to transfer data to and from memory.  Saying Y is safe and improves
 	  performance.
 
+config BLK_DEV_IDE_CELLEB
+	bool "Toshiba's Cell Reference Set IDE support"
+	depends on PPC_CELLEB
+	help
+	  This driver provides support for the built-in IDE controller on
+	  Toshiba Cell Reference Board.
+	  If unsure, say Y.
+
 config BLK_DEV_IDE_SWARM
 	tristate "IDE for Sibyte evaluation boards"
 	depends on SIBYTE_SB1xxx_SOC
Index: linux-2.6.19/drivers/ide/Makefile
diff -u linux-2.6.19/drivers/ide/Makefile:1.1.1.1 linux-2.6.19/drivers/ide/Makefile:1.2
--- linux-2.6.19/drivers/ide/Makefile:1.1.1.1	Fri Oct  6 10:41:06 2006
+++ linux-2.6.19/drivers/ide/Makefile	Fri Oct  6 12:26:35 2006
@@ -36,6 +36,7 @@
 # built-in only drivers from ppc/
 ide-core-$(CONFIG_BLK_DEV_MPC8xx_IDE)	+= ppc/mpc8xx.o
 ide-core-$(CONFIG_BLK_DEV_IDE_PMAC)	+= ppc/pmac.o
+ide-core-$(CONFIG_BLK_DEV_IDE_CELLEB)	+= ppc/scc_pata.o
 
 # built-in only drivers from h8300/
 ide-core-$(CONFIG_H8300)		+= h8300/ide-h8300.o
Index: linux-2.6.19/drivers/ide/ppc/scc_pata.c
diff -u /dev/null linux-2.6.19/drivers/ide/ppc/scc_pata.c:1.1
--- /dev/null	Thu Nov  9 18:40:51 2006
+++ linux-2.6.19/drivers/ide/ppc/scc_pata.c	Tue Oct 10 10:23:19 2006
@@ -0,0 +1,812 @@
+/*
+ * Support for IDE interfaces on Celleb platform
+ *
+ * (C) Copyright 2006 TOSHIBA CORPORATION
+ *
+ * This code is based on drivers/ide/pci/siimage.c:
+ * Copyright (C) 2001-2002	Andre Hedrick <andre@linux-ide.org>
+ * Copyright (C) 2003		Red Hat <alan@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/delay.h>
+#include <linux/hdreg.h>
+#include <linux/ide.h>
+#include <linux/init.h>
+
+#define PCI_DEVICE_ID_TOSHIBA_SCC_ATA            0x01b4
+
+#define SCC_PATA_NAME           "scc IDE"
+
+#define TDVHSEL_MASTER          0x00000001
+#define TDVHSEL_SLAVE           0x00000004
+
+#define MODE_JCUSFEN            0x00000080
+
+#define CCKCTRL_ATARESET        0x00040000
+#define CCKCTRL_BUFCNT          0x00020000
+#define CCKCTRL_CRST            0x00010000
+#define CCKCTRL_OCLKEN          0x00000100
+#define CCKCTRL_ATACLKOEN       0x00000002
+#define CCKCTRL_LCLKEN          0x00000001
+
+#define QCHCD_IOS_SS		0x00000001
+
+#define QCHSD_STPDIAG		0x00020000
+
+#define INTMASK_MSK             0xD1000012
+#define INTSTS_SERROR		0x80000000
+#define INTSTS_PRERR		0x40000000
+#define INTSTS_RERR		0x10000000
+#define INTSTS_ICERR		0x01000000
+#define INTSTS_BMSINT		0x00000010
+#define INTSTS_BMHE		0x00000008
+#define INTSTS_IOIRQS           0x00000004
+#define INTSTS_INTRQ            0x00000002
+#define INTSTS_ACTEINT          0x00000001
+
+#define ECMODE_VALUE 0x01
+
+static struct scc_ports {
+	unsigned long ctl, dma;
+	unsigned char hwif_id;  /* for removing hwif from system */
+} scc_ports[MAX_HWIFS];
+
+/* PIO transfer mode  table */
+/* JCHST */
+static unsigned long JCHSTtbl[2][7] = {
+	{0x0E, 0x05, 0x02, 0x03, 0x02, 0x00, 0x00},   /* 100MHz */
+	{0x13, 0x07, 0x04, 0x04, 0x03, 0x00, 0x00}    /* 133MHz */
+};
+
+/* JCHHT */
+static unsigned long JCHHTtbl[2][7] = {
+	{0x0E, 0x02, 0x02, 0x02, 0x02, 0x00, 0x00},   /* 100MHz */
+	{0x13, 0x03, 0x03, 0x03, 0x03, 0x00, 0x00}    /* 133MHz */
+};
+
+/* JCHCT */
+static unsigned long JCHCTtbl[2][7] = {
+	{0x1D, 0x1D, 0x1C, 0x0B, 0x06, 0x00, 0x00},   /* 100MHz */
+	{0x27, 0x26, 0x26, 0x0E, 0x09, 0x00, 0x00}    /* 133MHz */
+};
+
+
+/* DMA transfer mode  table */
+/* JCHDCTM/JCHDCTS */
+static unsigned long JCHDCTxtbl[2][7] = {
+	{0x0A, 0x06, 0x04, 0x03, 0x01, 0x00, 0x00},   /* 100MHz */
+	{0x0E, 0x09, 0x06, 0x04, 0x02, 0x01, 0x00}    /* 133MHz */
+};
+
+/* JCSTWTM/JCSTWTS  */
+static unsigned long JCSTWTxtbl[2][7] = {
+	{0x06, 0x04, 0x03, 0x02, 0x02, 0x02, 0x00},   /* 100MHz */
+	{0x09, 0x06, 0x04, 0x02, 0x02, 0x02, 0x02}    /* 133MHz */
+};
+
+/* JCTSS */
+static unsigned long JCTSStbl[2][7] = {
+	{0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x00},   /* 100MHz */
+	{0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05}    /* 133MHz */
+};
+
+/* JCENVT */
+static unsigned long JCENVTtbl[2][7] = {
+	{0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x00},   /* 100MHz */
+	{0x02, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02}    /* 133MHz */
+};
+
+/* JCACTSELS/JCACTSELM */
+static unsigned long JCACTSELtbl[2][7] = {
+	{0x00, 0x00, 0x00, 0x00, 0x01, 0x01, 0x00},   /* 100MHz */
+	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}    /* 133MHz */
+};
+
+
+static u8 scc_ide_inb(unsigned long port)
+{
+	u32 data = in_be32((void*)port);
+	return (u8)data;
+}
+
+static u16 scc_ide_inw(unsigned long port)
+{
+	u32 data = in_be32((void*)port);
+	return (u16)data;
+}
+
+static u32 scc_ide_inl(unsigned long port)
+{
+	u32 data = in_be32((void*)port);
+	return data;
+}
+
+static void scc_ide_insw(unsigned long port, void *addr, u32 count)
+{
+	u16 *ptr = (u16 *)addr;
+	while (count--) {
+		*ptr++ = le16_to_cpu(in_be32((void*)port));
+	}
+}
+
+static void scc_ide_insl(unsigned long port, void *addr, u32 count)
+{
+	u16 *ptr = (u16 *)addr;
+	while (count--) {
+		*ptr++ = le16_to_cpu(in_be32((void*)port));
+		*ptr++ = le16_to_cpu(in_be32((void*)port));
+	}
+}
+
+static void scc_ide_outb(u8 addr, unsigned long port)
+{
+	out_be32((void*)port, addr);
+}
+
+static void scc_ide_outw(u16 addr, unsigned long port)
+{
+	out_be32((void*)port, addr);
+}
+
+static void scc_ide_outl(u32 addr, unsigned long port)
+{
+	out_be32((void*)port, addr);
+}
+
+static void
+scc_ide_outbsync(ide_drive_t * drive, u8 addr, unsigned long port)
+{
+	ide_hwif_t *hwif = HWIF(drive);
+	
+	out_be32((void*)port, addr);
+	__asm__ __volatile__("eieio":::"memory");	
+	in_be32((void*)(hwif->dma_base + 0x01c));
+	__asm__ __volatile__("eieio":::"memory");
+}
+
+static void
+scc_ide_outsw(unsigned long port, void *addr, u32 count)
+{
+	u16 *ptr = (u16 *)addr;
+	while (count--) {
+		out_be32((void*)port, cpu_to_le16(*ptr++));
+	}
+}
+
+static void
+scc_ide_outsl(unsigned long port, void *addr, u32 count)
+{
+	u16 *ptr = (u16 *)addr;
+	while (count--) {
+		out_be32((void*)port, cpu_to_le16(*ptr++));
+		out_be32((void*)port, cpu_to_le16(*ptr++));
+	}
+}
+
+/**
+ *	scc_ratemask	-	Compute available modes
+ *	@drive: IDE drive
+ *
+ *	Compute the available speeds for the devices on the interface.
+ *	Enforce UDMA33 as a limit if there is no 80pin cable present.
+ */
+ 
+static u8 scc_ratemask(ide_drive_t *drive)
+{
+	u8 mode = 4;
+	
+	if (!eighty_ninty_three(drive))
+		mode = min(mode, (u8)1);
+	return mode;
+}
+
+/**
+ *	scc_tuneproc	-	tune a drive PIO mode
+ *	@drive: drive to tune
+ *	@mode_wanted: the target operating mode
+ *
+ *	Load the timing settings for this device mode into the
+ *	controller.
+ */
+ 
+static void scc_tuneproc(ide_drive_t *drive, byte mode_wanted)
+{
+	ide_hwif_t *hwif = HWIF(drive);
+	struct scc_ports *ports = ide_get_hwifdata(hwif);
+	unsigned long ctl_base = ports->ctl;
+	unsigned long cckctrl_port = ctl_base + 0xff0;
+	unsigned long piosht_port = ctl_base + 0x000;
+	unsigned long pioct_port = ctl_base + 0x004;
+	unsigned long reg;
+	unsigned char speed = XFER_PIO_0;
+	int offset;
+	
+	mode_wanted = ide_get_best_pio_mode(drive, mode_wanted, 4, NULL);
+	switch (mode_wanted) {
+	case 4:	
+		speed = XFER_PIO_4;
+		break;
+	case 3:	
+		speed = XFER_PIO_3;
+		break;
+	case 2:	
+		speed = XFER_PIO_2;
+		break;
+	case 1:		
+		speed = XFER_PIO_1;
+		break;
+	case 0:
+	default:
+		speed = XFER_PIO_0;
+		break;
+	}
+	
+	reg = hwif->INL(cckctrl_port);
+	if (reg & CCKCTRL_ATACLKOEN) {
+		offset = 1; /* 133MHz */
+	} else {
+		offset = 0; /* 100MHz */
+	}
+	reg = JCHSTtbl[offset][mode_wanted] << 16 | JCHHTtbl[offset][mode_wanted];
+	hwif->OUTL(reg, piosht_port);
+	reg = JCHCTtbl[offset][mode_wanted];
+	hwif->OUTL(reg, pioct_port);
+	
+	ide_config_drive_speed(drive, speed);
+}
+
+/**
+ *	scc_tune_chipset	-	tune a drive DMA mode
+ *	@drive: Drive to set up
+ *	@xferspeed: speed we want to achieve
+ *
+ *	Load the timing settings for this device mode into the
+ *	controller.
+ */
+ 
+static int scc_tune_chipset(ide_drive_t *drive, byte xferspeed)
+{
+	ide_hwif_t *hwif = HWIF(drive);
+	u8 speed = ide_rate_filter(scc_ratemask(drive), xferspeed);
+	struct scc_ports *ports = ide_get_hwifdata(hwif);
+	unsigned long ctl_base = ports->ctl;
+	unsigned long cckctrl_port = ctl_base + 0xff0;
+	unsigned long mdmact_port = ctl_base + 0x008;
+	unsigned long mcrcst_port = ctl_base + 0x00c;
+	unsigned long sdmact_port = ctl_base + 0x010;
+	unsigned long scrcst_port = ctl_base + 0x014;
+	unsigned long udenvt_port = ctl_base + 0x018;
+	unsigned long tdvhsel_port   = ctl_base + 0x020;
+	int is_slave = (&hwif->drives[1] == drive);
+	int offset, idx;
+	unsigned long reg;
+	unsigned long jcactsel;    
+	
+	reg = hwif->INL(cckctrl_port);
+	if (reg & CCKCTRL_ATACLKOEN) {
+		offset = 1; /* 133MHz */
+	} else {
+		offset = 0; /* 100MHz */
+	}
+	
+	switch (speed) {
+	case XFER_UDMA_6:
+		idx = 6;
+		break;
+	case XFER_UDMA_5:
+		idx = 5;
+		break;
+	case XFER_UDMA_4:
+		idx = 4;
+		break;
+	case XFER_UDMA_3:
+		idx = 3;
+		break;
+	case XFER_UDMA_2:
+		idx = 2;
+		break;
+	case XFER_UDMA_1:
+		idx = 1;
+		break;
+	case XFER_UDMA_0:
+		idx = 0;
+		break;
+	default:
+		return 1;
+	}
+	
+	jcactsel = JCACTSELtbl[offset][idx];
+	if (is_slave) {
+		hwif->OUTL(JCHDCTxtbl[offset][idx], sdmact_port);
+		hwif->OUTL(JCSTWTxtbl[offset][idx], scrcst_port);
+		jcactsel = jcactsel << 2 ;
+		hwif->OUTL( (hwif->INL( tdvhsel_port ) & ~TDVHSEL_SLAVE) | jcactsel, tdvhsel_port );
+	} else {
+		hwif->OUTL(JCHDCTxtbl[offset][idx], mdmact_port);
+		hwif->OUTL(JCSTWTxtbl[offset][idx], mcrcst_port);
+		hwif->OUTL( (hwif->INL( tdvhsel_port ) & ~TDVHSEL_MASTER) | jcactsel, tdvhsel_port );
+	}
+	reg = JCTSStbl[offset][idx] << 16 | JCENVTtbl[offset][idx];
+	hwif->OUTL(reg, udenvt_port);
+	
+	return ide_config_drive_speed(drive, speed);
+}
+
+/**
+ *	scc_config_chipset_for_dma	-	configure for DMA
+ *	@drive: drive to configure
+ *
+ *	Called by scc_config_drive_for_dma().
+ */
+ 
+static int scc_config_chipset_for_dma(ide_drive_t *drive)
+{
+	u8 speed = ide_dma_speed(drive, scc_ratemask(drive));
+	
+	if (!speed)
+		return 0;
+	
+	if (ide_set_xfer_rate(drive, speed))
+		return 0;
+	
+	if (!drive->init_speed)
+		drive->init_speed = speed;
+	
+	return ide_dma_enable(drive);
+}
+
+/**
+ *	scc_configure_drive_for_dma	-	set up for DMA transfers
+ *	@drive: drive we are going to set up
+ *
+ *	Set up the drive for DMA, tune the controller and drive as 
+ *	required. 
+ *      If the drive isn't suitable for DMA or we hit other problems
+ *      then we will drop down to PIO and set up PIO appropriately.
+ *      (return 1)
+ */
+ 
+static int scc_config_drive_for_dma(ide_drive_t *drive)
+{
+	ide_hwif_t *hwif = HWIF(drive);
+	struct hd_driveid *id = drive->id;
+  
+	if ((id->capability & 1) != 0 && drive->autodma) {
+		if (ide_use_dma(drive)) {
+			if (scc_config_chipset_for_dma(drive))
+				return hwif->ide_dma_on(drive);
+		}
+		goto fast_ata_pio;
+	} else if ((id->capability & 8) || (id->field_valid & 2)) {
+	fast_ata_pio:
+		hwif->tuneproc(drive, 4);
+		hwif->ide_dma_off_quietly(drive);
+	}
+	return 1; /* DMA is not supported */
+}
+
+/**
+ *	scc_ide_dma_end	-	Stop DMA
+ *	@drive: IDE drive
+ *
+ *	Check and clear INT Status register.
+ *      Then call __ide_dma_end().
+ */
+
+static int scc_ide_dma_end(ide_drive_t * drive)
+{
+	ide_hwif_t *hwif = HWIF(drive);
+	unsigned long intsts_port = hwif->dma_base + 0x014;
+	u32 reg;
+	
+	while (1) {
+		reg = hwif->INL(intsts_port);
+		
+		if (reg & INTSTS_SERROR) {
+			printk(KERN_WARNING "%s: SERROR\n", SCC_PATA_NAME);
+			hwif->OUTL(INTSTS_SERROR|INTSTS_BMSINT, intsts_port);
+			
+			hwif->OUTB(hwif->INB(hwif->dma_command) & ~QCHCD_IOS_SS,
+				   hwif->dma_command);
+			continue;
+		}
+		
+		if (reg & INTSTS_PRERR) {
+			u32 maea0, maec0;
+			unsigned long ctl_base = hwif->config_data;
+			
+			maea0 = hwif->INL(ctl_base + 0xF50);
+			maec0 = hwif->INL(ctl_base + 0xF54);
+			
+			printk(KERN_WARNING "%s: PRERR [addr:%x cmd:%x]\n", SCC_PATA_NAME, maea0, maec0);
+			
+			hwif->OUTL(INTSTS_PRERR|INTSTS_BMSINT, intsts_port);
+			
+			hwif->OUTB(hwif->INB(hwif->dma_command) & ~QCHCD_IOS_SS,
+				   hwif->dma_command);
+			continue;
+		}
+		
+		if (reg & INTSTS_RERR) {
+			printk(KERN_WARNING "%s: Response Error\n", SCC_PATA_NAME);
+			hwif->OUTL(INTSTS_RERR|INTSTS_BMSINT, intsts_port);
+			
+			hwif->OUTB(hwif->INB(hwif->dma_command) & ~QCHCD_IOS_SS,
+				   hwif->dma_command);
+			continue;
+		}
+		
+		if (reg & INTSTS_ICERR) {
+			hwif->OUTB(hwif->INB(hwif->dma_command) & ~QCHCD_IOS_SS,
+				   hwif->dma_command);
+			
+			printk(KERN_WARNING "%s: Illegal Configuration\n", SCC_PATA_NAME);
+			hwif->OUTL(INTSTS_ICERR|INTSTS_BMSINT, intsts_port);
+			continue;
+		}
+		
+		if (reg & INTSTS_BMSINT) {
+			printk(KERN_WARNING "%s: Internal Bus Error\n", SCC_PATA_NAME);
+			hwif->OUTL(INTSTS_BMSINT, intsts_port);
+			
+			ide_do_reset(drive);
+			continue;
+		}
+
+		if (reg & INTSTS_BMHE) {
+			hwif->OUTL(INTSTS_BMHE, intsts_port);
+			continue;
+		}
+		
+		if (reg & INTSTS_ACTEINT) {
+			hwif->OUTL(INTSTS_ACTEINT, intsts_port);
+			continue;
+		}
+		
+		if (reg & INTSTS_IOIRQS) {
+			hwif->OUTL(INTSTS_IOIRQS, intsts_port);
+			continue;
+		}
+		break;
+	}
+	
+	return __ide_dma_end(drive);
+}
+
+/**
+ *	setup_mmio_scc	-	map CTRL/BMID region
+ *	@dev: PCI device we are configuring
+ *	@name: device name
+ *
+ */
+ 
+static int setup_mmio_scc (struct pci_dev *dev, const char *name)
+{
+	unsigned long ctl_base = pci_resource_start(dev, 0);
+	unsigned long dma_base = pci_resource_start(dev, 1);
+	unsigned long ctl_size = pci_resource_len(dev, 0);
+	unsigned long dma_size = pci_resource_len(dev, 1);
+	void *ctl_addr;
+	void *dma_addr;
+	int i;
+  
+	for (i = 0; i < MAX_HWIFS; i++) {
+		if (scc_ports[i].ctl == 0)
+			break;
+	}
+	if (i >= MAX_HWIFS)
+		return -ENOMEM;
+	
+	if (!request_mem_region(ctl_base, ctl_size, name)) {
+		printk(KERN_WARNING "%s: IDE controller MMIO ports not available.\n", SCC_PATA_NAME);
+		goto fail_0;
+	}
+	
+	if (!request_mem_region(dma_base, dma_size, name)) {
+		printk(KERN_WARNING "%s: IDE controller MMIO ports not available.\n", SCC_PATA_NAME);
+		goto fail_1;
+	}
+	
+	if ((ctl_addr = ioremap(ctl_base, ctl_size)) == NULL)
+		goto fail_2;
+	
+	if ((dma_addr = ioremap(dma_base, dma_size)) == NULL)
+		goto fail_3;
+	
+	pci_set_master(dev);
+	scc_ports[i].ctl = (unsigned long)ctl_addr;
+	scc_ports[i].dma = (unsigned long)dma_addr;
+	pci_set_drvdata(dev, (void *) &scc_ports[i]);
+	
+	return 1;
+	
+ fail_3:
+	iounmap(ctl_addr);
+ fail_2:
+	release_mem_region(dma_base, dma_size);
+ fail_1:
+	release_mem_region(ctl_base, ctl_size);
+ fail_0:
+	return -ENOMEM;
+}
+
+/**
+ *	init_setup_scc	-	set up an SCC PATA Controller
+ *	@dev: PCI device
+ *	@d: IDE PCI device
+ *
+ *	Perform the initial set up for this device.
+ */
+
+static int __devinit init_setup_scc(struct pci_dev *dev, ide_pci_device_t *d)
+{
+	unsigned long ctl_base;
+	unsigned long dma_base;
+	unsigned long cckctrl_port;
+	unsigned long intmask_port;
+	unsigned long mode_port;
+	unsigned long ecmode_port;
+	unsigned long dma_status_port;
+	u32 reg = 0;	
+	struct scc_ports *ports;
+	int rc;
+	
+	rc = setup_mmio_scc(dev, d->name);
+	if (rc < 0) {
+		return rc;
+	}
+	
+	ports = pci_get_drvdata(dev);
+	ctl_base = ports->ctl;
+	dma_base = ports->dma;
+	cckctrl_port = ctl_base + 0xff0;
+	intmask_port = dma_base + 0x010;
+	mode_port = ctl_base + 0x024;
+	ecmode_port = ctl_base + 0xf00;
+	dma_status_port = dma_base + 0x004;
+	
+	/* controller initialization */
+	reg = 0;
+	out_be32((void*)cckctrl_port, reg);
+	reg |= CCKCTRL_ATACLKOEN;
+	out_be32((void*)cckctrl_port, reg);
+	reg |= CCKCTRL_LCLKEN | CCKCTRL_OCLKEN;
+	out_be32((void*)cckctrl_port, reg);
+	reg |= CCKCTRL_CRST;
+	out_be32((void*)cckctrl_port, reg);
+	
+	for (;;) {
+		reg = in_be32((void*)cckctrl_port);
+		if (reg & CCKCTRL_CRST)
+			break;
+		udelay(5000);
+	}
+	
+	reg |= CCKCTRL_ATARESET;
+	out_be32((void*)cckctrl_port, reg);
+	
+	out_be32((void*)ecmode_port, ECMODE_VALUE);
+	out_be32((void*)mode_port, MODE_JCUSFEN);
+	out_be32((void*)intmask_port, INTMASK_MSK);
+
+	return ide_setup_pci_device(dev, d);
+}
+
+/**
+ *	init_mmio_iops_scc	-	set up the iops for MMIO
+ *	@hwif: interface to set up
+ *
+ */
+
+static void __devinit init_mmio_iops_scc(ide_hwif_t *hwif)
+{
+	struct pci_dev *dev = hwif->pci_dev;
+	struct scc_ports *ports = pci_get_drvdata(dev);
+	unsigned long dma_base = ports->dma;
+	
+	ide_set_hwifdata(hwif, ports);
+	
+	hwif->INB = scc_ide_inb;
+	hwif->INW = scc_ide_inw;
+	hwif->INL = scc_ide_inl;
+	hwif->INSW = scc_ide_insw;
+	hwif->INSL = scc_ide_insl;
+	hwif->OUTB = scc_ide_outb;
+	hwif->OUTBSYNC = scc_ide_outbsync;
+	hwif->OUTW = scc_ide_outw;
+	hwif->OUTL = scc_ide_outl;
+	hwif->OUTSW = scc_ide_outsw;
+	hwif->OUTSL = scc_ide_outsl;
+	
+	hwif->io_ports[IDE_DATA_OFFSET] = dma_base + 0x20;
+	hwif->io_ports[IDE_ERROR_OFFSET] = dma_base + 0x24;
+	hwif->io_ports[IDE_NSECTOR_OFFSET] = dma_base + 0x28;
+	hwif->io_ports[IDE_SECTOR_OFFSET] = dma_base + 0x2c;
+	hwif->io_ports[IDE_LCYL_OFFSET] = dma_base + 0x30;
+	hwif->io_ports[IDE_HCYL_OFFSET] = dma_base + 0x34;
+	hwif->io_ports[IDE_SELECT_OFFSET] = dma_base + 0x38;
+	hwif->io_ports[IDE_STATUS_OFFSET] = dma_base + 0x3c;
+	hwif->io_ports[IDE_CONTROL_OFFSET] = dma_base + 0x40;
+	
+	hwif->irq = hwif->pci_dev->irq;
+	hwif->dma_base = dma_base;
+	hwif->config_data = ports->ctl;
+	hwif->mmio = 2;
+}
+
+/**
+ *	init_iops_scc	-	set up iops
+ *	@hwif: interface to set up
+ *
+ *	Do the basic setup for the SCC hardware interface
+ *	and then do the MMIO setup.
+ */
+
+static void __devinit init_iops_scc(ide_hwif_t *hwif)
+{
+	struct pci_dev *dev =  hwif->pci_dev;
+	hwif->hwif_data = NULL;
+	if (pci_get_drvdata(dev) == NULL)
+		return;
+	init_mmio_iops_scc(hwif);
+}
+
+/**
+ *	init_hwif_scc	-	set up hwif
+ *	@hwif: interface to set up
+ *
+ *	We do the basic set up of the interface structure. The SCC
+ *	requires several custom handlers so we override the default
+ *	ide DMA handlers appropriately.
+ */
+
+static void __devinit init_hwif_scc(ide_hwif_t *hwif)
+{
+	struct scc_ports *ports = ide_get_hwifdata(hwif);
+
+	ports->hwif_id = hwif->index;
+	
+	hwif->dma_command = hwif->dma_base;
+	hwif->dma_status = hwif->dma_base + 0x04;
+	hwif->dma_prdtable = hwif->dma_base + 0x08;
+	
+	hwif->OUTL(hwif->dmatable_dma, (hwif->dma_base + 0x018)); /* PTERADD */
+	
+	hwif->ide_dma_end = scc_ide_dma_end;
+	hwif->speedproc = scc_tune_chipset;
+	hwif->tuneproc = scc_tuneproc;
+	hwif->ide_dma_check = scc_config_drive_for_dma;
+	
+	hwif->drives[0].autotune = IDE_TUNE_AUTO;
+	hwif->drives[1].autotune = IDE_TUNE_AUTO;
+	
+	if (hwif->INL(hwif->config_data + 0xff0) & CCKCTRL_ATACLKOEN) {
+		hwif->ultra_mask = 0x7f; /* 133MHz */
+	} else {
+		hwif->ultra_mask = 0x3f; /* 100MHz */
+	}
+	hwif->mwdma_mask = 0x00;
+	hwif->swdma_mask = 0x00;
+	hwif->atapi_dma = 1;
+	
+	/* we support 80c cable only. */
+	hwif->udma_four = 1;
+	
+	hwif->autodma = 0;
+	if (!noautodma)
+		hwif->autodma = 1;
+	hwif->drives[0].autodma = hwif->autodma;
+	hwif->drives[1].autodma = hwif->autodma;
+}
+
+#define DECLARE_SCC_DEV(name_str)			\
+  {							\
+      .name		= name_str,			\
+      .init_setup	= init_setup_scc,		\
+      .init_iops	= init_iops_scc,		\
+      .init_hwif	= init_hwif_scc,		\
+      .channels	= 1,					\
+      .autodma	= AUTODMA,				\
+      .bootable	= ON_BOARD,				\
+  }
+
+static ide_pci_device_t scc_chipsets[] __devinitdata = {
+	/* 0 */ DECLARE_SCC_DEV("sccIDE"),
+};
+
+/**
+ *	scc_init_one	-	pci layer discovery entry
+ *	@dev: PCI device
+ *	@id: ident table entry
+ *
+ *	Called by the PCI code when it finds an SCC PATA controller.
+ *	We then use the IDE PCI generic helper to do most of the work.
+ */
+
+static int __devinit scc_init_one(struct pci_dev *dev, const struct pci_device_id *id)
+{
+	ide_pci_device_t *d = &scc_chipsets[id->driver_data];
+	return d->init_setup(dev, d);
+}
+
+/**
+ *	scc_remove	-	pci layer remove entry
+ *	@dev: PCI device
+ *
+ *	Called by the PCI code when it removes an SCC PATA controller.
+ */
+
+static void __devexit scc_remove(struct pci_dev *dev)
+{
+	struct scc_ports *ports = pci_get_drvdata(dev);
+	ide_hwif_t *hwif = &ide_hwifs[ports->hwif_id];
+	unsigned long ctl_base = pci_resource_start(dev, 0);
+	unsigned long dma_base = pci_resource_start(dev, 1);
+	unsigned long ctl_size = pci_resource_len(dev, 0);
+	unsigned long dma_size = pci_resource_len(dev, 1);
+	
+	if (hwif->dmatable_cpu) {
+		pci_free_consistent(hwif->pci_dev,
+				    PRD_ENTRIES * PRD_BYTES,
+				    hwif->dmatable_cpu,
+				    hwif->dmatable_dma);
+		hwif->dmatable_cpu = NULL;
+	}
+	
+	ide_unregister(hwif->index);
+	
+	hwif->chipset = ide_unknown;
+	iounmap((void*)ports->dma);
+	iounmap((void*)ports->ctl);
+	release_mem_region(dma_base, dma_size);
+	release_mem_region(ctl_base, ctl_size);
+	memset(ports, 0, sizeof(*ports));
+}
+
+static struct pci_device_id scc_pci_tbl[] = {
+	{ PCI_VENDOR_ID_TOSHIBA_2, PCI_DEVICE_ID_TOSHIBA_SCC_ATA,  PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
+	{ 0, },
+};
+MODULE_DEVICE_TABLE(pci, scc_pci_tbl);
+
+static struct pci_driver driver = {
+	.name = "SCC IDE",
+	.id_table = scc_pci_tbl,
+	.probe = scc_init_one,
+	.remove = scc_remove,
+};
+
+static int scc_ide_init(void)
+{
+	return ide_pci_register_driver(&driver);
+}
+
+module_init(scc_ide_init);
+/* -- No exit code?
+static void scc_ide_exit(void)
+{
+	ide_pci_unregister_driver(&driver);
+}
+module_exit(scc_ide_exit);
+ */
+
+
+MODULE_DESCRIPTION("PCI driver module for Toshiba SCC IDE");
+MODULE_LICENSE("GPL");

Re: [PATCH 11/16] PATA driver for Celleb

[Christoph Hellwig]

On Wed, Nov 15, 2006 at 06:49:15PM +0900, Ishizaki Kou wrote:
> This patch adds PATA driver for Celleb platform.

This needs to go to linux-ide for review.  But IIRC new PATA drivers
should use libata these days, not the old ide layer.

Re: [PATCH 11/16] PATA driver for Celleb

[Benjamin Herrenschmidt]

On Wed, 2006-11-15 at 19:44 +0100, Christoph Hellwig wrote:
> On Wed, Nov 15, 2006 at 06:49:15PM +0900, Ishizaki Kou wrote:
> > This patch adds PATA driver for Celleb platform.
> 
> This needs to go to linux-ide for review.  But IIRC new PATA drivers
> should use libata these days, not the old ide layer.

While I agree, there are still issues with libata vs. devices that needs
special IOps. libata allows you to override writing/reading taskfiles,
but there are cases, specifically in the EH/bus reset code, where it
still assumes the standard taskfile byte access format for a few things
and while it's possible to "override" these, that means re-implementing
a lot of logic in the driver.

I need to sort that out with Jeff one of these days as there is a
similar problem with the mpc52xx IDE driver.

In the meantime, I reckon it's ok to have a drivers/ide driver for
2.6.20

I have another question for Toshiba though. From my experience, the
Spider chip is very lousy vs. ordering of requests (it has several
severe violations of the PCI spec on the PCI and PCIe busses, requiring
the workarounds I've implemented in my recent serie of patches, the
worst one being that reads don't flush pending writes in the other
direction).

What is the kind of ordering guarantees provided by the IDE interface in
that case ? The main issue with IDE DMA is that you need to make sure
that by the time your ide_dma_end callback returns, all outstanding DMAs
done by the chip have completed. If the chip was writing to memory (due
to an IDE DMA READ operation), you must guarantee that all the data are
visible to the BE by the time you return from ide_dma_end().

This is typically not working with standard IDE controllers on spider as
they rely on an IO read from the device status register to perform that
flush of pending DMAs, according to the PCI spec, but on spider, no
flush occurs unless the "dummy read" function is used. That's the reason
why I implement a dummy read access after every MMIO read access in my
workarounds.

Thus there are two potential issues you need to clarify here:

 - Does your IDE controller guarantee some sort of ordering or not at
all ? I see that you have your own dma_end callback which does a read
from the "intsts_port" register. Will that guarantee a flush of
outstanding DMAs to the BE ?

 - Regarding PCI and PCIe on SCC, I didn't see any code in your PCI
support code for handling that issue. Is it fixed ? If not, then I
suppose you'll have problem with most PCI device/drivers on the field as
they do rely on such ordering to be provided. There was also an issue
with prefetch on PCI, for which I currently disable the prefetch in the
workarounds for the cell blade. I don't know if you handle that at all
neither.

Regards,
Ben.

Re: [PATCH 11/16] PATA driver for Celleb

[Akira Iguchi]

Hi, Christoph-san, Ben-san.

I'm Akira Iguchi, one of Toshiba Linux member.
We thank you very much for commenting.

>On Wed, 2006-11-15 at 19:44 +0100, Christoph Hellwig wrote:
>> On Wed, Nov 15, 2006 at 06:49:15PM +0900, Ishizaki Kou wrote:
>> > This patch adds PATA driver for Celleb platform.
>> 
>> This needs to go to linux-ide for review.  But IIRC new PATA drivers
>> should use libata these days, not the old ide layer.
>
>While I agree, there are still issues with libata vs. devices that needs
>special IOps. libata allows you to override writing/reading taskfiles,
>but there are cases, specifically in the EH/bus reset code, where it
>still assumes the standard taskfile byte access format for a few things
>and while it's possible to "override" these, that means re-implementing
>a lot of logic in the driver.
>
>I need to sort that out with Jeff one of these days as there is a
>similar problem with the mpc52xx IDE driver.
>
>In the meantime, I reckon it's ok to have a drivers/ide driver for
>2.6.20

We use a drivers/ide driver because its design is more suitable for
SCC IDE controller than libata driver. But we will try to implement
the libata driver as needed. This work needs to override many 
callbacks of ata_port_operations by modifying generic helpers.


>I have another question for Toshiba though. From my experience, the
>Spider chip is very lousy vs. ordering of requests (it has several
>severe violations of the PCI spec on the PCI and PCIe busses, requiring
>the workarounds I've implemented in my recent serie of patches, the
>worst one being that reads don't flush pending writes in the other
>direction).
>
>What is the kind of ordering guarantees provided by the IDE interface in
>that case ? The main issue with IDE DMA is that you need to make sure
>that by the time your ide_dma_end callback returns, all outstanding DMAs
>done by the chip have completed. If the chip was writing to memory (due
>to an IDE DMA READ operation), you must guarantee that all the data are
>visible to the BE by the time you return from ide_dma_end().
>
>This is typically not working with standard IDE controllers on spider as
>they rely on an IO read from the device status register to perform that
>flush of pending DMAs, according to the PCI spec, but on spider, no
>flush occurs unless the "dummy read" function is used. That's the reason
>why I implement a dummy read access after every MMIO read access in my
>workarounds.
>
>Thus there are two potential issues you need to clarify here:
>
> - Does your IDE controller guarantee some sort of ordering or not at
>all ? I see that you have your own dma_end callback which does a read
>from the "intsts_port" register. Will that guarantee a flush of
>outstanding DMAs to the BE ?

We use the PRD transfer end read function on SCC IDE controller
to handle this issue.
This function guarantees that issue of DMA End interrupt and
setting the interrupt bit of status register are done after 
the outstanding write data to memory was pushed. Pushing is 
implemented by the dummy read of IDE controller, which reads 
the same path as the path of the write data. 
Therefore this function is same as the flush of outstanding DMAs.
To use this function, we specify the PRD table address(dmatable_dma) 
in PRD Transfer End Read Base Address Register(dma_base + 0x018) as
the path. Of course it also requires to set a strong order in 
the IOIF space.


> - Regarding PCI and PCIe on SCC, I didn't see any code in your PCI
>support code for handling that issue. Is it fixed ? If not, then I
>suppose you'll have problem with most PCI device/drivers on the field as
>they do rely on such ordering to be provided. There was also an issue
>with prefetch on PCI, for which I currently disable the prefetch in the
>workarounds for the cell blade. I don't know if you handle that at all
>neither.

We've noticed the problem you mentioned, and been developing the support
for dummy read(please refer to previous comments for IDE) and etc. to solve
the problem, but we haven't finished yet.  We also currently  disable the
prefetch by initial setup of SCC.  We are trying now so that we can post
the fixed driver code in next patch.

>Regards,
>Ben.
>

Best regards,
Akira Iguchi
Toshiba

Re: [PATCH 11/16] PATA driver for Celleb

[Christoph Hellwig]

On Thu, Nov 16, 2006 at 10:55:00AM +1100, Benjamin Herrenschmidt wrote:
> > This needs to go to linux-ide for review.  But IIRC new PATA drivers
> > should use libata these days, not the old ide layer.
> 
> While I agree, there are still issues with libata vs. devices that needs
> special IOps. libata allows you to override writing/reading taskfiles,
> but there are cases, specifically in the EH/bus reset code, where it
> still assumes the standard taskfile byte access format for a few things
> and while it's possible to "override" these, that means re-implementing
> a lot of logic in the driver.
> 
> I need to sort that out with Jeff one of these days as there is a
> similar problem with the mpc52xx IDE driver.
> 
> In the meantime, I reckon it's ok to have a drivers/ide driver for
> 2.6.20

I don't know.  And I think we can't make the judgement here, we should
leave it to the IDE maintainers to a proper review and make this
decision, that's why this patch needs to go to linux-ide ASAP.