[PATCH 0/5] MIPS: OCTEON: Convert some device to use Device Tree.

[PATCH 0/5] MIPS: OCTEON: Convert some device to use Device Tree.

[David Daney]

From: David Daney <david.daney@cavium.com>

This patch set depends on the previous set to add Device Tree to
OCTEON.  That set can be found here:

http://www.linux-mips.org/archives/linux-mips/2012-03/msg00170.html

The patches are for drivers scattered throughout the tree, but there
is an order dependency as well as the dependency mentioned above, so
it may be preferable to merge them all via Ralf's linux-mips.org tree
rather that separately via the various device sub-system maintainers.

The mdio-octeon.c and octeon_mgmt patches were already Acked-by davem.

The i2c has been seen before, but to my knowledge was never acked.

The octeon_ethernet patch is in staging and has also been seen before.

The serial patch is new.

David Daney (5):
  i2c: Convert i2c-octeon.c to use device tree.
  netdev: mdio-octeon.c: Convert to use device tree.
  netdev: octeon_mgmt: Convert to use device tree.
  staging: octeon_ethernet: Convert to use device tree.
  MIPS: Octeon: Use device tree to register serial ports.

 arch/mips/cavium-octeon/octeon-irq.c      |    8 -
 arch/mips/cavium-octeon/octeon-platform.c |  176 ----------------
 arch/mips/cavium-octeon/serial.c          |  134 +++++--------
 arch/mips/include/asm/octeon/octeon.h     |    5 -
 drivers/i2c/busses/i2c-octeon.c           |   94 +++++----
 drivers/net/ethernet/octeon/octeon_mgmt.c |  312 +++++++++++++++++++----------
 drivers/net/phy/mdio-octeon.c             |   92 ++++++---
 drivers/staging/octeon/ethernet-mdio.c    |   28 ++--
 drivers/staging/octeon/ethernet.c         |  153 +++++++++-----
 drivers/staging/octeon/octeon-ethernet.h  |    3 +
 10 files changed, 487 insertions(+), 518 deletions(-)

Cc: netdev@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:  David S. Miller <davem@davemloft.net>
Cc: "Jean Delvare (PC drivers, core)" <khali@linux-fr.org>
Cc: "Ben Dooks (embedded platforms)" <ben-linux@fluff.org>
Cc: "Wolfram Sang (embedded platforms)" <w.sang@pengutronix.de>
Cc: linux-i2c@vger.kernel.org
-- 
1.7.2.3

[PATCH 1/5] i2c: Convert i2c-octeon.c to use device tree.

[David Daney]

From: David Daney <david.daney@cavium.com>

There are three parts to this:

1) Remove the definitions of OCTEON_IRQ_TWSI and OCTEON_IRQ_TWSI2.
   The interrupts are specified by the device tree and these hard
   coded irq numbers block the used of the irq lines by the irq_domain
   code.

2) Remove platform device setup code from octeon-platform.c, it is
   now unused.

3) Convert i2c-octeon.c to use device tree.  Part of this includes
   using the devm_* functions instead of the raw counterparts, thus
   simplifying error handling.  No functionality is changed.

Signed-off-by: David Daney <david.daney@cavium.com>
Cc: "Jean Delvare (PC drivers, core)" <khali@linux-fr.org>
Cc: "Ben Dooks (embedded platforms)" <ben-linux@fluff.org>
Cc: "Wolfram Sang (embedded platforms)" <w.sang@pengutronix.de>
Cc: linux-i2c@vger.kernel.org
---

Should probably go via Ralf's linux-mips.org tree.

 arch/mips/cavium-octeon/octeon-irq.c      |    2 -
 arch/mips/cavium-octeon/octeon-platform.c |   84 -------------------------
 arch/mips/include/asm/octeon/octeon.h     |    5 --
 drivers/i2c/busses/i2c-octeon.c           |   94 ++++++++++++++++-------------
 4 files changed, 52 insertions(+), 133 deletions(-)

diff --git a/arch/mips/cavium-octeon/octeon-irq.c b/arch/mips/cavium-octeon/octeon-irq.c
index 550c03d..7912f96 100644
--- a/arch/mips/cavium-octeon/octeon-irq.c
+++ b/arch/mips/cavium-octeon/octeon-irq.c
@@ -1176,13 +1176,11 @@ static void __init octeon_irq_init_ciu(void)
 	for (i = 0; i < 4; i++)
 		octeon_irq_set_ciu_mapping(i + OCTEON_IRQ_PCI_MSI0, 0, i + 40, chip, handle_level_irq);
 
-	octeon_irq_set_ciu_mapping(OCTEON_IRQ_TWSI, 0, 45, chip, handle_level_irq);
 	octeon_irq_set_ciu_mapping(OCTEON_IRQ_RML, 0, 46, chip, handle_level_irq);
 	for (i = 0; i < 4; i++)
 		octeon_irq_set_ciu_mapping(i + OCTEON_IRQ_TIMER0, 0, i + 52, chip, handle_edge_irq);
 
 	octeon_irq_set_ciu_mapping(OCTEON_IRQ_USB0, 0, 56, chip, handle_level_irq);
-	octeon_irq_set_ciu_mapping(OCTEON_IRQ_TWSI2, 0, 59, chip, handle_level_irq);
 	octeon_irq_set_ciu_mapping(OCTEON_IRQ_MII0, 0, 62, chip, handle_level_irq);
 	octeon_irq_set_ciu_mapping(OCTEON_IRQ_BOOTDMA, 0, 63, chip, handle_level_irq);
 
diff --git a/arch/mips/cavium-octeon/octeon-platform.c b/arch/mips/cavium-octeon/octeon-platform.c
index 2754bc2..f62a40f 100644
--- a/arch/mips/cavium-octeon/octeon-platform.c
+++ b/arch/mips/cavium-octeon/octeon-platform.c
@@ -168,90 +168,6 @@ out:
 }
 device_initcall(octeon_rng_device_init);
 
-static struct i2c_board_info __initdata octeon_i2c_devices[] = {
-	{
-		I2C_BOARD_INFO("ds1337", 0x68),
-	},
-};
-
-static int __init octeon_i2c_devices_init(void)
-{
-	return i2c_register_board_info(0, octeon_i2c_devices,
-				       ARRAY_SIZE(octeon_i2c_devices));
-}
-arch_initcall(octeon_i2c_devices_init);
-
-#define OCTEON_I2C_IO_BASE 0x1180000001000ull
-#define OCTEON_I2C_IO_UNIT_OFFSET 0x200
-
-static struct octeon_i2c_data octeon_i2c_data[2];
-
-static int __init octeon_i2c_device_init(void)
-{
-	struct platform_device *pd;
-	int ret = 0;
-	int port, num_ports;
-
-	struct resource i2c_resources[] = {
-		{
-			.flags	= IORESOURCE_MEM,
-		}, {
-			.flags	= IORESOURCE_IRQ,
-		}
-	};
-
-	if (OCTEON_IS_MODEL(OCTEON_CN56XX) || OCTEON_IS_MODEL(OCTEON_CN52XX))
-		num_ports = 2;
-	else
-		num_ports = 1;
-
-	for (port = 0; port < num_ports; port++) {
-		octeon_i2c_data[port].sys_freq = octeon_get_io_clock_rate();
-		/*FIXME: should be examined. At the moment is set for 100Khz */
-		octeon_i2c_data[port].i2c_freq = 100000;
-
-		pd = platform_device_alloc("i2c-octeon", port);
-		if (!pd) {
-			ret = -ENOMEM;
-			goto out;
-		}
-
-		pd->dev.platform_data = octeon_i2c_data + port;
-
-		i2c_resources[0].start =
-			OCTEON_I2C_IO_BASE + (port * OCTEON_I2C_IO_UNIT_OFFSET);
-		i2c_resources[0].end = i2c_resources[0].start + 0x1f;
-		switch (port) {
-		case 0:
-			i2c_resources[1].start = OCTEON_IRQ_TWSI;
-			i2c_resources[1].end = OCTEON_IRQ_TWSI;
-			break;
-		case 1:
-			i2c_resources[1].start = OCTEON_IRQ_TWSI2;
-			i2c_resources[1].end = OCTEON_IRQ_TWSI2;
-			break;
-		default:
-			BUG();
-		}
-
-		ret = platform_device_add_resources(pd,
-						    i2c_resources,
-						    ARRAY_SIZE(i2c_resources));
-		if (ret)
-			goto fail;
-
-		ret = platform_device_add(pd);
-		if (ret)
-			goto fail;
-	}
-	return ret;
-fail:
-	platform_device_put(pd);
-out:
-	return ret;
-}
-device_initcall(octeon_i2c_device_init);
-
 /* Octeon SMI/MDIO interface.  */
 static int __init octeon_mdiobus_device_init(void)
 {
diff --git a/arch/mips/include/asm/octeon/octeon.h b/arch/mips/include/asm/octeon/octeon.h
index f72f768..1e2486e 100644
--- a/arch/mips/include/asm/octeon/octeon.h
+++ b/arch/mips/include/asm/octeon/octeon.h
@@ -215,11 +215,6 @@ struct octeon_cf_data {
 	int		dma_engine;	/* -1 for no DMA */
 };
 
-struct octeon_i2c_data {
-	unsigned int	sys_freq;
-	unsigned int	i2c_freq;
-};
-
 extern void octeon_write_lcd(const char *s);
 extern void octeon_check_cpu_bist(void);
 extern int octeon_get_boot_debug_flag(void);
diff --git a/drivers/i2c/busses/i2c-octeon.c b/drivers/i2c/busses/i2c-octeon.c
index ee139a5..d5e2a4c 100644
--- a/drivers/i2c/busses/i2c-octeon.c
+++ b/drivers/i2c/busses/i2c-octeon.c
@@ -2,7 +2,7 @@
  * (C) Copyright 2009-2010
  * Nokia Siemens Networks, michael.lawnick.ext@nsn.com
  *
- * Portions Copyright (C) 2010 Cavium Networks, Inc.
+ * Portions Copyright (C) 2010, 2011 Cavium Networks, Inc.
  *
  * This is a driver for the i2c adapter in Cavium Networks' OCTEON processors.
  *
@@ -11,17 +11,18 @@
  * warranty of any kind, whether express or implied.
  */
 
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of_i2c.h>
+#include <linux/delay.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/init.h>
-
-#include <linux/io.h>
 #include <linux/i2c.h>
-#include <linux/interrupt.h>
-#include <linux/delay.h>
-#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/of.h>
 
 #include <asm/octeon/octeon.h>
 
@@ -121,10 +122,8 @@ static u8 octeon_i2c_read_sw(struct octeon_i2c *i2c, u64 eop_reg)
  */
 static void octeon_i2c_write_int(struct octeon_i2c *i2c, u64 data)
 {
-	u64 tmp;
-
 	__raw_writeq(data, i2c->twsi_base + TWSI_INT);
-	tmp = __raw_readq(i2c->twsi_base + TWSI_INT);
+	__raw_readq(i2c->twsi_base + TWSI_INT);
 }
 
 /**
@@ -515,94 +514,101 @@ static int __devinit octeon_i2c_probe(struct platform_device *pdev)
 {
 	int irq, result = 0;
 	struct octeon_i2c *i2c;
-	struct octeon_i2c_data *i2c_data;
 	struct resource *res_mem;
+	const __be32 *data;
+	int len;
 
 	/* All adaptors have an irq.  */
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
 		return irq;
 
-	i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);
+	i2c = devm_kzalloc(&pdev->dev, sizeof(*i2c), GFP_KERNEL);
 	if (!i2c) {
 		dev_err(&pdev->dev, "kzalloc failed\n");
 		result = -ENOMEM;
 		goto out;
 	}
 	i2c->dev = &pdev->dev;
-	i2c_data = pdev->dev.platform_data;
 
 	res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
 	if (res_mem == NULL) {
 		dev_err(i2c->dev, "found no memory resource\n");
 		result = -ENXIO;
-		goto fail_region;
+		goto out;
 	}
+	i2c->twsi_phys = res_mem->start;
+	i2c->regsize = resource_size(res_mem);
 
-	if (i2c_data == NULL) {
-		dev_err(i2c->dev, "no I2C frequency data\n");
+	/*
+	 * "clock-rate" is a legacy binding, the official binding is
+	 * "clock-frequency".  Try the official one first and then
+	 * fall back if it doesn't exist.
+	 */
+	data = of_get_property(pdev->dev.of_node, "clock-frequency", &len);
+	if (!data || len != sizeof(*data))
+		data = of_get_property(pdev->dev.of_node, "clock-rate", &len);
+	if (data && len == sizeof(*data)) {
+		i2c->twsi_freq = be32_to_cpup(data);
+	} else {
+		dev_err(i2c->dev,
+			"no I2C 'clock-rate' or 'clock-frequency' property\n");
 		result = -ENXIO;
-		goto fail_region;
+		goto out;
 	}
 
-	i2c->twsi_phys = res_mem->start;
-	i2c->regsize = resource_size(res_mem);
-	i2c->twsi_freq = i2c_data->i2c_freq;
-	i2c->sys_freq = i2c_data->sys_freq;
+	i2c->sys_freq = octeon_get_io_clock_rate();
 
-	if (!request_mem_region(i2c->twsi_phys, i2c->regsize, res_mem->name)) {
+	if (!devm_request_mem_region(&pdev->dev, i2c->twsi_phys, i2c->regsize,
+				      res_mem->name)) {
 		dev_err(i2c->dev, "request_mem_region failed\n");
-		goto fail_region;
+		goto out;
 	}
-	i2c->twsi_base = ioremap(i2c->twsi_phys, i2c->regsize);
+	i2c->twsi_base = devm_ioremap(&pdev->dev, i2c->twsi_phys, i2c->regsize);
 
 	init_waitqueue_head(&i2c->queue);
 
 	i2c->irq = irq;
 
-	result = request_irq(i2c->irq, octeon_i2c_isr, 0, DRV_NAME, i2c);
+	result = devm_request_irq(&pdev->dev, i2c->irq,
+				  octeon_i2c_isr, 0, DRV_NAME, i2c);
 	if (result < 0) {
 		dev_err(i2c->dev, "failed to attach interrupt\n");
-		goto fail_irq;
+		goto out;
 	}
 
 	result = octeon_i2c_initlowlevel(i2c);
 	if (result) {
 		dev_err(i2c->dev, "init low level failed\n");
-		goto  fail_add;
+		goto  out;
 	}
 
 	result = octeon_i2c_setclock(i2c);
 	if (result) {
 		dev_err(i2c->dev, "clock init failed\n");
-		goto  fail_add;
+		goto  out;
 	}
 
 	i2c->adap = octeon_i2c_ops;
 	i2c->adap.dev.parent = &pdev->dev;
-	i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
+	i2c->adap.dev.of_node = pdev->dev.of_node;
 	i2c_set_adapdata(&i2c->adap, i2c);
 	platform_set_drvdata(pdev, i2c);
 
-	result = i2c_add_numbered_adapter(&i2c->adap);
+	result = i2c_add_adapter(&i2c->adap);
 	if (result < 0) {
 		dev_err(i2c->dev, "failed to add adapter\n");
 		goto fail_add;
 	}
-
 	dev_info(i2c->dev, "version %s\n", DRV_VERSION);
 
-	return result;
+	of_i2c_register_devices(&i2c->adap);
+
+	return 0;
 
 fail_add:
 	platform_set_drvdata(pdev, NULL);
-	free_irq(i2c->irq, i2c);
-fail_irq:
-	iounmap(i2c->twsi_base);
-	release_mem_region(i2c->twsi_phys, i2c->regsize);
-fail_region:
-	kfree(i2c);
 out:
 	return result;
 };
@@ -613,19 +619,24 @@ static int __devexit octeon_i2c_remove(struct platform_device *pdev)
 
 	i2c_del_adapter(&i2c->adap);
 	platform_set_drvdata(pdev, NULL);
-	free_irq(i2c->irq, i2c);
-	iounmap(i2c->twsi_base);
-	release_mem_region(i2c->twsi_phys, i2c->regsize);
-	kfree(i2c);
 	return 0;
 };
 
+static struct of_device_id octeon_i2c_match[] = {
+	{
+		.compatible = "cavium,octeon-3860-twsi",
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, octeon_i2c_match);
+
 static struct platform_driver octeon_i2c_driver = {
 	.probe		= octeon_i2c_probe,
 	.remove		= __devexit_p(octeon_i2c_remove),
 	.driver		= {
 		.owner	= THIS_MODULE,
 		.name	= DRV_NAME,
+		.of_match_table = octeon_i2c_match,
 	},
 };
 
@@ -635,4 +646,3 @@ MODULE_AUTHOR("Michael Lawnick <michael.lawnick.ext@nsn.com>");
 MODULE_DESCRIPTION("I2C-Bus adapter for Cavium OCTEON processors");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_VERSION);
-MODULE_ALIAS("platform:" DRV_NAME);
-- 
1.7.2.3

Re: [PATCH 1/5] i2c: Convert i2c-octeon.c to use device tree.

[Rob Herring]

On 03/26/2012 07:27 PM, David Daney wrote:
> From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> 
> There are three parts to this:
> 
> 1) Remove the definitions of OCTEON_IRQ_TWSI and OCTEON_IRQ_TWSI2.
>    The interrupts are specified by the device tree and these hard
>    coded irq numbers block the used of the irq lines by the irq_domain
>    code.
> 
> 2) Remove platform device setup code from octeon-platform.c, it is
>    now unused.
> 
> 3) Convert i2c-octeon.c to use device tree.  Part of this includes
>    using the devm_* functions instead of the raw counterparts, thus
>    simplifying error handling.  No functionality is changed.

Shouldn't this patch go after converting the platform to DT?

> Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> Cc: "Jean Delvare (PC drivers, core)" <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> Cc: "Ben Dooks (embedded platforms)" <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
> Cc: "Wolfram Sang (embedded platforms)" <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
> 
> Should probably go via Ralf's linux-mips.org tree.
> 
>  arch/mips/cavium-octeon/octeon-irq.c      |    2 -
>  arch/mips/cavium-octeon/octeon-platform.c |   84 -------------------------
>  arch/mips/include/asm/octeon/octeon.h     |    5 --
>  drivers/i2c/busses/i2c-octeon.c           |   94 ++++++++++++++++-------------
>  4 files changed, 52 insertions(+), 133 deletions(-)

snip

>  
> -	if (i2c_data == NULL) {
> -		dev_err(i2c->dev, "no I2C frequency data\n");
> +	/*
> +	 * "clock-rate" is a legacy binding, the official binding is
> +	 * "clock-frequency".  Try the official one first and then
> +	 * fall back if it doesn't exist.
> +	 */
> +	data = of_get_property(pdev->dev.of_node, "clock-frequency", &len);
> +	if (!data || len != sizeof(*data))
> +		data = of_get_property(pdev->dev.of_node, "clock-rate", &len);
> +	if (data && len == sizeof(*data)) {
> +		i2c->twsi_freq = be32_to_cpup(data);

Can't you use of_property_read_u32?

Does the legacy binding really exist as DT support is new?

Otherwise,

Acked-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>

> +	} else {
> +		dev_err(i2c->dev,
> +			"no I2C 'clock-rate' or 'clock-frequency' property\n");
>  		result = -ENXIO;
> -		goto fail_region;
> +		goto out;
>  	}
>  
> -	i2c->twsi_phys = res_mem->start;
> -	i2c->regsize = resource_size(res_mem);
> -	i2c->twsi_freq = i2c_data->i2c_freq;
> -	i2c->sys_freq = i2c_data->sys_freq;
> +	i2c->sys_freq = octeon_get_io_clock_rate();
>  
> -	if (!request_mem_region(i2c->twsi_phys, i2c->regsize, res_mem->name)) {
> +	if (!devm_request_mem_region(&pdev->dev, i2c->twsi_phys, i2c->regsize,
> +				      res_mem->name)) {
>  		dev_err(i2c->dev, "request_mem_region failed\n");
> -		goto fail_region;
> +		goto out;
>  	}
> -	i2c->twsi_base = ioremap(i2c->twsi_phys, i2c->regsize);
> +	i2c->twsi_base = devm_ioremap(&pdev->dev, i2c->twsi_phys, i2c->regsize);
>  
>  	init_waitqueue_head(&i2c->queue);
>  
>  	i2c->irq = irq;
>  
> -	result = request_irq(i2c->irq, octeon_i2c_isr, 0, DRV_NAME, i2c);
> +	result = devm_request_irq(&pdev->dev, i2c->irq,
> +				  octeon_i2c_isr, 0, DRV_NAME, i2c);
>  	if (result < 0) {
>  		dev_err(i2c->dev, "failed to attach interrupt\n");
> -		goto fail_irq;
> +		goto out;
>  	}
>  
>  	result = octeon_i2c_initlowlevel(i2c);
>  	if (result) {
>  		dev_err(i2c->dev, "init low level failed\n");
> -		goto  fail_add;
> +		goto  out;
>  	}
>  
>  	result = octeon_i2c_setclock(i2c);
>  	if (result) {
>  		dev_err(i2c->dev, "clock init failed\n");
> -		goto  fail_add;
> +		goto  out;
>  	}
>  
>  	i2c->adap = octeon_i2c_ops;
>  	i2c->adap.dev.parent = &pdev->dev;
> -	i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
> +	i2c->adap.dev.of_node = pdev->dev.of_node;
>  	i2c_set_adapdata(&i2c->adap, i2c);
>  	platform_set_drvdata(pdev, i2c);
>  
> -	result = i2c_add_numbered_adapter(&i2c->adap);
> +	result = i2c_add_adapter(&i2c->adap);
>  	if (result < 0) {
>  		dev_err(i2c->dev, "failed to add adapter\n");
>  		goto fail_add;
>  	}
> -
>  	dev_info(i2c->dev, "version %s\n", DRV_VERSION);
>  
> -	return result;
> +	of_i2c_register_devices(&i2c->adap);
> +
> +	return 0;
>  
>  fail_add:
>  	platform_set_drvdata(pdev, NULL);
> -	free_irq(i2c->irq, i2c);
> -fail_irq:
> -	iounmap(i2c->twsi_base);
> -	release_mem_region(i2c->twsi_phys, i2c->regsize);
> -fail_region:
> -	kfree(i2c);
>  out:
>  	return result;
>  };
> @@ -613,19 +619,24 @@ static int __devexit octeon_i2c_remove(struct platform_device *pdev)
>  
>  	i2c_del_adapter(&i2c->adap);
>  	platform_set_drvdata(pdev, NULL);
> -	free_irq(i2c->irq, i2c);
> -	iounmap(i2c->twsi_base);
> -	release_mem_region(i2c->twsi_phys, i2c->regsize);
> -	kfree(i2c);
>  	return 0;
>  };
>  
> +static struct of_device_id octeon_i2c_match[] = {
> +	{
> +		.compatible = "cavium,octeon-3860-twsi",
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, octeon_i2c_match);
> +
>  static struct platform_driver octeon_i2c_driver = {
>  	.probe		= octeon_i2c_probe,
>  	.remove		= __devexit_p(octeon_i2c_remove),
>  	.driver		= {
>  		.owner	= THIS_MODULE,
>  		.name	= DRV_NAME,
> +		.of_match_table = octeon_i2c_match,
>  	},
>  };
>  
> @@ -635,4 +646,3 @@ MODULE_AUTHOR("Michael Lawnick <michael.lawnick.ext-OYasijW0DpE@public.gmane.org>");
>  MODULE_DESCRIPTION("I2C-Bus adapter for Cavium OCTEON processors");
>  MODULE_LICENSE("GPL");
>  MODULE_VERSION(DRV_VERSION);
> -MODULE_ALIAS("platform:" DRV_NAME);

Re: [PATCH 1/5] i2c: Convert i2c-octeon.c to use device tree.

[Rob Herring]

On 03/26/2012 08:20 PM, Rob Herring wrote:
> On 03/26/2012 07:27 PM, David Daney wrote:
>> From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>>
>> There are three parts to this:
>>
>> 1) Remove the definitions of OCTEON_IRQ_TWSI and OCTEON_IRQ_TWSI2.
>>    The interrupts are specified by the device tree and these hard
>>    coded irq numbers block the used of the irq lines by the irq_domain
>>    code.
>>
>> 2) Remove platform device setup code from octeon-platform.c, it is
>>    now unused.
>>
>> 3) Convert i2c-octeon.c to use device tree.  Part of this includes
>>    using the devm_* functions instead of the raw counterparts, thus
>>    simplifying error handling.  No functionality is changed.
> 
> Shouldn't this patch go after converting the platform to DT?

Nevermind this comment...

> 
>> Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>> Cc: "Jean Delvare (PC drivers, core)" <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
>> Cc: "Ben Dooks (embedded platforms)" <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
>> Cc: "Wolfram Sang (embedded platforms)" <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> ---
>>
>> Should probably go via Ralf's linux-mips.org tree.
>>
>>  arch/mips/cavium-octeon/octeon-irq.c      |    2 -
>>  arch/mips/cavium-octeon/octeon-platform.c |   84 -------------------------
>>  arch/mips/include/asm/octeon/octeon.h     |    5 --
>>  drivers/i2c/busses/i2c-octeon.c           |   94 ++++++++++++++++-------------
>>  4 files changed, 52 insertions(+), 133 deletions(-)
> 
> snip
> 
>>  
>> -	if (i2c_data == NULL) {
>> -		dev_err(i2c->dev, "no I2C frequency data\n");
>> +	/*
>> +	 * "clock-rate" is a legacy binding, the official binding is
>> +	 * "clock-frequency".  Try the official one first and then
>> +	 * fall back if it doesn't exist.
>> +	 */
>> +	data = of_get_property(pdev->dev.of_node, "clock-frequency", &len);
>> +	if (!data || len != sizeof(*data))
>> +		data = of_get_property(pdev->dev.of_node, "clock-rate", &len);
>> +	if (data && len == sizeof(*data)) {
>> +		i2c->twsi_freq = be32_to_cpup(data);
> 
> Can't you use of_property_read_u32?
> 
> Does the legacy binding really exist as DT support is new?
> 
> Otherwise,
> 
> Acked-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> 
>> +	} else {
>> +		dev_err(i2c->dev,
>> +			"no I2C 'clock-rate' or 'clock-frequency' property\n");
>>  		result = -ENXIO;
>> -		goto fail_region;
>> +		goto out;
>>  	}
>>  
>> -	i2c->twsi_phys = res_mem->start;
>> -	i2c->regsize = resource_size(res_mem);
>> -	i2c->twsi_freq = i2c_data->i2c_freq;
>> -	i2c->sys_freq = i2c_data->sys_freq;
>> +	i2c->sys_freq = octeon_get_io_clock_rate();
>>  
>> -	if (!request_mem_region(i2c->twsi_phys, i2c->regsize, res_mem->name)) {
>> +	if (!devm_request_mem_region(&pdev->dev, i2c->twsi_phys, i2c->regsize,
>> +				      res_mem->name)) {
>>  		dev_err(i2c->dev, "request_mem_region failed\n");
>> -		goto fail_region;
>> +		goto out;
>>  	}
>> -	i2c->twsi_base = ioremap(i2c->twsi_phys, i2c->regsize);
>> +	i2c->twsi_base = devm_ioremap(&pdev->dev, i2c->twsi_phys, i2c->regsize);
>>  
>>  	init_waitqueue_head(&i2c->queue);
>>  
>>  	i2c->irq = irq;
>>  
>> -	result = request_irq(i2c->irq, octeon_i2c_isr, 0, DRV_NAME, i2c);
>> +	result = devm_request_irq(&pdev->dev, i2c->irq,
>> +				  octeon_i2c_isr, 0, DRV_NAME, i2c);
>>  	if (result < 0) {
>>  		dev_err(i2c->dev, "failed to attach interrupt\n");
>> -		goto fail_irq;
>> +		goto out;
>>  	}
>>  
>>  	result = octeon_i2c_initlowlevel(i2c);
>>  	if (result) {
>>  		dev_err(i2c->dev, "init low level failed\n");
>> -		goto  fail_add;
>> +		goto  out;
>>  	}
>>  
>>  	result = octeon_i2c_setclock(i2c);
>>  	if (result) {
>>  		dev_err(i2c->dev, "clock init failed\n");
>> -		goto  fail_add;
>> +		goto  out;
>>  	}
>>  
>>  	i2c->adap = octeon_i2c_ops;
>>  	i2c->adap.dev.parent = &pdev->dev;
>> -	i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
>> +	i2c->adap.dev.of_node = pdev->dev.of_node;
>>  	i2c_set_adapdata(&i2c->adap, i2c);
>>  	platform_set_drvdata(pdev, i2c);
>>  
>> -	result = i2c_add_numbered_adapter(&i2c->adap);
>> +	result = i2c_add_adapter(&i2c->adap);
>>  	if (result < 0) {
>>  		dev_err(i2c->dev, "failed to add adapter\n");
>>  		goto fail_add;
>>  	}
>> -
>>  	dev_info(i2c->dev, "version %s\n", DRV_VERSION);
>>  
>> -	return result;
>> +	of_i2c_register_devices(&i2c->adap);
>> +
>> +	return 0;
>>  
>>  fail_add:
>>  	platform_set_drvdata(pdev, NULL);
>> -	free_irq(i2c->irq, i2c);
>> -fail_irq:
>> -	iounmap(i2c->twsi_base);
>> -	release_mem_region(i2c->twsi_phys, i2c->regsize);
>> -fail_region:
>> -	kfree(i2c);
>>  out:
>>  	return result;
>>  };
>> @@ -613,19 +619,24 @@ static int __devexit octeon_i2c_remove(struct platform_device *pdev)
>>  
>>  	i2c_del_adapter(&i2c->adap);
>>  	platform_set_drvdata(pdev, NULL);
>> -	free_irq(i2c->irq, i2c);
>> -	iounmap(i2c->twsi_base);
>> -	release_mem_region(i2c->twsi_phys, i2c->regsize);
>> -	kfree(i2c);
>>  	return 0;
>>  };
>>  
>> +static struct of_device_id octeon_i2c_match[] = {
>> +	{
>> +		.compatible = "cavium,octeon-3860-twsi",
>> +	},
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, octeon_i2c_match);
>> +
>>  static struct platform_driver octeon_i2c_driver = {
>>  	.probe		= octeon_i2c_probe,
>>  	.remove		= __devexit_p(octeon_i2c_remove),
>>  	.driver		= {
>>  		.owner	= THIS_MODULE,
>>  		.name	= DRV_NAME,
>> +		.of_match_table = octeon_i2c_match,
>>  	},
>>  };
>>  
>> @@ -635,4 +646,3 @@ MODULE_AUTHOR("Michael Lawnick <michael.lawnick.ext-OYasijW0DpE@public.gmane.org>");
>>  MODULE_DESCRIPTION("I2C-Bus adapter for Cavium OCTEON processors");
>>  MODULE_LICENSE("GPL");
>>  MODULE_VERSION(DRV_VERSION);
>> -MODULE_ALIAS("platform:" DRV_NAME);
> 

Re: [PATCH 1/5] i2c: Convert i2c-octeon.c to use device tree.

[David Daney]

On 03/26/2012 06:20 PM, Rob Herring wrote:
[...]
>> ---
>>
>> Should probably go via Ralf's linux-mips.org tree.
>>
>>   arch/mips/cavium-octeon/octeon-irq.c      |    2 -
>>   arch/mips/cavium-octeon/octeon-platform.c |   84 -------------------------
>>   arch/mips/include/asm/octeon/octeon.h     |    5 --
>>   drivers/i2c/busses/i2c-octeon.c           |   94 ++++++++++++++++-------------
>>   4 files changed, 52 insertions(+), 133 deletions(-)
>
> snip
>
>>
>> -	if (i2c_data == NULL) {
>> -		dev_err(i2c->dev, "no I2C frequency data\n");
>> +	/*
>> +	 * "clock-rate" is a legacy binding, the official binding is
>> +	 * "clock-frequency".  Try the official one first and then
>> +	 * fall back if it doesn't exist.
>> +	 */
>> +	data = of_get_property(pdev->dev.of_node, "clock-frequency",&len);
>> +	if (!data || len != sizeof(*data))
>> +		data = of_get_property(pdev->dev.of_node, "clock-rate",&len);
>> +	if (data&&  len == sizeof(*data)) {
>> +		i2c->twsi_freq = be32_to_cpup(data);
>
> Can't you use of_property_read_u32?

I will investigate, and use it if possible.

>
> Does the legacy binding really exist as DT support is new?
>

Yes, unfortunately some bootloaders escaped into the wild with the 
"clock-rate" binding.

> Otherwise,
>
> Acked-by: Rob Herring<rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>

Thanks,

Re: [PATCH 1/5] i2c: Convert i2c-octeon.c to use device tree.

[Wolfram Sang]

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

> >>-	if (i2c_data == NULL) {
> >>-		dev_err(i2c->dev, "no I2C frequency data\n");
> >>+	/*
> >>+	 * "clock-rate" is a legacy binding, the official binding is
> >>+	 * "clock-frequency".  Try the official one first and then
> >>+	 * fall back if it doesn't exist.
> >>+	 */
> >>+	data = of_get_property(pdev->dev.of_node, "clock-frequency",&len);
> >>+	if (!data || len != sizeof(*data))
> >>+		data = of_get_property(pdev->dev.of_node, "clock-rate",&len);
> >>+	if (data&&  len == sizeof(*data)) {
> >>+		i2c->twsi_freq = be32_to_cpup(data);
> >
> >Can't you use of_property_read_u32?
> 
> I will investigate, and use it if possible.

Any outcome?

And shouldn't the bindings be documented? Or are they only standard and
we hide the legacy one?

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

Re: [PATCH 1/5] i2c: Convert i2c-octeon.c to use device tree.

[David Daney]

On 04/18/2012 08:16 AM, Wolfram Sang wrote:
>>>> -	if (i2c_data == NULL) {
>>>> -		dev_err(i2c->dev, "no I2C frequency data\n");
>>>> +	/*
>>>> +	 * "clock-rate" is a legacy binding, the official binding is
>>>> +	 * "clock-frequency".  Try the official one first and then
>>>> +	 * fall back if it doesn't exist.
>>>> +	 */
>>>> +	data = of_get_property(pdev->dev.of_node, "clock-frequency",&len);
>>>> +	if (!data || len != sizeof(*data))
>>>> +		data = of_get_property(pdev->dev.of_node, "clock-rate",&len);
>>>> +	if (data&&   len == sizeof(*data)) {
>>>> +		i2c->twsi_freq = be32_to_cpup(data);
>>>
>>> Can't you use of_property_read_u32?
>>
>> I will investigate, and use it if possible.
>
> Any outcome?

Yes, I have implemented Rob's suggestions.  A new patch set reflecting 
this is coming soon.

>
> And shouldn't the bindings be documented? Or are they only standard and
> we hide the legacy one?
>

Yes, they are documented here:

http://patchwork.linux-mips.org/patch/3536/

look in the cavium-i2c.txt file.

Thanks,
David Daney