Re: [PATCH V3 3/6] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver
From: Yu Tu
Date: Tue Aug 30 2022 - 03:37:39 EST
On 2022/8/30 14:44, Jerome Brunet wrote:
[ EXTERNAL EMAIL ]
On Tue 30 Aug 2022 at 14:13, Yu Tu <yu.tu@xxxxxxxxxxx> wrote:
On 2022/8/29 17:48, Jerome Brunet wrote:
[ EXTERNAL EMAIL ]
On Mon 15 Aug 2022 at 21:20, Yu Tu <yu.tu@xxxxxxxxxxx> 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?