On Wed, Jul 05, 2023 at 05:47:45AM -0500, Besar Wicaksono wrote: > Arm Coresight PMU driver consists of main standard code and > vendor backend code. Both are currently built as a single module. > This patch adds vendor registration API to separate the two to > keep things modular. The main driver requests each known backend > module during initialization and defer device binding process. > The backend module then registers an init callback to the main > driver and continue the device driver binding process. > > Signed-off-by: Besar Wicaksono <bwicaksono@xxxxxxxxxx> > --- > > Changes from v4: > * Fix warning reported by kernel test robot > v4: https://lore.kernel.org/linux-arm-kernel/20230620041438.32514-1-bwicaksono@xxxxxxxxxx/T/#u One minor comment below, but this mostly looks good to me. I'd like Suzuki's Ack before I queue it, though. > + /* Load implementer module and initialize the callbacks. */ > + if (match) { > + mutex_lock(&arm_cspmu_lock); > + > + if (match->impl_init_ops) { > + if (try_module_get(match->module)) { > + cspmu->impl.match = match; > + ret = match->impl_init_ops(cspmu); > + module_put(match->module); Why is it safe to drop the module reference here? If I'm understanding the flow correctly, ->impl_init_ops() will populate more function pointers in the cspmu->impl.ops structure, and we don't appear to take a module reference when calling those. What happens if the backend module is unloaded while the core module is executed those functions? Cheers, Will
