[PATCH] staging: vt6655: refactor byMinChannel to min_channel

[PATCH] staging: vt6655: refactor byMinChannel to min_channel

[Alberto Merciai]

Replace camelcase hungarian notated variable "byMinChannel"
into linux kernel coding style equivalent variable "min_channel".

Signed-off-by: Alberto Merciai <alb3rt0.m3rciai@gmail.com>
---
 drivers/staging/vt6655/device.h      | 2 +-
 drivers/staging/vt6655/device_main.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/vt6655/device.h b/drivers/staging/vt6655/device.h
index 6ea4165e4929..109d20a010ab 100644
--- a/drivers/staging/vt6655/device.h
+++ b/drivers/staging/vt6655/device.h
@@ -200,7 +200,7 @@ struct vnt_private {
 	unsigned char byTopOFDMBasicRate;
 	unsigned char byTopCCKBasicRate;
 
-	unsigned char byMinChannel;
+	unsigned char min_channel;
 	unsigned char max_channel;
 
 	unsigned char preamble_type;
diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
index 313f0241d989..4d92b5df5312 100644
--- a/drivers/staging/vt6655/device_main.c
+++ b/drivers/staging/vt6655/device_main.c
@@ -228,7 +228,7 @@ static void device_init_registers(struct vnt_private *priv)
 	spin_unlock_irqrestore(&priv->lock, flags);
 
 	/* Get Channel range */
-	priv->byMinChannel = 1;
+	priv->min_channel = 1;
 	priv->max_channel = CB_MAX_CHANNEL;
 
 	/* Get Antena */
-- 
2.25.1

Re: [PATCH] staging: vt6655: refactor byMinChannel to min_channel

[Pavel Skripkin]

On 12/4/21 03:40, Alberto Merciai wrote:
> Replace camelcase hungarian notated variable "byMinChannel"
> into linux kernel coding style equivalent variable "min_channel".
> 
> Signed-off-by: Alberto Merciai <alb3rt0.m3rciai@gmail.com>
> ---
>   drivers/staging/vt6655/device.h      | 2 +-
>   drivers/staging/vt6655/device_main.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/vt6655/device.h b/drivers/staging/vt6655/device.h
> index 6ea4165e4929..109d20a010ab 100644
> --- a/drivers/staging/vt6655/device.h
> +++ b/drivers/staging/vt6655/device.h
> @@ -200,7 +200,7 @@ struct vnt_private {
>   	unsigned char byTopOFDMBasicRate;
>   	unsigned char byTopCCKBasicRate;
>   
> -	unsigned char byMinChannel;
> +	unsigned char min_channel;
>   	unsigned char max_channel;
>   
>   	unsigned char preamble_type;
> diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
> index 313f0241d989..4d92b5df5312 100644
> --- a/drivers/staging/vt6655/device_main.c
> +++ b/drivers/staging/vt6655/device_main.c
> @@ -228,7 +228,7 @@ static void device_init_registers(struct vnt_private *priv)
>   	spin_unlock_irqrestore(&priv->lock, flags);
>   
>   	/* Get Channel range */
> -	priv->byMinChannel = 1;
> +	priv->min_channel = 1;
>   	priv->max_channel = CB_MAX_CHANNEL;
>   
>   	/* Get Antena */
> 

Looks like this variable is set, but never used. Shouldn't it be just 
removed?

Same for max_channel (or byMaxChannel).




With regards,
Pavel Skripkin

Re: [PATCH] staging: vt6655: refactor byMinChannel to min_channel

[Alberto Merciai]

On Sat, Dec 04, 2021 at 11:27:43AM +0300, Pavel Skripkin wrote:
> Looks like this variable is set, but never used. Shouldn't it be just
> removed?
> 
> Same for max_channel (or byMaxChannel).

To be honest, I would prefer leave them where they are. I'm only
refactoring the code considering less the logic aspect.

I don't know if these variables will become usefull in a second
moment.

Anyway, If you consider that this could be a good moment to remove
them we can proceed.

What do you think about that?

Thanks,
Alberto

Re: [PATCH] staging: vt6655: refactor byMinChannel to min_channel

[Pavel Skripkin]

On 12/4/21 12:09, Alberto Merciai wrote:
> On Sat, Dec 04, 2021 at 11:27:43AM +0300, Pavel Skripkin wrote:
>> Looks like this variable is set, but never used. Shouldn't it be just
>> removed?
>> 
>> Same for max_channel (or byMaxChannel).
> 
> To be honest, I would prefer leave them where they are. I'm only
> refactoring the code considering less the logic aspect.
> 
> I don't know if these variables will become usefull in a second
> moment.
> 
> Anyway, If you consider that this could be a good moment to remove
> them we can proceed.
> 
> What do you think about that?
> 

I think, it should be up to maintainers to decide, but them can be 
removed now and if somebody will need them later, he can bring them back 
without any pain. For now it's just useless memory usage.


I skimmed through priv fields usage and looks like many of them are just 
set, but never used. I guess, some kind of evolution removed all uses of 
them...




With regards,
Pavel Skripkin

Re: [PATCH] staging: vt6655: refactor byMinChannel to min_channel

[Greg Kroah-Hartman]

On Sat, Dec 04, 2021 at 10:09:41AM +0100, Alberto Merciai wrote:
> On Sat, Dec 04, 2021 at 11:27:43AM +0300, Pavel Skripkin wrote:
> > Looks like this variable is set, but never used. Shouldn't it be just
> > removed?
> > 
> > Same for max_channel (or byMaxChannel).
> 
> To be honest, I would prefer leave them where they are. I'm only
> refactoring the code considering less the logic aspect.
> 
> I don't know if these variables will become usefull in a second
> moment.

Fix the code for today, do not worry about tomorrow for stuff like this.

> Anyway, If you consider that this could be a good moment to remove
> them we can proceed.

Please remove things like this that are not needed.

thanks,

greg k-h

Re: [PATCH] staging: vt6655: refactor byMinChannel to min_channel

[Alberto Merciai]

> I think, it should be up to maintainers to decide, but them can be removed
> now and if somebody will need them later, he can bring them back without any
> pain. For now it's just useless memory usage.
Cool! I agree with you.

> I skimmed through priv fields usage and looks like many of them are just
> set, but never used. I guess, some kind of evolution removed all uses of
We can work on it.

Regards,
Alberto

Re: [PATCH] staging: vt6655: refactor byMinChannel to min_channel

[Alberto Merciai]

On Sat, Dec 04, 2021 at 10:29:10AM +0100, Greg Kroah-Hartman wrote:
> On Sat, Dec 04, 2021 at 10:09:41AM +0100, Alberto Merciai wrote:
> > On Sat, Dec 04, 2021 at 11:27:43AM +0300, Pavel Skripkin wrote:
> > > Looks like this variable is set, but never used. Shouldn't it be just
> > > removed?
> > > 
> > > Same for max_channel (or byMaxChannel).
> > 
> > To be honest, I would prefer leave them where they are. I'm only
> > refactoring the code considering less the logic aspect.
> > 
> > I don't know if these variables will become usefull in a second
> > moment.
> 
> Fix the code for today, do not worry about tomorrow for stuff like this.
Wise word... :)

> > Anyway, If you consider that this could be a good moment to remove
> > them we can proceed.
> 
> Please remove things like this that are not needed.
I will proceed.

Thanks,

Alberto

Re: [PATCH] staging: vt6655: refactor byMinChannel to min_channel

[Dan Carpenter]

On Sat, Dec 04, 2021 at 10:09:41AM +0100, Alberto Merciai wrote:
> On Sat, Dec 04, 2021 at 11:27:43AM +0300, Pavel Skripkin wrote:
> > Looks like this variable is set, but never used. Shouldn't it be just
> > removed?
> > 
> > Same for max_channel (or byMaxChannel).
> 
> To be honest, I would prefer leave them where they are. I'm only
> refactoring the code considering less the logic aspect.
> 

Pavel is right.

You should always be looking for that kind of stuff instead of just
doing a sed script.  And the thing is you're in the best place to think
about how each struct member is used right now because you're modifying
each use to make sure it compiles.  There is a possibility that no one
else will think about it ever again now that checkpatch doesn't
complain.

So in a way, it's better to leave the checkpatch warning as-is until
someone can fix it properly.

> I don't know if these variables will become usefull in a second
> moment.

No.  We do not write code for the future.  No one knows what the future
will hold.  Delete any code which is not currently useful.

regards,
dan carpenter