On Fri 18 Feb 08:10 PST 2022, Doug Anderson wrote:
> Hi,
>
> On Tue, Feb 15, 2022 at 8:54 PM Bjorn Andersson
> <bjorn.andersson@xxxxxxxxxx> wrote:
> >
> > +static int lpg_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> > + const struct pwm_state *state)
> > +{
> > + struct lpg *lpg = container_of(chip, struct lpg, pwm);
> > + struct lpg_channel *chan = &lpg->channels[pwm->hwpwm];
> > + int ret;
> > +
> > + if (state->polarity != PWM_POLARITY_NORMAL)
> > + return -EINVAL;
> > +
> > + mutex_lock(&lpg->lock);
> > +
> > + if (state->enabled) {
> > + ret = lpg_calc_freq(chan, state->period);
> > + if (ret < 0)
> > + goto out_unlock;
> > +
> > + lpg_calc_duty(chan, state->duty_cycle);
> > + }
> > + chan->enabled = state->enabled;
> > +
> > + lpg_apply(chan);
> > +
> > + triled_set(lpg, chan->triled_mask, chan->enabled ? chan->triled_mask : 0);
> > +
> > +out_unlock:
> > + mutex_unlock(&lpg->lock);
> > +
> > + return ret;
> > +}
>
> My compiler (correctly) yelled that `ret` is returned uninitialized if
> `state->enabled` is false.
You're absolutely correct. I am however not able to figure out how to
get my compiler (aarc64 gcc 11.2.0) to give me a warning about this.
If anyone have any suggestions I'd be very happy.
> I initialized `ret` to 0 and the problem
> went away. I assume that the patch will need to spin to fix that
> unless everything else looks great and a maintainer wants to fix that
> when applying.
>
> With that fix, I was able to use Bjorn's patch along with Satya's
> patches adding pm8350c support (removing the now defunct
> "pwm_9bit_mask" property) to make the PWM on my board work. Thus, once
> the error my compiler complained about is fixed I'm happy with my
> `Tested-by` being added.
>
Thanks! I will initialize ret and send out v13 including your T-b.
Regards,
Bjorn
> For now I haven't actually reviewed the code here, but if folks feel
> like it needs an extra pair of eyes then please yell and I'll find
> some time to do it.
>
> -Doug
