[PATCH] staging: octeon: Use net_device_stats from struct net_device
Instead of using a private copy of struct net_device_stats in struct octeon_ethernet, use stats from struct net_device. Also remove the now unnecessary .ndo_get_stats function. Signed-off-by: Tobias Klauser --- drivers/staging/octeon/ethernet-rx.c | 6 +++--- drivers/staging/octeon/ethernet-tx.c | 10 +- drivers/staging/octeon/ethernet.c| 18 +- drivers/staging/octeon/octeon-ethernet.h | 2 -- 4 files changed, 17 insertions(+), 19 deletions(-) diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c index fc849d4a1b5d..7f8cf875157c 100644 --- a/drivers/staging/octeon/ethernet-rx.c +++ b/drivers/staging/octeon/ethernet-rx.c @@ -356,8 +356,8 @@ static int cvm_oct_poll(struct oct_rx_group *rx_group, int budget) /* Increment RX stats for virtual ports */ if (port >= CVMX_PIP_NUM_INPUT_PORTS) { - priv->stats.rx_packets++; - priv->stats.rx_bytes += skb->len; + dev->stats.rx_packets++; + dev->stats.rx_bytes += skb->len; } netif_receive_skb(skb); } else { @@ -365,7 +365,7 @@ static int cvm_oct_poll(struct oct_rx_group *rx_group, int budget) * Drop any packet received for a device that * isn't up. */ - priv->stats.rx_dropped++; + dev->stats.rx_dropped++; dev_kfree_skb_irq(skb); } } else { diff --git a/drivers/staging/octeon/ethernet-tx.c b/drivers/staging/octeon/ethernet-tx.c index 0b8053205091..ff4119e8de42 100644 --- a/drivers/staging/octeon/ethernet-tx.c +++ b/drivers/staging/octeon/ethernet-tx.c @@ -459,7 +459,7 @@ int cvm_oct_xmit(struct sk_buff *skb, struct net_device *dev) case QUEUE_DROP: skb->next = to_free_list; to_free_list = skb; - priv->stats.tx_dropped++; + dev->stats.tx_dropped++; break; case QUEUE_HW: cvmx_fau_atomic_add32(FAU_NUM_PACKET_BUFFERS_TO_FREE, -1); @@ -534,7 +534,7 @@ int cvm_oct_xmit_pow(struct sk_buff *skb, struct net_device *dev) if (unlikely(!work)) { printk_ratelimited("%s: Failed to allocate a work queue entry\n", dev->name); - priv->stats.tx_dropped++; + dev->stats.tx_dropped++; dev_kfree_skb_any(skb); return 0; } @@ -545,7 +545,7 @@ int cvm_oct_xmit_pow(struct sk_buff *skb, struct net_device *dev) printk_ratelimited("%s: Failed to allocate a packet buffer\n", dev->name); cvmx_fpa_free(work, CVMX_FPA_WQE_POOL, 1); - priv->stats.tx_dropped++; + dev->stats.tx_dropped++; dev_kfree_skb_any(skb); return 0; } @@ -662,8 +662,8 @@ int cvm_oct_xmit_pow(struct sk_buff *skb, struct net_device *dev) /* Submit the packet to the POW */ cvmx_pow_work_submit(work, work->word1.tag, work->word1.tag_type, cvmx_wqe_get_qos(work), cvmx_wqe_get_grp(work)); - priv->stats.tx_packets++; - priv->stats.tx_bytes += skb->len; + dev->stats.tx_packets++; + dev->stats.tx_bytes += skb->len; dev_consume_skb_any(skb); return 0; } diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c index a379734a54b1..429e24adfcf5 100644 --- a/drivers/staging/octeon/ethernet.c +++ b/drivers/staging/octeon/ethernet.c @@ -228,17 +228,17 @@ static struct net_device_stats *cvm_oct_common_get_stats(struct net_device *dev) cvmx_pko_get_port_status(priv->port, 1, &tx_status); } - priv->stats.rx_packets += rx_status.inb_packets; - priv->stats.tx_packets += tx_status.packets; - priv->stats.rx_bytes += rx_status.inb_octets; - priv->stats.tx_bytes += tx_status.octets; - priv->stats.multicast += rx_status.multicast_packets; - priv->stats.rx_crc_errors += rx_status.inb_errors; - priv->stats.rx_frame_errors += rx_status.fcs_align_err_packets; - priv->stats.rx_dropped += rx_status.dropped_packets; + dev->stats.rx_packets += rx_status.inb_packets; + dev->stats.tx_packets += tx_status.packets; + dev->stats.rx_bytes += rx_
Re: [PATCH] staging: cxt1e1: use kzalloc instead of kmalloc/memset 0
On 2014-03-05 at 03:37:15 +0100, Daeseok Youn wrote: > > Signed-off-by: Daeseok Youn > --- > drivers/staging/cxt1e1/sbecom_inline_linux.h |6 ++ > 1 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/cxt1e1/sbecom_inline_linux.h > b/drivers/staging/cxt1e1/sbecom_inline_linux.h > index ba3ff3e..6dd1b55 100644 > --- a/drivers/staging/cxt1e1/sbecom_inline_linux.h > +++ b/drivers/staging/cxt1e1/sbecom_inline_linux.h > @@ -46,11 +46,9 @@ voidpci_write_32 (u_int32_t *p, u_int32_t v); > static inline void * > OS_kmalloc (size_t size) > { > -char *ptr = kmalloc (size, GFP_KERNEL | GFP_DMA); > + char *ptr = kzalloc(size, GFP_KERNEL | GFP_DMA); > > -if (ptr) > -memset (ptr, 0, size); > -return ptr; > + return ptr; > } It would probably be even better to get rid of this function altogether and replace all calls to it by kmalloc/kzalloc. >From a quick look at the users of OS_kmalloc, it also looks like GFP_DMA isn't needed for all of them. Cheers Tobias ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 7/7] staging: cxt1e1: remove unneeded a value
On 2014-03-05 at 02:24:22 +0100, Daeseok Youn wrote: > > It doesn't need to assign name array address to np pointer. > > Signed-off-by: Daeseok Youn > --- > drivers/staging/cxt1e1/linux.c |5 ++--- > 1 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/cxt1e1/linux.c b/drivers/staging/cxt1e1/linux.c > index 5bb42ae..cae8c66 100644 > --- a/drivers/staging/cxt1e1/linux.c > +++ b/drivers/staging/cxt1e1/linux.c > @@ -205,15 +205,14 @@ status_t > c4_wq_port_init(mpi_t *pi) > { > > - charname[16], *np; /* NOTE: name of the queue limited by system > + charname[16]; /* NOTE: name of the queue limited by system >* to 10 characters */ > > if (pi->wq_port) > return 0; /* already initialized */ > > - np = name; > memset(name, 0, 16); This isn't necessary since s{,n}printf() adds a terminating '\0'. > - sprintf(np, "%s%d", pi->up->devname, pi->portnum); /* IE pmcc4-01) */ > + sprintf(name, "%s%d", pi->up->devname, pi->portnum); /* IE pmcc4-01) */ Better use snprintf() here, even if the comment above claims the name never to be no longer than 10 characters. Cheers Tobias ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: lustre: storage class should be before const qualifier
The C99 specification states in section 6.11.5: The placement of a storage-class specifier other than at the beginning of the declaration specifiers in a declaration is an obsolescent feature. Signed-off-by: Tobias Klauser --- drivers/staging/lustre/lustre/lov/lov_object.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/lov/lov_object.c b/drivers/staging/lustre/lustre/lov/lov_object.c index fe0b70a..7a85caf 100644 --- a/drivers/staging/lustre/lustre/lov/lov_object.c +++ b/drivers/staging/lustre/lustre/lov/lov_object.c @@ -512,7 +512,7 @@ static int lov_attr_get_raid0(const struct lu_env *env, struct cl_object *obj, return result; } -const static struct lov_layout_operations lov_dispatch[] = { +static const struct lov_layout_operations lov_dispatch[] = { [LLT_EMPTY] = { .llo_init = lov_init_empty, .llo_delete= lov_delete_empty, -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 7/7] staging: cxt1e1: remove unneeded a value
On 2014-03-06 at 08:19:19 +0100, DaeSeok Youn wrote: > 2014-03-05 19:13 GMT+09:00 Tobias Klauser : > > On 2014-03-05 at 02:24:22 +0100, Daeseok Youn > > wrote: > >> > >> It doesn't need to assign name array address to np pointer. > >> > >> Signed-off-by: Daeseok Youn > >> --- > >> drivers/staging/cxt1e1/linux.c |5 ++--- > >> 1 files changed, 2 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/staging/cxt1e1/linux.c > >> b/drivers/staging/cxt1e1/linux.c > >> index 5bb42ae..cae8c66 100644 > >> --- a/drivers/staging/cxt1e1/linux.c > >> +++ b/drivers/staging/cxt1e1/linux.c > >> @@ -205,15 +205,14 @@ status_t > >> c4_wq_port_init(mpi_t *pi) > >> { > >> > >> - charname[16], *np; /* NOTE: name of the queue limited by > >> system > >> + charname[16]; /* NOTE: name of the queue limited by system > >>* to 10 characters */ > >> > >> if (pi->wq_port) > >> return 0; /* already initialized */ > >> > >> - np = name; > >> memset(name, 0, 16); > > > > This isn't necessary since s{,n}printf() adds a terminating '\0'. > Yes, I have looked at lib/vsprintf.c. I found it adds null to a string > in the end of vsnprintf() function. > I will remove memset() line. > > > > >> - sprintf(np, "%s%d", pi->up->devname, pi->portnum); /* IE pmcc4-01) */ > >> + sprintf(name, "%s%d", pi->up->devname, pi->portnum); /* IE pmcc4-01) > >> */ > > > > Better use snprintf() here, even if the comment above claims the name > > never to be no longer than 10 characters. > OK. I will replace sprintf with snprintf() and set a string length to "10". It's probably fine to leave the string at length 16 (since there's also a number appended to it) and the use sizeof(name) for the snprintf call. Cheers Tobias ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vt6656: Use proper target pointer in memcpy()
The coverity scanner marked these two memcpy()'s as causing a buffer overflow in CIDs 142743 and 142744. This is due the h_dest member of struct ethhdr being used as a target (size ETH_ALEN) in memcpy, but the copy is of size ETH_HLEN. However, the intention here seems to be to copy the entire ethernet header. Make that clear by specifying the proper destination buffer. Also remove the unnecessary casts of the source argument. Signed-off-by: Tobias Klauser --- drivers/staging/vt6656/rxtx.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/vt6656/rxtx.c b/drivers/staging/vt6656/rxtx.c index 1ff1446..19a8741 100644 --- a/drivers/staging/vt6656/rxtx.c +++ b/drivers/staging/vt6656/rxtx.c @@ -2186,7 +2186,7 @@ int nsDMA_tx_packet(struct vnt_private *pDevice, return STATUS_RESOURCES; } -memcpy(pDevice->sTxEthHeader.h_dest, (u8 *)(skb->data), ETH_HLEN); + memcpy(&pDevice->sTxEthHeader, skb->data, ETH_HLEN); //mike add:station mode check eapol-key challenge---> { @@ -2509,7 +2509,7 @@ int bRelayPacketSend(struct vnt_private *pDevice, u8 *pbySkbData, u32 uDataLen, return false; } -memcpy(pDevice->sTxEthHeader.h_dest, (u8 *)pbySkbData, ETH_HLEN); + memcpy(&pDevice->sTxEthHeader, pbySkbData, ETH_HLEN); if (pDevice->bEncryptionEnable == true) { bNeedEncryption = true; -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vt6656: Fix dereference before NULL check
pFifoHead is dereferenced before it is checked for NULL which implies it can potentially be NULL. Thus move the check before the dereference. Found by the coverity scanner, CID 1127221. Reported-by: Signed-off-by: Tobias Klauser --- drivers/staging/vt6656/rxtx.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/vt6656/rxtx.c b/drivers/staging/vt6656/rxtx.c index 19a8741..1b1155a 100644 --- a/drivers/staging/vt6656/rxtx.c +++ b/drivers/staging/vt6656/rxtx.c @@ -805,6 +805,9 @@ static u16 s_vGenerateTxParameter(struct vnt_private *pDevice, u16 wFifoCtl; u8 byFBOption = AUTO_FB_NONE; + if (!pFifoHead) + return 0; + pFifoHead->current_rate = cpu_to_le16(wCurrentRate); wFifoCtl = pFifoHead->wFIFOCtl; @@ -813,9 +816,6 @@ static u16 s_vGenerateTxParameter(struct vnt_private *pDevice, else if (wFifoCtl & FIFOCTL_AUTO_FB_1) byFBOption = AUTO_FB_1; - if (!pFifoHead) - return 0; - if (byPktType == PK_TYPE_11GB || byPktType == PK_TYPE_11GA) { if (need_rts) { struct vnt_rrv_time_rts *pBuf = -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vt6656: Fix dereference before NULL check
On 2014-04-25 at 15:11:05 +0200, Dan Carpenter wrote: > On Fri, Apr 25, 2014 at 01:54:51PM +0200, Tobias Klauser wrote: > > pFifoHead is dereferenced before it is checked for NULL which implies it > > can potentially be NULL. Thus move the check before the dereference. > > > > Found by the coverity scanner, CID 1127221. > > > > pFifoHead is this: > > struct vnt_tx_fifo_head *pFifoHead = &tx_buffer->fifo_head; > > Even if tx_buffer were NULL, the address of tx_buffer->fifo_head would > be non-NULL. The NULL test serves no purpose. If tx_buffer were NULL > we would still get an oops though. According to smatch, tx_buffer is > never NULL but I haven't verified manually. Hm, silly me... Thanks for pointing out Dan. I manually checked now and it seems that tx_buffer can never be NULL. So it should be fine to remove the test altogether. I'll send an updated patch. Thanks ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vt6656: Remove unnecesssary NULL check
pFifoHead points to tx_buffer->fifo_head which can never be NULL. We also don't need to check for tx_buffer being NULL instead, since it always points to ->Data of struct vnt_usb_send_context - the pointer to which was checked before calling s_vGenerateTxParameter(). Silences a dereference before NULL check warning reported by the coverity scanner in CID 1127221. Cc: Dan Carpenter Signed-off-by: Tobias Klauser --- drivers/staging/vt6656/rxtx.c |3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/staging/vt6656/rxtx.c b/drivers/staging/vt6656/rxtx.c index 19a8741..28f40e6 100644 --- a/drivers/staging/vt6656/rxtx.c +++ b/drivers/staging/vt6656/rxtx.c @@ -813,9 +813,6 @@ static u16 s_vGenerateTxParameter(struct vnt_private *pDevice, else if (wFifoCtl & FIFOCTL_AUTO_FB_1) byFBOption = AUTO_FB_1; - if (!pFifoHead) - return 0; - if (byPktType == PK_TYPE_11GB || byPktType == PK_TYPE_11GA) { if (need_rts) { struct vnt_rrv_time_rts *pBuf = -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] hv: Remove unnecessary comparison of unsigned against 0
pfncount is of type u32 and thus can never be smaller than 0. Found by the coverity scanner, CID 143213. Signed-off-by: Tobias Klauser --- drivers/hv/channel.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index 602ca86..17c8689 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -674,8 +674,7 @@ int vmbus_sendpacket_multipagebuffer(struct vmbus_channel *channel, u32 pfncount = NUM_PAGES_SPANNED(multi_pagebuffer->offset, multi_pagebuffer->len); - - if ((pfncount < 0) || (pfncount > MAX_MULTIPAGE_BUFFER_COUNT)) + if (pfncount > MAX_MULTIPAGE_BUFFER_COUNT) return -EINVAL; /* -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH net-next] net: Remove usage of net_device last_rx member
The network stack no longer uses the last_rx member of struct net_device since the bonding driver switched to use its own private last_rx in commit 9f242738376d ("bonding: use last_arp_rx in slave_last_rx()"). However, some drivers still (ab)use the field for their own purposes and some driver just update it without actually using it. Previously, there was an accompanying comment for the last_rx member added in commit 4dc89133f49b ("net: add a comment on netdev->last_rx") which asked drivers not to update is, unless really needed. However, this commend was removed in commit f8ff080dacec ("bonding: remove useless updating of slave->dev->last_rx"), so some drivers added later on still did update last_rx. Remove all usage of last_rx and switch three drivers (sky2, atp and smc91c92_cs) which actually read and write it to use their own private copy in netdev_priv. Compile-tested with allyesconfig and allmodconfig on x86 and arm. Cc: Eric Dumazet Cc: Jay Vosburgh Cc: Veaceslav Falico Cc: Andy Gospodarek Cc: Mirko Lindner Cc: Stephen Hemminger Signed-off-by: Tobias Klauser --- arch/m68k/emu/nfeth.c | 1 - drivers/net/ethernet/cavium/liquidio/lio_main.c| 1 - drivers/net/ethernet/cavium/liquidio/lio_vf_main.c | 1 - drivers/net/ethernet/hisilicon/hns/hns_enet.c | 1 - drivers/net/ethernet/intel/e1000e/netdev.c | 6 +++--- drivers/net/ethernet/intel/igb/igb_main.c | 6 +++--- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 7 +++ drivers/net/ethernet/marvell/sky2.c| 6 +++--- drivers/net/ethernet/marvell/sky2.h| 1 + drivers/net/ethernet/qualcomm/emac/emac-mac.c | 1 - drivers/net/ethernet/realtek/atp.c | 7 +++ drivers/net/ethernet/smsc/smc91c92_cs.c| 6 -- drivers/net/irda/bfin_sir.c| 5 ++--- drivers/net/irda/sh_sir.c | 1 - drivers/staging/ks7010/ks_hostif.c | 2 -- drivers/staging/netlogic/xlr_net.c | 1 - drivers/staging/rtl8192e/rtllib_rx.c | 1 - drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c | 4 drivers/staging/wlan-ng/hfa384x_usb.c | 1 - drivers/staging/wlan-ng/p80211netdev.c | 2 -- include/linux/netdevice.h | 3 --- net/batman-adv/bridge_loop_avoidance.c | 1 - net/batman-adv/distributed-arp-table.c | 1 - net/batman-adv/soft-interface.c| 2 -- 24 files changed, 22 insertions(+), 46 deletions(-) diff --git a/arch/m68k/emu/nfeth.c b/arch/m68k/emu/nfeth.c index fc4be028c418..e45ce4243aaa 100644 --- a/arch/m68k/emu/nfeth.c +++ b/arch/m68k/emu/nfeth.c @@ -124,7 +124,6 @@ static inline void recv_packet(struct net_device *dev) skb->protocol = eth_type_trans(skb, dev); netif_rx(skb); - dev->last_rx = jiffies; dev->stats.rx_packets++; dev->stats.rx_bytes += pktlen; diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c index 2b89ec291b8b..5ee3f007c613 100644 --- a/drivers/net/ethernet/cavium/liquidio/lio_main.c +++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c @@ -2360,7 +2360,6 @@ liquidio_push_packet(u32 octeon_id __attribute__((unused)), if (packet_was_received) { droq->stats.rx_bytes_received += len; droq->stats.rx_pkts_received++; - netdev->last_rx = jiffies; } else { droq->stats.rx_dropped++; netif_info(lio, rx_err, lio->netdev, diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c index 19d88fb387ce..e96cf6cdecfd 100644 --- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c +++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c @@ -1571,7 +1571,6 @@ liquidio_push_packet(u32 octeon_id __attribute__((unused)), if (packet_was_received) { droq->stats.rx_bytes_received += len; droq->stats.rx_pkts_received++; - netdev->last_rx = jiffies; } else { droq->stats.rx_dropped++; netif_info(lio, rx_err, lio->netdev, diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c index b7cb61385ad8..f7b75e96c1c3 100644 --- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c +++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c @@ -797,7 +797,6 @@ static void hns_nic_rx_up_pro(struct hns_nic_ring_data *ring_data, skb->protocol = eth_type_trans(skb, ndev); (void)napi_gro_receive(&ring_data->napi, skb); - ndev->last_rx = jiffies; } static int hns_desc_unu
Re: [PATCH 2/2] staging: emxx_udc: remove macro ERR
On 2015-01-21 at 23:56:06 +0100, Ahmad Hassan wrote: > Removed ERR macro since it is not used anymore in the emxx_udc.c file. > > Signed-off-by: Ahmad Hassan > --- > drivers/staging/emxx_udc/emxx_udc.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/staging/emxx_udc/emxx_udc.h > b/drivers/staging/emxx_udc/emxx_udc.h > index ee1b80d..0758cd5 100644 > --- a/drivers/staging/emxx_udc/emxx_udc.h > +++ b/drivers/staging/emxx_udc/emxx_udc.h > @@ -644,6 +644,5 @@ typedef volatile union { > } USB_REG_ACCESS; > > /*-*/ While at it, you could also remove the line above. > -#define ERR(stuff...)printk(KERN_ERR "udc: " stuff) > > #endif /* _LINUX_EMXX_H */ > -- > 2.2.1 > > ___ > devel mailing list > de...@linuxdriverproject.org > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] PCI: hv: Make unnecessarily global IRQ masking functions static
Make hv_irq_mask and hv_irq_unmask static as they are only used in pci-hyperv.c This fixes a sparse warning. Signed-off-by: Tobias Klauser --- drivers/pci/host/pci-hyperv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c index 763ff8745828..06c98695c06c 100644 --- a/drivers/pci/host/pci-hyperv.c +++ b/drivers/pci/host/pci-hyperv.c @@ -755,7 +755,7 @@ static int hv_set_affinity(struct irq_data *data, const struct cpumask *dest, return parent->chip->irq_set_affinity(parent, dest, force); } -void hv_irq_mask(struct irq_data *data) +static void hv_irq_mask(struct irq_data *data) { pci_msi_mask_irq(data); } @@ -770,7 +770,7 @@ void hv_irq_mask(struct irq_data *data) * is built out of this PCI bus's instance GUID and the function * number of the device. */ -void hv_irq_unmask(struct irq_data *data) +static void hv_irq_unmask(struct irq_data *data) { struct msi_desc *msi_desc = irq_data_get_msi_desc(data); struct irq_cfg *cfg = irqd_cfg(data); -- 2.9.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: ft1000: Replace printk with pr_info in ft1000_cs.c
On 2014-05-13 at 16:43:57 +0200, Masanari Iida wrote: > This patch replaced printk with pr_info. It would be even better to use dev_err(), since these are error messages and you can use the struct dev from link->dev. You can also drop the "ft1000:" prefix then. Cheers Tobias > > Signed-off-by: Masanari Iida > --- > drivers/staging/ft1000/ft1000-pcmcia/ft1000_cs.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_cs.c > b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_cs.c > index f376ca4..00c86db 100644 > --- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_cs.c > +++ b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_cs.c > @@ -95,20 +95,20 @@ static int ft1000_config(struct pcmcia_device *link) > /* setup IO window */ > ret = pcmcia_loop_config(link, ft1000_confcheck, NULL); > if (ret) { > - printk(KERN_INFO "ft1000: Could not configure pcmcia\n"); > + pr_info("ft1000: Could not configure pcmcia\n"); > return -ENODEV; > } > > /* configure device */ > ret = pcmcia_enable_device(link); > if (ret) { > - printk(KERN_INFO "ft1000: could not enable pcmcia\n"); > + pr_info("ft1000: could not enable pcmcia\n"); > goto failed; > } > > link->priv = init_ft1000_card(link, &ft1000_reset); > if (!link->priv) { > - printk(KERN_INFO "ft1000: Could not register as network > device\n"); > + pr_info("ft1000: Could not register as network device\n"); > goto failed; > } > > -- > 2.0.0.rc3.2.g998f840 > > ___ > devel mailing list > de...@linuxdriverproject.org > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] staging/wlan-ng: code refactoring
On 2014-05-12 at 17:22:46 +0200, Denis Pithon wrote: > Extract new static function from p80211netdev_rx_bh() to fix coding > style issue (too many leading tabs). > > Signed-off-by: Denis Pithon > --- > drivers/staging/wlan-ng/p80211netdev.c | 74 > -- > 1 file changed, 43 insertions(+), 31 deletions(-) > > diff --git a/drivers/staging/wlan-ng/p80211netdev.c > b/drivers/staging/wlan-ng/p80211netdev.c > index 2da3cbb..61a3092 100644 > --- a/drivers/staging/wlan-ng/p80211netdev.c > +++ b/drivers/staging/wlan-ng/p80211netdev.c > @@ -240,6 +240,48 @@ void p80211netdev_rx(wlandevice_t *wlandev, struct > sk_buff *skb) > tasklet_schedule(&wlandev->rx_bh); > } > > +#define CONV_TO_ETHER_SKIPPED0x01 > +#define CONV_TO_ETHER_FAILED 0x02 > + > +/** > + * convert_frame_to_ether - conversion from 802.11 frame to ethernet frame > + * @wlandev: pointer to WLAN device > + * @skb: pointer to socket buffer > + * > + * Returns: 0 if conversion succeeded > + * CONV_TO_ETHER_FAILED if conversion failed > + * CONV_TO_ETHER_SKIPPED if frame is ignored > + */ > +static int convert_frame_to_ether(wlandevice_t *wlandev, struct sk_buff *skb) You should probably add a module/driver specific prefix to the function name here, to stay consistent with the other functions in the module, e.g. p80211_convert_frame_to_ether() > +{ > + struct p80211_hdr_a3 *hdr; > + > + hdr = (struct p80211_hdr_a3 *) skb->data; > + if (p80211_rx_typedrop(wlandev, hdr->fc)) > + return CONV_TO_ETHER_SKIPPED; > + > + /* perform mcast filtering */ > + if (wlandev->netdev->flags & IFF_ALLMULTI) { > + /* allow my local address through */ > + if (memcmp(hdr->a1, wlandev->netdev->dev_addr, ETH_ALEN) != 0) { While at it, you could use one of the ether_addr_equal* functions here (depending on the address alignment). > + /* but reject anything else that isn't multicast */ > + if (!(hdr->a1[0] & 0x01)) > + return CONV_TO_ETHER_SKIPPED; And here you could use !is_multicast_ether_addr() instead of open coding it. > + } > + } > + > + if (skb_p80211_to_ether(wlandev, wlandev->ethconv, skb) == 0) { > + skb->dev->last_rx = jiffies; > + wlandev->linux_stats.rx_packets++; > + wlandev->linux_stats.rx_bytes += skb->len; > + netif_rx_ni(skb); > + return 0; > + } > + > + pr_debug("p80211_to_ether failed.\n"); > + return CONV_TO_ETHER_FAILED; > +} > + > /** > * p80211netdev_rx_bh - deferred processing of all received frames > * > @@ -250,7 +292,6 @@ static void p80211netdev_rx_bh(unsigned long arg) > wlandevice_t *wlandev = (wlandevice_t *) arg; > struct sk_buff *skb = NULL; > netdevice_t *dev = wlandev->netdev; > - struct p80211_hdr_a3 *hdr; > > /* Let's empty our our queue */ > while ((skb = skb_dequeue(&wlandev->nsd_rxq))) { > @@ -273,37 +314,8 @@ static void p80211netdev_rx_bh(unsigned long arg) > netif_rx_ni(skb); > continue; > } else { > - hdr = (struct p80211_hdr_a3 *) skb->data; > - if (p80211_rx_typedrop(wlandev, hdr->fc)) { > - dev_kfree_skb(skb); > - continue; > - } > - > - /* perform mcast filtering */ > - if (wlandev->netdev->flags & IFF_ALLMULTI) { > - /* allow my local address through */ > - if (memcmp > - (hdr->a1, wlandev->netdev->dev_addr, > - ETH_ALEN) != 0) { > - /* but reject anything else that > -isn't multicast */ > - if (!(hdr->a1[0] & 0x01)) { > - dev_kfree_skb(skb); > - continue; > - } > - } > - } > - > - if (skb_p80211_to_ether > - (wlandev, wlandev->ethconv, skb) == 0) { > - skb->dev->last_rx = jiffies; > - wlandev->linux_stats.rx_packets++; > - wlandev->linux_stats.rx_bytes += > - skb->len; > - netif_rx_ni(skb); > + if (!convert_frame_to_ether(wlandev, skb)) >
Re: [PATCH 0/3] Staging: bcm: Bcmchar.c cleanup patches
On 2014-05-20 at 15:04:50 +0200, Matthias Beyer wrote: > Hi, > > I did some cleanup for the file > > drivers/staging/bcm/Bcmchar.c > > The patches shorten some lines and do some code outsourcing from large > functions > into smaller ones. > > Can someone tell me how to compile (and maybe even test) my patches? You could compile test them as follows: make allyesconfig make drivers/staging/bcm/ In order to test the changes, you'd the the according hardware. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging/lustre/llite: Remove unnecessary check for NULL before iput()
iput() already checks for the inode being NULL, thus it's unnecessary to check before calling. Signed-off-by: Tobias Klauser --- drivers/staging/lustre/lustre/llite/llite_lib.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c b/drivers/staging/lustre/lustre/llite/llite_lib.c index 7372986..deca27e 100644 --- a/drivers/staging/lustre/lustre/llite/llite_lib.c +++ b/drivers/staging/lustre/lustre/llite/llite_lib.c @@ -591,8 +591,7 @@ static int client_common_fill_super(struct super_block *sb, char *md, char *dt, return err; out_root: - if (root) - iput(root); + iput(root); out_lock_cn_cb: obd_fid_fini(sbi->ll_dt_exp->exp_obd); out_dt: -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: xillybus: Consolidate return statements in xilly_probe()
No need for two return statements, just call xillybus_do_cleanup() in case of an error before returning. Signed-off-by: Tobias Klauser --- drivers/staging/xillybus/xillybus_pcie.c |7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/staging/xillybus/xillybus_pcie.c b/drivers/staging/xillybus/xillybus_pcie.c index a4fe51c..518ba6c 100644 --- a/drivers/staging/xillybus/xillybus_pcie.c +++ b/drivers/staging/xillybus/xillybus_pcie.c @@ -200,11 +200,8 @@ static int xilly_probe(struct pci_dev *pdev, } rc = xillybus_endpoint_discovery(endpoint); - - if (!rc) - return 0; - - xillybus_do_cleanup(&endpoint->cleanup, endpoint); + if (rc) + xillybus_do_cleanup(&endpoint->cleanup, endpoint); return rc; } -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: xillybus: Consolidate return statements in xilly_probe()
Hi Eli On 2014-06-18 at 14:30:35 +0200, Eli Billauer wrote: > Hello, > > Thanks for the patch. However in the patch I'm trying to push, > xillybus_do_cleanup() is eliminated completely from the code (with > no success so far, because it depends on inserting a patch that adds > a devres version of dma_map_single). > > See https://lkml.org/lkml/2014/6/1/10 > > So your patch makes sense, and I have no objection to it, but it > fixes a piece of code that must go away soon anyhow. Thanks for the notice. I wasn't aware of your pending changes. I'll leave it up to Greg to decide whether it makes sense to apply the patch for the time being. Thanks Tobias > Thanks, >Eli > > On 18/06/14 15:10, Tobias Klauser wrote: > >No need for two return statements, just call xillybus_do_cleanup() in > >case of an error before returning. > > > >Signed-off-by: Tobias Klauser > >--- > > drivers/staging/xillybus/xillybus_pcie.c |7 ++- > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > >diff --git a/drivers/staging/xillybus/xillybus_pcie.c > >b/drivers/staging/xillybus/xillybus_pcie.c > >index a4fe51c..518ba6c 100644 > >--- a/drivers/staging/xillybus/xillybus_pcie.c > >+++ b/drivers/staging/xillybus/xillybus_pcie.c > >@@ -200,11 +200,8 @@ static int xilly_probe(struct pci_dev *pdev, > > } > > > > rc = xillybus_endpoint_discovery(endpoint); > >- > >-if (!rc) > >-return 0; > >- > >-xillybus_do_cleanup(&endpoint->cleanup, endpoint); > >+if (rc) > >+xillybus_do_cleanup(&endpoint->cleanup, endpoint); > > > > return rc; > > } > > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: xillybus: Move out of staging
On 2014-08-31 at 09:57:01 +0200, Eli Billauer wrote: > This driver has been functional and stable throughout the year it has spent > in the staging area. It has been patched for minor bugs, coding style issues > and improvements during this period, so I suppose its time has come. > > I shall continue being responsible for its maintenance. In this case a entry in MAINTAINERS would make sense. This way you'll make sure patches get submitted to you. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/4] staging: ozwpan: use kmalloc_array over kmalloc with multiply
Your From: line does not contain a real name and does not match your Signed-off-by line, please check your e-mail client settings. On 2014-09-07 at 20:28:25 +0200, anicoara wrote: No changelog text? Please add a short notice, describing why this change is done. > Signed-off-by: Adrian Nicoara > --- > Patch submitted as part of the Eudyptula challenge. > > drivers/staging/ozwpan/ozhcd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c > index ba2168f..4cfe56e 100644 > --- a/drivers/staging/ozwpan/ozhcd.c > +++ b/drivers/staging/ozwpan/ozhcd.c > @@ -1315,7 +1315,7 @@ static int oz_build_endpoints_for_config(struct usb_hcd > *hcd, > if (num_iface) { > struct oz_interface *iface; > > - iface = kmalloc(num_iface*sizeof(struct oz_interface), > + iface = kmalloc_array(num_iface, sizeof(struct oz_interface), > mem_flags | __GFP_ZERO); The line above should be adjusted to match the opening parenthesis. > if (!iface) > return -ENOMEM; > -- > 2.0.1 > > ___ > devel mailing list > de...@linuxdriverproject.org > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] staging: ozwpan: fix redundant return in void function
Your From: line does not contain a real name and does not match your Signed-off-by line, please check your e-mail client settings. On 2014-09-07 at 20:25:35 +0200, anicoara wrote: > Signed-off-by: Adrian Nicoara No changelog text? Please add a short notice, describing why this change is done. > --- > Patch submitted as part of the Eudyptula challenge. > > drivers/staging/ozwpan/ozproto.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/staging/ozwpan/ozproto.c > b/drivers/staging/ozwpan/ozproto.c > index cae0e6f..3d3a3a8 100644 > --- a/drivers/staging/ozwpan/ozproto.c > +++ b/drivers/staging/ozwpan/ozproto.c > @@ -112,7 +112,6 @@ static void oz_send_conn_rsp(struct oz_pd *pd, u8 status) > } > oz_dbg(ON, "TX: OZ_ELT_CONNECT_RSP %d", status); > dev_queue_xmit(skb); > - return; > } > > /* > -- > 2.0.1 > > ___ > devel mailing list > de...@linuxdriverproject.org > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] staging: ozwpan: fix redundant else after break or return
Your From: line does not contain a real name and does not match your Signed-off-by line, please check your e-mail client settings. On 2014-09-07 at 20:24:03 +0200, anicoara wrote: > Signed-off-by: Adrian Nicoara No changelog text? Please add a short notice, describing why this change is done. > --- > Patch submitted as part of the Eudyptula challenge. > > drivers/staging/ozwpan/ozhcd.c | 8 +++- > drivers/staging/ozwpan/ozpd.c | 10 -- > 2 files changed, 7 insertions(+), 11 deletions(-) > > diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c > index b30c4d87..ba2168f 100644 > --- a/drivers/staging/ozwpan/ozhcd.c > +++ b/drivers/staging/ozwpan/ozhcd.c > @@ -1010,10 +1010,9 @@ void oz_hcd_data_ind(void *hport, u8 endpoint, const > u8 *data, int data_len) > urb->actual_length = copy_len; > oz_complete_urb(port->ozhcd->hcd, urb, 0); > return; > - } else { > - oz_dbg(ON, "buffering frame as URB is not available\n"); > - oz_hcd_buffer_data(ep, data, data_len); > } > + oz_dbg(ON, "buffering frame as URB is not available\n"); > + oz_hcd_buffer_data(ep, data, data_len); > break; > case USB_ENDPOINT_XFER_ISOC: > oz_hcd_buffer_data(ep, data, data_len); > @@ -1903,8 +1902,7 @@ static int oz_hcd_hub_status_data(struct usb_hcd *hcd, > char *buf) > spin_unlock_bh(&ozhcd->hcd_lock); > if (buf[0] != 0 || buf[1] != 0) > return 2; > - else > - return 0; > + return 0; > } > > /* > diff --git a/drivers/staging/ozwpan/ozpd.c b/drivers/staging/ozwpan/ozpd.c > index 26c1049..852c288 100644 > --- a/drivers/staging/ozwpan/ozpd.c > +++ b/drivers/staging/ozwpan/ozpd.c > @@ -496,11 +496,10 @@ static int oz_send_next_queued_frame(struct oz_pd *pd, > int more_data) > oz_dbg(TX_FRAMES, "Sending ISOC Frame, nb_isoc= %d\n", > pd->nb_queued_isoc_frames); > return 0; > - } else { > - kfree_skb(skb); > - oz_dbg(TX_FRAMES, "Dropping ISOC Frame>\n"); > - return -1; > } > + kfree_skb(skb); > + oz_dbg(TX_FRAMES, "Dropping ISOC Frame>\n"); > + return -1; > } > > pd->last_sent_frame = e; > @@ -813,8 +812,7 @@ int oz_send_isoc_unit(struct oz_pd *pd, u8 ep_num, const > u8 *data, int len) > atomic_inc(&g_submitted_isoc); > if (dev_queue_xmit(skb) < 0) > return -1; > - else > - return 0; > + return 0; > } > > out: kfree_skb(skb); > -- > 2.0.1 > > ___ > devel mailing list > de...@linuxdriverproject.org > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] staging: ozwpan: fix missing blank line after declaration
Your From: line does not contain a real name and does not match your Signed-off-by line, please check your e-mail client settings. On 2014-09-07 at 20:21:41 +0200, anicoara wrote: Changelog text is missing. Please add a short notice, describing why this change is done. > Signed-off-by: Adrian Nicoara > --- > Patch submitted as part of the Eudyptula challenge. > > drivers/staging/ozwpan/ozcdev.c| 5 + > drivers/staging/ozwpan/ozeltbuf.c | 1 + > drivers/staging/ozwpan/ozpd.c | 6 ++ > drivers/staging/ozwpan/ozproto.c | 4 > drivers/staging/ozwpan/ozusbsvc.c | 2 ++ > drivers/staging/ozwpan/ozusbsvc1.c | 4 > 6 files changed, 22 insertions(+) > > diff --git a/drivers/staging/ozwpan/ozcdev.c b/drivers/staging/ozwpan/ozcdev.c > index c73d396..da0e1fd 100644 > --- a/drivers/staging/ozwpan/ozcdev.c > +++ b/drivers/staging/ozwpan/ozcdev.c > @@ -263,6 +263,7 @@ static long oz_cdev_ioctl(struct file *filp, unsigned int > cmd, > switch (cmd) { > case OZ_IOCTL_GET_PD_LIST: { > struct oz_pd_list list; > + > oz_dbg(ON, "OZ_IOCTL_GET_PD_LIST\n"); > memset(&list, 0, sizeof(list)); > list.count = oz_get_pd_list(list.addr, OZ_MAX_PDS); > @@ -273,6 +274,7 @@ static long oz_cdev_ioctl(struct file *filp, unsigned int > cmd, > break; > case OZ_IOCTL_SET_ACTIVE_PD: { > u8 addr[ETH_ALEN]; > + > oz_dbg(ON, "OZ_IOCTL_SET_ACTIVE_PD\n"); > if (copy_from_user(addr, (void __user *)arg, ETH_ALEN)) > return -EFAULT; > @@ -281,6 +283,7 @@ static long oz_cdev_ioctl(struct file *filp, unsigned int > cmd, > break; > case OZ_IOCTL_GET_ACTIVE_PD: { > u8 addr[ETH_ALEN]; > + > oz_dbg(ON, "OZ_IOCTL_GET_ACTIVE_PD\n"); > spin_lock_bh(&g_cdev.lock); > ether_addr_copy(addr, g_cdev.active_addr); > @@ -292,6 +295,7 @@ static long oz_cdev_ioctl(struct file *filp, unsigned int > cmd, > case OZ_IOCTL_ADD_BINDING: > case OZ_IOCTL_REMOVE_BINDING: { > struct oz_binding_info b; > + > if (copy_from_user(&b, (void __user *)arg, > sizeof(struct oz_binding_info))) { > return -EFAULT; > @@ -320,6 +324,7 @@ static unsigned int oz_cdev_poll(struct file *filp, > poll_table *wait) > spin_lock_bh(&dev->lock); > if (dev->active_pd) { > struct oz_serial_ctx *ctx = oz_cdev_claim_ctx(dev->active_pd); > + > if (ctx) { > if (ctx->rd_in != ctx->rd_out) > ret |= POLLIN | POLLRDNORM; > diff --git a/drivers/staging/ozwpan/ozeltbuf.c > b/drivers/staging/ozwpan/ozeltbuf.c > index 400cc75..01b25da 100644 > --- a/drivers/staging/ozwpan/ozeltbuf.c > +++ b/drivers/staging/ozwpan/ozeltbuf.c > @@ -174,6 +174,7 @@ int oz_queue_elt_info(struct oz_elt_buf *buf, u8 isoc, u8 > id, > == OZ_USB_ENDPOINT_DATA) && > (body->format == OZ_DATA_F_ISOC_FIXED)) { > u8 unit_count = body->frame_number; > + > body->frame_number = st->frame_number; > st->frame_number += unit_count; > } > diff --git a/drivers/staging/ozwpan/ozpd.c b/drivers/staging/ozwpan/ozpd.c > index 7726410..26c1049 100644 > --- a/drivers/staging/ozwpan/ozpd.c > +++ b/drivers/staging/ozwpan/ozpd.c > @@ -106,6 +106,7 @@ struct oz_pd *oz_pd_alloc(const u8 *mac_addr) > > if (pd) { > int i; > + > atomic_set(&pd->ref_count, 2); > for (i = 0; i < OZ_NB_APPS; i++) > spin_lock_init(&pd->app_lock[i]); > @@ -153,6 +154,7 @@ static void oz_pd_free(struct work_struct *work) > > list_for_each_safe(e, n, &pd->tx_queue) { > struct oz_tx_frame *f = list_entry(e, struct oz_tx_frame, link); > + > if (f->skb != NULL) > kfree_skb(f->skb); > oz_retire_frame(pd, f); > @@ -249,6 +251,7 @@ void oz_pd_heartbeat(struct oz_pd *pd, u16 apps) > hrtimer_cancel(&pd->heartbeat); > if (pd->mode & OZ_F_ISOC_ANYTIME) { > int count = 8; > + > while (count-- && (oz_send_isoc_frame(pd) >= 0)) > ; > } > @@ -752,6 +755,7 @@ int oz_send_isoc_unit(struct oz_pd *pd, u8 ep_num, const > u8 *data, int len) > } else { > struct oz_hdr oz; > struct oz_isoc_large iso; > + > spin_lock_bh(&pd->stream_lock); > iso.frame_number = st->frame_num; > st->frame_num += nb_units; > @@ -774,8 +778,10 @@ int oz_send_isoc_unit(struct oz_pd *pd, u8 ep_num, const > u8 *data, int len) > if (!(pd->mode & OZ_F
Re: [PATCH v2 4/4] staging: ozwpan: use kmalloc_array over kmalloc with multiply
On 2014-09-08 at 21:02:49 +0200, Adrian Nicoara wrote: > Cleanup checkpatch.pl warnings. Please state which warning this is fixing. Same goes for patches 1-3. > Signed-off-by: Adrian Nicoara > --- > > Somehow I missed the comment Tobias made on the line indentation. I fixed the > patch, and added here as a reply - the previous patch should be ignored. > > drivers/staging/ozwpan/ozhcd.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c > index ba2168f..e880452 100644 > --- a/drivers/staging/ozwpan/ozhcd.c > +++ b/drivers/staging/ozwpan/ozhcd.c > @@ -1315,8 +1315,8 @@ static int oz_build_endpoints_for_config(struct usb_hcd > *hcd, > if (num_iface) { > struct oz_interface *iface; > > - iface = kmalloc(num_iface*sizeof(struct oz_interface), > - mem_flags | __GFP_ZERO); > + iface = kmalloc_array(num_iface, sizeof(struct oz_interface), > + mem_flags | __GFP_ZERO); This still isn't entirely correct. It should rather look like this: iface = kmalloc_array(num_iface, sizeof(struct oz_interface), mem_flags | __GFP_ZERO); Assume a tab width of 8 (as per CodingStyle) and use spaces to get the alignment right. > if (!iface) > return -ENOMEM; > spin_lock_bh(&ozhcd->hcd_lock); > -- > 2.0.1 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: vt6655: Remove unused member from struct vnt_private
The pci_state member of struct vnt_private is used nowhere in the code, so remove it. Supposedly it was used to save the PCI configuration space which is now done using pci_save_state(). Signed-off-by: Tobias Klauser --- drivers/staging/vt6655/device.h | 4 1 file changed, 4 deletions(-) diff --git a/drivers/staging/vt6655/device.h b/drivers/staging/vt6655/device.h index 7db0404..5e41b19 100644 --- a/drivers/staging/vt6655/device.h +++ b/drivers/staging/vt6655/device.h @@ -333,10 +333,6 @@ typedef struct __device_opt { struct vnt_private { struct pci_dev *pcid; -#ifdef CONFIG_PM - u32 pci_state[16]; -#endif - // netdev struct net_device *dev; struct net_device_stats stats; -- 2.0.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: vt6655: Remove unnecessary condition around include
ethtool (and SIOCETHTOOL in particular) is part of Linux since the pre-git era, it thus makes no sense no sense to make the include of linux/ethtool.h conditional. Also remove the unused define DEVICE_ETHTOOL_IOCTL_SUPPORT. Signed-off-by: Tobias Klauser --- drivers/staging/vt6655/device.h | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/staging/vt6655/device.h b/drivers/staging/vt6655/device.h index e37469a..ddd356a 100644 --- a/drivers/staging/vt6655/device.h +++ b/drivers/staging/vt6655/device.h @@ -55,12 +55,7 @@ #include #include #include -#ifdef SIOCETHTOOL -#define DEVICE_ETHTOOL_IOCTL_SUPPORT #include -#else -#undef DEVICE_ETHTOOL_IOCTL_SUPPORT -#endif /* Include Wireless Extension definition and check version - Jean II */ #include #include // New driver API -- 2.0.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: vt6655: Use net_device_stats from struct net_device
Instead of using an own copy of struct net_device_stats in struct vnt_private, use stats from struct net_device. Also remove the thus unnecessary device_get_stats(), as it would now just return netdev->stats, which is the default in dev_get_stats(). Signed-off-by: Tobias Klauser --- drivers/staging/vt6655/device.h | 1 - drivers/staging/vt6655/device_main.c | 11 +-- drivers/staging/vt6655/dpc.c | 2 +- 3 files changed, 2 insertions(+), 12 deletions(-) diff --git a/drivers/staging/vt6655/device.h b/drivers/staging/vt6655/device.h index 5e41b19..e37469a 100644 --- a/drivers/staging/vt6655/device.h +++ b/drivers/staging/vt6655/device.h @@ -335,7 +335,6 @@ struct vnt_private { // netdev struct net_device *dev; - struct net_device_stats stats; //dma addr, rx/tx pool dma_addr_t pool_dma; diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c index f33795f..763768a 100644 --- a/drivers/staging/vt6655/device_main.c +++ b/drivers/staging/vt6655/device_main.c @@ -264,7 +264,6 @@ static void vt6655_init_info(struct pci_dev *pcid, static void device_free_info(struct vnt_private *pDevice); static bool device_get_pci_info(struct vnt_private *, struct pci_dev *pcid); static void device_print_info(struct vnt_private *pDevice); -static struct net_device_stats *device_get_stats(struct net_device *dev); static void device_init_diversity_timer(struct vnt_private *pDevice); static int device_open(struct net_device *dev); static int device_xmit(struct sk_buff *skb, struct net_device *dev); @@ -807,7 +806,6 @@ static const struct net_device_ops device_netdev_ops = { .ndo_open = device_open, .ndo_stop = device_close, .ndo_do_ioctl = device_ioctl, - .ndo_get_stats = device_get_stats, .ndo_start_xmit = device_xmit, .ndo_set_rx_mode= device_set_multi, }; @@ -1409,7 +1407,7 @@ static int device_tx_srv(struct vnt_private *pDevice, unsigned int uIdx) unsigned char byTsr1; unsigned intuFrameSize, uFIFOHeaderSize; PSTxBufHead pTxBufHead; - struct net_device_stats *pStats = &pDevice->stats; + struct net_device_stats *pStats = &pDevice->dev->stats; struct sk_buff *skb; unsigned intuNodeIndex; PSMgmtObject pMgmt = pDevice->pMgmt; @@ -2590,13 +2588,6 @@ static void device_set_multi(struct net_device *dev) { pr_debug("pDevice->byRxMode = %x\n", pDevice->byRxMode); } -static struct net_device_stats *device_get_stats(struct net_device *dev) -{ - struct vnt_private *pDevice = netdev_priv(dev); - - return &pDevice->stats; -} - static int device_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) { struct vnt_private *pDevice = netdev_priv(dev); diff --git a/drivers/staging/vt6655/dpc.c b/drivers/staging/vt6655/dpc.c index 6fb6cdf..8515b8c 100644 --- a/drivers/staging/vt6655/dpc.c +++ b/drivers/staging/vt6655/dpc.c @@ -274,7 +274,7 @@ device_receive_frame( ) { PDEVICE_RD_INFO pRDInfo = pCurrRD->pRDInfo; - struct net_device_stats *pStats = &pDevice->stats; + struct net_device_stats *pStats = &pDevice->dev->stats; struct sk_buff *skb; PSMgmtObjectpMgmt = pDevice->pMgmt; PSRxMgmtPacket pRxPacket = &(pDevice->pMgmt->sRxPacket); -- 2.0.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192u: delete unused function CAM_read_entry
On 2014-09-17 at 21:17:29 +0200, Benedict Boerger wrote: > Fix the sparse warning: symbol 'CAM_read_entry' was not declared. Should it > be static? > > The function CAM_read_entry is not used and therefore deleted. Your patch is missing a Signed-off-by line. Please check Documentation/SubmittingPatches for the proper format to submit patches. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH linux-next] et131x: Promote staging et131x driver to drivers/net
On 2014-09-22 at 23:28:03 +0200, Mark Einon wrote: > This patch moves the et131x gigabit ethernet driver from drivers/staging > to drivers/net/ethernet/agere. > > There are no known issues at this time. > > Signed-off-by: Mark Einon > --- > > This patch will only apply once the last few pending changes > make their way from staging-next to linux-next, but posting > now in the hope that feedback can be given and this change can > make it in before the next merge window. > > drivers/net/ethernet/Kconfig|1 + > drivers/net/ethernet/Makefile |1 + > drivers/net/ethernet/agere/Kconfig | 31 + > drivers/net/ethernet/agere/Makefile |5 + > drivers/net/ethernet/agere/et131x.c | 4412 > +++ > drivers/net/ethernet/agere/et131x.h | 1533 > drivers/staging/Kconfig |2 - > drivers/staging/Makefile|1 - > drivers/staging/et131x/Kconfig | 10 - > drivers/staging/et131x/Makefile |5 - > drivers/staging/et131x/README | 15 - > drivers/staging/et131x/et131x.c | 4412 > --- > drivers/staging/et131x/et131x.h | 1533 > 13 files changed, 5983 insertions(+), 5978 deletions(-) > create mode 100644 drivers/net/ethernet/agere/Kconfig > create mode 100644 drivers/net/ethernet/agere/Makefile > create mode 100644 drivers/net/ethernet/agere/et131x.c > create mode 100644 drivers/net/ethernet/agere/et131x.h > delete mode 100644 drivers/staging/et131x/Kconfig > delete mode 100644 drivers/staging/et131x/Makefile > delete mode 100644 drivers/staging/et131x/README > delete mode 100644 drivers/staging/et131x/et131x.c > delete mode 100644 drivers/staging/et131x/et131x.h [...] > diff --git a/drivers/net/ethernet/agere/et131x.c > b/drivers/net/ethernet/agere/et131x.c > new file mode 100644 > index 000..93afd61 > --- /dev/null > +++ b/drivers/net/ethernet/agere/et131x.c > @@ -0,0 +1,4412 @@ [...] > +/* et131x_rx_dma_memory_alloc > + * > + * Allocates Free buffer ring 1 for sure, free buffer ring 0 if required, > + * and the Packet Status Ring. > + */ > +static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter) > +{ > + u8 id; > + u32 i, j; > + u32 bufsize; > + u32 psr_size; > + u32 fbr_chunksize; > + struct rx_ring *rx_ring = &adapter->rx_ring; > + struct fbr_lookup *fbr; > + > + /* Alloc memory for the lookup table */ > + rx_ring->fbr[0] = kmalloc(sizeof(*fbr), GFP_KERNEL); > + if (rx_ring->fbr[0] == NULL) > + return -ENOMEM; > + rx_ring->fbr[1] = kmalloc(sizeof(*fbr), GFP_KERNEL); > + if (rx_ring->fbr[1] == NULL) > + return -ENOMEM; If you fail here, et131x_rx_dma_memory_free() will be called by et131x_adapter_memory_free() in the error handling path which in turn will access the members of the already allocated rx_ring->fbr[0] (e.g. ->buffsize). Since it is allocated with kmalloc() these members contain arbitrary values and cause problems in et131x_rx_dma_memory_free(). I' suggest to do the cleanup (i.e. free rx_ring->fbr[0] directly here and not call et131x_rx_dma_memory_free() in the error handling path. You might want to check that memory allocation failures are properly handled in the rest of the driver as well. There are multiple other cases where et131x_adapter_memory_free() is called on partially allocated/initialized memory. > + > + /* The first thing we will do is configure the sizes of the buffer > + * rings. These will change based on jumbo packet support. Larger > + * jumbo packets increases the size of each entry in FBR0, and the > + * number of entries in FBR0, while at the same time decreasing the > + * number of entries in FBR1. > + * > + * FBR1 holds "large" frames, FBR0 holds "small" frames. If FBR1 > + * entries are huge in order to accommodate a "jumbo" frame, then it > + * will have less entries. Conversely, FBR1 will now be relied upon > + * to carry more "normal" frames, thus it's entry size also increases > + * and the number of entries goes up too (since it now carries > + * "small" + "regular" packets. > + * > + * In this scheme, we try to maintain 512 entries between the two > + * rings. Also, FBR1 remains a constant size - when it's size doubles > + * the number of entries halves. FBR0 increases in size, however. > + */ > + if (adapter->registry_jumbo_packet < 2048) { > + rx_ring->fbr[0]->buffsize = 256; > + rx_ring->fbr[0]->num_entries = 512; > + rx_ring->fbr[1]->buffsize = 2048; > + rx_ring->fbr[1]->num_entries = 512; > + } else if (adapter->registry_jumbo_packet < 4096) { > + rx_ring->fbr[0]->buffsize = 512; > + rx_ring->fbr[0]->num_entries = 1024; > + rx_ring->fbr[1]->buffsize = 4096; > + rx_ring->fbr[1]->num_entries = 512; > + }
Re: [RFC PATCH linux-next] et131x: Promote staging et131x driver to drivers/net
On 2014-09-23 at 11:46:15 +0200, Mark Einon wrote: > On Tue, Sep 23, 2014 at 09:22:53AM +0200, Tobias Klauser wrote: > > Hi Tobias, > > Thanks for the details review. I've replied below - > > [...] > > > +/* et131x_rx_dma_memory_alloc > > > + * > > > + * Allocates Free buffer ring 1 for sure, free buffer ring 0 if required, > > > + * and the Packet Status Ring. > > > + */ > > > +static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter) > > > +{ > > > + u8 id; > > > + u32 i, j; > > > + u32 bufsize; > > > + u32 psr_size; > > > + u32 fbr_chunksize; > > > + struct rx_ring *rx_ring = &adapter->rx_ring; > > > + struct fbr_lookup *fbr; > > > + > > > + /* Alloc memory for the lookup table */ > > > + rx_ring->fbr[0] = kmalloc(sizeof(*fbr), GFP_KERNEL); > > > + if (rx_ring->fbr[0] == NULL) > > > + return -ENOMEM; > > > + rx_ring->fbr[1] = kmalloc(sizeof(*fbr), GFP_KERNEL); > > > + if (rx_ring->fbr[1] == NULL) > > > + return -ENOMEM; > > > > If you fail here, et131x_rx_dma_memory_free() will be called by > > et131x_adapter_memory_free() in the error handling path which in turn > > will access the members of the already allocated rx_ring->fbr[0] (e.g. > > ->buffsize). Since it is allocated with kmalloc() these members contain > > arbitrary values and cause problems in et131x_rx_dma_memory_free(). I' > > suggest to do the cleanup (i.e. free rx_ring->fbr[0] directly here and > > not call et131x_rx_dma_memory_free() in the error handling path. You > > might want to check that memory allocation failures are properly handled > > in the rest of the driver as well. There are multiple other cases where > > et131x_adapter_memory_free() is called on partially > > allocated/initialized memory. > > > > I don't think this is the case - the members of rx_ring->fbr[x] are not > accessed unless this pointer is non-NULL in et131x_rx_dma_memory_free() > (see code snippet below), which is exactly why the kmalloc code above > would fail, so buffsize never gets accessed and the code is cleaned up > correctly. Actually, for further explanation, this thread discusses > these changes: > > http://www.spinics.net/lists/linux-driver-devel/msg42128.html Hm, won't rx_ring->fbr[0] be accessed, since it was successfully allocated? However, it isn't properly initialized, so at least the ->ring_virtaddr will get accessed and if this is non-NULL by accident also further members? > > > +/* et131x_rx_dma_memory_free - Free all memory allocated within this > > > module */ > > > +static void et131x_rx_dma_memory_free(struct et131x_adapter *adapter) > > > +{ > > [...] > > > > + /* Free Free Buffer Rings */ > > > + for (id = 0; id < NUM_FBRS; id++) { > > > + fbr = rx_ring->fbr[id]; > > > + > > > + if (!fbr || !fbr->ring_virtaddr) > > > + continue; > > > + > > > + /* First the packet memory */ > > > + for (ii = 0; ii < fbr->num_entries / FBR_CHUNKS; ii++) { > > > + if (fbr->mem_virtaddrs[ii]) { > > > + bufsize = fbr->buffsize * FBR_CHUNKS; > > [...] > > > > +static SIMPLE_DEV_PM_OPS(et131x_pm_ops, et131x_suspend, et131x_resume); > > > +#define ET131X_PM_OPS (&et131x_pm_ops) > > > +#else > > > +#define ET131X_PM_OPS NULL > > > +#endif > > > > No need for the #define here, just assigne et131x_pm_ops to .driver.pm > > directly, its members will be NULL and thus never called. Also, you can > > make et131x_pm_ops const. > > Ok, I can change this. > > Btw, this appears to be a fairly standard way of using .driver.pm among > ethernet drivers, e.g. see 3com/3c59x.c, atheros, marvell... - perhaps > there is a case for changing all instances of this code? Yes, this would probably make sense. Looks like a good job for a coccinelle patch. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 05/10] added media specific (MS) TCP drivers
On 2014-11-03 at 21:42:52 +0100, Stephanie Wallick wrote: > This is where we handle media specific packets and transport. The MS driver > interfaces with a media agnostic (MA) driver via a series of transfer pairs. > Transfer pairs consist of a set of functions to pass MA USB packets back > and forth between MA and MS drivers. There is one transfer pair per device > endpoint and one transfer pair for control/management traffic. When the MA > driver needs to send an MA USB packet, it hands the packet off to the MS > layer where the packet is converted into an MS form and sent via TCP over > the underlying ethernet or wireless medium. When the MS driver receives a > packet, it converts it into an MA USB packet and hands it off the the MA > driver for handling. > > In addition, the MS driver provides an interface to inititate connection > events. > Because there are no physical MA USB ports in an MA USB host, the host must be > notified via software when a device is connected. > > Lastly, the MS driver contains a number of ioctl functions that are used by a > utility to adjust medium-related driver parameters and connect or disconnect > the > MA USB host and device drivers. > > Signed-off-by: Sean O. Stalley > Signed-off-by: Stephanie Wallick > --- > drivers/staging/mausb/drivers/mausb_ioctl.c | 373 +++ > drivers/staging/mausb/drivers/mausb_ioctl.h | 99 + > drivers/staging/mausb/drivers/mausb_msapi.c | 110 ++ > drivers/staging/mausb/drivers/mausb_msapi.h | 232 > drivers/staging/mausb/drivers/mausb_tcp-device.c | 147 > drivers/staging/mausb/drivers/mausb_tcp-host.c | 144 > drivers/staging/mausb/drivers/mausb_tcp.c| 446 > +++ > drivers/staging/mausb/drivers/mausb_tcp.h| 129 +++ > 8 files changed, 1680 insertions(+) > create mode 100644 drivers/staging/mausb/drivers/mausb_ioctl.c > create mode 100644 drivers/staging/mausb/drivers/mausb_ioctl.h > create mode 100644 drivers/staging/mausb/drivers/mausb_msapi.c > create mode 100644 drivers/staging/mausb/drivers/mausb_msapi.h > create mode 100644 drivers/staging/mausb/drivers/mausb_tcp-device.c > create mode 100644 drivers/staging/mausb/drivers/mausb_tcp-host.c > create mode 100644 drivers/staging/mausb/drivers/mausb_tcp.c > create mode 100644 drivers/staging/mausb/drivers/mausb_tcp.h > > diff --git a/drivers/staging/mausb/drivers/mausb_ioctl.c > b/drivers/staging/mausb/drivers/mausb_ioctl.c > new file mode 100644 > index 000..0c6c6bd > --- /dev/null > +++ b/drivers/staging/mausb/drivers/mausb_ioctl.c [...] > +/** > + * This function is used to send a message to the user, in other words, the > + * calling process. It basically copies the message one byte at a time. > + * > + * @msg: The message to be sent to the user. > + * @buffer: The buffer in which to put the message. This buffer was given to > + * us to fill. > + */ > +void to_user(char *msg, long unsigned int buffer) > +{ > + int length = (int)strlen(msg); > + int bytes = 0; > + > + while (length && *msg) { > + put_user(*(msg++), (char *)buffer++); > + length--; > + bytes++; > + } Any reason not to use copy_to_user here? That way, access_ok would only need to be executed once for the whole range. In any case, the return value of put_user/copy_to_user will need to be checked. > + > + put_user('\0', (char *)buffer + bytes); > +} [...] > +/** > + * This function is used to read from the device file. From the perspective > of > + * the device, the user is reading information from us. This is one of the > + * entry points to this module. > + * > + * @file:The device file. We don't use it directly, but it's passed in. > + * @buffer: The buffer to put the message into. > + * @length: The max length to be read. > + * @offset: File offset, which we don't use but it is passed in nontheless. > + */ > +static ssize_t mausb_read(struct file *file, char __user *buffer, > + size_t length, loff_t *offset) > +{ > + int bytes_read = 0; > + > + if (*message_point == 0) > + return 0; > + while (length && *message_point) { > + put_user(*(message_point++), buffer++); > + length--; > + bytes_read++; > + } See comment for to_user above. Why not use copy_to_user? > + > + return bytes_read; > +} > + > +/** > + * This function is used to write to the device file. From the perspective of > + * the device, the user is writing information to us. This is one of the > + * entry points to this module. > + * > + * @file:The device file. We don't use it directly, but it's passed in. > + * @buffer: The buffer that holds the message. > + * @length: The length of the message to be written. > + * @offset: File offset, which we don't use but it is passed in nontheless. > + */ > +static ssize_t mausb_write(struct file *file, const
[PATCH] staging: wlan-ng: Use net_device_stats from struct net_device
Instead of using an own copy of struct net_device_stats in struct wlandevice, use stats from struct net_device. Also remove the thus unnecessary .ndo_get_stats function, as it would now just return netdev->stats, which is the default in dev_get_stats(). Furthermore, convert prefix increment of stats counters to the more common postfix increment idiom. Signed-off-by: Tobias Klauser --- drivers/staging/wlan-ng/hfa384x_usb.c | 18 +++ drivers/staging/wlan-ng/p80211netdev.c | 40 ++ drivers/staging/wlan-ng/p80211netdev.h | 1 - drivers/staging/wlan-ng/prism2sta.c| 2 +- 4 files changed, 17 insertions(+), 44 deletions(-) diff --git a/drivers/staging/wlan-ng/hfa384x_usb.c b/drivers/staging/wlan-ng/hfa384x_usb.c index 07cee56..2f63e0c 100644 --- a/drivers/staging/wlan-ng/hfa384x_usb.c +++ b/drivers/staging/wlan-ng/hfa384x_usb.c @@ -3158,8 +3158,8 @@ static void hfa384x_usbin_callback(struct urb *urb) /* Check for short packet */ if (urb->actual_length == 0) { - ++(wlandev->linux_stats.rx_errors); - ++(wlandev->linux_stats.rx_length_errors); + wlandev->netdev->stats.rx_errors++; + wlandev->netdev->stats.rx_length_errors++; action = RESUBMIT; } break; @@ -3169,7 +3169,7 @@ static void hfa384x_usbin_callback(struct urb *urb) wlandev->netdev->name); if (!test_and_set_bit(WORK_RX_HALT, &hw->usb_flags)) schedule_work(&hw->usb_work); - ++(wlandev->linux_stats.rx_errors); + wlandev->netdev->stats.rx_errors++; action = ABORT; break; @@ -3180,12 +3180,12 @@ static void hfa384x_usbin_callback(struct urb *urb) !timer_pending(&hw->throttle)) { mod_timer(&hw->throttle, jiffies + THROTTLE_JIFFIES); } - ++(wlandev->linux_stats.rx_errors); + wlandev->netdev->stats.rx_errors++; action = ABORT; break; case -EOVERFLOW: - ++(wlandev->linux_stats.rx_over_errors); + wlandev->netdev->stats.rx_over_errors++; action = RESUBMIT; break; @@ -3204,7 +3204,7 @@ static void hfa384x_usbin_callback(struct urb *urb) default: pr_debug("urb status=%d, transfer flags=0x%x\n", urb->status, urb->transfer_flags); - ++(wlandev->linux_stats.rx_errors); + wlandev->netdev->stats.rx_errors++; action = RESUBMIT; break; } @@ -3712,7 +3712,7 @@ static void hfa384x_usbout_callback(struct urb *urb) if (!test_and_set_bit (WORK_TX_HALT, &hw->usb_flags)) schedule_work(&hw->usb_work); - ++(wlandev->linux_stats.tx_errors); + wlandev->netdev->stats.tx_errors++; break; } @@ -3728,7 +3728,7 @@ static void hfa384x_usbout_callback(struct urb *urb) mod_timer(&hw->throttle, jiffies + THROTTLE_JIFFIES); } - ++(wlandev->linux_stats.tx_errors); + wlandev->netdev->stats.tx_errors++; netif_stop_queue(wlandev->netdev); break; } @@ -3741,7 +3741,7 @@ static void hfa384x_usbout_callback(struct urb *urb) default: netdev_info(wlandev->netdev, "unknown urb->status=%d\n", urb->status); - ++(wlandev->linux_stats.tx_errors); + wlandev->netdev->stats.tx_errors++; break; } /* switch */ } diff --git a/drivers/staging/wlan-ng/p80211netdev.c b/drivers/staging/wlan-ng/p80211netdev.c index 00b186c..29afa57 100644 --- a/drivers/staging/wlan-ng/p80211netdev.c +++ b/drivers/staging/wlan-ng/p80211netdev.c @@ -92,7 +92,6 @@ /* netdevice method functions */ static int p80211knetdev_init(netdevice_t *netdev); -static struct net_device_stats *p80211knetdev_get_stats(netdevice_t *netdev); static int p80211knetdev_open(netdevice_t *netdev); static int p80211knetdev_stop(netdevice_t *netdev); static int p80211knetdev_hard_start_xmit(struct sk_buff *skb, @@ -134,30 +133,6 @@ static int p80211k
[PATCH] staging: gdm72xx: Use net_device_stats from struct net_device
Instead of using an own copy of struct net_device_stats in struct nic, use stats from struct net_device. Also remove the thus unnecessary .ndo_get_stats function, as it would now just return netdev->stats, which is the default in dev_get_stats(). Signed-off-by: Tobias Klauser --- drivers/staging/gdm72xx/gdm_wimax.c | 17 - drivers/staging/gdm72xx/gdm_wimax.h | 1 - 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/drivers/staging/gdm72xx/gdm_wimax.c b/drivers/staging/gdm72xx/gdm_wimax.c index 4148013..6f44798 100644 --- a/drivers/staging/gdm72xx/gdm_wimax.c +++ b/drivers/staging/gdm72xx/gdm_wimax.c @@ -359,8 +359,8 @@ int gdm_wimax_send_tx(struct sk_buff *skb, struct net_device *dev) return ret; } - nic->stats.tx_packets++; - nic->stats.tx_bytes += skb->len - HCI_HEADER_SIZE; + dev->stats.tx_packets++; + dev->stats.tx_bytes += skb->len - HCI_HEADER_SIZE; kfree_skb(skb); return ret; } @@ -445,13 +445,6 @@ static int gdm_wimax_set_mac_addr(struct net_device *dev, void *p) return 0; } -static struct net_device_stats *gdm_wimax_stats(struct net_device *dev) -{ - struct nic *nic = netdev_priv(dev); - - return &nic->stats; -} - static int gdm_wimax_open(struct net_device *dev) { struct nic *nic = netdev_priv(dev); @@ -707,7 +700,6 @@ static int gdm_wimax_get_prepared_info(struct net_device *dev, char *buf, static void gdm_wimax_netif_rx(struct net_device *dev, char *buf, int len) { - struct nic *nic = netdev_priv(dev); struct sk_buff *skb; int ret; @@ -720,8 +712,8 @@ static void gdm_wimax_netif_rx(struct net_device *dev, char *buf, int len) } skb_reserve(skb, 2); - nic->stats.rx_packets++; - nic->stats.rx_bytes += len; + dev->stats.rx_packets++; + dev->stats.rx_bytes += len; memcpy(skb_put(skb, len), buf, len); @@ -877,7 +869,6 @@ static struct net_device_ops gdm_netdev_ops = { .ndo_stop = gdm_wimax_close, .ndo_set_config = gdm_wimax_set_config, .ndo_start_xmit = gdm_wimax_tx, - .ndo_get_stats = gdm_wimax_stats, .ndo_set_mac_address= gdm_wimax_set_mac_addr, .ndo_do_ioctl = gdm_wimax_ioctl, }; diff --git a/drivers/staging/gdm72xx/gdm_wimax.h b/drivers/staging/gdm72xx/gdm_wimax.h index 7e2c888..798dcc3 100644 --- a/drivers/staging/gdm72xx/gdm_wimax.h +++ b/drivers/staging/gdm72xx/gdm_wimax.h @@ -46,7 +46,6 @@ struct phy_dev { struct nic { struct net_device *netdev; struct phy_dev *phy_dev; - struct net_device_stats stats; struct data_s sdk_data[SIOC_DATA_MAX]; #if defined(CONFIG_WIMAX_GDM72XX_QOS) struct qos_cb_s qos; -- 2.0.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: et131x: Use net_device_stats from struct net_device
Instead of using an own copy of struct net_device_stats in struct et131x_adapter, use stats from struct net_device. Signed-off-by: Tobias Klauser --- drivers/staging/et131x/et131x.c | 18 -- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c index 08356b6..ada0243 100644 --- a/drivers/staging/et131x/et131x.c +++ b/drivers/staging/et131x/et131x.c @@ -532,8 +532,6 @@ struct et131x_adapter { /* Stats */ struct ce_stats stats; - - struct net_device_stats net_stats; }; static int eeprom_wait_ready(struct pci_dev *pdev, u32 *status) @@ -2618,7 +2616,7 @@ static struct rfd *nic_rx_pkts(struct et131x_adapter *adapter) return NULL; } - adapter->net_stats.rx_bytes += rfd->len; + adapter->netdev->stats.rx_bytes += rfd->len; memcpy(skb_put(skb, rfd->len), fbr->virt[buff_index], rfd->len); @@ -2666,7 +2664,7 @@ static void et131x_handle_recv_interrupt(struct et131x_adapter *adapter) continue; /* Increment the number of packets we received */ - adapter->net_stats.rx_packets++; + adapter->netdev->stats.rx_packets++; /* Set the status on the packet, either resources or success */ if (rx_ring->num_ready_recv < RFD_LOW_WATER_MARK) @@ -3037,7 +3035,7 @@ static int et131x_send_packets(struct sk_buff *skb, struct net_device *netdev) dev_kfree_skb_any(skb); skb = NULL; - adapter->net_stats.tx_dropped++; + adapter->netdev->stats.tx_dropped++; } else { status = send_packet(skb, adapter); if (status != 0 && status != -ENOMEM) { @@ -3046,7 +3044,7 @@ static int et131x_send_packets(struct sk_buff *skb, struct net_device *netdev) */ dev_kfree_skb_any(skb); skb = NULL; - adapter->net_stats.tx_dropped++; + adapter->netdev->stats.tx_dropped++; } } } @@ -3065,7 +3063,7 @@ static inline void free_send_packet(struct et131x_adapter *adapter, { unsigned long flags; struct tx_desc *desc = NULL; - struct net_device_stats *stats = &adapter->net_stats; + struct net_device_stats *stats = &adapter->netdev->stats; struct tx_ring *tx_ring = &adapter->tx_ring; u64 dma_addr; @@ -3110,7 +3108,7 @@ static inline void free_send_packet(struct et131x_adapter *adapter, /* Add the TCB to the Ready Q */ spin_lock_irqsave(&adapter->tcb_ready_qlock, flags); - adapter->net_stats.tx_packets++; + stats->tx_packets++; if (tx_ring->tcb_qtail) tx_ring->tcb_qtail->next = tcb; @@ -4134,7 +4132,7 @@ out: static struct net_device_stats *et131x_stats(struct net_device *netdev) { struct et131x_adapter *adapter = netdev_priv(netdev); - struct net_device_stats *stats = &adapter->net_stats; + struct net_device_stats *stats = &adapter->netdev->stats; struct ce_stats *devstat = &adapter->stats; stats->rx_errors = devstat->rx_length_errs + @@ -4426,7 +4424,7 @@ static void et131x_tx_timeout(struct net_device *netdev) tcb->index, tcb->flags); - adapter->net_stats.tx_errors++; + adapter->netdev->stats.tx_errors++; /* perform reset of tx/rx */ et131x_disable_txrx(netdev); -- 2.0.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: gdm72xx: Remove unnecessary memset of netdev private data
The memory for struct net_device private data is allocated using kzalloc/vzalloc in alloc_netdev_mqs, thus there is no need to zero it again in the driver. Signed-off-by: Tobias Klauser --- drivers/staging/gdm72xx/gdm_wimax.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/gdm72xx/gdm_wimax.c b/drivers/staging/gdm72xx/gdm_wimax.c index 6f44798..47e1dd9 100644 --- a/drivers/staging/gdm72xx/gdm_wimax.c +++ b/drivers/staging/gdm72xx/gdm_wimax.c @@ -893,8 +893,6 @@ int register_wimax_device(struct phy_dev *phy_dev, struct device *pdev) memcpy(dev->dev_addr, gdm_wimax_macaddr, sizeof(gdm_wimax_macaddr)); nic = netdev_priv(dev); - memset(nic, 0, sizeof(*nic)); - nic->netdev = dev; nic->phy_dev = phy_dev; phy_dev->netdev = dev; -- 2.0.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] drivers: staging: vt6655: ioctl.c - missing __user annotation
On 2014-07-28 at 09:58:12 +0200, Anil Belur wrote: > From: Anil Belur > > - private_ioctl() the internally calls copy_{to,from}_user() and does > not use '__user' while refrencing user space pointers. > - this patch passes __user annotation as a cast, when the pointer is > being refernced. Wouldn't it be better to annotate the data member in struct tagSCmdRequest with __user instead of introducing all these casts? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: dgnc: remove commented code
On 2014-07-25 at 18:49:01 +0200, Seunghun Lee wrote: > This patch removes commented code in dgnc driver. > > CC: Lidza Louina > CC: Mark Hounschell > Signed-off-by: Seunghun Lee > --- > drivers/staging/dgnc/dgnc_cls.c |5 +- > drivers/staging/dgnc/dgnc_trace.c | 123 > - > drivers/staging/dgnc/dgnc_trace.h | 10 --- > drivers/staging/dgnc/dgnc_tty.c | 18 -- > drivers/staging/dgnc/digi.h |1 - > 5 files changed, 1 insertion(+), 156 deletions(-) > > diff --git a/drivers/staging/dgnc/dgnc_cls.c b/drivers/staging/dgnc/dgnc_cls.c > index 8e265c2..4b65306 100644 > --- a/drivers/staging/dgnc/dgnc_cls.c > +++ b/drivers/staging/dgnc/dgnc_cls.c > @@ -1046,10 +1046,7 @@ static void cls_flush_uart_read(struct channel_t *ch) >* I believe this is a BUG in this UART. >* So for now, we will leave the code #ifdef'ed out... >*/ The comment above relates to the deleted code, so it should be updated accordingly. > -#if 0 > - writeb((UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR), > - &ch->ch_cls_uart->isr_fcr); > -#endif > + > udelay(10); > } ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: dgnc: removes comment related to the delete code.
On 2014-07-29 at 15:02:42 +0200, Seunghun Lee wrote: > This patch removes comment related to the delete code. > > CC: Lidza Louina > CC: Mark Hounschell > Signed-off-by: Seunghun Lee > --- > drivers/staging/dgnc/dgnc_cls.c |3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/staging/dgnc/dgnc_cls.c b/drivers/staging/dgnc/dgnc_cls.c > index 4b65306..06fc68b 100644 > --- a/drivers/staging/dgnc/dgnc_cls.c > +++ b/drivers/staging/dgnc/dgnc_cls.c > @@ -1042,9 +1042,6 @@ static void cls_flush_uart_read(struct channel_t *ch) >* >* However, doing the statement below also incorrectly flushes >* write data as well as just basically trashing the FIFO. This bit also references the commented out/deleted code. IMO it would make sense to rephrase this entire comment block such that it explains what the original intention of this entire function was and the problem associated with it. How about something like: /* * For complete POSIX compatibility, we should be purging the read FIFO * in the UART here. * * However, clearing the read FIFO (UART_FCR_CLEAR_RCVR) also * incorrectly flushes write data as well as just basically trashing the * FIFO. * * Presumably, this is a bug in this UART. */ > - * > - * I believe this is a BUG in this UART. > - * So for now, we will leave the code #ifdef'ed out... >*/ > > udelay(10); > -- > 1.7.9.5 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/1] staging: vt6655: ioctl.c - missing __user annotation
On 2014-07-30 at 12:41:59 +0200, Anil Belur wrote: > From: Anil Belur This line is only necessary if you're sending the patch on behalf of someone else. Also there is no need to have the 1/1 in the Subject line as you're only sending one patch, not an entire series. > v2: > - private_ioctl() internally calls copy_{to,from}_user() and does > not use '__user' which gives out several sparse warnings > - this patch adds __user annotation to the data member of struct > tagSCmdRequest as suggested by tklau...@distanz.ch Patch history should go below the '---' as it doesn't need to be included in the commit message. > - sparse warnings fixed: > drivers/staging/vt6655/ioctl.c:78:51: warning: incorrect type in argument 2 > (different address spaces) > drivers/staging/vt6655/ioctl.c:78:51:expected void const [noderef] > asn:1>*from > drivers/staging/vt6655/ioctl.c:78:51:got void *data It's sufficient to only show one instance of the sparse warning in the commit message, since they're all the same. > drivers/staging/vt6655/ioctl.c:117:55: warning: incorrect type in argument 2 > (different address spaces) > drivers/staging/vt6655/ioctl.c:117:55:expected void const [noderef] > asn:1>*from > drivers/staging/vt6655/ioctl.c:117:55:got void *data > drivers/staging/vt6655/ioctl.c:149:46: warning: incorrect type in argument 1 > (different address spaces) > drivers/staging/vt6655/ioctl.c:149:46:expected void [noderef] asn:1>*to > drivers/staging/vt6655/ioctl.c:149:46:got void *data > drivers/staging/vt6655/ioctl.c:166:51: warning: incorrect type in argument 2 > (different address spaces) > drivers/staging/vt6655/ioctl.c:166:51:expected void const [noderef] > asn:1>*from > drivers/staging/vt6655/ioctl.c:166:51:got void *data > drivers/staging/vt6655/ioctl.c:212:50: warning: incorrect type in argument 2 > (different address spaces) > drivers/staging/vt6655/ioctl.c:212:50:expected void const [noderef] > asn:1>*from > drivers/staging/vt6655/ioctl.c:212:50:got void *data > drivers/staging/vt6655/ioctl.c:276:38: warning: incorrect type in argument 1 > (different address spaces) > drivers/staging/vt6655/ioctl.c:276:38:expected void [noderef] asn:1>*to > drivers/staging/vt6655/ioctl.c:276:38:got void *data > drivers/staging/vt6655/ioctl.c:292:38: warning: incorrect type in argument 1 > (different address spaces) > drivers/staging/vt6655/ioctl.c:292:38:expected void [noderef] asn:1>*to > drivers/staging/vt6655/ioctl.c:292:38:got void *data > drivers/staging/vt6655/ioctl.c:300:48: warning: incorrect type in argument 2 > (different address spaces) > drivers/staging/vt6655/ioctl.c:300:48:expected void const [noderef] > asn:1>*from > drivers/staging/vt6655/ioctl.c:300:48:got void *data > drivers/staging/vt6655/ioctl.c:344:38: warning: incorrect type in argument 1 > (different address spaces) > drivers/staging/vt6655/ioctl.c:344:38:expected void [noderef] asn:1>*to > drivers/staging/vt6655/ioctl.c:344:38:got void *data > drivers/staging/vt6655/ioctl.c:353:38: warning: incorrect type in argument 1 > (different address spaces) > drivers/staging/vt6655/ioctl.c:353:38:expected void [noderef] asn:1>*to > drivers/staging/vt6655/ioctl.c:353:38:got void *data > drivers/staging/vt6655/ioctl.c:360:38: warning: incorrect type in argument 1 > (different address spaces) > drivers/staging/vt6655/ioctl.c:360:38:expected void [noderef] asn:1>*to > drivers/staging/vt6655/ioctl.c:360:38:got void *data > drivers/staging/vt6655/ioctl.c:401:49: warning: incorrect type in argument 2 > (different address spaces) > drivers/staging/vt6655/ioctl.c:401:49:expected void const [noderef] > asn:1>*from > drivers/staging/vt6655/ioctl.c:401:49:got void *data > drivers/staging/vt6655/ioctl.c:424:49: warning: incorrect type in argument 2 > (different address spaces) > drivers/staging/vt6655/ioctl.c:424:49:expected void const [noderef] > asn:1>*from > drivers/staging/vt6655/ioctl.c:424:49:got void *data > drivers/staging/vt6655/ioctl.c:440:49: warning: incorrect type in argument 2 > (different address spaces) > drivers/staging/vt6655/ioctl.c:440:49:expected void const [noderef] > asn:1>*from > drivers/staging/vt6655/ioctl.c:440:49:got void *data > drivers/staging/vt6655/ioctl.c:457:49: warning: incorrect type in argument 2 > (different address spaces) > drivers/staging/vt6655/ioctl.c:457:49:expected void const [noderef] > asn:1>*from > drivers/staging/vt6655/ioctl.c:457:49:got void *data > drivers/staging/vt6655/ioctl.c:479:54: warning: incorrect type in argument 2 > (different address spaces) > drivers/staging/vt6655/ioctl.c:479:54:expected void const [noderef] > asn:1>*from > drivers/staging/vt6655/ioctl.c:479:54:got void *data > drivers/staging/vt6655/ioctl.c:563:38: warning: incorrect type in argument 1 > (different address spaces) > drivers/staging/vt6655/i
Re: [PATCH] staging: vt6655: Fix device table definition.
On 2014-07-29 at 19:44:24 +0200, fernando.apesteg...@gmail.com wrote: > From: Fernando Apesteguia > > Add static to the definition of the pci device table. > > Signed-off-by: Fernando Apesteguia This change is already part of Greg's tree, see commit 9e4c5c2837a4 ("staging: vt6655: statify some variables") in branch staging-next. Please make sure you work against that branch (which is also a part of linux-next) for any patches you submit. Cheers Tobias ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RESEND] staging: wlan-ng: prism2mgmt.c Fix break not useful
On 2014-08-11 at 18:15:58 +0200, Jeshwanth Kumar N K wrote: > Fixes up warning, break is not useful after a goto or return statement > > Signed-off-by: Jeshwanth Kumar N K > --- > drivers/staging/wlan-ng/prism2mgmt.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/staging/wlan-ng/prism2mgmt.c > b/drivers/staging/wlan-ng/prism2mgmt.c > index e6a82d3..5837b0e 100644 > --- a/drivers/staging/wlan-ng/prism2mgmt.c > +++ b/drivers/staging/wlan-ng/prism2mgmt.c > @@ -1168,7 +1168,6 @@ int prism2mgmt_wlansniff(wlandevice_t *wlandev, void > *msgp) > msg->resultcode.data = P80211ENUM_resultcode_success; > result = 0; > goto exit; > - break; Better yet, just return 0 directly here instead of the goto, using a goto just to return makes no sense. > case P80211ENUM_truth_true: > /* Disable the port (if enabled), only check Port 0 */ > if (hw->port_enabled[0]) { > @@ -1315,12 +1314,10 @@ int prism2mgmt_wlansniff(wlandevice_t *wlandev, void > *msgp) > msg->resultcode.data = P80211ENUM_resultcode_success; > result = 0; > goto exit; > - break; Same here. > default: > msg->resultcode.data = P80211ENUM_resultcode_invalid_parameters; > result = 0; > goto exit; > - break; Here as well. With the above three users of the 'exit' label, you could remove it as well. Cheers Tobias ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: wlan-ng: prism2mgmt.c Fix break not useful
On 2014-08-12 at 20:54:48 +0200, Jeshwanth Kumar N K wrote: > removed goto label, and directly returning 0 not through goto. > > Signed-off-by: Jeshwanth Kumar N K > --- > drivers/staging/wlan-ng/prism2mgmt.c | 14 -- > 1 file changed, 4 insertions(+), 10 deletions(-) > > diff --git a/drivers/staging/wlan-ng/prism2mgmt.c > b/drivers/staging/wlan-ng/prism2mgmt.c > index 5837b0e..9218399 100644 > --- a/drivers/staging/wlan-ng/prism2mgmt.c > +++ b/drivers/staging/wlan-ng/prism2mgmt.c > @@ -1107,8 +1107,7 @@ int prism2mgmt_wlansniff(wlandevice_t *wlandev, void > *msgp) > if (wlandev->netdev->type == ARPHRD_ETHER) { > msg->resultcode.data = > P80211ENUM_resultcode_invalid_parameters; > - result = 0; > - goto exit; > + return 0; > } > /* Disable monitor mode */ > result = hfa384x_cmd_monitor(hw, HFA384x_MONITOR_DISABLE); > @@ -1166,8 +1165,7 @@ int prism2mgmt_wlansniff(wlandevice_t *wlandev, void > *msgp) > > netdev_info(wlandev->netdev, "monitor mode disabled\n"); > msg->resultcode.data = P80211ENUM_resultcode_success; > - result = 0; > - goto exit; > + return 0; > case P80211ENUM_truth_true: > /* Disable the port (if enabled), only check Port 0 */ > if (hw->port_enabled[0]) { > @@ -1312,17 +1310,13 @@ int prism2mgmt_wlansniff(wlandevice_t *wlandev, void > *msgp) > } > > msg->resultcode.data = P80211ENUM_resultcode_success; > - result = 0; > - goto exit; > + return 0; > default: > msg->resultcode.data = P80211ENUM_resultcode_invalid_parameters; > - result = 0; > - goto exit; > + return 0; > } > > failed: > msg->resultcode.data = P80211ENUM_resultcode_refused; > result = 0; > -exit: > - return result; You will still need the return here for the cases where the 'failed' label is the jump target. Also, this change leads to a compiler warning: drivers/staging/wlan-ng/prism2usb.c: In function ‘prism2mgmt_wlansniff’: drivers/staging/wlan-ng/prism2mgmt.c:1322:1: warning: control reaches end of non-void function [-Wreturn-type] In any case, resetting the return value to 0 makes the function always return successfully and seems to signal the error condition by setting msg->resultcode.data. This looks a bit odd to me and also doesn't correspond to the documentation of the function's return value. It might be worthwile to take a closer look there as well. Thanks Tobias ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: wlan-ng: prism2mgmt.c Fix break not useful
On 2014-08-13 at 21:11:19 +0200, Jeshwanth Kumar N K wrote: > On Wed, Aug 13, 2014 at 12:06 PM, Tobias Klauser > wrote: > > > On 2014-08-12 at 20:54:48 +0200, Jeshwanth Kumar N K < > > jeshkumar...@gmail.com> wrote: > > > removed goto label, and directly returning 0 not through goto. > > > > > > Signed-off-by: Jeshwanth Kumar N K > > > --- > > > drivers/staging/wlan-ng/prism2mgmt.c | 14 -- > > > 1 file changed, 4 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/staging/wlan-ng/prism2mgmt.c > > b/drivers/staging/wlan-ng/prism2mgmt.c > > > index 5837b0e..9218399 100644 > > > --- a/drivers/staging/wlan-ng/prism2mgmt.c > > > +++ b/drivers/staging/wlan-ng/prism2mgmt.c > > > @@ -1107,8 +1107,7 @@ int prism2mgmt_wlansniff(wlandevice_t *wlandev, > > void *msgp) > > > if (wlandev->netdev->type == ARPHRD_ETHER) { > > > msg->resultcode.data = > > > P80211ENUM_resultcode_invalid_parameters; > > > - result = 0; > > > - goto exit; > > > + return 0; > > > } > > > /* Disable monitor mode */ > > > result = hfa384x_cmd_monitor(hw, HFA384x_MONITOR_DISABLE); > > > @@ -1166,8 +1165,7 @@ int prism2mgmt_wlansniff(wlandevice_t *wlandev, > > void *msgp) > > > > > > netdev_info(wlandev->netdev, "monitor mode disabled\n"); > > > msg->resultcode.data = P80211ENUM_resultcode_success; > > > - result = 0; > > > - goto exit; > > > + return 0; > > > case P80211ENUM_truth_true: > > > /* Disable the port (if enabled), only check Port 0 */ > > > if (hw->port_enabled[0]) { > > > @@ -1312,17 +1310,13 @@ int prism2mgmt_wlansniff(wlandevice_t *wlandev, > > void *msgp) > > > } > > > > > > msg->resultcode.data = P80211ENUM_resultcode_success; > > > - result = 0; > > > - goto exit; > > > + return 0; > > > default: > > > msg->resultcode.data = > > P80211ENUM_resultcode_invalid_parameters; > > > - result = 0; > > > - goto exit; > > > + return 0; > > > } > > > > > > failed: > > > msg->resultcode.data = P80211ENUM_resultcode_refused; > > > result = 0; > > > -exit: > > > - return result; > > > > You will still need the return here for the cases where the 'failed' > > label is the jump target. Also, this change leads to a compiler warning: > > > > drivers/staging/wlan-ng/prism2usb.c: In function ‘prism2mgmt_wlansniff’: > > drivers/staging/wlan-ng/prism2mgmt.c:1322:1: warning: control reaches end > > of non-void function [-Wreturn-type] > > > > In any case, resetting the return value to 0 makes the function always > > return > > successfully and seems to signal the error condition by setting > > msg->resultcode.data. This looks a bit odd to me and also doesn't > > correspond to > > the documentation of the function's return value. It might be worthwile to > > take > > a closer look there as well. > > > > Thanks > > Tobias > > Hi Jeshwanth > Thanks, I have changed it in local now (not yet submitted). But yes as you > said it's not returning 0 every time. > > In comment it's mentioned that "<0 success, but we're waiting for > something to finish.". This means this function implementation is not yet > finished? I assume so, but I'm not really familiar with the workings of this driver. > If there is no 2nd value of return, this can be made as void function? or > need to understand the logic behind? I would leave it as is, unless you clearly understand the logic behind it. I'd suggest for your current patch to just change the return/goto/break part and then take care about the return value in a separate patch once you figured out how the function should really behave. You'll most probably also need the hardware in question then to test your changes on. > And I didn't find it is checking P80211ENUM_resultcode_refused anywhere in > the code. Hm yes, strage. So it seems the error conditions in this functions are not handled at all except for the pr_debug messages. Thanks Tobias ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel