[RFC PATCH dev-5.1 0/6] Aspeed I2C buffer/DMA mode support

[RFC PATCH dev-5.1 0/6] Aspeed I2C buffer/DMA mode support

[Jae Hyun Yoo]

This patch series adds buffer mode and DMA mode transfer support for the
Aspeed I2C driver. With this change, default transfer mode will be set to
buffer mode for better performance, and DMA mode can be selectively used
depends on platform configuration.

* Buffer mode
  AST2400:
    It has 2 KBytes (256 Bytes x 8 pages) of I2C SRAM buffer pool from
    0x1e78a800 to 0x1e78afff that can be used for all busses with
    buffer pool manipulation. To simplify implementation for supporting
    both AST2400 and AST2500, it assigns each 128 Bytes per bus without
    using buffer pool manipulation so total 1792 Bytes of I2C SRAM
    buffer will be used.

  AST2500:
    It has 16 Bytes of individual I2C SRAM buffer per each bus and its
    range is from 0x1e78a200 to 0x1e78a2df, so it doesn't have 'buffer
    page selection' bit field in the Function control register, and
    neither 'base address pointer' bit field in the Pool buffer control
    register it has. To simplify implementation for supporting both
    AST2400 and AST2500, it writes zeros on those register bit fields
    but it's okay because it does nothing in AST2500.

* DMA mode
  Only AST2500 supports DMA mode under some limitations:
    I2C is sharing the DMA H/W with UHCI host controller and MCTP
    controller. Since those controllers operate with DMA mode only, I2C
    has to use buffer mode or byte mode instead if one of those
    controllers is enabled. Also make sure that if SD/eMMC or Port80
    snoop uses DMA mode instead of PIO or FIFO respectively, I2C can't
    use DMA mode..

I'm submitting this series as an RFC because it needs more test on real
AST2400 BMC mahines, also it needs to check if QEMU can handle this change
so please review and test it.

Jae Hyun Yoo (6):
  dt-bindings: i2c: aspeed: add buffer and DMA mode transfer support
  ARM: dts: aspeed: add I2C buffer mode support
  irqchip/aspeed-i2c-ic: add I2C SRAM enabling control
  i2c: aspeed: fix master pending state handling
  i2c: aspeed: add buffer mode transfer support
  i2c: aspeed: add DMA mode transfer support

 .../devicetree/bindings/i2c/i2c-aspeed.txt    |  52 +-
 arch/arm/boot/dts/aspeed-g4.dtsi              |  42 +-
 arch/arm/boot/dts/aspeed-g5.dtsi              |  42 +-
 drivers/i2c/busses/i2c-aspeed.c               | 469 ++++++++++++++++--
 drivers/irqchip/irq-aspeed-i2c-ic.c           |   8 +
 5 files changed, 548 insertions(+), 65 deletions(-)

-- 
2.22.0

[RFC PATCH dev-5.1 1/6] dt-bindings: i2c: aspeed: add buffer and DMA mode transfer support

[Jae Hyun Yoo]

Append bindings to support buffer mode and DMA mode transfer.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 .../devicetree/bindings/i2c/i2c-aspeed.txt    | 52 +++++++++++++++++--
 1 file changed, 49 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
index 8fbd8633a387..4fb04b580c42 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
@@ -3,7 +3,11 @@ Device tree configuration for the I2C busses on the AST24XX and AST25XX SoCs.
 Required Properties:
 - #address-cells	: should be 1
 - #size-cells		: should be 0
-- reg			: address offset and range of bus
+- reg			: address offset and range of bus registers and buffer
+- reg-names		: "bus-regs" and "buf" are recognized by Aspeed I2C
+			  driver. "bus-regs" is default. "buf" is optional in
+			  case of enabling I2C dedicated SRAM for buffer mode
+			  transfer support
 - compatible		: should be "aspeed,ast2400-i2c-bus"
 			  or "aspeed,ast2500-i2c-bus"
 - clocks		: root clock of bus, should reference the APB
@@ -16,6 +20,16 @@ Optional Properties:
 - bus-frequency	: frequency of the bus clock in Hz defaults to 100 kHz when not
 		  specified
 - multi-master	: states that there is another master active on this bus.
+- aspeed,dma-buf-size	: should be 4096 in case of AST2500.
+			  Only AST2500 supports DMA mode under some limitations:
+			  I2C is sharing the DMA H/W with UHCI host controller
+			  and MCTP controller. Since those controllers operate
+			  with DMA mode only, I2C has to use buffer mode or byte
+			  mode instead if one of those controllers is enabled.
+			  Also make sure that if SD/eMMC or Port80 snoop uses
+			  DMA mode instead of PIO or FIFO respectively, I2C
+			  can't use DMA mode. IF both DMA and buffer modes are
+			  enabled, DMA mode will be selected.
 
 Example:
 
@@ -27,7 +41,7 @@ i2c {
 
 	i2c_ic: interrupt-controller@0 {
 		#interrupt-cells = <1>;
-		compatible = "aspeed,ast2400-i2c-ic";
+		compatible = "aspeed,ast2500-i2c-ic";
 		reg = <0x0 0x40>;
 		interrupts = <12>;
 		interrupt-controller;
@@ -38,11 +52,43 @@ i2c {
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 		reg = <0x40 0x40>;
-		compatible = "aspeed,ast2400-i2c-bus";
+		reg-names = "bus-regs";
+		compatible = "aspeed,ast2500-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
 		bus-frequency = <100000>;
 		interrupts = <0>;
 		interrupt-parent = <&i2c_ic>;
 	};
+
+	/* buffer mode transfer enabled */
+	i2c1: i2c-bus@80 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		#interrupt-cells = <1>;
+		reg = <0x80 0x40>, <0x210 0x10>;
+		reg-names = "bus-regs", "buf";
+		compatible = "aspeed,ast2500-i2c-bus";
+		clocks = <&syscon ASPEED_CLK_APB>;
+		resets = <&syscon ASPEED_RESET_I2C>;
+		bus-frequency = <100000>;
+		interrupts = <1>;
+		interrupt-parent = <&i2c_ic>;
+	};
+
+	/* DMA mode transfer enabled */
+	i2c2: i2c-bus@c0 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		#interrupt-cells = <1>;
+		reg = <0xc0 0x40>;
+		reg-names = "bus-regs";
+		aspeed,dma-buf-size = <4096>;
+		compatible = "aspeed,ast2500-i2c-bus";
+		clocks = <&syscon ASPEED_CLK_APB>;
+		resets = <&syscon ASPEED_RESET_I2C>;
+		bus-frequency = <100000>;
+		interrupts = <2>;
+		interrupt-parent = <&i2c_ic>;
+	};
 };
-- 
2.22.0

[RFC PATCH dev-5.1 2/6] ARM: dts: aspeed: add I2C buffer mode support

[Jae Hyun Yoo]

Byte mode currently this driver uses makes lots of interrupt call
which isn't good for performance and it makes the driver very
timing sensitive. To improve performance of the driver, this commit
adds buffer mode transfer support which uses I2C SRAM buffer
instead of using a single byte buffer.

AST2400:
It has 2 KBytes (256 Bytes x 8 pages) of I2C SRAM buffer pool from
0x1e78a800 to 0x1e78afff that can be used for all busses with
buffer pool manipulation. To simplify implementation for supporting
both AST2400 and AST2500, it assigns each 128 Bytes per bus without
using buffer pool manipulation so total 1792 Bytes of I2C SRAM
buffer will be used.

AST2500:
It has 16 Bytes of individual I2C SRAM buffer per each bus and its
range is from 0x1e78a200 to 0x1e78a2df, so it doesn't have 'buffer
page selection' bit field in the Function control register, and
neither 'base address pointer' bit field in the Pool buffer control
register it has. To simplify implementation for supporting both
AST2400 and AST2500, it writes zeros on those register bit fields
but it's okay because it does nothing in AST2500.

This commit fixes all I2C bus nodes to support buffer mode
transfer.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 arch/arm/boot/dts/aspeed-g4.dtsi | 42 +++++++++++++++++++++-----------
 arch/arm/boot/dts/aspeed-g5.dtsi | 42 +++++++++++++++++++++-----------
 2 files changed, 56 insertions(+), 28 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
index 6011692df15a..041985fc8acf 100644
--- a/arch/arm/boot/dts/aspeed-g4.dtsi
+++ b/arch/arm/boot/dts/aspeed-g4.dtsi
@@ -424,7 +424,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x40 0x40>;
+		reg = <0x40 0x40>, <0x800 0x80>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2400-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -440,7 +441,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x80 0x40>;
+		reg = <0x80 0x40>, <0x880 0x80>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2400-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -456,7 +458,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0xc0 0x40>;
+		reg = <0xc0 0x40>, <0x900 0x80>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2400-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -473,7 +476,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x100 0x40>;
+		reg = <0x100 0x40>, <0x980 0x80>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2400-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -490,7 +494,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x140 0x40>;
+		reg = <0x140 0x40>, <0xa00 0x80>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2400-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -507,7 +512,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x180 0x40>;
+		reg = <0x180 0x40>, <0xa80 0x80>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2400-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -524,7 +530,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x1c0 0x40>;
+		reg = <0x1c0 0x40>, <0xb00 0x80>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2400-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -541,7 +548,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x300 0x40>;
+		reg = <0x300 0x40>, <0xb80 0x80>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2400-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -558,7 +566,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x340 0x40>;
+		reg = <0x340 0x40>, <0xc00 0x80>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2400-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -575,7 +584,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x380 0x40>;
+		reg = <0x380 0x40>, <0xc80 0x80>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2400-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -592,7 +602,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x3c0 0x40>;
+		reg = <0x3c0 0x40>, <0xd00 0x80>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2400-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -609,7 +620,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x400 0x40>;
+		reg = <0x400 0x40>, <0xd80 0x80>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2400-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -626,7 +638,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x440 0x40>;
+		reg = <0x440 0x40>, <0xe00 0x80>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2400-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -643,7 +656,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x480 0x40>;
+		reg = <0x480 0x40>, <0xe80 0x80>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2400-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index 383510d47140..8bc002d3fb0a 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -546,7 +546,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x40 0x40>;
+		reg = <0x40 0x40>, <0x200 0x10>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2500-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -562,7 +563,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x80 0x40>;
+		reg = <0x80 0x40>, <0x210 0x10>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2500-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -578,7 +580,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0xc0 0x40>;
+		reg = <0xc0 0x40>, <0x220 0x10>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2500-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -595,7 +598,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x100 0x40>;
+		reg = <0x100 0x40>, <0x230 0x10>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2500-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -612,7 +616,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x140 0x40>;
+		reg = <0x140 0x40>, <0x240 0x10>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2500-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -629,7 +634,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x180 0x40>;
+		reg = <0x180 0x40>, <0x250 0x10>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2500-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -646,7 +652,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x1c0 0x40>;
+		reg = <0x1c0 0x40>, <0x260 0x10>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2500-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -663,7 +670,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x300 0x40>;
+		reg = <0x300 0x40>, <0x270 0x10>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2500-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -680,7 +688,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x340 0x40>;
+		reg = <0x340 0x40>, <0x280 0x10>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2500-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -697,7 +706,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x380 0x40>;
+		reg = <0x380 0x40>, <0x290 0x10>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2500-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -714,7 +724,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x3c0 0x40>;
+		reg = <0x3c0 0x40>, <0x2a0 0x10>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2500-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -731,7 +742,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x400 0x40>;
+		reg = <0x400 0x40>, <0x2b0 0x10>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2500-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -748,7 +760,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x440 0x40>;
+		reg = <0x440 0x40>, <0x2c0 0x10>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2500-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
@@ -765,7 +778,8 @@
 		#size-cells = <0>;
 		#interrupt-cells = <1>;
 
-		reg = <0x480 0x40>;
+		reg = <0x480 0x40>, <0x2d0 0x10>;
+		reg-names = "bus-regs", "buf";
 		compatible = "aspeed,ast2500-i2c-bus";
 		clocks = <&syscon ASPEED_CLK_APB>;
 		resets = <&syscon ASPEED_RESET_I2C>;
-- 
2.22.0

[RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM enabling control

[Jae Hyun Yoo]

This commit adds I2C SRAM enabling control for AST2500 SoC to
support buffer mode and DMA mode transfer. The SRAM is enabled by
default in AST2400 SoC.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/irqchip/irq-aspeed-i2c-ic.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c b/drivers/irqchip/irq-aspeed-i2c-ic.c
index f20200af0992..99985b22a9fa 100644
--- a/drivers/irqchip/irq-aspeed-i2c-ic.c
+++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
@@ -18,6 +18,9 @@
 #include <linux/of_irq.h>
 #include <linux/io.h>
 
+/* I2C Global Control Register (AST2500) */
+#define ASPEED_I2C_GLOBAL_CTRL_REG	0xc
+#define  ASPEED_I2C_SRAM_BUFFER_EN	BIT(0)
 
 #define ASPEED_I2C_IC_NUM_BUS 14
 
@@ -100,6 +103,11 @@ static int __init aspeed_i2c_ic_of_init(struct device_node *node,
 	irq_set_chained_handler_and_data(i2c_ic->parent_irq,
 					 aspeed_i2c_ic_irq_handler, i2c_ic);
 
+	/* Enable I2C SRAM buffer in case of AST2500 */
+	if (of_device_is_compatible(node, "aspeed,ast2500-i2c-ic"))
+		writel(ASPEED_I2C_SRAM_BUFFER_EN,
+		       i2c_ic->base + ASPEED_I2C_GLOBAL_CTRL_REG);
+
 	pr_info("i2c controller registered, irq %d\n", i2c_ic->parent_irq);
 
 	return 0;
-- 
2.22.0

[RFC PATCH dev-5.1 4/6] i2c: aspeed: fix master pending state handling

[Jae Hyun Yoo]

In case of a master pending state, it should not trigger the master
command because this H/W is sharing the same data buffer for slave
and master operation, so this commit fixes the issue with making
the master command triggering happens when the state goes to active
state.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/i2c/busses/i2c-aspeed.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 6c8b38fd6e64..cde9dbac2d46 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -339,18 +339,19 @@ static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
 	struct i2c_msg *msg = &bus->msgs[bus->msgs_index];
 	u8 slave_addr = i2c_8bit_addr_from_msg(msg);
 
-	bus->master_state = ASPEED_I2C_MASTER_START;
-
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
 	/*
 	 * If it's requested in the middle of a slave session, set the master
 	 * state to 'pending' then H/W will continue handling this master
 	 * command when the bus comes back to the idle state.
 	 */
-	if (bus->slave_state != ASPEED_I2C_SLAVE_INACTIVE)
+	if (bus->slave_state != ASPEED_I2C_SLAVE_INACTIVE) {
 		bus->master_state = ASPEED_I2C_MASTER_PENDING;
+		return;
+	}
 #endif /* CONFIG_I2C_SLAVE */
 
+	bus->master_state = ASPEED_I2C_MASTER_START;
 	bus->buf_index = 0;
 
 	if (msg->flags & I2C_M_RD) {
@@ -435,7 +436,7 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 		if (bus->slave_state != ASPEED_I2C_SLAVE_INACTIVE)
 			goto out_no_complete;
 
-		bus->master_state = ASPEED_I2C_MASTER_START;
+		aspeed_i2c_do_start(bus);
 	}
 #endif /* CONFIG_I2C_SLAVE */
 
-- 
2.22.0

[RFC PATCH dev-5.1 5/6] i2c: aspeed: add buffer mode transfer support

[Jae Hyun Yoo]

Byte mode currently this driver uses makes lots of interrupt call
which isn't good for performance and it makes the driver very
timing sensitive. To improve performance of the driver, this commit
adds buffer mode transfer support which uses I2C SRAM buffer
instead of using a single byte buffer.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/i2c/busses/i2c-aspeed.c | 252 ++++++++++++++++++++++++++++----
 1 file changed, 225 insertions(+), 27 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index cde9dbac2d46..9e6f71de5c7e 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -10,6 +10,7 @@
  *  published by the Free Software Foundation.
  */
 
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/err.h>
@@ -38,6 +39,7 @@
 #define ASPEED_I2C_INTR_STS_REG				0x10
 #define ASPEED_I2C_CMD_REG				0x14
 #define ASPEED_I2C_DEV_ADDR_REG				0x18
+#define ASPEED_I2C_BUF_CTRL_REG				0x1c
 #define ASPEED_I2C_BYTE_BUF_REG				0x20
 
 /* Global Register Definition */
@@ -46,6 +48,7 @@
 
 /* Device Register Definition */
 /* 0x00 : I2CD Function Control Register  */
+#define ASPEED_I2CD_BUFFER_PAGE_SEL_MASK		GENMASK(22, 20)
 #define ASPEED_I2CD_MULTI_MASTER_DIS			BIT(15)
 #define ASPEED_I2CD_SDA_DRIVE_1T_EN			BIT(8)
 #define ASPEED_I2CD_M_SDA_DRIVE_1T_EN			BIT(7)
@@ -105,6 +108,8 @@
 #define ASPEED_I2CD_BUS_RECOVER_CMD			BIT(11)
 
 /* Command Bit */
+#define ASPEED_I2CD_RX_BUFF_ENABLE			BIT(7)
+#define ASPEED_I2CD_TX_BUFF_ENABLE			BIT(6)
 #define ASPEED_I2CD_M_STOP_CMD				BIT(5)
 #define ASPEED_I2CD_M_S_RX_CMD_LAST			BIT(4)
 #define ASPEED_I2CD_M_RX_CMD				BIT(3)
@@ -115,6 +120,13 @@
 /* 0x18 : I2CD Slave Device Address Register   */
 #define ASPEED_I2CD_DEV_ADDR_MASK			GENMASK(6, 0)
 
+/* 0x1c : I2CD Buffer Control Register */
+/* Use 8-bits or 6-bits wide bit fileds to support both AST2400 and AST2500 */
+#define ASPEED_I2CD_BUF_RX_COUNT_MASK			GENMASK(31, 24)
+#define ASPEED_I2CD_BUF_RX_SIZE_MASK			GENMASK(23, 16)
+#define ASPEED_I2CD_BUF_TX_COUNT_MASK			GENMASK(15, 8)
+#define ASPEED_I2CD_BUF_OFFSET_MASK			GENMASK(5, 0)
+
 enum aspeed_i2c_master_state {
 	ASPEED_I2C_MASTER_INACTIVE,
 	ASPEED_I2C_MASTER_PENDING,
@@ -160,6 +172,11 @@ struct aspeed_i2c_bus {
 	int				master_xfer_result;
 	/* Multi-master */
 	bool				multi_master;
+	/* Buffer mode */
+	void __iomem			*buf_base;
+	size_t				buf_size;
+	u8				buf_offset;
+	u8				buf_page;
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
 	struct i2c_client		*slave;
 	enum aspeed_i2c_slave_state	slave_state;
@@ -241,6 +258,7 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 {
 	u32 command, irq_handled = 0;
 	struct i2c_client *slave = bus->slave;
+	int i, len;
 	u8 value;
 
 	if (!slave)
@@ -263,7 +281,11 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 
 	/* Slave was sent something. */
 	if (irq_status & ASPEED_I2CD_INTR_RX_DONE) {
-		value = readl(bus->base + ASPEED_I2C_BYTE_BUF_REG) >> 8;
+		if (bus->buf_base &&
+		    bus->slave_state == ASPEED_I2C_SLAVE_WRITE_RECEIVED)
+			value = readb(bus->buf_base);
+		else
+			value = readl(bus->base + ASPEED_I2C_BYTE_BUF_REG) >> 8;
 		/* Handle address frame. */
 		if (bus->slave_state == ASPEED_I2C_SLAVE_START) {
 			if (value & 0x1)
@@ -278,6 +300,18 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 
 	/* Slave was asked to stop. */
 	if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
+		if (bus->buf_base &&
+		    bus->slave_state == ASPEED_I2C_SLAVE_WRITE_RECEIVED) {
+			len = FIELD_GET(ASPEED_I2CD_BUF_RX_COUNT_MASK,
+					readl(bus->base +
+					      ASPEED_I2C_BUF_CTRL_REG));
+			for (i = 0; i < len; i++) {
+				value = readb(bus->buf_base + i);
+				i2c_slave_event(slave,
+						I2C_SLAVE_WRITE_RECEIVED,
+						&value);
+			}
+		}
 		irq_handled |= ASPEED_I2CD_INTR_NORMAL_STOP;
 		bus->slave_state = ASPEED_I2C_SLAVE_STOP;
 	}
@@ -310,9 +344,36 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 	case ASPEED_I2C_SLAVE_WRITE_REQUESTED:
 		bus->slave_state = ASPEED_I2C_SLAVE_WRITE_RECEIVED;
 		i2c_slave_event(slave, I2C_SLAVE_WRITE_REQUESTED, &value);
+		if (bus->buf_base) {
+			writel(FIELD_PREP(ASPEED_I2CD_BUF_RX_SIZE_MASK,
+					  bus->buf_size - 1) |
+			       FIELD_PREP(ASPEED_I2CD_BUF_OFFSET_MASK,
+					  bus->buf_offset),
+			       bus->base + ASPEED_I2C_BUF_CTRL_REG);
+			writel(ASPEED_I2CD_RX_BUFF_ENABLE,
+			       bus->base + ASPEED_I2C_CMD_REG);
+		}
 		break;
 	case ASPEED_I2C_SLAVE_WRITE_RECEIVED:
 		i2c_slave_event(slave, I2C_SLAVE_WRITE_RECEIVED, &value);
+		if (bus->buf_base) {
+			len = FIELD_GET(ASPEED_I2CD_BUF_RX_COUNT_MASK,
+					readl(bus->base +
+					      ASPEED_I2C_BUF_CTRL_REG));
+			for (i = 1; i < len; i++) {
+				value = readb(bus->buf_base + i);
+				i2c_slave_event(slave,
+						I2C_SLAVE_WRITE_RECEIVED,
+						&value);
+			}
+			writel(FIELD_PREP(ASPEED_I2CD_BUF_RX_SIZE_MASK,
+					  bus->buf_size - 1) |
+			       FIELD_PREP(ASPEED_I2CD_BUF_OFFSET_MASK,
+					  bus->buf_offset),
+			       bus->base + ASPEED_I2C_BUF_CTRL_REG);
+			writel(ASPEED_I2CD_RX_BUFF_ENABLE,
+			       bus->base + ASPEED_I2C_CMD_REG);
+		}
 		break;
 	case ASPEED_I2C_SLAVE_STOP:
 		i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
@@ -338,6 +399,8 @@ static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
 	u32 command = ASPEED_I2CD_M_START_CMD | ASPEED_I2CD_M_TX_CMD;
 	struct i2c_msg *msg = &bus->msgs[bus->msgs_index];
 	u8 slave_addr = i2c_8bit_addr_from_msg(msg);
+	u8 wbuf[4];
+	int len;
 
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
 	/*
@@ -356,12 +419,66 @@ static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
 
 	if (msg->flags & I2C_M_RD) {
 		command |= ASPEED_I2CD_M_RX_CMD;
-		/* Need to let the hardware know to NACK after RX. */
-		if (msg->len == 1 && !(msg->flags & I2C_M_RECV_LEN))
-			command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
+
+		if (bus->buf_base && !(msg->flags & I2C_M_RECV_LEN)) {
+			command |= ASPEED_I2CD_RX_BUFF_ENABLE;
+
+			if (msg->len > bus->buf_size) {
+				len = bus->buf_size;
+			} else {
+				len = msg->len;
+				command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
+			}
+
+			writel(FIELD_PREP(ASPEED_I2CD_BUF_RX_SIZE_MASK,
+					  len - 1) |
+			       FIELD_PREP(ASPEED_I2CD_BUF_OFFSET_MASK,
+					  bus->buf_offset),
+			       bus->base + ASPEED_I2C_BUF_CTRL_REG);
+		} else {
+			/* Need to let the hardware know to NACK after RX. */
+			if (msg->len == 1 && !(msg->flags & I2C_M_RECV_LEN))
+				command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
+		}
+	} else {
+		if (bus->buf_base) {
+			int i;
+
+			command |= ASPEED_I2CD_TX_BUFF_ENABLE;
+
+			if (msg->len + 1 > bus->buf_size)
+				len = bus->buf_size;
+			else
+				len = msg->len + 1;
+
+			/*
+			 * Yeah, it looks clumsy but byte writings on a remapped
+			 * I2C SRAM cause corruptions so use this way to make
+			 * dword writings.
+			 */
+			wbuf[0] = slave_addr;
+			for (i = 1; i < len; i++) {
+				wbuf[i % 4] = msg->buf[i - 1];
+				if (i % 4 == 3)
+					writel(*(u32 *)wbuf,
+					       bus->buf_base + i - 3);
+			}
+			if (--i % 4 != 3)
+				writel(*(u32 *)wbuf,
+				       bus->buf_base + i - (i % 4));
+
+			bus->buf_index = len - 1;
+
+			writel(FIELD_PREP(ASPEED_I2CD_BUF_TX_COUNT_MASK,
+					  len - 1) |
+			       FIELD_PREP(ASPEED_I2CD_BUF_OFFSET_MASK,
+					  bus->buf_offset),
+			       bus->base + ASPEED_I2C_BUF_CTRL_REG);
+		}
 	}
 
-	writel(slave_addr, bus->base + ASPEED_I2C_BYTE_BUF_REG);
+	if (!(command & ASPEED_I2CD_TX_BUFF_ENABLE))
+		writel(slave_addr, bus->base + ASPEED_I2C_BYTE_BUF_REG);
 	writel(command, bus->base + ASPEED_I2C_CMD_REG);
 }
 
@@ -401,7 +518,7 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 	u32 irq_handled = 0, command = 0;
 	struct i2c_msg *msg;
 	u8 recv_byte;
-	int ret;
+	int ret, len;
 
 	if (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE) {
 		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
@@ -514,11 +631,43 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 		/* fall through */
 	case ASPEED_I2C_MASTER_TX_FIRST:
 		if (bus->buf_index < msg->len) {
+			command = ASPEED_I2CD_M_TX_CMD;
+
+			if (bus->buf_base) {
+				u8 wbuf[4];
+				int i;
+
+				command |= ASPEED_I2CD_TX_BUFF_ENABLE;
+
+				if (msg->len - bus->buf_index > bus->buf_size)
+					len = bus->buf_size;
+				else
+					len = msg->len - bus->buf_index;
+
+				for (i = 0; i < len; i++) {
+					wbuf[i % 4] = msg->buf[bus->buf_index
+							       + i];
+					if (i % 4 == 3)
+						writel(*(u32 *)wbuf,
+						       bus->buf_base + i - 3);
+				}
+				if (--i % 4 != 3)
+					writel(*(u32 *)wbuf,
+					       bus->buf_base + i - (i % 4));
+
+				bus->buf_index += len;
+
+				writel(FIELD_PREP(ASPEED_I2CD_BUF_TX_COUNT_MASK,
+						  len - 1) |
+				       FIELD_PREP(ASPEED_I2CD_BUF_OFFSET_MASK,
+						  bus->buf_offset),
+				       bus->base + ASPEED_I2C_BUF_CTRL_REG);
+			} else {
+				writel(msg->buf[bus->buf_index++],
+				       bus->base + ASPEED_I2C_BYTE_BUF_REG);
+			}
+			writel(command, bus->base + ASPEED_I2C_CMD_REG);
 			bus->master_state = ASPEED_I2C_MASTER_TX;
-			writel(msg->buf[bus->buf_index++],
-			       bus->base + ASPEED_I2C_BYTE_BUF_REG);
-			writel(ASPEED_I2CD_M_TX_CMD,
-			       bus->base + ASPEED_I2C_CMD_REG);
 		} else {
 			aspeed_i2c_next_msg_or_stop(bus);
 		}
@@ -535,25 +684,56 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 		}
 		irq_handled |= ASPEED_I2CD_INTR_RX_DONE;
 
-		recv_byte = readl(bus->base + ASPEED_I2C_BYTE_BUF_REG) >> 8;
-		msg->buf[bus->buf_index++] = recv_byte;
-
-		if (msg->flags & I2C_M_RECV_LEN) {
-			if (unlikely(recv_byte > I2C_SMBUS_BLOCK_MAX)) {
-				bus->cmd_err = -EPROTO;
-				aspeed_i2c_do_stop(bus);
-				goto out_no_complete;
+		if (bus->buf_base && !(msg->flags & I2C_M_RECV_LEN)) {
+			len = FIELD_GET(ASPEED_I2CD_BUF_RX_COUNT_MASK,
+					readl(bus->base +
+					      ASPEED_I2C_BUF_CTRL_REG));
+			memcpy_fromio(msg->buf + bus->buf_index,
+				      bus->buf_base, len);
+			bus->buf_index += len;
+		} else {
+			recv_byte = readl(bus->base + ASPEED_I2C_BYTE_BUF_REG)
+				    >> 8;
+			msg->buf[bus->buf_index++] = recv_byte;
+
+			if (msg->flags & I2C_M_RECV_LEN) {
+				if (unlikely(recv_byte > I2C_SMBUS_BLOCK_MAX)) {
+					bus->cmd_err = -EPROTO;
+					aspeed_i2c_do_stop(bus);
+					goto out_no_complete;
+				}
+				msg->len = recv_byte +
+						((msg->flags & I2C_CLIENT_PEC) ?
+						2 : 1);
+				msg->flags &= ~I2C_M_RECV_LEN;
 			}
-			msg->len = recv_byte +
-					((msg->flags & I2C_CLIENT_PEC) ? 2 : 1);
-			msg->flags &= ~I2C_M_RECV_LEN;
 		}
 
 		if (bus->buf_index < msg->len) {
-			bus->master_state = ASPEED_I2C_MASTER_RX;
 			command = ASPEED_I2CD_M_RX_CMD;
-			if (bus->buf_index + 1 == msg->len)
-				command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
+			bus->master_state = ASPEED_I2C_MASTER_RX;
+			if (bus->buf_base) {
+				command |= ASPEED_I2CD_RX_BUFF_ENABLE;
+
+				if (msg->len - bus->buf_index >
+				    bus->buf_size) {
+					len = bus->buf_size;
+				} else {
+					len = msg->len - bus->buf_index;
+					command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
+				}
+
+				writel(FIELD_PREP(ASPEED_I2CD_BUF_RX_SIZE_MASK,
+						  len - 1) |
+				       FIELD_PREP(ASPEED_I2CD_BUF_TX_COUNT_MASK,
+						  0) |
+				       FIELD_PREP(ASPEED_I2CD_BUF_OFFSET_MASK,
+						  bus->buf_offset),
+				       bus->base + ASPEED_I2C_BUF_CTRL_REG);
+			} else {
+				if (bus->buf_index + 1 == msg->len)
+					command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
+			}
 			writel(command, bus->base + ASPEED_I2C_CMD_REG);
 		} else {
 			aspeed_i2c_next_msg_or_stop(bus);
@@ -893,6 +1073,9 @@ static int aspeed_i2c_init(struct aspeed_i2c_bus *bus,
 	if (ret < 0)
 		return ret;
 
+	fun_ctrl_reg |= FIELD_PREP(ASPEED_I2CD_BUFFER_PAGE_SEL_MASK,
+				   bus->buf_page);
+
 	if (of_property_read_bool(pdev->dev.of_node, "multi-master"))
 		bus->multi_master = true;
 	else
@@ -958,7 +1141,7 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	if (!bus)
 		return -ENOMEM;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "bus-regs");
 	bus->base = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(bus->base))
 		return PTR_ERR(bus->base);
@@ -993,6 +1176,21 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 		bus->get_clk_reg_val = (u32 (*)(struct device *, u32))
 				match->data;
 
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "buf");
+	bus->buf_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(bus->buf_base) || resource_size(res) < 2) {
+		bus->buf_base = NULL;
+	} else {
+		bus->buf_size = resource_size(res);
+		if (of_device_is_compatible(pdev->dev.of_node,
+					    "aspeed,ast2400-i2c-bus")) {
+			bus->buf_page = ((res->start >> 8) &
+					 GENMASK(3, 0)) - 8;
+			bus->buf_offset = (res->start >> 2) &
+					  ASPEED_I2CD_BUF_OFFSET_MASK;
+		}
+	}
+
 	/* Initialize the I2C adapter */
 	spin_lock_init(&bus->lock);
 	init_completion(&bus->cmd_complete);
@@ -1029,8 +1227,8 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, bus);
 
-	dev_info(bus->dev, "i2c bus %d registered, irq %d\n",
-		 bus->adap.nr, irq);
+	dev_info(bus->dev, "i2c bus %d registered (%s mode), irq %d\n",
+		 bus->adap.nr, bus->buf_base ? "buffer" : "byte", irq);
 
 	return 0;
 }
-- 
2.22.0

[RFC PATCH dev-5.1 6/6] i2c: aspeed: add DMA mode transfer support

[Jae Hyun Yoo]

This commit adds DMA mode transfer support for better performance.

Only AST2500 supports DMA mode under some limitations:
I2C is sharing the DMA H/W with UHCI host controller and MCTP
controller. Since those controllers operate with DMA mode only, I2C
has to use buffer mode or byte mode instead if one of those
controllers is enabled. Also make sure that if SD/eMMC or Port80
snoop uses DMA mode instead of PIO or FIFO respectively, I2C can't
use DMA mode.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/i2c/busses/i2c-aspeed.c | 254 ++++++++++++++++++++++++++++----
 1 file changed, 228 insertions(+), 26 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 9e6f71de5c7e..aac7fce1b2f2 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -31,6 +31,9 @@
 #include <linux/reset.h>
 #include <linux/slab.h>
 
+#include <linux/dma-mapping.h>
+#include <linux/dmapool.h>
+
 /* I2C Register */
 #define ASPEED_I2C_FUN_CTRL_REG				0x00
 #define ASPEED_I2C_AC_TIMING_REG1			0x04
@@ -41,6 +44,8 @@
 #define ASPEED_I2C_DEV_ADDR_REG				0x18
 #define ASPEED_I2C_BUF_CTRL_REG				0x1c
 #define ASPEED_I2C_BYTE_BUF_REG				0x20
+#define ASPEED_I2C_DMA_ADDR_REG				0x24
+#define ASPEED_I2C_DMA_LEN_REG				0x28
 
 /* Global Register Definition */
 /* 0x00 : I2C Interrupt Status Register  */
@@ -108,6 +113,8 @@
 #define ASPEED_I2CD_BUS_RECOVER_CMD			BIT(11)
 
 /* Command Bit */
+#define ASPEED_I2CD_RX_DMA_ENABLE			BIT(9)
+#define ASPEED_I2CD_TX_DMA_ENABLE			BIT(8)
 #define ASPEED_I2CD_RX_BUFF_ENABLE			BIT(7)
 #define ASPEED_I2CD_TX_BUFF_ENABLE			BIT(6)
 #define ASPEED_I2CD_M_STOP_CMD				BIT(5)
@@ -127,6 +134,13 @@
 #define ASPEED_I2CD_BUF_TX_COUNT_MASK			GENMASK(15, 8)
 #define ASPEED_I2CD_BUF_OFFSET_MASK			GENMASK(5, 0)
 
+/* 0x24 : I2CD DMA Mode Buffet Address Register */
+#define ASPEED_I2CD_DMA_ADDR_MASK			GENMASK(31, 2)
+#define ASPEED_I2CD_DMA_ALIGN				4
+
+/* 0x28 : I2CD DMA Transfer Length Register */
+#define ASPEED_I2CD_DMA_LEN_MASK			GENMASK(11, 0)
+
 enum aspeed_i2c_master_state {
 	ASPEED_I2C_MASTER_INACTIVE,
 	ASPEED_I2C_MASTER_PENDING,
@@ -177,6 +191,12 @@ struct aspeed_i2c_bus {
 	size_t				buf_size;
 	u8				buf_offset;
 	u8				buf_page;
+	/* DMA mode */
+	struct dma_pool			*dma_pool;
+	dma_addr_t			dma_handle;
+	u8				*dma_buf;
+	size_t				dma_buf_size;
+	size_t				dma_len;
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
 	struct i2c_client		*slave;
 	enum aspeed_i2c_slave_state	slave_state;
@@ -281,8 +301,11 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 
 	/* Slave was sent something. */
 	if (irq_status & ASPEED_I2CD_INTR_RX_DONE) {
-		if (bus->buf_base &&
+		if (bus->dma_buf &&
 		    bus->slave_state == ASPEED_I2C_SLAVE_WRITE_RECEIVED)
+			value = bus->dma_buf[0];
+		else if (bus->buf_base &&
+			 bus->slave_state == ASPEED_I2C_SLAVE_WRITE_RECEIVED)
 			value = readb(bus->buf_base);
 		else
 			value = readl(bus->base + ASPEED_I2C_BYTE_BUF_REG) >> 8;
@@ -300,8 +323,20 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 
 	/* Slave was asked to stop. */
 	if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
-		if (bus->buf_base &&
+		if (bus->dma_buf &&
 		    bus->slave_state == ASPEED_I2C_SLAVE_WRITE_RECEIVED) {
+			len = bus->dma_buf_size -
+			      FIELD_GET(ASPEED_I2CD_DMA_LEN_MASK,
+					readl(bus->base +
+					      ASPEED_I2C_DMA_LEN_REG));
+			for (i = 0; i < len; i++) {
+				value = bus->dma_buf[i];
+				i2c_slave_event(slave,
+						I2C_SLAVE_WRITE_RECEIVED,
+						&value);
+			}
+		} else if (bus->buf_base &&
+			  bus->slave_state == ASPEED_I2C_SLAVE_WRITE_RECEIVED) {
 			len = FIELD_GET(ASPEED_I2CD_BUF_RX_COUNT_MASK,
 					readl(bus->base +
 					      ASPEED_I2C_BUF_CTRL_REG));
@@ -344,7 +379,15 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 	case ASPEED_I2C_SLAVE_WRITE_REQUESTED:
 		bus->slave_state = ASPEED_I2C_SLAVE_WRITE_RECEIVED;
 		i2c_slave_event(slave, I2C_SLAVE_WRITE_REQUESTED, &value);
-		if (bus->buf_base) {
+		if (bus->dma_buf) {
+			writel(bus->dma_handle & ASPEED_I2CD_DMA_ADDR_MASK,
+			       bus->base + ASPEED_I2C_DMA_ADDR_REG);
+			writel(FIELD_PREP(ASPEED_I2CD_DMA_LEN_MASK,
+					  bus->dma_buf_size),
+			       bus->base + ASPEED_I2C_DMA_LEN_REG);
+			writel(ASPEED_I2CD_RX_DMA_ENABLE,
+			       bus->base + ASPEED_I2C_CMD_REG);
+		} else if (bus->buf_base) {
 			writel(FIELD_PREP(ASPEED_I2CD_BUF_RX_SIZE_MASK,
 					  bus->buf_size - 1) |
 			       FIELD_PREP(ASPEED_I2CD_BUF_OFFSET_MASK,
@@ -356,7 +399,25 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 		break;
 	case ASPEED_I2C_SLAVE_WRITE_RECEIVED:
 		i2c_slave_event(slave, I2C_SLAVE_WRITE_RECEIVED, &value);
-		if (bus->buf_base) {
+		if (bus->dma_buf) {
+			len = bus->dma_buf_size -
+			      FIELD_GET(ASPEED_I2CD_DMA_LEN_MASK,
+					readl(bus->base +
+					      ASPEED_I2C_DMA_LEN_REG));
+			for (i = 1; i < len; i++) {
+				value = bus->dma_buf[i];
+				i2c_slave_event(slave,
+						I2C_SLAVE_WRITE_RECEIVED,
+						&value);
+			}
+			writel(bus->dma_handle & ASPEED_I2CD_DMA_ADDR_MASK,
+			       bus->base + ASPEED_I2C_DMA_ADDR_REG);
+			writel(FIELD_PREP(ASPEED_I2CD_DMA_LEN_MASK,
+					  bus->dma_buf_size),
+			       bus->base + ASPEED_I2C_DMA_LEN_REG);
+			writel(ASPEED_I2CD_RX_DMA_ENABLE,
+			       bus->base + ASPEED_I2C_CMD_REG);
+		} else if (bus->buf_base) {
 			len = FIELD_GET(ASPEED_I2CD_BUF_RX_COUNT_MASK,
 					readl(bus->base +
 					      ASPEED_I2C_BUF_CTRL_REG));
@@ -420,7 +481,23 @@ static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
 	if (msg->flags & I2C_M_RD) {
 		command |= ASPEED_I2CD_M_RX_CMD;
 
-		if (bus->buf_base && !(msg->flags & I2C_M_RECV_LEN)) {
+		if (bus->dma_buf && !(msg->flags & I2C_M_RECV_LEN)) {
+			command |= ASPEED_I2CD_RX_DMA_ENABLE;
+
+			if (msg->len > bus->dma_buf_size) {
+				len = bus->dma_buf_size;
+			} else {
+				len = msg->len;
+				command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
+			}
+
+			writel(bus->dma_handle & ASPEED_I2CD_DMA_ADDR_MASK,
+			       bus->base + ASPEED_I2C_DMA_ADDR_REG);
+			writel(FIELD_PREP(ASPEED_I2CD_DMA_LEN_MASK,
+					  len),
+			       bus->base + ASPEED_I2C_DMA_LEN_REG);
+			bus->dma_len = len;
+		} else if (bus->buf_base && !(msg->flags & I2C_M_RECV_LEN)) {
 			command |= ASPEED_I2CD_RX_BUFF_ENABLE;
 
 			if (msg->len > bus->buf_size) {
@@ -441,7 +518,26 @@ static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
 				command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
 		}
 	} else {
-		if (bus->buf_base) {
+		if (bus->dma_buf) {
+			command |= ASPEED_I2CD_TX_DMA_ENABLE;
+
+			if (msg->len + 1 > bus->dma_buf_size)
+				len = bus->dma_buf_size;
+			else
+				len = msg->len + 1;
+
+			bus->dma_buf[0] = slave_addr;
+			memcpy(bus->dma_buf + 1, msg->buf, len);
+
+			bus->buf_index = len - 1;
+
+			writel(bus->dma_handle & ASPEED_I2CD_DMA_ADDR_MASK,
+			       bus->base + ASPEED_I2C_DMA_ADDR_REG);
+			writel(FIELD_PREP(ASPEED_I2CD_DMA_LEN_MASK,
+					  len),
+			       bus->base + ASPEED_I2C_DMA_LEN_REG);
+			bus->dma_len = len;
+		} else if (bus->buf_base) {
 			int i;
 
 			command |= ASPEED_I2CD_TX_BUFF_ENABLE;
@@ -477,7 +573,8 @@ static void aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
 		}
 	}
 
-	if (!(command & ASPEED_I2CD_TX_BUFF_ENABLE))
+	if (!(command & (ASPEED_I2CD_TX_BUFF_ENABLE |
+			 ASPEED_I2CD_TX_DMA_ENABLE)))
 		writel(slave_addr, bus->base + ASPEED_I2C_BYTE_BUF_REG);
 	writel(command, bus->base + ASPEED_I2C_CMD_REG);
 }
@@ -633,7 +730,28 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 		if (bus->buf_index < msg->len) {
 			command = ASPEED_I2CD_M_TX_CMD;
 
-			if (bus->buf_base) {
+			if (bus->dma_buf) {
+				command |= ASPEED_I2CD_TX_DMA_ENABLE;
+
+				if (msg->len - bus->buf_index >
+				    bus->dma_buf_size)
+					len = bus->dma_buf_size;
+				else
+					len = msg->len - bus->buf_index;
+
+				memcpy(bus->dma_buf, msg->buf + bus->buf_index,
+				       len);
+
+				bus->buf_index += len;
+
+				writel(bus->dma_handle &
+				       ASPEED_I2CD_DMA_ADDR_MASK,
+				       bus->base + ASPEED_I2C_DMA_ADDR_REG);
+				writel(FIELD_PREP(ASPEED_I2CD_DMA_LEN_MASK,
+						  len),
+				       bus->base + ASPEED_I2C_DMA_LEN_REG);
+				bus->dma_len = len;
+			} else if (bus->buf_base) {
 				u8 wbuf[4];
 				int i;
 
@@ -684,7 +802,15 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 		}
 		irq_handled |= ASPEED_I2CD_INTR_RX_DONE;
 
-		if (bus->buf_base && !(msg->flags & I2C_M_RECV_LEN)) {
+		if (bus->dma_buf && !(msg->flags & I2C_M_RECV_LEN)) {
+			len = bus->dma_len -
+			      FIELD_GET(ASPEED_I2CD_DMA_LEN_MASK,
+					readl(bus->base +
+					      ASPEED_I2C_DMA_LEN_REG));
+
+			memcpy(msg->buf + bus->buf_index, bus->dma_buf, len);
+			bus->buf_index += len;
+		} else if (bus->buf_base && !(msg->flags & I2C_M_RECV_LEN)) {
 			len = FIELD_GET(ASPEED_I2CD_BUF_RX_COUNT_MASK,
 					readl(bus->base +
 					      ASPEED_I2C_BUF_CTRL_REG));
@@ -712,7 +838,25 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 		if (bus->buf_index < msg->len) {
 			command = ASPEED_I2CD_M_RX_CMD;
 			bus->master_state = ASPEED_I2C_MASTER_RX;
-			if (bus->buf_base) {
+			if (bus->dma_buf) {
+				command |= ASPEED_I2CD_RX_DMA_ENABLE;
+
+				if (msg->len - bus->buf_index >
+				    bus->dma_buf_size) {
+					len = bus->dma_buf_size;
+				} else {
+					len = msg->len - bus->buf_index;
+					command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
+				}
+
+				writel(bus->dma_handle &
+				       ASPEED_I2CD_DMA_ADDR_MASK,
+				       bus->base + ASPEED_I2C_DMA_ADDR_REG);
+				writel(FIELD_PREP(ASPEED_I2CD_DMA_LEN_MASK,
+						  len),
+				       bus->base + ASPEED_I2C_DMA_LEN_REG);
+				bus->dma_len = len;
+			} else if (bus->buf_base) {
 				command |= ASPEED_I2CD_RX_BUFF_ENABLE;
 
 				if (msg->len - bus->buf_index >
@@ -1176,18 +1320,63 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 		bus->get_clk_reg_val = (u32 (*)(struct device *, u32))
 				match->data;
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "buf");
-	bus->buf_base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(bus->buf_base) || resource_size(res) < 2) {
-		bus->buf_base = NULL;
-	} else {
-		bus->buf_size = resource_size(res);
-		if (of_device_is_compatible(pdev->dev.of_node,
-					    "aspeed,ast2400-i2c-bus")) {
-			bus->buf_page = ((res->start >> 8) &
-					 GENMASK(3, 0)) - 8;
-			bus->buf_offset = (res->start >> 2) &
-					  ASPEED_I2CD_BUF_OFFSET_MASK;
+	/*
+	 * Only AST2500 supports DMA mode under some limitations:
+	 * I2C is sharing the DMA H/W with UHCI host controller and MCTP
+	 * controller. Since those controllers operate with DMA mode only, I2C
+	 * has to use buffer mode or byte mode instead if one of those
+	 * controllers is enabled. Also make sure that if SD/eMMC or Port80
+	 * snoop uses DMA mode instead of PIO or FIFO respectively, I2C can't
+	 * use DMA mode.
+	 */
+	if (!IS_ENABLED(CONFIG_USB_UHCI_ASPEED) &&
+	    of_device_is_compatible(pdev->dev.of_node,
+				    "aspeed,ast2500-i2c-bus")) {
+		ret = device_property_read_u32(&pdev->dev,
+					       "aspeed,dma-buf-size",
+					       &bus->dma_buf_size);
+		if (!ret) {
+			if (bus->dma_buf_size > ASPEED_I2CD_DMA_LEN_MASK)
+				bus->dma_buf_size = ASPEED_I2CD_DMA_LEN_MASK;
+		}
+	}
+
+	if (bus->dma_buf_size) {
+		if (dma_set_mask(&pdev->dev, DMA_BIT_MASK(32))) {
+			dev_warn(&pdev->dev, "No suitable DMA available\n");
+		} else {
+			bus->dma_pool = dma_pool_create("i2c-aspeed",
+							&pdev->dev,
+							bus->dma_buf_size,
+							ASPEED_I2CD_DMA_ALIGN,
+							0);
+			if (bus->dma_pool)
+				bus->dma_buf = dma_pool_alloc(bus->dma_pool,
+							      GFP_KERNEL,
+							      &bus->dma_handle);
+
+			if (!bus->dma_buf) {
+				dev_warn(&pdev->dev,
+					 "Cannot allocate DMA buffer\n");
+				dma_pool_destroy(bus->dma_pool);
+			}
+		}
+	}
+
+	if (!bus->dma_buf) {
+		res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "buf");
+		bus->buf_base = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(bus->buf_base) || resource_size(res) < 2) {
+			bus->buf_base = NULL;
+		} else {
+			bus->buf_size = resource_size(res);
+			if (of_device_is_compatible(pdev->dev.of_node,
+						    "aspeed,ast2400-i2c-bus")) {
+				bus->buf_page = ((res->start >> 8) &
+						 GENMASK(3, 0)) - 8;
+				bus->buf_offset = (res->start >> 2) &
+						  ASPEED_I2CD_BUF_OFFSET_MASK;
+			}
 		}
 	}
 
@@ -1213,24 +1402,33 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	 */
 	ret = aspeed_i2c_init(bus, pdev);
 	if (ret < 0)
-		return ret;
+		goto out_free_dma_buf;
 
 	irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
 	ret = devm_request_irq(&pdev->dev, irq, aspeed_i2c_bus_irq,
 			       0, dev_name(&pdev->dev), bus);
 	if (ret < 0)
-		return ret;
+		goto out_free_dma_buf;
 
 	ret = i2c_add_adapter(&bus->adap);
 	if (ret < 0)
-		return ret;
+		goto out_free_dma_buf;
 
 	platform_set_drvdata(pdev, bus);
 
 	dev_info(bus->dev, "i2c bus %d registered (%s mode), irq %d\n",
-		 bus->adap.nr, bus->buf_base ? "buffer" : "byte", irq);
+		 bus->adap.nr, bus->dma_buf ? "dma" :
+					      bus->buf_base ? "buffer" : "byte",
+		 irq);
 
 	return 0;
+
+out_free_dma_buf:
+	if (bus->dma_buf)
+		dma_pool_free(bus->dma_pool, bus->dma_buf, bus->dma_handle);
+	dma_pool_destroy(bus->dma_pool);
+
+	return ret;
 }
 
 static int aspeed_i2c_remove_bus(struct platform_device *pdev)
@@ -1248,6 +1446,10 @@ static int aspeed_i2c_remove_bus(struct platform_device *pdev)
 
 	reset_control_assert(bus->rst);
 
+	if (bus->dma_buf)
+		dma_pool_free(bus->dma_pool, bus->dma_buf, bus->dma_handle);
+	dma_pool_destroy(bus->dma_pool);
+
 	i2c_del_adapter(&bus->adap);
 
 	return 0;
-- 
2.22.0

Re: [RFC PATCH dev-5.1 4/6] i2c: aspeed: fix master pending state handling

[Tao Ren]

On 6/20/19 12:49 PM, Jae Hyun Yoo wrote:
> In case of a master pending state, it should not trigger the master
> command because this H/W is sharing the same data buffer for slave
> and master operation, so this commit fixes the issue with making
> the master command triggering happens when the state goes to active
> state.
> 
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>

Thank you Jae for the patch. I believe I hit the bug while debugging BMC-BIC multi-master communication on my Minipack ast2500 BMC platform. Let me apply the patch and run some testing, will try to share my results by this Friday (Pacific Time).


Cheers,

Tao

Re: [RFC PATCH dev-5.1 4/6] i2c: aspeed: fix master pending state handling

[Jae Hyun Yoo]

On 6/20/2019 1:30 PM, Tao Ren wrote:
> On 6/20/19 12:49 PM, Jae Hyun Yoo wrote:
>> In case of a master pending state, it should not trigger the master
>> command because this H/W is sharing the same data buffer for slave
>> and master operation, so this commit fixes the issue with making
>> the master command triggering happens when the state goes to active
>> state.
>>
>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> 
> Thank you Jae for the patch. I believe I hit the bug while debugging BMC-BIC multi-master communication on my Minipack ast2500 BMC platform. Let me apply the patch and run some testing, will try to share my results by this Friday (Pacific Time).

Thank you Tao for your help for testing it. It would be really helpful
to complete this patch implementation. Will wait the test results.

Regards,
Jae

> 
> 
> Cheers,
> 
> Tao
> 

RE: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM enabling control

[Ryan Chen]

Hello Jae,
	The i2c register setting must after scu reset. - APEED_I2C_SRAM_BUFFER_EN
	My recommend aspeed-i2c-ic.c need be probe after scu reset. And all others i2c bus is no needed for scu reset. 

Ryan 

-----Original Message-----
From: openbmc [mailto:openbmc-bounces+ryan_chen=aspeedtech.com@lists.ozlabs.org] On Behalf Of Jae Hyun Yoo
Sent: Friday, June 21, 2019 3:49 AM
To: Brendan Higgins <brendanhiggins@google.com>; Benjamin Herrenschmidt <benh@kernel.crashing.org>; C?ric Le Goater <clg@kaod.org>; Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>
Cc: openbmc@lists.ozlabs.org; Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Subject: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM enabling control

This commit adds I2C SRAM enabling control for AST2500 SoC to support buffer mode and DMA mode transfer. The SRAM is enabled by default in AST2400 SoC.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/irqchip/irq-aspeed-i2c-ic.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c b/drivers/irqchip/irq-aspeed-i2c-ic.c
index f20200af0992..99985b22a9fa 100644
--- a/drivers/irqchip/irq-aspeed-i2c-ic.c
+++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
@@ -18,6 +18,9 @@
 #include <linux/of_irq.h>
 #include <linux/io.h>
 
+/* I2C Global Control Register (AST2500) */
+#define ASPEED_I2C_GLOBAL_CTRL_REG	0xc
+#define  ASPEED_I2C_SRAM_BUFFER_EN	BIT(0)
 
 #define ASPEED_I2C_IC_NUM_BUS 14
 
@@ -100,6 +103,11 @@ static int __init aspeed_i2c_ic_of_init(struct device_node *node,
 	irq_set_chained_handler_and_data(i2c_ic->parent_irq,
 					 aspeed_i2c_ic_irq_handler, i2c_ic);
 
+	/* Enable I2C SRAM buffer in case of AST2500 */
+	if (of_device_is_compatible(node, "aspeed,ast2500-i2c-ic"))
+		writel(ASPEED_I2C_SRAM_BUFFER_EN,
+		       i2c_ic->base + ASPEED_I2C_GLOBAL_CTRL_REG);
+
 	pr_info("i2c controller registered, irq %d\n", i2c_ic->parent_irq);
 
 	return 0;
--
2.22.0

Re: [RFC PATCH dev-5.1 0/6] Aspeed I2C buffer/DMA mode support

[Cédric Le Goater]

On 20/06/2019 21:49, Jae Hyun Yoo wrote:
> This patch series adds buffer mode and DMA mode transfer support for the
> Aspeed I2C driver. With this change, default transfer mode will be set to
> buffer mode for better performance, and DMA mode can be selectively used
> depends on platform configuration.
> 
> * Buffer mode
>   AST2400:
>     It has 2 KBytes (256 Bytes x 8 pages) of I2C SRAM buffer pool from
>     0x1e78a800 to 0x1e78afff that can be used for all busses with
>     buffer pool manipulation. To simplify implementation for supporting
>     both AST2400 and AST2500, it assigns each 128 Bytes per bus without
>     using buffer pool manipulation so total 1792 Bytes of I2C SRAM
>     buffer will be used.
> 
>   AST2500:
>     It has 16 Bytes of individual I2C SRAM buffer per each bus and its
>     range is from 0x1e78a200 to 0x1e78a2df, so it doesn't have 'buffer
>     page selection' bit field in the Function control register, and
>     neither 'base address pointer' bit field in the Pool buffer control
>     register it has. To simplify implementation for supporting both
>     AST2400 and AST2500, it writes zeros on those register bit fields
>     but it's okay because it does nothing in AST2500.
> 
> * DMA mode
>   Only AST2500 supports DMA mode under some limitations:
>     I2C is sharing the DMA H/W with UHCI host controller and MCTP
>     controller. Since those controllers operate with DMA mode only, I2C
>     has to use buffer mode or byte mode instead if one of those
>     controllers is enabled. Also make sure that if SD/eMMC or Port80
>     snoop uses DMA mode instead of PIO or FIFO respectively, I2C can't
>     use DMA mode..
> 
> I'm submitting this series as an RFC because it needs more test on real
> AST2400 BMC mahines, also it needs to check if QEMU can handle this change
> so please review and test it.

QEMU should have some support for the I2C DMA mode. Lightly tested  
though. The DT would activate it, right ? 

C. 

> Jae Hyun Yoo (6):
>   dt-bindings: i2c: aspeed: add buffer and DMA mode transfer support
>   ARM: dts: aspeed: add I2C buffer mode support
>   irqchip/aspeed-i2c-ic: add I2C SRAM enabling control
>   i2c: aspeed: fix master pending state handling
>   i2c: aspeed: add buffer mode transfer support
>   i2c: aspeed: add DMA mode transfer support
> 
>  .../devicetree/bindings/i2c/i2c-aspeed.txt    |  52 +-
>  arch/arm/boot/dts/aspeed-g4.dtsi              |  42 +-
>  arch/arm/boot/dts/aspeed-g5.dtsi              |  42 +-
>  drivers/i2c/busses/i2c-aspeed.c               | 469 ++++++++++++++++--
>  drivers/irqchip/irq-aspeed-i2c-ic.c           |   8 +
>  5 files changed, 548 insertions(+), 65 deletions(-)
> 

Re: [RFC PATCH dev-5.1 0/6] Aspeed I2C buffer/DMA mode support

[Jae Hyun Yoo]

On 6/21/2019 8:46 AM, Cédric Le Goater wrote:
> On 20/06/2019 21:49, Jae Hyun Yoo wrote:
>> This patch series adds buffer mode and DMA mode transfer support for the
>> Aspeed I2C driver. With this change, default transfer mode will be set to
>> buffer mode for better performance, and DMA mode can be selectively used
>> depends on platform configuration.
>>
>> * Buffer mode
>>    AST2400:
>>      It has 2 KBytes (256 Bytes x 8 pages) of I2C SRAM buffer pool from
>>      0x1e78a800 to 0x1e78afff that can be used for all busses with
>>      buffer pool manipulation. To simplify implementation for supporting
>>      both AST2400 and AST2500, it assigns each 128 Bytes per bus without
>>      using buffer pool manipulation so total 1792 Bytes of I2C SRAM
>>      buffer will be used.
>>
>>    AST2500:
>>      It has 16 Bytes of individual I2C SRAM buffer per each bus and its
>>      range is from 0x1e78a200 to 0x1e78a2df, so it doesn't have 'buffer
>>      page selection' bit field in the Function control register, and
>>      neither 'base address pointer' bit field in the Pool buffer control
>>      register it has. To simplify implementation for supporting both
>>      AST2400 and AST2500, it writes zeros on those register bit fields
>>      but it's okay because it does nothing in AST2500.
>>
>> * DMA mode
>>    Only AST2500 supports DMA mode under some limitations:
>>      I2C is sharing the DMA H/W with UHCI host controller and MCTP
>>      controller. Since those controllers operate with DMA mode only, I2C
>>      has to use buffer mode or byte mode instead if one of those
>>      controllers is enabled. Also make sure that if SD/eMMC or Port80
>>      snoop uses DMA mode instead of PIO or FIFO respectively, I2C can't
>>      use DMA mode..
>>
>> I'm submitting this series as an RFC because it needs more test on real
>> AST2400 BMC mahines, also it needs to check if QEMU can handle this change
>> so please review and test it.
> 
> QEMU should have some support for the I2C DMA mode. Lightly tested
> though. The DT would activate it, right ?
> 
> C.

Hi Cédric,

Right, DMA should be enabled in dt by adding below setting into each bus
node if you are going to use I2C DMA mode.

aspeed,dma-buf-size = <4096>;

Please see an example in the 1/6 patch of this series.

Also, you should disable USB host features by disabling CONFIG_USB from
kernel configs. You could remove it from defconfig like this:
https://gerrit.openbmc-project.xyz/c/openbmc/meta-aspeed/+/22808
Or, you can simply add CONFIG_USB=n into your machine layer kernel
config overlay.

Thanks,
Jae

>> Jae Hyun Yoo (6):
>>    dt-bindings: i2c: aspeed: add buffer and DMA mode transfer support
>>    ARM: dts: aspeed: add I2C buffer mode support
>>    irqchip/aspeed-i2c-ic: add I2C SRAM enabling control
>>    i2c: aspeed: fix master pending state handling
>>    i2c: aspeed: add buffer mode transfer support
>>    i2c: aspeed: add DMA mode transfer support
>>
>>   .../devicetree/bindings/i2c/i2c-aspeed.txt    |  52 +-
>>   arch/arm/boot/dts/aspeed-g4.dtsi              |  42 +-
>>   arch/arm/boot/dts/aspeed-g5.dtsi              |  42 +-
>>   drivers/i2c/busses/i2c-aspeed.c               | 469 ++++++++++++++++--
>>   drivers/irqchip/irq-aspeed-i2c-ic.c           |   8 +
>>   5 files changed, 548 insertions(+), 65 deletions(-)
>>
> 

Re: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM enabling control

[Jae Hyun Yoo]

On 6/20/2019 5:33 PM, Ryan Chen wrote:
> Hello Jae,
> 	The i2c register setting must after scu reset. - APEED_I2C_SRAM_BUFFER_EN
> 	My recommend aspeed-i2c-ic.c need be probe after scu reset. And all others i2c bus is no needed for scu reset.

Hello Ryan,

This module is registered after the SCU reset.
Thank you for the information.

Regards,
Jae

> 
> Ryan
> 
> -----Original Message-----
> From: openbmc [mailto:openbmc-bounces+ryan_chen=aspeedtech.com@lists.ozlabs.org] On Behalf Of Jae Hyun Yoo
> Sent: Friday, June 21, 2019 3:49 AM
> To: Brendan Higgins <brendanhiggins@google.com>; Benjamin Herrenschmidt <benh@kernel.crashing.org>; C?ric Le Goater <clg@kaod.org>; Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>
> Cc: openbmc@lists.ozlabs.org; Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> Subject: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM enabling control
> 
> This commit adds I2C SRAM enabling control for AST2500 SoC to support buffer mode and DMA mode transfer. The SRAM is enabled by default in AST2400 SoC.
> 
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> ---
>   drivers/irqchip/irq-aspeed-i2c-ic.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c b/drivers/irqchip/irq-aspeed-i2c-ic.c
> index f20200af0992..99985b22a9fa 100644
> --- a/drivers/irqchip/irq-aspeed-i2c-ic.c
> +++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
> @@ -18,6 +18,9 @@
>   #include <linux/of_irq.h>
>   #include <linux/io.h>
>   
> +/* I2C Global Control Register (AST2500) */
> +#define ASPEED_I2C_GLOBAL_CTRL_REG	0xc
> +#define  ASPEED_I2C_SRAM_BUFFER_EN	BIT(0)
>   
>   #define ASPEED_I2C_IC_NUM_BUS 14
>   
> @@ -100,6 +103,11 @@ static int __init aspeed_i2c_ic_of_init(struct device_node *node,
>   	irq_set_chained_handler_and_data(i2c_ic->parent_irq,
>   					 aspeed_i2c_ic_irq_handler, i2c_ic);
>   
> +	/* Enable I2C SRAM buffer in case of AST2500 */
> +	if (of_device_is_compatible(node, "aspeed,ast2500-i2c-ic"))
> +		writel(ASPEED_I2C_SRAM_BUFFER_EN,
> +		       i2c_ic->base + ASPEED_I2C_GLOBAL_CTRL_REG);
> +
>   	pr_info("i2c controller registered, irq %d\n", i2c_ic->parent_irq);
>   
>   	return 0;
> --
> 2.22.0
> 

Re: [RFC PATCH dev-5.1 4/6] i2c: aspeed: fix master pending state handling

[Tao Ren]

On 6/20/19 1:34 PM, Jae Hyun Yoo wrote:
> On 6/20/2019 1:30 PM, Tao Ren wrote:
>> On 6/20/19 12:49 PM, Jae Hyun Yoo wrote:
>>> In case of a master pending state, it should not trigger the master
>>> command because this H/W is sharing the same data buffer for slave
>>> and master operation, so this commit fixes the issue with making
>>> the master command triggering happens when the state goes to active
>>> state.
>>>
>>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>>
>> Thank you Jae for the patch. I believe I hit the bug while debugging BMC-BIC multi-master communication on my Minipack ast2500 BMC platform. Let me apply the patch and run some testing, will try to share my results by this Friday (Pacific Time).
> 
> Thank you Tao for your help for testing it. It would be really helpful
> to complete this patch implementation. Will wait the test results.
> 
> Regards,
> Jae

Hi Jae,

Although my problem is not fixed by this patch, nothing gets worse :) And patch itself looks reasonable to me.

The problem I'm facing is: i2c-0 (multi-master) stays in "SLAVE_ACTIVE" state: slave_state is brought back to SLAVE_START again and again due to SLAVE_MATCH interrupt. I didn't get chance to check master state machine, but I'd assume it's always pending which explains why userspace applications get "connection timed out" error for i2c master transfers. 


Cheers,

Tao

Re: [RFC PATCH dev-5.1 5/6] i2c: aspeed: add buffer mode transfer support

[Tao Ren]

On 6/20/19 12:49 PM, Jae Hyun Yoo wrote:
> Byte mode currently this driver uses makes lots of interrupt call
> which isn't good for performance and it makes the driver very
> timing sensitive. To improve performance of the driver, this commit
> adds buffer mode transfer support which uses I2C SRAM buffer
> instead of using a single byte buffer.
> 
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>

Let me apply the patch to my ast2500 BMC platform and will share results earlier next week.


Cheers,

Tao

Re: [RFC PATCH dev-5.1 4/6] i2c: aspeed: fix master pending state handling

[Jae Hyun Yoo]

On 6/21/2019 3:11 PM, Tao Ren wrote:
> On 6/20/19 1:34 PM, Jae Hyun Yoo wrote:
>> On 6/20/2019 1:30 PM, Tao Ren wrote:
>>> On 6/20/19 12:49 PM, Jae Hyun Yoo wrote:
>>>> In case of a master pending state, it should not trigger the master
>>>> command because this H/W is sharing the same data buffer for slave
>>>> and master operation, so this commit fixes the issue with making
>>>> the master command triggering happens when the state goes to active
>>>> state.
>>>>
>>>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>>>
>>> Thank you Jae for the patch. I believe I hit the bug while debugging BMC-BIC multi-master communication on my Minipack ast2500 BMC platform. Let me apply the patch and run some testing, will try to share my results by this Friday (Pacific Time).
>>
>> Thank you Tao for your help for testing it. It would be really helpful
>> to complete this patch implementation. Will wait the test results.
>>
>> Regards,
>> Jae
> 
> Hi Jae,
> 
> Although my problem is not fixed by this patch, nothing gets worse :) And patch itself looks reasonable to me.

Hi Tao,

Thanks for your review.

> 
> The problem I'm facing is: i2c-0 (multi-master) stays in "SLAVE_ACTIVE" state: slave_state is brought back to SLAVE_START again and again due to SLAVE_MATCH interrupt. I didn't get chance to check master state machine, but I'd assume it's always pending which explains why userspace applications get "connection timed out" error for i2c master transfers.

I haven't seen this issue in my machine. Probably it needs to enable
'slave timeout' feature which AST2500 provides.

Thanks,
Jae

> 
> Cheers,
> 
> Tao
> 

Re: [RFC PATCH dev-5.1 5/6] i2c: aspeed: add buffer mode transfer support

[Jae Hyun Yoo]

On 6/21/2019 3:29 PM, Tao Ren wrote:
> On 6/20/19 12:49 PM, Jae Hyun Yoo wrote:
>> Byte mode currently this driver uses makes lots of interrupt call
>> which isn't good for performance and it makes the driver very
>> timing sensitive. To improve performance of the driver, this commit
>> adds buffer mode transfer support which uses I2C SRAM buffer
>> instead of using a single byte buffer.
>>
>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> 
> Let me apply the patch to my ast2500 BMC platform and will share results earlier next week.

Thanks Tao! It would be helpful.

Regards,
Jae

> Cheers,
> 
> Tao
> 

Re: [RFC PATCH dev-5.1 5/6] i2c: aspeed: add buffer mode transfer support

[Tao Ren]

On 6/21/19 3:34 PM, Jae Hyun Yoo wrote:
> On 6/21/2019 3:29 PM, Tao Ren wrote:
>> On 6/20/19 12:49 PM, Jae Hyun Yoo wrote:
>>> Byte mode currently this driver uses makes lots of interrupt call
>>> which isn't good for performance and it makes the driver very
>>> timing sensitive. To improve performance of the driver, this commit
>>> adds buffer mode transfer support which uses I2C SRAM buffer
>>> instead of using a single byte buffer.
>>>
>>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>>
>> Let me apply the patch to my ast2500 BMC platform and will share results earlier next week.
> 
> Thanks Tao! It would be helpful.

Hi Jae,

I applied patches 1-5 (except DMA mode) to my local tree: buffer mode is enabled by default, and reg_base/buf_base also looks correct to me, but I'm seeing some pca9548/lm75 driver binding failures on CMM BMC. I'm wondering if I missed some dependency patches as I'm still working on kernel 5.0 (although i2c-aspeed.c is up to date)?

BTW, USB is enabled in my image because both EHCI and UHCI are needed. But I guess it won't impact buffer mode?

Let me see if I can quickly set up kernel 5.1 and re-run testing; meanwhile, kindly let me know what information will be helpful for your debugging.


Cheers,

Tao

Re: [RFC PATCH dev-5.1 5/6] i2c: aspeed: add buffer mode transfer support

[Jae Hyun Yoo]

On 6/24/2019 4:54 PM, Tao Ren wrote:
> On 6/21/19 3:34 PM, Jae Hyun Yoo wrote:
>> On 6/21/2019 3:29 PM, Tao Ren wrote:
>>> On 6/20/19 12:49 PM, Jae Hyun Yoo wrote:
>>>> Byte mode currently this driver uses makes lots of interrupt call
>>>> which isn't good for performance and it makes the driver very
>>>> timing sensitive. To improve performance of the driver, this commit
>>>> adds buffer mode transfer support which uses I2C SRAM buffer
>>>> instead of using a single byte buffer.
>>>>
>>>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>>>
>>> Let me apply the patch to my ast2500 BMC platform and will share results earlier next week.
>>
>> Thanks Tao! It would be helpful.
> 
> Hi Jae,
> 
> I applied patches 1-5 (except DMA mode) to my local tree: buffer mode is enabled by default, and reg_base/buf_base also looks correct to me, but I'm seeing some pca9548/lm75 driver binding failures on CMM BMC. I'm wondering if I missed some dependency patches as I'm still working on kernel 5.0 (although i2c-aspeed.c is up to date)?

Hi Tao,

It seems that I2C SRAM isn't enabled in your system. I forgot that Intel
local tree has I2C driver in u-boot so SRAM is enabled because u-boot
makes an I2C reset, but in other OpenBMC systems, it would not work
correctly because SRAM enabling control is triggered before an I2C
reset. I will move the SRAM control code from irq-aspeed-i2c-ic module
to i2c-aspeed module just after the deasserting of I2C reset. Will
submit v2 soon.

> 
> BTW, USB is enabled in my image because both EHCI and UHCI are needed. But I guess it won't impact buffer mode?

This patch series adds DMA support code but it doesn't set the transfer
mode to DMA. It sets buffer mode as default transfer mode. Even in case
you enable DMA, if CONFIG_USB_UHCI_ASPEED is enabled then buffer mode
will be selected instead of DMA by checking code in i2c-aspeed module.

Thanks,
Jae

> 
> Let me see if I can quickly set up kernel 5.1 and re-run testing; meanwhile, kindly let me know what information will be helpful for your debugging.
> 
> 
> Cheers,
> 
> Tao
> 

Re: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM enabling control

[Jae Hyun Yoo]

On 6/21/2019 11:41 AM, Jae Hyun Yoo wrote:
> On 6/20/2019 5:33 PM, Ryan Chen wrote:
>> Hello Jae,
>>     The i2c register setting must after scu reset. - 
>> APEED_I2C_SRAM_BUFFER_EN
>>     My recommend aspeed-i2c-ic.c need be probe after scu reset. And 
>> all others i2c bus is no needed for scu reset.
> 
> Hello Ryan,
> 
> This module is registered after the SCU reset.
> Thank you for the information.
> 
> Regards,
> Jae

Hello Ryan,

I got your point now. You meant the I2C H/W reset through SCU04
register, right? I'll move the SRAM buffer enable control from
irq-aspeed-i2c-ic module to i2c-aspeed module so that the SRAM can be
enabled correctly.

Thanks for your pointing it out.

Jae

>>
>> Ryan
>>
>> -----Original Message-----
>> From: openbmc 
>> [mailto:openbmc-bounces+ryan_chen=aspeedtech.com@lists.ozlabs.org] On 
>> Behalf Of Jae Hyun Yoo
>> Sent: Friday, June 21, 2019 3:49 AM
>> To: Brendan Higgins <brendanhiggins@google.com>; Benjamin 
>> Herrenschmidt <benh@kernel.crashing.org>; C?ric Le Goater 
>> <clg@kaod.org>; Joel Stanley <joel@jms.id.au>; Andrew Jeffery 
>> <andrew@aj.id.au>
>> Cc: openbmc@lists.ozlabs.org; Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>> Subject: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM 
>> enabling control
>>
>> This commit adds I2C SRAM enabling control for AST2500 SoC to support 
>> buffer mode and DMA mode transfer. The SRAM is enabled by default in 
>> AST2400 SoC.
>>
>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>> ---
>>   drivers/irqchip/irq-aspeed-i2c-ic.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c 
>> b/drivers/irqchip/irq-aspeed-i2c-ic.c
>> index f20200af0992..99985b22a9fa 100644
>> --- a/drivers/irqchip/irq-aspeed-i2c-ic.c
>> +++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
>> @@ -18,6 +18,9 @@
>>   #include <linux/of_irq.h>
>>   #include <linux/io.h>
>> +/* I2C Global Control Register (AST2500) */
>> +#define ASPEED_I2C_GLOBAL_CTRL_REG    0xc
>> +#define  ASPEED_I2C_SRAM_BUFFER_EN    BIT(0)
>>   #define ASPEED_I2C_IC_NUM_BUS 14
>> @@ -100,6 +103,11 @@ static int __init aspeed_i2c_ic_of_init(struct 
>> device_node *node,
>>       irq_set_chained_handler_and_data(i2c_ic->parent_irq,
>>                        aspeed_i2c_ic_irq_handler, i2c_ic);
>> +    /* Enable I2C SRAM buffer in case of AST2500 */
>> +    if (of_device_is_compatible(node, "aspeed,ast2500-i2c-ic"))
>> +        writel(ASPEED_I2C_SRAM_BUFFER_EN,
>> +               i2c_ic->base + ASPEED_I2C_GLOBAL_CTRL_REG);
>> +
>>       pr_info("i2c controller registered, irq %d\n", i2c_ic->parent_irq);
>>       return 0;
>> -- 
>> 2.22.0
>>

RE: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM enabling control

[Ryan Chen]

Hello Jae,
	Actually, my recommend is following.
	1. Move i2c scu reset to irq-aspeed-i2c-ic.c and also keep SRAM buffer enable.
	2. remove i2c each bus reset. 
Ryan 

-----Original Message-----
From: Jae Hyun Yoo [mailto:jae.hyun.yoo@linux.intel.com] 
Sent: Wednesday, June 26, 2019 1:23 AM
To: Ryan Chen <ryan_chen@aspeedtech.com>; Brendan Higgins <brendanhiggins@google.com>; Benjamin Herrenschmidt <benh@kernel.crashing.org>; C?ric Le Goater <clg@kaod.org>; Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>
Cc: openbmc@lists.ozlabs.org
Subject: Re: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM enabling control

On 6/21/2019 11:41 AM, Jae Hyun Yoo wrote:
> On 6/20/2019 5:33 PM, Ryan Chen wrote:
>> Hello Jae,
>>     The i2c register setting must after scu reset. - 
>> APEED_I2C_SRAM_BUFFER_EN
>>     My recommend aspeed-i2c-ic.c need be probe after scu reset. And 
>> all others i2c bus is no needed for scu reset.
> 
> Hello Ryan,
> 
> This module is registered after the SCU reset.
> Thank you for the information.
> 
> Regards,
> Jae

Hello Ryan,

I got your point now. You meant the I2C H/W reset through SCU04 register, right? I'll move the SRAM buffer enable control from irq-aspeed-i2c-ic module to i2c-aspeed module so that the SRAM can be enabled correctly.

Thanks for your pointing it out.

Jae

>>
>> Ryan
>>
>> -----Original Message-----
>> From: openbmc
>> [mailto:openbmc-bounces+ryan_chen=aspeedtech.com@lists.ozlabs.org] On 
>> Behalf Of Jae Hyun Yoo
>> Sent: Friday, June 21, 2019 3:49 AM
>> To: Brendan Higgins <brendanhiggins@google.com>; Benjamin 
>> Herrenschmidt <benh@kernel.crashing.org>; C?ric Le Goater 
>> <clg@kaod.org>; Joel Stanley <joel@jms.id.au>; Andrew Jeffery 
>> <andrew@aj.id.au>
>> Cc: openbmc@lists.ozlabs.org; Jae Hyun Yoo 
>> <jae.hyun.yoo@linux.intel.com>
>> Subject: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM 
>> enabling control
>>
>> This commit adds I2C SRAM enabling control for AST2500 SoC to support 
>> buffer mode and DMA mode transfer. The SRAM is enabled by default in
>> AST2400 SoC.
>>
>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>> ---
>>   drivers/irqchip/irq-aspeed-i2c-ic.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c
>> b/drivers/irqchip/irq-aspeed-i2c-ic.c
>> index f20200af0992..99985b22a9fa 100644
>> --- a/drivers/irqchip/irq-aspeed-i2c-ic.c
>> +++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
>> @@ -18,6 +18,9 @@
>>   #include <linux/of_irq.h>
>>   #include <linux/io.h>
>> +/* I2C Global Control Register (AST2500) */ #define 
>> +ASPEED_I2C_GLOBAL_CTRL_REG    0xc #define  ASPEED_I2C_SRAM_BUFFER_EN    
>> +BIT(0)
>>   #define ASPEED_I2C_IC_NUM_BUS 14
>> @@ -100,6 +103,11 @@ static int __init aspeed_i2c_ic_of_init(struct 
>> device_node *node,
>>       irq_set_chained_handler_and_data(i2c_ic->parent_irq,
>>                        aspeed_i2c_ic_irq_handler, i2c_ic);
>> +    /* Enable I2C SRAM buffer in case of AST2500 */
>> +    if (of_device_is_compatible(node, "aspeed,ast2500-i2c-ic"))
>> +        writel(ASPEED_I2C_SRAM_BUFFER_EN,
>> +               i2c_ic->base + ASPEED_I2C_GLOBAL_CTRL_REG);
>> +
>>       pr_info("i2c controller registered, irq %d\n", 
>> i2c_ic->parent_irq);
>>       return 0;
>> --
>> 2.22.0
>>

Re: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM enabling control

[Jae Hyun Yoo]

On 6/25/2019 10:10 PM, Ryan Chen wrote:
> Hello Jae,
> 	Actually, my recommend is following.
> 	1. Move i2c scu reset to irq-aspeed-i2c-ic.c and also keep SRAM buffer enable.
> 	2. remove i2c each bus reset.

Hello Ryan,

My thought is, irq-aspeed-i2c-ic module should do things only irqchip
drivers do. i2c-aspeed module has been doing the I2C SCU reset control
correctly so far without making any problem.

Regards,
Jae

> Ryan
> 
> -----Original Message-----
> From: Jae Hyun Yoo [mailto:jae.hyun.yoo@linux.intel.com]
> Sent: Wednesday, June 26, 2019 1:23 AM
> To: Ryan Chen <ryan_chen@aspeedtech.com>; Brendan Higgins <brendanhiggins@google.com>; Benjamin Herrenschmidt <benh@kernel.crashing.org>; C?ric Le Goater <clg@kaod.org>; Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>
> Cc: openbmc@lists.ozlabs.org
> Subject: Re: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM enabling control
> 
> On 6/21/2019 11:41 AM, Jae Hyun Yoo wrote:
>> On 6/20/2019 5:33 PM, Ryan Chen wrote:
>>> Hello Jae,
>>>      The i2c register setting must after scu reset. -
>>> APEED_I2C_SRAM_BUFFER_EN
>>>      My recommend aspeed-i2c-ic.c need be probe after scu reset. And
>>> all others i2c bus is no needed for scu reset.
>>
>> Hello Ryan,
>>
>> This module is registered after the SCU reset.
>> Thank you for the information.
>>
>> Regards,
>> Jae
> 
> Hello Ryan,
> 
> I got your point now. You meant the I2C H/W reset through SCU04 register, right? I'll move the SRAM buffer enable control from irq-aspeed-i2c-ic module to i2c-aspeed module so that the SRAM can be enabled correctly.
> 
> Thanks for your pointing it out.
> 
> Jae
> 
>>>
>>> Ryan
>>>
>>> -----Original Message-----
>>> From: openbmc
>>> [mailto:openbmc-bounces+ryan_chen=aspeedtech.com@lists.ozlabs.org] On
>>> Behalf Of Jae Hyun Yoo
>>> Sent: Friday, June 21, 2019 3:49 AM
>>> To: Brendan Higgins <brendanhiggins@google.com>; Benjamin
>>> Herrenschmidt <benh@kernel.crashing.org>; C?ric Le Goater
>>> <clg@kaod.org>; Joel Stanley <joel@jms.id.au>; Andrew Jeffery
>>> <andrew@aj.id.au>
>>> Cc: openbmc@lists.ozlabs.org; Jae Hyun Yoo
>>> <jae.hyun.yoo@linux.intel.com>
>>> Subject: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM
>>> enabling control
>>>
>>> This commit adds I2C SRAM enabling control for AST2500 SoC to support
>>> buffer mode and DMA mode transfer. The SRAM is enabled by default in
>>> AST2400 SoC.
>>>
>>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>>> ---
>>>    drivers/irqchip/irq-aspeed-i2c-ic.c | 8 ++++++++
>>>    1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c
>>> b/drivers/irqchip/irq-aspeed-i2c-ic.c
>>> index f20200af0992..99985b22a9fa 100644
>>> --- a/drivers/irqchip/irq-aspeed-i2c-ic.c
>>> +++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
>>> @@ -18,6 +18,9 @@
>>>    #include <linux/of_irq.h>
>>>    #include <linux/io.h>
>>> +/* I2C Global Control Register (AST2500) */ #define
>>> +ASPEED_I2C_GLOBAL_CTRL_REG    0xc #define  ASPEED_I2C_SRAM_BUFFER_EN
>>> +BIT(0)
>>>    #define ASPEED_I2C_IC_NUM_BUS 14
>>> @@ -100,6 +103,11 @@ static int __init aspeed_i2c_ic_of_init(struct
>>> device_node *node,
>>>        irq_set_chained_handler_and_data(i2c_ic->parent_irq,
>>>                         aspeed_i2c_ic_irq_handler, i2c_ic);
>>> +    /* Enable I2C SRAM buffer in case of AST2500 */
>>> +    if (of_device_is_compatible(node, "aspeed,ast2500-i2c-ic"))
>>> +        writel(ASPEED_I2C_SRAM_BUFFER_EN,
>>> +               i2c_ic->base + ASPEED_I2C_GLOBAL_CTRL_REG);
>>> +
>>>        pr_info("i2c controller registered, irq %d\n",
>>> i2c_ic->parent_irq);
>>>        return 0;
>>> --
>>> 2.22.0
>>>