[PATCH 1/6] scripts/dtc: check: Add additional i2c reg flags support

[PATCH 1/6] scripts/dtc: check: Add additional i2c reg flags support

[Sergey.Semin]

From: Serge Semin <Sergey.Semin@baikalelectronics.ru>

Recently the I2C-controllers slave interface support was added to the
kernel I2C subsystem. In this case Linux can be used as, for example,
a I2C EEPROM machine. See [1] for details. Other than instantiating
the EEPROM-slave device from user-space there is a way to declare the
device in dts. In this case firstly the I2C bus controller must support
the slave interface. Secondly I2C-slave sub-node of that controller
must have "reg"-property with flag I2C_OWN_SLAVE_ADDRESS set (flag is
declared in [2]). That flag is declared as (1 << 30), which when set
makes dtc unhappy about too big address set for a I2C-slave:

Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64: I2C bus unit address format error, expected "40000064"
Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64:reg: I2C address must be less than 10-bits, got "0x40000064"

Similar problem would have happened if we had set the 10-bit address
flag I2C_TEN_BIT_ADDRESS in the "reg"-property.

In order to fix the problem we suggest to alter the I2C-bus reg-check
algorithm, so one would be aware of the upper bits set. Normally if no
flag specified, the 7-bit address is expected in the "reg"-property.
If I2C_TEN_BIT_ADDRESS is set, then the 10-bit address check will be
performed. The I2C_OWN_SLAVE_ADDRESS flag will be just ignored.

[1] Documentation/i2c/slave-interface.rst
[2] include/dt-bindings/i2c/i2c.h

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
---
 scripts/dtc/checks.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/scripts/dtc/checks.c b/scripts/dtc/checks.c
index 756f0fa9203f..01c1ad895e6d 100644
--- a/scripts/dtc/checks.c
+++ b/scripts/dtc/checks.c
@@ -1025,6 +1025,7 @@ static void check_i2c_bus_reg(struct check *c, struct dt_info *dti, struct node
 	const char *unitname = get_unitname(node);
 	char unit_addr[17];
 	uint32_t reg = 0;
+	uint32_t addr;
 	int len;
 	cell_t *cells = NULL;
 
@@ -1041,17 +1042,21 @@ static void check_i2c_bus_reg(struct check *c, struct dt_info *dti, struct node
 	}
 
 	reg = fdt32_to_cpu(*cells);
-	snprintf(unit_addr, sizeof(unit_addr), "%x", reg);
+	addr = reg & 0x3FFFFFFFU;
+	snprintf(unit_addr, sizeof(unit_addr), "%x", addr);
 	if (!streq(unitname, unit_addr))
 		FAIL(c, dti, node, "I2C bus unit address format error, expected \"%s\"",
 		     unit_addr);
 
 	for (len = prop->val.len; len > 0; len -= 4) {
 		reg = fdt32_to_cpu(*(cells++));
-		if (reg > 0x3ff)
+		addr = reg & 0x3FFFFFFFU;
+		if ((reg & (1 << 31)) && addr > 0x3ff)
 			FAIL_PROP(c, dti, node, prop, "I2C address must be less than 10-bits, got \"0x%x\"",
-				  reg);
-
+				  addr);
+		else if (!(reg & (1 << 31)) && addr > 0x7f)
+			FAIL_PROP(c, dti, node, prop, "I2C address must be less than 7-bits, got \"0x%x\"",
+				  addr);
 	}
 }
 WARNING(i2c_bus_reg, check_i2c_bus_reg, NULL, &reg_format, &i2c_bus_bridge);
-- 
2.25.1

[PATCH 2/6] dt-bindings: i2c: Replace DW I2C legacy bindings with YAML-based one

[Sergey.Semin]

From: Serge Semin <Sergey.Semin@baikalelectronics.ru>

Modern device tree bindings are supposed to be created as YAML-files
in accordance with dt-schema. This commit replaces Synopsys DW I2C
legacy bare text bindings with YAML file. As before the bindings file
states that the corresponding dts node is supposed to be compatible
either with generic DW I2C controller or with Microsemi Ocelot SoC I2C
one, to have registers, interrupts and clocks properties. In addition
the node may have clock-frequency, i2c-sda-hold-time-ns,
i2c-scl-falling-time-ns and i2c-sda-falling-time-ns optional properties.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
---
 .../bindings/i2c/i2c-designware.txt           |  73 --------
 .../bindings/i2c/snps,designware-i2c.yaml     | 156 ++++++++++++++++++
 2 files changed, 156 insertions(+), 73 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-designware.txt
 create mode 100644 Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml

