Hello Thierry, On Wed, Jul 19, 2023 at 09:59:29AM +0200, Thierry Reding wrote: > On Tue, Jul 18, 2023 at 08:18:32PM +0200, Uwe Kleine-König wrote: > > This function allocates a struct pwm_chip and driver data. Compared to > > the status quo the split into pwm_chip and driver data is new, otherwise > > it doesn't change anything relevant (yet). > > > > The intention is that after all drivers are switched to use this > > allocation function, its possible to add a struct device to struct > > pwm_chip to properly track the latter's lifetime without touching all > > drivers again. Proper lifetime tracking is a necessary precondition to > > introduce character device support for PWMs (that implements atomic > > setting and doesn't suffer from the sysfs overhead of the /sys/class/pwm > > userspace support). > > > > The new function pwmchip_priv() (obviously?) only works for chips > > allocated with devm_pwmchip_alloc(). > > If this is supposed to be similar to the GPIO chardev, why doesn't GPIO > require this way of allocating a struct gpio_chip? For each gpio_chip there is an external struct gpio_device that is allocated in gpiochip_add(). This works but has some downsides. E.g. gpiochip_remove() has to grab a mutex that is also held while userspace polls a gpioctrl device and so gpiochip_remove blocks. While this works (as in: it doesn't crash) it's in the category "not as good as possible". I'm convinced that getting this right before adding the complexity of chardev support is a good idea. > I'm not a fan of doing all this upfront work without seeing where this > is ultimately headed. Please hold off on reworking everything until > you have a complete proposal that can be reviewed in full. I see some benefit of the conversion started in this patch set stand alone, too. But I agree it's for now a bit theoretic. With the goal to pick the agreed good approach for pwm this is a bigger pile of work. Completing it in private and only present the complete proposal has the downside that I get feedback for the house's seating only when the roof is assembled. While me spending much time isn't your problem, I have a problem with it. Splitting the task in chunks that go in one after another is surely the quicker and more effective approach. So it would be great if you could understand the issue tackled here even though there are subsystems that do it differently (and less optimal) and the usecase isn't complete yet. Thanks Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
