[PATCH v3 0/3] OF-platform PATA driver

[PATCH v3 0/3] OF-platform PATA driver

[Anton Vorontsov]

Hi all,

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


Changes since v2:
- "PPC" added to PATA_PLATFORM "depends on" Kconfig entry;
  I didn't remove EMBEDDED "depends on" -- this wasn't discussed
  much and these patches should not depend on the decision.
- cosmetic fixes;
- "s/ioport_shift/reg_shift/g" patch dropped.

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.

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

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

[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 <avorontsov@ru.mvista.com>
Acked-by: Olof Johansson <olof@lixom.net>
---
 drivers/ata/pata_platform.c   |  144 ++++++++++++++++++++++++----------------
 include/linux/pata_platform.h |    9 +++
 2 files changed, 95 insertions(+), 58 deletions(-)

diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c
index ac03a90..224bb6c 100644
--- a/drivers/ata/pata_platform.c
+++ b/drivers/ata/pata_platform.c
@@ -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,13 @@ 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
+ *	@__pio_mask: PIO mask
  *
  *	Register a platform bus IDE interface. Such interfaces are PIO and we
  *	assume do not support IRQ sharing.
@@ -135,42 +135,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 +157,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 +186,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 +211,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,
diff --git a/include/linux/pata_platform.h b/include/linux/pata_platform.h
index 5799e8d..6a7a92d 100644
--- a/include/linux/pata_platform.h
+++ b/include/linux/pata_platform.h
@@ -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 */
-- 
1.5.2.2

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

[Paul Mundt]

On Fri, Dec 14, 2007 at 09:24:29PM +0300, Anton Vorontsov wrote:
> 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 <avorontsov@ru.mvista.com>
> Acked-by: Olof Johansson <olof@lixom.net>

Looks fine to me now, thanks for cleaning it up Anton.

Acked-by: Paul Mundt <lethal@linux-sh.org>

[PATCH 2/3] [libata] pata_of_platform: OF-Platform PATA device driver

[Anton Vorontsov]

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

Also add || PPC to the PATA_PLATFORM's "depends on" Kconfig entry,
needed for PA Semi Electra.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Reviewed-by: Olof Johansson <olof@lixom.net>
---
 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

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index ba63619..64b4964 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -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_RPC || 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
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index b13feb2..ebcee64 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -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
diff --git a/drivers/ata/pata_of_platform.c b/drivers/ata/pata_of_platform.c
new file mode 100644
index 0000000..4daf118
--- /dev/null
+++ b/drivers/ata/pata_of_platform.c
@@ -0,0 +1,104 @@
+/*
+ * OF-platform PATA driver
+ *
+ * Copyright (c) 2007  MontaVista Software, Inc.
+ *                     Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/pata_platform.h>
+
+static int __devinit pata_of_platform_probe(struct of_device *ofdev,
+					    const struct of_device_id *match)
+{
+	int ret;
+	struct device_node *dn = ofdev->node;
+	struct resource io_res;
+	struct resource ctl_res;
+	struct resource irq_res;
+	unsigned int reg_shift = 0;
+	int pio_mode = 0;
+	int pio_mask;
+	const u32 *prop;
+
+	ret = of_address_to_resource(dn, 0, &io_res);
+	if (ret) {
+		dev_err(&ofdev->dev, "can't get IO address from "
+			"device tree\n");
+		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;
+	}
+
+	ret = of_irq_to_resource(dn, 0, &irq_res);
+	if (ret == NO_IRQ)
+		irq_res.start = irq_res.end = -1;
+	else
+		irq_res.flags = 0;
+
+	prop = (u32 *)of_get_property(dn, "reg-shift", NULL);
+	if (prop)
+		reg_shift = *prop;
+
+	prop = (u32 *)of_get_property(dn, "pio-mode", NULL);
+	if (prop) {
+		pio_mode = *prop;
+		if (pio_mode > 6) {
+			dev_err(&ofdev->dev, "invalid pio-mode\n");
+			return -EINVAL;
+		}
+	} else {
+		dev_info(&ofdev->dev, "pio-mode unspecified, assuming PIO0\n");
+	}
+
+	pio_mask = 1 << pio_mode;
+	pio_mask |= (1 << pio_mode) - 1;
+
+	return __pata_platform_probe(&ofdev->dev, &io_res, &ctl_res, &irq_res,
+				     reg_shift, pio_mask);
+}
+
+static int __devexit pata_of_platform_remove(struct of_device *ofdev)
+{
+	return __pata_platform_remove(&ofdev->dev);
+}
+
+static struct of_device_id pata_of_platform_match[] = {
+	{ .compatible = "ata-generic", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, pata_of_platform_match);
+
+static struct of_platform_driver pata_of_platform_driver = {
+	.name		= "pata_of_platform",
+	.match_table	= pata_of_platform_match,
+	.probe		= pata_of_platform_probe,
+	.remove		= __devexit_p(pata_of_platform_remove),
+};
+
+static int __init pata_of_platform_init(void)
+{
+	return of_register_platform_driver(&pata_of_platform_driver);
+}
+module_init(pata_of_platform_init);
+
+static void __exit pata_of_platform_exit(void)
+{
+	of_unregister_platform_driver(&pata_of_platform_driver);
+}
+module_exit(pata_of_platform_exit);
+
+MODULE_DESCRIPTION("OF-platform PATA driver");
+MODULE_AUTHOR("Anton Vorontsov <avorontsov@ru.mvista.com>");
+MODULE_LICENSE("GPL");
-- 
1.5.2.2

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

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 410 bytes --]

On Fri, 14 Dec 2007 21:24:38 +0300 Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
>
> +	prop = (u32 *)of_get_property(dn, "reg-shift", NULL);

This cast is unnecessary.

> +	if (prop)
> +		reg_shift = *prop;
> +
> +	prop = (u32 *)of_get_property(dn, "pio-mode", NULL);

So is this one.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

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

[Anton Vorontsov]

On Sun, Dec 16, 2007 at 01:09:22AM +1100, Stephen Rothwell wrote:
> On Fri, 14 Dec 2007 21:24:38 +0300 Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> >
> > +	prop = (u32 *)of_get_property(dn, "reg-shift", NULL);
> 
> This cast is unnecessary.
> 
> > +	if (prop)
> > +		reg_shift = *prop;
> > +
> > +	prop = (u32 *)of_get_property(dn, "pio-mode", NULL);
> 
> So is this one.

Thanks. I changed "prop" to const but obviously forgot to remove casts.

- - - - 
From: Anton Vorontsov <avorontsov@ru.mvista.com>

[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.

Also add || PPC to the PATA_PLATFORM's "depends on" Kconfig entry,
needed for PA Semi Electra.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Reviewed-by: Olof Johansson <olof@lixom.net>
---
 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

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index ba63619..64b4964 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -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_RPC || 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
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index b13feb2..ebcee64 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -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
diff --git a/drivers/ata/pata_of_platform.c b/drivers/ata/pata_of_platform.c
new file mode 100644
index 0000000..b7bc4e4
--- /dev/null
+++ b/drivers/ata/pata_of_platform.c
@@ -0,0 +1,104 @@
+/*
+ * OF-platform PATA driver
+ *
+ * Copyright (c) 2007  MontaVista Software, Inc.
+ *                     Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/pata_platform.h>
+
+static int __devinit pata_of_platform_probe(struct of_device *ofdev,
+					    const struct of_device_id *match)
+{
+	int ret;
+	struct device_node *dn = ofdev->node;
+	struct resource io_res;
+	struct resource ctl_res;
+	struct resource irq_res;
+	unsigned int reg_shift = 0;
+	int pio_mode = 0;
+	int pio_mask;
+	const u32 *prop;
+
+	ret = of_address_to_resource(dn, 0, &io_res);
+	if (ret) {
+		dev_err(&ofdev->dev, "can't get IO address from "
+			"device tree\n");
+		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;
+	}
+
+	ret = of_irq_to_resource(dn, 0, &irq_res);
+	if (ret == NO_IRQ)
+		irq_res.start = irq_res.end = -1;
+	else
+		irq_res.flags = 0;
+
+	prop = of_get_property(dn, "reg-shift", NULL);
+	if (prop)
+		reg_shift = *prop;
+
+	prop = of_get_property(dn, "pio-mode", NULL);
+	if (prop) {
+		pio_mode = *prop;
+		if (pio_mode > 6) {
+			dev_err(&ofdev->dev, "invalid pio-mode\n");
+			return -EINVAL;
+		}
+	} else {
+		dev_info(&ofdev->dev, "pio-mode unspecified, assuming PIO0\n");
+	}
+
+	pio_mask = 1 << pio_mode;
+	pio_mask |= (1 << pio_mode) - 1;
+
+	return __pata_platform_probe(&ofdev->dev, &io_res, &ctl_res, &irq_res,
+				     reg_shift, pio_mask);
+}
+
+static int __devexit pata_of_platform_remove(struct of_device *ofdev)
+{
+	return __pata_platform_remove(&ofdev->dev);
+}
+
+static struct of_device_id pata_of_platform_match[] = {
+	{ .compatible = "ata-generic", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, pata_of_platform_match);
+
+static struct of_platform_driver pata_of_platform_driver = {
+	.name		= "pata_of_platform",
+	.match_table	= pata_of_platform_match,
+	.probe		= pata_of_platform_probe,
+	.remove		= __devexit_p(pata_of_platform_remove),
+};
+
+static int __init pata_of_platform_init(void)
+{
+	return of_register_platform_driver(&pata_of_platform_driver);
+}
+module_init(pata_of_platform_init);
+
+static void __exit pata_of_platform_exit(void)
+{
+	of_unregister_platform_driver(&pata_of_platform_driver);
+}
+module_exit(pata_of_platform_exit);
+
+MODULE_DESCRIPTION("OF-platform PATA driver");
+MODULE_AUTHOR("Anton Vorontsov <avorontsov@ru.mvista.com>");
+MODULE_LICENSE("GPL");
-- 
1.5.2.2

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

[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 <avorontsov@ru.mvista.com>
Acked-by: Olof Johansson <olof@lixom.net>
---
 arch/powerpc/boot/dts/mpc8349emitx.dts    |   17 ++++++++++++++++-
 arch/powerpc/platforms/83xx/mpc834x_itx.c |   17 +++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts b/arch/powerpc/boot/dts/mpc8349emitx.dts
index 5072f6d..8440943 100644
--- a/arch/powerpc/boot/dts/mpc8349emitx.dts
+++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
@@ -249,6 +249,21 @@
 		device_type = "pci";
 	};
 
+	localbus@e0005000 {
+		#address-cells = <2>;
+		#size-cells = <1>;
+		compatible = "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>;
+		};
+	};
 };
diff --git a/arch/powerpc/platforms/83xx/mpc834x_itx.c b/arch/powerpc/platforms/83xx/mpc834x_itx.c
index aa76819..4797850 100644
--- a/arch/powerpc/platforms/83xx/mpc834x_itx.c
+++ b/arch/powerpc/platforms/83xx/mpc834x_itx.c
@@ -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
-- 
1.5.2.2

[PATCH v4 0/3] OF-platform PATA driver

[Anton Vorontsov]

Hi all,

Here is the resend (aka v4) version of the OF-platform PATA
driver and related patches.

Changes since v3:
- Acked-by: Paul Mundt <lethal@linux-sh.org>
- In the powerpc specific patch: update defconfig and use
  machine_device_initcall -- this is new call found in the
  galak/powerpc.git.

Changes since v2:
- "PPC" added to PATA_PLATFORM "depends on" Kconfig entry;
  I didn't remove EMBEDDED "depends on" -- this wasn't discussed
  much and these patches should not depend on the decision.
- cosmetic fixes;
- "s/ioport_shift/reg_shift/g" patch dropped.

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.

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

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

[Anton Vorontsov]

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

Patch also updates defconfig and adds some code to probe localbus.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Acked-by: Olof Johansson <olof@lixom.net>
---
 arch/powerpc/boot/dts/mpc8349emitx.dts     |   17 ++++++++++++++++-
 arch/powerpc/configs/mpc834x_itx_defconfig |    3 ++-
 arch/powerpc/platforms/83xx/mpc834x_itx.c  |   12 ++++++++++++
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts b/arch/powerpc/boot/dts/mpc8349emitx.dts
index e354f26..06d5b9c 100644
--- a/arch/powerpc/boot/dts/mpc8349emitx.dts
+++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
@@ -253,6 +253,21 @@
 		device_type = "pci";
 	};
 
+	localbus@e0005000 {
+		#address-cells = <2>;
+		#size-cells = <1>;
+		compatible = "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>;
+		};
+	};
 };
diff --git a/arch/powerpc/configs/mpc834x_itx_defconfig b/arch/powerpc/configs/mpc834x_itx_defconfig
index 6feb86e..2fbe4e5 100644
--- a/arch/powerpc/configs/mpc834x_itx_defconfig
+++ b/arch/powerpc/configs/mpc834x_itx_defconfig
@@ -570,7 +570,8 @@ CONFIG_SATA_SIL=y
 # CONFIG_PATA_SIS is not set
 # CONFIG_PATA_VIA is not set
 # CONFIG_PATA_WINBOND is not set
-# CONFIG_PATA_PLATFORM is not set
+CONFIG_PATA_PLATFORM=y
+CONFIG_PATA_OF_PLATFORM=y
 CONFIG_MD=y
 CONFIG_BLK_DEV_MD=y
 CONFIG_MD_LINEAR=y
diff --git a/arch/powerpc/platforms/83xx/mpc834x_itx.c b/arch/powerpc/platforms/83xx/mpc834x_itx.c
index aa76819..50e8f63 100644
--- a/arch/powerpc/platforms/83xx/mpc834x_itx.c
+++ b/arch/powerpc/platforms/83xx/mpc834x_itx.c
@@ -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,17 @@
 
 #include "mpc83xx.h"
 
+static struct of_device_id __initdata mpc834x_itx_ids[] = {
+	{ .compatible = "fsl,pq2pro-localbus", },
+	{},
+};
+
+static int __init mpc834x_itx_declare_of_platform_devices(void)
+{
+	return of_platform_bus_probe(NULL, mpc834x_itx_ids, NULL);
+}
+machine_device_initcall(mpc834x_itx, mpc834x_itx_declare_of_platform_devices);
+
 /* ************************************************************************
  *
  * Setup the architecture
-- 
1.5.2.2

[PATCH 0/3] OF-platform PATA driver

[Anton Vorontsov]

Hi all,

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

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


Thanks for the previous review! This time I'm collecting acks,
don't be shy to give 'em generously. ;-)


Good luck,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

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

[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 <avorontsov@ru.mvista.com>
---
 arch/powerpc/boot/dts/mpc8349emitx.dts    |   17 ++++++++++++++++-
 arch/powerpc/platforms/83xx/mpc834x_itx.c |   17 +++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts b/arch/powerpc/boot/dts/mpc8349emitx.dts
index 5072f6d..7a97068 100644
--- a/arch/powerpc/boot/dts/mpc8349emitx.dts
+++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
@@ -249,6 +249,21 @@
 		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", "pata-platform";
+			reg = <3 0 10 3 20c 4>;
+			ioport-shift = <1>;
+			interrupts = <17 8>;
+			interrupt-parent = <&ipic>;
+		};
+	};
 };
diff --git a/arch/powerpc/platforms/83xx/mpc834x_itx.c b/arch/powerpc/platforms/83xx/mpc834x_itx.c
index aa76819..ea5f176 100644
--- a/arch/powerpc/platforms/83xx/mpc834x_itx.c
+++ b/arch/powerpc/platforms/83xx/mpc834x_itx.c
@@ -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
-- 
1.5.2.2

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

[Sergei Shtylyov]

Hello.

Anton Vorontsov wrote:

> 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 <avorontsov@ru.mvista.com>
> ---
>  arch/powerpc/boot/dts/mpc8349emitx.dts    |   17 ++++++++++++++++-
>  arch/powerpc/platforms/83xx/mpc834x_itx.c |   17 +++++++++++++++++
>  2 files changed, 33 insertions(+), 1 deletions(-)

> diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts b/arch/powerpc/boot/dts/mpc8349emitx.dts
> index 5072f6d..7a97068 100644
> --- a/arch/powerpc/boot/dts/mpc8349emitx.dts
> +++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
> @@ -249,6 +249,21 @@
>  		device_type = "pci";
>  	};
>  
> +	localbus@e0005000 {
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +		compatible = "fsl,mpc8349emitx-localbus",

    Board compatible bus?

> +			     "fsl,mpc8349e-localbus",
> +			     "fsl,pq2pro-localbus";
> +		reg = <e0005000 d8>;
> +		ranges = <3 0 f0000000 210>;
>  
> -
> +		pata@3,0 {
> +			compatible = "fsl,mpc8349emitx-pata", "pata-platform";
> +			reg = <3 0 10 3 20c 4>;
> +			ioport-shift = <1>;

    Bleh... that shift again. And this is surely not a good name for a 
property (where's I/O ports in your case?) -- why not call it "reg-shift" 
(well, I'd call it "reg-size" or "reg-stride" myself :-)?

MBR, Sergei

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

[Anton Vorontsov]

On Tue, Nov 27, 2007 at 06:49:13PM +0300, Sergei Shtylyov wrote:
> Hello.

Hi Sergei,

> Anton Vorontsov wrote:
> 
> >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 <avorontsov@ru.mvista.com>
> >---
> > arch/powerpc/boot/dts/mpc8349emitx.dts    |   17 ++++++++++++++++-
> > arch/powerpc/platforms/83xx/mpc834x_itx.c |   17 +++++++++++++++++
> > 2 files changed, 33 insertions(+), 1 deletions(-)
> 
> >diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts 
> >b/arch/powerpc/boot/dts/mpc8349emitx.dts
> >index 5072f6d..7a97068 100644
> >--- a/arch/powerpc/boot/dts/mpc8349emitx.dts
> >+++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
> >@@ -249,6 +249,21 @@
> > 		device_type = "pci";
> > 	};
> > 
> >+	localbus@e0005000 {
> >+		#address-cells = <2>;
> >+		#size-cells = <1>;
> >+		compatible = "fsl,mpc8349emitx-localbus",
> 
>    Board compatible bus?

This is what Documentation/powerpc/booting-without-of.txt suggests
for localbuses. I'm following.

> >+			     "fsl,mpc8349e-localbus",
> >+			     "fsl,pq2pro-localbus";
> >+		reg = <e0005000 d8>;
> >+		ranges = <3 0 f0000000 210>;
> > 
> >-
> >+		pata@3,0 {
> >+			compatible = "fsl,mpc8349emitx-pata", 
> >"pata-platform";
> >+			reg = <3 0 10 3 20c 4>;
> >+			ioport-shift = <1>;
> 
>    Bleh... that shift again. And this is surely not a good name for a 
> property (where's I/O ports in your case?) -- why not call it "reg-shift" 
> (well, I'd call it "reg-size" or "reg-stride" myself :-)?

1. "shift" because pata_platform using that name. I don't see any
   reason to contrive indirections. ioport-shift is what the whole
   Linux kernel using nowadays, and ioport-shift dts property
   anyway Linux-specific.

   I'm just following todays' conventions.

   If you feel really bad about that, I think better to fix that in
   the source of the badness -- pata_platform. It's easy, I can do
   that. Would you ack patch that converts whole pata_platform and
   users? Would Paul ack it?

   Still, is there any hardware that needs not power of 2 stride?

2. "ioport" because shift^Wstride ;-) applies only to the io range
   (yes, it's obvious, but worth open-wording, no?).


And btw, I can get rid of ioport-shift at all. And do fixups in
the pata_of_platform driver via .compatible matching. But I don't
want: it feels bad to list every needs-to-fixup board in the common
driver. It also feels not so great creating something like
pata-platform-stride-{1,2,4,...} compatible stuff. Heh.


Thanks,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

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

[Sergei Shtylyov]

Anton Vorontsov wrote:

>>>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 <avorontsov@ru.mvista.com>
>>>---
>>>arch/powerpc/boot/dts/mpc8349emitx.dts    |   17 ++++++++++++++++-
>>>arch/powerpc/platforms/83xx/mpc834x_itx.c |   17 +++++++++++++++++
>>>2 files changed, 33 insertions(+), 1 deletions(-)

>>>diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts 
>>>b/arch/powerpc/boot/dts/mpc8349emitx.dts
>>>index 5072f6d..7a97068 100644
>>>--- a/arch/powerpc/boot/dts/mpc8349emitx.dts
>>>+++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
>>>@@ -249,6 +249,21 @@
>>>		device_type = "pci";
>>>	};
>>>
>>>+	localbus@e0005000 {
>>>+		#address-cells = <2>;
>>>+		#size-cells = <1>;
>>>+		compatible = "fsl,mpc8349emitx-localbus",
>>
>>   Board compatible bus?

> This is what Documentation/powerpc/booting-without-of.txt suggests
> for localbuses. I'm following.

    Hm...

>>>+			     "fsl,mpc8349e-localbus",
>>>+			     "fsl,pq2pro-localbus";
>>>+		reg = <e0005000 d8>;
>>>+		ranges = <3 0 f0000000 210>;
>>>
>>>-
>>>+		pata@3,0 {
>>>+			compatible = "fsl,mpc8349emitx-pata", 
>>>"pata-platform";
>>>+			reg = <3 0 10 3 20c 4>;
>>>+			ioport-shift = <1>;

>>   Bleh... that shift again. And this is surely not a good name for a 
>>property (where's I/O ports in your case?) -- why not call it "reg-shift" 
>>(well, I'd call it "reg-size" or "reg-stride" myself :-)?

> 1. "shift" because pata_platform using that name. I don't see any
>    reason to contrive indirections. ioport-shift is what the whole
>    Linux kernel using nowadays, and ioport-shift dts property
>    anyway Linux-specific.

    It's just a bad name. There's not even I/O ports in this case (and 
moreover, the *real* I/O mapped device would always have a shift of 0, I bet 
-- larger strides are for memory mapped devices).

>    I'm just following todays' conventions.

>    If you feel really bad about that, I think better to fix that in
>    the source of the badness -- pata_platform. It's easy, I can do

    I only feel really bad about the "ioport" part, I can live with "shift" 
part. :-)

>    that. Would you ack patch that converts whole pata_platform and
>    users? Would Paul ack it?

    I don't understand -- why the property name should duplicate pata_platform 
field name? :-O

>    Still, is there any hardware that needs not power of 2 stride?

    Not really -- "size" just seems better, aesthetically. :-)

> 2. "ioport" because shift^Wstride ;-) applies only to the io range
>    (yes, it's obvious, but worth open-wording, no?).

    Contrarywise, to memory range.

> And btw, I can get rid of ioport-shift at all. And do fixups in
> the pata_of_platform driver via .compatible matching. But I don't
> want: it feels bad to list every needs-to-fixup board in the common
> driver. It also feels not so great creating something like
> pata-platform-stride-{1,2,4,...} compatible stuff. Heh.

    I didn't propose neither of that. :-)
    All I want is that "ioport-*" be renamed.

> Thanks,

MBR, Sergei

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

[Anton Vorontsov]

On Tue, Nov 27, 2007 at 07:46:13PM +0300, Sergei Shtylyov wrote:
[...]
> >>>+			ioport-shift = <1>;
> 
> >>  Bleh... that shift again. And this is surely not a good name for a 
> >>property (where's I/O ports in your case?) -- why not call it "reg-shift" 
> >>(well, I'd call it "reg-size" or "reg-stride" myself :-)?
> 
> >1. "shift" because pata_platform using that name. I don't see any
> >   reason to contrive indirections. ioport-shift is what the whole
> >   Linux kernel using nowadays, and ioport-shift dts property
> >   anyway Linux-specific.
> 
>    It's just a bad name. There's not even I/O ports in this case (and 
> moreover, the *real* I/O mapped device would always have a shift of 0, I 
> bet -- larger strides are for memory mapped devices).
> 
> >   I'm just following todays' conventions.
> 
> >   If you feel really bad about that, I think better to fix that in
> >   the source of the badness -- pata_platform. It's easy, I can do
> 
>    I only feel really bad about the "ioport" part, I can live with "shift" 
> part. :-)
> 
> >   that. Would you ack patch that converts whole pata_platform and
> >   users? Would Paul ack it?
> 
>    I don't understand -- why the property name should duplicate 
>    pata_platform field name? :-O

Because:

> >1.  [...] I don't see any reason to contrive indirections.

That is, different names for single thing is worse than single
bogus name.

>   Not really -- "size" just seems better, aesthetically. :-)

reg-size will look confusing. Is it ata registers' size? No,
can't be. So, what is it? It's stride/shift because of bus, on
which ata resides.

> >And btw, I can get rid of ioport-shift at all. And do fixups in
> >the pata_of_platform driver via .compatible matching. But I don't
> >want: it feels bad to list every needs-to-fixup board in the common
> >driver. It also feels not so great creating something like
> >pata-platform-stride-{1,2,4,...} compatible stuff. Heh.
> 
>    I didn't propose neither of that. :-)

Yup, that was "by the way"...

>    All I want is that "ioport-*" be renamed.

I give up.

The final name is..? I can think out wrong one, so you'd better
convoy me on that way. ;-) reg-shift sounds okay? Or reg-stride
better? No size, please.


Thanks,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

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

[Sergei Shtylyov]

Anton Vorontsov wrote:

>>   All I want is that "ioport-*" be renamed.

> I give up.

    Don't. :-)

> The final name is..? I can think out wrong one, so you'd better
> convoy me on that way. ;-) reg-shift sounds okay?

    Yes.

> Thanks,

WBR, Sergei

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

[Anton Vorontsov]

[ Had cut too much in the previous reply ]

On Tue, Nov 27, 2007 at 07:46:13PM +0300, Sergei Shtylyov wrote:
[...]
> >2. "ioport" because shift^Wstride ;-) applies only to the io range
> >   (yes, it's obvious, but worth open-wording, no?).
> 
>    Contrarywise, to memory range.

By io range I meant "I/O base", in contrast to "CTL base".

There is no need to apply shifting for CTL. That's why ioport-*
appeared in the first place.

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

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

[Sergei Shtylyov]

Anton Vorontsov wrote:

>>>2. "ioport" because shift^Wstride ;-) applies only to the io range
>>>  (yes, it's obvious, but worth open-wording, no?).

>>   Contrarywise, to memory range.

> By io range I meant "I/O base", in contrast to "CTL base".

> There is no need to apply shifting for CTL. That's why ioport-*
> appeared in the first place.

    So, a matter of wrong terminology then. The thing that you meant by "I/O" 
is actually called "command block".

MBR, Sergei

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

[Anton Vorontsov]

On Tue, Nov 27, 2007 at 08:34:11PM +0300, Sergei Shtylyov wrote:
> Anton Vorontsov wrote:
> 
> >>>2. "ioport" because shift^Wstride ;-) applies only to the io range
> >>> (yes, it's obvious, but worth open-wording, no?).
> 
> >>  Contrarywise, to memory range.
> 
> >By io range I meant "I/O base", in contrast to "CTL base".
> 
> >There is no need to apply shifting for CTL. That's why ioport-*
> >appeared in the first place.
> 
>    So, a matter of wrong terminology then. The thing that you meant by 
>    "I/O" is actually called "command block".

Yes. And IO is the second name. It's used widespread in the
drivers/ide/.

Now you understand why I'm so reluctant to hanging up different
labels on the single thing? ;-)

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

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

[Sergei Shtylyov]

Anton Vorontsov wrote:

>>>>>2. "ioport" because shift^Wstride ;-) applies only to the io range
>>>>>(yes, it's obvious, but worth open-wording, no?).

>>>> Contrarywise, to memory range.

>>>By io range I meant "I/O base", in contrast to "CTL base".

>>>There is no need to apply shifting for CTL. That's why ioport-*
>>>appeared in the first place.

>>   So, a matter of wrong terminology then. The thing that you meant by 
>>   "I/O" is actually called "command block".

> Yes. And IO is the second name.

    I'd say the first place in drivers/ide belongs to the historic name 
"taskfile". The "command block" which is as ATA standard calls it, is hardly used.

> It's used widespread in the drivers/ide/.

    Don't remember seeing it.

> Now you understand why I'm so reluctant to hanging up different
> labels on the single thing? ;-)

    :-)

MBR, Sergei

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

[Anton Vorontsov]

On Tue, Nov 27, 2007 at 09:07:14PM +0300, Sergei Shtylyov wrote:
> Anton Vorontsov wrote:
> 
> >>>>>2. "ioport" because shift^Wstride ;-) applies only to the io range
> >>>>>(yes, it's obvious, but worth open-wording, no?).
> 
> >>>> Contrarywise, to memory range.
> 
> >>>By io range I meant "I/O base", in contrast to "CTL base".
> 
> >>>There is no need to apply shifting for CTL. That's why ioport-*
> >>>appeared in the first place.
> 
> >>   So, a matter of wrong terminology then. The thing that you meant by 
> >>   "I/O" is actually called "command block".
> 
> > Yes. And IO is the second name.
> 
>     I'd say the first place in drivers/ide belongs to the historic name 
> "taskfile". The "command block" which is as ATA standard calls it, is hardly used.
> 
> > It's used widespread in the drivers/ide/.
> 
>     Don't remember seeing it.

Oops, thinko -- not drivers/ide/, but include/linux/ide.h.
Grep for io_addr.

> > Now you understand why I'm so reluctant to hanging up different
> > labels on the single thing? ;-)
> 
>     :-)

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

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

[Kumar Gala]

On Nov 27, 2007, at 9:49 AM, Sergei Shtylyov wrote:

> Hello.
>
> Anton Vorontsov wrote:
>
>> 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 <avorontsov@ru.mvista.com>
>> ---
>> arch/powerpc/boot/dts/mpc8349emitx.dts    |   17 ++++++++++++++++-
>> arch/powerpc/platforms/83xx/mpc834x_itx.c |   17 +++++++++++++++++
>> 2 files changed, 33 insertions(+), 1 deletions(-)
>
>> diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts b/arch/powerpc/=20=

>> boot/dts/mpc8349emitx.dts
>> index 5072f6d..7a97068 100644
>> --- a/arch/powerpc/boot/dts/mpc8349emitx.dts
>> +++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
>> @@ -249,6 +249,21 @@
>> 		device_type =3D "pci";
>> 	};
>>
>> +	localbus@e0005000 {
>> +		#address-cells =3D <2>;
>> +		#size-cells =3D <1>;
>> +		compatible =3D "fsl,mpc8349emitx-localbus",
>
>    Board compatible bus?
>
>> +			     "fsl,mpc8349e-localbus",
>> +			     "fsl,pq2pro-localbus";
>> +		reg =3D <e0005000 d8>;
>> +		ranges =3D <3 0 f0000000 210>;
>>
>> -
>> +		pata@3,0 {
>> +			compatible =3D "fsl,mpc8349emitx-pata", =
"pata-platform";
>> +			reg =3D <3 0 10 3 20c 4>;
>> +			ioport-shift =3D <1>;
>
>    Bleh... that shift again. And this is surely not a good name for a
> property (where's I/O ports in your case?) -- why not call it "reg-=20
> shift"
> (well, I'd call it "reg-size" or "reg-stride" myself :-)?

I'm coming into this late, but if ioport-shift applies to reg (which I =20=

think it does) it should really be called "reg-shift".  The ePAPR is =20
using that property name:

Specifies in bytes how far the discrete device registers are separated =20=

from each other. The
individual register location is calculated by using following formula: =20=

=93registers address=94 <<
reg-shift. If unspecified the default value is 0.

- k