diff --git a/Documentation/devicetree/bindings/i2c/i2c-designware.txt b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
deleted file mode 100644
index 08be4d3846e5..000000000000
--- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
+++ /dev/null
@@ -1,73 +0,0 @@
-* Synopsys DesignWare I2C
-
-Required properties :
-
- - compatible : should be "snps,designware-i2c"
-                or "mscc,ocelot-i2c" with "snps,designware-i2c" for fallback
- - reg : Offset and length of the register set for the device
- - interrupts : <IRQ> where IRQ is the interrupt number.
- - clocks : phandles for the clocks, see the description of clock-names below.
-   The phandle for the "ic_clk" clock is required. The phandle for the "pclk"
-   clock is optional. If a single clock is specified but no clock-name, it is
-   the "ic_clk" clock. If both clocks are listed, the "ic_clk" must be first.
-
-Recommended properties :
-
- - clock-frequency : desired I2C bus clock frequency in Hz.
-
-Optional properties :
-
- - clock-names : Contains the names of the clocks:
-    "ic_clk", for the core clock used to generate the external I2C clock.
-    "pclk", the interface clock, required for register access.
-
- - reg : for "mscc,ocelot-i2c", a second register set to configure the SDA hold
-   time, named ICPU_CFG:TWI_DELAY in the datasheet.
-
- - i2c-sda-hold-time-ns : should contain the SDA hold time in nanoseconds.
-   This option is only supported in hardware blocks version 1.11a or newer and
-   on Microsemi SoCs ("mscc,ocelot-i2c" compatible).
-
- - i2c-scl-falling-time-ns : should contain the SCL falling time in nanoseconds.
-   This value which is by default 300ns is used to compute the tLOW period.
-
- - i2c-sda-falling-time-ns : should contain the SDA falling time in nanoseconds.
-   This value which is by default 300ns is used to compute the tHIGH period.
-
-Examples :
-
-	i2c@f0000 {
-		#address-cells = <1>;
-		#size-cells = <0>;
-		compatible = "snps,designware-i2c";
-		reg = <0xf0000 0x1000>;
-		interrupts = <11>;
-		clock-frequency = <400000>;
-	};
-
-	i2c@1120000 {
-		#address-cells = <1>;
-		#size-cells = <0>;
-		compatible = "snps,designware-i2c";
-		reg = <0x1120000 0x1000>;
-		interrupt-parent = <&ictl>;
-		interrupts = <12 1>;
-		clock-frequency = <400000>;
-		i2c-sda-hold-time-ns = <300>;
-		i2c-sda-falling-time-ns = <300>;
-		i2c-scl-falling-time-ns = <300>;
-	};
-
-	i2c@1120000 {
-		#address-cells = <1>;
-		#size-cells = <0>;
-		reg = <0x2000 0x100>;
-		clock-frequency = <400000>;
-		clocks = <&i2cclk>;
-		interrupts = <0>;
-
-		eeprom@64 {
-			compatible = "linux,slave-24c02";
-			reg = <0x40000064>;
-		};
-	};
diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
new file mode 100644
index 000000000000..3f348f1ce172
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
@@ -0,0 +1,156 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/snps,designware-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare APB I2C Device Tree Bindings
+
+maintainers:
+  - Jarkko Nikula <jarkko.nikula@linux.intel.com>
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+if:
+  properties:
+    compatible:
+      contains:
+        const: mscc,ocelot-i2c
+then:
+  properties:
+    reg:
+      minItems: 1
+      items:
+        - description: DW APB I2C controller memory mapped registers.
+        - description: ICPU_CFG:TWI_DELAY registers to setup the SDA hold time.
+
+properties:
+  compatible:
+    oneOf:
+      - description: Generic Synopsys DesignWare I2C controller.
+        const: snps,designware-i2c
+      - description: Microsemi Ocelot SoCs I2C controller.
+        items:
+          - const: mscc,ocelot-i2c
+          - const: snps,designware-i2c
+
+  reg:
+    items:
+      - description: DW APB I2C controller memory mapped registers.
+
+  "#address-cells": true
+
+  "#size-cells": true
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 1
+    items:
+      - description: I2C controller reference clock source.
+      - description: APB interface clock source.
+
+  clock-names:
+    minItems: 1
+    items:
+      - const: ref
+      - const: pclk
+
+  resets:
+    maxItems: 1
+
+  clock-frequency:
+    description: Desired I2C bus clock frequency in Hz.
+    enum: [100000, 400000, 1000000, 3400000]
+    default: 400000
+
+  i2c-sda-hold-time-ns:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      The property should contain the SDA hold time in nanoseconds. This option
+      is only supported in hardware blocks version 1.11a or newer or on
+      Microsemi SoCs.
+
+  i2c-scl-falling-time-ns:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      The property should contain the SCL falling time in nanoseconds.
+      This value is used to compute the tLOW period.
+    default: 300
+
+  i2c-sda-falling-time-ns:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      The property should contain the SDA falling time in nanoseconds.
+      This value is used to compute the tHIGH period.
+    default: 300
+
+  dmas:
+    items:
+      - description: TX DMA Channel.
+      - description: RX DMA Channel.
+
+  dma-names:
+    items:
+      - const: tx
+      - const: rx
+
+patternProperties:
+  "^.*@[0-9a-f]+$":
+    type: object
+    properties:
+      reg:
+        maxItems: 1
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+  - interrupts
+
+examples:
+  - |
+    i2c@f0000 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      compatible = "snps,designware-i2c";
+      reg = <0xf0000 0x1000>;
+      interrupts = <11>;
+      clock-frequency = <400000>;
+    };
+
+  - |
+    i2c@1120000 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      compatible = "snps,designware-i2c";
+      reg = <0x1120000 0x1000>;
+      interrupt-parent = <&ictl>;
+      interrupts = <12 1>;
+      clock-frequency = <400000>;
+      i2c-sda-hold-time-ns = <300>;
+      i2c-sda-falling-time-ns = <300>;
+      i2c-scl-falling-time-ns = <300>;
+    };
+
+  - |
+    i2c@1120000 {
+      compatible = "snps,designware-i2c";
+      #address-cells = <1>;
+      #size-cells = <0>;
+      reg = <0x2000 0x100>;
+      clock-frequency = <400000>;
+      clocks = <&i2cclk>;
+      interrupts = <0>;
+
+      eeprom@64 {
+        compatible = "linux,slave-24c02";
+        reg = <0x40000064>;
+      };
+    };
+...
-- 
2.25.1

Re: [PATCH 2/6] dt-bindings: i2c: Replace DW I2C legacy bindings with YAML-based one

[Rob Herring]

On Fri, Mar 06, 2020 at 04:19:51PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> Modern device tree bindings are supposed to be created as YAML-files
> in accordance with dt-schema. This commit replaces Synopsys DW I2C
> legacy bare text bindings with YAML file. As before the bindings file
> states that the corresponding dts node is supposed to be compatible
> either with generic DW I2C controller or with Microsemi Ocelot SoC I2C
> one, to have registers, interrupts and clocks properties. In addition
> the node may have clock-frequency, i2c-sda-hold-time-ns,
> i2c-scl-falling-time-ns and i2c-sda-falling-time-ns optional properties.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> ---
>  .../bindings/i2c/i2c-designware.txt           |  73 --------
>  .../bindings/i2c/snps,designware-i2c.yaml     | 156 ++++++++++++++++++
>  2 files changed, 156 insertions(+), 73 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-designware.txt
>  create mode 100644 Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-designware.txt b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> deleted file mode 100644
> index 08be4d3846e5..000000000000
> --- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> +++ /dev/null
> @@ -1,73 +0,0 @@
> -* Synopsys DesignWare I2C
> -
> -Required properties :
> -
> - - compatible : should be "snps,designware-i2c"
> -                or "mscc,ocelot-i2c" with "snps,designware-i2c" for fallback
> - - reg : Offset and length of the register set for the device
> - - interrupts : <IRQ> where IRQ is the interrupt number.
> - - clocks : phandles for the clocks, see the description of clock-names below.
> -   The phandle for the "ic_clk" clock is required. The phandle for the "pclk"
> -   clock is optional. If a single clock is specified but no clock-name, it is
> -   the "ic_clk" clock. If both clocks are listed, the "ic_clk" must be first.
> -
> -Recommended properties :
> -
> - - clock-frequency : desired I2C bus clock frequency in Hz.
> -
> -Optional properties :
> -
> - - clock-names : Contains the names of the clocks:
> -    "ic_clk", for the core clock used to generate the external I2C clock.
> -    "pclk", the interface clock, required for register access.
> -
> - - reg : for "mscc,ocelot-i2c", a second register set to configure the SDA hold
> -   time, named ICPU_CFG:TWI_DELAY in the datasheet.
> -
> - - i2c-sda-hold-time-ns : should contain the SDA hold time in nanoseconds.
> -   This option is only supported in hardware blocks version 1.11a or newer and
> -   on Microsemi SoCs ("mscc,ocelot-i2c" compatible).
> -
> - - i2c-scl-falling-time-ns : should contain the SCL falling time in nanoseconds.
> -   This value which is by default 300ns is used to compute the tLOW period.
> -
> - - i2c-sda-falling-time-ns : should contain the SDA falling time in nanoseconds.
> -   This value which is by default 300ns is used to compute the tHIGH period.
> -
> -Examples :
> -
> -	i2c@f0000 {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -		compatible = "snps,designware-i2c";
> -		reg = <0xf0000 0x1000>;
> -		interrupts = <11>;
> -		clock-frequency = <400000>;
> -	};
> -
> -	i2c@1120000 {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -		compatible = "snps,designware-i2c";
> -		reg = <0x1120000 0x1000>;
> -		interrupt-parent = <&ictl>;
> -		interrupts = <12 1>;
> -		clock-frequency = <400000>;
> -		i2c-sda-hold-time-ns = <300>;
> -		i2c-sda-falling-time-ns = <300>;
> -		i2c-scl-falling-time-ns = <300>;
> -	};
> -
> -	i2c@1120000 {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -		reg = <0x2000 0x100>;
> -		clock-frequency = <400000>;
> -		clocks = <&i2cclk>;
> -		interrupts = <0>;
> -
> -		eeprom@64 {
> -			compatible = "linux,slave-24c02";
> -			reg = <0x40000064>;
> -		};
> -	};
> diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> new file mode 100644
> index 000000000000..3f348f1ce172
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> @@ -0,0 +1,156 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i2c/snps,designware-i2c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synopsys DesignWare APB I2C Device Tree Bindings
> +
> +maintainers:
> +  - Jarkko Nikula <jarkko.nikula@linux.intel.com>
> +
> +allOf:
> +  - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        const: mscc,ocelot-i2c
> +then:
> +  properties:
> +    reg:
> +      minItems: 1
> +      items:
> +        - description: DW APB I2C controller memory mapped registers.
> +        - description: ICPU_CFG:TWI_DELAY registers to setup the SDA hold time.

