On Thu, Jul 20, 2023 at 4:40 PM Oleksii Moisieiev <Oleksii_Moisieiev@xxxxxxxx> wrote: > andy.shevchenko@xxxxxxxxx writes: > > Tue, Jun 06, 2023 at 04:22:28PM +0000, Oleksii Moisieiev kirjoitti: ... > >> + devm_kfree(pmx->dev, pmx->functions[selector].groups); > > > > Red Flag. Please, elaborate. > > Thank you for the review. > I did some research regarding this and now I'm confused. Could you > please explain to me why it's a red flag? > IIUC devm_alloc/free functions are the calls to the resource-managed > alloc/free command, which is bound to the device. > pinctrl-scmi driver does devm_pinctrl_register_and_init which does > devres_alloc and doesn't open devres_group like > scmi_alloc_init_protocol_instance (thanks to Cristian detailed > explanation). > > As was mentioned in Documentation/driver-api/driver-model/devres.rst: > > ``` > No matter what, all devres entries are released on driver detach. On > release, the associated release function is invoked and then the > devres entry is freed. > ``` Precisely. So, why do you intervene in this? > Also there is devm_pinctrl_get call listed in the managed interfaces. > > My understanding is that all resources, bound to the particular device > will be freed on driver detach. > > Also I found some examples of using devm_alloc/free like from dt_node_to_map > call in pinctrl-simple.c driver. > > I agree that I need to implement .remove callback with proper cleanup, > but why can't I use devm_* here? You can use devm_*(), but what's the point if you call release yourself? That's quite a red flag usually shows a bigger issue (misunderstanding of the objects lifetimes and their interaction). > Maybe I've misunderstood your point. -- With Best Regards, Andy Shevchenko
