[RESEND PATCH v2 5/5] net: dwc_eth_qos: Add glue driver for Intel MAC

[Philip Oberfichtner]

Hi Marek,

On Sun, Jun 30, 2024 at 07:38:43AM +0200, Marek Vasut wrote:
> On 6/24/24 10:34 AM, Philip Oberfichtner wrote:
> 
> > +++ b/drivers/net/dwc_eth_qos_intel.c
> > @@ -0,0 +1,446 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2023 DENX Software Engineering GmbH
> > + * Philip Oberfichtner <pro at denx.de>
> > + *
> > + * Based on linux v6.1.38, especially drivers/net/ethernet/stmicro/stmmac
> 
> It is 2024 now , also you might want to update this to match Linux 6.6.y
> which is the current LTS .
> 

Ack for V3.

> > + */
> > +
> > +#include <asm/io.h>
> > +#include <dm.h>
> > +#include <linux/delay.h>
> > +#include <miiphy.h>
> > +#include <net.h>
> > +#include <pci.h>
> > +
> > +#include "dwc_eth_qos.h"
> > +#include "dwc_eth_qos_intel.h"
> > +
> > +static struct pci_device_id intel_pci_ids[] = {
> > +	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_EHL_RGMII1G) },
> > +	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_EHL_SGMII1) },
> > +	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_EHL_SGMII2G5) },
> > +	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_EHL_PSE0_RGMII1G) },
> > +	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_EHL_PSE0_SGMII1G) },
> > +	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_EHL_PSE0_SGMII2G5) },
> > +	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_EHL_PSE1_RGMII1G) },
> > +	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_EHL_PSE1_SGMII1G) },
> > +	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_EHL_PSE1_SGMII2G5) },
> > +	{}
> > +};
> > +
> > +static int __pci_config(struct udevice *dev)
> > +{
> > +	u32 val;
> > +
> > +	/* Try to enable I/O accesses and bus-mastering */
> > +	dm_pci_read_config32(dev, PCI_COMMAND, &val);
> > +	val |= PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER;
> > +	dm_pci_write_config32(dev, PCI_COMMAND, val);
> > +
> > +	/* Make sure it worked */
> > +	dm_pci_read_config32(dev, PCI_COMMAND, &val);
> > +	if (!(val & PCI_COMMAND_MEMORY)) {
> > +		pr_err("%s: Can't enable I/O memory\n", __func__);
> 
> dev_err() please
> 
> > +		return -ENOSPC;
> > +	}
> > +
> > +	if (!(val & PCI_COMMAND_MASTER)) {
> > +		pr_err("%s: Can't enable bus-mastering\n", __func__);
> 
> dev_err() , fix globally.
> 

Ack for V3.

> > +		return -EPERM;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void __limit_fifo_size(struct udevice *dev)
> 
> Please drop the leading __ from function names .
> 

Ack for V3.

> > +{
> > +	/*
> > +	 * As described in Intel Erratum EHL22, Document Number: 636674-2.1,
> > +	 * the PSE GbE Controllers advertise a wrong RX and TX fifo size.
> > +	 * Software should limit this value to 64KB.
> > +	 */
> > +	struct eqos_priv *eqos = dev_get_priv(dev);
> > +
> > +	eqos->tx_fifo_sz = 0x8000;
> > +	eqos->rx_fifo_sz = 0x8000;
> > +}
> > +
> > +static int __serdes_status_poll(struct udevice *dev,
> > +				unsigned char phyaddr, unsigned char phyreg,
> > +				unsigned short mask, unsigned short val)
> > +{
> > +	struct eqos_priv *eqos = dev_get_priv(dev);
> > +	unsigned int retries = 10;
> > +	unsigned short val_rd;
> > +
> > +	do {
> > +		miiphy_read(eqos->mii->name, phyaddr, phyreg, &val_rd);
> > +		if ((val_rd & mask) == (val & mask))
> > +			return 0;
> > +		udelay(POLL_DELAY_US);
> > +	} while (--retries);
> 
> Is this some implementation of phy_read_mmd_poll_timeout() ?
> 

Well ... sort of, but see below.


> > +	return -ETIMEDOUT;
> > +}
> > +
> > + /* Returns -ve if MAC is unknown and 0 on success */
> > +static int __mac_check_pse(const struct udevice *dev, bool *is_pse)
> > +{
> > +	struct pci_child_plat *plat = dev_get_parent_plat(dev);
> > +
> > +	if (!plat || plat->vendor != PCI_VENDOR_ID_INTEL)
> > +		return -ENXIO;
> > +
> > +	switch (plat->device) {
> > +	case PCI_DEVICE_ID_INTEL_EHL_PSE0_RGMII1G:
> > +	case PCI_DEVICE_ID_INTEL_EHL_PSE1_RGMII1G:
> > +	case PCI_DEVICE_ID_INTEL_EHL_PSE0_SGMII1G:
> > +	case PCI_DEVICE_ID_INTEL_EHL_PSE1_SGMII1G:
> > +	case PCI_DEVICE_ID_INTEL_EHL_PSE0_SGMII2G5:
> > +	case PCI_DEVICE_ID_INTEL_EHL_PSE1_SGMII2G5:
> > +		*is_pse = 1;
> > +		return 0;
> > +
> > +	case PCI_DEVICE_ID_INTEL_EHL_RGMII1G:
> > +	case PCI_DEVICE_ID_INTEL_EHL_SGMII1:
> > +	case PCI_DEVICE_ID_INTEL_EHL_SGMII2G5:
> > +		*is_pse = 0;
> > +		return 0;
> > +	};
> > +
> > +	return -ENXIO;
> > +}
> > +
> > +/* Check if we're in 2G5 mode */
> > +static bool __serdes_link_mode_2500(struct udevice *dev)
> > +{
> > +	const unsigned char phyad = INTEL_MGBE_ADHOC_ADDR;
> > +	struct eqos_priv *eqos = dev_get_priv(dev);
> > +	unsigned short data;
> > +
> > +	miiphy_read(eqos->mii->name, phyad, SERDES_GCR, &data);
> > +	if (((data & SERDES_LINK_MODE_MASK) >> SERDES_LINK_MODE_SHIFT) ==
> > +	    SERDES_LINK_MODE_2G5)
> 
> Use FIELD_PREP() and FIELD_GET() to simplify this bitfield extraction.
> 

Ack for V3.

> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> > +static int __serdes_powerup(struct udevice *dev)
> > +{
> > +	/* Based on linux/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c */
> > +
> > +	const unsigned char phyad = INTEL_MGBE_ADHOC_ADDR;
> > +	struct eqos_priv *eqos = dev_get_priv(dev);
> > +	unsigned short data;
> > +	int ret;
> > +	bool is_pse;
> > +
> > +	/* Set the serdes rate and the PCLK rate */
> > +	miiphy_read(eqos->mii->name, phyad, SERDES_GCR0, &data);
> > +
> > +	data &= ~SERDES_RATE_MASK;
> > +	data &= ~SERDES_PCLK_MASK;
> > +
> > +	if (__serdes_link_mode_2500(dev))
> > +		data |= SERDES_RATE_PCIE_GEN2 << SERDES_RATE_PCIE_SHIFT |
> > +			SERDES_PCLK_37p5MHZ << SERDES_PCLK_SHIFT;
> > +	else
> > +		data |= SERDES_RATE_PCIE_GEN1 << SERDES_RATE_PCIE_SHIFT |
> > +			SERDES_PCLK_70MHZ << SERDES_PCLK_SHIFT;
> > +
> > +	miiphy_write(eqos->mii->name, phyad, SERDES_GCR0, data);
> > +
> > +	/* assert clk_req */
> > +	miiphy_read(eqos->mii->name, phyad, SERDES_GCR0, &data);
> > +	data |= SERDES_PLL_CLK;
> > +	miiphy_write(eqos->mii->name, phyad, SERDES_GCR0, data);
> 
> Could this use phy_modify() ?
> 


I do agree using the phy_read()-API all over the place would be nice.
But I currently cannot do so, as I do not have a struct phy_device.
I am not accessing an actual PHY here, but a MAC-internal submodule.

Please also refer to my comment in the xpcs_access() function below,
which suffers from the same problem.

Any advice on how to make this integrate more smoothly with the standard
API is very welcome.

Best regards,
Philip


> [...]

-- 
=====================================================================
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-22  Fax: +49-8142-66989-80   Email: pro at denx.de
=====================================================================