Re: [PATCH 18/18] gpio: mvebu: Make use of devm_pwmchip_alloc() function

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 2218 bytes --]

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@pengutronix.de> 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]