Re: [PATCH v3 101/108] drm/bridge: ti-sn65dsi86: Make use of devm_pwmchip_alloc() function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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/ |

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux