Re: [PATCH 2/2] net: Add driver for LiteX's LiteETH network interface

On Mon, Aug 09, 2021 at 03:20:57AM +0000, Joel Stanley wrote:

On Sat, 7 Aug 2021 at 19:19, Andrew Lunn [off-list ref] wrote:

quoted

quoted

+static void liteeth_reset_hw(struct liteeth *priv)
+{
+     /* Reset, twice */
+     writeb(0, priv->base + LITEETH_PHY_CRG_RESET);
+     udelay(10);
+     writeb(1, priv->base + LITEETH_PHY_CRG_RESET);
+     udelay(10);
+     writeb(0, priv->base + LITEETH_PHY_CRG_RESET);
+     udelay(10);

What is this actually resetting?

This comes from the reference firmware that many (but not all) litex
systems run before loading their operating system.

I'm not completely sure how necessary it still is; I will drop it for now.

Which did not answer my question. Once we know what is being reset, we
can maybe suggest when/how it should be reset.

quoted

quoted

+static int liteeth_probe(struct platform_device *pdev)
+{
+     struct net_device *netdev;
+     void __iomem *buf_base;
+     struct resource *res;
+     struct liteeth *priv;
+     int irq, err;
+
+     netdev = alloc_etherdev(sizeof(*priv));
+     if (!netdev)
+             return -ENOMEM;
+
+     priv = netdev_priv(netdev);
+     priv->netdev = netdev;
+     priv->dev = &pdev->dev;
+
+     irq = platform_get_irq(pdev, 0);
+     if (irq < 0) {
+             dev_err(&pdev->dev, "Failed to get IRQ\n");
+             goto err;
+     }
+
+     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+     priv->base = devm_ioremap_resource(&pdev->dev, res);
+     if (IS_ERR(priv->base)) {
+             err = PTR_ERR(priv->base);
+             goto err;
+     }
+
+     res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+     priv->mdio_base = devm_ioremap_resource(&pdev->dev, res);
+     if (IS_ERR(priv->mdio_base)) {
+             err = PTR_ERR(priv->mdio_base);
+             goto err;
+     }

So you don't have any PHY handling, or any MDIO bus master code. So i
would drop this, until the MDIO architecture question is answered. I
also wonder how much use the MAC driver is without any PHY code?
Unless you have a good reason, i don't think we should merge this
until it makes the needed calls into phylib. It is not much code to
add.

You mean I should skip out the parsing of the mdio base until I'm
using it? That's reasonable.

It could be we insist you add MDIO and PHY handling. But first we need
to understand the architecture.

   Andrew