This won't work (it would be good to have an example that exercises 
this). You need to move this definition to the main 'reg' definition 
below and then do:

if:
  properties:
    compatible:
      not:
        contains:
          const: mscc,ocelot-i2c
then:
  properties:
    reg:
      maxItems: 1

> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - description: Generic Synopsys DesignWare I2C controller.
> +        const: snps,designware-i2c
> +      - description: Microsemi Ocelot SoCs I2C controller.
> +        items:
> +          - const: mscc,ocelot-i2c
> +          - const: snps,designware-i2c
> +
> +  reg:
> +    items:
> +      - description: DW APB I2C controller memory mapped registers.
> +
> +  "#address-cells": true
> +
> +  "#size-cells": true
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 1
> +    items:
> +      - description: I2C controller reference clock source.
> +      - description: APB interface clock source.
> +
> +  clock-names:
> +    minItems: 1
> +    items:
> +      - const: ref
> +      - const: pclk
> +
> +  resets:
> +    maxItems: 1
> +
> +  clock-frequency:
> +    description: Desired I2C bus clock frequency in Hz.
> +    enum: [100000, 400000, 1000000, 3400000]
> +    default: 400000
> +
> +  i2c-sda-hold-time-ns:
> +    $ref: /schemas/types.yaml#/definitions/uint32

Anything with a unit suffix has a type, so you can drop this.

> +    description: |
> +      The property should contain the SDA hold time in nanoseconds. This option
> +      is only supported in hardware blocks version 1.11a or newer or on
> +      Microsemi SoCs.
> +
> +  i2c-scl-falling-time-ns:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      The property should contain the SCL falling time in nanoseconds.
> +      This value is used to compute the tLOW period.
> +    default: 300
> +
> +  i2c-sda-falling-time-ns:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      The property should contain the SDA falling time in nanoseconds.
> +      This value is used to compute the tHIGH period.
> +    default: 300
> +
> +  dmas:
> +    items:
> +      - description: TX DMA Channel.
> +      - description: RX DMA Channel.
> +
> +  dma-names:
> +    items:
> +      - const: tx
> +      - const: rx
> +

> +patternProperties:
> +  "^.*@[0-9a-f]+$":
> +    type: object
> +    properties:
> +      reg:
> +        maxItems: 1

No need for this as i2c-controller.yaml does this.

> +
> +additionalProperties: false

This doesn't work with i2c-controller.yaml. Change it to 
'unevaluatedProperties: false' and eventually that will do what we need.

> +
> +required:
> +  - compatible
> +  - reg
> +  - "#address-cells"
> +  - "#size-cells"
> +  - interrupts
> +
> +examples:
> +  - |
> +    i2c@f0000 {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      compatible = "snps,designware-i2c";
> +      reg = <0xf0000 0x1000>;
> +      interrupts = <11>;
> +      clock-frequency = <400000>;
> +    };
> +
> +  - |
> +    i2c@1120000 {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      compatible = "snps,designware-i2c";
> +      reg = <0x1120000 0x1000>;
> +      interrupt-parent = <&ictl>;
> +      interrupts = <12 1>;
> +      clock-frequency = <400000>;
> +      i2c-sda-hold-time-ns = <300>;
> +      i2c-sda-falling-time-ns = <300>;
> +      i2c-scl-falling-time-ns = <300>;
> +    };
> +
> +  - |
> +    i2c@1120000 {
> +      compatible = "snps,designware-i2c";
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      reg = <0x2000 0x100>;
> +      clock-frequency = <400000>;
> +      clocks = <&i2cclk>;
> +      interrupts = <0>;
> +
> +      eeprom@64 {
> +        compatible = "linux,slave-24c02";
> +        reg = <0x40000064>;
> +      };
> +    };
> +...
> -- 
> 2.25.1
> 

Re: [PATCH 2/6] dt-bindings: i2c: Replace DW I2C legacy bindings with YAML-based one

[Sergey Semin]

