Re: [net-next,v7,3/3] net: stmmac: Add glue layer for Spacemit K3 SoC

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: stmmac: Add glue layer for Spacemit K3 SoC

The ethernet controller on Spacemit K3 SoC is Synopsys DesignWare
MAC (version 5.40a), with the following special points:
1. The rate of the tx clock line is auto changed when the mac speed
   rate is changed, and no need for changing the input tx clock.
2. This controller require a extra syscon device to configure the
   interface type, enable wake up interrupt and delay configuration
   if needed.

Add Spacemit dwmac driver support on the Spacemit K3 SoC.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-spacemit.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-spacemit.c
> new file mode 100644
> index 000000000000..22aa25a7417b
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-spacemit.c
> @@ -0,0 +1,228 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Spacemit DWMAC platform driver
> + *
> + * Copyright (C) 2026 Inochi Amaoto <inochiama@gmail.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/math.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +
> +#include "stmmac_platform.h"

[ ... ]

> +struct spacmit_dwmac {
> +	struct regmap *apmu;
> +	unsigned int ctrl_offset;
> +	unsigned int dline_offset;
> +};

This isn't a bug, but the struct name is 'spacmit_dwmac' (missing 'e'),
while the driver filename is 'dwmac-spacemit.c' and all function names use
'spacemit_*' (e.g., spacemit_dwmac_probe, spacemit_dwmac_set_delay). Should
the struct be named 'spacemit_dwmac' for consistency?

[ ... ]

> +static int spacemit_dwmac_fix_delay(struct spacmit_dwmac *dwmac,
> +				    struct plat_stmmacenet_data *plat_dat,
> +				    unsigned int tx_delay,
> +				    unsigned int rx_delay)
> +{
> +	int rx_code;
> +	int tx_code;
> +
> +	rx_code = spacemit_dwmac_detected_delay_value(rx_delay);
> +	if (rx_code < 0)
> +		return rx_code;
> +
> +	tx_code = spacemit_dwmac_detected_delay_value(tx_delay);
> +	if (tx_code < 0)
> +		return tx_code;
> +
> +	return spacemit_dwmac_set_delay(dwmac, tx_code, rx_code);
> +}

The 'plat_dat' parameter appears to be unused in spacemit_dwmac_fix_delay().
The function only uses 'dwmac', 'tx_delay', and 'rx_delay'. Is this
parameter needed?

[ ... ]