diff mbox series

[03/17] ARM: dts: r8a7742: Add I2C and IIC support

Message ID 1589555337-5498-4-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com
State Not Applicable
Delegated to: David Miller
Headers show
Series RZ/G1H describe I2C, IIC, MMC0, SATA, AVB, RWDT and APMU nodes | expand

Commit Message

Lad Prabhakar May 15, 2020, 3:08 p.m. UTC
Add the I2C[0-3] and IIC[0-3] devices nodes to the R8A7742 device tree.

Automatic transmission for PMIC control is not available on IIC3 hence
compatible string "renesas,rcar-gen2-iic" and "renesas,rmobile-iic" is
not added to iic3 node.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
---
 arch/arm/boot/dts/r8a7742.dtsi | 122 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 122 insertions(+)

Comments

Wolfram Sang May 15, 2020, 5:10 p.m. UTC | #1
On Fri, May 15, 2020 at 04:08:43PM +0100, Lad Prabhakar wrote:
> Add the I2C[0-3] and IIC[0-3] devices nodes to the R8A7742 device tree.
> 
> Automatic transmission for PMIC control is not available on IIC3 hence
> compatible string "renesas,rcar-gen2-iic" and "renesas,rmobile-iic" is
> not added to iic3 node.

Makes sense.

However, both versions (with and without automatic transmission) are
described with the same "renesas,iic-r8a7742" compatible. Is it possible
to detect the reduced variant at runtime somehow?

My concern is that the peculiarity of this SoC might be forgotten if we
describe it like this and ever add "automatic transmissions" somewhen.
Lad, Prabhakar May 18, 2020, 8:58 a.m. UTC | #2
Hi Wolfram,

Thank for the review.

On Fri, May 15, 2020 at 6:10 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> On Fri, May 15, 2020 at 04:08:43PM +0100, Lad Prabhakar wrote:
> > Add the I2C[0-3] and IIC[0-3] devices nodes to the R8A7742 device tree.
> >
> > Automatic transmission for PMIC control is not available on IIC3 hence
> > compatible string "renesas,rcar-gen2-iic" and "renesas,rmobile-iic" is
> > not added to iic3 node.
>
> Makes sense.
>
> However, both versions (with and without automatic transmission) are
> described with the same "renesas,iic-r8a7742" compatible. Is it possible
> to detect the reduced variant at runtime somehow?
>
I couldn't find anything the manual that would be useful to detect at runtime.

> My concern is that the peculiarity of this SoC might be forgotten if we
> describe it like this and ever add "automatic transmissions" somewhen.
>
Agreed.

Cheers,
--Prabhakar
Wolfram Sang May 18, 2020, 9:26 a.m. UTC | #3
> > However, both versions (with and without automatic transmission) are
> > described with the same "renesas,iic-r8a7742" compatible. Is it possible
> > to detect the reduced variant at runtime somehow?
> >
> I couldn't find anything the manual that would be useful to detect at runtime.
> 
> > My concern is that the peculiarity of this SoC might be forgotten if we
> > describe it like this and ever add "automatic transmissions" somewhen.
> >
> Agreed.

Well, I guess reading from a register which is supposed to not be there
on the modified IP core is too hackish.

Leaves us with a seperate compatible entry for it?
Lad, Prabhakar May 18, 2020, 9:43 a.m. UTC | #4
Hi Wolfram,

On Mon, May 18, 2020 at 10:26 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
>
> > > However, both versions (with and without automatic transmission) are
> > > described with the same "renesas,iic-r8a7742" compatible. Is it possible
> > > to detect the reduced variant at runtime somehow?
> > >
> > I couldn't find anything the manual that would be useful to detect at runtime.
> >
> > > My concern is that the peculiarity of this SoC might be forgotten if we
> > > describe it like this and ever add "automatic transmissions" somewhen.
> > >
> > Agreed.
>
> Well, I guess reading from a register which is supposed to not be there
> on the modified IP core is too hackish.
>
> Leaves us with a seperate compatible entry for it?
>
Sounds okay to me, how about "renesas,iic-no-dvfs" ? So that this
could be used on all the SoC's which don't support DVFS.

Cheers,
--Prabhakar
Wolfram Sang May 18, 2020, 9:53 a.m. UTC | #5
Hi Prabhakar,

> > Leaves us with a seperate compatible entry for it?
> >
> Sounds okay to me, how about "renesas,iic-no-dvfs" ? So that this
> could be used on all the SoC's which don't support DVFS.

Well, the feature missing is used for DVFS, but its name is "automatic
transmission". So, I'd rather suggest "-no-auto" as suffix. Also, there
are already quite some IIC variants out there, so plain "iic" won't
catch them all. My suggestion would be "renesas,rcar-gen2-iic-no-auto".

All the best,

   Wolfram
Geert Uytterhoeven May 18, 2020, 10:10 a.m. UTC | #6
Hi Wolfram,

On Mon, May 18, 2020 at 11:26 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> > > However, both versions (with and without automatic transmission) are
> > > described with the same "renesas,iic-r8a7742" compatible. Is it possible
> > > to detect the reduced variant at runtime somehow?
> > >
> > I couldn't find anything the manual that would be useful to detect at runtime.

Hence if we really need that (see below), we need a quirk based on compatible
value + base address.

> > > My concern is that the peculiarity of this SoC might be forgotten if we
> > > describe it like this and ever add "automatic transmissions" somewhen.
> > >
> > Agreed.
>
> Well, I guess reading from a register which is supposed to not be there
> on the modified IP core is too hackish.

According to the Hardware User's Manual Rev. 1.00, the registers do exist
on all RZ/G1, except for RZ/G1E (see below).

   "(automatic transmission can be used as a hardware function, but this is
    not meaningful for actual use cases)."

(whatever that comment may mean?)

> Leaves us with a seperate compatible entry for it?

On R-Car E3 and RZ/G2E, which have a single IIC instance, we
handled that by:

        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.

On R-Car E2 and RZ/G1E, we forgot, and used both SoC-specific and
family-specific compatible values.

Gr{oetje,eeting}s,

                        Geert
Lad, Prabhakar May 22, 2020, 7:14 p.m. UTC | #7
Hi Wolfram,

On Mon, May 18, 2020 at 11:10 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Wolfram,
>
> On Mon, May 18, 2020 at 11:26 AM Wolfram Sang
> <wsa+renesas@sang-engineering.com> wrote:
> > > > However, both versions (with and without automatic transmission) are
> > > > described with the same "renesas,iic-r8a7742" compatible. Is it possible
> > > > to detect the reduced variant at runtime somehow?
> > > >
> > > I couldn't find anything the manual that would be useful to detect at runtime.
>
> Hence if we really need that (see below), we need a quirk based on compatible
> value + base address.
>
> > > > My concern is that the peculiarity of this SoC might be forgotten if we
> > > > describe it like this and ever add "automatic transmissions" somewhen.
> > > >
> > > Agreed.
> >
> > Well, I guess reading from a register which is supposed to not be there
> > on the modified IP core is too hackish.
>
> According to the Hardware User's Manual Rev. 1.00, the registers do exist
> on all RZ/G1, except for RZ/G1E (see below).
>
>    "(automatic transmission can be used as a hardware function, but this is
>     not meaningful for actual use cases)."
>
> (whatever that comment may mean?)
>
> > Leaves us with a seperate compatible entry for it?
>
> On R-Car E3 and RZ/G2E, which have a single IIC instance, we
> handled that by:
>
>         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.
>
> On R-Car E2 and RZ/G1E, we forgot, and used both SoC-specific and
> family-specific compatible values.
>
What are your thoughts on the above.

Cheers,
--Prabhakar

> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
Wolfram Sang May 22, 2020, 8:17 p.m. UTC | #8
> > According to the Hardware User's Manual Rev. 1.00, the registers do exist
> > on all RZ/G1, except for RZ/G1E (see below).
> >
> >    "(automatic transmission can be used as a hardware function, but this is
> >     not meaningful for actual use cases)."
> >
> > (whatever that comment may mean?)

Strange comment, in deed. Given the paragraph before, I would guess Gen1
maybe had a "fitting" PMIC where SoC/PMIC handled DVFS kind of magically
with this automatic transfer feature? And Gen2 has not.

> > On R-Car E3 and RZ/G2E, which have a single IIC instance, we
> > handled that by:
> >
> >         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.

From a "describe the HW" point of view, this still makes sense to me.
Although, it is unlikely we will add support for the automatic
transmission feature (maybe famous last words).

> > On R-Car E2 and RZ/G1E, we forgot, and used both SoC-specific and
> > family-specific compatible values.

Okay, but we can fix DTs when they have bugs, or?
Geert Uytterhoeven May 25, 2020, 8:23 a.m. UTC | #9
Hi Wolfram,

On Fri, May 22, 2020 at 10:17 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> > > According to the Hardware User's Manual Rev. 1.00, the registers do exist
> > > on all RZ/G1, except for RZ/G1E (see below).
> > >
> > >    "(automatic transmission can be used as a hardware function, but this is
> > >     not meaningful for actual use cases)."
> > >
> > > (whatever that comment may mean?)
>
> Strange comment, in deed. Given the paragraph before, I would guess Gen1
> maybe had a "fitting" PMIC where SoC/PMIC handled DVFS kind of magically
> with this automatic transfer feature? And Gen2 has not.
>
> > > On R-Car E3 and RZ/G2E, which have a single IIC instance, we
> > > handled that by:
> > >
> > >         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.
>
> From a "describe the HW" point of view, this still makes sense to me.
> Although, it is unlikely we will add support for the automatic
> transmission feature (maybe famous last words).

;-)

> > > On R-Car E2 and RZ/G1E, we forgot, and used both SoC-specific and
> > > family-specific compatible values.
>
> Okay, but we can fix DTs when they have bugs, or?

We can.  But we also have to consider DT backwards compatibility: i.e.
using an old DTB with a future kernel implementing the automatic
transmission feature.

Fortunately R-Car E2 and RZ/G1E have SoC-specific compatible values,
so we can easily blacklist it in the driver based on that.
Blacklisting the last instance on the other SoCs is uglier, as it needs a
quirk that checks both the SoC-compatible value and the absence of the
generic compatible value. But it can still be done.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.9.

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/r8a7742.dtsi b/arch/arm/boot/dts/r8a7742.dtsi
index 305d808..f28c32d 100644
--- a/arch/arm/boot/dts/r8a7742.dtsi
+++ b/arch/arm/boot/dts/r8a7742.dtsi
@@ -359,6 +359,128 @@ 
 			ranges = <0 0 0xe6300000 0x40000>;
 		};
 
+		i2c0: i2c@e6508000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a7742",
+				     "renesas,rcar-gen2-i2c";
+			reg = <0 0xe6508000 0 0x40>;
+			interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 931>;
+			power-domains = <&sysc R8A7742_PD_ALWAYS_ON>;
+			resets = <&cpg 931>;
+			i2c-scl-internal-delay-ns = <110>;
+			status = "disabled";
+		};
+
+		i2c1: i2c@e6518000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a7742",
+				     "renesas,rcar-gen2-i2c";
+			reg = <0 0xe6518000 0 0x40>;
+			interrupts = <GIC_SPI 288 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 930>;
+			power-domains = <&sysc R8A7742_PD_ALWAYS_ON>;
+			resets = <&cpg 930>;
+			i2c-scl-internal-delay-ns = <6>;
+			status = "disabled";
+		};
+
+		i2c2: i2c@e6530000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a7742",
+				     "renesas,rcar-gen2-i2c";
+			reg = <0 0xe6530000 0 0x40>;
+			interrupts = <GIC_SPI 286 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 929>;
+			power-domains = <&sysc R8A7742_PD_ALWAYS_ON>;
+			resets = <&cpg 929>;
+			i2c-scl-internal-delay-ns = <6>;
+			status = "disabled";
+		};
+
+		i2c3: i2c@e6540000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a7742",
+				     "renesas,rcar-gen2-i2c";
+			reg = <0 0xe6540000 0 0x40>;
+			interrupts = <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 928>;
+			power-domains = <&sysc R8A7742_PD_ALWAYS_ON>;
+			resets = <&cpg 928>;
+			i2c-scl-internal-delay-ns = <110>;
+			status = "disabled";
+		};
+
+		iic0: i2c@e6500000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,iic-r8a7742",
+				     "renesas,rcar-gen2-iic",
+				     "renesas,rmobile-iic";
+			reg = <0 0xe6500000 0 0x425>;
+			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 318>;
+			dmas = <&dmac0 0x61>, <&dmac0 0x62>,
+			       <&dmac1 0x61>, <&dmac1 0x62>;
+			dma-names = "tx", "rx", "tx", "rx";
+			power-domains = <&sysc R8A7742_PD_ALWAYS_ON>;
+			resets = <&cpg 318>;
+			status = "disabled";
+		};
+
+		iic1: i2c@e6510000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,iic-r8a7742",
+				     "renesas,rcar-gen2-iic",
+				     "renesas,rmobile-iic";
+			reg = <0 0xe6510000 0 0x425>;
+			interrupts = <GIC_SPI 175 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 323>;
+			dmas = <&dmac0 0x65>, <&dmac0 0x66>,
+			       <&dmac1 0x65>, <&dmac1 0x66>;
+			dma-names = "tx", "rx", "tx", "rx";
+			power-domains = <&sysc R8A7742_PD_ALWAYS_ON>;
+			resets = <&cpg 323>;
+			status = "disabled";
+		};
+
+		iic2: i2c@e6520000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,iic-r8a7742",
+				     "renesas,rcar-gen2-iic",
+				     "renesas,rmobile-iic";
+			reg = <0 0xe6520000 0 0x425>;
+			interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 300>;
+			dmas = <&dmac0 0x69>, <&dmac0 0x6a>,
+			       <&dmac1 0x69>, <&dmac1 0x6a>;
+			dma-names = "tx", "rx", "tx", "rx";
+			power-domains = <&sysc R8A7742_PD_ALWAYS_ON>;
+			resets = <&cpg 300>;
+			status = "disabled";
+		};
+
+		iic3: i2c@e60b0000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,iic-r8a7742";
+			reg = <0 0xe60b0000 0 0x425>;
+			interrupts = <GIC_SPI 173 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 926>;
+			dmas = <&dmac0 0x77>, <&dmac0 0x78>,
+			       <&dmac1 0x77>, <&dmac1 0x78>;
+			dma-names = "tx", "rx", "tx", "rx";
+			power-domains = <&sysc R8A7742_PD_ALWAYS_ON>;
+			resets = <&cpg 926>;
+			status = "disabled";
+		};
+
 		dmac0: dma-controller@e6700000 {
 			compatible = "renesas,dmac-r8a7742",
 				     "renesas,rcar-dmac";