| Message ID | 1537530911-443-5-git-send-email-fabrizio.castro@bp.renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add SDHI2 support to iwg23s | expand |
On Fri, Sep 21, 2018 at 12:55:11PM +0100, Fabrizio Castro wrote: > Add uSD card support to the iwg23s single board computer powered > by the RZ/G1C SoC (a.k.a. r8a77470). > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > Reviewed-by: Biju Das <biju.das@bp.renesas.com> Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
On Fri, Sep 21, 2018 at 12:55:11PM +0100, Fabrizio Castro wrote: > Add uSD card support to the iwg23s single board computer powered > by the RZ/G1C SoC (a.k.a. r8a77470). > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > Reviewed-by: Biju Das <biju.das@bp.renesas.com> > --- > Hello Simon, > > this patch can only be taken after patch "pinctrl: sh-pfc: r8a77470: > Add SDHI2 voltage switch" from this series appears on a release candidate > or a release. What is the nature of that dependency. Does adding this patch without its dependency cause a regression? > Shall I re-send it at a later stage or are you happy to keep it around > and defer its application to when its dependency is sorted? I'd rather that you repost or otherwise ping me once any dependency is sorted.
Hello Simon, Thank you for your feedback. > -----Original Message----- > From: Simon Horman <horms@verge.net.au> > Sent: 24 September 2018 10:14 > To: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Laurent > Pinchart <laurent.pinchart@ideasonboard.com>; Geert Uytterhoeven <geert+renesas@glider.be>; Linus Walleij > <linus.walleij@linaro.org>; Wolfram Sang <wsa+renesas@sang-engineering.com>; Magnus Damm <magnus.damm@gmail.com>; > linux-mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-renesas-soc@vger.kernel.org; linux-gpio@vger.kernel.org; Chris > Paterson <Chris.Paterson2@renesas.com>; Biju Das <biju.das@bp.renesas.com> > Subject: Re: [PATCH 4/4] ARM: dts: iwg23s-sbc: Add uSD card support > > On Fri, Sep 21, 2018 at 12:55:11PM +0100, Fabrizio Castro wrote: > > Add uSD card support to the iwg23s single board computer powered > > by the RZ/G1C SoC (a.k.a. r8a77470). > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > Reviewed-by: Biju Das <biju.das@bp.renesas.com> > > --- > > Hello Simon, > > > > this patch can only be taken after patch "pinctrl: sh-pfc: r8a77470: > > Add SDHI2 voltage switch" from this series appears on a release candidate > > or a release. > > What is the nature of that dependency. Does adding this patch without > its dependency cause a regression? Since the SDHI2 pins definition contain "power-source" property, adding this patch without its dependency will cause an error at boot up as the kernel would be looking for flag SH_PFC_PIN_CFG_IO_VOLTAGE for each pin contained in sdhi2_pins and sdhi2_pins_uhs, and since that particular flag would be missing (as such a definition comes from patch "pinctrl: sh-pfc: r8a77470: Add SDHI2 voltage switch") the SD card would not be functional, but this won't have any impact on the rest of the system. > > > Shall I re-send it at a later stage or are you happy to keep it around > > and defer its application to when its dependency is sorted? > > I'd rather that you repost or otherwise ping me once any dependency is sorted. Ok, I will do that. Thanks, Fab Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
Hi Fabrizio, On Mon, Sep 24, 2018 at 11:30 AM Fabrizio Castro <fabrizio.castro@bp.renesas.com> wrote: > > -----Original Message----- > > From: Simon Horman <horms@verge.net.au> > > Sent: 24 September 2018 10:14 > > To: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Laurent > > Pinchart <laurent.pinchart@ideasonboard.com>; Geert Uytterhoeven <geert+renesas@glider.be>; Linus Walleij > > <linus.walleij@linaro.org>; Wolfram Sang <wsa+renesas@sang-engineering.com>; Magnus Damm <magnus.damm@gmail.com>; > > linux-mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-renesas-soc@vger.kernel.org; linux-gpio@vger.kernel.org; Chris > > Paterson <Chris.Paterson2@renesas.com>; Biju Das <biju.das@bp.renesas.com> > > Subject: Re: [PATCH 4/4] ARM: dts: iwg23s-sbc: Add uSD card support > > > > On Fri, Sep 21, 2018 at 12:55:11PM +0100, Fabrizio Castro wrote: > > > Add uSD card support to the iwg23s single board computer powered > > > by the RZ/G1C SoC (a.k.a. r8a77470). > > > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > Reviewed-by: Biju Das <biju.das@bp.renesas.com> > > > --- > > > Hello Simon, > > > > > > this patch can only be taken after patch "pinctrl: sh-pfc: r8a77470: > > > Add SDHI2 voltage switch" from this series appears on a release candidate > > > or a release. > > > > What is the nature of that dependency. Does adding this patch without > > its dependency cause a regression? > > Since the SDHI2 pins definition contain "power-source" property, adding this > patch without its dependency will cause an error at boot up as the kernel > would be looking for flag SH_PFC_PIN_CFG_IO_VOLTAGE for each pin contained > in sdhi2_pins and sdhi2_pins_uhs, and since that particular flag would be missing > (as such a definition comes from patch "pinctrl: sh-pfc: r8a77470: Add SDHI2 voltage > switch") the SD card would not be functional, but this won't have any impact on > the rest of the system. But that won't be a regression, as currently there's no support for SDHI2 anyway, right? All pieces will start working when both the pinctrl and DTS support will be merged together. This is different from the case where you first add a device node to enable a device (which makes it work), and later add pinctrl properties (which may break it, if the pinctrl driver doesn't have support for it yet). Gr{oetje,eeting}s, Geert
Hello Geert, Thank you for your feedback. > Subject: Re: [PATCH 4/4] ARM: dts: iwg23s-sbc: Add uSD card support > > Hi Fabrizio, > > On Mon, Sep 24, 2018 at 11:30 AM Fabrizio Castro > <fabrizio.castro@bp.renesas.com> wrote: > > > -----Original Message----- > > > From: Simon Horman <horms@verge.net.au> > > > Sent: 24 September 2018 10:14 > > > To: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Laurent > > > Pinchart <laurent.pinchart@ideasonboard.com>; Geert Uytterhoeven <geert+renesas@glider.be>; Linus Walleij > > > <linus.walleij@linaro.org>; Wolfram Sang <wsa+renesas@sang-engineering.com>; Magnus Damm <magnus.damm@gmail.com>; > > > linux-mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-renesas-soc@vger.kernel.org; linux-gpio@vger.kernel.org; Chris > > > Paterson <Chris.Paterson2@renesas.com>; Biju Das <biju.das@bp.renesas.com> > > > Subject: Re: [PATCH 4/4] ARM: dts: iwg23s-sbc: Add uSD card support > > > > > > On Fri, Sep 21, 2018 at 12:55:11PM +0100, Fabrizio Castro wrote: > > > > Add uSD card support to the iwg23s single board computer powered > > > > by the RZ/G1C SoC (a.k.a. r8a77470). > > > > > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > > Reviewed-by: Biju Das <biju.das@bp.renesas.com> > > > > --- > > > > Hello Simon, > > > > > > > > this patch can only be taken after patch "pinctrl: sh-pfc: r8a77470: > > > > Add SDHI2 voltage switch" from this series appears on a release candidate > > > > or a release. > > > > > > What is the nature of that dependency. Does adding this patch without > > > its dependency cause a regression? > > > > Since the SDHI2 pins definition contain "power-source" property, adding this > > patch without its dependency will cause an error at boot up as the kernel > > would be looking for flag SH_PFC_PIN_CFG_IO_VOLTAGE for each pin contained > > in sdhi2_pins and sdhi2_pins_uhs, and since that particular flag would be missing > > (as such a definition comes from patch "pinctrl: sh-pfc: r8a77470: Add SDHI2 voltage > > switch") the SD card would not be functional, but this won't have any impact on > > the rest of the system. > > But that won't be a regression, as currently there's no support for > SDHI2 anyway, > right? All pieces will start working when both the pinctrl and DTS support will > be merged together. Exactly, it won't be a regression, you'd just get weird messages from the kernel, that's all. Thanks, Fab > > This is different from the case where you first add a device node to enable a > device (which makes it work), and later add pinctrl properties (which may > break it, if the pinctrl driver doesn't have support for it yet). > > 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 Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
On Mon, Sep 24, 2018 at 03:09:49PM +0000, Fabrizio Castro wrote: > Hello Geert, > > Thank you for your feedback. > > > Subject: Re: [PATCH 4/4] ARM: dts: iwg23s-sbc: Add uSD card support > > > > Hi Fabrizio, > > > > On Mon, Sep 24, 2018 at 11:30 AM Fabrizio Castro > > <fabrizio.castro@bp.renesas.com> wrote: > > > > -----Original Message----- > > > > From: Simon Horman <horms@verge.net.au> > > > > Sent: 24 September 2018 10:14 > > > > To: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > > Cc: Ulf Hansson <ulf.hansson@linaro.org>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Laurent > > > > Pinchart <laurent.pinchart@ideasonboard.com>; Geert Uytterhoeven <geert+renesas@glider.be>; Linus Walleij > > > > <linus.walleij@linaro.org>; Wolfram Sang <wsa+renesas@sang-engineering.com>; Magnus Damm <magnus.damm@gmail.com>; > > > > linux-mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-renesas-soc@vger.kernel.org; linux-gpio@vger.kernel.org; Chris > > > > Paterson <Chris.Paterson2@renesas.com>; Biju Das <biju.das@bp.renesas.com> > > > > Subject: Re: [PATCH 4/4] ARM: dts: iwg23s-sbc: Add uSD card support > > > > > > > > On Fri, Sep 21, 2018 at 12:55:11PM +0100, Fabrizio Castro wrote: > > > > > Add uSD card support to the iwg23s single board computer powered > > > > > by the RZ/G1C SoC (a.k.a. r8a77470). > > > > > > > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > > > Reviewed-by: Biju Das <biju.das@bp.renesas.com> > > > > > --- > > > > > Hello Simon, > > > > > > > > > > this patch can only be taken after patch "pinctrl: sh-pfc: r8a77470: > > > > > Add SDHI2 voltage switch" from this series appears on a release candidate > > > > > or a release. > > > > > > > > What is the nature of that dependency. Does adding this patch without > > > > its dependency cause a regression? > > > > > > Since the SDHI2 pins definition contain "power-source" property, adding this > > > patch without its dependency will cause an error at boot up as the kernel > > > would be looking for flag SH_PFC_PIN_CFG_IO_VOLTAGE for each pin contained > > > in sdhi2_pins and sdhi2_pins_uhs, and since that particular flag would be missing > > > (as such a definition comes from patch "pinctrl: sh-pfc: r8a77470: Add SDHI2 voltage > > > switch") the SD card would not be functional, but this won't have any impact on > > > the rest of the system. > > > > But that won't be a regression, as currently there's no support for > > SDHI2 anyway, > > right? All pieces will start working when both the pinctrl and DTS support will > > be merged together. > > Exactly, it won't be a regression, you'd just get weird messages from the kernel, that's all. That is fine. But lets finalise the discussion of the bindings before I apply this patch.
diff --git a/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts b/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts index 22da819..cd9c3fc 100644 --- a/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts +++ b/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts @@ -6,6 +6,7 @@ */ /dts-v1/; +#include <dt-bindings/gpio/gpio.h> #include "r8a77470.dtsi" / { model = "iWave iW-RainboW-G23S single board computer based on RZ/G1C"; @@ -25,6 +26,29 @@ device_type = "memory"; reg = <0 0x40000000 0 0x20000000>; }; + + vcc_sdhi2: regulator-vcc-sdhi2 { + compatible = "regulator-fixed"; + + regulator-name = "SDHI2 Vcc"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + + enable-active-high; + }; + + vccq_sdhi2: regulator-vccq-sdhi2 { + compatible = "regulator-gpio"; + + regulator-name = "SDHI2 VccQ"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + + gpios = <&gpio2 24 GPIO_ACTIVE_LOW>; + gpios-states = <1>; + states = <3300000 1 + 1800000 0>; + }; }; &avb { @@ -50,6 +74,18 @@ groups = "scif1_data_b"; function = "scif1"; }; + + sdhi2_pins: sd2 { + groups = "sdhi2_data4", "sdhi2_ctrl"; + function = "sdhi2"; + power-source = <3300>; + }; + + sdhi2_pins_uhs: sd2_uhs { + groups = "sdhi2_data4", "sdhi2_ctrl"; + function = "sdhi2"; + power-source = <1800>; + }; }; &scif1 { @@ -58,3 +94,16 @@ status = "okay"; }; + +&sdhi2 { + pinctrl-0 = <&sdhi2_pins>; + pinctrl-1 = <&sdhi2_pins_uhs>; + pinctrl-names = "default", "state_uhs"; + + vmmc-supply = <&vcc_sdhi2>; + vqmmc-supply = <&vccq_sdhi2>; + bus-width = <4>; + cd-gpios = <&gpio4 20 GPIO_ACTIVE_LOW>; + sd-uhs-sdr50; + status = "okay"; +};