Hi Alexandre and Geert, On Friday, March 17, 2017, Alexandre Belloni wrote: > On 17/03/2017 at 14:55:48 +0100, Geert Uytterhoeven wrote: > > >> it would still be good to have phandles to the external clock > > >> sources as well, as that describes the hardware topology. > > > > > > I'm confused, you mean make new clocks node for RTC_X1 (fixed at > > > 32.768kHZ) and > > > RTC_X3 (fixed at 4MHz), but then not really do anything with then? > > > (the driver > > > > You still do something with them, as they'll be linked from the rtc > device node. > > > > > doesn't need them) > > > > The driver may use them in the future. > > E.g. to select among X1, X3, or EXTAL, depending on availability. > > > > I'm on Geert's side here, I wouldn't rely on the bootloader to set > everything up correctly. Part of my hesitation to putting in the clock source selection was that this RTC block is used in a lot of parts, and it's always the same, except for the last 3 registers which are different from device to device because different SoCs have different clock sources options and ways to set them up. So, making a driver that just 'reads' the time seemed safer/easier. Anyway, I'll add the clock setup in if that's the general consensus. But, before I send out an updated version, what were you thinking my DT should look like? Should I make a "count-source" property for user to select what clock will be used for the actually counting? clocks { ranges; #address-cells = <1>; #size-cells = <1>; . . . + rtc_x1_clk: rtc_x1 { + #clock-cells = <0>; + compatible = "fixed-clock"; + /* If clk present, value must be set by board to 32678 */ + clock-frequency = <0>; + }; + + rtc_x3_clk: rtc_x3 { + #clock-cells = <0>; + compatible = "fixed-clock"; + /* If clk present, value must be set by board to 4000000 */ + clock-frequency = <0>; + }; + + rtc: rtc@fcff1000 { + compatible = "renesas,r7s72100-rtc", "renesas,sh-rtc"; + reg = <0xfcff1000 0x2e>; + interrupts = <GIC_SPI 276 IRQ_TYPE_EDGE_RISING + GIC_SPI 277 IRQ_TYPE_EDGE_RISING + GIC_SPI 278 IRQ_TYPE_EDGE_RISING>; + interrupt-names = "alarm", "period", "carry"; + clocks = <&mstp6_clks R7S72100_CLK_RTC>, <&rtc_x1_clk>, + <&extal_clk> , <&rtc_x3_clk>; + clock-names = "fck", "rtc_x1", "extal", "rtc_x3"; + power-domains = <&cpg_clocks>; + count-source = "rtc_x1"; <<<<<<< this would be in the board dts file + status = "disabled"; + }; Thank you, Chris -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html