Skip to contentSkip to conversation

i2c-nomadik changes

RFC PATCH Superseded Alessandro Rubini 4 participants 14 messages May 30, 2012 – Jun 1, 2012

Subsystems (4)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

cover 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
Signature · 1 line
1.7.7.2
#1 lore raw

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

---
 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
arch/arm/mach-ux500/board-mop500.c +1 -1
index fba8ade..2117683 100644
@@ -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>
 
arch/arm/mach-ux500/devices-common.h +1 -1
index 6e47065..50d1292 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/i2c-nomadik.h>
 #include <mach/crypto-ux500.h>
 
 struct spi_master_cntlr;
arch/arm/plat-nomadik/include/plat/i2c.h deleted +0 -39
deleted file mode 100644
index 8ba70ff..0000000
@@ -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 */
drivers/i2c/busses/i2c-nomadik.c +1 -2
index 5267ab9..09b7589 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/i2c-nomadik.h>
 
 #define DRIVER_NAME "nmk-i2c"
 
include/linux/i2c-nomadik.h new file +39 -0
new file mode 100644
index 0000000..9350fe9
@@ -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 */
Signature · 1 line
1.7.7.2
#2 lore raw

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

Quoted 7 lines · Alessandro Rubini

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.

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

#3 lore raw

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

Quoted 7 lines · Linus Walleij
Quoted 5 lines · Alessandro Rubini

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.

Quoted 5 lines
Quoted 2 lines

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.

Quoted 2 lines

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.

Signature · 2 lines
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius
#4 lore raw

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

In reply to Alessandro Rubini
Quoted 59 lines · Alessandro Rubini

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.

(quoted diff) +0 -2
-
-#endif	/* __PLAT_I2C_H */
drivers/i2c/busses/i2c-nomadik.c +1 -2
index 5267ab9..09b7589 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/i2c-nomadik.h>
 
  #define DRIVER_NAME "nmk-i2c"
 
include/linux/i2c-nomadik.h new file +29 -0
new file mode 100644
index 0000000..9350fe9
@@ -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
+ */

include/linux/i2c-nomadik.h line 39

Quoted 10 lines
include/linux/i2c-nomadik.h new file +10 -0
+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?

Signature · 2 lines
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius
#5 lore raw

RFC PATCH 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 to register an AMBA device, not a platform device.

Cc Srinidhi Kasagar <[email protected]>
Cc STEricsson_nomadik_linux at list.st.com
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(-)
arch/arm/mach-ux500/devices-common.h +3 -18
index 50d1292..d73fed5 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
#6 lore raw

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

Quoted 10 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 to register an AMBA device, not a platform device.

Cc Srinidhi Kasagar <[email protected]>
Cc STEricsson_nomadik_linux at list.st.com
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

#7 lore raw

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

Quoted 13 lines · Linus Walleij
Quoted 10 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 to register an AMBA device, not a platform device.

Cc Srinidhi Kasagar<[email protected]>
Cc STEricsson_nomadik_linux at list.st.com
Cc Linus Walleij<[email protected]>
Quoted 4 lines

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?

Signature · 2 lines
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius
#8 lore raw

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

Quoted 6 lines · Ben Dooks
Quoted 3 lines · Linus Walleij

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

#9 lore raw

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

Quoted 4 lines

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

#10 lore raw

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

Quoted 2 lines · Alessandro Rubini

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

#11 lore raw

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

Quoted 11 lines · Linus Walleij
Quoted 2 lines · Alessandro Rubini

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.

Signature · 5 lines
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
#12 lore raw

RFC PATCH 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
#13 lore raw
In reply to Alessandro Rubini
Quoted 5 lines · Alessandro Rubini

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.

Quoted 2 lines

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