From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: linux-pwm@vger.kernel.org
Cc: linux-hardening@vger.kernel.org,
Kees Cook <keescook@chromium.org>,
kernel@pengutronix.de,
"Gustavo A. R. Silva" <gustavoars@kernel.org>,
John Ernberg <john.ernberg@actia.se>,
Thorsten Scherer <T.Scherer@eckelmann.de>,
Marek Szyprowski <m.szyprowski@samsung.com>,
Trevor Gamblin <tgamblin@baylibre.com>,
David Lechner <dlechner@baylibre.com>
Subject: Re: [PATCH 0/8] pwm: Add support for character devices
Date: Sat, 13 Apr 2024 11:22:33 +0200 [thread overview]
Message-ID: <ywvi3wqcte5wfwq7twg26smtu7rgjv5z2tbdu6mz5cehjlxf72@2h77sey4xsv2> (raw)
In-Reply-To: <cover.1710670958.git.u.kleine-koenig@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 3429 bytes --]
Hello,
On Sun, Mar 17, 2024 at 11:40:31AM +0100, Uwe Kleine-König wrote:
> After the necessary changes to the lowlevel drivers got in for v6.9-rc1
> here come some changes to the core to implement /dev/pwmchipX character
> devices.
>
> In my tests on an ARM STM32MP1 programming a PWM using the character
> device is ~4 times faster than just changing duty_cycle via the sysfs
> API. It also has the advantage that (similar to pwm_apply_*) the target
> state is provided to the kernel with a single call, instead of having to
> program the individual settings one after another via sysfs (in the
> right order to not cross states not supported by the driver).
>
> Note the representation of a PWM waveform is different here compared to
> the in-kernel representation. A PWM waveform is represented using:
>
> period
> duty_cycle
> duty_offset
>
> A disabled PWM is represented by period = 0. For an inversed wave use:
>
> duty_offset = duty_cycle
> duty_cycle = period - duty_cycle;
>
> . However there are some difficulties yet that make it hard to provide a
> consistent API to userspace and so for now duty_offset isn't (fully)
> supported yet. That needs some more consideration and can be added
> later.
>
> A userspace lib together with some simple test programs making use of
> this new API can be found at
>
> https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/libpwm.git
>
> .
>
> The start of the series is some cleanup and preparation. The lifetime
> and locking patches are needed to not crash the kernel when a character
> device is open while a lowlevel driver goes away.
This series is already in next for some time, but I'm not sure that I
want to really send it to Linus in the next merge window as there are a
few issues with it:
- A (false positive) lockdep warning reported by Marek Szyprowski.
See https://lore.kernel.org/all/5a49cadd-21b7-4384-9e7d-9105ccc288b3@samsung.com
- A speculation warning flagged by smatch that I don't understand
completely yet (and failed to attract attention by people that know
more about about it)
See https://lore.kernel.org/all/1e3dc81d-dcd4-4b04-85b1-23937e2f0acd@moroto.mountain
- I'm a bit unhappy about the rounding behaviour. Actually I'd like to
only provide userspace access via the character device to drivers
that adhere to the rounding rules for new drivers (that is: First
pick the maximal period that isn't bigger than the requested period.
Then for the chosen period pick the maximal duty_cycle that isn't
bigger than the requested one) to give a consistent behaviour. This
is further complicated by the fact that the character device exposes
a more flexible API (involving a duty_offset instead of polarity) and
the natural extension for the rounding rules with duty_offset is
different than for inverted polarity configurations.
I currently consider introducing a new callback that in the long run
should replace .apply() and that properly implements the duty_offset
stuff. Then the character device could only be provided for the drivers
implementing .apply2().
I'm open for feedback, e.g. suggestions for a better name for .apply2().
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2024-04-13 9:22 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-17 10:40 [PATCH 0/8] pwm: Add support for character devices Uwe Kleine-König
2024-03-17 10:40 ` [PATCH 1/8] pwm: Ensure that pwm_chips are allocated using pwmchip_alloc() Uwe Kleine-König
2024-03-17 10:40 ` [PATCH 2/8] pwm: Give some sysfs related variables and functions better names Uwe Kleine-König
2024-03-17 10:40 ` [PATCH 3/8] pwm: Move contents of sysfs.c into core.c Uwe Kleine-König
2024-03-17 10:40 ` [PATCH 4/8] pwm: Ensure a struct pwm has the same lifetime as its pwm_chip Uwe Kleine-König
2024-03-17 10:40 ` [PATCH 5/8] pwm: Add a struct device to struct pwm_chip Uwe Kleine-König
2024-03-28 18:44 ` David Lechner
2024-03-29 10:19 ` Uwe Kleine-König
2024-03-17 10:40 ` [PATCH 6/8] pwm: Make pwmchip_[sg]et_drvdata() a wrapper around dev_set_drvdata() Uwe Kleine-König
2024-03-17 10:40 ` [PATCH 7/8] pwm: Add more locking Uwe Kleine-König
2024-03-18 7:28 ` Thorsten Scherer
[not found] ` <CGME20240404120932eucas1p1b3c1e07bf6f41f6330725148b0268b13@eucas1p1.samsung.com>
2024-04-04 12:09 ` Marek Szyprowski
2024-04-04 15:33 ` Uwe Kleine-König
2024-03-17 10:40 ` [PATCH 8/8] pwm: Add support for pwmchip devices for faster and easier userspace access Uwe Kleine-König
2024-04-08 15:42 ` John Ernberg
2024-04-09 7:49 ` Uwe Kleine-König
2024-04-15 11:27 ` John Ernberg
2024-05-07 1:11 ` Kent Gibson
2024-05-07 6:12 ` Uwe Kleine-König
2024-04-13 9:22 ` Uwe Kleine-König [this message]
2024-04-22 18:59 ` [PATCH 0/8] pwm: Add support for character devices David Lechner
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=ywvi3wqcte5wfwq7twg26smtu7rgjv5z2tbdu6mz5cehjlxf72@2h77sey4xsv2 \
--to=u.kleine-koenig@pengutronix.de \
--cc=T.Scherer@eckelmann.de \
--cc=dlechner@baylibre.com \
--cc=gustavoars@kernel.org \
--cc=john.ernberg@actia.se \
--cc=keescook@chromium.org \
--cc=kernel@pengutronix.de \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=tgamblin@baylibre.com \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).