linux-clk.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] clk: qcom: clk-rcg2: Introduce a cfg offset for RCGs
@ 2019-01-28 11:53 Vinod Koul
  2019-01-28 11:53 ` [PATCH 2/2] clk: qcom: gcc-qcs404: Add cfg_offset for blsp1_uart3 clock Vinod Koul
  2019-01-29 22:42 ` [PATCH 1/2] clk: qcom: clk-rcg2: Introduce a cfg offset for RCGs Stephen Boyd
  0 siblings, 2 replies; 5+ messages in thread
From: Vinod Koul @ 2019-01-28 11:53 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: Khasim Syed Mohammed, Bjorn Andersson, Taniya Das, Andy Gross,
	David Brown, linux-arm-msm, linux-clk, Anu Ramanathan, Shawn Guo,
	Vinod Koul

From: Taniya Das <tdas@codeaurora.org>

The RCG CFG/M/N/D register base could be at a different offset than
the CMD register, so introduce a cfg_offset to identify the offset
with respect to the CMD register.

Signed-off-by: Taniya Das <tdas@codeaurora.org>
Signed-off-by: Anu Ramanathan <anur@codeaurora.org>
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/clk/qcom/clk-rcg.h  |  2 ++
 drivers/clk/qcom/clk-rcg2.c | 30 +++++++++++++++++++-----------
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
index e5eca8a1abe4..f06783c20688 100644
--- a/drivers/clk/qcom/clk-rcg.h
+++ b/drivers/clk/qcom/clk-rcg.h
@@ -140,6 +140,7 @@ extern const struct clk_ops clk_dyn_rcg_ops;
  * @parent_map: map from software's parent index to hardware's src_sel field
  * @freq_tbl: frequency table
  * @clkr: regmap clock handle
+ * @cfg_off: defines the cfg register offset from the CMD_RCGR
  *
  */
 struct clk_rcg2 {
@@ -150,6 +151,7 @@ struct clk_rcg2 {
 	const struct parent_map	*parent_map;
 	const struct freq_tbl	*freq_tbl;
 	struct clk_regmap	clkr;
+	u8			cfg_off;
 };
 
 #define to_clk_rcg2(_hw) container_of(to_clk_regmap(_hw), struct clk_rcg2, clkr)
diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
index 6e3bd195d012..106848e3313f 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -74,7 +74,8 @@ static u8 clk_rcg2_get_parent(struct clk_hw *hw)
 	u32 cfg;
 	int i, ret;
 
-	ret = regmap_read(rcg->clkr.regmap, rcg->cmd_rcgr + CFG_REG, &cfg);
+	ret = regmap_read(rcg->clkr.regmap,
+			  rcg->cmd_rcgr + rcg->cfg_off + CFG_REG, &cfg);
 	if (ret)
 		goto err;
 
@@ -123,7 +124,8 @@ static int clk_rcg2_set_parent(struct clk_hw *hw, u8 index)
 	int ret;
 	u32 cfg = rcg->parent_map[index].cfg << CFG_SRC_SEL_SHIFT;
 
-	ret = regmap_update_bits(rcg->clkr.regmap, rcg->cmd_rcgr + CFG_REG,
+	ret = regmap_update_bits(rcg->clkr.regmap,
+				 rcg->cmd_rcgr + rcg->cfg_off + CFG_REG,
 				 CFG_SRC_SEL_MASK, cfg);
 	if (ret)
 		return ret;
@@ -162,13 +164,16 @@ clk_rcg2_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
 	u32 cfg, hid_div, m = 0, n = 0, mode = 0, mask;
 
-	regmap_read(rcg->clkr.regmap, rcg->cmd_rcgr + CFG_REG, &cfg);
+	regmap_read(rcg->clkr.regmap,
+		    rcg->cmd_rcgr + rcg->cfg_off + CFG_REG, &cfg);
 
 	if (rcg->mnd_width) {
 		mask = BIT(rcg->mnd_width) - 1;
-		regmap_read(rcg->clkr.regmap, rcg->cmd_rcgr + M_REG, &m);
+		regmap_read(rcg->clkr.regmap,
+			    rcg->cmd_rcgr + rcg->cfg_off + M_REG, &m);
 		m &= mask;
-		regmap_read(rcg->clkr.regmap, rcg->cmd_rcgr + N_REG, &n);
+		regmap_read(rcg->clkr.regmap,
+			    rcg->cmd_rcgr + rcg->cfg_off + N_REG, &n);
 		n =  ~n;
 		n &= mask;
 		n += m;
@@ -263,17 +268,20 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
 	if (rcg->mnd_width && f->n) {
 		mask = BIT(rcg->mnd_width) - 1;
 		ret = regmap_update_bits(rcg->clkr.regmap,
-				rcg->cmd_rcgr + M_REG, mask, f->m);
+					 rcg->cmd_rcgr + rcg->cfg_off + M_REG,
+					 mask, f->m);
 		if (ret)
 			return ret;
 
 		ret = regmap_update_bits(rcg->clkr.regmap,
-				rcg->cmd_rcgr + N_REG, mask, ~(f->n - f->m));
+					 rcg->cmd_rcgr + rcg->cfg_off + N_REG,
+					 mask, ~(f->n - f->m));
 		if (ret)
 			return ret;
 
 		ret = regmap_update_bits(rcg->clkr.regmap,
-				rcg->cmd_rcgr + D_REG, mask, ~f->n);
+					 rcg->cmd_rcgr + rcg->cfg_off + D_REG,
+					 mask, ~f->n);
 		if (ret)
 			return ret;
 	}
@@ -284,9 +292,9 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
 	cfg |= rcg->parent_map[index].cfg << CFG_SRC_SEL_SHIFT;
 	if (rcg->mnd_width && f->n && (f->m != f->n))
 		cfg |= CFG_MODE_DUAL_EDGE;
-
-	return regmap_update_bits(rcg->clkr.regmap, rcg->cmd_rcgr + CFG_REG,
-					mask, cfg);
+	return regmap_update_bits(rcg->clkr.regmap,
+				    rcg->cmd_rcgr + rcg->cfg_off + CFG_REG,
+				    mask, cfg);
 }
 
 static int clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
-- 
2.20.1


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

* [PATCH 2/2] clk: qcom: gcc-qcs404: Add cfg_offset for blsp1_uart3 clock
  2019-01-28 11:53 [PATCH 1/2] clk: qcom: clk-rcg2: Introduce a cfg offset for RCGs Vinod Koul
@ 2019-01-28 11:53 ` Vinod Koul
  2019-01-29 22:42   ` Stephen Boyd
  2019-01-29 22:42 ` [PATCH 1/2] clk: qcom: clk-rcg2: Introduce a cfg offset for RCGs Stephen Boyd
  1 sibling, 1 reply; 5+ messages in thread
From: Vinod Koul @ 2019-01-28 11:53 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: Khasim Syed Mohammed, Bjorn Andersson, Taniya Das, Andy Gross,
	David Brown, linux-arm-msm, linux-clk, Anu Ramanathan, Shawn Guo,
	Vinod Koul

From: Taniya Das <tdas@codeaurora.org>

The CFG/M/N/D registers are at an offset of 0x20 from the CMD register
for blsp1_uart3 clock, so add it.

Signed-off-by: Taniya Das <tdas@codeaurora.org>
Signed-off-by: Anu Ramanathan <anur@codeaurora.org>
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/clk/qcom/gcc-qcs404.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/qcom/gcc-qcs404.c b/drivers/clk/qcom/gcc-qcs404.c
index 64da032bb9ed..493e055299b4 100644
--- a/drivers/clk/qcom/gcc-qcs404.c
+++ b/drivers/clk/qcom/gcc-qcs404.c
@@ -678,6 +678,7 @@ static struct clk_rcg2 blsp1_uart3_apps_clk_src = {
 	.cmd_rcgr = 0x4014,
 	.mnd_width = 16,
 	.hid_width = 5,
+	.cfg_off = 0x20,
 	.parent_map = gcc_parent_map_0,
 	.freq_tbl = ftbl_blsp1_uart0_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
-- 
2.20.1


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

* Re: [PATCH 1/2] clk: qcom: clk-rcg2: Introduce a cfg offset for RCGs
  2019-01-28 11:53 [PATCH 1/2] clk: qcom: clk-rcg2: Introduce a cfg offset for RCGs Vinod Koul
  2019-01-28 11:53 ` [PATCH 2/2] clk: qcom: gcc-qcs404: Add cfg_offset for blsp1_uart3 clock Vinod Koul
@ 2019-01-29 22:42 ` Stephen Boyd
  2019-01-30  4:34   ` Vinod Koul
  1 sibling, 1 reply; 5+ messages in thread
From: Stephen Boyd @ 2019-01-29 22:42 UTC (permalink / raw)
  To: Michael Turquette, Vinod Koul
  Cc: Khasim Syed Mohammed, Bjorn Andersson, Taniya Das, Andy Gross,
	David Brown, linux-arm-msm, linux-clk, Anu Ramanathan, Shawn Guo,
	Vinod Koul

Quoting Vinod Koul (2019-01-28 03:53:58)
> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
> index e5eca8a1abe4..f06783c20688 100644
> --- a/drivers/clk/qcom/clk-rcg.h
> +++ b/drivers/clk/qcom/clk-rcg.h
> @@ -140,6 +140,7 @@ extern const struct clk_ops clk_dyn_rcg_ops;
>   * @parent_map: map from software's parent index to hardware's src_sel field
>   * @freq_tbl: frequency table
>   * @clkr: regmap clock handle
> + * @cfg_off: defines the cfg register offset from the CMD_RCGR
>   *

Please remove this extra line here. Also, shouldn't it say offset from
CMD_RCGR + CFG_REG?


>   */
>  struct clk_rcg2 {
> @@ -150,6 +151,7 @@ struct clk_rcg2 {
>         const struct parent_map *parent_map;
>         const struct freq_tbl   *freq_tbl;
>         struct clk_regmap       clkr;
> +       u8                      cfg_off;
>  };
>  
>  #define to_clk_rcg2(_hw) container_of(to_clk_regmap(_hw), struct clk_rcg2, clkr)
> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> index 6e3bd195d012..106848e3313f 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> @@ -74,7 +74,8 @@ static u8 clk_rcg2_get_parent(struct clk_hw *hw)
>         u32 cfg;
>         int i, ret;
>  
> -       ret = regmap_read(rcg->clkr.regmap, rcg->cmd_rcgr + CFG_REG, &cfg);
> +       ret = regmap_read(rcg->clkr.regmap,
> +                         rcg->cmd_rcgr + rcg->cfg_off + CFG_REG, &cfg);

Maybe we should define CFG_REG as CFG_REG(rcg) and then do the math
there?

#define CFG_REG(rcg) (rcg)->cfg_off + 0x4

>         if (ret)
>                 goto err;
>  
> @@ -263,17 +268,20 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
>         if (rcg->mnd_width && f->n) {
>                 mask = BIT(rcg->mnd_width) - 1;
>                 ret = regmap_update_bits(rcg->clkr.regmap,
> -                               rcg->cmd_rcgr + M_REG, mask, f->m);
> +                                        rcg->cmd_rcgr + rcg->cfg_off + M_REG,
> +                                        mask, f->m);
>                 if (ret)
>                         return ret;
>  
>                 ret = regmap_update_bits(rcg->clkr.regmap,
> -                               rcg->cmd_rcgr + N_REG, mask, ~(f->n - f->m));
> +                                        rcg->cmd_rcgr + rcg->cfg_off + N_REG,
> +                                        mask, ~(f->n - f->m));
>                 if (ret)
>                         return ret;
>  
>                 ret = regmap_update_bits(rcg->clkr.regmap,
> -                               rcg->cmd_rcgr + D_REG, mask, ~f->n);
> +                                        rcg->cmd_rcgr + rcg->cfg_off + D_REG,
> +                                        mask, ~f->n);

Ah the MND registers also move. Wow that's so sad. Do a similar thing
for all these too?

#define D_REG(rcg) (rcg)->cfg_off + 0x8
etc...

All just to make things fit on the same number of lines as before! We
could also throw the rcg->cmd_rcgr part into the register named macros
to make things even shorter. It was mostly OK when it was just adding
the offset to the base, but now we have another offset so I think we can
roll it all into the macro so that we can just read "regmap_read
FOO_REG" and ignore the rest.

>                 if (ret)
>                         return ret;
>         }

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

* Re: [PATCH 2/2] clk: qcom: gcc-qcs404: Add cfg_offset for blsp1_uart3 clock
  2019-01-28 11:53 ` [PATCH 2/2] clk: qcom: gcc-qcs404: Add cfg_offset for blsp1_uart3 clock Vinod Koul
@ 2019-01-29 22:42   ` Stephen Boyd
  0 siblings, 0 replies; 5+ messages in thread
From: Stephen Boyd @ 2019-01-29 22:42 UTC (permalink / raw)
  To: Michael Turquette, Vinod Koul
  Cc: Khasim Syed Mohammed, Bjorn Andersson, Taniya Das, Andy Gross,
	David Brown, linux-arm-msm, linux-clk, Anu Ramanathan, Shawn Guo,
	Vinod Koul

Quoting Vinod Koul (2019-01-28 03:53:59)
> diff --git a/drivers/clk/qcom/gcc-qcs404.c b/drivers/clk/qcom/gcc-qcs404.c
> index 64da032bb9ed..493e055299b4 100644
> --- a/drivers/clk/qcom/gcc-qcs404.c
> +++ b/drivers/clk/qcom/gcc-qcs404.c
> @@ -678,6 +678,7 @@ static struct clk_rcg2 blsp1_uart3_apps_clk_src = {
>         .cmd_rcgr = 0x4014,
>         .mnd_width = 16,
>         .hid_width = 5,
> +       .cfg_off = 0x20,

And it's one single clk! None of the other blsp clks have this problem?

>         .parent_map = gcc_parent_map_0,
>         .freq_tbl = ftbl_blsp1_uart0_apps_clk_src,
>         .clkr.hw.init = &(struct clk_init_data){

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

* Re: [PATCH 1/2] clk: qcom: clk-rcg2: Introduce a cfg offset for RCGs
  2019-01-29 22:42 ` [PATCH 1/2] clk: qcom: clk-rcg2: Introduce a cfg offset for RCGs Stephen Boyd
@ 2019-01-30  4:34   ` Vinod Koul
  0 siblings, 0 replies; 5+ messages in thread
From: Vinod Koul @ 2019-01-30  4:34 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Michael Turquette, Khasim Syed Mohammed, Bjorn Andersson,
	Taniya Das, Andy Gross, David Brown, linux-arm-msm, linux-clk,
	Anu Ramanathan, Shawn Guo

On 29-01-19, 14:42, Stephen Boyd wrote:
> Quoting Vinod Koul (2019-01-28 03:53:58)
> > diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
> > index e5eca8a1abe4..f06783c20688 100644
> > --- a/drivers/clk/qcom/clk-rcg.h
> > +++ b/drivers/clk/qcom/clk-rcg.h
> > @@ -140,6 +140,7 @@ extern const struct clk_ops clk_dyn_rcg_ops;
> >   * @parent_map: map from software's parent index to hardware's src_sel field
> >   * @freq_tbl: frequency table
> >   * @clkr: regmap clock handle
> > + * @cfg_off: defines the cfg register offset from the CMD_RCGR
> >   *
> 
> Please remove this extra line here. Also, shouldn't it say offset from
> CMD_RCGR + CFG_REG?

Sure but I dont like to mix so will send that as a separate patch :)

and will update the comment too.

> 
> 
> >   */
> >  struct clk_rcg2 {
> > @@ -150,6 +151,7 @@ struct clk_rcg2 {
> >         const struct parent_map *parent_map;
> >         const struct freq_tbl   *freq_tbl;
> >         struct clk_regmap       clkr;
> > +       u8                      cfg_off;
> >  };
> >  
> >  #define to_clk_rcg2(_hw) container_of(to_clk_regmap(_hw), struct clk_rcg2, clkr)
> > diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> > index 6e3bd195d012..106848e3313f 100644
> > --- a/drivers/clk/qcom/clk-rcg2.c
> > +++ b/drivers/clk/qcom/clk-rcg2.c
> > @@ -74,7 +74,8 @@ static u8 clk_rcg2_get_parent(struct clk_hw *hw)
> >         u32 cfg;
> >         int i, ret;
> >  
> > -       ret = regmap_read(rcg->clkr.regmap, rcg->cmd_rcgr + CFG_REG, &cfg);
> > +       ret = regmap_read(rcg->clkr.regmap,
> > +                         rcg->cmd_rcgr + rcg->cfg_off + CFG_REG, &cfg);
> 
> Maybe we should define CFG_REG as CFG_REG(rcg) and then do the math
> there?
> 
> #define CFG_REG(rcg) (rcg)->cfg_off + 0x4

Sure that looks better

> 
> >         if (ret)
> >                 goto err;
> >  
> > @@ -263,17 +268,20 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
> >         if (rcg->mnd_width && f->n) {
> >                 mask = BIT(rcg->mnd_width) - 1;
> >                 ret = regmap_update_bits(rcg->clkr.regmap,
> > -                               rcg->cmd_rcgr + M_REG, mask, f->m);
> > +                                        rcg->cmd_rcgr + rcg->cfg_off + M_REG,
> > +                                        mask, f->m);
> >                 if (ret)
> >                         return ret;
> >  
> >                 ret = regmap_update_bits(rcg->clkr.regmap,
> > -                               rcg->cmd_rcgr + N_REG, mask, ~(f->n - f->m));
> > +                                        rcg->cmd_rcgr + rcg->cfg_off + N_REG,
> > +                                        mask, ~(f->n - f->m));
> >                 if (ret)
> >                         return ret;
> >  
> >                 ret = regmap_update_bits(rcg->clkr.regmap,
> > -                               rcg->cmd_rcgr + D_REG, mask, ~f->n);
> > +                                        rcg->cmd_rcgr + rcg->cfg_off + D_REG,
> > +                                        mask, ~f->n);
> 
> Ah the MND registers also move. Wow that's so sad. Do a similar thing
> for all these too?
> 
> #define D_REG(rcg) (rcg)->cfg_off + 0x8
> etc...
> 
> All just to make things fit on the same number of lines as before! We
> could also throw the rcg->cmd_rcgr part into the register named macros
> to make things even shorter. It was mostly OK when it was just adding
> the offset to the base, but now we have another offset so I think we can
> roll it all into the macro so that we can just read "regmap_read
> FOO_REG" and ignore the rest.

Correct that will make it neater, will do so

-- 
~Vinod

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

end of thread, other threads:[~2019-01-30  4:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-28 11:53 [PATCH 1/2] clk: qcom: clk-rcg2: Introduce a cfg offset for RCGs Vinod Koul
2019-01-28 11:53 ` [PATCH 2/2] clk: qcom: gcc-qcs404: Add cfg_offset for blsp1_uart3 clock Vinod Koul
2019-01-29 22:42   ` Stephen Boyd
2019-01-29 22:42 ` [PATCH 1/2] clk: qcom: clk-rcg2: Introduce a cfg offset for RCGs Stephen Boyd
2019-01-30  4:34   ` Vinod Koul

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