Generic platform device IDE driver

This is intended purely for the simple NO_DMA ide_generic case.. nothing complicated.

What do people think about this, is there a better way to do this?

Ar Mer, 2006-10-04 am 16:45 +0900, ysgrifennodd Paul Mundt:

Quoted 4 lines

This is intended purely for the simple NO_DMA ide_generic case.. nothing complicated.

What do people think about this, is there a better way to do this?

drivers/ide is going away over time.

I think the concept is nice and it's sort of reflected in the libata VLB drivers. I think it would be a very good way to get good platform drivers for libata for the embedded platforms.

Moving the existing drivers/ide stuff to a new drivers/ide variant is wasted work however.

Ok, I wasn't sure if libata was intended for anything outside of the SATA case (especially non-PCI), but if that's the way to go, I'll look at hacking something up under libata.

Ar Iau, 2006-10-05 am 05:05 +0900, ysgrifennodd Paul Mundt:

Quoted 3 lines

Ok, I wasn't sure if libata was intended for anything outside of the SATA case (especially non-PCI), but if that's the way to go, I'll look at hacking something up under libata.

Take a look at 2.6.18-mm or 2.6.19-* and you should see all you need in that including the pcmcia driver and other users who set up much the same way a generic platform device driver would.

Quoted 9 lines · Alan Cox

Ar Iau, 2006-10-05 am 05:05 +0900, ysgrifennodd Paul Mundt:

Quoted 3 lines

Ok, I wasn't sure if libata was intended for anything outside of the SATA case (especially non-PCI), but if that's the way to go, I'll look at hacking something up under libata.

Take a look at 2.6.18-mm or 2.6.19-* and you should see all you need in that including the pcmcia driver and other users who set up much the same way a generic platform device driver would.

Ok, just hacked this together quickly, how does it look? I'm booting this on an SH-4A (R7780RP) from current git and all works fine..

Thankfully you did most of the heavy lifting :-)

I tried you patch on our CPCI405 PowerPC board (arch/ppc/platforms/4xx/cpci405.c).
It seems to work fine together with our onboard CompactFlash slot.

Because our IDE registers are memory mapped, I had to patch the resource .start and .end address by subtracting _IO_BASE, so that the pata code can use the IO way to talk to the IDE registers.

Perhaps it is a good idea to update the pata platform driver to be able to handle both _IO and _MEM resources. The _IO resources be be handled as it is already done by your code and for _MEM resources the pata platform driver can do the ioremapping as I currently do in my board setup.

Quoted 4 lines · Matthias Fuchs

Perhaps it is a good idea to update the pata platform driver to be able to handle both _IO and _MEM resources. The _IO resources be be handled as it is already done by your code and for _MEM resources the pata platform driver can do the ioremapping as I currently do in my board setup.

Yes, that's one thing I was thinking of as well.. Here's a patch that makes an attempt at that, can you give it a try and see if it works for you? This applies on top of the earlier patch. None of the ARM, SH, or H8300 cases need to do the remapping at least.

Quoted 4 lines · Paul Mundt

Yes, that's one thing I was thinking of as well.. Here's a patch that makes an attempt at that, can you give it a try and see if it works for you? This applies on top of the earlier patch. None of the ARM, SH, or H8300 cases need to do the remapping at least.

It's likely that ARM will switch over to using the MMIO resources if this patch makes it in. There's certain ARM platforms which would benefit from this change (since inb() and friends are more complex than they necessarily need to be.)

However, one issue needs to be solved before we could do that - how do we handle the case where the IDE registers are on a 4 byte spacing interval instead of the usual 1 byte?

I notice that this driver is calling ata_std_ports() which handles the standard setup. Maybe that needs to become a little more inteligent?

Quoted 5 lines · Russell King

Quoted 4 lines · Paul Mundt

Yes, that's one thing I was thinking of as well.. Here's a patch that makes an attempt at that, can you give it a try and see if it works for you? This applies on top of the earlier patch. None of the ARM, SH, or H8300 cases need to do the remapping at least.

Quoted 9 lines

It's likely that ARM will switch over to using the MMIO resources if this patch makes it in. There's certain ARM platforms which would benefit from this change (since inb() and friends are more complex than they necessarily need to be.)

However, one issue needs to be solved before we could do that - how do we handle the case where the IDE registers are on a 4 byte spacing interval instead of the usual 1 byte?

We could solve this in the driver, but it sounds like this is something that libata should have some visibility of directly.

Quoted 3 lines

I notice that this driver is calling ata_std_ports() which handles the standard setup. Maybe that needs to become a little more inteligent?

If we go this route, I suppose the easiest option will be simply to have a private structure with a few function pointers for these sorts of things, or we can simply have an element for the spacing interval if you don't forsee needing to change the ioaddrs in any fashion beyond the register spacing.. Which approach would you be more comfortable with? Are there any other items that you're concerned with in the current driver?

Currently the platforms that have very simplistic needs for IDE devices are forced to invent their own driver for adding the built-in devices. At the moment ARM is the in-tree user in this category, h8300 could be switched to something more generic, and SH is roughly in the same category as ARM.

