On Thu, Aug 24, 2023 at 09:32:06AM +0200, Bartosz Golaszewski wrote: > On Tue, Aug 22, 2023 at 9:08 AM Bartosz Golaszewski <brgl@xxxxxxxx> wrote: > > > > On Tue, Aug 22, 2023 at 2:21 AM Kent Gibson <warthog618@xxxxxxxxx> wrote: > > > > > > On Mon, Aug 21, 2023 at 05:33:39PM +0200, Bartosz Golaszewski wrote: > > > > From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx> > > > > > > > > gpio-mockup relies on the GPIO devices being registered in module's __init > > > > function and them being unregistered in __exit. This works with the GPIO > > > > subsystem as it only takes a reference to the underlying owner module when > > > > a GPIO descriptor is requested and not when the GPIO device is > > > > instantiated. > > > > > > > > This behavior may change in the future in the kernel so make the behavior > > > > of libgpiomockup more correct and have it unbind all mockup devices over > > > > sysfs before unloading the module. > > > > > > > > > > Never knew that unbinding was even an option. > > > Maybe update gpio-mockup's documentation? > > > > > > > Yeah, I might once we agree on that reference counting patch. > > > > > Just clarifying what the potential impact of the existing libgpiomockup > > > behaviour and future kernel behaviour is - the kernel may log errors but > > > otherwise correctly handle userspace unloading behaving badly? > > > So this patch is pre-emptory noise reduction? > > > > > > > No, it's a bug-fix-in-advance. gpio-mockup will fail to unload (until > > we unbind all devices anyway) if we couple the module's reference with > > struct gpio_device. So will every driver that registers devices from > > its module_init() function and tears them down in module_exit(). But > > these drivers are wrong to do so in the first place and unloading them > > sound like a rare thing to do anyway, so I'm willing to give it a try. > > > > Bartosz > > So what do you think Kent? Does it make sense to have it in v1.6? I > would need to make a new bugfix release but I have something else > queued anyway. > If the plan is to change the kernel such that it will no longer unload modules with bound devices then the patch totally makes sense. Cheers, Kent.