[PATCH v3 0/3] QCOM QUP I2C - Add noise rejection, convert to YAML

This patch series converts the i2c-qup bindings to YAML and then
adds support for noise rejection, which is needed for some noisy
hardware, like the touchscreen on the F(x)Tec Pro1.
After adding noise rejection, the touchscreen stopped showing
ghost touch issues and lockups.

Tested on F(x)Tec Pro1 (MSM8998).

Changes in v3, as per Bjorn's review:
 - Moved bitfield.h to maintain sort order
 - Removed useless variable initialization
 - Removed linebreak

Changes in v2:
 - Fixed ARM (32) build error (added bitfield.h inclusion)


AngeloGioacchino Del Regno (3):
  dt-bindings: i2c: qcom,i2c-qup: Convert txt to YAML schema
  i2c: qup: Introduce SCL/SDA noise rejection
  dt-bindings: i2c: qcom,i2c-qup: Document noise rejection properties

 .../devicetree/bindings/i2c/qcom,i2c-qup.txt  |  40 -------
 .../devicetree/bindings/i2c/qcom,i2c-qup.yaml | 101 ++++++++++++++++++
 drivers/i2c/busses/i2c-qup.c                  |  15 +++
 3 files changed, 116 insertions(+), 40 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
 create mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml

-- 
2.29.2

Document the new noise rejection properties "qcom,noise-reject-sda"
and "qcom,noise-reject-scl".

Signed-off-by: AngeloGioacchino Del Regno <redacted>
---
 .../devicetree/bindings/i2c/qcom,i2c-qup.yaml      | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml
index c5c7db3ac2a6..3f14dd65c6b9 100644
--- a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml
+++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml

@@ -58,6 +58,20 @@ properties:
   '#size-cells':
     const: 0
 
+  qcom,noise-reject-sda:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Noise rejection level for the SDA line.
+    minimum: 0
+    maximum: 3
+    default: 0
+
+  qcom,noise-reject-scl:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Noise rejection level for the SCL line.
+    minimum: 0
+    maximum: 3
+    default: 0
+
 required:
   - compatible
   - clocks

-- 
2.29.2

On Thu, 14 Jan 2021 19:04:15 +0100, AngeloGioacchino Del Regno wrote:

Document the new noise rejection properties "qcom,noise-reject-sda"
and "qcom,noise-reject-scl".

Signed-off-by: AngeloGioacchino Del Regno <redacted>
---
 .../devicetree/bindings/i2c/qcom,i2c-qup.yaml      | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

+  qcom,noise-reject-sda:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Noise rejection level for the SDA line.
+    minimum: 0
+    maximum: 3
+    default: 0

What does this u32 describe? I wonder if we can introduce a generic
property instead of a vendor-specific one.

Il 28/01/21 10:13, Wolfram Sang ha scritto:

quoted

+  qcom,noise-reject-sda:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Noise rejection level for the SDA line.
+    minimum: 0
+    maximum: 3
+    default: 0

What does this u32 describe? I wonder if we can introduce a generic
property instead of a vendor-specific one.

Describes a "noise rejection level"... zero is disabled, 1-2-3 are
min-med-high. That's what the hardware accepts.

Also, there's no "real" documentation on this, so it wasn't possible
to map it to any real measurement unit as well...

Convert the qcom,i2c-qup binding to YAML schema.

Signed-off-by: AngeloGioacchino Del Regno <redacted>
---
 .../devicetree/bindings/i2c/qcom,i2c-qup.txt  | 40 ---------
 .../devicetree/bindings/i2c/qcom,i2c-qup.yaml | 87 +++++++++++++++++++
 2 files changed, 87 insertions(+), 40 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
 create mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml

diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
deleted file mode 100644
index dc71754a56af..000000000000
--- a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
+++ /dev/null

@@ -1,40 +0,0 @@
-Qualcomm Universal Peripheral (QUP) I2C controller
-
-Required properties:
- - compatible: Should be:
-   * "qcom,i2c-qup-v1.1.1" for 8660, 8960 and 8064.
-   * "qcom,i2c-qup-v2.1.1" for 8974 v1.
-   * "qcom,i2c-qup-v2.2.1" for 8974 v2 and later.
- - reg: Should contain QUP register address and length.
- - interrupts: Should contain I2C interrupt.
-
- - clocks: A list of phandles + clock-specifiers, one for each entry in
-   clock-names.
- - clock-names: Should contain:
-   * "core" for the core clock
-   * "iface" for the AHB clock
-
- - #address-cells: Should be <1> Address cells for i2c device address
- - #size-cells: Should be <0> as i2c addresses have no size component
-
-Optional properties:
- - clock-frequency: Should specify the desired i2c bus clock frequency in Hz,
-                    defaults to 100kHz if omitted.
-
-Child nodes should conform to i2c bus binding.
-
-Example:
-
- i2c@f9924000 {
- 	compatible = "qcom,i2c-qup-v2.2.1";
- 	reg = <0xf9924000 0x1000>;
- 	interrupts = <0 96 0>;
-
- 	clocks = <&gcc GCC_BLSP1_QUP2_I2C_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>;
- 	clock-names = "core", "iface";
-
- 	clock-frequency = <355000>;
-
- 	#address-cells = <1>;
- 	#size-cells = <0>;
- };

diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml
new file mode 100644
index 000000000000..c5c7db3ac2a6
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-qup.yaml

@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/i2c/qcom,i2c-qup.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Universal Peripheral (QUP) I2C controller
+
+maintainers:
+  - Andy Gross <agross@kernel.org>
+  - Bjorn Andersson <bjorn.andersson@linaro.org>
+
+description: Binding for Qualcomm "QUP" I2C controllers
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - qcom,i2c-qup-v1.1.1
+      - qcom,i2c-qup-v2.1.1
+      - qcom,i2c-qup-v2.2.1
+
+  reg:
+    items:
+      - description: QUP I2C register iospace
+
+  clocks:
+    items:
+      - description: Core QUP I2C clock
+      - description: AHB clock
+
+  clock-names:
+    items:
+      - const: core
+      - const: iface
+
+  clock-frequency:
+    minimum: 100000
+    maximum: 1000000
+    default: 100000
+
+  dmas:
+    items:
+      - description: RX DMA Channel phandle
+      - description: TX DMA Channel phandle
+
+  dma-names:
+    items:
+      - const: rx
+      - const: tx
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+required:
+  - compatible
+  - clocks
+  - clock-names
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-msm8998.h>
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    i2c@c175000 {
+        compatible = "qcom,i2c-qup-v2.2.1";
+        reg = <0x0c175000 0x600>;
+        interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&gcc GCC_BLSP1_QUP1_I2C_APPS_CLK>,
+                 <&gcc GCC_BLSP1_AHB_CLK>;
+        clock-names = "core", "iface";
+        clock-frequency = <400000>;
+        dmas = <&blsp_dma 4>, <&blsp_dma 5>;
+        dma-names = "rx", "tx";
+        #address-cells = <1>;
+        #size-cells = <0>;
+    };

-- 
2.29.2

On Thu, Jan 14, 2021 at 07:04:13PM +0100, AngeloGioacchino Del Regno wrote:

Just 'maxItems: 1' is sufficient for a single entry.

+
+  clocks:
+    items:
+      - description: Core QUP I2C clock
+      - description: AHB clock
+
+  clock-names:
+    items:
+      - const: core
+      - const: iface
+
+  clock-frequency:
+    minimum: 100000
+    maximum: 1000000
+    default: 100000
+
+  dmas:
+    items:
+      - description: RX DMA Channel phandle
+      - description: TX DMA Channel phandle
+
+  dma-names:
+    items:
+      - const: rx
+      - const: tx
+

+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0

Can drop these. Covered by i2c-controller.yaml

+
+required:
+  - compatible
+  - clocks
+  - clock-names
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-msm8998.h>
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    i2c@c175000 {
+        compatible = "qcom,i2c-qup-v2.2.1";
+        reg = <0x0c175000 0x600>;
+        interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&gcc GCC_BLSP1_QUP1_I2C_APPS_CLK>,
+                 <&gcc GCC_BLSP1_AHB_CLK>;
+        clock-names = "core", "iface";
+        clock-frequency = <400000>;
+        dmas = <&blsp_dma 4>, <&blsp_dma 5>;
+        dma-names = "rx", "tx";
+        #address-cells = <1>;
+        #size-cells = <0>;
+    };
-- 
2.29.2

Hi Angelo,

On Mon, Jan 25, 2021 at 04:16:33PM -0600, Rob Herring wrote:

On Thu, Jan 14, 2021 at 07:04:13PM +0100, AngeloGioacchino Del Regno wrote:

quoted

Convert the qcom,i2c-qup binding to YAML schema.

Signed-off-by: AngeloGioacchino Del Regno <redacted>

Do you plan to update this accordings to Rob's comments? It would be a
shame to let this go to waste...

Happy hacking,

   Wolfram

Some I2C devices may be glitchy due to electrical noise coming
from the device itself or because of possible board design issues.
To overcome this issue, the QUP's I2C in Qualcomm SoCs supports
a noise rejection setting for both SCL and SDA lines.

Introduce a setting for noise rejection through device properties,
"qcom,noise-reject-sda" and "qcom,noise-reject-scl", which will
be used to set the level of noise rejection sensitivity.
If the properties are not specified, noise rejection will not be
enabled.

Signed-off-by: AngeloGioacchino Del Regno <redacted>
---
 drivers/i2c/busses/i2c-qup.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index 5a47915869ae..a01f6f32ba67 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c

@@ -7,6 +7,7 @@
 
 #include <linux/acpi.h>
 #include <linux/atomic.h>
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/dmaengine.h>

@@ -39,6 +40,8 @@
 #define QUP_MX_READ_CNT		0x208
 #define QUP_IN_FIFO_BASE	0x218
 #define QUP_I2C_CLK_CTL		0x400
+#define  QUP_I2C_CLK_CTL_SDA_NR	GENMASK(27, 26)
+#define  QUP_I2C_CLK_CTL_SCL_NR	GENMASK(25, 24)
 #define QUP_I2C_STATUS		0x404
 #define QUP_I2C_MASTER_GEN	0x408
 

@@ -1663,6 +1666,7 @@ static int qup_i2c_probe(struct platform_device *pdev)
 	int ret, fs_div, hs_div;
 	u32 src_clk_freq = DEFAULT_SRC_CLK;
 	u32 clk_freq = DEFAULT_CLK_FREQ;
+	u32 noise_reject_scl, noise_reject_sda;
 	int blocks;
 	bool is_qup_v1;
 

@@ -1860,6 +1864,17 @@ static int qup_i2c_probe(struct platform_device *pdev)
 		qup->clk_ctl = ((fs_div / 2) << 16) | (hs_div << 8) | (fs_div & 0xff);
 	}
 
+	/* SCL/SDA Noise rejection (optional) */
+	ret = device_property_read_u32(qup->dev, "qcom,noise-reject-scl",
+				      &noise_reject_scl);
+	if (ret == 0)
+		qup->clk_ctl |= FIELD_PREP(QUP_I2C_CLK_CTL_SCL_NR, noise_reject_scl);
+
+	ret = device_property_read_u32(qup->dev, "qcom,noise-reject-sda",
+				      &noise_reject_sda);
+	if (ret == 0)
+		qup->clk_ctl |= FIELD_PREP(QUP_I2C_CLK_CTL_SDA_NR, noise_reject_sda);
+
 	/*
 	 * Time it takes for a byte to be clocked out on the bus.
 	 * Each byte takes 9 clock cycles (8 bits + 1 ack).

-- 
2.29.2