| Message ID | Pine.LNX.4.64.1309231651230.11505@axis700.grange (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, Sep 23, 2013 at 05:38:47PM +0200, Guennadi Liakhovetski wrote: > Add SDHI0, SDHI1 and MMCIF interfaces to armadillo800eva-reference with > regulators and pin configurations. > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com> > --- > > v4: > > 1. now that PFC pinctrl is usable with DT, we can use proper regulators > and pin configurations on armadillo800eva > > 2. corrected SDHI compatibility strings > > 3. RFC because I don't know how to enable choosing between CON14 and CON8. > In .c version this is done by reading GPIO 6. To do the same in DT mode > we'd probably have to use some run-time DT patching, which isn't possible > yet, AFAICS. I agree with your reasoning there, though perhaps Laurent or Magnus have a more enlightened view of things. To my way of thinking it would be good to merge this change as is, which an appropriate disclaimer about functionality in the changelog above the scissors. But I do not feel strongly about this and I am happy to wait for fuller functionality. > > .../boot/dts/r8a7740-armadillo800eva-reference.dts | 83 ++++++++++++++++++++ > arch/arm/boot/dts/r8a7740.dtsi | 33 ++++++++ > 2 files changed, 116 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts b/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts > index c638e4a..af15be0 100644 > --- a/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts > +++ b/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts > @@ -34,6 +34,44 @@ > regulator-boot-on; > }; > > + vcc_sdhi0: regulator@1 { > + compatible = "regulator-fixed"; > + > + regulator-name = "SDHI0 Vcc"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + > + gpio = <&pfc 75 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + }; > + > + vcc_sdhi1: regulator@2 { > + compatible = "regulator-fixed"; > + > + regulator-name = "SDHI1 Vcc"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + > + gpio = <&pfc 16 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + }; > + > + vccq_sdhi0: regulator@3 { > + compatible = "regulator-gpio"; > + > + regulator-name = "SDHI0 VccQ"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + vin-supply = <&vcc_sdhi0>; > + > + enable-gpio = <&pfc 74 GPIO_ACTIVE_HIGH>; > + gpios = <&pfc 17 GPIO_ACTIVE_HIGH>; > + states = <3300000 0 > + 1800000 1>; > + > + enable-active-high; > + }; > + > leds { > compatible = "gpio-leds"; > led1 { > @@ -76,4 +114,49 @@ > renesas,groups = "intc_irq10"; > renesas,function = "intc"; > }; > + > + mmc0_pins: mmc0 { > + renesas,groups = "mmc0_data8_1", "mmc0_ctrl_1"; > + renesas,function = "mmc0"; > + }; > + > + sdhi0_pins: sdhi0 { > + renesas,groups = "sdhi0_data4", "sdhi0_ctrl", "sdhi0_wp"; > + renesas,function = "sdhi0"; > + }; > + > + sdhi1_pins: sdhi1 { > + renesas,groups = "sdhi1_data4", "sdhi1_ctrl", "sdhi1_cd", "sdhi1_wp"; > + renesas,function = "sdhi1"; > + }; > +}; > + > +&mmcif0 { > + pinctrl-0 = <&mmc0_pins>; > + pinctrl-names = "default"; > + > + vmmc-supply = <®_3p3v>; > + bus-width = <8>; > + non-removable; > + status = "okay"; > +}; > + > +&sdhi0 { > + pinctrl-0 = <&sdhi0_pins>; > + pinctrl-names = "default"; > + > + vmmc-supply = <&vcc_sdhi0>; > + vqmmc-supply = <&vccq_sdhi0>; > + bus-width = <4>; > + broken-cd; > + status = "okay"; > +}; > + > +&sdhi1 { > + pinctrl-0 = <&sdhi1_pins>; > + pinctrl-names = "default"; > + > + vmmc-supply = <&vcc_sdhi1>; > + bus-width = <4>; > + status = "okay"; > }; > diff --git a/arch/arm/boot/dts/r8a7740.dtsi b/arch/arm/boot/dts/r8a7740.dtsi > index 44d3d52..018c02d 100644 > --- a/arch/arm/boot/dts/r8a7740.dtsi > +++ b/arch/arm/boot/dts/r8a7740.dtsi > @@ -159,4 +159,37 @@ > status = "disabled"; > #pwm-cells = <3>; > }; > + > + mmcif0: mmcif@e6bd0000 { > + compatible = "renesas,sh-mmcif"; > + reg = <0xe6bd0000 0x100>; > + interrupt-parent = <&gic>; > + interrupts = <0 56 4 > + 0 57 4>; > + status = "disabled"; > + }; > + > + sdhi0: sdhi@e6850000 { > + compatible = "renesas,sdhi-r8a7740"; > + reg = <0xe6850000 0x100>; > + interrupt-parent = <&gic>; > + interrupts = <0 117 4 > + 0 118 4 > + 0 119 4>; > + cap-sd-highspeed; > + cap-sdio-irq; > + status = "disabled"; > + }; > + > + sdhi1: sdhi@e6860000 { > + compatible = "renesas,sdhi-r8a7740"; > + reg = <0xe6860000 0x100>; > + interrupt-parent = <&gic>; > + interrupts = <0 121 4 > + 0 122 4 > + 0 123 4>; > + cap-sd-highspeed; > + cap-sdio-irq; > + status = "disabled"; > + }; > }; > -- > 1.7.2.5 > -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ Updated devicetree ML addres ] On Wed, Sep 25, 2013 at 02:36:36PM +0900, Simon Horman wrote: > On Mon, Sep 23, 2013 at 05:38:47PM +0200, Guennadi Liakhovetski wrote: > > Add SDHI0, SDHI1 and MMCIF interfaces to armadillo800eva-reference with > > regulators and pin configurations. > > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com> > > --- > > > > v4: > > > > 1. now that PFC pinctrl is usable with DT, we can use proper regulators > > and pin configurations on armadillo800eva > > > > 2. corrected SDHI compatibility strings > > > > 3. RFC because I don't know how to enable choosing between CON14 and CON8. > > In .c version this is done by reading GPIO 6. To do the same in DT mode > > we'd probably have to use some run-time DT patching, which isn't possible > > yet, AFAICS. > > I agree with your reasoning there, though perhaps Laurent or Magnus > have a more enlightened view of things. > > To my way of thinking it would be good to merge this change as is, > which an appropriate disclaimer about functionality in the changelog > above the scissors. But I do not feel strongly about this and I > am happy to wait for fuller functionality. > > > > > .../boot/dts/r8a7740-armadillo800eva-reference.dts | 83 ++++++++++++++++++++ > > arch/arm/boot/dts/r8a7740.dtsi | 33 ++++++++ > > 2 files changed, 116 insertions(+), 0 deletions(-) > > > > diff --git a/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts b/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts > > index c638e4a..af15be0 100644 > > --- a/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts > > +++ b/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts > > @@ -34,6 +34,44 @@ > > regulator-boot-on; > > }; > > > > + vcc_sdhi0: regulator@1 { > > + compatible = "regulator-fixed"; > > + > > + regulator-name = "SDHI0 Vcc"; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + > > + gpio = <&pfc 75 GPIO_ACTIVE_HIGH>; > > + enable-active-high; > > + }; > > + > > + vcc_sdhi1: regulator@2 { > > + compatible = "regulator-fixed"; > > + > > + regulator-name = "SDHI1 Vcc"; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + > > + gpio = <&pfc 16 GPIO_ACTIVE_HIGH>; > > + enable-active-high; > > + }; > > + > > + vccq_sdhi0: regulator@3 { > > + compatible = "regulator-gpio"; > > + > > + regulator-name = "SDHI0 VccQ"; > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <3300000>; > > + vin-supply = <&vcc_sdhi0>; > > + > > + enable-gpio = <&pfc 74 GPIO_ACTIVE_HIGH>; > > + gpios = <&pfc 17 GPIO_ACTIVE_HIGH>; > > + states = <3300000 0 > > + 1800000 1>; > > + > > + enable-active-high; > > + }; > > + > > leds { > > compatible = "gpio-leds"; > > led1 { > > @@ -76,4 +114,49 @@ > > renesas,groups = "intc_irq10"; > > renesas,function = "intc"; > > }; > > + > > + mmc0_pins: mmc0 { > > + renesas,groups = "mmc0_data8_1", "mmc0_ctrl_1"; > > + renesas,function = "mmc0"; > > + }; > > + > > + sdhi0_pins: sdhi0 { > > + renesas,groups = "sdhi0_data4", "sdhi0_ctrl", "sdhi0_wp"; > > + renesas,function = "sdhi0"; > > + }; > > + > > + sdhi1_pins: sdhi1 { > > + renesas,groups = "sdhi1_data4", "sdhi1_ctrl", "sdhi1_cd", "sdhi1_wp"; > > + renesas,function = "sdhi1"; > > + }; > > +}; > > + > > +&mmcif0 { > > + pinctrl-0 = <&mmc0_pins>; > > + pinctrl-names = "default"; > > + > > + vmmc-supply = <®_3p3v>; > > + bus-width = <8>; > > + non-removable; > > + status = "okay"; > > +}; > > + > > +&sdhi0 { > > + pinctrl-0 = <&sdhi0_pins>; > > + pinctrl-names = "default"; > > + > > + vmmc-supply = <&vcc_sdhi0>; > > + vqmmc-supply = <&vccq_sdhi0>; > > + bus-width = <4>; > > + broken-cd; > > + status = "okay"; > > +}; > > + > > +&sdhi1 { > > + pinctrl-0 = <&sdhi1_pins>; > > + pinctrl-names = "default"; > > + > > + vmmc-supply = <&vcc_sdhi1>; > > + bus-width = <4>; > > + status = "okay"; > > }; > > diff --git a/arch/arm/boot/dts/r8a7740.dtsi b/arch/arm/boot/dts/r8a7740.dtsi > > index 44d3d52..018c02d 100644 > > --- a/arch/arm/boot/dts/r8a7740.dtsi > > +++ b/arch/arm/boot/dts/r8a7740.dtsi > > @@ -159,4 +159,37 @@ > > status = "disabled"; > > #pwm-cells = <3>; > > }; > > + > > + mmcif0: mmcif@e6bd0000 { > > + compatible = "renesas,sh-mmcif"; > > + reg = <0xe6bd0000 0x100>; > > + interrupt-parent = <&gic>; > > + interrupts = <0 56 4 > > + 0 57 4>; > > + status = "disabled"; > > + }; > > + > > + sdhi0: sdhi@e6850000 { > > + compatible = "renesas,sdhi-r8a7740"; > > + reg = <0xe6850000 0x100>; > > + interrupt-parent = <&gic>; > > + interrupts = <0 117 4 > > + 0 118 4 > > + 0 119 4>; > > + cap-sd-highspeed; > > + cap-sdio-irq; > > + status = "disabled"; > > + }; > > + > > + sdhi1: sdhi@e6860000 { > > + compatible = "renesas,sdhi-r8a7740"; > > + reg = <0xe6860000 0x100>; > > + interrupt-parent = <&gic>; > > + interrupts = <0 121 4 > > + 0 122 4 > > + 0 123 4>; > > + cap-sd-highspeed; > > + cap-sdio-irq; > > + status = "disabled"; > > + }; > > }; > > -- > > 1.7.2.5 > > -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, (CC'ing Linus Walleij) On Wednesday 25 September 2013 14:36:36 Simon Horman wrote: > On Mon, Sep 23, 2013 at 05:38:47PM +0200, Guennadi Liakhovetski wrote: > > Add SDHI0, SDHI1 and MMCIF interfaces to armadillo800eva-reference with > > regulators and pin configurations. > > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com> > > --- > > > > v4: > > > > 1. now that PFC pinctrl is usable with DT, we can use proper regulators > > and pin configurations on armadillo800eva > > > > 2. corrected SDHI compatibility strings > > > > 3. RFC because I don't know how to enable choosing between CON14 and CON8. > > In .c version this is done by reading GPIO 6. To do the same in DT mode > > we'd probably have to use some run-time DT patching, which isn't possible > > yet, AFAICS. > > I agree with your reasoning there, though perhaps Laurent or Magnus > have a more enlightened view of things. I'm tempted to say this should be handled by the boot loader, which should then patch the DT accordingly. This is probably just a poor attempt not to solve the problem in Linux though :-) Linus, do you have an opinion on this ? The board has two connectors (MMC/SD 1 and wifi module) that are not usable concurrently. The user can select which connector to use through a hardware switch that existing board code reads at init time to determine which platform devices to register and how to configure pin muxing. Are you aware of a similar problem on other boards that would have been solved already ? > To my way of thinking it would be good to merge this change as is, > which an appropriate disclaimer about functionality in the changelog > above the scissors. But I do not feel strongly about this and I > am happy to wait for fuller functionality.
On Wed, Sep 25, 2013 at 11:05 AM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: >> > 3. RFC because I don't know how to enable choosing between CON14 and CON8. >> > In .c version this is done by reading GPIO 6. To do the same in DT mode >> > we'd probably have to use some run-time DT patching, which isn't possible >> > yet, AFAICS. >> >> I agree with your reasoning there, though perhaps Laurent or Magnus >> have a more enlightened view of things. > > I'm tempted to say this should be handled by the boot loader, which should > then patch the DT accordingly. This is probably just a poor attempt not to > solve the problem in Linux though :-) This is not the first time we have come to the conclusion that Linux need to modify the device tree. And I clearly remember Grant stating that it is in principle a living datastructure, it's not like it's read-only. The actual restrictions seem to be more about things like if you need to read this GPIO to figure out how to set up the device tree you already need the base system initialized to access the GPIO so it becomes a chicken-and-egg problem. > Linus, do you have an opinion on this ? The board has two connectors (MMC/SD 1 > and wifi module) that are not usable concurrently. The user can select which > connector to use through a hardware switch that existing board code reads at > init time to determine which platform devices to register and how to configure > pin muxing. Are you aware of a similar problem on other boards that would have > been solved already ? Just because the pin control tables *can* be initialized from the device tree doesn't mean all of them *have to*. No more than all devices in the system have to come from the device tree. You can still use platform_device_register() and in some cases it is the most reasonable thing to do. So use the nice auto-detection scheme you have, and use pinctrl_register_mappings() from the machine to set up the right mapping depending on what was detected. Naturally this needs to happen before that MMC/SD or Wifi module gets probed. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi On Thu, 26 Sep 2013, Linus Walleij wrote: > On Wed, Sep 25, 2013 at 11:05 AM, Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: > > >> > 3. RFC because I don't know how to enable choosing between CON14 and CON8. > >> > In .c version this is done by reading GPIO 6. To do the same in DT mode > >> > we'd probably have to use some run-time DT patching, which isn't possible > >> > yet, AFAICS. > >> > >> I agree with your reasoning there, though perhaps Laurent or Magnus > >> have a more enlightened view of things. > > > > I'm tempted to say this should be handled by the boot loader, which should > > then patch the DT accordingly. This is probably just a poor attempt not to > > solve the problem in Linux though :-) > > This is not the first time we have come to the conclusion that Linux > need to modify the device tree. In fact, I'm not even sure we have to select. Please correct me if I'm wrong, but to me it looks like that switch switches the SDHI1 (second SD-card) interface to either connect to an SD-card connector, or to an SDIO WLAN module. Do we really have to distinguish between the two? Cannot we just enable the interface and probe the card? Obviously, the card detection will not work in the SDIO case. Is this the only difference? If so, maybe we could make a special card-detection routine, that would return true, if the switch is in SDIO position, and read the real CD pin, if the switch activates the slot? Thanks Guennadi > And I clearly remember Grant stating that it is in principle a living > datastructure, it's not like it's read-only. > > The actual restrictions seem to be more about things like if you > need to read this GPIO to figure out how to set up the device tree > you already need the base system initialized to access the GPIO > so it becomes a chicken-and-egg problem. > > > Linus, do you have an opinion on this ? The board has two connectors (MMC/SD 1 > > and wifi module) that are not usable concurrently. The user can select which > > connector to use through a hardware switch that existing board code reads at > > init time to determine which platform devices to register and how to configure > > pin muxing. Are you aware of a similar problem on other boards that would have > > been solved already ? > > Just because the pin control tables *can* be initialized from the device > tree doesn't mean all of them *have to*. > > No more than all devices in the system have to come from the device > tree. You can still use platform_device_register() and in some cases > it is the most reasonable thing to do. > > So use the nice auto-detection scheme you have, and use > pinctrl_register_mappings() from the machine to set up the right > mapping depending on what was detected. Naturally this needs > to happen before that MMC/SD or Wifi module gets probed. > > Yours, > Linus Walleij > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 23, 2013 at 8:38 AM, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > Add SDHI0, SDHI1 and MMCIF interfaces to armadillo800eva-reference with > regulators and pin configurations. > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com> > --- > > v4: > > 1. now that PFC pinctrl is usable with DT, we can use proper regulators > and pin configurations on armadillo800eva Good, thanks for your efforts. > 2. corrected SDHI compatibility strings Ok, thanks. > 3. RFC because I don't know how to enable choosing between CON14 and CON8. > In .c version this is done by reading GPIO 6. To do the same in DT mode > we'd probably have to use some run-time DT patching, which isn't possible > yet, AFAICS. Regarding run time checking, this seems to me that it is something that cannot be supported by DT at this time. Because of that, would it be possible to reduce the scope of this patch to only cover the MMC device that is unrelated to GPIO 6? I propose that the other two devices should be handled my platform device code until DT can do runtime switching or when we can use pinctrl together with bind/unbind to select which MMC device to use. Thanks, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Magnus On Thu, 26 Sep 2013, Magnus Damm wrote: > On Mon, Sep 23, 2013 at 8:38 AM, Guennadi Liakhovetski > <g.liakhovetski@gmx.de> wrote: > > Add SDHI0, SDHI1 and MMCIF interfaces to armadillo800eva-reference with > > regulators and pin configurations. > > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com> > > --- > > > > v4: > > > > 1. now that PFC pinctrl is usable with DT, we can use proper regulators > > and pin configurations on armadillo800eva > > Good, thanks for your efforts. > > > 2. corrected SDHI compatibility strings > > Ok, thanks. > > > 3. RFC because I don't know how to enable choosing between CON14 and CON8. > > In .c version this is done by reading GPIO 6. To do the same in DT mode > > we'd probably have to use some run-time DT patching, which isn't possible > > yet, AFAICS. > > Regarding run time checking, this seems to me that it is something > that cannot be supported by DT at this time. Because of that, would it > be possible to reduce the scope of this patch to only cover the MMC > device that is unrelated to GPIO 6? > > I propose that the other two devices should be handled my platform > device code until DT can do runtime switching or when we can use > pinctrl together with bind/unbind to select which MMC device to use. Yes, this is another possibility. But in an earlier email I proposed another solution, which could work, if my assumption about the use of the same pin configuration in both cases is correct. I'll try to flip the switch and test if I can detect the WLAN module (if it is mounted on my board). I'll report back later. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Guennadi, On Thu, Sep 26, 2013 at 2:57 AM, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > Hi Magnus > > On Thu, 26 Sep 2013, Magnus Damm wrote: > >> On Mon, Sep 23, 2013 at 8:38 AM, Guennadi Liakhovetski >> <g.liakhovetski@gmx.de> wrote: >> > Add SDHI0, SDHI1 and MMCIF interfaces to armadillo800eva-reference with >> > regulators and pin configurations. >> > >> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com> >> > --- >> > >> > v4: >> > >> > 1. now that PFC pinctrl is usable with DT, we can use proper regulators >> > and pin configurations on armadillo800eva >> >> Good, thanks for your efforts. >> >> > 2. corrected SDHI compatibility strings >> >> Ok, thanks. >> >> > 3. RFC because I don't know how to enable choosing between CON14 and CON8. >> > In .c version this is done by reading GPIO 6. To do the same in DT mode >> > we'd probably have to use some run-time DT patching, which isn't possible >> > yet, AFAICS. >> >> Regarding run time checking, this seems to me that it is something >> that cannot be supported by DT at this time. Because of that, would it >> be possible to reduce the scope of this patch to only cover the MMC >> device that is unrelated to GPIO 6? Any thoughts about this? Please break out things that are clear and send them off before blocking for non-obvious things. >> I propose that the other two devices should be handled my platform >> device code until DT can do runtime switching or when we can use >> pinctrl together with bind/unbind to select which MMC device to use. > > Yes, this is another possibility. But in an earlier email I proposed > another solution, which could work, if my assumption about the use of the > same pin configuration in both cases is correct. I'll try to flip the > switch and test if I can detect the WLAN module (if it is mounted on my > board). I'll report back later. Ok, thanks. / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Linus, On Thursday 26 September 2013 09:46:24 Linus Walleij wrote: > On Wed, Sep 25, 2013 at 11:05 AM, Laurent Pinchart wrote: > >> > 3. RFC because I don't know how to enable choosing between CON14 and > >> > CON8. > >> > In .c version this is done by reading GPIO 6. To do the same in DT mode > >> > we'd probably have to use some run-time DT patching, which isn't > >> > possible yet, AFAICS. > >> > >> I agree with your reasoning there, though perhaps Laurent or Magnus > >> have a more enlightened view of things. > > > > I'm tempted to say this should be handled by the boot loader, which should > > then patch the DT accordingly. This is probably just a poor attempt not to > > solve the problem in Linux though :-) > > This is not the first time we have come to the conclusion that Linux > need to modify the device tree. > > And I clearly remember Grant stating that it is in principle a living > datastructure, it's not like it's read-only. > > The actual restrictions seem to be more about things like if you > need to read this GPIO to figure out how to set up the device tree > you already need the base system initialized to access the GPIO > so it becomes a chicken-and-egg problem. Exactly. > > Linus, do you have an opinion on this ? The board has two connectors > > (MMC/SD 1 and wifi module) that are not usable concurrently. The user can > > select which connector to use through a hardware switch that existing > > board code reads at init time to determine which platform devices to > > register and how to configure pin muxing. Are you aware of a similar > > problem on other boards that would have been solved already ? > > Just because the pin control tables *can* be initialized from the device > tree doesn't mean all of them *have to*. > > No more than all devices in the system have to come from the device > tree. You can still use platform_device_register() and in some cases > it is the most reasonable thing to do. > > So use the nice auto-detection scheme you have, and use > pinctrl_register_mappings() from the machine to set up the right > mapping depending on what was detected. Naturally this needs > to happen before that MMC/SD or Wifi module gets probed. I'd like to avoid moving the MMC/SD controller registration from the soc.dtsi file to board code because one board does something weird. SoC devices should really be declared in soc.dtsi, albeit in a disabled state. Is there a way to patch the device tree in the board code init function to turn "disabled" into "okay" based on the state of a GPIO ? I suppose not if the GPIO controller is instantiated from DT. Is there any board function we could hook this in ?
On Fri, Sep 27, 2013 at 2:55 AM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > Is there a way to patch the device tree in the board code init function to > turn "disabled" into "okay" based on the state of a GPIO ? Can't you just locate the node using of_find_node_by_path() or whatever and then use this: int of_update_property(struct device_node *np, struct property *newprop)? It's not like I'm a master of DT but... I'd very much like to know as well, because it'd be very useful :-) Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Linus, On Friday 27 September 2013 16:08:31 Linus Walleij wrote: > On Fri, Sep 27, 2013 at 2:55 AM, Laurent Pinchart wrote: > > Is there a way to patch the device tree in the board code init function to > > turn "disabled" into "okay" based on the state of a GPIO ? > > Can't you just locate the node using of_find_node_by_path() > or whatever and then use this: > int of_update_property(struct device_node *np, struct property *newprop)? Where can I do that ? I need the PFC/GPIO devices to have been probed, so board init isn't an option, but I also need the device to be enabled not to have been probed yet. One option would be to use a bus notifier to find out when the PFC/GPIO devices are available, and extend of_update_property() with a notification mechanism (either generic, or specific to the enabled propery) to detect when a device gets enabled and probe it. Before implementing that, I'd like to know if this approach has a chance to be accepted. > It's not like I'm a master of DT but... I'd very much like to know as well, > because it'd be very useful :-)
On Sun, Sep 29, 2013 at 8:49 AM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > On Friday 27 September 2013 16:08:31 Linus Walleij wrote: >> Can't you just locate the node using of_find_node_by_path() >> or whatever and then use this: >> int of_update_property(struct device_node *np, struct property *newprop)? > > Where can I do that ? I need the PFC/GPIO devices to have been probed, so > board init isn't an option, but I also need the device to be enabled not to > have been probed yet. > > One option would be to use a bus notifier to find out when the PFC/GPIO > devices are available, and extend of_update_property() with a notification > mechanism (either generic, or specific to the enabled propery) to detect when > a device gets enabled and probe it. Argh that sounds awfully complicated... I would consider doing this right inside the PCF/GPIO driver right at the end of its probe function. As it is related to pins anyway... or is that too ugly? Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Linus, [Updated the devicetree mailing list e-mail address] On Monday 30 September 2013 01:20:46 Linus Walleij wrote: > On Sun, Sep 29, 2013 at 8:49 AM, Laurent Pinchart wrote: > > On Friday 27 September 2013 16:08:31 Linus Walleij wrote: > >> Can't you just locate the node using of_find_node_by_path() > >> or whatever and then use this: > >> int of_update_property(struct device_node *np, struct property *newprop)? > > > > Where can I do that ? I need the PFC/GPIO devices to have been probed, so > > board init isn't an option, but I also need the device to be enabled not > > to have been probed yet. > > > > One option would be to use a bus notifier to find out when the PFC/GPIO > > devices are available, and extend of_update_property() with a notification > > mechanism (either generic, or specific to the enabled propery) to detect > > when a device gets enabled and probe it. > > Argh that sounds awfully complicated... > > I would consider doing this right inside the PCF/GPIO driver right at the > end of its probe function. As it is related to pins anyway... or is that too > ugly? As a reminder, we need to decide whether to register an SDHI (SD controller) device or a WiFi module device, and thus configure pinmuxing appropriately, based on the state of a GPIO connected to a user-accessible switch. This behaviour is board-specific, not SoC-specific. That's why I believe the code should be in board code.
On Thu, Sep 26, 2013 at 8:46 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Wed, Sep 25, 2013 at 11:05 AM, Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: > >>> > 3. RFC because I don't know how to enable choosing between CON14 and CON8. >>> > In .c version this is done by reading GPIO 6. To do the same in DT mode >>> > we'd probably have to use some run-time DT patching, which isn't possible >>> > yet, AFAICS. >>> >>> I agree with your reasoning there, though perhaps Laurent or Magnus >>> have a more enlightened view of things. >> >> I'm tempted to say this should be handled by the boot loader, which should >> then patch the DT accordingly. This is probably just a poor attempt not to >> solve the problem in Linux though :-) > > This is not the first time we have come to the conclusion that Linux > need to modify the device tree. > > And I clearly remember Grant stating that it is in principle a living > datastructure, it's not like it's read-only. You still don't want the kernel modifying the DT. The DT is primarily a communication mechanism from the firmware/platform to the kernel. If the platform is responsible for describing the correct configuration then it belongs in the DT. If however the kernel needs to do the work of figuring out which configuration to use at runtime, then it would be better for the DT to describe the possible configurations and let the kernel choose the appropriate one. > The actual restrictions seem to be more about things like if you > need to read this GPIO to figure out how to set up the device tree > you already need the base system initialized to access the GPIO > so it becomes a chicken-and-egg problem. >> Linus, do you have an opinion on this ? The board has two connectors (MMC/SD 1 >> and wifi module) that are not usable concurrently. The user can select which >> connector to use through a hardware switch that existing board code reads at >> init time to determine which platform devices to register and how to configure >> pin muxing. Are you aware of a similar problem on other boards that would have >> been solved already ? > > Just because the pin control tables *can* be initialized from the device > tree doesn't mean all of them *have to*. Right. Also, there are patches floating about that allow additional device tree fragments to be loaded after the rest of the system is booted. It was specifically designed for the beaglebone expansion connectors. That might be the way to solve your problem here. (I really want to get that feature merged, but I sheepishly admit that I haven't been able to spend any time on it) :-( g. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Grant, On Monday 30 September 2013 14:15:15 Grant Likely wrote: > On Thu, Sep 26, 2013 at 8:46 AM, Linus Walleij wrote: > > On Wed, Sep 25, 2013 at 11:05 AM, Laurent Pinchart wrote: > >>> > 3. RFC because I don't know how to enable choosing between CON14 and > >>> > CON8. In .c version this is done by reading GPIO 6. To do the same in > >>> > DT mode we'd probably have to use some run-time DT patching, which > >>> > isn't possible yet, AFAICS. > >>> > >>> I agree with your reasoning there, though perhaps Laurent or Magnus > >>> have a more enlightened view of things. > >> > >> I'm tempted to say this should be handled by the boot loader, which > >> should then patch the DT accordingly. This is probably just a poor > >> attempt not to solve the problem in Linux though :-) > > > > This is not the first time we have come to the conclusion that Linux > > need to modify the device tree. > > > > And I clearly remember Grant stating that it is in principle a living > > datastructure, it's not like it's read-only. > > You still don't want the kernel modifying the DT. The DT is primarily > a communication mechanism from the firmware/platform to the kernel. If > the platform is responsible for describing the correct configuration > then it belongs in the DT. If however the kernel needs to do the work > of figuring out which configuration to use at runtime, then it would > be better for the DT to describe the possible configurations and let > the kernel choose the appropriate one. Could you please elaborate a bit on how you envision this being implemented ? > > The actual restrictions seem to be more about things like if you > > need to read this GPIO to figure out how to set up the device tree > > you already need the base system initialized to access the GPIO > > so it becomes a chicken-and-egg problem. > > > >> Linus, do you have an opinion on this ? The board has two connectors > >> (MMC/SD 1 and wifi module) that are not usable concurrently. The user > >> can select which connector to use through a hardware switch that > >> existing board code reads at init time to determine which platform > >> devices to register and how to configure pin muxing. Are you aware of a > >> similar problem on other boards that would have been solved already ? > > > > Just because the pin control tables *can* be initialized from the device > > tree doesn't mean all of them *have to*. > > Right. Also, there are patches floating about that allow additional device > tree fragments to be loaded after the rest of the system is booted. It was > specifically designed for the beaglebone expansion connectors. That might be > the way to solve your problem here. (I really want to get that feature > merged, but I sheepishly admit that I haven't been able to spend any time on > it) :-(
On Mon, Sep 30, 2013 at 1:10 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > Hi Linus, > > [Updated the devicetree mailing list e-mail address] > > On Monday 30 September 2013 01:20:46 Linus Walleij wrote: >> On Sun, Sep 29, 2013 at 8:49 AM, Laurent Pinchart wrote: >> > On Friday 27 September 2013 16:08:31 Linus Walleij wrote: >> >> Can't you just locate the node using of_find_node_by_path() >> >> or whatever and then use this: >> >> int of_update_property(struct device_node *np, struct property *newprop)? >> > >> > Where can I do that ? I need the PFC/GPIO devices to have been probed, so >> > board init isn't an option, but I also need the device to be enabled not >> > to have been probed yet. >> > >> > One option would be to use a bus notifier to find out when the PFC/GPIO >> > devices are available, and extend of_update_property() with a notification >> > mechanism (either generic, or specific to the enabled propery) to detect >> > when a device gets enabled and probe it. >> >> Argh that sounds awfully complicated... >> >> I would consider doing this right inside the PCF/GPIO driver right at the >> end of its probe function. As it is related to pins anyway... or is that too >> ugly? > > As a reminder, we need to decide whether to register an SDHI (SD controller) > device or a WiFi module device, and thus configure pinmuxing appropriately, > based on the state of a GPIO connected to a user-accessible switch. This > behaviour is board-specific, not SoC-specific. That's why I believe the code > should be in board code. Hm, I might have misunderstood this earlier, so if the board code is going to register *either* SDHI *or* WIFI, both as some kind of struct device then as you say: > The board has two connectors (MMC/SD 1 > and wifi module) that are not usable concurrently. The user can select which > connector to use through a hardware switch that existing board code reads at > init time (...) So depending on what you read there you're going to register a struct device named mmc0 or wifi0 or something, then you just register the pin control tables for both cases and the apropriate default state will be selected by the device core right before the device driver gets probed. (grep for pinctrl_bind_pins). It is perfectly fine to have unused pin states in the table. Then the problem is unrelated to pin control and more about how to register these devices, nothing to do with pin control. Such as if they should both be marked as "disabled" in the DT and activated by the kernel, or registered by adding a platform device or something... But I guess I got it wrong? Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Linus (and Guennadi and Grant, there are questions for your below), On Tuesday 08 October 2013 13:19:04 Linus Walleij wrote: > On Mon, Sep 30, 2013 at 1:10 PM, Laurent Pinchart wrote: > > Hi Linus, > > > > [Updated the devicetree mailing list e-mail address] > > > > On Monday 30 September 2013 01:20:46 Linus Walleij wrote: > >> On Sun, Sep 29, 2013 at 8:49 AM, Laurent Pinchart wrote: > >> > On Friday 27 September 2013 16:08:31 Linus Walleij wrote: > >> >> Can't you just locate the node using of_find_node_by_path() > >> >> or whatever and then use this: > >> >> int of_update_property(struct device_node *np, struct property > >> >> *newprop)? > >> > > >> > Where can I do that ? I need the PFC/GPIO devices to have been probed, > >> > so board init isn't an option, but I also need the device to be enabled > >> > not to have been probed yet. > >> > > >> > One option would be to use a bus notifier to find out when the PFC/GPIO > >> > devices are available, and extend of_update_property() with a > >> > notification mechanism (either generic, or specific to the enabled > >> > propery) to detect when a device gets enabled and probe it. > >> > >> Argh that sounds awfully complicated... > >> > >> I would consider doing this right inside the PCF/GPIO driver right at the > >> end of its probe function. As it is related to pins anyway... or is that > >> too ugly? > > > > As a reminder, we need to decide whether to register an SDHI (SD > > controller) device or a WiFi module device, and thus configure pinmuxing > > appropriately, based on the state of a GPIO connected to a > > user-accessible switch. This behaviour is board-specific, not > > SoC-specific. That's why I believe the code should be in board code. > > Hm, I might have misunderstood this earlier, so if the board code is > going to register *either* SDHI *or* WIFI, both as some kind of > struct device then as you say: > > > The board has two connectors (MMC/SD 1 > > and wifi module) that are not usable concurrently. The user can select > > which connector to use through a hardware switch that existing board code > > reads at init time (...) > > So depending on what you read there you're going to register a struct device > named mmc0 or wifi0 or something, then you just register the pin control > tables for both cases and the apropriate default state will be selected by > the device core right before the device driver gets probed. (grep for > pinctrl_bind_pins). It is perfectly fine to have unused pin states in the > table. > > Then the problem is unrelated to pin control and more about how to register > these devices, nothing to do with pin control. Correct. > Such as if they should both be marked as "disabled" in the DT and activated > by the kernel, or registered by adding a platform device or something... I believe a way to mark a disabled device as enabled from board code would be interesting. The function could be called from a platform bus notifier that reacts on the GPIO device being probed. It's a bit of a corner case, but it doesn't sound too hackish to me. Grant, would you be fine with such a mechanism ? The devices would be marked with status = "disabled" in DT and a new function would be added to enable them. That function would modify the DT status to "okay" and trigger device registration. > But I guess I got it wrong? I believe you got it right. At least your understanding matches mine :-) Guennadi, could you please confirm ?
diff --git a/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts b/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts index c638e4a..af15be0 100644 --- a/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts +++ b/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts @@ -34,6 +34,44 @@ regulator-boot-on; }; + vcc_sdhi0: regulator@1 { + compatible = "regulator-fixed"; + + regulator-name = "SDHI0 Vcc"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + + gpio = <&pfc 75 GPIO_ACTIVE_HIGH>; + enable-active-high; + }; + + vcc_sdhi1: regulator@2 { + compatible = "regulator-fixed"; + + regulator-name = "SDHI1 Vcc"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + + gpio = <&pfc 16 GPIO_ACTIVE_HIGH>; + enable-active-high; + }; + + vccq_sdhi0: regulator@3 { + compatible = "regulator-gpio"; + + regulator-name = "SDHI0 VccQ"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + vin-supply = <&vcc_sdhi0>; + + enable-gpio = <&pfc 74 GPIO_ACTIVE_HIGH>; + gpios = <&pfc 17 GPIO_ACTIVE_HIGH>; + states = <3300000 0 + 1800000 1>; + + enable-active-high; + }; + leds { compatible = "gpio-leds"; led1 { @@ -76,4 +114,49 @@ renesas,groups = "intc_irq10"; renesas,function = "intc"; }; + + mmc0_pins: mmc0 { + renesas,groups = "mmc0_data8_1", "mmc0_ctrl_1"; + renesas,function = "mmc0"; + }; + + sdhi0_pins: sdhi0 { + renesas,groups = "sdhi0_data4", "sdhi0_ctrl", "sdhi0_wp"; + renesas,function = "sdhi0"; + }; + + sdhi1_pins: sdhi1 { + renesas,groups = "sdhi1_data4", "sdhi1_ctrl", "sdhi1_cd", "sdhi1_wp"; + renesas,function = "sdhi1"; + }; +}; + +&mmcif0 { + pinctrl-0 = <&mmc0_pins>; + pinctrl-names = "default"; + + vmmc-supply = <®_3p3v>; + bus-width = <8>; + non-removable; + status = "okay"; +}; + +&sdhi0 { + pinctrl-0 = <&sdhi0_pins>; + pinctrl-names = "default"; + + vmmc-supply = <&vcc_sdhi0>; + vqmmc-supply = <&vccq_sdhi0>; + bus-width = <4>; + broken-cd; + status = "okay"; +}; + +&sdhi1 { + pinctrl-0 = <&sdhi1_pins>; + pinctrl-names = "default"; + + vmmc-supply = <&vcc_sdhi1>; + bus-width = <4>; + status = "okay"; }; diff --git a/arch/arm/boot/dts/r8a7740.dtsi b/arch/arm/boot/dts/r8a7740.dtsi index 44d3d52..018c02d 100644 --- a/arch/arm/boot/dts/r8a7740.dtsi +++ b/arch/arm/boot/dts/r8a7740.dtsi @@ -159,4 +159,37 @@ status = "disabled"; #pwm-cells = <3>; }; + + mmcif0: mmcif@e6bd0000 { + compatible = "renesas,sh-mmcif"; + reg = <0xe6bd0000 0x100>; + interrupt-parent = <&gic>; + interrupts = <0 56 4 + 0 57 4>; + status = "disabled"; + }; + + sdhi0: sdhi@e6850000 { + compatible = "renesas,sdhi-r8a7740"; + reg = <0xe6850000 0x100>; + interrupt-parent = <&gic>; + interrupts = <0 117 4 + 0 118 4 + 0 119 4>; + cap-sd-highspeed; + cap-sdio-irq; + status = "disabled"; + }; + + sdhi1: sdhi@e6860000 { + compatible = "renesas,sdhi-r8a7740"; + reg = <0xe6860000 0x100>; + interrupt-parent = <&gic>; + interrupts = <0 121 4 + 0 122 4 + 0 123 4>; + cap-sd-highspeed; + cap-sdio-irq; + status = "disabled"; + }; };
Add SDHI0, SDHI1 and MMCIF interfaces to armadillo800eva-reference with regulators and pin configurations. Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com> --- v4: 1. now that PFC pinctrl is usable with DT, we can use proper regulators and pin configurations on armadillo800eva 2. corrected SDHI compatibility strings 3. RFC because I don't know how to enable choosing between CON14 and CON8. In .c version this is done by reading GPIO 6. To do the same in DT mode we'd probably have to use some run-time DT patching, which isn't possible yet, AFAICS. .../boot/dts/r8a7740-armadillo800eva-reference.dts | 83 ++++++++++++++++++++ arch/arm/boot/dts/r8a7740.dtsi | 33 ++++++++ 2 files changed, 116 insertions(+), 0 deletions(-)