Hi Biju,
On Wed, Aug 4, 2021 at 8:08 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> Add SDHI clk mux support to select SDHI clock from different clock
> sources.
>
> As per HW manual, direct clock switching from 533MHz to 400MHz and
> vice versa is not recommended. So added support for handling this
> in mux.
>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
Thanks for your patch!
> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> @@ -55,6 +55,15 @@
> #define GET_REG_SAMPLL_CLK1(val) ((val >> 22) & 0xfff)
> #define GET_REG_SAMPLL_CLK2(val) ((val >> 12) & 0xfff)
>
> +struct sd_hw_data {
> + struct clk_hw hw;
> + u32 conf;
> + u32 mux_flags;
Do you need mux_flags? Or can this be hardcoded to zero?
> + struct rzg2l_cpg_priv *priv;
> +};
> +
> +#define to_sd_hw_data(_hw) container_of(_hw, struct sd_hw_data, hw)
> +
> /**
> * struct rzg2l_cpg_priv - Clock Pulse Generator Private Data
> *
> @@ -150,6 +159,100 @@ rzg2l_cpg_mux_clk_register(const struct cpg_core_clk *core,
> return clk_hw->clk;
> }
>
> +static int rzg2l_cpg_sd_clk_mux_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> + struct sd_hw_data *hwdata = to_sd_hw_data(hw);
> +
> + return clk_mux_determine_rate_flags(hw, req, hwdata->mux_flags);
> +}
> +
> +static int rzg2l_cpg_sd_clk_mux_set_parent(struct clk_hw *hw, u8 index)
> +{
> + struct sd_hw_data *hwdata = to_sd_hw_data(hw);
> + struct rzg2l_cpg_priv *priv = hwdata->priv;
> + u32 off = GET_REG_OFFSET(hwdata->conf);
> + u32 shift = GET_SHIFT(hwdata->conf);
> + const u32 clk_src_266 = 2;
> + u32 bitmask;
> +
> + /*
> + * 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(hwdata->conf) - 1, 0) << shift) << 16;
> + if (index != clk_src_266)
> + writel(bitmask | ((clk_src_266 + 1) << shift), priv->base + off);
I'm wondering if you should poll (using readl_poll_timeout()) until
the CPG_CLKSTATUS.SELSDHIx_STS bit is cleared, to indicate switching
has completed?
> +
> + writel(bitmask | ((index + 1) << shift), priv->base + off);
> +
> + return 0;
> +}
> +
> +static u8 rzg2l_cpg_sd_clk_mux_get_parent(struct clk_hw *hw)
> +{
> + struct sd_hw_data *hwdata = to_sd_hw_data(hw);
> + struct rzg2l_cpg_priv *priv = hwdata->priv;
> + u32 val = readl(priv->base + GET_REG_OFFSET(hwdata->conf));
> +
> + val >>= GET_SHIFT(hwdata->conf);
> + val &= GENMASK(GET_WIDTH(hwdata->conf) - 1, 0);
> + if (val)
> + val--;
> + else
> + /* Prohibited clk source, change it to 533 MHz(reset value) */
> + rzg2l_cpg_sd_clk_mux_set_parent(hw, 0);
Please add curly braces (to both branches).
> +
> + return val;
> +}
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
