| Message ID | 1589555337-5498-4-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RZ/G1H describe I2C, IIC, MMC0, SATA, AVB, RWDT and APMU nodes | expand |
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.
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
> > 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?
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
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
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
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
> > 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?
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 --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";