09.11.2020 08:10, Viresh Kumar пишет:

On 09-11-20, 08:08, Dmitry Osipenko wrote:

quoted

09.11.2020 08:00, Viresh Kumar пишет:

quoted

On 06-11-20, 21:41, Frank Lee wrote:

quoted

On Fri, Nov 6, 2020 at 9:18 PM Dmitry Osipenko [off-list ref] wrote:

quoted

06.11.2020 09:15, Viresh Kumar пишет:

quoted

Setting regulators for count as 0 doesn't sound good to me.

But, I understand that you don't want to have that if (have_regulator)
check, and it is a fair request. What I will instead do is, allow all
dev_pm_opp_put*() API to start accepting a NULL pointer for the OPP
table and fail silently. And so you won't be required to have this
unwanted check. But you will be required to save the pointer returned
back by dev_pm_opp_set_regulators(), which is the right thing to do
anyways.

Perhaps even a better variant could be to add a devm versions of the OPP
API functions, then drivers won't need to care about storing the
opp_table pointer if it's unused by drivers.

I think so. The consumer may not be so concerned about the status of
these OPP tables.
If the driver needs to manage the release, it needs to add a pointer
to their driver global structure.

Maybe it's worth having these devm interfaces for opp.

Sure if there are enough users of this, I am all for it. I was fine
with the patches you sent, just that there were not a lot of users of
it and so I pushed them back. If we find that we have more users of it
now, we can surely get that back.

There was already attempt to add the devm? Could you please give me a
link to the patches?

I already prepared a patch which adds the devm helpers. It helps to keep
code cleaner and readable.

https://lore.kernel.org/lkml/20201012135517.19468-1-frank@allwinnertech.com/ (local)

Thanks, I made it in a different way by simply adding helpers to the
pm_opp.h which use devm_add_action_or_reset(). This doesn't require to
add new kernel symbols.

static inline int devm_pm_opp_of_add_table(struct device *dev)
{
	int err;

	err = dev_pm_opp_of_add_table(dev);
	if (err)
		return err;

	err = devm_add_action_or_reset(dev, (void*)dev_pm_opp_remove_table,
				       dev);
	if (err)
		return err;

	return 0;
}