| Message ID | 20250126134616.37334-6-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add RZ/G3E SDHI support | expand |
On Sun, Jan 26, 2025 at 01:46:07PM +0000, Biju Das wrote: > Add SDHI0-SDHI2 nodes to RZ/G3E ("R9A09G047") SoC DTSI. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > This patch depend upon [1] > [1] https://lore.kernel.org/all/20250120094715.25802-12-biju.das.jz@bp.renesas.com/ > --- > arch/arm64/boot/dts/renesas/r9a09g047.dtsi | 57 ++++++++++++++++++++++ > 1 file changed, 57 insertions(+) > > diff --git a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi > index 2023f70d3329..099d13b83f18 100644 > --- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi > +++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi > @@ -518,6 +518,63 @@ gic: interrupt-controller@14900000 { > interrupt-controller; > interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>; > }; > + > + sdhi0: mmc@15c00000 { > + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; > + reg = <0x0 0x15c00000 0 0x10000>; > + interrupts = <GIC_SPI 735 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 736 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 0xa3>, <&cpg CPG_MOD 0xa5>, > + <&cpg CPG_MOD 0xa4>, <&cpg CPG_MOD 0xa6>; > + clock-names = "core", "clkh", "cd", "aclk"; > + resets = <&cpg 0xa7>; > + power-domains = <&cpg>; > + status = "disabled"; > + > + vqmmc_sdhi0: vqmmc-regulator { > + regulator-name = "SDHI0-VQMMC"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + }; > + }; > + > + sdhi1: mmc@15c10000 { > + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; > + reg = <0x0 0x15c10000 0 0x10000>; > + interrupts = <GIC_SPI 737 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 738 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 0xa7>, <&cpg CPG_MOD 0xa9>, > + <&cpg CPG_MOD 0xa8>, <&cpg CPG_MOD 0xaa>; > + clock-names = "core", "clkh", "cd", "aclk"; > + resets = <&cpg 0xa8>; > + power-domains = <&cpg>; > + status = "disabled"; > + > + vqmmc_sdhi1: vqmmc-regulator { > + regulator-name = "SDHI1-VQMMC"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + }; > + }; > + > + sdhi2: mmc@15c20000 { > + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; > + reg = <0x0 0x15c20000 0 0x10000>; > + interrupts = <GIC_SPI 739 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 740 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 0xab>, <&cpg CPG_MOD 0xad>, > + <&cpg CPG_MOD 0xac>, <&cpg CPG_MOD 0xae>; > + clock-names = "core", "clkh", "cd", "aclk"; > + resets = <&cpg 0xa9>; > + power-domains = <&cpg>; > + status = "disabled"; > + > + vqmmc_sdhi2: vqmmc-regulator { > + regulator-name = "SDHI2-VQMMC"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + }; > + }; > }; > > timer { > -- > 2.43.0 > Reviewed-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
Hi Biju, On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote: > Add SDHI0-SDHI2 nodes to RZ/G3E ("R9A09G047") SoC DTSI. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> Thanks for your patch! > --- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi > +++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi > @@ -518,6 +518,63 @@ gic: interrupt-controller@14900000 { > interrupt-controller; > interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>; > }; > + > + sdhi0: mmc@15c00000 { > + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; > + reg = <0x0 0x15c00000 0 0x10000>; > + interrupts = <GIC_SPI 735 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 736 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 0xa3>, <&cpg CPG_MOD 0xa5>, > + <&cpg CPG_MOD 0xa4>, <&cpg CPG_MOD 0xa6>; > + clock-names = "core", "clkh", "cd", "aclk"; > + resets = <&cpg 0xa7>; > + power-domains = <&cpg>; > + status = "disabled"; > + > + vqmmc_sdhi0: vqmmc-regulator { > + regulator-name = "SDHI0-VQMMC"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + }; > + }; > + > + sdhi1: mmc@15c10000 { > + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; > + reg = <0x0 0x15c10000 0 0x10000>; > + interrupts = <GIC_SPI 737 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 738 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 0xa7>, <&cpg CPG_MOD 0xa9>, > + <&cpg CPG_MOD 0xa8>, <&cpg CPG_MOD 0xaa>; > + clock-names = "core", "clkh", "cd", "aclk"; > + resets = <&cpg 0xa8>; > + power-domains = <&cpg>; > + status = "disabled"; > + > + vqmmc_sdhi1: vqmmc-regulator { > + regulator-name = "SDHI1-VQMMC"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + }; > + }; > + > + sdhi2: mmc@15c20000 { > + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; > + reg = <0x0 0x15c20000 0 0x10000>; > + interrupts = <GIC_SPI 739 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 740 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 0xab>, <&cpg CPG_MOD 0xad>, > + <&cpg CPG_MOD 0xac>, <&cpg CPG_MOD 0xae>; > + clock-names = "core", "clkh", "cd", "aclk"; > + resets = <&cpg 0xa9>; > + power-domains = <&cpg>; > + status = "disabled"; > + > + vqmmc_sdhi2: vqmmc-regulator { > + regulator-name = "SDHI2-VQMMC"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + }; > + }; > }; > > timer { Shouldn't the vqmmc-regulator subnodes be added in the board DTS, when needed (i.e. at least for SDHI[12])? Or do you expect the board DTS to /delete-node/ them when they are not needed? Is it possible that SDHI0 does not need the regulator control, e.g. in case of a fixed voltage? The rest LGTM. Gr{oetje,eeting}s, Geert
Hi Geert, Thanks for the feedback. > -----Original Message----- > From: Geert Uytterhoeven <geert@linux-m68k.org> > Sent: 28 January 2025 11:33 Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; > biju.das.au <biju.das.au@gmail.com> > Subject: Re: [PATCH 5/7] arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes > > Hi Biju, > > On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote: > > Add SDHI0-SDHI2 nodes to RZ/G3E ("R9A09G047") SoC DTSI. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Thanks for your patch! > > > --- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi > > +++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi > > @@ -518,6 +518,63 @@ gic: interrupt-controller@14900000 { > > interrupt-controller; > > interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>; > > }; > > + > > + sdhi0: mmc@15c00000 { > > + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; > > + reg = <0x0 0x15c00000 0 0x10000>; > > + interrupts = <GIC_SPI 735 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 736 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&cpg CPG_MOD 0xa3>, <&cpg CPG_MOD 0xa5>, > > + <&cpg CPG_MOD 0xa4>, <&cpg CPG_MOD 0xa6>; > > + clock-names = "core", "clkh", "cd", "aclk"; > > + resets = <&cpg 0xa7>; > > + power-domains = <&cpg>; > > + status = "disabled"; > > + > > + vqmmc_sdhi0: vqmmc-regulator { > > + regulator-name = "SDHI0-VQMMC"; > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <3300000>; > > + }; > > + }; > > + > > + sdhi1: mmc@15c10000 { > > + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; > > + reg = <0x0 0x15c10000 0 0x10000>; > > + interrupts = <GIC_SPI 737 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 738 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&cpg CPG_MOD 0xa7>, <&cpg CPG_MOD 0xa9>, > > + <&cpg CPG_MOD 0xa8>, <&cpg CPG_MOD 0xaa>; > > + clock-names = "core", "clkh", "cd", "aclk"; > > + resets = <&cpg 0xa8>; > > + power-domains = <&cpg>; > > + status = "disabled"; > > + > > + vqmmc_sdhi1: vqmmc-regulator { > > + regulator-name = "SDHI1-VQMMC"; > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <3300000>; > > + }; > > + }; > > + > > + sdhi2: mmc@15c20000 { > > + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; > > + reg = <0x0 0x15c20000 0 0x10000>; > > + interrupts = <GIC_SPI 739 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 740 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&cpg CPG_MOD 0xab>, <&cpg CPG_MOD 0xad>, > > + <&cpg CPG_MOD 0xac>, <&cpg CPG_MOD 0xae>; > > + clock-names = "core", "clkh", "cd", "aclk"; > > + resets = <&cpg 0xa9>; > > + power-domains = <&cpg>; > > + status = "disabled"; > > + > > + vqmmc_sdhi2: vqmmc-regulator { > > + regulator-name = "SDHI2-VQMMC"; > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <3300000>; > > + }; > > + }; > > }; > > > > timer { > > Shouldn't the vqmmc-regulator subnodes be added in the board DTS, when needed (i.e. at least for > SDHI[12])? Or do you expect the board DTS to /delete-node/ them when they are not needed? I agree. I have provided an example in next patch using /delete-node/ to use gpio-regulator. I am ok for moving it to the board DTS as well. When I sent patch, I am not sure which is the best in terms of user point of view? Now I got the answer to move vqmmc-regulator subnodes to add in the board DTS for atleast SDHI[12]. I will address this in next version. Even for SDHI0 fix type, if we plan to use fixed regulator for eMMC? > > Is it possible that SDHI0 does not need the regulator control, e.g. > in case of a fixed voltage? Yes, for eMMC(fixed case) it is not needed. Without Regulator: ------------------ root@smarc-rzg3e:~# /sdhi_test.sh Read at address 0x15C003C8 (0xffffb3f983c8): 0x00000001 [ 1.884227] mmc0: new HS200 MMC card at address 0001 [ 1.890480] mmcblk0: mmc0:0001 0IM20F 59.3 GiB [ 1.898777] mmcblk0: p1 $ dd if=/dev/urandom of=/tmp/mmcblk0p1-random bs=1024 count=10240 10240+0 records in 10240+0 records out 10485760 bytes (10 MB, 10 MiB) copied, 0.131063 s, 80.0 MB/s With regulator: ---------------- root@smarc-rzg3e:~# /sdhi_test.sh Read at address 0x15C003C8 (0xffffa1ca73c8): 0x00010001 [ 1.853728] mmc0: new HS200 MMC card at address 0001 [ 1.864529] mmcblk0: mmc0:0001 0IM20F 59.3 GiB [ 1.873859] mmcblk0: p1 $ dd if=/dev/urandom of=/tmp/mmcblk0p1-random bs=1024 count=10240 10240+0 records in 10240+0 records out 10485760 bytes (10 MB, 10 MiB) copied, 0.131909 s, 79.5 MB/s Cheers, Biju
Hi Biju, On Tue, 28 Jan 2025 at 13:11, Biju Das <biju.das.jz@bp.renesas.com> wrote: > > -----Original Message----- > > From: Geert Uytterhoeven <geert@linux-m68k.org> > > Sent: 28 January 2025 11:33 > Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; > > biju.das.au <biju.das.au@gmail.com> > > Subject: Re: [PATCH 5/7] arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes > > > > On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > Add SDHI0-SDHI2 nodes to RZ/G3E ("R9A09G047") SoC DTSI. > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > Thanks for your patch! > > > > > --- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi > > > +++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi > > > @@ -518,6 +518,63 @@ gic: interrupt-controller@14900000 { > > > interrupt-controller; > > > interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>; > > > }; > > > + > > > + sdhi0: mmc@15c00000 { > > > + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; > > > + reg = <0x0 0x15c00000 0 0x10000>; > > > + interrupts = <GIC_SPI 735 IRQ_TYPE_LEVEL_HIGH>, > > > + <GIC_SPI 736 IRQ_TYPE_LEVEL_HIGH>; > > > + clocks = <&cpg CPG_MOD 0xa3>, <&cpg CPG_MOD 0xa5>, > > > + <&cpg CPG_MOD 0xa4>, <&cpg CPG_MOD 0xa6>; > > > + clock-names = "core", "clkh", "cd", "aclk"; > > > + resets = <&cpg 0xa7>; > > > + power-domains = <&cpg>; > > > + status = "disabled"; > > > + > > > + vqmmc_sdhi0: vqmmc-regulator { > > > + regulator-name = "SDHI0-VQMMC"; > > > + regulator-min-microvolt = <1800000>; > > > + regulator-max-microvolt = <3300000>; > > > + }; > > > + }; > > > + > > > + sdhi1: mmc@15c10000 { > > > + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; > > > + reg = <0x0 0x15c10000 0 0x10000>; > > > + interrupts = <GIC_SPI 737 IRQ_TYPE_LEVEL_HIGH>, > > > + <GIC_SPI 738 IRQ_TYPE_LEVEL_HIGH>; > > > + clocks = <&cpg CPG_MOD 0xa7>, <&cpg CPG_MOD 0xa9>, > > > + <&cpg CPG_MOD 0xa8>, <&cpg CPG_MOD 0xaa>; > > > + clock-names = "core", "clkh", "cd", "aclk"; > > > + resets = <&cpg 0xa8>; > > > + power-domains = <&cpg>; > > > + status = "disabled"; > > > + > > > + vqmmc_sdhi1: vqmmc-regulator { > > > + regulator-name = "SDHI1-VQMMC"; > > > + regulator-min-microvolt = <1800000>; > > > + regulator-max-microvolt = <3300000>; > > > + }; > > > + }; > > > + > > > + sdhi2: mmc@15c20000 { > > > + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; > > > + reg = <0x0 0x15c20000 0 0x10000>; > > > + interrupts = <GIC_SPI 739 IRQ_TYPE_LEVEL_HIGH>, > > > + <GIC_SPI 740 IRQ_TYPE_LEVEL_HIGH>; > > > + clocks = <&cpg CPG_MOD 0xab>, <&cpg CPG_MOD 0xad>, > > > + <&cpg CPG_MOD 0xac>, <&cpg CPG_MOD 0xae>; > > > + clock-names = "core", "clkh", "cd", "aclk"; > > > + resets = <&cpg 0xa9>; > > > + power-domains = <&cpg>; > > > + status = "disabled"; > > > + > > > + vqmmc_sdhi2: vqmmc-regulator { > > > + regulator-name = "SDHI2-VQMMC"; > > > + regulator-min-microvolt = <1800000>; > > > + regulator-max-microvolt = <3300000>; > > > + }; > > > + }; > > > }; > > > > > > timer { > > > > Shouldn't the vqmmc-regulator subnodes be added in the board DTS, when needed (i.e. at least for > > SDHI[12])? Or do you expect the board DTS to /delete-node/ them when they are not needed? > > I agree. > > I have provided an example in next patch using /delete-node/ to use gpio-regulator. Ah, my fault trying to get my reviews out sooner rather than later ;-) > I am ok for moving it to the board DTS as well. When I sent patch, I am not sure which > is the best in terms of user point of view? > > Now I got the answer to move vqmmc-regulator subnodes to add in the board DTS for > atleast SDHI[12]. I will address this in next version. > > Even for SDHI0 fix type, if we plan to use fixed regulator for eMMC? > > > > > Is it possible that SDHI0 does not need the regulator control, e.g. > > in case of a fixed voltage? > > Yes, for eMMC(fixed case) it is not needed. Upon second thought: as the internal regulator is always present, what about setting its status to disabled in the SoC .dtsi, and changing it to okay in the board DTS when needed, like done for other components? Gr{oetje,eeting}s, Geert
Hi Geert, > -----Original Message----- > From: Geert Uytterhoeven <geert@linux-m68k.org> > Sent: 28 January 2025 13:26 > Subject: Re: [PATCH 5/7] arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes > > Hi Biju, > > On Tue, 28 Jan 2025 at 13:11, Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > -----Original Message----- > > > From: Geert Uytterhoeven <geert@linux-m68k.org> > > > Sent: 28 January 2025 11:33 > > Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; > > > biju.das.au <biju.das.au@gmail.com> > > > Subject: Re: [PATCH 5/7] arm64: dts: renesas: r9a09g047: Add > > > SDHI0-SDHI2 nodes > > > > > > On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > > Add SDHI0-SDHI2 nodes to RZ/G3E ("R9A09G047") SoC DTSI. > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > > Thanks for your patch! > > > > > > > --- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi > > > > +++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi > > > > @@ -518,6 +518,63 @@ gic: interrupt-controller@14900000 { > > > > interrupt-controller; > > > > interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>; > > > > }; > > > > + > > > > + sdhi0: mmc@15c00000 { > > > > + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; > > > > + reg = <0x0 0x15c00000 0 0x10000>; > > > > + interrupts = <GIC_SPI 735 IRQ_TYPE_LEVEL_HIGH>, > > > > + <GIC_SPI 736 IRQ_TYPE_LEVEL_HIGH>; > > > > + clocks = <&cpg CPG_MOD 0xa3>, <&cpg CPG_MOD 0xa5>, > > > > + <&cpg CPG_MOD 0xa4>, <&cpg CPG_MOD 0xa6>; > > > > + clock-names = "core", "clkh", "cd", "aclk"; > > > > + resets = <&cpg 0xa7>; > > > > + power-domains = <&cpg>; > > > > + status = "disabled"; > > > > + > > > > + vqmmc_sdhi0: vqmmc-regulator { > > > > + regulator-name = "SDHI0-VQMMC"; > > > > + regulator-min-microvolt = <1800000>; > > > > + regulator-max-microvolt = <3300000>; > > > > + }; > > > > + }; > > > > + > > > > + sdhi1: mmc@15c10000 { > > > > + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; > > > > + reg = <0x0 0x15c10000 0 0x10000>; > > > > + interrupts = <GIC_SPI 737 IRQ_TYPE_LEVEL_HIGH>, > > > > + <GIC_SPI 738 IRQ_TYPE_LEVEL_HIGH>; > > > > + clocks = <&cpg CPG_MOD 0xa7>, <&cpg CPG_MOD 0xa9>, > > > > + <&cpg CPG_MOD 0xa8>, <&cpg CPG_MOD 0xaa>; > > > > + clock-names = "core", "clkh", "cd", "aclk"; > > > > + resets = <&cpg 0xa8>; > > > > + power-domains = <&cpg>; > > > > + status = "disabled"; > > > > + > > > > + vqmmc_sdhi1: vqmmc-regulator { > > > > + regulator-name = "SDHI1-VQMMC"; > > > > + regulator-min-microvolt = <1800000>; > > > > + regulator-max-microvolt = <3300000>; > > > > + }; > > > > + }; > > > > + > > > > + sdhi2: mmc@15c20000 { > > > > + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; > > > > + reg = <0x0 0x15c20000 0 0x10000>; > > > > + interrupts = <GIC_SPI 739 IRQ_TYPE_LEVEL_HIGH>, > > > > + <GIC_SPI 740 IRQ_TYPE_LEVEL_HIGH>; > > > > + clocks = <&cpg CPG_MOD 0xab>, <&cpg CPG_MOD 0xad>, > > > > + <&cpg CPG_MOD 0xac>, <&cpg CPG_MOD 0xae>; > > > > + clock-names = "core", "clkh", "cd", "aclk"; > > > > + resets = <&cpg 0xa9>; > > > > + power-domains = <&cpg>; > > > > + status = "disabled"; > > > > + > > > > + vqmmc_sdhi2: vqmmc-regulator { > > > > + regulator-name = "SDHI2-VQMMC"; > > > > + regulator-min-microvolt = <1800000>; > > > > + regulator-max-microvolt = <3300000>; > > > > + }; > > > > + }; > > > > }; > > > > > > > > timer { > > > > > > Shouldn't the vqmmc-regulator subnodes be added in the board DTS, > > > when needed (i.e. at least for SDHI[12])? Or do you expect the board DTS to /delete-node/ them > when they are not needed? > > > > I agree. > > > > I have provided an example in next patch using /delete-node/ to use gpio-regulator. > > Ah, my fault trying to get my reviews out sooner rather than later ;-) > > > I am ok for moving it to the board DTS as well. When I sent patch, I > > am not sure which is the best in terms of user point of view? > > > > Now I got the answer to move vqmmc-regulator subnodes to add in the > > board DTS for atleast SDHI[12]. I will address this in next version. > > > > Even for SDHI0 fix type, if we plan to use fixed regulator for eMMC? > > > > > > > > Is it possible that SDHI0 does not need the regulator control, e.g. > > > in case of a fixed voltage? > > > > Yes, for eMMC(fixed case) it is not needed. > > Upon second thought: as the internal regulator is always present, what about setting its status to > disabled in the SoC .dtsi, and changing it to okay in the board DTS when needed, like done for other > components? OK for me. Cheers, Biju
Hi Geert, Thanks for the feedback. > -----Original Message----- > From: Geert Uytterhoeven <geert@linux-m68k.org> > Sent: 28 January 2025 13:26 > Subject: Re: [PATCH 5/7] arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes > > Hi Biju, > > On Tue, 28 Jan 2025 at 13:11, Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > -----Original Message----- > > > From: Geert Uytterhoeven <geert@linux-m68k.org> > > > Sent: 28 January 2025 11:33 > > Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; > > > biju.das.au <biju.das.au@gmail.com> > > > Subject: Re: [PATCH 5/7] arm64: dts: renesas: r9a09g047: Add > > > SDHI0-SDHI2 nodes > > > > > > On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > > Add SDHI0-SDHI2 nodes to RZ/G3E ("R9A09G047") SoC DTSI. > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > > Thanks for your patch! > > > > > > > --- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi > > > > +++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi > > > > @@ -518,6 +518,63 @@ gic: interrupt-controller@14900000 { > > > > interrupt-controller; > > > > interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>; > > > > }; > > > > + > > > > + sdhi0: mmc@15c00000 { > > > > + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; > > > > + reg = <0x0 0x15c00000 0 0x10000>; > > > > + interrupts = <GIC_SPI 735 IRQ_TYPE_LEVEL_HIGH>, > > > > + <GIC_SPI 736 IRQ_TYPE_LEVEL_HIGH>; > > > > + clocks = <&cpg CPG_MOD 0xa3>, <&cpg CPG_MOD 0xa5>, > > > > + <&cpg CPG_MOD 0xa4>, <&cpg CPG_MOD 0xa6>; > > > > + clock-names = "core", "clkh", "cd", "aclk"; > > > > + resets = <&cpg 0xa7>; > > > > + power-domains = <&cpg>; > > > > + status = "disabled"; > > > > + > > > > + vqmmc_sdhi0: vqmmc-regulator { > > > > + regulator-name = "SDHI0-VQMMC"; > > > > + regulator-min-microvolt = <1800000>; > > > > + regulator-max-microvolt = <3300000>; > > > > + }; > > > > + }; > > > > + > > > > + sdhi1: mmc@15c10000 { > > > > + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; > > > > + reg = <0x0 0x15c10000 0 0x10000>; > > > > + interrupts = <GIC_SPI 737 IRQ_TYPE_LEVEL_HIGH>, > > > > + <GIC_SPI 738 IRQ_TYPE_LEVEL_HIGH>; > > > > + clocks = <&cpg CPG_MOD 0xa7>, <&cpg CPG_MOD 0xa9>, > > > > + <&cpg CPG_MOD 0xa8>, <&cpg CPG_MOD 0xaa>; > > > > + clock-names = "core", "clkh", "cd", "aclk"; > > > > + resets = <&cpg 0xa8>; > > > > + power-domains = <&cpg>; > > > > + status = "disabled"; > > > > + > > > > + vqmmc_sdhi1: vqmmc-regulator { > > > > + regulator-name = "SDHI1-VQMMC"; > > > > + regulator-min-microvolt = <1800000>; > > > > + regulator-max-microvolt = <3300000>; > > > > + }; > > > > + }; > > > > + > > > > + sdhi2: mmc@15c20000 { > > > > + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; > > > > + reg = <0x0 0x15c20000 0 0x10000>; > > > > + interrupts = <GIC_SPI 739 IRQ_TYPE_LEVEL_HIGH>, > > > > + <GIC_SPI 740 IRQ_TYPE_LEVEL_HIGH>; > > > > + clocks = <&cpg CPG_MOD 0xab>, <&cpg CPG_MOD 0xad>, > > > > + <&cpg CPG_MOD 0xac>, <&cpg CPG_MOD 0xae>; > > > > + clock-names = "core", "clkh", "cd", "aclk"; > > > > + resets = <&cpg 0xa9>; > > > > + power-domains = <&cpg>; > > > > + status = "disabled"; > > > > + > > > > + vqmmc_sdhi2: vqmmc-regulator { > > > > + regulator-name = "SDHI2-VQMMC"; > > > > + regulator-min-microvolt = <1800000>; > > > > + regulator-max-microvolt = <3300000>; > > > > + }; > > > > + }; > > > > }; > > > > > > > > timer { > > > > > > Shouldn't the vqmmc-regulator subnodes be added in the board DTS, > > > when needed (i.e. at least for SDHI[12])? Or do you expect the board DTS to /delete-node/ them > when they are not needed? > > > > I agree. > > > > I have provided an example in next patch using /delete-node/ to use gpio-regulator. > > Ah, my fault trying to get my reviews out sooner rather than later ;-) > > > I am ok for moving it to the board DTS as well. When I sent patch, I > > am not sure which is the best in terms of user point of view? > > > > Now I got the answer to move vqmmc-regulator subnodes to add in the > > board DTS for atleast SDHI[12]. I will address this in next version. > > > > Even for SDHI0 fix type, if we plan to use fixed regulator for eMMC? > > > > > > > > Is it possible that SDHI0 does not need the regulator control, e.g. > > > in case of a fixed voltage? > > > > Yes, for eMMC(fixed case) it is not needed. > > Upon second thought: as the internal regulator is always present, what about setting its status to > disabled in the SoC .dtsi, and changing it to okay in the board DTS when needed, like done for other > components? Agreed. Apart from that, I am planning the below changes for V2: SD0 fixed voltage(eMMC): fixed regulator based on SoM Schematics. SD2 : Internal regulator. There will be a new patch for supporting SD0 non fixed voltage(SD0) using internal regulator. Cheers, Biju
diff --git a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi index 2023f70d3329..099d13b83f18 100644 --- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi +++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi @@ -518,6 +518,63 @@ gic: interrupt-controller@14900000 { interrupt-controller; interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>; }; + + sdhi0: mmc@15c00000 { + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; + reg = <0x0 0x15c00000 0 0x10000>; + interrupts = <GIC_SPI 735 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 736 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 0xa3>, <&cpg CPG_MOD 0xa5>, + <&cpg CPG_MOD 0xa4>, <&cpg CPG_MOD 0xa6>; + clock-names = "core", "clkh", "cd", "aclk"; + resets = <&cpg 0xa7>; + power-domains = <&cpg>; + status = "disabled"; + + vqmmc_sdhi0: vqmmc-regulator { + regulator-name = "SDHI0-VQMMC"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + }; + }; + + sdhi1: mmc@15c10000 { + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; + reg = <0x0 0x15c10000 0 0x10000>; + interrupts = <GIC_SPI 737 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 738 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 0xa7>, <&cpg CPG_MOD 0xa9>, + <&cpg CPG_MOD 0xa8>, <&cpg CPG_MOD 0xaa>; + clock-names = "core", "clkh", "cd", "aclk"; + resets = <&cpg 0xa8>; + power-domains = <&cpg>; + status = "disabled"; + + vqmmc_sdhi1: vqmmc-regulator { + regulator-name = "SDHI1-VQMMC"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + }; + }; + + sdhi2: mmc@15c20000 { + compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057"; + reg = <0x0 0x15c20000 0 0x10000>; + interrupts = <GIC_SPI 739 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 740 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 0xab>, <&cpg CPG_MOD 0xad>, + <&cpg CPG_MOD 0xac>, <&cpg CPG_MOD 0xae>; + clock-names = "core", "clkh", "cd", "aclk"; + resets = <&cpg 0xa9>; + power-domains = <&cpg>; + status = "disabled"; + + vqmmc_sdhi2: vqmmc-regulator { + regulator-name = "SDHI2-VQMMC"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + }; + }; }; timer {
Add SDHI0-SDHI2 nodes to RZ/G3E ("R9A09G047") SoC DTSI. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- This patch depend upon [1] [1] https://lore.kernel.org/all/20250120094715.25802-12-biju.das.jz@bp.renesas.com/ --- arch/arm64/boot/dts/renesas/r9a09g047.dtsi | 57 ++++++++++++++++++++++ 1 file changed, 57 insertions(+)