[PATCH v1 0/5] Add Block event interrupt support for I2C protocol

[PATCH v1 0/5] Add Block event interrupt support for I2C protocol

[Jyothi Kumar Seerapu]

The I2C driver gets an interrupt upon transfer completion.
For multiple messages in a single transfer, N interrupts will be
received for N messages, leading to significant software interrupt
latency. To mitigate this latency, utilize Block Event Interrupt (BEI)
only when an interrupt is necessary. This means large transfers can be
split into multiple chunks of 64 messages internally, without expecting
interrupts for the first 63 transfers, only the last one will trigger
an interrupt indicating 64 transfers completed.

By implementing BEI, multi-message transfers can be divided into
chunks of 64 messages, improving overall transfer time.
This optimization reduces transfer time from 168 ms to 48 ms for a
series of 200 I2C write messages in a single transfer, with a
clock frequency support of 100 kHz.

BEI optimizations are currently implemented for I2C write transfers only,
as there is no use case for multiple I2C read messages in a single transfer
at this time.

Jyothi Kumar Seerapu (5):
  dt-bindings: dmaengine: qcom: gpi: Add additional arg to dma-cell
    property
  arm64: dts: qcom: Add support for configuring channel TRE size
  dmaengine: qcom: gpi: Add provision to support TRE size as the fourth
    argument of dma-cells property
  dmaengine: qcom: gpi: Add GPI Block event interrupt support
  i2c: i2c-qcom-geni: Add Block event interrupt support

 .../devicetree/bindings/dma/qcom,gpi.yaml     |   6 +-
 arch/arm64/boot/dts/qcom/sc7280.dtsi          | 132 +++++------
 drivers/dma/qcom/gpi.c                        |  74 ++++++-
 drivers/i2c/busses/i2c-qcom-geni.c            | 205 ++++++++++++++++--
 include/linux/dma/qcom-gpi-dma.h              |  37 ++++
 5 files changed, 355 insertions(+), 99 deletions(-)


base-commit: 55bcd2e0d04c1171d382badef1def1fd04ef66c5
-- 
2.17.1

[PATCH v1 1/5] dt-bindings: dmaengine: qcom: gpi: Add additional arg to dma-cell property

[Jyothi Kumar Seerapu]

When high performance with multiple i2c messages in a single transfer
is required, employ Block Event Interrupt (BEI) to trigger interrupts
after specific messages transfer and the last message transfer,
thereby reducing interrupts.

For each i2c message transfer, a series of Transfer Request Elements(TREs)
must be programmed, including config tre for frequency configuration,
go tre for holding i2c address and dma tre for holding dma buffer address,
length as per the hardware programming guide. For transfer using BEI,
multiple I2C messages may necessitate the preparation of config, go,
and tx DMA TREs. However, a channel TRE size of 64 is often insufficient,
potentially leading to failures due to inadequate memory space.

Add additional argument to dma-cell property for channel TRE size.
With this, adjust the channel TRE size via the device tree.
The default size is 64, but clients can modify this value based on
their specific requirements.

Signed-off-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com>
---
 Documentation/devicetree/bindings/dma/qcom,gpi.yaml | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
index 4df4e61895d2..002495921643 100644
--- a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
+++ b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
@@ -54,14 +54,16 @@ properties:
     maxItems: 13
 
   "#dma-cells":
-    const: 3
+    minItems: 3
+    maxItems: 4
     description: >
       DMA clients must use the format described in dma.txt, giving a phandle
-      to the DMA controller plus the following 3 integer cells:
+      to the DMA controller plus the following 4 integer cells:
       - channel: if set to 0xffffffff, any available channel will be allocated
         for the client. Otherwise, the exact channel specified will be used.
       - seid: serial id of the client as defined in the SoC documentation.
       - client: type of the client as defined in dt-bindings/dma/qcom-gpi.h
+      - channel-tre-size: size of the channel TRE (transfer ring element)
 
   iommus:
     maxItems: 1
-- 
2.17.1

[PATCH v1 2/5] arm64: dts: qcom: Add support for configuring channel TRE size

[Jyothi Kumar Seerapu]

When high performance with multiple i2c messages in a single transfer
is required, employ Block Event Interrupt (BEI) to trigger interrupts
after specific messages transfer and the last message transfer,
thereby reducing interrupts.
For each i2c message transfer, a series of Transfer Request Elements(TREs)
must be programmed, including config tre for frequency configuration,
go tre for holding i2c address and dma tre for holding dma buffer address,
length as per the hardware programming guide. For transfer using BEI,
multiple I2C messages may necessitate the preparation of config, go,
and tx DMA TREs. However, a channel TRE size of 64 is often insufficient,
potentially leading to failures due to inadequate memory space.

Adjust the channel TRE size through the device tree.
The default size is 64, but clients can modify this value based on
their heigher channel TRE size requirements.

Signed-off-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 132 +++++++++++++--------------
 1 file changed, 66 insertions(+), 66 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 3d8410683402..c7c0e15ff9d3 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -1064,7 +1064,7 @@
 		};
 
 		gpi_dma0: dma-controller@900000 {
-			#dma-cells = <3>;
+			#dma-cells = <4>;
 			compatible = "qcom,sc7280-gpi-dma", "qcom,sm6350-gpi-dma";
 			reg = <0 0x00900000 0 0x60000>;
 			interrupts = <GIC_SPI 244 IRQ_TYPE_LEVEL_HIGH>,
@@ -1114,8 +1114,8 @@
 							"qup-memory";
 				power-domains = <&rpmhpd SC7280_CX>;
 				required-opps = <&rpmhpd_opp_low_svs>;
-				dmas = <&gpi_dma0 0 0 QCOM_GPI_I2C>,
-				       <&gpi_dma0 1 0 QCOM_GPI_I2C>;
+				dmas = <&gpi_dma0 0 0 QCOM_GPI_I2C 64>,
+				       <&gpi_dma0 1 0 QCOM_GPI_I2C 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1135,8 +1135,8 @@
 				interconnects = <&clk_virt MASTER_QUP_CORE_0 0 &clk_virt SLAVE_QUP_CORE_0 0>,
 						<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_0 0>;
 				interconnect-names = "qup-core", "qup-config";
-				dmas = <&gpi_dma0 0 0 QCOM_GPI_SPI>,
-				       <&gpi_dma0 1 0 QCOM_GPI_SPI>;
+				dmas = <&gpi_dma0 0 0 QCOM_GPI_SPI 64>,
+				       <&gpi_dma0 1 0 QCOM_GPI_SPI 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1174,8 +1174,8 @@
 							"qup-memory";
 				power-domains = <&rpmhpd SC7280_CX>;
 				required-opps = <&rpmhpd_opp_low_svs>;
-				dmas = <&gpi_dma0 0 1 QCOM_GPI_I2C>,
-				       <&gpi_dma0 1 1 QCOM_GPI_I2C>;
+				dmas = <&gpi_dma0 0 1 QCOM_GPI_I2C 64>,
+				       <&gpi_dma0 1 1 QCOM_GPI_I2C 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1195,8 +1195,8 @@
 				interconnects = <&clk_virt MASTER_QUP_CORE_0 0 &clk_virt SLAVE_QUP_CORE_0 0>,
 						<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_0 0>;
 				interconnect-names = "qup-core", "qup-config";
-				dmas = <&gpi_dma0 0 1 QCOM_GPI_SPI>,
-				       <&gpi_dma0 1 1 QCOM_GPI_SPI>;
+				dmas = <&gpi_dma0 0 1 QCOM_GPI_SPI 64>,
+				       <&gpi_dma0 1 1 QCOM_GPI_SPI 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1234,8 +1234,8 @@
 							"qup-memory";
 				power-domains = <&rpmhpd SC7280_CX>;
 				required-opps = <&rpmhpd_opp_low_svs>;
-				dmas = <&gpi_dma0 0 2 QCOM_GPI_I2C>,
-				       <&gpi_dma0 1 2 QCOM_GPI_I2C>;
+				dmas = <&gpi_dma0 0 2 QCOM_GPI_I2C 64>,
+				       <&gpi_dma0 1 2 QCOM_GPI_I2C 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1255,8 +1255,8 @@
 				interconnects = <&clk_virt MASTER_QUP_CORE_0 0 &clk_virt SLAVE_QUP_CORE_0 0>,
 						<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_0 0>;
 				interconnect-names = "qup-core", "qup-config";
-				dmas = <&gpi_dma0 0 2 QCOM_GPI_SPI>,
-				       <&gpi_dma0 1 2 QCOM_GPI_SPI>;
+				dmas = <&gpi_dma0 0 2 QCOM_GPI_SPI 64>,
+				       <&gpi_dma0 1 2 QCOM_GPI_SPI 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1294,8 +1294,8 @@
 							"qup-memory";
 				power-domains = <&rpmhpd SC7280_CX>;
 				required-opps = <&rpmhpd_opp_low_svs>;
-				dmas = <&gpi_dma0 0 3 QCOM_GPI_I2C>,
-				       <&gpi_dma0 1 3 QCOM_GPI_I2C>;
+				dmas = <&gpi_dma0 0 3 QCOM_GPI_I2C 64>,
+				       <&gpi_dma0 1 3 QCOM_GPI_I2C 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1315,8 +1315,8 @@
 				interconnects = <&clk_virt MASTER_QUP_CORE_0 0 &clk_virt SLAVE_QUP_CORE_0 0>,
 						<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_0 0>;
 				interconnect-names = "qup-core", "qup-config";
-				dmas = <&gpi_dma0 0 3 QCOM_GPI_SPI>,
-				       <&gpi_dma0 1 3 QCOM_GPI_SPI>;
+				dmas = <&gpi_dma0 0 3 QCOM_GPI_SPI 64>,
+				       <&gpi_dma0 1 3 QCOM_GPI_SPI 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1354,8 +1354,8 @@
 							"qup-memory";
 				power-domains = <&rpmhpd SC7280_CX>;
 				required-opps = <&rpmhpd_opp_low_svs>;
-				dmas = <&gpi_dma0 0 4 QCOM_GPI_I2C>,
-				       <&gpi_dma0 1 4 QCOM_GPI_I2C>;
+				dmas = <&gpi_dma0 0 4 QCOM_GPI_I2C 64>,
+				       <&gpi_dma0 1 4 QCOM_GPI_I2C 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1375,8 +1375,8 @@
 				interconnects = <&clk_virt MASTER_QUP_CORE_0 0 &clk_virt SLAVE_QUP_CORE_0 0>,
 						<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_0 0>;
 				interconnect-names = "qup-core", "qup-config";
-				dmas = <&gpi_dma0 0 4 QCOM_GPI_SPI>,
-				       <&gpi_dma0 1 4 QCOM_GPI_SPI>;
+				dmas = <&gpi_dma0 0 4 QCOM_GPI_SPI 64>,
+				       <&gpi_dma0 1 4 QCOM_GPI_SPI 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1414,8 +1414,8 @@
 							"qup-memory";
 				power-domains = <&rpmhpd SC7280_CX>;
 				required-opps = <&rpmhpd_opp_low_svs>;
-				dmas = <&gpi_dma0 0 5 QCOM_GPI_I2C>,
-				       <&gpi_dma0 1 5 QCOM_GPI_I2C>;
+				dmas = <&gpi_dma0 0 5 QCOM_GPI_I2C 64>,
+				       <&gpi_dma0 1 5 QCOM_GPI_I2C 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1435,8 +1435,8 @@
 				interconnects = <&clk_virt MASTER_QUP_CORE_0 0 &clk_virt SLAVE_QUP_CORE_0 0>,
 						<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_0 0>;
 				interconnect-names = "qup-core", "qup-config";
-				dmas = <&gpi_dma0 0 5 QCOM_GPI_SPI>,
-				       <&gpi_dma0 1 5 QCOM_GPI_SPI>;
+				dmas = <&gpi_dma0 0 5 QCOM_GPI_SPI 64>,
+				       <&gpi_dma0 1 5 QCOM_GPI_SPI 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1474,8 +1474,8 @@
 							"qup-memory";
 				power-domains = <&rpmhpd SC7280_CX>;
 				required-opps = <&rpmhpd_opp_low_svs>;
-				dmas = <&gpi_dma0 0 6 QCOM_GPI_I2C>,
-				       <&gpi_dma0 1 6 QCOM_GPI_I2C>;
+				dmas = <&gpi_dma0 0 6 QCOM_GPI_I2C 64>,
+				       <&gpi_dma0 1 6 QCOM_GPI_I2C 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1495,8 +1495,8 @@
 				interconnects = <&clk_virt MASTER_QUP_CORE_0 0 &clk_virt SLAVE_QUP_CORE_0 0>,
 						<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_0 0>;
 				interconnect-names = "qup-core", "qup-config";
-				dmas = <&gpi_dma0 0 6 QCOM_GPI_SPI>,
-				       <&gpi_dma0 1 6 QCOM_GPI_SPI>;
+				dmas = <&gpi_dma0 0 6 QCOM_GPI_SPI 64>,
+				       <&gpi_dma0 1 6 QCOM_GPI_SPI 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1534,8 +1534,8 @@
 							"qup-memory";
 				power-domains = <&rpmhpd SC7280_CX>;
 				required-opps = <&rpmhpd_opp_low_svs>;
-				dmas = <&gpi_dma0 0 7 QCOM_GPI_I2C>,
-				       <&gpi_dma0 1 7 QCOM_GPI_I2C>;
+				dmas = <&gpi_dma0 0 7 QCOM_GPI_I2C 64>,
+				       <&gpi_dma0 1 7 QCOM_GPI_I2C 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1555,8 +1555,8 @@
 				interconnects = <&clk_virt MASTER_QUP_CORE_0 0 &clk_virt SLAVE_QUP_CORE_0 0>,
 						<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_0 0>;
 				interconnect-names = "qup-core", "qup-config";
-				dmas = <&gpi_dma0 0 7 QCOM_GPI_SPI>,
-				       <&gpi_dma0 1 7 QCOM_GPI_SPI>;
+				dmas = <&gpi_dma0 0 7 QCOM_GPI_SPI 64>,
+				       <&gpi_dma0 1 7 QCOM_GPI_SPI 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1579,7 +1579,7 @@
 		};
 
 		gpi_dma1: dma-controller@a00000 {
-			#dma-cells = <3>;
+			#dma-cells = <4>;
 			compatible = "qcom,sc7280-gpi-dma", "qcom,sm6350-gpi-dma";
 			reg = <0 0x00a00000 0 0x60000>;
 			interrupts = <GIC_SPI 279 IRQ_TYPE_LEVEL_HIGH>,
@@ -1629,8 +1629,8 @@
 							"qup-memory";
 				power-domains = <&rpmhpd SC7280_CX>;
 				required-opps = <&rpmhpd_opp_low_svs>;
-				dmas = <&gpi_dma1 0 0 QCOM_GPI_I2C>,
-				       <&gpi_dma1 1 0 QCOM_GPI_I2C>;
+				dmas = <&gpi_dma1 0 0 QCOM_GPI_I2C 64>,
+				       <&gpi_dma1 1 0 QCOM_GPI_I2C 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1650,8 +1650,8 @@
 				interconnects = <&clk_virt MASTER_QUP_CORE_1 0 &clk_virt SLAVE_QUP_CORE_1 0>,
 						<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_1 0>;
 				interconnect-names = "qup-core", "qup-config";
-				dmas = <&gpi_dma1 0 0 QCOM_GPI_SPI>,
-				       <&gpi_dma1 1 0 QCOM_GPI_SPI>;
+				dmas = <&gpi_dma1 0 0 QCOM_GPI_SPI 64>,
+				       <&gpi_dma1 1 0 QCOM_GPI_SPI 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1689,8 +1689,8 @@
 							"qup-memory";
 				power-domains = <&rpmhpd SC7280_CX>;
 				required-opps = <&rpmhpd_opp_low_svs>;
-				dmas = <&gpi_dma1 0 1 QCOM_GPI_I2C>,
-				       <&gpi_dma1 1 1 QCOM_GPI_I2C>;
+				dmas = <&gpi_dma1 0 1 QCOM_GPI_I2C 64>,
+				       <&gpi_dma1 1 1 QCOM_GPI_I2C 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1710,8 +1710,8 @@
 				interconnects = <&clk_virt MASTER_QUP_CORE_1 0 &clk_virt SLAVE_QUP_CORE_1 0>,
 						<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_1 0>;
 				interconnect-names = "qup-core", "qup-config";
-				dmas = <&gpi_dma1 0 1 QCOM_GPI_SPI>,
-				       <&gpi_dma1 1 1 QCOM_GPI_SPI>;
+				dmas = <&gpi_dma1 0 1 QCOM_GPI_SPI 64>,
+				       <&gpi_dma1 1 1 QCOM_GPI_SPI 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1749,8 +1749,8 @@
 							"qup-memory";
 				power-domains = <&rpmhpd SC7280_CX>;
 				required-opps = <&rpmhpd_opp_low_svs>;
-				dmas = <&gpi_dma1 0 2 QCOM_GPI_I2C>,
-				       <&gpi_dma1 1 2 QCOM_GPI_I2C>;
+				dmas = <&gpi_dma1 0 2 QCOM_GPI_I2C 64>,
+				       <&gpi_dma1 1 2 QCOM_GPI_I2C 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1770,8 +1770,8 @@
 				interconnects = <&clk_virt MASTER_QUP_CORE_1 0 &clk_virt SLAVE_QUP_CORE_1 0>,
 						<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_1 0>;
 				interconnect-names = "qup-core", "qup-config";
-				dmas = <&gpi_dma1 0 2 QCOM_GPI_SPI>,
-				       <&gpi_dma1 1 2 QCOM_GPI_SPI>;
+				dmas = <&gpi_dma1 0 2 QCOM_GPI_SPI 64>,
+				       <&gpi_dma1 1 2 QCOM_GPI_SPI 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1809,8 +1809,8 @@
 							"qup-memory";
 				power-domains = <&rpmhpd SC7280_CX>;
 				required-opps = <&rpmhpd_opp_low_svs>;
-				dmas = <&gpi_dma1 0 3 QCOM_GPI_I2C>,
-				       <&gpi_dma1 1 3 QCOM_GPI_I2C>;
+				dmas = <&gpi_dma1 0 3 QCOM_GPI_I2C 64>,
+				       <&gpi_dma1 1 3 QCOM_GPI_I2C 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1830,8 +1830,8 @@
 				interconnects = <&clk_virt MASTER_QUP_CORE_1 0 &clk_virt SLAVE_QUP_CORE_1 0>,
 						<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_1 0>;
 				interconnect-names = "qup-core", "qup-config";
-				dmas = <&gpi_dma1 0 3 QCOM_GPI_SPI>,
-				       <&gpi_dma1 1 3 QCOM_GPI_SPI>;
+				dmas = <&gpi_dma1 0 3 QCOM_GPI_SPI 64>,
+				       <&gpi_dma1 1 3 QCOM_GPI_SPI 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1869,8 +1869,8 @@
 							"qup-memory";
 				power-domains = <&rpmhpd SC7280_CX>;
 				required-opps = <&rpmhpd_opp_low_svs>;
-				dmas = <&gpi_dma1 0 4 QCOM_GPI_I2C>,
-				       <&gpi_dma1 1 4 QCOM_GPI_I2C>;
+				dmas = <&gpi_dma1 0 4 QCOM_GPI_I2C 64>,
+				       <&gpi_dma1 1 4 QCOM_GPI_I2C 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1890,8 +1890,8 @@
 				interconnects = <&clk_virt MASTER_QUP_CORE_1 0 &clk_virt SLAVE_QUP_CORE_1 0>,
 						<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_1 0>;
 				interconnect-names = "qup-core", "qup-config";
-				dmas = <&gpi_dma1 0 4 QCOM_GPI_SPI>,
-				       <&gpi_dma1 1 4 QCOM_GPI_SPI>;
+				dmas = <&gpi_dma1 0 4 QCOM_GPI_SPI 64>,
+				       <&gpi_dma1 1 4 QCOM_GPI_SPI 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1929,8 +1929,8 @@
 							"qup-memory";
 				power-domains = <&rpmhpd SC7280_CX>;
 				required-opps = <&rpmhpd_opp_low_svs>;
-				dmas = <&gpi_dma1 0 5 QCOM_GPI_I2C>,
-				       <&gpi_dma1 1 5 QCOM_GPI_I2C>;
+				dmas = <&gpi_dma1 0 5 QCOM_GPI_I2C 64>,
+				       <&gpi_dma1 1 5 QCOM_GPI_I2C 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1950,8 +1950,8 @@
 				interconnects = <&clk_virt MASTER_QUP_CORE_1 0 &clk_virt SLAVE_QUP_CORE_1 0>,
 						<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_1 0>;
 				interconnect-names = "qup-core", "qup-config";
-				dmas = <&gpi_dma1 0 5 QCOM_GPI_SPI>,
-				       <&gpi_dma1 1 5 QCOM_GPI_SPI>;
+				dmas = <&gpi_dma1 0 5 QCOM_GPI_SPI 64>,
+				       <&gpi_dma1 1 5 QCOM_GPI_SPI 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -1989,8 +1989,8 @@
 							"qup-memory";
 				power-domains = <&rpmhpd SC7280_CX>;
 				required-opps = <&rpmhpd_opp_low_svs>;
-				dmas = <&gpi_dma1 0 6 QCOM_GPI_I2C>,
-				       <&gpi_dma1 1 6 QCOM_GPI_I2C>;
+				dmas = <&gpi_dma1 0 6 QCOM_GPI_I2C 64>,
+				       <&gpi_dma1 1 6 QCOM_GPI_I2C 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -2010,8 +2010,8 @@
 				interconnects = <&clk_virt MASTER_QUP_CORE_1 0 &clk_virt SLAVE_QUP_CORE_1 0>,
 						<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_1 0>;
 				interconnect-names = "qup-core", "qup-config";
-				dmas = <&gpi_dma1 0 6 QCOM_GPI_SPI>,
-				       <&gpi_dma1 1 6 QCOM_GPI_SPI>;
+				dmas = <&gpi_dma1 0 6 QCOM_GPI_SPI 64>,
+				       <&gpi_dma1 1 6 QCOM_GPI_SPI 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -2049,8 +2049,8 @@
 							"qup-memory";
 				power-domains = <&rpmhpd SC7280_CX>;
 				required-opps = <&rpmhpd_opp_low_svs>;
-				dmas = <&gpi_dma1 0 7 QCOM_GPI_I2C>,
-				       <&gpi_dma1 1 7 QCOM_GPI_I2C>;
+				dmas = <&gpi_dma1 0 7 QCOM_GPI_I2C 64>,
+				       <&gpi_dma1 1 7 QCOM_GPI_I2C 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
@@ -2070,8 +2070,8 @@
 				interconnects = <&clk_virt MASTER_QUP_CORE_1 0 &clk_virt SLAVE_QUP_CORE_1 0>,
 						<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_QUP_1 0>;
 				interconnect-names = "qup-core", "qup-config";
-				dmas = <&gpi_dma1 0 7 QCOM_GPI_SPI>,
-				       <&gpi_dma1 1 7 QCOM_GPI_SPI>;
+				dmas = <&gpi_dma1 0 7 QCOM_GPI_SPI 64>,
+				       <&gpi_dma1 1 7 QCOM_GPI_SPI 64>;
 				dma-names = "tx", "rx";
 				status = "disabled";
 			};
-- 
2.17.1

[PATCH v1 3/5] dmaengine: qcom: gpi: Add provision to support TRE size as the fourth argument of dma-cells property

[Jyothi Kumar Seerapu]

The current GPI driver hardcodes the channel TRE (Transfer Ring Element)
size to 64. For scenarios requiring high performance with multiple
messages in a transfer, use Block Event Interrupt (BEI).
This method triggers interrupt after specific message transfers and
the last message transfer, effectively reducing the number of interrupts.
For multiple transfers utilizing BEI, a channel TRE size of 64 is
insufficient and may lead to transfer failures, indicated by errors
related to unavailable memory space.

Added provision to modify the channel TRE size via the device tree.
The Default channel TRE size is set to 64, but this value can update
in the device tree which will then be parsed by the GPI driver.

Signed-off-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com>
---
 drivers/dma/qcom/gpi.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
index 52a7c8f2498f..3c89b4a88ac1 100644
--- a/drivers/dma/qcom/gpi.c
+++ b/drivers/dma/qcom/gpi.c
@@ -234,7 +234,7 @@ enum msm_gpi_tce_code {
 #define GPI_RX_CHAN		(1)
 #define STATE_IGNORE		(U32_MAX)
 #define EV_FACTOR		(2)
-#define REQ_OF_DMA_ARGS		(5) /* # of arguments required from client */
+#define REQ_OF_DMA_ARGS		(3) /* # of arguments required from client */
 #define CHAN_TRES		64
 
 struct __packed xfer_compl_event {
@@ -481,6 +481,7 @@ struct gchan {
 	u32 chid;
 	u32 seid;
 	u32 protocol;
+	u32 num_chan_tres;
 	struct gpii *gpii;
 	enum gpi_ch_state ch_state;
 	enum gpi_pm_state pm_state;
@@ -1903,8 +1904,8 @@ static int gpi_ch_init(struct gchan *gchan)
 	}
 
 	/* allocate memory for event ring */
-	elements = CHAN_TRES << ev_factor;
-	ret = gpi_alloc_ring(&gpii->ev_ring, elements,
+	elements = max(gpii->gchan[0].num_chan_tres, gpii->gchan[1].num_chan_tres);
+	ret = gpi_alloc_ring(&gpii->ev_ring, elements << ev_factor,
 			     sizeof(union gpi_event), gpii);
 	if (ret)
 		goto exit_gpi_init;
@@ -2042,7 +2043,7 @@ static int gpi_alloc_chan_resources(struct dma_chan *chan)
 	mutex_lock(&gpii->ctrl_lock);
 
 	/* allocate memory for transfer ring */
-	ret = gpi_alloc_ring(&gchan->ch_ring, CHAN_TRES,
+	ret = gpi_alloc_ring(&gchan->ch_ring, gchan->num_chan_tres,
 			     sizeof(struct gpi_tre), gpii);
 	if (ret)
 		goto xfer_alloc_err;
@@ -2107,9 +2108,9 @@ static struct dma_chan *gpi_of_dma_xlate(struct of_phandle_args *args,
 	int gpii;
 	struct gchan *gchan;
 
-	if (args->args_count < 3) {
-		dev_err(gpi_dev->dev, "gpii require minimum 2 args, client passed:%d args\n",
-			args->args_count);
+	if (args->args_count < REQ_OF_DMA_ARGS) {
+		dev_err(gpi_dev->dev, "gpii require minimum %d args, client passed:%d args\n",
+			REQ_OF_DMA_ARGS, args->args_count);
 		return NULL;
 	}
 
@@ -2138,6 +2139,16 @@ static struct dma_chan *gpi_of_dma_xlate(struct of_phandle_args *args,
 	gchan->seid = seid;
 	gchan->protocol = args->args[2];
 
+	/*
+	 * If the channel tre size entry is present in device tree and
+	 * channel tre size is greater than 64 then parse the value from
+	 * the device tree, otherwise use the default value, which is 64.
+	 */
+	if (args->args_count > REQ_OF_DMA_ARGS && args->args[3] > CHAN_TRES)
+		gchan->num_chan_tres = args->args[3];
+	else
+		gchan->num_chan_tres = CHAN_TRES;
+
 	return dma_get_slave_channel(&gchan->vc.chan);
 }
 
-- 
2.17.1

[PATCH v1 4/5] dmaengine: qcom: gpi: Add GPI Block event interrupt support

[Jyothi Kumar Seerapu]

GSI hardware generates an interrupt for each transfer completion.
For multiple messages within a single transfer, this results
in receiving N interrupts for N messages, which can introduce
significant software interrupt latency. To mitigate this latency,
utilize Block Event Interrupt (BEI) only when an interrupt is necessary.
When using BEI, consider splitting a single multi-message transfer into
chunks of 64. This approach can enhance overall transfer time and
efficiency.

Signed-off-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com>
---
 drivers/dma/qcom/gpi.c           | 49 ++++++++++++++++++++++++++++++++
 include/linux/dma/qcom-gpi-dma.h | 37 ++++++++++++++++++++++++
 2 files changed, 86 insertions(+)

diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
index 3c89b4a88ac1..b8ca119114d2 100644
--- a/drivers/dma/qcom/gpi.c
+++ b/drivers/dma/qcom/gpi.c
@@ -1694,6 +1694,9 @@ static int gpi_create_i2c_tre(struct gchan *chan, struct gpi_desc *desc,
 
 		tre->dword[3] = u32_encode_bits(TRE_TYPE_DMA, TRE_FLAGS_TYPE);
 		tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_IEOT);
+
+		if (i2c->flags & QCOM_GPI_BLOCK_EVENT_IRQ)
+			tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_BEI);
 	}
 
 	for (i = 0; i < tre_idx; i++)
@@ -2099,6 +2102,52 @@ static int gpi_find_avail_gpii(struct gpi_dev *gpi_dev, u32 seid)
 	return -EIO;
 }
 
+/**
+ * gpi_multi_desc_process() - Process received transfers from GSI HW
+ * @dev: pointer to the corresponding dev node
+ * @multi_xfer: pointer to the gpi_multi_xfer
+ * @num_xfers: total number of transfers
+ * @transfer_timeout_msecs: transfer timeout value
+ * @transfer_comp: completion object of the transfer
+ *
+ * This function is used to process the received transfers based on the
+ * completion events
+ *
+ * Return: On success returns 0, otherwise return error code
+ */
+int gpi_multi_desc_process(struct device *dev, struct gpi_multi_xfer *multi_xfer,
+			   u32 num_xfers, u32 transfer_timeout_msecs,
+			   struct completion *transfer_comp)
+{
+	int i;
+	u32 max_irq_cnt, time_left;
+
+	max_irq_cnt = num_xfers / NUM_MSGS_PER_IRQ;
+	if (num_xfers % NUM_MSGS_PER_IRQ)
+		max_irq_cnt++;
+
+	/*
+	 * Wait for the interrupts of the processed transfers in multiple
+	 * of 64 and for the last transfer. If the hardware is fast and
+	 * already processed all the transfers then no need to wait.
+	 */
+	for (i = 0; i < max_irq_cnt; i++) {
+		reinit_completion(transfer_comp);
+		if (max_irq_cnt != multi_xfer->irq_cnt) {
+			time_left = wait_for_completion_timeout(transfer_comp,
+								transfer_timeout_msecs);
+			if (!time_left) {
+				dev_err(dev, "%s: Transfer timeout\n", __func__);
+				return -ETIMEDOUT;
+			}
+		}
+		if (num_xfers > multi_xfer->msg_idx_cnt)
+			return 0;
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gpi_multi_desc_process);
+
 /* gpi_of_dma_xlate: open client requested channel */
 static struct dma_chan *gpi_of_dma_xlate(struct of_phandle_args *args,
 					 struct of_dma *of_dma)
diff --git a/include/linux/dma/qcom-gpi-dma.h b/include/linux/dma/qcom-gpi-dma.h
index 6680dd1a43c6..ca0465627a21 100644
--- a/include/linux/dma/qcom-gpi-dma.h
+++ b/include/linux/dma/qcom-gpi-dma.h
@@ -15,6 +15,12 @@ enum spi_transfer_cmd {
 	SPI_DUPLEX,
 };
 
+#define QCOM_GPI_BLOCK_EVENT_IRQ	BIT(0)
+
+#define QCOM_GPI_MAX_NUM_MSGS		200
+#define NUM_MSGS_PER_IRQ		64
+#define MIN_NUM_OF_MSGS_MULTI_DESC	4
+
 /**
  * struct gpi_spi_config - spi config for peripheral
  *
@@ -51,6 +57,29 @@ enum i2c_op {
 	I2C_READ,
 };
 
+/**
+ * struct gpi_multi_xfer - Used for multi transfer support
+ *
+ * @msg_idx_cnt: message index for the transfer
+ * @buf_idx: dma buffer index
+ * @unmap_msg_cnt: unampped transfer index
+ * @freed_msg_cnt: freed transfer index
+ * @irq_cnt: received interrupt count
+ * @irq_msg_cnt: transfer message count for the received irqs
+ * @dma_buf: virtual address of the buffer
+ * @dma_addr: dma address of the buffer
+ */
+struct gpi_multi_xfer {
+	u32 msg_idx_cnt;
+	u32 buf_idx;
+	u32 unmap_msg_cnt;
+	u32 freed_msg_cnt;
+	u32 irq_cnt;
+	u32 irq_msg_cnt;
+	void *dma_buf[QCOM_GPI_MAX_NUM_MSGS];
+	dma_addr_t *dma_addr[QCOM_GPI_MAX_NUM_MSGS];
+};
+
 /**
  * struct gpi_i2c_config - i2c config for peripheral
  *
@@ -65,6 +94,8 @@ enum i2c_op {
  * @rx_len: receive length for buffer
  * @op: i2c cmd
  * @muli-msg: is part of multi i2c r-w msgs
+ * @flags: true for block event interrupt support
+ * @multi_xfer: indicates transfer has multi messages
  */
 struct gpi_i2c_config {
 	u8 set_config;
@@ -78,6 +109,12 @@ struct gpi_i2c_config {
 	u32 rx_len;
 	enum i2c_op op;
 	bool multi_msg;
+	u8 flags;
+	struct gpi_multi_xfer multi_xfer;
 };
 
+int gpi_multi_desc_process(struct device *dev, struct gpi_multi_xfer *multi_xfer,
+			   u32 num_xfers, u32 tranfer_timeout_msecs,
+			   struct completion *transfer_comp);
+
 #endif /* QCOM_GPI_DMA_H */
-- 
2.17.1

[PATCH v1 5/5] i2c: i2c-qcom-geni: Add Block event interrupt support

[Jyothi Kumar Seerapu]

The I2C driver gets an interrupt upon transfer completion.
For multiple messages in a single transfer, N interrupts will be
received for N messages, leading to significant software interrupt
latency. To mitigate this latency, utilize Block Event Interrupt (BEI)
only when an interrupt is necessary. This means large transfers can be
split into multiple chunks of 64 messages internally, without expecting
interrupts for the first 63 transfers, only the last one will trigger
an interrupt indicating 64 transfers completed.

By implementing BEI, multi-message transfers can be divided into
chunks of 64 messages, improving overall transfer time.
This optimization reduces transfer time from 168 ms to 48 ms for a
series of 200 I2C write messages in a single transfer, with a
clock frequency support of 100 kHz.

BEI optimizations are currently implemented for I2C write transfers only,
as there is no use case for multiple I2C read messages in a single transfer
at this time.

Signed-off-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com>
---
 drivers/i2c/busses/i2c-qcom-geni.c | 205 +++++++++++++++++++++++++----
 1 file changed, 181 insertions(+), 24 deletions(-)

diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 212336f724a6..a73dc1738a62 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -99,6 +99,10 @@ struct geni_i2c_dev {
 	struct dma_chan *rx_c;
 	bool gpi_mode;
 	bool abort_done;
+	bool is_tx_multi_xfer;
+	u32 num_msgs;
+	u32 tx_irq_cnt;
+	struct gpi_i2c_config *gpi_config;
 };
 
 struct geni_i2c_desc {
@@ -485,6 +489,7 @@ static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
 static void i2c_gpi_cb_result(void *cb, const struct dmaengine_result *result)
 {
 	struct geni_i2c_dev *gi2c = cb;
+	struct gpi_multi_xfer *tx_multi_xfer;
 
 	if (result->result != DMA_TRANS_NOERROR) {
 		dev_err(gi2c->se.dev, "DMA txn failed:%d\n", result->result);
@@ -493,7 +498,21 @@ static void i2c_gpi_cb_result(void *cb, const struct dmaengine_result *result)
 		dev_dbg(gi2c->se.dev, "DMA xfer has pending: %d\n", result->residue);
 	}
 
-	complete(&gi2c->done);
+	if (gi2c->is_tx_multi_xfer) {
+		tx_multi_xfer = &gi2c->gpi_config->multi_xfer;
+
+		/*
+		 * Send Completion for last message or multiple of NUM_MSGS_PER_IRQ.
+		 */
+		if ((tx_multi_xfer->irq_msg_cnt == gi2c->num_msgs - 1) ||
+		    (!((tx_multi_xfer->irq_msg_cnt + 1) % NUM_MSGS_PER_IRQ))) {
+			tx_multi_xfer->irq_cnt++;
+			complete(&gi2c->done);
+		}
+		tx_multi_xfer->irq_msg_cnt++;
+	} else {
+		complete(&gi2c->done);
+	}
 }
 
 static void geni_i2c_gpi_unmap(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
@@ -511,7 +530,41 @@ static void geni_i2c_gpi_unmap(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
 	}
 }
 
-static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
+/**
+ * gpi_i2c_multi_desc_unmap() - unmaps the buffers post multi message TX transfers
+ * @dev: pointer to the corresponding dev node
+ * @gi2c: i2c dev handle
+ * @msgs: i2c messages array
+ * @peripheral: pointer to the gpi_i2c_config
+ */
+static void gpi_i2c_multi_desc_unmap(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[],
+				     struct gpi_i2c_config *peripheral)
+{
+	u32 msg_xfer_cnt, wr_idx = 0;
+	struct gpi_multi_xfer *tx_multi_xfer = &peripheral->multi_xfer;
+
+	/*
+	 * In error case, need to unmap all messages based on the msg_idx_cnt.
+	 * Non-error case unmap all the processed messages.
+	 */
+	if (gi2c->err)
+		msg_xfer_cnt = tx_multi_xfer->msg_idx_cnt;
+	else
+		msg_xfer_cnt = tx_multi_xfer->irq_cnt * NUM_MSGS_PER_IRQ;
+
+	/* Unmap the processed DMA buffers based on the received interrupt count */
+	for (; tx_multi_xfer->unmap_msg_cnt < msg_xfer_cnt; tx_multi_xfer->unmap_msg_cnt++) {
+		if (tx_multi_xfer->unmap_msg_cnt == gi2c->num_msgs)
+			break;
+		wr_idx = tx_multi_xfer->unmap_msg_cnt % QCOM_GPI_MAX_NUM_MSGS;
+		geni_i2c_gpi_unmap(gi2c, &msgs[tx_multi_xfer->unmap_msg_cnt],
+				   tx_multi_xfer->dma_buf[wr_idx],
+				   tx_multi_xfer->dma_addr[wr_idx], NULL, NULL);
+		tx_multi_xfer->freed_msg_cnt++;
+	}
+}
+
+static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], int cur_msg_idx,
 			struct dma_slave_config *config, dma_addr_t *dma_addr_p,
 			void **buf, unsigned int op, struct dma_chan *dma_chan)
 {
@@ -523,26 +576,49 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
 	enum dma_transfer_direction dma_dirn;
 	struct dma_async_tx_descriptor *desc;
 	int ret;
+	struct gpi_multi_xfer *gi2c_gpi_xfer;
+	dma_cookie_t cookie;
 
 	peripheral = config->peripheral_config;
-
-	dma_buf = i2c_get_dma_safe_msg_buf(msg, 1);
-	if (!dma_buf)
+	gi2c_gpi_xfer = &peripheral->multi_xfer;
+	gi2c_gpi_xfer->msg_idx_cnt = cur_msg_idx;
+	dma_buf = gi2c_gpi_xfer->dma_buf[gi2c_gpi_xfer->buf_idx];
+	addr = gi2c_gpi_xfer->dma_addr[gi2c_gpi_xfer->buf_idx];
+
+	dma_buf = i2c_get_dma_safe_msg_buf(&msgs[gi2c_gpi_xfer->msg_idx_cnt], 1);
+	if (!dma_buf) {
+		gi2c->err = -ENOMEM;
 		return -ENOMEM;
+	}
 
 	if (op == I2C_WRITE)
 		map_dirn = DMA_TO_DEVICE;
 	else
 		map_dirn = DMA_FROM_DEVICE;
 
-	addr = dma_map_single(gi2c->se.dev->parent, dma_buf, msg->len, map_dirn);
+	addr = dma_map_single(gi2c->se.dev->parent,
+			      dma_buf, msgs[gi2c_gpi_xfer->msg_idx_cnt].len,
+			      map_dirn);
 	if (dma_mapping_error(gi2c->se.dev->parent, addr)) {
-		i2c_put_dma_safe_msg_buf(dma_buf, msg, false);
+		i2c_put_dma_safe_msg_buf(dma_buf, &msgs[gi2c_gpi_xfer->msg_idx_cnt],
+					 false);
+		gi2c->err = -ENOMEM;
 		return -ENOMEM;
 	}
 
+	if (gi2c->is_tx_multi_xfer) {
+		if (((gi2c_gpi_xfer->msg_idx_cnt + 1) % NUM_MSGS_PER_IRQ))
+			peripheral->flags |= QCOM_GPI_BLOCK_EVENT_IRQ;
+		else
+			peripheral->flags &= ~QCOM_GPI_BLOCK_EVENT_IRQ;
+
+		/* BEI bit to be cleared for last TRE */
+		if (gi2c_gpi_xfer->msg_idx_cnt == gi2c->num_msgs - 1)
+			peripheral->flags &= ~QCOM_GPI_BLOCK_EVENT_IRQ;
+	}
+
 	/* set the length as message for rx txn */
-	peripheral->rx_len = msg->len;
+	peripheral->rx_len = msgs[gi2c_gpi_xfer->msg_idx_cnt].len;
 	peripheral->op = op;
 
 	ret = dmaengine_slave_config(dma_chan, config);
@@ -560,25 +636,56 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
 	else
 		dma_dirn = DMA_DEV_TO_MEM;
 
-	desc = dmaengine_prep_slave_single(dma_chan, addr, msg->len, dma_dirn, flags);
+	desc = dmaengine_prep_slave_single(dma_chan, addr,
+					   msgs[gi2c_gpi_xfer->msg_idx_cnt].len,
+					   dma_dirn, flags);
 	if (!desc) {
 		dev_err(gi2c->se.dev, "prep_slave_sg failed\n");
-		ret = -EIO;
+		gi2c->err = -EIO;
 		goto err_config;
 	}
 
 	desc->callback_result = i2c_gpi_cb_result;
 	desc->callback_param = gi2c;
 
-	dmaengine_submit(desc);
-	*buf = dma_buf;
-	*dma_addr_p = addr;
+	if (!((msgs[cur_msg_idx].flags & I2C_M_RD) && op == I2C_WRITE)) {
+		gi2c_gpi_xfer->msg_idx_cnt++;
+		gi2c_gpi_xfer->buf_idx = (cur_msg_idx + 1) % QCOM_GPI_MAX_NUM_MSGS;
+	}
+	cookie = dmaengine_submit(desc);
+	if (dma_submit_error(cookie)) {
+		dev_err(gi2c->se.dev,
+			"%s: dmaengine_submit failed (%d)\n", __func__, cookie);
+		return -EINVAL;
+	}
 
+	if (gi2c->is_tx_multi_xfer) {
+		dma_async_issue_pending(gi2c->tx_c);
+		if ((cur_msg_idx == (gi2c->num_msgs - 1)) ||
+		    (gi2c_gpi_xfer->msg_idx_cnt >=
+		     QCOM_GPI_MAX_NUM_MSGS + gi2c_gpi_xfer->freed_msg_cnt)) {
+			ret = gpi_multi_desc_process(gi2c->se.dev, gi2c_gpi_xfer,
+						     gi2c->num_msgs, XFER_TIMEOUT,
+						     &gi2c->done);
+			if (ret) {
+				dev_dbg(gi2c->se.dev,
+					"I2C multi write msg transfer timeout: %d\n",
+					ret);
+				gi2c->err = -ETIMEDOUT;
+				goto err_config;
+			}
+		}
+	} else {
+		/* Non multi descriptor message transfer */
+		*buf = dma_buf;
+		*dma_addr_p = addr;
+	}
 	return 0;
 
 err_config:
-	dma_unmap_single(gi2c->se.dev->parent, addr, msg->len, map_dirn);
-	i2c_put_dma_safe_msg_buf(dma_buf, msg, false);
+	dma_unmap_single(gi2c->se.dev->parent, addr,
+			 msgs[cur_msg_idx].len, map_dirn);
+	i2c_put_dma_safe_msg_buf(dma_buf, &msgs[cur_msg_idx], false);
 	return ret;
 }
 
@@ -590,6 +697,7 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
 	unsigned long time_left;
 	dma_addr_t tx_addr, rx_addr;
 	void *tx_buf = NULL, *rx_buf = NULL;
+	struct gpi_multi_xfer *tx_multi_xfer;
 	const struct geni_i2c_clk_fld *itr = gi2c->clk_fld;
 
 	config.peripheral_config = &peripheral;
@@ -603,6 +711,39 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
 	peripheral.set_config = 1;
 	peripheral.multi_msg = false;
 
+	gi2c->gpi_config = &peripheral;
+	gi2c->num_msgs = num;
+	gi2c->is_tx_multi_xfer = false;
+	gi2c->tx_irq_cnt = 0;
+
+	tx_multi_xfer = &peripheral.multi_xfer;
+	tx_multi_xfer->msg_idx_cnt = 0;
+	tx_multi_xfer->buf_idx = 0;
+	tx_multi_xfer->unmap_msg_cnt = 0;
+	tx_multi_xfer->freed_msg_cnt = 0;
+	tx_multi_xfer->irq_msg_cnt = 0;
+	tx_multi_xfer->irq_cnt = 0;
+
+	/*
+	 * If number of write messages are four and higher then
+	 * configure hardware for multi descriptor transfers with BEI.
+	 */
+	if (num >= MIN_NUM_OF_MSGS_MULTI_DESC) {
+		gi2c->is_tx_multi_xfer = true;
+		for (i = 0; i < num; i++) {
+			if (msgs[i].flags & I2C_M_RD) {
+				/*
+				 * Multi descriptor transfer with BEI
+				 * support is enabled for write transfers.
+				 * Add BEI optimization support for read
+				 * transfers later.
+				 */
+				gi2c->is_tx_multi_xfer = false;
+				break;
+			}
+		}
+	}
+
 	for (i = 0; i < num; i++) {
 		gi2c->cur = &msgs[i];
 		gi2c->err = 0;
@@ -613,14 +754,16 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
 			peripheral.stretch = 1;
 
 		peripheral.addr = msgs[i].addr;
+		if (i > 0 && (!(msgs[i].flags & I2C_M_RD)))
+			peripheral.multi_msg = false;
 
-		ret =  geni_i2c_gpi(gi2c, &msgs[i], &config,
+		ret =  geni_i2c_gpi(gi2c, msgs, i, &config,
 				    &tx_addr, &tx_buf, I2C_WRITE, gi2c->tx_c);
 		if (ret)
 			goto err;
 
 		if (msgs[i].flags & I2C_M_RD) {
-			ret =  geni_i2c_gpi(gi2c, &msgs[i], &config,
+			ret =  geni_i2c_gpi(gi2c, msgs, i, &config,
 					    &rx_addr, &rx_buf, I2C_READ, gi2c->rx_c);
 			if (ret)
 				goto err;
@@ -628,18 +771,28 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
 			dma_async_issue_pending(gi2c->rx_c);
 		}
 
-		dma_async_issue_pending(gi2c->tx_c);
-
-		time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
-		if (!time_left)
-			gi2c->err = -ETIMEDOUT;
+		if (!gi2c->is_tx_multi_xfer) {
+			dma_async_issue_pending(gi2c->tx_c);
+			time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
+			if (!time_left) {
+				dev_err(gi2c->se.dev, "%s:I2C timeout\n", __func__);
+				gi2c->err = -ETIMEDOUT;
+			}
+		}
 
 		if (gi2c->err) {
 			ret = gi2c->err;
 			goto err;
 		}
 
-		geni_i2c_gpi_unmap(gi2c, &msgs[i], tx_buf, tx_addr, rx_buf, rx_addr);
+		if (!gi2c->is_tx_multi_xfer) {
+			geni_i2c_gpi_unmap(gi2c, &msgs[i], tx_buf, tx_addr, rx_buf, rx_addr);
+		} else {
+			if (gi2c->tx_irq_cnt != tx_multi_xfer->irq_cnt) {
+				gi2c->tx_irq_cnt = tx_multi_xfer->irq_cnt;
+				gpi_i2c_multi_desc_unmap(gi2c, msgs, &peripheral);
+			}
+		}
 	}
 
 	return num;
@@ -648,7 +801,11 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
 	dev_err(gi2c->se.dev, "GPI transfer failed: %d\n", ret);
 	dmaengine_terminate_sync(gi2c->rx_c);
 	dmaengine_terminate_sync(gi2c->tx_c);
-	geni_i2c_gpi_unmap(gi2c, &msgs[i], tx_buf, tx_addr, rx_buf, rx_addr);
+	if (gi2c->is_tx_multi_xfer)
+		gpi_i2c_multi_desc_unmap(gi2c, msgs, &peripheral);
+	else
+		geni_i2c_gpi_unmap(gi2c, &msgs[i], tx_buf, tx_addr, rx_buf, rx_addr);
+
 	return ret;
 }
 
-- 
2.17.1

Re: [PATCH v1 1/5] dt-bindings: dmaengine: qcom: gpi: Add additional arg to dma-cell property

[Rob Herring (Arm)]

On Tue, 15 Oct 2024 17:37:46 +0530, Jyothi Kumar Seerapu wrote:
> When high performance with multiple i2c messages in a single transfer
> is required, employ Block Event Interrupt (BEI) to trigger interrupts
> after specific messages transfer and the last message transfer,
> thereby reducing interrupts.
> 
> For each i2c message transfer, a series of Transfer Request Elements(TREs)
> must be programmed, including config tre for frequency configuration,
> go tre for holding i2c address and dma tre for holding dma buffer address,
> length as per the hardware programming guide. For transfer using BEI,
> multiple I2C messages may necessitate the preparation of config, go,
> and tx DMA TREs. However, a channel TRE size of 64 is often insufficient,
> potentially leading to failures due to inadequate memory space.
> 
> Add additional argument to dma-cell property for channel TRE size.
> With this, adjust the channel TRE size via the device tree.
> The default size is 64, but clients can modify this value based on
> their specific requirements.
> 
> Signed-off-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com>
> ---
>  Documentation/devicetree/bindings/dma/qcom,gpi.yaml | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/dma/qcom,gpi.yaml: properties:#dma-cells: 'minItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/dma/qcom,gpi.yaml: properties:#dma-cells: 'maxItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
	from schema $id: http://devicetree.org/meta-schemas/core.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241015120750.21217-2-quic_jseerapu@quicinc.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v1 1/5] dt-bindings: dmaengine: qcom: gpi: Add additional arg to dma-cell property

[Krzysztof Kozlowski]

On 15/10/2024 14:07, Jyothi Kumar Seerapu wrote:
> When high performance with multiple i2c messages in a single transfer
> is required, employ Block Event Interrupt (BEI) to trigger interrupts
> after specific messages transfer and the last message transfer,
> thereby reducing interrupts.
> 
> For each i2c message transfer, a series of Transfer Request Elements(TREs)
> must be programmed, including config tre for frequency configuration,
> go tre for holding i2c address and dma tre for holding dma buffer address,
> length as per the hardware programming guide. For transfer using BEI,
> multiple I2C messages may necessitate the preparation of config, go,
> and tx DMA TREs. However, a channel TRE size of 64 is often insufficient,
> potentially leading to failures due to inadequate memory space.

Please kindly test the patches before you sent them. Upstream is not a
testing service.

Best regards,
Krzysztof

Re: [PATCH v1 2/5] arm64: dts: qcom: Add support for configuring channel TRE size

[Krzysztof Kozlowski]

On 15/10/2024 14:07, Jyothi Kumar Seerapu wrote:
> When high performance with multiple i2c messages in a single transfer
> is required, employ Block Event Interrupt (BEI) to trigger interrupts
> after specific messages transfer and the last message transfer,
> thereby reducing interrupts.
> For each i2c message transfer, a series of Transfer Request Elements(TREs)
> must be programmed, including config tre for frequency configuration,
> go tre for holding i2c address and dma tre for holding dma buffer address,
> length as per the hardware programming guide. For transfer using BEI,
> multiple I2C messages may necessitate the preparation of config, go,
> and tx DMA TREs. However, a channel TRE size of 64 is often insufficient,
> potentially leading to failures due to inadequate memory space.
> 
> Adjust the channel TRE size through the device tree.
> The default size is 64, but clients can modify this value based on
> their heigher channel TRE size requirements.
> 
> Signed-off-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 132 +++++++++++++--------------
>  1 file changed, 66 insertions(+), 66 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 3d8410683402..c7c0e15ff9d3 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -1064,7 +1064,7 @@
>  		};
>  
>  		gpi_dma0: dma-controller@900000 {
> -			#dma-cells = <3>;
> +			#dma-cells = <4>;
>  			compatible = "qcom,sc7280-gpi-dma", "qcom,sm6350-gpi-dma";
>  			reg = <0 0x00900000 0 0x60000>;
>  			interrupts = <GIC_SPI 244 IRQ_TYPE_LEVEL_HIGH>,
> @@ -1114,8 +1114,8 @@
>  							"qup-memory";
>  				power-domains = <&rpmhpd SC7280_CX>;
>  				required-opps = <&rpmhpd_opp_low_svs>;
> -				dmas = <&gpi_dma0 0 0 QCOM_GPI_I2C>,
> -				       <&gpi_dma0 1 0 QCOM_GPI_I2C>;
> +				dmas = <&gpi_dma0 0 0 QCOM_GPI_I2C 64>,
> +				       <&gpi_dma0 1 0 QCOM_GPI_I2C 64>;

So everywhere is 64, thus this is fixed. Deduce it from the compatible

Best regards,
Krzysztof

Re: [PATCH v1 1/5] dt-bindings: dmaengine: qcom: gpi: Add additional arg to dma-cell property

[Rob Herring]

On Tue, Oct 15, 2024 at 05:37:46PM +0530, Jyothi Kumar Seerapu wrote:
> When high performance with multiple i2c messages in a single transfer
> is required, employ Block Event Interrupt (BEI) to trigger interrupts
> after specific messages transfer and the last message transfer,
> thereby reducing interrupts.
> 
> For each i2c message transfer, a series of Transfer Request Elements(TREs)
> must be programmed, including config tre for frequency configuration,
> go tre for holding i2c address and dma tre for holding dma buffer address,
> length as per the hardware programming guide. For transfer using BEI,
> multiple I2C messages may necessitate the preparation of config, go,
> and tx DMA TREs. However, a channel TRE size of 64 is often insufficient,
> potentially leading to failures due to inadequate memory space.
> 
> Add additional argument to dma-cell property for channel TRE size.

No such property 'dma-cell'

> With this, adjust the channel TRE size via the device tree.
> The default size is 64, but clients can modify this value based on
> their specific requirements.
> 
> Signed-off-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com>
> ---
>  Documentation/devicetree/bindings/dma/qcom,gpi.yaml | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
> index 4df4e61895d2..002495921643 100644
> --- a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
> +++ b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
> @@ -54,14 +54,16 @@ properties:
>      maxItems: 13
>  
>    "#dma-cells":
> -    const: 3
> +    minItems: 3
> +    maxItems: 4
>      description: >
>        DMA clients must use the format described in dma.txt, giving a phandle
> -      to the DMA controller plus the following 3 integer cells:
> +      to the DMA controller plus the following 4 integer cells:
>        - channel: if set to 0xffffffff, any available channel will be allocated
>          for the client. Otherwise, the exact channel specified will be used.
>        - seid: serial id of the client as defined in the SoC documentation.
>        - client: type of the client as defined in dt-bindings/dma/qcom-gpi.h
> +      - channel-tre-size: size of the channel TRE (transfer ring element)
>  
>    iommus:
>      maxItems: 1
> -- 
> 2.17.1
> 

Re: [PATCH v1 1/5] dt-bindings: dmaengine: qcom: gpi: Add additional arg to dma-cell property

[Vinod Koul]

On 15-10-24, 17:37, Jyothi Kumar Seerapu wrote:
> When high performance with multiple i2c messages in a single transfer
> is required, employ Block Event Interrupt (BEI) to trigger interrupts
> after specific messages transfer and the last message transfer,
> thereby reducing interrupts.
> 
> For each i2c message transfer, a series of Transfer Request Elements(TREs)
> must be programmed, including config tre for frequency configuration,
> go tre for holding i2c address and dma tre for holding dma buffer address,
> length as per the hardware programming guide. For transfer using BEI,
> multiple I2C messages may necessitate the preparation of config, go,
> and tx DMA TREs. However, a channel TRE size of 64 is often insufficient,
> potentially leading to failures due to inadequate memory space.
> 
> Add additional argument to dma-cell property for channel TRE size.
> With this, adjust the channel TRE size via the device tree.
> The default size is 64, but clients can modify this value based on
> their specific requirements.
> 
> Signed-off-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com>
> ---
>  Documentation/devicetree/bindings/dma/qcom,gpi.yaml | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
> index 4df4e61895d2..002495921643 100644
> --- a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
> +++ b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
> @@ -54,14 +54,16 @@ properties:
>      maxItems: 13
>  
>    "#dma-cells":
> -    const: 3
> +    minItems: 3
> +    maxItems: 4
>      description: >
>        DMA clients must use the format described in dma.txt, giving a phandle
> -      to the DMA controller plus the following 3 integer cells:
> +      to the DMA controller plus the following 4 integer cells:
>        - channel: if set to 0xffffffff, any available channel will be allocated
>          for the client. Otherwise, the exact channel specified will be used.
>        - seid: serial id of the client as defined in the SoC documentation.
>        - client: type of the client as defined in dt-bindings/dma/qcom-gpi.h
> +      - channel-tre-size: size of the channel TRE (transfer ring element)

This is a firmware /software property, why should this be in hardware
description?

-- 
~Vinod

Re: [PATCH v1 2/5] arm64: dts: qcom: Add support for configuring channel TRE size

[Bjorn Andersson]

On Tue, Oct 15, 2024 at 03:33:00PM GMT, Krzysztof Kozlowski wrote:
> On 15/10/2024 14:07, Jyothi Kumar Seerapu wrote:
> > When high performance with multiple i2c messages in a single transfer
> > is required, employ Block Event Interrupt (BEI) to trigger interrupts
> > after specific messages transfer and the last message transfer,
> > thereby reducing interrupts.
> > For each i2c message transfer, a series of Transfer Request Elements(TREs)
> > must be programmed, including config tre for frequency configuration,
> > go tre for holding i2c address and dma tre for holding dma buffer address,
> > length as per the hardware programming guide. For transfer using BEI,
> > multiple I2C messages may necessitate the preparation of config, go,
> > and tx DMA TREs. However, a channel TRE size of 64 is often insufficient,
> > potentially leading to failures due to inadequate memory space.
> > 
> > Adjust the channel TRE size through the device tree.
> > The default size is 64, but clients can modify this value based on
> > their heigher channel TRE size requirements.
> > 
> > Signed-off-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com>
> > ---
> >  arch/arm64/boot/dts/qcom/sc7280.dtsi | 132 +++++++++++++--------------
> >  1 file changed, 66 insertions(+), 66 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > index 3d8410683402..c7c0e15ff9d3 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > @@ -1064,7 +1064,7 @@
> >  		};
> >  
> >  		gpi_dma0: dma-controller@900000 {
> > -			#dma-cells = <3>;
> > +			#dma-cells = <4>;
> >  			compatible = "qcom,sc7280-gpi-dma", "qcom,sm6350-gpi-dma";
> >  			reg = <0 0x00900000 0 0x60000>;
> >  			interrupts = <GIC_SPI 244 IRQ_TYPE_LEVEL_HIGH>,
> > @@ -1114,8 +1114,8 @@
> >  							"qup-memory";
> >  				power-domains = <&rpmhpd SC7280_CX>;
> >  				required-opps = <&rpmhpd_opp_low_svs>;
> > -				dmas = <&gpi_dma0 0 0 QCOM_GPI_I2C>,
> > -				       <&gpi_dma0 1 0 QCOM_GPI_I2C>;
> > +				dmas = <&gpi_dma0 0 0 QCOM_GPI_I2C 64>,
> > +				       <&gpi_dma0 1 0 QCOM_GPI_I2C 64>;
> 
> So everywhere is 64, thus this is fixed. Deduce it from the compatible
> 

If I understand correctly, it's a software tunable property, used to
balance how many TRE elements that should be preallocated.

If so, it would not be a property of the hardware/compatible, but rather
a result of profiling and a balance between memory "waste" and
performance.

Regards,
Bjorn

> Best regards,
> Krzysztof
> 

Re: [PATCH v1 5/5] i2c: i2c-qcom-geni: Add Block event interrupt support

[Andi Shyti]

Hi Jyothi,

...

> @@ -523,26 +576,49 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
>  	enum dma_transfer_direction dma_dirn;
>  	struct dma_async_tx_descriptor *desc;
>  	int ret;
> +	struct gpi_multi_xfer *gi2c_gpi_xfer;
> +	dma_cookie_t cookie;
>  
>  	peripheral = config->peripheral_config;
> -
> -	dma_buf = i2c_get_dma_safe_msg_buf(msg, 1);
> -	if (!dma_buf)
> +	gi2c_gpi_xfer = &peripheral->multi_xfer;
> +	gi2c_gpi_xfer->msg_idx_cnt = cur_msg_idx;
> +	dma_buf = gi2c_gpi_xfer->dma_buf[gi2c_gpi_xfer->buf_idx];
> +	addr = gi2c_gpi_xfer->dma_addr[gi2c_gpi_xfer->buf_idx];
> +
> +	dma_buf = i2c_get_dma_safe_msg_buf(&msgs[gi2c_gpi_xfer->msg_idx_cnt], 1);
> +	if (!dma_buf) {
> +		gi2c->err = -ENOMEM;
>  		return -ENOMEM;
> +	}
>  
>  	if (op == I2C_WRITE)
>  		map_dirn = DMA_TO_DEVICE;
>  	else
>  		map_dirn = DMA_FROM_DEVICE;
>  
> -	addr = dma_map_single(gi2c->se.dev->parent, dma_buf, msg->len, map_dirn);
> +	addr = dma_map_single(gi2c->se.dev->parent,
> +			      dma_buf, msgs[gi2c_gpi_xfer->msg_idx_cnt].len,

You could save msgs[gi2c_gpi_xfer->msg_idx_cnt] in a separate
variable to avoid this extra indexing.

> +			      map_dirn);
>  	if (dma_mapping_error(gi2c->se.dev->parent, addr)) {
> -		i2c_put_dma_safe_msg_buf(dma_buf, msg, false);
> +		i2c_put_dma_safe_msg_buf(dma_buf, &msgs[gi2c_gpi_xfer->msg_idx_cnt],
> +					 false);
> +		gi2c->err = -ENOMEM;
>  		return -ENOMEM;
>  	}
>  
> +	if (gi2c->is_tx_multi_xfer) {
> +		if (((gi2c_gpi_xfer->msg_idx_cnt + 1) % NUM_MSGS_PER_IRQ))
> +			peripheral->flags |= QCOM_GPI_BLOCK_EVENT_IRQ;
> +		else
> +			peripheral->flags &= ~QCOM_GPI_BLOCK_EVENT_IRQ;
> +
> +		/* BEI bit to be cleared for last TRE */
> +		if (gi2c_gpi_xfer->msg_idx_cnt == gi2c->num_msgs - 1)
> +			peripheral->flags &= ~QCOM_GPI_BLOCK_EVENT_IRQ;
> +	}
> +
>  	/* set the length as message for rx txn */
> -	peripheral->rx_len = msg->len;
> +	peripheral->rx_len = msgs[gi2c_gpi_xfer->msg_idx_cnt].len;
>  	peripheral->op = op;
>  
>  	ret = dmaengine_slave_config(dma_chan, config);
> @@ -560,25 +636,56 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
>  	else
>  		dma_dirn = DMA_DEV_TO_MEM;
>  
> -	desc = dmaengine_prep_slave_single(dma_chan, addr, msg->len, dma_dirn, flags);
> +	desc = dmaengine_prep_slave_single(dma_chan, addr,
> +					   msgs[gi2c_gpi_xfer->msg_idx_cnt].len,
> +					   dma_dirn, flags);
>  	if (!desc) {
>  		dev_err(gi2c->se.dev, "prep_slave_sg failed\n");
> -		ret = -EIO;
> +		gi2c->err = -EIO;
>  		goto err_config;
>  	}
>  
>  	desc->callback_result = i2c_gpi_cb_result;
>  	desc->callback_param = gi2c;
>  
> -	dmaengine_submit(desc);
> -	*buf = dma_buf;
> -	*dma_addr_p = addr;
> +	if (!((msgs[cur_msg_idx].flags & I2C_M_RD) && op == I2C_WRITE)) {
> +		gi2c_gpi_xfer->msg_idx_cnt++;
> +		gi2c_gpi_xfer->buf_idx = (cur_msg_idx + 1) % QCOM_GPI_MAX_NUM_MSGS;
> +	}
> +	cookie = dmaengine_submit(desc);
> +	if (dma_submit_error(cookie)) {
> +		dev_err(gi2c->se.dev,
> +			"%s: dmaengine_submit failed (%d)\n", __func__, cookie);
> +		return -EINVAL;

goto err_config?

> +	}
>  
> +	if (gi2c->is_tx_multi_xfer) {
> +		dma_async_issue_pending(gi2c->tx_c);
> +		if ((cur_msg_idx == (gi2c->num_msgs - 1)) ||
> +		    (gi2c_gpi_xfer->msg_idx_cnt >=
> +		     QCOM_GPI_MAX_NUM_MSGS + gi2c_gpi_xfer->freed_msg_cnt)) {
> +			ret = gpi_multi_desc_process(gi2c->se.dev, gi2c_gpi_xfer,
> +						     gi2c->num_msgs, XFER_TIMEOUT,
> +						     &gi2c->done);
> +			if (ret) {
> +				dev_dbg(gi2c->se.dev,
> +					"I2C multi write msg transfer timeout: %d\n",
> +					ret);

if you are returning an error, then print an error.

> +				gi2c->err = -ETIMEDOUT;

gi2c->err = ret?

> +				goto err_config;
> +			}
> +		}
> +	} else {
> +		/* Non multi descriptor message transfer */
> +		*buf = dma_buf;
> +		*dma_addr_p = addr;
> +	}
>  	return 0;
>  
>  err_config:
> -	dma_unmap_single(gi2c->se.dev->parent, addr, msg->len, map_dirn);
> -	i2c_put_dma_safe_msg_buf(dma_buf, msg, false);
> +	dma_unmap_single(gi2c->se.dev->parent, addr,
> +			 msgs[cur_msg_idx].len, map_dirn);
> +	i2c_put_dma_safe_msg_buf(dma_buf, &msgs[cur_msg_idx], false);
>  	return ret;

I would have one more label here:

   out:
	gi2c->err = ret;

	return ret;

in order to avoid always assigning twice

>  }
>  
> @@ -590,6 +697,7 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
>  	unsigned long time_left;
>  	dma_addr_t tx_addr, rx_addr;
>  	void *tx_buf = NULL, *rx_buf = NULL;
> +	struct gpi_multi_xfer *tx_multi_xfer;
>  	const struct geni_i2c_clk_fld *itr = gi2c->clk_fld;
>  
>  	config.peripheral_config = &peripheral;
> @@ -603,6 +711,39 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
>  	peripheral.set_config = 1;
>  	peripheral.multi_msg = false;
>  
> +	gi2c->gpi_config = &peripheral;
> +	gi2c->num_msgs = num;
> +	gi2c->is_tx_multi_xfer = false;
> +	gi2c->tx_irq_cnt = 0;
> +
> +	tx_multi_xfer = &peripheral.multi_xfer;
> +	tx_multi_xfer->msg_idx_cnt = 0;
> +	tx_multi_xfer->buf_idx = 0;
> +	tx_multi_xfer->unmap_msg_cnt = 0;
> +	tx_multi_xfer->freed_msg_cnt = 0;
> +	tx_multi_xfer->irq_msg_cnt = 0;
> +	tx_multi_xfer->irq_cnt = 0;

you can initialize tx_multi_xfer to "{ };" to avoid all these
" = 0"

> +
> +	/*
> +	 * If number of write messages are four and higher then
> +	 * configure hardware for multi descriptor transfers with BEI.
> +	 */
> +	if (num >= MIN_NUM_OF_MSGS_MULTI_DESC) {
> +		gi2c->is_tx_multi_xfer = true;
> +		for (i = 0; i < num; i++) {
> +			if (msgs[i].flags & I2C_M_RD) {
> +				/*
> +				 * Multi descriptor transfer with BEI
> +				 * support is enabled for write transfers.
> +				 * Add BEI optimization support for read
> +				 * transfers later.
> +				 */
> +				gi2c->is_tx_multi_xfer = false;
> +				break;
> +			}
> +		}
> +	}
> +
>  	for (i = 0; i < num; i++) {
>  		gi2c->cur = &msgs[i];
>  		gi2c->err = 0;
> @@ -613,14 +754,16 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
>  			peripheral.stretch = 1;
>  
>  		peripheral.addr = msgs[i].addr;
> +		if (i > 0 && (!(msgs[i].flags & I2C_M_RD)))
> +			peripheral.multi_msg = false;
>  
> -		ret =  geni_i2c_gpi(gi2c, &msgs[i], &config,
> +		ret =  geni_i2c_gpi(gi2c, msgs, i, &config,

what is the point of passing 'i' if you always refer to msgs[i]
in geni_i2c_gpi()?

>  				    &tx_addr, &tx_buf, I2C_WRITE, gi2c->tx_c);
>  		if (ret)
>  			goto err;
>  
>  		if (msgs[i].flags & I2C_M_RD) {
> -			ret =  geni_i2c_gpi(gi2c, &msgs[i], &config,
> +			ret =  geni_i2c_gpi(gi2c, msgs, i, &config,
>  					    &rx_addr, &rx_buf, I2C_READ, gi2c->rx_c);
>  			if (ret)
>  				goto err;
> @@ -628,18 +771,28 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
>  			dma_async_issue_pending(gi2c->rx_c);
>  		}
>  
> -		dma_async_issue_pending(gi2c->tx_c);
> -
> -		time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
> -		if (!time_left)
> -			gi2c->err = -ETIMEDOUT;
> +		if (!gi2c->is_tx_multi_xfer) {
> +			dma_async_issue_pending(gi2c->tx_c);
> +			time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
> +			if (!time_left) {
> +				dev_err(gi2c->se.dev, "%s:I2C timeout\n", __func__);
> +				gi2c->err = -ETIMEDOUT;
> +			}
> +		}
>  
>  		if (gi2c->err) {
>  			ret = gi2c->err;
>  			goto err;
>  		}
>  
> -		geni_i2c_gpi_unmap(gi2c, &msgs[i], tx_buf, tx_addr, rx_buf, rx_addr);
> +		if (!gi2c->is_tx_multi_xfer) {
> +			geni_i2c_gpi_unmap(gi2c, &msgs[i], tx_buf, tx_addr, rx_buf, rx_addr);
> +		} else {
> +			if (gi2c->tx_irq_cnt != tx_multi_xfer->irq_cnt) {

   else if (...) {
   	...
   }

Andi

Re: [PATCH v1 2/5] arm64: dts: qcom: Add support for configuring channel TRE size

[Krzysztof Kozlowski]

On 16/10/2024 16:35, Bjorn Andersson wrote:
>>> @@ -1064,7 +1064,7 @@
>>>  		};
>>>  
>>>  		gpi_dma0: dma-controller@900000 {
>>> -			#dma-cells = <3>;
>>> +			#dma-cells = <4>;
>>>  			compatible = "qcom,sc7280-gpi-dma", "qcom,sm6350-gpi-dma";
>>>  			reg = <0 0x00900000 0 0x60000>;
>>>  			interrupts = <GIC_SPI 244 IRQ_TYPE_LEVEL_HIGH>,
>>> @@ -1114,8 +1114,8 @@
>>>  							"qup-memory";
>>>  				power-domains = <&rpmhpd SC7280_CX>;
>>>  				required-opps = <&rpmhpd_opp_low_svs>;
>>> -				dmas = <&gpi_dma0 0 0 QCOM_GPI_I2C>,
>>> -				       <&gpi_dma0 1 0 QCOM_GPI_I2C>;
>>> +				dmas = <&gpi_dma0 0 0 QCOM_GPI_I2C 64>,
>>> +				       <&gpi_dma0 1 0 QCOM_GPI_I2C 64>;
>>
>> So everywhere is 64, thus this is fixed. Deduce it from the compatible
>>
> 
> If I understand correctly, it's a software tunable property, used to
> balance how many TRE elements that should be preallocated.
> 
> If so, it would not be a property of the hardware/compatible, but rather
> a result of profiling and a balance between memory "waste" and
> performance.

In such case I would prefer it being runtime-calculated by the driver,
based on frequency or expected bandwidth.

And in any case if this is about to stay, having here default values
means all upstream users don't need it. What's not upstream, does not
exist in such context. We don't add features which are not used by upstream.

Best regards,
Krzysztof

Re: [PATCH v1 5/5] i2c: i2c-qcom-geni: Add Block event interrupt support

[kernel test robot]

Hi Jyothi,

kernel test robot noticed the following build errors:

[auto build test ERROR on 55bcd2e0d04c1171d382badef1def1fd04ef66c5]

url:    https://github.com/intel-lab-lkp/linux/commits/Jyothi-Kumar-Seerapu/dt-bindings-dmaengine-qcom-gpi-Add-additional-arg-to-dma-cell-property/20241015-202637
base:   55bcd2e0d04c1171d382badef1def1fd04ef66c5
patch link:    https://lore.kernel.org/r/20241015120750.21217-6-quic_jseerapu%40quicinc.com
patch subject: [PATCH v1 5/5] i2c: i2c-qcom-geni: Add Block event interrupt support
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20241019/202410190549.hGAfByqg-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241019/202410190549.hGAfByqg-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410190549.hGAfByqg-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/i2c/busses/i2c-qcom-geni.c:562:8: error: incompatible pointer to integer conversion passing 'dma_addr_t *' (aka 'unsigned long long *') to parameter of type 'dma_addr_t' (aka 'unsigned long long'); dereference with * [-Wint-conversion]
     562 |                                    tx_multi_xfer->dma_addr[wr_idx], NULL, NULL);
         |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                                    *
   drivers/i2c/busses/i2c-qcom-geni.c:519:36: note: passing argument to parameter 'tx_addr' here
     519 |                                void *tx_buf, dma_addr_t tx_addr,
         |                                                         ^
>> drivers/i2c/busses/i2c-qcom-geni.c:562:47: error: incompatible pointer to integer conversion passing 'void *' to parameter of type 'dma_addr_t' (aka 'unsigned long long') [-Wint-conversion]
     562 |                                    tx_multi_xfer->dma_addr[wr_idx], NULL, NULL);
         |                                                                           ^~~~
   include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
       8 | #define NULL ((void *)0)
         |              ^~~~~~~~~~~
   drivers/i2c/busses/i2c-qcom-geni.c:520:36: note: passing argument to parameter 'rx_addr' here
     520 |                                void *rx_buf, dma_addr_t rx_addr)
         |                                                         ^
>> drivers/i2c/busses/i2c-qcom-geni.c:586:7: error: incompatible pointer to integer conversion assigning to 'dma_addr_t' (aka 'unsigned long long') from 'dma_addr_t *' (aka 'unsigned long long *'); dereference with * [-Wint-conversion]
     586 |         addr = gi2c_gpi_xfer->dma_addr[gi2c_gpi_xfer->buf_idx];
         |              ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                *
   3 errors generated.

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for OMAP2PLUS_MBOX
   Depends on [n]: MAILBOX [=y] && (ARCH_OMAP2PLUS || ARCH_K3)
   Selected by [y]:
   - TI_K3_M4_REMOTEPROC [=y] && REMOTEPROC [=y] && (ARCH_K3 || COMPILE_TEST [=y])


vim +562 drivers/i2c/busses/i2c-qcom-geni.c

   532	
   533	/**
   534	 * gpi_i2c_multi_desc_unmap() - unmaps the buffers post multi message TX transfers
   535	 * @dev: pointer to the corresponding dev node
   536	 * @gi2c: i2c dev handle
   537	 * @msgs: i2c messages array
   538	 * @peripheral: pointer to the gpi_i2c_config
   539	 */
   540	static void gpi_i2c_multi_desc_unmap(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[],
   541					     struct gpi_i2c_config *peripheral)
   542	{
   543		u32 msg_xfer_cnt, wr_idx = 0;
   544		struct gpi_multi_xfer *tx_multi_xfer = &peripheral->multi_xfer;
   545	
   546		/*
   547		 * In error case, need to unmap all messages based on the msg_idx_cnt.
   548		 * Non-error case unmap all the processed messages.
   549		 */
   550		if (gi2c->err)
   551			msg_xfer_cnt = tx_multi_xfer->msg_idx_cnt;
   552		else
   553			msg_xfer_cnt = tx_multi_xfer->irq_cnt * NUM_MSGS_PER_IRQ;
   554	
   555		/* Unmap the processed DMA buffers based on the received interrupt count */
   556		for (; tx_multi_xfer->unmap_msg_cnt < msg_xfer_cnt; tx_multi_xfer->unmap_msg_cnt++) {
   557			if (tx_multi_xfer->unmap_msg_cnt == gi2c->num_msgs)
   558				break;
   559			wr_idx = tx_multi_xfer->unmap_msg_cnt % QCOM_GPI_MAX_NUM_MSGS;
   560			geni_i2c_gpi_unmap(gi2c, &msgs[tx_multi_xfer->unmap_msg_cnt],
   561					   tx_multi_xfer->dma_buf[wr_idx],
 > 562					   tx_multi_xfer->dma_addr[wr_idx], NULL, NULL);
   563			tx_multi_xfer->freed_msg_cnt++;
   564		}
   565	}
   566	
   567	static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], int cur_msg_idx,
   568				struct dma_slave_config *config, dma_addr_t *dma_addr_p,
   569				void **buf, unsigned int op, struct dma_chan *dma_chan)
   570	{
   571		struct gpi_i2c_config *peripheral;
   572		unsigned int flags;
   573		void *dma_buf;
   574		dma_addr_t addr;
   575		enum dma_data_direction map_dirn;
   576		enum dma_transfer_direction dma_dirn;
   577		struct dma_async_tx_descriptor *desc;
   578		int ret;
   579		struct gpi_multi_xfer *gi2c_gpi_xfer;
   580		dma_cookie_t cookie;
   581	
   582		peripheral = config->peripheral_config;
   583		gi2c_gpi_xfer = &peripheral->multi_xfer;
   584		gi2c_gpi_xfer->msg_idx_cnt = cur_msg_idx;
   585		dma_buf = gi2c_gpi_xfer->dma_buf[gi2c_gpi_xfer->buf_idx];
 > 586		addr = gi2c_gpi_xfer->dma_addr[gi2c_gpi_xfer->buf_idx];
   587	
   588		dma_buf = i2c_get_dma_safe_msg_buf(&msgs[gi2c_gpi_xfer->msg_idx_cnt], 1);
   589		if (!dma_buf) {
   590			gi2c->err = -ENOMEM;
   591			return -ENOMEM;
   592		}
   593	
   594		if (op == I2C_WRITE)
   595			map_dirn = DMA_TO_DEVICE;
   596		else
   597			map_dirn = DMA_FROM_DEVICE;
   598	
   599		addr = dma_map_single(gi2c->se.dev->parent,
   600				      dma_buf, msgs[gi2c_gpi_xfer->msg_idx_cnt].len,
   601				      map_dirn);
   602		if (dma_mapping_error(gi2c->se.dev->parent, addr)) {
   603			i2c_put_dma_safe_msg_buf(dma_buf, &msgs[gi2c_gpi_xfer->msg_idx_cnt],
   604						 false);
   605			gi2c->err = -ENOMEM;
   606			return -ENOMEM;
   607		}
   608	
   609		if (gi2c->is_tx_multi_xfer) {
   610			if (((gi2c_gpi_xfer->msg_idx_cnt + 1) % NUM_MSGS_PER_IRQ))
   611				peripheral->flags |= QCOM_GPI_BLOCK_EVENT_IRQ;
   612			else
   613				peripheral->flags &= ~QCOM_GPI_BLOCK_EVENT_IRQ;
   614	
   615			/* BEI bit to be cleared for last TRE */
   616			if (gi2c_gpi_xfer->msg_idx_cnt == gi2c->num_msgs - 1)
   617				peripheral->flags &= ~QCOM_GPI_BLOCK_EVENT_IRQ;
   618		}
   619	
   620		/* set the length as message for rx txn */
   621		peripheral->rx_len = msgs[gi2c_gpi_xfer->msg_idx_cnt].len;
   622		peripheral->op = op;
   623	
   624		ret = dmaengine_slave_config(dma_chan, config);
   625		if (ret) {
   626			dev_err(gi2c->se.dev, "dma config error: %d for op:%d\n", ret, op);
   627			goto err_config;
   628		}
   629	
   630		peripheral->set_config = 0;
   631		peripheral->multi_msg = true;
   632		flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
   633	
   634		if (op == I2C_WRITE)
   635			dma_dirn = DMA_MEM_TO_DEV;
   636		else
   637			dma_dirn = DMA_DEV_TO_MEM;
   638	
   639		desc = dmaengine_prep_slave_single(dma_chan, addr,
   640						   msgs[gi2c_gpi_xfer->msg_idx_cnt].len,
   641						   dma_dirn, flags);
   642		if (!desc) {
   643			dev_err(gi2c->se.dev, "prep_slave_sg failed\n");
   644			gi2c->err = -EIO;
   645			goto err_config;
   646		}
   647	
   648		desc->callback_result = i2c_gpi_cb_result;
   649		desc->callback_param = gi2c;
   650	
   651		if (!((msgs[cur_msg_idx].flags & I2C_M_RD) && op == I2C_WRITE)) {
   652			gi2c_gpi_xfer->msg_idx_cnt++;
   653			gi2c_gpi_xfer->buf_idx = (cur_msg_idx + 1) % QCOM_GPI_MAX_NUM_MSGS;
   654		}
   655		cookie = dmaengine_submit(desc);
   656		if (dma_submit_error(cookie)) {
   657			dev_err(gi2c->se.dev,
   658				"%s: dmaengine_submit failed (%d)\n", __func__, cookie);
   659			return -EINVAL;
   660		}
   661	
   662		if (gi2c->is_tx_multi_xfer) {
   663			dma_async_issue_pending(gi2c->tx_c);
   664			if ((cur_msg_idx == (gi2c->num_msgs - 1)) ||
   665			    (gi2c_gpi_xfer->msg_idx_cnt >=
   666			     QCOM_GPI_MAX_NUM_MSGS + gi2c_gpi_xfer->freed_msg_cnt)) {
   667				ret = gpi_multi_desc_process(gi2c->se.dev, gi2c_gpi_xfer,
   668							     gi2c->num_msgs, XFER_TIMEOUT,
   669							     &gi2c->done);
   670				if (ret) {
   671					dev_dbg(gi2c->se.dev,
   672						"I2C multi write msg transfer timeout: %d\n",
   673						ret);
   674					gi2c->err = -ETIMEDOUT;
   675					goto err_config;
   676				}
   677			}
   678		} else {
   679			/* Non multi descriptor message transfer */
   680			*buf = dma_buf;
   681			*dma_addr_p = addr;
   682		}
   683		return 0;
   684	
   685	err_config:
   686		dma_unmap_single(gi2c->se.dev->parent, addr,
   687				 msgs[cur_msg_idx].len, map_dirn);
   688		i2c_put_dma_safe_msg_buf(dma_buf, &msgs[cur_msg_idx], false);
   689		return ret;
   690	}
   691	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1 5/5] i2c: i2c-qcom-geni: Add Block event interrupt support

[kernel test robot]

Hi Jyothi,

kernel test robot noticed the following build errors:

[auto build test ERROR on 55bcd2e0d04c1171d382badef1def1fd04ef66c5]

url:    https://github.com/intel-lab-lkp/linux/commits/Jyothi-Kumar-Seerapu/dt-bindings-dmaengine-qcom-gpi-Add-additional-arg-to-dma-cell-property/20241015-202637
base:   55bcd2e0d04c1171d382badef1def1fd04ef66c5
patch link:    https://lore.kernel.org/r/20241015120750.21217-6-quic_jseerapu%40quicinc.com
patch subject: [PATCH v1 5/5] i2c: i2c-qcom-geni: Add Block event interrupt support
config: i386-allmodconfig (https://download.01.org/0day-ci/archive/20241019/202410191055.bi1pWTAY-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241019/202410191055.bi1pWTAY-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410191055.bi1pWTAY-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "gpi_multi_desc_process" [drivers/i2c/busses/i2c-qcom-geni.ko] undefined!

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for GET_FREE_REGION
   Depends on [n]: SPARSEMEM [=n]
   Selected by [m]:
   - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
   WARNING: unmet direct dependencies detected for OMAP2PLUS_MBOX
   Depends on [n]: MAILBOX [=y] && (ARCH_OMAP2PLUS || ARCH_K3)
   Selected by [m]:
   - TI_K3_M4_REMOTEPROC [=m] && REMOTEPROC [=y] && (ARCH_K3 || COMPILE_TEST [=y])

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1 1/5] dt-bindings: dmaengine: qcom: gpi: Add additional arg to dma-cell property

[Jyothi Kumar Seerapu]

On 10/15/2024 7:01 PM, Krzysztof Kozlowski wrote:
> On 15/10/2024 14:07, Jyothi Kumar Seerapu wrote:
>> When high performance with multiple i2c messages in a single transfer
>> is required, employ Block Event Interrupt (BEI) to trigger interrupts
>> after specific messages transfer and the last message transfer,
>> thereby reducing interrupts.
>>
>> For each i2c message transfer, a series of Transfer Request Elements(TREs)
>> must be programmed, including config tre for frequency configuration,
>> go tre for holding i2c address and dma tre for holding dma buffer address,
>> length as per the hardware programming guide. For transfer using BEI,
>> multiple I2C messages may necessitate the preparation of config, go,
>> and tx DMA TREs. However, a channel TRE size of 64 is often insufficient,
>> potentially leading to failures due to inadequate memory space.
> Please kindly test the patches before you sent them. Upstream is not a
> testing service.

Sure, i will take care to test the required patches.


>
> Best regards,
> Krzysztof
>

Re: [PATCH v1 3/5] dmaengine: qcom: gpi: Add provision to support TRE size as the fourth argument of dma-cells property

[Konrad Dybcio]

On 15.10.2024 2:07 PM, Jyothi Kumar Seerapu wrote:
> The current GPI driver hardcodes the channel TRE (Transfer Ring Element)
> size to 64. For scenarios requiring high performance with multiple
> messages in a transfer, use Block Event Interrupt (BEI).
> This method triggers interrupt after specific message transfers and
> the last message transfer, effectively reducing the number of interrupts.
> For multiple transfers utilizing BEI, a channel TRE size of 64 is
> insufficient and may lead to transfer failures, indicated by errors
> related to unavailable memory space.
> 
> Added provision to modify the channel TRE size via the device tree.
> The Default channel TRE size is set to 64, but this value can update
> in the device tree which will then be parsed by the GPI driver.
> 
> Signed-off-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com>
> ---

1. Is the total memory pool for these shared?

2. Is there any scenario where we want TRE size to be lower and
   not higher? Are there any drawbacks to always keeping them at
   SOME_MAX_VALUE?

3. Is this something we should configure at boot time (in firmware)?
   Perhaps this could be decided based on client device settings (which
   may or may not require adding some field in the i2c framework)


Konrad

Re: [PATCH v1 1/5] dt-bindings: dmaengine: qcom: gpi: Add additional arg to dma-cell property

[Jyothi Kumar Seerapu]

On 10/16/2024 10:24 AM, Vinod Koul wrote:
> On 15-10-24, 17:37, Jyothi Kumar Seerapu wrote:
>> When high performance with multiple i2c messages in a single transfer
>> is required, employ Block Event Interrupt (BEI) to trigger interrupts
>> after specific messages transfer and the last message transfer,
>> thereby reducing interrupts.
>>
>> For each i2c message transfer, a series of Transfer Request Elements(TREs)
>> must be programmed, including config tre for frequency configuration,
>> go tre for holding i2c address and dma tre for holding dma buffer address,
>> length as per the hardware programming guide. For transfer using BEI,
>> multiple I2C messages may necessitate the preparation of config, go,
>> and tx DMA TREs. However, a channel TRE size of 64 is often insufficient,
>> potentially leading to failures due to inadequate memory space.
>>
>> Add additional argument to dma-cell property for channel TRE size.
>> With this, adjust the channel TRE size via the device tree.
>> The default size is 64, but clients can modify this value based on
>> their specific requirements.
>>
>> Signed-off-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com>
>> ---
>>   Documentation/devicetree/bindings/dma/qcom,gpi.yaml | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
>> index 4df4e61895d2..002495921643 100644
>> --- a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
>> +++ b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
>> @@ -54,14 +54,16 @@ properties:
>>       maxItems: 13
>>   
>>     "#dma-cells":
>> -    const: 3
>> +    minItems: 3
>> +    maxItems: 4
>>       description: >
>>         DMA clients must use the format described in dma.txt, giving a phandle
>> -      to the DMA controller plus the following 3 integer cells:
>> +      to the DMA controller plus the following 4 integer cells:
>>         - channel: if set to 0xffffffff, any available channel will be allocated
>>           for the client. Otherwise, the exact channel specified will be used.
>>         - seid: serial id of the client as defined in the SoC documentation.
>>         - client: type of the client as defined in dt-bindings/dma/qcom-gpi.h
>> +      - channel-tre-size: size of the channel TRE (transfer ring element)
> This is a firmware /software property, why should this be in hardware
> description?

This is a software property and here trying to add channel tre size as a 
4th argument of dma-cells property.

In V2, i have reverted the DT and binding changes related to adding new 
argument for dma-cells property and used GPI driver defined value.


Regards,

JyothiKumar


>

Re: [PATCH v1 2/5] arm64: dts: qcom: Add support for configuring channel TRE size

[Jyothi Kumar Seerapu]

On 10/17/2024 12:40 PM, Krzysztof Kozlowski wrote:
> On 16/10/2024 16:35, Bjorn Andersson wrote:
>>>> @@ -1064,7 +1064,7 @@
>>>>   		};
>>>>   
>>>>   		gpi_dma0: dma-controller@900000 {
>>>> -			#dma-cells = <3>;
>>>> +			#dma-cells = <4>;
>>>>   			compatible = "qcom,sc7280-gpi-dma", "qcom,sm6350-gpi-dma";
>>>>   			reg = <0 0x00900000 0 0x60000>;
>>>>   			interrupts = <GIC_SPI 244 IRQ_TYPE_LEVEL_HIGH>,
>>>> @@ -1114,8 +1114,8 @@
>>>>   							"qup-memory";
>>>>   				power-domains = <&rpmhpd SC7280_CX>;
>>>>   				required-opps = <&rpmhpd_opp_low_svs>;
>>>> -				dmas = <&gpi_dma0 0 0 QCOM_GPI_I2C>,
>>>> -				       <&gpi_dma0 1 0 QCOM_GPI_I2C>;
>>>> +				dmas = <&gpi_dma0 0 0 QCOM_GPI_I2C 64>,
>>>> +				       <&gpi_dma0 1 0 QCOM_GPI_I2C 64>;
>>>
>>> So everywhere is 64, thus this is fixed. Deduce it from the compatible
>>>
>>
>> If I understand correctly, it's a software tunable property, used to
>> balance how many TRE elements that should be preallocated.
>>
>> If so, it would not be a property of the hardware/compatible, but rather
>> a result of profiling and a balance between memory "waste" and
>> performance.
> 
> In such case I would prefer it being runtime-calculated by the driver,
> based on frequency or expected bandwidth.
> 
> And in any case if this is about to stay, having here default values
> means all upstream users don't need it. What's not upstream, does not
> exist in such context. We don't add features which are not used by upstream.
> 
> Best regards,
> Krzysztof
> 

Thanks Krzysztof and Bjorn for the review comments.

I have reverted the changes of supporting channel tre size from DT and 
will make use of existing channel tre size defined in GPI driver which 
is 64.

Regards,
JyothiKumar.

Re: [PATCH v1 1/5] dt-bindings: dmaengine: qcom: gpi: Add additional arg to dma-cell property

[Jyothi Kumar Seerapu]

On 10/15/2024 7:31 PM, Rob Herring wrote:
> On Tue, Oct 15, 2024 at 05:37:46PM +0530, Jyothi Kumar Seerapu wrote:
>> When high performance with multiple i2c messages in a single transfer
>> is required, employ Block Event Interrupt (BEI) to trigger interrupts
>> after specific messages transfer and the last message transfer,
>> thereby reducing interrupts.
>>
>> For each i2c message transfer, a series of Transfer Request Elements(TREs)
>> must be programmed, including config tre for frequency configuration,
>> go tre for holding i2c address and dma tre for holding dma buffer address,
>> length as per the hardware programming guide. For transfer using BEI,
>> multiple I2C messages may necessitate the preparation of config, go,
>> and tx DMA TREs. However, a channel TRE size of 64 is often insufficient,
>> potentially leading to failures due to inadequate memory space.
>>
>> Add additional argument to dma-cell property for channel TRE size.
> 
> No such property 'dma-cell'

Thanks for pointing it out, yeah it should be 'dma-cells'.
> 
>> With this, adjust the channel TRE size via the device tree.
>> The default size is 64, but clients can modify this value based on
>> their specific requirements.
>>
>> Signed-off-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com>
>> ---
>>   Documentation/devicetree/bindings/dma/qcom,gpi.yaml | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
>> index 4df4e61895d2..002495921643 100644
>> --- a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
>> +++ b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
>> @@ -54,14 +54,16 @@ properties:
>>       maxItems: 13
>>   
>>     "#dma-cells":
>> -    const: 3
>> +    minItems: 3
>> +    maxItems: 4
>>       description: >
>>         DMA clients must use the format described in dma.txt, giving a phandle
>> -      to the DMA controller plus the following 3 integer cells:
>> +      to the DMA controller plus the following 4 integer cells:
>>         - channel: if set to 0xffffffff, any available channel will be allocated
>>           for the client. Otherwise, the exact channel specified will be used.
>>         - seid: serial id of the client as defined in the SoC documentation.
>>         - client: type of the client as defined in dt-bindings/dma/qcom-gpi.h
>> +      - channel-tre-size: size of the channel TRE (transfer ring element)
>>   
>>     iommus:
>>       maxItems: 1
>> -- 
>> 2.17.1
>>

Re: [PATCH v1 1/5] dt-bindings: dmaengine: qcom: gpi: Add additional arg to dma-cell property

[Jyothi Kumar Seerapu]

On 10/16/2024 10:24 AM, Vinod Koul wrote:
> On 15-10-24, 17:37, Jyothi Kumar Seerapu wrote:
>> When high performance with multiple i2c messages in a single transfer
>> is required, employ Block Event Interrupt (BEI) to trigger interrupts
>> after specific messages transfer and the last message transfer,
>> thereby reducing interrupts.
>>
>> For each i2c message transfer, a series of Transfer Request Elements(TREs)
>> must be programmed, including config tre for frequency configuration,
>> go tre for holding i2c address and dma tre for holding dma buffer address,
>> length as per the hardware programming guide. For transfer using BEI,
>> multiple I2C messages may necessitate the preparation of config, go,
>> and tx DMA TREs. However, a channel TRE size of 64 is often insufficient,
>> potentially leading to failures due to inadequate memory space.
>>
>> Add additional argument to dma-cell property for channel TRE size.
>> With this, adjust the channel TRE size via the device tree.
>> The default size is 64, but clients can modify this value based on
>> their specific requirements.
>>
>> Signed-off-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com>
>> ---
>>   Documentation/devicetree/bindings/dma/qcom,gpi.yaml | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
>> index 4df4e61895d2..002495921643 100644
>> --- a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
>> +++ b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml
>> @@ -54,14 +54,16 @@ properties:
>>       maxItems: 13
>>   
>>     "#dma-cells":
>> -    const: 3
>> +    minItems: 3
>> +    maxItems: 4
>>       description: >
>>         DMA clients must use the format described in dma.txt, giving a phandle
>> -      to the DMA controller plus the following 3 integer cells:
>> +      to the DMA controller plus the following 4 integer cells:
>>         - channel: if set to 0xffffffff, any available channel will be allocated
>>           for the client. Otherwise, the exact channel specified will be used.
>>         - seid: serial id of the client as defined in the SoC documentation.
>>         - client: type of the client as defined in dt-bindings/dma/qcom-gpi.h
>> +      - channel-tre-size: size of the channel TRE (transfer ring element)
> 
> This is a firmware /software property, why should this be in hardware
> description?
> 
Hi, Yes, this is a software property and added as a 4th argument of 
"dma-cells" for configuring channel tre size, so added the description 
for dma-cells.
In V2 patch, i reverted the changes and will handle with the default 
channel tre size present in the GPI driver.

Regards,
JyothiKumar

Re: [PATCH v1 1/5] dt-bindings: dmaengine: qcom: gpi: Add additional arg to dma-cell property

[Jyothi Kumar Seerapu]

On 10/15/2024 6:56 PM, Rob Herring (Arm) wrote:
> 
> On Tue, 15 Oct 2024 17:37:46 +0530, Jyothi Kumar Seerapu wrote:
>> When high performance with multiple i2c messages in a single transfer
>> is required, employ Block Event Interrupt (BEI) to trigger interrupts
>> after specific messages transfer and the last message transfer,
>> thereby reducing interrupts.
>>
>> For each i2c message transfer, a series of Transfer Request Elements(TREs)
>> must be programmed, including config tre for frequency configuration,
>> go tre for holding i2c address and dma tre for holding dma buffer address,
>> length as per the hardware programming guide. For transfer using BEI,
>> multiple I2C messages may necessitate the preparation of config, go,
>> and tx DMA TREs. However, a channel TRE size of 64 is often insufficient,
>> potentially leading to failures due to inadequate memory space.
>>
>> Add additional argument to dma-cell property for channel TRE size.
>> With this, adjust the channel TRE size via the device tree.
>> The default size is 64, but clients can modify this value based on
>> their specific requirements.
>>
>> Signed-off-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com>
>> ---
>>   Documentation/devicetree/bindings/dma/qcom,gpi.yaml | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/dma/qcom,gpi.yaml: properties:#dma-cells: 'minItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
> 	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/dma/qcom,gpi.yaml: properties:#dma-cells: 'maxItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
> 	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241015120750.21217-2-quic_jseerapu@quicinc.com
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
> 

Thanks, i followed the instructions and resolved errors which observed 
with 'make dt_binding_check'.
But in V2 patch, i have reverted the DT and binding changes related to 
adding new argument for dma-cells property and instead used existing 
value for channel TRE size in GPI driver.

Regrads,
JyothiKumar

Re: [PATCH v1 5/5] i2c: i2c-qcom-geni: Add Block event interrupt support

[Jyothi Kumar Seerapu]

On 10/16/2024 8:36 PM, Andi Shyti wrote:
> Hi Jyothi,
> 
> ...
> 
>> @@ -523,26 +576,49 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
>>   	enum dma_transfer_direction dma_dirn;
>>   	struct dma_async_tx_descriptor *desc;
>>   	int ret;
>> +	struct gpi_multi_xfer *gi2c_gpi_xfer;
>> +	dma_cookie_t cookie;
>>   
>>   	peripheral = config->peripheral_config;
>> -
>> -	dma_buf = i2c_get_dma_safe_msg_buf(msg, 1);
>> -	if (!dma_buf)
>> +	gi2c_gpi_xfer = &peripheral->multi_xfer;
>> +	gi2c_gpi_xfer->msg_idx_cnt = cur_msg_idx;
>> +	dma_buf = gi2c_gpi_xfer->dma_buf[gi2c_gpi_xfer->buf_idx];
>> +	addr = gi2c_gpi_xfer->dma_addr[gi2c_gpi_xfer->buf_idx];
>> +
>> +	dma_buf = i2c_get_dma_safe_msg_buf(&msgs[gi2c_gpi_xfer->msg_idx_cnt], 1);
>> +	if (!dma_buf) {
>> +		gi2c->err = -ENOMEM;
>>   		return -ENOMEM;
>> +	}
>>   
>>   	if (op == I2C_WRITE)
>>   		map_dirn = DMA_TO_DEVICE;
>>   	else
>>   		map_dirn = DMA_FROM_DEVICE;
>>   
>> -	addr = dma_map_single(gi2c->se.dev->parent, dma_buf, msg->len, map_dirn);
>> +	addr = dma_map_single(gi2c->se.dev->parent,
>> +			      dma_buf, msgs[gi2c_gpi_xfer->msg_idx_cnt].len,
> 
> You could save msgs[gi2c_gpi_xfer->msg_idx_cnt] in a separate
> variable to avoid this extra indexing.
> 
Thanks Andi, moved gi2c_gpi_xfer->msg_idx_cnt to separate local variable.
>> +			      map_dirn);
>>   	if (dma_mapping_error(gi2c->se.dev->parent, addr)) {
>> -		i2c_put_dma_safe_msg_buf(dma_buf, msg, false);
>> +		i2c_put_dma_safe_msg_buf(dma_buf, &msgs[gi2c_gpi_xfer->msg_idx_cnt],
>> +					 false);
>> +		gi2c->err = -ENOMEM;
>>   		return -ENOMEM;
>>   	}
>>   
>> +	if (gi2c->is_tx_multi_xfer) {
>> +		if (((gi2c_gpi_xfer->msg_idx_cnt + 1) % NUM_MSGS_PER_IRQ))
>> +			peripheral->flags |= QCOM_GPI_BLOCK_EVENT_IRQ;
>> +		else
>> +			peripheral->flags &= ~QCOM_GPI_BLOCK_EVENT_IRQ;
>> +
>> +		/* BEI bit to be cleared for last TRE */
>> +		if (gi2c_gpi_xfer->msg_idx_cnt == gi2c->num_msgs - 1)
>> +			peripheral->flags &= ~QCOM_GPI_BLOCK_EVENT_IRQ;
>> +	}
>> +
>>   	/* set the length as message for rx txn */
>> -	peripheral->rx_len = msg->len;
>> +	peripheral->rx_len = msgs[gi2c_gpi_xfer->msg_idx_cnt].len;
>>   	peripheral->op = op;
>>   
>>   	ret = dmaengine_slave_config(dma_chan, config);
>> @@ -560,25 +636,56 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
>>   	else
>>   		dma_dirn = DMA_DEV_TO_MEM;
>>   
>> -	desc = dmaengine_prep_slave_single(dma_chan, addr, msg->len, dma_dirn, flags);
>> +	desc = dmaengine_prep_slave_single(dma_chan, addr,
>> +					   msgs[gi2c_gpi_xfer->msg_idx_cnt].len,
>> +					   dma_dirn, flags);
>>   	if (!desc) {
>>   		dev_err(gi2c->se.dev, "prep_slave_sg failed\n");
>> -		ret = -EIO;
>> +		gi2c->err = -EIO;
>>   		goto err_config;
>>   	}
>>   
>>   	desc->callback_result = i2c_gpi_cb_result;
>>   	desc->callback_param = gi2c;
>>   
>> -	dmaengine_submit(desc);
>> -	*buf = dma_buf;
>> -	*dma_addr_p = addr;
>> +	if (!((msgs[cur_msg_idx].flags & I2C_M_RD) && op == I2C_WRITE)) {
>> +		gi2c_gpi_xfer->msg_idx_cnt++;
>> +		gi2c_gpi_xfer->buf_idx = (cur_msg_idx + 1) % QCOM_GPI_MAX_NUM_MSGS;
>> +	}
>> +	cookie = dmaengine_submit(desc);
>> +	if (dma_submit_error(cookie)) {
>> +		dev_err(gi2c->se.dev,
>> +			"%s: dmaengine_submit failed (%d)\n", __func__, cookie);
>> +		return -EINVAL;
> 
> goto err_config?
yes, updated it.
> 
>> +	}
>>   
>> +	if (gi2c->is_tx_multi_xfer) {
>> +		dma_async_issue_pending(gi2c->tx_c);
>> +		if ((cur_msg_idx == (gi2c->num_msgs - 1)) ||
>> +		    (gi2c_gpi_xfer->msg_idx_cnt >=
>> +		     QCOM_GPI_MAX_NUM_MSGS + gi2c_gpi_xfer->freed_msg_cnt)) {
>> +			ret = gpi_multi_desc_process(gi2c->se.dev, gi2c_gpi_xfer,
>> +						     gi2c->num_msgs, XFER_TIMEOUT,
>> +						     &gi2c->done);
>> +			if (ret) {
>> +				dev_dbg(gi2c->se.dev,
>> +					"I2C multi write msg transfer timeout: %d\n",
>> +					ret);
> 
> if you are returning an error, then print an error.
sure, updated it to error in V2 patch.
> 
>> +				gi2c->err = -ETIMEDOUT;
> 
> gi2c->err = ret?
Yes in this case, ret is -ETIMEDOUT, so updated in V2 patch as 
gi2c->err= ret.
> 
>> +				goto err_config;
>> +			}
>> +		}
>> +	} else {
>> +		/* Non multi descriptor message transfer */
>> +		*buf = dma_buf;
>> +		*dma_addr_p = addr;
>> +	}
>>   	return 0;
>>   
>>   err_config:
>> -	dma_unmap_single(gi2c->se.dev->parent, addr, msg->len, map_dirn);
>> -	i2c_put_dma_safe_msg_buf(dma_buf, msg, false);
>> +	dma_unmap_single(gi2c->se.dev->parent, addr,
>> +			 msgs[cur_msg_idx].len, map_dirn);
>> +	i2c_put_dma_safe_msg_buf(dma_buf, &msgs[cur_msg_idx], false);
>>   	return ret;
> 
> I would have one more label here:
> 
>     out:
> 	gi2c->err = ret;
> 
> 	return ret;
> 
> in order to avoid always assigning twice
Thanks, added new label as out and handled it.
> 
>>   }
>>   
>> @@ -590,6 +697,7 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
>>   	unsigned long time_left;
>>   	dma_addr_t tx_addr, rx_addr;
>>   	void *tx_buf = NULL, *rx_buf = NULL;
>> +	struct gpi_multi_xfer *tx_multi_xfer;
>>   	const struct geni_i2c_clk_fld *itr = gi2c->clk_fld;
>>   
>>   	config.peripheral_config = &peripheral;
>> @@ -603,6 +711,39 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
>>   	peripheral.set_config = 1;
>>   	peripheral.multi_msg = false;
>>   
>> +	gi2c->gpi_config = &peripheral;
>> +	gi2c->num_msgs = num;
>> +	gi2c->is_tx_multi_xfer = false;
>> +	gi2c->tx_irq_cnt = 0;
>> +
>> +	tx_multi_xfer = &peripheral.multi_xfer;
>> +	tx_multi_xfer->msg_idx_cnt = 0;
>> +	tx_multi_xfer->buf_idx = 0;
>> +	tx_multi_xfer->unmap_msg_cnt = 0;
>> +	tx_multi_xfer->freed_msg_cnt = 0;
>> +	tx_multi_xfer->irq_msg_cnt = 0;
>> +	tx_multi_xfer->irq_cnt = 0;
> 
> you can initialize tx_multi_xfer to "{ };" to avoid all these
> " = 0"
Sure, done memset tx_multi_xfer to 0 in V2 patch.
> 
>> +
>> +	/*
>> +	 * If number of write messages are four and higher then
>> +	 * configure hardware for multi descriptor transfers with BEI.
>> +	 */
>> +	if (num >= MIN_NUM_OF_MSGS_MULTI_DESC) {
>> +		gi2c->is_tx_multi_xfer = true;
>> +		for (i = 0; i < num; i++) {
>> +			if (msgs[i].flags & I2C_M_RD) {
>> +				/*
>> +				 * Multi descriptor transfer with BEI
>> +				 * support is enabled for write transfers.
>> +				 * Add BEI optimization support for read
>> +				 * transfers later.
>> +				 */
>> +				gi2c->is_tx_multi_xfer = false;
>> +				break;
>> +			}
>> +		}
>> +	}
>> +
>>   	for (i = 0; i < num; i++) {
>>   		gi2c->cur = &msgs[i];
>>   		gi2c->err = 0;
>> @@ -613,14 +754,16 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
>>   			peripheral.stretch = 1;
>>   
>>   		peripheral.addr = msgs[i].addr;
>> +		if (i > 0 && (!(msgs[i].flags & I2C_M_RD)))
>> +			peripheral.multi_msg = false;
>>   
>> -		ret =  geni_i2c_gpi(gi2c, &msgs[i], &config,
>> +		ret =  geni_i2c_gpi(gi2c, msgs, i, &config,
> 
> what is the point of passing 'i' if you always refer to msgs[i]
> in geni_i2c_gpi()?
Handled with new variable in "geni_i2c_gpi "and so no need to pass 
current i2c msg index, removed it in V2 patch.
> 
>>   				    &tx_addr, &tx_buf, I2C_WRITE, gi2c->tx_c);
>>   		if (ret)
>>   			goto err;
>>   
>>   		if (msgs[i].flags & I2C_M_RD) {
>> -			ret =  geni_i2c_gpi(gi2c, &msgs[i], &config,
>> +			ret =  geni_i2c_gpi(gi2c, msgs, i, &config,
>>   					    &rx_addr, &rx_buf, I2C_READ, gi2c->rx_c);
>>   			if (ret)
>>   				goto err;
>> @@ -628,18 +771,28 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
>>   			dma_async_issue_pending(gi2c->rx_c);
>>   		}
>>   
>> -		dma_async_issue_pending(gi2c->tx_c);
>> -
>> -		time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
>> -		if (!time_left)
>> -			gi2c->err = -ETIMEDOUT;
>> +		if (!gi2c->is_tx_multi_xfer) {
>> +			dma_async_issue_pending(gi2c->tx_c);
>> +			time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
>> +			if (!time_left) {
>> +				dev_err(gi2c->se.dev, "%s:I2C timeout\n", __func__);
>> +				gi2c->err = -ETIMEDOUT;
>> +			}
>> +		}
>>   
>>   		if (gi2c->err) {
>>   			ret = gi2c->err;
>>   			goto err;
>>   		}
>>   
>> -		geni_i2c_gpi_unmap(gi2c, &msgs[i], tx_buf, tx_addr, rx_buf, rx_addr);
>> +		if (!gi2c->is_tx_multi_xfer) {
>> +			geni_i2c_gpi_unmap(gi2c, &msgs[i], tx_buf, tx_addr, rx_buf, rx_addr);
>> +		} else {
>> +			if (gi2c->tx_irq_cnt != tx_multi_xfer->irq_cnt) {
> 
>     else if (...) {
>     	...
>     }
Sure, else if used here in V2 patch.
> 
> Andi

Regards,
JyothiKumar

Re: [PATCH v1 5/5] i2c: i2c-qcom-geni: Add Block event interrupt support

[Jyothi Kumar Seerapu]

On 10/19/2024 8:42 AM, kernel test robot wrote:
> Hi Jyothi,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on 55bcd2e0d04c1171d382badef1def1fd04ef66c5]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Jyothi-Kumar-Seerapu/dt-bindings-dmaengine-qcom-gpi-Add-additional-arg-to-dma-cell-property/20241015-202637
> base:   55bcd2e0d04c1171d382badef1def1fd04ef66c5
> patch link:    https://lore.kernel.org/r/20241015120750.21217-6-quic_jseerapu%40quicinc.com
> patch subject: [PATCH v1 5/5] i2c: i2c-qcom-geni: Add Block event interrupt support
> config: i386-allmodconfig (https://download.01.org/0day-ci/archive/20241019/202410191055.bi1pWTAY-lkp@intel.com/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241019/202410191055.bi1pWTAY-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202410191055.bi1pWTAY-lkp@intel.com/
> 
> All errors (new ones prefixed by >>, old ones prefixed by <<):
> 
>>> ERROR: modpost: "gpi_multi_desc_process" [drivers/i2c/busses/i2c-qcom-geni.ko] undefined!
> 
> Kconfig warnings: (for reference only)
>     WARNING: unmet direct dependencies detected for GET_FREE_REGION
>     Depends on [n]: SPARSEMEM [=n]
>     Selected by [m]:
>     - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
>     WARNING: unmet direct dependencies detected for OMAP2PLUS_MBOX
>     Depends on [n]: MAILBOX [=y] && (ARCH_OMAP2PLUS || ARCH_K3)
>     Selected by [m]:
>     - TI_K3_M4_REMOTEPROC [=m] && REMOTEPROC [=y] && (ARCH_K3 || COMPILE_TEST [=y])
> 

Fixed the reported issue in V2 patch.

Regards,
JyothiKumar

Re: [PATCH v1 5/5] i2c: i2c-qcom-geni: Add Block event interrupt support

[Jyothi Kumar Seerapu]

On 10/19/2024 3:41 AM, kernel test robot wrote:
> Hi Jyothi,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on 55bcd2e0d04c1171d382badef1def1fd04ef66c5]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Jyothi-Kumar-Seerapu/dt-bindings-dmaengine-qcom-gpi-Add-additional-arg-to-dma-cell-property/20241015-202637
> base:   55bcd2e0d04c1171d382badef1def1fd04ef66c5
> patch link:    https://lore.kernel.org/r/20241015120750.21217-6-quic_jseerapu%40quicinc.com
> patch subject: [PATCH v1 5/5] i2c: i2c-qcom-geni: Add Block event interrupt support
> config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20241019/202410190549.hGAfByqg-lkp@intel.com/config)
> compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241019/202410190549.hGAfByqg-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202410190549.hGAfByqg-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>>> drivers/i2c/busses/i2c-qcom-geni.c:562:8: error: incompatible pointer to integer conversion passing 'dma_addr_t *' (aka 'unsigned long long *') to parameter of type 'dma_addr_t' (aka 'unsigned long long'); dereference with * [-Wint-conversion]
>       562 |                                    tx_multi_xfer->dma_addr[wr_idx], NULL, NULL);
>           |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>           |                                    *
>     drivers/i2c/busses/i2c-qcom-geni.c:519:36: note: passing argument to parameter 'tx_addr' here
>       519 |                                void *tx_buf, dma_addr_t tx_addr,
>           |                                                         ^
>>> drivers/i2c/busses/i2c-qcom-geni.c:562:47: error: incompatible pointer to integer conversion passing 'void *' to parameter of type 'dma_addr_t' (aka 'unsigned long long') [-Wint-conversion]
>       562 |                                    tx_multi_xfer->dma_addr[wr_idx], NULL, NULL);
>           |                                                                           ^~~~
>     include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
>         8 | #define NULL ((void *)0)
>           |              ^~~~~~~~~~~
>     drivers/i2c/busses/i2c-qcom-geni.c:520:36: note: passing argument to parameter 'rx_addr' here
>       520 |                                void *rx_buf, dma_addr_t rx_addr)
>           |                                                         ^
>>> drivers/i2c/busses/i2c-qcom-geni.c:586:7: error: incompatible pointer to integer conversion assigning to 'dma_addr_t' (aka 'unsigned long long') from 'dma_addr_t *' (aka 'unsigned long long *'); dereference with * [-Wint-conversion]
>       586 |         addr = gi2c_gpi_xfer->dma_addr[gi2c_gpi_xfer->buf_idx];
>           |              ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>           |                *
>     3 errors generated.

Fixed the reported issues which are comiltation warnings in V2 patch.

> 
> Kconfig warnings: (for reference only)
>     WARNING: unmet direct dependencies detected for OMAP2PLUS_MBOX
>     Depends on [n]: MAILBOX [=y] && (ARCH_OMAP2PLUS || ARCH_K3)
>     Selected by [y]:
>     - TI_K3_M4_REMOTEPROC [=y] && REMOTEPROC [=y] && (ARCH_K3 || COMPILE_TEST [=y])
> 
> 
> vim +562 drivers/i2c/busses/i2c-qcom-geni.c
> 
>     532	
>     533	/**
>     534	 * gpi_i2c_multi_desc_unmap() - unmaps the buffers post multi message TX transfers
>     535	 * @dev: pointer to the corresponding dev node
>     536	 * @gi2c: i2c dev handle
>     537	 * @msgs: i2c messages array
>     538	 * @peripheral: pointer to the gpi_i2c_config
>     539	 */
>     540	static void gpi_i2c_multi_desc_unmap(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[],
>     541					     struct gpi_i2c_config *peripheral)
>     542	{
>     543		u32 msg_xfer_cnt, wr_idx = 0;
>     544		struct gpi_multi_xfer *tx_multi_xfer = &peripheral->multi_xfer;
>     545	
>     546		/*
>     547		 * In error case, need to unmap all messages based on the msg_idx_cnt.
>     548		 * Non-error case unmap all the processed messages.
>     549		 */
>     550		if (gi2c->err)
>     551			msg_xfer_cnt = tx_multi_xfer->msg_idx_cnt;
>     552		else
>     553			msg_xfer_cnt = tx_multi_xfer->irq_cnt * NUM_MSGS_PER_IRQ;
>     554	
>     555		/* Unmap the processed DMA buffers based on the received interrupt count */
>     556		for (; tx_multi_xfer->unmap_msg_cnt < msg_xfer_cnt; tx_multi_xfer->unmap_msg_cnt++) {
>     557			if (tx_multi_xfer->unmap_msg_cnt == gi2c->num_msgs)
>     558				break;
>     559			wr_idx = tx_multi_xfer->unmap_msg_cnt % QCOM_GPI_MAX_NUM_MSGS;
>     560			geni_i2c_gpi_unmap(gi2c, &msgs[tx_multi_xfer->unmap_msg_cnt],
>     561					   tx_multi_xfer->dma_buf[wr_idx],
>   > 562					   tx_multi_xfer->dma_addr[wr_idx], NULL, NULL);
>     563			tx_multi_xfer->freed_msg_cnt++;
>     564		}
>     565	}
>     566	
>     567	static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], int cur_msg_idx,
>     568				struct dma_slave_config *config, dma_addr_t *dma_addr_p,
>     569				void **buf, unsigned int op, struct dma_chan *dma_chan)
>     570	{
>     571		struct gpi_i2c_config *peripheral;
>     572		unsigned int flags;
>     573		void *dma_buf;
>     574		dma_addr_t addr;
>     575		enum dma_data_direction map_dirn;
>     576		enum dma_transfer_direction dma_dirn;
>     577		struct dma_async_tx_descriptor *desc;
>     578		int ret;
>     579		struct gpi_multi_xfer *gi2c_gpi_xfer;
>     580		dma_cookie_t cookie;
>     581	
>     582		peripheral = config->peripheral_config;
>     583		gi2c_gpi_xfer = &peripheral->multi_xfer;
>     584		gi2c_gpi_xfer->msg_idx_cnt = cur_msg_idx;
>     585		dma_buf = gi2c_gpi_xfer->dma_buf[gi2c_gpi_xfer->buf_idx];
>   > 586		addr = gi2c_gpi_xfer->dma_addr[gi2c_gpi_xfer->buf_idx];
>     587	
>     588		dma_buf = i2c_get_dma_safe_msg_buf(&msgs[gi2c_gpi_xfer->msg_idx_cnt], 1);
>     589		if (!dma_buf) {
>     590			gi2c->err = -ENOMEM;
>     591			return -ENOMEM;
>     592		}
>     593	
>     594		if (op == I2C_WRITE)
>     595			map_dirn = DMA_TO_DEVICE;
>     596		else
>     597			map_dirn = DMA_FROM_DEVICE;
>     598	
>     599		addr = dma_map_single(gi2c->se.dev->parent,
>     600				      dma_buf, msgs[gi2c_gpi_xfer->msg_idx_cnt].len,
>     601				      map_dirn);
>     602		if (dma_mapping_error(gi2c->se.dev->parent, addr)) {
>     603			i2c_put_dma_safe_msg_buf(dma_buf, &msgs[gi2c_gpi_xfer->msg_idx_cnt],
>     604						 false);
>     605			gi2c->err = -ENOMEM;
>     606			return -ENOMEM;
>     607		}
>     608	
>     609		if (gi2c->is_tx_multi_xfer) {
>     610			if (((gi2c_gpi_xfer->msg_idx_cnt + 1) % NUM_MSGS_PER_IRQ))
>     611				peripheral->flags |= QCOM_GPI_BLOCK_EVENT_IRQ;
>     612			else
>     613				peripheral->flags &= ~QCOM_GPI_BLOCK_EVENT_IRQ;
>     614	
>     615			/* BEI bit to be cleared for last TRE */
>     616			if (gi2c_gpi_xfer->msg_idx_cnt == gi2c->num_msgs - 1)
>     617				peripheral->flags &= ~QCOM_GPI_BLOCK_EVENT_IRQ;
>     618		}
>     619	
>     620		/* set the length as message for rx txn */
>     621		peripheral->rx_len = msgs[gi2c_gpi_xfer->msg_idx_cnt].len;
>     622		peripheral->op = op;
>     623	
>     624		ret = dmaengine_slave_config(dma_chan, config);
>     625		if (ret) {
>     626			dev_err(gi2c->se.dev, "dma config error: %d for op:%d\n", ret, op);
>     627			goto err_config;
>     628		}
>     629	
>     630		peripheral->set_config = 0;
>     631		peripheral->multi_msg = true;
>     632		flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
>     633	
>     634		if (op == I2C_WRITE)
>     635			dma_dirn = DMA_MEM_TO_DEV;
>     636		else
>     637			dma_dirn = DMA_DEV_TO_MEM;
>     638	
>     639		desc = dmaengine_prep_slave_single(dma_chan, addr,
>     640						   msgs[gi2c_gpi_xfer->msg_idx_cnt].len,
>     641						   dma_dirn, flags);
>     642		if (!desc) {
>     643			dev_err(gi2c->se.dev, "prep_slave_sg failed\n");
>     644			gi2c->err = -EIO;
>     645			goto err_config;
>     646		}
>     647	
>     648		desc->callback_result = i2c_gpi_cb_result;
>     649		desc->callback_param = gi2c;
>     650	
>     651		if (!((msgs[cur_msg_idx].flags & I2C_M_RD) && op == I2C_WRITE)) {
>     652			gi2c_gpi_xfer->msg_idx_cnt++;
>     653			gi2c_gpi_xfer->buf_idx = (cur_msg_idx + 1) % QCOM_GPI_MAX_NUM_MSGS;
>     654		}
>     655		cookie = dmaengine_submit(desc);
>     656		if (dma_submit_error(cookie)) {
>     657			dev_err(gi2c->se.dev,
>     658				"%s: dmaengine_submit failed (%d)\n", __func__, cookie);
>     659			return -EINVAL;
>     660		}
>     661	
>     662		if (gi2c->is_tx_multi_xfer) {
>     663			dma_async_issue_pending(gi2c->tx_c);
>     664			if ((cur_msg_idx == (gi2c->num_msgs - 1)) ||
>     665			    (gi2c_gpi_xfer->msg_idx_cnt >=
>     666			     QCOM_GPI_MAX_NUM_MSGS + gi2c_gpi_xfer->freed_msg_cnt)) {
>     667				ret = gpi_multi_desc_process(gi2c->se.dev, gi2c_gpi_xfer,
>     668							     gi2c->num_msgs, XFER_TIMEOUT,
>     669							     &gi2c->done);
>     670				if (ret) {
>     671					dev_dbg(gi2c->se.dev,
>     672						"I2C multi write msg transfer timeout: %d\n",
>     673						ret);
>     674					gi2c->err = -ETIMEDOUT;
>     675					goto err_config;
>     676				}
>     677			}
>     678		} else {
>     679			/* Non multi descriptor message transfer */
>     680			*buf = dma_buf;
>     681			*dma_addr_p = addr;
>     682		}
>     683		return 0;
>     684	
>     685	err_config:
>     686		dma_unmap_single(gi2c->se.dev->parent, addr,
>     687				 msgs[cur_msg_idx].len, map_dirn);
>     688		i2c_put_dma_safe_msg_buf(dma_buf, &msgs[cur_msg_idx], false);
>     689		return ret;
>     690	}
>     691	
> 

Re: [PATCH v1 3/5] dmaengine: qcom: gpi: Add provision to support TRE size as the fourth argument of dma-cells property

[Jyothi Kumar Seerapu]

On 10/25/2024 11:47 PM, Konrad Dybcio wrote:
> On 15.10.2024 2:07 PM, Jyothi Kumar Seerapu wrote:
>> The current GPI driver hardcodes the channel TRE (Transfer Ring Element)
>> size to 64. For scenarios requiring high performance with multiple
>> messages in a transfer, use Block Event Interrupt (BEI).
>> This method triggers interrupt after specific message transfers and
>> the last message transfer, effectively reducing the number of interrupts.
>> For multiple transfers utilizing BEI, a channel TRE size of 64 is
>> insufficient and may lead to transfer failures, indicated by errors
>> related to unavailable memory space.
>>
>> Added provision to modify the channel TRE size via the device tree.
>> The Default channel TRE size is set to 64, but this value can update
>> in the device tree which will then be parsed by the GPI driver.
>>
>> Signed-off-by: Jyothi Kumar Seerapu <quic_jseerapu@quicinc.com>
>> ---
> 
> 1. Is the total memory pool for these shared?
Total memory we need preallocate and so for each serial engine this 
mentioned channel TRE size be used for config, go, dma tres preparation.
> 
> 2. Is there any scenario where we want TRE size to be lower and
>     not higher? Are there any drawbacks to always keeping them at
>     SOME_MAX_VALUE?
We are keeping minimum channel tre size to 64 to make sure that enough 
size is present to handle the requested transfers.
> 
> 3. Is this something we should configure at boot time (in firmware)?
>     Perhaps this could be decided based on client device settings (which
>     may or may not require adding some field in the i2c framework)
> 
This memory is for software usecase and preallocated prior to GPI driver 
allocated this memory to channels and events handling.

I have reverted the changes related to adding new argument for dma-cells 
property and instead used existing value for channel TRE size in GPI 
driver, which is 64.

> 
> Konrad