Hi Chris,
On Mon, Mar 20, 2017 at 4:05 PM, Chris Brandt <Chris.Brandt@xxxxxxxxxxx> wrote:
> 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
IMHO that's software policy, not hardware description, so it doesn't
belong in DT.
However, by scanning the list of allowed clock names, and looking at clock
rates, the driver can find a suitable clock itself.
> + status = "disabled";
> + };
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
--
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
