Paul Cercueil <paul@xxxxxxxxxxxxxxx> writes:
> Hi Aidan,
>
> Le dim. 23 oct. 2022 à 15:56:49 +0100, Aidan MacDonald
> <aidanmacdonald.0x0@xxxxxxxxx> 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@xxxxxxxxx>
>> ---
>> 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
>>