On Thu, Mar 12, 2020 at 04:43:08PM -0500, Rob Herring wrote:
> On Fri, Mar 06, 2020 at 04:19:51PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > 
> > Modern device tree bindings are supposed to be created as YAML-files
> > in accordance with dt-schema. This commit replaces Synopsys DW I2C
> > legacy bare text bindings with YAML file. As before the bindings file
> > states that the corresponding dts node is supposed to be compatible
> > either with generic DW I2C controller or with Microsemi Ocelot SoC I2C
> > one, to have registers, interrupts and clocks properties. In addition
> > the node may have clock-frequency, i2c-sda-hold-time-ns,
> > i2c-scl-falling-time-ns and i2c-sda-falling-time-ns optional properties.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: Paul Burton <paulburton@kernel.org>
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > ---
> >  .../bindings/i2c/i2c-designware.txt           |  73 --------
> >  .../bindings/i2c/snps,designware-i2c.yaml     | 156 ++++++++++++++++++
> >  2 files changed, 156 insertions(+), 73 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-designware.txt
> >  create mode 100644 Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-designware.txt b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> > deleted file mode 100644
> > index 08be4d3846e5..000000000000
> > --- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> > +++ /dev/null
> > @@ -1,73 +0,0 @@
> > -* Synopsys DesignWare I2C
> > -
> > -Required properties :
> > -
> > - - compatible : should be "snps,designware-i2c"
> > -                or "mscc,ocelot-i2c" with "snps,designware-i2c" for fallback
> > - - reg : Offset and length of the register set for the device
> > - - interrupts : <IRQ> where IRQ is the interrupt number.
> > - - clocks : phandles for the clocks, see the description of clock-names below.
> > -   The phandle for the "ic_clk" clock is required. The phandle for the "pclk"
> > -   clock is optional. If a single clock is specified but no clock-name, it is
> > -   the "ic_clk" clock. If both clocks are listed, the "ic_clk" must be first.
> > -
> > -Recommended properties :
> > -
> > - - clock-frequency : desired I2C bus clock frequency in Hz.
> > -
> > -Optional properties :
> > -
> > - - clock-names : Contains the names of the clocks:
> > -    "ic_clk", for the core clock used to generate the external I2C clock.
> > -    "pclk", the interface clock, required for register access.
> > -
> > - - reg : for "mscc,ocelot-i2c", a second register set to configure the SDA hold
> > -   time, named ICPU_CFG:TWI_DELAY in the datasheet.
> > -
> > - - i2c-sda-hold-time-ns : should contain the SDA hold time in nanoseconds.
> > -   This option is only supported in hardware blocks version 1.11a or newer and
> > -   on Microsemi SoCs ("mscc,ocelot-i2c" compatible).
> > -
> > - - i2c-scl-falling-time-ns : should contain the SCL falling time in nanoseconds.
> > -   This value which is by default 300ns is used to compute the tLOW period.
> > -
> > - - i2c-sda-falling-time-ns : should contain the SDA falling time in nanoseconds.
> > -   This value which is by default 300ns is used to compute the tHIGH period.
> > -
> > -Examples :
> > -
> > -	i2c@f0000 {
> > -		#address-cells = <1>;
> > -		#size-cells = <0>;
> > -		compatible = "snps,designware-i2c";
> > -		reg = <0xf0000 0x1000>;
> > -		interrupts = <11>;
> > -		clock-frequency = <400000>;
> > -	};
> > -
> > -	i2c@1120000 {
> > -		#address-cells = <1>;
> > -		#size-cells = <0>;
> > -		compatible = "snps,designware-i2c";
> > -		reg = <0x1120000 0x1000>;
> > -		interrupt-parent = <&ictl>;
> > -		interrupts = <12 1>;
> > -		clock-frequency = <400000>;
> > -		i2c-sda-hold-time-ns = <300>;
> > -		i2c-sda-falling-time-ns = <300>;
> > -		i2c-scl-falling-time-ns = <300>;
> > -	};
> > -
> > -	i2c@1120000 {
> > -		#address-cells = <1>;
> > -		#size-cells = <0>;
> > -		reg = <0x2000 0x100>;
> > -		clock-frequency = <400000>;
> > -		clocks = <&i2cclk>;
> > -		interrupts = <0>;
> > -
> > -		eeprom@64 {
> > -			compatible = "linux,slave-24c02";
> > -			reg = <0x40000064>;
> > -		};
> > -	};
> > diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> > new file mode 100644
> > index 000000000000..3f348f1ce172
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
> > @@ -0,0 +1,156 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/i2c/snps,designware-i2c.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Synopsys DesignWare APB I2C Device Tree Bindings
> > +
> > +maintainers:
> > +  - Jarkko Nikula <jarkko.nikula@linux.intel.com>
> > +
> > +allOf:
> > +  - $ref: /schemas/i2c/i2c-controller.yaml#
> > +
> > +if:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        const: mscc,ocelot-i2c
> > +then:
> > +  properties:
> > +    reg:
> > +      minItems: 1
> > +      items:
> > +        - description: DW APB I2C controller memory mapped registers.
> > +        - description: ICPU_CFG:TWI_DELAY registers to setup the SDA hold time.
> 
> This won't work (it would be good to have an example that exercises 
> this). You need to move this definition to the main 'reg' definition 
> below and then do:
> 
> if:
>   properties:
>     compatible:
>       not:
>         contains:
>           const: mscc,ocelot-i2c
> then:
>   properties:
>     reg:
>       maxItems: 1
> 

Hm, I thought I tested this. Apparently I didn't do this well. Thanks for
pointing me out to this problem. I'll add the mscc,ocelot-i2c node to the examples
list to cover the optional two-regs case as well.

> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - description: Generic Synopsys DesignWare I2C controller.
> > +        const: snps,designware-i2c
> > +      - description: Microsemi Ocelot SoCs I2C controller.
> > +        items:
> > +          - const: mscc,ocelot-i2c
> > +          - const: snps,designware-i2c
> > +
> > +  reg:
> > +    items:
> > +      - description: DW APB I2C controller memory mapped registers.
> > +
> > +  "#address-cells": true
> > +
> > +  "#size-cells": true
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    minItems: 1
> > +    items:
> > +      - description: I2C controller reference clock source.
> > +      - description: APB interface clock source.
> > +
> > +  clock-names:
> > +    minItems: 1
> > +    items:
> > +      - const: ref
> > +      - const: pclk
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +  clock-frequency:
> > +    description: Desired I2C bus clock frequency in Hz.
> > +    enum: [100000, 400000, 1000000, 3400000]
> > +    default: 400000
> > +
> > +  i2c-sda-hold-time-ns:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> 
> Anything with a unit suffix has a type, so you can drop this.
> 

Yes, but this and the following time-related properties are supposed to be
single numbers (see ./drivers/i2c/i2c-core-base.c for details), while "^.*-ns$"
is defined as uint32-array. So if I removed this $ref, I would have to add a
constraint like:

+ maxItems: 1
+ items:
+   maxItems: 1

which isn't that obvious, than just explicit uint32-type setting.

> > +    description: |
> > +      The property should contain the SDA hold time in nanoseconds. This option
> > +      is only supported in hardware blocks version 1.11a or newer or on
> > +      Microsemi SoCs.
> > +
> > +  i2c-scl-falling-time-ns:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: |
> > +      The property should contain the SCL falling time in nanoseconds.
> > +      This value is used to compute the tLOW period.
> > +    default: 300
> > +
> > +  i2c-sda-falling-time-ns:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: |
> > +      The property should contain the SDA falling time in nanoseconds.
> > +      This value is used to compute the tHIGH period.
> > +    default: 300
> > +
> > +  dmas:
> > +    items:
> > +      - description: TX DMA Channel.
> > +      - description: RX DMA Channel.
> > +
> > +  dma-names:
> > +    items:
> > +      - const: tx
> > +      - const: rx
> > +
> 
> > +patternProperties:
> > +  "^.*@[0-9a-f]+$":
> > +    type: object
> > +    properties:
> > +      reg:
> > +        maxItems: 1
> 
> No need for this as i2c-controller.yaml does this.
> 

Agreed.

> > +
> > +additionalProperties: false
> 
> This doesn't work with i2c-controller.yaml. Change it to 
> 'unevaluatedProperties: false' and eventually that will do what we need.
> 

Yeah, I tried the "unevaluatedProperties: false" setting and discovered that
it didn't work. That's why I defined all the possible properties in this
DT schema with at least boolean type including the sub-node
pattern-based properties and set the "additionalProperties:
false", so unknown properties would cause the dt_binding_check error.

Since you are saying that unevaluatedProperties: is not implemented,
but it will be in future, and it's ok that currently we may have some
unevaluated properties left declared, then I'll do as you say:
remove all dummy boolean properties and replace "additionalProperties"
property with "unevaluatedProperties" one.

Regards,
-Sergey

> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - "#address-cells"
> > +  - "#size-cells"
> > +  - interrupts
> > +
> > +examples:
> > +  - |
> > +    i2c@f0000 {
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +      compatible = "snps,designware-i2c";
> > +      reg = <0xf0000 0x1000>;
> > +      interrupts = <11>;
> > +      clock-frequency = <400000>;
> > +    };
> > +
> > +  - |
> > +    i2c@1120000 {
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +      compatible = "snps,designware-i2c";
> > +      reg = <0x1120000 0x1000>;
> > +      interrupt-parent = <&ictl>;
> > +      interrupts = <12 1>;
> > +      clock-frequency = <400000>;
> > +      i2c-sda-hold-time-ns = <300>;
> > +      i2c-sda-falling-time-ns = <300>;
> > +      i2c-scl-falling-time-ns = <300>;
> > +    };
> > +
> > +  - |
> > +    i2c@1120000 {
> > +      compatible = "snps,designware-i2c";
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +      reg = <0x2000 0x100>;
> > +      clock-frequency = <400000>;
> > +      clocks = <&i2cclk>;
> > +      interrupts = <0>;
> > +
> > +      eeprom@64 {
> > +        compatible = "linux,slave-24c02";
> > +        reg = <0x40000064>;
> > +      };
> > +    };
> > +...
> > -- 
> > 2.25.1
> > 

