Re: [PATCH net-next 5/5] net: stmmac: support the TC9564 SoC

[Mohd Ayaan Anwar <mohd.anwar@oss.qualcomm.com>]

Hi Alex,

On Sun, Oct 18, 2026 at 10:01:28PM -0500, Alex Elder wrote:
> From: Daniel Thompson <daniel@riscstar.com>
> 
> Add support for the Ethernet XGMAC functionality present in the
> Toshiba TC9564 SoC.
> 
> The TC9564 is a fairly complex chip that attaches to a host via
> the upstream port of a PCIe switch it implements.  That switch
> has three downstream ports, and one of those has an embedded PCI
> endpoint.  The PCI endpoint implements two PCI functions, and a
> 10 Gbps-capable XGMAC interface is attached to each.
> 
> The TC9564 implements additional peripherals, and access to them
> is provided by a PCI endpoint bus associated with BAR 4 of the
> first PCI function.  A PCI driver sets up the PCI function for
> use, and peripherals using the bus use platform drivers, matched
> by compatible string.
> 
> We use the pci-ep-bus approach for the stmmac driver as well, with
> each PCI function exposing access to its BAR 4 via an endpoint bus.
> The XGMAC device is defined within a pci-ep-bus node, and is bound
> to the platform driver based on its compatible string.
> 
> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
> Co-developed-by: Alex Elder <elder@riscstar.com>
> Signed-off-by: Alex Elder <elder@riscstar.com>
> ---
> ...
> +/* Fields and values for the EMACTL registers */
> +#define EMAC_SP_SEL_MASK		GENMASK(3, 0)
> +#define SP_SEL_2500BASEX		4
> +#define SP_SEL_SGMII_1000M		5
> +#define SP_SEL_SGMII_100M		6
> +#define SP_SEL_SGMII_10M		7
> +#define EMAC_PHY_INF_SEL_MASK		GENMASK(5, 4)
> +#define PCS_CLK_PHY			1	/* Clock from PHY */
> +#define EMAC_INV_SGM_SIG_DET		BIT(6)	/* 1 = polarity inverted */
> +#define EMAC_LPIHWCLKEN			BIT(8)	/* 1 = low power mode */
> +#define EMAC_INIT_DONE			BIT(21)
> +

Nit: extra tab before BIT(8) and BIT(21).

> +
> +#define PMA_HWT_REFCK_R_EN			0x1080
> +#define PMA_HWT_REFCK_TERM_EN			0x1090
> +#define PMA_HWT_REFCK_STRIDE			0x0014
> +
> +#define PMA_COMM_CFG_0_1			0x1888
> +#define PMA_COMM_CFG_0_1_STRIDE			0x0008

Nit: extra tab before 0x1090 and 0x0008.

> +
> +static int tc9564_probe_config_dt(struct tc9564_data *td)
> +{
> +	struct platform_device *pdev = to_platform_device(td->dev);
> +	struct device_node *np = td->dev->of_node;
> +	struct plat_stmmacenet_data *plat;
> +	phy_interface_t phy_interface;
> +	struct device *dev = td->dev;
> +	int ret;
> +
> +	ret = device_get_phy_mode(dev);
> +	if (ret < 0)
> +		return -ENODEV;
> +	phy_interface = ret;
> +

Nit: I think the call to devm_stmmac_probe_config_dt() will get the PHY
mode from firmware and store it in plat_dat->phy_interface so this is
probably redundant.

> +	/* The platform structure is allocated with devm_kzalloc() */
> +	plat = devm_stmmac_probe_config_dt(pdev, td->res.mac);
> +	if (IS_ERR(plat))
> +		return PTR_ERR(plat);
> +


> +static int tc9564_dwmac_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct tc9564_data *td;
> +	struct device_node *np;
> +	struct regmap *regmap;
> +	int ret;
> +
> +	td = devm_kzalloc(dev, sizeof(*td), GFP_KERNEL);
> +	if (!td)
> +		return -ENOMEM;
> +
> +	td->dev = dev;
> +
> +	np = dev_of_node(dev);
> +	regmap = syscon_regmap_lookup_by_phandle_args(np,
> +						      "toshiba,config-syscon",
> +						      1, &td->emac_ctl_offset);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap),
> +				     "failed to get config regmap\n");
> +	td->config_regmap = regmap;
> +
> +	ret = tc9564_reset_init(td);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to initialize resets\n");
> +
> +	ret = tc9564_clock_init(td);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to initialize clocks\n");
> +
> +	ret = tc9564_probe_config_dt(td);
> +	if (ret)
> +		return ret;
> +
> +	ret = stmmac_get_platform_resources(pdev, &td->res);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to initialize stmmac resources\n");
> +
> +	tc9564_mac_enable(td);
> +
> +	ret = stmmac_dvr_probe(dev, td->plat, &td->res);
> +	if (ret) {
> +		ret = dev_err_probe(dev, ret, "failed stmmac probe\n");
> +		goto err_disable_mac;
> +	}

Nit: Could this use devm_stmmac_pltfr_probe() and drop
tc9564_dwmac_remove()?
tc9564_mac_disable() could be registered as the priv->plat->exit
callback.

> +
> +	return 0;
> +
> +err_disable_mac:
> +	tc9564_mac_disable(td);
> +
> +	return ret;
> +}
> +

	Ayaan