Re: pata_cs5536: ATA driver for Geode companion chip

Re: pata_cs5536: ATA driver for Geode companion chip

[Jordan Crouse]

Hi Martin - make sure you CC me on responses, I'm not on linux-ide.

> I'll try this later and see what comes up.  But I'm not sure I see the
> advantage of having two ways to configure the 5536.

There are multiple ways to do it, simply because of the way the architecture
is designed underneath.  Thats why we enforce the policy
of using the PCI mechanism when ever we can to encourage everybody on the
same page.

> Anyway.  That's my personal take on it.  If there's a general
> consensus that the timings should be in pata_amd.c then so be it.

No - I think if we're going to have the different timings, they should
be in their own driver, not bothering the other users of this IP.

> The board I'm using has a BIOS which exposes the IDE registers in PCI
> space but doesn't appear to listen when they are written to.  I'm
> hoping that will get fixed eventually.

Thats a bug, pure and simple.  If the PCI space is there, but the IDE
registers are not - then you are deep into breakage mode, and you need
to yell very loud and very long at your BIOS vendor. 

In the standard AMD VSA, writeos to PCI space 0x48 (DTC), 0x49 (CAST) and 
0x50 (ETC) should work - and the change should be immediately reflected
in the MSR.   You can see for yourself, the code is open:

http://dev.laptop.org/git?p=geode-vsa;a=blob;f=sysmgr/pci_wr.c;h=9a984998ba3b8ed06358a85faf16a6ddebdad4a9;hb=HEAD#l359

Not all BIOS vendors follow the AMD model, but most do.

> PS. So what's the story wrt. a PRD length of 0?

Alan is right, the NSC/Cyrix block had issues.  Thats why when we started
on the 5536, we searched around in AMD's attic, and found
the 8111 IDE block under an pile of old clothes and Life magazines.
We dusted it off and stuck it in.  So this fundamentally different
silicon then before.

I believe that the datasheet is correct - at least, there aren't any
errata, and we've been pretending to be a AMD74xx for quite a while now
with no ill effects in this regard.

Jordan
-- 
Jordan Crouse
Systems Software Development Engineer 
Advanced Micro Devices, Inc.

Re: pata_cs5536: ATA driver for Geode companion chip

[Martin K. Petersen]

>>>>> "Jordan" == Jordan Crouse <jordan.crouse@amd.com> writes:

Jordan> In the standard AMD VSA, writeos to PCI space 0x48 (DTC), 0x49
Jordan> (CAST) and 0x50 (ETC) should work - and the change should be
Jordan> immediately reflected in the MSR.  

I know what the problem is.  I just got a BIOS update tonight which
was supposed to fix the issue.  However, pata_amd.c still failed
spectacularly at configuring the device whereas pata_cs5536 worked
fine.  

For kicks I switched my driver over to using PCI config registers
instead of MSRs.  And it still worked!

When I changed my driver I did a quickie conversion from wrmsr() to
pci_write_config_dword().  If I use pci_write_config_byte() instead
(like pata_amd.c), things fail miserably.  A register dump reveals
that any value I write becomes a 0.  Yay!

-- 
Martin K. Petersen      http://mkp.net/

Re: pata_cs5536: ATA driver for Geode companion chip

[Alan Cox]

> When I changed my driver I did a quickie conversion from wrmsr() to
> pci_write_config_dword().  If I use pci_write_config_byte() instead
> (like pata_amd.c), things fail miserably.  A register dump reveals
> that any value I write becomes a 0.  Yay!

Fantastic - that means we have a way to tell if you have broken firmware
and which driver to pick...

Re: pata_cs5536: ATA driver for Geode companion chip

[Alan Cox]

> For kicks I switched my driver over to using PCI config registers
> instead of MSRs.  And it still worked!
> 
> When I changed my driver I did a quickie conversion from wrmsr() to
> pci_write_config_dword().  If I use pci_write_config_byte() instead
> (like pata_amd.c), things fail miserably.  A register dump reveals
> that any value I write becomes a 0.  Yay!

At that point its probaly a good idea to take CS5536 out of AMD and use
your driver for it, whether in PCI or MSR mode (or perhaps supporting
either)

Alan

Re: pata_cs5536: ATA driver for Geode companion chip

[Jordan Crouse]

On 04/10/07 16:58 +0100, Alan Cox wrote:
> > For kicks I switched my driver over to using PCI config registers
> > instead of MSRs.  And it still worked!
> > 
> > When I changed my driver I did a quickie conversion from wrmsr() to
> > pci_write_config_dword().  If I use pci_write_config_byte() instead
> > (like pata_amd.c), things fail miserably.  A register dump reveals
> > that any value I write becomes a 0.  Yay!
> 
> At that point its probaly a good idea to take CS5536 out of AMD and use
> your driver for it, whether in PCI or MSR mode (or perhaps supporting
> either)

