Re: [PATCH net-next v2 3/3] net: phy: mediatek: add driver for built-in 2.5G ethernet PHY on MT7988

["SkyLake Huang (黃啟澤)" <SkyLake.Huang@mediatek.com>]

On Wed, 2025-02-26 at 14:32 +0100, Andrew Lunn wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On Wed, Feb 26, 2025 at 06:48:34AM +0000, SkyLake Huang (黃啟澤) wrote:
> > On Wed, 2025-02-19 at 09:33 +0000, Russell King (Oracle) wrote:
> > > 
> > > External email : Please do not click links or open attachments
> > > until
> > > you have verified the sender or the content.
> > > 
> > > 
> > > On Wed, Feb 19, 2025 at 04:39:10PM +0800, Sky Huang wrote:
> > > > +static int mt798x_2p5ge_phy_config_init(struct phy_device
> > > > *phydev)
> > > > +{
> > > > +     struct pinctrl *pinctrl;
> > > > +     int ret;
> > > > +
> > > > +     /* Check if PHY interface type is compatible */
> > > > +     if (phydev->interface != PHY_INTERFACE_MODE_INTERNAL)
> > > > +             return -ENODEV;
> > > > +
> > > > +     ret = mt798x_2p5ge_phy_load_fw(phydev);
> > > > +     if (ret < 0)
> > > > +             return ret;
> > > 
> > > Firmware should not be loaded in the .config_init method. The
> > > above
> > > call will block while holding the RTNL which will prevent all
> > > other
> > > network configuration until the firmware has been loaded or the
> > > load
> > > fails.
> > > 
> > > Thanks.
> > > 
> > > --
> > > RMK's Patch system:
> > > https://urldefense.com/v3/__https://www.armlinux.org.uk/developer/patches/__;!!CTRNKA9wMg0ARbw!iV-1ViPFsUV-lLj7aIycan8nery6sQO3t6mkpdlb_GW8hswhxc4ejJozxqkU3s2WzxSizs4kfdC77yr7HGGRIuU$
> > > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> > Hi Russell,
> > mt798x_p5ge_phy_load_fw() will only load firmware once after driver
> > is
> > probed through priv->fw_loaded. And actually, firmware loading
> > procedure only takes about 11ms. This was discussed earlier in:
> > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20240520113456.21675-6-SkyLake.Huang@mediatek.com/*25856462__;Iw!!CTRNKA9wMg0ARbw!nEJAqWq9eSPyvD4sikg0DKqNCU5OPGCJx42J6muU1dWrHsNliA4BR1OU7r_XBf53OC02GphE1odIIYTT6687$
> > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20240520113456.21675-6-SkyLake.Huang@mediatek.com/*25857174__;Iw!!CTRNKA9wMg0ARbw!nEJAqWq9eSPyvD4sikg0DKqNCU5OPGCJx42J6muU1dWrHsNliA4BR1OU7r_XBf53OC02GphE1odIIc2tO_Yi$
> 
> Ideally, all PHY drivers should look like each other. That makes
> maintenance simpler. Currently, air_en8811h.c, aquantia_main.c, and
> qt2025.rs load firmware in there probe function. Is there a good
> reason this driver needs to be different?
> 
>        Andrew
  Actually, I wrote fw loading flow in .probe at first. However, during
boot time, .probe is called at very early stage (about the first 2s
after booting into Linux Kernel). At that time, filesystem isn't ready
yet and phy driver can't locate /lib/firmware/mediatek/mt7988/i2p5ge-
phy-pmb.bin. Also, adding "-EPROBE_DEFER" doesn't help.
  I'm not sure how aquantia and qt2025 drivers handle this.

BRs,
Sky