On 01/24/2011 05:58 PM, Shinya Kuribayashi wrote:
Hi,
On 1/15/2011 4:27 AM, dirk.brandewie@xxxxxxxxx wrote:
From: Dirk Brandewie<dirk.brandewie@xxxxxxxxx>
This patch splits i2c-designware.c into a core library and associated
header file and two bus specific parts for platform bus and PCI bus.
Signed-off-by: Dirk Brandewie<dirk.brandewie@xxxxxxxxx>
---
drivers/i2c/busses/Makefile | 4 +
drivers/i2c/busses/i2c-designware-core.c | 541 ++++++++++++++++++++++++++++++
drivers/i2c/busses/i2c-designware-core.h | 204 +++++++++++
drivers/i2c/busses/i2c-designware-pci.c | 183 ++++++++++
drivers/i2c/busses/i2c-designware-plat.c | 198 +++++++++++
5 files changed, 1130 insertions(+), 0 deletions(-)
create mode 100644 drivers/i2c/busses/i2c-designware-core.c
create mode 100644 drivers/i2c/busses/i2c-designware-core.h
create mode 100644 drivers/i2c/busses/i2c-designware-pci.c
create mode 100644 drivers/i2c/busses/i2c-designware-plat.c
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 0b9aa00..e1c0774 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -34,6 +34,10 @@ obj-$(CONFIG_I2C_BLACKFIN_TWI) += i2c-bfin-twi.o
obj-$(CONFIG_I2C_CPM) += i2c-cpm.o
obj-$(CONFIG_I2C_DAVINCI) += i2c-davinci.o
obj-$(CONFIG_I2C_DESIGNWARE) += i2c-designware.o
+obj-$(CONFIG_I2C_DESIGNWARE_PCI) += i2c_dw_pci.o
+i2c_dw_pci-objs := i2c-designware-pci.o i2c-designware-core.o
+obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM) += i2c_dw_plat.o
+i2c_dw_plat-objs := i2c-designware-plat.o i2c-designware-core.o
obj-$(CONFIG_I2C_GPIO) += i2c-gpio.o
obj-$(CONFIG_I2C_HIGHLANDER) += i2c-highlander.o
obj-$(CONFIG_I2C_IBM_IIC) += i2c-ibm_iic.o
It would be nice (and consistent) we could name these new modules as
simply 'i2c-designware-pci.ko' and 'i2c-designware-plat.ko'. Is it
possible using<modulename>-objs build machinery?
I will look into it, I just used a name that made sense (to me) at the time :-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
new file mode 100644
index 0000000..9dca409
--- /dev/null
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -0,0 +1,541 @@
[...]
I've checked the diff between an existing i2c-designware.c and this
-core.c, and found that some unnecessary changes were made along with
-core.c duplication, which should be avoided as this patch is not
generated using git rename dection and hence hard to review.
Will fix when I re-roll the patch set
--- drivers/i2c/busses/i2c-designware.c 2011-01-22 23:07:42.571983857 +0900
+++ drivers/i2c/busses/i2c-designware-core.c 2011-01-23 10:29:34.178022264 +0900
@@ -170,57 +64,7 @@ static char *abort_sources[] = {
"lost arbitration",
};
[...]
-
-static u32
+u32
i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset)
{
/*
@@ -259,7 +103,7 @@ i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL,
return (ic_clk * (tSYMBOL + tf) + 5000) / 10000 - 3 + offset;
}
-static u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset)
+u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset)
{
/*
* Conditional expression:
i2c_dw_scl_[hl]cnt() can be static.
Yep
@@ -330,11 +175,13 @@ static void i2c_dw_init(struct dw_i2c_de
DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
writel(ic_con, dev->base + DW_IC_CON);
}
+EXPORT_SYMBOL(i2c_dw_init);
Don't have to make it EXPORT_SYMBOL()-ed. How about declaring it
with extern in -core.h ? Am I missing something?
Nope I will fix
/*
* Waiting for bus not busy
*/
-static int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
+int
+i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
{
int timeout = TIMEOUT;
static.
@@ -350,7 +197,8 @@ static int i2c_dw_wait_bus_not_busy(stru
return 0;
}
-static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
+void
+i2c_dw_xfer_init(struct dw_i2c_dev *dev)
{
struct i2c_msg *msgs = dev->msgs;
u32 ic_con;
static.
@@ -382,7 +230,7 @@ static void i2c_dw_xfer_init(struct dw_i
* messages into the tx buffer. Even if the size of i2c_msg data is
* longer than the size of the tx buffer, it handles everything.
*/
-static void
+void
i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
{
struct i2c_msg *msgs = dev->msgs;
static.
@@ -456,7 +304,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
writel(intr_mask, dev->base + DW_IC_INTR_MASK);
}
-static void
+void
i2c_dw_read(struct dw_i2c_dev *dev)
{
struct i2c_msg *msgs = dev->msgs;
static.
@@ -492,7 +340,7 @@ i2c_dw_read(struct dw_i2c_dev *dev)
}
}
-static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
+int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
{
unsigned long abort_source = dev->abort_source;
int i;
static.
@@ -518,7 +366,7 @@ static int i2c_dw_handle_tx_abort(struct
/*
* Prepare controller for a transaction and call i2c_dw_xfer_msg
*/
-static int
+int
i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
{
struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
@@ -580,8 +428,9 @@ done:
return ret;
}
+EXPORT_SYMBOL(i2c_dw_xfer);
-static u32 i2c_dw_func(struct i2c_adapter *adap)
+u32 i2c_dw_func(struct i2c_adapter *adap)
{
return I2C_FUNC_I2C |
I2C_FUNC_10BIT_ADDR |
@@ -590,8 +439,9 @@ static u32 i2c_dw_func(struct i2c_adapte
I2C_FUNC_SMBUS_WORD_DATA |
I2C_FUNC_SMBUS_I2C_BLOCK;
}
+EXPORT_SYMBOL(i2c_dw_func);
Two functions above, i2c_dw_xfer() and i2c_dw_func(), shouldn't be
EXPORT_SYMBOL()-ed, either.
Ack
-static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
+u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
{
u32 stat;
static.
@@ -650,7 +500,7 @@ static u32 i2c_dw_read_clear_intrbits(st
* Interrupt service routine. This gets called whenever an I2C interrupt
* occurs.
*/
-static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
+irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
{
struct dw_i2c_dev *dev = dev_id;
u32 stat;
[...]
+EXPORT_SYMBOL(i2c_dw_isr);
No EXPORT_SYMBOL().
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
new file mode 100644
index 0000000..9558ef2
--- /dev/null
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -0,0 +1,204 @@
[...]
+#include<linux/kernel.h>
+#include<linux/module.h>
+#include<linux/delay.h>
+#include<linux/i2c.h>
+#include<linux/clk.h>
+#include<linux/errno.h>
+#include<linux/sched.h>
+#include<linux/err.h>
+#include<linux/interrupt.h>
+#include<linux/platform_device.h>
+#include<linux/io.h>
+#include<linux/slab.h>
We could remove most of these headers, I've not sorted out yet.
+#define DW_IC_TX_ABRT_NOACK (DW_IC_TX_ABRT_7B_ADDR_NOACK | \
+ DW_IC_TX_ABRT_10ADDR1_NOACK | \
+ DW_IC_TX_ABRT_10ADDR2_NOACK | \
+ DW_IC_TX_ABRT_TXDATA_NOACK | \
+ DW_IC_TX_ABRT_GCALL_NOACK)
+
+
Two blank lines.
+
+extern void i2c_dw_init(struct dw_i2c_dev *dev);
+extern int i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
+ int num);
+extern u32 i2c_dw_func(struct i2c_adapter *adap);
+extern irqreturn_t i2c_dw_isr(int this_irq, void *dev_id);
Yeah, we have these in place.
diff --git a/drivers/i2c/busses/i2c-designware-pci.c b/drivers/i2c/busses/i2c-designware-pci.c
new file mode 100644
index 0000000..11185b2
--- /dev/null
+++ b/drivers/i2c/busses/i2c-designware-pci.c
@@ -0,0 +1,183 @@
[...]
+#include<linux/kernel.h>
+#include<linux/module.h>
+#include<linux/delay.h>
+#include<linux/i2c.h>
+#include<linux/errno.h>
+#include<linux/sched.h>
+#include<linux/err.h>
+#include<linux/interrupt.h>
+#include<linux/platform_device.h>
+#include<linux/io.h>
+#include<linux/slab.h>
+#include "i2c-designware-core.h"
I guess we could omit more headers.
diff --git a/drivers/i2c/busses/i2c-designware-plat.c b/drivers/i2c/busses/i2c-designware-plat.c
new file mode 100644
index 0000000..b8e5aa4
--- /dev/null
+++ b/drivers/i2c/busses/i2c-designware-plat.c
@@ -0,0 +1,198 @@
[...]
+#include<linux/kernel.h>
+#include<linux/module.h>
+#include<linux/delay.h>
+#include<linux/i2c.h>
+#include<linux/clk.h>
+#include<linux/errno.h>
+#include<linux/sched.h>
+#include<linux/err.h>
+#include<linux/interrupt.h>
+#include<linux/platform_device.h>
+#include<linux/io.h>
+#include<linux/slab.h>
+
+#include "i2c-designware-core.h"
Same here.
And the following is the diff between -pci.c and -plat.c.
--- drivers/i2c/busses/i2c-designware-pci.c 2011-01-23 10:29:34.178022264 +0900
+++ drivers/i2c/busses/i2c-designware-plat.c 2011-01-23 10:29:34.190022267 +0900
@@ -25,11 +25,11 @@
* ----------------------------------------------------------------------------
*
*/
-
#include<linux/kernel.h>
#include<linux/module.h>
#include<linux/delay.h>
#include<linux/i2c.h>
+#include<linux/clk.h>
#include<linux/errno.h>
#include<linux/sched.h>
#include<linux/err.h>
@@ -37,6 +37,7 @@
#include<linux/platform_device.h>
#include<linux/io.h>
#include<linux/slab.h>
+
#include "i2c-designware-core.h"
static struct i2c_algorithm i2c_dw_algo = {
@@ -83,6 +84,12 @@ static int __devinit dw_i2c_probe(struct
dev->irq = irq;
platform_set_drvdata(pdev, dev);
+ dev->clk = clk_get(&pdev->dev, NULL);
+ if (IS_ERR(dev->clk)) {
+ r = -ENODEV;
+ goto err_free_mem;
+ }
+ clk_enable(dev->clk);
dev->base = ioremap(mem->start, resource_size(mem));
if (dev->base == NULL) {
@@ -127,6 +134,10 @@ err_free_irq:
free_irq(dev->irq, dev);
err_iounmap:
iounmap(dev->base);
+err_unuse_clocks:
+ clk_disable(dev->clk);
+ clk_put(dev->clk);
+ dev->clk = NULL;
err_free_mem:
platform_set_drvdata(pdev, NULL);
put_device(&pdev->dev);
@@ -146,6 +157,10 @@ static int __devexit dw_i2c_remove(struc
i2c_del_adapter(&dev->adapter);
put_device(&pdev->dev);
+ clk_disable(dev->clk);
+ clk_put(dev->clk);
+ dev->clk = NULL;
+
writel(0, dev->base + DW_IC_ENABLE);
free_irq(dev->irq, dev);
kfree(dev);
-pci.c is primary meant for the Intel Moorsetown and Medfield SoCs
as of now, and we're fine with removing clkdev API dependencies from
-pci.c file.
That said, two concerns. First, clkdev API support isn't related to
PCI support, so it might be reasonable to disable clk_ procedures appear
in the above patch simply using #ifdef HAVE_CLK, rather than completely
removed. The other is, how about separating such changes into another
commit, or cook it along with PATCH 3/9?
I will rework this so the clk infrasttucture can be used with PCI.
I'll follow up other patches later.
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
