From mboxrd@z Thu Jan 1 00:00:00 1970 From: boris.brezillon@free-electrons.com (Boris Brezillon) Date: Thu, 2 Jul 2015 09:43:19 +0200 Subject: [RFC PATCH 13/15] pwm: rockchip: add support for atomic update In-Reply-To: <1709826.dVqTms8REP@diego> References: <1435738921-25027-1-git-send-email-boris.brezillon@free-electrons.com> <1435738921-25027-14-git-send-email-boris.brezillon@free-electrons.com> <1709826.dVqTms8REP@diego> Message-ID: <20150702094319.11794da8@bbrezillon> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Heiko, On Wed, 01 Jul 2015 23:48:31 +0200 Heiko St?bner wrote: > Hi Boris, > > > Am Mittwoch, 1. Juli 2015, 10:21:59 schrieb Boris Brezillon: > > Implement the ->apply() function to add support for atomic update. > > > > Signed-off-by: Boris Brezillon > > --- > > @@ -110,6 +113,26 @@ static void rockchip_pwm_set_enable_v2(struct pwm_chip > > *chip, writel_relaxed(val, pc->base + pc->data->regs.ctrl); > > } > > > > +static void rockchip_pwm_init_v2(struct pwm_chip *chip, struct pwm_device > > *pwm) +{ > > + struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip); > > + u32 enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE | > > + PWM_CONTINUOUS; > > + u32 val; > > + > > + val = readl(pc->base + pc->data->regs.ctrl); > > + > > + if ((val & enable_conf) != enable_conf) > > + return; > > + > > + pwm->state.enabled = true; > > + > > + enable_conf = PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSITIVE; > > + > > + if ((val & enable_conf) == enable_conf) > > + pwm->state.polarity = PWM_POLARITY_INVERSED; > > the inactive setting does not affect the polarity of the running pwm, only what > to do when it gets turned off. Also PWM_DUTY_NEGATIVE is the "0" value for the > bit so also is bad to compare against (and results in wrong readings). So I > would suggest changing this like Indeed > > - enable_conf = PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSITIVE; > + enable_conf = PWM_DUTY_POSITIVE; > > - if ((val & enable_conf) == enable_conf) > + if ((val & enable_conf) != enable_conf) Or just: if(val & PWM_DUTY_POSITIVE) Thanks for the fix. Best Regards, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Brezillon Subject: Re: [RFC PATCH 13/15] pwm: rockchip: add support for atomic update Date: Thu, 2 Jul 2015 09:43:19 +0200 Message-ID: <20150702094319.11794da8@bbrezillon> References: <1435738921-25027-1-git-send-email-boris.brezillon@free-electrons.com> <1435738921-25027-14-git-send-email-boris.brezillon@free-electrons.com> <1709826.dVqTms8REP@diego> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from down.free-electrons.com ([37.187.137.238]:32931 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752179AbbGBHnY convert rfc822-to-8bit (ORCPT ); Thu, 2 Jul 2015 03:43:24 -0400 In-Reply-To: <1709826.dVqTms8REP@diego> Sender: linux-leds-owner@vger.kernel.org List-Id: linux-leds@vger.kernel.org To: Heiko =?UTF-8?B?U3TDvGJuZXI=?= Cc: linux-arm-kernel@lists.infradead.org, Thierry Reding , linux-pwm@vger.kernel.org, Alexandre Courbot , linux-fbdev@vger.kernel.org, Lee Jones , Stephen Warren , Tomi Valkeinen , Bryan Wu , Liam Girdwood , Doug Anderson , Mark Brown , Richard Purdie , Jingoo Han , linux-tegra@vger.kernel.org, Maxime Ripard , Jean-Christophe Plagniol-Villard , Jacek Anaszewski , linux-leds@vger.kernel.org Hi Heiko, On Wed, 01 Jul 2015 23:48:31 +0200 Heiko St=C3=BCbner wrote: > Hi Boris, >=20 >=20 > Am Mittwoch, 1. Juli 2015, 10:21:59 schrieb Boris Brezillon: > > Implement the ->apply() function to add support for atomic update. > >=20 > > Signed-off-by: Boris Brezillon > > --- > > @@ -110,6 +113,26 @@ static void rockchip_pwm_set_enable_v2(struct = pwm_chip > > *chip, writel_relaxed(val, pc->base + pc->data->regs.ctrl); > > } > >=20 > > +static void rockchip_pwm_init_v2(struct pwm_chip *chip, struct pwm= _device > > *pwm) +{ > > + struct rockchip_pwm_chip *pc =3D to_rockchip_pwm_chip(chip); > > + u32 enable_conf =3D PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE= | > > + PWM_CONTINUOUS; > > + u32 val; > > + > > + val =3D readl(pc->base + pc->data->regs.ctrl); > > + > > + if ((val & enable_conf) !=3D enable_conf) > > + return; > > + > > + pwm->state.enabled =3D true; > > + > > + enable_conf =3D PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSITIVE; > > + > > + if ((val & enable_conf) =3D=3D enable_conf) > > + pwm->state.polarity =3D PWM_POLARITY_INVERSED; >=20 > the inactive setting does not affect the polarity of the running pwm,= only what=20 > to do when it gets turned off. Also PWM_DUTY_NEGATIVE is the "0" valu= e for the=20 > bit so also is bad to compare against (and results in wrong readings)= =2E So I=20 > would suggest changing this like Indeed >=20 > - enable_conf =3D PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSITIVE; > + enable_conf =3D PWM_DUTY_POSITIVE; > =20 > - if ((val & enable_conf) =3D=3D enable_conf) > + if ((val & enable_conf) !=3D enable_conf) Or just: if(val & PWM_DUTY_POSITIVE) Thanks for the fix. Best Regards, Boris --=20 Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Brezillon Date: Thu, 02 Jul 2015 07:43:19 +0000 Subject: Re: [RFC PATCH 13/15] pwm: rockchip: add support for atomic update Message-Id: <20150702094319.11794da8@bbrezillon> List-Id: References: <1435738921-25027-1-git-send-email-boris.brezillon@free-electrons.com> <1435738921-25027-14-git-send-email-boris.brezillon@free-electrons.com> <1709826.dVqTms8REP@diego> In-Reply-To: <1709826.dVqTms8REP@diego> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: linux-arm-kernel@lists.infradead.org Hi Heiko, On Wed, 01 Jul 2015 23:48:31 +0200 Heiko St=C3=BCbner wrote: > Hi Boris, >=20 >=20 > Am Mittwoch, 1. Juli 2015, 10:21:59 schrieb Boris Brezillon: > > Implement the ->apply() function to add support for atomic update. > >=20 > > Signed-off-by: Boris Brezillon > > --- > > @@ -110,6 +113,26 @@ static void rockchip_pwm_set_enable_v2(struct pwm_= chip > > *chip, writel_relaxed(val, pc->base + pc->data->regs.ctrl); > > } > >=20 > > +static void rockchip_pwm_init_v2(struct pwm_chip *chip, struct pwm_dev= ice > > *pwm) +{ > > + struct rockchip_pwm_chip *pc =3D to_rockchip_pwm_chip(chip); > > + u32 enable_conf =3D PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE | > > + PWM_CONTINUOUS; > > + u32 val; > > + > > + val =3D readl(pc->base + pc->data->regs.ctrl); > > + > > + if ((val & enable_conf) !=3D enable_conf) > > + return; > > + > > + pwm->state.enabled =3D true; > > + > > + enable_conf =3D PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSITIVE; > > + > > + if ((val & enable_conf) =3D enable_conf) > > + pwm->state.polarity =3D PWM_POLARITY_INVERSED; >=20 > the inactive setting does not affect the polarity of the running pwm, onl= y what=20 > to do when it gets turned off. Also PWM_DUTY_NEGATIVE is the "0" value fo= r the=20 > bit so also is bad to compare against (and results in wrong readings). So= I=20 > would suggest changing this like Indeed >=20 > - enable_conf =3D PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSITIVE; > + enable_conf =3D PWM_DUTY_POSITIVE; > =20 > - if ((val & enable_conf) =3D enable_conf) > + if ((val & enable_conf) !=3D enable_conf) Or just: if(val & PWM_DUTY_POSITIVE) Thanks for the fix. Best Regards, Boris --=20 Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com