On Wed, Dec 06, 2023 at 02:06:11PM +0200, Laurent Pinchart wrote:
> On Thu, Nov 23, 2023 at 11:10:18AM +0100, Uwe Kleine-König wrote:
> > Once the series is completely applied, the pwm_chip isn't allocated
> > using devm_kzalloc any more. You're only looking at an intermediate
> > state where I push the broken lifetime tracking from all drivers into a
> > single function in the core that is then fixed after all drivers are
> > converted to it.
> 
> Indeed, I missed that devm_pwm_alloc() got changed later in the series
> to not call devm_kzalloc(). The naming is quite unfortunate, a
> devm_*_alloc() function really gives a strong hint that the
> corresponding cleanup at device remove time will be a free(), not a
> put() :-S That's pretty confusing for the readers.

Note there is a v4 in the meantime. My suggestion to rename
pwmchip_alloc() to pwmchip_get_new() could address this concern. Would
you like that? I didn't get any feedback about it when I suggested it
somewhere in the v3 thread. (I'm not sure I like it, given that
foo_alloc() is quite usual for other subsystems.)

> > If you find issues with the complete series applied, please tell me.
> 
> One thing I still dislike is forcing drivers to dynamically allocate the
> pwm_chip series.

A struct pwm_chip must be allocated dynamically as it's reference
counted by a struct device. Given that nearly all drivers allocate their
driver data dynamically, too, this isn't a big issue IMO.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |