Hello Clément,
On Thu, Nov 14, 2019 at 11:47:00PM +0100, Clément Péron wrote:
> On Wed, 13 Nov 2019 at 09:58, Uwe Kleine-König
> <[email protected]> wrote:
> > On Fri, Nov 08, 2019 at 09:45:14AM +0100, Clément Péron wrote:
> > > From: Jernej Skrabec <[email protected]>
> > > + /*
> > > + * Although it would make much more sense to check for bypass in
> > > + * sun4i_pwm_calculate(), value of bypass bit also depends on
> > > "enabled".
> >
> > I don't understand this reasoning. sun4i_pwm_calculate knows about
> > .enabled and also sun4i_pwm->data->has_direct_mod_clk_output. Maybe just
> > add a bool *bypass as parameter and move the logic there?
>
> I asked myself the same question, however the sun4i_pwm_calculate is
> only called when period or duty_cycle is updated not when state is
> toggled between disabled / enabled.
>
> Should we also call it when the state is switched to enabled ?
Given that the check
if ((cstate.period != state->period) ||
(cstate.duty_cycle != state->duty_cycle)) {
is not perfect anyhow (because if I toggle between
pwm_apply_state(pwm, { .period = 50000001, .duty_cycle = 25000000,
.enabled = true });
and
pwm_apply_state(pwm, { .period = 50000000, .duty_cycle = 25000000,
.enabled = true });
the code recalculates every parameter while it (probably) doesn't make a
difference.) And also given that cstate is filled by pwm_get_state which
might change its semantic slightly in the future I would say calculating
the needed parameter always is also cleaner. (But I'm aware this isn't
objective enough to overrule different opinions.) While I'm a fan of
avoid unneeded calculations, I think having a simple driver is more
important.
(And apart from that I don't like lowlevel drivers calling the pwm API
that is designed for consumers.)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/20191115073547.iflhs6vqdsmyjla3%40pengutronix.de.