Re: [PATCH] Staging: et131x: fix coding style issues

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 25, 2012 at 06:56:40PM +0100, Adnan Ali wrote:
> This commit fixes coding style issues including braces
> position and line wrapping.
> 
> Signed-off-by: Adnan Ali <adnan.ali@xxxxxxxxxxxxxxx>
> Reviewed-by: Jannis Pohlmann <jannis.pohlmann@xxxxxxxxxxxxxxx>
> ---
>  drivers/staging/et131x/et131x.c |   11 ++++-------
>  1 files changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
> index 5b11c5e..cf02336 100644
> --- a/drivers/staging/et131x/et131x.c
> +++ b/drivers/staging/et131x/et131x.c
> @@ -85,8 +85,7 @@
>  MODULE_AUTHOR("Victor Soriano <vjsoriano@xxxxxxxxx>");
>  MODULE_AUTHOR("Mark Einon <mark.einon@xxxxxxxxx>");
>  MODULE_LICENSE("Dual BSD/GPL");
> -MODULE_DESCRIPTION("10/100/1000 Base-T Ethernet Driver "
> -		   "for the ET1310 by Agere Systems");
> +MODULE_DESCRIPTION("10/100/1000 Base-T Ethernet Driver for the ET1310 by Agere Systems");
>  
>  /* EEPROM defines */
>  #define MAX_NUM_REGISTER_POLLS          1000
> @@ -2967,11 +2966,10 @@ static struct rfd *nic_rx_pkts(struct et131x_adapter *adapter)
>  		(ring_index == 0 &&
>  		buff_index > rx_local->fbr[1]->num_entries - 1) ||
>  		(ring_index == 1 &&
> -		buff_index > rx_local->fbr[0]->num_entries - 1))
> +		buff_index > rx_local->fbr[0]->num_entries - 1)) {
>  #else
> -	if (ring_index != 1 || buff_index > rx_local->fbr[0]->num_entries - 1)
> +	if (ring_index != 1 || buff_index > rx_local->fbr[0]->num_entries - 1) {
>  #endif
> -	{

Mark, why do we have these nasty ifdefs?  It seems like this should
be an option at module load so that distros can support either way.
(But that is a stock response, I haven't looked at the code).

>  		/* Illegal buffer or ring index cannot be used by S/W*/
>  		dev_err(&adapter->pdev->dev,
>  			  "NICRxPkts PSR Entry %d indicates "
> @@ -4326,8 +4324,7 @@ static int et131x_mii_probe(struct net_device *netdev)
>  	phydev->advertising = phydev->supported;
>  	adapter->phydev = phydev;
>  
> -	dev_info(&adapter->pdev->dev, "attached PHY driver [%s] "
> -		 "(mii_bus:phy_addr=%s)\n",
> +	dev_info(&adapter->pdev->dev, "attached PHY driver [%s] (mii_bus:phy_addr=%s)\n",
>  		 phydev->drv->name, dev_name(&phydev->dev));

It's better to put this on the next line so it doesn't go over the
80 character limit.

	dev_info(&adapter->pdev->dev,
		 "attached PHY driver [%s] (mii_bus:phy_addr=%s)\n",
		 phydev->drv->name, dev_name(&phydev->dev));

Run your patches through checkpatch.pl before sending them.

The 80 character limit is not absolutely set in stone, but the rest
of the file uses it, and it's easy to do in this case.

regards,
dan carpenter


_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel


[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux