Hi Claudiu,
On Fri, Sep 29, 2023 at 7:39 AM Claudiu <claudiu.beznea@xxxxxxxxx> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>
> Refactor SD MUX driver to be able to reuse the same code on RZ/G3S.
> RZ/G2{L, UL} has a limitation with regards to switching the clock source
> for SD MUX (MUX clock source has to be switched to 266MHz before switching
> b/w 533MHz and 400MHz). This limitation has been introduced as a clock
> notifier that is registered on platform based initialization data thus the
> SD MUX code could be reused on RZ/G3S.
>
> As both RZ/G2{L, UL} and RZ/G3S has specific bits in specific registers
> to check if the clock switching has been done, this configuration (register
> offset, register bits and bits width) is now passed though
> struct cpg_core_clk::sconf (status configuration) from platform specific
> initialization code.
>
> Along with struct cpg_core_clk::sconf the mux table indices are also
> passed from platform specific initialization code.
>
> Also, mux flags are now passed to DEF_SD_MUX() as they will be later
> used by RZ/G3S.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> ---
>
> Changes in v2:
> - s/indexes/indices in commit description
> - mentioned in commit description that mux flags can now be passed to
> driver though DEF_SD_MUX() macro
> - removed SoC specific names from macros' names
> - added spaces after { and before } when initializing arrays
> - preserved the order of .[gs]set_parent() API definitions for simpler
> diff b/w versions
> - removed SD_MUX_NOTIF macro
Thanks for the update!
> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> @@ -142,6 +146,77 @@ static void rzg2l_cpg_del_clk_provider(void *data)
> of_clk_del_provider(data);
> }
>
> +/* Must be called in atomic context. */
> +static int rzg2l_cpg_wait_clk_update_done(void __iomem *base, u32 conf)
> +{
> + u32 bitmask = GENMASK(GET_WIDTH(conf) - 1, 0) << GET_SHIFT(conf);
> + u32 off = GET_REG_OFFSET(conf);
> + u32 val;
> +
> + return readl_poll_timeout_atomic(base + off, val, !(val & bitmask), 10, 200);
> +}
> +
> +int rzg2l_cpg_sd_mux_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 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);
> + const u32 clk_src_266 = 3;
> + unsigned long flags;
> + u32 bitmask;
> + int ret;
> +
> + if (event != PRE_RATE_CHANGE || (cnd->new_rate / MEGA == 266))
> + return 0;
include/linux/clk.h:
* PRE_RATE_CHANGE - called immediately before the clk rate is changed,
* to indicate that the rate change will proceed. Drivers must
* immediately terminate any operations that will be affected by the
* rate change. Callbacks may either return NOTIFY_DONE, NOTIFY_OK,
* NOTIFY_STOP or NOTIFY_BAD.
> +
> + spin_lock_irqsave(&priv->rmw_lock, flags);
> +
> + /*
> + * As per the HW manual, we should not directly switch from 533 MHz to
> + * 400 MHz and vice versa. To change the setting from 2’b01 (533 MHz)
> + * to 2’b10 (400 MHz) or vice versa, Switch to 2’b11 (266 MHz) first,
> + * and then switch to the target setting (2’b01 (533 MHz) or 2’b10
> + * (400 MHz)).
> + * Setting a value of '0' to the SEL_SDHI0_SET or SEL_SDHI1_SET clock
> + * switching register is prohibited.
> + * The clock mux has 3 input clocks(533 MHz, 400 MHz, and 266 MHz), and
> + * the index to value mapping is done by adding 1 to the index.
> + */
> + bitmask = (GENMASK(GET_WIDTH(clk_hw_data->conf) - 1, 0) << shift) << 16;
> + writel(bitmask | (clk_src_266 << 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 switch to safe clk source\n");
> +
> + return ret;
Likewise.
> +}
>
> static const struct clk_ops rzg2l_cpg_sd_clk_mux_ops = {
> .determine_rate = __clk_mux_determine_rate_closest,
> - .set_parent = rzg2l_cpg_sd_clk_mux_set_parent,
> - .get_parent = rzg2l_cpg_sd_clk_mux_get_parent,
> + .set_parent = rzg2l_cpg_sd_mux_clk_set_parent,
> + .get_parent = rzg2l_cpg_sd_mux_clk_get_parent,
Please keep the old names, for consistency with
__clk_mux_determine_rate_closest() and drivers/clk/clk-mux.c, and to
reduce the diff.
Any existing inconsistent use of "clk_mux" vs. "mux_clk" can be resolved
later with a separate patch, if anyone cares.
> --- a/drivers/clk/renesas/rzg2l-cpg.h
> +++ b/drivers/clk/renesas/rzg2l-cpg.h
> @@ -272,4 +276,6 @@ extern const struct rzg2l_cpg_info r9a07g044_cpg_info;
> 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);
rzg2l_cpg_sd_clk_mux_notifier()?
> +
> #endif
The rest LGTM.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
