1. Patchew
  2. linux
  3. [PATCH v2 0/4] clk: spacemit: fix i2s clock
  4. View patch

[PATCH v2 2/4] clk: spacemit: introduce pre-div for ddn clock

Troy Mitchell posted 4 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH v2 2/4] clk: spacemit: introduce pre-div for ddn clock
Posted by Troy Mitchell 1 month, 3 weeks ago 
The original DDN operations applied an implicit divide-by-2, which should
not be a default behavior.

This patch removes that assumption, letting each clock define its
actual behavior explicitly.

Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
 drivers/clk/spacemit/ccu_ddn.c | 12 ++++++------
 drivers/clk/spacemit/ccu_ddn.h |  6 ++++--
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/spacemit/ccu_ddn.c b/drivers/clk/spacemit/ccu_ddn.c
index be311b045698e95a688a35858a8ac1bcfbffd2c7..06d86748182bd1959cdab5c18d0a882ee25dcade 100644
--- a/drivers/clk/spacemit/ccu_ddn.c
+++ b/drivers/clk/spacemit/ccu_ddn.c
@@ -22,21 +22,21 @@
 
 #include "ccu_ddn.h"
 
-static unsigned long ccu_ddn_calc_rate(unsigned long prate,
-				       unsigned long num, unsigned long den)
+static unsigned long ccu_ddn_calc_rate(unsigned long prate, unsigned long num,
+				       unsigned long den, unsigned int pre_div)
 {
-	return prate * den / 2 / num;
+	return prate * den / pre_div / num;
 }
 
 static unsigned long ccu_ddn_calc_best_rate(struct ccu_ddn *ddn,
 					    unsigned long rate, unsigned long prate,
 					    unsigned long *num, unsigned long *den)
 {
-	rational_best_approximation(rate, prate / 2,
+	rational_best_approximation(rate, prate / ddn->pre_div,
 				    ddn->den_mask >> ddn->den_shift,
 				    ddn->num_mask >> ddn->num_shift,
 				    den, num);
-	return ccu_ddn_calc_rate(prate, *num, *den);
+	return ccu_ddn_calc_rate(prate, *num, *den, ddn->pre_div);
 }
 
 static long ccu_ddn_round_rate(struct clk_hw *hw, unsigned long rate,
@@ -58,7 +58,7 @@ static unsigned long ccu_ddn_recalc_rate(struct clk_hw *hw, unsigned long prate)
 	num = (val & ddn->num_mask) >> ddn->num_shift;
 	den = (val & ddn->den_mask) >> ddn->den_shift;
 
-	return ccu_ddn_calc_rate(prate, num, den);
+	return ccu_ddn_calc_rate(prate, num, den, ddn->pre_div);
 }
 
 static int ccu_ddn_set_rate(struct clk_hw *hw, unsigned long rate,
diff --git a/drivers/clk/spacemit/ccu_ddn.h b/drivers/clk/spacemit/ccu_ddn.h
index a52fabe77d62eba16426867a9c13481e72f025c0..4838414a8e8dc04af49d3b8d39280efedbd75616 100644
--- a/drivers/clk/spacemit/ccu_ddn.h
+++ b/drivers/clk/spacemit/ccu_ddn.h
@@ -18,13 +18,14 @@ struct ccu_ddn {
 	unsigned int num_shift;
 	unsigned int den_mask;
 	unsigned int den_shift;
+	unsigned int pre_div;
 };
 
 #define CCU_DDN_INIT(_name, _parent, _flags) \
 	CLK_HW_INIT_HW(#_name, &_parent.common.hw, &spacemit_ccu_ddn_ops, _flags)
 
 #define CCU_DDN_DEFINE(_name, _parent, _reg_ctrl, _num_shift, _num_width,	\
-		       _den_shift, _den_width, _flags)				\
+		       _den_shift, _den_width, _pre_div, _flags)		\
 static struct ccu_ddn _name = {							\
 	.common = {								\
 		.reg_ctrl	= _reg_ctrl,					\
@@ -33,7 +34,8 @@ static struct ccu_ddn _name = {							\
 	.num_mask	= GENMASK(_num_shift + _num_width - 1, _num_shift),	\
 	.num_shift	= _num_shift,						\
 	.den_mask	= GENMASK(_den_shift + _den_width - 1, _den_shift),	\
-	.den_shift	= _den_shift,					\
+	.den_shift	= _den_shift,						\
+	.pre_div	= _pre_div,						\
 }
 
 static inline struct ccu_ddn *hw_to_ccu_ddn(struct clk_hw *hw)

-- 
2.50.1
Re: [PATCH v2 2/4] clk: spacemit: introduce pre-div for ddn clock
Posted by Haylen Chu 1 month, 2 weeks ago 
On Mon, Aug 11, 2025 at 10:04:28PM +0800, Troy Mitchell wrote:
> The original DDN operations applied an implicit divide-by-2, which should
> not be a default behavior.
> 
> This patch removes that assumption, letting each clock define its
> actual behavior explicitly.
> 
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> ---
>  drivers/clk/spacemit/ccu_ddn.c | 12 ++++++------
>  drivers/clk/spacemit/ccu_ddn.h |  6 ++++--
>  2 files changed, 10 insertions(+), 8 deletions(-)

The code change looks good to me, but

> diff --git a/drivers/clk/spacemit/ccu_ddn.h b/drivers/clk/spacemit/ccu_ddn.h
> index a52fabe77d62eba16426867a9c13481e72f025c0..4838414a8e8dc04af49d3b8d39280efedbd75616 100644
> --- a/drivers/clk/spacemit/ccu_ddn.h
> +++ b/drivers/clk/spacemit/ccu_ddn.h
> @@ -18,13 +18,14 @@ struct ccu_ddn {
>  	unsigned int num_shift;
>  	unsigned int den_mask;
>  	unsigned int den_shift;
> +	unsigned int pre_div;
>  };
>  
>  #define CCU_DDN_INIT(_name, _parent, _flags) \
>  	CLK_HW_INIT_HW(#_name, &_parent.common.hw, &spacemit_ccu_ddn_ops, _flags)
>  
>  #define CCU_DDN_DEFINE(_name, _parent, _reg_ctrl, _num_shift, _num_width,	\
> -		       _den_shift, _den_width, _flags)				\
> +		       _den_shift, _den_width, _pre_div, _flags)		\

You changed the definition of CCU_DDN_DEFINE without adjusting consumers
of this macro. If I'm correct, this creates a build failure.

>  static struct ccu_ddn _name = {							\
>  	.common = {								\
>  		.reg_ctrl	= _reg_ctrl,					\
> @@ -33,7 +34,8 @@ static struct ccu_ddn _name = {							\
>  	.num_mask	= GENMASK(_num_shift + _num_width - 1, _num_shift),	\
>  	.num_shift	= _num_shift,						\
>  	.den_mask	= GENMASK(_den_shift + _den_width - 1, _den_shift),	\
> -	.den_shift	= _den_shift,					\
> +	.den_shift	= _den_shift,						\
> +	.pre_div	= _pre_div,						\
>  }
>  
>  static inline struct ccu_ddn *hw_to_ccu_ddn(struct clk_hw *hw)
> 
> -- 
> 2.50.1
> 

Best regards,
Haylen Chu
Re: [PATCH v2 2/4] clk: spacemit: introduce pre-div for ddn clock
Posted by Troy Mitchell 1 month, 2 weeks ago 
On Mon, Aug 18, 2025 at 02:04:39AM +0000, Haylen Chu wrote:
> On Mon, Aug 11, 2025 at 10:04:28PM +0800, Troy Mitchell wrote:
> > The original DDN operations applied an implicit divide-by-2, which should
> > not be a default behavior.
> > 
> > This patch removes that assumption, letting each clock define its
> > actual behavior explicitly.
> > 
> > Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> > ---
> >  drivers/clk/spacemit/ccu_ddn.c | 12 ++++++------
> >  drivers/clk/spacemit/ccu_ddn.h |  6 ++++--
> >  2 files changed, 10 insertions(+), 8 deletions(-)
Hi Haylen,
Thanks for ur review.

> 
> The code change looks good to me, but
> 
> > diff --git a/drivers/clk/spacemit/ccu_ddn.h b/drivers/clk/spacemit/ccu_ddn.h
> > index a52fabe77d62eba16426867a9c13481e72f025c0..4838414a8e8dc04af49d3b8d39280efedbd75616 100644
> > --- a/drivers/clk/spacemit/ccu_ddn.h
> > +++ b/drivers/clk/spacemit/ccu_ddn.h
> > @@ -18,13 +18,14 @@ struct ccu_ddn {
> >  	unsigned int num_shift;
> >  	unsigned int den_mask;
> >  	unsigned int den_shift;
> > +	unsigned int pre_div;
> >  };
> >  
> >  #define CCU_DDN_INIT(_name, _parent, _flags) \
> >  	CLK_HW_INIT_HW(#_name, &_parent.common.hw, &spacemit_ccu_ddn_ops, _flags)
> >  
> >  #define CCU_DDN_DEFINE(_name, _parent, _reg_ctrl, _num_shift, _num_width,	\
> > -		       _den_shift, _den_width, _flags)				\
> > +		       _den_shift, _den_width, _pre_div, _flags)		\
> 
> You changed the definition of CCU_DDN_DEFINE without adjusting consumers
> of this macro. If I'm correct, this creates a build failure.
So I need to adjust consumers in same patch?

> 
> >  static struct ccu_ddn _name = {							\
> >  	.common = {								\
> >  		.reg_ctrl	= _reg_ctrl,					\
> > @@ -33,7 +34,8 @@ static struct ccu_ddn _name = {							\
> >  	.num_mask	= GENMASK(_num_shift + _num_width - 1, _num_shift),	\
> >  	.num_shift	= _num_shift,						\
> >  	.den_mask	= GENMASK(_den_shift + _den_width - 1, _den_shift),	\
> > -	.den_shift	= _den_shift,					\
> > +	.den_shift	= _den_shift,						\
> > +	.pre_div	= _pre_div,						\
> >  }
> >  
> >  static inline struct ccu_ddn *hw_to_ccu_ddn(struct clk_hw *hw)
> > 
> > -- 
> > 2.50.1
> > 
> 
> Best regards,
> Haylen Chu
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.