2012-05-30 17:46:57

by Alessandro Rubini

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

The patch set turns a platform device into an amba device. Its users,
i.e. mach-ux500, are converted too. The set has no external
dependencies, although is has no visible effect until other patches
are accepted.

Reasoning:

In the STA2X11 I/O hub we are exporting a number of AMBA peripherals
to the PCI world. Using a generic pci-amba driver there is no further
code for each device, provided they are already registered under the
AMBA bus. I already submitted the generic bridge
(https://lkml.org/lkml/2012/5/28/194).

Status of this patch set:

The users of the driver (i.e. mach-ux600) compile properly, but I
couldn't test on any board. The driver, as is, works on x86 under the
PCI bridge, with the not-yet-upstream clock framework and platform
data for sta2x11 devices. However, this set is not introducing any
dead code as it only adds more flexibility. The big part of it is a
massive s/platform_device/amba_device/ and s/pdev/adev/ .

The cell-id I used in the table are the one for ux500 (only as far as
I know from the stn-8815 manuals) and the one for STA2X11.

Alessandro Rubini (3):
i2c-nomadik: move header from <plat/i2c.h> to <linux/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 | 23 +----
arch/arm/plat-nomadik/include/plat/i2c.h | 39 --------
drivers/i2c/busses/Kconfig | 5 +-
drivers/i2c/busses/i2c-nomadik.c | 140 +++++++++++++++---------------
include/linux/i2c-nomadik.h | 39 ++++++++
6 files changed, 116 insertions(+), 132 deletions(-)
delete mode 100644 arch/arm/plat-nomadik/include/plat/i2c.h
create mode 100644 include/linux/i2c-nomadik.h

--
1.7.7.2


2012-05-30 17:46:41

by Alessandro Rubini

[permalink] [raw]
Subject: [PATCH RFC 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 to register an AMBA device, not a platform device.

Signed-off-by: Alessandro Rubini <[email protected]>
Acked-by: Giancarlo Asnaghi <[email protected]>
Cc: Srinidhi Kasagar <[email protected]>
Cc: [email protected]
Cc: Linus Walleij <[email protected]>
---
arch/arm/mach-ux500/devices-common.h | 21 +-----
drivers/i2c/busses/i2c-nomadik.c | 137 +++++++++++++++++-----------------
2 files changed, 71 insertions(+), 87 deletions(-)

diff --git a/arch/arm/mach-ux500/devices-common.h b/arch/arm/mach-ux500/devices-common.h
index 50d1292..d73fed5 100644
--- a/arch/arm/mach-ux500/devices-common.h
+++ b/arch/arm/mach-ux500/devices-common.h
@@ -56,27 +56,12 @@ 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),
- };
-
- 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);
+ return amba_apb_device_add(parent, "nmk-i2c", 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 09b7589..292e72c 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -14,7 +14,7 @@
*/
#include <linux/init.h>
#include <linux/module.h>
-#include <linux/platform_device.h>
+#include <linux/amba/bus.h>
#include <linux/slab.h>
#include <linux/interrupt.h>
#include <linux/i2c.h>
@@ -135,7 +135,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 +149,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 +216,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 +363,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 +422,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 +430,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 +509,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 +517,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 +556,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 +629,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 +644,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 +666,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 +789,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 +833,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 +846,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 +860,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 +898,67 @@ static const struct i2c_algorithm nmk_i2c_algo = {
.functionality = nmk_i2c_functionality
};

-static int __devinit nmk_i2c_probe(struct platform_device *pdev)
+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);
+ pm_runtime_enable(&adev->dev);

- 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);
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);

/* fetch the controller configuration from machine */
dev->cfg.clk_freq = pdata->clk_freq;
@@ -981,13 +969,13 @@ 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;
}

@@ -998,25 +986,22 @@ 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);
+ pm_runtime_disable(&adev->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 +1016,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 = 0x00018024,
+ .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 +1064,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-05-30 17:46:45

by Alessandro Rubini

[permalink] [raw]
Subject: [PATCH RFC 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]>
---
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-05-30 17:46:56

by Alessandro Rubini

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

The header and driver were only used by arm/mach-nomadik and
arm/mach-u8500. 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]>
---
arch/arm/mach-ux500/board-mop500.c | 2 +-
arch/arm/mach-ux500/devices-common.h | 2 +-
arch/arm/plat-nomadik/include/plat/i2c.h | 39 ------------------------------
drivers/i2c/busses/i2c-nomadik.c | 3 +-
include/linux/i2c-nomadik.h | 39 ++++++++++++++++++++++++++++++
5 files changed, 42 insertions(+), 43 deletions(-)
delete mode 100644 arch/arm/plat-nomadik/include/plat/i2c.h
create mode 100644 include/linux/i2c-nomadik.h

diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c
index fba8ade..2117683 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/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..50d1292 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/i2c-nomadik.h>
#include <mach/crypto-ux500.h>

struct spi_master_cntlr;
diff --git a/arch/arm/plat-nomadik/include/plat/i2c.h b/arch/arm/plat-nomadik/include/plat/i2c.h
deleted file mode 100644
index 8ba70ff..0000000
--- a/arch/arm/plat-nomadik/include/plat/i2c.h
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * Copyright (C) 2009 ST-Ericsson
- *
- * 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.
- */
-#ifndef __PLAT_I2C_H
-#define __PLAT_I2C_H
-
-enum i2c_freq_mode {
- I2C_FREQ_MODE_STANDARD, /* up to 100 Kb/s */
- I2C_FREQ_MODE_FAST, /* up to 400 Kb/s */
- I2C_FREQ_MODE_HIGH_SPEED, /* up to 3.4 Mb/s */
- I2C_FREQ_MODE_FAST_PLUS, /* up to 1 Mb/s */
-};
-
-/**
- * struct nmk_i2c_controller - client specific controller configuration
- * @clk_freq: clock frequency for the operation mode
- * @slsu: Slave data setup time in ns.
- * The needed setup time for three modes of operation
- * are 250ns, 100ns and 10ns respectively thus leading
- * to the values of 14, 6, 2 for a 48 MHz i2c clk
- * @tft: Tx FIFO Threshold in bytes
- * @rft: Rx FIFO Threshold in bytes
- * @timeout Slave response timeout(ms)
- * @sm: speed mode
- */
-struct nmk_i2c_controller {
- unsigned long clk_freq;
- unsigned short slsu;
- unsigned char tft;
- unsigned char rft;
- int timeout;
- enum i2c_freq_mode sm;
-};
-
-#endif /* __PLAT_I2C_H */
diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 5267ab9..09b7589 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/i2c-nomadik.h>

#define DRIVER_NAME "nmk-i2c"

diff --git a/include/linux/i2c-nomadik.h b/include/linux/i2c-nomadik.h
new file mode 100644
index 0000000..9350fe9
--- /dev/null
+++ b/include/linux/i2c-nomadik.h
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2009 ST-Ericsson
+ *
+ * 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.
+ */
+#ifndef __LINUX_I2C_NOMADIK_H
+#define __LINUX_I2C_NOMADIK_H
+
+enum i2c_freq_mode {
+ I2C_FREQ_MODE_STANDARD, /* up to 100 Kb/s */
+ I2C_FREQ_MODE_FAST, /* up to 400 Kb/s */
+ I2C_FREQ_MODE_HIGH_SPEED, /* up to 3.4 Mb/s */
+ I2C_FREQ_MODE_FAST_PLUS, /* up to 1 Mb/s */
+};
+
+/**
+ * struct nmk_i2c_controller - client specific controller configuration
+ * @clk_freq: clock frequency for the operation mode
+ * @slsu: Slave data setup time in ns.
+ * The needed setup time for three modes of operation
+ * are 250ns, 100ns and 10ns respectively thus leading
+ * to the values of 14, 6, 2 for a 48 MHz i2c clk
+ * @tft: Tx FIFO Threshold in bytes
+ * @rft: Rx FIFO Threshold in bytes
+ * @timeout Slave response timeout(ms)
+ * @sm: speed mode
+ */
+struct nmk_i2c_controller {
+ unsigned long clk_freq;
+ unsigned short slsu;
+ unsigned char tft;
+ unsigned char rft;
+ int timeout;
+ enum i2c_freq_mode sm;
+};
+
+#endif /* __LINUX_I2C_NOMADIK_H */
--
1.7.7.2

2012-05-31 01:47:16

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH RFC 1/3] i2c-nomadik: move header from <plat/i2c.h> to <linux/i2c-nomadik.h>

On Thu, May 31, 2012 at 1:45 AM, Alessandro Rubini <[email protected]> wrote:

> The header and driver were only used by arm/mach-nomadik and
> arm/mach-u8500. ?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]>

The new place to put pure platform data headers is:
include/linux/platform_data/i2c-nomadik.h so please use this.

Pls use git format-patch -M so we get a nice movement patch
without all the -/+ lines. (No big deal though.)

Yours,
Linus Walleij

2012-05-31 01:51:48

by Linus Walleij

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

On Thu, May 31, 2012 at 1:45 AM, Alessandro Rubini <[email protected]> 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 to register an AMBA device, not a platform device.
>
> Signed-off-by: Alessandro Rubini <[email protected]>
> Acked-by: Giancarlo Asnaghi <[email protected]>
> Cc: Srinidhi Kasagar <[email protected]>
> Cc: [email protected]
> Cc: Linus Walleij <[email protected]>

This will affect Lee Jones' device tree bindings for the same device.

You will need some add-on patch to the db8500 device tree moving this
over to the AMBA (PrimeCell) bus abstraction.

Lee can you test this and help Alessandro add the proper chunks to
the device tree as well?

Apart from that this looks good to me, I'll test it on the MOP500
as soon as I can.

Yours,
Linus Walleij

2012-05-31 01:55:39

by Linus Walleij

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

On Thu, May 31, 2012 at 1:45 AM, Alessandro Rubini <[email protected]> wrote:

> In the STA2X11 I/O hub we are exporting a number of AMBA peripherals
> to the PCI world. Using a generic pci-amba driver there is no further
> code for each device, provided they are already registered under the
> AMBA bus. ?I already submitted the generic bridge
> (https://lkml.org/lkml/2012/5/28/194).

Yeah I buy this concept, it's a good way to share this code.

> The cell-id I used in the table are the one for ux500 (only as far as
> I know from the stn-8815 manuals) and the one for STA2X11.

I've tried a few times to get this driver working on the S8815 Calao
dongle version of Nomadik 8815, but failed miserably (I managed
to get DMA and MMC working, which I will likely try to merge.)
I'm still trying to figure out why sometimes now and then.

So this doesn't make anything worse anyway.

Thanks for this nice patch set!

Yours,
Linus Walleij

2012-05-31 08:38:56

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH RFC 1/3] i2c-nomadik: move header from <plat/i2c.h> to <linux/i2c-nomadik.h>

On 31/05/12 02:47, Linus Walleij wrote:
> On Thu, May 31, 2012 at 1:45 AM, Alessandro Rubini<[email protected]> wrote:
>
>> The header and driver were only used by arm/mach-nomadik and
>> arm/mach-u8500. 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]>
>
> The new place to put pure platform data headers is:
> include/linux/platform_data/i2c-nomadik.h so please use this.

Aha, didn't see your reply before making mine.

> Pls use git format-patch -M so we get a nice movement patch
> without all the -/+ lines. (No big deal though.)

Agreed, useful for when actually submitting.


--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

2012-05-31 08:45:48

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH RFC 1/3] i2c-nomadik: move header from <plat/i2c.h> to <linux/i2c-nomadik.h>

On 30/05/12 18:45, Alessandro Rubini wrote:
> The header and driver were only used by arm/mach-nomadik and
> arm/mach-u8500. 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]>

> -
> -#endif /* __PLAT_I2C_H */
> diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
> index 5267ab9..09b7589 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/i2c-nomadik.h>
>
> #define DRIVER_NAME "nmk-i2c"
>
> diff --git a/include/linux/i2c-nomadik.h b/include/linux/i2c-nomadik.h
> new file mode 100644
> index 0000000..9350fe9
> --- /dev/null
> +++ b/include/linux/i2c-nomadik.h
> @@ -0,0 +1,39 @@
> +/*
> + * Copyright (C) 2009 ST-Ericsson
> + *
> + * 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.
> + */
> +#ifndef __LINUX_I2C_NOMADIK_H
> +#define __LINUX_I2C_NOMADIK_H
> +
> +enum i2c_freq_mode {
> + I2C_FREQ_MODE_STANDARD, /* up to 100 Kb/s */
> + I2C_FREQ_MODE_FAST, /* up to 400 Kb/s */
> + I2C_FREQ_MODE_HIGH_SPEED, /* up to 3.4 Mb/s */
> + I2C_FREQ_MODE_FAST_PLUS, /* up to 1 Mb/s */
> +};
> +
> +/**
> + * struct nmk_i2c_controller - client specific controller configuration
> + * @clk_freq: clock frequency for the operation mode
> + * @slsu: Slave data setup time in ns.
> + * The needed setup time for three modes of operation
> + * are 250ns, 100ns and 10ns respectively thus leading
> + * to the values of 14, 6, 2 for a 48 MHz i2c clk
> + * @tft: Tx FIFO Threshold in bytes
> + * @rft: Rx FIFO Threshold in bytes
> + * @timeout Slave response timeout(ms)
> + * @sm: speed mode
> + */
> +struct nmk_i2c_controller {
> + unsigned long clk_freq;
> + unsigned short slsu;
> + unsigned char tft;
> + unsigned char rft;
> + int timeout;
> + enum i2c_freq_mode sm;
> +};
> +
> +#endif /* __LINUX_I2C_NOMADIK_H */

How about moving it to include/linux/platform_data so that we're
not filling include/linux with a bunch of driver specific includes?

--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

2012-05-31 08:40:32

by Ben Dooks

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

On 31/05/12 02:51, Linus Walleij wrote:
> On Thu, May 31, 2012 at 1:45 AM, Alessandro Rubini<[email protected]> 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 to register an AMBA device, not a platform device.
>>
>> Signed-off-by: Alessandro Rubini<[email protected]>
>> Acked-by: Giancarlo Asnaghi<[email protected]>
>> Cc: Srinidhi Kasagar<[email protected]>
>> Cc: [email protected]
>> Cc: Linus Walleij<[email protected]>
>
> This will affect Lee Jones' device tree bindings for the same device.
>
> You will need some add-on patch to the db8500 device tree moving this
> over to the AMBA (PrimeCell) bus abstraction.

What does AMBA win us over the platform device here?

--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

2012-05-31 10:16:56

by Linus Walleij

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

On Thu, May 31, 2012 at 4:37 PM, Ben Dooks <[email protected]> wrote:
> On 31/05/12 02:51, Linus Walleij wrote:
>>
>> You will need some add-on patch to the db8500 device tree moving this
>> over to the AMBA (PrimeCell) bus abstraction.
>
> What does AMBA win us over the platform device here?

In Alessandro's case I guess it's that he can use his PCI-to-AMBA wrapper, and
there is no similar PCI-to platformdevice wrapper. So it saves some work
on his part.

But since the device has PrimeCell ID registers, this is really the
right thing to
do anyway, since autodetecting from hardware is allways NiceToDo(TM).
Else ST and us shouldn't have put the magic numbers (0xB105F00D) in there
anyway.

Yours,
Linus Walleij

2012-05-31 18:02:55

by Alessandro Rubini

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

> But since the device has PrimeCell ID registers, this is really the
> right thing to do anyway, since autodetecting from hardware is
> allways NiceToDo(TM). Else ST and us shouldn't have put the magic
> numbers (0xB105F00D) in there anyway.

Exactly. The pci-amba.c driver just says "here's an amba thing at
my BAR0 address", and then it is idenfitied automatically. There is
also automatic modalias support in place.

So I'll resubmit in the right way (-M, include/linux/platform_data)
as soon as I have information for ftd fixup.

Thanks
/alessandro

2012-06-01 01:53:18

by Linus Walleij

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

On Fri, Jun 1, 2012 at 2:02 AM, Alessandro Rubini <[email protected]> wrote:

> So I'll resubmit in the right way (-M, include/linux/platform_data)
> as soon as I have information for ftd fixup.

OK if nothing happens you can probably just go in and patch:
arch/arm/boot/dts/db8500.dtsi

I suspect it's just the compatible= that needs to be changed to
"arm,primecell" or just have that string added to the others that
are already there for each i2c device entry.

Yours,
Linus Walleij

2012-06-01 02:24:15

by Lee Jones

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

On 01/06/12 09:53, Linus Walleij wrote:
> On Fri, Jun 1, 2012 at 2:02 AM, Alessandro Rubini<[email protected]> wrote:
>
>> So I'll resubmit in the right way (-M, include/linux/platform_data)
>> as soon as I have information for ftd fixup.
>
> OK if nothing happens you can probably just go in and patch:
> arch/arm/boot/dts/db8500.dtsi
>
> I suspect it's just the compatible= that needs to be changed to
> "arm,primecell" or just have that string added to the others that
> are already there for each i2c device entry.

I already have a patch to enabled I2C. If this patch goes in, I will fix
it up to register itself as a Primecell device instead. No problem.

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