linux-clk.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 02/18] drivers: clk: renesas: rzg2l-cpg: Add support to handle MUX clocks
       [not found] <20210722141351.13668-1-biju.das.jz@bp.renesas.com>
@ 2021-07-22 14:13 ` Biju Das
  2021-07-23 10:26   ` Sergei Shtylyov
  2021-07-26 10:53   ` Geert Uytterhoeven
  2021-07-22 14:13 ` [PATCH net-next 03/18] drivers: clk: renesas: r9a07g044-cpg: Add ethernet clock sources Biju Das
  2021-07-22 14:13 ` [PATCH net-next 04/18] drivers: clk: renesas: r9a07g044-cpg: Add GbEthernet clock/reset Biju Das
  2 siblings, 2 replies; 12+ messages in thread
From: Biju Das @ 2021-07-22 14:13 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: Biju Das, Geert Uytterhoeven, linux-renesas-soc, linux-clk,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Add support to handle mux clocks inorder to select a clock source
from multiple sources.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/clk/renesas/rzg2l-cpg.c | 24 ++++++++++++++++++++++++
 drivers/clk/renesas/rzg2l-cpg.h |  9 +++++++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
index 3b3b2c3347f3..491b10da5766 100644
--- a/drivers/clk/renesas/rzg2l-cpg.c
+++ b/drivers/clk/renesas/rzg2l-cpg.c
@@ -130,6 +130,27 @@ rzg2l_cpg_div_clk_register(const struct cpg_core_clk *core,
 	return clk_hw->clk;
 }
 
+static struct clk * __init
+rzg2l_cpg_mux_clk_register(const struct cpg_core_clk *core,
+			   void __iomem *base,
+			   struct rzg2l_cpg_priv *priv)
+{
+	const struct clk_hw *clk_hw;
+
+	clk_hw = devm_clk_hw_register_mux(priv->dev, core->name,
+					  core->parent_names, core->num_parents,
+					  core->flag,
+					  base + GET_REG_OFFSET(core->conf),
+					  GET_SHIFT(core->conf),
+					  GET_WIDTH(core->conf),
+					  core->mux_flags, &priv->rmw_lock);
+
+	if (IS_ERR(clk_hw))
+		return ERR_CAST(clk_hw);
+
+	return clk_hw->clk;
+}
+
 struct pll_clk {
 	struct clk_hw hw;
 	unsigned int conf;
@@ -288,6 +309,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_MUX:
+		clk = rzg2l_cpg_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 63695280ce8b..148db5de253b 100644
--- a/drivers/clk/renesas/rzg2l-cpg.h
+++ b/drivers/clk/renesas/rzg2l-cpg.h
@@ -43,6 +43,7 @@ struct cpg_core_clk {
 	const struct clk_div_table *dtable;
 	const char * const *parent_names;
 	int flag;
+	int mux_flags;
 	int num_parents;
 };
 
@@ -54,6 +55,9 @@ enum clk_types {
 
 	/* Clock with divider */
 	CLK_TYPE_DIV,
+
+	/* Clock with clock source selector */
+	CLK_TYPE_MUX,
 };
 
 #define DEF_TYPE(_name, _id, _type...) \
@@ -69,6 +73,11 @@ enum clk_types {
 #define DEF_DIV(_name, _id, _parent, _conf, _dtable, _flag) \
 	DEF_TYPE(_name, _id, CLK_TYPE_DIV, .conf = _conf, \
 		 .parent = _parent, .dtable = _dtable, .flag = _flag)
+#define DEF_MUX(_name, _id, _conf, _parent_names, _num_parents, _flag, \
+		_mux_flags) \
+	DEF_TYPE(_name, _id, CLK_TYPE_MUX, .conf = _conf, \
+		 .parent_names = _parent_names, .num_parents = _num_parents, \
+		 .flag = _flag, .mux_flags = _mux_flags)
 
 /**
  * struct rzg2l_mod_clk - Module Clocks definitions
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 03/18] drivers: clk: renesas: r9a07g044-cpg: Add ethernet clock sources
       [not found] <20210722141351.13668-1-biju.das.jz@bp.renesas.com>
  2021-07-22 14:13 ` [PATCH net-next 02/18] drivers: clk: renesas: rzg2l-cpg: Add support to handle MUX clocks Biju Das
