From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752609AbdK2Hei (ORCPT ); Wed, 29 Nov 2017 02:34:38 -0500 Received: from mail-wr0-f195.google.com ([209.85.128.195]:42176 "EHLO mail-wr0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752077AbdK2Heg (ORCPT ); Wed, 29 Nov 2017 02:34:36 -0500 X-Google-Smtp-Source: AGs4zMZw3TMYbgdW/ypqJLy+p61f1oAYlOxKRaDA4l6Nit+BQ/Sgb6sIiqRgSTT4QX4srLrFy9rZWQ== Message-ID: <1511940871.27425.4.camel@baylibre.com> Subject: Re: [PATCH] pwm: meson: fix harware duty calculation From: Jerome Brunet To: Yixun Lan , Thierry Reding , linux-pwm@vger.kernel.org, linux-amlogic@lists.infradead.org Cc: Neil Armstrong , Carlo Caione , Kevin Hilman , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Jian Hu Date: Wed, 29 Nov 2017 08:34:31 +0100 In-Reply-To: <20171129030308.22036-1-yixun.lan@amlogic.com> References: <20171129030308.22036-1-yixun.lan@amlogic.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.26.2 (3.26.2-1.fc27) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2017-11-29 at 11:03 +0800, Yixun Lan wrote: > From: Jian Hu > > 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 > Signed-off-by: Yixun Lan > --- > 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;