| Message ID | 1559895251-13931-1-git-send-email-fabrizio.castro@bp.renesas.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 015a75077d7b9d95ff882d0a6bbf0913df36a593 |
| Delegated to: | Simon Horman |
| Headers | show |
| Series | arm64: dts: renesas: hihope-common: Add uSD and eMMC | expand |
Hi Fabrizio, On Fri, Jun 7, 2019 at 10:14 AM Fabrizio Castro <fabrizio.castro@bp.renesas.com> wrote: > This patch adds uSD and eMMC support to the HiHope RZ/G2M > board. > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> Thanks for your patch! > --- a/arch/arm64/boot/dts/renesas/hihope-common.dtsi > +++ b/arch/arm64/boot/dts/renesas/hihope-common.dtsi > +&sdhi3 { > + pinctrl-0 = <&sdhi3_pins>; > + pinctrl-1 = <&sdhi3_pins>; > + pinctrl-names = "default", "state_uhs"; > + > + vmmc-supply = <®_3p3v>; > + vqmmc-supply = <®_1p8v>; > + bus-width = <8>; > + mmc-hs200-1_8v; Does the eMMC support HS400, too? > + non-removable; > + fixed-emmc-driver-type = <1>; > + status = "okay"; > +}; Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
Hello Geert, Thank you for your feedback! > From: Geert Uytterhoeven <geert@linux-m68k.org> > Sent: 12 June 2019 09:28 > Subject: Re: [PATCH] arm64: dts: renesas: hihope-common: Add uSD and eMMC > > Hi Fabrizio, > > On Fri, Jun 7, 2019 at 10:14 AM Fabrizio Castro > <fabrizio.castro@bp.renesas.com> wrote: > > This patch adds uSD and eMMC support to the HiHope RZ/G2M > > board. > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > Thanks for your patch! > > > --- a/arch/arm64/boot/dts/renesas/hihope-common.dtsi > > +++ b/arch/arm64/boot/dts/renesas/hihope-common.dtsi > > > +&sdhi3 { > > + pinctrl-0 = <&sdhi3_pins>; > > + pinctrl-1 = <&sdhi3_pins>; > > + pinctrl-names = "default", "state_uhs"; > > + > > + vmmc-supply = <®_3p3v>; > > + vqmmc-supply = <®_1p8v>; > > + bus-width = <8>; > > + mmc-hs200-1_8v; > > Does the eMMC support HS400, too? The eMMC does support HS400, but if I list it here the eMMC won't work properly as the SoC can't deal with it. Wolfram's patch blacklists HS400 for the RZ/G2M (revisions "ES1.[012]"): https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg40630.html I was thinking about sending a follow-up patch to list hs400 here after Wolfram's patch appears in a RC, would that be okay with you? Thanks, Fab > > > + non-removable; > > + fixed-emmc-driver-type = <1>; > > + status = "okay"; > > +}; > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > 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
Hi Fabrizio, On Wed, Jun 12, 2019 at 10:39 AM Fabrizio Castro <fabrizio.castro@bp.renesas.com> wrote: > > From: Geert Uytterhoeven <geert@linux-m68k.org> > > Sent: 12 June 2019 09:28 > > Subject: Re: [PATCH] arm64: dts: renesas: hihope-common: Add uSD and eMMC > > > > On Fri, Jun 7, 2019 at 10:14 AM Fabrizio Castro > > <fabrizio.castro@bp.renesas.com> wrote: > > > This patch adds uSD and eMMC support to the HiHope RZ/G2M > > > board. > > > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > > Thanks for your patch! > > > > > --- a/arch/arm64/boot/dts/renesas/hihope-common.dtsi > > > +++ b/arch/arm64/boot/dts/renesas/hihope-common.dtsi > > > > > +&sdhi3 { > > > + pinctrl-0 = <&sdhi3_pins>; > > > + pinctrl-1 = <&sdhi3_pins>; > > > + pinctrl-names = "default", "state_uhs"; > > > + > > > + vmmc-supply = <®_3p3v>; > > > + vqmmc-supply = <®_1p8v>; > > > + bus-width = <8>; > > > + mmc-hs200-1_8v; > > > > Does the eMMC support HS400, too? > > The eMMC does support HS400, but if I list it here the eMMC won't work properly as the SoC can't deal with it. > Wolfram's patch blacklists HS400 for the RZ/G2M (revisions "ES1.[012]"): > https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg40630.html > I was thinking about sending a follow-up patch to list hs400 here after Wolfram's patch appears in a RC, would > that be okay with you? Fine for me. Gr{oetje,eeting}s, Geert
> I was thinking about sending a follow-up patch to list hs400 here > after Wolfram's patch appears in a RC, would that be okay with you? Do we need that? The *board* does not handle HS400, so why add the property? Similar setting would be an I2C device which can do 400kHz but the board layout doesn't allow for such speeds, so we are limited to 100kHz. Or?
Hello Wolfram, Thank you for the feedback! > From: Wolfram Sang <wsa@the-dreams.de> > Sent: 12 June 2019 10:45 > Subject: Re: [PATCH] arm64: dts: renesas: hihope-common: Add uSD and eMMC > > > > I was thinking about sending a follow-up patch to list hs400 here > > after Wolfram's patch appears in a RC, would that be okay with you? > > Do we need that? The *board* does not handle HS400, so why add the > property? The SoC the design currently comes with doesn't handle HS400, but they may replace that in the future with one that is based off R-Car M3-W version 3.0 (which does support HS400). Also, the HiHope will come in two flavours, the HiHope RZ/G2M (the flavour we are currently adding support for), and the HiHope RZ/G2N (sporting an RZ/G2N, which is based off R-Car M3-N). File arch/arm64/boot/dts/renesas/hihope-common.dtsi contains common definitions for the mother boards for both flavours (including the eMMC). My understanding is that R-Car M3-N does support HS400, hence the need for the property here. Thanks, Fab > > Similar setting would be an I2C device which can do 400kHz but the board > layout doesn't allow for such speeds, so we are limited to 100kHz. > > Or?
> File arch/arm64/boot/dts/renesas/hihope-common.dtsi contains common > definitions for the mother boards for both flavours (including the > eMMC). My understanding is that R-Car M3-N does support HS400, hence > the need for the property here. I won't be super strict here, yet I think it is more elegant to add the HS400 properties to the board DTS files, not the dtsi. I mean we could add them to the SoC dtsi otherwise.
Hello Wolfram, Thank you for your feedback! > From: Wolfram Sang <wsa@the-dreams.de> > Sent: 12 June 2019 11:15 > Subject: Re: [PATCH] arm64: dts: renesas: hihope-common: Add uSD and eMMC > > > > File arch/arm64/boot/dts/renesas/hihope-common.dtsi contains common > > definitions for the mother boards for both flavours (including the > > eMMC). My understanding is that R-Car M3-N does support HS400, hence > > the need for the property here. > > I won't be super strict here, yet I think it is more elegant to add the > HS400 properties to the board DTS files, not the dtsi. I mean we could > add them to the SoC dtsi otherwise. We will give both approaches a shot in due time, once the relevant HW will be available, but I do wonder why we need to make a distinction in the DT when we have a quirk in the driver (the real problem is that HS400 doesn't work only on some revisions of the RZ/G2M chip, do we need to create different .dtsi for different revisions of the same SoC? Or perhaps different versions of the same board dts?)? We could simply put a comment in the DT once we add the compatibility of the eMMC with HS400, something like: "HS400 won't work on RZ/G2M rev=1.2" and let the driver handle the difference between revisions of the SoC, this would keep things as compact as they possibly can be, as well as simplify design and maintenance. I hope this helps. Thanks, Fab
Hi Wolfram, On Wed, Jun 12, 2019 at 11:45 AM Wolfram Sang <wsa@the-dreams.de> wrote: > > I was thinking about sending a follow-up patch to list hs400 here > > after Wolfram's patch appears in a RC, would that be okay with you? > > Do we need that? The *board* does not handle HS400, so why add the > property? > > Similar setting would be an I2C device which can do 400kHz but the board > layout doesn't allow for such speeds, so we are limited to 100kHz. Do we know the HiHope board layout does not support HS400? Gr{oetje,eeting}s, Geert
Hi Wolfram, On Wed, Jun 12, 2019 at 12:15 PM Wolfram Sang <wsa@the-dreams.de> wrote: > > File arch/arm64/boot/dts/renesas/hihope-common.dtsi contains common > > definitions for the mother boards for both flavours (including the > > eMMC). My understanding is that R-Car M3-N does support HS400, hence > > the need for the property here. > > I won't be super strict here, yet I think it is more elegant to add the > HS400 properties to the board DTS files, not the dtsi. I mean we could > add them to the SoC dtsi otherwise. This _is_ the (shared) board DTS, cfr. salvator-common.dtsi and ulcb.dtsi, which do have mmc-hs400-1_8v properties. Gr{oetje,eeting}s, Geert
I think it is confusing to describe a HS400 property if the HW is not capable of it. Even if the driver has a safety check and will prevent it from being used. But as I said, I won't be insisting. I understood it simplifies things in grouping boards. From my side, we can close the case here.
On Wed, Jun 12, 2019 at 01:19:00PM +0200, Wolfram Sang wrote: > > I think it is confusing to describe a HS400 property if the HW is not > capable of it. Even if the driver has a safety check and will prevent it > from being used. > > But as I said, I won't be insisting. I understood it simplifies things > in grouping boards. From my side, we can close the case here. I would slightly prefer if the property was left out for one thing, how can we verify the board supports HS400 at this time?
On Wed, Jun 12, 2019 at 10:27:56AM +0200, Geert Uytterhoeven wrote: > Hi Fabrizio, > > On Fri, Jun 7, 2019 at 10:14 AM Fabrizio Castro > <fabrizio.castro@bp.renesas.com> wrote: > > This patch adds uSD and eMMC support to the HiHope RZ/G2M > > board. > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > Thanks for your patch! > > > --- a/arch/arm64/boot/dts/renesas/hihope-common.dtsi > > +++ b/arch/arm64/boot/dts/renesas/hihope-common.dtsi > > > +&sdhi3 { > > + pinctrl-0 = <&sdhi3_pins>; > > + pinctrl-1 = <&sdhi3_pins>; > > + pinctrl-names = "default", "state_uhs"; > > + > > + vmmc-supply = <®_3p3v>; > > + vqmmc-supply = <®_1p8v>; > > + bus-width = <8>; > > + mmc-hs200-1_8v; > > Does the eMMC support HS400, too? > > > + non-removable; > > + fixed-emmc-driver-type = <1>; > > + status = "okay"; > > +}; > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Thanks, applied for inclusion in v5.3.
Hello Simon, Thank you for your feedback! > From: Simon Horman <horms@verge.net.au> > Sent: 12 June 2019 12:55 > Subject: Re: [PATCH] arm64: dts: renesas: hihope-common: Add uSD and eMMC > > On Wed, Jun 12, 2019 at 01:19:00PM +0200, Wolfram Sang wrote: > > > > I think it is confusing to describe a HS400 property if the HW is not > > capable of it. Even if the driver has a safety check and will prevent it > > from being used. > > > > But as I said, I won't be insisting. I understood it simplifies things > > in grouping boards. From my side, we can close the case here. > > I would slightly prefer if the property was left out > for one thing, how can we verify the board supports HS400 at this time? We can't, hence the compatibility only with HS200, for now. Thanks, Fab
diff --git a/arch/arm64/boot/dts/renesas/hihope-common.dtsi b/arch/arm64/boot/dts/renesas/hihope-common.dtsi index 4cc924d..b6411b1 100644 --- a/arch/arm64/boot/dts/renesas/hihope-common.dtsi +++ b/arch/arm64/boot/dts/renesas/hihope-common.dtsi @@ -16,6 +16,37 @@ bootargs = "ignore_loglevel"; stdout-path = "serial0:115200n8"; }; + + reg_1p8v: regulator0 { + compatible = "regulator-fixed"; + regulator-name = "fixed-1.8V"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-boot-on; + regulator-always-on; + }; + + reg_3p3v: regulator1 { + compatible = "regulator-fixed"; + regulator-name = "fixed-3.3V"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-boot-on; + regulator-always-on; + }; + + vccq_sdhi0: regulator-vccq-sdhi0 { + compatible = "regulator-gpio"; + + regulator-name = "SDHI0 VccQ"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + + gpios = <&gpio6 30 GPIO_ACTIVE_HIGH>; + gpios-states = <1>; + states = <3300000 1 + 1800000 0>; + }; }; &extal_clk { @@ -39,6 +70,24 @@ groups = "scif_clk_a"; function = "scif_clk"; }; + + sdhi0_pins: sd0 { + groups = "sdhi0_data4", "sdhi0_ctrl"; + function = "sdhi0"; + power-source = <3300>; + }; + + sdhi0_pins_uhs: sd0_uhs { + groups = "sdhi0_data4", "sdhi0_ctrl"; + function = "sdhi0"; + power-source = <1800>; + }; + + sdhi3_pins: sd3 { + groups = "sdhi3_data8", "sdhi3_ctrl", "sdhi3_ds"; + function = "sdhi3"; + power-source = <1800>; + }; }; &scif2 { @@ -51,3 +100,31 @@ &scif_clk { clock-frequency = <14745600>; }; + +&sdhi0 { + pinctrl-0 = <&sdhi0_pins>; + pinctrl-1 = <&sdhi0_pins_uhs>; + pinctrl-names = "default", "state_uhs"; + + vmmc-supply = <®_3p3v>; + vqmmc-supply = <&vccq_sdhi0>; + cd-gpios = <&gpio3 12 GPIO_ACTIVE_LOW>; + bus-width = <4>; + sd-uhs-sdr50; + sd-uhs-sdr104; + status = "okay"; +}; + +&sdhi3 { + pinctrl-0 = <&sdhi3_pins>; + pinctrl-1 = <&sdhi3_pins>; + pinctrl-names = "default", "state_uhs"; + + vmmc-supply = <®_3p3v>; + vqmmc-supply = <®_1p8v>; + bus-width = <8>; + mmc-hs200-1_8v; + non-removable; + fixed-emmc-driver-type = <1>; + status = "okay"; +};
This patch adds uSD and eMMC support to the HiHope RZ/G2M board. Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> --- This patch applies on top of renesas-devel-20190606-v5.2-rc3 arch/arm64/boot/dts/renesas/hihope-common.dtsi | 77 ++++++++++++++++++++++++++ 1 file changed, 77 insertions(+)