@ 2021-07-22 14:13 ` Biju Das
  2021-07-26 10:50   ` Geert Uytterhoeven
  2021-07-22 14:13 ` [PATCH net-next 04/18] drivers: clk: renesas: r9a07g044-cpg: Add GbEthernet clock/reset Biju Das
  2 siblings, 1 reply; 12+ messages in thread
From: Biju Das @ 2021-07-22 14:13 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: Biju Das, Geert Uytterhoeven, linux-renesas-soc, linux-clk,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Ethernet reference clock can be sourced from PLL5_2 or PLL6_2. Add support
for ethernet source clock selection using SEL_PLL_6_2 mux.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/clk/renesas/r9a07g044-cpg.c | 17 +++++++++++++++++
 drivers/clk/renesas/rzg2l-cpg.h     |  6 ++++++
 2 files changed, 23 insertions(+)

diff --git a/drivers/clk/renesas/r9a07g044-cpg.c b/drivers/clk/renesas/r9a07g044-cpg.c
index 9e9e8fb6d00d..c78bea2f6ea8 100644
--- a/drivers/clk/renesas/r9a07g044-cpg.c
+++ b/drivers/clk/renesas/r9a07g044-cpg.c
@@ -35,8 +35,10 @@ enum clk_ids {
 	CLK_PLL3_DIV4,
 	CLK_PLL4,
 	CLK_PLL5,
+	CLK_PLL5_2,
 	CLK_PLL5_DIV2,
 	CLK_PLL6,
+	CLK_PLL6_DIV2,
 	CLK_P1_DIV2,
 
 	/* Module Clocks */
@@ -53,6 +55,9 @@ static const struct clk_div_table dtable_1_32[] = {
 	{0, 0},
 };
 
+/* Mux clock tables */
+static const char * const sel_pll6_2[]	= { ".pll6_2_div2", ".pll5_2_div2" };
+
 static const struct cpg_core_clk r9a07g044_core_clks[] __initconst = {
 	/* External Clock Inputs */
 	DEF_INPUT("extal", CLK_EXTAL),
@@ -64,6 +69,11 @@ static const struct cpg_core_clk r9a07g044_core_clks[] __initconst = {
 	DEF_FIXED(".pll2", CLK_PLL2, CLK_EXTAL, 133, 2),
 	DEF_FIXED(".pll3", CLK_PLL3, CLK_EXTAL, 133, 2),
 
+	DEF_FIXED(".pll5", CLK_PLL5, CLK_EXTAL, 125, 1),
+	DEF_FIXED(".pll5_2", CLK_PLL5_2, CLK_PLL5, 1, 6),
+
+	DEF_FIXED(".pll6", CLK_PLL6, CLK_EXTAL, 125, 6),
+
 	DEF_FIXED(".pll2_div2", CLK_PLL2_DIV2, CLK_PLL2, 1, 2),
 	DEF_FIXED(".pll2_div16", CLK_PLL2_DIV16, CLK_PLL2, 1, 16),
 	DEF_FIXED(".pll2_div20", CLK_PLL2_DIV20, CLK_PLL2, 1, 20),
@@ -73,6 +83,9 @@ static const struct cpg_core_clk r9a07g044_core_clks[] __initconst = {
 	DEF_FIXED(".pll3_div2_4_2", CLK_PLL3_DIV2_4_2, CLK_PLL3_DIV2_4, 1, 2),
 	DEF_FIXED(".pll3_div4", CLK_PLL3_DIV4, CLK_PLL3, 1, 4),
 
+	DEF_FIXED(".pll5_2_div2", CLK_PLL5_DIV2, CLK_PLL5_2, 1, 2),
+	DEF_FIXED(".pll6_2_div2", CLK_PLL6_DIV2, CLK_PLL6, 1, 2),
+
 	/* Core output clk */
 	DEF_FIXED("I", R9A07G044_CLK_I, CLK_PLL1, 1, 1),
 	DEF_DIV("P0", R9A07G044_CLK_P0, CLK_PLL2_DIV16, DIVPL2A,
@@ -83,6 +96,10 @@ static const struct cpg_core_clk r9a07g044_core_clks[] __initconst = {
 	DEF_FIXED("P1_DIV2", CLK_P1_DIV2, R9A07G044_CLK_P1, 1, 2),
 	DEF_DIV("P2", R9A07G044_CLK_P2, CLK_PLL3_DIV2_4_2,
 		DIVPL3A, dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
+	DEF_FIXED("M0", R9A07G044_CLK_M0, CLK_PLL3_DIV2_4, 1, 1),
+	DEF_FIXED("ZT", R9A07G044_CLK_ZT, CLK_PLL3_DIV2_4_2, 1, 1),
+	DEF_MUX("HP", R9A07G044_CLK_HP, SEL_PLL6_2,
+		sel_pll6_2, ARRAY_SIZE(sel_pll6_2), 0, CLK_MUX_HIWORD_MASK),
 };
 
 static struct rzg2l_mod_clk r9a07g044_mod_clks[] = {
diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h
index 148db5de253b..5202c0512483 100644
--- a/drivers/clk/renesas/rzg2l-cpg.h
+++ b/drivers/clk/renesas/rzg2l-cpg.h
@@ -11,6 +11,7 @@
 
 #define CPG_PL2_DDIV		(0x204)
 #define CPG_PL3A_DDIV		(0x208)
+#define CPG_PL6_ETH_SSEL	(0x418)
 
 /* n = 0/1/2 for PLL1/4/6 */
 #define CPG_SAMPLL_CLK1(n)	(0x04 + (16 * n))
@@ -24,6 +25,11 @@
 #define DIVPL3A		DDIV_PACK(CPG_PL3A_DDIV, 0, 3)
 #define DIVPL3B		DDIV_PACK(CPG_PL3A_DDIV, 4, 3)
 
+#define SEL_PLL_PACK(offset, bitpos, size) \
+		(((offset) << 20) | ((bitpos) << 12) | ((size) << 8))
+
+#define SEL_PLL6_2	SEL_PLL_PACK(CPG_PL6_ETH_SSEL, 0, 1)
+
 /**
  * Definitions of CPG Core Clocks
  *
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 04/18] drivers: clk: renesas: r9a07g044-cpg: Add GbEthernet clock/reset
       [not found] <20210722141351.13668-1-biju.das.jz@bp.renesas.com>
  2021-07-22 14:13 ` [PATCH net-next 02/18] drivers: clk: renesas: rzg2l-cpg: Add support to handle MUX clocks Biju Das
  2021-07-22 14:13 ` [PATCH net-next 03/18] drivers: clk: renesas: r9a07g044-cpg: Add ethernet clock sources Biju Das
@ 2021-07-22 14:13 ` Biju Das
  2021-07-26 10:11   ` Geert Uytterhoeven
  2 siblings, 1 reply; 12+ messages in thread
From: Biju Das @ 2021-07-22 14:13 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: Biju Das, Geert Uytterhoeven, linux-renesas-soc, linux-clk,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Add ETH{0,1} clock/reset entries to CPG driver.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/clk/renesas/r9a07g044-cpg.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/clk/renesas/r9a07g044-cpg.c b/drivers/clk/renesas/r9a07g044-cpg.c
index c78bea2f6ea8..8be09bcdf0f6 100644
--- a/drivers/clk/renesas/r9a07g044-cpg.c
+++ b/drivers/clk/renesas/r9a07g044-cpg.c
@@ -137,6 +137,14 @@ static struct rzg2l_mod_clk r9a07g044_mod_clks[] = {
 				0x578, 2),
 	DEF_MOD("usb_pclk",	R9A07G044_USB_PCLK, R9A07G044_CLK_P1,
 				0x578, 3),
+	DEF_MOD("eth0_axi",	R9A07G044_ETH0_CLK_AXI, R9A07G044_CLK_M0,
+				0x57c, 0),
+	DEF_MOD("eth0_chi",	R9A07G044_ETH0_CLK_CHI, R9A07G044_CLK_ZT,
+				0x57c, 0),
+	DEF_MOD("eth1_axi",	R9A07G044_ETH1_CLK_AXI, R9A07G044_CLK_M0,
+				0x57c, 1),
+	DEF_MOD("eth1_chi",	R9A07G044_ETH1_CLK_CHI, R9A07G044_CLK_ZT,
+				0x57c, 1),
 	DEF_MOD("i2c0",		R9A07G044_I2C0_PCLK, R9A07G044_CLK_P0,
 				0x580, 0),
 	DEF_MOD("i2c1",		R9A07G044_I2C1_PCLK, R9A07G044_CLK_P0,
@@ -181,6 +189,8 @@ static struct rzg2l_reset r9a07g044_resets[] = {
 	DEF_RST(R9A07G044_USB_U2H1_HRESETN, 0x878, 1),
 	DEF_RST(R9A07G044_USB_U2P_EXL_SYSRST, 0x878, 2),
 	DEF_RST(R9A07G044_USB_PRESETN, 0x878, 3),
+	DEF_RST(R9A07G044_ETH0_RST_HW_N, 0x87c, 0),
+	DEF_RST(R9A07G044_ETH1_RST_HW_N, 0x87c, 1),
 	DEF_RST(R9A07G044_I2C0_MRST, 0x880, 0),
 	DEF_RST(R9A07G044_I2C1_MRST, 0x880, 1),
 	DEF_RST(R9A07G044_I2C2_MRST, 0x880, 2),
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 02/18] drivers: clk: renesas: rzg2l-cpg: Add support to handle MUX clocks
  2021-07-22 14:13 ` [PATCH net-next 02/18] drivers: clk: renesas: rzg2l-cpg: Add support to handle MUX clocks Biju Das
@ 2021-07-23 10:26   ` Sergei Shtylyov
  2021-07-23 12:12     ` Biju Das
  2021-07-26 10:53   ` Geert Uytterhoeven
  1 sibling, 1 reply; 12+ messages in thread
From: Sergei Shtylyov @ 2021-07-23 10:26 UTC (permalink / raw)
  To: Biju Das, Michael Turquette, Stephen Boyd
  Cc: Geert Uytterhoeven, linux-renesas-soc, linux-clk, Chris Paterson,
	Biju Das, Prabhakar Mahadev Lad

Hello!

On 22.07.2021 17:13, Biju Das wrote:

> Add support to handle mux clocks inorder to select a clock source

    In order.

> from multiple sources.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>   drivers/clk/renesas/rzg2l-cpg.c | 24 ++++++++++++++++++++++++
>   drivers/clk/renesas/rzg2l-cpg.h |  9 +++++++++
>   2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
> index 3b3b2c3347f3..491b10da5766 100644
> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> @@ -130,6 +130,27 @@ rzg2l_cpg_div_clk_register(const struct cpg_core_clk *core,
>   	return clk_hw->clk;
>   }
>   
> +static struct clk * __init
> +rzg2l_cpg_mux_clk_register(const struct cpg_core_clk *core,
> +			   void __iomem *base,
> +			   struct rzg2l_cpg_priv *priv)
> +{
> +	const struct clk_hw *clk_hw;
> +
> +	clk_hw = devm_clk_hw_register_mux(priv->dev, core->name,
> +					  core->parent_names, core->num_parents,
> +					  core->flag,
> +					  base + GET_REG_OFFSET(core->conf),
> +					  GET_SHIFT(core->conf),
> +					  GET_WIDTH(core->conf),
> +					  core->mux_flags, &priv->rmw_lock);
> +

    Empty line is hardly needed here...

> +	if (IS_ERR(clk_hw))
> +		return ERR_CAST(clk_hw);
> +
> +	return clk_hw->clk;
> +}
> +
>   struct pll_clk {
>   	struct clk_hw hw;
>   	unsigned int conf;
[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH net-next 02/18] drivers: clk: renesas: rzg2l-cpg: Add support to handle MUX clocks
  2021-07-23 10:26   ` Sergei Shtylyov
@ 2021-07-23 12:12     ` Biju Das
  0 siblings, 0 replies; 12+ messages in thread
From: Biju Das @ 2021-07-23 12:12 UTC (permalink / raw)
  To: Sergei Shtylyov, Michael Turquette, Stephen Boyd
  Cc: Geert Uytterhoeven, linux-renesas-soc, linux-clk, Chris Paterson,
	Biju Das, Prabhakar Mahadev Lad


Hi Sergei,

Thanks for the review.

> Subject: Re: [PATCH net-next 02/18] drivers: clk: renesas: rzg2l-cpg: Add
> support to handle MUX clocks
> 
> Hello!
> 
> On 22.07.2021 17:13, Biju Das wrote:
> 
> > Add support to handle mux clocks inorder to select a clock source
> 
>     In order.

Ok.
> 
> > from multiple sources.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >   drivers/clk/renesas/rzg2l-cpg.c | 24 ++++++++++++++++++++++++
> >   drivers/clk/renesas/rzg2l-cpg.h |  9 +++++++++
> >   2 files changed, 33 insertions(+)
> >
> > diff --git a/drivers/clk/renesas/rzg2l-cpg.c
> > b/drivers/clk/renesas/rzg2l-cpg.c index 3b3b2c3347f3..491b10da5766
> > 100644
> > --- a/drivers/clk/renesas/rzg2l-cpg.c
> > +++ b/drivers/clk/renesas/rzg2l-cpg.c
> > @@ -130,6 +130,27 @@ rzg2l_cpg_div_clk_register(const struct
> cpg_core_clk *core,
> >   	return clk_hw->clk;
> >   }
> >
> > +static struct clk * __init
> > +rzg2l_cpg_mux_clk_register(const struct cpg_core_clk *core,
> > +			   void __iomem *base,
> > +			   struct rzg2l_cpg_priv *priv)
> > +{
> > +	const struct clk_hw *clk_hw;
> > +
> > +	clk_hw = devm_clk_hw_register_mux(priv->dev, core->name,
> > +					  core->parent_names, core->num_parents,
> > +					  core->flag,
> > +					  base + GET_REG_OFFSET(core->conf),
> > +					  GET_SHIFT(core->conf),
> > +					  GET_WIDTH(core->conf),
> > +					  core->mux_flags, &priv->rmw_lock);
> > +
> 
>     Empty line is hardly needed here...

OK. Will fix it in next version.

Cheers,
Biju

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 04/18] drivers: clk: renesas: r9a07g044-cpg: Add GbEthernet clock/reset
  2021-07-22 14:13 ` [PATCH net-next 04/18] drivers: clk: renesas: r9a07g044-cpg: Add GbEthernet clock/reset Biju Das
@ 2021-07-26 10:11   ` Geert Uytterhoeven
  2021-07-26 10:18     ` Biju Das
  0 siblings, 1 reply; 12+ messages in thread
From: Geert Uytterhoeven @ 2021-07-26 10:11 UTC (permalink / raw)
  To: Biju Das
  Cc: Michael Turquette, Stephen Boyd, Linux-Renesas, linux-clk,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Biju,

On Thu, Jul 22, 2021 at 4:14 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Add ETH{0,1} clock/reset entries to CPG driver.
>
> 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/r9a07g044-cpg.c
> +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> @@ -137,6 +137,14 @@ static struct rzg2l_mod_clk r9a07g044_mod_clks[] = {
>                                 0x578, 2),
>         DEF_MOD("usb_pclk",     R9A07G044_USB_PCLK, R9A07G044_CLK_P1,
>                                 0x578, 3),
> +       DEF_MOD("eth0_axi",     R9A07G044_ETH0_CLK_AXI, R9A07G044_CLK_M0,
> +                               0x57c, 0),
> +       DEF_MOD("eth0_chi",     R9A07G044_ETH0_CLK_CHI, R9A07G044_CLK_ZT,
> +                               0x57c, 0),
> +       DEF_MOD("eth1_axi",     R9A07G044_ETH1_CLK_AXI, R9A07G044_CLK_M0,
> +                               0x57c, 1),
> +       DEF_MOD("eth1_chi",     R9A07G044_ETH1_CLK_CHI, R9A07G044_CLK_ZT,
> +                               0x57c, 1),

The AXI and CHI clocks use the same register bits, so this won't work
as expected. E.g. when disabling one clock, the other clock will be
disabled, too. The correct way to handle this is to create a new clock
type for coupled clocks, which sets the CPG_CLKON_ETH.CLK[01]_ON bit
when at least one clock is enabled, and clears the bit only when both
clocks are disabled.

>         DEF_MOD("i2c0",         R9A07G044_I2C0_PCLK, R9A07G044_CLK_P0,
>                                 0x580, 0),
>         DEF_MOD("i2c1",         R9A07G044_I2C1_PCLK, R9A07G044_CLK_P0,
> @@ -181,6 +189,8 @@ static struct rzg2l_reset r9a07g044_resets[] = {
>         DEF_RST(R9A07G044_USB_U2H1_HRESETN, 0x878, 1),
>         DEF_RST(R9A07G044_USB_U2P_EXL_SYSRST, 0x878, 2),
>         DEF_RST(R9A07G044_USB_PRESETN, 0x878, 3),
> +       DEF_RST(R9A07G044_ETH0_RST_HW_N, 0x87c, 0),
> +       DEF_RST(R9A07G044_ETH1_RST_HW_N, 0x87c, 1),
>         DEF_RST(R9A07G044_I2C0_MRST, 0x880, 0),
>         DEF_RST(R9A07G044_I2C1_MRST, 0x880, 1),
>         DEF_RST(R9A07G044_I2C2_MRST, 0x880, 2),

This part is OK.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH net-next 04/18] drivers: clk: renesas: r9a07g044-cpg: Add GbEthernet clock/reset
  2021-07-26 10:11   ` Geert Uytterhoeven
@ 2021-07-26 10:18     ` Biju Das
  0 siblings, 0 replies; 12+ messages in thread
From: Biju Das @ 2021-07-26 10:18 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Michael Turquette, Stephen Boyd, Linux-Renesas, linux-clk,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH net-next 04/18] drivers: clk: renesas: r9a07g044-cpg:
> Add GbEthernet clock/reset
> 
> Hi Biju,
> 
> On Thu, Jul 22, 2021 at 4:14 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Add ETH{0,1} clock/reset entries to CPG driver.
> >
> > 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/r9a07g044-cpg.c
> > +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> > @@ -137,6 +137,14 @@ static struct rzg2l_mod_clk r9a07g044_mod_clks[] =
> {
> >                                 0x578, 2),
> >         DEF_MOD("usb_pclk",     R9A07G044_USB_PCLK, R9A07G044_CLK_P1,
> >                                 0x578, 3),
> > +       DEF_MOD("eth0_axi",     R9A07G044_ETH0_CLK_AXI,
> R9A07G044_CLK_M0,
> > +                               0x57c, 0),
> > +       DEF_MOD("eth0_chi",     R9A07G044_ETH0_CLK_CHI,
> R9A07G044_CLK_ZT,
> > +                               0x57c, 0),
> > +       DEF_MOD("eth1_axi",     R9A07G044_ETH1_CLK_AXI,
> R9A07G044_CLK_M0,
> > +                               0x57c, 1),
> > +       DEF_MOD("eth1_chi",     R9A07G044_ETH1_CLK_CHI,
> R9A07G044_CLK_ZT,
> > +                               0x57c, 1),
> 
> The AXI and CHI clocks use the same register bits, so this won't work as
> expected. E.g. when disabling one clock, the other clock will be disabled,
> too. The correct way to handle this is to create a new clock type for
> coupled clocks, which sets the CPG_CLKON_ETH.CLK[01]_ON bit when at least
> one clock is enabled, and clears the bit only when both clocks are
> disabled.

OK. Will create new clk type and add the logic to handle the same.

Regards,
Biju

> 
> >         DEF_MOD("i2c0",         R9A07G044_I2C0_PCLK, R9A07G044_CLK_P0,
> >                                 0x580, 0),
> >         DEF_MOD("i2c1",         R9A07G044_I2C1_PCLK, R9A07G044_CLK_P0,
> > @@ -181,6 +189,8 @@ static struct rzg2l_reset r9a07g044_resets[] = {
> >         DEF_RST(R9A07G044_USB_U2H1_HRESETN, 0x878, 1),
> >         DEF_RST(R9A07G044_USB_U2P_EXL_SYSRST, 0x878, 2),
> >         DEF_RST(R9A07G044_USB_PRESETN, 0x878, 3),
> > +       DEF_RST(R9A07G044_ETH0_RST_HW_N, 0x87c, 0),
> > +       DEF_RST(R9A07G044_ETH1_RST_HW_N, 0x87c, 1),
> >         DEF_RST(R9A07G044_I2C0_MRST, 0x880, 0),
> >         DEF_RST(R9A07G044_I2C1_MRST, 0x880, 1),
> >         DEF_RST(R9A07G044_I2C2_MRST, 0x880, 2),
> 
> This part is OK.
> 
> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 03/18] drivers: clk: renesas: r9a07g044-cpg: Add ethernet clock sources
  2021-07-22 14:13 ` [PATCH net-next 03/18] drivers: clk: renesas: r9a07g044-cpg: Add ethernet clock sources Biju Das
@ 2021-07-26 10:50   ` Geert Uytterhoeven
  2021-07-26 11:43     ` Biju Das
  0 siblings, 1 reply; 12+ messages in thread
From: Geert Uytterhoeven @ 2021-07-26 10:50 UTC (permalink / raw)
  To: Biju Das
  Cc: Michael Turquette, Stephen Boyd, Geert Uytterhoeven,
	Linux-Renesas, linux-clk, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad

Hi Biju,

On Thu, Jul 22, 2021 at 4:14 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Ethernet reference clock can be sourced from PLL5_2 or PLL6_2. Add support
> for ethernet source clock selection using SEL_PLL_6_2 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/r9a07g044-cpg.c
> +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> @@ -35,8 +35,10 @@ enum clk_ids {
>         CLK_PLL3_DIV4,
>         CLK_PLL4,
>         CLK_PLL5,
> +       CLK_PLL5_2,

Why do you need this? We already have CLK_PLL5_DIV2?

>         CLK_PLL5_DIV2,
>         CLK_PLL6,
> +       CLK_PLL6_DIV2,
>         CLK_P1_DIV2,
>
>         /* Module Clocks */
> @@ -53,6 +55,9 @@ static const struct clk_div_table dtable_1_32[] = {
>         {0, 0},
>  };
>
> +/* Mux clock tables */
> +static const char * const sel_pll6_2[] = { ".pll6_2_div2", ".pll5_2_div2" };

".pll6_div2", ".pll5_div2"

> +
>  static const struct cpg_core_clk r9a07g044_core_clks[] __initconst = {
>         /* External Clock Inputs */
>         DEF_INPUT("extal", CLK_EXTAL),
> @@ -64,6 +69,11 @@ static const struct cpg_core_clk r9a07g044_core_clks[] __initconst = {
>         DEF_FIXED(".pll2", CLK_PLL2, CLK_EXTAL, 133, 2),
>         DEF_FIXED(".pll3", CLK_PLL3, CLK_EXTAL, 133, 2),
>
> +       DEF_FIXED(".pll5", CLK_PLL5, CLK_EXTAL, 125, 1),
> +       DEF_FIXED(".pll5_2", CLK_PLL5_2, CLK_PLL5, 1, 6),

Suffix "_2" but divided by 6?
Why do you need this clock?

> +
> +       DEF_FIXED(".pll6", CLK_PLL6, CLK_EXTAL, 125, 6),
> +
>         DEF_FIXED(".pll2_div2", CLK_PLL2_DIV2, CLK_PLL2, 1, 2),
>         DEF_FIXED(".pll2_div16", CLK_PLL2_DIV16, CLK_PLL2, 1, 16),
>         DEF_FIXED(".pll2_div20", CLK_PLL2_DIV20, CLK_PLL2, 1, 20),
> @@ -73,6 +83,9 @@ static const struct cpg_core_clk r9a07g044_core_clks[] __initconst = {
>         DEF_FIXED(".pll3_div2_4_2", CLK_PLL3_DIV2_4_2, CLK_PLL3_DIV2_4, 1, 2),
>         DEF_FIXED(".pll3_div4", CLK_PLL3_DIV4, CLK_PLL3, 1, 4),
>
> +       DEF_FIXED(".pll5_2_div2", CLK_PLL5_DIV2, CLK_PLL5_2, 1, 2),

".pll5_div2"
Figure 8.3 ("Clock System Diagram (2)") says the parent is CLK_PLL5?

> +       DEF_FIXED(".pll6_2_div2", CLK_PLL6_DIV2, CLK_PLL6, 1, 2),

".pll6_div2"

> +
>         /* Core output clk */
>         DEF_FIXED("I", R9A07G044_CLK_I, CLK_PLL1, 1, 1),
>         DEF_DIV("P0", R9A07G044_CLK_P0, CLK_PLL2_DIV16, DIVPL2A,
> @@ -83,6 +96,10 @@ static const struct cpg_core_clk r9a07g044_core_clks[] __initconst = {
>         DEF_FIXED("P1_DIV2", CLK_P1_DIV2, R9A07G044_CLK_P1, 1, 2),
>         DEF_DIV("P2", R9A07G044_CLK_P2, CLK_PLL3_DIV2_4_2,
>                 DIVPL3A, dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
> +       DEF_FIXED("M0", R9A07G044_CLK_M0, CLK_PLL3_DIV2_4, 1, 1),
> +       DEF_FIXED("ZT", R9A07G044_CLK_ZT, CLK_PLL3_DIV2_4_2, 1, 1),
> +       DEF_MUX("HP", R9A07G044_CLK_HP, SEL_PLL6_2,
> +               sel_pll6_2, ARRAY_SIZE(sel_pll6_2), 0, CLK_MUX_HIWORD_MASK),

OK.

>  };
>
>  static struct rzg2l_mod_clk r9a07g044_mod_clks[] = {
> diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h
> index 148db5de253b..5202c0512483 100644
> --- a/drivers/clk/renesas/rzg2l-cpg.h
> +++ b/drivers/clk/renesas/rzg2l-cpg.h
> @@ -11,6 +11,7 @@
>
>  #define CPG_PL2_DDIV           (0x204)
>  #define CPG_PL3A_DDIV          (0x208)
> +#define CPG_PL6_ETH_SSEL       (0x418)
>
>  /* n = 0/1/2 for PLL1/4/6 */
>  #define CPG_SAMPLL_CLK1(n)     (0x04 + (16 * n))
> @@ -24,6 +25,11 @@
>  #define DIVPL3A                DDIV_PACK(CPG_PL3A_DDIV, 0, 3)
>  #define DIVPL3B                DDIV_PACK(CPG_PL3A_DDIV, 4, 3)
>
> +#define SEL_PLL_PACK(offset, bitpos, size) \
> +               (((offset) << 20) | ((bitpos) << 12) | ((size) << 8))

I think the addition of SEL_PLL_PACK() should be moved to
[PATCH 02/18].

> +
> +#define SEL_PLL6_2     SEL_PLL_PACK(CPG_PL6_ETH_SSEL, 0, 1)
> +
>  /**
>   * Definitions of CPG Core Clocks
>   *

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 02/18] drivers: clk: renesas: rzg2l-cpg: Add support to handle MUX clocks
  2021-07-22 14:13 ` [PATCH net-next 02/18] drivers: clk: renesas: rzg2l-cpg: Add support to handle MUX clocks Biju Das
  2021-07-23 10:26   ` Sergei Shtylyov
@ 2021-07-26 10:53   ` Geert Uytterhoeven
  2021-07-26 12:23     ` Biju Das
  1 sibling, 1 reply; 12+ messages in thread
From: Geert Uytterhoeven @ 2021-07-26 10:53 UTC (permalink / raw)
  To: Biju Das
  Cc: Michael Turquette, Stephen Boyd, Linux-Renesas, linux-clk,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Biju,


On Thu, Jul 22, 2021 at 4:14 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Add support to handle mux clocks inorder to select a clock source
> from multiple sources.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

> --- a/drivers/clk/renesas/rzg2l-cpg.h
> +++ b/drivers/clk/renesas/rzg2l-cpg.h
> @@ -43,6 +43,7 @@ struct cpg_core_clk {
>         const struct clk_div_table *dtable;
>         const char * const *parent_names;
>         int flag;
> +       int mux_flags;
>         int num_parents;
>  };
>

I'd move SEL_PLL_PACK() from [PATCH 03/18] here, as it applies to
"_conf" in DEF_MUX() below.

> @@ -54,6 +55,9 @@ enum clk_types {
>
>         /* Clock with divider */
>         CLK_TYPE_DIV,
> +
> +       /* Clock with clock source selector */
> +       CLK_TYPE_MUX,
>  };
>
>  #define DEF_TYPE(_name, _id, _type...) \
> @@ -69,6 +73,11 @@ enum clk_types {
>  #define DEF_DIV(_name, _id, _parent, _conf, _dtable, _flag) \
>         DEF_TYPE(_name, _id, CLK_TYPE_DIV, .conf = _conf, \
>                  .parent = _parent, .dtable = _dtable, .flag = _flag)
> +#define DEF_MUX(_name, _id, _conf, _parent_names, _num_parents, _flag, \
> +               _mux_flags) \
> +       DEF_TYPE(_name, _id, CLK_TYPE_MUX, .conf = _conf, \
> +                .parent_names = _parent_names, .num_parents = _num_parents, \
> +                .flag = _flag, .mux_flags = _mux_flags)
>
>  /**
>   * struct rzg2l_mod_clk - Module Clocks definitions

With the above fixed:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH net-next 03/18] drivers: clk: renesas: r9a07g044-cpg: Add ethernet clock sources
  2021-07-26 10:50   ` Geert Uytterhoeven
@ 2021-07-26 11:43     ` Biju Das
  2021-07-26 11:50       ` Geert Uytterhoeven
  0 siblings, 1 reply; 12+ messages in thread
From: Biju Das @ 2021-07-26 11:43 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Michael Turquette, Stephen Boyd, Geert Uytterhoeven,
	Linux-Renesas, linux-clk, Chris Paterson, Biju Das,
	Prabhakar Mahadev Lad

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH net-next 03/18] drivers: clk: renesas: r9a07g044-cpg:
> Add ethernet clock sources
> 
> Hi Biju,
> 
> On Thu, Jul 22, 2021 at 4:14 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Ethernet reference clock can be sourced from PLL5_2 or PLL6_2. Add
> > support for ethernet source clock selection using SEL_PLL_6_2 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/r9a07g044-cpg.c
> > +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> > @@ -35,8 +35,10 @@ enum clk_ids {
> >         CLK_PLL3_DIV4,
> >         CLK_PLL4,
> >         CLK_PLL5,
> > +       CLK_PLL5_2,
> 
> Why do you need this? We already have CLK_PLL5_DIV2?

As per clock list HP =250 MHz.

With PLL5_DIV2 the frequency, what we get is 3000 x 1/2 = 1500 MHz, which is incorrect

There is some issue with HW manual. It is not clear. I have requested to fix those issue.

Actually PLL5 generates 3 clk outputs (As per page 403 of manual)

FOUTPOSTDIV= 3000
FOUT3 = 500
FOUT1PH0 = 1500

To match the clock list document and Figure 8.3 Clock system diagram for HP clock.
This clock has to be FOUT3.

FOUT3(second PLL5 clock source)* 1/2 =250 MHz(HP clock).

On the HW manual, Divider name for PLL5 is mentioned as PLL_5_2_DIV12 and for PLL6 as PLL6_2_DIV2.

> 
> >         CLK_PLL5_DIV2,
> >         CLK_PLL6,
> > +       CLK_PLL6_DIV2,
> >         CLK_P1_DIV2,
> >
> >         /* Module Clocks */
> > @@ -53,6 +55,9 @@ static const struct clk_div_table dtable_1_32[] = {
> >         {0, 0},
> >  };
> >
> > +/* Mux clock tables */
> > +static const char * const sel_pll6_2[] = { ".pll6_2_div2",
> > +".pll5_2_div2" };
> 
> ".pll6_div2", ".pll5_div2"

Basically PLL6 generates 2 clock outputs. That is the reason it is termed as PLL6_2 for second clock
And pll5 generates 3 clock outputs and pll5_2(second clock source) is used for HP.

To match the register definition, I will change this to ".pll6_2_div2" and ".pll5_2_div12". Is it ok?

> 
> > +
> >  static const struct cpg_core_clk r9a07g044_core_clks[] __initconst = {
> >         /* External Clock Inputs */
> >         DEF_INPUT("extal", CLK_EXTAL), @@ -64,6 +69,11 @@ static const
> > struct cpg_core_clk r9a07g044_core_clks[] __initconst = {
> >         DEF_FIXED(".pll2", CLK_PLL2, CLK_EXTAL, 133, 2),
> >         DEF_FIXED(".pll3", CLK_PLL3, CLK_EXTAL, 133, 2),
> >
> > +       DEF_FIXED(".pll5", CLK_PLL5, CLK_EXTAL, 125, 1),
> > +       DEF_FIXED(".pll5_2", CLK_PLL5_2, CLK_PLL5, 1, 6),
> 
> Suffix "_2" but divided by 6?
> Why do you need this clock?

To generate 500 MHz Clock, so that it can match with clock list document.

> 
> > +
> > +       DEF_FIXED(".pll6", CLK_PLL6, CLK_EXTAL, 125, 6),
> > +
> >         DEF_FIXED(".pll2_div2", CLK_PLL2_DIV2, CLK_PLL2, 1, 2),
> >         DEF_FIXED(".pll2_div16", CLK_PLL2_DIV16, CLK_PLL2, 1, 16),
> >         DEF_FIXED(".pll2_div20", CLK_PLL2_DIV20, CLK_PLL2, 1, 20), @@
> > -73,6 +83,9 @@ static const struct cpg_core_clk r9a07g044_core_clks[]
> __initconst = {
> >         DEF_FIXED(".pll3_div2_4_2", CLK_PLL3_DIV2_4_2, CLK_PLL3_DIV2_4,
> 1, 2),
> >         DEF_FIXED(".pll3_div4", CLK_PLL3_DIV4, CLK_PLL3, 1, 4),
> >
> > +       DEF_FIXED(".pll5_2_div2", CLK_PLL5_DIV2, CLK_PLL5_2, 1, 2),
> 
> ".pll5_div2"
> Figure 8.3 ("Clock System Diagram (2)") says the parent is CLK_PLL5?

There is some issue in HW manual. Actually parent is second clock out from PLL5. See page 403.


> 
> > +       DEF_FIXED(".pll6_2_div2", CLK_PLL6_DIV2, CLK_PLL6, 1, 2),
> 
> ".pll6_div2"
> 
> > +
> >         /* Core output clk */
> >         DEF_FIXED("I", R9A07G044_CLK_I, CLK_PLL1, 1, 1),
> >         DEF_DIV("P0", R9A07G044_CLK_P0, CLK_PLL2_DIV16, DIVPL2A, @@
> > -83,6 +96,10 @@ static const struct cpg_core_clk r9a07g044_core_clks[]
> __initconst = {
> >         DEF_FIXED("P1_DIV2", CLK_P1_DIV2, R9A07G044_CLK_P1, 1, 2),
> >         DEF_DIV("P2", R9A07G044_CLK_P2, CLK_PLL3_DIV2_4_2,
> >                 DIVPL3A, dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
> > +       DEF_FIXED("M0", R9A07G044_CLK_M0, CLK_PLL3_DIV2_4, 1, 1),
> > +       DEF_FIXED("ZT", R9A07G044_CLK_ZT, CLK_PLL3_DIV2_4_2, 1, 1),
> > +       DEF_MUX("HP", R9A07G044_CLK_HP, SEL_PLL6_2,
> > +               sel_pll6_2, ARRAY_SIZE(sel_pll6_2), 0,
> > + CLK_MUX_HIWORD_MASK),
> 
> OK.
> 
> >  };
> >
> >  static struct rzg2l_mod_clk r9a07g044_mod_clks[] = { diff --git
> > a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h
> > index 148db5de253b..5202c0512483 100644
> > --- a/drivers/clk/renesas/rzg2l-cpg.h
> > +++ b/drivers/clk/renesas/rzg2l-cpg.h
> > @@ -11,6 +11,7 @@
> >
> >  #define CPG_PL2_DDIV           (0x204)
> >  #define CPG_PL3A_DDIV          (0x208)
> > +#define CPG_PL6_ETH_SSEL       (0x418)
> >
> >  /* n = 0/1/2 for PLL1/4/6 */
> >  #define CPG_SAMPLL_CLK1(n)     (0x04 + (16 * n))
> > @@ -24,6 +25,11 @@
> >  #define DIVPL3A                DDIV_PACK(CPG_PL3A_DDIV, 0, 3)
> >  #define DIVPL3B                DDIV_PACK(CPG_PL3A_DDIV, 4, 3)
> >
> > +#define SEL_PLL_PACK(offset, bitpos, size) \
> > +               (((offset) << 20) | ((bitpos) << 12) | ((size) << 8))
> 
> I think the addition of SEL_PLL_PACK() should be moved to [PATCH 02/18].
> 

OK.

Regards,
Biju

> > +
> > +#define SEL_PLL6_2     SEL_PLL_PACK(CPG_PL6_ETH_SSEL, 0, 1)
> > +
> >  /**
> >   * Definitions of CPG Core Clocks
> >   *
> 
> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 03/18] drivers: clk: renesas: r9a07g044-cpg: Add ethernet clock sources
  2021-07-26 11:43     ` Biju Das
@ 2021-07-26 11:50       ` Geert Uytterhoeven
  0 siblings, 0 replies; 12+ messages in thread
From: Geert Uytterhoeven @ 2021-07-26 11:50 UTC (permalink / raw)
  To: Biju Das
  Cc: Michael Turquette, Stephen Boyd, Linux-Renesas, linux-clk,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad

Hi Biju,

On Mon, Jul 26, 2021 at 1:43 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH net-next 03/18] drivers: clk: renesas: r9a07g044-cpg:
> > Add ethernet clock sources
> > On Thu, Jul 22, 2021 at 4:14 PM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > Ethernet reference clock can be sourced from PLL5_2 or PLL6_2. Add
> > > support for ethernet source clock selection using SEL_PLL_6_2 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/r9a07g044-cpg.c
> > > +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> > > @@ -35,8 +35,10 @@ enum clk_ids {
> > >         CLK_PLL3_DIV4,
> > >         CLK_PLL4,
> > >         CLK_PLL5,
> > > +       CLK_PLL5_2,
> >
> > Why do you need this? We already have CLK_PLL5_DIV2?
>
> As per clock list HP =250 MHz.
>
> With PLL5_DIV2 the frequency, what we get is 3000 x 1/2 = 1500 MHz, which is incorrect
>
> There is some issue with HW manual. It is not clear. I have requested to fix those issue.
>
> Actually PLL5 generates 3 clk outputs (As per page 403 of manual)
>
> FOUTPOSTDIV= 3000
> FOUT3 = 500
> FOUT1PH0 = 1500
>
> To match the clock list document and Figure 8.3 Clock system diagram for HP clock.
> This clock has to be FOUT3.
>
> FOUT3(second PLL5 clock source)* 1/2 =250 MHz(HP clock).
>
> On the HW manual, Divider name for PLL5 is mentioned as PLL_5_2_DIV12 and for PLL6 as PLL6_2_DIV2.
>
> >
> > >         CLK_PLL5_DIV2,
> > >         CLK_PLL6,
> > > +       CLK_PLL6_DIV2,
> > >         CLK_P1_DIV2,
> > >
> > >         /* Module Clocks */
> > > @@ -53,6 +55,9 @@ static const struct clk_div_table dtable_1_32[] = {
> > >         {0, 0},
> > >  };
> > >
> > > +/* Mux clock tables */
> > > +static const char * const sel_pll6_2[] = { ".pll6_2_div2",
> > > +".pll5_2_div2" };
> >
> > ".pll6_div2", ".pll5_div2"
>
> Basically PLL6 generates 2 clock outputs. That is the reason it is termed as PLL6_2 for second clock
> And pll5 generates 3 clock outputs and pll5_2(second clock source) is used for HP.
>
> To match the register definition, I will change this to ".pll6_2_div2" and ".pll5_2_div12". Is it ok?

OK, thanks for the explanation!

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH net-next 02/18] drivers: clk: renesas: rzg2l-cpg: Add support to handle MUX clocks
  2021-07-26 10:53   ` Geert Uytterhoeven
@ 2021-07-26 12:23     ` Biju Das
  0 siblings, 0 replies; 12+ messages in thread
From: Biju Das @ 2021-07-26 12:23 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Michael Turquette, Stephen Boyd, Linux-Renesas, linux-clk,
	Chris Paterson, Biju Das, Prabhakar Mahadev Lad


Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH net-next 02/18] drivers: clk: renesas: rzg2l-cpg: Add
> support to handle MUX clocks
> 
> Hi Biju,
> 
> 
> On Thu, Jul 22, 2021 at 4:14 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Add support to handle mux clocks inorder to select a clock source from
> > multiple sources.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> > --- a/drivers/clk/renesas/rzg2l-cpg.h
> > +++ b/drivers/clk/renesas/rzg2l-cpg.h
> > @@ -43,6 +43,7 @@ struct cpg_core_clk {
> >         const struct clk_div_table *dtable;
> >         const char * const *parent_names;
> >         int flag;
> > +       int mux_flags;
> >         int num_parents;
> >  };
> >
> 
> I'd move SEL_PLL_PACK() from [PATCH 03/18] here, as it applies to "_conf"
> in DEF_MUX() below.

OK. Will move in this patch

Regards,
Biju

> 
> > @@ -54,6 +55,9 @@ enum clk_types {
> >
> >         /* Clock with divider */
> >         CLK_TYPE_DIV,
> > +
> > +       /* Clock with clock source selector */
> > +       CLK_TYPE_MUX,
> >  };
> >
> >  #define DEF_TYPE(_name, _id, _type...) \ @@ -69,6 +73,11 @@ enum
> > clk_types {  #define DEF_DIV(_name, _id, _parent, _conf, _dtable,
> > _flag) \
> >         DEF_TYPE(_name, _id, CLK_TYPE_DIV, .conf = _conf, \
> >                  .parent = _parent, .dtable = _dtable, .flag = _flag)
> > +#define DEF_MUX(_name, _id, _conf, _parent_names, _num_parents, _flag,
> \
> > +               _mux_flags) \
> > +       DEF_TYPE(_name, _id, CLK_TYPE_MUX, .conf = _conf, \
> > +                .parent_names = _parent_names, .num_parents =
> _num_parents, \
> > +                .flag = _flag, .mux_flags = _mux_flags)
> >
> >  /**
> >   * struct rzg2l_mod_clk - Module Clocks definitions
> 
> With the above fixed:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-07-26 12:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210722141351.13668-1-biju.das.jz@bp.renesas.com>
2021-07-22 14:13 ` [PATCH net-next 02/18] drivers: clk: renesas: rzg2l-cpg: Add support to handle MUX clocks Biju Das
2021-07-23 10:26   ` Sergei Shtylyov
2021-07-23 12:12     ` Biju Das
2021-07-26 10:53   ` Geert Uytterhoeven
2021-07-26 12:23     ` Biju Das
2021-07-22 14:13 ` [PATCH net-next 03/18] drivers: clk: renesas: r9a07g044-cpg: Add ethernet clock sources Biju Das
2021-07-26 10:50   ` Geert Uytterhoeven
2021-07-26 11:43     ` Biju Das
2021-07-26 11:50       ` Geert Uytterhoeven
2021-07-22 14:13 ` [PATCH net-next 04/18] drivers: clk: renesas: r9a07g044-cpg: Add GbEthernet clock/reset Biju Das
2021-07-26 10:11   ` Geert Uytterhoeven
2021-07-26 10:18     ` Biju Das

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).