| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250905153500.GH553991@horms.kernel.org>
Date: Fri, 5 Sep 2025 16:35:00 +0100
From: Simon Horman <horms@...nel.org>
To: Vivian Wang <wangruikang@...as.ac.cn>
Cc: Andrew Lunn <andrew+netdev@...n.ch>, Jakub Kicinski <kuba@...nel.org>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Yixun Lan <dlan@...too.org>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
Philipp Zabel <p.zabel@...gutronix.de>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>, Alexandre Ghiti <alex@...ti.fr>,
Vivian Wang <uwu@...m.page>,
Vadim Fedorenko <vadim.fedorenko@...ux.dev>,
Junhui Liu <junhui.liu@...moral.tech>,
Maxime Chevallier <maxime.chevallier@...tlin.com>,
netdev@...r.kernel.org, devicetree@...r.kernel.org,
linux-riscv@...ts.infradead.org, spacemit@...ts.linux.dev,
linux-kernel@...r.kernel.org,
Troy Mitchell <troy.mitchell@...ux.spacemit.com>
Subject: Re: [PATCH net-next v9 2/5] net: spacemit: Add K1 Ethernet MAC
On Fri, Sep 05, 2025 at 07:09:31PM +0800, Vivian Wang wrote:
> The Ethernet MACs found on SpacemiT K1 appears to be a custom design
> that only superficially resembles some other embedded MACs. SpacemiT
> refers to them as "EMAC", so let's just call the driver "k1_emac".
>
> Supports RGMII and RMII interfaces. Includes support for MAC hardware
> statistics counters. PTP support is not implemented.
>
> Signed-off-by: Vivian Wang <wangruikang@...as.ac.cn>
> Reviewed-by: Maxime Chevallier <maxime.chevallier@...tlin.com>
> Reviewed-by: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
> Reviewed-by: Troy Mitchell <troy.mitchell@...ux.spacemit.com>
> Tested-by: Junhui Liu <junhui.liu@...moral.tech>
> Tested-by: Troy Mitchell <troy.mitchell@...ux.spacemit.com>
...
> diff --git a/drivers/net/ethernet/spacemit/k1_emac.c b/drivers/net/ethernet/spacemit/k1_emac.c
...
> +static void emac_init_hw(struct emac_priv *priv)
> +{
> + /* Destination address for 802.3x Ethernet flow control */
> + u8 fc_dest_addr[ETH_ALEN] = { 0x01, 0x80, 0xc2, 0x00, 0x00, 0x01 };
> +
> + u32 rxirq = 0, dma = 0;
> +
> + regmap_set_bits(priv->regmap_apmu,
> + priv->regmap_apmu_offset + APMU_EMAC_CTRL_REG,
> + AXI_SINGLE_ID);
> +
> + /* Disable transmit and receive units */
> + emac_wr(priv, MAC_RECEIVE_CONTROL, 0x0);
> + emac_wr(priv, MAC_TRANSMIT_CONTROL, 0x0);
> +
> + /* Enable MAC address 1 filtering */
> + emac_wr(priv, MAC_ADDRESS_CONTROL, MREGBIT_MAC_ADDRESS1_ENABLE);
> +
> + /* Zero initialize the multicast hash table */
> + emac_wr(priv, MAC_MULTICAST_HASH_TABLE1, 0x0);
> + emac_wr(priv, MAC_MULTICAST_HASH_TABLE2, 0x0);
> + emac_wr(priv, MAC_MULTICAST_HASH_TABLE3, 0x0);
> + emac_wr(priv, MAC_MULTICAST_HASH_TABLE4, 0x0);
> +
> + /* Configure thresholds */
> + emac_wr(priv, MAC_TRANSMIT_FIFO_ALMOST_FULL, DEFAULT_TX_ALMOST_FULL);
> + emac_wr(priv, MAC_TRANSMIT_PACKET_START_THRESHOLD,
> + DEFAULT_TX_THRESHOLD);
> + emac_wr(priv, MAC_RECEIVE_PACKET_START_THRESHOLD, DEFAULT_RX_THRESHOLD);
> +
> + /* Configure flow control (enabled in emac_adjust_link() later) */
> + emac_set_mac_addr_reg(priv, fc_dest_addr, MAC_FC_SOURCE_ADDRESS_HIGH);
> + emac_wr(priv, MAC_FC_PAUSE_HIGH_THRESHOLD, DEFAULT_FC_FIFO_HIGH);
> + emac_wr(priv, MAC_FC_HIGH_PAUSE_TIME, DEFAULT_FC_PAUSE_TIME);
> + emac_wr(priv, MAC_FC_PAUSE_LOW_THRESHOLD, 0);
> +
> + /* RX IRQ mitigation */
> + rxirq = EMAC_RX_FRAMES & MREGBIT_RECEIVE_IRQ_FRAME_COUNTER_MASK;
> + rxirq |= (EMAC_RX_COAL_TIMEOUT
> + << MREGBIT_RECEIVE_IRQ_TIMEOUT_COUNTER_SHIFT) &
> + MREGBIT_RECEIVE_IRQ_TIMEOUT_COUNTER_MASK;
Probably this driver can benefit from using FIELD_PREP and FIELD_GET
in a number of places. In this case I think it would mean that
MREGBIT_RECEIVE_IRQ_TIMEOUT_COUNTER_SHIFT can be removed entirely.
> +
> + rxirq |= MREGBIT_RECEIVE_IRQ_MITIGATION_ENABLE;
> + emac_wr(priv, DMA_RECEIVE_IRQ_MITIGATION_CTRL, rxirq);
...
> +/* Returns number of packets received */
> +static int emac_rx_clean_desc(struct emac_priv *priv, int budget)
> +{
> + struct net_device *ndev = priv->ndev;
> + struct emac_rx_desc_buffer *rx_buf;
> + struct emac_desc_ring *rx_ring;
> + struct sk_buff *skb = NULL;
> + struct emac_desc *rx_desc;
> + u32 got = 0, skb_len, i;
> + int status;
> +
> + rx_ring = &priv->rx_ring;
> +
> + i = rx_ring->tail;
> +
> + while (budget--) {
> + rx_desc = &((struct emac_desc *)rx_ring->desc_addr)[i];
> +
> + /* Stop checking if rx_desc still owned by DMA */
> + if (READ_ONCE(rx_desc->desc0) & RX_DESC_0_OWN)
> + break;
> +
> + dma_rmb();
> +
> + rx_buf = &rx_ring->rx_desc_buf[i];
> +
> + if (!rx_buf->skb)
> + break;
> +
> + got++;
> +
> + dma_unmap_single(&priv->pdev->dev, rx_buf->dma_addr,
> + rx_buf->dma_len, DMA_FROM_DEVICE);
> +
> + status = emac_rx_frame_status(priv, rx_desc);
> + if (unlikely(status == RX_FRAME_DISCARD)) {
> + ndev->stats.rx_dropped++;
As per the comment in struct net-device,
ndev->stats should not be used in modern drivers.
Probably you want to implement NETDEV_PCPU_STAT_TSTATS.
Sorry for not mentioning this in an earlier review of
stats in this driver.
> + dev_kfree_skb_irq(rx_buf->skb);
> + rx_buf->skb = NULL;
> + } else {
> + skb = rx_buf->skb;
> + skb_len = rx_frame_len(rx_desc) - ETH_FCS_LEN;
> + skb_put(skb, skb_len);
> + skb->dev = ndev;
> + ndev->hard_header_len = ETH_HLEN;
> +
> + skb->protocol = eth_type_trans(skb, ndev);
> +
> + skb->ip_summed = CHECKSUM_NONE;
> +
> + napi_gro_receive(&priv->napi, skb);
> +
> + ndev->stats.rx_packets++;
> + ndev->stats.rx_bytes += skb_len;
> +
> + memset(rx_desc, 0, sizeof(struct emac_desc));
> + rx_buf->skb = NULL;
> + }
> +
> + if (++i == rx_ring->total_cnt)
> + i = 0;
> + }
> +
> + rx_ring->tail = i;
> +
> + emac_alloc_rx_desc_buffers(priv);
> +
> + return got;
> +}
...
Powered by blists - more mailing lists