Re: [PATCH] libata: Add Intel SCH PATA Support

Re: [PATCH] libata: Add Intel SCH PATA Support

[Sergei Shtylyov]

Alek Du wrote:

> This patch adds Intel SCH chipsets

    Looks good, yet there's some nits...

> (US15W, US15L, UL11L) PATA controller support.

    From the Intel's documentation I got the impression that the part numbers 
start with AF82 (82 being the usual Intel's chip number prefix).

> Signed-off-by: Alek Du <alek.du@intel.com>

> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
> index b693d82..ec589e2 100644
> --- a/drivers/ata/Makefile
> +++ b/drivers/ata/Makefile
> @@ -4,6 +4,7 @@ obj-$(CONFIG_ATA)		+= libata.o
>  obj-$(CONFIG_SATA_AHCI)		+= ahci.o
>  obj-$(CONFIG_SATA_SVW)		+= sata_svw.o
>  obj-$(CONFIG_ATA_PIIX)		+= ata_piix.o
> +obj-$(CONFIG_ATA_SCH)		+= ata_sch.o

    The name of the drivers for the pure PATA chips are prefixed with 'pata_', 
not 'ata_'.

>  obj-$(CONFIG_SATA_PROMISE)	+= sata_promise.o
>  obj-$(CONFIG_SATA_QSTOR)	+= sata_qstor.o
>  obj-$(CONFIG_SATA_SIL)		+= sata_sil.o
> diff --git a/drivers/ata/ata_sch.c b/drivers/ata/ata_sch.c
> new file mode 100644
> index 0000000..2c7b358
> --- /dev/null
> +++ b/drivers/ata/ata_sch.c
> @@ -0,0 +1,271 @@

[...]

> +/* see SCH datasheet page 351 */
> +enum {
> +	D0TIM	= 0x80,		/* Device 0 Timing Register */
> +	D1TIM	= 0x84,		/* Device 1 Timing Register */
> +	PIO	= 0x07,		/* PIO Mode Bit Mask */

    The SCH datasheet calls this field PM and since you've got all other names 
form there, why not call it PM too?

> +	MDM	= (0x03 << 8),	/* Multi-word DMA Mode Bit Mask */
> +	UDM	= (0x07 << 16), /* Ultra DMA Mode Bit Mask */
> +	PPE	= (1 << 30),	/* Prefetch/Post Enable */
> +	USD	= (1 << 31),	/* Use Synchronous DMA */
> +};

> +enum sch_controller_ids {
> +	/* controller IDs */
> +	sch_pata_100,		/* SCH up to UDMA 100 */
> +};

    I don't see why you need the above.

> +static unsigned int in_module_init = 1;
> +
> +static struct ata_port_operations sch_pata_ops = {
> +	.inherits		= &ata_bmdma_port_ops,
> +	.cable_detect		= ata_cable_unknown,
> +	.set_piomode		= sch_set_piomode,
> +	.set_dmamode		= sch_set_dmamode,
> +	.prereset		= ata_sff_prereset,

    ata_sff_prereset is the default method, why not just inherit it?

> +/**
> + *	sch_set_piomode - Initialize host controller PATA PIO timings
> + *	@ap: Port whose timings we are configuring
> + *	@adev: um
> + *
> + *	Set PIO mode for device, in host controller PCI config space.
> + *
> + *	LOCKING:
> + *	None (inherited from caller).
> + */
> +
> +static void sch_set_piomode(struct ata_port *ap, struct ata_device *adev)
> +{
> +	unsigned int pio	= adev->pio_mode - XFER_PIO_0;
> +	struct pci_dev *dev	= to_pci_dev(ap->host->dev);
> +	unsigned int port	= adev->devno ? D1TIM : D0TIM;
> +	unsigned int data;
> +
> +	/* SCH only support primary channel with one master and one slave*/
> +	if (ap->port_no != 0)
> +		return;

    There should be no references to the secondary channel from libata since 
you should explicitly specify that there is no secondary channel.

> +
> +	pci_read_config_dword(dev, port, &data);
> +	/* see SCH datasheet page 351 */
> +	/* enable PIO with PPE*/

    No space before */...

> +	data &= ~(PIO);

    Unneeded parens.

> +	data |= (PPE |pio);

    You shouldn't enable prefetch for ATAPI deives I think -- look at what 
ata_piix does...
    Also, unneeded parens, no space after | operator. Be consistent please.

> +	/* mask reserved bits */

    Why?

> +	data = data & (USD|PPE|UDM|MDM|PIO);

    No spaces between | operator and operands.

> +	pci_write_config_dword(dev, port, data);
> +}
> +
> +/**
> + *	sch_set_dmamode - Initialize host controller PATA DMA timings
> + *	@ap: Port whose timings we are configuring
> + *	@adev: um
> + *
> + *	Set MW/UDMA mode for device, in host controller PCI config space.
> + *
> + *	LOCKING:
> + *	None (inherited from caller).
> + */
> +
> +static void sch_set_dmamode(struct ata_port *ap, struct ata_device *adev)
> +{
> +	unsigned int dma_mode	= adev->dma_mode;
> +	struct pci_dev *dev	= to_pci_dev(ap->host->dev);
> +	unsigned int port	= adev->devno ? D1TIM : D0TIM;
> +	unsigned int data;
> +
> +	/* SCH only support primary channel with one master and one slave*/
> +	if (ap->port_no != 0)
> +		return;

    Same comment as in sch_set_piomode().

> +
> +	pci_read_config_dword(dev, port, &data);
> +	/* see SCH datasheet page 351 */
> +	if (dma_mode >= XFER_UDMA_0) {
> +		/* enable Synchronous DMA mode */
> +		data |= USD;
> +		data &= ~(UDM);

    Unneeded parens.

> +		data |= (dma_mode - XFER_UDMA_0) << 16;
> +	} else { /* must be MWDMA mode, since we masked SWDMA already */
> +		data &= ~(USD|MDM);

    No spaces between | and operands.

> +		data |= (dma_mode - XFER_MW_DMA_0) << 8;
> +	}
> +	/* mask reserved bits */

    Why again?

> +	data = data & (USD|PPE|UDM|MDM|PIO);

    No spaces between | and operands.

MBR, Sergei

Re: [PATCH] libata: Add Intel SCH PATA Support

[Sergei Shtylyov]

Alek Du wrote:

>>>(US15W, US15L, UL11L) PATA controller support.

>>    From the Intel's documentation I got the impression that the part numbers
>>start with AF82 (82 being the usual Intel's chip number prefix).

> From documents I got, no 82xxx part number assigned. The formal names for SCH are
> just like US15W, US15L and UL11L.

    Look into the spec. update, "Identification Information" section.

>>    You shouldn't enable prefetch for ATAPI deives I think -- look at what
>>ata_piix does...

> Why? 

    Prefetch usually doesn't work with ATAPI devices since the prefetch logic 
operates in units of 512-byte sectors and not all ATAPI data transfers are a 
multiple of 512 bytes.

MBR, Sergei

[PATCH] libata: Add Intel SCH PATA Support

[Alek Du]

Hi Sergei and All,

I modified the driver again according to comments from Sergei, please help to review it.


>From 9a318d7ff2fc55811374b5b959524f5afcb8d5a6 Mon Sep 17 00:00:00 2001
From: Alek Du <alek.du@intel.com>
Date: Mon, 5 May 2008 14:32:04 +0800
Subject: [PATCH] libata: Add Intel SCH PATA Support

This patch adds Intel SCH chipsets (AF82US15W, AF82US15L, AF82UL11L) PATA controller
support.

Signed-off-by: Alek Du <alek.du@intel.com>
---
 drivers/ata/Kconfig    |    9 ++
 drivers/ata/Makefile   |    1 +
 drivers/ata/pata_sch.c |  244 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 254 insertions(+), 0 deletions(-)
 create mode 100644 drivers/ata/pata_sch.c

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 1c11df9..1a59f30 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -697,6 +697,15 @@ config PATA_SCC
 
 	  If unsure, say N.
 
+config PATA_SCH
+	tristate "Intel SCH PATA support"
+	depends on PCI
+	help
+	  This option enables support for Intel SCH PATA on the Intel
+	  SCH (US15W, US15L, UL11L) series host controllers.
+
+	  If unsure, say N.
+
 config PATA_BF54X
 	tristate "Blackfin 54x ATAPI support"
 	depends on BF542 || BF548 || BF549
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index b693d82..674965f 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_PATA_SIS)		+= pata_sis.o
 obj-$(CONFIG_PATA_TRIFLEX)	+= pata_triflex.o
 obj-$(CONFIG_PATA_IXP4XX_CF)	+= pata_ixp4xx_cf.o
 obj-$(CONFIG_PATA_SCC)		+= pata_scc.o
+obj-$(CONFIG_PATA_SCH)		+= pata_sch.o
 obj-$(CONFIG_PATA_BF54X)	+= pata_bf54x.o
 obj-$(CONFIG_PATA_PLATFORM)	+= pata_platform.o
 obj-$(CONFIG_PATA_OF_PLATFORM)	+= pata_of_platform.o
diff --git a/drivers/ata/pata_sch.c b/drivers/ata/pata_sch.c
new file mode 100644
index 0000000..84a2384
--- /dev/null
+++ b/drivers/ata/pata_sch.c
@@ -0,0 +1,244 @@
+/*
+ *    pata_sch.c - Intel SCH PATA controllers
+ *
+ *  Copyright (c) 2008 Alek Du <alek.du@intel.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License 2 as published
+ *  by the Free Software Foundation.
+ *
+ *  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; see the file COPYING.  If not, write to
+ *  the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ */
+
+/*
+ *  Supports:
+ *    Intel SCH (AF82US15W, AF82US15L, AF82UL11L) chipsets
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/init.h>
+#include <linux/blkdev.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <scsi/scsi_host.h>
+#include <linux/libata.h>
+#include <linux/dmi.h>
+
+#define DRV_NAME	"pata_sch"
+#define DRV_VERSION	"0.2"
+
+/* see SCH datasheet page 351 */
+enum {
+	D0TIM	= 0x80,		/* Device 0 Timing Register */
+	D1TIM	= 0x84,		/* Device 1 Timing Register */
+	PM	= 0x07,		/* PIO Mode Bit Mask */
+	MDM	= (0x03 << 8),	/* Multi-word DMA Mode Bit Mask */
+	UDM	= (0x07 << 16), /* Ultra DMA Mode Bit Mask */
+	PPE	= (1 << 30),	/* Prefetch/Post Enable */
+	USD	= (1 << 31),	/* Use Synchronous DMA */
+};
+
+static int sch_init_one(struct pci_dev *pdev,
+			 const struct pci_device_id *ent);
+static void sch_set_piomode(struct ata_port *ap, struct ata_device *adev);
+static void sch_set_dmamode(struct ata_port *ap, struct ata_device *adev);
+#ifdef CONFIG_PM
+static int sch_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
+static int sch_pci_device_resume(struct pci_dev *pdev);
+#endif
+
+static unsigned int in_module_init = 1;
+
+static const struct pci_device_id sch_pci_tbl[] = {
+	/* Intel SCH PATA Controller */
+	{ 0x8086, 0x811A, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
+	{ }	/* terminate list */
+};
+
+static struct pci_driver sch_pci_driver = {
+	.name			= DRV_NAME,
+	.id_table		= sch_pci_tbl,
+	.probe			= sch_init_one,
+	.remove			= ata_pci_remove_one,
+#ifdef CONFIG_PM
+	.suspend		= sch_pci_device_suspend,
+	.resume			= sch_pci_device_resume,
+#endif
+};
+
+static struct scsi_host_template sch_sht = {
+	ATA_BMDMA_SHT(DRV_NAME),
+};
+
+static struct ata_port_operations sch_pata_ops = {
+	.inherits		= &ata_bmdma_port_ops,
+	.cable_detect		= ata_cable_unknown,
+	.set_piomode		= sch_set_piomode,
+	.set_dmamode		= sch_set_dmamode,
+};
+
+static struct ata_port_info sch_port_info = {
+	.flags		= 0,
+	.pio_mask	= ATA_PIO4,   /* pio0-4 */
+	.mwdma_mask	= ATA_MWDMA2, /* mwdma0-2 */
+	.udma_mask	= ATA_UDMA5,  /* udma0-5 */
+	.port_ops	= &sch_pata_ops,
+};
+
+MODULE_AUTHOR("Alek Du <alek.du@intel.com>");
+MODULE_DESCRIPTION("SCSI low-level driver for Intel SCH PATA controllers");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(pci, sch_pci_tbl);
+MODULE_VERSION(DRV_VERSION);
+
+/**
+ *	sch_set_piomode - Initialize host controller PATA PIO timings
+ *	@ap: Port whose timings we are configuring
+ *	@adev: um
+ *
+ *	Set PIO mode for device, in host controller PCI config space.
+ *
+ *	LOCKING:
+ *	None (inherited from caller).
+ */
+
+static void sch_set_piomode(struct ata_port *ap, struct ata_device *adev)
+{
+	unsigned int pio	= adev->pio_mode - XFER_PIO_0;
+	struct pci_dev *dev	= to_pci_dev(ap->host->dev);
+	unsigned int port	= adev->devno ? D1TIM : D0TIM;
+	unsigned int data;
+
+	pci_read_config_dword(dev, port, &data);
+	/* see SCH datasheet page 351 */
+	/* set PIO mode without PPE */
+	data &= ~(PM | PPE);
+	data |= pio;
+	pci_write_config_dword(dev, port, data);
+}
+
+/**
+ *	sch_set_dmamode - Initialize host controller PATA DMA timings
+ *	@ap: Port whose timings we are configuring
+ *	@adev: um
+ *
+ *	Set MW/UDMA mode for device, in host controller PCI config space.
+ *
+ *	LOCKING:
+ *	None (inherited from caller).
+ */
+
+static void sch_set_dmamode(struct ata_port *ap, struct ata_device *adev)
+{
+	unsigned int dma_mode	= adev->dma_mode;
+	struct pci_dev *dev	= to_pci_dev(ap->host->dev);
+	unsigned int port	= adev->devno ? D1TIM : D0TIM;
+	unsigned int data;
+
+	pci_read_config_dword(dev, port, &data);
+	/* see SCH datasheet page 351 */
+	if (dma_mode >= XFER_UDMA_0) {
+		/* enable Synchronous DMA mode */
+		data |= USD;
+		data &= ~UDM;
+		data |= (dma_mode - XFER_UDMA_0) << 16;
+	} else { /* must be MWDMA mode, since we masked SWDMA already */
+		data &= ~(USD | MDM);
+		data |= (dma_mode - XFER_MW_DMA_0) << 8;
+	}
+	pci_write_config_dword(dev, port, data);
+}
+
+#ifdef CONFIG_PM
+
+static int sch_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg)
+{
+	struct ata_host *host = dev_get_drvdata(&pdev->dev);
+	int rc;
+
+	rc = ata_host_suspend(host, mesg);
+	if (rc)
+		return rc;
+	ata_pci_device_do_suspend(pdev, mesg);
+	return 0;
+}
+
+static int sch_pci_device_resume(struct pci_dev *pdev)
+{
+	struct ata_host *host = dev_get_drvdata(&pdev->dev);
+	int rc;
+
+	rc = ata_pci_device_do_resume(pdev);
+	if (rc == 0)
+		ata_host_resume(host);
+	return rc;
+}
+
+#endif
+
+/**
+ *	sch_init_one - Register SCH ATA PCI device with kernel services
+ *	@pdev: PCI device to register
+ *	@ent: Entry in sch_pci_tbl matching with @pdev
+ *
+ *	LOCKING:
+ *	Inherited from PCI layer (may sleep).
+ *
+ *	RETURNS:
+ *	Zero on success, or -ERRNO value.
+ */
+
+static int __devinit sch_init_one(struct pci_dev *pdev,
+				   const struct pci_device_id *ent)
+{
+	static int printed_version;
+	const struct ata_port_info *ppi[] = { &sch_port_info, NULL };
+	struct ata_host *host;
+	int rc;
+
+	if (!printed_version++)
+		dev_printk(KERN_DEBUG, &pdev->dev,
+			   "version " DRV_VERSION "\n");
+
+	/* no hotplugging support (FIXME) */
+	if (!in_module_init)
+		return -ENODEV;
+	/* enable device and prepare host */
+	rc = pcim_enable_device(pdev);
+	if (rc)
+		return rc;
+	rc = ata_pci_sff_prepare_host(pdev, ppi, &host);
+	if (rc)
+		return rc;
+	pci_set_master(pdev);
+	return ata_pci_sff_activate_host(host, ata_sff_interrupt, &sch_sht);
+}
+
+static int __init sch_init(void)
+{
+	int rc;
+
+	rc = pci_register_driver(&sch_pci_driver);
+	if (rc)
+		return rc;
+	in_module_init = 0;
+	return 0;
+}
+
+static void __exit sch_exit(void)
+{
+	pci_unregister_driver(&sch_pci_driver);
+}
+
+module_init(sch_init);
+module_exit(sch_exit);
-- 
1.5.2.5

BR,
Alek

Re: [PATCH] libata: Add Intel SCH PATA Support

[Alan Cox]

> +	/* no hotplugging support (FIXME) */
> +	if (!in_module_init)
> +		return -ENODEV;

I see no reason for this check ? AHCI/PIIX sometimes need this because
they rely on BIOS set values [and nobody has fixed it to only check on
those chips]. Your new driver seems clean of such cases.

Alan

Re: [PATCH] libata: Add Intel SCH PATA Support

[Sergei Shtylyov]

Hello.

Alek Du wrote:

> I modified the driver again according to comments from Sergei, please help to review it.

    You even were somewhat over-zealous. ;-)

> This patch adds Intel SCH chipsets (AF82US15W, AF82US15L, AF82UL11L) PATA controller
> support.

> Signed-off-by: Alek Du <alek.du@intel.com>

> diff --git a/drivers/ata/pata_sch.c b/drivers/ata/pata_sch.c
> new file mode 100644
> index 0000000..84a2384
> --- /dev/null
> +++ b/drivers/ata/pata_sch.c
> @@ -0,0 +1,244 @@
[...]
> +/**
> + *	sch_set_piomode - Initialize host controller PATA PIO timings
> + *	@ap: Port whose timings we are configuring
> + *	@adev: um

    Hm, I'm seing the same "um" un ata_piix, and wondering what it means. :-)
do_pata_set_dmamode at least calls this "drive in question"...

> + *
> + *	Set PIO mode for device, in host controller PCI config space.
> + *
> + *	LOCKING:
> + *	None (inherited from caller).
> + */
> +
> +static void sch_set_piomode(struct ata_port *ap, struct ata_device *adev)
> +{
> +	unsigned int pio	= adev->pio_mode - XFER_PIO_0;
> +	struct pci_dev *dev	= to_pci_dev(ap->host->dev);
> +	unsigned int port	= adev->devno ? D1TIM : D0TIM;
> +	unsigned int data;
> +
> +	pci_read_config_dword(dev, port, &data);
> +	/* see SCH datasheet page 351 */
> +	/* set PIO mode without PPE */
> +	data &= ~(PM | PPE);
> +	data |= pio;

    I didn't mean you shouldn't ever set PPE, what I meant was:

         if (adev->class == ATA_DEV_ATA)
                 data |= PPE;

MBR, Sergei

Re: [PATCH] libata: Add Intel SCH PATA Support

[Jeff Garzik]

Alek Du wrote:
> Hi Sergei and All,
> 
> I modified the driver again according to comments from Sergei, please help to review it.
> 
> 
>>From 9a318d7ff2fc55811374b5b959524f5afcb8d5a6 Mon Sep 17 00:00:00 2001
> From: Alek Du <alek.du@intel.com>
> Date: Mon, 5 May 2008 14:32:04 +0800
> Subject: [PATCH] libata: Add Intel SCH PATA Support
> 
> This patch adds Intel SCH chipsets (AF82US15W, AF82US15L, AF82UL11L) PATA controller
> support.
> 
> Signed-off-by: Alek Du <alek.du@intel.com>
> ---
>  drivers/ata/Kconfig    |    9 ++
>  drivers/ata/Makefile   |    1 +
>  drivers/ata/pata_sch.c |  244 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 254 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/ata/pata_sch.c

overall, looks good.

your suspend/resume routines are copies of the in-kernel
ata_pci_device_suspend/resume, and should be removed (just refer 
directly to the generic functions)

agree w/ Alan and Sergei's comments, too

	Jeff

Re: [PATCH] libata: Add Intel SCH PATA Support

[Jeff Garzik]

Also...



Alek Du wrote:
> +/*
> + *  Supports:
> + *    Intel SCH (AF82US15W, AF82US15L, AF82UL11L) chipsets
> + */

a link to the public datasheet would be nice, somewhere in a comment


> +static const struct pci_device_id sch_pci_tbl[] = {
> +	/* Intel SCH PATA Controller */
> +	{ 0x8086, 0x811A, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },

recommend using PCI_VDEVICE() macro

[and it would be nice if someone converted ata_piix.c, too]


> +	{ }	/* terminate list */

> +static int __init sch_init(void)
> +{
> +	int rc;
> +
> +	rc = pci_register_driver(&sch_pci_driver);
> +	if (rc)
> +		return rc;
> +	in_module_init = 0;
> +	return 0;
> +}

with the in_module_init thing removed (per Alan's comment), this can 
become 'return pci_register_driver(...)' nicely

again: [PATCH] libata: Add Intel SCH PATA Support

[Alek Du]

Hi Jeff, Alan, Sergei, Bartlomiej

Modified again according to your good comments! Hope this time it is ok. :-)


>From 7a7f6630e32b135f6d659db8e189748b4e7fd1a4 Mon Sep 17 00:00:00 2001
From: Alek Du <alek.du@intel.com>
Date: Mon, 5 May 2008 14:32:04 +0800
Subject: [PATCH] libata: Add Intel SCH PATA Support

This patch adds Intel SCH chipsets (AF82US15W, AF82US15L, AF82UL11L) PATA controller
support.

Signed-off-by: Alek Du <alek.du@intel.com>
---
 drivers/ata/Kconfig    |    9 ++
 drivers/ata/Makefile   |    1 +
 drivers/ata/pata_sch.c |  206 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 216 insertions(+), 0 deletions(-)
 create mode 100644 drivers/ata/pata_sch.c

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 1c11df9..1a59f30 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -697,6 +697,15 @@ config PATA_SCC
 
 	  If unsure, say N.
 
+config PATA_SCH
+	tristate "Intel SCH PATA support"
+	depends on PCI
+	help
+	  This option enables support for Intel SCH PATA on the Intel
+	  SCH (US15W, US15L, UL11L) series host controllers.
+
+	  If unsure, say N.
+
 config PATA_BF54X
 	tristate "Blackfin 54x ATAPI support"
 	depends on BF542 || BF548 || BF549
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index b693d82..674965f 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_PATA_SIS)		+= pata_sis.o
 obj-$(CONFIG_PATA_TRIFLEX)	+= pata_triflex.o
 obj-$(CONFIG_PATA_IXP4XX_CF)	+= pata_ixp4xx_cf.o
 obj-$(CONFIG_PATA_SCC)		+= pata_scc.o
+obj-$(CONFIG_PATA_SCH)		+= pata_sch.o
 obj-$(CONFIG_PATA_BF54X)	+= pata_bf54x.o
 obj-$(CONFIG_PATA_PLATFORM)	+= pata_platform.o
 obj-$(CONFIG_PATA_OF_PLATFORM)	+= pata_of_platform.o
diff --git a/drivers/ata/pata_sch.c b/drivers/ata/pata_sch.c
new file mode 100644
index 0000000..c8cc027
--- /dev/null
+++ b/drivers/ata/pata_sch.c
@@ -0,0 +1,206 @@
+/*
+ *  pata_sch.c - Intel SCH PATA controllers
+ *
+ *  Copyright (c) 2008 Alek Du <alek.du@intel.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License 2 as published
+ *  by the Free Software Foundation.
+ *
+ *  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; see the file COPYING.  If not, write to
+ *  the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ */
+
+/*
+ *  Supports:
+ *    Intel SCH (AF82US15W, AF82US15L, AF82UL11L) chipsets -- see spec at:
+ *    http://download.intel.com/design/chipsets/embedded/datashts/319537.pdf
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/init.h>
+#include <linux/blkdev.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <scsi/scsi_host.h>
+#include <linux/libata.h>
+#include <linux/dmi.h>
+
+#define DRV_NAME	"pata_sch"
+#define DRV_VERSION	"0.2"
+
+/* see SCH datasheet page 351 */
+enum {
+	D0TIM	= 0x80,		/* Device 0 Timing Register */
+	D1TIM	= 0x84,		/* Device 1 Timing Register */
+	PM	= 0x07,		/* PIO Mode Bit Mask */
+	MDM	= (0x03 << 8),	/* Multi-word DMA Mode Bit Mask */
+	UDM	= (0x07 << 16), /* Ultra DMA Mode Bit Mask */
+	PPE	= (1 << 30),	/* Prefetch/Post Enable */
+	USD	= (1 << 31),	/* Use Synchronous DMA */
+};
+
+static int sch_init_one(struct pci_dev *pdev,
+			 const struct pci_device_id *ent);
+static void sch_set_piomode(struct ata_port *ap, struct ata_device *adev);
+static void sch_set_dmamode(struct ata_port *ap, struct ata_device *adev);
+
+static const struct pci_device_id sch_pci_tbl[] = {
+	/* Intel SCH PATA Controller */
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SCH_IDE), 0 },
+	{ }	/* terminate list */
+};
+
+static struct pci_driver sch_pci_driver = {
+	.name			= DRV_NAME,
+	.id_table		= sch_pci_tbl,
+	.probe			= sch_init_one,
+	.remove			= ata_pci_remove_one,
+#ifdef CONFIG_PM
+	.suspend		= ata_pci_device_suspend,
+	.resume			= ata_pci_device_resume,
+#endif
+};
+
+static struct scsi_host_template sch_sht = {
+	ATA_BMDMA_SHT(DRV_NAME),
+};
+
+static struct ata_port_operations sch_pata_ops = {
+	.inherits		= &ata_bmdma_port_ops,
+	.cable_detect		= ata_cable_unknown,
+	.set_piomode		= sch_set_piomode,
+	.set_dmamode		= sch_set_dmamode,
+};
+
+static struct ata_port_info sch_port_info = {
+	.flags		= 0,
+	.pio_mask	= ATA_PIO4,   /* pio0-4 */
+	.mwdma_mask	= ATA_MWDMA2, /* mwdma0-2 */
+	.udma_mask	= ATA_UDMA5,  /* udma0-5 */
+	.port_ops	= &sch_pata_ops,
+};
+
+MODULE_AUTHOR("Alek Du <alek.du@intel.com>");
+MODULE_DESCRIPTION("SCSI low-level driver for Intel SCH PATA controllers");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(pci, sch_pci_tbl);
+MODULE_VERSION(DRV_VERSION);
+
+/**
+ *	sch_set_piomode - Initialize host controller PATA PIO timings
+ *	@ap: Port whose timings we are configuring
+ *	@adev: ATA device
+ *
+ *	Set PIO mode for device, in host controller PCI config space.
+ *
+ *	LOCKING:
+ *	None (inherited from caller).
+ */
+
+static void sch_set_piomode(struct ata_port *ap, struct ata_device *adev)
+{
+	unsigned int pio	= adev->pio_mode - XFER_PIO_0;
+	struct pci_dev *dev	= to_pci_dev(ap->host->dev);
+	unsigned int port	= adev->devno ? D1TIM : D0TIM;
+	unsigned int data;
+
+	pci_read_config_dword(dev, port, &data);
+	/* see SCH datasheet page 351 */
+	/* set PIO mode */
+	data &= ~(PM | PPE);
+	data |= pio;
+	/* enable PPE for block device */
+	if (adev->class == ATA_DEV_ATA)
+		data |= PPE;
+	pci_write_config_dword(dev, port, data);
+}
+
+/**
+ *	sch_set_dmamode - Initialize host controller PATA DMA timings
+ *	@ap: Port whose timings we are configuring
+ *	@adev: ATA device
+ *
+ *	Set MW/UDMA mode for device, in host controller PCI config space.
+ *
+ *	LOCKING:
+ *	None (inherited from caller).
+ */
+
+static void sch_set_dmamode(struct ata_port *ap, struct ata_device *adev)
+{
+	unsigned int dma_mode	= adev->dma_mode;
+	struct pci_dev *dev	= to_pci_dev(ap->host->dev);
+	unsigned int port	= adev->devno ? D1TIM : D0TIM;
+	unsigned int data;
+
+	pci_read_config_dword(dev, port, &data);
+	/* see SCH datasheet page 351 */
+	if (dma_mode >= XFER_UDMA_0) {
+		/* enable Synchronous DMA mode */
+		data |= USD;
+		data &= ~UDM;
+		data |= (dma_mode - XFER_UDMA_0) << 16;
+	} else { /* must be MWDMA mode, since we masked SWDMA already */
+		data &= ~(USD | MDM);
+		data |= (dma_mode - XFER_MW_DMA_0) << 8;
+	}
+	pci_write_config_dword(dev, port, data);
+}
+
+/**
+ *	sch_init_one - Register SCH ATA PCI device with kernel services
+ *	@pdev: PCI device to register
+ *	@ent: Entry in sch_pci_tbl matching with @pdev
+ *
+ *	LOCKING:
+ *	Inherited from PCI layer (may sleep).
+ *
+ *	RETURNS:
+ *	Zero on success, or -ERRNO value.
+ */
+
+static int __devinit sch_init_one(struct pci_dev *pdev,
+				   const struct pci_device_id *ent)
+{
+	static int printed_version;
+	const struct ata_port_info *ppi[] = { &sch_port_info, NULL };
+	struct ata_host *host;
+	int rc;
+
+	if (!printed_version++)
+		dev_printk(KERN_DEBUG, &pdev->dev,
+			   "version " DRV_VERSION "\n");
+
+	/* enable device and prepare host */
+	rc = pcim_enable_device(pdev);
+	if (rc)
+		return rc;
+	rc = ata_pci_sff_prepare_host(pdev, ppi, &host);
+	if (rc)
+		return rc;
+	pci_set_master(pdev);
+	return ata_pci_sff_activate_host(host, ata_sff_interrupt, &sch_sht);
+}
+
+static int __init sch_init(void)
+{
+	return pci_register_driver(&sch_pci_driver);
+}
+
+static void __exit sch_exit(void)
+{
+	pci_unregister_driver(&sch_pci_driver);
+}
+
+module_init(sch_init);
+module_exit(sch_exit);
-- 
1.5.2.5

BR,
Alek

Re: again: [PATCH] libata: Add Intel SCH PATA Support

[Jeff Garzik]

Alek Du wrote:
> Hi Jeff, Alan, Sergei, Bartlomiej
> 
> Modified again according to your good comments! Hope this time it is ok. :-)
> 
> 
> From 7a7f6630e32b135f6d659db8e189748b4e7fd1a4 Mon Sep 17 00:00:00 2001
> From: Alek Du <alek.du@intel.com>
> Date: Mon, 5 May 2008 14:32:04 +0800
> Subject: [PATCH] libata: Add Intel SCH PATA Support
> 
> This patch adds Intel SCH chipsets (AF82US15W, AF82US15L, AF82UL11L) PATA controller
> support.
> 
> Signed-off-by: Alek Du <alek.du@intel.com>
> ---
>  drivers/ata/Kconfig    |    9 ++
>  drivers/ata/Makefile   |    1 +
>  drivers/ata/pata_sch.c |  206 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 216 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/ata/pata_sch.c

