[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZZPnaziDZEcv5GGw@shell.armlinux.org.uk>
Date: Tue, 2 Jan 2024 10:37:31 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Yanteng Si <siyanteng@...ngson.cn>
Cc: andrew@...n.ch, hkallweit1@...il.com, peppe.cavallaro@...com,
alexandre.torgue@...s.st.com, joabreu@...opsys.com,
fancer.lancer@...il.com, Jose.Abreu@...opsys.com,
chenhuacai@...ngson.cn, guyinggang@...ngson.cn,
netdev@...r.kernel.org, chris.chenfeiyang@...il.com
Subject: Re: [PATCH net-next v7 2/9] net: stmmac: dwmac-loongson: Refactor
code for loongson_dwmac_probe()
On Tue, Dec 19, 2023 at 10:17:05PM +0800, Yanteng Si wrote:
> Add a setup() function to initialize data, and simplify code for
> loongson_dwmac_probe().
Not all changes in this patch are described.
> +static int loongson_gmac_data(struct pci_dev *pdev,
> + struct plat_stmmacenet_data *plat)
> +{
> + loongson_default_data(pdev, plat);
> +
> + plat->multicast_filter_bins = 256;
> +
> + plat->mdio_bus_data->phy_mask = 0;
>
> - /* Default to phy auto-detection */
> plat->phy_addr = -1;
> + plat->phy_interface = PHY_INTERFACE_MODE_RGMII_ID;
This presumably sets the default phy_interface mode?
> - plat->bus_id = of_alias_get_id(np, "ethernet");
> - if (plat->bus_id < 0)
> - plat->bus_id = pci_dev_id(pdev);
> + pci_set_master(pdev);
> +
> + info = (struct stmmac_pci_info *)id->driver_data;
> + ret = info->setup(pdev, plat);
> + if (ret)
> + goto err_disable_device;
loongson_gmac_data() gets called from here...
> +
> + bus_id = of_alias_get_id(np, "ethernet");
> + if (bus_id >= 0)
> + plat->bus_id = bus_id;
>
> phy_mode = device_get_phy_mode(&pdev->dev);
> if (phy_mode < 0) {
This gets the PHY interface mode, and errors out if it can't be found in
firmware.
> @@ -110,11 +137,7 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id
> }
>
> plat->phy_interface = phy_mode;
So this ends up always overwriting the value written in
loongson_gmac_data(). So it seems to be that initialising
plat->phy_interface in loongson_gmac_data() is just patch noise and
serves no real purpose.
> - plat->mac_interface = PHY_INTERFACE_MODE_GMII;
This has now gone - and is not described, and I'm left wondering what
the implication of that is on the driver. It also makes me wonder
whether loongson_gmac_data() should've been setting mac_interface
rather than phy_interface.
> res.wol_irq = of_irq_get_byname(np, "eth_wake_irq");
> if (res.wol_irq < 0) {
> - dev_info(&pdev->dev, "IRQ eth_wake_irq not found, using macirq\n");
> + dev_info(&pdev->dev,
> + "IRQ eth_wake_irq not found, using macirq\n");
Whitespace cleanups should be a separate patch.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Powered by blists - more mailing lists