Hi Daniele, On 2015.03.12 00:05, Daniele Alessandrelli wrote: > Fix all the trivial style issues (as reported by checkpatch.pl) not requiring > code refactoring. A following patch is expected to fix the remaining issues by > performing some code refactoring. > > Signed-off-by: Daniele Alessandrelli <daniele.alessandrelli@xxxxxxxxx> > --- > drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c | 199 +++++++++++++---------- > 1 file changed, 110 insertions(+), 89 deletions(-) > > ==Changelog== > > v4: updated to leatest Greg's tree (and added changelog) > > v3: resent (unchanged) because of error in the other patch of the patchset > > v2: updated to apply cleanly to Greg's tree > > v1: initial realease > > diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c > index bc959ff..5ff4da8 100644 > --- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c > +++ b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c > @@ -28,8 +28,8 @@ > #include <linux/timer.h> > #include <linux/interrupt.h> > #include <linux/in.h> > -#include <asm/io.h> > -#include <asm/bitops.h> > +#include <linux/io.h> > +#include <linux/bitops.h> > > #include <linux/netdevice.h> > #include <linux/etherdevice.h> > @@ -67,8 +67,7 @@ static void ft1000_disable_interrupts(struct net_device *dev); > > /* new kernel */ > MODULE_AUTHOR(""); > -MODULE_DESCRIPTION > -("Support for Flarion Flash OFDM NIC Device. Support for PCMCIA when used with ft1000_cs."); > +MODULE_DESCRIPTION("Support for Flarion Flash OFDM NIC Device. Support for PCMCIA when used with ft1000_cs."); > MODULE_LICENSE("GPL"); > MODULE_SUPPORTED_DEVICE("FT1000"); > > @@ -267,7 +266,8 @@ void ft1000_write_dpram_mag_32(struct net_device *dev, int offset, u32 value) > /*--------------------------------------------------------------------------- > > Function: ft1000_enable_interrupts > - Description: This function will enable interrupts base on the current interrupt mask. > + Description: This function will enable interrupts base on the current > + interrupt mask. > Input: > dev - device structure > Output: > @@ -375,7 +375,8 @@ static int ft1000_reset_card(struct net_device *dev) > /* Make sure we free any memory reserve for provisioning */ > while (list_empty(&info->prov_list) == 0) { > pr_debug("deleting provisioning record\n"); > - ptr = list_entry(info->prov_list.next, struct prov_record, list); > + ptr = list_entry(info->prov_list.next, struct prov_record, > + list); > list_del(&ptr->list); > kfree(ptr->pprov_data); > kfree(ptr); > @@ -406,7 +407,8 @@ static int ft1000_reset_card(struct net_device *dev) > FT1000_DPRAM_MAG_RX_BASE); > for (i = 0; i < MAX_DSP_SESS_REC / 2; i++) { > info->DSPSess.MagRec[i] = > - inl(dev->base_addr + FT1000_REG_MAG_DPDATA); > + inl(dev->base_addr > + + FT1000_REG_MAG_DPDATA); > } > } > spin_unlock_irqrestore(&info->dpram_lock, flags); > @@ -435,11 +437,12 @@ static int ft1000_reset_card(struct net_device *dev) > mdelay(10); > pr_debug("Take DSP out of reset\n"); > > - /* Wait for 0xfefe indicating dsp ready before starting download */ > + /* Wait for 0xfefe indicating dsp ready before starting > + * download */ Minor nitpick: multi-line comments beggining /* and ending */ go on seperate lines. You can see this style used later in unchanged places visible in this patch. > for (i = 0; i < 50; i++) { > - tempword = > - ft1000_read_dpram_mag_16(dev, FT1000_MAG_DPRAM_FEFE, > - FT1000_MAG_DPRAM_FEFE_INDX); > + tempword = ft1000_read_dpram_mag_16(dev, > + FT1000_MAG_DPRAM_FEFE, > + FT1000_MAG_DPRAM_FEFE_INDX); > if (tempword == 0xfefe) > break; > mdelay(20); > @@ -466,8 +469,8 @@ static int ft1000_reset_card(struct net_device *dev) > > if (info->AsicID == ELECTRABUZZ_ID) { > /* > - * Need to initialize the FIFO length counter to zero in order to sync up > - * with the DSP > + * Need to initialize the FIFO length counter to zero in order > + * to sync up with the DSP > */ > info->fifo_cnt = 0; > ft1000_write_dpram(dev, FT1000_FIFO_LEN, info->fifo_cnt); > @@ -664,8 +667,8 @@ static void ft1000_hbchk(u_long data) > return; > } > /* > - * Set dedicated area to hi and ring appropriate doorbell according > - * to hi/ho heartbeat protocol > + * Set dedicated area to hi and ring appropriate doorbell > + * according to hi/ho heartbeat protocol > */ > if (info->AsicID == ELECTRABUZZ_ID) { > ft1000_write_dpram(dev, FT1000_HI_HO, hi); > @@ -687,13 +690,15 @@ static void ft1000_hbchk(u_long data) > if (info->AsicID == ELECTRABUZZ_ID) > ft1000_write_dpram(dev, FT1000_HI_HO, hi); > else > - ft1000_write_dpram_mag_16(dev, FT1000_MAG_HI_HO, hi_mag, FT1000_MAG_HI_HO_INDX); > + ft1000_write_dpram_mag_16(dev, FT1000_MAG_HI_HO, > + hi_mag, FT1000_MAG_HI_HO_INDX); > > if (info->AsicID == ELECTRABUZZ_ID) > tempword = ft1000_read_dpram(dev, FT1000_HI_HO); > else > - tempword = ntohs(ft1000_read_dpram_mag_16(dev, FT1000_MAG_HI_HO, FT1000_MAG_HI_HO_INDX)); > - > + tempword = ntohs(ft1000_read_dpram_mag_16(dev, > + FT1000_MAG_HI_HO, > + FT1000_MAG_HI_HO_INDX)); > } > > if (tempword != hi) { > @@ -755,7 +760,8 @@ static void ft1000_hbchk(u_long data) > Output: > > -------------------------------------------------------------------------*/ > -static void ft1000_send_cmd(struct net_device *dev, u16 *ptempbuffer, int size, u16 qtype) > +static void ft1000_send_cmd(struct net_device *dev, u16 *ptempbuffer, int size, > + u16 qtype) > { > struct ft1000_info *info = netdev_priv(dev); > int i; > @@ -849,12 +855,12 @@ static bool ft1000_receive_cmd(struct net_device *dev, u16 *pbuffer, > unsigned long flags; > > if (info->AsicID == ELECTRABUZZ_ID) { > - size = (ft1000_read_dpram(dev, *pnxtph)) + sizeof(struct pseudo_hdr); > + size = (ft1000_read_dpram(dev, *pnxtph)) > + + sizeof(struct pseudo_hdr); Is there any reason that ft1000_read_dpram() is in parantheses? > } else { > - size = > - ntohs(ft1000_read_dpram_mag_16 > - (dev, FT1000_MAG_PH_LEN, > - FT1000_MAG_PH_LEN_INDX)) + sizeof(struct pseudo_hdr); > + size = ntohs(ft1000_read_dpram_mag_16(dev, FT1000_MAG_PH_LEN, > + FT1000_MAG_PH_LEN_INDX)) > + + sizeof(struct pseudo_hdr); > } > if (size > maxsz) { > pr_debug("Invalid command length = %d\n", size); > @@ -955,7 +961,8 @@ static void ft1000_proc_drvmsg(struct net_device *dev) > > if (ft1000_receive_cmd(dev, &cmdbuffer[0], MAX_CMD_SQSIZE, &tempword)) { > > - /* Get the message type which is total_len + PSEUDO header + msgtype + message body */ > + /* Get the message type which is total_len + PSEUDO header > + * + msgtype + message body */ Same comment about comment style. > pdrvmsg = (struct drv_msg *)&cmdbuffer[0]; > msgtype = ntohs(pdrvmsg->type); > pr_debug("Command message type = 0x%x\n", msgtype); > @@ -966,8 +973,8 @@ static void ft1000_proc_drvmsg(struct net_device *dev) > while (list_empty(&info->prov_list) == 0) { > pr_debug("Sending a provisioning message\n"); > /* Make sure SLOWQ doorbell is clear */ > - tempword = > - ft1000_read_reg(dev, FT1000_REG_DOORBELL); > + tempword = ft1000_read_reg(dev, > + FT1000_REG_DOORBELL); > i = 0; > while (tempword & FT1000_DB_DPRAM_TX) { > mdelay(5); > @@ -975,9 +982,8 @@ static void ft1000_proc_drvmsg(struct net_device *dev) > if (i == 10) > break; > } > - ptr = > - list_entry(info->prov_list.next, > - struct prov_record, list); > + ptr = list_entry(info->prov_list.next, > + struct prov_record, list); > len = *(u16 *)ptr->pprov_data; > len = htons(len); > > @@ -996,14 +1002,15 @@ static void ft1000_proc_drvmsg(struct net_device *dev) > ppseudo_hdr->checksum); > } > > - ft1000_send_cmd(dev, (u16 *)ptr->pprov_data, len, SLOWQ_TYPE); > + ft1000_send_cmd(dev, (u16 *)ptr->pprov_data, > + len, SLOWQ_TYPE); > list_del(&ptr->list); > kfree(ptr->pprov_data); > kfree(ptr); > } > /* > - * Indicate adapter is ready to take application messages after all > - * provisioning messages are sent > + * Indicate adapter is ready to take application > + * messages after all provisioning messages are sent > */ > info->CardReady = 1; > break; > @@ -1091,8 +1098,8 @@ static void ft1000_proc_drvmsg(struct net_device *dev) > tempword = ft1000_read_reg(dev, FT1000_REG_DOORBELL); > if (tempword & FT1000_DB_DPRAM_TX) { > mdelay(10); > - tempword = > - ft1000_read_reg(dev, FT1000_REG_DOORBELL); > + tempword = ft1000_read_reg(dev, > + FT1000_REG_DOORBELL); > if (tempword & FT1000_DB_DPRAM_TX) > mdelay(10); > } > @@ -1127,7 +1134,9 @@ static void ft1000_proc_drvmsg(struct net_device *dev) > info->DSPInfoBlk[8] = 0x7200; > info->DSPInfoBlk[9] = > htons(info->DSPInfoBlklen); > - ft1000_send_cmd(dev, (u16 *)info->DSPInfoBlk, (u16)(info->DSPInfoBlklen+4), 0); > + ft1000_send_cmd(dev, (u16 *)info->DSPInfoBlk, > + (u16)(info->DSPInfoBlklen+4), > + 0); > } > > break; > @@ -1141,8 +1150,8 @@ static void ft1000_proc_drvmsg(struct net_device *dev) > tempword = ft1000_read_reg(dev, FT1000_REG_DOORBELL); > if (tempword & FT1000_DB_DPRAM_TX) { > mdelay(10); > - tempword = > - ft1000_read_reg(dev, FT1000_REG_DOORBELL); > + tempword = ft1000_read_reg(dev, > + FT1000_REG_DOORBELL); > if (tempword & FT1000_DB_DPRAM_TX) > mdelay(10); > } > @@ -1188,7 +1197,8 @@ static void ft1000_proc_drvmsg(struct net_device *dev) > *pmsg++ = convert.wrd; > *pmsg++ = htons(info->DrvErrNum); > > - ft1000_send_cmd(dev, (u16 *)&tempbuffer[0], (u16)(0x0012), 0); > + ft1000_send_cmd(dev, (u16 *)&tempbuffer[0], > + (u16)(0x0012), 0); > info->DrvErrNum = 0; > } > > @@ -1279,28 +1289,28 @@ static int ft1000_parse_dpram_msg(struct net_device *dev) > FT1000_MAG_TOTAL_LEN_INDX)); > } > pr_debug("total length = %d\n", total_len); > - if ((total_len < MAX_CMD_SQSIZE) && (total_len > sizeof(struct pseudo_hdr))) { > + if ((total_len < MAX_CMD_SQSIZE) > + && (total_len > sizeof(struct pseudo_hdr))) { > total_len += nxtph; > /* > - * ft1000_read_reg will return a value that needs to be byteswap > - * in order to get DSP_QID_OFFSET. > + * ft1000_read_reg will return a value that needs to be > + * byteswap in order to get DSP_QID_OFFSET. > */ > if (info->AsicID == ELECTRABUZZ_ID) { > - portid = > - (ft1000_read_dpram > - (dev, > - DSP_QID_OFFSET + FT1000_DPRAM_RX_BASE + > - 2) >> 8) & 0xff; > + portid = (ft1000_read_dpram(dev, DSP_QID_OFFSET > + + FT1000_DPRAM_RX_BASE + 2) > + >> 8) & 0xff; > } else { > - portid = > - (ft1000_read_dpram_mag_16 > - (dev, FT1000_MAG_PORT_ID, > - FT1000_MAG_PORT_ID_INDX) & 0xff); > + portid = (ft1000_read_dpram_mag_16(dev, > + FT1000_MAG_PORT_ID, > + FT1000_MAG_PORT_ID_INDX) > + & 0xff); Is there any reason this is in parantheses? > } > pr_debug("DSP_QID = 0x%x\n", portid); > > if (portid == DRIVERID) { > - /* We are assumming one driver message from the DSP at a time. */ > + /* We are assumming one driver message from the > + * DSP at a time. */ Same comment about multi-line comments. > ft1000_proc_drvmsg(dev); > } > } > @@ -1435,35 +1445,41 @@ static void ft1000_flush_fifo(struct net_device *dev, u16 DrvErrNum) > } else { > info->DSP_TIME[0] = > ft1000_read_dpram_mag_16(dev, > - FT1000_MAG_DSP_TIMER0, > - FT1000_MAG_DSP_TIMER0_INDX); > + FT1000_MAG_DSP_TIMER0, > + FT1000_MAG_DSP_TIMER0_INDX); > info->DSP_TIME[1] = > ft1000_read_dpram_mag_16(dev, > - FT1000_MAG_DSP_TIMER1, > - FT1000_MAG_DSP_TIMER1_INDX); > + FT1000_MAG_DSP_TIMER1, > + FT1000_MAG_DSP_TIMER1_INDX); > info->DSP_TIME[2] = > ft1000_read_dpram_mag_16(dev, > - FT1000_MAG_DSP_TIMER2, > - FT1000_MAG_DSP_TIMER2_INDX); > + FT1000_MAG_DSP_TIMER2, > + FT1000_MAG_DSP_TIMER2_INDX); > info->DSP_TIME[3] = > ft1000_read_dpram_mag_16(dev, > - FT1000_MAG_DSP_TIMER3, > - FT1000_MAG_DSP_TIMER3_INDX); > + FT1000_MAG_DSP_TIMER3, > + FT1000_MAG_DSP_TIMER3_INDX); > } > if (tempword == 0) { > /* > - * Let's check if ASIC reads are still ok by reading the Mask register > - * which is never zero at this point of the code. > + * Let's check if ASIC reads are still ok by > + * reading the Mask register which is never zero > + * at this point of the code. > */ > tempword = > inw(dev->base_addr + > FT1000_REG_SUP_IMASK); > if (tempword == 0) { > - /* This indicates that we can not communicate with the ASIC */ > - info->DrvErrNum = > - FIFO_FLUSH_BADCNT; > + /* > + * This indicates that we can not > + * communicate with the ASIC > + */ > + info->DrvErrNum = FIFO_FLUSH_BADCNT; > } else { > - /* Let's assume that we really flush the FIFO */ > + /* > + * Let's assume that we really flush > + * the FIFO > + */ You are using the correct comment style here :) Make them consistent atleast in this patch. > pcmcia->PktIntfErr++; > return; > } > @@ -1549,7 +1565,6 @@ static int ft1000_copy_up_pkt(struct net_device *dev) > skb = dev_alloc_skb(len + 12 + 2); > > if (skb == NULL) { > - pr_debug("No Network buffers available\n"); > /* Read High word to complete 32 bit access */ > if (info->AsicID == MAGNEMITE_ID) > tempword = ft1000_read_reg(dev, FT1000_REG_MAG_DFRH); > @@ -1670,7 +1685,8 @@ static int ft1000_copy_up_pkt(struct net_device *dev) > info->stats.rx_bytes += (len + 12); > > if (info->AsicID == ELECTRABUZZ_ID) { > - /* track how many bytes have been read from FIFO - round up to 16 bit word */ > + /* track how many bytes have been read from FIFO - round up to > + * 16 bit word */ > tempword = len + 16; > if (tempword & 0x01) > tempword++; > @@ -1734,9 +1750,11 @@ static int ft1000_copy_down_pkt(struct net_device *dev, u16 *packet, u16 len) > else > pseudo.blk.length = ntohs(len); > > - pseudo.blk.source = DSPID; /* Need to swap to get in correct order */ > + pseudo.blk.source = DSPID; /* Need to swap to get in correct > + order */ > pseudo.blk.destination = HOSTID; > - pseudo.blk.portdest = NETWORKID; /* Need to swap to get in correct order */ > + pseudo.blk.portdest = NETWORKID; /* Need to swap to get in > + correct order */ > pseudo.blk.portsrc = DSPAIRID; > pseudo.blk.sh_str_id = 0; > pseudo.blk.control = 0; > @@ -1840,7 +1858,8 @@ static int ft1000_open(struct net_device *dev) > { > ft1000_reset_card(dev); > > - /* schedule ft1000_hbchk to perform periodic heartbeat checks on DSP and ASIC */ > + /* schedule ft1000_hbchk to perform periodic heartbeat checks on DSP > + * and ASIC */ > init_timer(&poll_timer); > poll_timer.expires = jiffies + (2 * HZ); > poll_timer.data = (u_long)dev; > @@ -1925,7 +1944,8 @@ static irqreturn_t ft1000_interrupt(int irq, void *dev_id) > /* Read interrupt type */ > inttype = ft1000_read_reg(dev, FT1000_REG_SUP_ISR); > > - /* Make sure we process all interrupt before leaving the ISR due to the edge trigger interrupt type */ > + /* Make sure we process all interrupt before leaving the ISR due to the > + * edge trigger interrupt type */ > while (inttype) { > if (inttype & ISR_DOORBELL_PEND) > ft1000_parse_dpram_msg(dev); > @@ -1935,20 +1955,18 @@ static irqreturn_t ft1000_interrupt(int irq, void *dev_id) > > cnt = 0; > do { > - /* Check if we have packets in the Downlink FIFO */ > + /* Check if we have packets in the Downlink > + * FIFO */ > if (info->AsicID == ELECTRABUZZ_ID) { > - tempword = > - ft1000_read_reg(dev, > - FT1000_REG_DFIFO_STAT); > + tempword = ft1000_read_reg(dev, > + FT1000_REG_DFIFO_STAT); > } else { > - tempword = > - ft1000_read_reg(dev, > - FT1000_REG_MAG_DFSR); > + tempword = ft1000_read_reg(dev, > + FT1000_REG_MAG_DFSR); > } > - if (tempword & 0x1f) > - ft1000_copy_up_pkt(dev); > - else > + if (!(tempword & 0x1f)) > break; > + ft1000_copy_up_pkt(dev); > cnt++; > } while (cnt < MAX_RCV_LOOP); > > @@ -1980,7 +1998,8 @@ void stop_ft1000_card(struct net_device *dev) > > /* Make sure we free any memory reserve for provisioning */ > while (list_empty(&info->prov_list) == 0) { > - ptr = list_entry(info->prov_list.next, struct prov_record, list); > + ptr = list_entry(info->prov_list.next, struct prov_record, > + list); > list_del(&ptr->list); > kfree(ptr->pprov_data); > kfree(ptr); > @@ -2035,8 +2054,7 @@ struct net_device *init_ft1000_card(struct pcmcia_device *link, > struct ft1000_pcmcia *pcmcia; > struct net_device *dev; > > - static const struct net_device_ops ft1000ops = /* Slavius 21.10.2009 due to kernel changes */ > - { > + static const struct net_device_ops ft1000ops = { > .ndo_open = &ft1000_open, > .ndo_stop = &ft1000_close, > .ndo_start_xmit = &ft1000_start_xmit, > @@ -2099,7 +2117,7 @@ struct net_device *init_ft1000_card(struct pcmcia_device *link, > /* dev->open = &ft1000_open; */ > /* dev->stop = &ft1000_close; */ > > - dev->netdev_ops = &ft1000ops; /* Slavius 21.10.2009 due to kernel changes */ > + dev->netdev_ops = &ft1000ops; > > pr_debug("device name = %s\n", dev->name); > > @@ -2110,7 +2128,8 @@ struct net_device *init_ft1000_card(struct pcmcia_device *link, > goto err_dev; > } > > - if (request_irq(dev->irq, ft1000_interrupt, IRQF_SHARED, dev->name, dev)) { > + if (request_irq(dev->irq, ft1000_interrupt, IRQF_SHARED, dev->name, > + dev)) { > netdev_err(dev, "Could not request_irq\n"); > goto err_dev; > } > @@ -2128,13 +2147,15 @@ struct net_device *init_ft1000_card(struct pcmcia_device *link, > info->AsicID = ft1000_read_reg(dev, FT1000_REG_ASIC_ID); > if (info->AsicID == ELECTRABUZZ_ID) { > pr_debug("ELECTRABUZZ ASIC\n"); > - if (request_firmware(&fw_entry, "ft1000.img", &link->dev) != 0) { > + if (request_firmware(&fw_entry, "ft1000.img", > + &link->dev) != 0) { > pr_info("Could not open ft1000.img\n"); > goto err_unreg; > } > } else { > pr_debug("MAGNEMITE ASIC\n"); > - if (request_firmware(&fw_entry, "ft2000.img", &link->dev) != 0) { > + if (request_firmware(&fw_entry, "ft2000.img", > + &link->dev) != 0) { You can just remove that "!= 0". -- Thanks, Giedrius _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel