On 25 May 2012 19:32, Dan Carpenter <dan.carpenter@xxxxxxxxxx> 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 <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).
Hi Dan,
That particular define used for the ifdefs has always been defined
since the earliest version of the driver I've encountered, so it can
probably be removed at some point - It's on my TODO list, and I should
update the in tree version at some point.
Unfortunately due to recent changes, the driver has a few more issues
that just coding style to fix at present. Startup is very
hit-and-miss, for instance, and I've not had any external reports of
these bugs - which leads me to suspect that no one else is testing the
code on a real device... I'm slowly working my way through these.
Cheers,
Mark
>
>> /* 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
