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 <[email protected]>
> Signed-off-by: Stefan Wahren <[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
>
>