The only constant that really tends to change between these platforms are the I/O base and the IRQ, with everything else rather constant. The ctl base could also change, but everyone seems to keep it at 0x206, so it's debatable whether it's worth leaving this configurable or not.

I've hacked together a quick driver, 'ide-platform' (for lack of a better name), that allows for these specifics to be passed in via platform device resources (registering a device per-port) rather than having to add more of these things to drivers/ide.

With this we can remove ide_arm (and the h8300 driver can likely be reworked to use this too), and I don't have to invent a new driver for SH that does effectively the same thing.

This is intended purely for the simple NO_DMA ide_generic case.. nothing complicated.

What do people think about this, is there a better way to do this?

Signed-off-by Paul Mundt <[email protected]>

--

 drivers/ide/Makefile       |    3 +-
 drivers/ide/ide-platform.c |   60 +++++++++++++++++++++++++++++++++ 
++++++++++++
 drivers/ide/ide.c          |    3 ++
 3 files changed, 65 insertions(+), 1 deletion(-)

commit f2da2c8c9b5e648ca9a43d9d3fed9bcd356f0efd

Author: Paul Mundt <[email protected]>
Date:   Wed Oct 4 16:41:21 2006 +0900

ide: Simple platform device IDE driver.

This adds a trivial platform device IDE driver. Users are
required to
register a couple of resources:

- I/O Base
- CTL Base
- IRQ

Signed-off-by Paul Mundt <[email protected]>

drivers/ide/Makefile +0 -1

index 569fae7..24c9a0f 100644
@@ -13,7 +13,8 @@ EXTRA_CFLAGS				+= -Idrivers/ide
 
 obj-$(CONFIG_BLK_DEV_IDE)		+= pci/
 
-ide-core-y += ide.o ide-io.o ide-iops.o ide-lib.o ide-probe.o ide- 

taskfile.o
+ide-core-y += ide.o ide-io.o ide-iops.o ide-lib.o ide-probe.o ide- taskfile.o \

drivers/ide/Makefile +1 -0

+	      ide-platform.o
 
 ide-core-$(CONFIG_BLK_DEV_CMD640)	+= pci/cmd640.o

drivers/ide/ide-platform.c new file +6 -0

new file mode 100644
index 0000000..6d89878
@@ -0,0 +1,60 @@
+/*
+ * Generic IDE platform device driver
+ *
+ * Copyright (C) 2006  Paul Mundt
+ *
+ * This file is subject to the terms and conditions of the GNU  

General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */

drivers/ide/ide-platform.c new file +51 -0

+#include <linux/init.h>
+#include <linux/ide.h>
+#include <linux/platform_device.h>
+
+/*
+ * Users use per-port registration with a simple set of 3 resources
+ * per port:
+ * 		- I/O Base (IORESOURCE_IO)
+ * 		- CTL Base (IORESOURCE_IO)
+ * 		- IRQ	   (IORESOURCE_IRQ)
+ */
+static int __devinit ide_platform_probe(struct platform_device *dev)
+{
+	struct resource *io_res, *ctl_res;
+	hw_regs_t hw;
+
+	if (unlikely(dev->num_resources != 3)) {
+		dev_err(&dev->dev, "invalid number of resources\n");
+		return -EINVAL;
+	}
+
+	io_res = platform_get_resource(dev, IORESOURCE_IO, 0);
+	if (unlikely(io_res == NULL))
+		return -EINVAL;
+
+	ctl_res = platform_get_resource(dev, IORESOURCE_IO, 1);
+	if (unlikely(ctl_res == NULL))
+		return -EINVAL;
+
+	memset(&hw, 0, sizeof(hw_regs_t));
+	ide_std_init_ports(&hw, io_res->start, ctl_res->start);
+
+	hw.irq		= platform_get_irq(dev, 0);
+	hw.chipset	= ide_generic;
+
+	return ide_register_hw(&hw, NULL);
+}
+
+static struct platform_driver ide_platform_driver = {
+	.probe		= ide_platform_probe,
+	.driver		= {
+		.name	= "ide-platform",
+		.owner	= THIS_MODULE,
+	},
+};
+
+void __init ide_platform_init(void)
+{
+	printk(KERN_INFO "ide-platform: platform device IDE interface\n");
+	platform_driver_register(&ide_platform_driver);
+}

drivers/ide/ide.c +3 -1

index 287a662..994cdd4 100644
@@ -1782,6 +1782,7 @@ done:
 }
 
 extern void pnpide_init(void);
+extern void ide_platform_init(void);
 extern void h8300_ide_init(void);
 
 /*

@@ -1793,6 +1794,8 @@ #ifdef CONFIG_BLK_DEV_IDEPCI
 	ide_scan_pcibus(ide_scan_direction);
 #endif /* CONFIG_BLK_DEV_IDEPCI */
 
+	ide_platform_init();
+
 #ifdef CONFIG_ETRAX_IDE
 	{
 		extern void init_e100_ide(void);
-

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/

question: where is linux/ide-platform.h header?

Quoted 2 lines · girish

question: where is linux/ide-platform.h header?

answer: there isn't one, as it's not needed. The reason for using platform devices is so we get _away_ from this ridiculous static set of definitions for I/O addresses and IRQs that are sprinkled around a lot of these drivers.