diff mbox series

[v1,1/5] clk: ingenic: Make PLL clock "od" field optional

Message ID 20221023145653.177234-2-aidanmacdonald.0x0@gmail.com
State New
Headers
Series Add support for X1000 audio clocks |

Commit Message

Aidan MacDonald Oct. 23, 2022, 2:56 p.m. UTC 
  Add support for defining PLL clocks with od_bits = 0, meaning that
OD is fixed to 1 and there is no OD field in the register.

Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
 drivers/clk/ingenic/cgu.c | 28 +++++++++++++++++++---------
 drivers/clk/ingenic/cgu.h |  3 ++-
 2 files changed, 21 insertions(+), 10 deletions(-)

Comments

Paul Cercueil Oct. 23, 2022, 3:20 p.m. UTC | #1 
Hi Aidan,

Le dim. 23 oct. 2022 à 15:56:49 +0100, Aidan MacDonald 
<aidanmacdonald.0x0@gmail.com> a écrit :
> Add support for defining PLL clocks with od_bits = 0, meaning that
> OD is fixed to 1 and there is no OD field in the register.
> 
> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
> ---
>  drivers/clk/ingenic/cgu.c | 28 +++++++++++++++++++---------
>  drivers/clk/ingenic/cgu.h |  3 ++-
>  2 files changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
> index 861c50d6cb24..7dc2e2567d53 100644
> --- a/drivers/clk/ingenic/cgu.c
> +++ b/drivers/clk/ingenic/cgu.c
> @@ -96,8 +96,11 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, 
> unsigned long parent_rate)
>  	m += pll_info->m_offset;
>  	n = (ctl >> pll_info->n_shift) & GENMASK(pll_info->n_bits - 1, 0);
>  	n += pll_info->n_offset;
> -	od_enc = ctl >> pll_info->od_shift;
> -	od_enc &= GENMASK(pll_info->od_bits - 1, 0);
> +
> +	if (pll_info->od_bits > 0) {
> +		od_enc = ctl >> pll_info->od_shift;
> +		od_enc &= GENMASK(pll_info->od_bits - 1, 0);
> +	}
> 
>  	if (pll_info->bypass_bit >= 0) {
>  		ctl = readl(cgu->base + pll_info->bypass_reg);
> @@ -108,12 +111,17 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, 
> unsigned long parent_rate)
>  			return parent_rate;
>  	}
> 
> -	for (od = 0; od < pll_info->od_max; od++) {
> -		if (pll_info->od_encoding[od] == od_enc)
> -			break;
> +	if (pll_info->od_bits > 0) {
> +		for (od = 0; od < pll_info->od_max; od++) {
> +			if (pll_info->od_encoding[od] == od_enc)
> +				break;
> +		}
> +		BUG_ON(od == pll_info->od_max);
> +		od++;
> +	} else {
> +		/* OD is fixed to 1 if no OD field is present. */
> +		od = 1;
>  	}
> -	BUG_ON(od == pll_info->od_max);
> -	od++;

I think if pll_info->od_max is 0 you get the same result without 
modifying this code. You just need to modify the BUG_ON() to only 
trigger if pll_info->od_max > 0.

Cheers,
-Paul

> 
>  	return div_u64((u64)parent_rate * m * pll_info->rate_multiplier,
>  		n * od);
> @@ -215,8 +223,10 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned 
> long req_rate,
>  	ctl &= ~(GENMASK(pll_info->n_bits - 1, 0) << pll_info->n_shift);
>  	ctl |= (n - pll_info->n_offset) << pll_info->n_shift;
> 
> -	ctl &= ~(GENMASK(pll_info->od_bits - 1, 0) << pll_info->od_shift);
> -	ctl |= pll_info->od_encoding[od - 1] << pll_info->od_shift;
> +	if (pll_info->od_bits > 0) {
> +		ctl &= ~(GENMASK(pll_info->od_bits - 1, 0) << pll_info->od_shift);
> +		ctl |= pll_info->od_encoding[od - 1] << pll_info->od_shift;
> +	}
> 
>  	writel(ctl, cgu->base + pll_info->reg);
> 
> diff --git a/drivers/clk/ingenic/cgu.h b/drivers/clk/ingenic/cgu.h
> index 147b7df0d657..567142b584bb 100644
> --- a/drivers/clk/ingenic/cgu.h
> +++ b/drivers/clk/ingenic/cgu.h
> @@ -33,7 +33,8 @@
>   * @od_shift: the number of bits to shift the post-VCO divider value 
> by (ie.
>   *            the index of the lowest bit of the post-VCO divider 
> value in
>   *            the PLL's control register)
> - * @od_bits: the size of the post-VCO divider field in bits
> + * @od_bits: the size of the post-VCO divider field in bits, or 0 if 
> no
> + *	     OD field exists (then the OD is fixed to 1)
>   * @od_max: the maximum post-VCO divider value
>   * @od_encoding: a pointer to an array mapping post-VCO divider 
> values to
>   *               their encoded values in the PLL control register, 
> or -1 for
> --
> 2.38.1
>
Aidan MacDonald Oct. 23, 2022, 10:45 p.m. UTC | #2 
Paul Cercueil <paul@crapouillou.net> writes:

> Hi Aidan,
>
> Le dim. 23 oct. 2022 à 15:56:49 +0100, Aidan MacDonald
> <aidanmacdonald.0x0@gmail.com> a écrit :
>> Add support for defining PLL clocks with od_bits = 0, meaning that
>> OD is fixed to 1 and there is no OD field in the register.
>> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
>> ---
>>  drivers/clk/ingenic/cgu.c | 28 +++++++++++++++++++---------
>>  drivers/clk/ingenic/cgu.h |  3 ++-
>>  2 files changed, 21 insertions(+), 10 deletions(-)
>> diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
>> index 861c50d6cb24..7dc2e2567d53 100644
>> --- a/drivers/clk/ingenic/cgu.c
>> +++ b/drivers/clk/ingenic/cgu.c
>> @@ -96,8 +96,11 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned long
>> parent_rate)
>>  	m += pll_info->m_offset;
>>  	n = (ctl >> pll_info->n_shift) & GENMASK(pll_info->n_bits - 1, 0);
>>  	n += pll_info->n_offset;
>> -	od_enc = ctl >> pll_info->od_shift;
>> -	od_enc &= GENMASK(pll_info->od_bits - 1, 0);
>> +
>> +	if (pll_info->od_bits > 0) {
>> +		od_enc = ctl >> pll_info->od_shift;
>> +		od_enc &= GENMASK(pll_info->od_bits - 1, 0);
>> +	}
>>  	if (pll_info->bypass_bit >= 0) {
>>  		ctl = readl(cgu->base + pll_info->bypass_reg);
>> @@ -108,12 +111,17 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned
>> long parent_rate)
>>  			return parent_rate;
>>  	}
>> -	for (od = 0; od < pll_info->od_max; od++) {
>> -		if (pll_info->od_encoding[od] == od_enc)
>> -			break;
>> +	if (pll_info->od_bits > 0) {
>> +		for (od = 0; od < pll_info->od_max; od++) {
>> +			if (pll_info->od_encoding[od] == od_enc)
>> +				break;
>> +		}
>> +		BUG_ON(od == pll_info->od_max);
>> +		od++;
>> +	} else {
>> +		/* OD is fixed to 1 if no OD field is present. */
>> +		od = 1;
>>  	}
>> -	BUG_ON(od == pll_info->od_max);
>> -	od++;
>
> I think if pll_info->od_max is 0 you get the same result without modifying this
> code. You just need to modify the BUG_ON() to only trigger if pll_info->od_max > 0.
>
> Cheers,
> -Paul
>

Yeah, you're right, that's simpler. Thanks,
Aidan

>>  	return div_u64((u64)parent_rate * m * pll_info->rate_multiplier,
>>  		n * od);
>> @@ -215,8 +223,10 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned long
>> req_rate,
>>  	ctl &= ~(GENMASK(pll_info->n_bits - 1, 0) << pll_info->n_shift);
>>  	ctl |= (n - pll_info->n_offset) << pll_info->n_shift;
>> -	ctl &= ~(GENMASK(pll_info->od_bits - 1, 0) << pll_info->od_shift);
>> -	ctl |= pll_info->od_encoding[od - 1] << pll_info->od_shift;
>> +	if (pll_info->od_bits > 0) {
>> +		ctl &= ~(GENMASK(pll_info->od_bits - 1, 0) << pll_info->od_shift);
>> +		ctl |= pll_info->od_encoding[od - 1] << pll_info->od_shift;
>> +	}
>>  	writel(ctl, cgu->base + pll_info->reg);
>> diff --git a/drivers/clk/ingenic/cgu.h b/drivers/clk/ingenic/cgu.h
>> index 147b7df0d657..567142b584bb 100644
>> --- a/drivers/clk/ingenic/cgu.h
>> +++ b/drivers/clk/ingenic/cgu.h
>> @@ -33,7 +33,8 @@
>>   * @od_shift: the number of bits to shift the post-VCO divider value by (ie.
>>   *            the index of the lowest bit of the post-VCO divider value in
>>   *            the PLL's control register)
>> - * @od_bits: the size of the post-VCO divider field in bits
>> + * @od_bits: the size of the post-VCO divider field in bits, or 0 if no
>> + *	     OD field exists (then the OD is fixed to 1)
>>   * @od_max: the maximum post-VCO divider value
>>   * @od_encoding: a pointer to an array mapping post-VCO divider values to
>>   *               their encoded values in the PLL control register, or -1 for
>> --
>> 2.38.1
>>
diff mbox series

Patch

diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
index 861c50d6cb24..7dc2e2567d53 100644
--- a/drivers/clk/ingenic/cgu.c
+++ b/drivers/clk/ingenic/cgu.c
@@ -96,8 +96,11 @@  ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 	m += pll_info->m_offset;
 	n = (ctl >> pll_info->n_shift) & GENMASK(pll_info->n_bits - 1, 0);
 	n += pll_info->n_offset;
-	od_enc = ctl >> pll_info->od_shift;
-	od_enc &= GENMASK(pll_info->od_bits - 1, 0);
+
+	if (pll_info->od_bits > 0) {
+		od_enc = ctl >> pll_info->od_shift;
+		od_enc &= GENMASK(pll_info->od_bits - 1, 0);
+	}
 
 	if (pll_info->bypass_bit >= 0) {
 		ctl = readl(cgu->base + pll_info->bypass_reg);
@@ -108,12 +111,17 @@  ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 			return parent_rate;
 	}
 
-	for (od = 0; od < pll_info->od_max; od++) {
-		if (pll_info->od_encoding[od] == od_enc)
-			break;
+	if (pll_info->od_bits > 0) {
+		for (od = 0; od < pll_info->od_max; od++) {
+			if (pll_info->od_encoding[od] == od_enc)
+				break;
+		}
+		BUG_ON(od == pll_info->od_max);
+		od++;
+	} else {
+		/* OD is fixed to 1 if no OD field is present. */
+		od = 1;
 	}
-	BUG_ON(od == pll_info->od_max);
-	od++;
 
 	return div_u64((u64)parent_rate * m * pll_info->rate_multiplier,
 		n * od);
@@ -215,8 +223,10 @@  ingenic_pll_set_rate(struct clk_hw *hw, unsigned long req_rate,
 	ctl &= ~(GENMASK(pll_info->n_bits - 1, 0) << pll_info->n_shift);
 	ctl |= (n - pll_info->n_offset) << pll_info->n_shift;
 
-	ctl &= ~(GENMASK(pll_info->od_bits - 1, 0) << pll_info->od_shift);
-	ctl |= pll_info->od_encoding[od - 1] << pll_info->od_shift;
+	if (pll_info->od_bits > 0) {
+		ctl &= ~(GENMASK(pll_info->od_bits - 1, 0) << pll_info->od_shift);
+		ctl |= pll_info->od_encoding[od - 1] << pll_info->od_shift;
+	}
 
 	writel(ctl, cgu->base + pll_info->reg);
 
diff --git a/drivers/clk/ingenic/cgu.h b/drivers/clk/ingenic/cgu.h
index 147b7df0d657..567142b584bb 100644
--- a/drivers/clk/ingenic/cgu.h
+++ b/drivers/clk/ingenic/cgu.h
@@ -33,7 +33,8 @@ 
  * @od_shift: the number of bits to shift the post-VCO divider value by (ie.
  *            the index of the lowest bit of the post-VCO divider value in
  *            the PLL's control register)
- * @od_bits: the size of the post-VCO divider field in bits
+ * @od_bits: the size of the post-VCO divider field in bits, or 0 if no
+ *	     OD field exists (then the OD is fixed to 1)
  * @od_max: the maximum post-VCO divider value
  * @od_encoding: a pointer to an array mapping post-VCO divider values to
  *               their encoded values in the PLL control register, or -1 for