lkml.org 
[lkml]   [2019]   [Jun]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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@pengutronix.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

\
 
 \ /
  Last update: 2019-06-12 21:47    [W:0.010 / U:63.108 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site