diff mbox series

[PATCH/RFC,4/6] dt-bindings: i2c: renesas,iic: Convert to json-schema

Message ID ecfaf6be5e8c285db2bcc823bb1dd89931fa5c29.1620138454.git.geert+renesas@glider.be (mailing list archive)
State Under Review
Delegated to: Geert Uytterhoeven
Headers show
Series dt-bindings: i2c: renesas: Convert to json-schema | expand

Commit Message

Geert Uytterhoeven May 4, 2021, 2:35 p.m. UTC
Convert the Renesas R-Mobile I2C Bus Interface (IIC) Device Tree binding
documentation to json-schema.

Document missing properties.
R-Mobile A1 and SH-Mobile have multiple interrupts.
Update the example to match reality.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
It is still not clear to me which instances on R-Car Gen2 and RZ/G1
support automatic transmission, cfr. this note for the IIC3 instance in
the Hardware User's Manual:

    Automatic transmission for PMIC control (DVFS) is not available
    because the second generation R-Car series and RZ/G series products
    do not support the DVFS function (automatic transmission can be used
    as a hardware function, but this is not meaningful for actual use
    cases).

The only thing that's clear is that R-Car E2 and RZ/G1E do not have the
automatic transmission registers.  Note that current r8a7745.dtsi and
r8a7794.dtsi do not reflect that.

This binding interpretes the note as that IIC3 does not support
automatic transmission on R-Car H2, M2-W, M2-N, and V2H, and RZ/G1H,
RZ/H1M, RZ/G1N, while the other (if any) IIC instances on these SoCs do
support automatic transmission.  This matches with the current
r8a774[234].dtsi, which do not declare compatibility with the
family-specific and generic versions.  Note that the current
r8a779[0123].dtsi still do, and thus should be updated!

Possible alternative interpretations of the note are:
  - Only IIC3 has the automatic transmission registers.  But the
    automatic transmission feature is not useful as the SoCs lack DVFS
    support.
  - All IIC instances have the automatic transmission registers, and
    IIC3 is traditionally used to control the PMIC.  But as the SoCs
    lack DVFS support, the automatic transmission feature is not useful.
    As R-Car V2H has only instance IIC3, and is documented to have
    automatic transmissiong registers, this might be the correct
    interpration?
  - Anything else?

Thanks for your comments!
---
 .../devicetree/bindings/i2c/renesas,iic.txt   |  72 --------
 .../bindings/i2c/renesas,rmobile-iic.yaml     | 160 ++++++++++++++++++
 MAINTAINERS                                   |   2 +-
 3 files changed, 161 insertions(+), 73 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/i2c/renesas,iic.txt
 create mode 100644 Documentation/devicetree/bindings/i2c/renesas,rmobile-iic.yaml

Comments

Wolfram Sang May 5, 2021, 7:33 a.m. UTC | #1
> Possible alternative interpretations of the note are:
>   - Only IIC3 has the automatic transmission registers.  But the
>     automatic transmission feature is not useful as the SoCs lack DVFS
>     support.

I immediately thought "yeah, this is it", but had to do some resarch
where my assumption comes from. I found it in older H2 datasheets
(v0.9). Here in 56.1:

"Automatic transmission for PMIC control: The IIC3 module of the R-Car
H2 and M2 supports automatic data transmission under PMIC control
(DVFS)."

Or table 56.8:

"Registers of IIC Command for Automatic Transmission Mode (IIC3 only)
[R-CarH2, M2, and V2H]"


This all is a strong indication for IIC3 only. Which SoCs that have is
still confusing. Table 56.8 mentions V2H but 56.1 doesn't. Then again, I
could imagine that V2H has it but simply DVFS is not advertised for V2H.
And in the later documents, DVFS advertisement was removed for H2 and M2
as well.

