[PATCH v5 0/3] Add basic support for the I2C units of the Armada 3700

[PATCH v5 0/3] Add basic support for the I2C units of the Armada 3700

[Romain Perier]

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 (3):
  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                      | 24 +++++++++++++++++++++--
 5 files changed, 46 insertions(+), 3 deletions(-)

-- 
2.9.3

[PATCH v5 1/3] i2c: pxa: Add support for the I2C units found in Armada 3700

[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.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---

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 | 24 ++++++++++++++++++++++--
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index d252276..2f56a26 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -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
diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index e28b825..374d17c 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -98,6 +98,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_PXA3XX  },
 	{ },
 };
 MODULE_DEVICE_TABLE(platform, i2c_pxa_id_table);
@@ -122,7 +123,9 @@ MODULE_DEVICE_TABLE(platform, i2c_pxa_id_table);
 #define ICR_SADIE	(1 << 13)	   /* slave address detected int enable */
 #define ICR_UR		(1 << 14)	   /* unit reset */
 #define ICR_FM		(1 << 15)	   /* fast mode */
+#define ICR_BUSMODE_FM	(1 << 16)	   /* shifted fast mode for armada-3700 */
 #define ICR_HS		(1 << 16)	   /* High Speed mode */
+#define ICR_BUSMODE_HS	(1 << 17)	   /* shifted high speed mode for armada-3700 */
 #define ICR_GPIOEN	(1 << 19)	   /* enable GPIO mode for SCL in HS */
 
 #define ISR_RWM		(1 << 0)	   /* read/write mode */
@@ -193,6 +196,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 +508,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");
@@ -1137,6 +1142,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_PXA3XX },
 	{}
 };
 MODULE_DEVICE_TABLE(of, i2c_pxa_dt_ids);
@@ -1158,6 +1164,13 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
 		i2c->use_pio = 1;
 	if (of_get_property(np, "mrvl,i2c-fast-mode", NULL))
 		i2c->fast_mode = 1;
+	if (of_device_is_compatible(np, "marvell,armada-3700-i2c")) {
+		i2c->fm_mask = ICR_BUSMODE_FM;
+		i2c->hs_mask = ICR_BUSMODE_HS;
+	} else {
+		i2c->fm_mask = ICR_FM;
+		i2c->hs_mask = ICR_HS;
+	}
 
 	*i2c_types = (enum pxa_i2c_types)(of_id->data);
 
@@ -1181,6 +1194,13 @@ static int i2c_pxa_probe_pdata(struct platform_device *pdev,
 			i2c->master_code = 0xe;
 		i2c->rate = plat->rate;
 	}
+	if (!strcmp(id->name, "armada-3700-i2c")) {
+		i2c->fm_mask = ICR_BUSMODE_FM;
+		i2c->hs_mask = ICR_BUSMODE_HS;
+	} else {
+		i2c->fm_mask = ICR_FM;
+		i2c->hs_mask = ICR_HS;
+	}
 	return 0;
 }
 
-- 
2.9.3

[PATCH v5 1/3] i2c: pxa: Add support for the I2C units found in Armada 3700

[Wolfram Sang]

> +	if (of_device_is_compatible(np, "marvell,armada-3700-i2c")) {
> +		i2c->fm_mask = ICR_BUSMODE_FM;
> +		i2c->hs_mask = ICR_BUSMODE_HS;
> +	} else {
> +		i2c->fm_mask = ICR_FM;
> +		i2c->hs_mask = ICR_HS;
> +	}
>  
>  	*i2c_types = (enum pxa_i2c_types)(of_id->data);
>  
> @@ -1181,6 +1194,13 @@ static int i2c_pxa_probe_pdata(struct platform_device *pdev,
>  			i2c->master_code = 0xe;
>  		i2c->rate = plat->rate;
>  	}
> +	if (!strcmp(id->name, "armada-3700-i2c")) {
> +		i2c->fm_mask = ICR_BUSMODE_FM;
> +		i2c->hs_mask = ICR_BUSMODE_HS;
> +	} else {
> +		i2c->fm_mask = ICR_FM;
> +		i2c->hs_mask = ICR_HS;
> +	}

Okay, having the same code twice is not nice as well.

Sorry for missing this in the first review and going a step back, but I
think now the best solution is to have again a REGS_A3700 struct, but we
should extend it with new entries for the shifted bits. Then in the init
code, you can do something like:

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

Makes sense?

Thanks,

   Wolfram

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161129/53f523c7/attachment-0001.sig>

[PATCH v5 1/3] i2c: pxa: Add support for the I2C units found in Armada 3700

[Romain Perier]

Hello,

Le 29/11/2016 ? 22:17, Wolfram Sang a ?crit :
>> +	if (of_device_is_compatible(np, "marvell,armada-3700-i2c")) {
>> +		i2c->fm_mask = ICR_BUSMODE_FM;
>> +		i2c->hs_mask = ICR_BUSMODE_HS;
>> +	} else {
>> +		i2c->fm_mask = ICR_FM;
>> +		i2c->hs_mask = ICR_HS;
>> +	}
>>
>>  	*i2c_types = (enum pxa_i2c_types)(of_id->data);
>>
>> @@ -1181,6 +1194,13 @@ static int i2c_pxa_probe_pdata(struct platform_device *pdev,
>>  			i2c->master_code = 0xe;
>>  		i2c->rate = plat->rate;
>>  	}
>> +	if (!strcmp(id->name, "armada-3700-i2c")) {
>> +		i2c->fm_mask = ICR_BUSMODE_FM;
>> +		i2c->hs_mask = ICR_BUSMODE_HS;
>> +	} else {
>> +		i2c->fm_mask = ICR_FM;
>> +		i2c->hs_mask = ICR_HS;
>> +	}
>
> Okay, having the same code twice is not nice as well.
>
> Sorry for missing this in the first review and going a step back, but I
> think now the best solution is to have again a REGS_A3700 struct, but we
> should extend it with new entries for the shifted bits. Then in the init
> code, you can do something like:
>
> 	i2c->fm_mask = pxa_reg_layout[i2c_type].fm_mask ?: ICR_FM;
>
> Makes sense?

Mhhhh... makes sense yes, it is simpler and would remove the duplicated 
code, yes (no no need to modify probe_dt and probe_pdata in this case).
What do you prefer everything in one commit or two seperated commit ? 
(one including the new fields for fm_mask and another one to add support 
for a3700-i2c).

Thanks,
Romain

-- 
Romain Perier, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v5 1/3] i2c: pxa: Add support for the I2C units found in Armada 3700

[Wolfram Sang]

> What do you prefer everything in one commit or two seperated commit ? (one
> including the new fields for fm_mask and another one to add support for
> a3700-i2c).

One commit is fine!

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161130/0fcbc15f/attachment-0001.sig>

[PATCH v5 2/3] arm64: dts: marvell: Add I2C definitions for the Armada 3700

[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.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm64/boot/dts/marvell/armada-3720-db.dts |  4 ++++
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi   | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
index 1372e9a6..16d84af 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
@@ -62,6 +62,10 @@
 	};
 };
 
+&i2c0 {
+	status = "okay";
+};
+
 /* CON3 */
 &sata {
 	status = "okay";
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index e9bd587..1b0fd21 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -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>;
-- 
2.9.3

[PATCH v5 3/3] dt-bindings: i2c: pxa: Update the documentation for the Armada 3700

[Romain Perier]

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

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
Acked-by: Rob Herring <robh@kernel.org>
---

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(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-pxa.txt b/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
index 12b78ac..d30f0b1 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-pxa.txt
@@ -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 :
 
-- 
2.9.3