Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <a68eae4c-a991-ff15-3800-86d39b4e4868@kernel.org>
Date:   Fri, 14 Jul 2023 08:13:40 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH v4] wifi: ath5k: remove parameter check in
 ath5k_debug_init_device()
Content-Language: en-US
To:     Minjie Du <duminjie@vivo.com>,
        Nick Kossifidis <mickflemm@gmail.com>,
        Luis Chamberlain <mcgrof@kernel.org>,
        Kalle Valo <kvalo@kernel.org>,
        "open list:ATHEROS ATH5K WIRELESS DRIVER" 
        <linux-wireless@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Cc:     opensource.kernel@vivo.com
References: <20230713082433.11485-1-duminjie@vivo.com>
From:   Jiri Slaby <jirislaby@kernel.org>
In-Reply-To: <20230713082433.11485-1-duminjie@vivo.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 13. 07. 23, 10:24, Minjie Du wrote:
> The parameter 'phydir' error check is mistake.

It's not actually a parameter.

> debugfs_create_file() will return early if 'phydir' is an error pointer,
> so the error check is removed.

What about something like:
Subject: wifi: ath5k: remove phydir check from ath5k_debug_init_device()

'phydir' returned from debugfs_create_dir() is checked against NULL. As 
the debugfs API returns an error pointer, the returned value can never 
be NULL.

So instead, remove the check completely as it is unneeded as per 
documentation. (Other debugfs calls are a nop in the error case.)

?

Or something like that, feel free to reword.


> Signed-off-by: Minjie Du <duminjie@vivo.com>
> ---
> v1: use IS_ERR() to error check.
> v2: use IS_ERR_OR_NULL() to error check.
> v3: remove the error check.
> v4: fix the patch format.
> ---
>   drivers/net/wireless/ath/ath5k/debug.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath5k/debug.c b/drivers/net/wireless/ath/ath5k/debug.c
> index 4b41160e5..ec130510a 100644
> --- a/drivers/net/wireless/ath/ath5k/debug.c
> +++ b/drivers/net/wireless/ath/ath5k/debug.c
> @@ -982,8 +982,6 @@ ath5k_debug_init_device(struct ath5k_hw *ah)
>   	ah->debug.level = ath5k_debug;
>   
>   	phydir = debugfs_create_dir("ath5k", ah->hw->wiphy->debugfsdir);
> -	if (!phydir)
> -		return;
>   
>   	debugfs_create_file("debug", 0600, phydir, ah, &fops_debug);
>   	debugfs_create_file("registers", 0400, phydir, ah, &registers_fops);

-- 
js
suse labs