Hello Bart, On Sun, Jul 30, 2023 at 12:07:33PM +0200, Bartosz Golaszewski wrote: > On Sat, Jul 29, 2023 at 11:37 PM Uwe Kleine-König > <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > > On Sat, Jul 29, 2023 at 04:09:40PM +0200, Bartosz Golaszewski wrote: > > > Looks good to me (although I have my reservations about the concept of > > > foo_alloc() for subsystems in the kernel...). > > > > Wolfram's EOSS talk[1] mentioned "__cleanup__ + kref as suggested by Bartosz? > > Paradigm shift, probably looong way to go". I guess that's what you'd > > prefer? Do you have a link for me to read about this? > > > > For now I prefer the gpiolib model. One structure allocated and > controlled by the driver (struct gpio_chip) which needs to live only > as long as the device is bound to a driver and a second structure > private to the subsystem, allocated and controlled by the subsystem > (struct gpio_device) which also contains the referenced counted struct > device and is only released by the device's release callback. > > IMO there shouldn't be any need for PWM drivers to dereference struct > device held by struct pwm_chip. If anything - it should be passed to > the drivers in subsystem callbacks. > > I may be wrong of course, I don't know this subsystem very well but it > seems to follow a pattern that's pretty common in the kernel and > causes ownership confusion. A difficulty I see is that as of now the ops-pointer is maintained in driver-allocated data. So it's not possible to call the .free callback once the driver is gone. So either unbinding the driver must be delayed until all consumers are gone, or the reference to a PWM that a consumer holds must be invalidated. Both options are not optimal. But I have to admit that I didn't grasp the device core completely (yet?), so I might well miss something. Also I like the concept of "..._alloc" and find it clear enough. I'm not aware of "ownership confusions". I'm open to hear about these if you have something to point out. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
