ata: Add Intel SCH PATA support

PATCH In review Alek Du 4 participants 23 messages Apr 26, 2008 – Apr 30, 2008

Subsystems (2)

PCI libata

lore

Share & tools

lore

Alek Du

author

2008-04-26 06:06 UTC #0 lore raw

PATCH 1/1 ata: Add Intel SCH PATA support

Seems I need to post the patch here instead of LKML

[PATCH] ata: Add Intel SCH PATA support

This patch adds Intel SCH chipsets (US15W, US15L, UL11L) PATA controller support.

Signed-off-by Alek Du <[email protected]>

---
 include/linux/pci_ids.h |    2 ++
 drivers/ata/ata_piix.c  |   40 ++++++++++++++++++++++++++++++++++++++--
 drivers/ide/pci/piix.c  |    2 ++
 3 files changed, 42 insertions(+), 2 deletions(-)

include/linux/pci_ids.h +2 -0

index 70eb3c8..b72b3b4 100644
@@ -2413,6 +2413,8 @@
 #define PCI_DEVICE_ID_INTEL_82443GX_0  0x71a0
 #define PCI_DEVICE_ID_INTEL_82443GX_2  0x71a2
 #define PCI_DEVICE_ID_INTEL_82372FB_1  0x7601
+#define PCI_DEVICE_ID_INTEL_SCH_LPC    0x8119
+#define PCI_DEVICE_ID_INTEL_SCH_IDE    0x811a
 #define PCI_DEVICE_ID_INTEL_82454GX    0x84c4
 #define PCI_DEVICE_ID_INTEL_82450GX    0x84c5
 #define PCI_DEVICE_ID_INTEL_82451NX    0x84ca

drivers/ata/ata_piix.c +38 -2

index b7c38ee..7f95a9a 100644
@@ -214,6 +214,8 @@ static const struct pci_device_id piix_pci_tbl[] = {
        /* ICH7/7-R (i945, i975) UDMA 100*/
        { 0x8086, 0x27DF, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
        { 0x8086, 0x269E, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
+       /* INTEL SCH UDMA 100 */
+       { 0x8086, 0x811A, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
        /* ICH8 Mobile PATA Controller */
        { 0x8086, 0x2850, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },

@@ -561,6 +563,10 @@ struct ich_laptop {
        u16 subdevice;
 };
 
+struct sch_80 {
+       u16 device;
+};
+
 /*
  *     List of laptops that use short cables rather than 80 wire
  */

@@ -577,6 +583,17 @@ static const struct ich_laptop ich_laptop[] = {
        { 0, }
 };
 
+/*
+ *     List of chipsets whose port IOCFG and enable bit registers are reserved
+ */
+
+static const struct sch_80 sch_80[] = {
+       /* devid */
+       { 0x811A },     /* Intel SCH chipset */
+       /* end marker */
+       { 0, }
+};
+
 /**
  *     ich_pata_cable_detect - Probe host controller cable detect info
  *     @ap: Port for which cable detect info is desired

@@ -592,6 +609,7 @@ static int ich_pata_cable_detect(struct ata_port *ap)
 {
        struct pci_dev *pdev = to_pci_dev(ap->host->dev);
        const struct ich_laptop *lap = &ich_laptop[0];
+       const struct sch_80 *sch = &sch_80[0];
        u8 tmp, mask;
 
        /* Check for specials - Acer Aspire 5602WLMi */

@@ -604,6 +622,13 @@ static int ich_pata_cable_detect(struct ata_port *ap)
                lap++;
        }
 
+       /* Check for specials - Intel SCH chipset */
+       while (sch->device) {
+               if (sch->device == pdev->device)
+                       return ATA_CBL_PATA80;
+               sch++;
+       }
+
        /* check BIOS cable detect results */
        mask = ap->port_no == 0 ? PIIX_80C_PRI : PIIX_80C_SEC;
        pci_read_config_byte(pdev, PIIX_IOCFG, &tmp);

@@ -624,9 +649,20 @@ static int piix_pata_prereset(struct ata_link *link, unsigned long deadline)
 {
        struct ata_port *ap = link->ap;
        struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+       const struct sch_80 *sch = &sch_80[0];
+       int skip_check = 0;
 
-       if (!pci_test_config_bits(pdev, &piix_enable_bits[ap->port_no]))
-               return -ENOENT;
+       /* Check for specials - Intel SCH chipset */
+       while (sch->device) {
+               if (sch->device == pdev->device) {
+                       skip_check = 1;
+                       break;
+               }
+               sch++;
+       }
+       if (!skip_check)
+               if (!pci_test_config_bits(pdev, &piix_enable_bits[ap->port_no]))
+                       return -ENOENT;
        return ata_sff_prereset(link, deadline);
 }

drivers/ide/pci/piix.c +2 -0

index decef0f..dce830b 100644
@@ -377,6 +377,7 @@ static const struct ide_port_info piix_pci_info[] __devinitdata = {
        /* 22 */ DECLARE_ICH_DEV("ICH4",        ATA_UDMA5),
        /* 23 */ DECLARE_ICH_DEV("ESB2",        ATA_UDMA5),
        /* 24 */ DECLARE_ICH_DEV("ICH8M",       ATA_UDMA5),
+       /* 25 */ DECLARE_ICH_DEV("SCH",         ATA_UDMA5),
 };
 
 /**

@@ -450,6 +451,7 @@ static const struct pci_device_id piix_pci_tbl[] = {
        { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_82801DB_1),  22 },
        { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ESB2_18),    23 },
        { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ICH8_6),     24 },
+       { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SCH_IDE),      25 },
        { 0, },
 };
 MODULE_DEVICE_TABLE(pci, piix_pci_tbl);

--
1.5.2.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Alan Cox

2008-04-26 09:37 UTC #1 lore raw

drivers/ata/ata_piix.c struct ich_laptop { · line 568

Quoted 3 lines

drivers/ata/ata_piix.c +3 -0

+struct sch_80 {
+       u16 device;
+};

That seems somewhat overkill

drivers/ata/ata_piix.c static int ich_pata_cable_detect(struct ata_port *ap) · line 629

Quoted 5 lines

drivers/ata/ata_piix.c +5 -0

+       /* Check for specials - Intel SCH chipset */
+       while (sch->device) {
+               if (sch->device == pdev->device)
+                       return ATA_CBL_PATA80;
+               sch++;

PATA_80 means "controller *knows* we are 80 wire". PATA_UNK means "controller can't do detect."

drivers/ata/ata_piix.c static int piix_pata_prereset(struct ata_link *link, unsigned long deadline) · line 661

Quoted 7 lines

drivers/ata/ata_piix.c +7 -0

+       /* Check for specials - Intel SCH chipset */
+       while (sch->device) {
+               if (sch->device == pdev->device) {
+                       skip_check = 1;
+                       break;
+               }
+               sch++;

This is horrible. If you just defined a new set of methods for your new controller (remembering we now have inheritance anyway) you'd just use the ata_sff_prereset and remove all the special casing code. At the very least write the routine once only;)

Quoted 7 lines

drivers/ata/ata_piix.c +4 -0

+       }
+       if (!skip_check)
+               if (!pci_test_config_bits(pdev, &piix_enable_bits[ap->port_no]))
+                       return -ENOENT;
        return ata_sff_prereset(link, deadline);
 }

Alek Du

author

2008-04-26 23:58 UTC #2 lore raw

Alan,
Thanks for your comments:

Quoted 7 lines · On Sat, 26 Apr 2008 17:30:12 +0800 "Alan Cox" <[email protected]> wrote:

Re: [PATCH] ata: Add Intel SCH PATA support
drivers/ata/ata_piix.c struct ich_laptop { · line 568
Quoted 3 lines
drivers/ata/ata_piix.c +3 -0
+struct sch_80 {
+       u16 device;
+};
That seems somewhat overkill

I have to consider there would be other PATA controllers work this way.

Quoted 9 lines

drivers/ata/ata_piix.c static int ich_pata_cable_detect(struct ata_port *ap) · line 629
Quoted 6 lines
drivers/ata/ata_piix.c +5 -0
+       /* Check for specials - Intel SCH chipset */
+       while (sch->device) {
+               if (sch->device == pdev->device)
+                       return ATA_CBL_PATA80;
+               sch++;
PATA_80 means "controller *knows* we are 80 wire". PATA_UNK means "controller can't do detect."

I need to force SCH to use PATA 80 pins to enable UDMA5, just like the way ich_laptop force to use 40 pins.

Quoted 9 lines

drivers/ata/ata_piix.c static int piix_pata_prereset(struct ata_link *link, unsigned long deadline) · line 661

Quoted 8 lines

drivers/ata/ata_piix.c +7 -0

+       /* Check for specials - Intel SCH chipset */
+       while (sch->device) {
+               if (sch->device == pdev->device) {
+                       skip_check = 1;
+                       break;
+               }
+               sch++;

Quoted 5 lines

This is horrible. If you just defined a new set of methods for your new controller (remembering we now have inheritance anyway) you'd just use the ata_sff_prereset and remove all the special casing code. At the very least write the routine once only;)

hmmm... for non-sch chipsets, there is no impact, this will only affect sch. You mean I should create a ata_sch.c file instead of patching ata_piix.c ?

Quoted 7 lines

drivers/ata/ata_piix.c +4 -0

+       }
+       if (!skip_check)
+               if (!pci_test_config_bits(pdev, &piix_enable_bits[ap->port_no]))
+                       return -ENOENT;
        return ata_sff_prereset(link, deadline);
 }

Alan Cox

2008-04-27 08:41 UTC #3 lore raw

Quoted 5 lines

Quoted 2 lines
PATA_80 means "controller *knows* we are 80 wire". PATA_UNK means "controller can't do detect."
I need to force SCH to use PATA 80 pins to enable UDMA5, just like the way ich_laptop force to use 40 pins.

What if I plug in a non 80 wire device ?

Quoted 2 lines

hmmm... for non-sch chipsets, there is no impact, this will only affect sch. You mean I should create a ata_sch.c file instead of patching ata_piix.c ?

No an sch subtype in the driver just as we have types for ich/piix etc.

Alek Du

author

2008-04-27 14:10 UTC #4 lore raw

Alan,

One problem I still do not understand, if I return PATA_UNK for cable detection result, could the driver finally set the disk to UDMA5 mode if I plug in 80 wire disk?

Thanks,
Alek

Quoted 13 lines · On Sun, 27 Apr 2008 09:34:34 +0100 Alan Cox <[email protected]> wrote:

Quoted 5 lines
Quoted 2 lines
PATA_80 means "controller *knows* we are 80 wire". PATA_UNK means "controller can't do detect."
I need to force SCH to use PATA 80 pins to enable UDMA5, just like the way ich_laptop force to use 40 pins.
What if I plug in a non 80 wire device ?
Quoted 2 lines
hmmm... for non-sch chipsets, there is no impact, this will only affect sch. You mean I should create a ata_sch.c file instead of patching ata_piix.c ?
No an sch subtype in the driver just as we have types for ich/piix etc.

Alan Cox

2008-04-27 14:38 UTC #5 lore raw

Quoted 4 lines · On Sun, 27 Apr 2008 22:03:15 +0800 Alek Du <[email protected]> wrote:

Alan,
One problem I still do not understand, if I return PATA_UNK for cable detection result, could the driver finally set the disk to UDMA5 mode if I plug in 80 wire disk?

You tell me - you've got the hardware documentation I assume. Assuming the system implements detection then it should. If a PATA controller doesn't implement any detection (cable or host side) and it does UDMA5 then I believe the correct description for it is usually "broken" as people will have horrible problems trying to use things like flash drives with it.

Alan

Alek Du

author

2008-04-28 01:27 UTC #6 lore raw

Quoted 10 lines · On Sun, 27 Apr 2008 22:31:11 +0800 "Alan Cox" <[email protected]> wrote:

Re: [PATCH] ata: Add Intel SCH PATA support
Quoted 4 lines · On Sun, 27 Apr 2008 22:03:15 +0800 Alek Du <[email protected]> wrote:
Alan,
One problem I still do not understand, if I return PATA_UNK for cable detection result, could the driver finally set the disk to UDMA5 mode if I plug in 80 wire disk?

Quoted 7 lines

You tell me - you've got the hardware documentation I assume. Assuming the system implements detection then it should. If a PATA controller doesn't implement any detection (cable or host side) and it does UDMA5 then I believe the correct description for it is usually "broken" as people will have horrible problems trying to use things like flash drives with it.
Alan

The data sheet of SCH (http://download.intel.com/design/chipsets/embedded/datashts/319537.pdf) page 347 shows the only supported PATA registers, for those not shown should be treated as "Reserved". Unfortunately, IOCFG (0x54) and port enable bits (0x41, 0x43) are all reserved.

Let's regard SCH PATA controller is an usually "broken" one :-), what do you suggest me to work around it? Force it to 40-wire pin? or use a KConfig option to select 40-wire or 80-wire?

Thanks,
Alek

Alan Cox

2008-04-28 09:23 UTC #7 lore raw

Quoted 2 lines

Let's regard SCH PATA controller is an usually "broken" one :-), what do you suggest me to work around it? Force it to 40-wire pin? or use a KConfig option to select 40-wire or 80-wire?

The drive side detect depends upon hardware features. If the correct capacitors are present in the design then the drive side detect should work.

Alan

Sergei Shtylyov

2008-04-29 17:23 UTC #8 lore raw

In reply to Alek Du

Quoted 16 lines · Alek Du

Quoted 15 lines
Re: [PATCH] ata: Add Intel SCH PATA support
Quoted 4 lines · On Sun, 27 Apr 2008 22:03:15 +0800 Alek Du <[email protected]> wrote:
Alan,
One problem I still do not understand, if I return PATA_UNK for cable detection result, could the driver finally set the disk to UDMA5 mode if I plug in 80 wire disk?
You tell me - you've got the hardware documentation I assume. Assuming the system implements detection then it should. If a PATA controller doesn't implement any detection (cable or host side) and it does UDMA5 then I believe the correct description for it is usually "broken" as people will have horrible problems trying to use things like flash drives with it.

Quoted 5 lines

Quoted 1 line
Alan
The data sheet of SCH (http://download.intel.com/design/chipsets/embedded/datashts/319537.pdf) page 347 shows
the only supported PATA registers, for those not shown should be treated as "Reserved". Unfortunately, IOCFG (0x54) and port enable bits (0x41, 0x43) are all reserved.

If you'd actually compared the datasheet to the driver and/or the PIIX/ICH
datasheets, you would have seen that SCH is not compatible to PIIX/ICH. It doesn't have PIO/DMA and UDMA timing and UDMA enable registers at config. space offsets 0x4x but instead has pair of drive timing registers (SCH PATA controller is single-channel) controlling PIO/DMA/UDMA speeds each at 0x8x, laid out completely differently: PIIX/ICH timing registers fields encoded cycle counts which SCH registers fileds encode mode numbers themselves.

Quoted 1 line

Let's regard SCH PATA controller is an usually "broken" one :-), what do you suggest me to work around it?

Let's regard it as the one deserving its own driver. ;-)

Quoted 2 lines

Thanks,
Alek

MBR, Sergei

Alek Du

author

2008-04-28 04:12 UTC #9 lore raw

PATCH 1/1 ata: Add Intel SCH PATA support (revised)

In reply to Alan Cox

Hi Alan, Jeff

I re-write the patch according to the kind comments from Alan, I also did the test on my SCH machine. Please review it again. Thanks a lot.

This patch adds Intel SCH chipsets (US15W, US15L, UL11L) PATA controller support.

Signed-off-by Alek Du <[email protected]>

---
 drivers/ata/ata_piix.c  |   29 +++++++++++++++++++++++++++++
 drivers/ide/pci/piix.c  |    2 ++
 include/linux/pci_ids.h |    2 ++
 3 files changed, 33 insertions(+), 0 deletions(-)

drivers/ata/ata_piix.c +29 -0

index ea2c764..863c473 100644
@@ -136,6 +136,7 @@ enum piix_controller_ids {
 	ich_pata_33,		/* ICH up to UDMA 33 only */
 	ich_pata_66,		/* ICH up to 66 Mhz */
 	ich_pata_100,		/* ICH up to UDMA 100 */
+	sch_pata_100,		/* SCH up to UDMA 100 */
 	ich5_sata,
 	ich6_sata,
 	ich6m_sata,

@@ -164,6 +165,7 @@ static void piix_set_piomode(struct ata_port *ap, struct ata_device *adev);
 static void piix_set_dmamode(struct ata_port *ap, struct ata_device *adev);
 static void ich_set_dmamode(struct ata_port *ap, struct ata_device *adev);
 static int ich_pata_cable_detect(struct ata_port *ap);
+static int sch_pata_cable_detect(struct ata_port *ap);
 static u8 piix_vmw_bmdma_status(struct ata_port *ap);
 static int piix_sidpr_scr_read(struct ata_port *ap, unsigned int reg, u32 *val);
 static int piix_sidpr_scr_write(struct ata_port *ap, unsigned int reg, u32 val);

@@ -216,6 +218,8 @@ static const struct pci_device_id piix_pci_tbl[] = {
 	{ 0x8086, 0x269E, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
 	/* ICH8 Mobile PATA Controller */
 	{ 0x8086, 0x2850, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
+	/* Intel SCH PATA Controller */
+	{ 0x8086, 0x811A, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sch_pata_100 },
 
 	/* NOTE: The following PCI ids must be kept in sync with the
 	 * list in drivers/pci/quirks.c.

@@ -311,6 +315,13 @@ static struct ata_port_operations ich_pata_ops = {
 	.set_dmamode		= ich_set_dmamode,
 };
 
+static struct ata_port_operations sch_pata_ops = {
+	.inherits		= &piix_pata_ops,
+	.cable_detect		= sch_pata_cable_detect,
+	.prereset		= ata_sff_prereset,
+	.set_dmamode		= ich_set_dmamode,
+};
+
 static struct ata_port_operations piix_sata_ops = {
 	.inherits		= &ata_bmdma_port_ops,
 };

@@ -470,6 +481,15 @@ static struct ata_port_info piix_port_info[] = {
 		.port_ops	= &ich_pata_ops,
 	},
 
+	[sch_pata_100] =
+	{
+		.flags		= PIIX_PATA_FLAGS | PIIX_FLAG_CHECKINTR,
+		.pio_mask	= 0x1f,	/* pio0-4 */
+		.mwdma_mask	= 0x06, /* mwdma1-2 */
+		.udma_mask	= ATA_UDMA5, /* udma0-5 */
+		.port_ops	= &sch_pata_ops,
+	},
+
 	[ich5_sata] =
 	{
 		.flags		= PIIX_SATA_FLAGS,

@@ -614,6 +634,15 @@ static int ich_pata_cable_detect(struct ata_port *ap)
 }
 
 /**
+ * 	Intel SCH PATA controller could not detect cable type
+ * 	and always return ATA_CBL_PATA_UNK here.
+ */
+static int sch_pata_cable_detect(struct ata_port *ap)
+{
+	return ATA_CBL_PATA_UNK;
+}
+
+/**
  *	piix_pata_prereset - prereset for PATA host controller
  *	@link: Target link
  *	@deadline: deadline jiffies for the operation

drivers/ide/pci/piix.c +2 -0

index 21c5dd2..31607fe 100644
@@ -380,6 +380,7 @@ static const struct ide_port_info piix_pci_info[] __devinitdata = {
 	/* 22 */ DECLARE_ICH_DEV("ICH4",	ATA_UDMA5),
 	/* 23 */ DECLARE_ICH_DEV("ESB2",	ATA_UDMA5),
 	/* 24 */ DECLARE_ICH_DEV("ICH8M",	ATA_UDMA5),
+	/* 25 */ DECLARE_ICH_DEV("SCH",		ATA_UDMA5),
 };
 
 /**

@@ -453,6 +454,7 @@ static const struct pci_device_id piix_pci_tbl[] = {
 	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_82801DB_1),  22 },
 	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ESB2_18),    23 },
 	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ICH8_6),     24 },
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SCH_IDE),    25 },
 	{ 0, },
 };
 MODULE_DEVICE_TABLE(pci, piix_pci_tbl);

include/linux/pci_ids.h +2 -0

index 70eb3c8..e5a53da 100644
@@ -2413,6 +2413,8 @@
 #define PCI_DEVICE_ID_INTEL_82443GX_0	0x71a0
 #define PCI_DEVICE_ID_INTEL_82443GX_2	0x71a2
 #define PCI_DEVICE_ID_INTEL_82372FB_1	0x7601
+#define PCI_DEVICE_ID_INTEL_SCH_LPC	0x8119
+#define PCI_DEVICE_ID_INTEL_SCH_IDE	0x811a
 #define PCI_DEVICE_ID_INTEL_82454GX	0x84c4
 #define PCI_DEVICE_ID_INTEL_82450GX	0x84c5
 #define PCI_DEVICE_ID_INTEL_82451NX	0x84ca

Signature · 7 lines

1.5.2.5

Thanks,
Alek


On Sun, 27 Apr 2008 22:31:11 +0800

Quoted 17 lines · Alan Cox

Re: [PATCH] ata: Add Intel SCH PATA support
Quoted 4 lines · On Sun, 27 Apr 2008 22:03:15 +0800 Alek Du <[email protected]> wrote:
Alan,
One problem I still do not understand, if I return PATA_UNK for cable detection result, could the driver finally set the disk to UDMA5 mode if I plug in 80 wire disk?
You tell me - you've got the hardware documentation I assume. Assuming the system implements detection then it should. If a PATA controller doesn't implement any detection (cable or host side) and it does UDMA5 then I believe the correct description for it is usually "broken" as people will have horrible problems trying to use things like flash drives with it.
Alan

Alan Cox

2008-04-28 08:48 UTC #10 lore raw

Re: [PATCH] ata: Add Intel SCH PATA support (revised)

Quoted 3 lines · On Mon, 28 Apr 2008 12:05:17 +0800 Alek Du <[email protected]> wrote:

Hi Alan, Jeff
I re-write the patch according to the kind comments from Alan, I also did the test on my SCH machine. Please review it again. Thanks a lot.

Much nicer. We have a standard library helper for cable types you can use

ata_cable_unknown()

does the same as your private routine.

Alan

Alek Du

author

2008-04-28 09:14 UTC #11 lore raw

Re: [PATCH] ata: Add Intel SCH PATA support (revised)

Alan,

Fixed and tested, please review it:

This patch adds Intel SCH chipsets (US15W, US15L, UL11L) PATA controller support.

Signed-off-by Alek Du <[email protected]>

---
 drivers/ata/ata_piix.c  |   19 +++++++++++++++++++
 drivers/ide/pci/piix.c  |    2 ++
 include/linux/pci_ids.h |    2 ++
 3 files changed, 23 insertions(+), 0 deletions(-)

drivers/ata/ata_piix.c +19 -0

index ea2c764..4ec4178 100644
@@ -136,6 +136,7 @@ enum piix_controller_ids {
 	ich_pata_33,		/* ICH up to UDMA 33 only */
 	ich_pata_66,		/* ICH up to 66 Mhz */
 	ich_pata_100,		/* ICH up to UDMA 100 */
+	sch_pata_100,		/* SCH up to UDMA 100 */
 	ich5_sata,
 	ich6_sata,
 	ich6m_sata,

@@ -216,6 +217,8 @@ static const struct pci_device_id piix_pci_tbl[] = {
 	{ 0x8086, 0x269E, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
 	/* ICH8 Mobile PATA Controller */
 	{ 0x8086, 0x2850, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
+	/* Intel SCH PATA Controller */
+	{ 0x8086, 0x811A, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sch_pata_100 },
 
 	/* NOTE: The following PCI ids must be kept in sync with the
 	 * list in drivers/pci/quirks.c.

@@ -311,6 +314,13 @@ static struct ata_port_operations ich_pata_ops = {
 	.set_dmamode		= ich_set_dmamode,
 };
 
+static struct ata_port_operations sch_pata_ops = {
+	.inherits		= &piix_pata_ops,
+	.cable_detect		= ata_cable_unknown,
+	.prereset		= ata_sff_prereset,
+	.set_dmamode		= ich_set_dmamode,
+};
+
 static struct ata_port_operations piix_sata_ops = {
 	.inherits		= &ata_bmdma_port_ops,
 };

@@ -470,6 +480,15 @@ static struct ata_port_info piix_port_info[] = {
 		.port_ops	= &ich_pata_ops,
 	},
 
+	[sch_pata_100] =
+	{
+		.flags		= PIIX_PATA_FLAGS | PIIX_FLAG_CHECKINTR,
+		.pio_mask	= 0x1f,	/* pio0-4 */
+		.mwdma_mask	= 0x06, /* mwdma1-2 */
+		.udma_mask	= ATA_UDMA5, /* udma0-5 */
+		.port_ops	= &sch_pata_ops,
+	},
+
 	[ich5_sata] =
 	{
 		.flags		= PIIX_SATA_FLAGS,

drivers/ide/pci/piix.c +2 -0

index 21c5dd2..31607fe 100644
@@ -380,6 +380,7 @@ static const struct ide_port_info piix_pci_info[] __devinitdata = {
 	/* 22 */ DECLARE_ICH_DEV("ICH4",	ATA_UDMA5),
 	/* 23 */ DECLARE_ICH_DEV("ESB2",	ATA_UDMA5),
 	/* 24 */ DECLARE_ICH_DEV("ICH8M",	ATA_UDMA5),
+	/* 25 */ DECLARE_ICH_DEV("SCH",		ATA_UDMA5),
 };
 
 /**

@@ -453,6 +454,7 @@ static const struct pci_device_id piix_pci_tbl[] = {
 	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_82801DB_1),  22 },
 	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ESB2_18),    23 },
 	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ICH8_6),     24 },
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SCH_IDE),    25 },
 	{ 0, },
 };
 MODULE_DEVICE_TABLE(pci, piix_pci_tbl);

include/linux/pci_ids.h +2 -0

index 70eb3c8..e5a53da 100644
@@ -2413,6 +2413,8 @@
 #define PCI_DEVICE_ID_INTEL_82443GX_0	0x71a0
 #define PCI_DEVICE_ID_INTEL_82443GX_2	0x71a2
 #define PCI_DEVICE_ID_INTEL_82372FB_1	0x7601
+#define PCI_DEVICE_ID_INTEL_SCH_LPC	0x8119
+#define PCI_DEVICE_ID_INTEL_SCH_IDE	0x811a
 #define PCI_DEVICE_ID_INTEL_82454GX	0x84c4
 #define PCI_DEVICE_ID_INTEL_82450GX	0x84c5
 #define PCI_DEVICE_ID_INTEL_82451NX	0x84ca

Signature · 7 lines

1.5.2.5


Thanks,
Alek

On Mon, 28 Apr 2008 16:41:23 +0800

Quoted 18 lines · Alan Cox

Re: [PATCH] ata: Add Intel SCH PATA support (revised)
Quoted 3 lines · On Mon, 28 Apr 2008 12:05:17 +0800 Alek Du <[email protected]> wrote:
Hi Alan, Jeff
I re-write the patch according to the kind comments from Alan, I also did the test on my SCH machine. Please review it again. Thanks a lot.
Much nicer. We have a standard library helper for cable types you can use
ata_cable_unknown()
does the same as your private routine.
Alan

Alan Cox

Acked-by

2008-04-28 09:28 UTC #12 lore raw

Re: [PATCH] ata: Add Intel SCH PATA support (revised)

Quoted 8 lines · On Mon, 28 Apr 2008 17:07:51 +0800 Alek Du <[email protected]> wrote:

Alan,
Fixed and tested, please review it:
This patch adds Intel SCH chipsets (US15W, US15L, UL11L) PATA controller support.
Signed-off-by Alek Du <[email protected]>

Acked-by Alan Cox <[email protected]>

Bartlomiej Zolnierkiewicz

2008-04-28 18:45 UTC #13 lore raw

Re: [PATCH] ata: Add Intel SCH PATA support (revised)

In reply to Alek Du

Hi,

Quoted 70 lines · Alek Du

Alan,

Fixed and tested, please review it:

This patch adds Intel SCH chipsets (US15W, US15L, UL11L) PATA controller support.

Signed-off-by Alek Du <[email protected]>

---
 drivers/ata/ata_piix.c  |   19 +++++++++++++++++++
 drivers/ide/pci/piix.c  |    2 ++
 include/linux/pci_ids.h |    2 ++
 3 files changed, 23 insertions(+), 0 deletions(-)

drivers/ata/ata_piix.c +19 -0

index ea2c764..4ec4178 100644
@@ -136,6 +136,7 @@ enum piix_controller_ids {
 	ich_pata_33,		/* ICH up to UDMA 33 only */
 	ich_pata_66,		/* ICH up to 66 Mhz */
 	ich_pata_100,		/* ICH up to UDMA 100 */
+	sch_pata_100,		/* SCH up to UDMA 100 */
 	ich5_sata,
 	ich6_sata,
 	ich6m_sata,

@@ -216,6 +217,8 @@ static const struct pci_device_id piix_pci_tbl[] = {
 	{ 0x8086, 0x269E, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
 	/* ICH8 Mobile PATA Controller */
 	{ 0x8086, 0x2850, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
+	/* Intel SCH PATA Controller */
+	{ 0x8086, 0x811A, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sch_pata_100 },
 
 	/* NOTE: The following PCI ids must be kept in sync with the
 	 * list in drivers/pci/quirks.c.

@@ -311,6 +314,13 @@ static struct ata_port_operations ich_pata_ops = {
 	.set_dmamode		= ich_set_dmamode,
 };
 
+static struct ata_port_operations sch_pata_ops = {
+	.inherits		= &piix_pata_ops,
+	.cable_detect		= ata_cable_unknown,
+	.prereset		= ata_sff_prereset,
+	.set_dmamode		= ich_set_dmamode,
+};
+
 static struct ata_port_operations piix_sata_ops = {
 	.inherits		= &ata_bmdma_port_ops,
 };

@@ -470,6 +480,15 @@ static struct ata_port_info piix_port_info[] = {
 		.port_ops	= &ich_pata_ops,
 	},
 
+	[sch_pata_100] =
+	{
+		.flags		= PIIX_PATA_FLAGS | PIIX_FLAG_CHECKINTR,
+		.pio_mask	= 0x1f,	/* pio0-4 */
+		.mwdma_mask	= 0x06, /* mwdma1-2 */
+		.udma_mask	= ATA_UDMA5, /* udma0-5 */
+		.port_ops	= &sch_pata_ops,
+	},
+
 	[ich5_sata] =
 	{
 		.flags		= PIIX_SATA_FLAGS,

drivers/ide/pci/piix.c +0 -0

index 21c5dd2..31607fe 100644

drivers/ide/pci/piix.c static const struct ide_port_info piix_pci_info[] __devinitdata = { · line 383

Quoted 4 lines

drivers/ide/pci/piix.c +1 -0

@@ -380,6 +380,7 @@ static const struct ide_port_info piix_pci_info[] __devinitdata = {
 	/* 22 */ DECLARE_ICH_DEV("ICH4",	ATA_UDMA5),
 	/* 23 */ DECLARE_ICH_DEV("ESB2",	ATA_UDMA5),
 	/* 24 */ DECLARE_ICH_DEV("ICH8M",	ATA_UDMA5),
+	/* 25 */ DECLARE_ICH_DEV("SCH",		ATA_UDMA5),

DECLARE_ICH_DEV() cannot be used here - seems like DEFINE_SCH_DEV() needs to be defined (which will lack .enablebits field but otherwise be identical to DECLARE_ICH_DEV)

also seems that piix_cable_detect() should be updated to check for dev->device == PCI_DEVICE_ID_INTEL_SCH_IDE and return ATA_CBL_PATA80 (it is OK to use it instead of ATA_CBL_UNK in drivers/ide/ - generic cable detection code won't override drive side cable detection).

otherwise everything looks good

Thanks,
Bart

Alek Du

author

2008-04-29 00:21 UTC #14 lore raw

Re: [PATCH] ata: Add Intel SCH PATA support (revised)

Hi Bartlomiej,

Quoted 75 lines · On Tue, 29 Apr 2008 03:01:32 +0800 "Bartlomiej Zolnierkiewicz"…

Re: [PATCH] ata: Add Intel SCH PATA support (revised)

Hi,

Quoted 70 lines · Alek Du

Alan,

Fixed and tested, please review it:

This patch adds Intel SCH chipsets (US15W, US15L, UL11L) PATA controller support.

Signed-off-by Alek Du <[email protected]>

---
 drivers/ata/ata_piix.c  |   19 +++++++++++++++++++
 drivers/ide/pci/piix.c  |    2 ++
 include/linux/pci_ids.h |    2 ++
 3 files changed, 23 insertions(+), 0 deletions(-)

drivers/ata/ata_piix.c +19 -0

index ea2c764..4ec4178 100644
@@ -136,6 +136,7 @@ enum piix_controller_ids {
      ich_pata_33,            /* ICH up to UDMA 33 only */
      ich_pata_66,            /* ICH up to 66 Mhz */
      ich_pata_100,           /* ICH up to UDMA 100 */
+     sch_pata_100,           /* SCH up to UDMA 100 */
      ich5_sata,
      ich6_sata,
      ich6m_sata,

@@ -216,6 +217,8 @@ static const struct pci_device_id piix_pci_tbl[] = {
      { 0x8086, 0x269E, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
      /* ICH8 Mobile PATA Controller */
      { 0x8086, 0x2850, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
+     /* Intel SCH PATA Controller */
+     { 0x8086, 0x811A, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sch_pata_100 },
 
      /* NOTE: The following PCI ids must be kept in sync with the
       * list in drivers/pci/quirks.c.

@@ -311,6 +314,13 @@ static struct ata_port_operations ich_pata_ops = {
      .set_dmamode            = ich_set_dmamode,
 };
 
+static struct ata_port_operations sch_pata_ops = {
+     .inherits               = &piix_pata_ops,
+     .cable_detect           = ata_cable_unknown,
+     .prereset               = ata_sff_prereset,
+     .set_dmamode            = ich_set_dmamode,
+};
+
 static struct ata_port_operations piix_sata_ops = {
      .inherits               = &ata_bmdma_port_ops,
 };

@@ -470,6 +480,15 @@ static struct ata_port_info piix_port_info[] = {
              .port_ops       = &ich_pata_ops,
      },
 
+     [sch_pata_100] =
+     {
+             .flags          = PIIX_PATA_FLAGS | PIIX_FLAG_CHECKINTR,
+             .pio_mask       = 0x1f, /* pio0-4 */
+             .mwdma_mask     = 0x06, /* mwdma1-2 */
+             .udma_mask      = ATA_UDMA5, /* udma0-5 */
+             .port_ops       = &sch_pata_ops,
+     },
+
      [ich5_sata] =
      {
              .flags          = PIIX_SATA_FLAGS,

drivers/ide/pci/piix.c +0 -0

index 21c5dd2..31607fe 100644
@@ -380,6 +380,7 @@ static const struct ide_port_info piix_pci_info[] __devinitdata = {

Quoted 9 lines

drivers/ide/pci/piix.c static const struct ide_port_info piix_pci_info[] __devinitdata = { · line 383
Quoted 4 lines
drivers/ide/pci/piix.c +1 -0
      /* 22 */ DECLARE_ICH_DEV("ICH4",        ATA_UDMA5),
      /* 23 */ DECLARE_ICH_DEV("ESB2",        ATA_UDMA5),
      /* 24 */ DECLARE_ICH_DEV("ICH8M",       ATA_UDMA5),
+     /* 25 */ DECLARE_ICH_DEV("SCH",         ATA_UDMA5),
DECLARE_ICH_DEV() cannot be used here - seems like DEFINE_SCH_DEV() needs to be defined (which will lack .enablebits field but otherwise be identical to DECLARE_ICH_DEV)

I do not understand here, where DECLARE_ICH_DEV() used here? I just defined a new sub_type sch_pata_100, and inherit most ops from piix instead of cable_detect and prereset.

Quoted 5 lines

also seems that piix_cable_detect() should be updated to check for dev->device == PCI_DEVICE_ID_INTEL_SCH_IDE and return ATA_CBL_PATA80 (it is OK to use it instead of ATA_CBL_UNK in drivers/ide/ - generic cable detection code won't override drive side cable detection).

My first version of patch is to modify piix_cable_detect like your way. But Alan suggested use this way -- it is much cleaner. And according to my test, return ATA_CBL_UNK do no harm. The driver is set to UDMA5 when 80 cable is used.

Quoted 4 lines

otherwise everything looks good
Thanks,
Bart

Thanks,
Alek

Alek Du

author

2008-04-29 03:48 UTC #15 lore raw

Re: [PATCH] ata: Add Intel SCH PATA support (revised)

Quoted 6 lines · On Tue, 29 Apr 2008 08:14:11 +0800 Alek Du <[email protected]> wrote:

Quoted 3 lines
needs to be defined (which will lack .enablebits field but otherwise be identical to DECLARE_ICH_DEV)
I do not understand here, where DECLARE_ICH_DEV() used here? I just defined a new sub_type sch_pata_100, and inherit most ops from piix instead of cable_detect and prereset.

I see now. I posted a new patch which defined DECLARE_SCH_DEV. Thanks a lot.

Quoted 16 lines

Quoted 5 lines
also seems that piix_cable_detect() should be updated to check for dev->device == PCI_DEVICE_ID_INTEL_SCH_IDE and return ATA_CBL_PATA80 (it is OK to use it instead of ATA_CBL_UNK in drivers/ide/ - generic cable detection code won't override drive side cable detection).
My first version of patch is to modify piix_cable_detect like your way. But Alan suggested use this way -- it is much cleaner. And according to my test, return ATA_CBL_UNK do no harm. The driver is set to UDMA5 when 80 cable is used.
Quoted 4 lines
otherwise everything looks good
Thanks,
Bart
Thanks,
Alek

Alek Du

author

2008-04-29 03:45 UTC #16 lore raw

Re: [PATCH] ata: Add Intel SCH PATA support (revised)

In reply to Bartlomiej Zolnierkiewicz

Hi Alan and Bartlomiej,

Modified the patch again. Add and use DECLARE_SCH_DEV instead of DECLARE_ICH_DEV to avoid enable_bits. But I still use ATA_CBL_UNK, because according to my test, it works fine -- it could set the disk to UDMA5 mode when using 80 wire cable. Please review it again, thanks.

This patch adds Intel SCH chipsets (US15W, US15L, UL11L) PATA controller support.

Signed-off-by Alek Du <[email protected]>

---
 drivers/ata/ata_piix.c  |   19 +++++++++++++++++++
 drivers/ide/pci/piix.c  |   15 +++++++++++++++
 include/linux/pci_ids.h |    2 ++
 3 files changed, 36 insertions(+), 0 deletions(-)

drivers/ata/ata_piix.c +19 -0

index ea2c764..4ec4178 100644
@@ -136,6 +136,7 @@ enum piix_controller_ids {
 	ich_pata_33,		/* ICH up to UDMA 33 only */
 	ich_pata_66,		/* ICH up to 66 Mhz */
 	ich_pata_100,		/* ICH up to UDMA 100 */
+	sch_pata_100,		/* SCH up to UDMA 100 */
 	ich5_sata,
 	ich6_sata,
 	ich6m_sata,

@@ -216,6 +217,8 @@ static const struct pci_device_id piix_pci_tbl[] = {
 	{ 0x8086, 0x269E, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
 	/* ICH8 Mobile PATA Controller */
 	{ 0x8086, 0x2850, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
+	/* Intel SCH PATA Controller */
+	{ 0x8086, 0x811A, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sch_pata_100 },
 
 	/* NOTE: The following PCI ids must be kept in sync with the
 	 * list in drivers/pci/quirks.c.

@@ -311,6 +314,13 @@ static struct ata_port_operations ich_pata_ops = {
 	.set_dmamode		= ich_set_dmamode,
 };
 
+static struct ata_port_operations sch_pata_ops = {
+	.inherits		= &piix_pata_ops,
+	.cable_detect		= ata_cable_unknown,
+	.prereset		= ata_sff_prereset,
+	.set_dmamode		= ich_set_dmamode,
+};
+
 static struct ata_port_operations piix_sata_ops = {
 	.inherits		= &ata_bmdma_port_ops,
 };

@@ -470,6 +480,15 @@ static struct ata_port_info piix_port_info[] = {
 		.port_ops	= &ich_pata_ops,
 	},
 
+	[sch_pata_100] =
+	{
+		.flags		= PIIX_PATA_FLAGS | PIIX_FLAG_CHECKINTR,
+		.pio_mask	= 0x1f,	/* pio0-4 */
+		.mwdma_mask	= 0x06, /* mwdma1-2 */
+		.udma_mask	= ATA_UDMA5, /* udma0-5 */
+		.port_ops	= &sch_pata_ops,
+	},
+
 	[ich5_sata] =
 	{
 		.flags		= PIIX_SATA_FLAGS,

drivers/ide/pci/piix.c +15 -0

index 21c5dd2..d224a67 100644
@@ -340,6 +340,19 @@ static const struct ide_port_ops piix_port_ops = {
 		.udma_mask	= udma, \
 	}
 
+#define DECLARE_SCH_DEV(name_str, udma) \
+	{ \
+		.name		= name_str, \
+		.init_chipset	= init_chipset_ich, \
+		.init_hwif	= init_hwif_ich, \
+		.port_ops	= &piix_port_ops, \
+		.host_flags	= IDE_HFLAGS_PIIX, \
+		.pio_mask	= ATA_PIO4, \
+		.swdma_mask	= ATA_SWDMA2_ONLY, \
+		.mwdma_mask	= ATA_MWDMA12_ONLY, \
+		.udma_mask	= udma, \
+	}
+
 static const struct ide_port_info piix_pci_info[] __devinitdata = {
 	/*  0 */ DECLARE_PIIX_DEV("PIIXa",	0x00),	/* no udma */
 	/*  1 */ DECLARE_PIIX_DEV("PIIXb",	0x00),	/* no udma */

@@ -380,6 +393,7 @@ static const struct ide_port_info piix_pci_info[] __devinitdata = {
 	/* 22 */ DECLARE_ICH_DEV("ICH4",	ATA_UDMA5),
 	/* 23 */ DECLARE_ICH_DEV("ESB2",	ATA_UDMA5),
 	/* 24 */ DECLARE_ICH_DEV("ICH8M",	ATA_UDMA5),
+	/* 25 */ DECLARE_SCH_DEV("SCH",		ATA_UDMA5),
 };
 
 /**

@@ -453,6 +467,7 @@ static const struct pci_device_id piix_pci_tbl[] = {
 	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_82801DB_1),  22 },
 	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ESB2_18),    23 },
 	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_ICH8_6),     24 },
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_SCH_IDE),    25 },
 	{ 0, },
 };
 MODULE_DEVICE_TABLE(pci, piix_pci_tbl);

include/linux/pci_ids.h +2 -0

index 70eb3c8..e5a53da 100644
@@ -2413,6 +2413,8 @@
 #define PCI_DEVICE_ID_INTEL_82443GX_0	0x71a0
 #define PCI_DEVICE_ID_INTEL_82443GX_2	0x71a2
 #define PCI_DEVICE_ID_INTEL_82372FB_1	0x7601
+#define PCI_DEVICE_ID_INTEL_SCH_LPC	0x8119
+#define PCI_DEVICE_ID_INTEL_SCH_IDE	0x811a
 #define PCI_DEVICE_ID_INTEL_82454GX	0x84c4
 #define PCI_DEVICE_ID_INTEL_82450GX	0x84c5
 #define PCI_DEVICE_ID_INTEL_82451NX	0x84ca

Signature · 6 lines

1.5.2.5




On Tue, 29 Apr 2008 03:01:32 +0800

Quoted 23 lines · Bartlomiej Zolnierkiewicz

drivers/ide/pci/piix.c static const struct ide_port_info piix_pci_info[] __devinitdata = { · line 383
Quoted 9 lines
drivers/ide/pci/piix.c +1 -0
index 21c5dd2..31607fe 100644
@@ -380,6 +380,7 @@ static const struct ide_port_info piix_pci_info[] __devinitdata = {
      /* 22 */ DECLARE_ICH_DEV("ICH4",        ATA_UDMA5),
      /* 23 */ DECLARE_ICH_DEV("ESB2",        ATA_UDMA5),
      /* 24 */ DECLARE_ICH_DEV("ICH8M",       ATA_UDMA5),
+     /* 25 */ DECLARE_ICH_DEV("SCH",         ATA_UDMA5),
DECLARE_ICH_DEV() cannot be used here - seems like DEFINE_SCH_DEV() needs to be defined (which will lack .enablebits field but otherwise be identical to DECLARE_ICH_DEV)
also seems that piix_cable_detect() should be updated to check for dev->device == PCI_DEVICE_ID_INTEL_SCH_IDE and return ATA_CBL_PATA80 (it is OK to use it instead of ATA_CBL_UNK in drivers/ide/ - generic cable detection code won't override drive side cable detection).
otherwise everything looks good
Thanks,
Bart

Sergei Shtylyov

2008-04-29 16:38 UTC #17 lore raw

Re: [PATCH] ata: Add Intel SCH PATA support (revised)

Hello.

Quoted 9 lines · Alek Du

Modified the patch again. Add and use DECLARE_SCH_DEV instead of DECLARE_ICH_DEV to avoid enable_bits. But I still use ATA_CBL_UNK, because according to my test, it works fine -- it could set the disk to UDMA5 mode when using 80 wire cable. Please review it again, thanks.
This patch adds Intel SCH chipsets (US15W, US15L, UL11L) PATA controller support.
Signed-off-by Alek Du <[email protected]>

NAK this patch completely -- both libata and IDE parts.
Luckily, I have just had a quick look at the datasheet to which you've
linked and that was enough to conclude that SCH is *not* compatible to ICH/PIIX family, so absolutely needs a separate driver!

Quoted 22 lines

drivers/ata/ata_piix.c +3 -0

index ea2c764..4ec4178 100644
@@ -136,6 +136,7 @@ enum piix_controller_ids {
 	ich_pata_33,		/* ICH up to UDMA 33 only */
 	ich_pata_66,		/* ICH up to 66 Mhz */
 	ich_pata_100,		/* ICH up to UDMA 100 */
+	sch_pata_100,		/* SCH up to UDMA 100 */
 	ich5_sata,
 	ich6_sata,
 	ich6m_sata,

@@ -216,6 +217,8 @@ static const struct pci_device_id piix_pci_tbl[] = {
 	{ 0x8086, 0x269E, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
 	/* ICH8 Mobile PATA Controller */
 	{ 0x8086, 0x2850, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
+	/* Intel SCH PATA Controller */
+	{ 0x8086, 0x811A, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sch_pata_100 },
 
 	/* NOTE: The following PCI ids must be kept in sync with the
 	 * list in drivers/pci/quirks.c.

drivers/ata/ata_piix.c static struct ata_port_operations ich_pata_ops = { · line 321

Quoted 8 lines

drivers/ata/ata_piix.c +5 -0

@@ -311,6 +314,13 @@ static struct ata_port_operations ich_pata_ops = {
 	.set_dmamode		= ich_set_dmamode,
 };
 
+static struct ata_port_operations sch_pata_ops = {
+	.inherits		= &piix_pata_ops,
+	.cable_detect		= ata_cable_unknown,
+	.prereset		= ata_sff_prereset,
+	.set_dmamode		= ich_set_dmamode,

The ich_set_dmamode() won't work with SCH.

Quoted 6 lines

drivers/ata/ata_piix.c +2 -0

+};
+
 static struct ata_port_operations piix_sata_ops = {
 	.inherits		= &ata_bmdma_port_ops,
 };

drivers/ata/ata_piix.c static struct ata_port_info piix_port_info[] = { · line 487

Quoted 8 lines

drivers/ata/ata_piix.c +5 -0

@@ -470,6 +480,15 @@ static struct ata_port_info piix_port_info[] = {
 		.port_ops	= &ich_pata_ops,
 	},
 
+	[sch_pata_100] =
+	{
+		.flags		= PIIX_PATA_FLAGS | PIIX_FLAG_CHECKINTR,
+		.pio_mask	= 0x1f,	/* pio0-4 */
+		.mwdma_mask	= 0x06, /* mwdma1-2 */

Wrong, SCH does support MWDMA0, so the mask would be 0x07...

drivers/ide/pci/piix.c static const struct ide_port_ops piix_port_ops = { · line 343

Quoted 9 lines

drivers/ide/pci/piix.c +1 -0

index 21c5dd2..d224a67 100644
@@ -340,6 +340,19 @@ static const struct ide_port_ops piix_port_ops = {
 		.udma_mask	= udma, \
 	}
 
+#define DECLARE_SCH_DEV(name_str, udma) \

Well, a single instance hardly deserved its own macro, could just put the
whole initializer in its array slot like for MPIIX (which probably shouldn't be in this driver anyway though :-)...

drivers/ide/pci/piix.c static const struct ide_port_ops piix_port_ops = { · line 346

Quoted 3 lines

drivers/ide/pci/piix.c +3 -0

+	{ \
+		.name		= name_str, \
+		.init_chipset	= init_chipset_ich, \

This method read/writes IOCFG register which as you've yourself noted is
missing on SCH (among with any ICH compatible registers ;-).

drivers/ide/pci/piix.c static const struct ide_port_ops piix_port_ops = { · line 351

Quoted 5 lines

drivers/ide/pci/piix.c +5 -0

+		.init_hwif	= init_hwif_ich, \
+		.port_ops	= &piix_port_ops, \
+		.host_flags	= IDE_HFLAGS_PIIX, \
+		.pio_mask	= ATA_PIO4, \
+		.swdma_mask	= ATA_SWDMA2_ONLY, \

Wrong, no SWDMA support in SCH.

drivers/ide/pci/piix.c static const struct ide_port_ops piix_port_ops = { · line 352

Quoted 1 line

drivers/ide/pci/piix.c +1 -0
+		.mwdma_mask	= ATA_MWDMA12_ONLY, \

Shoud have been ATA_MWDMA2.

drivers/ide/pci/piix.c static const struct ide_port_ops piix_port_ops = { · line 353

Quoted 1 line

drivers/ide/pci/piix.c +1 -0
+		.udma_mask	= udma, \

This is fixed to AATA_UDMA5 so there was no need to parametrize...

MBR, Sergei

Alek Du

author

2008-04-30 02:56 UTC #18 lore raw

Re: [PATCH] ata: Add Intel SCH PATA support (revised)

Hi Sergei,

Quoted 7 lines · On Wed, 30 Apr 2008 00:38:17 +0800 "Sergei Shtylyov" <[email protected]>…

NAK this patch completely -- both libata and IDE parts. Luckily, I have just had a quick look at the datasheet to which you've
linked and that was enough to conclude that SCH is *not* compatible to ICH/PIIX family, so absolutely needs a separate driver!

Most registers are same except for port enable bit, IOCFG and DMA timing. Now I know why my patch just works -- the BIOS set the timing correctly. Thanks for point this out. I will post a separate driver patch later.

Thanks,
Alek

Sergei Shtylyov

2008-04-30 11:17 UTC #19 lore raw

Re: [PATCH] ata: Add Intel SCH PATA support (revised)

Hello.

Quoted 1 line · Alek Du

Quoted 1 line
NAK this patch completely -- both libata and IDE parts.

In the future it would be worth to post 2 separate patches, one for IDE and
one for libata -- since they should go thru the different maintainer trees.

Quoted 5 lines

Quoted 3 lines
Luckily, I have just had a quick look at the datasheet to which you've
linked and that was enough to conclude that SCH is *not* compatible to ICH/PIIX family, so absolutely needs a separate driver!
Most registers are same

Well, if you mean the standard BMDMA and standard PCI config. space
registers (0x00 thru 0x3f), then those are mostly the same for the majority of the PCI IDE controllers. Why not stick your code into one of the other driver instead? ;-)

Quoted 1 line

except for port enable bit, IOCFG and DMA timing. Now

And also PIO and UDMA timings/enable. Actually, all the vendor-specific
PCI config. registers are different from PIIX/ICH -- and that's what makes the PATA drivers different from each other.

Quoted 1 line

I know why my patch just works -- the BIOS set the timing correctly.

Yeah, and we have the special drivers which rely on the BIOS doing all the
mode programming...

Quoted 1 line

Thanks for point this out. I will post a separate driver patch later.

In the meantwhile the only viable solution would be to stick your code
into drivers/ide/pci/generic.c and drivers/ata/ata_generic.c. No more hacks, please. :-)

Quoted 2 lines

Thanks,
Alek

MBR, Sergei

Sergei Shtylyov

2008-04-26 16:46 UTC #20 lore raw

In reply to Alek Du

Hello.

Quoted 1 line · Alek Du

Seems I need to post the patch here instead of LKML

Your patch is whitespace damaged -- all tabs converted to spaces.

Quoted 21 lines

[PATCH] ata: Add Intel SCH PATA support

This patch adds Intel SCH chipsets (US15W, US15L, UL11L) PATA controller support.

Signed-off-by Alek Du <[email protected]>

drivers/ata/ata_piix.c +2 -0

index b7c38ee..7f95a9a 100644
@@ -214,6 +214,8 @@ static const struct pci_device_id piix_pci_tbl[] = {
        /* ICH7/7-R (i945, i975) UDMA 100*/
        { 0x8086, 0x27DF, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
        { 0x8086, 0x269E, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
+       /* INTEL SCH UDMA 100 */
+       { 0x8086, 0x811A, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
        /* ICH8 Mobile PATA Controller */
        { 0x8086, 0x2850, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },

drivers/ata/ata_piix.c struct ich_laptop { · line 569

Quoted 7 lines

drivers/ata/ata_piix.c +4 -0

@@ -561,6 +563,10 @@ struct ich_laptop {
        u16 subdevice;
 };
 
+struct sch_80 {
+       u16 device;
+};
+

Why create a structure which has only single field?

MBR, Sergei

Alek Du

author

2008-04-26 23:39 UTC #21 lore raw

Sergei,

Quoted 10 lines · On Sun, 27 Apr 2008 00:46:14 +0800 "Sergei Shtylyov" <[email protected]>…

Re: [PATCH] ata: Add Intel SCH PATA support
Hello.
Quoted 1 line · Alek Du
Seems I need to post the patch here instead of LKML
Your patch is whitespace damaged -- all tabs converted to spaces.

I forward the patch from LKML and seems my clawsmail client eat all tabs ....

Quoted 24 lines

[PATCH] ata: Add Intel SCH PATA support

This patch adds Intel SCH chipsets (US15W, US15L, UL11L) PATA controller support.

Signed-off-by Alek Du <[email protected]>

drivers/ata/ata_piix.c +2 -0

index b7c38ee..7f95a9a 100644
@@ -214,6 +214,8 @@ static const struct pci_device_id piix_pci_tbl[] = {
        /* ICH7/7-R (i945, i975) UDMA 100*/
        { 0x8086, 0x27DF, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
        { 0x8086, 0x269E, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
+       /* INTEL SCH UDMA 100 */
+       { 0x8086, 0x811A, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },
        /* ICH8 Mobile PATA Controller */
        { 0x8086, 0x2850, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich_pata_100 },

@@ -561,6 +563,10 @@ struct ich_laptop {
        u16 subdevice;
 };

Quoted 7 lines

drivers/ata/ata_piix.c struct ich_laptop { · line 569
Quoted 4 lines
drivers/ata/ata_piix.c +4 -0
+struct sch_80 {
+       u16 device;
+};
+
Why create a structure which has only single field?

Currently, it only use device ID to decide if force to go SCH way (force to PATA_80 and skip port enabling bit check), but it would be easier for expanding if I define a structure here. (looks like ich_laptop)

Quoted 1 line

MBR, Sergei

Alan Cox

2008-04-26 23:57 UTC #22 lore raw

Quoted 3 lines

Currently, it only use device ID to decide if force to go SCH way (force to PATA_80 and skip port enabling bit check), but it would be easier for expanding if I define a structure here. (looks like ich_laptop)

Just define an sch_pata type for those controller types and fix up the relevant methods accordingly. Much cleaner