| Message ID | 20170121030604.7672-3-chris.brandt@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
On Fri, Jan 20, 2017 at 10:06:03PM -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. The 2nd clock is for the internal > card detect logic. > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
On Sat, Jan 21, 2017 at 4:06 AM, 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. The 2nd clock is for the internal > card detect logic. > > Signed-off-by: Chris Brandt <chris.brandt@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
On Fri, Jan 20, 2017 at 10:06:03PM -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. The 2nd clock is for the internal > card detect logic. > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> > --- > v4: > * just explain there might be 2 clocks, don't explain how > we will use them in the driver > v3: > * add more clarification about why there are sometimes 2 clocks > and what you should do with them. > * remove 'status = "disabled"' from example > v2: > * fix spelling and change wording > * changed clock name from "carddetect" to "cd" > --- > Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 24 ++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt > index a1650ed..1464c16 100644 > --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt > +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt > @@ -25,8 +25,32 @@ 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, on > + some variations of this controller, the internal card detection This should be explicit as to which compatible strings have 2 clocks and which have 1 clock. > + logic that exists in this controller is sectioned off to be run by a > + separate second clock source to allow the main core clock to be turned > + off to save power. > + If 2 clocks are specified by the hardware, 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 { mmc@... > + 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; > + }; > -- > 2.10.1 > >
Hello Rob, On Monday, January 23, 2017, Rob Herring wrote: > > --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt > > +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt > > @@ -25,8 +25,32 @@ 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, on > > + some variations of this controller, the internal card detection > > This should be explicit as to which compatible strings have 2 clocks and > which have 1 clock. OK. I'll make a list like I did for sh_mmcif: https://patchwork.kernel.org/patch/9512091/ > > + > > +Example showing 2 clocks: > > + sdhi0: sd@e804e000 { > > mmc@... I'm confused. I see that for all SDHI controllers, it either "sd@" or "sdhci@". $ grep sdhi $(find arch/arm/boot/dts -name "*.dtsi") $ grep sdhci $(find arch/arm/boot/dts -name "*.dtsi") Regards, Chris
On Mon, Jan 23, 2017 at 11:56 AM, Chris Brandt <Chris.Brandt@renesas.com> wrote: > Hello Rob, > > > On Monday, January 23, 2017, Rob Herring wrote: >> > --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt >> > +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt >> > @@ -25,8 +25,32 @@ 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, on >> > + some variations of this controller, the internal card detection >> >> This should be explicit as to which compatible strings have 2 clocks and >> which have 1 clock. > > OK. I'll make a list like I did for sh_mmcif: > > https://patchwork.kernel.org/patch/9512091/ > > > >> > + >> > +Example showing 2 clocks: >> > + sdhi0: sd@e804e000 { >> >> mmc@... > > I'm confused. I see that for all SDHI controllers, it either "sd@" or "sdhci@". > > $ grep sdhi $(find arch/arm/boot/dts -name "*.dtsi") > > $ grep sdhci $(find arch/arm/boot/dts -name "*.dtsi") Yes, there's lots of variation. Node names are supposed to be generic for their class of device (e.g. ethernet, pci, usb, interrupt-controller, etc.). I'd be fine with "sd", but think "mmc" is more common. Either way, we should pick one moving forward. "sdhci" should not be used as that's a specific implementation. Rob
On Thursday, January 26, 2017, Rob Herring wrote: > >> > + > >> > +Example showing 2 clocks: > >> > + sdhi0: sd@e804e000 { > >> > >> mmc@... > > > > I'm confused. I see that for all SDHI controllers, it either "sd@" or > "sdhci@". > > > > $ grep sdhi $(find arch/arm/boot/dts -name "*.dtsi") > > > > $ grep sdhci $(find arch/arm/boot/dts -name "*.dtsi") > > Yes, there's lots of variation. Node names are supposed to be generic for > their class of device (e.g. ethernet, pci, usb, interrupt-controller, > etc.). I'd be fine with "sd", but think "mmc" is more common. Either way, > we should pick one moving forward. "sdhci" > should not be used as that's a specific implementation. > > Rob For now, I took the example back out of tmio_mmc.txt since I figured people can refer back to mmc.txt for what the naming should be (since the directory name is mmc, you would think the golden rules were in mmc.txt) > I'd be fine with "sd", but think "mmc" is more common. NOTE: mmc.txt currently has 2 examples: sdhci@ab000000 { mmc3: mmc@01c12000 { Chris
On 26 January 2017 at 15:39, Rob Herring <robh@kernel.org> wrote: > On Mon, Jan 23, 2017 at 11:56 AM, Chris Brandt <Chris.Brandt@renesas.com> wrote: >> Hello Rob, >> >> >> On Monday, January 23, 2017, Rob Herring wrote: >>> > --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt >>> > +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt >>> > @@ -25,8 +25,32 @@ 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, on >>> > + some variations of this controller, the internal card detection >>> >>> This should be explicit as to which compatible strings have 2 clocks and >>> which have 1 clock. >> >> OK. I'll make a list like I did for sh_mmcif: >> >> https://patchwork.kernel.org/patch/9512091/ >> >> >> >>> > + >>> > +Example showing 2 clocks: >>> > + sdhi0: sd@e804e000 { >>> >>> mmc@... >> >> I'm confused. I see that for all SDHI controllers, it either "sd@" or "sdhci@". >> >> $ grep sdhi $(find arch/arm/boot/dts -name "*.dtsi") >> >> $ grep sdhci $(find arch/arm/boot/dts -name "*.dtsi") > > Yes, there's lots of variation. Node names are supposed to be generic > for their class of device (e.g. ethernet, pci, usb, > interrupt-controller, etc.). I'd be fine with "sd", but think "mmc" is > more common. Either way, we should pick one moving forward. "sdhci" > should not be used as that's a specific implementation. I think we discussed this earlier. The problem with these devices is that it covers several type of devices, so it's hard to find a generic node name. The devices are: MMC, eMMC, SD, SDIO. I don't have strong opinions on whether to chose a generic node name, and perhaps "mmc" is the best we can get? Whatever we decide, we should update all mmc DTS docs, to avoid future confusion. Kind regards Uffe
diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt index a1650ed..1464c16 100644 --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt @@ -25,8 +25,32 @@ 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, on + some variations of this controller, the internal card detection + logic that exists in this controller is sectioned off to be run by a + separate second clock source to allow the main core clock to be turned + off to save power. + If 2 clocks are specified by the hardware, 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; + };
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. The 2nd clock is for the internal card detect logic. Signed-off-by: Chris Brandt <chris.brandt@renesas.com> --- v4: * just explain there might be 2 clocks, don't explain how we will use them in the driver v3: * add more clarification about why there are sometimes 2 clocks and what you should do with them. * remove 'status = "disabled"' from example v2: * fix spelling and change wording * changed clock name from "carddetect" to "cd" --- Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+)