On Tue, 2014-07-01 at 18:36 +0200, Stefan Wahren wrote: > This patch adds the Ethernet over SPI driver for the > Qualcomm QCA7000 HomePlug GreenPHY. Beyond trivial comments about multi-line alignment and such, it seems that qcaspi_receive might never return. The dual while {} loops may never decrement available if read_legacy or read_burst have errors. count should be a local variable to the first while not at function scope btw: here are just those trivial bits: --- drivers/net/ethernet/qualcomm/qca_7k.c | 1 - drivers/net/ethernet/qualcomm/qca_debug.c | 26 ++++----- drivers/net/ethernet/qualcomm/qca_framing.c | 1 - drivers/net/ethernet/qualcomm/qca_spi.c | 88 +++++++++++++++-------------- 4 files changed, 58 insertions(+), 58 deletions(-) diff --git a/drivers/net/ethernet/qualcomm/qca_7k.c b/drivers/net/ethernet/qualcomm/qca_7k.c index f7fc52b..2ddfc16 100644 --- a/drivers/net/ethernet/qualcomm/qca_7k.c +++ b/drivers/net/ethernet/qualcomm/qca_7k.c @@ -146,4 +146,3 @@ qcaspi_tx_cmd(struct qcaspi *qca, u16 cmd) return ret; } - diff --git a/drivers/net/ethernet/qualcomm/qca_debug.c b/drivers/net/ethernet/qualcomm/qca_debug.c index e88448c..8dd3936 100644 --- a/drivers/net/ethernet/qualcomm/qca_debug.c +++ b/drivers/net/ethernet/qualcomm/qca_debug.c @@ -70,7 +70,7 @@ qcaspi_info_show(struct seq_file *s, void *what) struct qcaspi *qca = s->private; seq_printf(s, "RX buffer size : %lu\n", - (unsigned long) qca->buffer_size); + (unsigned long)qca->buffer_size); seq_puts(s, "TX ring state : "); @@ -84,10 +84,10 @@ qcaspi_info_show(struct seq_file *s, void *what) seq_puts(s, "\n"); seq_printf(s, "TX ring size : %u\n", - qca->txr.size); + qca->txr.size); seq_printf(s, "Sync state : %u (", - (unsigned int) qca->sync); + (unsigned int)qca->sync); switch (qca->sync) { case QCASPI_SYNC_UNKNOWN: seq_puts(s, "QCASPI_SYNC_UNKNOWN"); @@ -105,22 +105,22 @@ qcaspi_info_show(struct seq_file *s, void *what) seq_puts(s, ")\n"); seq_printf(s, "IRQ : %d\n", - qca->spi_dev->irq); + qca->spi_dev->irq); seq_printf(s, "INTR REQ : %u\n", - qca->intr_req); + qca->intr_req); seq_printf(s, "INTR SVC : %u\n", - qca->intr_svc); + qca->intr_svc); seq_printf(s, "SPI max speed : %lu\n", - (unsigned long) qca->spi_dev->max_speed_hz); + (unsigned long)qca->spi_dev->max_speed_hz); seq_printf(s, "SPI mode : %x\n", - qca->spi_dev->mode); + qca->spi_dev->mode); seq_printf(s, "SPI chip select : %u\n", - (unsigned int) qca->spi_dev->chip_select); + (unsigned int)qca->spi_dev->chip_select); seq_printf(s, "SPI legacy mode : %u\n", - (unsigned int) qca->legacy_mode); + (unsigned int)qca->legacy_mode); seq_printf(s, "SPI burst length : %u\n", - (unsigned int) qca->burst_len); + (unsigned int)qca->burst_len); return 0; } @@ -152,7 +152,7 @@ qcaspi_init_device_debugfs(struct qcaspi *qca) return; } debugfs_create_file("info", S_IFREG | S_IRUGO, device_root, qca, - &qcaspi_info_ops); + &qcaspi_info_ops); } void @@ -215,7 +215,7 @@ qcaspi_get_strings(struct net_device *dev, u32 stringset, u8 *buf) switch (stringset) { case ETH_SS_STATS: memcpy(buf, &qcaspi_gstrings_stats, - sizeof(qcaspi_gstrings_stats)); + sizeof(qcaspi_gstrings_stats)); break; default: WARN_ON(1); diff --git a/drivers/net/ethernet/qualcomm/qca_framing.c b/drivers/net/ethernet/qualcomm/qca_framing.c index 3c8d8d4..9bf2f19 100644 --- a/drivers/net/ethernet/qualcomm/qca_framing.c +++ b/drivers/net/ethernet/qualcomm/qca_framing.c @@ -152,4 +152,3 @@ qcafrm_fsm_decode(struct qcafrm_handle *handle, u8 *buf, u16 buf_len, u8 recv_by return ret; } - diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c b/drivers/net/ethernet/qualcomm/qca_spi.c index 30d8e23..42ff580 100644 --- a/drivers/net/ethernet/qualcomm/qca_spi.c +++ b/drivers/net/ethernet/qualcomm/qca_spi.c @@ -88,9 +88,9 @@ void end_spi_intr_handling(struct qcaspi *qca, u16 intr_cause) { u16 intr_enable = (SPI_INT_CPU_ON | - SPI_INT_PKT_AVLBL | - SPI_INT_RDBUF_ERR | - SPI_INT_WRBUF_ERR); + SPI_INT_PKT_AVLBL | + SPI_INT_RDBUF_ERR | + SPI_INT_WRBUF_ERR); qcaspi_write_register(qca, SPI_REG_INTR_CAUSE, intr_cause); qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, intr_enable); @@ -215,10 +215,10 @@ qcaspi_tx_frame(struct qcaspi *qca, struct sk_buff *skb) if (qca->legacy_mode) { bytes_written = qcaspi_write_legacy(qca, - skb->data + offset, count); + skb->data + offset, count); } else { bytes_written = qcaspi_write_burst(qca, - skb->data + offset, count); + skb->data + offset, count); } if (bytes_written != count) @@ -296,7 +296,7 @@ qcaspi_receive(struct qcaspi *qca) /* Allocate rx SKB if we don't have one available. */ if (!qca->rx_skb) { qca->rx_skb = netdev_alloc_skb(qca->net_dev, - qca->net_dev->mtu + VLAN_ETH_HLEN); + qca->net_dev->mtu + VLAN_ETH_HLEN); if (!qca->rx_skb) { netdev_dbg(qca->net_dev, "out of RX resources\n"); qca->stats.out_of_mem++; @@ -307,7 +307,7 @@ qcaspi_receive(struct qcaspi *qca) /* Read the packet size. */ qcaspi_read_register(qca, SPI_REG_RDBUF_BYTE_AVA, &available); netdev_dbg(qca->net_dev, "qcaspi_receive: SPI_REG_RDBUF_BYTE_AVA: Value: %08x\n", - available); + available); if (available == 0) { netdev_dbg(qca->net_dev, "qcaspi_receive called without any data being available!\n"); @@ -326,16 +326,16 @@ qcaspi_receive(struct qcaspi *qca) if (qca->legacy_mode) { bytes_read = qcaspi_read_legacy(qca, qca->rx_buffer, - count); + count); } else { bytes_read = qcaspi_read_burst(qca, qca->rx_buffer, - count); + count); } cp = qca->rx_buffer; netdev_dbg(qca->net_dev, "available: %d, byte read: %d\n", - available, bytes_read); + available, bytes_read); if (bytes_read) available -= bytes_read; @@ -346,9 +346,9 @@ qcaspi_receive(struct qcaspi *qca) s32 retcode; retcode = qcafrm_fsm_decode(&qca->frm_handle, - qca->rx_skb->data, - skb_tailroom(qca->rx_skb), - *cp); + qca->rx_skb->data, + skb_tailroom(qca->rx_skb), + *cp); cp++; switch (retcode) { case QCAFRM_GATHER: @@ -374,7 +374,7 @@ qcaspi_receive(struct qcaspi *qca) qca->rx_skb->ip_summed = CHECKSUM_UNNECESSARY; netif_rx_ni(qca->rx_skb); qca->rx_skb = netdev_alloc_skb(qca->net_dev, - qca->net_dev->mtu + VLAN_ETH_HLEN); + qca->net_dev->mtu + VLAN_ETH_HLEN); if (!qca->rx_skb) { netdev_dbg(qca->net_dev, "out of RX resources\n"); n_stats->rx_errors++; @@ -443,7 +443,7 @@ qcaspi_qca7k_sync(struct qcaspi *qca, int event) } else { /* ensure that the WRBUF is empty */ qcaspi_read_register(qca, SPI_REG_WRBUF_SPC_AVA, - &wrbuf_space); + &wrbuf_space); if (wrbuf_space != QCASPI_HW_BUF_LEN) { netdev_dbg(qca->net_dev, "sync: got CPU on, but wrbuf not empty. reset!\n"); qca->sync = QCASPI_SYNC_UNKNOWN; @@ -487,7 +487,7 @@ qcaspi_qca7k_sync(struct qcaspi *qca, int event) case QCASPI_SYNC_RESET: reset_count++; netdev_dbg(qca->net_dev, "sync: waiting for CPU on, count %u.\n", - reset_count); + reset_count); if (reset_count >= QCASPI_RESET_TIMEOUT) { /* reset did not seem to take place, try again */ qca->sync = QCASPI_SYNC_UNKNOWN; @@ -501,7 +501,7 @@ qcaspi_qca7k_sync(struct qcaspi *qca, int event) static int qcaspi_spi_thread(void *data) { - struct qcaspi *qca = (struct qcaspi *) data; + struct qcaspi *qca = data; u16 intr_cause = 0; netdev_info(qca->net_dev, "SPI thread created\n"); @@ -515,14 +515,14 @@ qcaspi_spi_thread(void *data) set_current_state(TASK_RUNNING); netdev_dbg(qca->net_dev, "have work to do. int: %d, tx_skb: %p\n", - qca->intr_req - qca->intr_svc, - qca->txr.skb[qca->txr.head]); + qca->intr_req - qca->intr_svc, + qca->txr.skb[qca->txr.head]); qcaspi_qca7k_sync(qca, QCASPI_EVENT_UPDATE); if (qca->sync != QCASPI_SYNC_READY) { netdev_dbg(qca->net_dev, "sync: not ready %u, turn off carrier and flush\n", - (unsigned int) qca->sync); + (unsigned int)qca->sync); netif_stop_queue(qca->net_dev); netif_carrier_off(qca->net_dev); qcaspi_flush_tx_ring(qca); @@ -584,10 +584,11 @@ qcaspi_spi_thread(void *data) static irqreturn_t qcaspi_intr_handler(int irq, void *data) { - struct qcaspi *qca = (struct qcaspi *) data; + struct qcaspi *qca = data; + qca->intr_req++; if (qca->spi_thread && - qca->spi_thread->state != TASK_RUNNING) + qca->spi_thread->state != TASK_RUNNING) wake_up_process(qca->spi_thread); return IRQ_HANDLED; @@ -608,19 +609,19 @@ qcaspi_netdev_open(struct net_device *dev) qcafrm_fsm_init(&qca->frm_handle); qca->spi_thread = kthread_run((void *)qcaspi_spi_thread, - qca, "%s", dev->name); + qca, "%s", dev->name); if (IS_ERR(qca->spi_thread)) { netdev_err(dev, "%s: unable to start kernel thread.\n", - QCASPI_DRV_NAME); + QCASPI_DRV_NAME); return PTR_ERR(qca->spi_thread); } ret = request_irq(qca->spi_dev->irq, qcaspi_intr_handler, 0, - dev->name, qca); + dev->name, qca); if (ret) { netdev_err(dev, "%s: unable to get IRQ %d (irqval=%d).\n", - QCASPI_DRV_NAME, qca->spi_dev->irq, ret); + QCASPI_DRV_NAME, qca->spi_dev->irq, ret); kthread_stop(qca->spi_thread); return ret; } @@ -670,7 +671,7 @@ qcaspi_netdev_xmit(struct sk_buff *skb, struct net_device *dev) if ((skb_headroom(skb) < QCAFRM_HEADER_LEN) || (skb_tailroom(skb) < QCAFRM_FOOTER_LEN + pad_len)) { tskb = skb_copy_expand(skb, QCAFRM_HEADER_LEN, - QCAFRM_FOOTER_LEN + pad_len, GFP_ATOMIC); + QCAFRM_FOOTER_LEN + pad_len, GFP_ATOMIC); if (!tskb) { netdev_dbg(qca->net_dev, "could not allocate tx_buff\n"); qca->stats.out_of_mem++; @@ -694,7 +695,7 @@ qcaspi_netdev_xmit(struct sk_buff *skb, struct net_device *dev) qcafrm_create_footer(ptmp); netdev_dbg(qca->net_dev, "Tx-ing packet: Size: 0x%08x\n", - skb->len); + skb->len); qca->txr.size += skb->len + QCASPI_HW_PKT_LEN; @@ -713,7 +714,7 @@ qcaspi_netdev_xmit(struct sk_buff *skb, struct net_device *dev) dev->trans_start = jiffies; if (qca->spi_thread && - qca->spi_thread->state != TASK_RUNNING) + qca->spi_thread->state != TASK_RUNNING) wake_up_process(qca->spi_thread); return NETDEV_TX_OK; @@ -723,8 +724,9 @@ void qcaspi_netdev_tx_timeout(struct net_device *dev) { struct qcaspi *qca = netdev_priv(dev); + netdev_info(qca->net_dev, "Transmit timeout at %ld, latency %ld\n", - jiffies, jiffies - dev->trans_start); + jiffies, jiffies - dev->trans_start); qca->net_dev->stats.tx_errors++; /* wake the queue if there is room */ if (qcaspi_tx_ring_has_space(&qca->txr)) @@ -742,7 +744,7 @@ qcaspi_netdev_init(struct net_device *dev) qca->burst_len = qcaspi_burst_len; qca->spi_thread = NULL; qca->buffer_size = (dev->mtu + VLAN_ETH_HLEN + QCAFRM_HEADER_LEN + - QCAFRM_FOOTER_LEN + 4) * 4; + QCAFRM_FOOTER_LEN + 4) * 4; memset(&qca->stats, 0, sizeof(struct qcaspi_stats)); @@ -764,6 +766,7 @@ static void qcaspi_netdev_uninit(struct net_device *dev) { struct qcaspi *qca = netdev_priv(dev); + kfree(qca->rx_buffer); qca->buffer_size = 0; if (qca->rx_skb) @@ -859,13 +862,13 @@ qca_spi_probe(struct spi_device *spi_device) } if (of_find_property(spi_device->dev.of_node, - "qca,legacy-mode", NULL)) { + "qca,legacy-mode", NULL)) { legacy_mode = 1; } if (qcaspi_clkspeed == 0) { if (of_find_property(spi_device->dev.of_node, - "spi-max-frequency", NULL)) { + "spi-max-frequency", NULL)) { qcaspi_clkspeed = spi_device->max_speed_hz; } else { qcaspi_clkspeed = QCASPI_CLK_SPEED; @@ -875,29 +878,29 @@ qca_spi_probe(struct spi_device *spi_device) if ((qcaspi_clkspeed < QCASPI_CLK_SPEED_MIN) || (qcaspi_clkspeed > QCASPI_CLK_SPEED_MAX)) { dev_info(&spi_device->dev, "Invalid clkspeed: %d\n", - qcaspi_clkspeed); + qcaspi_clkspeed); return -EINVAL; } if ((qcaspi_burst_len < QCASPI_BURST_LEN_MIN) || (qcaspi_burst_len > QCASPI_BURST_LEN_MAX)) { dev_info(&spi_device->dev, "Invalid burst len: %d\n", - qcaspi_burst_len); + qcaspi_burst_len); return -EINVAL; } if ((qcaspi_pluggable < QCASPI_PLUGGABLE_MIN) || (qcaspi_pluggable > QCASPI_PLUGGABLE_MAX)) { dev_info(&spi_device->dev, "Invalid pluggable: %d\n", - qcaspi_pluggable); + qcaspi_pluggable); return -EINVAL; } dev_info(&spi_device->dev, "ver=%s, clkspeed=%d, burst_len=%d, pluggable=%d\n", - QCASPI_DRV_VERSION, - qcaspi_clkspeed, - qcaspi_burst_len, - qcaspi_pluggable); + QCASPI_DRV_VERSION, + qcaspi_clkspeed, + qcaspi_burst_len, + qcaspi_pluggable); spi_device->mode = SPI_MODE_3; spi_device->max_speed_hz = qcaspi_clkspeed; @@ -930,7 +933,7 @@ qca_spi_probe(struct spi_device *spi_device) if (!is_valid_ether_addr(qca->net_dev->dev_addr)) { eth_hw_addr_random(qca->net_dev); dev_info(&spi_device->dev, "Using random MAC address: %pM\n", - qca->net_dev->dev_addr); + qca->net_dev->dev_addr); } netif_carrier_off(qca->net_dev); @@ -949,7 +952,7 @@ qca_spi_probe(struct spi_device *spi_device) if (register_netdev(qcaspi_devs)) { dev_info(&spi_device->dev, "Unable to register net device %s\n", - qcaspi_devs->name); + qcaspi_devs->name); free_netdev(qcaspi_devs); return -EFAULT; } @@ -998,4 +1001,3 @@ MODULE_AUTHOR("Qualcomm Atheros Communications"); MODULE_AUTHOR("Stefan Wahren <stefan.wahren@xxxxxxxx>"); MODULE_LICENSE("Dual BSD/GPL"); MODULE_VERSION(QCASPI_DRV_VERSION); - -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html