Hi Dan 

       I'm sure I have tested patch with checkpatch.pl and gave me zero 
warnings and errors. Unless script in my kernel tree has bugs.

Cheers

Adnan

Dan Carpenter <[email protected]> wrote:

>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 <[email protected]>
>> Reviewed-by: Jannis Pohlmann <[email protected]>
>> ---
>>  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 <[email protected]>");
>>  MODULE_AUTHOR("Mark Einon <[email protected]>");
>>  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
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel

Reply via email to