i2c: adds support for i2c bus on Frescale CPM1/CPM2 controllers

drivers/i2c/busses/i2c-cpm.c new file +18 -0

+static int cpm_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+
+	while (tptr < num) {
+		/* Check for outstanding messages */
+		dev_dbg(&adap->dev, "test ready.\n");
+		if (!(tbdf[tptr].cbd_sc & BD_SC_READY)) {
+			dev_dbg(&adap->dev, "ready.\n");
+			rmsg = &msgs[tptr];
+			ret = cpm_i2c_check_message(adap, rmsg, tptr, rptr);
+			tptr++;
+			if (rmsg->flags & I2C_M_RD)
+				rptr++;
+			if (ret) {
+				cpm_i2c_force_close(adap);
+				mutex_unlock(&cpm->i2c_mutex);
+				return ret;
+			}

+static struct of_device_id cpm_i2c_match[] = {

Hi Jochen,

Just a few trivial things.

drivers/i2c/busses/i2c-cpm.c line 137

Quoted 11 lines · On Wed, 02 Jan 2008 20:52:00 +0100 Jochen Friedrich <[email protected]> wrote:

+++ b/drivers/i2c/busses/i2c-cpm.c

drivers/i2c/busses/i2c-cpm.c new file +9 -0

+
+static irqreturn_t cpm_i2c_interrupt(int irq, void *dev_id)
+{
+	struct i2c_adapter *adap;
+	struct cpm_i2c *cpm;
+	struct i2c_reg __iomem *i2c_reg;
+	int i;
+
+	adap = (struct i2c_adapter *) dev_id;

This cast is unnecessary. In fact, you could just pass dev_id to the following call to i2c_get_adapdata() and eliminate adap completely.

Review

Quoted 2 lines

(quoted diff) +2 -0

+	/* Get 'me going again.
+	 */

For short comments, just make them one line. Similarly later as well.

drivers/i2c/busses/i2c-cpm.c line 210

Quoted 3 lines

drivers/i2c/busses/i2c-cpm.c new file +3 -0

+	/* This chip can't do zero length writes. However, the i2c core uses
+	   them to scan for devices. The best we can do is to convert them
+	   into 1 byte reads */

For multiline comments, we normally do
/*
* blah ...
* more blah
*/

Quoted 18 lines

drivers/i2c/busses/i2c-cpm.c new file +18 -0

+static int cpm_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+
+	while (tptr < num) {
+		/* Check for outstanding messages */
+		dev_dbg(&adap->dev, "test ready.\n");
+		if (!(tbdf[tptr].cbd_sc & BD_SC_READY)) {
+			dev_dbg(&adap->dev, "ready.\n");
+			rmsg = &msgs[tptr];
+			ret = cpm_i2c_check_message(adap, rmsg, tptr, rptr);
+			tptr++;
+			if (rmsg->flags & I2C_M_RD)
+				rptr++;
+			if (ret) {
+				cpm_i2c_force_close(adap);
+				mutex_unlock(&cpm->i2c_mutex);
+				return ret;
+			}

drivers/i2c/busses/i2c-cpm.c line 420

Quoted 10 lines

drivers/i2c/busses/i2c-cpm.c new file +10 -0

+		} else {
+			dev_dbg(&adap->dev, "not ready.\n");
+			ret = wait_event_interruptible_timeout(cpm->i2c_wait,
+				!(tbdf[tptr].cbd_sc & BD_SC_READY), 1 * HZ);
+			if (ret == 0) {
+				cpm_i2c_force_close(adap);
+				dev_dbg(&adap->dev, "I2C read: timeout!\n");
+				mutex_unlock(&cpm->i2c_mutex);
+				return -EREMOTEIO;
+			}

You might want to consolidate the two error paths above using gotos to an error return section below.

drivers/i2c/busses/i2c-cpm.c line 647

Quoted 6 lines

drivers/i2c/busses/i2c-cpm.c new file +6 -0

+static void of_register_i2c_devices(struct i2c_adapter *adap,
+				    struct device_node *adap_node)
+{
+	struct device_node *node = NULL;
+
+	while ((node = of_get_next_child(adap_node, node))) {

Use

	for_each_child_of_node(adap_node, node) {

instead and you don't need to initialise "node" above.

Quoted 1 line

+static struct of_device_id cpm_i2c_match[] = {

const?

Do you have an updated patch addressing Stephen's comment?

Note: you'd rather send updates of this patch to the i2c list rather than LKML.

Hi Jean,

Quoted 4 lines

Do you have an updated patch addressing Stephen's comment?

Note: you'd rather send updates of this patch to the i2c list rather than LKML.

I'm currently looking at that last patches from Jon and try to make this driver fit in there (+ the class override stuff).

Note: will do. I just subscribed to this list.

Hallo Jochen,

Quoted 9 lines · Jochen Friedrich

Hi Jean,

Quoted 4 lines

Do you have an updated patch addressing Stephen's comment?

Note: you'd rather send updates of this patch to the i2c list rather than LKML.

I'm currently looking at that last patches from Jon and try to make this driver fit in there (+ the class override stuff).

I'm not sure what patches exactly you're talking about, but the aliasing stuff Jon and myself have been working on are still under debate. Your patch shouldn't assume that any of these patches will go upstream immediately, so your patch adding the i2c-cpm driver can ignore them. I'll update the patches before merging them as needed, do not worry too much about that.

Quoted 2 lines

Note: will do. I just subscribed to this list.

Note hat you do not need to be subscribed to post to the i2c list (but of course, if you have some interest in Linux i2c development, subscribing was a good idea anyway.)

--
Jean Delvare