On 8/13/2024 4:22 PM, Ulf Hansson wrote: > On Wed, 7 Aug 2024 at 09:16, Dikshita Agarwal <quic_dikshita@xxxxxxxxxxx> wrote: >> >> Add the devres-enabled version of dev_pm_domain_attach|detach_list. >> If client drivers use devm_pm_domain_attach_list() to attach the >> PM domains, devm_pm_domain_detach_list() will be invoked implicitly >> during remove phase. >> >> Signed-off-by: Dikshita Agarwal <quic_dikshita@xxxxxxxxxxx> >> --- >> drivers/base/power/common.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ >> include/linux/pm_domain.h | 13 +++++++++++++ >> 2 files changed, 57 insertions(+) >> >> diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c >> index 327d168..729d6c2 100644 >> --- a/drivers/base/power/common.c >> +++ b/drivers/base/power/common.c >> @@ -277,6 +277,50 @@ int dev_pm_domain_attach_list(struct device *dev, >> EXPORT_SYMBOL_GPL(dev_pm_domain_attach_list); >> >> /** >> + * devm_pm_domain_detach_list - devres-enabled version of dev_pm_domain_detach_list. >> + * @_list: The list of PM domains to detach. >> + * >> + * This function reverse the actions from devm_pm_domain_attach_list(). >> + * it will be invoked during the remove phase from drivers implicitly if driver >> + * uses devm_pm_domain_attach_list() to attach the PM domains. >> + */ >> +void devm_pm_domain_detach_list(void *_list) >> +{ >> + struct dev_pm_domain_list *list = _list; >> + >> + dev_pm_domain_detach_list(list); >> +} >> +EXPORT_SYMBOL_GPL(devm_pm_domain_detach_list); > > I think this function should be internal and hence made static - > unless there is a good reason to export it? Yeah, it should be static and no need of exporting it. Will make the changes. > >> + >> +/** >> + * devm_pm_domain_attach_list - devres-enabled version of dev_pm_domain_attach_list >> + * @dev: The device used to lookup the PM domains for. >> + * @data: The data used for attaching to the PM domains. >> + * @list: An out-parameter with an allocated list of attached PM domains. >> + * >> + * NOTE: this will also handle calling devm_pm_domain_detach_list() for >> + * you during remove phase. >> + * >> + * Returns the number of attached PM domains or a negative error code in case of >> + * a failure. >> + */ >> +int devm_pm_domain_attach_list(struct device *dev, >> + const struct dev_pm_domain_attach_data *data, >> + struct dev_pm_domain_list **list) >> +{ >> + int ret, num_pds = 0; > > There is no need to initialize num_pds to 0 here, as the below calls > take care of it. > Right, will be fixed in next revision. >> + >> + num_pds = dev_pm_domain_attach_list(dev, data, list); >> + > > We should add a check if num_pds is zero here, as in that case there > is no reason to add a devres callback for it. > Sure, will add the below check. if (!num_pds) return 0; Thanks, Dikshita >> + ret = devm_add_action_or_reset(dev, devm_pm_domain_detach_list, *list); >> + if (ret) >> + return ret; >> + >> + return num_pds; >> +} >> +EXPORT_SYMBOL_GPL(devm_pm_domain_attach_list); > > [...] > > Kind regards > Uffe