On Friday 02 February 2018 12:52 AM, David Lechner wrote: >> +static const char * const da850_pll1_obsclk_parent_names[] >> __initconst = { >> + "oscin", > > Re: the issue of "ref_clk" vs. "oscin"... > > This is one of the places where having the otherwise unnecessary "oscin" > clock > really helps out. The PLL driver doesn't control "ref_clk" - it comes > from somewhere > else. And in the case of DT, it may not even be named "ref_clk", so we > really > don't want to hard-code the name "ref_clk" here. TBH, I don't really see what is wrong with mandating the name "ref_clk" as the reference clock name to be provided. And for all board-files and DTs to supply the same name. > If we have to allow a variable name here, it just makes more work in the > driver > shuffling names around. > > And the name "oscin" totally makes sense here because the TRM lists this > input to the > mux as "OSCIN". Fine with me if you feel it simplifies implementation for you (and also because of the distinction you want to make between the external "before CLKMODE" clock and internal "after CLKMODE" clock). What I do care about though is: a) In the DT case, ability for different boards to provide different ref_clk frequencies. We never really had this in the legacy board file way (except some rudimentary support on DM6467T). And its fine to continue with status quo for board files. b) In the DT case, ability for board to specify whether it uses the on-chip oscillator or has an external clean clock provider. >> +void __init da850_pll_clk_init(void __iomem *pll0, void __iomem *pll1) >> +{ >> + const struct davinci_pll_sysclk_info *info; >> + >> + davinci_pll_clk_register(&da850_pll0_info, "ref_clk", pll0); > > And really, we probably shouldn't be hard-coding "ref_clk" here either. > Basically, we are making the assumption that the board file has registered > a clock named "ref_clk". It would probably be better to pass the name > as a parameter. As I noted before, I am not sure if this level of naming flexibility is needed. Every board needs to have one anyway. They might as well call it by the same name. That said, I wont oppose it either if you decide to have that flexibility. Thanks, Sekhar -- 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