Skip to contentSkip to conversation

Add basic support for the I2C units of the Armada 3700

PATCH Superseded Romain Perier 3 participants 7 messages Nov 30, 2016

Subsystems (3)

v6 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

v6 cover Add basic support for the I2C units of the Armada 3700

This series add basic support for the I2C bus interface units present in the Armada 3700 to the pxa-i2c driver. It also add the definitions of the device nodes to the devicetree at the SoC level and for its official development board: the Armada 3720 DB.

Romain Perier (4):
i2c: pxa: Add definition of fast and high speed modes via the regs
layout
i2c: pxa: Add support for the I2C units found in Armada 3700
arm64: dts: marvell: Add I2C definitions for the Armada 3700
dt-bindings: i2c: pxa: Update the documentation for the Armada 3700

 Documentation/devicetree/bindings/i2c/i2c-pxa.txt |  1 +
 arch/arm64/boot/dts/marvell/armada-3720-db.dts    |  4 ++++
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi      | 18 ++++++++++++++++
 drivers/i2c/busses/Kconfig                        |  2 +-
 drivers/i2c/busses/i2c-pxa.c                      | 26 +++++++++++++++++++++--
 5 files changed, 48 insertions(+), 3 deletions(-)
Signature · 1 line
2.9.3
#1 lore raw

PATCH v6 1/4 i2c: pxa: Add definition of fast and high speed modes via the regs layout

So far, the bit masks for the fast and high speed mode were statically defined. Some IP blocks might use different bits for these modes.

This commit introduces new fields in order to enable the definition of different bit masks for these features. If these fields are undefined, ICR_FM and ICR_HS are selected to preserve backward compatibility with other IPs.

---
 drivers/i2c/busses/i2c-pxa.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-pxa.c +9 -2
index e28b825..dc9d0a6 100644
@@ -48,6 +48,8 @@ struct pxa_reg_layout {
 	u32 isar;
 	u32 ilcr;
 	u32 iwcr;
+	u32 fm;
+	u32 hs;
 };
 
 enum pxa_i2c_types {
@@ -193,6 +195,8 @@ struct pxa_i2c {
 	unsigned char		master_code;
 	unsigned long		rate;
 	bool			highmode_enter;
+	u32			fm_mask;
+	u32			hs_mask;
 };
 
 #define _IBMR(i2c)	((i2c)->reg_ibmr)
@@ -503,8 +507,8 @@ static void i2c_pxa_reset(struct pxa_i2c *i2c)
 		writel(i2c->slave_addr, _ISAR(i2c));
 
 	/* set control register values */
-	writel(I2C_ICR_INIT | (i2c->fast_mode ? ICR_FM : 0), _ICR(i2c));
-	writel(readl(_ICR(i2c)) | (i2c->high_mode ? ICR_HS : 0), _ICR(i2c));
+	writel(I2C_ICR_INIT | (i2c->fast_mode ? i2c->fm_mask : 0), _ICR(i2c));
+	writel(readl(_ICR(i2c)) | (i2c->high_mode ? i2c->hs_mask : 0), _ICR(i2c));
 
 #ifdef CONFIG_I2C_PXA_SLAVE
 	dev_info(&i2c->adap.dev, "Enabling slave mode\n");
@@ -1234,6 +1238,9 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	i2c->reg_idbr = i2c->reg_base + pxa_reg_layout[i2c_type].idbr;
 	i2c->reg_icr = i2c->reg_base + pxa_reg_layout[i2c_type].icr;
 	i2c->reg_isr = i2c->reg_base + pxa_reg_layout[i2c_type].isr;
+	i2c->fm_mask = pxa_reg_layout[i2c_type].fm ? pxa_reg_layout[i2c_type].fm : ICR_FM;
+	i2c->hs_mask = pxa_reg_layout[i2c_type].hs ? pxa_reg_layout[i2c_type].hs : ICR_HS;
+
 	if (i2c_type != REGS_CE4100)
 		i2c->reg_isar = i2c->reg_base + pxa_reg_layout[i2c_type].isar;
 
Signature · 1 line
2.9.3
#2 lore raw

PATCH v6 1/4 i2c: pxa: Add definition of fast and high speed modes via the regs layout

Hello,

drivers/i2c/busses/i2c-pxa.c static int i2c_pxa_probe(struct platform_device *dev) · line 1242

Quoted 8 lines · Romain Perier

#ifdef CONFIG_I2C_PXA_SLAVE

drivers/i2c/busses/i2c-pxa.c +2 -0
 	dev_info(&i2c->adap.dev, "Enabling slave mode\n");
@@ -1234,6 +1238,9 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	i2c->reg_idbr = i2c->reg_base + pxa_reg_layout[i2c_type].idbr;
 	i2c->reg_icr = i2c->reg_base + pxa_reg_layout[i2c_type].icr;
 	i2c->reg_isr = i2c->reg_base + pxa_reg_layout[i2c_type].isr;
+	i2c->fm_mask = pxa_reg_layout[i2c_type].fm ? pxa_reg_layout[i2c_type].fm : ICR_FM;
+	i2c->hs_mask = pxa_reg_layout[i2c_type].hs ? pxa_reg_layout[i2c_type].hs : ICR_HS;

These lines are too long according to checkpatch. What about using what Wolfram originally suggested, i.e:

	i2c->fm_mask = pxa_reg_layout[i2c_type].fm ?: ICR_FM;

which does exactly the same, but fits within 80 characters ?

Thanks,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
#3 lore raw

PATCH v6 2/4 i2c: pxa: Add support for the I2C units found in Armada 3700

In reply to Romain Perier

The Armada 3700 has two I2C controllers that is compliant with the I2C Bus Specificiation 2.1, supports multi-master and different bus speed:
Standard mode (up to 100 KHz), Fast mode (up to 400 KHz), High speed mode (up to 3.4 Mhz).

This IP block has a lot of similarity with the PXA, except some register offsets and bitfield. This commits adds a basic support for this I2C unit.

---

Changes in v6:
- Revert back A3700_REGS, as asked by Wolfram and define fm_mask and hs_mask in the register layout. I moved the generic code for fm_mask and hs_mask to a seperated commit (1/4)

Changes in v5:
- Don't define registers for armada-3700, we can re-use the ones for PXA3XX.
- Define registers mask when OF is not used, in probe_pdata.

Changes in v4:
- Replaced the type of hs_mask and fm_mask by u32, instead of unsigned int, As writel() take an u32 as first argument...

Changes in v3:
- Replaced the type of hs_mask and fm_mask by unsigned int, instead of unsigned long.

 drivers/i2c/busses/Kconfig   |  2 +-
 drivers/i2c/busses/i2c-pxa.c | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)
drivers/i2c/busses/Kconfig +1 -1
index d252276..2f56a26 100644
@@ -763,7 +763,7 @@ config I2C_PUV3
 
 config I2C_PXA
 	tristate "Intel PXA2XX I2C adapter"
-	depends on ARCH_PXA || ARCH_MMP || (X86_32 && PCI && OF)
+	depends on ARCH_PXA || ARCH_MMP || ARCH_MVEBU || (X86_32 && PCI && OF)
 	help
 	  If you have devices in the PXA I2C bus, say yes to this option.
 	  This driver can also be built as a module.  If so, the module
drivers/i2c/busses/i2c-pxa.c +15 -0
index dc9d0a6..0ded4bc 100644
@@ -57,8 +57,12 @@ enum pxa_i2c_types {
 	REGS_PXA3XX,
 	REGS_CE4100,
 	REGS_PXA910,
+	REGS_A3700,
 };
 
+#define ICR_BUSMODE_FM	(1 << 16)	   /* shifted fast mode for armada-3700 */
+#define ICR_BUSMODE_HS	(1 << 17)	   /* shifted high speed mode for armada-3700 */
+
 /*
  * I2C registers definitions
  */
@@ -93,6 +97,15 @@ static struct pxa_reg_layout pxa_reg_layout[] = {
 		.ilcr = 0x28,
 		.iwcr = 0x30,
 	},
+	[REGS_A3700] = {
+		.ibmr =	0x00,
+		.idbr =	0x04,
+		.icr =	0x08,
+		.isr =	0x0c,
+		.isar =	0x10,
+		.fm = ICR_BUSMODE_FM,
+		.hs = ICR_BUSMODE_HS,
+	},
 };
 
 static const struct platform_device_id i2c_pxa_id_table[] = {
@@ -100,6 +113,7 @@ static const struct platform_device_id i2c_pxa_id_table[] = {
 	{ "pxa3xx-pwri2c",	REGS_PXA3XX },
 	{ "ce4100-i2c",		REGS_CE4100 },
 	{ "pxa910-i2c",		REGS_PXA910 },
+	{ "armada-3700-i2c",	REGS_A3700  },
 	{ },
 };
 MODULE_DEVICE_TABLE(platform, i2c_pxa_id_table);
@@ -1141,6 +1155,7 @@ static const struct of_device_id i2c_pxa_dt_ids[] = {
 	{ .compatible = "mrvl,pxa-i2c", .data = (void *)REGS_PXA2XX },
 	{ .compatible = "mrvl,pwri2c", .data = (void *)REGS_PXA3XX },
 	{ .compatible = "mrvl,mmp-twsi", .data = (void *)REGS_PXA910 },
+	{ .compatible = "marvell,armada-3700-i2c", .data = (void *)REGS_A3700 },
 	{}
 };
 MODULE_DEVICE_TABLE(of, i2c_pxa_dt_ids);
Signature · 1 line
2.9.3
#4 lore raw

PATCH v6 2/4 i2c: pxa: Add support for the I2C units found in Armada 3700

Hi Romain,

Quoted 11 lines · Romain Perier

The Armada 3700 has two I2C controllers that is compliant with the I2C Bus Specificiation 2.1, supports multi-master and different bus speed:
Standard mode (up to 100 KHz), Fast mode (up to 400 KHz), High speed mode (up to 3.4 Mhz).

This IP block has a lot of similarity with the PXA, except some register offsets and bitfield. This commits adds a basic support for this I2C unit.

As the code had changed I tested it agin and it continues to work, so you can keep my Tested-by.

However I have a small remark, see below:

Quoted 29 lines

---

Changes in v6:
- Revert back A3700_REGS, as asked by Wolfram and define fm_mask and hs_mask in the register layout. I moved the generic code for fm_mask and hs_mask to a seperated commit (1/4)

Changes in v5:
- Don't define registers for armada-3700, we can re-use the ones for PXA3XX.
- Define registers mask when OF is not used, in probe_pdata.

Changes in v4:
- Replaced the type of hs_mask and fm_mask by u32, instead of unsigned int, As writel() take an u32 as first argument...

Changes in v3:
- Replaced the type of hs_mask and fm_mask by unsigned int, instead of unsigned long.

 drivers/i2c/busses/Kconfig   |  2 +-
 drivers/i2c/busses/i2c-pxa.c | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)
drivers/i2c/busses/Kconfig +0 -0
index d252276..2f56a26 100644

drivers/i2c/busses/Kconfig config I2C_PUV3 · line 769

Quoted 8 lines
drivers/i2c/busses/Kconfig +1 -1
@@ -763,7 +763,7 @@ config I2C_PUV3
 
 config I2C_PXA
 	tristate "Intel PXA2XX I2C adapter"
-	depends on ARCH_PXA || ARCH_MMP || (X86_32 && PCI && OF)
+	depends on ARCH_PXA || ARCH_MMP || ARCH_MVEBU || (X86_32 && PCI && OF)
 	help
 	  If you have devices in the PXA I2C bus, say yes to this option.
 	  This driver can also be built as a module.  If so, the module

Maybe you could add that this driver is no longer only for the PXA family but also for the Armada 37xx.

Gregory

Quoted 52 lines
drivers/i2c/busses/i2c-pxa.c +15 -0
index dc9d0a6..0ded4bc 100644
@@ -57,8 +57,12 @@ enum pxa_i2c_types {
 	REGS_PXA3XX,
 	REGS_CE4100,
 	REGS_PXA910,
+	REGS_A3700,
 };
 
+#define ICR_BUSMODE_FM	(1 << 16)	   /* shifted fast mode for armada-3700 */
+#define ICR_BUSMODE_HS	(1 << 17)	   /* shifted high speed mode for armada-3700 */
+
 /*
  * I2C registers definitions
  */
@@ -93,6 +97,15 @@ static struct pxa_reg_layout pxa_reg_layout[] = {
 		.ilcr = 0x28,
 		.iwcr = 0x30,
 	},
+	[REGS_A3700] = {
+		.ibmr =	0x00,
+		.idbr =	0x04,
+		.icr =	0x08,
+		.isr =	0x0c,
+		.isar =	0x10,
+		.fm = ICR_BUSMODE_FM,
+		.hs = ICR_BUSMODE_HS,
+	},
 };
 
 static const struct platform_device_id i2c_pxa_id_table[] = {
@@ -100,6 +113,7 @@ static const struct platform_device_id i2c_pxa_id_table[] = {
 	{ "pxa3xx-pwri2c",	REGS_PXA3XX },
 	{ "ce4100-i2c",		REGS_CE4100 },
 	{ "pxa910-i2c",		REGS_PXA910 },
+	{ "armada-3700-i2c",	REGS_A3700  },
 	{ },
 };
 MODULE_DEVICE_TABLE(platform, i2c_pxa_id_table);
@@ -1141,6 +1155,7 @@ static const struct of_device_id i2c_pxa_dt_ids[] = {
 	{ .compatible = "mrvl,pxa-i2c", .data = (void *)REGS_PXA2XX },
 	{ .compatible = "mrvl,pwri2c", .data = (void *)REGS_PXA3XX },
 	{ .compatible = "mrvl,mmp-twsi", .data = (void *)REGS_PXA910 },
+	{ .compatible = "marvell,armada-3700-i2c", .data = (void *)REGS_A3700 },
 	{}
 };
 MODULE_DEVICE_TABLE(of, i2c_pxa_dt_ids);
Signature · 1 line
2.9.3
Signature · 4 lines
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#5 lore raw

PATCH v6 3/4 arm64: dts: marvell: Add I2C definitions for the Armada 3700

In reply to Romain Perier

The Armada 3700 has two i2c bus interface units, this commit adds the definitions of the corresponding device nodes. It also enables the node on the development board for this SoC.

---
 arch/arm64/boot/dts/marvell/armada-3720-db.dts |  4 ++++
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi   | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+)
arch/arm64/boot/dts/marvell/armada-3720-db.dts +4 -0
index 1372e9a6..16d84af 100644
@@ -62,6 +62,10 @@
 	};
 };
 
+&i2c0 {
+	status = "okay";
+};
+
 /* CON3 */
 &sata {
 	status = "okay";
arch/arm64/boot/dts/marvell/armada-37xx.dtsi +18 -0
index e9bd587..1b0fd21 100644
@@ -98,6 +98,24 @@
 			/* 32M internal register @ 0xd000_0000 */
 			ranges = <0x0 0x0 0xd0000000 0x2000000>;
 
+			i2c0: i2c at 11000 {
+				compatible = "marvell,armada-3700-i2c";
+				reg = <0x11000 0x24>;
+				clocks = <&nb_periph_clk 10>;
+				interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
+				mrvl,i2c-fast-mode;
+				status = "disabled";
+			};
+
+			i2c1: i2c at 11080 {
+				compatible = "marvell,armada-3700-i2c";
+				reg = <0x11080 0x24>;
+				clocks = <&nb_periph_clk 9>;
+				interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
+				mrvl,i2c-fast-mode;
+				status = "disabled";
+			};
+
 			uart0: serial at 12000 {
 				compatible = "marvell,armada-3700-uart";
 				reg = <0x12000 0x400>;
Signature · 1 line
2.9.3
#6 lore raw

PATCH v6 4/4 dt-bindings: i2c: pxa: Update the documentation for the Armada 3700

In reply to Romain Perier

This commit documents the compatible string to have the compatibility for the I2C unit found in the Armada 3700.

---

Changes in v5:
- Added the tag 'Acked-by', by Rob Herring

Changes in v2:
- Fixed wrong compatible string, it should be "marvell,armada-3700-i2c" and not "marvell,armada-3700".

 Documentation/devicetree/bindings/i2c/i2c-pxa.txt | 1 +
 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/i2c/i2c-pxa.txt +1 -0
index 12b78ac..d30f0b1 100644
@@ -7,6 +7,7 @@ Required properties :
    compatible processor, e.g. pxa168, pxa910, mmp2, mmp3.
    For the pxa2xx/pxa3xx, an additional node "mrvl,pxa-i2c" is required
    as shown in the example below.
+   For the Armada 3700, the compatible should be "marvell,armada-3700-i2c".
 
 Recommended properties :
 
Signature · 1 line
2.9.3