On Wed, 2017-11-29 at 11:03 +0800, Yixun Lan wrote:
> From: Jian Hu <jian...@amlogic.com>
>
> The actual HIGH/LOW signal output from the PWM is equal to
> the value programed to HW register plus one, this is designed by HW.
>
> This fix should apply to all Meson SoC(include GX/GXL/GXBB, Meson6,8)
>
> Fixes: 211ed630753d ("pwm: Add support for Meson PWM Controller")
> Signed-off-by: Jian Hu <jian...@amlogic.com>
> Signed-off-by: Yixun Lan <yixun....@amlogic.com>
> ---
> drivers/pwm/pwm-meson.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
> index d589331d1884..78d9b8c1a4bc 100644
> --- a/drivers/pwm/pwm-meson.c
> +++ b/drivers/pwm/pwm-meson.c
> @@ -193,6 +193,11 @@ static int meson_pwm_calc(struct meson_pwm *meson,
> break;
> }
>
> + if (cnt < 2) {
> + dev_err(meson->chip.dev, "invalid period\n");
> + return -EINVAL;
> + }
> +
> if (pre_div == MISC_CLK_DIV_MASK) {
> dev_err(meson->chip.dev, "unable to get period pre_div\n");
> return -EINVAL;
> @@ -201,19 +206,23 @@ static int meson_pwm_calc(struct meson_pwm *meson,
> dev_dbg(meson->chip.dev, "period=%u pre_div=%u cnt=%u\n", period,
> pre_div, cnt);
>
> + /*
> + * Due to the design of hardware, values of 'hi', 'lo' are 1 based
> + * which mean the actual output from hardware is 'hi' + 1, 'lo' + 1
> + */
> if (duty == period) {
> channel->pre_div = pre_div;
> - channel->hi = cnt;
> + channel->hi = cnt - 1;
> channel->lo = 0;
> } else if (duty == 0) {
> channel->pre_div = pre_div;
> channel->hi = 0;
> - channel->lo = cnt;
> + channel->lo = cnt - 1;
> } else {
> /* Then check is we can have the duty with the same pre_div
> */
> duty_cnt = DIV_ROUND_CLOSEST_ULL((u64)duty * 1000,
> fin_ps * (pre_div + 1));
> - if (duty_cnt > 0xffff) {
> + if (duty_cnt > 0xffff || !duty_cnt) {
duty_cnt = 0 is a valid value here. It will be the case for duty != 0 but low
enough for the HW (calculation) to approximate the duty cycle to zero.
> dev_err(meson->chip.dev, "unable to get duty
> cycle\n");
> return -EINVAL;
> }
> @@ -222,8 +231,8 @@ static int meson_pwm_calc(struct meson_pwm *meson,
> duty, pre_div, duty_cnt);
>
> channel->pre_div = pre_div;
> - channel->hi = duty_cnt;
> - channel->lo = cnt - duty_cnt;
> + channel->hi = duty_cnt - 1;
As explained above, duty_cnt could be zero, you need to take care of this here
> + channel->lo = cnt - duty_cnt - 1;
Same here, it is possible duty_cnt to be egual to cnt so you also need to be
careful here
> }
>
> return 0;
