Skip to contentSkip to conversation

i2c-nomadik changes

PATCH Superseded Alessandro Rubini 2 participants 5 messages Jun 7, 2012 – Jun 8, 2012

Subsystems (4)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

v2 cover i2c-nomadik changes

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

Slightly edited for typos explanation, otherwise same as V1:

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. By 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 and I'm waiting for feedback on it (https://lkml.org/lkml/2012/5/28/194).

Status of this patch set:

The users of the driver (i.e. mach-ux500) 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. Most of the patch is just two renames: s/platform_device/amba_device/ and s/pdev/adev/ .

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               |   23 +---
 drivers/i2c/busses/Kconfig                         |    5 +-
 drivers/i2c/busses/i2c-nomadik.c                   |  140 ++++++++++----------
 .../linux/platform_data/i2c-nomadik.h              |    6 +-
 5 files changed, 80 insertions(+), 96 deletions(-)
 rename arch/arm/plat-nomadik/include/plat/i2c.h => include/linux/platform_data/i2c-nomadik.h (91%)
Signature · 1 line
1.7.7.2
#1 lore raw

PATCH v2 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.

---
 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%)
arch/arm/mach-ux500/board-mop500.c +1 -1
index 9c74ac5..0bf24bb 100644
@@ -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>
 
arch/arm/mach-ux500/devices-common.h +1 -1
index 6e47065..23cf734 100644
@@ -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;
drivers/i2c/busses/i2c-nomadik.c +1 -2
index 5267ab9..752f1fd 100644
@@ -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"
 
include/linux/platform_data/i2c-nomadik.h renamed from arch/arm/plat-nomadik/include/plat/i2c.h +3 -3
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
@@ -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 */
Signature · 1 line
1.7.7.2
#2 lore raw

PATCH v2 2/3 i2c-nomadik: turn the platform driver to an amba driver

In reply to Alessandro Rubini

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.

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

---
 arch/arm/mach-ux500/devices-common.h |   21 +-----
 drivers/i2c/busses/i2c-nomadik.c     |  137 +++++++++++++++++-----------------
 2 files changed, 71 insertions(+), 87 deletions(-)
arch/arm/mach-ux500/devices-common.h +3 -18
index 23cf734..cfee556 100644
@@ -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 *
drivers/i2c/busses/i2c-nomadik.c +68 -69

343 lines not on this page · raw message

Signature · 1 line
1.7.7.2
Tested-by on v2 2/3
#3 lore raw

Re: [PATCH V2 2/3] i2c-nomadik: turn the platform driver to an amba driver

Quoted 11 lines · Alessandro Rubini

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.

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

OK I tested this now and it causes a fat, ugly regression on ux500 and makes it hang at boot.

But don't worry! Because I found all the causes.

1) The devices created from dbx500_add_i2c() must have a serious name like "nmk-i2c.0" etc. (Caused platform to hang)

2) Each adapter needs a unique adap->nr, this patch made the driver into a singleton that couldn't handle multiple instances.

3) Runtime PM screams because the AMBA runtime PM semantics differ from the semantics of platform devices. AMBA calls probe with pm enabled and one get() refcound and will also disable it if probe fails. So just put() it at the end of probe. (C.f. drivers/spi/spi-pl022.c)

After fixing this it works fine on ux500. I also changed:

4) The Nomadik PrimeCell ID looks tilted 4 bits. (Haven't tested, just looked in the reference manual)

Suggested patch to fix the patch below, beware that gmail might have whitespace-mangled it so you might need to edit it manually.

(quoted diff) +0 -0

b/arch/arm/mach-ux500/devices-common.h
index cfee556..ecdd838 100644

arch/arm/mach-ux500/devices-common.h +5 -2
@@ -60,8 +60,11 @@ static inline struct amba_device *
 dbx500_add_i2c(struct device *parent, int id, resource_size_t base, int irq,
 	       struct nmk_i2c_controller *data)
 {
-	return amba_apb_device_add(parent, "nmk-i2c", base, SZ_4K, irq, 0,
-				   data, 0);
+	/* Conjure a name similar to what the platform device used to have */
+	char name[16];
+
+	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 *
drivers/i2c/busses/i2c-nomadik.c +3 -0
index e9ed731..6db453f 100644
@@ -15,6 +15,7 @@
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/amba/bus.h>
+#include <linux/atomic.h>
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/i2c.h>
@@ -898,6 +899,8 @@ static const struct i2c_algorithm nmk_i2c_algo = {
 	.functionality	= nmk_i2c_functionality
 };
 
+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;
@@ -941,7 +944,6 @@ static int nmk_i2c_probe(struct amba_device *adev,

const struct amba_id *id)
}

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

dev->clk = clk_get(&adev->dev, NULL);
if (IS_ERR(dev->clk)) {
@@ -957,8 +959,10 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
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 %pR", adap->nr, &adev->res);
+ atomic_inc(&adapter_id);

/* fetch the controller configuration from machine */ dev->cfg.clk_freq = pdata->clk_freq;
@@ -979,6 +983,8 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
goto err_add_adap;
}

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

err_add_adap:
@@ -986,7 +992,6 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
err_no_clk:
if (dev->regulator)
regulator_put(dev->regulator);
- pm_runtime_disable(&adev->dev);
free_irq(dev->irq, dev);
err_irq:
iounmap(dev->virtbase);
@@ -1025,7 +1030,7 @@ static int nmk_i2c_remove(struct amba_device *adev)

static struct amba_id nmk_i2c_ids[] = {
{
- .id = 0x00018024,
+ .id = 0x00180024,
.mask = 0x00ffffff,
},
{

If you apply this and respin you can add

On all three patches.

Yours,
Linus Walleij

#4 lore raw

PATCH v2 3/3 i2c-nomadik: depend on ARM_AMBA, not PLAT_NOMADIK

In reply to Alessandro Rubini

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

---
 drivers/i2c/busses/Kconfig |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
drivers/i2c/busses/Kconfig +3 -2
index 7244c8b..e9f9c5d 100644
@@ -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"
Signature · 1 line
1.7.7.2