applied

for future reference, multiple revisions of patches are typically done like

   Subject: [PATCH v2] libata: Add Intel SCH PATA Support
   Subject: [PATCH v3] libata: Add Intel SCH PATA Support
   etc.

Re: again: [PATCH] libata: Add Intel SCH PATA Support

[Bartlomiej Zolnierkiewicz]

On Tuesday 06 May 2008, Alek Du wrote:
> Hi Jeff, Alan, Sergei, Bartlomiej
> 
> Modified again according to your good comments! Hope this time it is ok. :-)

Hi,

Looks good, are you going to submit IDE version soon?

Thanks,
Bart

Re: again: [PATCH] libata: Add Intel SCH PATA Support

[Alan Cox]

On Tue, 6 May 2008 18:20:42 +0200
Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:

> On Tuesday 06 May 2008, Alek Du wrote:
> > Hi Jeff, Alan, Sergei, Bartlomiej
> > 
> > Modified again according to your good comments! Hope this time it is ok. :-)
> 
> Hi,
> 
> Looks good, are you going to submit IDE version soon?

Is there any point adding new drivers to the old IDE layer nowdays ?

Alan

Re: again: [PATCH] libata: Add Intel SCH PATA Support

[Alek Du]

Hi,

On Tue, 6 May 2008 16:59:37 +0100
Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> On Tue, 6 May 2008 18:20:42 +0200
> Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> 
> > On Tuesday 06 May 2008, Alek Du wrote:
> > > Hi Jeff, Alan, Sergei, Bartlomiej
> > > 
> > > Modified again according to your good comments! Hope this time it is ok. :-)
> > 
> > Hi,
> > 
> > Looks good, are you going to submit IDE version soon?
> 
> Is there any point adding new drivers to the old IDE layer nowdays ?

Yeah, I really was hesitating to write the corresponding IDE driver for a while. If it really could help
people who have good reason to use IDE layer instead of libata, I will do that.

Thanks,
Alek

Re: again: [PATCH] libata: Add Intel SCH PATA Support

[Bartlomiej Zolnierkiewicz]

On Tuesday 06 May 2008, Alan Cox wrote:
> On Tue, 6 May 2008 18:20:42 +0200
> Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> 
> > On Tuesday 06 May 2008, Alek Du wrote:
> > > Hi Jeff, Alan, Sergei, Bartlomiej
> > > 
> > > Modified again according to your good comments! Hope this time it is ok. :-)
> > 
> > Hi,
> > 
> > Looks good, are you going to submit IDE version soon?
> 
> Is there any point adding new drivers to the old IDE layer nowdays ?

Not all distros want to use libata SCSI to ATA emulation layer for PATA,
and IDE/ATA subsystem is more embedded friendly (code-size wise etc.).

Thanks,
Bart