OF-platform PATA driver

PATCH Superseded Anton Vorontsov 7 participants 29 messages Dec 4, 2007 – Dec 20, 2007

Subsystems (8)

Device tree bindings Arm (32-bit) Arm sub-architectures PowerPC +4 more subsystems

lore

Share & tools

lore

Anton Vorontsov

author

2007-12-04 17:00 UTC #0 lore raw

v2 cover OF-platform PATA driver

Hi all,

Here is the second version of the OF-platform PATA driver and related patches.

Arnd, the first patch has been changed, so technically it lost your Acked-by.

- - -
Changes since v1:
- __pata_platform_probe now accepts pio_mask argument;
- pata-platform compatible property renamed to ata-generic;
- pata_of_platform understands pio-mode property. It's used to pass pio_mask to the __pata_platform_probe. That is, in ata-generic context pio-mode means "pio mode the bus already configured for";
- New optional patch that renames pata_platform_info's ioport_shift to reg_shift.

Changes since RFC:
- nuked drivers/ata/pata_platform.h;
- powerpc bits: proper localbus node added.

Signature · 4 lines

Anton Vorontsov
email: [email protected]
backup email: [email protected]
irc://irc.freenode.net/bd2

Anton Vorontsov

author

2007-12-04 17:02 UTC #1 lore raw

PATCH v2 1/4 pata_platform: make probe and remove functions device type neutral

Split pata_platform_{probe,remove} into two pieces:
1. pata_platform_{probe,remove} -- platform_device-dependant bits;
2. __ptata_platform_{probe,remove} -- device type neutral bits.

This is done to not duplicate code for the OF-platform driver.

Signed-off-by Anton Vorontsov <[email protected]>

---
 drivers/ata/pata_platform.c   |  143 ++++++++++++++++++++++++-----------------
 include/linux/pata_platform.h |    9 +++
 2 files changed, 94 insertions(+), 58 deletions(-)

drivers/ata/pata_platform.c +85 ~~-58~~

index ac03a90..d4cb231 100644
@@ -93,14 +93,9 @@ static struct ata_port_operations pata_platform_port_ops = {
 };
 
 static void pata_platform_setup_port(struct ata_ioports *ioaddr,
-				     struct pata_platform_info *info)
+				     unsigned int shift)
 {
-	unsigned int shift = 0;
-
 	/* Fixup the port shift for platforms that need it */
-	if (info && info->ioport_shift)
-		shift = info->ioport_shift;
-
 	ioaddr->data_addr	= ioaddr->cmd_addr + (ATA_REG_DATA    << shift);
 	ioaddr->error_addr	= ioaddr->cmd_addr + (ATA_REG_ERR     << shift);
 	ioaddr->feature_addr	= ioaddr->cmd_addr + (ATA_REG_FEATURE << shift);

@@ -114,8 +109,12 @@ static void pata_platform_setup_port(struct ata_ioports *ioaddr,
 }
 
 /**
- *	pata_platform_probe		-	attach a platform interface
- *	@pdev: platform device
+ *	__pata_platform_probe		-	attach a platform interface
+ *	@dev: device
+ *	@io_res: Resource representing I/O base
+ *	@ctl_res: Resource representing CTL base
+ *	@irq_res: Resource representing IRQ and its flags
+ *	@ioport_shift: I/O port shift
  *
  *	Register a platform bus IDE interface. Such interfaces are PIO and we
  *	assume do not support IRQ sharing.

@@ -135,42 +134,18 @@ static void pata_platform_setup_port(struct ata_ioports *ioaddr,
  *
  *	If no IRQ resource is present, PIO polling mode is used instead.
  */
-static int __devinit pata_platform_probe(struct platform_device *pdev)
+int __devinit __pata_platform_probe(struct device *dev,
+				    struct resource *io_res,
+				    struct resource *ctl_res,
+				    struct resource *irq_res,
+				    unsigned int ioport_shift,
+				    int __pio_mask)
 {
-	struct resource *io_res, *ctl_res;
 	struct ata_host *host;
 	struct ata_port *ap;
-	struct pata_platform_info *pp_info;
 	unsigned int mmio;
-	int irq;
-
-	/*
-	 * Simple resource validation ..
-	 */
-	if ((pdev->num_resources != 3) && (pdev->num_resources != 2)) {
-		dev_err(&pdev->dev, "invalid number of resources\n");
-		return -EINVAL;
-	}
-
-	/*
-	 * Get the I/O base first
-	 */
-	io_res = platform_get_resource(pdev, IORESOURCE_IO, 0);
-	if (io_res == NULL) {
-		io_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-		if (unlikely(io_res == NULL))
-			return -EINVAL;
-	}
-
-	/*
-	 * Then the CTL base
-	 */
-	ctl_res = platform_get_resource(pdev, IORESOURCE_IO, 1);
-	if (ctl_res == NULL) {
-		ctl_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-		if (unlikely(ctl_res == NULL))
-			return -EINVAL;
-	}
+	int irq = 0;
+	int irq_flags = 0;
 
 	/*
 	 * Check for MMIO

@@ -181,20 +156,21 @@ static int __devinit pata_platform_probe(struct platform_device *pdev)
 	/*
 	 * And the IRQ
 	 */
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		irq = 0;	/* no irq */
+	if (irq_res && irq_res->start > 0) {
+		irq = irq_res->start;
+		irq_flags = irq_res->flags;
+	}
 
 	/*
 	 * Now that that's out of the way, wire up the port..
 	 */
-	host = ata_host_alloc(&pdev->dev, 1);
+	host = ata_host_alloc(dev, 1);
 	if (!host)
 		return -ENOMEM;
 	ap = host->ports[0];
 
 	ap->ops = &pata_platform_port_ops;
-	ap->pio_mask = pio_mask;
+	ap->pio_mask = __pio_mask;
 	ap->flags |= ATA_FLAG_SLAVE_POSS;
 
 	/*

@@ -209,25 +185,24 @@ static int __devinit pata_platform_probe(struct platform_device *pdev)
 	 * Handle the MMIO case
 	 */
 	if (mmio) {
-		ap->ioaddr.cmd_addr = devm_ioremap(&pdev->dev, io_res->start,
+		ap->ioaddr.cmd_addr = devm_ioremap(dev, io_res->start,
 				io_res->end - io_res->start + 1);
-		ap->ioaddr.ctl_addr = devm_ioremap(&pdev->dev, ctl_res->start,
+		ap->ioaddr.ctl_addr = devm_ioremap(dev, ctl_res->start,
 				ctl_res->end - ctl_res->start + 1);
 	} else {
-		ap->ioaddr.cmd_addr = devm_ioport_map(&pdev->dev, io_res->start,
+		ap->ioaddr.cmd_addr = devm_ioport_map(dev, io_res->start,
 				io_res->end - io_res->start + 1);
-		ap->ioaddr.ctl_addr = devm_ioport_map(&pdev->dev, ctl_res->start,
+		ap->ioaddr.ctl_addr = devm_ioport_map(dev, ctl_res->start,
 				ctl_res->end - ctl_res->start + 1);
 	}
 	if (!ap->ioaddr.cmd_addr || !ap->ioaddr.ctl_addr) {
-		dev_err(&pdev->dev, "failed to map IO/CTL base\n");
+		dev_err(dev, "failed to map IO/CTL base\n");
 		return -ENOMEM;
 	}
 
 	ap->ioaddr.altstatus_addr = ap->ioaddr.ctl_addr;
 
-	pp_info = pdev->dev.platform_data;
-	pata_platform_setup_port(&ap->ioaddr, pp_info);
+	pata_platform_setup_port(&ap->ioaddr, ioport_shift);
 
 	ata_port_desc(ap, "%s cmd 0x%llx ctl 0x%llx", mmio ? "mmio" : "ioport",
 		      (unsigned long long)io_res->start,

@@ -235,26 +210,78 @@ static int __devinit pata_platform_probe(struct platform_device *pdev)
 
 	/* activate */
 	return ata_host_activate(host, irq, irq ? ata_interrupt : NULL,
-				 pp_info ? pp_info->irq_flags : 0,
-				 &pata_platform_sht);
+				 irq_flags, &pata_platform_sht);
 }
+EXPORT_SYMBOL_GPL(__pata_platform_probe);
 
 /**
- *	pata_platform_remove	-	unplug a platform interface
- *	@pdev: platform device
+ *	__pata_platform_remove		-	unplug a platform interface
+ *	@dev: device
  *
  *	A platform bus ATA device has been unplugged. Perform the needed
  *	cleanup. Also called on module unload for any active devices.
  */
-static int __devexit pata_platform_remove(struct platform_device *pdev)
+int __devexit __pata_platform_remove(struct device *dev)
 {
-	struct device *dev = &pdev->dev;
 	struct ata_host *host = dev_get_drvdata(dev);
 
 	ata_host_detach(host);
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(__pata_platform_remove);
+
+static int __devinit pata_platform_probe(struct platform_device *pdev)
+{
+	struct resource *io_res;
+	struct resource *ctl_res;
+	struct resource *irq_res;
+	struct pata_platform_info *pp_info = pdev->dev.platform_data;
+
+	/*
+	 * Simple resource validation ..
+	 */
+	if ((pdev->num_resources != 3) && (pdev->num_resources != 2)) {
+		dev_err(&pdev->dev, "invalid number of resources\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * Get the I/O base first
+	 */
+	io_res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+	if (io_res == NULL) {
+		io_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+		if (unlikely(io_res == NULL))
+			return -EINVAL;
+	}
+
+	/*
+	 * Then the CTL base
+	 */
+	ctl_res = platform_get_resource(pdev, IORESOURCE_IO, 1);
+	if (ctl_res == NULL) {
+		ctl_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+		if (unlikely(ctl_res == NULL))
+			return -EINVAL;
+	}
+
+	/*
+	 * And the IRQ
+	 */
+	irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (irq_res)
+		irq_res->flags = pp_info ? pp_info->irq_flags : 0;
+
+	return __pata_platform_probe(&pdev->dev, io_res, ctl_res, irq_res,
+				     pp_info ? pp_info->ioport_shift : 0,
+				     pio_mask);
+}
+
+static int __devexit pata_platform_remove(struct platform_device *pdev)
+{
+	return __pata_platform_remove(&pdev->dev);
+}
 
 static struct platform_driver pata_platform_driver = {
 	.probe		= pata_platform_probe,

include/linux/pata_platform.h +9 -0

index 5799e8d..6a7a92d 100644
@@ -15,4 +15,13 @@ struct pata_platform_info {
 	unsigned int irq_flags;
 };
 
+extern int __devinit __pata_platform_probe(struct device *dev,
+					   struct resource *io_res,
+					   struct resource *ctl_res,
+					   struct resource *irq_res,
+					   unsigned int ioport_shift,
+					   int __pio_mask);
+
+extern int __devexit __pata_platform_remove(struct device *dev);
+
 #endif /* __LINUX_PATA_PLATFORM_H */

Signature · 1 line

1.5.2.2

Olof Johansson

Acked-by on v2 1/4

2007-12-04 20:36 UTC #2 lore raw

Re: [PATCH v2 1/4] [libata] pata_platform: make probe and remove functions device type neutral

Quoted 7 lines · Anton Vorontsov

Split pata_platform_{probe,remove} into two pieces:
1. pata_platform_{probe,remove} -- platform_device-dependant bits;
2. __ptata_platform_{probe,remove} -- device type neutral bits.
This is done to not duplicate code for the OF-platform driver.
Signed-off-by Anton Vorontsov <[email protected]>

Acked-by Olof Johansson <[email protected]>

Tested with both the old pata_platform and new pata_of_platform drivers on PA Semi Electra (see separate post with pata_of_platform cutover patch).

-Olof

Anton Vorontsov

author

2007-12-05 15:33 UTC #3 lore raw

Re: [PATCH v2 1/4] [libata] pata_platform: make probe and remove functions device type neutral

In reply to Anton Vorontsov

Quoted 5 lines · Anton Vorontsov

Split pata_platform_{probe,remove} into two pieces:
1. pata_platform_{probe,remove} -- platform_device-dependant bits;
2. __ptata_platform_{probe,remove} -- device type neutral bits.
This is done to not duplicate code for the OF-platform driver.

Paul, do you have any objections regarding that one?

If there is no objections, could you stamp Acked-by on it? Thus I can resend this set one more time for the merge, w/o fourth patch.

Quoted 9 lines

/**
- * pata_platform_probe - attach a platform interface
- * @pdev: platform device
+ * __pata_platform_probe - attach a platform interface
+ * @dev: device
+ * @io_res: Resource representing I/O base
+ * @ctl_res: Resource representing CTL base
+ * @irq_res: Resource representing IRQ and its flags
+ * @ioport_shift: I/O port shift

+ * @__pio_mask: PIO mask

^^^ added

Thanks,

Signature · 4 lines

Anton Vorontsov
email: [email protected]
backup email: [email protected]
irc://irc.freenode.net/bd2

Anton Vorontsov

author

2007-12-04 17:03 UTC #4 lore raw

PATCH v2 2/4 pata_of_platform: OF-Platform PATA device driver

In reply to Anton Vorontsov

This driver nicely wraps around pata_platform library functions, and provides OF platform bus bindings to the PATA devices.

Signed-off-by Anton Vorontsov <[email protected]>

---
 drivers/ata/Kconfig            |   10 ++++
 drivers/ata/Makefile           |    1 +
 drivers/ata/pata_of_platform.c |  102 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+), 0 deletions(-)
 create mode 100644 drivers/ata/pata_of_platform.c

drivers/ata/Kconfig +10 -0

index ba63619..5a492fa 100644
@@ -614,6 +614,16 @@ config PATA_PLATFORM
 
 	  If unsure, say N.
 
+config PATA_OF_PLATFORM
+	tristate "OpenFirmware platform device PATA support"
+	depends on PATA_PLATFORM && PPC_OF
+	help
+	  This option enables support for generic directly connected ATA
+	  devices commonly found on embedded systems with OpenFirmware
+	  bindings.
+
+	  If unsure, say N.
+
 config PATA_ICSIDE
 	tristate "Acorn ICS PATA support"
 	depends on ARM && ARCH_ACORN

drivers/ata/Makefile +1 -0

index b13feb2..ebcee64 100644
@@ -67,6 +67,7 @@ obj-$(CONFIG_PATA_IXP4XX_CF)	+= pata_ixp4xx_cf.o
 obj-$(CONFIG_PATA_SCC)		+= pata_scc.o
 obj-$(CONFIG_PATA_BF54X)	+= pata_bf54x.o
 obj-$(CONFIG_PATA_PLATFORM)	+= pata_platform.o
+obj-$(CONFIG_PATA_OF_PLATFORM)	+= pata_of_platform.o
 obj-$(CONFIG_PATA_ICSIDE)	+= pata_icside.o
 # Should be last but two libata driver
 obj-$(CONFIG_PATA_ACPI)		+= pata_acpi.o

drivers/ata/pata_of_platform.c new file +102 -0

102 lines not on this page · raw message

Signature · 1 line

1.5.2.2

Olof Johansson

2007-12-04 18:44 UTC #5 lore raw

Re: [PATCH v2 2/4] [libata] pata_of_platform: OF-Platform PATA device driver

Hi,

Quoted 19 lines · Anton Vorontsov

This driver nicely wraps around pata_platform library functions, and provides OF platform bus bindings to the PATA devices.
Signed-off-by Anton Vorontsov <[email protected]>
---
 drivers/ata/Kconfig            |   10 ++++
 drivers/ata/Makefile           |    1 +
 drivers/ata/pata_of_platform.c |  102 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+), 0 deletions(-)
 create mode 100644 drivers/ata/pata_of_platform.c
drivers/ata/Kconfig +0 -0
index ba63619..5a492fa 100644
@@ -614,6 +614,16 @@ config PATA_PLATFORM
 
 	  If unsure, say N.
 

drivers/ata/Kconfig config PATA_PLATFORM · line 626

Quoted 10 lines

drivers/ata/Kconfig +10 -0

+config PATA_OF_PLATFORM
+	tristate "OpenFirmware platform device PATA support"
+	depends on PATA_PLATFORM && PPC_OF
+	help
+	  This option enables support for generic directly connected ATA
+	  devices commonly found on embedded systems with OpenFirmware
+	  bindings.
+
+	  If unsure, say N.
+

There's a typo in the dependencies for PATA_PLATFORM that you should change:

depends on EMBEDDED || ARCH_RPC

(note ARCH_>R<PC). With my font it's hard to tell a difference :)

It should really be:

depends on EMBEDDED || PPC

Care to change that while you're at it?

drivers/ata/Kconfig config PATA_PLATFORM · line 629

Quoted 3 lines

drivers/ata/Kconfig +3 -0

+static struct of_device_id pata_of_platform_match[] = {
+	{ .compatible = "ata-generic", },
+};

Needs to be terminated by empty entry, and please add:
MODULE_DEVICE_TABLE(of, pata_of_platform_match);

-Olof

Anton Vorontsov

author

2007-12-04 19:45 UTC #6 lore raw

Re: [PATCH v2 2/4] [libata] pata_of_platform: OF-Platform PATA device driver

Quoted 4 lines · Olof Johansson

Hi,
Quoted 1 line · Anton Vorontsov
This driver nicely wraps around pata_platform library functions,

[..]

Quoted 11 lines

There's a typo in the dependencies for PATA_PLATFORM that you should change:
depends on EMBEDDED || ARCH_RPC
(note ARCH_>R<PC). With my font it's hard to tell a difference :)
It should really be:
depends on EMBEDDED || PPC
Care to change that while you're at it?

Yup.

Quoted 6 lines

Review
Quoted 4 lines
(quoted diff) +3 -0
+static struct of_device_id pata_of_platform_match[] = {
+	{ .compatible = "ata-generic", },
+};
Needs to be terminated by empty entry,

Fixed.

Quoted 2 lines

and please add:
MODULE_DEVICE_TABLE(of, pata_of_platform_match);

Ugh. I forgot about device table. Done.

Much thanks for spotting these.

- - - -
From: Anton Vorontsov <[email protected]>
Subject: [PATCH v2.1] [libata] pata_of_platform: OF-Platform PATA device driver

This driver nicely wraps around pata_platform library functions, and provides OF platform bus bindings to the PATA devices.

In addition fix ARCH_RPC typo in the PATA_PLATFORM Kconfig entry, spotted by Olof Johansson.

Signed-off-by Anton Vorontsov <[email protected]>

---
 drivers/ata/Kconfig            |   12 ++++-
 drivers/ata/Makefile           |    1 +
 drivers/ata/pata_of_platform.c |  104 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 116 insertions(+), 1 deletions(-)
 create mode 100644 drivers/ata/pata_of_platform.c

drivers/ata/Kconfig +11 -1

index ba63619..e067112 100644
@@ -607,13 +607,23 @@ config PATA_WINBOND_VLB
 
 config PATA_PLATFORM
 	tristate "Generic platform device PATA support"
-	depends on EMBEDDED || ARCH_RPC
+	depends on EMBEDDED || ARCH_PPC
 	help
 	  This option enables support for generic directly connected ATA
 	  devices commonly found on embedded systems.
 
 	  If unsure, say N.
 
+config PATA_OF_PLATFORM
+	tristate "OpenFirmware platform device PATA support"
+	depends on PATA_PLATFORM && PPC_OF
+	help
+	  This option enables support for generic directly connected ATA
+	  devices commonly found on embedded systems with OpenFirmware
+	  bindings.
+
+	  If unsure, say N.
+
 config PATA_ICSIDE
 	tristate "Acorn ICS PATA support"
 	depends on ARM && ARCH_ACORN

drivers/ata/Makefile +1 -0

index b13feb2..ebcee64 100644
@@ -67,6 +67,7 @@ obj-$(CONFIG_PATA_IXP4XX_CF)	+= pata_ixp4xx_cf.o
 obj-$(CONFIG_PATA_SCC)		+= pata_scc.o
 obj-$(CONFIG_PATA_BF54X)	+= pata_bf54x.o
 obj-$(CONFIG_PATA_PLATFORM)	+= pata_platform.o
+obj-$(CONFIG_PATA_OF_PLATFORM)	+= pata_of_platform.o
 obj-$(CONFIG_PATA_ICSIDE)	+= pata_icside.o
 # Should be last but two libata driver
 obj-$(CONFIG_PATA_ACPI)		+= pata_acpi.o

drivers/ata/pata_of_platform.c new file +104 -0

104 lines not on this page · raw message

Signature · 1 line

1.5.2.2

Olof Johansson

2007-12-04 19:57 UTC #7 lore raw

Re: [PATCH v2 2/4] [libata] pata_of_platform: OF-Platform PATA device driver

drivers/ata/Kconfig config PATA_WINBOND_VLB · line 610

Quoted 3 lines · Anton Vorontsov

drivers/ata/Kconfig +1 -1

 	tristate "Generic platform device PATA support"
-	depends on EMBEDDED || ARCH_RPC
+	depends on EMBEDDED || ARCH_PPC

It needs to be || PPC, not || ARCH_PPC.

-Olof

Anton Vorontsov

author

2007-12-04 20:33 UTC #8 lore raw

Re: [PATCH v2 2/4] [libata] pata_of_platform: OF-Platform PATA device driver

Quoted 6 lines · Olof Johansson

Review
Quoted 3 lines · Anton Vorontsov
(quoted diff) +1 -1
 	tristate "Generic platform device PATA support"
-	depends on EMBEDDED || ARCH_RPC
+	depends on EMBEDDED || ARCH_PPC
It needs to be || PPC, not || ARCH_PPC.

D'oh.

- - - -
From: Anton Vorontsov <[email protected]>
Subject: [PATCH v2.2] [libata] pata_of_platform: OF-Platform PATA device driver

This driver nicely wraps around pata_platform library functions, and provides OF platform bus bindings to the PATA devices.

In addition fix ARCH_RPC typo in the PATA_PLATFORM Kconfig entry, spotted by Olof Johansson.

Signed-off-by Anton Vorontsov <[email protected]>

---
 drivers/ata/Kconfig            |   12 ++++-
 drivers/ata/Makefile           |    1 +
 drivers/ata/pata_of_platform.c |  104 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 116 insertions(+), 1 deletions(-)
 create mode 100644 drivers/ata/pata_of_platform.c

drivers/ata/Kconfig +11 -1

index ba63619..299ff1f 100644
@@ -607,13 +607,23 @@ config PATA_WINBOND_VLB
 
 config PATA_PLATFORM
 	tristate "Generic platform device PATA support"
-	depends on EMBEDDED || ARCH_RPC
+	depends on EMBEDDED || PPC
 	help
 	  This option enables support for generic directly connected ATA
 	  devices commonly found on embedded systems.
 
 	  If unsure, say N.
 
+config PATA_OF_PLATFORM
+	tristate "OpenFirmware platform device PATA support"
+	depends on PATA_PLATFORM && PPC_OF
+	help
+	  This option enables support for generic directly connected ATA
+	  devices commonly found on embedded systems with OpenFirmware
+	  bindings.
+
+	  If unsure, say N.
+
 config PATA_ICSIDE
 	tristate "Acorn ICS PATA support"
 	depends on ARM && ARCH_ACORN

drivers/ata/Makefile +1 -0

index b13feb2..ebcee64 100644
@@ -67,6 +67,7 @@ obj-$(CONFIG_PATA_IXP4XX_CF)	+= pata_ixp4xx_cf.o
 obj-$(CONFIG_PATA_SCC)		+= pata_scc.o
 obj-$(CONFIG_PATA_BF54X)	+= pata_bf54x.o
 obj-$(CONFIG_PATA_PLATFORM)	+= pata_platform.o
+obj-$(CONFIG_PATA_OF_PLATFORM)	+= pata_of_platform.o
 obj-$(CONFIG_PATA_ICSIDE)	+= pata_icside.o
 # Should be last but two libata driver
 obj-$(CONFIG_PATA_ACPI)		+= pata_acpi.o

drivers/ata/pata_of_platform.c new file +104 -0

104 lines not on this page · raw message

Signature · 1 line

1.5.2.2

Olof Johansson

Reviewed-by

2007-12-04 20:35 UTC #9 lore raw

Re: [PATCH v2 2/4] [libata] pata_of_platform: OF-Platform PATA device driver

Quoted 14 lines · Anton Vorontsov

Quoted 6 lines · Olof Johansson
drivers/ata/Kconfig config PATA_WINBOND_VLB
Quoted 3 lines · Anton Vorontsov
drivers/ata/Kconfig +1 -1
 	tristate "Generic platform device PATA support"
-	depends on EMBEDDED || ARCH_RPC
+	depends on EMBEDDED || ARCH_PPC
It needs to be || PPC, not || ARCH_PPC.
D'oh.
- - - -
From: Anton Vorontsov <[email protected]>
Subject: [PATCH v2.2] [libata] pata_of_platform: OF-Platform PATA device driver

Quoted 7 lines

This driver nicely wraps around pata_platform library functions, and provides OF platform bus bindings to the PATA devices.
In addition fix ARCH_RPC typo in the PATA_PLATFORM Kconfig entry, spotted by Olof Johansson.
Signed-off-by Anton Vorontsov <[email protected]>

Reviewed-by Olof Johansson <[email protected]>

Paul Mundt

2007-12-05 00:49 UTC #10 lore raw

Re: [PATCH v2 2/4] [libata] pata_of_platform: OF-Platform PATA device driver

In reply to Olof Johansson

Quoted 7 lines · Olof Johansson

drivers/ata/Kconfig config PATA_WINBOND_VLB · line 610
Quoted 3 lines · Anton Vorontsov
drivers/ata/Kconfig +1 -1
 	tristate "Generic platform device PATA support"
-	depends on EMBEDDED || ARCH_RPC
+	depends on EMBEDDED || ARCH_PPC
It needs to be || PPC, not || ARCH_PPC.

Wrong. It needs to be EMBEDDED || ARCH_RPC || PPC.

ARCH_RPC is not a typo, it's an ARM platform. Please grep first :-)

Olof Johansson

2007-12-05 03:22 UTC #11 lore raw

Re: [PATCH v2 2/4] [libata] pata_of_platform: OF-Platform PATA device driver

Quoted 11 lines · Paul Mundt

Quoted 7 lines · Olof Johansson
drivers/ata/Kconfig config PATA_WINBOND_VLB · line 610
Quoted 3 lines · Anton Vorontsov
drivers/ata/Kconfig +1 -1
 	tristate "Generic platform device PATA support"
-	depends on EMBEDDED || ARCH_RPC
+	depends on EMBEDDED || ARCH_PPC
It needs to be || PPC, not || ARCH_PPC.
Wrong. It needs to be EMBEDDED || ARCH_RPC || PPC.
ARCH_RPC is not a typo, it's an ARM platform. Please grep first :-)

I'm sorry, but seeing ARCH_RPC and not having an arch/rpc made me suspect it being a typo. It surprises me that the ARM guys chose such a generic prefix as ARCH_ for their specific platforms. (powerpc uses PPC_<platform>).

Anyway, thanks for catching it.

-Olof

Scott Wood

2007-12-05 18:39 UTC #12 lore raw

Re: [PATCH v2 2/4] [libata] pata_of_platform: OF-Platform PATA device driver

In reply to Paul Mundt

Quoted 9 lines · Paul Mundt

Quoted 7 lines · Olof Johansson
drivers/ata/Kconfig config PATA_WINBOND_VLB · line 610
Quoted 3 lines · Anton Vorontsov
drivers/ata/Kconfig +1 -1
 	tristate "Generic platform device PATA support"
-	depends on EMBEDDED || ARCH_RPC
+	depends on EMBEDDED || ARCH_PPC
It needs to be || PPC, not || ARCH_PPC.
Wrong. It needs to be EMBEDDED || ARCH_RPC || PPC.

Why is it dependent on anything other than platform bus support and ATA?

-Scott

Olof Johansson

2007-12-14 08:17 UTC #13 lore raw

Re: [PATCH v2 2/4] [libata] pata_of_platform: OF-Platform PATA device driver

Quoted 9 lines · Scott Wood

Quoted 8 lines · Paul Mundt
Quoted 7 lines · Olof Johansson
drivers/ata/Kconfig config PATA_WINBOND_VLB · line 610
Quoted 3 lines · Anton Vorontsov
drivers/ata/Kconfig +1 -1
 	tristate "Generic platform device PATA support"
-	depends on EMBEDDED || ARCH_RPC
+	depends on EMBEDDED || ARCH_PPC
It needs to be || PPC, not || ARCH_PPC.

Quoted 3 lines

Quoted 1 line
Wrong. It needs to be EMBEDDED || ARCH_RPC || PPC.
Why is it dependent on anything other than platform bus support and ATA?

There's no way to specify dependency on platform bus as a config option, that's likely what EMBEDDED was meant to do. Some platforms have platform bus in spite of being !EMBEDDED, arch/arm/configs/rpc_defconfig seems to be among those. PPC too.

-Olof

Scott Wood

2007-12-14 16:10 UTC #14 lore raw

Re: [PATCH v2 2/4] [libata] pata_of_platform: OF-Platform PATA device driver

Quoted 7 lines · Olof Johansson

Quoted 1 line · Scott Wood
Why is it dependent on anything other than platform bus support and ATA?
There's no way to specify dependency on platform bus as a config option, that's likely what EMBEDDED was meant to do. Some platforms have platform bus in spite of being !EMBEDDED, arch/arm/configs/rpc_defconfig seems to be among those. PPC too.

If platform bus can't be configged out, then the only dependency should be the implicit one on ATA (is there anything else whose absence will stop the code from building, or working if the appropriate platform device is passed in?).

-Scott

Olof Johansson

2007-12-14 16:27 UTC #15 lore raw

Re: [PATCH v2 2/4] [libata] pata_of_platform: OF-Platform PATA device driver

Quoted 9 lines · Scott Wood

Quoted 7 lines · Olof Johansson
Quoted 1 line · Scott Wood
Why is it dependent on anything other than platform bus support and ATA?
There's no way to specify dependency on platform bus as a config option, that's likely what EMBEDDED was meant to do. Some platforms have platform bus in spite of being !EMBEDDED, arch/arm/configs/rpc_defconfig seems to be among those. PPC too.

Quoted 4 lines

If platform bus can't be configged out, then the only dependency should be the implicit one on ATA (is there anything else whose absence will stop the code from building, or working if the appropriate platform device is passed in?).

Sure, sounds good to me.

-Olof

Anton Vorontsov

author

2007-12-04 17:03 UTC #16 lore raw

PATCH v2 3/4 MPC8349E-mITX: introduce localbus and pata nodes

In reply to Anton Vorontsov

This patch adds localbus and pata nodes to use CF IDE interface on MPC8349E-mITX boards.

Patch also adds code to probe localbus.

Signed-off-by Anton Vorontsov <[email protected]>

---
 arch/powerpc/boot/dts/mpc8349emitx.dts    |   18 +++++++++++++++++-
 arch/powerpc/platforms/83xx/mpc834x_itx.c |   17 +++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletions(-)

arch/powerpc/boot/dts/mpc8349emitx.dts +17 -1

index 5072f6d..c459b0a 100644
@@ -249,6 +249,22 @@
 		device_type = "pci";
 	};
 
+	localbus@e0005000 {
+		#address-cells = <2>;
+		#size-cells = <1>;
+		compatible = "fsl,mpc8349emitx-localbus",
+			     "fsl,mpc8349e-localbus",
+			     "fsl,pq2pro-localbus";
+		reg = <e0005000 d8>;
+		ranges = <3 0 f0000000 210>;
 
-
+		pata@3,0 {
+			compatible = "fsl,mpc8349emitx-pata", "ata-generic";
+			reg = <3 0 10 3 20c 4>;
+			reg-shift = <1>;
+			pio-mode = <6>;
+			interrupts = <17 8>;
+			interrupt-parent = <&ipic>;
+		};
+	};
 };

arch/powerpc/platforms/83xx/mpc834x_itx.c +17 -0

index aa76819..ea5f176 100644
@@ -23,6 +23,7 @@
 #include <linux/delay.h>
 #include <linux/seq_file.h>
 #include <linux/root_dev.h>
+#include <linux/of_platform.h>
 
 #include <asm/system.h>
 #include <asm/atomic.h>

@@ -37,6 +38,22 @@
 
 #include "mpc83xx.h"
 
+static struct of_device_id mpc834x_itx_ids[] = {
+	{ .name = "localbus", },
+	{},
+};
+
+static int __init mpc834x_itx_declare_of_platform_devices(void)
+{
+	if (!machine_is(mpc834x_itx))
+		return 0;
+
+	of_platform_bus_probe(NULL, mpc834x_itx_ids, NULL);
+
+	return 0;
+}
+device_initcall(mpc834x_itx_declare_of_platform_devices);
+
 /* ************************************************************************
  *
  * Setup the architecture

Signature · 1 line

1.5.2.2

Olof Johansson

2007-12-04 19:12 UTC #17 lore raw

Re: [PATCH v2 3/4] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes

Hi,

Quoted 9 lines · Anton Vorontsov

arch/powerpc/boot/dts/mpc8349emitx.dts +1 -0

index 5072f6d..c459b0a 100644
@@ -249,6 +249,22 @@
 		device_type = "pci";
 	};
 
+	localbus@e0005000 {

arch/powerpc/boot/dts/mpc8349emitx.dts line 261

Quoted 10 lines

arch/powerpc/boot/dts/mpc8349emitx.dts +8 -1

+		#address-cells = <2>;
+		#size-cells = <1>;
+		compatible = "fsl,mpc8349emitx-localbus",
+			     "fsl,mpc8349e-localbus",
+			     "fsl,pq2pro-localbus";
+		reg = <e0005000 d8>;
+		ranges = <3 0 f0000000 210>;
 
-
+		pata@3,0 {

What's the ,0 for?

Quoted 22 lines

arch/powerpc/boot/dts/mpc8349emitx.dts +8 -0

+			compatible = "fsl,mpc8349emitx-pata", "ata-generic";
+			reg = <3 0 10 3 20c 4>;
+			reg-shift = <1>;
+			pio-mode = <6>;
+			interrupts = <17 8>;
+			interrupt-parent = <&ipic>;
+		};
+	};
 };

arch/powerpc/platforms/83xx/mpc834x_itx.c +1 -0

index aa76819..ea5f176 100644
@@ -23,6 +23,7 @@
 #include <linux/delay.h>
 #include <linux/seq_file.h>
 #include <linux/root_dev.h>
+#include <linux/of_platform.h>
 
 #include <asm/system.h>
 #include <asm/atomic.h>

arch/powerpc/platforms/83xx/mpc834x_itx.c line 44

Quoted 7 lines

arch/powerpc/platforms/83xx/mpc834x_itx.c +4 -0

@@ -37,6 +38,22 @@
 
 #include "mpc83xx.h"
 
+static struct of_device_id mpc834x_itx_ids[] = {
+	{ .name = "localbus", },
+	{},
+};

Please add the "compatible" field here (fsl,pq2pro-localbus should do just fine), instead of the name.

(Sorry for not spotting this sooner)

-Olof

Scott Wood

2007-12-04 19:23 UTC #18 lore raw

Re: [PATCH v2 3/4] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes

Quoted 20 lines · Olof Johansson

Hi,

Quoted 17 lines · Anton Vorontsov

arch/powerpc/boot/dts/mpc8349emitx.dts +8 -0

index 5072f6d..c459b0a 100644
@@ -249,6 +249,22 @@
 		device_type = "pci";
 	};
 
+	localbus@e0005000 {
+		#address-cells = <2>;
+		#size-cells = <1>;
+		compatible = "fsl,mpc8349emitx-localbus",
+			     "fsl,mpc8349e-localbus",
+			     "fsl,pq2pro-localbus";
+		reg = <e0005000 d8>;
+		ranges = <3 0 f0000000 210>;

Quoted 4 lines

arch/powerpc/boot/dts/mpc8349emitx.dts line 261
Quoted 2 lines
arch/powerpc/boot/dts/mpc8349emitx.dts +1 -1
-
+		pata@3,0 {
What's the ,0 for?

chipselect 3, offset 0.

-Scott

Anton Vorontsov

author

2007-12-04 19:41 UTC #19 lore raw

Re: [PATCH v2 3/4] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes

In reply to Olof Johansson

Quoted 5 lines · Olof Johansson

Hi,
Quoted 2 lines · Anton Vorontsov
arch/powerpc/boot/dts/mpc8349emitx.dts +0 -0
index 5072f6d..c459b0a 100644

[...]

Quoted 7 lines

arch/powerpc/boot/dts/mpc8349emitx.dts
Quoted 4 lines
arch/powerpc/boot/dts/mpc8349emitx.dts +4 -0
+static struct of_device_id mpc834x_itx_ids[] = {
+	{ .name = "localbus", },
+	{},
+};
Please add the "compatible" field here (fsl,pq2pro-localbus should do just fine), instead of the name.

Done, thanks.

- - - -
From: Anton Vorontsov <[email protected]>
Subject: [PATCH v2.1] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes

This patch adds localbus and pata nodes to use CF IDE interface on MPC8349E-mITX boards.

Patch also adds code to probe localbus.

Signed-off-by Anton Vorontsov <[email protected]>

---
 arch/powerpc/boot/dts/mpc8349emitx.dts    |   18 +++++++++++++++++-
 arch/powerpc/platforms/83xx/mpc834x_itx.c |   17 +++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletions(-)

arch/powerpc/boot/dts/mpc8349emitx.dts +17 -1

index 5072f6d..c459b0a 100644
@@ -249,6 +249,22 @@
 		device_type = "pci";
 	};
 
+	localbus@e0005000 {
+		#address-cells = <2>;
+		#size-cells = <1>;
+		compatible = "fsl,mpc8349emitx-localbus",
+			     "fsl,mpc8349e-localbus",
+			     "fsl,pq2pro-localbus";
+		reg = <e0005000 d8>;
+		ranges = <3 0 f0000000 210>;
 
-
+		pata@3,0 {
+			compatible = "fsl,mpc8349emitx-pata", "ata-generic";
+			reg = <3 0 10 3 20c 4>;
+			reg-shift = <1>;
+			pio-mode = <6>;
+			interrupts = <17 8>;
+			interrupt-parent = <&ipic>;
+		};
+	};
 };

arch/powerpc/platforms/83xx/mpc834x_itx.c +17 -0

index aa76819..4797850 100644
@@ -23,6 +23,7 @@
 #include <linux/delay.h>
 #include <linux/seq_file.h>
 #include <linux/root_dev.h>
+#include <linux/of_platform.h>
 
 #include <asm/system.h>
 #include <asm/atomic.h>

@@ -37,6 +38,22 @@
 
 #include "mpc83xx.h"
 
+static struct of_device_id mpc834x_itx_ids[] = {
+	{ .compatible = "fsl,pq2pro-localbus", },
+	{},
+};
+
+static int __init mpc834x_itx_declare_of_platform_devices(void)
+{
+	if (!machine_is(mpc834x_itx))
+		return 0;
+
+	of_platform_bus_probe(NULL, mpc834x_itx_ids, NULL);
+
+	return 0;
+}
+device_initcall(mpc834x_itx_declare_of_platform_devices);
+
 /* ************************************************************************
  *
  * Setup the architecture

Signature · 1 line

1.5.2.2

Olof Johansson

Acked-by

2007-12-04 20:36 UTC #20 lore raw

Re: [PATCH v2 3/4] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes

Quoted 6 lines · Anton Vorontsov

This patch adds localbus and pata nodes to use CF IDE interface on MPC8349E-mITX boards.
Patch also adds code to probe localbus.
Signed-off-by Anton Vorontsov <[email protected]>

Acked-by Olof Johansson <[email protected]>

-Olof

Anton Vorontsov

author

2007-12-04 17:03 UTC #21 lore raw

PATCH v2 4/4 pata_platform: s/ioport_shift/reg_shift/g

In reply to Anton Vorontsov

This patch renames ioport_shift member of pata_platform_info structure to reg_shift. Users of pata_platform are followed appropriately.

Rationale of that change is: shifting applies to the whole memory mapped region, not only to the command block of the ATA registers, despite the fact that shifting is meaningless for ctl register.

This patch should make Sergei Shtylyov a bit more happy. ;-)

Signed-off-by Anton Vorontsov <[email protected]>

---
 arch/arm/mach-rpc/riscpc.c                      |    2 +-
 arch/blackfin/mach-bf527/boards/ezkit.c         |    2 +-
 arch/blackfin/mach-bf533/boards/cm_bf533.c      |    2 +-
 arch/blackfin/mach-bf533/boards/ezkit.c         |    2 +-
 arch/blackfin/mach-bf533/boards/stamp.c         |    2 +-
 arch/blackfin/mach-bf537/boards/cm_bf537.c      |    2 +-
 arch/blackfin/mach-bf537/boards/generic_board.c |    2 +-
 arch/blackfin/mach-bf537/boards/stamp.c         |    2 +-
 arch/blackfin/mach-bf561/boards/cm_bf561.c      |    2 +-
 arch/blackfin/mach-bf561/boards/ezkit.c         |    2 +-
 arch/sh/boards/landisk/setup.c                  |    2 +-
 arch/sh/boards/renesas/r7780rp/setup.c          |    2 +-
 arch/sh/boards/renesas/rts7751r2d/setup.c       |    2 +-
 drivers/ata/pata_platform.c                     |    8 ++++----
 drivers/ide/legacy/ide_platform.c               |    4 ++--
 include/linux/pata_platform.h                   |    4 ++--
 16 files changed, 21 insertions(+), 21 deletions(-)

arch/arm/mach-rpc/riscpc.c +1 -1

index a454451..be413c1 100644
@@ -161,7 +161,7 @@ static struct platform_device serial_device = {
 };
 
 static struct pata_platform_info pata_platform_data = {
-	.ioport_shift		= 2,
+	.reg_shift		= 2,
 };
 
 static struct resource pata_resources[] = {

arch/blackfin/mach-bf527/boards/ezkit.c +1 -1

index 003e2ac..c9b20bd 100644
@@ -685,7 +685,7 @@ static struct platform_device bfin_sport1_uart_device = {
 #define PATA_INT	55
 
 static struct pata_platform_info bfin_pata_platform_data = {
-	.ioport_shift = 1,
+	.reg_shift = 1,
 	.irq_type = IRQF_TRIGGER_HIGH | IRQF_DISABLED,
 };

arch/blackfin/mach-bf533/boards/cm_bf533.c +1 -1

index 21df2f3..6cc697d 100644
@@ -289,7 +289,7 @@ static struct platform_device isp1362_hcd_device = {
 #define PATA_INT	38
 
 static struct pata_platform_info bfin_pata_platform_data = {
-	.ioport_shift = 2,
+	.reg_shift = 2,
 	.irq_type = IRQF_TRIGGER_HIGH | IRQF_DISABLED,
 };

arch/blackfin/mach-bf533/boards/ezkit.c +1 -1

index be85203..030dc64 100644
@@ -223,7 +223,7 @@ static struct platform_device bfin_uart_device = {
 #define PATA_INT	55
 
 static struct pata_platform_info bfin_pata_platform_data = {
-	.ioport_shift = 1,
+	.reg_shift = 1,
 	.irq_type = IRQF_TRIGGER_HIGH | IRQF_DISABLED,
 };

arch/blackfin/mach-bf533/boards/stamp.c +1 -1

index 8fde8d8..6ff0b88 100644
@@ -340,7 +340,7 @@ static struct platform_device bfin_sport1_uart_device = {
 #define PATA_INT	55
 
 static struct pata_platform_info bfin_pata_platform_data = {
-	.ioport_shift = 1,
+	.reg_shift = 1,
 	.irq_type = IRQF_TRIGGER_HIGH | IRQF_DISABLED,
 };

arch/blackfin/mach-bf537/boards/cm_bf537.c +1 -1

index c0fb06d..031da0f 100644
@@ -340,7 +340,7 @@ static struct platform_device bfin_mac_device = {
 #define PATA_INT	64
 
 static struct pata_platform_info bfin_pata_platform_data = {
-	.ioport_shift = 2,
+	.reg_shift = 2,
 	.irq_type = IRQF_TRIGGER_HIGH | IRQF_DISABLED,
 };

arch/blackfin/mach-bf537/boards/generic_board.c +1 -1

index 09f4bfb..e217eb2 100644
@@ -613,7 +613,7 @@ static struct platform_device bfin_sport1_uart_device = {
 #define PATA_INT	55
 
 static struct pata_platform_info bfin_pata_platform_data = {
-	.ioport_shift = 1,
+	.reg_shift = 1,
 	.irq_type = IRQF_TRIGGER_HIGH | IRQF_DISABLED,
 };

arch/blackfin/mach-bf537/boards/stamp.c +1 -1

index 07b0dc2..daeb0d5 100644
@@ -640,7 +640,7 @@ static struct platform_device bfin_sport1_uart_device = {
 #define PATA_INT	55
 
 static struct pata_platform_info bfin_pata_platform_data = {
-	.ioport_shift = 1,
+	.reg_shift = 1,
 	.irq_flags = IRQF_TRIGGER_HIGH | IRQF_DISABLED,
 };

arch/blackfin/mach-bf561/boards/cm_bf561.c +1 -1

index c19cd29..5628e34 100644
@@ -280,7 +280,7 @@ static struct platform_device bfin_uart_device = {
 #define PATA_INT	119
 
 static struct pata_platform_info bfin_pata_platform_data = {
-	.ioport_shift = 2,
+	.reg_shift = 2,
 	.irq_type = IRQF_TRIGGER_HIGH | IRQF_DISABLED,
 };

arch/blackfin/mach-bf561/boards/ezkit.c +1 -1

index 4ff8f6e..6894472 100644
@@ -213,7 +213,7 @@ static struct spi_board_info bfin_spi_board_info[] __initdata = {
 #define PATA_INT	55
 
 static struct pata_platform_info bfin_pata_platform_data = {
-	.ioport_shift = 1,
+	.reg_shift = 1,
 	.irq_type = IRQF_TRIGGER_HIGH | IRQF_DISABLED,
 };

arch/sh/boards/landisk/setup.c +1 -1

index eda7176..46330f7 100644
@@ -31,7 +31,7 @@ static void landisk_power_off(void)
 static struct resource cf_ide_resources[3];
 
 static struct pata_platform_info pata_info = {
-	.ioport_shift	= 1,
+	.reg_shift	= 1,
 };
 
 static struct platform_device cf_ide_device = {

arch/sh/boards/renesas/r7780rp/setup.c +1 -1

index 0fdc0bc..1fc9c9b 100644
@@ -94,7 +94,7 @@ static struct resource cf_ide_resources[] = {
 };
 
 static struct pata_platform_info pata_info = {
-	.ioport_shift	= 1,
+	.reg_shift	= 1,
 };
 
 static struct platform_device cf_ide_device  = {

arch/sh/boards/renesas/rts7751r2d/setup.c +1 -1

index 8125d20..8bcca50 100644
@@ -62,7 +62,7 @@ static struct resource cf_ide_resources[] = {
 };
 
 static struct pata_platform_info pata_info = {
-	.ioport_shift	= 1,
+	.reg_shift	= 1,
 };
 
 static struct platform_device cf_ide_device  = {

drivers/ata/pata_platform.c +4 -4

index d4cb231..3e7d2eb 100644
@@ -114,7 +114,7 @@ static void pata_platform_setup_port(struct ata_ioports *ioaddr,
  *	@io_res: Resource representing I/O base
  *	@ctl_res: Resource representing CTL base
  *	@irq_res: Resource representing IRQ and its flags
- *	@ioport_shift: I/O port shift
+ *	@reg_shift: I/O port shift
  *
  *	Register a platform bus IDE interface. Such interfaces are PIO and we
  *	assume do not support IRQ sharing.

@@ -138,7 +138,7 @@ int __devinit __pata_platform_probe(struct device *dev,
 				    struct resource *io_res,
 				    struct resource *ctl_res,
 				    struct resource *irq_res,
-				    unsigned int ioport_shift,
+				    unsigned int reg_shift,
 				    int __pio_mask)
 {
 	struct ata_host *host;

@@ -202,7 +202,7 @@ int __devinit __pata_platform_probe(struct device *dev,
 
 	ap->ioaddr.altstatus_addr = ap->ioaddr.ctl_addr;
 
-	pata_platform_setup_port(&ap->ioaddr, ioport_shift);
+	pata_platform_setup_port(&ap->ioaddr, reg_shift);
 
 	ata_port_desc(ap, "%s cmd 0x%llx ctl 0x%llx", mmio ? "mmio" : "ioport",
 		      (unsigned long long)io_res->start,

@@ -274,7 +274,7 @@ static int __devinit pata_platform_probe(struct platform_device *pdev)
 		irq_res->flags = pp_info ? pp_info->irq_flags : 0;
 
 	return __pata_platform_probe(&pdev->dev, io_res, ctl_res, irq_res,
-				     pp_info ? pp_info->ioport_shift : 0,
+				     pp_info ? pp_info->reg_shift : 0,
 				     pio_mask);
 }

drivers/ide/legacy/ide_platform.c +2 -2

index 7bb79f5..3cee1b2 100644
@@ -41,9 +41,9 @@ static ide_hwif_t *__devinit plat_ide_locate_hwif(void __iomem *base,
 
 	hwif->io_ports[IDE_DATA_OFFSET] = port;
 
-	port += (1 << pdata->ioport_shift);
+	port += (1 << pdata->reg_shift);
 	for (i = IDE_ERROR_OFFSET; i <= IDE_STATUS_OFFSET;
-	     i++, port += (1 << pdata->ioport_shift))
+	     i++, port += (1 << pdata->reg_shift))
 		hwif->io_ports[i] = port;
 
 	hwif->io_ports[IDE_CONTROL_OFFSET] = (unsigned long)ctrl;

include/linux/pata_platform.h +2 -2

index 6a7a92d..a0fc6b0 100644
@@ -7,7 +7,7 @@ struct pata_platform_info {
 	 * constantly spaced and need larger than the 1-byte
 	 * spacing used by ata_std_ports().
 	 */
-	unsigned int ioport_shift;
+	unsigned int reg_shift;
 	/* 
 	 * Indicate platform specific irq types and initial
 	 * IRQ flags when call request_irq()

@@ -19,7 +19,7 @@ extern int __devinit __pata_platform_probe(struct device *dev,
 					   struct resource *io_res,
 					   struct resource *ctl_res,
 					   struct resource *irq_res,
-					   unsigned int ioport_shift,
+					   unsigned int reg_shift,
 					   int __pio_mask);
 
 extern int __devexit __pata_platform_remove(struct device *dev);

Signature · 1 line

1.5.2.2

Sergei Shtylyov

2007-12-04 17:07 UTC #22 lore raw

Re: [PATCH v2 4/4] [libata] pata_platform: s/ioport_shift/reg_shift/g

Hello.

Quoted 9 lines · Anton Vorontsov

This patch renames ioport_shift member of pata_platform_info structure to reg_shift. Users of pata_platform are followed appropriately.
Rationale of that change is: shifting applies to the whole memory mapped region, not only to the command block of the ATA registers, despite the fact that shifting is meaningless for ctl register.
This patch should make Sergei Shtylyov a bit more happy. ;-)

In fact, I don't care about how platform driver calls this stuff. ;-)
I only care about the property name.

MBR, Sergei

Paul Mundt

Nacked-by on v2 4/4 NAK

2007-12-05 00:57 UTC #23 lore raw

Re: [PATCH v2 4/4] [libata] pata_platform: s/ioport_shift/reg_shift/g

In reply to Anton Vorontsov

Quoted 8 lines · Anton Vorontsov

This patch renames ioport_shift member of pata_platform_info structure to reg_shift. Users of pata_platform are followed appropriately.
Rationale of that change is: shifting applies to the whole memory mapped region, not only to the command block of the ATA registers, despite the fact that shifting is meaningless for ctl register.

It's called ioport_shift because of the fact it shifts an ioport, namely a struct ata_ioport *. We could rename it, but I really don't see the point. If you don't like the choice of name on your platform, add a comment to your platform-specific code noting this particular outrage so it can be grepped for by future generations ;-)

Nacked-by Paul Mundt <[email protected]>

Olof Johansson

2007-12-04 20:40 UTC #24 lore raw

PATCH 1/1 pata_of_platform: Move electra-ide support over to new framework

In reply to Anton Vorontsov

[POWERPC] Move electra-ide support over to new pata_of_platform framework

Move electra-ide glue over to the new pata_of_platform framework, and add the quirks needed to that driver.

Signed-off-by Olof Johansson <[email protected]>

---

I'll remove the electra-ide stuff from arch/powerpc/platforms/pasemi once this hits a common tree, since otherwise I'd be without IDE until they converge (i.e. 2.6.25 merge window).

-Olof

drivers/ata/pata_of_platform.c +15 -5

index 4daf118..3e9675a 100644
@@ -34,11 +34,20 @@ static int __devinit pata_of_platform_probe(struct of_device *ofdev,
 		return -EINVAL;
 	}
 
-	ret = of_address_to_resource(dn, 1, &ctl_res);
-	if (ret) {
-		dev_err(&ofdev->dev, "can't get CTL address from "
-			"device tree\n");
-		return -EINVAL;
+	if (of_device_is_compatible(dn, "electra-ide")) {
+		/* Altstatus is really at offset 0x3f6 from the primary window
+		 * on electra-ide. Adjust ctl_res and io_res accordingly.
+		 */
+		ctl_res = io_res;
+		ctl_res.start = ctl_res.start+0x3f6;
+		io_res.end = ctl_res.start-1;
+	} else {
+		ret = of_address_to_resource(dn, 1, &ctl_res);
+		if (ret) {
+			dev_err(&ofdev->dev, "can't get CTL address from "
+				"device tree\n");
+			return -EINVAL;
+		}
 	}
 
 	ret = of_irq_to_resource(dn, 0, &irq_res);

@@ -76,6 +85,7 @@ static int __devexit pata_of_platform_remove(struct of_device *ofdev)
 
 static struct of_device_id pata_of_platform_match[] = {
 	{ .compatible = "ata-generic", },
+	{ .compatible = "electra-ide", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, pata_of_platform_match);

Jeff Garzik

2007-12-04 20:50 UTC #25 lore raw

Re: [PATCH] pata_of_platform: Move electra-ide support over to new framework

Quoted 6 lines · Olof Johansson

[POWERPC] Move electra-ide support over to new pata_of_platform framework
Move electra-ide glue over to the new pata_of_platform framework, and add the quirks needed to that driver.

Quoted 7 lines

Signed-off-by Olof Johansson <[email protected]>
---
I'll remove the electra-ide stuff from arch/powerpc/platforms/pasemi once this hits a common tree, since otherwise I'd be without IDE until they converge (i.e. 2.6.25 merge window).

FWIW I'm presuming this work will go via a powerpc tree not libata... Generally that's not the case, but here it's largely an arch-specific work.

Jeff

Olof Johansson

2007-12-04 20:59 UTC #26 lore raw

Re: [PATCH] pata_of_platform: Move electra-ide support over to new framework

Quoted 10 lines · Jeff Garzik

Quoted 8 lines · Olof Johansson
[POWERPC] Move electra-ide support over to new pata_of_platform framework Move electra-ide glue over to the new pata_of_platform framework, and add the quirks needed to that driver.
Signed-off-by Olof Johansson <[email protected]>
---
I'll remove the electra-ide stuff from arch/powerpc/platforms/pasemi once this hits a common tree, since otherwise I'd be without IDE until they converge (i.e. 2.6.25 merge window).

Quoted 2 lines

FWIW I'm presuming this work will go via a powerpc tree not libata... Generally that's not the case, but here it's largely an arch-specific work.

Ok, that works. I was thinking of letting this patch go through libata, and do the removal through the powerpc merge path. But I can do both through powerpc.

-Olof

Paul Mackerras

2007-12-20 03:53 UTC #27 lore raw

Re: [PATCH] pata_of_platform: Move electra-ide support over to new framework

Quoted 6 lines · Olof Johansson

Quoted 2 lines
FWIW I'm presuming this work will go via a powerpc tree not libata... Generally that's not the case, but here it's largely an arch-specific work.
Ok, that works. I was thinking of letting this patch go through libata, and do the removal through the powerpc merge path. But I can do both through powerpc.

Are you going to send me a patch to do this addition plus the removal of the old stuff, or do you want me to merge this patch as is?

Paul.

Olof Johansson

2007-12-20 15:26 UTC #28 lore raw

Re: [PATCH] pata_of_platform: Move electra-ide support over to new framework

Quoted 11 lines · Paul Mackerras

Quoted 6 lines · Olof Johansson
Quoted 2 lines
FWIW I'm presuming this work will go via a powerpc tree not libata... Generally that's not the case, but here it's largely an arch-specific work.
Ok, that works. I was thinking of letting this patch go through libata, and do the removal through the powerpc merge path. But I can do both through powerpc.
Are you going to send me a patch to do this addition plus the removal of the old stuff, or do you want me to merge this patch as is?

Yes, I will once the series it depends on has been sorted out and is picked up somewhere, since it depends on it.

-Olof