Re: [PATCH net-next v5 09/10] net: stmmac: mediatek: add support for TX clock output enable feature

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] mt8195_set_interface()
  (drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c) now sets…

--

net: stmmac: mediatek: add support for TX clock output enable feature

This commit adds a definition for the MT8189 TX clock output enable bit in
the pericfg Ethernet control 0 register. It also adds a mac_txclk_out_en
flag to the variant data and sets the bit in mt8195_set_interface() when
the flag is true.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> index 3f024d455cbd5..1a0ab95dfbf0e 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c

[ ... ]

> @@ -307,6 +311,9 @@ static int mt8195_set_interface(struct mediatek_dwmac_plat_data *plat,
>  	/* MT8195 only support external PHY */
>  	intf_val |= MT8195_EXT_PHY_MODE;
>  
> +	if (plat->variant->mac_txclk_out_en)
> +		intf_val |= MT8189_CTRL0_TXC_OUT_OP_EN;
> +

[Severity: Medium]
Should this bit depend on the interface mode? As written,
MT8189_CTRL0_TXC_OUT_OP_EN is set no matter what phy_intf_sel,
plat->rmii_clk_from_mac or plat->rmii_rxc are.

Later in the series, "net: stmmac: mediatek: add support for MT8189 SoC"
sets .mac_txclk_out_en = true for mt8189_gmac_variant. That means the TX
clock output gets enabled in every mode.

The MT8189 binding forbids the MAC from sourcing the RMII reference clock:

  mediatek,rmii-clk-from-mac: false

It still allows mediatek,rmii-rxc, and it does not limit phy-mode. So with
phy-mode = "rmii" and no mediatek,rmii-rxc, the external reference clock
comes in on the TXC pin, as mt8195_set_delay() describes:

    /* the rmii reference clock from outside is connected
     * to TXC pin, the reference clock will be adjusted
     * by TXC delay macro circuit.
     */

mt8195_set_delay() also accepts PHY_INTERFACE_MODE_MII. In MII mode the
PHY drives TX_CLK.

In either of those setups, would the MAC's TXC output driver be turned on
while the PHY or an oscillator is already driving that pin?

If the hardware ignores bit 20 when TXC is used as an input, could the
commit message say so? If it doesn't, would it be safer to set the bit only
for the modes where the MAC drives TXC, such as RGMII?

>  	regmap_write(plat->peri_regmap,
>  		     reg_offset + MT8195_PERI_ETH_CTRL0,
>  		     intf_val);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110-dwmac-mediatek-mt8189-v5-0-7d99d6292962%40collabora.com