On Tue, Aug 20, 2024 at 10:23:06AM -0700, Stephen Boyd wrote: > Quoting Stephen Boyd (2024-08-20 10:19:48) > > Quoting Andy Shevchenko (2024-08-20 10:16:40) > > > On Tue, Aug 20, 2024 at 10:01:07AM -0700, Stephen Boyd wrote: > > > > Quoting Andy Shevchenko (2024-08-20 03:16:09) > > > > > On Mon, Aug 19, 2024 at 03:38:19PM -0700, Stephen Boyd wrote: ... > > > > > > + ptr = devres_alloc(devm_typec_switch_unregister, sizeof(*ptr), GFP_KERNEL); > > > > > > + if (!ptr) > > > > > > + return ERR_PTR(-ENOMEM); > > > > > > + > > > > > > + switch_dev = typec_switch_register(parent ,desc); > > > > > > (Side note: wrong location of the white space) > > > > Thanks. > > > > > > > > > > > + if (!IS_ERR(switch_dev)) { > > > > > > (Side note: positive conditional is okay) > > > > > > > > > + *ptr = switch_dev; > > > > > > + devres_add(parent, ptr); > > > > > > + } else { > > > > > > + devres_free(ptr); > > > > > > + } > > > > > > > > > > devm_add_action_or_reset() ? > > > > > > > > No. We don't want to call the 'action' devm_typec_switch_unregister() > > > > when it fails because that would unregister a switch that has never been > > > > registered. > > > > > > Hmm... With devm_add_action_or_reset() we first do things and then try to add > > > them to the managed resources. In that case it won't be like you described. > > > > > > What do I miss? > > > > I believe you've missed that this is a wrapper around struct device and > > the error path is different (put_device() vs. device_unregister()). > > Or you're suggesting devm_add_action_or_reset() after registering the > switch? Sorry it wasn't clear to me. Yes, after successful switch registration. -- With Best Regards, Andy Shevchenko