<snip>
>> +
>> + /* Define fixed clocks.
>> + * Notice: the following clocks are fixed value on NPCM7XX and should
>> + * not be changed.
>> + * therefor they are not exposed to the dev tree .
>
> I am not convinced. The top level .dtsi is usually SoC specific.
>
> - CLKREF should be fixed on 25MHz. I didn't want to put it in DT since
> it might appear as something that is changeable.
> on npcm750 all clocks are set in ROM +BB (several possible settings)
> , so this entire driver is only used for reading the current clock
> settings. All clocks are read only and derived from CLKREF. CLKREF is
> fixed so no point in putting it in DT and exposing it outside of this
> driver.
I understand your rationale, but the *.dtsi is typically used to describe the
SoC, things that cannot change. For example, consider
arch/arm/boot/dts/nuvoton-npcm750.dtsi, it describes the CPU cores, cache
hierarchy, peripherals and what busses they reside on, memory layout, etc. None
of these things are configurable; the *.dtsi just describes the SoC. Typically,
a *.dts is made for a particular board and imports a *.dtsi and does some
configuration, but it is not supposed to override things like what I listed
above (amoung other things).
There are several reasons that you put this information that cannot change in a
*.dtsi. For one it is a convenient, all in one place description of the SoC that
serves as a form of documentation. But more importantly, if you were to create a
variation of the 750 with a different number of cores, a different memory
layout, a different reference clock, etc, you could potentiallly use the same
drivers for both SoCs, and you would only have to write a new *.dtsi. I know you
may have no intention to do this, but many other CPU vendors do. And for this
reason, this is how drivers are expected to be written.
I hope this makes sense.
>
>> + */
>> + pr_debug("\tclk register fixed clocks\n");
>> + hw = clk_hw_register_fixed_rate(NULL, NPCM7XX_CLK_S_REFCLK,
>> + NULL, 0, 25000000);
>> + hw = clk_hw_register_fixed_rate(NULL, NPCM7XX_CLK_S_SYSBYPCK,
>> + NULL, 0, 800000000); // rarely used. mostly testing. TBD: remove
>> + hw = clk_hw_register_fixed_rate(NULL, NPCM7XX_CLK_S_MCBYPCK,
>> + NULL, 0, 800000000); // rarely used. mostly testing. TBD:remove
>> +
>> +
<snip>
--
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
