All users ignore the return value of icc_provider_del(). Consequently
make it not return an error code.
Signed-off-by: Uwe Kleine-König <[email protected]>
---
drivers/interconnect/core.c | 10 +++-------
include/linux/interconnect-provider.h | 2 +-
2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index 808f6e7a8048..25debded65a8 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -1057,29 +1057,25 @@ EXPORT_SYMBOL_GPL(icc_provider_add);
/**
* icc_provider_del() - delete previously added interconnect provider
* @provider: the interconnect provider that will be removed from topology
- *
- * Return: 0 on success, or an error code otherwise
*/
-int icc_provider_del(struct icc_provider *provider)
+void icc_provider_del(struct icc_provider *provider)
{
mutex_lock(&icc_lock);
if (provider->users) {
pr_warn("interconnect provider still has %d users\n",
provider->users);
mutex_unlock(&icc_lock);
- return -EBUSY;
+ return;
}
if (!list_empty(&provider->nodes)) {
pr_warn("interconnect provider still has nodes\n");
mutex_unlock(&icc_lock);
- return -EBUSY;
+ return;
}
list_del(&provider->provider_list);
mutex_unlock(&icc_lock);
-
- return 0;
}
EXPORT_SYMBOL_GPL(icc_provider_del);
diff --git a/include/linux/interconnect-provider.h b/include/linux/interconnect-provider.h
index 6bd01f7159c6..191f083d1f3b 100644
--- a/include/linux/interconnect-provider.h
+++ b/include/linux/interconnect-provider.h
@@ -123,7 +123,7 @@ void icc_node_add(struct icc_node *node, struct icc_provider *provider);
void icc_node_del(struct icc_node *node);
int icc_nodes_remove(struct icc_provider *provider);
int icc_provider_add(struct icc_provider *provider);
-int icc_provider_del(struct icc_provider *provider);
+void icc_provider_del(struct icc_provider *provider);
struct icc_node_data *of_icc_get_from_provider(struct of_phandle_args *spec);
void icc_sync_state(struct device *dev);
--
2.36.1
Hi Uwe,
Thanks for the patchset!
On 15.07.22 23:36, Uwe Kleine-König wrote:
> All users ignore the return value of icc_provider_del(). Consequently
> make it not return an error code.
>
> Signed-off-by: Uwe Kleine-König <[email protected]>
> ---
> drivers/interconnect/core.c | 10 +++-------
> include/linux/interconnect-provider.h | 2 +-
> 2 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> index 808f6e7a8048..25debded65a8 100644
> --- a/drivers/interconnect/core.c
> +++ b/drivers/interconnect/core.c
> @@ -1057,29 +1057,25 @@ EXPORT_SYMBOL_GPL(icc_provider_add);
> /**
> * icc_provider_del() - delete previously added interconnect provider
> * @provider: the interconnect provider that will be removed from topology
> - *
> - * Return: 0 on success, or an error code otherwise
> */
> -int icc_provider_del(struct icc_provider *provider)
> +void icc_provider_del(struct icc_provider *provider)
> {
> mutex_lock(&icc_lock);
> if (provider->users) {
> pr_warn("interconnect provider still has %d users\n",
> provider->users);
> mutex_unlock(&icc_lock);
> - return -EBUSY;
> + return;
> }
Looks like provider->users is now useless, so we should remove it. But that could be a separate
clean-up.
>
> if (!list_empty(&provider->nodes)) {
> pr_warn("interconnect provider still has nodes\n");
> mutex_unlock(&icc_lock);
> - return -EBUSY;
> + return;
> }
>
> list_del(&provider->provider_list);
> mutex_unlock(&icc_lock);
> -
> - return 0;
> }
> EXPORT_SYMBOL_GPL(icc_provider_del);
>
> diff --git a/include/linux/interconnect-provider.h b/include/linux/interconnect-provider.h
> index 6bd01f7159c6..191f083d1f3b 100644
> --- a/include/linux/interconnect-provider.h
> +++ b/include/linux/interconnect-provider.h
> @@ -123,7 +123,7 @@ void icc_node_add(struct icc_node *node, struct icc_provider *provider);
> void icc_node_del(struct icc_node *node);
> int icc_nodes_remove(struct icc_provider *provider);
> int icc_provider_add(struct icc_provider *provider);
> -int icc_provider_del(struct icc_provider *provider);
> +void icc_provider_del(struct icc_provider *provider);
> struct icc_node_data *of_icc_get_from_provider(struct of_phandle_args *spec);
> void icc_sync_state(struct device *dev);
We should also squash the following:
--- a/include/linux/interconnect-provider.h
+++ b/include/linux/interconnect-provider.h
@@ -172,7 +172,7 @@ static inline int icc_provider_add(struct icc_provider *provider)
return -ENOTSUPP;
}
-static inline int icc_provider_del(struct icc_provider *provider)
+static inline void icc_provider_del(struct icc_provider *provider)
{
return -ENOTSUPP;
}
Thanks,
Georgi
On Mon, Jul 18, 2022 at 12:10:34PM +0300, Georgi Djakov wrote:
>
> Hi Uwe,
>
> Thanks for the patchset!
>
> On 15.07.22 23:36, Uwe Kleine-K?nig wrote:
> > All users ignore the return value of icc_provider_del(). Consequently
> > make it not return an error code.
> >
> > Signed-off-by: Uwe Kleine-K?nig <[email protected]>
> > ---
> > drivers/interconnect/core.c | 10 +++-------
> > include/linux/interconnect-provider.h | 2 +-
> > 2 files changed, 4 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> > index 808f6e7a8048..25debded65a8 100644
> > --- a/drivers/interconnect/core.c
> > +++ b/drivers/interconnect/core.c
> > @@ -1057,29 +1057,25 @@ EXPORT_SYMBOL_GPL(icc_provider_add);
> > /**
> > * icc_provider_del() - delete previously added interconnect provider
> > * @provider: the interconnect provider that will be removed from topology
> > - *
> > - * Return: 0 on success, or an error code otherwise
> > */
> > -int icc_provider_del(struct icc_provider *provider)
> > +void icc_provider_del(struct icc_provider *provider)
> > {
> > mutex_lock(&icc_lock);
> > if (provider->users) {
> > pr_warn("interconnect provider still has %d users\n",
> > provider->users);
> > mutex_unlock(&icc_lock);
> > - return -EBUSY;
> > + return;
> > }
>
> Looks like provider->users is now useless, so we should remove it. But that
> could be a separate clean-up.
Well, it's still used to emit the warning. If this can trigger there is
indeed a problem though. If there are still users, they should hold a
reference to the device preventing its release.
> > if (!list_empty(&provider->nodes)) {
> > pr_warn("interconnect provider still has nodes\n");
> > mutex_unlock(&icc_lock);
> > - return -EBUSY;
> > + return;
> > }
> > list_del(&provider->provider_list);
> > mutex_unlock(&icc_lock);
> > -
> > - return 0;
> > }
> > EXPORT_SYMBOL_GPL(icc_provider_del);
> > diff --git a/include/linux/interconnect-provider.h b/include/linux/interconnect-provider.h
> > index 6bd01f7159c6..191f083d1f3b 100644
> > --- a/include/linux/interconnect-provider.h
> > +++ b/include/linux/interconnect-provider.h
> > @@ -123,7 +123,7 @@ void icc_node_add(struct icc_node *node, struct icc_provider *provider);
> > void icc_node_del(struct icc_node *node);
> > int icc_nodes_remove(struct icc_provider *provider);
> > int icc_provider_add(struct icc_provider *provider);
> > -int icc_provider_del(struct icc_provider *provider);
> > +void icc_provider_del(struct icc_provider *provider);
> > struct icc_node_data *of_icc_get_from_provider(struct of_phandle_args *spec);
> > void icc_sync_state(struct device *dev);
>
> We should also squash the following:
>
> --- a/include/linux/interconnect-provider.h
> +++ b/include/linux/interconnect-provider.h
> @@ -172,7 +172,7 @@ static inline int icc_provider_add(struct icc_provider *provider)
> return -ENOTSUPP;
> }
>
> -static inline int icc_provider_del(struct icc_provider *provider)
> +static inline void icc_provider_del(struct icc_provider *provider)
> {
> return -ENOTSUPP;
> }
Sent a v2 with this change (and also removed the return statement).
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |