2012-06-11 20:57:08

by Alessandro Rubini

[permalink] [raw]
Subject: [PATCH V3 0/3] i2c-nomadik changes

V3:
- fixed according to Linusw feedback (merged the patch he posted)
- added Tested-by: Linusw on his permission

V2:
- use linux/platform_data/i2c-nomadik.h not linux/i2c-nomadik.h
- format patch with -M
- added lee jones to the Cc: list (for device tree support)
- minor rewording of the commit messages


Alessandro Rubini (3):
i2c-nomadik: move header to <linux/platform_data/i2c-nomadik.h>
i2c-nomadik: turn the platform driver to an amba driver
i2c-nomadik: depend on ARM_AMBA, not PLAT_NOMADIK

arch/arm/mach-ux500/board-mop500.c | 2 +-
arch/arm/mach-ux500/devices-common.h | 24 +---
drivers/i2c/busses/Kconfig | 5 +-
drivers/i2c/busses/i2c-nomadik.c | 145 ++++++++++----------
.../linux/platform_data/i2c-nomadik.h | 6 +-
5 files changed, 87 insertions(+), 95 deletions(-)
rename arch/arm/plat-nomadik/include/plat/i2c.h => include/linux/platform_data/i2c-nomadik.h (91%)

--
1.7.7.2


2012-06-11 20:57:12

by Alessandro Rubini

[permalink] [raw]
Subject: [PATCH V3 3/3] i2c-nomadik: depend on ARM_AMBA, not PLAT_NOMADIK

The gateware device has been used outside of the Nomadik world, using
the pci-amba bridge driver, so loosen the dependencies.

Signed-off-by: Alessandro Rubini <[email protected]>
Acked-by: Giancarlo Asnaghi <[email protected]>
Tested-by: Linus Walleij <[email protected]>
---
drivers/i2c/busses/Kconfig | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 7244c8b..e9f9c5d 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -479,10 +479,11 @@ config I2C_MXS

config I2C_NOMADIK
tristate "ST-Ericsson Nomadik/Ux500 I2C Controller"
- depends on PLAT_NOMADIK
+ depends on ARM_AMBA
help
If you say yes to this option, support will be included for the
- I2C interface from ST-Ericsson's Nomadik and Ux500 architectures.
+ I2C interface from ST-Ericsson's Nomadik and Ux500 architectures,
+ as well as the STA2X11 PCIe I/O HUB.

config I2C_NUC900
tristate "NUC900 I2C Driver"
--
1.7.7.2

2012-06-11 20:57:20

by Alessandro Rubini

[permalink] [raw]
Subject: [PATCH V3 2/3] i2c-nomadik: turn the platform driver to an amba driver

The i2c-nomadik gateware is really a PrimeCell APB device. By hosting
the driver under the amba bus we can access it more easily, for
example using the generic pci-amba driver. The patch also fixes the
mach-ux500 users, so they register an amba device instead than a
platform device.

Signed-off-by: Alessandro Rubini <[email protected]>
Acked-by: Giancarlo Asnaghi <[email protected]>
Tested-by: Linus Walleij <[email protected]>
---
arch/arm/mach-ux500/devices-common.h | 22 +----
drivers/i2c/busses/i2c-nomadik.c | 142 +++++++++++++++++----------------
2 files changed, 78 insertions(+), 86 deletions(-)