[PATCH 3/6] dt-bindings: i2c: dw: Add Baikal-T1 SoC I2C controller

[Sergey.Semin]

From: Serge Semin <Sergey.Semin@baikalelectronics.ru>

Just add the "be,bt1-i2c" compatible string to the bindings. The rest of
the DW APB I2C properties can be freely used to describe the Baikal-T1
I2C controller dts-node.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
---
 Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
index 3f348f1ce172..eab2c9293665 100644
--- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
@@ -34,6 +34,8 @@ properties:
         items:
           - const: mscc,ocelot-i2c
           - const: snps,designware-i2c
+      - description: Baikal-T1 SoCs I2C controller.
+        const: be,bt1-i2c
 
   reg:
     items:
-- 
2.25.1

Re: [PATCH 3/6] dt-bindings: i2c: dw: Add Baikal-T1 SoC I2C controller

[Rob Herring]

On Fri, 6 Mar 2020 16:19:52 +0300, <Sergey.Semin@baikalelectronics.ru> wrote:
> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> Just add the "be,bt1-i2c" compatible string to the bindings. The rest of
> the DW APB I2C properties can be freely used to describe the Baikal-T1
> I2C controller dts-node.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> ---
>  Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

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

Re: [PATCH 3/6] dt-bindings: i2c: dw: Add Baikal-T1 SoC I2C controller

[Sergey Semin]

On Thu, Mar 12, 2020 at 04:43:40PM -0500, Rob Herring wrote:
> On Fri, 6 Mar 2020 16:19:52 +0300, <Sergey.Semin@baikalelectronics.ru> wrote:
> > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > 
> > Just add the "be,bt1-i2c" compatible string to the bindings. The rest of
> > the DW APB I2C properties can be freely used to describe the Baikal-T1
> > I2C controller dts-node.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: Paul Burton <paulburton@kernel.org>
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > ---
> >  Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> 
> Acked-by: Rob Herring <robh@kernel.org>

Seeing you and us having doubts regarding our vendor prefix and the
corresponding patch still hasn't been accepted, in the next patchset release
perhaps I will have to change the compatible string of this driver. It depends
on a result of the discussion: https://lkml.org/lkml/2020/3/13/239

Rob, could you get back to it, so we could come up with a solution?

Currently most of our team members are leaning towards "baikal,t1" = "vendor,chip"
prefixes to all the SoC specific devices. So the Baikal-T1 I2C compatible string
would be renamed to "baikal,t1-i2c". What do you think?

Regards,
-Sergey

[PATCH 4/6] i2c: designware: Detect the FIFO size in the common code

[Sergey.Semin]

From: Serge Semin <Sergey.Semin@baikalelectronics.ru>

The problem with detecting the FIFO depth in the platform driver
is that in order to implement this we have to access the controller
IC_COMP_PARAM_1 register. Currently it's done before the
i2c_dw_set_reg_access() method execution, which is errors prone since
the method determines the registers endianness and access mode and we
can't use dw_readl/dw_writel accessors before this information is
retrieved. We also can't move the i2c_dw_set_reg_access() function
invocation to after the master/slave probe functions call (when endianness
and access mode are determined), since the FIFO depth information is used
by them for initializations. So in order to fix the problem we have no
choice but to move the FIFO size detection methods to the common code and
call it at the probe stage.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-i2c@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/i2c/busses/i2c-designware-common.c  | 22 +++++++++++++++++++
 drivers/i2c/busses/i2c-designware-core.h    |  1 +
 drivers/i2c/busses/i2c-designware-master.c  |  2 ++
 drivers/i2c/busses/i2c-designware-platdrv.c | 24 ---------------------
 drivers/i2c/busses/i2c-designware-slave.c   |  2 ++
 5 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 2de7452fcd6d..4291ff6246d8 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -344,6 +344,28 @@ int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
 		return -EIO;
 }
 
+void i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
+{
+	u32 param, tx_fifo_depth, rx_fifo_depth;
+
+	/*
+	 * Try to detect the FIFO depth if not set by interface driver,
+	 * the depth could be from 2 to 256 from HW spec.
+	 */
+	param = dw_readl(dev, DW_IC_COMP_PARAM_1);
+	tx_fifo_depth = ((param >> 16) & 0xff) + 1;
+	rx_fifo_depth = ((param >> 8)  & 0xff) + 1;
+	if (!dev->tx_fifo_depth) {
+		dev->tx_fifo_depth = tx_fifo_depth;
+		dev->rx_fifo_depth = rx_fifo_depth;
+	} else if (tx_fifo_depth >= 2) {
+		dev->tx_fifo_depth = min_t(u32, dev->tx_fifo_depth,
+				tx_fifo_depth);
+		dev->rx_fifo_depth = min_t(u32, dev->rx_fifo_depth,
+				rx_fifo_depth);
+	}
+}
+
 u32 i2c_dw_func(struct i2c_adapter *adap)
 {
 	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 67edbbde1070..3fbc9f22fcf1 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -297,6 +297,7 @@ int i2c_dw_acquire_lock(struct dw_i2c_dev *dev);
 void i2c_dw_release_lock(struct dw_i2c_dev *dev);
 int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev);
 int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev);
+void i2c_dw_set_fifo_size(struct dw_i2c_dev *dev);
 u32 i2c_dw_func(struct i2c_adapter *adap);
 void i2c_dw_disable(struct dw_i2c_dev *dev);
 void i2c_dw_disable_int(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index e8b328242256..05da900cf375 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -698,6 +698,8 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
 	if (ret)
 		return ret;
 
+	i2c_dw_set_fifo_size(dev);
+
 	ret = dev->init(dev);
 	if (ret)
 		return ret;
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 3b7d58c2fe85..cb494273bb60 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -219,28 +219,6 @@ static void i2c_dw_configure_slave(struct dw_i2c_dev *dev)
 	dev->mode = DW_IC_SLAVE;
 }
 
-static void dw_i2c_set_fifo_size(struct dw_i2c_dev *dev)
-{
-	u32 param, tx_fifo_depth, rx_fifo_depth;
-
-	/*
-	 * Try to detect the FIFO depth if not set by interface driver,
-	 * the depth could be from 2 to 256 from HW spec.
-	 */
-	param = i2c_dw_read_comp_param(dev);
-	tx_fifo_depth = ((param >> 16) & 0xff) + 1;
-	rx_fifo_depth = ((param >> 8)  & 0xff) + 1;
-	if (!dev->tx_fifo_depth) {
-		dev->tx_fifo_depth = tx_fifo_depth;
-		dev->rx_fifo_depth = rx_fifo_depth;
-	} else if (tx_fifo_depth >= 2) {
-		dev->tx_fifo_depth = min_t(u32, dev->tx_fifo_depth,
-				tx_fifo_depth);
-		dev->rx_fifo_depth = min_t(u32, dev->rx_fifo_depth,
-				rx_fifo_depth);
-	}
-}
-
 static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev)
 {
 	pm_runtime_disable(dev->dev);
@@ -362,8 +340,6 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 				div_u64(clk_khz * t->sda_hold_ns + 500000, 1000000);
 	}
 
-	dw_i2c_set_fifo_size(dev);
-
 	adap = &dev->adapter;
 	adap->owner = THIS_MODULE;
 	adap->class = I2C_CLASS_DEPRECATED;
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index f5f001738df5..0fc3aa31d46a 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -260,6 +260,8 @@ int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
 	if (ret)
 		return ret;
 
+	i2c_dw_set_fifo_size(dev);
+
 	ret = dev->init(dev);
 	if (ret)
 		return ret;
-- 
2.25.1

Re: [PATCH 4/6] i2c: designware: Detect the FIFO size in the common code

[Andy Shevchenko]

On Fri, Mar 06, 2020 at 04:19:54PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> The problem with detecting the FIFO depth in the platform driver
> is that in order to implement this we have to access the controller
> IC_COMP_PARAM_1 register. Currently it's done before the
> i2c_dw_set_reg_access() method execution, which is errors prone since
> the method determines the registers endianness and access mode and we
> can't use dw_readl/dw_writel accessors before this information is
> retrieved. We also can't move the i2c_dw_set_reg_access() function
> invocation to after the master/slave probe functions call (when endianness
> and access mode are determined), since the FIFO depth information is used
> by them for initializations. So in order to fix the problem we have no
> choice but to move the FIFO size detection methods to the common code and
> call it at the probe stage.

Sounds reasonable.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-i2c@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/i2c/busses/i2c-designware-common.c  | 22 +++++++++++++++++++
>  drivers/i2c/busses/i2c-designware-core.h    |  1 +
>  drivers/i2c/busses/i2c-designware-master.c  |  2 ++
>  drivers/i2c/busses/i2c-designware-platdrv.c | 24 ---------------------
>  drivers/i2c/busses/i2c-designware-slave.c   |  2 ++
>  5 files changed, 27 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
> index 2de7452fcd6d..4291ff6246d8 100644
> --- a/drivers/i2c/busses/i2c-designware-common.c
> +++ b/drivers/i2c/busses/i2c-designware-common.c
> @@ -344,6 +344,28 @@ int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
>  		return -EIO;
>  }
>  
> +void i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
> +{
> +	u32 param, tx_fifo_depth, rx_fifo_depth;
> +
> +	/*
> +	 * Try to detect the FIFO depth if not set by interface driver,
> +	 * the depth could be from 2 to 256 from HW spec.
> +	 */
> +	param = dw_readl(dev, DW_IC_COMP_PARAM_1);
> +	tx_fifo_depth = ((param >> 16) & 0xff) + 1;
> +	rx_fifo_depth = ((param >> 8)  & 0xff) + 1;
> +	if (!dev->tx_fifo_depth) {
> +		dev->tx_fifo_depth = tx_fifo_depth;
> +		dev->rx_fifo_depth = rx_fifo_depth;
> +	} else if (tx_fifo_depth >= 2) {
> +		dev->tx_fifo_depth = min_t(u32, dev->tx_fifo_depth,
> +				tx_fifo_depth);
> +		dev->rx_fifo_depth = min_t(u32, dev->rx_fifo_depth,
> +				rx_fifo_depth);
> +	}
> +}
> +
>  u32 i2c_dw_func(struct i2c_adapter *adap)
>  {
>  	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
> diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
> index 67edbbde1070..3fbc9f22fcf1 100644
> --- a/drivers/i2c/busses/i2c-designware-core.h
> +++ b/drivers/i2c/busses/i2c-designware-core.h
> @@ -297,6 +297,7 @@ int i2c_dw_acquire_lock(struct dw_i2c_dev *dev);
>  void i2c_dw_release_lock(struct dw_i2c_dev *dev);
>  int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev);
>  int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev);
> +void i2c_dw_set_fifo_size(struct dw_i2c_dev *dev);
>  u32 i2c_dw_func(struct i2c_adapter *adap);
>  void i2c_dw_disable(struct dw_i2c_dev *dev);
>  void i2c_dw_disable_int(struct dw_i2c_dev *dev);
> diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
> index e8b328242256..05da900cf375 100644
> --- a/drivers/i2c/busses/i2c-designware-master.c
> +++ b/drivers/i2c/busses/i2c-designware-master.c
> @@ -698,6 +698,8 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
>  	if (ret)
>  		return ret;
>  
> +	i2c_dw_set_fifo_size(dev);
> +
>  	ret = dev->init(dev);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 3b7d58c2fe85..cb494273bb60 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -219,28 +219,6 @@ static void i2c_dw_configure_slave(struct dw_i2c_dev *dev)
>  	dev->mode = DW_IC_SLAVE;
>  }
>  
> -static void dw_i2c_set_fifo_size(struct dw_i2c_dev *dev)
> -{
> -	u32 param, tx_fifo_depth, rx_fifo_depth;
> -
> -	/*
> -	 * Try to detect the FIFO depth if not set by interface driver,
> -	 * the depth could be from 2 to 256 from HW spec.
> -	 */
> -	param = i2c_dw_read_comp_param(dev);
> -	tx_fifo_depth = ((param >> 16) & 0xff) + 1;
> -	rx_fifo_depth = ((param >> 8)  & 0xff) + 1;
> -	if (!dev->tx_fifo_depth) {
> -		dev->tx_fifo_depth = tx_fifo_depth;
> -		dev->rx_fifo_depth = rx_fifo_depth;
> -	} else if (tx_fifo_depth >= 2) {
> -		dev->tx_fifo_depth = min_t(u32, dev->tx_fifo_depth,
> -				tx_fifo_depth);
> -		dev->rx_fifo_depth = min_t(u32, dev->rx_fifo_depth,
> -				rx_fifo_depth);
> -	}
> -}
> -
>  static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev)
>  {
>  	pm_runtime_disable(dev->dev);
> @@ -362,8 +340,6 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
>  				div_u64(clk_khz * t->sda_hold_ns + 500000, 1000000);
>  	}
>  
> -	dw_i2c_set_fifo_size(dev);
> -
>  	adap = &dev->adapter;
>  	adap->owner = THIS_MODULE;
>  	adap->class = I2C_CLASS_DEPRECATED;
> diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
> index f5f001738df5..0fc3aa31d46a 100644
> --- a/drivers/i2c/busses/i2c-designware-slave.c
> +++ b/drivers/i2c/busses/i2c-designware-slave.c
> @@ -260,6 +260,8 @@ int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
>  	if (ret)
>  		return ret;
>  
> +	i2c_dw_set_fifo_size(dev);
> +
>  	ret = dev->init(dev);
>  	if (ret)
>  		return ret;
> -- 
> 2.25.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/6] i2c: designware: Detect the FIFO size in the common code

[Jarkko Nikula]

On 3/6/20 5:01 PM, Andy Shevchenko wrote:
> On Fri, Mar 06, 2020 at 04:19:54PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
>> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
>>
>> The problem with detecting the FIFO depth in the platform driver
>> is that in order to implement this we have to access the controller
>> IC_COMP_PARAM_1 register. Currently it's done before the
>> i2c_dw_set_reg_access() method execution, which is errors prone since
>> the method determines the registers endianness and access mode and we
>> can't use dw_readl/dw_writel accessors before this information is
>> retrieved. We also can't move the i2c_dw_set_reg_access() function
>> invocation to after the master/slave probe functions call (when endianness
>> and access mode are determined), since the FIFO depth information is used
>> by them for initializations. So in order to fix the problem we have no
>> choice but to move the FIFO size detection methods to the common code and
>> call it at the probe stage.
> 
> Sounds reasonable.
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Re: [PATCH 4/6] i2c: designware: Detect the FIFO size in the common code

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1350 bytes --]

On Fri, Mar 06, 2020 at 04:19:54PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> The problem with detecting the FIFO depth in the platform driver
> is that in order to implement this we have to access the controller
> IC_COMP_PARAM_1 register. Currently it's done before the
> i2c_dw_set_reg_access() method execution, which is errors prone since
> the method determines the registers endianness and access mode and we
> can't use dw_readl/dw_writel accessors before this information is
> retrieved. We also can't move the i2c_dw_set_reg_access() function
> invocation to after the master/slave probe functions call (when endianness
> and access mode are determined), since the FIFO depth information is used
> by them for initializations. So in order to fix the problem we have no
> choice but to move the FIFO size detection methods to the common code and
> call it at the probe stage.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-i2c@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 5/6] i2c: designware: Discard i2c_dw_read_comp_param() function

[Sergey.Semin]

From: Serge Semin <Sergey.Semin@baikalelectronics.ru>

There is no code left in the kernel which would be using the function.
So just remove it.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-i2c@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/i2c/busses/i2c-designware-common.c | 6 ------
 drivers/i2c/busses/i2c-designware-core.h   | 1 -
 2 files changed, 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 4291ff6246d8..72e93c1aa9bc 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -388,11 +388,5 @@ void i2c_dw_disable_int(struct dw_i2c_dev *dev)
 	dw_writel(dev, 0, DW_IC_INTR_MASK);
 }
 
-u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev)
-{
-	return dw_readl(dev, DW_IC_COMP_PARAM_1);
-}
-EXPORT_SYMBOL_GPL(i2c_dw_read_comp_param);
-
 MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter core");
 MODULE_LICENSE("GPL");
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 3fbc9f22fcf1..b220ad64c38d 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -314,7 +314,6 @@ static inline void __i2c_dw_disable_nowait(struct dw_i2c_dev *dev)
 
 void __i2c_dw_disable(struct dw_i2c_dev *dev);
 
-extern u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev);
 extern int i2c_dw_probe(struct dw_i2c_dev *dev);
 #if IS_ENABLED(CONFIG_I2C_DESIGNWARE_SLAVE)
 extern int i2c_dw_probe_slave(struct dw_i2c_dev *dev);
-- 
2.25.1

Re: [PATCH 5/6] i2c: designware: Discard i2c_dw_read_comp_param() function

[Jarkko Nikula]

On 3/6/20 3:19 PM, Sergey.Semin@baikalelectronics.ru wrote:
> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> There is no code left in the kernel which would be using the function.
> So just remove it.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-i2c@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>   drivers/i2c/busses/i2c-designware-common.c | 6 ------
>   drivers/i2c/busses/i2c-designware-core.h   | 1 -
>   2 files changed, 7 deletions(-)
> 
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Re: [PATCH 5/6] i2c: designware: Discard i2c_dw_read_comp_param() function

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 625 bytes --]

On Fri, Mar 06, 2020 at 04:19:56PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> There is no code left in the kernel which would be using the function.
> So just remove it.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-i2c@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 6/6] i2c: designware: Add Baikal-T1 SoC I2C controller support

[Sergey.Semin]

From: Serge Semin <Sergey.Semin@baikalelectronics.ru>

A third I2C controller embedded into the Baikal-T1 SoC is also fully
based on the DW APB I2C core, but its registers are indirectly
accessible via "command/data in/data out" trio. There is no difference
other than that. So in order to have that controller supported by the
common DW APB I2C driver we only need to introduce a new flag
ACCESS_INDIRECT and use the access-registers to reach the I2C controller
normal registers space in the dw_readl/dw_writel methods. Currently this
flag is only enabled for the controllers with "be,bt1-i2c" compatible
string.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-i2c@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/i2c/busses/i2c-designware-common.c  | 79 ++++++++++++++++++---
 drivers/i2c/busses/i2c-designware-core.h    | 14 ++++
 drivers/i2c/busses/i2c-designware-master.c  |  1 +
 drivers/i2c/busses/i2c-designware-platdrv.c |  1 +
 drivers/i2c/busses/i2c-designware-slave.c   |  1 +
 5 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 72e93c1aa9bc..d3010bb6ad42 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -15,6 +15,7 @@
 #include <linux/err.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/spinlock.h>
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
@@ -53,15 +54,75 @@ static char *abort_sources[] = {
 		"incorrect slave-transmitter mode configuration",
 };
 
+static inline u32 dw_read_reg(struct dw_i2c_dev *dev, int offset)
+{
+	if (dev->flags & ACCESS_16BIT)
+		return readw_relaxed(dev->base + offset) |
+			(readw_relaxed(dev->base + offset + 2) << 16);
+	else
+		return readl_relaxed(dev->base + offset);
+}
+
+static inline void dw_write_reg(struct dw_i2c_dev *dev, u32 b, int offset)
+{
+	if (dev->flags & ACCESS_16BIT) {
+		writew_relaxed((u16)b, dev->base + offset);
+		writew_relaxed((u16)(b >> 16), dev->base + offset + 2);
+	} else {
+		writel_relaxed(b, dev->base + offset);
+	}
+}
+
+static inline u32 dw_read_ind(struct dw_i2c_dev *dev, int offset)
+{
+	unsigned long flags, ok = DW_IC_IND_RETRY;
+	u32 cmd, value = 0;
+
+	cmd = DW_IC_CTL_GO | (offset & DW_IC_CTL_ADDR_MASK);
+
+	spin_lock_irqsave(&dev->ind_lock, flags);
+
+	dw_write_reg(dev, cmd, DW_IC_CTL);
+
+	while ((dw_read_reg(dev, DW_IC_CTL) & DW_IC_CTL_GO) && ok--);
+	if (ok)
+		value = dw_read_reg(dev, DW_IC_DO);
+	else
+		dev_err(dev->dev, "Register 0x%02x read timedout\n", offset);
+
+	spin_unlock_irqrestore(&dev->ind_lock, flags);
+
+	return value;
+}
+
+static inline void dw_write_ind(struct dw_i2c_dev *dev, u32 b, int offset)
+{
+	unsigned long flags, ok = DW_IC_IND_RETRY;
+	u32 cmd;
+
+	cmd = DW_IC_CTL_GO | DW_IC_CTL_WR | (offset & DW_IC_CTL_ADDR_MASK);
+
+	spin_lock_irqsave(&dev->ind_lock, flags);
+
+	dw_write_reg(dev, b, DW_IC_DI);
+	dw_write_reg(dev, cmd, DW_IC_CTL);
+
+	while ((dw_read_reg(dev, DW_IC_CTL) & DW_IC_CTL_GO) && ok--);
+
+	spin_unlock_irqrestore(&dev->ind_lock, flags);
+
+	if (!ok)
+		dev_err(dev->dev, "Register 0x%02x write timedout\n", offset);
+}
+
 u32 dw_readl(struct dw_i2c_dev *dev, int offset)
 {
 	u32 value;
 
-	if (dev->flags & ACCESS_16BIT)
-		value = readw_relaxed(dev->base + offset) |
-			(readw_relaxed(dev->base + offset + 2) << 16);
+	if (dev->flags & ACCESS_INDIRECT)
+		value = dw_read_ind(dev, offset);
 	else
-		value = readl_relaxed(dev->base + offset);
+		value = dw_read_reg(dev, offset);
 
 	if (dev->flags & ACCESS_SWAP)
 		return swab32(value);
@@ -74,12 +135,10 @@ void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
 	if (dev->flags & ACCESS_SWAP)
 		b = swab32(b);
 
-	if (dev->flags & ACCESS_16BIT) {
-		writew_relaxed((u16)b, dev->base + offset);
-		writew_relaxed((u16)(b >> 16), dev->base + offset + 2);
-	} else {
-		writel_relaxed(b, dev->base + offset);
-	}
+	if (dev->flags & ACCESS_INDIRECT)
+		dw_write_ind(dev, b, offset);
+	else
+		dw_write_reg(dev, b, offset);
 }
 
 /**
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index b220ad64c38d..0857b3842283 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -170,11 +170,23 @@
 					 DW_IC_TX_ABRT_TXDATA_NOACK | \
 					 DW_IC_TX_ABRT_GCALL_NOACK)
 
+/*
+ * Access registers for the IC space described above.
+ */
+#define DW_IC_CTL		0x0
+#define DW_IC_DI		0x4
+#define DW_IC_DO		0x8
+#define DW_IC_IND_RETRY		100
+
+#define DW_IC_CTL_GO		BIT(31)
+#define DW_IC_CTL_WR		BIT(8)
+#define DW_IC_CTL_ADDR_MASK	GENMASK(7, 0)
 
 /**
  * struct dw_i2c_dev - private i2c-designware data
  * @dev: driver model device node
  * @base: IO registers pointer
+ * @ind_lock: Spin-lock for indirectly accesible registers
  * @cmd_complete: tx completion indicator
  * @clk: input reference clock
  * @pclk: clock required to access the registers
@@ -226,6 +238,7 @@ struct dw_i2c_dev {
 	struct device		*dev;
 	void __iomem		*base;
 	void __iomem		*ext;
+	spinlock_t		ind_lock;
 	struct completion	cmd_complete;
 	struct clk		*clk;
 	struct clk		*pclk;
@@ -280,6 +293,7 @@ struct dw_i2c_dev {
 #define ACCESS_16BIT		0x00000002
 #define ACCESS_INTR_MASK	0x00000004
 #define ACCESS_NO_IRQ_SUSPEND	0x00000008
+#define ACCESS_INDIRECT		0x00000010
 
 #define MODEL_CHERRYTRAIL	0x00000100
 #define MODEL_MSCC_OCELOT	0x00000200
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 05da900cf375..59365dd4dc66 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -684,6 +684,7 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
 	unsigned long irq_flags;
 	int ret;
 
+	spin_lock_init(&dev->ind_lock);
 	init_completion(&dev->cmd_complete);
 
 	dev->init = i2c_dw_init_master;
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index cb494273bb60..683c456c4e1c 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -176,6 +176,7 @@ static int dw_i2c_of_configure(struct platform_device *pdev)
 static const struct of_device_id dw_i2c_of_match[] = {
 	{ .compatible = "snps,designware-i2c", },
 	{ .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
+	{ .compatible = "be,bt1-i2c", .data = (void *)ACCESS_INDIRECT },
 	{},
 };
 MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index 0fc3aa31d46a..a6e65dcc693b 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -246,6 +246,7 @@ int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
 	struct i2c_adapter *adap = &dev->adapter;
 	int ret;
 
+	spin_lock_init(&dev->ind_lock);
 	init_completion(&dev->cmd_complete);
 
 	dev->init = i2c_dw_init_slave;
-- 
2.25.1

Re: [PATCH 6/6] i2c: designware: Add Baikal-T1 SoC I2C controller support

[Andy Shevchenko]

On Fri, Mar 06, 2020 at 04:19:58PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> A third I2C controller embedded into the Baikal-T1 SoC is also fully
> based on the DW APB I2C core, but its registers are indirectly
> accessible via "command/data in/data out" trio. There is no difference
> other than that. So in order to have that controller supported by the
> common DW APB I2C driver we only need to introduce a new flag
> ACCESS_INDIRECT and use the access-registers to reach the I2C controller
> normal registers space in the dw_readl/dw_writel methods. Currently this
> flag is only enabled for the controllers with "be,bt1-i2c" compatible
> string.

See my response to cover letter.

>  drivers/i2c/busses/i2c-designware-common.c  | 79 ++++++++++++++++++---
>  drivers/i2c/busses/i2c-designware-core.h    | 14 ++++
>  drivers/i2c/busses/i2c-designware-master.c  |  1 +
>  drivers/i2c/busses/i2c-designware-platdrv.c |  1 +
>  drivers/i2c/busses/i2c-designware-slave.c   |  1 +

This should be split at least to two patches (preparation in the core followed
by switching users).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 6/6] i2c: designware: Add Baikal-T1 SoC I2C controller support

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 992 bytes --]

On Fri, Mar 06, 2020 at 04:19:58PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> A third I2C controller embedded into the Baikal-T1 SoC is also fully
> based on the DW APB I2C core, but its registers are indirectly
> accessible via "command/data in/data out" trio. There is no difference
> other than that. So in order to have that controller supported by the
> common DW APB I2C driver we only need to introduce a new flag
> ACCESS_INDIRECT and use the access-registers to reach the I2C controller
> normal registers space in the dw_readl/dw_writel methods. Currently this
> flag is only enabled for the controllers with "be,bt1-i2c" compatible
> string.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>

Patches 2,3, and 6 have comments which should be addressed. Patches 4+5
can be dropped when sending next version.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 6/6] i2c: designware: Add Baikal-T1 SoC I2C controller support

[Sergey Semin]

On Sat, Mar 21, 2020 at 07:33:49PM +0100, Wolfram Sang wrote:
> On Fri, Mar 06, 2020 at 04:19:58PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > 
> > A third I2C controller embedded into the Baikal-T1 SoC is also fully
> > based on the DW APB I2C core, but its registers are indirectly
> > accessible via "command/data in/data out" trio. There is no difference
> > other than that. So in order to have that controller supported by the
> > common DW APB I2C driver we only need to introduce a new flag
> > ACCESS_INDIRECT and use the access-registers to reach the I2C controller
> > normal registers space in the dw_readl/dw_writel methods. Currently this
> > flag is only enabled for the controllers with "be,bt1-i2c" compatible
> > string.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> 
> Patches 2,3, and 6 have comments which should be addressed. Patches 4+5
> can be dropped when sending next version.
> 

Great! Thanks. I'll have respond on the requests very soon.

Regards,
-Sergey