| Message ID | 20170118172502.13876-3-chris.brandt@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
On Wed, Jan 18, 2017 at 12:25:01PM -0500, Chris Brandt wrote: > In the case of a single clock source, you don't need names. However, > if the controller has 2 clock sources, you need to name them correctly > so the driver can find the 2nd one. > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
On 18 January 2017 at 18:25, Chris Brandt <chris.brandt@renesas.com> wrote: > In the case of a single clock source, you don't need names. However, > if the controller has 2 clock sources, you need to name them correctly > so the driver can find the 2nd one. > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> > --- > v2: > * fix spelling and change wording > * changed clock name from "carddetect" to "cd" > --- > Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt > index a1650ed..90370cd 100644 > --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt > +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt > @@ -25,8 +25,29 @@ Required properties: > "renesas,sdhi-r8a7795" - SDHI IP on R8A7795 SoC > "renesas,sdhi-r8a7796" - SDHI IP on R8A7796 SoC > > +- clocks: Most controllers only have 1 clock source per channel. However, some > + have a second clock dedicated to card detection. If 2 clocks are > + specified, you must name them as "core" and "cd". If the controller > + only has 1 clock, naming is not required. Could you please elaborate a bit on the card detection clock? I guess that there is some kind of internal card detection logic (native card detect) in the SDHI IP, which requires a separate clock for it to work? Perhaps you can state that somehow? > + > Optional properties: > - toshiba,mmc-wrprotect-disable: write-protect detection is unavailable > - pinctrl-names: should be "default", "state_uhs" > - pinctrl-0: should contain default/high speed pin ctrl > - pinctrl-1: should contain uhs mode pin ctrl > + > +Example showing 2 clocks: > + sdhi0: sd@e804e000 { > + compatible = "renesas,sdhi-r7s72100"; > + reg = <0xe804e000 0x100>; > + interrupts = <GIC_SPI 270 IRQ_TYPE_LEVEL_HIGH > + GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH > + GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>; > + > + clocks = <&mstp12_clks R7S72100_CLK_SDHI00>, > + <&mstp12_clks R7S72100_CLK_SDHI01>; > + clock-names = "core", "cd"; > + cap-sd-highspeed; > + cap-sdio-irq; > + status = "disabled"; The last line seems a bit odd to include in an example. > + }; > -- > 2.10.1 > > Kind regards Uffe
Hello Ulf, Friday, January 20, 2017, Ulf Hansson wrote: > > +- clocks: Most controllers only have 1 clock source per channel. > However, some > > + have a second clock dedicated to card detection. If 2 clocks > are > > + specified, you must name them as "core" and "cd". If the > controller > > + only has 1 clock, naming is not required. > > Could you please elaborate a bit on the card detection clock? > > I guess that there is some kind of internal card detection logic (native > card detect) in the SDHI IP, which requires a separate clock for it to > work? Perhaps you can state that somehow? The reality is that the chip guys cut up the standard SDHI IP to add a 'cool new feature', but all I want to do is put it back the way it was. NOTE: The design guys like to reuse IP blocks from previous designs that they know worked and didn't have bugs. So, there is a good chance you will see this change in future RZ/A devices (or even other future Renesas SoC devices). To remove an unwanted feature adds additional design risk of breaking something else....and given the cost of redoing silicon mask layers...no engineer wants that mistake on their hands. > > +Example showing 2 clocks: > > + sdhi0: sd@e804e000 { > > + compatible = "renesas,sdhi-r7s72100"; > > + reg = <0xe804e000 0x100>; > > + interrupts = <GIC_SPI 270 IRQ_TYPE_LEVEL_HIGH > > + GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH > > + GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>; > > + > > + clocks = <&mstp12_clks R7S72100_CLK_SDHI00>, > > + <&mstp12_clks R7S72100_CLK_SDHI01>; > > + clock-names = "core", "cd"; > > + cap-sd-highspeed; > > + cap-sdio-irq; > > + status = "disabled"; > > The last line seems a bit odd to include in an example. You're right. I'll take that out. Thanks, Chris
On 20 January 2017 at 17:05, Chris Brandt <Chris.Brandt@renesas.com> wrote: > Hello Ulf, > > Friday, January 20, 2017, Ulf Hansson wrote: >> > +- clocks: Most controllers only have 1 clock source per channel. >> However, some >> > + have a second clock dedicated to card detection. If 2 clocks >> are >> > + specified, you must name them as "core" and "cd". If the >> controller >> > + only has 1 clock, naming is not required. >> >> Could you please elaborate a bit on the card detection clock? >> >> I guess that there is some kind of internal card detection logic (native >> card detect) in the SDHI IP, which requires a separate clock for it to >> work? Perhaps you can state that somehow? > > > The reality is that the chip guys cut up the standard SDHI IP to add a > 'cool new feature', but all I want to do is put it back the way it was. > > NOTE: The design guys like to reuse IP blocks from previous designs that they > know worked and didn't have bugs. So, there is a good chance you will see this > change in future RZ/A devices (or even other future Renesas SoC devices). > To remove an unwanted feature adds additional design risk of breaking > something else....and given the cost of redoing silicon mask layers...no > engineer wants that mistake on their hands. So, one should be aware of that runtime PM support in these case is going to be suboptimal. For example, when using this native card detect, you will need to keep the controller runtime PM resumed as the be able to keep the clock on and to be able to fetch the irq. Wasting power. Most SoC vendors are therefore using a GPIO card detect instead, although I assume you already knew that. :-) [...] Kind regards Uffe
Hello Uffe, On Friday, January 20, 2017, Ulf Hansson wrote: > > The reality is that the chip guys cut up the standard SDHI IP to add a > > 'cool new feature', but all I want to do is put it back the way it was. > > > > NOTE: The design guys like to reuse IP blocks from previous designs > > that they know worked and didn't have bugs. So, there is a good chance > > you will see this change in future RZ/A devices (or even other future > Renesas SoC devices). > > To remove an unwanted feature adds additional design risk of breaking > > something else....and given the cost of redoing silicon mask > > layers...no engineer wants that mistake on their hands. > > So, one should be aware of that runtime PM support in these case is going > to be suboptimal. > For example, when using this native card detect, you will need to keep the > controller runtime PM resumed as the be able to keep the clock on and to > be able to fetch the irq. Wasting power. Wolfram already pointed that out in a reply to Geert: On Tuesday, January 17, 2017, Wolfram Sang wrote: > > If we handle them as one, won't we miss card detect events due to the > > card detect clock being disabled while SDHI is idle? > > You mean this? > > 1208 /* > 1209 * While using internal tmio hardware logic for card > detection, we need > 1210 * to ensure it stays powered for it to work. > 1211 */ > 1212 if (_host->native_hotplug) > 1213 pm_runtime_get_noresume(&pdev->dev); As per your request, I'll go back and add some text to describing that even though this specific HW has a separate clock for card detect for PM, the existing driver infrastructure doesn't handle that so both clocks need to be treated as one. > Most SoC vendors are therefore using a GPIO card detect instead, although > I assume you already knew that. :-) My first objective for the RZ/A1 platform is to move things from a local custom BSP into upstream and get things 'working'. Later I can go back and tweak things here and there for runtime PM and such. Thank you, Chris
diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt index a1650ed..90370cd 100644 --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt @@ -25,8 +25,29 @@ Required properties: "renesas,sdhi-r8a7795" - SDHI IP on R8A7795 SoC "renesas,sdhi-r8a7796" - SDHI IP on R8A7796 SoC +- clocks: Most controllers only have 1 clock source per channel. However, some + have a second clock dedicated to card detection. If 2 clocks are + specified, you must name them as "core" and "cd". If the controller + only has 1 clock, naming is not required. + Optional properties: - toshiba,mmc-wrprotect-disable: write-protect detection is unavailable - pinctrl-names: should be "default", "state_uhs" - pinctrl-0: should contain default/high speed pin ctrl - pinctrl-1: should contain uhs mode pin ctrl + +Example showing 2 clocks: + sdhi0: sd@e804e000 { + compatible = "renesas,sdhi-r7s72100"; + reg = <0xe804e000 0x100>; + interrupts = <GIC_SPI 270 IRQ_TYPE_LEVEL_HIGH + GIC_SPI 271 IRQ_TYPE_LEVEL_HIGH + GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>; + + clocks = <&mstp12_clks R7S72100_CLK_SDHI00>, + <&mstp12_clks R7S72100_CLK_SDHI01>; + clock-names = "core", "cd"; + cap-sd-highspeed; + cap-sdio-irq; + status = "disabled"; + };
In the case of a single clock source, you don't need names. However, if the controller has 2 clock sources, you need to name them correctly so the driver can find the 2nd one. Signed-off-by: Chris Brandt <chris.brandt@renesas.com> --- v2: * fix spelling and change wording * changed clock name from "carddetect" to "cd" --- Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)