Messages in this thread | | | Date | Wed, 21 Sep 2022 16:40:34 +0800 | Subject | Re: [PATCH V3 3/6] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver | From | Yu Tu <> |
| |
Hi Jerome,
On 2022/8/30 15:37, Yu Tu wrote: > > > On 2022/8/30 14:44, Jerome Brunet wrote: >> [ EXTERNAL EMAIL ] >> >> >> On Tue 30 Aug 2022 at 14:13, Yu Tu <yu.tu@amlogic.com> wrote: >> >>> On 2022/8/29 17:48, Jerome Brunet wrote: >>>> [ EXTERNAL EMAIL ] >>>> On Mon 15 Aug 2022 at 21:20, Yu Tu <yu.tu@amlogic.com> wrote: >>>> >>>>>>>> + >>>>>>>> +static struct clk_regmap s4_hdmi_pll_dco = { >>>>>>>> + .data = &(struct meson_clk_pll_data){ >>>>>>>> + .en = { >>>>>>>> + .reg_off = ANACTRL_HDMIPLL_CTRL0, >>>>>>>> + .shift = 28, >>>>>>>> + .width = 1, >>>>>>>> + }, >>>>>>>> + .m = { >>>>>>>> + .reg_off = ANACTRL_HDMIPLL_CTRL0, >>>>>>>> + .shift = 0, >>>>>>>> + .width = 8, >>>>>>>> + }, >>>>>>>> + .n = { >>>>>>>> + .reg_off = ANACTRL_HDMIPLL_CTRL0, >>>>>>>> + .shift = 10, >>>>>>>> + .width = 5, >>>>>>>> + }, >>>>>>>> + .frac = { >>>>>>>> + .reg_off = ANACTRL_HDMIPLL_CTRL1, >>>>>>>> + .shift = 0, >>>>>>>> + .width = 17, >>>>>>>> + }, >>>>>>>> + .l = { >>>>>>>> + .reg_off = ANACTRL_HDMIPLL_CTRL0, >>>>>>>> + .shift = 31, >>>>>>>> + .width = 1, >>>>>>>> + }, >>>>>>>> + .rst = { >>>>>>>> + .reg_off = ANACTRL_HDMIPLL_CTRL0, >>>>>>>> + .shift = 29, >>>>>>>> + .width = 1, >>>>>>>> + }, >>>>>>>> + }, >>>>>>>> + .hw.init = &(struct clk_init_data){ >>>>>>>> + .name = "hdmi_pll_dco", >>>>>>>> + .ops = &meson_clk_pll_ro_ops, >>>>>>>> + .parent_data = (const struct clk_parent_data []) { >>>>>>>> + { .fw_name = "xtal", } >>>>>>>> + }, >>>>>>>> + .num_parents = 1, >>>>>>>> + /* >>>>>>>> + * Display directly handle hdmi pll registers ATM, we need >>>>>>>> + * NOCACHE to keep our view of the clock as accurate as >>>>>>>> + * possible >>>>>>>> + */ >>>>>>> >>>>>>> Is it really ? >>>>>>> >>>>>>> Given that HDMI support for the s4 is there yet, the >>>>>>> addresses have changes and the region is no longer a syscon, it >>>>>>> is time >>>>>>> for the HDMI driver to get fixed. >>>>> The HDMI PLL is configured in the Uboot phase and does not change the >>>>> frequency in the kernel phase. So we use the NOCACHE flag and >>>>> "ro_ops". >>>> That's no reason to put NOCACHE or ro-ops >>>> If you want the frequencies to be statically assinged, the correct way >>>> would be through assigned-rate in DT I guess. >>> >>> Okay. You're right. However, when registering with OPS, HDMI PLL will be >>> reset. It takes time for PLL to stabilize the output frequency, which >>> will >>> lead to the startup screen flashing. >>> >>> I would like to know how to solve this problem if not using ro_ops. >>> >>>> >> >> You can add new ops or tweak the current init function. > > HDMI PLL is not different from other PLLS, so I think adding OPS is weird. > >> >> Safest would be to do the following : >> * Check if the PLLs is already on. >> * Check if the 'pll->init_regs' matches what is already set >> - if so, you can skip the reset >> - if not, you need to reset as usual > > static int meson_clk_pll_init(struct clk_hw *hw) > { > struct clk_regmap *clk = to_clk_regmap(hw); > struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); > > > if (pll->init_count) { > meson_parm_write(clk->map, &pll->rst, 1); > regmap_multi_reg_write(clk->map, pll->init_regs, > | pll->init_count); > meson_parm_write(clk->map, &pll->rst, 0); > } > > > return 0; > } > > Because the init function looks like this. Therefore, HDMI PLL > init_count is not given. Can I change it like this?
I don't know if this change meets your requirements? Please give us your valuable advice.
| |