Re: [PATCH V2 08/12 net-next] qca_spi: Improve calculation of RX buffer size
From: Jacob Keller <jacob.e.keller@intel.com>
Date: 2023-12-20 23:00:39
Also in:
lkml
On 12/18/2023 3:26 PM, Stefan Wahren wrote:
There are two points with the calculation of RX buffer size which are not optimal: 1. mtu is a mutual parameter, but actually we 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.
This is a bit confusing why you said no functional change. At first my thought was that dev->mtu could easily be smaller than the maximum MTU. (thats why its configurable after all?). But actually the diff context cuts off a few lines that are extremely relevant:
struct qcaspi *qca = netdev_priv(dev);
dev->mtu = QCAFRM_MAX_MTU;We assign dev->mtu here to QCAFRM_MAX_MTU
dev->type = ARPHRD_ETHER;
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) * 4;And then use it here.
memset(&qca->stats, 0, sizeof(struct qcaspi_stats));Makes sense to use the constant directly. It might have helped to have a bit more of this context in the commit message, but its still a good cleanup. Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
quoted hunk ↗ jump to hunk
Signed-off-by: Stefan Wahren <wahrenst@gmx.net> --- 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