| Message ID | 20221023145653.177234-2-aidanmacdonald.0x0@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for X1000 audio clocks | expand |
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 >
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 --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
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(-)