On Tue, Oct 29, 2024 at 09:42:36AM +0100, Steffen Trumtrar wrote: > +static int xgmac_mdio_read(struct mii_bus *bus, int mdio_addr, int mdio_reg) > +{ > + struct xgmac_priv *xgmac = bus->priv; > + u32 val; > + u32 hw_addr; > + u32 idle; > + int ret; > + > + ret = readl_poll_timeout(&xgmac->mac_regs->mdio_data, idle, > + !(idle & XGMAC_MAC_MDIO_ADDRESS_SBUSY), > + XGMAC_TIMEOUT_100MS); > + if (ret) { > + pr_err("MDIO not idle at entry: %d\n", ret); dev_err() please. Same for other pr_ style functions in this file. > + return ret; > + } > + > + /* Set clause 22 format */ > + val = BIT(mdio_addr); > + writel(val, &xgmac->mac_regs->mdio_clause_22_port); > + > + hw_addr = (mdio_addr << XGMAC_MAC_MDIO_ADDRESS_PA_SHIFT) | > + (mdio_reg & XGMAC_MAC_MDIO_REG_ADDR_C22P_MASK); > + > + val = xgmac->config->config_mac_mdio << > + XGMAC_MAC_MDIO_ADDRESS_CR_SHIFT; > + > + val |= XGMAC_MAC_MDIO_ADDRESS_SADDR | > + XGMAC_MDIO_SINGLE_CMD_ADDR_CMD_READ | > + XGMAC_MAC_MDIO_ADDRESS_SBUSY; > + > + ret = readl_poll_timeout(&xgmac->mac_regs->mdio_data, idle, > + !(idle & XGMAC_MAC_MDIO_ADDRESS_SBUSY), > + XGMAC_TIMEOUT_100MS); > + if (ret) { > + pr_err("MDIO not idle at entry: %d\n", ret); > + return ret; > + } > + > + writel(hw_addr, &xgmac->mac_regs->mdio_address); > + writel(val, &xgmac->mac_regs->mdio_data); > + > + ret = readl_poll_timeout(&xgmac->mac_regs->mdio_data, idle, > + !(idle & XGMAC_MAC_MDIO_ADDRESS_SBUSY), > + XGMAC_TIMEOUT_100MS); > + if (ret) { > + pr_err("MDIO read didn't complete: %d\n", ret); > + return ret; > + } > + > + val = readl(&xgmac->mac_regs->mdio_data); > + val &= XGMAC_MAC_MDIO_DATA_GD_MASK; > + > + return val; > +} > + > +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 = dma; lower_32_bits() > + tx_desc->des1 = 0; upper_32_bits() go here? > + 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); > + > + 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[xgmac->rx_desc_idx]; > + pkt = phys_to_virt(dma); You should store the virtual address somewhere in your private data and use it here rather than converting back the dma address. > + length = rx_desc->des3 & XGMAC_RDES3_PKT_LENGTH_MASK; > + > + dma_sync_single_for_cpu(edev->parent, (unsigned long)pkt, length, > + DMA_FROM_DEVICE); Use the dma address directly here. > + net_receive(edev, pkt, length); > + dma_sync_single_for_device(edev->parent, (unsigned long)pkt, > + length, DMA_FROM_DEVICE); > + > + /* Read Format RX descriptor */ > + rx_desc = &xgmac->rx_descs[xgmac->rx_desc_idx]; > + rx_desc->des0 = dma; > + 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((ulong)rx_desc, &xgmac->dma_regs->ch0_rxdesc_tail_pointer); calculate the address based on xgmac->rx_descs_phys rather than assuming a 1:1 mapping. > + > + xgmac->rx_desc_idx++; > + xgmac->rx_desc_idx %= XGMAC_DESCRIPTORS_NUM; > +} > + > +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; > + ret = dev_get_drvdata(dev, (const void **)&xgmac->config); Use device_get_match_data() instead. > + if (ret < 0) { > + pr_err("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 = (void *)(xgmac->regs + XGMAC_MAC_REGS_BASE); > + xgmac->mtl_regs = (void *)(xgmac->regs + XGMAC_MTL_REGS_BASE); > + xgmac->dma_regs = (void *)(xgmac->regs + XGMAC_DMA_REGS_BASE); xgmac->regs already is of type void *, no need to cast- > + > +/* DMA Registers */ > + > +#define XGMAC_DMA_REGS_BASE 0x3000 > + > +struct xgmac_dma_regs { > + u32 mode; /* 0x3000 */ > + u32 sysbus_mode; /* 0x3004 */ > + u32 unused_3008[(0x3100 - 0x3008) / 4]; /* 0x3008 */ > + u32 ch0_control; /* 0x3100 */ > + u32 ch0_tx_control; /* 0x3104 */ > + u32 ch0_rx_control; /* 0x3108 */ > + u32 slot_func_control_status; /* 0x310c */ > + u32 ch0_txdesc_list_haddress; /* 0x3110 */ > + u32 ch0_txdesc_list_address; /* 0x3114 */ > + u32 ch0_rxdesc_list_haddress; /* 0x3118 */ > + u32 ch0_rxdesc_list_address; /* 0x311c */ > + u32 unused_3120; /* 0x3120 */ Is this really unused or should it be the upper 32 bit of ch0_txdesc_tail_pointer? > + u32 ch0_txdesc_tail_pointer; /* 0x3124 */ > + u32 unused_3128; /* 0x3128 */ Same for ch0_rxdesc_tail_pointer. > + u32 ch0_rxdesc_tail_pointer; /* 0x312c */ > + u32 ch0_txdesc_ring_length; /* 0x3130 */ > + u32 ch0_rxdesc_ring_length; /* 0x3134 */ > + u32 unused_3138[(0x3160 - 0x3138) / 4]; /* 0x3138 */ > + u32 ch0_status; /* 0x3160 */ > +}; > + 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 |