Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <ED361B6C-0BE2-4FDD-AF29-497E14A183A6@protonmail.com>
References: <20220118191839.64086-1-povik+lin@protonmail.com> <20220118191839.64086-3-povik+lin@protonmail.com> <20220120053810.71C17C340E0@smtp.kernel.org> <ED361B6C-0BE2-4FDD-AF29-497E14A183A6@protonmail.com>
Subject: Re: [PATCH v2 2/3] clk: clk-apple-nco: Add driver for Apple NCO
From:   Stephen Boyd <sboyd@kernel.org>
Cc:     Martin =?utf-8?q?Povi=C5=A1er?= <povik+lin@protonmail.com>,
        Michael Turquette <mturquette@baylibre.com>,
        Rob Herring <robh+dt@kernel.org>,
        Hector Martin <marcan@marcan.st>,
        Sven Peter <sven@svenpeter.dev>, alyssa@rosenzweig.io,
        linux-clk <linux-clk@vger.kernel.org>,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        Mark Kettenis <kettenis@openbsd.org>
To:     Martin =?utf-8?q?Povi=C5=A1er?= <povik@protonmail.com>
Date:   Thu, 20 Jan 2022 12:59:05 -0800
User-Agent: alot/0.10
Message-Id: <20220120205907.4E365C340E0@smtp.kernel.org>
Precedence: bulk

Quoting Martin Povi=C5=A1er (2022-01-20 04:11:34)
>=20
> > On 20. 1. 2022, at 6:38, Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Martin Povi=C5=A1er (2022-01-18 11:21:10)
> >> +
> >> +static int nco_set_rate(struct clk_hw *hw, unsigned long rate,
> >> +                               unsigned long parent_rate)
> >> +{
> >> +       struct nco_channel *chan =3D to_nco_channel(hw);
> >> +       unsigned long flags;
> >> +       u32 div;
> >> +       s32 inc1, inc2;
> >> +       bool was_enabled;
> >> +
> >> +       div =3D 2 * parent_rate / rate;
> >> +       inc1 =3D 2 * parent_rate - div * rate;
> >> +       inc2 =3D -((s32) (rate - inc1));
> >
> > Is the cast necessary?
>=20
> Answering that prompted me to get back to reading some C specification an=
d now
> I am confident in moving away from signed types here and in nco_recalc_ra=
te
> altogether.

Great! Operating with only unsigned types makes this easier to
understand.

> >> +       struct nco_tables *tbl;
> >> +       unsigned int nchannels;
> >> +       int ret, i;
> >> +
> >> +       regs =3D devm_platform_get_and_ioremap_resource(pdev, 0, &regs=
_res);
> >> +       if (IS_ERR(regs))
> >> +               return PTR_ERR(regs);
> >> +
> >> +       if (resource_size(regs_res) < NCO_CHANNEL_REGSIZE)
> >> +               return -EINVAL;
> >> +       nchannels =3D (resource_size(regs_res) - NCO_CHANNEL_REGSIZE)
> >> +                       / NCO_CHANNEL_STRIDE + 1;
> >
> > Is this some sort of DIV_ROUND_UP()?
>=20
> Almost. I will shop around for a macro replacement.

Alright.