linux-kernel - Re: [PATCH v2 04/14] pwm: meson: change MISC_CLK_SEL_WIDTH to MISC_CLK_SEL

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]

Date:   Wed, 12 Jun 2019 21:47:07 +0200
From:   Martin Blumenstingl <martin.blumenstingl@...glemail.com>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
Cc:     linux-amlogic@...ts.infradead.org, linux-pwm@...r.kernel.org,
        thierry.reding@...il.com, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org,
        Neil Armstrong <narmstrong@...libre.com>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>, linux-clk@...r.kernel.org
Subject: Re: [PATCH v2 04/14] pwm: meson: change MISC_CLK_SEL_WIDTH to MISC_CLK_SEL_MASK

Hi Uwe,

On Tue, Jun 11, 2019 at 6:33 PM Uwe Kleine-König
<u.kleine-koenig@...gutronix.de> wrote:
[...]
> > @@ -463,7 +463,7 @@ static int meson_pwm_init_channels(struct meson_pwm *meson,
> >
> >               channel->mux.reg = meson->base + REG_MISC_AB;
> >               channel->mux.shift = mux_reg_shifts[i];
> > -             channel->mux.mask = BIT(MISC_CLK_SEL_WIDTH) - 1;
> > +             channel->mux.mask = MISC_CLK_SEL_MASK;
> >               channel->mux.flags = 0;
> >               channel->mux.lock = &meson->lock;
> >               channel->mux.table = NULL;
>
> IMHO clk_mux is ugly here. It could easily just take
>
>         .mask = 3 << mux_reg_shifts[i],
in most cases that would be even nicer to read because it could be expressed as:
  .mask = GENMASK(5, 4)

so I like your idea in general
though I think it should not block this patch

[...]
> Apart from that, I wonder if the pwm-meson driver should better use
> clk_register_mux instead of open coding it. (Though there doesn't seem
> to exists a devm_ variant of it.)
I tried to use clk_register_mux in the past. it works but it's not as
nice to read as the open-coded variant because it takes 10 parameters.
I find it easier to read 13 separate lines compared to reading a
function call with 10 parameters


Martin