Re: [PATCH net-next v6 3/7] net: ftgmac100: Add AST2700 compatible support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] net: ftgmac100: missing MAC reset deassertion for AST2700 in
  ftgmac100_probe()
--

commit a3986ee942e4f49e58a12de29523571685a498c7
Author: Jacky Chou <jacky_chou@aspeedtech.com>

net: ftgmac100: Add AST2700 compatible support

This commit adds match data and an OF match table entry for the Aspeed
AST2700 FTGMAC100 Ethernet controller.

> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 6d2fe5c2f3903..67b1fa464a421 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c

[ ... ]

> @@ -2017,7 +2018,8 @@ static int ftgmac100_probe(struct platform_device *pdev)
>  
>  	if (priv->mac_id == FTGMAC100_AST2400 ||
>  	    priv->mac_id == FTGMAC100_AST2500 ||
> -	    priv->mac_id == FTGMAC100_AST2600) {
> +	    priv->mac_id == FTGMAC100_AST2600 ||
> +	    priv->mac_id == FTGMAC100_AST2700) {
>  		priv->rxdes0_edorr_mask = BIT(30);
>  		priv->txdes0_edotr_mask = BIT(30);
>  		priv->is_aspeed = true;

[Severity: High]
Does ftgmac100_probe() leave the MAC held in reset on AST2700, and read MAC
registers before the clock and reset are enabled?

On AST2400, AST2500 and AST2600, enabling the MAC clock gate in the clock
driver automatically deasserts the SCU MAC reset.  On AST2700,
SCU1_CLK_GATE_MAC0CLK..2 only clear SCU1_CLK_STOP, while SCU1_RESET_MAC0..2
are separate reset controls.

ftgmac100_probe() never calls reset_control_deassert(priv->rst), and
ftgmac100_reset_and_config_mac() only deasserts priv->rst when
phydev->interface is PHY_INTERFACE_MODE_RMII:

drivers/net/ethernet/faraday/ftgmac100.c:ftgmac100_reset_and_config_mac() {
	...
	/* Aspeed RMII needs SCU reset to clear status */
	if (priv->is_aspeed && priv->netdev->phydev->interface == PHY_INTERFACE_MODE_RMII) {
		...
		err = reset_control_deassert(priv->rst);
	...
}

When the bootloader has not released the MAC reset or enabled the clock,
RGMII interfaces remain in hardware reset and ftgmac100_reset_mac() fails
with -EIO.  In addition, ftgmac100_initial_mac() runs just above this hunk
and reads FTGMAC100_OFFSET_MAC_MADR and FTGMAC100_OFFSET_MAC_LADR before
ftgmac100_setup_clk() enables priv->clk and before priv->rst is acquired
and deasserted.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-support_ast2700-v6-0-238020bf46cf@aspeedtech.com?part=3