Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <7bd4f2fb-70ce-3724-130f-f5c75ee7bde2@kernel.org>
Date:   Mon, 18 Jul 2022 12:10:34 +0300
MIME-Version: 1.0
Subject: Re: [PATCH 7/8] interconnect: Make icc_provider_del() return void
Content-Language: en-US
To:     =?UTF-8?Q?Uwe_Kleine-K=c3=b6nig?= <u.kleine-koenig@pengutronix.de>
Cc:     kernel@pengutronix.de, linux-pm@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20220715203652.89912-1-u.kleine-koenig@pengutronix.de>
 <20220715203652.89912-8-u.kleine-koenig@pengutronix.de>
From:   Georgi Djakov <djakov@kernel.org>
In-Reply-To: <20220715203652.89912-8-u.kleine-koenig@pengutronix.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


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 <u.kleine-koenig@pengutronix.de>
> ---
>   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