From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: "thierry.reding@gmail.com" <thierry.reding@gmail.com>,
"linux-pwm@vger.kernel.org" <linux-pwm@vger.kernel.org>,
"linux-renesas-soc@vger.kernel.org"
<linux-renesas-soc@vger.kernel.org>
Subject: RE: [PATCH v2 1/4] pwm: rcar: Add support "atomic" API
Date: Tue, 8 Jan 2019 08:11:41 +0000 [thread overview]
Message-ID: <TY2PR01MB48122303FCD3B880701F101DD88A0@TY2PR01MB4812.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <20190108074331.lqwtex2pyumilfyn@pengutronix.de>
Hello Uwe,
Thank you for the review!
> From: Uwe Kleine-Konig, Sent: Tuesday, January 8, 2019 4:44 PM
<snip>
> > +static int rcar_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> > + struct pwm_state *state)
> > +{
> > + struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip);
> > + struct pwm_state cur_state;
> > + int div, ret;
> > +
> > + /* This HW doesn't support changing polarity */
> > + pwm_get_state(pwm, &cur_state);
> > + if (state->polarity != cur_state.polarity)
> > + return -ENOTSUPP;
>
> Does the driver only support normal polarity or only inversed polarity?
> If so checking against that would be more clear here.
This driver only supports normal polarity. So, I'll change the code as the following:
/* This HW/driver only support normal polarity */
pwm_get_state(pwm, &cur_state);
if (state->polarity != PWM_POLARITY_NORMAL)
return -ENOTSUPP;
> > + div = rcar_pwm_get_clock_division(rp, state->period);
> > + if (div < 0)
> > + return div;
> > +
> > + rcar_pwm_update(rp, RCAR_PWMCR_SYNC, RCAR_PWMCR_SYNC, RCAR_PWMCR);
> > +
> > + ret = rcar_pwm_set_counter(rp, div, state->duty_cycle, state->period);
> > + if (!ret)
> > + rcar_pwm_set_clock_control(rp, div);
> > +
> > + /* The SYNC should be set to 0 even if rcar_pwm_set_counter failed */
> > + rcar_pwm_update(rp, RCAR_PWMCR_SYNC, 0, RCAR_PWMCR);
> > +
> > + if (!ret && state->enabled)
> > + ret = rcar_pwm_enable(chip, pwm);
> > +
> > + if (!state->enabled) {
> > + rcar_pwm_disable(chip, pwm);
> > + ret = 0;
> > + }
>
> Assume the PWM runs with duty cycle 33% when
> pwm_apply_state({ .enabled=0, .duty_cycle=66, .period=100 }) is called.
>
> Does this might result in a 66% wave form being emitted? If yes, this
> needs fixing.
Thank you for the pointing it out. This code is possible to output 66% wave form
between the second rcar_pwm_update() and rcar_pwm_disable(). So, I'll fix this.
Best regards,
Yoshihiro Shimoda
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
next prev parent reply other threads:[~2019-01-08 8:11 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-08 3:28 [PATCH v2 0/4] pwm: rcar: Add support "atomic" API and improve calculation Yoshihiro Shimoda
2019-01-08 3:28 ` [PATCH v2 1/4] pwm: rcar: Add support "atomic" API Yoshihiro Shimoda
2019-01-08 7:43 ` Uwe Kleine-König
2019-01-08 8:11 ` Yoshihiro Shimoda [this message]
2019-01-08 3:28 ` [PATCH v2 2/4] pwm: rcar: Use "atomic" API on rcar_pwm_resume() Yoshihiro Shimoda
2019-01-08 7:47 ` Uwe Kleine-König
2019-01-08 8:46 ` Yoshihiro Shimoda
2019-01-08 9:12 ` Uwe Kleine-König
2019-01-08 8:56 ` Geert Uytterhoeven
2019-01-08 9:19 ` Uwe Kleine-König
2019-01-08 9:35 ` Geert Uytterhoeven
2019-01-10 9:51 ` Uwe Kleine-König
2019-01-08 3:28 ` [PATCH v2 3/4] pwm: rcar: remove legacy APIs Yoshihiro Shimoda
2019-01-08 7:49 ` Uwe Kleine-König
2019-01-08 3:28 ` [PATCH v2 4/4] pwm: rcar: improve calculation of divider Yoshihiro Shimoda
2019-01-08 7:49 ` Uwe Kleine-König
2019-01-08 8:21 ` Geert Uytterhoeven
2019-01-08 9:30 ` Yoshihiro Shimoda
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=TY2PR01MB48122303FCD3B880701F101DD88A0@TY2PR01MB4812.jpnprd01.prod.outlook.com \
--to=yoshihiro.shimoda.uh@renesas.com \
--cc=linux-pwm@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=thierry.reding@gmail.com \
--cc=u.kleine-koenig@pengutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).