On Tue, Feb 18, 2025 at 10:21:34AM +0100, Steffen Trumtrar wrote:
> The Designware XGMAC is designed for 1/2.5/5/10G ethernet applications.
> It is mostly identical to the eqos driver but has a different register
> layout.
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@xxxxxxxxxxxxxx>
> ---
> drivers/net/Kconfig | 18 +
> drivers/net/Makefile | 2 +
> drivers/net/designware_xgmac.c | 856 +++++++++++++++++++++++++++++++++
> drivers/net/designware_xgmac.h | 298 ++++++++++++
> drivers/net/designware_xgmac_socfpga.c | 156 ++++++
> 5 files changed, 1330 insertions(+)
>
> diff --git a/drivers/net/designware_xgmac.c b/drivers/net/designware_xgmac.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..ca79ab4cb4a39c053fff0df13f587cbde9c2e6e7
> --- /dev/null
> +static int xgmac_set_full_duplex(struct device *dev)
> +{
> + struct xgmac_priv *xgmac = dev_get_priv(dev);
The edev to priv conversion is inconsistent in the driver.
Sometimes it's done like:
struct xgmac_priv *xgmac = dev_get_priv(edev->parent);
Other places have:
struct xgmac_priv *xgmac = edev->priv;
Please add a
static inline to_xgmac(struct eth_device *edev)
{
return container_of(edev, struct xgmac_priv, netdev);
}
and use it everywhere.
Also, the functions purely internal to the driver should take a
struct xgmac_priv * directly instead of doing the edev to priv
conversion multiple times.
> +static void xgmac_adjust_link(struct eth_device *edev)
> +{
> + struct device *dev = edev->parent;
> + bool en_calibration;
en_calibration is set but not used.
> + int ret;
> +
> + if (edev->phydev->duplex)
> + ret = xgmac_set_full_duplex(dev);
> + else
> + ret = xgmac_set_half_duplex(dev);
just return void from these functions. They do not fail.
> + if (ret < 0) {
> + dev_err(dev, "xgmac_set_*_duplex() failed: %d\n", ret);
> + }
> +
> + switch (edev->phydev->speed) {
> + case SPEED_1000:
> + en_calibration = true;
> + ret = xgmac_set_gmii_speed(dev);
> + break;
> + case SPEED_100:
> + en_calibration = true;
> + ret = xgmac_set_mii_speed_100(dev);
> + break;
> + case SPEED_10:
> + en_calibration = false;
> + ret = xgmac_set_mii_speed_10(dev);
> + break;
> + default:
> + dev_err(dev, "invalid speed %d\n", edev->phydev->speed);
> + }
> + if (ret < 0)
> + dev_err(dev, "xgmac_set_*mii_speed*() failed: %d\n", ret);
> +}
> +
> +static int xgmac_start(struct eth_device *edev)
> +{
> + struct xgmac_priv *xgmac = edev->priv;
> + int ret, i;
> + u32 val, tx_fifo_sz, rx_fifo_sz, tqs, rqs, pbl;
> + ulong last_rx_desc;
> + ulong desc_pad;
> + u32 idle;
> +
> + ret = phy_device_connect(edev, &xgmac->miibus, xgmac->phy_addr,
> + xgmac_adjust_link, 0, xgmac->interface);
xgmac->phy_addr is set to 0 during init and used only here. Replace with
-1 to default to scan the bus. Usually the address argument is unused
in phy_device_connect() when the phy is specified in the device tree.
When it's not, then -1 is better than hardcoding phy0.
> +static void xgmac_stop(struct eth_device *edev)
> +{
> + struct xgmac_priv *xgmac = edev->priv;
> + u64 start_time;
> + u32 val;
> + u32 trcsts;
> + u32 txqsts;
> + u32 prxq;
> + u32 rxqsts;
> +
> + debug("%s(dev=%p):\n", __func__, edev);
dev_dbg()
> +static int xgmac_send(struct eth_device *edev, void *packet, int length)
> +{
> + struct xgmac_priv *xgmac = edev->priv;
> + struct xgmac_desc *tx_desc;
> + dma_addr_t dma;
> + u32 des3_prev, des3;
> + int ret;
> +
> + tx_desc = &xgmac->tx_descs[xgmac->tx_desc_idx];
> + xgmac->tx_desc_idx++;
> + xgmac->tx_desc_idx %= XGMAC_DESCRIPTORS_NUM;
> +
> + dma = dma_map_single(edev->parent, packet, length, DMA_TO_DEVICE);
> + if (dma_mapping_error(edev->parent, dma))
> + return -EFAULT;
> +
> + tx_desc->des0 = lower_32_bits(dma);
> + tx_desc->des1 = upper_32_bits(dma);
> + tx_desc->des2 = length;
> + /*
> + * Make sure that if HW sees the _OWN write below, it will see all the
> + * writes to the rest of the descriptor too.
> + */
> + barrier();
> +
> + des3_prev = XGMAC_DESC3_OWN | XGMAC_DESC3_FD | XGMAC_DESC3_LD | length;
> + writel(des3_prev, &tx_desc->des3);
> + writel((ulong)(tx_desc + 1), &xgmac->dma_regs->ch0_txdesc_tail_pointer); // <-- TODO
> +
> + ret = readl_poll_timeout(&tx_desc->des3, des3,
> + !(des3 & XGMAC_DESC3_OWN),
> + 100 * USEC_PER_MSEC);
> +
> + dma_unmap_single(edev->parent, dma, length, DMA_TO_DEVICE);
> +
> + if (ret == -ETIMEDOUT)
> + debug("%s: TX timeout 0x%08x\n", __func__, des3);
dev_dbg()
> +
> + return ret;
> +}
> +
> +static void xgmac_recv(struct eth_device *edev)
> +{
> + struct xgmac_priv *xgmac = edev->priv;
> + struct xgmac_desc *rx_desc;
> + dma_addr_t dma;
> + void *pkt;
> + int length;
> +
> + rx_desc = &xgmac->rx_descs[xgmac->rx_desc_idx];
> +
> + if (rx_desc->des3 & XGMAC_DESC3_OWN)
> + return;
> +
> + dma = xgmac->dma_rx_buf_phys[xgmac->rx_desc_idx];
> + pkt = xgmac->dma_rx_buf_virt[xgmac->rx_desc_idx];
> + length = rx_desc->des3 & XGMAC_RDES3_PKT_LENGTH_MASK;
> +
> + dma_sync_single_for_cpu(edev->parent, dma, length, DMA_FROM_DEVICE);
> + net_receive(edev, pkt, length);
> + dma_sync_single_for_device(edev->parent, dma, length, DMA_FROM_DEVICE);
> +
> + /* Read Format RX descriptor */
> + rx_desc = &xgmac->rx_descs[xgmac->rx_desc_idx];
> + rx_desc->des0 = dma;
This is a 32bit variable whereas the dma address potentially has 64bit.
Does des1 take the upper 32bit? If yes you should add that, otherwise
a warning would be good.
> + rx_desc->des1 = 0;
> + rx_desc->des2 = 0;
> + /*
> + * Make sure that if HW sees the _OWN write below, it will see all the
> + * writes to the rest of the descriptor too.
> + */
> + rx_desc->des3 = XGMAC_DESC3_OWN;
> + barrier();
> +
> + writel(xgmac->rx_descs_phys + (xgmac->rx_desc_idx * XGMAC_DESCRIPTOR_SIZE),
> + &xgmac->dma_regs->ch0_rxdesc_tail_pointer);
> +
> + xgmac->rx_desc_idx++;
> + xgmac->rx_desc_idx %= XGMAC_DESCRIPTORS_NUM;
> +}
> +
> +static void xgmac_probe_dt(struct device *dev, struct xgmac_priv *xgmac)
> +{
> + struct device_node *child;
> + int phy_reset;
> + int ret;
> +
> + xgmac->interface = of_get_phy_mode(dev->of_node);
> + xgmac->phy_addr = 0;
> +
> + /* Set MDIO bus device node, if present. */
> + for_each_child_of_node(dev->of_node, child) {
> + if (of_device_is_compatible(child, "snps,dwmac-mdio") ||
> + (child->name && !of_node_cmp(child->name, "mdio"))) {
> + xgmac->miibus.dev.of_node = child;
> + break;
> + }
> + }
> +
> + phy_reset = of_get_named_gpio(dev->of_node, "phy-reset-gpios", 0);
> + if (gpio_is_valid(phy_reset)) {
> + ret = gpio_request(phy_reset, "phy-reset");
> + if (ret)
> + return;
> +
> + xgmac->phy_reset_gpio = phy_reset;
> + }
The phy reset GPIO is optional, but used unconditionally. You'll end up
toggling GPIO 0 when no reset GPIO is given.
Also you could switch to the gpiod API.
> +
> +}
> +
> +int xgmac_probe(struct device *dev)
> +{
> + struct device_node *np = dev->of_node;
> + struct mii_bus *miibus;
> + struct xgmac_priv *xgmac;
> + struct resource *iores;
> + struct eth_device *edev;
> + int ret = 0;
> +
> + xgmac = xzalloc(sizeof(*xgmac));
> +
> + xgmac->dev = dev;
> + xgmac->config = device_get_match_data(dev);
> + if (ret < 0) {
if (!xgmac->config)
> + dev_err(dev, "xgmac_probe() failed to get driver data: %d\n", ret);
> + return ret;
> + }
> +
> + iores = dev_request_mem_resource(dev, 0);
> + if (IS_ERR(iores))
> + return PTR_ERR(iores);
> + xgmac->regs = IOMEM(iores->start);
> +
> + xgmac->mac_regs = xgmac->regs + XGMAC_MAC_REGS_BASE;
> + xgmac->mtl_regs = xgmac->regs + XGMAC_MTL_REGS_BASE;
> + xgmac->dma_regs = xgmac->regs + XGMAC_DMA_REGS_BASE;
> +
> + xgmac_probe_dt(dev, xgmac);
> +
> + edev = &xgmac->netdev;
> + dev->priv = edev->priv = xgmac;
> +
> + edev->parent = dev;
> + edev->open = xgmac_start;
> + edev->send = xgmac_send;
> + edev->halt = xgmac_stop;
> + edev->recv = xgmac_recv;
> + edev->get_ethaddr = xgmac_read_rom_hwaddr;
> + edev->set_ethaddr = xgmac_write_hwaddr;
> +
> + of_property_read_u32(np, "max-speed", &xgmac->max_speed);
This is unused in the driver.
> diff --git a/drivers/net/designware_xgmac_socfpga.c b/drivers/net/designware_xgmac_socfpga.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..7d8dba3c17cdb3fe7c82a6fa9315fecfdbfcc32a
> --- /dev/null
> +++ b/drivers/net/designware_xgmac_socfpga.c
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2023, Intel Corporation
> + */
> +#define DEBUG
Drop please
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
