| Message ID | 20220427094823.3319-4-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add RZ/G2L Display clock support | expand |
Hi Biju, On Wed, Apr 27, 2022 at 11:48 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > M3 clock is sourced from DSI Divider (DSIDIVA * DSIDIVB) > > This patch add support for DSI divider clk by combining > DSIDIVA and DSIDIVB . > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > V1->V2: > * Replaced round_rate with determine_rate > * Update rate variable during set_rate > * Added get_vclk_parent_rate helper function > * Replaced pll5_params with mux_dsi_div_params for dsi div values. Thanks for the update! > --- a/drivers/clk/renesas/rzg2l-cpg.c > +++ b/drivers/clk/renesas/rzg2l-cpg.c > +static unsigned long rzg2l_cpg_get_vclk_parent_rate(struct clk_hw *hw, > + unsigned long rate) > +{ > + unsigned long parent_rate; Please drop the blank line. > + > + struct dsi_div_hw_data *dsi_div = to_dsi_div_hw_data(hw); > + struct rzg2l_cpg_priv *priv = dsi_div->priv; > + struct rzg2l_pll5_param params; Reverse Xmas tree order? > + > + parent_rate = rzg2l_cpg_get_foutpostdiv_rate(¶ms, rate); > + > + if (priv->mux_dsi_div_params.clksrc) > + parent_rate /= 2; > + > + return parent_rate; > +} > + > +static int rzg2l_cpg_dsi_div_determine_rate(struct clk_hw *hw, > + struct clk_rate_request *req) > +{ > + req->best_parent_rate = rzg2l_cpg_get_vclk_parent_rate(hw, req->rate); > + > + return 0; So any value of req->rate passed is supported, and req->rate never needs to be updated? > +} > + > +static int rzg2l_cpg_dsi_div_set_rate(struct clk_hw *hw, > + unsigned long rate, > + unsigned long parent_rate) > +{ > + struct dsi_div_hw_data *dsi_div = to_dsi_div_hw_data(hw); > + struct rzg2l_cpg_priv *priv = dsi_div->priv; > + > + /* > + * MUX -->DIV_DSI_{A,B} -->M3 -->VCLK > + * > + * Based on the dot clock, the DSI divider clock sets the divider value, > + * calculates the pll parameters for generating FOUTPOSTDIV and the clk > + * source for the MUX and propagates that info to the parents. > + */ > + > + if (!rate) > + return -EINVAL; > + > + dsi_div->rate = rate; So any non-zero value of rate is supported? > + writel(CPG_PL5_SDIV_DIV_DSI_A_WEN | CPG_PL5_SDIV_DIV_DSI_B_WEN | > + (priv->mux_dsi_div_params.dsi_div_a << 0) | > + (priv->mux_dsi_div_params.dsi_div_b << 8), > + priv->base + CPG_PL5_SDIV); > + > + return 0; > +} Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH v2 3/9] clk: renesas: rzg2l: Add DSI divider clk > support > > Hi Biju, > > On Wed, Apr 27, 2022 at 11:48 AM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > M3 clock is sourced from DSI Divider (DSIDIVA * DSIDIVB) > > > > This patch add support for DSI divider clk by combining DSIDIVA and > > DSIDIVB . > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > V1->V2: > > * Replaced round_rate with determine_rate > > * Update rate variable during set_rate > > * Added get_vclk_parent_rate helper function > > * Replaced pll5_params with mux_dsi_div_params for dsi div values. > > Thanks for the update! > > > --- a/drivers/clk/renesas/rzg2l-cpg.c > > +++ b/drivers/clk/renesas/rzg2l-cpg.c > > > +static unsigned long rzg2l_cpg_get_vclk_parent_rate(struct clk_hw *hw, > > + unsigned long > > +rate) { > > + unsigned long parent_rate; > > Please drop the blank line. OK. > > > + > > + struct dsi_div_hw_data *dsi_div = to_dsi_div_hw_data(hw); > > + struct rzg2l_cpg_priv *priv = dsi_div->priv; > > + struct rzg2l_pll5_param params; > > Reverse Xmas tree order? OK. > > > + > > + parent_rate = rzg2l_cpg_get_foutpostdiv_rate(¶ms, rate); > > + > > + if (priv->mux_dsi_div_params.clksrc) > > + parent_rate /= 2; > > + > > + return parent_rate; > > +} > > + > > +static int rzg2l_cpg_dsi_div_determine_rate(struct clk_hw *hw, > > + struct clk_rate_request > > +*req) { > > + req->best_parent_rate = rzg2l_cpg_get_vclk_parent_rate(hw, > > +req->rate); > > + > > + return 0; > > So any value of req->rate passed is supported, and req->rate never needs > to be updated? Yes, We can add a check for freq greater than 148.5MHz and if it is greater, Assign req->rate to 148.5MHz. > > > +} > > + > > +static int rzg2l_cpg_dsi_div_set_rate(struct clk_hw *hw, > > + unsigned long rate, > > + unsigned long parent_rate) { > > + struct dsi_div_hw_data *dsi_div = to_dsi_div_hw_data(hw); > > + struct rzg2l_cpg_priv *priv = dsi_div->priv; > > + > > + /* > > + * MUX -->DIV_DSI_{A,B} -->M3 -->VCLK > > + * > > + * Based on the dot clock, the DSI divider clock sets the > divider value, > > + * calculates the pll parameters for generating FOUTPOSTDIV and > the clk > > + * source for the MUX and propagates that info to the parents. > > + */ > > + > > + if (!rate) > > + return -EINVAL; > > + > > + dsi_div->rate = rate; > > So any non-zero value of rate is supported? Currently yes, But If we add a check for > 148.5Mhz in determine_rate, it will allow only upto 148.5Mhz. Regards, Biju > > > + writel(CPG_PL5_SDIV_DIV_DSI_A_WEN | CPG_PL5_SDIV_DIV_DSI_B_WEN | > > + (priv->mux_dsi_div_params.dsi_div_a << 0) | > > + (priv->mux_dsi_div_params.dsi_div_b << 8), > > + priv->base + CPG_PL5_SDIV); > > + > > + return 0; > > +} > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux- > m68k.org > > In personal conversations with technical people, I call myself a hacker. > But when I'm talking to journalists I just say "programmer" or something > like that. > -- Linus Torvalds
Hi Biju, On Fri, Apr 29, 2022 at 11:50 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > Subject: Re: [PATCH v2 3/9] clk: renesas: rzg2l: Add DSI divider clk > > support > > On Wed, Apr 27, 2022 at 11:48 AM Biju Das <biju.das.jz@bp.renesas.com> > > wrote: > > > M3 clock is sourced from DSI Divider (DSIDIVA * DSIDIVB) > > > > > > This patch add support for DSI divider clk by combining DSIDIVA and > > > DSIDIVB . > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > + > > > + parent_rate = rzg2l_cpg_get_foutpostdiv_rate(¶ms, rate); > > > + > > > + if (priv->mux_dsi_div_params.clksrc) > > > + parent_rate /= 2; > > > + > > > + return parent_rate; > > > +} > > > + > > > +static int rzg2l_cpg_dsi_div_determine_rate(struct clk_hw *hw, > > > + struct clk_rate_request > > > +*req) { > > > + req->best_parent_rate = rzg2l_cpg_get_vclk_parent_rate(hw, > > > +req->rate); > > > + > > > + return 0; > > > > So any value of req->rate passed is supported, and req->rate never needs > > to be updated? > > Yes, We can add a check for freq greater than 148.5MHz and if it is greater, > Assign req->rate to 148.5MHz. > > > > > > +} > > > + > > > +static int rzg2l_cpg_dsi_div_set_rate(struct clk_hw *hw, > > > + unsigned long rate, > > > + unsigned long parent_rate) { > > > + struct dsi_div_hw_data *dsi_div = to_dsi_div_hw_data(hw); > > > + struct rzg2l_cpg_priv *priv = dsi_div->priv; > > > + > > > + /* > > > + * MUX -->DIV_DSI_{A,B} -->M3 -->VCLK > > > + * > > > + * Based on the dot clock, the DSI divider clock sets the > > divider value, > > > + * calculates the pll parameters for generating FOUTPOSTDIV and > > the clk > > > + * source for the MUX and propagates that info to the parents. > > > + */ > > > + > > > + if (!rate) > > > + return -EINVAL; > > > + > > > + dsi_div->rate = rate; > > > > So any non-zero value of rate is supported? > > Currently yes, But If we add a check for > 148.5Mhz in determine_rate, it will allow only upto 148.5Mhz. You still need that check here, too, as there is no guarantee users will call .set_rate() with the rounded parameters obtained from .determine_rate(). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH v2 3/9] clk: renesas: rzg2l: Add DSI divider clk > support > > Hi Biju, > > On Fri, Apr 29, 2022 at 11:50 AM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > > Subject: Re: [PATCH v2 3/9] clk: renesas: rzg2l: Add DSI divider clk > > > support > > > > On Wed, Apr 27, 2022 at 11:48 AM Biju Das > > > <biju.das.jz@bp.renesas.com> > > > wrote: > > > > M3 clock is sourced from DSI Divider (DSIDIVA * DSIDIVB) > > > > > > > > This patch add support for DSI divider clk by combining DSIDIVA > > > > and DSIDIVB . > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > + > > > > + parent_rate = rzg2l_cpg_get_foutpostdiv_rate(¶ms, > > > > + rate); > > > > + > > > > + if (priv->mux_dsi_div_params.clksrc) > > > > + parent_rate /= 2; > > > > + > > > > + return parent_rate; > > > > +} > > > > + > > > > +static int rzg2l_cpg_dsi_div_determine_rate(struct clk_hw *hw, > > > > + struct > > > > +clk_rate_request > > > > +*req) { > > > > + req->best_parent_rate = rzg2l_cpg_get_vclk_parent_rate(hw, > > > > +req->rate); > > > > + > > > > + return 0; > > > > > > So any value of req->rate passed is supported, and req->rate never > > > needs to be updated? > > > > Yes, We can add a check for freq greater than 148.5MHz and if it is > > greater, Assign req->rate to 148.5MHz. > > > > > > > > > +} > > > > + > > > > +static int rzg2l_cpg_dsi_div_set_rate(struct clk_hw *hw, > > > > + unsigned long rate, > > > > + unsigned long parent_rate) { > > > > + struct dsi_div_hw_data *dsi_div = to_dsi_div_hw_data(hw); > > > > + struct rzg2l_cpg_priv *priv = dsi_div->priv; > > > > + > > > > + /* > > > > + * MUX -->DIV_DSI_{A,B} -->M3 -->VCLK > > > > + * > > > > + * Based on the dot clock, the DSI divider clock sets the > > > divider value, > > > > + * calculates the pll parameters for generating > > > > + FOUTPOSTDIV and > > > the clk > > > > + * source for the MUX and propagates that info to the > parents. > > > > + */ > > > > + > > > > + if (!rate) > > > > + return -EINVAL; > > > > + > > > > + dsi_div->rate = rate; > > > > > > So any non-zero value of rate is supported? > > > > Currently yes, But If we add a check for > 148.5Mhz in determine_rate, > it will allow only upto 148.5Mhz. > > You still need that check here, too, as there is no guarantee users will > call .set_rate() with the rounded parameters obtained from > .determine_rate(). OK, will add a check and return -EINVAL for freq > 148.5 here. Cheers, Biju
diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c index 0120f239c94d..e0485d206d71 100644 --- a/drivers/clk/renesas/rzg2l-cpg.c +++ b/drivers/clk/renesas/rzg2l-cpg.c @@ -304,6 +304,124 @@ rzg2l_cpg_get_foutpostdiv_rate(struct rzg2l_pll5_param *params, return foutpostdiv_rate; } +struct dsi_div_hw_data { + struct clk_hw hw; + u32 conf; + unsigned long rate; + struct rzg2l_cpg_priv *priv; +}; + +#define to_dsi_div_hw_data(_hw) container_of(_hw, struct dsi_div_hw_data, hw) + +static unsigned long rzg2l_cpg_dsi_div_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct dsi_div_hw_data *dsi_div = to_dsi_div_hw_data(hw); + unsigned long rate = dsi_div->rate; + + if (!rate) + rate = parent_rate; + + return rate; +} + +static unsigned long rzg2l_cpg_get_vclk_parent_rate(struct clk_hw *hw, + unsigned long rate) +{ + unsigned long parent_rate; + + struct dsi_div_hw_data *dsi_div = to_dsi_div_hw_data(hw); + struct rzg2l_cpg_priv *priv = dsi_div->priv; + struct rzg2l_pll5_param params; + + parent_rate = rzg2l_cpg_get_foutpostdiv_rate(¶ms, rate); + + if (priv->mux_dsi_div_params.clksrc) + parent_rate /= 2; + + return parent_rate; +} + +static int rzg2l_cpg_dsi_div_determine_rate(struct clk_hw *hw, + struct clk_rate_request *req) +{ + req->best_parent_rate = rzg2l_cpg_get_vclk_parent_rate(hw, req->rate); + + return 0; +} + +static int rzg2l_cpg_dsi_div_set_rate(struct clk_hw *hw, + unsigned long rate, + unsigned long parent_rate) +{ + struct dsi_div_hw_data *dsi_div = to_dsi_div_hw_data(hw); + struct rzg2l_cpg_priv *priv = dsi_div->priv; + + /* + * MUX -->DIV_DSI_{A,B} -->M3 -->VCLK + * + * Based on the dot clock, the DSI divider clock sets the divider value, + * calculates the pll parameters for generating FOUTPOSTDIV and the clk + * source for the MUX and propagates that info to the parents. + */ + + if (!rate) + return -EINVAL; + + dsi_div->rate = rate; + writel(CPG_PL5_SDIV_DIV_DSI_A_WEN | CPG_PL5_SDIV_DIV_DSI_B_WEN | + (priv->mux_dsi_div_params.dsi_div_a << 0) | + (priv->mux_dsi_div_params.dsi_div_b << 8), + priv->base + CPG_PL5_SDIV); + + return 0; +} + +static const struct clk_ops rzg2l_cpg_dsi_div_ops = { + .recalc_rate = rzg2l_cpg_dsi_div_recalc_rate, + .determine_rate = rzg2l_cpg_dsi_div_determine_rate, + .set_rate = rzg2l_cpg_dsi_div_set_rate, +}; + +static struct clk * __init +rzg2l_cpg_dsi_div_clk_register(const struct cpg_core_clk *core, + struct clk **clks, + struct rzg2l_cpg_priv *priv) +{ + struct dsi_div_hw_data *clk_hw_data; + const struct clk *parent; + const char *parent_name; + struct clk_init_data init; + struct clk_hw *clk_hw; + int ret; + + parent = clks[core->parent & 0xffff]; + if (IS_ERR(parent)) + return ERR_CAST(parent); + + clk_hw_data = devm_kzalloc(priv->dev, sizeof(*clk_hw_data), GFP_KERNEL); + if (!clk_hw_data) + return ERR_PTR(-ENOMEM); + + clk_hw_data->priv = priv; + + parent_name = __clk_get_name(parent); + init.name = core->name; + init.ops = &rzg2l_cpg_dsi_div_ops; + init.flags = CLK_SET_RATE_PARENT; + init.parent_names = &parent_name; + init.num_parents = 1; + + clk_hw = &clk_hw_data->hw; + clk_hw->init = &init; + + ret = devm_clk_hw_register(priv->dev, clk_hw); + if (ret) + return ERR_PTR(ret); + + return clk_hw->clk; +} + struct pll5_mux_hw_data { struct clk_hw hw; u32 conf; @@ -733,6 +851,9 @@ rzg2l_cpg_register_core_clk(const struct cpg_core_clk *core, case CLK_TYPE_PLL5_4_MUX: clk = rzg2l_cpg_pll5_4_mux_clk_register(core, priv); break; + case CLK_TYPE_DSI_DIV: + clk = rzg2l_cpg_dsi_div_clk_register(core, priv->clks, priv); + break; default: goto fail; } diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h index 2503251cb18f..1be29cec0cb2 100644 --- a/drivers/clk/renesas/rzg2l-cpg.h +++ b/drivers/clk/renesas/rzg2l-cpg.h @@ -24,6 +24,7 @@ #define CPG_PL3_SSEL (0x408) #define CPG_PL6_SSEL (0x414) #define CPG_PL6_ETH_SSEL (0x418) +#define CPG_PL5_SDIV (0x420) #define CPG_OTHERFUNC1_REG (0xBE8) #define CPG_SIPLL5_STBY_RESETB BIT(0) @@ -38,6 +39,9 @@ #define CPG_OTHERFUNC1_REG_RES0_ON_WEN BIT(16) +#define CPG_PL5_SDIV_DIV_DSI_A_WEN BIT(16) +#define CPG_PL5_SDIV_DIV_DSI_B_WEN BIT(24) + #define CPG_CLKSTATUS_SELSDHI0_STS BIT(28) #define CPG_CLKSTATUS_SELSDHI1_STS BIT(29) @@ -53,6 +57,7 @@ (((offset) << 20) | ((bitpos) << 12) | ((size) << 8)) #define DIVPL1A DDIV_PACK(CPG_PL1_DDIV, 0, 2) #define DIVPL2A DDIV_PACK(CPG_PL2_DDIV, 0, 3) +#define DIVDSILPCLK DDIV_PACK(CPG_PL2_DDIV, 12, 2) #define DIVPL3A DDIV_PACK(CPG_PL3A_DDIV, 0, 3) #define DIVPL3B DDIV_PACK(CPG_PL3A_DDIV, 4, 3) #define DIVPL3C DDIV_PACK(CPG_PL3A_DDIV, 8, 3) @@ -114,6 +119,10 @@ enum clk_types { /* Clock for PLL5_4 clock source selector */ CLK_TYPE_PLL5_4_MUX, + + /* Clock for DSI divider */ + CLK_TYPE_DSI_DIV, + }; #define DEF_TYPE(_name, _id, _type...) \ @@ -142,6 +151,8 @@ enum clk_types { #define DEF_PLL5_4_MUX(_name, _id, _conf, _parent_names, _num_parents) \ DEF_TYPE(_name, _id, CLK_TYPE_PLL5_4_MUX, .conf = _conf, \ .parent_names = _parent_names, .num_parents = _num_parents) +#define DEF_DSI_DIV(_name, _id, _parent, _flag) \ + DEF_TYPE(_name, _id, CLK_TYPE_DSI_DIV, .parent = _parent, .flag = _flag) /** * struct rzg2l_mod_clk - Module Clocks definitions
M3 clock is sourced from DSI Divider (DSIDIVA * DSIDIVB) This patch add support for DSI divider clk by combining DSIDIVA and DSIDIVB . Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- V1->V2: * Replaced round_rate with determine_rate * Update rate variable during set_rate * Added get_vclk_parent_rate helper function * Replaced pll5_params with mux_dsi_div_params for dsi div values. RFC->V1 * Removed LUT and added an equation for computing VCLK --- drivers/clk/renesas/rzg2l-cpg.c | 121 ++++++++++++++++++++++++++++++++ drivers/clk/renesas/rzg2l-cpg.h | 11 +++ 2 files changed, 132 insertions(+)