[PATCH net-next 9/9] ptp: ocp: confirm the CPLD really left configuration mode after REFRESH

[Sagi Maimon]

The post-REFRESH check required DONE set, BUSY clear and no error code.
Those three conditions are already satisfied by the state SET_DONE leaves
behind, so they cannot distinguish a REFRESH that rebooted the part from
one whose frame was ACKed but never latched - and the I2C ACK alone was
taken as proof, clearing cpld_in_config_mode.

A part left that way stays in configuration mode running the old image
while "devlink dev flash ... component fw.cpld" reports success, which is
the opposite of what the documentation promises.

Test CPLD_STATUS_ENAB as well: leaving configuration mode is the one
thing only a REFRESH does, so it is what separates the two cases.  Put
cpld_in_config_mode back when ENAB is still set, so the exit path and the
recovery at the start of the next flash can act on it instead of
believing a mode change that never happened.

Fixes: a4b7c15aa78f ("ptp: ocp: add TAP CPLD flashing via devlink")
Signed-off-by: Sagi Maimon <maimon.sagi@xxxxxxxxx>
---
 drivers/ptp/ptp_ocp.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 4f2bf54a23c2..9c2b7403bfd0 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -5135,6 +5135,9 @@ static int adva_x1_cpld_flash(struct ptp_ocp *bp, struct devlink *devlink,
 
 	/* REFRESH reboots the CPLD out of configuration mode, so the exit
 	 * path must not send DIS_CFG afterwards even if a check below fails.
+	 * The ENAB test below confirms it really left; until then assume it
+	 * did, because sending DIS_CFG to a part that has rebooted is what
+	 * this flag exists to avoid.
 	 */
 	bp->cpld_in_config_mode = false;
 
@@ -5156,12 +5159,21 @@ static int adva_x1_cpld_flash(struct ptp_ocp *bp, struct devlink *devlink,
 	/* Require DONE set, not busy and no error code, as machxo2-spi.c does
 	 * after a refresh: without it a CRC or preamble error reads back as a
 	 * successful update.
+	 *
+	 * ENAB has to be clear too.  Those three conditions are already met
+	 * by the state SET_DONE leaves behind, so on their own they cannot
+	 * tell a REFRESH that rebooted the part from one whose frame was
+	 * ACKed but never latched - which leaves the part in configuration
+	 * mode still running the old image.  Leaving configuration mode is
+	 * the one thing only a REFRESH does.
 	 */
 	err = adva_x1_cpld_read_status(bp, &st);
 	if (err)
 		goto deselect;
+	if (st & CPLD_STATUS_ENAB)
+		bp->cpld_in_config_mode = true;
 	if (!(st & CPLD_STATUS_DONE) || (st & CPLD_STATUS_BUSY) ||
-	    (st & CPLD_STATUS_ERR)) {
+	    (st & CPLD_STATUS_ERR) || (st & CPLD_STATUS_ENAB)) {
 		dev_err(&bp->pdev->dev,
 			"CPLD refresh left status 0x%08x\n", st);
 		NL_SET_ERR_MSG_MOD(extack, "CPLD did not come back configured");
-- 
2.47.0