On Sat, Apr 13, 2024 at 4:22 AM Uwe Kleine-König <u.kleine-koe...@pengutronix.de> wrote: > > 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-9105ccc28...@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(). >
Waiting to merge this and giving this some more thought first does seem like a wise idea.