(PS: sorry for replying to the "wrong" thread, I accidently deleted the
"correct" one)
Rob Herring May 6, 2021, 8:54 p.m. UTC | #2
On Tue, May 04, 2021 at 04:51:11PM +0200, Geert Uytterhoeven wrote:
> Convert the Renesas R-Mobile I2C Bus Interface (IIC) Device Tree binding
> documentation to json-schema.
> 
> Document missing properties.
> R-Mobile A1 and SH-Mobile have multiple interrupts.
> Update the example to match reality.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> It is still not clear to me which instances on R-Car Gen2 and RZ/G1
> support automatic transmission, cfr. this note for the IIC3 instance in
> the Hardware User's Manual:
> 
>     Automatic transmission for PMIC control (DVFS) is not available
>     because the second generation R-Car series and RZ/G series products
>     do not support the DVFS function (automatic transmission can be used
>     as a hardware function, but this is not meaningful for actual use
>     cases).
> 
> The only thing that's clear is that R-Car E2 and RZ/G1E do not have the
> automatic transmission registers.  Note that current r8a7745.dtsi and
> r8a7794.dtsi do not reflect that.
> 
> This binding interpretes the note as that IIC3 does not support
> automatic transmission on R-Car H2, M2-W, M2-N, and V2H, and RZ/G1H,
> RZ/H1M, RZ/G1N, while the other (if any) IIC instances on these SoCs do
> support automatic transmission.  This matches with the current
> r8a774[234].dtsi, which do not declare compatibility with the
> family-specific and generic versions.  Note that the current
> r8a779[0123].dtsi still do, and thus should be updated!
> 
> Possible alternative interpretations of the note are:
>   - Only IIC3 has the automatic transmission registers.  But the
>     automatic transmission feature is not useful as the SoCs lack DVFS
>     support.
>   - All IIC instances have the automatic transmission registers, and
>     IIC3 is traditionally used to control the PMIC.  But as the SoCs
>     lack DVFS support, the automatic transmission feature is not useful.
>     As R-Car V2H has only instance IIC3, and is documented to have
>     automatic transmissiong registers, this might be the correct
>     interpration?
>   - Anything else?

Can't help there, but looks fine otherwise.

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

> 
> Thanks for your comments!
> ---
>  .../devicetree/bindings/i2c/renesas,iic.txt   |  72 --------
>  .../bindings/i2c/renesas,rmobile-iic.yaml     | 160 ++++++++++++++++++
>  MAINTAINERS                                   |   2 +-
>  3 files changed, 161 insertions(+), 73 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/i2c/renesas,iic.txt
>  create mode 100644 Documentation/devicetree/bindings/i2c/renesas,rmobile-iic.yaml
Geert Uytterhoeven May 26, 2021, 2:47 p.m. UTC | #3
On Wed, May 5, 2021 at 9:33 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> > Possible alternative interpretations of the note are:
> >   - Only IIC3 has the automatic transmission registers.  But the
> >     automatic transmission feature is not useful as the SoCs lack DVFS
> >     support.
>
> I immediately thought "yeah, this is it", but had to do some resarch
> where my assumption comes from. I found it in older H2 datasheets
> (v0.9). Here in 56.1:
>
> "Automatic transmission for PMIC control: The IIC3 module of the R-Car
> H2 and M2 supports automatic data transmission under PMIC control
> (DVFS)."
>
> Or table 56.8:
>
> "Registers of IIC Command for Automatic Transmission Mode (IIC3 only)
> [R-CarH2, M2, and V2H]"
>
>
> This all is a strong indication for IIC3 only. Which SoCs that have is
> still confusing. Table 56.8 mentions V2H but 56.1 doesn't. Then again, I
> could imagine that V2H has it but simply DVFS is not advertised for V2H.
> And in the later documents, DVFS advertisement was removed for H2 and M2
> as well.

From off-list experiments checking for the presence of automatic
transmission registers by writing to them, and seeing if they retain their
values, we know that all IIC instances checked on R-Car Gen2
SoCs do have these registers.  The same is true for R-Car E3, which
is explicitly documented _not_ to have the registers...
In addition, Wolfram tried transmitting something on R-Car H2 from
the U-Boot prompt, and noticed the ICINT.ADTE bit is set afterwards,
indicating success.

So I went one step further, and hooked up a logic analyzer, to see
if anything is actually transmitted.

Koelsch (R-Car M2-W):

  - IIC1 (I2C8) IIC1_SCL_C (GP6_22) EXIO Connector C pin 37
                IIC1_SDA_C (GP6_23) EXIO Connector C pin 39

    # Configure pinctrl for IIC1
    mw.l e6060000 f77fffff
    mw.l e6060098 08800000
    mw.l e6060000 ffffedff
    mw.l e6060058 00001200
    mw.l e6060000 3000c0c0
    mw.l e606001c cfff3f3f

    # Enable IIC0-2 clocks in SMSTPCR3
    mw e615013c ff7bfffe

    # Reset using ICCR clears all ICINT flags
    mw.b e6510004 1

    # Set up clock (ICC[LH])
    mw.b e6510010 0x69
    mw.b e6510014 0x1e

    # Write dummy data to ICATD00
    mw.b e6510100 bf

    # ICSTART.AutoStart = 1
    mw.b e6510070 80

    # Logic Analyzer says: S / P / P / P => broken?

    # Check ICINT; 01 => ADTE
    md.b e6510054 1


  - IIC3 (I2C6 DVFS) IIC3_SCL EXIO Connector C pin 19
                     IIC3_SDA EXIO Connector C pin 21

    # Enable IIC3 clock in SMSTPCR9
    mw e6150994 fbffffff

    # Reset using ICCR clears all ICINT flags
    mw.b e60b0004 1

    # Set up clock (ICC[LH])
    mw.b e60b0010 0x1b
    mw.b e60b0014 0x14

    # Write dummy data to ICATD00
    mw.b e60b0100 de

    # ICSTART.AutoStart = 1
    mw.b e60b0070 80

    # Logic Analyzer: S / Address: 0x6f / W / NACK => Good!

    # Check ICINT; 01 => ADTE
    md.b e60b0054 1


Ebisu (R-Car E3):

  - IIC (DVFS) IIC_SCL EXIO Connector D pin 75
               IIC_SDA EXIO Connector D pin 77

    # Enable IIC clock in SMSTPCR9
    mw e6150994 fbffffff

    # Reset using ICCR clears all ICINT flags
    mw.b e60b0004 1

    # Set up clock (ICC[LH])
    mw.b e60b0010 0x1b
    mw.b e60b0014 0x14

    # Write dummy data to ICATD00
    mw.b e60b0100 ad

    # ICSTART.AutoStart = 1
    mw.b e60b0070 80

    # Logic Analyzer: S / Address: 0x56 / R / NACK => Good!

    # Check ICINT; 01 => ADTE
    md.b e60b0054 1


Preliminary conclusions:
  1. Automatic transmission works on the last IIC instance on R-Car
     Gen2, which was originally intended for DVFS (which is not
     implemented, as of Hardware User's Manual Rev. 2.00).
     It works partially/not on other IIC instances. Perhaps I did
     something wrong in my setup?

  2. Despite the Hardware User's Manual stating the single IIC instance
     on R-Car E3 does not have the automatic transmission registers,
     the feature seems to be present and working.  So we can declare
     it to be compatible with the generic version.

As the Linux (or other OS?) i2c driver doesn't use automatic
transmission, and it's very unlikely it ever will (anyone with a
use case?), I'm inclined to simplify, and declare all IIC instances
compatible with the generic version.
If we ever want to implement support for automatic transmission,
we can still differentiate by the SoC-specific compatible values,
as they are present anyway, and may have to resort to checking
e.g. instance base addresses anyway.

Thoughts? Thanks!

Gr{oetje,eeting}s,

                        Geert
Wolfram Sang May 28, 2021, 1:51 p.m. UTC | #4
Hi Geert,

> In addition, Wolfram tried transmitting something on R-Car H2 from
> the U-Boot prompt, and noticed the ICINT.ADTE bit is set afterwards,
> indicating success.

Note that I tested this basic test on E2 as well.

> As the Linux (or other OS?) i2c driver doesn't use automatic
> transmission, and it's very unlikely it ever will (anyone with a
> use case?), I'm inclined to simplify, and declare all IIC instances
> compatible with the generic version.
> If we ever want to implement support for automatic transmission,
> we can still differentiate by the SoC-specific compatible values,
> as they are present anyway, and may have to resort to checking
> e.g. instance base addresses anyway.
> 
> Thoughts? Thanks!

I agree. So, if nobody speaks up in the next days, I will apply this
patch as is.

Thanks for your investigating!

   Wolfram
Geert Uytterhoeven May 28, 2021, 2:14 p.m. UTC | #5
Hi Wolfram,

On Fri, May 28, 2021 at 3:51 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> > In addition, Wolfram tried transmitting something on R-Car H2 from
> > the U-Boot prompt, and noticed the ICINT.ADTE bit is set afterwards,
> > indicating success.
>
> Note that I tested this basic test on E2 as well.
>
> > As the Linux (or other OS?) i2c driver doesn't use automatic
> > transmission, and it's very unlikely it ever will (anyone with a
> > use case?), I'm inclined to simplify, and declare all IIC instances
> > compatible with the generic version.
> > If we ever want to implement support for automatic transmission,
> > we can still differentiate by the SoC-specific compatible values,
> > as they are present anyway, and may have to resort to checking
> > e.g. instance base addresses anyway.
> >
> > Thoughts? Thanks!
>
> I agree. So, if nobody speaks up in the next days, I will apply this
> patch as is.

Actually I'm more inclined to drop the special handling for the compatible
values in the fourth set of items.  I.e. move them to the second and third
set, and update the DTS files....

> Thanks for your investigating!

You're welcome.
Have a nice weekend!

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/i2c/renesas,iic.txt b/Documentation/devicetree/bindings/i2c/renesas,iic.txt
deleted file mode 100644
index 93d412832e66b682..0000000000000000
--- a/Documentation/devicetree/bindings/i2c/renesas,iic.txt
+++ /dev/null
@@ -1,72 +0,0 @@ 
-Device tree configuration for Renesas IIC (sh_mobile) driver
-
-Required properties:
-- compatible      :
-			- "renesas,iic-r8a73a4" (R-Mobile APE6)
-			- "renesas,iic-r8a7740" (R-Mobile A1)
-			- "renesas,iic-r8a7742" (RZ/G1H)
-			- "renesas,iic-r8a7743" (RZ/G1M)
-			- "renesas,iic-r8a7744" (RZ/G1N)
-			- "renesas,iic-r8a7745" (RZ/G1E)
-			- "renesas,iic-r8a774a1" (RZ/G2M)
-			- "renesas,iic-r8a774b1" (RZ/G2N)
-			- "renesas,iic-r8a774c0" (RZ/G2E)
-			- "renesas,iic-r8a774e1" (RZ/G2H)
-			- "renesas,iic-r8a7790" (R-Car H2)
-			- "renesas,iic-r8a7791" (R-Car M2-W)
-			- "renesas,iic-r8a7792" (R-Car V2H)
-			- "renesas,iic-r8a7793" (R-Car M2-N)
-			- "renesas,iic-r8a7794" (R-Car E2)
-			- "renesas,iic-r8a7795" (R-Car H3)
-			- "renesas,iic-r8a7796" (R-Car M3-W)
-			- "renesas,iic-r8a77961" (R-Car M3-W+)
-			- "renesas,iic-r8a77965" (R-Car M3-N)
-			- "renesas,iic-r8a77990" (R-Car E3)
-			- "renesas,iic-sh73a0" (SH-Mobile AG5)
-			- "renesas,rcar-gen2-iic" (generic R-Car Gen2 or RZ/G1
-							compatible device)
-			- "renesas,rcar-gen3-iic" (generic R-Car Gen3 or RZ/G2
-							compatible device)
-			- "renesas,rmobile-iic" (generic device)
-
-			When compatible with a generic R-Car version, nodes
-			must list the SoC-specific version corresponding to
-			the platform first followed by the generic R-Car
-			version.
-
-			When compatible with "renesas,rmobile-iic" it should
-			be the last compatibility string listed.
-
-			The r8a77990 (R-Car E3) and r8a774c0 (RZ/G2E)
-			controllers are not considered compatible with
-			"renesas,rcar-gen3-iic" or "renesas,rmobile-iic"
-			due to the absence of automatic transmission registers.
-
-- reg             : address start and address range size of device
-- interrupts      : interrupt of device
-- clocks          : clock for device
-- #address-cells  : should be <1>
-- #size-cells     : should be <0>
-
-Optional properties:
-- clock-frequency : frequency of bus clock in Hz. Default 100kHz if unset.
-- dmas            : Must contain a list of two references to DMA
-		    specifiers, one for transmission, and one for
-		    reception.
-- dma-names       : Must contain a list of two DMA names, "tx" and "rx".
-
-
-Pinctrl properties might be needed, too. See there.
-
-Example:
-
-	iic0: i2c@e6500000 {
-		compatible = "renesas,iic-r8a7790", "renesas,rcar-gen2-iic",
-			     "renesas,rmobile-iic";
-		reg = <0 0xe6500000 0 0x425>;
-		interrupts = <0 174 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&mstp3_clks R8A7790_CLK_IIC0>;
-		clock-frequency = <400000>;
-		#address-cells = <1>;
-		#size-cells = <0>;
-	};
diff --git a/Documentation/devicetree/bindings/i2c/renesas,rmobile-iic.yaml b/Documentation/devicetree/bindings/i2c/renesas,rmobile-iic.yaml
new file mode 100644
index 0000000000000000..7aa87f64922c0fd2
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/renesas,rmobile-iic.yaml
@@ -0,0 +1,160 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/renesas,rmobile-iic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas R-Mobile I2C Bus Interface (IIC)
+
+maintainers:
+  - Wolfram Sang <wsa+renesas@sang-engineering.com>
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - renesas,iic-r8a73a4      # R-Mobile APE6
+              - renesas,iic-r8a7740      # R-Mobile A1
+              - renesas,iic-sh73a0       # SH-Mobile AG5
+          - const: renesas,rmobile-iic   # Generic
+
+      - items:
+          - enum:
+              - renesas,iic-r8a7742      # RZ/G1H IIC[012]
+              - renesas,iic-r8a7743      # RZ/G1M IIC[01]
+              - renesas,iic-r8a7744      # RZ/G1N IIC[01]
+              - renesas,iic-r8a7790      # R-Car H2 IIC[012]
+              - renesas,iic-r8a7791      # R-Car M2-W IIC[01]
+              - renesas,iic-r8a7793      # R-Car M2-N IIC[01]
+          - const: renesas,rcar-gen2-iic # R-Car Gen2 and RZ/G1
+          - const: renesas,rmobile-iic   # Generic
+
+      - items:
+          - enum:
+              - renesas,iic-r8a774a1     # RZ/G2M
+              - renesas,iic-r8a774b1     # RZ/G2N
+              - renesas,iic-r8a774e1     # RZ/G2H
+              - renesas,iic-r8a7795      # R-Car H3
+              - renesas,iic-r8a7796      # R-Car M3-W
+              - renesas,iic-r8a77961     # R-Car M3-W+
+              - renesas,iic-r8a77965     # R-Car M3-N
+          - const: renesas,rcar-gen3-iic # R-Car Gen3 and RZ/G2
+          - const: renesas,rmobile-iic   # Generic
+
+      - items:
+            # Instances lacking automatic transmission registers are not
+            # considered compatible with family-specific or generic versions
+          - enum:
+              - renesas,iic-r8a7742      # RZ/G1H IIC3
+              - renesas,iic-r8a7743      # RZ/G1M IIC3
+              - renesas,iic-r8a7744      # RZ/G1N IIC3
+              - renesas,iic-r8a7745      # RZ/G1E
+              - renesas,iic-r8a774c0     # RZ/G2E
+              - renesas,iic-r8a7790      # R-Car H2 IIC3
+              - renesas,iic-r8a7791      # R-Car M2-W IIC3
+              - renesas,iic-r8a7792      # R-Car V2H IIC3
+              - renesas,iic-r8a7793      # R-Car M2-N IIC3
+              - renesas,iic-r8a7794      # R-Car E2
+              - renesas,iic-r8a77990     # R-Car E3
+
+  reg:
+    maxItems: 1
+
+  interrupts: true
+
+  clock-frequency:
+    description:
+      Desired I2C bus clock frequency in Hz. The absence of this property
+      indicates the default frequency 100 kHz.
+
+  clocks:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  dmas:
+    minItems: 2
+    maxItems: 4
+    description:
+      Must contain a list of pairs of references to DMA specifiers, one for
+      transmission, and one for reception.
+
+  dma-names:
+    minItems: 2
+    maxItems: 4
+    items:
+      enum:
+        - tx
+        - rx
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - power-domains
+  - '#address-cells'
+  - '#size-cells'
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - renesas,iic-r8a7740
+              - renesas,iic-sh73a0
+    then:
+      properties:
+        interrupts:
+          items:
+            - description: Arbitration Lost Interrupt (ALI)
+            - description: Non-acknowledge Detection Interrupt (TACKI)
+            - description: Wait Interrupt (WAITI)
+            - description: Data Transmit Enable interrupt (DTEI)
+    else:
+      properties:
+        interrupts:
+          items:
+            - description: Single combined interrupt
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - renesas,rcar-gen2-iic
+              - renesas,rcar-gen3-iic
+    then:
+      required:
+        - resets
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r8a7790-cpg-mssr.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/power/r8a7790-sysc.h>
+
+    iic0: i2c@e6500000 {
+            compatible = "renesas,iic-r8a7790", "renesas,rcar-gen2-iic",
+                         "renesas,rmobile-iic";
+            reg = <0xe6500000 0x425>;
+            interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&cpg CPG_MOD 318>;
+            clock-frequency = <400000>;
+            dmas = <&dmac0 0x61>, <&dmac0 0x62>, <&dmac1 0x61>, <&dmac1 0x62>;
+            dma-names = "tx", "rx", "tx", "rx";
+            power-domains = <&sysc R8A7790_PD_ALWAYS_ON>;
+            resets = <&cpg 318>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 8a7ca33961d49daa..c8834d7a1aac0efd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15453,7 +15453,7 @@  RENESAS R-CAR I2C DRIVERS
 M:	Wolfram Sang <wsa+renesas@sang-engineering.com>
 S:	Supported
 F:	Documentation/devicetree/bindings/i2c/renesas,rcar-i2c.yaml
-F:	Documentation/devicetree/bindings/i2c/renesas,iic.txt
+F:	Documentation/devicetree/bindings/i2c/renesas,rmobile-iic.yaml
 F:	drivers/i2c/busses/i2c-rcar.c
 F:	drivers/i2c/busses/i2c-sh_mobile.c