On 03/20/2015 06:15 AM, Hyungwon Hwang wrote: > Dear Andrej, > > On Thu, 19 Mar 2015 10:32:10 +0100 > Andrzej Hajda <a.hajda@xxxxxxxxxxx> wrote: > >> On 03/19/2015 02:18 AM, Hyungwon Hwang wrote: >>> Dear Daniel, >>> >>> On Thu, 19 Mar 2015 01:13:21 +0000 >>> Daniel Stone <daniel-rLtY4a/8tF1rovVCs/uTlw@xxxxxxxxxxxxxxxx> wrote: >>> >>>> Hi Hyungwon, >>>> >>>> On 19 March 2015 at 01:02, Hyungwon Hwang >>>> <human.hwang-Sze3O3UU22JBDgjK7y7TUQ@xxxxxxxxxxxxxxxx> wrote: >>>>>>> + /* >>>>>>> + * The input PLL clock for MIPI DSI in Exynos5433 seems >>>>>>> to be fixed >>>>>>> + * by OSC CLK. >>>>>>> + */ >>>>>>> + fin = 24 * MHZ; >>>>>> Er, is this always true on other platforms as well? Shouldn't >>>>>> this be a part of the DeviceTree description? >>>>> I forgot to change the comment in development. Finally it is found >>>>> that all exynos mipi dsi's fin is OSC clk which is 24 MHz. So I >>>>> will remove the comment, but remain the code as it is. >>>> Fair enough. Should pll_clk be removed from the DT description >>>> then, if it's fixed to the oscillator? >>> Yes. It is redundant to represent pll_clk in DT, and it should be >>> removed. >> Why do you think OSC clk determines value of pll_clk? >> pll_clk is mapped to SCLK_MIPI[01] or SCLK_DSIM0 gate with few >> dividers and muxes above. So at least in theory it can differ from >> osc clk. Additionally this gate should be enabled so you cannot just >> remove it from DT. >> >> Regards >> Andrzej > As I found, pll clk is not SCLK_MIPI[01] but OSC CLK. SCLK_DSIM0 must > be controlled in this driver as it has been, as a gate clock of MIPI > DSI block, but not as a pll clk. SCLK_DSIM0 is not the input > clock of MIPI DPHY which provides fin in this code. So clock setting > and getting code was wrong, and must be removed. > > OSC CLK is not soc-depedendant but board-dependant, even though I have > not seen any board which does not use OSC CLK by 24 MHz. It must be > parsed from board DT file, which in this case, we can use the value in > pll_clk_rate (the variable name must be renamed also). > > Because ambiguous description in the technical document, I can be > wrong. Please let me know if I do not understand something. Thanks for > your comment. After some digging I agree that documentation is quite confusing and current code could be wrong. Anyway I wonder if it wouldn't be better to explicitly provide input clock for DSIM, or more precisely for its PLL instead of hardcoding 24MHz into the driver. Another thing that bothers me is relation of DPHY_PLL in clock controller to MIPI_DPHY in Exynos7420. There are two clocks used by MIPI_DPHY: - "Ref Clock" pinned to SCLK_MIPIDPHY_M4 connected to OSCCLK, - "PHY Clock" pinned to I_FOUT_DPHY_PLL connected to DPHY_PLL, The first clock seems to be your osc clock, but what is the role of the 2nd one? Regards Andrzej > Best regards, > Hyungwon Hwang > >>>>> Thanks for your review. I will send it again with the changes you >>>>> suggested. >>>> Thanks very much! >>>> >>>> Cheers, >>>> Daniel >>> Best regards, >>> Hyungwon Hwang >>> -- >>> To unsubscribe from this list: send the line "unsubscribe >>> devicetree" in the body of a message to >>> majordomo-u79uwXL29TY76Z2rM5mHXA@xxxxxxxxxxxxxxxx More majordomo >>> info at http://vger.kernel.org/majordomo-info.html >>> -- 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
