| Message ID | cover.1706182805.git.u.kleine-koenig@pengutronix.de |
|---|---|
| Headers | show |
| Series | pwm: Improve lifetime tracking for pwm_chips | expand |
On 1/25/24 6:09 AM, 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(). I think this looks good. Two questions: - Should you explicitly align the private data? Or do you believe the default alignment (currently pointer size aligned) is adequate? - Is there a non-devres version of the allocation function? -Alex > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > .../driver-api/driver-model/devres.rst | 1 + > Documentation/driver-api/pwm.rst | 10 ++++---- > drivers/pwm/core.c | 25 +++++++++++++++++++ > include/linux/pwm.h | 2 ++ > 4 files changed, 33 insertions(+), 5 deletions(-) > > diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst > index c5f99d834ec5..e4df72c408d2 100644 > --- a/Documentation/driver-api/driver-model/devres.rst > +++ b/Documentation/driver-api/driver-model/devres.rst > @@ -420,6 +420,7 @@ POWER > devm_reboot_mode_unregister() > > PWM > + devm_pwmchip_alloc() > devm_pwmchip_add() > devm_pwm_get() > devm_fwnode_pwm_get() > diff --git a/Documentation/driver-api/pwm.rst b/Documentation/driver-api/pwm.rst > index 3c28ccc4b611..cee66c7f0335 100644 > --- a/Documentation/driver-api/pwm.rst > +++ b/Documentation/driver-api/pwm.rst > @@ -143,11 +143,11 @@ to implement the pwm_*() functions itself. This means that it's impossible > to have multiple PWM drivers in the system. For this reason it's mandatory > for new drivers to use the generic PWM framework. > > -A new PWM controller/chip can be added using pwmchip_add() and removed > -again with pwmchip_remove(). pwmchip_add() takes a filled in struct > -pwm_chip as argument which provides a description of the PWM chip, the > -number of PWM devices provided by the chip and the chip-specific > -implementation of the supported PWM operations to the framework. > +A new PWM controller/chip can be allocated using devm_pwmchip_alloc, then added > +using pwmchip_add() and removed again with pwmchip_remove(). pwmchip_add() > +takes a filled in struct pwm_chip as argument which provides a description of > +the PWM chip, the number of PWM devices provided by the chip and the > +chip-specific implementation of the supported PWM operations to the framework. > > When implementing polarity support in a PWM driver, make sure to respect the > signal conventions in the PWM framework. By definition, normal polarity > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > index 1b4c3d0caa82..b821a2b0b172 100644 > --- a/drivers/pwm/core.c > +++ b/drivers/pwm/core.c > @@ -454,6 +454,31 @@ of_pwm_single_xlate(struct pwm_chip *chip, const struct of_phandle_args *args) > } > EXPORT_SYMBOL_GPL(of_pwm_single_xlate); > > +static void *pwmchip_priv(struct pwm_chip *chip) > +{ > + return (void *)chip + sizeof(*chip); > +} > + > +struct pwm_chip *devm_pwmchip_alloc(struct device *parent, unsigned int npwm, size_t sizeof_priv) > +{ > + struct pwm_chip *chip; > + size_t alloc_size; > + > + alloc_size = size_add(sizeof(*chip), sizeof_priv); > + > + chip = devm_kzalloc(parent, alloc_size, GFP_KERNEL); > + if (!chip) > + return ERR_PTR(-ENOMEM); > + > + chip->dev = parent; > + chip->npwm = npwm; > + > + pwmchip_set_drvdata(chip, pwmchip_priv(chip)); > + > + return chip; > +} > +EXPORT_SYMBOL_GPL(devm_pwmchip_alloc); > + > static void of_pwmchip_add(struct pwm_chip *chip) > { > if (!chip->dev || !chip->dev->of_node) > diff --git a/include/linux/pwm.h b/include/linux/pwm.h > index 2c49d2fe2fe7..8bc7504aa7d4 100644 > --- a/include/linux/pwm.h > +++ b/include/linux/pwm.h > @@ -403,6 +403,8 @@ static inline bool pwm_might_sleep(struct pwm_device *pwm) > int pwm_capture(struct pwm_device *pwm, struct pwm_capture *result, > unsigned long timeout); > > +struct pwm_chip *devm_pwmchip_alloc(struct device *parent, unsigned int npwm, size_t sizeof_priv); > + > int __pwmchip_add(struct pwm_chip *chip, struct module *owner); > #define pwmchip_add(chip) __pwmchip_add(chip, THIS_MODULE) > void pwmchip_remove(struct pwm_chip *chip);
Hello Alex, On Fri, Jan 26, 2024 at 08:56:33AM -0600, Alex Elder wrote: > On 1/25/24 6:09 AM, 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(). > > I think this looks good. Two questions: > - Should you explicitly align the private data? Or do you believe > the default alignment (currently pointer size aligned) is adequate? I'm not aware of a requirement for a higher order alignment (but I might well miss something). I did my tests on arm, nothing exploded there. Maybe the conservative approach of asserting the same alignment as kmalloc would be a good idea. I'll think and research about that. iio uses ARCH_DMA_MINALIGN, net uses 32 (NETDEV_ALIGN). > - Is there a non-devres version of the allocation function? Patch #109 introduces a non-devres variant. As it's not used it's a static function though. Can easily be changed is a use case pops up. Best regards Uwe
Hello, this is v5 of this series. The relevant changes since v4 (https://lore.kernel.org/linux-pwm/cover.1701860672.git.u.kleine-koenig@pengutronix.de): - New first patch to reshuffle functions in core.c. This is a preparation for the later changes which brings functions in a better order to not need declarations. - Fix kernel docs in several drivers - Added a few ack and review tags received for v4 - non-trivially rebased to current pwm/for-next (the changes to drivers/gpu/drm/bridge/ti-sn65dsi86.c were intrusive enough to not add the ack tag by Robert Foss I got). Handling got a bit more complicated with the recent addition of pwm_apply_atomic/pwm_apply_might_sleep the locking got more complicated. I didn't work out all the necessary details. So this series won't work as is. However as there is probably some more coordination needed to get the patches in that touch files outside of drivers/pwm and I'm confident they can stay as is, I want to get the biggest part of this series in (up to patch #106) during the next merge window and get them into next soon. After that I can spend the time necessary to fix the locking maybe to get the remaining bits in during the following merge window. There are patches touching drivers/gpu/drm/bridge/ti-sn65dsi86.c (#37 and #104), drivers/staging/greybus/pwm.c (#38 and #106), drivers/gpio/gpio-mvebu.c (#103) and drivers/leds/rgb/leds-qcom-lpg.c (#105). These depend on earlier patches in this series (#3, #39 and #40) The patches touching staging/greybus and leds-qcom-lpg already have a maintainer ack, so I'd merge them via my tree. For the other two it would be nice to get an ack to merge via my tree, too. But if you want to merge via your own tree, please tell, so we can coordinate accordingly. Best regards Uwe Uwe Kleine-König (111): pwm: Reorder symbols in core.c pwm: cros-ec: Change prototype of helpers to prepare further changes pwm: Provide a macro to get the parent device of a given chip pwm: ab8500: Make use of pwmchip_parent() macro pwm: atmel: Make use of pwmchip_parent() macro pwm: atmel-tcb: Make use of pwmchip_parent() macro pwm: bcm-kona: Make use of pwmchip_parent() macro pwm: crc: Make use of pwmchip_parent() macro pwm: cros-ec: Make use of pwmchip_parent() macro pwm: dwc: Make use of pwmchip_parent() macro pwm: ep93xx: Make use of pwmchip_parent() macro pwm: fsl-ftm: Make use of pwmchip_parent() macro pwm: img: Make use of parent device pointer in driver data pwm: imx27: Make use of pwmchip_parent() macro pwm: jz4740: Make use of pwmchip_parent() macro pwm: lpc18xx-sct: Make use of parent device pointer in driver data pwm: lpss: Make use of pwmchip_parent() macro pwm: mediatek: Make use of pwmchip_parent() macro pwm: meson: Make use of pwmchip_parent() macro pwm: mtk-disp: Make use of pwmchip_parent() macro pwm: omap: Make use of pwmchip_parent() macro pwm: pca9685: Store parent device in driver data pwm: raspberrypi-poe: Make use of pwmchip_parent() macro pwm: rcar: Make use of pwmchip_parent() macro pwm: rz-mtu3: Make use of pwmchip_parent() macro pwm: samsung: Make use of pwmchip_parent() macro pwm: sifive: Make use of pwmchip_parent() macro pwm: stm32-lp: Make use of pwmchip_parent() macro pwm: stm32: Make use of pwmchip_parent() macro pwm: stmpe: Make use of pwmchip_parent() macro pwm: sun4i: Make use of pwmchip_parent() macro pwm: tiecap: Make use of pwmchip_parent() macro pwm: tiehrpwm: Make use of pwmchip_parent() macro pwm: twl-led: Make use of pwmchip_parent() macro pwm: twl: Make use of pwmchip_parent() macro pwm: vt8500: Make use of pwmchip_parent() macro drm/bridge: ti-sn65dsi86: Make use of pwmchip_parent() macro staging: greybus: pwm: Make use of pwmchip_parent() macro pwm: Provide wrappers for storing and getting driver private data pwm: Provide devm_pwmchip_alloc() function pwm: ab8500: Make use of devm_pwmchip_alloc() function pwm: apple: Make use of devm_pwmchip_alloc() function pwm: atmel-hlcdc: Make use of devm_pwmchip_alloc() function pwm: atmel: Make use of devm_pwmchip_alloc() function pwm: atmel-tcb: Make use of devm_pwmchip_alloc() function pwm: bcm2835: Make use of devm_pwmchip_alloc() function pwm: bcm-iproc: Make use of devm_pwmchip_alloc() function pwm: bcm-kona: Make use of devm_pwmchip_alloc() function pwm: berlin: Make use of devm_pwmchip_alloc() function pwm: brcmstb: Make use of devm_pwmchip_alloc() function pwm: clk: Make use of devm_pwmchip_alloc() function pwm: clps711x: Make use of devm_pwmchip_alloc() function pwm: crc: Make use of devm_pwmchip_alloc() function pwm: cros-ec: Make use of devm_pwmchip_alloc() function pwm: dwc: Make use of devm_pwmchip_alloc() function pwm: ep93xx: Make use of devm_pwmchip_alloc() function pwm: fsl-ftm: Make use of devm_pwmchip_alloc() function pwm: hibvt: Make use of devm_pwmchip_alloc() function pwm: img: Make use of devm_pwmchip_alloc() function pwm: imx1: Make use of devm_pwmchip_alloc() function pwm: imx27: Make use of devm_pwmchip_alloc() function pwm: imx-tpm: Make use of devm_pwmchip_alloc() function pwm: intel-lgm: Make use of devm_pwmchip_alloc() function pwm: iqs620a: Make use of devm_pwmchip_alloc() function pwm: jz4740: Make use of devm_pwmchip_alloc() function pwm: keembay: Make use of devm_pwmchip_alloc() function pwm: lp3943: Make use of devm_pwmchip_alloc() function pwm: lpc18xx-sct: Make use of devm_pwmchip_alloc() function pwm: lpc32xx: Make use of devm_pwmchip_alloc() function pwm: lpss-*: Make use of devm_pwmchip_alloc() function pwm: mediatek: Make use of devm_pwmchip_alloc() function pwm: meson: Make use of devm_pwmchip_alloc() function pwm: microchip-core: Make use of devm_pwmchip_alloc() function pwm: mtk-disp: Make use of devm_pwmchip_alloc() function pwm: mxs: Make use of devm_pwmchip_alloc() function pwm: ntxec: Make use of devm_pwmchip_alloc() function pwm: omap-dmtimer: Make use of devm_pwmchip_alloc() function pwm: pca9685: Make use of devm_pwmchip_alloc() function pwm: pxa: Make use of devm_pwmchip_alloc() function pwm: raspberrypi-poe: Make use of devm_pwmchip_alloc() function pwm: rcar: Make use of devm_pwmchip_alloc() function pwm: renesas-tpu: Make use of devm_pwmchip_alloc() function pwm: rockchip: Make use of devm_pwmchip_alloc() function pwm: rz-mtu3: Make use of devm_pwmchip_alloc() function pwm: samsung: Make use of devm_pwmchip_alloc() function pwm: sifive: Make use of devm_pwmchip_alloc() function pwm: sl28cpld: Make use of devm_pwmchip_alloc() function pwm: spear: Make use of devm_pwmchip_alloc() function pwm: sprd: Make use of devm_pwmchip_alloc() function pwm: sti: Make use of devm_pwmchip_alloc() function pwm: stm32-lp: Make use of devm_pwmchip_alloc() function pwm: stm32: Make use of devm_pwmchip_alloc() function pwm: stmpe: Make use of devm_pwmchip_alloc() function pwm: sun4i: Make use of devm_pwmchip_alloc() function pwm: sunplus: Make use of devm_pwmchip_alloc() function pwm: tegra: Make use of devm_pwmchip_alloc() function pwm: tiecap: Make use of devm_pwmchip_alloc() function pwm: twl-led: Make use of devm_pwmchip_alloc() function pwm: twl: Make use of devm_pwmchip_alloc() function pwm: visconti: Make use of devm_pwmchip_alloc() function pwm: vt8500: Make use of devm_pwmchip_alloc() function pwm: xilinx: Make use of devm_pwmchip_alloc() function gpio: mvebu: Make use of devm_pwmchip_alloc() function drm/bridge: ti-sn65dsi86: Make use of devm_pwmchip_alloc() function leds: qcom-lpg: Make use of devm_pwmchip_alloc() function staging: greybus: pwm: Make use of devm_pwmchip_alloc() function pwm: Ensure that pwm_chips are allocated using pwmchip_alloc() pwm: Ensure a struct pwm has the same lifetime as its pwm_chip pwm: Ensure the memory backing a PWM chip isn't freed while used pwm: Make pwmchip_[sg]et_drvdata() a wrapper around dev_set_drvdata() WIP: pwm: Add support for pwmchip devices for faster and easier userspace access .../driver-api/driver-model/devres.rst | 1 + Documentation/driver-api/pwm.rst | 10 +- drivers/gpio/gpio-mvebu.c | 18 +- drivers/gpu/drm/bridge/ti-sn65dsi86.c | 31 +- drivers/leds/rgb/leds-qcom-lpg.c | 16 +- drivers/pwm/Kconfig | 4 - drivers/pwm/Makefile | 3 +- drivers/pwm/core.c | 958 ++++++++++++------ drivers/pwm/pwm-ab8500.c | 36 +- drivers/pwm/pwm-apple.c | 18 +- drivers/pwm/pwm-atmel-hlcdc.c | 35 +- drivers/pwm/pwm-atmel-tcb.c | 26 +- drivers/pwm/pwm-atmel.c | 37 +- drivers/pwm/pwm-bcm-iproc.c | 19 +- drivers/pwm/pwm-bcm-kona.c | 21 +- drivers/pwm/pwm-bcm2835.c | 19 +- drivers/pwm/pwm-berlin.c | 29 +- drivers/pwm/pwm-brcmstb.c | 17 +- drivers/pwm/pwm-clk.c | 27 +- drivers/pwm/pwm-clps711x.c | 17 +- drivers/pwm/pwm-crc.c | 22 +- drivers/pwm/pwm-cros-ec.c | 58 +- drivers/pwm/pwm-dwc-core.c | 25 +- drivers/pwm/pwm-dwc.c | 18 +- drivers/pwm/pwm-dwc.h | 9 +- drivers/pwm/pwm-ep93xx.c | 21 +- drivers/pwm/pwm-fsl-ftm.c | 48 +- drivers/pwm/pwm-hibvt.c | 25 +- drivers/pwm/pwm-img.c | 51 +- drivers/pwm/pwm-imx-tpm.c | 34 +- drivers/pwm/pwm-imx1.c | 20 +- drivers/pwm/pwm-imx27.c | 26 +- drivers/pwm/pwm-intel-lgm.c | 17 +- drivers/pwm/pwm-iqs620a.c | 37 +- drivers/pwm/pwm-jz4740.c | 36 +- drivers/pwm/pwm-keembay.c | 17 +- drivers/pwm/pwm-lp3943.c | 17 +- drivers/pwm/pwm-lpc18xx-sct.c | 35 +- drivers/pwm/pwm-lpc32xx.c | 21 +- drivers/pwm/pwm-lpss-pci.c | 10 +- drivers/pwm/pwm-lpss-platform.c | 10 +- drivers/pwm/pwm-lpss.c | 34 +- drivers/pwm/pwm-lpss.h | 1 - drivers/pwm/pwm-mediatek.c | 29 +- drivers/pwm/pwm-meson.c | 57 +- drivers/pwm/pwm-microchip-core.c | 17 +- drivers/pwm/pwm-mtk-disp.c | 25 +- drivers/pwm/pwm-mxs.c | 32 +- drivers/pwm/pwm-ntxec.c | 30 +- drivers/pwm/pwm-omap-dmtimer.c | 47 +- drivers/pwm/pwm-pca9685.c | 98 +- drivers/pwm/pwm-pxa.c | 19 +- drivers/pwm/pwm-raspberrypi-poe.c | 20 +- drivers/pwm/pwm-rcar.c | 25 +- drivers/pwm/pwm-renesas-tpu.c | 18 +- drivers/pwm/pwm-rockchip.c | 24 +- drivers/pwm/pwm-rz-mtu3.c | 39 +- drivers/pwm/pwm-samsung.c | 57 +- drivers/pwm/pwm-sifive.c | 30 +- drivers/pwm/pwm-sl28cpld.c | 13 +- drivers/pwm/pwm-spear.c | 18 +- drivers/pwm/pwm-sprd.c | 50 +- drivers/pwm/pwm-sti.c | 34 +- drivers/pwm/pwm-stm32-lp.c | 29 +- drivers/pwm/pwm-stm32.c | 53 +- drivers/pwm/pwm-stmpe.c | 58 +- drivers/pwm/pwm-sun4i.c | 38 +- drivers/pwm/pwm-sunplus.c | 17 +- drivers/pwm/pwm-tegra.c | 27 +- drivers/pwm/pwm-tiecap.c | 55 +- drivers/pwm/pwm-tiehrpwm.c | 72 +- drivers/pwm/pwm-twl-led.c | 58 +- drivers/pwm/pwm-twl.c | 50 +- drivers/pwm/pwm-visconti.c | 17 +- drivers/pwm/pwm-vt8500.c | 41 +- drivers/pwm/pwm-xilinx.c | 34 +- drivers/pwm/sysfs.c | 64 +- drivers/staging/greybus/pwm.c | 130 +-- include/linux/platform_data/x86/pwm-lpss.h | 4 +- include/linux/pwm.h | 39 +- include/uapi/linux/pwm.h | 23 + 81 files changed, 1870 insertions(+), 1555 deletions(-) create mode 100644 include/uapi/linux/pwm.h base-commit: 6530623212338fc0902e211ea624e790aacb00ef