| Message ID | 20170120194332.1683-3-chris.brandt@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> +- 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. Unfortunately, the existing driver architecture > + does not support such a separation of clocks. > + Additionally, it is prohibited to supply a clock to the core but not > + to the card detect circuit. That leaves you with if separate clocks > + are presented, you must treat them both as 1. > + 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. I know we are not perfect in this area, but DT binding should be OS agnostic and should refrain from specifying Linux internals. So, I'd think the comments about driver architecture should be put into the driver, not into the bindings. It is ultimately Ulf's decision, of course.
Hello Wolfram, Ulf's request was for more info, but it wasn't clear if he wanted more info in the commit logs or in the files themselves. On Friday, January 20, 2017, Wolfram Sang wrote: > > +- 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. Unfortunately, the existing driver architecture > > + does not support such a separation of clocks. > > + Additionally, it is prohibited to supply a clock to the core but > not > > + to the card detect circuit. That leaves you with if separate > clocks > > + are presented, you must treat them both as 1. > > + 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. > > I know we are not perfect in this area, but DT binding should be OS > agnostic and should refrain from specifying Linux internals. > > So, I'd think the comments about driver architecture should be put into > the driver, not into the bindings. It sounds like your suggestion is to just put the HW description in the DT bindings, but move the dirty details into the driver. Keep in DT: +- 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. Move somewhere in the driver (maybe where the 2nd clock is detected): + Unfortunately, the existing driver architecture + does not support such a separation of clocks. + Additionally, it is prohibited to supply a clock to the core but not + to the card detect circuit. That leaves you with if separate clocks + are presented, you must treat them both as 1. Is this what you were thinking? Thank you, Chris -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> It sounds like your suggestion is to just put the HW description in > the DT bindings, but move the dirty details into the driver. > > Keep in DT: > > +- 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. > > > > Move somewhere in the driver (maybe where the 2nd clock is detected): > > + Unfortunately, the existing driver architecture > + does not support such a separation of clocks. "such" is not explained. Maybe "does not support a seperation of clocks. It doesn't work with Runtime PM because ..."? > + Additionally, it is prohibited to supply a clock to the core but not > + to the card detect circuit. That leaves you with if separate clocks > + are presented, you must treat them both as 1. > > > Is this what you were thinking? Exactly.
diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt index a1650ed..9033c6a 100644 --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt @@ -25,8 +25,36 @@ 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. Unfortunately, the existing driver architecture + does not support such a separation of clocks. + Additionally, it is prohibited to supply a clock to the core but not + to the card detect circuit. That leaves you with if separate clocks + are presented, you must treat them both as 1. + 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 and must be enabled/disabled along with the main core clock. Signed-off-by: Chris Brandt <chris.brandt@renesas.com> --- 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 | 28 ++++++++++++++++++++++ 1 file changed, 28 insertions(+)