This series contains a wild collection of improvements for the
qca_spi driver. This is a follow-up series to the recent bugfixes [1].
Patch 1 & 2 in this series is the initially intended rework of
netdev_open/close. Patch 3 & 9 are minor functional improvements and
the rest is clean-up.
Changes in V3:
- rebase on net-next-20240124
- improve commit message in Patch 8
- add Reviewed-by's from Jacob Keller
- add Patch 13 for mailmap as suggested by Jacob
- add Patch 14 for MAINTAINERS entry
Changes in V2:
- rebase on net-next-20231218
- improve cover letter
Stefan Wahren (14):
qca_spi: Improve SPI thread creation
qca_spi: Improve SPI IRQ handling
qca_spi: Avoid skb_copy_expand in TX path
qca_7k_common: Drop unnecessary function description
qca_7k_common: Drop unused len from qcafrm_handle
qca_spi: Add QCASPI prefix to ring defines
qca_spi: Introduce QCASPI_RX_MAX_FRAMES
qca_spi: Improve calculation of RX buffer size
qca_spi: Log expected signature in error case
qca_spi: Adjust log of SPI_REG_RDBUF_BYTE_AVA
qca_7k: Replace BSD boilerplate with SPDX
qca_7k: Replace old mail address
mailmap: add entry for Stefan Wahren
MAINTAINERS: add entry for qca7k driver(s)
.mailmap | 1 +
MAINTAINERS | 7 ++
drivers/net/ethernet/qualcomm/qca_7k.c | 17 +---
drivers/net/ethernet/qualcomm/qca_7k.h | 16 +---
drivers/net/ethernet/qualcomm/qca_7k_common.c | 17 +---
drivers/net/ethernet/qualcomm/qca_7k_common.h | 29 +------
drivers/net/ethernet/qualcomm/qca_debug.c | 21 +----
drivers/net/ethernet/qualcomm/qca_debug.h | 15 +---
drivers/net/ethernet/qualcomm/qca_spi.c | 86 +++++++++----------
drivers/net/ethernet/qualcomm/qca_spi.h | 22 ++---
drivers/net/ethernet/qualcomm/qca_uart.c | 17 +---
11 files changed, 65 insertions(+), 183 deletions(-)
--
2.34.1
All known SPI registers of the QCA700x are 16 bit long. So adjust
the formater width accordingly.
Signed-off-by: Stefan Wahren <[email protected]>
Reviewed-by: Jacob Keller <[email protected]>
---
drivers/net/ethernet/qualcomm/qca_spi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c b/drivers/net/ethernet/qualcomm/qca_spi.c
index c5d240fe4146..d3109eb24b4d 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -359,7 +359,7 @@ qcaspi_receive(struct qcaspi *qca)
/* Read the packet size. */
qcaspi_read_register(qca, SPI_REG_RDBUF_BYTE_AVA, &available);
- netdev_dbg(net_dev, "qcaspi_receive: SPI_REG_RDBUF_BYTE_AVA: Value: %08x\n",
+ netdev_dbg(net_dev, "qcaspi_receive: SPI_REG_RDBUF_BYTE_AVA: Value: %04x\n",
available);
if (available > QCASPI_HW_BUF_LEN + QCASPI_HW_PKT_LEN) {
--
2.34.1
From: Stefan Wahren <[email protected]>
The company I2SE has been acquired a long time ago. Switch to
my private mail address before the I2SE account is deactivated.
Signed-off-by: Stefan Wahren <[email protected]>
Signed-off-by: Stefan Wahren <[email protected]>
Reviewed-by: Jacob Keller <[email protected]>
---
drivers/net/ethernet/qualcomm/qca_7k_common.c | 2 +-
drivers/net/ethernet/qualcomm/qca_spi.c | 2 +-
drivers/net/ethernet/qualcomm/qca_uart.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/qca_7k_common.c b/drivers/net/ethernet/qualcomm/qca_7k_common.c
index be2f754efd21..5302da587620 100644
--- a/drivers/net/ethernet/qualcomm/qca_7k_common.c
+++ b/drivers/net/ethernet/qualcomm/qca_7k_common.c
@@ -149,5 +149,5 @@ EXPORT_SYMBOL_GPL(qcafrm_fsm_decode);
MODULE_DESCRIPTION("Qualcomm Atheros QCA7000 common");
MODULE_AUTHOR("Qualcomm Atheros Communications");
-MODULE_AUTHOR("Stefan Wahren <[email protected]>");
+MODULE_AUTHOR("Stefan Wahren <[email protected]>");
MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c b/drivers/net/ethernet/qualcomm/qca_spi.c
index 7c63baeb3844..29be0fe0b235 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -1042,6 +1042,6 @@ module_spi_driver(qca_spi_driver);
MODULE_DESCRIPTION("Qualcomm Atheros QCA7000 SPI Driver");
MODULE_AUTHOR("Qualcomm Atheros Communications");
-MODULE_AUTHOR("Stefan Wahren <[email protected]>");
+MODULE_AUTHOR("Stefan Wahren <[email protected]>");
MODULE_LICENSE("Dual BSD/GPL");
MODULE_VERSION(QCASPI_DRV_VERSION);
diff --git a/drivers/net/ethernet/qualcomm/qca_uart.c b/drivers/net/ethernet/qualcomm/qca_uart.c
index d0db9d0adad7..321fd8d00730 100644
--- a/drivers/net/ethernet/qualcomm/qca_uart.c
+++ b/drivers/net/ethernet/qualcomm/qca_uart.c
@@ -397,6 +397,6 @@ module_serdev_device_driver(qca_uart_driver);
MODULE_DESCRIPTION("Qualcomm Atheros QCA7000 UART Driver");
MODULE_AUTHOR("Qualcomm Atheros Communications");
-MODULE_AUTHOR("Stefan Wahren <[email protected]>");
+MODULE_AUTHOR("Stefan Wahren <[email protected]>");
MODULE_LICENSE("Dual BSD/GPL");
MODULE_VERSION(QCAUART_DRV_VERSION);
--
2.34.1
qcafrm_fsm_decode has the almost the same function description in
qca_7k_common.c. So drop the comment here.
Signed-off-by: Stefan Wahren <[email protected]>
Reviewed-by: Jacob Keller <[email protected]>
---
drivers/net/ethernet/qualcomm/qca_7k_common.h | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/qca_7k_common.h b/drivers/net/ethernet/qualcomm/qca_7k_common.h
index 928554f11e35..71bdf5d9f8d7 100644
--- a/drivers/net/ethernet/qualcomm/qca_7k_common.h
+++ b/drivers/net/ethernet/qualcomm/qca_7k_common.h
@@ -128,17 +128,6 @@ static inline void qcafrm_fsm_init_uart(struct qcafrm_handle *handle)
handle->state = handle->init;
}
-/* Gather received bytes and try to extract a full Ethernet frame
- * by following a simple state machine.
- *
- * Return: QCAFRM_GATHER No Ethernet frame fully received yet.
- * QCAFRM_NOHEAD Header expected but not found.
- * QCAFRM_INVLEN QCA7K frame length is invalid
- * QCAFRM_NOTAIL Footer expected but not found.
- * > 0 Number of byte in the fully received
- * Ethernet frame
- */
-
s32 qcafrm_fsm_decode(struct qcafrm_handle *handle, u8 *buf, u16 buf_len, u8 recv_byte);
#endif /* _QCA_FRAMING_H */
--
2.34.1
There are two points with the calculation of RX buffer size which are
not optimal:
1. dev->mtu is a mutual parameter and it's currently initialized with
QCAFRM_MAX_MTU. But for RX buffer size calculation we always need the
maximum possible MTU. So better use the define directly.
2. This magic number 4 represent the hardware generated frame length
which is specific to SPI. We better replace this with the suitable
define.
There is no functional change.
Signed-off-by: Stefan Wahren <[email protected]>
Reviewed-by: Jacob Keller <[email protected]>
---
drivers/net/ethernet/qualcomm/qca_spi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c b/drivers/net/ethernet/qualcomm/qca_spi.c
index 80182f8a4a50..6d2859aad921 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -813,8 +813,8 @@ qcaspi_netdev_init(struct net_device *dev)
qca->clkspeed = qcaspi_clkspeed;
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) * QCASPI_RX_MAX_FRAMES;
+ qca->buffer_size = (QCAFRM_MAX_MTU + VLAN_ETH_HLEN + QCAFRM_HEADER_LEN +
+ QCAFRM_FOOTER_LEN + QCASPI_HW_PKT_LEN) * QCASPI_RX_MAX_FRAMES;
memset(&qca->stats, 0, sizeof(struct qcaspi_stats));
--
2.34.1
The functions qcaspi_netdev_open/close are responsible of request &
free of the SPI interrupt, which wasn't the best choice because
allocation problems are discovered not during probe. So let us split
IRQ allocation & enabling, so we can take advantage of a device
managed IRQ.
Signed-off-by: Stefan Wahren <[email protected]>
Reviewed-by: Jacob Keller <[email protected]>
---
drivers/net/ethernet/qualcomm/qca_spi.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c b/drivers/net/ethernet/qualcomm/qca_spi.c
index fc272ca7bdca..294d4ee3b38f 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -687,7 +687,6 @@ static int
qcaspi_netdev_open(struct net_device *dev)
{
struct qcaspi *qca = netdev_priv(dev);
- int ret = 0;
if (!qca)
return -EINVAL;
@@ -697,13 +696,7 @@ qcaspi_netdev_open(struct net_device *dev)
qca->sync = QCASPI_SYNC_UNKNOWN;
qcafrm_fsm_init_spi(&qca->frm_handle);
- ret = request_irq(qca->spi_dev->irq, qcaspi_intr_handler, 0,
- 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);
- return ret;
- }
+ enable_irq(qca->spi_dev->irq);
/* SPI thread takes care of TX queue */
kthread_unpark(qca->spi_thread);
@@ -720,7 +713,7 @@ qcaspi_netdev_close(struct net_device *dev)
kthread_park(qca->spi_thread);
qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, 0, wr_verify);
- free_irq(qca->spi_dev->irq, qca);
+ disable_irq(qca->spi_dev->irq);
return 0;
}
@@ -987,6 +980,15 @@ qca_spi_probe(struct spi_device *spi)
spi_set_drvdata(spi, qcaspi_devs);
+ ret = devm_request_irq(&spi->dev, spi->irq, qcaspi_intr_handler,
+ IRQF_NO_AUTOEN, qca->net_dev->name, qca);
+ if (ret) {
+ dev_err(&spi->dev, "Unable to get IRQ %d (irqval=%d).\n",
+ spi->irq, ret);
+ free_netdev(qcaspi_devs);
+ return ret;
+ }
+
ret = of_get_ethdev_address(spi->dev.of_node, qca->net_dev);
if (ret) {
eth_hw_addr_random(qca->net_dev);
--
2.34.1
Add a .mailmap entry because my old i2se.com address will be
deactivated in the near future.
Signed-off-by: Stefan Wahren <[email protected]>
---
.mailmap | 1 +
1 file changed, 1 insertion(+)
diff --git a/.mailmap b/.mailmap
index 04998f7bda81..95b4fe465cc6 100644
--- a/.mailmap
+++ b/.mailmap
@@ -568,6 +568,7 @@ Simon Kelley <[email protected]>
Sricharan Ramabadhran <[email protected]> <[email protected]>
Srinivas Ramana <[email protected]> <[email protected]>
Sriram R <[email protected]> <[email protected]>
+Stefan Wahren <[email protected]> <[email protected]>
Stéphane Witzmann <[email protected]>
Stephen Hemminger <[email protected]> <[email protected]>
Stephen Hemminger <[email protected]> <[email protected]>
--
2.34.1
All defines in qca_spi.h except of the two ring limit defines have
a QCASPI prefix. Since the name is quite generic add the QCASPI prefix
to avoid possible name conflicts.
Signed-off-by: Stefan Wahren <[email protected]>
Reviewed-by: Jacob Keller <[email protected]>
---
drivers/net/ethernet/qualcomm/qca_debug.c | 6 +++---
drivers/net/ethernet/qualcomm/qca_spi.c | 4 ++--
drivers/net/ethernet/qualcomm/qca_spi.h | 6 +++---
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/qca_debug.c b/drivers/net/ethernet/qualcomm/qca_debug.c
index 1822f2ad8f0d..857883d0b5ed 100644
--- a/drivers/net/ethernet/qualcomm/qca_debug.c
+++ b/drivers/net/ethernet/qualcomm/qca_debug.c
@@ -255,7 +255,7 @@ qcaspi_get_ringparam(struct net_device *dev, struct ethtool_ringparam *ring,
struct qcaspi *qca = netdev_priv(dev);
ring->rx_max_pending = QCASPI_RX_MAX_FRAMES;
- ring->tx_max_pending = TX_RING_MAX_LEN;
+ ring->tx_max_pending = QCASPI_TX_RING_MAX_LEN;
ring->rx_pending = QCASPI_RX_MAX_FRAMES;
ring->tx_pending = qca->txr.count;
}
@@ -275,8 +275,8 @@ qcaspi_set_ringparam(struct net_device *dev, struct ethtool_ringparam *ring,
if (qca->spi_thread)
kthread_park(qca->spi_thread);
- qca->txr.count = max_t(u32, ring->tx_pending, TX_RING_MIN_LEN);
- qca->txr.count = min_t(u16, qca->txr.count, TX_RING_MAX_LEN);
+ qca->txr.count = max_t(u32, ring->tx_pending, QCASPI_TX_RING_MIN_LEN);
+ qca->txr.count = min_t(u16, qca->txr.count, QCASPI_TX_RING_MAX_LEN);
if (qca->spi_thread)
kthread_unpark(qca->spi_thread);
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c b/drivers/net/ethernet/qualcomm/qca_spi.c
index 919a400412cb..2ca25844d91c 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -476,7 +476,7 @@ qcaspi_flush_tx_ring(struct qcaspi *qca)
* has been replaced by netif_tx_lock_bh() and so on.
*/
netif_tx_lock_bh(qca->net_dev);
- for (i = 0; i < TX_RING_MAX_LEN; i++) {
+ for (i = 0; i < QCASPI_TX_RING_MAX_LEN; i++) {
if (qca->txr.skb[i]) {
dev_kfree_skb(qca->txr.skb[i]);
qca->txr.skb[i] = NULL;
@@ -889,7 +889,7 @@ qcaspi_netdev_setup(struct net_device *dev)
memset(qca, 0, sizeof(struct qcaspi));
memset(&qca->txr, 0, sizeof(qca->txr));
- qca->txr.count = TX_RING_MAX_LEN;
+ qca->txr.count = QCASPI_TX_RING_MAX_LEN;
}
static const struct of_device_id qca_spi_of_match[] = {
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.h b/drivers/net/ethernet/qualcomm/qca_spi.h
index 3067356106f0..dcecb072b8eb 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.h
+++ b/drivers/net/ethernet/qualcomm/qca_spi.h
@@ -39,8 +39,8 @@
#define QCASPI_GOOD_SIGNATURE 0xAA55
-#define TX_RING_MAX_LEN 10
-#define TX_RING_MIN_LEN 2
+#define QCASPI_TX_RING_MAX_LEN 10
+#define QCASPI_TX_RING_MIN_LEN 2
/* sync related constants */
#define QCASPI_SYNC_UNKNOWN 0
@@ -54,7 +54,7 @@
#define QCASPI_EVENT_CPUON 1
struct tx_ring {
- struct sk_buff *skb[TX_RING_MAX_LEN];
+ struct sk_buff *skb[QCASPI_TX_RING_MAX_LEN];
u16 head;
u16 tail;
u16 size;
--
2.34.1
Currently qca_spi reserves enough space for 4 complete Ethernet over SPI
frames in the receive buffer. Unfortunately this is hidden under a magic
number. So replace it with a more self explaining define.
Signed-off-by: Stefan Wahren <[email protected]>
Reviewed-by: Jacob Keller <[email protected]>
---
drivers/net/ethernet/qualcomm/qca_spi.c | 2 +-
drivers/net/ethernet/qualcomm/qca_spi.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c b/drivers/net/ethernet/qualcomm/qca_spi.c
index 2ca25844d91c..80182f8a4a50 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -814,7 +814,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) * QCASPI_RX_MAX_FRAMES;
memset(&qca->stats, 0, sizeof(struct qcaspi_stats));
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.h b/drivers/net/ethernet/qualcomm/qca_spi.h
index dcecb072b8eb..f735ad77402f 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.h
+++ b/drivers/net/ethernet/qualcomm/qca_spi.h
@@ -41,6 +41,7 @@
#define QCASPI_TX_RING_MAX_LEN 10
#define QCASPI_TX_RING_MIN_LEN 2
+#define QCASPI_RX_MAX_FRAMES 4
/* sync related constants */
#define QCASPI_SYNC_UNKNOWN 0
--
2.34.1
This member is never used. So drop it.
Signed-off-by: Stefan Wahren <[email protected]>
Reviewed-by: Jacob Keller <[email protected]>
---
drivers/net/ethernet/qualcomm/qca_7k_common.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/qca_7k_common.h b/drivers/net/ethernet/qualcomm/qca_7k_common.h
index 71bdf5d9f8d7..088cca7f61db 100644
--- a/drivers/net/ethernet/qualcomm/qca_7k_common.h
+++ b/drivers/net/ethernet/qualcomm/qca_7k_common.h
@@ -107,9 +107,6 @@ struct qcafrm_handle {
/* Offset in buffer (borrowed for length too) */
u16 offset;
-
- /* Frame length as kept by this module */
- u16 len;
};
u16 qcafrm_create_header(u8 *buf, u16 len);
--
2.34.1
Most of the users doesn't know the expected signature of the QCA700x.
So provide it within the error message. Btw use lowercase for hex as
in the rest of the driver.
Signed-off-by: Stefan Wahren <[email protected]>
Reviewed-by: Jacob Keller <[email protected]>
---
drivers/net/ethernet/qualcomm/qca_spi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c b/drivers/net/ethernet/qualcomm/qca_spi.c
index 6d2859aad921..c5d240fe4146 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -1005,8 +1005,8 @@ qca_spi_probe(struct spi_device *spi)
qcaspi_read_register(qca, SPI_REG_SIGNATURE, &signature);
if (signature != QCASPI_GOOD_SIGNATURE) {
- dev_err(&spi->dev, "Invalid signature (0x%04X)\n",
- signature);
+ dev_err(&spi->dev, "Invalid signature (expected 0x%04x, read 0x%04x)\n",
+ QCASPI_GOOD_SIGNATURE, signature);
free_netdev(qcaspi_devs);
return -EFAULT;
}
--
2.34.1
From: Stefan Wahren <[email protected]>
According to MODULE_LICENSE the driver is under a dual license.
So replace the BSD license text with the proper SPDX tag.
Signed-off-by: Stefan Wahren <[email protected]>
Signed-off-by: Stefan Wahren <[email protected]>
Reviewed-by: Jacob Keller <[email protected]>
---
drivers/net/ethernet/qualcomm/qca_7k.c | 17 +----------------
drivers/net/ethernet/qualcomm/qca_7k.h | 16 +---------------
drivers/net/ethernet/qualcomm/qca_7k_common.c | 15 +--------------
drivers/net/ethernet/qualcomm/qca_7k_common.h | 15 +--------------
drivers/net/ethernet/qualcomm/qca_debug.c | 15 +--------------
drivers/net/ethernet/qualcomm/qca_debug.h | 15 +--------------
drivers/net/ethernet/qualcomm/qca_spi.c | 15 +--------------
drivers/net/ethernet/qualcomm/qca_spi.h | 15 +--------------
drivers/net/ethernet/qualcomm/qca_uart.c | 15 +--------------
9 files changed, 9 insertions(+), 129 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/qca_7k.c b/drivers/net/ethernet/qualcomm/qca_7k.c
index 4292c89bd35c..6263e4cf47fa 100644
--- a/drivers/net/ethernet/qualcomm/qca_7k.c
+++ b/drivers/net/ethernet/qualcomm/qca_7k.c
@@ -1,22 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
/*
- *
* Copyright (c) 2011, 2012, Qualcomm Atheros Communications Inc.
* Copyright (c) 2014, I2SE GmbH
- *
- * Permission to use, copy, modify, and/or distribute this software
- * for any purpose with or without fee is hereby granted, provided
- * that the above copyright notice and this permission notice appear
- * in all copies.
- *
- * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
- * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
- * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
- * THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
- * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
- * LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
- * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
- * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
- *
*/
/* This module implements the Qualcomm Atheros SPI protocol for
diff --git a/drivers/net/ethernet/qualcomm/qca_7k.h b/drivers/net/ethernet/qualcomm/qca_7k.h
index 356de8ec5d48..828ee9c27578 100644
--- a/drivers/net/ethernet/qualcomm/qca_7k.h
+++ b/drivers/net/ethernet/qualcomm/qca_7k.h
@@ -1,21 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
/*
* Copyright (c) 2011, 2012, Qualcomm Atheros Communications Inc.
* Copyright (c) 2014, I2SE GmbH
- *
- * Permission to use, copy, modify, and/or distribute this software
- * for any purpose with or without fee is hereby granted, provided
- * that the above copyright notice and this permission notice appear
- * in all copies.
- *
- * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
- * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
- * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
- * THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
- * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
- * LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
- * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
- * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
- *
*/
/* Qualcomm Atheros SPI register definition.
diff --git a/drivers/net/ethernet/qualcomm/qca_7k_common.c b/drivers/net/ethernet/qualcomm/qca_7k_common.c
index 6b511f05df61..be2f754efd21 100644
--- a/drivers/net/ethernet/qualcomm/qca_7k_common.c
+++ b/drivers/net/ethernet/qualcomm/qca_7k_common.c
@@ -1,20 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
/*
* Copyright (c) 2011, 2012, Atheros Communications Inc.
* Copyright (c) 2014, I2SE GmbH
- *
- * Permission to use, copy, modify, and/or distribute this software
- * for any purpose with or without fee is hereby granted, provided
- * that the above copyright notice and this permission notice appear
- * in all copies.
- *
- * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
- * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
- * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
- * THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
- * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
- * LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
- * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
- * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
/* Atheros ethernet framing. Every Ethernet frame is surrounded
diff --git a/drivers/net/ethernet/qualcomm/qca_7k_common.h b/drivers/net/ethernet/qualcomm/qca_7k_common.h
index 088cca7f61db..44ed66fdb407 100644
--- a/drivers/net/ethernet/qualcomm/qca_7k_common.h
+++ b/drivers/net/ethernet/qualcomm/qca_7k_common.h
@@ -1,20 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
/*
* Copyright (c) 2011, 2012, Atheros Communications Inc.
* Copyright (c) 2014, I2SE GmbH
- *
- * Permission to use, copy, modify, and/or distribute this software
- * for any purpose with or without fee is hereby granted, provided
- * that the above copyright notice and this permission notice appear
- * in all copies.
- *
- * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
- * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
- * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
- * THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
- * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
- * LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
- * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
- * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
/* Atheros Ethernet framing. Every Ethernet frame is surrounded by an atheros
diff --git a/drivers/net/ethernet/qualcomm/qca_debug.c b/drivers/net/ethernet/qualcomm/qca_debug.c
index 857883d0b5ed..ff3b89e9028e 100644
--- a/drivers/net/ethernet/qualcomm/qca_debug.c
+++ b/drivers/net/ethernet/qualcomm/qca_debug.c
@@ -1,20 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
/*
* Copyright (c) 2011, 2012, Qualcomm Atheros Communications Inc.
* Copyright (c) 2014, I2SE GmbH
- *
- * Permission to use, copy, modify, and/or distribute this software
- * for any purpose with or without fee is hereby granted, provided
- * that the above copyright notice and this permission notice appear
- * in all copies.
- *
- * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
- * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
- * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
- * THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
- * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
- * LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
- * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
- * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
/* This file contains debugging routines for use in the QCA7K driver.
diff --git a/drivers/net/ethernet/qualcomm/qca_debug.h b/drivers/net/ethernet/qualcomm/qca_debug.h
index 46a785844421..0d98cef3abc4 100644
--- a/drivers/net/ethernet/qualcomm/qca_debug.h
+++ b/drivers/net/ethernet/qualcomm/qca_debug.h
@@ -1,20 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
/*
* Copyright (c) 2011, 2012, Qualcomm Atheros Communications Inc.
* Copyright (c) 2014, I2SE GmbH
- *
- * Permission to use, copy, modify, and/or distribute this software
- * for any purpose with or without fee is hereby granted, provided
- * that the above copyright notice and this permission notice appear
- * in all copies.
- *
- * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
- * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
- * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
- * THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
- * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
- * LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
- * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
- * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
/* This file contains debugging routines for use in the QCA7K driver.
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c b/drivers/net/ethernet/qualcomm/qca_spi.c
index d3109eb24b4d..7c63baeb3844 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -1,20 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
/*
* Copyright (c) 2011, 2012, Qualcomm Atheros Communications Inc.
* Copyright (c) 2014, I2SE GmbH
- *
- * Permission to use, copy, modify, and/or distribute this software
- * for any purpose with or without fee is hereby granted, provided
- * that the above copyright notice and this permission notice appear
- * in all copies.
- *
- * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
- * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
- * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
- * THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
- * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
- * LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
- * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
- * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
/* This module implements the Qualcomm Atheros SPI protocol for
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.h b/drivers/net/ethernet/qualcomm/qca_spi.h
index f735ad77402f..d59cb2352cee 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.h
+++ b/drivers/net/ethernet/qualcomm/qca_spi.h
@@ -1,20 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
/*
* Copyright (c) 2011, 2012, Qualcomm Atheros Communications Inc.
* Copyright (c) 2014, I2SE GmbH
- *
- * Permission to use, copy, modify, and/or distribute this software
- * for any purpose with or without fee is hereby granted, provided
- * that the above copyright notice and this permission notice appear
- * in all copies.
- *
- * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
- * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
- * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
- * THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
- * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
- * LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
- * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
- * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
/* Qualcomm Atheros SPI register definition.
diff --git a/drivers/net/ethernet/qualcomm/qca_uart.c b/drivers/net/ethernet/qualcomm/qca_uart.c
index 223321897b96..d0db9d0adad7 100644
--- a/drivers/net/ethernet/qualcomm/qca_uart.c
+++ b/drivers/net/ethernet/qualcomm/qca_uart.c
@@ -1,20 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
/*
* Copyright (c) 2011, 2012, Qualcomm Atheros Communications Inc.
* Copyright (c) 2017, I2SE GmbH
- *
- * Permission to use, copy, modify, and/or distribute this software
- * for any purpose with or without fee is hereby granted, provided
- * that the above copyright notice and this permission notice appear
- * in all copies.
- *
- * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
- * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
- * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
- * THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
- * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
- * LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
- * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
- * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
/* This module implements the Qualcomm Atheros UART protocol for
--
2.34.1
Since upstreaming i contributed a lot to this driver(s),
so add myself as a maintainer.
Signed-off-by: Stefan Wahren <[email protected]>
---
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 8d1052fa6a69..5808f06b4ccf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18009,6 +18009,13 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git
F: Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml
F: drivers/net/wireless/ath/ath9k/
+QUALCOMM ATHEROS QCA7K ETHERNET DRIVER
+M: Stefan Wahren <[email protected]>
+L: [email protected]
+S: Maintained
+F: Documentation/devicetree/bindings/net/qca,qca7000.txt
+F: drivers/net/ethernet/qualcomm/qca*
+
QUALCOMM BAM-DMUX WWAN NETWORK DRIVER
M: Stephan Gerhold <[email protected]>
L: [email protected]
--
2.34.1
The skb spare room needs to be expanded for SPI header, footer
and possible padding within the TX path. So announce the necessary
space in order to avoid expensive skb_copy_expand calls.
Signed-off-by: Stefan Wahren <[email protected]>
Reviewed-by: Jacob Keller <[email protected]>
---
drivers/net/ethernet/qualcomm/qca_spi.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c b/drivers/net/ethernet/qualcomm/qca_spi.c
index 294d4ee3b38f..919a400412cb 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -877,6 +877,8 @@ qcaspi_netdev_setup(struct net_device *dev)
qcaspi_set_ethtool_ops(dev);
dev->watchdog_timeo = QCASPI_TX_TIMEOUT;
dev->priv_flags &= ~IFF_TX_SKB_SHARING;
+ dev->needed_tailroom = ALIGN(QCAFRM_FOOTER_LEN + QCAFRM_MIN_LEN, 4);
+ dev->needed_headroom = ALIGN(QCAFRM_HEADER_LEN, 4);
dev->tx_queue_len = 100;
/* MTU range: 46 - 1500 */
--
2.34.1
The qca_spi driver create/stop the SPI kernel thread in case
of netdev_open/close. This isn't optimal because there is no
need for such an expensive operation.
So improve this by moving create/stop of the SPI kernel into
the init/uninit ops. The open/close ops could just
'park/unpark' the SPI kernel thread.
Signed-off-by: Stefan Wahren <[email protected]>
---
drivers/net/ethernet/qualcomm/qca_spi.c | 33 ++++++++++++++-----------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c b/drivers/net/ethernet/qualcomm/qca_spi.c
index 5f3c11fb3fa2..fc272ca7bdca 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -697,25 +697,17 @@ qcaspi_netdev_open(struct net_device *dev)
qca->sync = QCASPI_SYNC_UNKNOWN;
qcafrm_fsm_init_spi(&qca->frm_handle);
- qca->spi_thread = kthread_run((void *)qcaspi_spi_thread,
- qca, "%s", dev->name);
-
- if (IS_ERR(qca->spi_thread)) {
- netdev_err(dev, "%s: unable to start kernel thread.\n",
- QCASPI_DRV_NAME);
- return PTR_ERR(qca->spi_thread);
- }
-
ret = request_irq(qca->spi_dev->irq, qcaspi_intr_handler, 0,
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);
- kthread_stop(qca->spi_thread);
return ret;
}
/* SPI thread takes care of TX queue */
+ kthread_unpark(qca->spi_thread);
+ wake_up_process(qca->spi_thread);
return 0;
}
@@ -725,15 +717,11 @@ qcaspi_netdev_close(struct net_device *dev)
{
struct qcaspi *qca = netdev_priv(dev);
- netif_stop_queue(dev);
+ kthread_park(qca->spi_thread);
qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, 0, wr_verify);
free_irq(qca->spi_dev->irq, qca);
- kthread_stop(qca->spi_thread);
- qca->spi_thread = NULL;
- qcaspi_flush_tx_ring(qca);
-
return 0;
}
@@ -825,6 +813,7 @@ static int
qcaspi_netdev_init(struct net_device *dev)
{
struct qcaspi *qca = netdev_priv(dev);
+ struct task_struct *thread;
dev->mtu = QCAFRM_MAX_MTU;
dev->type = ARPHRD_ETHER;
@@ -848,6 +837,15 @@ qcaspi_netdev_init(struct net_device *dev)
return -ENOBUFS;
}
+ thread = kthread_create(qcaspi_spi_thread, qca, "%s", dev->name);
+ if (IS_ERR(thread)) {
+ netdev_err(dev, "%s: unable to start kernel thread.\n",
+ QCASPI_DRV_NAME);
+ return PTR_ERR(thread);
+ }
+
+ qca->spi_thread = thread;
+
return 0;
}
@@ -856,6 +854,11 @@ qcaspi_netdev_uninit(struct net_device *dev)
{
struct qcaspi *qca = netdev_priv(dev);
+ if (qca->spi_thread) {
+ kthread_stop(qca->spi_thread);
+ qca->spi_thread = NULL;
+ }
+
kfree(qca->rx_buffer);
qca->buffer_size = 0;
dev_kfree_skb(qca->rx_skb);
--
2.34.1
On Wed, 24 Jan 2024 23:31:58 +0100 Stefan Wahren wrote:
> The qca_spi driver create/stop the SPI kernel thread in case
> of netdev_open/close. This isn't optimal because there is no
> need for such an expensive operation.
>
> So improve this by moving create/stop of the SPI kernel into
> the init/uninit ops. The open/close ops could just
> 'park/unpark' the SPI kernel thread.
What's the concern? I don't think that creating a thread is all
expensive. And we shouldn't have a thread sitting around when
the interface isn't use. I mean - if you ask me what's better
a small chance that the creation will fail at open or having
a parked and unused thread when device is down - I'd pick
the former.. But I may well be missing the point.
> @@ -825,6 +813,7 @@ static int
> qcaspi_netdev_init(struct net_device *dev)
> {
> struct qcaspi *qca = netdev_priv(dev);
> + struct task_struct *thread;
>
> dev->mtu = QCAFRM_MAX_MTU;
> dev->type = ARPHRD_ETHER;
> @@ -848,6 +837,15 @@ qcaspi_netdev_init(struct net_device *dev)
> return -ENOBUFS;
> }
>
> + thread = kthread_create(qcaspi_spi_thread, qca, "%s", dev->name);
> + if (IS_ERR(thread)) {
> + netdev_err(dev, "%s: unable to start kernel thread.\n",
> + QCASPI_DRV_NAME);
> + return PTR_ERR(thread);
I'm 90% sure this leaks resources on failure, too.
Rest of the series LGTM!
Hi Jakub,
Am 26.01.24 um 03:36 schrieb Jakub Kicinski:
> On Wed, 24 Jan 2024 23:31:58 +0100 Stefan Wahren wrote:
>> The qca_spi driver create/stop the SPI kernel thread in case
>> of netdev_open/close. This isn't optimal because there is no
>> need for such an expensive operation.
>>
>> So improve this by moving create/stop of the SPI kernel into
>> the init/uninit ops. The open/close ops could just
>> 'park/unpark' the SPI kernel thread.
> What's the concern? I don't think that creating a thread is all
> expensive. And we shouldn't have a thread sitting around when
> the interface isn't use. I mean - if you ask me what's better
> a small chance that the creation will fail at open or having
> a parked and unused thread when device is down - I'd pick
> the former.. But I may well be missing the point.
there is actually another reason for this change which is not mentioned
in the commit message. The pointer spi_thread can have 3 states:
- valid thread
- error pointer
- NULL
So the second motivation was to eliminate the possibility of an error
pointer
by using a local variable. So we only have to care about NULL.
I will change this in V4.
>
>> @@ -825,6 +813,7 @@ static int
>> qcaspi_netdev_init(struct net_device *dev)
>> {
>> struct qcaspi *qca = netdev_priv(dev);
>> + struct task_struct *thread;
>>
>> dev->mtu = QCAFRM_MAX_MTU;
>> dev->type = ARPHRD_ETHER;
>> @@ -848,6 +837,15 @@ qcaspi_netdev_init(struct net_device *dev)
>> return -ENOBUFS;
>> }
>>
>> + thread = kthread_create(qcaspi_spi_thread, qca, "%s", dev->name);
>> + if (IS_ERR(thread)) {
>> + netdev_err(dev, "%s: unable to start kernel thread.\n",
>> + QCASPI_DRV_NAME);
>> + return PTR_ERR(thread);
> I'm 90% sure this leaks resources on failure, too.
Oops
Best regards
>
> Rest of the series LGTM!