On Wed, Feb 14, 2024 at 10:30:50AM +0100, 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 pwmchip_alloc(). ... > +#define PWMCHIP_ALIGN ARCH_DMA_MINALIGN > + > +static void *pwmchip_priv(struct pwm_chip *chip) > +{ > + return (void *)chip + ALIGN(sizeof(*chip), PWMCHIP_ALIGN); > +} Why not use dma_get_cache_alignment() ? ... > +/* This is the counterpart to pwmchip_alloc */ pwmchip_alloc() ... > +EXPORT_SYMBOL_GPL(pwmchip_put); > +EXPORT_SYMBOL_GPL(pwmchip_alloc); > +EXPORT_SYMBOL_GPL(devm_pwmchip_alloc); Are these exported via namespace? If no, can they be from day 1? ... > +static inline void pwmchip_put(struct pwm_chip *chip) > +{ > +} Can be one line, but it's up to the present style in this header. -- With Best Regards, Andy Shevchenko