diff mbox series

[1/2] drivers: clk: renesas: rzg2l-cpg: Add SDHI clk mux support

Message ID 20210804180803.29087-2-biju.das.jz@bp.renesas.com (mailing list archive)
State Awaiting Upstream, archived
Headers show
Series Add SDHI clock and reset entries in cpg driver | expand

Commit Message

Biju Das Aug. 4, 2021, 6:08 p.m. UTC 
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@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
This patch sereies depend upon [1]
[1] https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=522063
---
 drivers/clk/renesas/rzg2l-cpg.c | 106 ++++++++++++++++++++++++++++++++
 drivers/clk/renesas/rzg2l-cpg.h |   8 +++
 2 files changed, 114 insertions(+)

Comments

Geert Uytterhoeven Sept. 6, 2021, 11:44 a.m. UTC | #1 
Hi Biju,

On Wed, Aug 4, 2021 at 8:08 PM Biju Das <biju.das.jz@bp.renesas.com> 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@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

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
Biju Das Sept. 20, 2021, 10:15 a.m. UTC | #2 
Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH 1/2] drivers: clk: renesas: rzg2l-cpg: Add SDHI clk
> mux support
> 
> Hi Biju,
> 
> On Wed, Aug 4, 2021 at 8:08 PM Biju Das <biju.das.jz@bp.renesas.com>
> 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@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> 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?

OK will 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?

OK will check this.

> 
> > +
> > +       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).

OK.

Regards,
Biju

> 
> > +
> > +       return val;
> > +}
> 
> 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
diff mbox series

Patch

diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
index 4d2af113b54e..524d5384b070 100644
--- 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;
+	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);
+
+	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);
+
+	return val;
+}
+
+static const struct clk_ops rzg2l_cpg_sd_clk_mux_ops = {
+	.determine_rate = rzg2l_cpg_sd_clk_mux_determine_rate,
+	.set_parent	= rzg2l_cpg_sd_clk_mux_set_parent,
+	.get_parent	= rzg2l_cpg_sd_clk_mux_get_parent,
+};
+
+static struct clk * __init
+rzg2l_cpg_sd_mux_clk_register(const struct cpg_core_clk *core,
+			      void __iomem *base,
+			      struct rzg2l_cpg_priv *priv)
+{
+	struct sd_hw_data *clk_hw_data;
+	struct clk_init_data init;
+	struct clk_hw *clk_hw;
+	int ret;
+
+	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;
+	clk_hw_data->conf = core->conf;
+	clk_hw_data->mux_flags = core->mux_flags;
+
+	init.name = GET_SHIFT(core->conf) ? "sd1" : "sd0";
+	init.ops = &rzg2l_cpg_sd_clk_mux_ops;
+	init.flags = core->flag;
+	init.num_parents = core->num_parents;
+	init.parent_names = core->parent_names;
+
+	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 pll_clk {
 	struct clk_hw hw;
 	unsigned int conf;
@@ -311,6 +414,9 @@  rzg2l_cpg_register_core_clk(const struct cpg_core_clk *core,
 	case CLK_TYPE_MUX:
 		clk = rzg2l_cpg_mux_clk_register(core, priv->base, priv);
 		break;
+	case CLK_TYPE_SD_MUX:
+		clk = rzg2l_cpg_sd_mux_clk_register(core, priv->base, priv);
+		break;
 	default:
 		goto fail;
 	}
diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h
index 191c403aa52f..7411e3f365c3 100644
--- a/drivers/clk/renesas/rzg2l-cpg.h
+++ b/drivers/clk/renesas/rzg2l-cpg.h
@@ -64,6 +64,9 @@  enum clk_types {
 
 	/* Clock with clock source selector */
 	CLK_TYPE_MUX,
+
+	/* Clock with SD clock source selector */
+	CLK_TYPE_SD_MUX,
 };
 
 #define DEF_TYPE(_name, _id, _type...) \
@@ -84,6 +87,11 @@  enum clk_types {
 	DEF_TYPE(_name, _id, CLK_TYPE_MUX, .conf = _conf, \
 		 .parent_names = _parent_names, .num_parents = _num_parents, \
 		 .flag = _flag, .mux_flags = _mux_flags)
+#define DEF_SD_MUX(_name, _id, _conf, _parent_names, _num_parents, _flag, \
+		   _mux_flags) \
+	DEF_TYPE(_name, _id, CLK_TYPE_SD_MUX, .conf = _conf, \
+		 .parent_names = _parent_names, .num_parents = _num_parents, \
+		 .flag = _flag, .mux_flags = _mux_flags)
 
 /**
  * struct rzg2l_mod_clk - Module Clocks definitions