| Message ID | 1505322341-9480-5-git-send-email-chris.paterson2@renesas.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Commit | acaefef7d48ce1872bdd4c864025ff9659588ad6 |
| Delegated to: | Simon Horman |
| Headers | show |
Hi Simon, Apologies for the delay in getting back to you. > -----Original Message----- > From: Simon Horman [mailto:horms@verge.net.au] > Sent: 15 September 2017 09:06 > To: Chris Paterson <Chris.Paterson2@renesas.com> > Cc: Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Magnus Damm <magnus.damm@gmail.com>; > Russell King <linux@armlinux.org.uk>; Fabrizio Castro <fabrizio.castro@bp.renesas.com>; devicetree@vger.kernel.org; linux-renesas- > soc@vger.kernel.org; linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH 4/8] ARM: dts: iwg22d: Enable SDHI0 controller > > On Wed, Sep 13, 2017 at 06:05:37PM +0100, Chris Paterson wrote: > > From: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > > Enable the SDHI0 controller on iWave RZ/G1E carrier board. > > ... > > > @@ -63,3 +88,15 @@ > > micrel,led-mode = <1>; > > }; > > }; > > + > > +&sdhi0 { > > +pinctrl-0 = <&sdhi0_pins>; > > +pinctrl-1 = <&sdhi0_pins_uhs>; > > +pinctrl-names = "default", "state_uhs"; > > + > > +vmmc-supply = <®_3p3v>; > > +vqmmc-supply = <&vccq_sdhi0>; > > +cd-gpios = <&gpio6 6 GPIO_ACTIVE_LOW>; > > I take that the absence of a wp-gpio means that that this is a µSD slot. > Could you help me by documenting this correctly on > http://elinux.org/index.php?title=Renesas-MMC-Enabled-Speeds ? > > For some reason I thought that SDHI0 wasn't exposed at all, so I guess > my reading of the documentation was incorrect. > > > I think you also want sd-uhs-sdr50 here too. > You can test it by removing the sd-uhs-sdr104 property. Whilst playing with uhs properties I have noticed that with the version of the DT I was using I couldn't put sdhi0 into an uhs mode and that was down to the fact that the driver wasn't driving the regulator to 1.8V. This happens because (in my case) the power regulator gets probed after the sdhi does, and the sdhi code doesn't seem to deal with it. Am I the only one with this problem or is it a known problem? Thanks, Fabrizio > > > +sd-uhs-sdr104; > > +status = "okay"; > > +}; > > -- > > 1.9.1 > > Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
On Wed, Sep 13, 2017 at 7:05 PM, Chris Paterson <chris.paterson2@renesas.com> wrote: > From: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > Enable the SDHI0 controller on iWave RZ/G1E carrier board. > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > Signed-off-by: Chris Paterson <chris.paterson2@renesas.com> 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
Hello Simon, Geert thank you for your patience on this one. We found out that there is an HW issue on SDHI0 , SDHC_CD# and SDHC_CMD (on the carrier board) are pulled up to 3.3V, therefore we are dropping UHS on this interface, and as result we are dropping this patch. We will send a V2 shortly. Thanks, Fabrizio > -----Original Message----- > From: Simon Horman [mailto:horms@verge.net.au] > Sent: 15 September 2017 09:06 > To: Chris Paterson <Chris.Paterson2@renesas.com> > Cc: Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Magnus Damm <magnus.damm@gmail.com>; > Russell King <linux@armlinux.org.uk>; Fabrizio Castro <fabrizio.castro@bp.renesas.com>; devicetree@vger.kernel.org; linux-renesas- > soc@vger.kernel.org; linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH 4/8] ARM: dts: iwg22d: Enable SDHI0 controller > > On Wed, Sep 13, 2017 at 06:05:37PM +0100, Chris Paterson wrote: > > From: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > > Enable the SDHI0 controller on iWave RZ/G1E carrier board. > > ... > > > @@ -63,3 +88,15 @@ > > micrel,led-mode = <1>; > > }; > > }; > > + > > +&sdhi0 { > > +pinctrl-0 = <&sdhi0_pins>; > > +pinctrl-1 = <&sdhi0_pins_uhs>; > > +pinctrl-names = "default", "state_uhs"; > > + > > +vmmc-supply = <®_3p3v>; > > +vqmmc-supply = <&vccq_sdhi0>; > > +cd-gpios = <&gpio6 6 GPIO_ACTIVE_LOW>; > > I take that the absence of a wp-gpio means that that this is a µSD slot. > Could you help me by documenting this correctly on > http://elinux.org/index.php?title=Renesas-MMC-Enabled-Speeds ? > > For some reason I thought that SDHI0 wasn't exposed at all, so I guess > my reading of the documentation was incorrect. > > > I think you also want sd-uhs-sdr50 here too. > You can test it by removing the sd-uhs-sdr104 property. > > > +sd-uhs-sdr104; > > +status = "okay"; > > +}; > > -- > > 1.9.1 > > Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
On Wed, Sep 20, 2017 at 01:40:20PM +0200, Geert Uytterhoeven wrote: > On Wed, Sep 13, 2017 at 7:05 PM, Chris Paterson > <chris.paterson2@renesas.com> wrote: > > From: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > > Enable the SDHI0 controller on iWave RZ/G1E carrier board. > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > Signed-off-by: Chris Paterson <chris.paterson2@renesas.com> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Thanks, applied.
On Thu, Sep 21, 2017 at 10:38:40AM +0200, Simon Horman wrote: > On Wed, Sep 20, 2017 at 01:40:20PM +0200, Geert Uytterhoeven wrote: > > On Wed, Sep 13, 2017 at 7:05 PM, Chris Paterson > > <chris.paterson2@renesas.com> wrote: > > > From: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > > > > Enable the SDHI0 controller on iWave RZ/G1E carrier board. > > > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > Signed-off-by: Chris Paterson <chris.paterson2@renesas.com> > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Thanks, applied. Sorry, I somehow overlooked Fabrizio's comments. I have dropped this patch for now.
Hello Simon, > -----Original Message----- > From: Simon Horman [mailto:horms@verge.net.au] > Sent: 21 September 2017 09:42 > To: Geert Uytterhoeven <geert@linux-m68k.org> > Cc: Chris Paterson <Chris.Paterson2@renesas.com>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; > Magnus Damm <magnus.damm@gmail.com>; Russell King <linux@armlinux.org.uk>; Fabrizio Castro > <fabrizio.castro@bp.renesas.com>; devicetree@vger.kernel.org; Linux-Renesas <linux-renesas-soc@vger.kernel.org>; linux-arm- > kernel@lists.infradead.org > Subject: Re: [PATCH 4/8] ARM: dts: iwg22d: Enable SDHI0 controller > > On Thu, Sep 21, 2017 at 10:38:40AM +0200, Simon Horman wrote: > > On Wed, Sep 20, 2017 at 01:40:20PM +0200, Geert Uytterhoeven wrote: > > > On Wed, Sep 13, 2017 at 7:05 PM, Chris Paterson > > > <chris.paterson2@renesas.com> wrote: > > > > From: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > > > > > > Enable the SDHI0 controller on iWave RZ/G1E carrier board. > > > > > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > > Signed-off-by: Chris Paterson <chris.paterson2@renesas.com> > > > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > Thanks, applied. > > Sorry, I somehow overlooked Fabrizio's comments. > I have dropped this patch for now. Thank you for doing this. We are working on a new patch now, I'll submit a V2 as soon as it is ready. Just to recap (all in one place), we found an HW issue and a SW one preventing SDHI0 from working properly at UHS SDR50 and SDR104, therefore we need a new patch to limit the speed of this interface. Thanks, Fabrizio Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
On Thu, Sep 21, 2017 at 08:53:00AM +0000, Fabrizio Castro wrote: > Hello Simon, > > > -----Original Message----- > > From: Simon Horman [mailto:horms@verge.net.au] > > Sent: 21 September 2017 09:42 > > To: Geert Uytterhoeven <geert@linux-m68k.org> > > Cc: Chris Paterson <Chris.Paterson2@renesas.com>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; > > Magnus Damm <magnus.damm@gmail.com>; Russell King <linux@armlinux.org.uk>; Fabrizio Castro > > <fabrizio.castro@bp.renesas.com>; devicetree@vger.kernel.org; Linux-Renesas <linux-renesas-soc@vger.kernel.org>; linux-arm- > > kernel@lists.infradead.org > > Subject: Re: [PATCH 4/8] ARM: dts: iwg22d: Enable SDHI0 controller > > > > On Thu, Sep 21, 2017 at 10:38:40AM +0200, Simon Horman wrote: > > > On Wed, Sep 20, 2017 at 01:40:20PM +0200, Geert Uytterhoeven wrote: > > > > On Wed, Sep 13, 2017 at 7:05 PM, Chris Paterson > > > > <chris.paterson2@renesas.com> wrote: > > > > > From: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > > > > > > > > Enable the SDHI0 controller on iWave RZ/G1E carrier board. > > > > > > > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > > > Signed-off-by: Chris Paterson <chris.paterson2@renesas.com> > > > > > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > > > Thanks, applied. > > > > Sorry, I somehow overlooked Fabrizio's comments. > > I have dropped this patch for now. > > Thank you for doing this. > We are working on a new patch now, I'll submit a V2 as soon as it is ready. > Just to recap (all in one place), we found an HW issue and a SW one preventing SDHI0 from working properly at UHS SDR50 and SDR104, therefore we need a new patch to limit the speed of this interface. Thanks for catching this, UHS can be quite troublesome in my experience.
> From: Simon Horman [mailto:horms@verge.net.au] > Sent: 21 September 2017 10:04 > > On Thu, Sep 21, 2017 at 08:53:00AM +0000, Fabrizio Castro wrote: > > Hello Simon, > > > > > -----Original Message----- > > > From: Simon Horman [mailto:horms@verge.net.au] > > > Sent: 21 September 2017 09:42 > > > To: Geert Uytterhoeven <geert@linux-m68k.org> > > > Cc: Chris Paterson <Chris.Paterson2@renesas.com>; Rob Herring > > > <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; > Magnus > > > Damm <magnus.damm@gmail.com>; Russell King > <linux@armlinux.org.uk>; > > > Fabrizio Castro <fabrizio.castro@bp.renesas.com>; > > > devicetree@vger.kernel.org; Linux-Renesas > > > <linux-renesas-soc@vger.kernel.org>; linux-arm- > > > kernel@lists.infradead.org > > > Subject: Re: [PATCH 4/8] ARM: dts: iwg22d: Enable SDHI0 controller > > > > > > On Thu, Sep 21, 2017 at 10:38:40AM +0200, Simon Horman wrote: > > > > On Wed, Sep 20, 2017 at 01:40:20PM +0200, Geert Uytterhoeven wrote: > > > > > On Wed, Sep 13, 2017 at 7:05 PM, Chris Paterson > > > > > <chris.paterson2@renesas.com> wrote: > > > > > > From: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > > > > > > > > > > Enable the SDHI0 controller on iWave RZ/G1E carrier board. > > > > > > > > > > > > Signed-off-by: Fabrizio Castro > > > > > > <fabrizio.castro@bp.renesas.com> > > > > > > Signed-off-by: Chris Paterson <chris.paterson2@renesas.com> > > > > > > > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > > > > > Thanks, applied. > > > > > > Sorry, I somehow overlooked Fabrizio's comments. > > > I have dropped this patch for now. > > > > Thank you for doing this. > > We are working on a new patch now, I'll submit a V2 as soon as it is ready. > > Just to recap (all in one place), we found an HW issue and a SW one > preventing SDHI0 from working properly at UHS SDR50 and SDR104, > therefore we need a new patch to limit the speed of this interface. > > Thanks for catching this, UHS can be quite troublesome in my experience. So we've seen!
diff --git a/arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts b/arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts index aac84c6..c34dbe7 100644 --- a/arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts +++ b/arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts @@ -24,6 +24,19 @@ bootargs = "ignore_loglevel rw root=/dev/nfs ip=dhcp"; stdout-path = "serial0:115200n8"; }; + + vccq_sdhi0: regulator-vccq-sdhi0 { + compatible = "regulator-gpio"; + + regulator-name = "SDHI0 VccQ"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + + gpios = <&gpio0 20 GPIO_ACTIVE_LOW>; + gpios-states = <1>; + states = <3300000 1 + 1800000 0>; + }; }; &pfc { @@ -36,6 +49,18 @@ groups = "avb_mdio", "avb_gmii"; function = "avb"; }; + + 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>; + }; }; &scif4 { @@ -63,3 +88,15 @@ micrel,led-mode = <1>; }; }; + +&sdhi0 { + pinctrl-0 = <&sdhi0_pins>; + pinctrl-1 = <&sdhi0_pins_uhs>; + pinctrl-names = "default", "state_uhs"; + + vmmc-supply = <®_3p3v>; + vqmmc-supply = <&vccq_sdhi0>; + cd-gpios = <&gpio6 6 GPIO_ACTIVE_LOW>; + sd-uhs-sdr104; + status = "okay"; +};