On Fri, Sep 02, 2016 at 05:15:31PM +0200, Sylwester Nawrocki wrote: > On 08/30/2016 12:36 PM, Krzysztof Kozlowski wrote: > > On 08/22/2016 06:31 PM, Sylwester Nawrocki wrote: > >> This patch adds code instantiating the EPLL, which is used as the > >> audio subsystem's root clock. > >> The requirement to specify the external root clock in clocks property > >> is also added. > > > > I think the requirement was there already but explained little bit > > differently: > > 19 External clock: > > 20 > > 21 There is clock that is generated outside the SoC. It > > 22 is expected that it is defined using standard clock bindings > > 23 with following clock-output-name: > > 24 > > 25 - "fin_pll" - PLL input clock from XXTI > > > > so you can just combine them. Driver now will require the fin_pll in two > > ways: > > 1. Old lookup by "fin_pll" name. > > 2. of_clk_get of yours. > > I'm not sure what do you mean exactly, I'm also adding in this patch a sentence > right after the text as quoted above saying that: > > "This clock should be listed in the clocks property of the controller node." > > The description of the consumer clock (the main clock controller's parent clock) > was not in the binding so far, there is no "clocks" property in the list of > required properties. First of all, in commit message, you are not adding the requirement... it was present already, I think. Wasn't it? As for the code, I am saying that you are duplicating the information. There is already an paragraph for external clock. You are adding a new one before and extending it... just make it simpler - mention external clock in one place. > > >> That ensures proper initialization order by exlicitly > >> specifying dependencies in devicetree. It prevents situations when the > >> SoC's clock controller driver has initialized, the external oscillator > >> clock is not yet registered and setting clock frequencies through > >> assigned-clock-rates property doesn't work properly due to unknown > >> external oscillator frequency. > >> > >> Signed-off-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx> > >> --- > >> .../devicetree/bindings/clock/exynos5410-clock.txt | 14 +++ > >> drivers/clk/samsung/clk-exynos5410.c | 30 +++++- > >> drivers/clk/samsung/clk-pll.c | 102 +++++++++++++++++++++ > >> drivers/clk/samsung/clk-pll.h | 1 + > >> 4 files changed, 145 insertions(+), 2 deletions(-) > >> > >> diff --git a/Documentation/devicetree/bindings/clock/exynos5410-clock.txt b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt > >> index aeab635..2aefc07 100644 > >> --- a/Documentation/devicetree/bindings/clock/exynos5410-clock.txt > >> +++ b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt > >> @@ -12,6 +12,9 @@ Required Properties: > >> > >> - #clock-cells: should be 1. > >> > >> +- clocks: should contain an entry specifying the root oscillator clock > >> + on XXTI pin (fin_pll). > > > > Combine with "external clock". > > Do you mean rephrasing this to something like: > > - clocks: should contain an entry specifying the external clock (fin_pll), > i.e. the root clock on XXTI pin. Could be, with removal of the existing paragraph. The only comment I have is about duplicating the information. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |