Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <sWPMeZZ7NP5UXWBsE8t41AGkBkpYIgpG@localhost>
References: <20221026194345.243007-1-aidanmacdonald.0x0@gmail.com> <20221026194345.243007-2-aidanmacdonald.0x0@gmail.com> <0GWEKR.T961XCYIYOL52@crapouillou.net> <sWPMeZZ7NP5UXWBsE8t41AGkBkpYIgpG@localhost>
Subject: Re: [PATCH v2 1/6] clk: ingenic: Make PLL clock "od" field optional
From:   Stephen Boyd <sboyd@kernel.org>
Cc:     mturquette@baylibre.com, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, zhouyu@wanyeetech.com,
        linux-mips@vger.kernel.org, linux-clk@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
To:     Aidan MacDonald <aidanmacdonald.0x0@gmail.com>,
        Paul Cercueil <paul@crapouillou.net>
Date:   Thu, 27 Oct 2022 14:51:02 -0700
User-Agent: alot/0.10
Message-Id: <20221027215104.E8CB4C433D6@smtp.kernel.org>
Precedence: bulk

Quoting Aidan MacDonald (2022-10-27 14:40:02)
>=20
> Paul Cercueil <paul@crapouillou.net> writes:
>=20
> > Hi Aidan,
> >
> > Le mer. 26 oct. 2022 =C3=A0 20:43:40 +0100, Aidan MacDonald
> > <aidanmacdonald.0x0@gmail.com> a =C3=A9crit :
> >> Add support for defining PLL clocks with od_bits =3D 0, meaning that
> >> OD is fixed to 1 and there is no OD field in the register. In this
> >> case od_max must also be 0, which is enforced with BUG_ON().
> >> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
> >> ---
> >> v1 -> v2: Simplify od lookup in ingenic_pll_recalc_rate() and
> >>           enforce od_max =3D=3D 0 when od_bits is zero.
> >>  drivers/clk/ingenic/cgu.c | 21 +++++++++++++++------
> >>  drivers/clk/ingenic/cgu.h |  3 ++-
> >>  2 files changed, 17 insertions(+), 7 deletions(-)
> >> diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
> >> index 861c50d6cb24..3481129114b1 100644
> >> --- a/drivers/clk/ingenic/cgu.c
> >> +++ b/drivers/clk/ingenic/cgu.c
> >> @@ -83,7 +83,7 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned =
long
> >> parent_rate)
> >>      const struct ingenic_cgu_clk_info *clk_info =3D to_clk_info(ingen=
ic_clk);
> >>      struct ingenic_cgu *cgu =3D ingenic_clk->cgu;
> >>      const struct ingenic_cgu_pll_info *pll_info;
> >> -    unsigned m, n, od_enc, od;
> >> +    unsigned m, n, od, od_enc =3D 0;
> >>      bool bypass;
> >>      u32 ctl;
> >> @@ -96,8 +96,11 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned=
 long
> >> parent_rate)
> >>      m +=3D pll_info->m_offset;
> >>      n =3D (ctl >> pll_info->n_shift) & GENMASK(pll_info->n_bits - 1, =
0);
> >>      n +=3D pll_info->n_offset;
> >> -    od_enc =3D ctl >> pll_info->od_shift;
> >> -    od_enc &=3D GENMASK(pll_info->od_bits - 1, 0);
> >> +
> >> +    if (pll_info->od_bits > 0) {
> >> +            od_enc =3D ctl >> pll_info->od_shift;
> >> +            od_enc &=3D GENMASK(pll_info->od_bits - 1, 0);
> >> +    }
> >>      if (pll_info->bypass_bit >=3D 0) {
> >>              ctl =3D readl(cgu->base + pll_info->bypass_reg);
> >> @@ -112,7 +115,11 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, unsign=
ed long
> >> parent_rate)
> >>              if (pll_info->od_encoding[od] =3D=3D od_enc)
> >>                      break;
> >>      }
> >
> > I'd add a space there.
> >
> > With that:
> > Reviewed-by: Paul Cercueil <paul@crapouillou.net>
> >
>=20
> Already done; the space is there in my outbox and on lore.kernel.org.
> I think you might've accidentally removed it. Stephen's already
> applied the series anyway, so...
>=20

I fixed the whitespace.