On 25 May 2012 19:32, 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).
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 [email protected] http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
