[PATCH v2] staging: vt6655: replace camel case by snake case

Pavan Bobba posted 1 patch 2 years, 3 months ago
There is a newer version of this series
drivers/staging/vt6655/srom.c | 48 +++++++++++++++++------------------
1 file changed, 24 insertions(+), 24 deletions(-)
[PATCH v2] staging: vt6655: replace camel case by snake case
Posted by Pavan Bobba 2 years, 3 months ago
1.Conversion of formal argument names from camel case to snake case for below functions:
      a.SROMvReadAllContents
      b.SROMvReadEtherAddress

2.Conversion of local variable names from camel case to snake case in function SROMvReadEtherAddress

Issue found by checkpatch

Signed-off-by: Pavan Bobba <opensource206@gmail.com>
---
 v1 -> v2: encoding of type information in variable names dropped
 
 drivers/staging/vt6655/srom.c | 48 +++++++++++++++++------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/vt6655/srom.c b/drivers/staging/vt6655/srom.c
index ee5ca4db74dc..9c13ad089d78 100644
--- a/drivers/staging/vt6655/srom.c
+++ b/drivers/staging/vt6655/srom.c
@@ -59,36 +59,36 @@
 unsigned char SROMbyReadEmbedded(void __iomem *iobase,
 				 unsigned char byContntOffset)
 {
-	unsigned short wDelay, wNoACK;
-	unsigned char byWait;
-	unsigned char byData;
-	unsigned char byOrg;
+	unsigned short delay, noack;
+	unsigned char wait;
+	unsigned char data;
+	unsigned char org;
 
-	byData = 0xFF;
-	byOrg = ioread8(iobase + MAC_REG_I2MCFG);
+	data = 0xFF;
+	org = ioread8(iobase + MAC_REG_I2MCFG);
 	/* turn off hardware retry for getting NACK */
-	iowrite8(byOrg & (~I2MCFG_NORETRY), iobase + MAC_REG_I2MCFG);
-	for (wNoACK = 0; wNoACK < W_MAX_I2CRETRY; wNoACK++) {
+	iowrite8(org & (~I2MCFG_NORETRY), iobase + MAC_REG_I2MCFG);
+	for (noack = 0; noack < W_MAX_I2CRETRY; noack++) {
 		iowrite8(EEP_I2C_DEV_ID, iobase + MAC_REG_I2MTGID);
 		iowrite8(byContntOffset, iobase + MAC_REG_I2MTGAD);
 
 		/* issue read command */
 		iowrite8(I2MCSR_EEMR, iobase + MAC_REG_I2MCSR);
 		/* wait DONE be set */
-		for (wDelay = 0; wDelay < W_MAX_TIMEOUT; wDelay++) {
-			byWait = ioread8(iobase + MAC_REG_I2MCSR);
-			if (byWait & (I2MCSR_DONE | I2MCSR_NACK))
+		for (delay = 0; delay < W_MAX_TIMEOUT; delay++) {
+			wait = ioread8(iobase + MAC_REG_I2MCSR);
+			if (wait & (I2MCSR_DONE | I2MCSR_NACK))
 				break;
 			udelay(CB_DELAY_LOOP_WAIT);
 		}
-		if ((wDelay < W_MAX_TIMEOUT) &&
-		    (!(byWait & I2MCSR_NACK))) {
+		if ((delay < W_MAX_TIMEOUT) &&
+		    (!(wait & I2MCSR_NACK))) {
 			break;
 		}
 	}
-	byData = ioread8(iobase + MAC_REG_I2MDIPT);
-	iowrite8(byOrg, iobase + MAC_REG_I2MCFG);
-	return byData;
+	data = ioread8(iobase + MAC_REG_I2MDIPT);
+	iowrite8(org, iobase + MAC_REG_I2MCFG);
+	return data;
 }
 
 /*
@@ -98,20 +98,20 @@ unsigned char SROMbyReadEmbedded(void __iomem *iobase,
  *  In:
  *      iobase          - I/O base address
  *  Out:
- *      pbyEepromRegs   - EEPROM content Buffer
+ *      eepromregs   - EEPROM content Buffer
  *
  * Return Value: none
  *
  */
-void SROMvReadAllContents(void __iomem *iobase, unsigned char *pbyEepromRegs)
+void SROMvReadAllContents(void __iomem *iobase, unsigned char *eepromregs)
 {
 	int     ii;
 
 	/* ii = Rom Address */
 	for (ii = 0; ii < EEP_MAX_CONTEXT_SIZE; ii++) {
-		*pbyEepromRegs = SROMbyReadEmbedded(iobase,
+		*eepromregs = SROMbyReadEmbedded(iobase,
 						    (unsigned char)ii);
-		pbyEepromRegs++;
+		eepromregs++;
 	}
 }
 
@@ -122,19 +122,19 @@ void SROMvReadAllContents(void __iomem *iobase, unsigned char *pbyEepromRegs)
  *  In:
  *      iobase          - I/O base address
  *  Out:
- *      pbyEtherAddress - Ethernet Address buffer
+ *      etheraddress - Ethernet Address buffer
  *
  * Return Value: none
  *
  */
 void SROMvReadEtherAddress(void __iomem *iobase,
-			   unsigned char *pbyEtherAddress)
+			   unsigned char *etheraddress)
 {
 	unsigned char ii;
 
 	/* ii = Rom Address */
 	for (ii = 0; ii < ETH_ALEN; ii++) {
-		*pbyEtherAddress = SROMbyReadEmbedded(iobase, ii);
-		pbyEtherAddress++;
+		*etheraddress = SROMbyReadEmbedded(iobase, ii);
+		etheraddress++;
 	}
 }
-- 
2.34.1
Re: [PATCH v2] staging: vt6655: replace camel case by snake case
Posted by Greg KH 2 years, 3 months ago
On Wed, Aug 23, 2023 at 06:23:07PM +0530, Pavan Bobba wrote:
> 1.Conversion of formal argument names from camel case to snake case for below functions:
>       a.SROMvReadAllContents
>       b.SROMvReadEtherAddress
> 
> 2.Conversion of local variable names from camel case to snake case in function SROMvReadEtherAddress

When you list the different things you are doing, that usually means you
want multiple patches.  Would you want to review something that mixed
something like this together?

Also, please read the kernel documentation for how to write a good
changelog, and subject, this subject is identical to the one that you
were told to change and you agreed to :(

> Issue found by checkpatch
> 
> Signed-off-by: Pavan Bobba <opensource206@gmail.com>
> ---
>  v1 -> v2: encoding of type information in variable names dropped

You also forgot to actually cc: the maintainer that could take this
patch :(



>  
>  drivers/staging/vt6655/srom.c | 48 +++++++++++++++++------------------
>  1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/staging/vt6655/srom.c b/drivers/staging/vt6655/srom.c
> index ee5ca4db74dc..9c13ad089d78 100644
> --- a/drivers/staging/vt6655/srom.c
> +++ b/drivers/staging/vt6655/srom.c
> @@ -59,36 +59,36 @@
>  unsigned char SROMbyReadEmbedded(void __iomem *iobase,
>  				 unsigned char byContntOffset)
>  {
> -	unsigned short wDelay, wNoACK;
> -	unsigned char byWait;
> -	unsigned char byData;
> -	unsigned char byOrg;
> +	unsigned short delay, noack;
> +	unsigned char wait;
> +	unsigned char data;
> +	unsigned char org;
>  
> -	byData = 0xFF;
> -	byOrg = ioread8(iobase + MAC_REG_I2MCFG);
> +	data = 0xFF;
> +	org = ioread8(iobase + MAC_REG_I2MCFG);
>  	/* turn off hardware retry for getting NACK */
> -	iowrite8(byOrg & (~I2MCFG_NORETRY), iobase + MAC_REG_I2MCFG);
> -	for (wNoACK = 0; wNoACK < W_MAX_I2CRETRY; wNoACK++) {
> +	iowrite8(org & (~I2MCFG_NORETRY), iobase + MAC_REG_I2MCFG);
> +	for (noack = 0; noack < W_MAX_I2CRETRY; noack++) {
>  		iowrite8(EEP_I2C_DEV_ID, iobase + MAC_REG_I2MTGID);
>  		iowrite8(byContntOffset, iobase + MAC_REG_I2MTGAD);
>  
>  		/* issue read command */
>  		iowrite8(I2MCSR_EEMR, iobase + MAC_REG_I2MCSR);
>  		/* wait DONE be set */
> -		for (wDelay = 0; wDelay < W_MAX_TIMEOUT; wDelay++) {
> -			byWait = ioread8(iobase + MAC_REG_I2MCSR);
> -			if (byWait & (I2MCSR_DONE | I2MCSR_NACK))
> +		for (delay = 0; delay < W_MAX_TIMEOUT; delay++) {
> +			wait = ioread8(iobase + MAC_REG_I2MCSR);
> +			if (wait & (I2MCSR_DONE | I2MCSR_NACK))
>  				break;
>  			udelay(CB_DELAY_LOOP_WAIT);
>  		}
> -		if ((wDelay < W_MAX_TIMEOUT) &&
> -		    (!(byWait & I2MCSR_NACK))) {
> +		if ((delay < W_MAX_TIMEOUT) &&
> +		    (!(wait & I2MCSR_NACK))) {
>  			break;
>  		}
>  	}
> -	byData = ioread8(iobase + MAC_REG_I2MDIPT);
> -	iowrite8(byOrg, iobase + MAC_REG_I2MCFG);
> -	return byData;
> +	data = ioread8(iobase + MAC_REG_I2MDIPT);
> +	iowrite8(org, iobase + MAC_REG_I2MCFG);
> +	return data;
>  }
>  
>  /*
> @@ -98,20 +98,20 @@ unsigned char SROMbyReadEmbedded(void __iomem *iobase,
>   *  In:
>   *      iobase          - I/O base address
>   *  Out:
> - *      pbyEepromRegs   - EEPROM content Buffer
> + *      eepromregs   - EEPROM content Buffer
>   *
>   * Return Value: none
>   *
>   */
> -void SROMvReadAllContents(void __iomem *iobase, unsigned char *pbyEepromRegs)
> +void SROMvReadAllContents(void __iomem *iobase, unsigned char *eepromregs)
>  {
>  	int     ii;
>  
>  	/* ii = Rom Address */
>  	for (ii = 0; ii < EEP_MAX_CONTEXT_SIZE; ii++) {
> -		*pbyEepromRegs = SROMbyReadEmbedded(iobase,
> +		*eepromregs = SROMbyReadEmbedded(iobase,
>  						    (unsigned char)ii);

You now have an alignment issue, right?  Did you run this through
checkpatch?

thanks,

greg k-h
Re: [PATCH v2] staging: vt6655: replace camel case by snake case
Posted by Julia Lawall 2 years, 3 months ago

On Wed, 23 Aug 2023, Pavan Bobba wrote:

> 1.Conversion of formal argument names from camel case to snake case for below functions:
>       a.SROMvReadAllContents
>       b.SROMvReadEtherAddress
>
> 2.Conversion of local variable names from camel case to snake case in function SROMvReadEtherAddress
>
> Issue found by checkpatch
>
> Signed-off-by: Pavan Bobba <opensource206@gmail.com>
> ---
>  v1 -> v2: encoding of type information in variable names dropped
>
>  drivers/staging/vt6655/srom.c | 48 +++++++++++++++++------------------
>  1 file changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/staging/vt6655/srom.c b/drivers/staging/vt6655/srom.c
> index ee5ca4db74dc..9c13ad089d78 100644
> --- a/drivers/staging/vt6655/srom.c
> +++ b/drivers/staging/vt6655/srom.c
> @@ -59,36 +59,36 @@
>  unsigned char SROMbyReadEmbedded(void __iomem *iobase,
>  				 unsigned char byContntOffset)
>  {
> -	unsigned short wDelay, wNoACK;
> -	unsigned char byWait;
> -	unsigned char byData;
> -	unsigned char byOrg;
> +	unsigned short delay, noack;
> +	unsigned char wait;
> +	unsigned char data;
> +	unsigned char org;
>
> -	byData = 0xFF;
> -	byOrg = ioread8(iobase + MAC_REG_I2MCFG);
> +	data = 0xFF;
> +	org = ioread8(iobase + MAC_REG_I2MCFG);
>  	/* turn off hardware retry for getting NACK */
> -	iowrite8(byOrg & (~I2MCFG_NORETRY), iobase + MAC_REG_I2MCFG);
> -	for (wNoACK = 0; wNoACK < W_MAX_I2CRETRY; wNoACK++) {
> +	iowrite8(org & (~I2MCFG_NORETRY), iobase + MAC_REG_I2MCFG);
> +	for (noack = 0; noack < W_MAX_I2CRETRY; noack++) {
>  		iowrite8(EEP_I2C_DEV_ID, iobase + MAC_REG_I2MTGID);
>  		iowrite8(byContntOffset, iobase + MAC_REG_I2MTGAD);
>
>  		/* issue read command */
>  		iowrite8(I2MCSR_EEMR, iobase + MAC_REG_I2MCSR);
>  		/* wait DONE be set */
> -		for (wDelay = 0; wDelay < W_MAX_TIMEOUT; wDelay++) {
> -			byWait = ioread8(iobase + MAC_REG_I2MCSR);
> -			if (byWait & (I2MCSR_DONE | I2MCSR_NACK))
> +		for (delay = 0; delay < W_MAX_TIMEOUT; delay++) {
> +			wait = ioread8(iobase + MAC_REG_I2MCSR);
> +			if (wait & (I2MCSR_DONE | I2MCSR_NACK))
>  				break;
>  			udelay(CB_DELAY_LOOP_WAIT);
>  		}
> -		if ((wDelay < W_MAX_TIMEOUT) &&
> -		    (!(byWait & I2MCSR_NACK))) {
> +		if ((delay < W_MAX_TIMEOUT) &&

Maybe the W_ should also be dropped?

> +		    (!(wait & I2MCSR_NACK))) {
>  			break;
>  		}
>  	}
> -	byData = ioread8(iobase + MAC_REG_I2MDIPT);
> -	iowrite8(byOrg, iobase + MAC_REG_I2MCFG);
> -	return byData;
> +	data = ioread8(iobase + MAC_REG_I2MDIPT);
> +	iowrite8(org, iobase + MAC_REG_I2MCFG);
> +	return data;
>  }
>
>  /*
> @@ -98,20 +98,20 @@ unsigned char SROMbyReadEmbedded(void __iomem *iobase,
>   *  In:
>   *      iobase          - I/O base address
>   *  Out:
> - *      pbyEepromRegs   - EEPROM content Buffer
> + *      eepromregs   - EEPROM content Buffer

Here you could add some spaces to get the - EEPROM to line up with the
rest.  Likewise later.

julia

>   *
>   * Return Value: none
>   *
>   */
> -void SROMvReadAllContents(void __iomem *iobase, unsigned char *pbyEepromRegs)
> +void SROMvReadAllContents(void __iomem *iobase, unsigned char *eepromregs)
>  {
>  	int     ii;
>
>  	/* ii = Rom Address */
>  	for (ii = 0; ii < EEP_MAX_CONTEXT_SIZE; ii++) {
> -		*pbyEepromRegs = SROMbyReadEmbedded(iobase,
> +		*eepromregs = SROMbyReadEmbedded(iobase,
>  						    (unsigned char)ii);
> -		pbyEepromRegs++;
> +		eepromregs++;
>  	}
>  }
>
> @@ -122,19 +122,19 @@ void SROMvReadAllContents(void __iomem *iobase, unsigned char *pbyEepromRegs)
>   *  In:
>   *      iobase          - I/O base address
>   *  Out:
> - *      pbyEtherAddress - Ethernet Address buffer
> + *      etheraddress - Ethernet Address buffer
>   *
>   * Return Value: none
>   *
>   */
>  void SROMvReadEtherAddress(void __iomem *iobase,
> -			   unsigned char *pbyEtherAddress)
> +			   unsigned char *etheraddress)
>  {
>  	unsigned char ii;
>
>  	/* ii = Rom Address */
>  	for (ii = 0; ii < ETH_ALEN; ii++) {
> -		*pbyEtherAddress = SROMbyReadEmbedded(iobase, ii);
> -		pbyEtherAddress++;
> +		*etheraddress = SROMbyReadEmbedded(iobase, ii);
> +		etheraddress++;
>  	}
>  }
> --
> 2.34.1
>
>
>
Re: [PATCH v2] staging: vt6655: replace camel case by snake case
Posted by Pavan Bobba 2 years, 3 months ago
On Wed, Aug 23, 2023 at 02:58:38PM +0200, Julia Lawall wrote:
> 
> >  		}
> > -		if ((wDelay < W_MAX_TIMEOUT) &&
> > -		    (!(byWait & I2MCSR_NACK))) {
> > +		if ((delay < W_MAX_TIMEOUT) &&
> 
> Maybe the W_ should also be dropped?
even non staging driver have this
https://elixir.bootlin.com/linux/v6.5-rc7/source/drivers/net/ethernet/via/via-velocity.h#L959

> >   *  Out:
> > - *      pbyEepromRegs   - EEPROM content Buffer
> > + *      eepromregs   - EEPROM content Buffer
> 
> Here you could add some spaces to get the - EEPROM to line up with the
> rest.  Likewise later.
> 
> julia

ok 

> >   *
> >   * Return Value: none
> >   *
> >   */
> > -void SROMvReadAllContents(void __iomem *iobase, unsigned char *pbyEepromRegs)
> > +void SROMvReadAllContents(void __iomem *iobase, unsigned char *eepromregs)
> >  {
> >  	int     ii;
> >
> >  	/* ii = Rom Address */
> >  	for (ii = 0; ii < EEP_MAX_CONTEXT_SIZE; ii++) {
> > -		*pbyEepromRegs = SROMbyReadEmbedded(iobase,
> > +		*eepromregs = SROMbyReadEmbedded(iobase,
> >  						    (unsigned char)ii);
> > -		pbyEepromRegs++;
> > +		eepromregs++;
> >  	}
> >  }
> >
> > @@ -122,19 +122,19 @@ void SROMvReadAllContents(void __iomem *iobase, unsigned char *pbyEepromRegs)
> >   *  In:
> >   *      iobase          - I/O base address
> >   *  Out:
> > - *      pbyEtherAddress - Ethernet Address buffer
> > + *      etheraddress - Ethernet Address buffer
> >   *
> >   * Return Value: none
> >   *
> >   */
> >  	}
> >  }
> > --
> > 2.34.1
> >
> >
> >