Agreed.  It sounds like there is still breakage in the BIOS - since the
reference VSA code accounts for accesses of different alignments, but its
probably not worth going back and forth about it.  Like I said, if Alan
and Jeff are game, we should go with the 5536 driver, and now that we've
debated it extensively, I'm fine with the MSR accesses - I'm satisfied
that we at least considered the alternative and dismissed it for technically
sound reasons.

Is there anything we can do to make the transition smooth for anybody who
may be using pata_amd in 2.6.23?

Jordan

-- 
Jordan Crouse
Systems Software Development Engineer 
Advanced Micro Devices, Inc.

Re: pata_cs5536: ATA driver for Geode companion chip

[Alan Cox]

> Agreed.  It sounds like there is still breakage in the BIOS - since the
> reference VSA code accounts for accesses of different alignments, but its
> probably not worth going back and forth about it.  Like I said, if Alan
> and Jeff are game, we should go with the 5536 driver, and now that we've
> debated it extensively, I'm fine with the MSR accesses - I'm satisfied
> that we at least considered the alternative and dismissed it for technically
> sound reasons.

Even if we use PCI accesses and keep them dword along with the right
timing it makes sense to have a 5536 driver for this.
> 
> Is there anything we can do to make the transition smooth for anybody who
> may be using pata_amd in 2.6.23?

We can make pata_amd pick it up if pata_cs5536 is not selected for now

Re: pata_cs5536: ATA driver for Geode companion chip

[Martin K. Petersen]

>>>>> "Alan" == Alan Cox <alan@lxorguk.ukuu.org.uk> writes:

Alan> Even if we use PCI accesses and keep them dword along with the
Alan> right timing it makes sense to have a 5536 driver for this.

Yeah.  Let me polish the PCI version and send that out...

-- 
Martin K. Petersen      http://mkp.net/

Re: pata_cs5536: ATA driver for Geode companion chip

[Martin K. Petersen]

>>>>> "Jordan" == Jordan Crouse <jordan.crouse@amd.com> writes:

Jordan> Agreed.  It sounds like there is still breakage in the BIOS -
Jordan> since the reference VSA code accounts for accesses of
Jordan> different alignments, but its probably not worth going back
Jordan> and forth about it.  Like I said, if Alan and Jeff are game,
Jordan> we should go with the 5536 driver, and now that we've debated
Jordan> it extensively, I'm fine with the MSR accesses - I'm satisfied
Jordan> that we at least considered the alternative and dismissed it
Jordan> for technically sound reasons.

I'm going to post two patches.  One using MSR accesses and one using
PCI.

Mixing the two access methods was the third option but I think that's
more of an academic exercise.  MSR will always work so why try poking
holes in PCI config space first?  The BIOS I have now exhibits one
problem.  The one from last week had another.  Instead of trying to
work around bug-du-jour I'd rather not depend on the BIOS getting
things right.  And it's less than a handful of register writes we're
talking about here.  Hardly worth it to come up with fallback
heuristics for something that trivial.

But both patches are working with the BIOS from yesterday.  Pick your
poison :)

-- 
Martin K. Petersen      http://mkp.net/

[PATCH] pata_cs5536: ATA driver for Geode companion chip (PCI)

[Martin K. Petersen]

This is a driver specifically for the ATA controller on the Geode
CS5536 companion chip.  The PCI device ID for this device was
previously claimed by pata_amd.c but the PIO timings were not correct.
This driver also works around a bug in some BIOSes that emulate
unaligned access to the PCI config registers poorly.

Signed-off-by: Martin K. Petersen <mkp@mkp.net>

---

diff -r de183081194a drivers/ata/Kconfig
--- a/drivers/ata/Kconfig	Tue Oct 02 18:00:56 2007 +0000
+++ b/drivers/ata/Kconfig	Fri Oct 05 01:04:49 2007 -0400
@@ -254,6 +254,15 @@ config PATA_CS5535
 
 	  If unsure, say N.
 
+config PATA_CS5536
+	tristate "CS5536 PATA support (Experimental)"
+	depends on PCI && X86 && !X86_64 && EXPERIMENTAL
+	help
+	  This option enables support for the AMD CS5536
+	  companion chip used with the Geode LX processor family.
+
+	  If unsure, say N.
+
 config PATA_CYPRESS
 	tristate "Cypress CY82C693 PATA support (Very Experimental)"
 	depends on PCI && EXPERIMENTAL
diff -r de183081194a drivers/ata/Makefile
--- a/drivers/ata/Makefile	Tue Oct 02 18:00:56 2007 +0000
+++ b/drivers/ata/Makefile	Fri Oct 05 01:04:49 2007 -0400
@@ -27,6 +27,7 @@ obj-$(CONFIG_PATA_CS5520)	+= pata_cs5520
 obj-$(CONFIG_PATA_CS5520)	+= pata_cs5520.o
 obj-$(CONFIG_PATA_CS5530)	+= pata_cs5530.o
 obj-$(CONFIG_PATA_CS5535)	+= pata_cs5535.o
+obj-$(CONFIG_PATA_CS5536)	+= pata_cs5536.o
 obj-$(CONFIG_PATA_CYPRESS)	+= pata_cypress.o
 obj-$(CONFIG_PATA_EFAR)		+= pata_efar.o
 obj-$(CONFIG_PATA_HPT366)	+= pata_hpt366.o
diff -r de183081194a drivers/ata/pata_amd.c
--- a/drivers/ata/pata_amd.c	Tue Oct 02 18:00:56 2007 +0000
+++ b/drivers/ata/pata_amd.c	Fri Oct 05 01:04:49 2007 -0400
@@ -693,7 +693,9 @@ static const struct pci_device_id amd[] 
 	{ PCI_VDEVICE(NVIDIA,	PCI_DEVICE_ID_NVIDIA_NFORCE_MCP67_IDE),	8 },
 	{ PCI_VDEVICE(NVIDIA,	PCI_DEVICE_ID_NVIDIA_NFORCE_MCP73_IDE),	8 },
 	{ PCI_VDEVICE(NVIDIA,	PCI_DEVICE_ID_NVIDIA_NFORCE_MCP77_IDE),	8 },
+#ifndef CONFIG_PATA_CS5536	/* Temporary */
 	{ PCI_VDEVICE(AMD,	PCI_DEVICE_ID_AMD_CS5536_IDE),		9 },
+#endif
 
 	{ },
 };
diff -r de183081194a drivers/ata/pata_cs5536.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/drivers/ata/pata_cs5536.c	Fri Oct 05 01:04:49 2007 -0400
@@ -0,0 +1,295 @@
+/*
+ * pata_cs5536.c	- CS5536 PATA for new ATA layer
+ *			  (C) 2007 Martin K. Petersen <mkp@mkp.net>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 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; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ * Documentation:
+ *	Available from AMD web site.
+ *
+ * The IDE timing registers for the CS5536 live in the Geode Machine
+ * Specific Register file and not PCI config space.  Most BIOSes
+ * virtualize the PCI registers so the chip looks like a standard IDE
+ * controller.  Unfortunately not all implementations get this right.
+ * In particular some have problems with unaligned accesses to the
+ * virtualized PCI registers.  This driver always does full dword
+ * writes to work around the issue.
+ */
+
+#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/libata.h>
+#include <scsi/scsi_host.h>
+
+#define DRV_NAME	"pata_cs5536"
+#define DRV_VERSION	"0.0.4"
+
+enum {
+	IDE_CFG			= 0x40,
+	IDE_CFG_CHANEN		= 0x2,
+	IDE_CFG_CABLE		= 0x42,
+
+	IDE_D0_SHIFT		= 24,
+	IDE_D1_SHIFT		= 16,
+	IDE_DRV_MASK		= 0xff,
+
+	IDE_DTC			= 0x48,
+
+	IDE_CAST		= 0x4c,
+	IDE_CAST_D0_SHIFT	= 6,
+	IDE_CAST_D1_SHIFT	= 4,
+	IDE_CAST_DRV_MASK	= 0x3,
+	IDE_CAST_CMD_MASK	= 0xff,
+	IDE_CAST_CMD_SHIFT	= 24,
+
+	IDE_ETC			= 0x50,
+	IDE_ETC_NODMA		= 0x03,
+};
+
+/**
+ *	cs5536_cable_detect	-	detect cable type
+ *	@ap: Port to detect on
+ *	@deadline: deadline jiffies for the operation
+ *
+ *	Perform cable detection for ATA66 capable cable. Return a libata
+ *	cable type.
+ */
+
+static int cs5536_cable_detect(struct ata_port *ap)
+{
+	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+	u8 reg;
+
+	pci_read_config_byte(pdev, IDE_CFG_CABLE, &reg);
+
+	if (reg & (1 << ap->port_no))
+		return ATA_CBL_PATA80;
+	else
+		return ATA_CBL_PATA40;
+}
+
+/**
+ *	cs5536_set_piomode		-	PIO setup
+ *	@ap: ATA interface
+ *	@adev: device on the interface
+ */
+
+static void cs5536_set_piomode(struct ata_port *ap, struct ata_device *adev)
+{
+	static const u8 drv_timings[5] = {
+		0x98, 0x55, 0x32, 0x21, 0x20
+	};
+
+	static const u8 addr_timings[5] = {
+		0x2, 0x1, 0x0, 0x0, 0x0
+	};
+
+	static const u8 cmd_timings[5] = {
+		0x99, 0x92, 0x90, 0x22, 0x20
+	};
+
+	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+	struct ata_device *pair = ata_dev_pair(adev);
+	int mode = adev->pio_mode - XFER_PIO_0;
+	int cmdmode = mode;
+	int dshift = ap->port_no ? IDE_D1_SHIFT : IDE_D0_SHIFT;
+	int cshift = ap->port_no ? IDE_CAST_D1_SHIFT : IDE_CAST_D0_SHIFT;
+	u32 dtc, cast, etc;
+
+	if (pair)
+		cmdmode = min(mode, pair->pio_mode - XFER_PIO_0);
+
+	pci_read_config_dword(pdev, IDE_DTC, &dtc);
+	pci_read_config_dword(pdev, IDE_CAST, &cast);
+	pci_read_config_dword(pdev, IDE_ETC, &etc);
+
+	dtc &= ~(IDE_DRV_MASK << dshift);
+	dtc |= drv_timings[mode] << dshift;
+
+	cast &= ~(IDE_CAST_DRV_MASK << cshift);
+	cast |= addr_timings[mode] << cshift;
+
+	cast &= ~(IDE_CAST_CMD_MASK << IDE_CAST_CMD_SHIFT);
+	cast |= cmd_timings[cmdmode] << IDE_CAST_CMD_SHIFT;
+
+	etc &= ~(IDE_DRV_MASK << dshift);
+	etc |= IDE_ETC_NODMA << dshift;
+
+	pci_write_config_dword(pdev, IDE_DTC, dtc);
+	pci_write_config_dword(pdev, IDE_CAST, cast);
+	pci_write_config_dword(pdev, IDE_ETC, etc);
+}
+
+/**
+ *	cs5536_set_dmamode		-	DMA timing setup
+ *	@ap: ATA interface
+ *	@adev: Device being configured
+ *
+ */
+
+static void cs5536_set_dmamode(struct ata_port *ap, struct ata_device *adev)
+{
+	static const u8 udma_timings[6] = {
+		0xc2, 0xc1, 0xc0, 0xc4, 0xc5, 0xc6
+	};
+
+	static const u8 mwdma_timings[3] = {
+		0x67, 0x21, 0x20
+	};
+
+	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+	u32 dtc, etc;
+	int mode = adev->dma_mode;
+	int dshift = ap->port_no ? IDE_D1_SHIFT  : IDE_D0_SHIFT;
+
+	if (mode >= XFER_UDMA_0) {
+		pci_read_config_dword(pdev, IDE_ETC, &etc);
+
+		etc &= ~(IDE_DRV_MASK << dshift);
+		etc |= udma_timings[mode - XFER_UDMA_0] << dshift;
+
+		pci_write_config_dword(pdev, IDE_ETC, etc);
+	} else { /* MWDMA */
+		pci_read_config_dword(pdev, IDE_DTC, &dtc);
+
+		dtc &= ~(IDE_DRV_MASK << dshift);
+		dtc |= mwdma_timings[mode] << dshift;
+
+		pci_write_config_dword(pdev, IDE_DTC, dtc);
+	}
+}
+
+static struct scsi_host_template cs5536_sht = {
+	.module			= THIS_MODULE,
+	.name			= DRV_NAME,
+	.ioctl			= ata_scsi_ioctl,
+	.queuecommand		= ata_scsi_queuecmd,
+	.can_queue		= ATA_DEF_QUEUE,
+	.this_id		= ATA_SHT_THIS_ID,
+	.sg_tablesize		= LIBATA_MAX_PRD,
+	.cmd_per_lun		= ATA_SHT_CMD_PER_LUN,
+	.emulated		= ATA_SHT_EMULATED,
+	.use_clustering		= ATA_SHT_USE_CLUSTERING,
+	.proc_name		= DRV_NAME,
+	.dma_boundary		= ATA_DMA_BOUNDARY,
+	.slave_configure	= ata_scsi_slave_config,
+	.slave_destroy		= ata_scsi_slave_destroy,
+	.bios_param		= ata_std_bios_param,
+};
+
+static struct ata_port_operations cs5536_port_ops = {
+	.port_disable	= ata_port_disable,
+	.set_piomode	= cs5536_set_piomode,
+	.set_dmamode	= cs5536_set_dmamode,
+	.mode_filter	= ata_pci_default_filter,
+
+	.tf_load	= ata_tf_load,
+	.tf_read	= ata_tf_read,
+	.check_status	= ata_check_status,
+	.exec_command	= ata_exec_command,
+	.dev_select	= ata_std_dev_select,
+
+	.freeze		= ata_bmdma_freeze,
+	.thaw		= ata_bmdma_thaw,
+	.error_handler	= ata_bmdma_error_handler,
+	.post_internal_cmd = ata_bmdma_post_internal_cmd,
+	.cable_detect	= cs5536_cable_detect,
+
+	.bmdma_setup	= ata_bmdma_setup,
+	.bmdma_start	= ata_bmdma_start,
+	.bmdma_stop	= ata_bmdma_stop,
+	.bmdma_status	= ata_bmdma_status,
+
+	.qc_prep	= ata_qc_prep,
+	.qc_issue	= ata_qc_issue_prot,
+
+	.data_xfer	= ata_data_xfer,
+
+	.irq_handler	= ata_interrupt,
+	.irq_clear	= ata_bmdma_irq_clear,
+	.irq_on		= ata_irq_on,
+	.irq_ack	= ata_irq_ack,
+
+	.port_start	= ata_port_start,
+};
+
+/**
+ *	cs5536_init_one
+ *	@dev: PCI device
+ *	@id: Entry in match table
+ *
+ */
+
+static int cs5536_init_one(struct pci_dev *dev, const struct pci_device_id *id)
+{
+	static const struct ata_port_info info = {
+		.sht = &cs5536_sht,
+		.flags = ATA_FLAG_SLAVE_POSS,
+		.pio_mask = 0x1f,
+		.mwdma_mask = 0x07,
+		.udma_mask = ATA_UDMA5,
+		.port_ops = &cs5536_port_ops
+	};
+
+	const struct ata_port_info *ppi[] = { &info, &ata_dummy_port_info };
+	u8 reg;
+
+	pci_read_config_byte(dev, IDE_CFG, &reg);
+
+	if ((reg & IDE_CFG_CHANEN) == 0) {
+		printk(KERN_ERR DRV_NAME ": disabled by BIOS\n");
+		return -ENODEV;
+	}
+
+	return ata_pci_init_one(dev, ppi);
+}
+
+static const struct pci_device_id cs5536[] = {
+	{ PCI_VDEVICE(AMD,	PCI_DEVICE_ID_AMD_CS5536_IDE), },
+	{ },
+};
+
+static struct pci_driver cs5536_pci_driver = {
+	.name		= DRV_NAME,
+	.id_table	= cs5536,
+	.probe		= cs5536_init_one,
+	.remove		= ata_pci_remove_one,
+#ifdef CONFIG_PM
+	.suspend	= ata_pci_device_suspend,
+	.resume		= ata_pci_device_resume,
+#endif
+};
+
+static int __init cs5536_init(void)
+{
+	return pci_register_driver(&cs5536_pci_driver);
+}
+
+static void __exit cs5536_exit(void)
+{
+	pci_unregister_driver(&cs5536_pci_driver);
+}
+
+MODULE_AUTHOR("Martin K. Petersen");
+MODULE_DESCRIPTION("low-level driver for the CS5536 IDE controller");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(pci, cs5536);
+MODULE_VERSION(DRV_VERSION);
+
+module_init(cs5536_init);
+module_exit(cs5536_exit);

[PATCH] pata_cs5536: ATA driver for Geode companion chip (MSR)

[Martin K. Petersen]

This is a driver specifically for the ATA controller on the Geode
CS5536 companion chip.  The PCI device ID for this device was
previously claimed by pata_amd.c but there were two problems with
that:

 - Not all Geode platforms emulate the ATA registers in PCI config
   space

 - CS5536 PIO timings are not identical to AMD74XX/8111

pata_cs5536.c relies on Geode Machine Specific Registers to configure
the ATA function and uses the correct PIO timings for the chip.

Signed-off-by: Martin K. Petersen <mkp@mkp.net>

---

diff -r de183081194a drivers/ata/Kconfig
--- a/drivers/ata/Kconfig	Tue Oct 02 18:00:56 2007 +0000
+++ b/drivers/ata/Kconfig	Fri Oct 05 01:20:22 2007 -0400
@@ -254,6 +254,15 @@ config PATA_CS5535
 
 	  If unsure, say N.
 
+config PATA_CS5536
+	tristate "CS5536 PATA support (Experimental)"
+	depends on PCI && X86 && !X86_64 && EXPERIMENTAL
+	help
+	  This option enables support for the AMD CS5536
+	  companion chip used with the Geode LX processor family.
+
+	  If unsure, say N.
+
 config PATA_CYPRESS
 	tristate "Cypress CY82C693 PATA support (Very Experimental)"
 	depends on PCI && EXPERIMENTAL
diff -r de183081194a drivers/ata/Makefile
--- a/drivers/ata/Makefile	Tue Oct 02 18:00:56 2007 +0000
+++ b/drivers/ata/Makefile	Fri Oct 05 01:20:22 2007 -0400
@@ -27,6 +27,7 @@ obj-$(CONFIG_PATA_CS5520)	+= pata_cs5520
 obj-$(CONFIG_PATA_CS5520)	+= pata_cs5520.o
 obj-$(CONFIG_PATA_CS5530)	+= pata_cs5530.o
 obj-$(CONFIG_PATA_CS5535)	+= pata_cs5535.o
+obj-$(CONFIG_PATA_CS5536)	+= pata_cs5536.o
 obj-$(CONFIG_PATA_CYPRESS)	+= pata_cypress.o
 obj-$(CONFIG_PATA_EFAR)		+= pata_efar.o
 obj-$(CONFIG_PATA_HPT366)	+= pata_hpt366.o
diff -r de183081194a drivers/ata/pata_amd.c
--- a/drivers/ata/pata_amd.c	Tue Oct 02 18:00:56 2007 +0000
+++ b/drivers/ata/pata_amd.c	Fri Oct 05 01:20:22 2007 -0400
@@ -693,7 +693,9 @@ static const struct pci_device_id amd[] 
 	{ PCI_VDEVICE(NVIDIA,	PCI_DEVICE_ID_NVIDIA_NFORCE_MCP67_IDE),	8 },
 	{ PCI_VDEVICE(NVIDIA,	PCI_DEVICE_ID_NVIDIA_NFORCE_MCP73_IDE),	8 },
 	{ PCI_VDEVICE(NVIDIA,	PCI_DEVICE_ID_NVIDIA_NFORCE_MCP77_IDE),	8 },
+#ifndef CONFIG_PATA_CS5536	/* Temporary */
 	{ PCI_VDEVICE(AMD,	PCI_DEVICE_ID_AMD_CS5536_IDE),		9 },
+#endif
 
 	{ },
 };
diff -r de183081194a drivers/ata/pata_cs5536.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/drivers/ata/pata_cs5536.c	Fri Oct 05 01:20:22 2007 -0400
@@ -0,0 +1,298 @@
+/*
+ * pata_cs5536.c	- CS5536 PATA for new ATA layer
+ *			  (C) 2007 Martin K. Petersen <mkp@mkp.net>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 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; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ * Documentation:
+ *	Available from AMD web site.
+ *
+ * Like its predecessor (CS5535), CS5536 exposes its IDE configuration
+ * registers to the Geode Machine Specific Register file instead of
+ * PCI config space.  The 5536 IDE registers are modeled after the AMD
+ * 74xx PCI IDE controller although some of the timing knobs are
+ * subtly different.  Initially the CS5536 was supported by the
+ * pata_amd.c driver as some Geode platforms emulate the PCI config
+ * register support in firmware.  This driver, however, does not
+ * depend on PCI config emulation and accesses the MSR registers
+ * directly.
+ */
+
+#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/libata.h>
+#include <scsi/scsi_host.h>
+#include <asm/msr.h>
+
+#define DRV_NAME	"pata_cs5536"
+#define DRV_VERSION	"0.0.3"
+
+enum {
+	MSR_IDE_BASE		= 0x51300000,
+
+	MSR_IDE_CFG		= (MSR_IDE_BASE + 0x10),
+	MSR_IDE_DTC		= (MSR_IDE_BASE + 0x12),
+	MSR_IDE_CAST		= (MSR_IDE_BASE + 0x13),
+	MSR_IDE_ETC		= (MSR_IDE_BASE + 0x14),
+
+	IDE_CFG_CHANEN          = 0x2,
+	IDE_CFG_CABLE           = 0x10000,
+	IDE_CFG_CABLE_MASK      = 0x30000,
+
+	IDE_D0_SHIFT		= 24,
+	IDE_D1_SHIFT		= 16,
+	IDE_DRV_MASK		= 0xff,
+
+	IDE_CAST_D0_SHIFT	= 6,
+	IDE_CAST_D1_SHIFT	= 4,
+	IDE_CAST_DRV_MASK	= 0x3,
+	IDE_CAST_CMD_MASK	= 0xff,
+	IDE_CAST_CMD_SHIFT	= 24,
+
+	IDE_ETC_NODMA		= 0x03,
+};
+
+/**
+ *	cs5536_cable_detect	-	detect cable type
+ *	@ap: Port to detect on
+ *	@deadline: deadline jiffies for the operation
+ *
+ *	Perform cable detection for ATA66 capable cable. Return a libata
+ *	cable type.
+ */
+
+static int cs5536_cable_detect(struct ata_port *ap)
+{
+	u32 reg, dummy;
+
+	rdmsr(MSR_IDE_CFG, reg, dummy);
+
+	if (reg & (IDE_CFG_CABLE << ap->port_no))
+		return ATA_CBL_PATA80;
+	else
+		return ATA_CBL_PATA40;
+}
+
+/**
+ *	cs5536_set_piomode		-	PIO setup
+ *	@ap: ATA interface
+ *	@adev: device on the interface
+ */
+
+static void cs5536_set_piomode(struct ata_port *ap, struct ata_device *adev)
+{
+	static const u8 drv_timings[5] = {
+		0x98, 0x55, 0x32, 0x21, 0x20
+	};
+
+	static const u8 addr_timings[5] = {
+		0x2, 0x1, 0x0, 0x0, 0x0
+	};
+
+	static const u8 cmd_timings[5] = {
+		0x99, 0x92, 0x90, 0x22, 0x20
+	};
+
+	struct ata_device *pair = ata_dev_pair(adev);
+	int mode = adev->pio_mode - XFER_PIO_0;
+	int cmdmode = mode;
+	int dshift = ap->port_no ? IDE_D1_SHIFT : IDE_D0_SHIFT;
+	int cshift = ap->port_no ? IDE_CAST_D1_SHIFT : IDE_CAST_D0_SHIFT;
+	u32 dtc, cast, etc, dummy;
+
+	if (pair)
+		cmdmode = min(mode, pair->pio_mode - XFER_PIO_0);
+
+	rdmsr(MSR_IDE_DTC, dtc, dummy);
+	rdmsr(MSR_IDE_CAST, cast, dummy);
+	rdmsr(MSR_IDE_ETC, etc, dummy);
+
+	dtc &= ~(IDE_DRV_MASK << dshift);
+	dtc |= drv_timings[mode] << dshift;
+
+	cast &= ~(IDE_CAST_DRV_MASK << cshift);
+	cast |= addr_timings[mode] << cshift;
+
+	cast &= ~(IDE_CAST_CMD_MASK << IDE_CAST_CMD_SHIFT);
+	cast |= cmd_timings[cmdmode] << IDE_CAST_CMD_SHIFT;
+
+	etc &= ~(IDE_DRV_MASK << dshift);
+	etc |= IDE_ETC_NODMA << dshift;
+
+	wrmsr(MSR_IDE_DTC, dtc, dummy);
+	wrmsr(MSR_IDE_CAST, cast, dummy);
+	wrmsr(MSR_IDE_ETC, etc, dummy);
+}
+
+/**
+ *	cs5536_set_dmamode		-	DMA timing setup
+ *	@ap: ATA interface
+ *	@adev: Device being configured
+ *
+ */
+
+static void cs5536_set_dmamode(struct ata_port *ap, struct ata_device *adev)
+{
+	static const u8 udma_timings[6] = {
+		0xc2, 0xc1, 0xc0, 0xc4, 0xc5, 0xc6
+	};
+
+	static const u8 mwdma_timings[3] = {
+		0x67, 0x21, 0x20
+	};
+
+	u32 dtc, etc, dummy;
+	int mode = adev->dma_mode;
+	int dshift = ap->port_no ? IDE_D1_SHIFT  : IDE_D0_SHIFT;
+
+	if (mode >= XFER_UDMA_0) {
+		rdmsr(MSR_IDE_ETC, etc, dummy);
+
+		etc &= ~(IDE_DRV_MASK << dshift);
+		etc |= udma_timings[mode - XFER_UDMA_0] << dshift;
+
+		wrmsr(MSR_IDE_ETC, etc, dummy);
+	} else { /* MWDMA */
+		rdmsr(MSR_IDE_DTC, dtc, dummy);
+
+		dtc &= ~(IDE_DRV_MASK << dshift);
+		dtc |= mwdma_timings[mode] << dshift;
+
+		wrmsr(MSR_IDE_DTC, dtc, dummy);
+	}
+}
+
+static struct scsi_host_template cs5536_sht = {
+	.module			= THIS_MODULE,
+	.name			= DRV_NAME,
+	.ioctl			= ata_scsi_ioctl,
+	.queuecommand		= ata_scsi_queuecmd,
+	.can_queue		= ATA_DEF_QUEUE,
+	.this_id		= ATA_SHT_THIS_ID,
+	.sg_tablesize		= LIBATA_MAX_PRD,
+	.cmd_per_lun		= ATA_SHT_CMD_PER_LUN,
+	.emulated		= ATA_SHT_EMULATED,
+	.use_clustering		= ATA_SHT_USE_CLUSTERING,
+	.proc_name		= DRV_NAME,
+	.dma_boundary		= ATA_DMA_BOUNDARY,
+	.slave_configure	= ata_scsi_slave_config,
+	.slave_destroy		= ata_scsi_slave_destroy,
+	.bios_param		= ata_std_bios_param,
+};
+
+static struct ata_port_operations cs5536_port_ops = {
+	.port_disable	= ata_port_disable,
+	.set_piomode	= cs5536_set_piomode,
+	.set_dmamode	= cs5536_set_dmamode,
+	.mode_filter	= ata_pci_default_filter,
+
+	.tf_load	= ata_tf_load,
+	.tf_read	= ata_tf_read,
+	.check_status	= ata_check_status,
+	.exec_command	= ata_exec_command,
+	.dev_select	= ata_std_dev_select,
+
+	.freeze		= ata_bmdma_freeze,
+	.thaw		= ata_bmdma_thaw,
+	.error_handler	= ata_bmdma_error_handler,
+	.post_internal_cmd = ata_bmdma_post_internal_cmd,
+	.cable_detect	= cs5536_cable_detect,
+
+	.bmdma_setup	= ata_bmdma_setup,
+	.bmdma_start	= ata_bmdma_start,
+	.bmdma_stop	= ata_bmdma_stop,
+	.bmdma_status	= ata_bmdma_status,
+
+	.qc_prep	= ata_qc_prep,
+	.qc_issue	= ata_qc_issue_prot,
+
+	.data_xfer	= ata_data_xfer,
+
+	.irq_handler	= ata_interrupt,
+	.irq_clear	= ata_bmdma_irq_clear,
+	.irq_on		= ata_irq_on,
+	.irq_ack	= ata_irq_ack,
+
+	.port_start	= ata_port_start,
+};
+
+/**
+ *	cs5536_init_one
+ *	@dev: PCI device
+ *	@id: Entry in match table
+ *
+ */
+
+static int cs5536_init_one(struct pci_dev *dev, const struct pci_device_id *id)
+{
+	static const struct ata_port_info info = {
+		.sht = &cs5536_sht,
+		.flags = ATA_FLAG_SLAVE_POSS,
+		.pio_mask = 0x1f,
+		.mwdma_mask = 0x07,
+		.udma_mask = ATA_UDMA5,
+		.port_ops = &cs5536_port_ops
+	};
+
+	const struct ata_port_info *ppi[] = { &info, &ata_dummy_port_info };
+	u32 cfg, dummy;
+
+	rdmsr(MSR_IDE_CFG, cfg, dummy);
+
+	if ((cfg & IDE_CFG_CHANEN) == 0) {
+		printk(KERN_ERR DRV_NAME ": disabled by BIOS\n");
+		return -ENODEV;
+	}
+
+	return ata_pci_init_one(dev, ppi);
+}
+
+static const struct pci_device_id cs5536[] = {
+	{ PCI_VDEVICE(AMD,	PCI_DEVICE_ID_AMD_CS5536_IDE), },
+	{ },
+};
+
+static struct pci_driver cs5536_pci_driver = {
+	.name		= DRV_NAME,
+	.id_table	= cs5536,
+	.probe		= cs5536_init_one,
+	.remove		= ata_pci_remove_one,
+#ifdef CONFIG_PM
+	.suspend	= ata_pci_device_suspend,
+	.resume		= ata_pci_device_resume,
+#endif
+};
+
+static int __init cs5536_init(void)
+{
+	return pci_register_driver(&cs5536_pci_driver);
+}
+
+static void __exit cs5536_exit(void)
+{
+	pci_unregister_driver(&cs5536_pci_driver);
+}
+
+MODULE_AUTHOR("Martin K. Petersen");
+MODULE_DESCRIPTION("low-level driver for the CS5536 IDE controller");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(pci, cs5536);
+MODULE_VERSION(DRV_VERSION);
+
+module_init(cs5536_init);
+module_exit(cs5536_exit);

Re: [PATCH] pata_cs5536: ATA driver for Geode companion chip (MSR)

[Alan Cox]

> pata_cs5536.c relies on Geode Machine Specific Registers to configure
> the ATA function and uses the correct PIO timings for the chip.

Any reason for not supporting both methods ? Though I guess as you say
msr is sufficient.


> +#ifndef CONFIG_PATA_CS5536	/* Temporary */

	!defined(FOO) && ! defined(FOO_MODULE)

otherwise ACK


Alan

Re: pata_cs5536: ATA driver for Geode companion chip (MSR)

[Jordan Crouse]

On 05/10/07 13:01 +0100, Alan Cox wrote:
> > pata_cs5536.c relies on Geode Machine Specific Registers to configure
> > the ATA function and uses the correct PIO timings for the chip.
> 
> Any reason for not supporting both methods ? Though I guess as you say
> msr is sufficient.
> 
> 
> > +#ifndef CONFIG_PATA_CS5536	/* Temporary */
> 
> 	!defined(FOO) && ! defined(FOO_MODULE)
> 
> otherwise ACK

ACK as well, but I put a call out on the Geode list for testing - I want
to make sure that we get some time on different platforms with different
BIOSes to make sure it is solid.

Jordan

-- 
Jordan Crouse
Systems Software Development Engineer 
Advanced Micro Devices, Inc.

Re: [PATCH] pata_cs5536: ATA driver for Geode companion chip (MSR)

[Martin K. Petersen]

>>>>> "Alan" == Alan Cox <alan@lxorguk.ukuu.org.uk> writes:

>> pata_cs5536.c relies on Geode Machine Specific Registers to
>> configure the ATA function and uses the correct PIO timings for the
>> chip.

Alan> Any reason for not supporting both methods ? Though I guess as
Alan> you say msr is sufficient.

Given that we don't have a good way to detect failure other than
libata failing to set PIO timings, I think it's going to be
unnecessary complexity.  As I wrote, the BIOS versions all behaved
differently (in a previous rev. the written value would look ok in the
PCI config register but not trickle back to MSR space).

In any case the friendly firmware fairy left me a working BIOS this
morning.  Byte-aligned PCI config space accesses do the right thing.
So as far as I'm concerned the case has been solv-ed.  I'm now
officially agnostic.

Jordan: You're probably dealing with issues like this more frequently
than we are.  What's your preference for pata_cs5536.c?  MSR or PCI?

-- 
Martin K. Petersen      http://mkp.net/