lkml.org 
[lkml]   [2023]   [Jul]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v3 3/4] pinctrl: Implementation of the generic scmi-pinctrl driver
Date

Hi Andy,

andy.shevchenko@gmail.com writes:

> Tue, Jun 06, 2023 at 04:22:28PM +0000, Oleksii Moisieiev kirjoitti:
>> scmi-pinctrl driver implements pinctrl driver interface and using
>> SCMI protocol to redirect messages from pinctrl subsystem SDK to
>> SCP firmware, which does the changes in HW.
>>
>> This setup expects SCP firmware (or similar system, such as ATF)
>> to be installed on the platform, which implements pinctrl driver
>> for the specific platform.
>>
>> SCMI-Pinctrl driver should be configured from the device-tree and uses
>> generic device-tree mappings for the configuration.

[snip]

> ...
>
>> +error:
>
> Labels shoud be self-explanatory, i.e. they should tell what _will_ be when goto.
>
>> + 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.
```

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?
Maybe I've misunderstood your point.

--
Thanks,
Oleksii
\
 
 \ /
  Last update: 2023-07-20 15:41    [W:0.407 / U:1.380 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site