Re: [PATCH net-next] net: phy: add basic driver for NXP CBTX PHY

[Vladimir Oltean <vladimir.oltean@nxp.com>]

Hi Heiner,

On Tue, Apr 11, 2023 at 06:49:42PM +0200, Heiner Kallweit wrote:
> > +	return phy_set_bits(phydev, CBTX_PDOWN_CTRL,
> > +			    CBTX_PDOWN_CTL_TRUE_PDOWN);
> 
> A comment may be helpful explaining how true_pdown mode
> is different from power-down mode set by C22 standard
> bit in BMCR as part of genphy_suspend().

The NXP documentation for True Power Down vs General Power Down did not
convince me, and I don't want to speculate and give an answer that might
be incorrect.

There are not many people who can help me give an answer right now
during the holidays. The high level idea is that the PHY may enter a
mode of lower power consumption.

If it's acceptable to you, I can implement suspend and resume as direct
calls to genphy_suspend() and genphy_resume(), and make the change later,
if needed.

> > +static int cbtx_config_intr(struct phy_device *phydev)
> > +{
> > +	int ret;
> > +
> > +	if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
> > +		ret = cbtx_ack_interrupts(phydev);
> > +		if (ret < 0)
> > +			return ret;
> > +
> > +		ret = phy_write(phydev, CBTX_IRQ_ENABLE,
> > +				CBTX_IRQ_AN_COMPLETE | CBTX_IRQ_LINK_DOWN);
> 
> I think you need also CBTX_IRQ_ENERGYON. Otherwise you won't get a
> link-up interrupt in forced mode.

I've tested just now with "ethtool -s p1 autoneg off speed 100 duplex full",
and you are exactly correct. I will add the media side energy detection logic
to the enabled IRQ sources in v2. Thanks.