Re: [PATCH v1 2/4] PM / devfreq: Add devm_devfreq_add_governor()

From: Chanwoo Choi <cwchoi00@gmail.com>
To: Dmitry Osipenko <digetx@gmail.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	Viresh Kumar <vireshk@kernel.org>, Nishanth Menon <nm@ti.com>,
	MyungJoo Ham <myungjoo.ham@samsung.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Chanwoo Choi <cw00.choi@samsung.com>
Cc: linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-pm@vger.kernel.org
Subject: Re: [PATCH v1 2/4] PM / devfreq: Add devm_devfreq_add_governor()
Date: Thu, 16 Sep 2021 03:23:34 +0900	[thread overview]
Message-ID: <504a75a9-3f39-3aed-2df5-4ca1e7f99afc@gmail.com> (raw)
In-Reply-To: <20210912184458.17995-3-digetx@gmail.com>

On 21. 9. 13. 오전 3:44, Dmitry Osipenko wrote:
> Add resource-managed variant of devfreq_add_governor().
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>   drivers/devfreq/devfreq.c  | 26 ++++++++++++++++++++++++++
>   drivers/devfreq/governor.h |  3 +++
>   2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 85faa7a5c7d1..d3af000ec290 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -1301,6 +1301,32 @@ int devfreq_add_governor(struct devfreq_governor *governor)
>   }
>   EXPORT_SYMBOL(devfreq_add_governor);
>   
> +static void devm_devfreq_remove_governor(void *governor)
> +{
> +	devfreq_remove_governor(governor);

Because devfreq_remove_governor has the return value,
you need to check the return value and then print error at least.

	WARN_ON(devfreq_remove_governor(governor));

> +}
> +
> +/**
> + * devm_devfreq_add_governor() - Add devfreq governor
> + * @dev:	device which adds devfreq governor
> + * @governor:	the devfreq governor to be added
> + *
> + * This is a resource-managed variant of devfreq_add_governor().
> + */
> +int devm_devfreq_add_governor(struct device *dev,
> +			      struct devfreq_governor *governor)
> +{
> +	int err;
> +
> +	err = devfreq_add_governor(governor);
> +	if (err)
> +		return err;
> +
> +	return devm_add_action_or_reset(dev, devm_devfreq_remove_governor,
> +					governor);
> +}
> +EXPORT_SYMBOL(devm_devfreq_add_governor);
> +
>   /**
>    * devfreq_remove_governor() - Remove devfreq feature from a device.
>    * @governor:	the devfreq governor to be removed
> diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
> index 2d69a0ce6291..0d70a9ad951e 100644
> --- a/drivers/devfreq/governor.h
> +++ b/drivers/devfreq/governor.h
> @@ -94,4 +94,7 @@ static inline int devfreq_update_stats(struct devfreq *df)
>   
>   	return df->profile->get_dev_status(df->dev.parent, &df->last_status);
>   }
> +
> +int devm_devfreq_add_governor(struct device *dev,
> +			      struct devfreq_governor *governor);

Better to add under devfreq_remove_governor definition in order to 
gather the similar functions.

>   #endif /* _GOVERNOR_H */
> 

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi