diff mbox series

[3/3] ath6kl: Reduce scopes for two variables in ath6kl_sdio_power_on()

Message ID 33b08664-fa25-4cef-86b2-49f65b4369c9@web.de
State New
Headers show
Series ath6kl: Adjustments for ath6kl_sdio_power_on() | expand

Commit Message

Markus Elfring Sept. 21, 2024, 12:06 p.m. UTC
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 21 Sep 2024 13:30:48 +0200

Adjust the definitions for the local variables "func" and "ret"
so that the corresponding setting will be performed a bit later.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/wireless/ath/ath6kl/sdio.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

--
2.46.0

Comments

Jeff Johnson Sept. 23, 2024, 4:41 p.m. UTC | #1
On 9/21/2024 5:06 AM, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 21 Sep 2024 13:30:48 +0200
> 
> Adjust the definitions for the local variables "func" and "ret"
> so that the corresponding setting will be performed a bit later.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/net/wireless/ath/ath6kl/sdio.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath6kl/sdio.c b/drivers/net/wireless/ath/ath6kl/sdio.c
> index e4d15cc9b36c..689f83f6bce5 100644
> --- a/drivers/net/wireless/ath/ath6kl/sdio.c
> +++ b/drivers/net/wireless/ath/ath6kl/sdio.c
> @@ -503,17 +503,15 @@ static void ath6kl_sdio_irq_handler(struct sdio_func *func)
>  static int ath6kl_sdio_power_on(struct ath6kl *ar)
>  {
>  	struct ath6kl_sdio *ar_sdio = ath6kl_sdio_priv(ar);
> -	struct sdio_func *func = ar_sdio->func;
> -	int ret = 0;
> 
>  	if (!ar_sdio->is_disabled)
>  		return 0;
> 
>  	ath6kl_dbg(ATH6KL_DBG_BOOT, "sdio power on\n");
> -
> +	struct sdio_func *func = ar_sdio->func;
>  	sdio_claim_host(func);
> 
> -	ret = sdio_enable_func(func);
> +	int ret = sdio_enable_func(func);
>  	sdio_release_host(func);
>  	if (ret) {
>  		ath6kl_err("Unable to enable sdio func: %d)\n", ret);
> --
> 2.46.0

NAK

no maintainer wants to spend time on patches like this which bring no real
value to code that is not actively being maintained, and which violates the
established understanding that, except under certain recently established
criteria, declarations and code should not be interleaved.

please avoid sending any patches to ath* drivers that do not fix bugs.

/jeff
Markus Elfring Sept. 23, 2024, 5 p.m. UTC | #2
>> Adjust the definitions for the local variables "func" and "ret"
>> so that the corresponding setting will be performed a bit later.>> +++ b/drivers/net/wireless/ath/ath6kl/sdio.c
>> @@ -503,17 +503,15 @@ static void ath6kl_sdio_irq_handler(struct sdio_func *func)
>>  static int ath6kl_sdio_power_on(struct ath6kl *ar)
>>  {
>>  	struct ath6kl_sdio *ar_sdio = ath6kl_sdio_priv(ar);
>> -	struct sdio_func *func = ar_sdio->func;
>> -	int ret = 0;
>>
>>  	if (!ar_sdio->is_disabled)
>>  		return 0;
>>
>>  	ath6kl_dbg(ATH6KL_DBG_BOOT, "sdio power on\n");
>> -
>> +	struct sdio_func *func = ar_sdio->func;
>>  	sdio_claim_host(func);
>>
>> -	ret = sdio_enable_func(func);
>> +	int ret = sdio_enable_func(func);
>>  	sdio_release_host(func);
>>  	if (ret) {
>>  		ath6kl_err("Unable to enable sdio func: %d)\n", ret);
>> --
>> 2.46.0
>
> NAK
>
> no maintainer wants to spend time on patches like this which bring no real
> value to code that is not actively being maintained, and which violates the
> established understanding that, except under certain recently established
> criteria, declarations and code should not be interleaved.
…

Would you find other software design options more acceptable for further
collateral evolution?

1. Additional compound statements (by using extra curly brackets)

2. Moving a bit of source code into an additional function implementation


Regards,
Markus
Jeff Johnson Sept. 23, 2024, 6:21 p.m. UTC | #3
On 9/23/2024 10:00 AM, Markus Elfring wrote:
>>> Adjust the definitions for the local variables "func" and "ret"
>>> so that the corresponding setting will be performed a bit later.
> …
>>> +++ b/drivers/net/wireless/ath/ath6kl/sdio.c
>>> @@ -503,17 +503,15 @@ static void ath6kl_sdio_irq_handler(struct sdio_func *func)
>>>  static int ath6kl_sdio_power_on(struct ath6kl *ar)
>>>  {
>>>  	struct ath6kl_sdio *ar_sdio = ath6kl_sdio_priv(ar);
>>> -	struct sdio_func *func = ar_sdio->func;
>>> -	int ret = 0;
>>>
>>>  	if (!ar_sdio->is_disabled)
>>>  		return 0;
>>>
>>>  	ath6kl_dbg(ATH6KL_DBG_BOOT, "sdio power on\n");
>>> -
>>> +	struct sdio_func *func = ar_sdio->func;
>>>  	sdio_claim_host(func);
>>>
>>> -	ret = sdio_enable_func(func);
>>> +	int ret = sdio_enable_func(func);
>>>  	sdio_release_host(func);
>>>  	if (ret) {
>>>  		ath6kl_err("Unable to enable sdio func: %d)\n", ret);
>>> --
>>> 2.46.0
>>
>> NAK
>>
>> no maintainer wants to spend time on patches like this which bring no real
>> value to code that is not actively being maintained, and which violates the
>> established understanding that, except under certain recently established
>> criteria, declarations and code should not be interleaved.
> …
> 
> Would you find other software design options more acceptable for further
> collateral evolution?
> 
> 1. Additional compound statements (by using extra curly brackets)
> 
> 2. Moving a bit of source code into an additional function implementation

I cannot speak for other maintainers, only myself. For myself I am far more
interested in changes which fix actual errors or warnings, and far more
interested in changes to code that is actually being widely used and where
active development is occurring, as opposed to drivers that have no ongoing
development and little deployment. At this time the ath.git maintainers are
100% focused on the ath12k MLO feature, and anything other than bug fixes to
ath12k or other ath drivers will be ignored.

/jeff
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath6kl/sdio.c b/drivers/net/wireless/ath/ath6kl/sdio.c
index e4d15cc9b36c..689f83f6bce5 100644
--- a/drivers/net/wireless/ath/ath6kl/sdio.c
+++ b/drivers/net/wireless/ath/ath6kl/sdio.c
@@ -503,17 +503,15 @@  static void ath6kl_sdio_irq_handler(struct sdio_func *func)
 static int ath6kl_sdio_power_on(struct ath6kl *ar)
 {
 	struct ath6kl_sdio *ar_sdio = ath6kl_sdio_priv(ar);
-	struct sdio_func *func = ar_sdio->func;
-	int ret = 0;

 	if (!ar_sdio->is_disabled)
 		return 0;

 	ath6kl_dbg(ATH6KL_DBG_BOOT, "sdio power on\n");
-
+	struct sdio_func *func = ar_sdio->func;
 	sdio_claim_host(func);

-	ret = sdio_enable_func(func);
+	int ret = sdio_enable_func(func);
 	sdio_release_host(func);
 	if (ret) {
 		ath6kl_err("Unable to enable sdio func: %d)\n", ret);