linux-kernel - Re: [PATCH v2 2/3] clk: clk-apple-nco: Add driver for Apple NCO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20220120205907.4E365C340E0@smtp.kernel.org>
Date:   Thu, 20 Jan 2022 12:59:05 -0800
From:   Stephen Boyd <sboyd@...nel.org>
To:     Martin Povišer <povik@...tonmail.com>
Cc:     Martin Povišer <povik+lin@...tonmail.com>,
        Michael Turquette <mturquette@...libre.com>,
        Rob Herring <robh+dt@...nel.org>,
        Hector Martin <marcan@...can.st>,
        Sven Peter <sven@...npeter.dev>, alyssa@...enzweig.io,
        linux-clk <linux-clk@...r.kernel.org>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Mark Kettenis <kettenis@...nbsd.org>
Subject: Re: [PATCH v2 2/3] clk: clk-apple-nco: Add driver for Apple NCO

Quoting Martin Povišer (2022-01-20 04:11:34)
> 
> > On 20. 1. 2022, at 6:38, Stephen Boyd <sboyd@...nel.org> wrote:
> >
> > Quoting Martin Povišer (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 = to_nco_channel(hw);
> >> +       unsigned long flags;
> >> +       u32 div;
> >> +       s32 inc1, inc2;
> >> +       bool was_enabled;
> >> +
> >> +       div = 2 * parent_rate / rate;
> >> +       inc1 = 2 * parent_rate - div * rate;
> >> +       inc2 = -((s32) (rate - inc1));
> >
> > Is the cast necessary?
> 
> Answering that prompted me to get back to reading some C specification and now
> I am confident in moving away from signed types here and in nco_recalc_rate
> altogether.

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

> >> +       struct nco_tables *tbl;
> >> +       unsigned int nchannels;
> >> +       int ret, i;
> >> +
> >> +       regs = 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 = (resource_size(regs_res) - NCO_CHANNEL_REGSIZE)
> >> +                       / NCO_CHANNEL_STRIDE + 1;
> >
> > Is this some sort of DIV_ROUND_UP()?
> 
> Almost. I will shop around for a macro replacement.

Alright.