diff --git a/arch/arm/mach-ux500/devices-common.h b/arch/arm/mach-ux500/devices-common.h
index 23cf734..ecdd838 100644
--- a/arch/arm/mach-ux500/devices-common.h
+++ b/arch/arm/mach-ux500/devices-common.h
@@ -56,27 +56,15 @@ dbx500_add_uart(struct device *parent, const char *name, resource_size_t base,

struct nmk_i2c_controller;

-static inline struct platform_device *
+static inline struct amba_device *
dbx500_add_i2c(struct device *parent, int id, resource_size_t base, int irq,
struct nmk_i2c_controller *data)
{
- struct resource res[] = {
- DEFINE_RES_MEM(base, SZ_4K),
- DEFINE_RES_IRQ(irq),
- };
+ /* Conjure a name similar to what the platform device used to have */
+ char name[16];

- struct platform_device_info pdevinfo = {
- .parent = parent,
- .name = "nmk-i2c",
- .id = id,
- .res = res,
- .num_res = ARRAY_SIZE(res),
- .data = data,
- .size_data = sizeof(*data),
- .dma_mask = DMA_BIT_MASK(32),
- };
-
- return platform_device_register_full(&pdevinfo);
+ snprintf(name, sizeof(name), "nmk-i2c.%d", id);
+ return amba_apb_device_add(parent, name, base, SZ_4K, irq, 0, data, 0);
}

static inline struct amba_device *
diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 752f1fd..6db453f 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -14,7 +14,8 @@
*/
#include <linux/init.h>
#include <linux/module.h>
-#include <linux/platform_device.h>
+#include <linux/amba/bus.h>
+#include <linux/atomic.h>
#include <linux/slab.h>
#include <linux/interrupt.h>
#include <linux/i2c.h>
@@ -135,7 +136,7 @@ struct i2c_nmk_client {

/**
* struct nmk_i2c_dev - private data structure of the controller.
- * @pdev: parent platform device.
+ * @adev: parent amba device.
* @adap: corresponding I2C adapter.
* @irq: interrupt line for the controller.
* @virtbase: virtual io memory area.
@@ -149,7 +150,7 @@ struct i2c_nmk_client {
* @busy: Busy doing transfer.
*/
struct nmk_i2c_dev {
- struct platform_device *pdev;
+ struct amba_device *adev;
struct i2c_adapter adap;
int irq;
void __iomem *virtbase;
@@ -216,7 +217,7 @@ static int flush_i2c_fifo(struct nmk_i2c_dev *dev)
}
}

- dev_err(&dev->pdev->dev,
+ dev_err(&dev->adev->dev,
"flushing operation timed out giving up after %d attempts",
LOOP_ATTEMPTS);

@@ -363,7 +364,7 @@ static void setup_i2c_controller(struct nmk_i2c_dev *dev)
* and high speed (up to 3.4 Mb/s)
*/
if (dev->cfg.sm > I2C_FREQ_MODE_FAST) {
- dev_err(&dev->pdev->dev,
+ dev_err(&dev->adev->dev,
"do not support this mode defaulting to std. mode\n");
brcr2 = i2c_clk/(100000 * 2) & 0xffff;
writel((brcr1 | brcr2), dev->virtbase + I2C_BRCR);
@@ -422,7 +423,7 @@ static int read_i2c(struct nmk_i2c_dev *dev)
&dev->xfer_complete, dev->adap.timeout);

if (timeout < 0) {
- dev_err(&dev->pdev->dev,
+ dev_err(&dev->adev->dev,
"wait_for_completion_timeout "
"returned %d waiting for event\n", timeout);
status = timeout;
@@ -430,7 +431,7 @@ static int read_i2c(struct nmk_i2c_dev *dev)

if (timeout == 0) {
/* Controller timed out */
- dev_err(&dev->pdev->dev, "read from slave 0x%x timed out\n",
+ dev_err(&dev->adev->dev, "read from slave 0x%x timed out\n",
dev->cli.slave_adr);
status = -ETIMEDOUT;
}
@@ -509,7 +510,7 @@ static int write_i2c(struct nmk_i2c_dev *dev)
&dev->xfer_complete, dev->adap.timeout);

if (timeout < 0) {
- dev_err(&dev->pdev->dev,
+ dev_err(&dev->adev->dev,
"wait_for_completion_timeout "
"returned %d waiting for event\n", timeout);
status = timeout;
@@ -517,7 +518,7 @@ static int write_i2c(struct nmk_i2c_dev *dev)

if (timeout == 0) {
/* Controller timed out */
- dev_err(&dev->pdev->dev, "write to slave 0x%x timed out\n",
+ dev_err(&dev->adev->dev, "write to slave 0x%x timed out\n",
dev->cli.slave_adr);
status = -ETIMEDOUT;
}
@@ -556,7 +557,7 @@ static int nmk_i2c_xfer_one(struct nmk_i2c_dev *dev, u16 flags)
if (((i2c_sr >> 2) & 0x3) == 0x3) {
/* get the abort cause */
cause = (i2c_sr >> 4) & 0x7;
- dev_err(&dev->pdev->dev, "%s\n",
+ dev_err(&dev->adev->dev, "%s\n",
cause >= ARRAY_SIZE(abort_causes) ?
"unknown reason" :
abort_causes[cause]);
@@ -629,7 +630,7 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,

if (dev->regulator)
regulator_enable(dev->regulator);
- pm_runtime_get_sync(&dev->pdev->dev);
+ pm_runtime_get_sync(&dev->adev->dev);

clk_enable(dev->clk);

@@ -644,7 +645,7 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,

for (i = 0; i < num_msgs; i++) {
if (unlikely(msgs[i].flags & I2C_M_TEN)) {
- dev_err(&dev->pdev->dev,
+ dev_err(&dev->adev->dev,
"10 bit addressing not supported\n");

status = -EINVAL;
@@ -666,7 +667,7 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,

out:
clk_disable(dev->clk);
- pm_runtime_put_sync(&dev->pdev->dev);
+ pm_runtime_put_sync(&dev->adev->dev);
if (dev->regulator)
regulator_disable(dev->regulator);

@@ -789,7 +790,7 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)

if (dev->cli.count) {
dev->result = -EIO;
- dev_err(&dev->pdev->dev,
+ dev_err(&dev->adev->dev,
"%lu bytes still remain to be xfered\n",
dev->cli.count);
(void) init_hw(dev);
@@ -833,7 +834,7 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
dev->result = -EIO;
(void) init_hw(dev);

- dev_err(&dev->pdev->dev, "Tx Fifo Over run\n");
+ dev_err(&dev->adev->dev, "Tx Fifo Over run\n");
complete(&dev->xfer_complete);

break;
@@ -846,10 +847,10 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
case I2C_IT_RFSE:
case I2C_IT_WTSR:
case I2C_IT_STD:
- dev_err(&dev->pdev->dev, "unhandled Interrupt\n");
+ dev_err(&dev->adev->dev, "unhandled Interrupt\n");
break;
default:
- dev_err(&dev->pdev->dev, "spurious Interrupt..\n");
+ dev_err(&dev->adev->dev, "spurious Interrupt..\n");
break;
}

@@ -860,8 +861,8 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
#ifdef CONFIG_PM
static int nmk_i2c_suspend(struct device *dev)
{
- struct platform_device *pdev = to_platform_device(dev);
- struct nmk_i2c_dev *nmk_i2c = platform_get_drvdata(pdev);
+ struct amba_device *adev = to_amba_device(dev);
+ struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);

if (nmk_i2c->busy)
return -EBUSY;
@@ -898,79 +899,70 @@ static const struct i2c_algorithm nmk_i2c_algo = {
.functionality = nmk_i2c_functionality
};

-static int __devinit nmk_i2c_probe(struct platform_device *pdev)
+static atomic_t adapter_id = ATOMIC_INIT(0);
+
+static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
{
int ret = 0;
- struct resource *res;
struct nmk_i2c_controller *pdata =
- pdev->dev.platform_data;
+ adev->dev.platform_data;
struct nmk_i2c_dev *dev;
struct i2c_adapter *adap;

+ if (!pdata) {
+ dev_warn(&adev->dev, "no platform data\n");
+ return -ENODEV;
+ }
dev = kzalloc(sizeof(struct nmk_i2c_dev), GFP_KERNEL);
if (!dev) {
- dev_err(&pdev->dev, "cannot allocate memory\n");
+ dev_err(&adev->dev, "cannot allocate memory\n");
ret = -ENOMEM;
goto err_no_mem;
}
dev->busy = false;
- dev->pdev = pdev;
- platform_set_drvdata(pdev, dev);
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- ret = -ENOENT;
- goto err_no_resource;
- }
+ dev->adev = adev;
+ amba_set_drvdata(adev, dev);

- if (request_mem_region(res->start, resource_size(res),
- DRIVER_NAME "I/O region") == NULL) {
- ret = -EBUSY;
- goto err_no_region;
- }
-
- dev->virtbase = ioremap(res->start, resource_size(res));
+ dev->virtbase = ioremap(adev->res.start, resource_size(&adev->res));
if (!dev->virtbase) {
ret = -ENOMEM;
goto err_no_ioremap;
}

- dev->irq = platform_get_irq(pdev, 0);
+ dev->irq = adev->irq[0];
ret = request_irq(dev->irq, i2c_irq_handler, 0,
DRIVER_NAME, dev);
if (ret) {
- dev_err(&pdev->dev, "cannot claim the irq %d\n", dev->irq);
+ dev_err(&adev->dev, "cannot claim the irq %d\n", dev->irq);
goto err_irq;
}

- dev->regulator = regulator_get(&pdev->dev, "v-i2c");
+ dev->regulator = regulator_get(&adev->dev, "v-i2c");
if (IS_ERR(dev->regulator)) {
- dev_warn(&pdev->dev, "could not get i2c regulator\n");
+ dev_warn(&adev->dev, "could not get i2c regulator\n");
dev->regulator = NULL;
}

- pm_suspend_ignore_children(&pdev->dev, true);
- pm_runtime_enable(&pdev->dev);
+ pm_suspend_ignore_children(&adev->dev, true);

- dev->clk = clk_get(&pdev->dev, NULL);
+ dev->clk = clk_get(&adev->dev, NULL);
if (IS_ERR(dev->clk)) {
- dev_err(&pdev->dev, "could not get i2c clock\n");
+ dev_err(&adev->dev, "could not get i2c clock\n");
ret = PTR_ERR(dev->clk);
goto err_no_clk;
}

adap = &dev->adap;
- adap->dev.parent = &pdev->dev;
+ adap->dev.parent = &adev->dev;
adap->owner = THIS_MODULE;
adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
adap->algo = &nmk_i2c_algo;
adap->timeout = pdata->timeout ? msecs_to_jiffies(pdata->timeout) :
msecs_to_jiffies(20000);
+ adap->nr = atomic_read(&adapter_id);
snprintf(adap->name, sizeof(adap->name),
- "Nomadik I2C%d at %lx", pdev->id, (unsigned long)res->start);
-
- /* fetch the controller id */
- adap->nr = pdev->id;
+ "Nomadik I2C%d at %pR", adap->nr, &adev->res);
+ atomic_inc(&adapter_id);

/* fetch the controller configuration from machine */
dev->cfg.clk_freq = pdata->clk_freq;
@@ -981,16 +973,18 @@ static int __devinit nmk_i2c_probe(struct platform_device *pdev)

i2c_set_adapdata(adap, dev);

- dev_info(&pdev->dev,
+ dev_info(&adev->dev,
"initialize %s on virtual base %p\n",
adap->name, dev->virtbase);

ret = i2c_add_numbered_adapter(adap);
if (ret) {
- dev_err(&pdev->dev, "failed to add adapter\n");
+ dev_err(&adev->dev, "failed to add adapter\n");
goto err_add_adap;
}

+ pm_runtime_put(&adev->dev);
+
return 0;

err_add_adap:
@@ -998,25 +992,21 @@ static int __devinit nmk_i2c_probe(struct platform_device *pdev)
err_no_clk:
if (dev->regulator)
regulator_put(dev->regulator);
- pm_runtime_disable(&pdev->dev);
free_irq(dev->irq, dev);
err_irq:
iounmap(dev->virtbase);
err_no_ioremap:
- release_mem_region(res->start, resource_size(res));
- err_no_region:
- platform_set_drvdata(pdev, NULL);
- err_no_resource:
+ amba_set_drvdata(adev, NULL);
kfree(dev);
err_no_mem:

return ret;
}

-static int __devexit nmk_i2c_remove(struct platform_device *pdev)
+static int nmk_i2c_remove(struct amba_device *adev)
{
- struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- struct nmk_i2c_dev *dev = platform_get_drvdata(pdev);
+ struct resource *res = &adev->res;
+ struct nmk_i2c_dev *dev = amba_get_drvdata(adev);

i2c_del_adapter(&dev->adap);
flush_i2c_fifo(dev);
@@ -1031,31 +1021,46 @@ static int __devexit nmk_i2c_remove(struct platform_device *pdev)
clk_put(dev->clk);
if (dev->regulator)
regulator_put(dev->regulator);
- pm_runtime_disable(&pdev->dev);
- platform_set_drvdata(pdev, NULL);
+ pm_runtime_disable(&adev->dev);
+ amba_set_drvdata(adev, NULL);
kfree(dev);

return 0;
}

-static struct platform_driver nmk_i2c_driver = {
- .driver = {
+static struct amba_id nmk_i2c_ids[] = {
+ {
+ .id = 0x00180024,
+ .mask = 0x00ffffff,
+ },
+ {
+ .id = 0x00380024,
+ .mask = 0x00ffffff,
+ },
+ {},
+};
+
+MODULE_DEVICE_TABLE(amba, nmk_i2c_ids);
+
+static struct amba_driver nmk_i2c_driver = {
+ .drv = {
.owner = THIS_MODULE,
.name = DRIVER_NAME,
.pm = &nmk_i2c_pm,
},
+ .id_table = nmk_i2c_ids,
.probe = nmk_i2c_probe,
- .remove = __devexit_p(nmk_i2c_remove),
+ .remove = nmk_i2c_remove,
};

static int __init nmk_i2c_init(void)
{
- return platform_driver_register(&nmk_i2c_driver);
+ return amba_driver_register(&nmk_i2c_driver);
}

static void __exit nmk_i2c_exit(void)
{
- platform_driver_unregister(&nmk_i2c_driver);
+ amba_driver_unregister(&nmk_i2c_driver);
}

subsys_initcall(nmk_i2c_init);
@@ -1064,4 +1069,3 @@ module_exit(nmk_i2c_exit);
MODULE_AUTHOR("Sachin Verma, Srinidhi KASAGAR");
MODULE_DESCRIPTION("Nomadik/Ux500 I2C driver");
MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:" DRIVER_NAME);
--
1.7.7.2

2012-06-11 20:57:07

by Alessandro Rubini

[permalink] [raw]
Subject: [PATCH V3 1/3] i2c-nomadik: move header to <linux/platform_data/i2c-nomadik.h>

The header and driver are only used by arm/mach-u8500 (and potentially
arm/mach-nomadik), but the STA2X11 I/O Hub exports on PCIe a number of
devices, including i2c-nomadik. This patch allows compilation of the
driver under x86.

Signed-off-by: Alessandro Rubini <[email protected]>
Acked-by: Giancarlo Asnaghi <[email protected]>
Tested-by: Linus Walleij <[email protected]>
---
arch/arm/mach-ux500/board-mop500.c | 2 +-
arch/arm/mach-ux500/devices-common.h | 2 +-
drivers/i2c/busses/i2c-nomadik.c | 3 +--
.../linux/platform_data/i2c-nomadik.h | 6 +++---
4 files changed, 6 insertions(+), 7 deletions(-)
rename arch/arm/plat-nomadik/include/plat/i2c.h => include/linux/platform_data/i2c-nomadik.h (91%)

diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c
index 9c74ac5..0bf24bb 100644
--- a/arch/arm/mach-ux500/board-mop500.c
+++ b/arch/arm/mach-ux500/board-mop500.c
@@ -15,6 +15,7 @@
#include <linux/platform_device.h>
#include <linux/io.h>
#include <linux/i2c.h>
+#include <linux/platform_data/i2c-nomadik.h>
#include <linux/gpio.h>
#include <linux/amba/bus.h>
#include <linux/amba/pl022.h>
@@ -39,7 +40,6 @@
#include <asm/mach/arch.h>
#include <asm/hardware/gic.h>

-#include <plat/i2c.h>
#include <plat/ste_dma40.h>
#include <plat/gpio-nomadik.h>

diff --git a/arch/arm/mach-ux500/devices-common.h b/arch/arm/mach-ux500/devices-common.h
index 6e47065..23cf734 100644
--- a/arch/arm/mach-ux500/devices-common.h
+++ b/arch/arm/mach-ux500/devices-common.h
@@ -12,7 +12,7 @@
#include <linux/dma-mapping.h>
#include <linux/sys_soc.h>
#include <linux/amba/bus.h>
-#include <plat/i2c.h>
+#include <linux/platform_data/i2c-nomadik.h>
#include <mach/crypto-ux500.h>

struct spi_master_cntlr;
diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 5267ab9..752f1fd 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -23,8 +23,7 @@
#include <linux/io.h>
#include <linux/regulator/consumer.h>
#include <linux/pm_runtime.h>
-
-#include <plat/i2c.h>
+#include <linux/platform_data/i2c-nomadik.h>

#define DRIVER_NAME "nmk-i2c"

diff --git a/arch/arm/plat-nomadik/include/plat/i2c.h b/include/linux/platform_data/i2c-nomadik.h
similarity index 91%
rename from arch/arm/plat-nomadik/include/plat/i2c.h
rename to include/linux/platform_data/i2c-nomadik.h
index 8ba70ff..c2303c3 100644
--- a/arch/arm/plat-nomadik/include/plat/i2c.h
+++ b/include/linux/platform_data/i2c-nomadik.h
@@ -5,8 +5,8 @@
* it under the terms of the GNU General Public License version 2, as
* published by the Free Software Foundation.
*/
-#ifndef __PLAT_I2C_H
-#define __PLAT_I2C_H
+#ifndef __PDATA_I2C_NOMADIK_H
+#define __PDATA_I2C_NOMADIK_H

enum i2c_freq_mode {
I2C_FREQ_MODE_STANDARD, /* up to 100 Kb/s */
@@ -36,4 +36,4 @@ struct nmk_i2c_controller {
enum i2c_freq_mode sm;
};

-#endif /* __PLAT_I2C_H */
+#endif /* __PDATA_I2C_NOMADIK_H */
--
1.7.7.2

2012-06-11 21:37:19

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH V3 0/3] i2c-nomadik changes

On Mon, Jun 11, 2012 at 10:56 PM, Alessandro Rubini <[email protected]> wrote:

> V3:
> ? ? ? ?- fixed according to Linusw feedback (merged the patch he posted)
> ? ? ? ?- added Tested-by: Linusw on his permission

I'm happy with this version, Wolfram if it looks OK to you,
can you please merge this into the I2C tree?

Yours,
Linus Walleij

2012-06-14 08:05:27

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH V3 2/3] i2c-nomadik: turn the platform driver to an amba driver

On Monday 11 June 2012, Alessandro Rubini wrote:
> The i2c-nomadik gateware is really a PrimeCell APB device. By hosting
> the driver under the amba bus we can access it more easily, for
> example using the generic pci-amba driver. The patch also fixes the
> mach-ux500 users, so they register an amba device instead than a
> platform device.
>
> Signed-off-by: Alessandro Rubini <[email protected]>
> Acked-by: Giancarlo Asnaghi <[email protected]>
> Tested-by: Linus Walleij <[email protected]>
> ---
> arch/arm/mach-ux500/devices-common.h | 22 +----
> drivers/i2c/busses/i2c-nomadik.c | 142 +++++++++++++++++----------------
> 2 files changed, 78 insertions(+), 86 deletions(-)

You change only one half of ux500 here: the part where the device gets defined
statically, but not not the definition in the device-tree.

I think all you need to do is mark the device compatible with primecell
and add an ID if necessary, but we should definitely make sure that that
path still works, because the one you patch is going away.

Arnd

2012-06-14 08:18:44

by Alessandro Rubini

[permalink] [raw]
Subject: Re: [PATCH V3 2/3] i2c-nomadik: turn the platform driver to an amba driver

> You change only one half of ux500 here: the part where the device
> gets defined statically, but not not the definition in the
> device-tree.

Yes, I'm aware. That's why I added Lee Jones as Cc:, on Linusw's
suggestion. Lee told me to go ahead and he'll fix the DT stuff.
Lee?

thanks
/alessandro

2012-06-14 08:39:13

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH V3 2/3] i2c-nomadik: turn the platform driver to an amba driver

On 14/06/12 09:17, Alessandro Rubini wrote:
>> You change only one half of ux500 here: the part where the device
>> gets defined statically, but not not the definition in the
>> device-tree.
>
> Yes, I'm aware. That's why I added Lee Jones as Cc:, on Linusw's
> suggestion. Lee told me to go ahead and he'll fix the DT stuff.

I did?

I have already DT:ed this driver. There are 3 patches on the MLs currently.

> [PATCH 1/3] i2c: Add Device Tree support to the Nomadik I2C driver
> [PATCH 2/3] ARM: ux500: Add i2c configurations to the Device Tree for DB8500 based devices
> [PATCH 3/3] Documentation: Device Tree binding information for i2c-nomadik driver

I'm more than happy for you to base your patches on these and make the
necessary changes. Off hand, I think the only changes you'll need to
make is the probing from DT itself.

So something like:

> --- a/arch/arm/boot/dts/db8500.dtsi
> +++ b/arch/arm/boot/dts/db8500.dtsi
> @@ -449,7 +449,7 @@
> };
>
> i2c@80004000 {
> - compatible = "stericsson,db8500-i2c", "st,nomadik-i2c";
> + compatible = "st,nomadik-i2c", "arm,primecell";
> reg = <0x80004000 0x1000>;
> interrupts = <0 21 0x4>;
> #address-cells = <1>;

... for each of the controllers.

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2012-06-14 17:26:05

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH V3 2/3] i2c-nomadik: turn the platform driver to an amba driver

On Thu, Jun 14, 2012 at 10:39 AM, Lee Jones <[email protected]> wrote:
> On 14/06/12 09:17, Alessandro Rubini wrote:
>>>
>>> You change only one half of ux500 here: the part where the device
>>> gets defined statically, but not not the definition in the
>>> device-tree.
>>
>> Yes, I'm aware. That's why I added Lee Jones as Cc:, on Linusw's
>> suggestion. ?Lee told me to go ahead and he'll fix the DT stuff.
>
> I did?

Yes :-)

> I have already DT:ed this driver. There are 3 patches on the MLs currently.
>
>> [PATCH 1/3] i2c: Add Device Tree support to the Nomadik I2C driver
>> [PATCH 2/3] ARM: ux500: Add i2c configurations to the Device Tree for
>> DB8500 based devices
>> [PATCH 3/3] Documentation: Device Tree binding information for i2c-nomadik
>> driver
>
> I'm more than happy for you to base your patches on these and make the
> necessary changes. Off hand, I think the only changes you'll need to make is
> the probing from DT itself.

Depends on whether this patch or yours go in first I guess. Which
is up to Wolfram. What I know is that Alessandros patches are
ready to merge IMO.

> So something like:
>
> - compatible = "stericsson,db8500-i2c", "st,nomadik-i2c";
> + compatible = "st,nomadik-i2c", "arm,primecell";

Yep that should do it.

Which brings me to the fear of what happens the day we have
deployed device trees out there and just cannot change things
like this easily :-/

Yours,
Linus Walleij