Re: [PATCH net-next] net: phy: add driver for the ZTE ZX279128S internal Gigabit PHY

[Agustin]

Thanks for the review.

Before sending a v2 I went back to the bench, and the result changes the 
picture, with the generic PHY driver and only what the boot firmware 
sets up, the four LAN PHYs link and pass traffic at 1000, 100 and 10 
Mb/s with no errors, across cold and warm boots and repeated renegotiation.

The TX DAC registers already hold the values this driver writes, and the 
PHY adjusts them itself with the link speed. It turned out to be a bug 
in our MAC driver (frames were sent to the wrong egress port), not 
anything in the PHY. That leaves only an LDO level and one undocumented 
register, with no measured benefit, so I'm withdrawing this patch. This 
is also consistent with Navid's ZX279128S series, which uses the generic 
driver for these PHYs. If I find a case where the vendor settings make a 
difference, I'll come back with data.

Sorry for the noise, and thanks again.

On 11/5/26 15:07, Andrew Lunn wrote:
> > +static int zte_gephy_config_init(struct phy_device *phydev)
> > +{
> > +	int ret, i, ldo;
> Given how minimal this driver is, there are some potential traps we
> could fall into later when we find out more about the device. To
> reduce the risk a bit, please could you add a test here:
>
>         if (phydev->interface != PHY_INTERFACE_MODE_INTERNAL)
>           	  return -EINVAL;
>
> > +static struct phy_driver zte_gephy_driver[] = {
> > +	{
> > +		PHY_ID_MATCH_MODEL(ZTE_GEPHY_PHY_ID),
> > +		.name			= "ZTE ZX279128S Gigabit PHY",
> > +		.flags			= PHY_IS_INTERNAL,
> > +		.config_init		= zte_gephy_config_init,
> > +		/* The boot firmware hands over with BMCR_PDOWN set. No
> > +		 * .suspend: powering the analog domain down and up at runtime
> > +		 * has not been tested.
> > +		 */
> > +		.resume			= genphy_resume,
> It looks odd with just resume, it makes it look like a workaround.
>
> Why not have config_init() take it out of power down?
>
>      Andrew