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
