From: Olivier MOYSAN <olivier.moysan@foss.st.com> To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de> Cc: Fabrice Gasnier <fabrice.gasnier@foss.st.com>, Thierry Reding <thierry.reding@gmail.com>, Maxime Coquelin <mcoquelin.stm32@gmail.com>, Alexandre Torgue <alexandre.torgue@foss.st.com>, Lee Jones <lee@kernel.org>, Benjamin Gaignard <benjamin.gaignard@linaro.org>, William Breathitt Gray <william.gray@linaro.org>, <linux-pwm@vger.kernel.org>, <linux-stm32@st-md-mailman.stormreply.com>, <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH] pwm: stm32: enforce settings for pwm capture Date: Wed, 14 Dec 2022 16:09:08 +0100 [thread overview] Message-ID: <2ab70bb7-dbf1-5f19-8118-6cfd9b5dc278@foss.st.com> (raw) In-Reply-To: <20221213105128.74skjowy5v7dlaf6@pengutronix.de> Hello Uwe, On 12/13/22 11:51, Uwe Kleine-König wrote: > Hello Olivier, > > [Cc: += William Breathitt Gray, linux-iio@v.k.o] > > On Tue, Dec 13, 2022 at 11:27:07AM +0100, Olivier Moysan wrote: >> The PWM capture assumes that the input selector is set to default >> input and that the slave mode is disabled. Force reset state for >> TISEL and SMCR registers to match this requirement. > > When does the problem occur? Only if the bootloader changed that > setting? Regarding the urgency: With the current knowledge I'd say this > patch is material for the next merge window. Do you recommend > backporting to stable? > Yes, the PWM may not be in the default expected state, if the configuration has been changed in the bootloader. This is not an actual case today, so this patch can wait the next merge window and there is no urgency to have it in stable. BRs Olivier >> Note that slave mode disabling is not a pre-requisite by itself >> for capture mode, as hardware supports it for PWM capture. >> However, the current implementation of the driver does not >> allow slave mode for PWM capture. Setting slave mode for PWM >> capture results in wrong capture values. > > What is your usecase for PWM capture support? I didn't double check, but > I think you're the first contributor to PWM capture since 2018 (i.e. the > commit you're fixing). > > Did you check if the counter subsystem would solve your problems? If it > doesn't I assume William would like to hear about that. > > Looking at drivers/counter/stm32-timer-cnt.c it does seem to work in > slave mode, TISEL isn't touched though. So maybe this driver needs a > similar fix? > > Apart from that the change looks reasonable: > > Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > Best regards > Uwe >
WARNING: multiple messages have this Message-ID (diff)
From: Olivier MOYSAN <olivier.moysan@foss.st.com> To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de> Cc: Fabrice Gasnier <fabrice.gasnier@foss.st.com>, Thierry Reding <thierry.reding@gmail.com>, Maxime Coquelin <mcoquelin.stm32@gmail.com>, Alexandre Torgue <alexandre.torgue@foss.st.com>, Lee Jones <lee@kernel.org>, Benjamin Gaignard <benjamin.gaignard@linaro.org>, William Breathitt Gray <william.gray@linaro.org>, <linux-pwm@vger.kernel.org>, <linux-stm32@st-md-mailman.stormreply.com>, <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH] pwm: stm32: enforce settings for pwm capture Date: Wed, 14 Dec 2022 16:09:08 +0100 [thread overview] Message-ID: <2ab70bb7-dbf1-5f19-8118-6cfd9b5dc278@foss.st.com> (raw) In-Reply-To: <20221213105128.74skjowy5v7dlaf6@pengutronix.de> Hello Uwe, On 12/13/22 11:51, Uwe Kleine-König wrote: > Hello Olivier, > > [Cc: += William Breathitt Gray, linux-iio@v.k.o] > > On Tue, Dec 13, 2022 at 11:27:07AM +0100, Olivier Moysan wrote: >> The PWM capture assumes that the input selector is set to default >> input and that the slave mode is disabled. Force reset state for >> TISEL and SMCR registers to match this requirement. > > When does the problem occur? Only if the bootloader changed that > setting? Regarding the urgency: With the current knowledge I'd say this > patch is material for the next merge window. Do you recommend > backporting to stable? > Yes, the PWM may not be in the default expected state, if the configuration has been changed in the bootloader. This is not an actual case today, so this patch can wait the next merge window and there is no urgency to have it in stable. BRs Olivier >> Note that slave mode disabling is not a pre-requisite by itself >> for capture mode, as hardware supports it for PWM capture. >> However, the current implementation of the driver does not >> allow slave mode for PWM capture. Setting slave mode for PWM >> capture results in wrong capture values. > > What is your usecase for PWM capture support? I didn't double check, but > I think you're the first contributor to PWM capture since 2018 (i.e. the > commit you're fixing). > > Did you check if the counter subsystem would solve your problems? If it > doesn't I assume William would like to hear about that. > > Looking at drivers/counter/stm32-timer-cnt.c it does seem to work in > slave mode, TISEL isn't touched though. So maybe this driver needs a > similar fix? > > Apart from that the change looks reasonable: > > Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > Best regards > Uwe > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-12-14 15:10 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-12-13 10:27 [PATCH] pwm: stm32: enforce settings for pwm capture Olivier Moysan 2022-12-13 10:27 ` Olivier Moysan 2022-12-13 10:51 ` Uwe Kleine-König 2022-12-13 10:51 ` Uwe Kleine-König 2022-12-14 15:09 ` Olivier MOYSAN [this message] 2022-12-14 15:09 ` Olivier MOYSAN 2023-01-17 21:43 ` Uwe Kleine-König 2023-01-17 21:43 ` Uwe Kleine-König 2023-04-12 15:41 ` Uwe Kleine-König 2023-04-12 15:41 ` Uwe Kleine-König 2023-01-03 12:34 ` Lee Jones 2023-01-03 12:34 ` Lee Jones
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=2ab70bb7-dbf1-5f19-8118-6cfd9b5dc278@foss.st.com \ --to=olivier.moysan@foss.st.com \ --cc=alexandre.torgue@foss.st.com \ --cc=benjamin.gaignard@linaro.org \ --cc=fabrice.gasnier@foss.st.com \ --cc=lee@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-pwm@vger.kernel.org \ --cc=linux-stm32@st-md-mailman.stormreply.com \ --cc=mcoquelin.stm32@gmail.com \ --cc=thierry.reding@gmail.com \ --cc=u.kleine-koenig@pengutronix.de \ --cc=william.gray@linaro.org \ /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.