| Message ID | 20230929053915.1530607-12-claudiu.beznea@bp.renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add new Renesas RZ/G3S SoC and RZ/G3S SMARC EVK | expand |
Hi Claudiu, On Fri, Sep 29, 2023 at 7:39 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Add a divider clock driver for RZ/G3S. This will be used in RZ/G3S > by SDHI, SPI, OCTA, I, I2, I3, P0, P1, P2, P3 core clocks. > The divider has some limitation for SDHI and OCTA clocks: > - SD div cannot be 1 if parent rate is 800MHz > - OCTA div cannot be 1 if parent rate is 400MHz > For these clocks a notifier could be registered from platform specific > clock driver and proper actions are taken before clock rate is changed, > if needed. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > --- > > Changes in v2: > - removed DIV_NOTIF macro Thanks for the update! > --- a/drivers/clk/renesas/rzg2l-cpg.c > +++ b/drivers/clk/renesas/rzg2l-cpg.c > @@ -91,6 +91,22 @@ struct sd_mux_hw_data { > > #define to_sd_mux_hw_data(_hw) container_of(_hw, struct sd_mux_hw_data, hw_data) > > +/** > + * struct div_hw_data - divider clock hardware data > + * @hw_data: clock hw data > + * @dtable: pointer to divider table > + * @invalid_rate: invalid rate for divider > + * @width: divider width > + */ > +struct div_hw_data { > + struct clk_hw_data hw_data; > + const struct clk_div_table *dtable; > + unsigned long invalid_rate; > + u32 width; > +}; > + > +#define to_div_hw_data(_hw) container_of(_hw, struct div_hw_data, hw_data) > + > struct rzg2l_pll5_param { > u32 pl5_fracin; > u8 pl5_refdiv; > @@ -200,6 +216,54 @@ int rzg2l_cpg_sd_mux_clk_notifier(struct notifier_block *nb, unsigned long event > return ret; > } > > +int rzg3s_cpg_div_clk_notifier(struct notifier_block *nb, unsigned long event, > + void *data) > +{ > + struct clk_notifier_data *cnd = data; > + struct clk_hw *hw = __clk_get_hw(cnd->clk); > + struct clk_hw_data *clk_hw_data = to_clk_hw_data(hw); > + struct div_hw_data *div_hw_data = to_div_hw_data(clk_hw_data); > + struct rzg2l_cpg_priv *priv = clk_hw_data->priv; > + u32 off = GET_REG_OFFSET(clk_hw_data->conf); > + u32 shift = GET_SHIFT(clk_hw_data->conf); > + u32 bitmask = GENMASK(GET_WIDTH(clk_hw_data->conf) - 1, 0); > + unsigned long flags; > + int ret = 0; > + u32 val; > + > + if (event != PRE_RATE_CHANGE || !div_hw_data->invalid_rate || > + div_hw_data->invalid_rate % cnd->new_rate) > + return 0; NOTIFY_DONE for event != PRE_RATE_CHANGE NOTIFY_OK for the other cases > + > + spin_lock_irqsave(&priv->rmw_lock, flags); > + > + val = readl(priv->base + off); > + val >>= shift; > + val &= bitmask; > + > + /* > + * There are different constraints for the user of this notifiers as follows: > + * 1/ SD div cannot be 1 (val == 0) if parent rate is 800MHz > + * 2/ OCTA div cannot be 1 (val == 0) if parent rate is 400MHz > + * As SD can have only one parent having 800MHz and OCTA div can have > + * only one parent having 400MHz we took into account the parent rate > + * at the beginning of function (by checking invalid_rate % new_rate). > + * Now it is time to check the hardware divider and update it accordingly. > + */ > + if (!val) { > + writel(((bitmask << shift) << 16) | BIT(shift), priv->base + off); Haven't you exchanged the (single) write-enable bit and the (multi-bit) division ratio setting? According to the docs, the write-enable bit is at 16 + shift, while the division ratio is at shift. Also, using bitmask as the division ratio means the maximum value that fits in the bitfield, which would be a prohibited setting in case of DIV_OCTA. Now, looking at rzg3s_div_clk_set_rate() below, perhaps you just wanted to set the ratio to value to 1, but used the wrong size for bitmask? > + /* Wait for the update done. */ > + ret = rzg2l_cpg_wait_clk_update_done(priv->base, clk_hw_data->sconf); > + } > + > + spin_unlock_irqrestore(&priv->rmw_lock, flags); > + > + if (ret) > + dev_err(priv->dev, "Failed to downgrade the div\n"); and return NOTIFY_BAD > + > + return ret; NOTIFY_OK > +} > + > static int rzg2l_register_notifier(struct clk_hw *hw, const struct cpg_core_clk *core, > struct rzg2l_cpg_priv *priv) > { > @@ -217,6 +281,146 @@ static int rzg2l_register_notifier(struct clk_hw *hw, const struct cpg_core_clk > return clk_notifier_register(hw->clk, nb); > } > > +static unsigned long rzg3s_div_clk_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + struct clk_hw_data *clk_hw_data = to_clk_hw_data(hw); > + struct div_hw_data *div_hw_data = to_div_hw_data(clk_hw_data); > + struct rzg2l_cpg_priv *priv = clk_hw_data->priv; > + u32 val; > + > + val = readl(priv->base + GET_REG_OFFSET(clk_hw_data->conf)); > + val >>= GET_SHIFT(clk_hw_data->conf); > + val &= GENMASK(GET_WIDTH(clk_hw_data->conf) - 1, 0); > + > + return divider_recalc_rate(hw, parent_rate, val, div_hw_data->dtable, > + CLK_DIVIDER_ROUND_CLOSEST, div_hw_data->width); > +} > + > +static bool rzg3s_div_clk_is_rate_valid(const unsigned long invalid_rate, unsigned long rate) > +{ > + if (invalid_rate && rate >= invalid_rate) > + return false; > + > + return true; > +} > + > +static long rzg3s_div_clk_round_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate) > +{ > + struct clk_hw_data *clk_hw_data = to_clk_hw_data(hw); > + struct div_hw_data *div_hw_data = to_div_hw_data(clk_hw_data); > + long round_rate; > + > + round_rate = divider_round_rate(hw, rate, parent_rate, div_hw_data->dtable, > + div_hw_data->width, CLK_DIVIDER_ROUND_CLOSEST); > + > + if (!rzg3s_div_clk_is_rate_valid(div_hw_data->invalid_rate, round_rate)) > + return -EINVAL; Shouldn't this return the closest rate that is actually supported instead? > + > + return round_rate; > +} But please implement .determine_rate() instead of .round_rate() in new drivers. > + > +static int rzg3s_div_clk_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long parent_rate) > +{ > + struct clk_hw_data *clk_hw_data = to_clk_hw_data(hw); > + struct div_hw_data *div_hw_data = to_div_hw_data(clk_hw_data); > + struct rzg2l_cpg_priv *priv = clk_hw_data->priv; > + u32 off = GET_REG_OFFSET(clk_hw_data->conf); > + u32 shift = GET_SHIFT(clk_hw_data->conf); > + unsigned long flags; > + u32 bitmask, val; > + int ret; > + > + /* > + * Some dividers cannot support some rates: > + * - SD div cannot support 800 MHz when parent is @800MHz and div = 1 > + * - OCTA div cannot support 400 MHz when parent is @400MHz and div = 1 > + * Check these scenarios. > + */ > + if (!rzg3s_div_clk_is_rate_valid(div_hw_data->invalid_rate, rate)) > + return -EINVAL; Can this actually happen? Wouldn't the notifier have prevented us from getting here? > + > + val = divider_get_val(rate, parent_rate, div_hw_data->dtable, div_hw_data->width, > + CLK_DIVIDER_ROUND_CLOSEST); > + > + bitmask = (GENMASK(GET_WIDTH(clk_hw_data->conf) - 1, 0) << shift) << 16; Is bitmask the (single) write-enable bit? If yes, that should be BIT(16 + shift), and the variable should be renamed to reflect that. I guess there should be a general "#define CPG_WEN BIT(16)", then you can simply use writel((CPG_WEN | val) << shift, ...); > + > + spin_lock_irqsave(&priv->rmw_lock, flags); > + writel(bitmask | (val << shift), priv->base + off); > + /* Wait for the update done. */ > + ret = rzg2l_cpg_wait_clk_update_done(priv->base, clk_hw_data->sconf); > + spin_unlock_irqrestore(&priv->rmw_lock, flags); > + > + return ret; > +} > + > +static const struct clk_ops rzg3s_div_clk_ops = { > + .recalc_rate = rzg3s_div_clk_recalc_rate, > + .round_rate = rzg3s_div_clk_round_rate, > + .set_rate = rzg3s_div_clk_set_rate, > +}; > + > +static struct clk * __init > +rzg3s_cpg_div_clk_register(const struct cpg_core_clk *core, struct clk **clks, > + void __iomem *base, struct rzg2l_cpg_priv *priv) > +{ > + struct div_hw_data *div_hw_data; > + struct clk_init_data init = {}; > + const struct clk_div_table *clkt; > + struct clk_hw *clk_hw; > + const struct clk *parent; > + const char *parent_name; > + u32 max; > + int ret; > + > + parent = clks[core->parent & 0xffff]; > + if (IS_ERR(parent)) > + return ERR_CAST(parent); > + > + parent_name = __clk_get_name(parent); > + > + div_hw_data = devm_kzalloc(priv->dev, sizeof(*div_hw_data), GFP_KERNEL); > + if (!div_hw_data) > + return ERR_PTR(-ENOMEM); > + > + init.name = core->name; > + init.flags = core->flag; > + init.ops = &rzg3s_div_clk_ops; > + init.parent_names = &parent_name; > + init.num_parents = 1; > + > + /* Get the maximum divider to retrieve div width. */ > + for (clkt = core->dtable; clkt->div; clkt++) { > + if (max < clkt->div) "max" is used uninitialized > + max = clkt->div; > + } > + > + div_hw_data->hw_data.priv = priv; > + div_hw_data->hw_data.conf = core->conf; > + div_hw_data->hw_data.sconf = core->sconf; > + div_hw_data->dtable = core->dtable; > + div_hw_data->invalid_rate = core->invalid_rate; > + div_hw_data->width = fls(max) - 1; Isn't that > + > + clk_hw = &div_hw_data->hw_data.hw; > + clk_hw->init = &init; > + > + ret = devm_clk_hw_register(priv->dev, clk_hw); > + if (ret) > + return ERR_PTR(ret); > + > + ret = rzg2l_register_notifier(clk_hw, core, priv); > + if (ret) { > + dev_err(priv->dev, "Failed to register notifier for %s\n", > + core->name); > + return ERR_PTR(ret); > + } > + > + return clk_hw->clk; > +} Gr{oetje,eeting}s, Geert
Hi, Geert, On 04.10.2023 15:30, Geert Uytterhoeven wrote: > Hi Claudiu, > > On Fri, Sep 29, 2023 at 7:39 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> >> Add a divider clock driver for RZ/G3S. This will be used in RZ/G3S >> by SDHI, SPI, OCTA, I, I2, I3, P0, P1, P2, P3 core clocks. >> The divider has some limitation for SDHI and OCTA clocks: >> - SD div cannot be 1 if parent rate is 800MHz >> - OCTA div cannot be 1 if parent rate is 400MHz >> For these clocks a notifier could be registered from platform specific >> clock driver and proper actions are taken before clock rate is changed, >> if needed. >> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> --- >> >> Changes in v2: >> - removed DIV_NOTIF macro > > Thanks for the update! > >> --- a/drivers/clk/renesas/rzg2l-cpg.c >> +++ b/drivers/clk/renesas/rzg2l-cpg.c >> @@ -91,6 +91,22 @@ struct sd_mux_hw_data { >> >> #define to_sd_mux_hw_data(_hw) container_of(_hw, struct sd_mux_hw_data, hw_data) >> >> +/** >> + * struct div_hw_data - divider clock hardware data >> + * @hw_data: clock hw data >> + * @dtable: pointer to divider table >> + * @invalid_rate: invalid rate for divider >> + * @width: divider width >> + */ >> +struct div_hw_data { >> + struct clk_hw_data hw_data; >> + const struct clk_div_table *dtable; >> + unsigned long invalid_rate; >> + u32 width; >> +}; >> + >> +#define to_div_hw_data(_hw) container_of(_hw, struct div_hw_data, hw_data) >> + >> struct rzg2l_pll5_param { >> u32 pl5_fracin; >> u8 pl5_refdiv; >> @@ -200,6 +216,54 @@ int rzg2l_cpg_sd_mux_clk_notifier(struct notifier_block *nb, unsigned long event >> return ret; >> } >> >> +int rzg3s_cpg_div_clk_notifier(struct notifier_block *nb, unsigned long event, >> + void *data) >> +{ >> + struct clk_notifier_data *cnd = data; >> + struct clk_hw *hw = __clk_get_hw(cnd->clk); >> + struct clk_hw_data *clk_hw_data = to_clk_hw_data(hw); >> + struct div_hw_data *div_hw_data = to_div_hw_data(clk_hw_data); >> + struct rzg2l_cpg_priv *priv = clk_hw_data->priv; >> + u32 off = GET_REG_OFFSET(clk_hw_data->conf); >> + u32 shift = GET_SHIFT(clk_hw_data->conf); >> + u32 bitmask = GENMASK(GET_WIDTH(clk_hw_data->conf) - 1, 0); >> + unsigned long flags; >> + int ret = 0; >> + u32 val; >> + >> + if (event != PRE_RATE_CHANGE || !div_hw_data->invalid_rate || >> + div_hw_data->invalid_rate % cnd->new_rate) >> + return 0; > > NOTIFY_DONE for event != PRE_RATE_CHANGE > NOTIFY_OK for the other cases Sure! > >> + >> + spin_lock_irqsave(&priv->rmw_lock, flags); >> + >> + val = readl(priv->base + off); >> + val >>= shift; >> + val &= bitmask; >> + >> + /* >> + * There are different constraints for the user of this notifiers as follows: >> + * 1/ SD div cannot be 1 (val == 0) if parent rate is 800MHz >> + * 2/ OCTA div cannot be 1 (val == 0) if parent rate is 400MHz >> + * As SD can have only one parent having 800MHz and OCTA div can have >> + * only one parent having 400MHz we took into account the parent rate >> + * at the beginning of function (by checking invalid_rate % new_rate). >> + * Now it is time to check the hardware divider and update it accordingly. >> + */ >> + if (!val) { >> + writel(((bitmask << shift) << 16) | BIT(shift), priv->base + off); > > Haven't you exchanged the (single) write-enable bit and the (multi-bit) > division ratio setting? According to the docs, the write-enable bit > is at 16 + shift, while the division ratio is at shift. Indeed, I messed this up. Though, I've tested quite some use cases and they all worked... I'll review this anyway, thanks for pointing it up. > > Also, using bitmask as the division ratio means the maximum value > that fits in the bitfield, which would be a prohibited setting in case > of DIV_OCTA. > > Now, looking at rzg3s_div_clk_set_rate() below, perhaps you just wanted > to set the ratio to value to 1, but used the wrong size for bitmask? Yes, the idea was to set a safe divider. > >> + /* Wait for the update done. */ >> + ret = rzg2l_cpg_wait_clk_update_done(priv->base, clk_hw_data->sconf); >> + } >> + >> + spin_unlock_irqrestore(&priv->rmw_lock, flags); >> + >> + if (ret) >> + dev_err(priv->dev, "Failed to downgrade the div\n"); > > and return NOTIFY_BAD Sure! > >> + >> + return ret; > > NOTIFY_OK Sure! > >> +} >> + >> static int rzg2l_register_notifier(struct clk_hw *hw, const struct cpg_core_clk *core, >> struct rzg2l_cpg_priv *priv) >> { >> @@ -217,6 +281,146 @@ static int rzg2l_register_notifier(struct clk_hw *hw, const struct cpg_core_clk >> return clk_notifier_register(hw->clk, nb); >> } >> >> +static unsigned long rzg3s_div_clk_recalc_rate(struct clk_hw *hw, >> + unsigned long parent_rate) >> +{ >> + struct clk_hw_data *clk_hw_data = to_clk_hw_data(hw); >> + struct div_hw_data *div_hw_data = to_div_hw_data(clk_hw_data); >> + struct rzg2l_cpg_priv *priv = clk_hw_data->priv; >> + u32 val; >> + >> + val = readl(priv->base + GET_REG_OFFSET(clk_hw_data->conf)); >> + val >>= GET_SHIFT(clk_hw_data->conf); >> + val &= GENMASK(GET_WIDTH(clk_hw_data->conf) - 1, 0); >> + >> + return divider_recalc_rate(hw, parent_rate, val, div_hw_data->dtable, >> + CLK_DIVIDER_ROUND_CLOSEST, div_hw_data->width); >> +} >> + >> +static bool rzg3s_div_clk_is_rate_valid(const unsigned long invalid_rate, unsigned long rate) >> +{ >> + if (invalid_rate && rate >= invalid_rate) >> + return false; >> + >> + return true; >> +} >> + >> +static long rzg3s_div_clk_round_rate(struct clk_hw *hw, unsigned long rate, >> + unsigned long *parent_rate) >> +{ >> + struct clk_hw_data *clk_hw_data = to_clk_hw_data(hw); >> + struct div_hw_data *div_hw_data = to_div_hw_data(clk_hw_data); >> + long round_rate; >> + >> + round_rate = divider_round_rate(hw, rate, parent_rate, div_hw_data->dtable, >> + div_hw_data->width, CLK_DIVIDER_ROUND_CLOSEST); >> + >> + if (!rzg3s_div_clk_is_rate_valid(div_hw_data->invalid_rate, round_rate)) >> + return -EINVAL; > > Shouldn't this return the closest rate that is actually supported instead? The divider_round_rate() already choose it as the closest rate that it is actually not supported, thus I chose to just return -EINVAL. I chose it this way to use divider_round_rate(). Don't know if there is way around this using divider_round_rate() I'll have a look. > >> + >> + return round_rate; >> +} > > But please implement .determine_rate() instead of .round_rate() in > new drivers. Indeed, I missed this one. > >> + >> +static int rzg3s_div_clk_set_rate(struct clk_hw *hw, unsigned long rate, >> + unsigned long parent_rate) >> +{ >> + struct clk_hw_data *clk_hw_data = to_clk_hw_data(hw); >> + struct div_hw_data *div_hw_data = to_div_hw_data(clk_hw_data); >> + struct rzg2l_cpg_priv *priv = clk_hw_data->priv; >> + u32 off = GET_REG_OFFSET(clk_hw_data->conf); >> + u32 shift = GET_SHIFT(clk_hw_data->conf); >> + unsigned long flags; >> + u32 bitmask, val; >> + int ret; >> + >> + /* >> + * Some dividers cannot support some rates: >> + * - SD div cannot support 800 MHz when parent is @800MHz and div = 1 >> + * - OCTA div cannot support 400 MHz when parent is @400MHz and div = 1 >> + * Check these scenarios. >> + */ >> + if (!rzg3s_div_clk_is_rate_valid(div_hw_data->invalid_rate, rate)) >> + return -EINVAL; > > Can this actually happen? Wouldn't the notifier have prevented us from > getting here? I remember I added it here as a result of testing. I'll double check it. > >> + >> + val = divider_get_val(rate, parent_rate, div_hw_data->dtable, div_hw_data->width, >> + CLK_DIVIDER_ROUND_CLOSEST); >> + >> + bitmask = (GENMASK(GET_WIDTH(clk_hw_data->conf) - 1, 0) << shift) << 16; > > Is bitmask the (single) write-enable bit? > > If yes, that should be BIT(16 + shift), and the variable should be > renamed to reflect that. > > I guess there should be a general "#define CPG_WEN BIT(16)", then you > can simply use > > writel((CPG_WEN | val) << shift, ...); OK. > >> + >> + spin_lock_irqsave(&priv->rmw_lock, flags); >> + writel(bitmask | (val << shift), priv->base + off); >> + /* Wait for the update done. */ >> + ret = rzg2l_cpg_wait_clk_update_done(priv->base, clk_hw_data->sconf); >> + spin_unlock_irqrestore(&priv->rmw_lock, flags); >> + >> + return ret; >> +} >> + >> +static const struct clk_ops rzg3s_div_clk_ops = { >> + .recalc_rate = rzg3s_div_clk_recalc_rate, >> + .round_rate = rzg3s_div_clk_round_rate, >> + .set_rate = rzg3s_div_clk_set_rate, >> +}; >> + >> +static struct clk * __init >> +rzg3s_cpg_div_clk_register(const struct cpg_core_clk *core, struct clk **clks, >> + void __iomem *base, struct rzg2l_cpg_priv *priv) >> +{ >> + struct div_hw_data *div_hw_data; >> + struct clk_init_data init = {}; >> + const struct clk_div_table *clkt; >> + struct clk_hw *clk_hw; >> + const struct clk *parent; >> + const char *parent_name; >> + u32 max; >> + int ret; >> + >> + parent = clks[core->parent & 0xffff]; >> + if (IS_ERR(parent)) >> + return ERR_CAST(parent); >> + >> + parent_name = __clk_get_name(parent); >> + >> + div_hw_data = devm_kzalloc(priv->dev, sizeof(*div_hw_data), GFP_KERNEL); >> + if (!div_hw_data) >> + return ERR_PTR(-ENOMEM); >> + >> + init.name = core->name; >> + init.flags = core->flag; >> + init.ops = &rzg3s_div_clk_ops; >> + init.parent_names = &parent_name; >> + init.num_parents = 1; >> + >> + /* Get the maximum divider to retrieve div width. */ >> + for (clkt = core->dtable; clkt->div; clkt++) { >> + if (max < clkt->div) > > "max" is used uninitialized Yes, you're right. Thank you for your review, Claudiu Beznea > >> + max = clkt->div; >> + } >> + >> + div_hw_data->hw_data.priv = priv; >> + div_hw_data->hw_data.conf = core->conf; >> + div_hw_data->hw_data.sconf = core->sconf; >> + div_hw_data->dtable = core->dtable; >> + div_hw_data->invalid_rate = core->invalid_rate; >> + div_hw_data->width = fls(max) - 1; > > Isn't that >> + >> + clk_hw = &div_hw_data->hw_data.hw; >> + clk_hw->init = &init; >> + >> + ret = devm_clk_hw_register(priv->dev, clk_hw); >> + if (ret) >> + return ERR_PTR(ret); >> + >> + ret = rzg2l_register_notifier(clk_hw, core, priv); >> + if (ret) { >> + dev_err(priv->dev, "Failed to register notifier for %s\n", >> + core->name); >> + return ERR_PTR(ret); >> + } >> + >> + return clk_hw->clk; >> +} > > Gr{oetje,eeting}s, > > Geert >
Hi Claudiu, On Wed, Oct 4, 2023 at 2:30 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Fri, Sep 29, 2023 at 7:39 AM Claudiu <claudiu.beznea@tuxon.dev> wrote: > > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > > > Add a divider clock driver for RZ/G3S. This will be used in RZ/G3S > > by SDHI, SPI, OCTA, I, I2, I3, P0, P1, P2, P3 core clocks. > > The divider has some limitation for SDHI and OCTA clocks: > > - SD div cannot be 1 if parent rate is 800MHz > > - OCTA div cannot be 1 if parent rate is 400MHz > > For these clocks a notifier could be registered from platform specific > > clock driver and proper actions are taken before clock rate is changed, > > if needed. > > > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > --- > > > > Changes in v2: > > - removed DIV_NOTIF macro > > --- a/drivers/clk/renesas/rzg2l-cpg.c > > +++ b/drivers/clk/renesas/rzg2l-cpg.c > > +static struct clk * __init > > +rzg3s_cpg_div_clk_register(const struct cpg_core_clk *core, struct clk **clks, > > + void __iomem *base, struct rzg2l_cpg_priv *priv) > > +{ > > + struct div_hw_data *div_hw_data; > > + struct clk_init_data init = {}; > > + const struct clk_div_table *clkt; > > + struct clk_hw *clk_hw; > > + const struct clk *parent; > > + const char *parent_name; > > + u32 max; > > + int ret; > > + > > + parent = clks[core->parent & 0xffff]; > > + if (IS_ERR(parent)) > > + return ERR_CAST(parent); > > + > > + parent_name = __clk_get_name(parent); > > + > > + div_hw_data = devm_kzalloc(priv->dev, sizeof(*div_hw_data), GFP_KERNEL); > > + if (!div_hw_data) > > + return ERR_PTR(-ENOMEM); > > + > > + init.name = core->name; > > + init.flags = core->flag; > > + init.ops = &rzg3s_div_clk_ops; > > + init.parent_names = &parent_name; > > + init.num_parents = 1; > > + > > + /* Get the maximum divider to retrieve div width. */ > > + for (clkt = core->dtable; clkt->div; clkt++) { > > + if (max < clkt->div) > > "max" is used uninitialized > > > + max = clkt->div; > > + } > > + > > + div_hw_data->hw_data.priv = priv; > > + div_hw_data->hw_data.conf = core->conf; > > + div_hw_data->hw_data.sconf = core->sconf; > > + div_hw_data->dtable = core->dtable; > > + div_hw_data->invalid_rate = core->invalid_rate; > > + div_hw_data->width = fls(max) - 1; > > Isn't that My apologies for not finishing my sentence; I wanted to write "Isn't that identical to __fls(max)?". But as the latter generates slightly worse code, it's not worth making that change. Gr{oetje,eeting}s, Geert
diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c index f4b70e07b9c6..3080e9391f71 100644 --- a/drivers/clk/renesas/rzg2l-cpg.c +++ b/drivers/clk/renesas/rzg2l-cpg.c @@ -91,6 +91,22 @@ struct sd_mux_hw_data { #define to_sd_mux_hw_data(_hw) container_of(_hw, struct sd_mux_hw_data, hw_data) +/** + * struct div_hw_data - divider clock hardware data + * @hw_data: clock hw data + * @dtable: pointer to divider table + * @invalid_rate: invalid rate for divider + * @width: divider width + */ +struct div_hw_data { + struct clk_hw_data hw_data; + const struct clk_div_table *dtable; + unsigned long invalid_rate; + u32 width; +}; + +#define to_div_hw_data(_hw) container_of(_hw, struct div_hw_data, hw_data) + struct rzg2l_pll5_param { u32 pl5_fracin; u8 pl5_refdiv; @@ -200,6 +216,54 @@ int rzg2l_cpg_sd_mux_clk_notifier(struct notifier_block *nb, unsigned long event return ret; } +int rzg3s_cpg_div_clk_notifier(struct notifier_block *nb, unsigned long event, + void *data) +{ + struct clk_notifier_data *cnd = data; + struct clk_hw *hw = __clk_get_hw(cnd->clk); + struct clk_hw_data *clk_hw_data = to_clk_hw_data(hw); + struct div_hw_data *div_hw_data = to_div_hw_data(clk_hw_data); + struct rzg2l_cpg_priv *priv = clk_hw_data->priv; + u32 off = GET_REG_OFFSET(clk_hw_data->conf); + u32 shift = GET_SHIFT(clk_hw_data->conf); + u32 bitmask = GENMASK(GET_WIDTH(clk_hw_data->conf) - 1, 0); + unsigned long flags; + int ret = 0; + u32 val; + + if (event != PRE_RATE_CHANGE || !div_hw_data->invalid_rate || + div_hw_data->invalid_rate % cnd->new_rate) + return 0; + + spin_lock_irqsave(&priv->rmw_lock, flags); + + val = readl(priv->base + off); + val >>= shift; + val &= bitmask; + + /* + * There are different constraints for the user of this notifiers as follows: + * 1/ SD div cannot be 1 (val == 0) if parent rate is 800MHz + * 2/ OCTA div cannot be 1 (val == 0) if parent rate is 400MHz + * As SD can have only one parent having 800MHz and OCTA div can have + * only one parent having 400MHz we took into account the parent rate + * at the beginning of function (by checking invalid_rate % new_rate). + * Now it is time to check the hardware divider and update it accordingly. + */ + if (!val) { + writel(((bitmask << shift) << 16) | BIT(shift), priv->base + off); + /* Wait for the update done. */ + ret = rzg2l_cpg_wait_clk_update_done(priv->base, clk_hw_data->sconf); + } + + spin_unlock_irqrestore(&priv->rmw_lock, flags); + + if (ret) + dev_err(priv->dev, "Failed to downgrade the div\n"); + + return ret; +} + static int rzg2l_register_notifier(struct clk_hw *hw, const struct cpg_core_clk *core, struct rzg2l_cpg_priv *priv) { @@ -217,6 +281,146 @@ static int rzg2l_register_notifier(struct clk_hw *hw, const struct cpg_core_clk return clk_notifier_register(hw->clk, nb); } +static unsigned long rzg3s_div_clk_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct clk_hw_data *clk_hw_data = to_clk_hw_data(hw); + struct div_hw_data *div_hw_data = to_div_hw_data(clk_hw_data); + struct rzg2l_cpg_priv *priv = clk_hw_data->priv; + u32 val; + + val = readl(priv->base + GET_REG_OFFSET(clk_hw_data->conf)); + val >>= GET_SHIFT(clk_hw_data->conf); + val &= GENMASK(GET_WIDTH(clk_hw_data->conf) - 1, 0); + + return divider_recalc_rate(hw, parent_rate, val, div_hw_data->dtable, + CLK_DIVIDER_ROUND_CLOSEST, div_hw_data->width); +} + +static bool rzg3s_div_clk_is_rate_valid(const unsigned long invalid_rate, unsigned long rate) +{ + if (invalid_rate && rate >= invalid_rate) + return false; + + return true; +} + +static long rzg3s_div_clk_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + struct clk_hw_data *clk_hw_data = to_clk_hw_data(hw); + struct div_hw_data *div_hw_data = to_div_hw_data(clk_hw_data); + long round_rate; + + round_rate = divider_round_rate(hw, rate, parent_rate, div_hw_data->dtable, + div_hw_data->width, CLK_DIVIDER_ROUND_CLOSEST); + + if (!rzg3s_div_clk_is_rate_valid(div_hw_data->invalid_rate, round_rate)) + return -EINVAL; + + return round_rate; +} + +static int rzg3s_div_clk_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long parent_rate) +{ + struct clk_hw_data *clk_hw_data = to_clk_hw_data(hw); + struct div_hw_data *div_hw_data = to_div_hw_data(clk_hw_data); + struct rzg2l_cpg_priv *priv = clk_hw_data->priv; + u32 off = GET_REG_OFFSET(clk_hw_data->conf); + u32 shift = GET_SHIFT(clk_hw_data->conf); + unsigned long flags; + u32 bitmask, val; + int ret; + + /* + * Some dividers cannot support some rates: + * - SD div cannot support 800 MHz when parent is @800MHz and div = 1 + * - OCTA div cannot support 400 MHz when parent is @400MHz and div = 1 + * Check these scenarios. + */ + if (!rzg3s_div_clk_is_rate_valid(div_hw_data->invalid_rate, rate)) + return -EINVAL; + + val = divider_get_val(rate, parent_rate, div_hw_data->dtable, div_hw_data->width, + CLK_DIVIDER_ROUND_CLOSEST); + + bitmask = (GENMASK(GET_WIDTH(clk_hw_data->conf) - 1, 0) << shift) << 16; + + spin_lock_irqsave(&priv->rmw_lock, flags); + writel(bitmask | (val << shift), priv->base + off); + /* Wait for the update done. */ + ret = rzg2l_cpg_wait_clk_update_done(priv->base, clk_hw_data->sconf); + spin_unlock_irqrestore(&priv->rmw_lock, flags); + + return ret; +} + +static const struct clk_ops rzg3s_div_clk_ops = { + .recalc_rate = rzg3s_div_clk_recalc_rate, + .round_rate = rzg3s_div_clk_round_rate, + .set_rate = rzg3s_div_clk_set_rate, +}; + +static struct clk * __init +rzg3s_cpg_div_clk_register(const struct cpg_core_clk *core, struct clk **clks, + void __iomem *base, struct rzg2l_cpg_priv *priv) +{ + struct div_hw_data *div_hw_data; + struct clk_init_data init = {}; + const struct clk_div_table *clkt; + struct clk_hw *clk_hw; + const struct clk *parent; + const char *parent_name; + u32 max; + int ret; + + parent = clks[core->parent & 0xffff]; + if (IS_ERR(parent)) + return ERR_CAST(parent); + + parent_name = __clk_get_name(parent); + + div_hw_data = devm_kzalloc(priv->dev, sizeof(*div_hw_data), GFP_KERNEL); + if (!div_hw_data) + return ERR_PTR(-ENOMEM); + + init.name = core->name; + init.flags = core->flag; + init.ops = &rzg3s_div_clk_ops; + init.parent_names = &parent_name; + init.num_parents = 1; + + /* Get the maximum divider to retrieve div width. */ + for (clkt = core->dtable; clkt->div; clkt++) { + if (max < clkt->div) + max = clkt->div; + } + + div_hw_data->hw_data.priv = priv; + div_hw_data->hw_data.conf = core->conf; + div_hw_data->hw_data.sconf = core->sconf; + div_hw_data->dtable = core->dtable; + div_hw_data->invalid_rate = core->invalid_rate; + div_hw_data->width = fls(max) - 1; + + clk_hw = &div_hw_data->hw_data.hw; + clk_hw->init = &init; + + ret = devm_clk_hw_register(priv->dev, clk_hw); + if (ret) + return ERR_PTR(ret); + + ret = rzg2l_register_notifier(clk_hw, core, priv); + if (ret) { + dev_err(priv->dev, "Failed to register notifier for %s\n", + core->name); + return ERR_PTR(ret); + } + + return clk_hw->clk; +} + static struct clk * __init rzg2l_cpg_div_clk_register(const struct cpg_core_clk *core, struct clk **clks, @@ -964,6 +1168,9 @@ rzg2l_cpg_register_core_clk(const struct cpg_core_clk *core, clk = rzg2l_cpg_div_clk_register(core, priv->clks, priv->base, priv); break; + case CLK_TYPE_G3S_DIV: + clk = rzg3s_cpg_div_clk_register(core, priv->clks, priv->base, priv); + break; case CLK_TYPE_MUX: clk = rzg2l_cpg_mux_clk_register(core, priv->base, priv); break; diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h index edac34037cf0..8312972cc496 100644 --- a/drivers/clk/renesas/rzg2l-cpg.h +++ b/drivers/clk/renesas/rzg2l-cpg.h @@ -89,6 +89,7 @@ struct cpg_core_clk { unsigned int sconf; const struct clk_div_table *dtable; const u32 *mtable; + const unsigned long invalid_rate; const char * const *parent_names; notifier_fn_t notifier; u32 flag; @@ -105,6 +106,7 @@ enum clk_types { /* Clock with divider */ CLK_TYPE_DIV, + CLK_TYPE_G3S_DIV, /* Clock with clock source selector */ CLK_TYPE_MUX, @@ -143,6 +145,12 @@ enum clk_types { DEF_TYPE(_name, _id, CLK_TYPE_DIV, .conf = _conf, \ .parent = _parent, .dtable = _dtable, \ .flag = CLK_DIVIDER_READ_ONLY) +#define DEF_G3S_DIV(_name, _id, _parent, _conf, _sconf, _dtable, _invalid_rate, \ + _clk_flags, _notif) \ + DEF_TYPE(_name, _id, CLK_TYPE_G3S_DIV, .conf = _conf, .sconf = _sconf, \ + .parent = _parent, .dtable = _dtable, \ + .invalid_rate = _invalid_rate, .flag = (_clk_flags), \ + .notifier = _notif) #define DEF_MUX(_name, _id, _conf, _parent_names) \ DEF_TYPE(_name, _id, CLK_TYPE_MUX, .conf = _conf, \ .parent_names = _parent_names, \ @@ -277,5 +285,6 @@ extern const struct rzg2l_cpg_info r9a07g054_cpg_info; extern const struct rzg2l_cpg_info r9a09g011_cpg_info; int rzg2l_cpg_sd_mux_clk_notifier(struct notifier_block *nb, unsigned long event, void *data); +int rzg3s_cpg_div_clk_notifier(struct notifier_block *nb, unsigned long event, void *data); #endif