Commit ca2dccde authored by Ioana Ciornei's avatar Ioana Ciornei Committed by Paolo Abeni
Browse files

net: phy: aquantia: wait for the suspend/resume operations to finish 



The Aquantia datasheet notes that after issuing a Processor-Intensive
MDIO operation, like changing the low-power state of the device, the
driver should wait for the operation to finish before issuing a new MDIO
command.

The new aqr107_wait_processor_intensive_op() function is added which can
be used after these kind of MDIO operations. At the moment, we are only
adding it at the end of the suspend/resume calls.

The issue was identified on a board featuring the AQR113C PHY, on
which commands like 'ip link (..) up / down' issued without any delays
between them would render the link on the PHY to remain down.
The issue was easy to reproduce with a one-liner:
 $ ip link set dev ethX down; ip link set dev ethX up; \
 ip link set dev ethX down; ip link set dev ethX up;

Fixes: ac9e81c2 ("net: phy: aquantia: add suspend / resume callbacks for AQR107 family")
Signed-off-by: default avatarIoana Ciornei <ioana.ciornei@nxp.com>
Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
Link: https://lore.kernel.org/r/20220906130451.1483448-1-ioana.ciornei@nxp.com


Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parent 64ae13ed

drivers/net/phy/aquantia_main.c

+49 −4
Original line number Diff line number Diff line
@@ -91,6 +91,9 @@ 
#define VEND1_GLOBAL_FW_ID_MAJOR		GENMASK(15, 8)

#define VEND1_GLOBAL_FW_ID_MINOR		GENMASK(7, 0)



#define VEND1_GLOBAL_GEN_STAT2			0xc831

#define VEND1_GLOBAL_GEN_STAT2_OP_IN_PROG	BIT(15)



#define VEND1_GLOBAL_RSVD_STAT1			0xc885

#define VEND1_GLOBAL_RSVD_STAT1_FW_BUILD_ID	GENMASK(7, 4)

#define VEND1_GLOBAL_RSVD_STAT1_PROV_ID		GENMASK(3, 0)
@@ -125,6 +128,12 @@ 
#define VEND1_GLOBAL_INT_VEND_MASK_GLOBAL2	BIT(1)

#define VEND1_GLOBAL_INT_VEND_MASK_GLOBAL3	BIT(0)



/* Sleep and timeout for checking if the Processor-Intensive

 * MDIO operation is finished

 */

#define AQR107_OP_IN_PROG_SLEEP		1000

#define AQR107_OP_IN_PROG_TIMEOUT	100000



struct aqr107_hw_stat {

	const char *name;

	int reg;
@@ -597,16 +606,52 @@ static void aqr107_link_change_notify(struct phy_device *phydev) 
		phydev_info(phydev, "Aquantia 1000Base-T2 mode active\n");

}



static int aqr107_wait_processor_intensive_op(struct phy_device *phydev)

{

	int val, err;



	/* The datasheet notes to wait at least 1ms after issuing a

	 * processor intensive operation before checking.

	 * We cannot use the 'sleep_before_read' parameter of read_poll_timeout

	 * because that just determines the maximum time slept, not the minimum.

	 */

	usleep_range(1000, 5000);



	err = phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND1,

					VEND1_GLOBAL_GEN_STAT2, val,

					!(val & VEND1_GLOBAL_GEN_STAT2_OP_IN_PROG),

					AQR107_OP_IN_PROG_SLEEP,

					AQR107_OP_IN_PROG_TIMEOUT, false);

	if (err) {

		phydev_err(phydev, "timeout: processor-intensive MDIO operation\n");

		return err;

	}



	return 0;

}



static int aqr107_suspend(struct phy_device *phydev)

{

	return phy_set_bits_mmd(phydev, MDIO_MMD_VEND1, MDIO_CTRL1,

	int err;



	err = phy_set_bits_mmd(phydev, MDIO_MMD_VEND1, MDIO_CTRL1,

			       MDIO_CTRL1_LPOWER);

	if (err)

		return err;



	return aqr107_wait_processor_intensive_op(phydev);

}



static int aqr107_resume(struct phy_device *phydev)

{

	return phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1, MDIO_CTRL1,

	int err;



	err = phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1, MDIO_CTRL1,

				 MDIO_CTRL1_LPOWER);

	if (err)

		return err;



	return aqr107_wait_processor_intensive_op(phydev);

}



static int aqr107_probe(struct phy_device *phydev)