Re: [PATCH net-next v2 3/7] net: pcs: xpcs: add 1000BASE-X AN interrupt support

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Tue, Aug 08, 2023 at 10:17:04AM +0800, Jiawen Wu wrote:
> Enable CL37 AN complete interrupt for DW XPCS. It requires to clear the
> bit(0) [CL37_ANCMPLT_INTR] of VR_MII_AN_INTR_STS after AN completed.
> 
> And there is a quirk for Wangxun devices to enable CL37 AN in backplane
> configurations because of the special hardware design.

Where is the interrupt handler?

> @@ -759,6 +762,8 @@ static int xpcs_config_aneg_c37_1000basex(struct dw_xpcs *xpcs,
>  		return ret;
>  
>  	ret &= ~DW_VR_MII_PCS_MODE_MASK;
> +	if (!xpcs->pcs.poll)
> +		ret |= DW_VR_MII_AN_INTR_EN;

Does this interrupt only work in 1000baseX mode?

>  	ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_CTRL, ret);
>  	if (ret < 0)
>  		return ret;
> @@ -1012,6 +1017,17 @@ static int xpcs_get_state_c37_1000basex(struct dw_xpcs *xpcs,
>  		if (bmsr < 0)
>  			return bmsr;
>  
> +		/* Clear AN complete interrupt */
> +		if (!xpcs->pcs.poll) {
> +			int an_intr;
> +
> +			an_intr = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS);
> +			if (an_intr & DW_VR_MII_AN_STS_C37_ANCMPLT_INTR) {
> +				an_intr &= ~DW_VR_MII_AN_STS_C37_ANCMPLT_INTR;
> +				xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, an_intr);
> +			}
> +		}
> +

get_state isn't supposed to be used as a way to acknowledge interrupts,
because that will get called quite a bit later after the interrupt has
been received.

As an example of PCS that use interrupts, please see the converted
mv88e6xxx PCS, for example:

 drivers/net/dsa/mv88e6xxx/pcs-6352.c

If the interrupt handler for the PCS is threaded, then it can access
the DW_VR_MII_AN_INTR_STS register to acknowledge the interrupt and
call phylink_pcs_change() or phylink_mac_change().

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!