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>, "kernel@pengutronix.de" <kernel@pengutronix.de> Subject: RE: [PATCH 5/5] pwm: rcar: add workaround to output "pseudo" low level Date: Wed, 12 Dec 2018 03:19:40 +0000 [thread overview] Message-ID: <OSBPR01MB2293BE0147A4A535136BDF82D8A70@OSBPR01MB2293.jpnprd01.prod.outlook.com> (raw) In-Reply-To: <20181210081103.zxayrzpbxiuvv6hb@pengutronix.de> Hi Uwe, > From: Uwe Kleine-Konig, Sent: Monday, December 10, 2018 5:11 PM > > Hello, > > On Mon, Dec 10, 2018 at 04:49:17AM +0000, Yoshihiro Shimoda wrote: > > > From: Uwe Kleine-Konig, Sent: Friday, December 7, 2018 6:14 PM > > > > > > On Fri, Dec 07, 2018 at 05:29:33PM +0900, Yoshihiro Shimoda wrote: > > <snip> > > > > +static void rcar_pwm_workaround_output_low(struct rcar_pwm_chip *rp) > > > > +{ > > > > + /* > > > > + * This PWM Timer cannot output low because setting 0x000 is > > > > + * prohibited on PWMCNT.PH0 (High-Level Period) bitfields. So, avoiding > > > > + * the prohibited, this function changes the value from 0 to 1 as > > > > + * pseudo low level. > > > > + * > > > > + * TODO: Add GPIO handling to output low level. > > > > + */ > > > > + if ((rp->pwmcnt & RCAR_PWMCNT_PH0_MASK) == 0) > > > > + rp->pwmcnt |= 1; > > > > > > In my eyes this is too broken to do. Not sure I have the complete > > > picture, but given a small period (say 2) this 1 cycle might result in > > > 50 % duty cycle. Depending on how the hardware behaves if you disable > > > it, better do this instead. > > > > You're right. > > But in the meantime I learned that the pwm gets active on disable, so > this won't help. > > > > Are you aware of the series adding such gpio support to the imx driver? > > > > I didn't know that. > > > > > @Thierry: So there are three drivers now that could benefit for a > > > generic approach. > > > > Should I wait for Thierry's opinion whether PWM subsystem will have > > a generic approach or not? > > Not sure how to preceed here. The needed procedure would be: > > set duty_cycle to 0% > delay long enough to be sure the duty cycle is active > switch to gpio > disable the hardware > > The additional blocker for rcar is that it doesn't support duty_cycle > 0%. > > So unless your hardware guys confirm that 0% works even though not > supported according to the hardware manual I have no good idea. > > In the past I suggested to weaken the requirements after pwm_disable, > but Thierry didn't like it. I read the following discussion once: https://patchwork.ozlabs.org/patch/959776/ I could not understand all this yet, but I think I should try to add a special gpio handling to the pwm-rcar.c driver instead of a generic approach because as you mentioned above, such special handling needs for the hardware. Best regards, Yoshihiro Shimoda > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | http://www.pengutronix.de/ |
WARNING: multiple messages have this Message-ID (diff)
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de> Cc: "linux-renesas-soc@vger.kernel.org" <linux-renesas-soc@vger.kernel.org>, "linux-pwm@vger.kernel.org" <linux-pwm@vger.kernel.org>, "thierry.reding@gmail.com" <thierry.reding@gmail.com>, "kernel@pengutronix.de" <kernel@pengutronix.de> Subject: RE: [PATCH 5/5] pwm: rcar: add workaround to output "pseudo" low level Date: Wed, 12 Dec 2018 03:19:40 +0000 [thread overview] Message-ID: <OSBPR01MB2293BE0147A4A535136BDF82D8A70@OSBPR01MB2293.jpnprd01.prod.outlook.com> (raw) In-Reply-To: <20181210081103.zxayrzpbxiuvv6hb@pengutronix.de> Hi Uwe, > From: Uwe Kleine-Konig, Sent: Monday, December 10, 2018 5:11 PM > > Hello, > > On Mon, Dec 10, 2018 at 04:49:17AM +0000, Yoshihiro Shimoda wrote: > > > From: Uwe Kleine-Konig, Sent: Friday, December 7, 2018 6:14 PM > > > > > > On Fri, Dec 07, 2018 at 05:29:33PM +0900, Yoshihiro Shimoda wrote: > > <snip> > > > > +static void rcar_pwm_workaround_output_low(struct rcar_pwm_chip *rp) > > > > +{ > > > > + /* > > > > + * This PWM Timer cannot output low because setting 0x000 is > > > > + * prohibited on PWMCNT.PH0 (High-Level Period) bitfields. So, avoiding > > > > + * the prohibited, this function changes the value from 0 to 1 as > > > > + * pseudo low level. > > > > + * > > > > + * TODO: Add GPIO handling to output low level. > > > > + */ > > > > + if ((rp->pwmcnt & RCAR_PWMCNT_PH0_MASK) == 0) > > > > + rp->pwmcnt |= 1; > > > > > > In my eyes this is too broken to do. Not sure I have the complete > > > picture, but given a small period (say 2) this 1 cycle might result in > > > 50 % duty cycle. Depending on how the hardware behaves if you disable > > > it, better do this instead. > > > > You're right. > > But in the meantime I learned that the pwm gets active on disable, so > this won't help. > > > > Are you aware of the series adding such gpio support to the imx driver? > > > > I didn't know that. > > > > > @Thierry: So there are three drivers now that could benefit for a > > > generic approach. > > > > Should I wait for Thierry's opinion whether PWM subsystem will have > > a generic approach or not? > > Not sure how to preceed here. The needed procedure would be: > > set duty_cycle to 0% > delay long enough to be sure the duty cycle is active > switch to gpio > disable the hardware > > The additional blocker for rcar is that it doesn't support duty_cycle > 0%. > > So unless your hardware guys confirm that 0% works even though not > supported according to the hardware manual I have no good idea. > > In the past I suggested to weaken the requirements after pwm_disable, > but Thierry didn't like it. I read the following discussion once: https://patchwork.ozlabs.org/patch/959776/ I could not understand all this yet, but I think I should try to add a special gpio handling to the pwm-rcar.c driver instead of a generic approach because as you mentioned above, such special handling needs for the hardware. 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:[~2018-12-12 3:20 UTC|newest] Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-12-07 8:29 [PATCH 0/5] pwm: rcar: Add support "atomic" API and workaround Yoshihiro Shimoda 2018-12-07 8:29 ` [PATCH 1/5] pwm: rcar: add rcar_pwm_calc_counter() to calculate PWMCNT value only Yoshihiro Shimoda 2018-12-07 9:00 ` Uwe Kleine-König 2018-12-07 9:00 ` Uwe Kleine-König 2018-12-07 8:29 ` [PATCH 2/5] pwm: rcar: Add support "atomic" API Yoshihiro Shimoda 2018-12-07 9:07 ` Uwe Kleine-König 2018-12-07 9:07 ` Uwe Kleine-König 2018-12-07 9:57 ` Geert Uytterhoeven 2018-12-07 10:45 ` Uwe Kleine-König 2018-12-07 10:45 ` Uwe Kleine-König 2018-12-10 4:58 ` Yoshihiro Shimoda 2018-12-07 8:29 ` [PATCH 3/5] pwm: rcar: Use "atomic" API on rcar_pwm_resume() Yoshihiro Shimoda 2018-12-07 8:29 ` [PATCH 4/5] pwm: rcar: remove legacy APIs Yoshihiro Shimoda 2018-12-07 8:29 ` [PATCH 5/5] pwm: rcar: add workaround to output "pseudo" low level Yoshihiro Shimoda 2018-12-07 9:13 ` Uwe Kleine-König 2018-12-07 9:13 ` Uwe Kleine-König 2018-12-10 4:49 ` Yoshihiro Shimoda 2018-12-10 8:11 ` Uwe Kleine-König 2018-12-10 8:11 ` Uwe Kleine-König 2018-12-12 3:19 ` Yoshihiro Shimoda [this message] 2018-12-12 3:19 ` Yoshihiro Shimoda 2018-12-12 7:37 ` Uwe Kleine-König 2018-12-12 7:37 ` Uwe Kleine-König 2018-12-12 10:49 ` Yoshihiro Shimoda 2018-12-12 10:49 ` Yoshihiro Shimoda 2018-12-13 9:47 ` Yoshihiro Shimoda 2018-12-13 9:47 ` Yoshihiro Shimoda 2018-12-13 9:52 ` Uwe Kleine-König 2018-12-13 9:52 ` Uwe Kleine-König 2018-12-13 10:53 ` Yoshihiro Shimoda 2018-12-13 10:53 ` Yoshihiro Shimoda 2018-12-07 21:49 ` pwm: rcar: improve calculation of divider Uwe Kleine-König 2018-12-07 21:49 ` Uwe Kleine-König 2018-12-09 20:55 ` Laurent Pinchart 2018-12-10 5:09 ` Yoshihiro Shimoda 2018-12-10 8:04 ` Uwe Kleine-König 2018-12-10 8:04 ` Uwe Kleine-König 2018-12-12 3:13 ` Yoshihiro Shimoda 2018-12-12 3:13 ` Yoshihiro Shimoda 2018-12-09 22:48 ` [PATCH 0/5] pwm: rcar: Add support "atomic" API and workaround Laurent Pinchart
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=OSBPR01MB2293BE0147A4A535136BDF82D8A70@OSBPR01MB2293.jpnprd01.prod.outlook.com \ --to=yoshihiro.shimoda.uh@renesas.com \ --cc=kernel@pengutronix.de \ --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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.