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

[Joel Stanley]

On Sat, 7 Aug 2021 at 19:19, Andrew Lunn <andrew@xxxxxxx> wrote:
>
> > +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.

>
> > +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.