Skip to contentSkip to conversation

ARM: ux500: resurrect I2C platform data

PATCH New Linus Walleij 4 participants 6 messages Aug 6, 2012

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ARM: ux500: resurrect I2C platform data

From: Linus Walleij <[email protected]>

Commit:
98582d9562b4bea6b0eb6e2bfafcd3fab3b60ccb
"ARM: ux500: Remove unused i2c platform_data initialisation code"

Deleted platform data intialization code that was used, not unused as indicated in the commit. The boot log (without devicetree) now looks like this:

nmk-i2c nmk-i2c.0: no platform data
nmk-i2c nmk-i2c.1: no platform data
nmk-i2c nmk-i2c.2: no platform data
nmk-i2c nmk-i2c.3: no platform data

And no I2C drivers register anymore.

Take this opportunity to bump the i2c bus speed to 400kHz. Some recent discussion revealed inconsistency in the board file for the MOP500 family. The board can very well do 400kHz on the I2C busses, so make it do that.

Cc Srinidhi Kasagar <[email protected]>
Cc Lee Jones <[email protected]>
---
 arch/arm/mach-ux500/board-mop500.c | 39 ++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)
arch/arm/mach-ux500/board-mop500.c +35 -4
index 8674a89..dbb9946 100644
@@ -327,12 +327,43 @@ static struct i2c_board_info __initdata mop500_i2c2_devices[] = {
 	},
 };
 
+#define U8500_I2C_CONTROLLER(id, _slsu, _tft, _rft, clk, t_out, _sm)	\
+static struct nmk_i2c_controller u8500_i2c##id##_data = { \
+	/*				\
+	 * slave data setup time, which is	\
+	 * 250 ns,100ns,10ns which is 14,6,2	\
+	 * respectively for a 48 Mhz	\
+	 * i2c clock			\
+	 */				\
+	.slsu		= _slsu,	\
+	/* Tx FIFO threshold */		\
+	.tft		= _tft,		\
+	/* Rx FIFO threshold */		\
+	.rft		= _rft,		\
+	/* std. mode operation */	\
+	.clk_freq	= clk,		\
+	/* Slave response timeout(ms) */\
+	.timeout	= t_out,	\
+	.sm		= _sm,		\
+}
+
+/*
+ * The board uses 4 i2c controllers, initialize all of
+ * them with slave data setup time of 250 ns,
+ * Tx & Rx FIFO threshold values as 1 resp. 8 and fast
+ * mode of operation
+ */
+U8500_I2C_CONTROLLER(0, 0xe, 1, 8, 400000, 200, I2C_FREQ_MODE_FAST);
+U8500_I2C_CONTROLLER(1, 0xe, 1, 8, 400000, 200, I2C_FREQ_MODE_FAST);
+U8500_I2C_CONTROLLER(2,	0xe, 1, 8, 400000, 200, I2C_FREQ_MODE_FAST);
+U8500_I2C_CONTROLLER(3,	0xe, 1, 8, 400000, 200, I2C_FREQ_MODE_FAST);
+
 static void __init mop500_i2c_init(struct device *parent)
 {
-	db8500_add_i2c0(parent, NULL);
-	db8500_add_i2c1(parent, NULL);
-	db8500_add_i2c2(parent, NULL);
-	db8500_add_i2c3(parent, NULL);
+	db8500_add_i2c0(parent, &u8500_i2c0_data);
+	db8500_add_i2c1(parent, &u8500_i2c1_data);
+	db8500_add_i2c2(parent, &u8500_i2c2_data);
+	db8500_add_i2c3(parent, &u8500_i2c3_data);
 }
 
 static struct gpio_keys_button mop500_gpio_keys[] = {
Signature · 1 line
1.7.11.3
#1 lore raw

PATCH 1/1 ARM: ux500: resurrect I2C platform data

Quoted 7 lines · Linus Walleij

Commit:
98582d9562b4bea6b0eb6e2bfafcd3fab3b60ccb
"ARM: ux500: Remove unused i2c platform_data initialisation code"

Deleted platform data intialization code that was used, not unused as indicated in the commit. The boot log (without devicetree) now looks like this:

The aforementioned commit was part of a patch-set. One of its affiliates moved this code into the Nomadik I2C driver; however, it was contested because Device Tree support was also added and the maintainer thought the properties introduced could be consolidated with other I2C driver's DT properties. I don't think anyone has taken up the consolidation work and for that reason it hasn't gone in.

I think moving this stuff into the driver is the right thing to do. Instead, I'll split out the configuration and DT enablement from the patch and resubmit.

Sound good to you?

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#2 lore raw

PATCH 1/1 ARM: ux500: resurrect I2C platform data

Quoted 8 lines · Lee Jones
Quoted 7 lines · Linus Walleij

Commit:
98582d9562b4bea6b0eb6e2bfafcd3fab3b60ccb
"ARM: ux500: Remove unused i2c platform_data initialisation code"

Deleted platform data intialization code that was used, not unused as indicated in the commit. The boot log (without devicetree) now looks like this:

Quoted 6 lines

The aforementioned commit was part of a patch-set. One of its affiliates moved this code into the Nomadik I2C driver; however, it was contested because Device Tree support was also added and the maintainer thought the properties introduced could be consolidated with other I2C driver's DT properties. I don't think anyone has taken up the consolidation work and for that reason it hasn't gone in.

Too bad it had to turn into a regression though :-P

Quoted 5 lines

I think moving this stuff into the driver is the right thing to do. Instead, I'll split out the configuration and DT enablement from the patch and resubmit.

Sound good to you?

Either way works with me, but the patch sent out need to be rebased and sent to Wolfram. Either he applies that to the I2C tree or I apply this to the ux500 tree. So it's for Wolfram to decide.

Yours,
Linus Walleij

#3 lore raw

PATCH 1/1 i2c: Add default configuration into the Nomadik I2C driver

In reply to Linus Walleij

From: Lee Jones <[email protected]>
Date: Mon, 6 Aug 2012 11:04:14 +0100
Subject: [PATCH 1/1] i2c: Add default configuration into the Nomadik I2C
driver.

At this moment in time there is only one known configuration for the Nomadik I2C driver. By not holding that configuration in the driver adds some unnecessary overhead in platform code. The configuration as already been removed from platform code, this patch checks for any over-riding configurations. If there aren't any, the default is used.

Cc linux-i2c at vger.kernel.org
---
 drivers/i2c/busses/i2c-nomadik.c |   24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)
drivers/i2c/busses/i2c-nomadik.c +18 -6
index a92440d..86aa73d 100644
@@ -333,10 +333,6 @@ static void setup_i2c_controller(struct nmk_i2c_dev *dev)
 
 	i2c_clk = clk_get_rate(dev->clk);
 
-	/* fallback to std. mode if machine has not provided it */
-	if (dev->cfg.clk_freq == 0)
-		dev->cfg.clk_freq = 100000;
-
 	/*
 	 * The spec says, in case of std. mode the divider is
 	 * 2 whereas it is 3 for fast and fastplus mode of
@@ -899,15 +895,31 @@ static const struct i2c_algorithm nmk_i2c_algo = {
 	.functionality	= nmk_i2c_functionality
 };
 
+static struct nmk_i2c_controller u8500_i2c = {
+	/*
+	 * Slave data setup time; 250ns, 100ns, and 10ns, which
+	 * is 14, 6 and 2 respectively for a 48Mhz i2c clock.
+	 */
+	.slsu           = 0xe,
+	.tft            = 1,      /* Tx FIFO threshold */
+	.rft            = 8,      /* Rx FIFO threshold */
+	.clk_freq       = 400000, /* std. mode operation */
+	.timeout        = 200,    /* Slave response timeout(ms) */
+	.sm             = I2C_FREQ_MODE_FAST,
+};
+
 static int __devinit nmk_i2c_probe(struct platform_device *pdev)
 {
 	int ret = 0;
 	struct resource *res;
-	struct nmk_i2c_controller *pdata =
-			pdev->dev.platform_data;
+	struct nmk_i2c_controller *pdata = pdev->dev.platform_data;
 	struct nmk_i2c_dev	*dev;
 	struct i2c_adapter *adap;
 
+	if (!pdata)
+		/* No i2c configuration found, using the default. */
+		pdata = &u8500_i2c;
+
 	dev = kzalloc(sizeof(struct nmk_i2c_dev), GFP_KERNEL);
 	if (!dev) {
 		dev_err(&pdev->dev, "cannot allocate memory\n");
Signature · 1 line
1.7.9.5
#4 lore raw

PATCH 1/1 i2c: Add default configuration into the Nomadik I2C driver

Quoted 5 lines · Lee Jones

From: Lee Jones <[email protected]>
Date: Mon, 6 Aug 2012 11:04:14 +0100
Subject: [PATCH 1/1] i2c: Add default configuration into the Nomadik I2C
driver.

Quoted 8 lines

At this moment in time there is only one known configuration for the Nomadik I2C driver. By not holding that configuration in the driver adds some unnecessary overhead in platform code. The configuration as already been removed from platform code, this patch checks for any over-riding configurations. If there aren't any, the default is used.

Cc linux-i2c at vger.kernel.org

Ah, I created my own solution for this as well by reverting back some platform data.

This works too, but the patch does not apply to v3.6-rc1. Please rebase and repost, then you can add my

Please not that you need to send I2C patches for embedded to Wolfram Sang.

Wolfram, please pick this for the -rc series because it's a regression in v3.6-rc1.

Yours,
Linus Walleij

#5 lore raw

PATCH 1/1 i2c: Add default configuration into the Nomadik I2C driver

In reply to Lee Jones
Quoted 35 lines · Lee Jones

From: Lee Jones <[email protected]>
Date: Mon, 6 Aug 2012 11:04:14 +0100
Subject: [PATCH 1/1] i2c: Add default configuration into the Nomadik I2C
driver.

At this moment in time there is only one known configuration for the Nomadik I2C driver. By not holding that configuration in the driver adds some unnecessary overhead in platform code. The configuration as already been removed from platform code, this patch checks for any over-riding configurations. If there aren't any, the default is used.

Cc linux-i2c at vger.kernel.org
---
 drivers/i2c/busses/i2c-nomadik.c |   24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)
drivers/i2c/busses/i2c-nomadik.c +0 -4
index a92440d..86aa73d 100644
@@ -333,10 +333,6 @@ static void setup_i2c_controller(struct nmk_i2c_dev *dev)
 
 	i2c_clk = clk_get_rate(dev->clk);
 
-	/* fallback to std. mode if machine has not provided it */
-	if (dev->cfg.clk_freq == 0)
-		dev->cfg.clk_freq = 100000;
-
 	/*
 	 * The spec says, in case of std. mode the divider is
 	 * 2 whereas it is 3 for fast and fastplus mode of
@@ -899,15 +895,31 @@ static const struct i2c_algorithm nmk_i2c_algo = {
 	.functionality	= nmk_i2c_functionality
 };

drivers/i2c/busses/i2c-nomadik.c static const struct i2c_algorithm nmk_i2c_algo = { · line 906

Quoted 10 lines
drivers/i2c/busses/i2c-nomadik.c +9 -0
 
+static struct nmk_i2c_controller u8500_i2c = {
+	/*
+	 * Slave data setup time; 250ns, 100ns, and 10ns, which
+	 * is 14, 6 and 2 respectively for a 48Mhz i2c clock.
+	 */
+	.slsu           = 0xe,
+	.tft            = 1,      /* Tx FIFO threshold */
+	.rft            = 8,      /* Rx FIFO threshold */
+	.clk_freq       = 400000, /* std. mode operation */

"Fast mode operation" in comment, or use your favorite "upto" keyword.

Otherwise it is Acked-by: srinidhi kasagar <[email protected]>

/srinidhi