[PATCH net-next v14 0/4] ptp: ocp: add TAP CPLD support for ADVA TimeCard X1

[PATCH net-next v14 0/4] ptp: ocp: add TAP CPLD support for ADVA TimeCard X1

[Sagi Maimon]

The ADVA TimeCard X1 (PCI device 0x0410) carries a Lattice MachXO3 CPLD
on the TMC I2C bus.  This series adds access to it and an update path
through devlink flash.

The card has two I2C controllers.  Linux registers only the one at
0x00150000, normally routed to the config EEPROMs; the one at 0x00120000
belongs to the on-card MicroBlaze and is routed to the TMC or M.2 bus as
its firmware selects.  A request/grant handshake re-routes the host
controller onto the TMC bus, where the PCA9548 and the CPLD behind its
channel 0 live.  Everything here is built on that handshake.

Patches 1 and 2 are pre-existing probe-error-path bugs that were buried
in the feature commit in v13; they are split out with Fixes: tags, as
requested.  Patch 1 is also a prerequisite: patch 3 adds a mutex that
ptp_ocp_detach() destroys, so devlink has to be unregistered first.
Patch 3 adds the bus arbitration and the read-only interfaces, patch 4
the flashing.

v13: https://lore.kernel.org/netdev/20260908122328.52641-1-maimon.sagi@gmail.com/T/#u

Changes since v13:

 - The CPLD is no longer flashed through the firmware-upload subsystem.
   It is a devlink flash component instead:

     devlink dev flash pci/0000:02:00.0 file cpld.bin component fw.cpld

   ->info_get() reports fw.cpld as a running version with
   DEVLINK_INFO_VERSION_TYPE_COMPONENT, carrying the USERCODE of the
   image in the part, which is also what makes the core accept the
   component name.  No component still means the card flash.  This
   removed fw_upload_ops, the adva-cpld.N node, the FW_LOADER/FW_UPLOAD
   selects and the cancellation state machine - and with it a race where
   a cancel arriving after one upload finished could abort the next.
   Patch 4.  (Jakub Kicinski)
 - cpld_status now contains the names of the flags that are set, space
   separated, and an empty line when none is, instead of
   "done=<0|1> busy=<0|1> failed=<0|1>".  Patch 3.  (Jakub Kicinski)
 - The commit was split; the two pre-existing fixes it contained are
   patches 1 and 2 with Fixes: tags.  (Jakub Kicinski)
 - The flash path now validates the state after REFRESH - DONE set, not
   busy, and no sysCONFIG error - and fails the devlink command rather
   than reporting success for an image that did not come back.  Patch 4.
 - adva_x1_mblaze_release() waits for the firmware to drop the grant
   instead of writing RELEASE and returning, and warns if it does not.
   Patch 3.
 - The i2c bus notifier only caches the adapter number for the card's
   own controller, not for an i2c-mux child registered on it.  The
   adapter is also resolved once from probe, because the notifier fires
   from inside platform_device_register() - before bp->i2c_ctrl is
   assigned - whenever the i2c controller driver is already loaded.
   Patch 3.
 - ptp_ocp_devlink_info_get() checks bp->has_cpld explicitly rather than
   relying on cpld_id being zero on boards without one.  Patch 3.
 - ptp_ocp_sync_work() no longer latches the one-shot ID read on -EBUSY,
   so a read that collided with a CPLD operation is retried.  Patch 3.
 - Documentation/networking/devlink/ptp_ocp.rst documents the flash
   components and states the accepted image format: a raw bitstream, a
   whole number of 16-byte pages; .jed and other containers are
   rejected.  Patch 4.

Tested on an ADVA TimeCard X1: a full erase/program/refresh cycle of a
6526-page image through "devlink dev flash ... component fw.cpld", the
CPLD reporting done afterwards and still done across a reboot, and both
rejection paths (a .jed image, and an unknown component name).


Sagi Maimon (4):
  ptp: ocp: unregister devlink before detach on probe error
  ptp: ocp: fix dpll cleanup on probe error
  ptp: ocp: add TAP CPLD access for ADVA TimeCard X1
  ptp: ocp: add TAP CPLD flashing via devlink

 Documentation/ABI/testing/sysfs-timecard     |  28 +
 Documentation/networking/devlink/index.rst   |   1 +
 Documentation/networking/devlink/ptp_ocp.rst |  70 ++
 MAINTAINERS                                  |   1 +
 drivers/ptp/ptp_ocp.c                        | 774 ++++++++++++++++++-
 5 files changed, 868 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/networking/devlink/ptp_ocp.rst


base-commit: 879e280b8486d4612ad1aa050d6fada2dd80cf1c
-- 
2.47.0

[PATCH net-next v14 1/4] ptp: ocp: unregister devlink before detach on probe error

[Sagi Maimon]

devlink_register() runs before the dpll setup in ptp_ocp_probe(), but the
failure paths after it went straight to ptp_ocp_detach() and then
devlink_free(), which opens with ASSERT_DEVLINK_NOT_REGISTERED(devlink).
Any dpll failure during probe therefore freed a devlink instance that was
still registered.

Route those paths through a new out_devlink label that unregisters first,
matching the order ptp_ocp_remove() already uses.

Fixes: 09eeb3aecc6c ("ptp_ocp: implement DPLL ops")
Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
---
 drivers/ptp/ptp_ocp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 3d26ec1f7b9e..401b04f12d2b 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -5119,12 +5119,12 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (IS_ERR(bp->dpll)) {
 		err = PTR_ERR(bp->dpll);
 		dev_err(&pdev->dev, "dpll_device_alloc failed\n");
-		goto out;
+		goto out_devlink;
 	}
 
 	err = dpll_device_register(bp->dpll, DPLL_TYPE_PPS, &dpll_ops, bp);
 	if (err)
-		goto out;
+		goto out_devlink;
 
 	for (i = 0; i < OCP_SMA_NUM; i++) {
 		bp->sma[i].dpll_pin = dpll_pin_get(clkid, i, THIS_MODULE,
@@ -5151,6 +5151,8 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		dpll_pin_put(bp->sma[i].dpll_pin, &bp->sma[i].tracker);
 	}
 	dpll_device_put(bp->dpll, &bp->tracker);
+out_devlink:
+	devlink_unregister(devlink);
 out:
 	ptp_ocp_detach(bp);
 out_disable:
-- 
2.47.0

[PATCH net-next v14 2/4] ptp: ocp: fix dpll cleanup on probe error

[Sagi Maimon]

Two problems on the dpll error paths in ptp_ocp_probe().

dpll_device_register() takes a second reference and stores bp as the
registration private data, and only dpll_device_unregister() drops it.
out_dpll dropped bp->tracker but never unregistered, so a dpll_pin_get()
or dpll_pin_register() failure left the device registered and visible to
userspace with a priv pointer into the storage devlink_free() releases a
few lines later, as well as leaking the registration object.

A failing dpll_device_register() jumped to the same label as the failing
dpll_device_get(), so the reference taken by dpll_device_get() was never
dropped either.  Give that case its own label.

Fixes: 09eeb3aecc6c ("ptp_ocp: implement DPLL ops")
Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
---
 drivers/ptp/ptp_ocp.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 401b04f12d2b..f4299878c27c 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -5124,7 +5124,7 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	err = dpll_device_register(bp->dpll, DPLL_TYPE_PPS, &dpll_ops, bp);
 	if (err)
-		goto out_devlink;
+		goto out_dpll_put;
 
 	for (i = 0; i < OCP_SMA_NUM; i++) {
 		bp->sma[i].dpll_pin = dpll_pin_get(clkid, i, THIS_MODULE,
@@ -5150,6 +5150,12 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		dpll_pin_unregister(bp->dpll, bp->sma[i].dpll_pin, &dpll_pins_ops, &bp->sma[i]);
 		dpll_pin_put(bp->sma[i].dpll_pin, &bp->sma[i].tracker);
 	}
+	/* dpll_device_register() takes a second reference and stores bp; only
+	 * dpll_device_unregister() drops it.  Without this the device stays
+	 * visible with a priv pointer into storage devlink_free() will release.
+	 */
+	dpll_device_unregister(bp->dpll, &dpll_ops, bp);
+out_dpll_put:
 	dpll_device_put(bp->dpll, &bp->tracker);
 out_devlink:
 	devlink_unregister(devlink);
-- 
2.47.0

[PATCH net-next v14 3/4] ptp: ocp: add TAP CPLD access for ADVA TimeCard X1

[Sagi Maimon]

The ADVA TimeCard X1 carries a Lattice MachXO3 CPLD on its TMC I2C bus.
Reaching it means taking that bus from the on-card MicroBlaze first.

The card has two I2C controllers.  Linux registers only the one at
0x00150000, normally routed to the config EEPROMs; the one at 0x00120000
belongs to the MicroBlaze and is routed to the TMC or M.2 bus as its
firmware selects.  A request/grant handshake re-routes the host controller
onto the TMC bus, where the PCA9548 and the CPLD behind its channel 0
live.  An operation takes the i2c core adapter lock for the whole grant
window and uses __i2c_transfer() internally; without that, a concurrent
transfer from ptp_ocp_read_eeprom(), from the nvmem attributes or from the
at24 sysfs files would be issued onto the TMC bus instead of to the
EEPROM.  The hand-back waits for the firmware to drop the grant before the
adapter lock is released, for the same reason.

On top of the arbitration this adds two read-only interfaces: a root-only
cpld_status attribute reporting the flags in the CPLD status register, and
the Lattice device ID as the fixed devlink version cpld.id.  Reading the
ID claims the bus, so it must not sit on DEVLINK_CMD_INFO_GET, which
carries no GENL_ADMIN_PERM; the driver reads it once from its own worker,
as soon as the i2c adapter has registered, and info_get() reports the
cached value.

Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
---
 Documentation/ABI/testing/sysfs-timecard     |  24 +
 Documentation/networking/devlink/index.rst   |   1 +
 Documentation/networking/devlink/ptp_ocp.rst |  35 ++
 MAINTAINERS                                  |   1 +
 drivers/ptp/ptp_ocp.c                        | 447 ++++++++++++++++++-
 5 files changed, 504 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/networking/devlink/ptp_ocp.rst

diff --git a/Documentation/ABI/testing/sysfs-timecard b/Documentation/ABI/testing/sysfs-timecard
index 3ae41b7634ac..c440d8930384 100644
--- a/Documentation/ABI/testing/sysfs-timecard
+++ b/Documentation/ABI/testing/sysfs-timecard
@@ -11,6 +11,30 @@ Contact:	Jonathan Lemon <jonathan.lemon@gmail.com>
 Description:	This directory contains the attributes of the Nth timecard
 		registered.
 
+What:		/sys/class/timecard/ocpN/cpld_status
+Date:		July 2026
+Contact:	Sagi Maimon <maimon.sagi@gmail.com>
+Description:	(RO, root only) The flags set in the status register of the
+		TAP CPLD, space separated, or an empty line when none is
+		set.  Only present on ADVA x1 TAP boards (PCI ID
+		0xad5a:0x0410).
+
+		==========  ================================================
+		done        the configuration flash was programmed
+			    successfully and is active
+		busy        an internal operation is in progress
+		failed      the last ISC operation failed
+		==========  ================================================
+
+		For example "busy" while programming, or "busy failed" if a
+		failure is latched while another operation runs.
+
+		A read arbitrates for the shared I2C bus and reprograms the
+		on-card mux, so it is restricted to root.  The Lattice device
+		ID of the CPLD is read once by the driver, shortly after
+		probe, and reported from that cached value as the fixed
+		"cpld.id" version by devlink dev info.
+
 What:		/sys/class/timecard/ocpN/available_clock_sources
 Date:		September 2021
 Contact:	Jonathan Lemon <jonathan.lemon@gmail.com>
diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
index d4a83fdcff7f..1af780c811ee 100644
--- a/Documentation/networking/devlink/index.rst
+++ b/Documentation/networking/devlink/index.rst
@@ -99,6 +99,7 @@ parameters, info versions, and other features it supports.
    nfp
    octeontx2
    prestera
+   ptp_ocp
    qed
    sfc
    stmmac
diff --git a/Documentation/networking/devlink/ptp_ocp.rst b/Documentation/networking/devlink/ptp_ocp.rst
new file mode 100644
index 000000000000..3f997cd9f3a3
--- /dev/null
+++ b/Documentation/networking/devlink/ptp_ocp.rst
@@ -0,0 +1,35 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+========================
+ptp_ocp devlink support
+========================
+
+This document describes the devlink features implemented by the ``ptp_ocp``
+device driver.
+
+Info versions
+=============
+
+The ``ptp_ocp`` driver reports the following versions
+
+.. list-table:: devlink info versions implemented
+   :widths: 5 5 90
+
+   * - Name
+     - Type
+     - Description
+   * - ``fw``
+     - running
+     - Version of the firmware running on the card.  Reported as ``loader``
+       instead when the card is running the firmware loader.
+   * - ``board.id``
+     - fixed
+     - Board identifier, read from the on-card EEPROM.
+   * - ``cpld.id``
+     - fixed
+     - Lattice device ID (IDCODE) of the TAP CPLD, formatted as ``0x%08x``.
+       Only present on ADVA TimeCard X1 boards, which are the only ones
+       carrying that CPLD.  Reading it claims the shared I2C bus and
+       reprograms the on-card mux, so the driver does that once from its
+       own worker and reports the cached value here; the version is
+       omitted until that read has succeeded.
diff --git a/MAINTAINERS b/MAINTAINERS
index 0e04d92d1b09..113f31434617 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20479,6 +20479,7 @@ OPENCOMPUTE PTP CLOCK DRIVER
 M:	Vadim Fedorenko <vadim.fedorenko@linux.dev>
 L:	netdev@vger.kernel.org
 S:	Maintained
+F:	Documentation/networking/devlink/ptp_ocp.rst
 F:	drivers/ptp/ptp_ocp.c
 
 OPENCORES I2C BUS DRIVER
diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index f4299878c27c..98cb26077181 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -20,10 +20,14 @@
 #include <linux/spi/altera.h>
 #include <net/devlink.h>
 #include <linux/i2c.h>
+#include <linux/iopoll.h>
 #include <linux/mtd/mtd.h>
 #include <linux/nvmem-consumer.h>
 #include <linux/crc16.h>
 #include <linux/dpll.h>
+#include <linux/unaligned.h>
+#include <linux/delay.h>
+#include <linux/firmware.h>
 
 #define PCI_DEVICE_ID_META_TIMECARD		0x0400
 
@@ -85,6 +89,8 @@ struct ptp_ocp_adva_info {
 	u8				signals_nr;
 	u8				freq_in_nr;
 	const struct ocp_attr_group	*attr_groups;
+	/* x1: TAP CPLD present */
+	bool				has_cpld;
 };
 
 #define OCP_CTRL_ENABLE		BIT(0)
@@ -163,7 +169,8 @@ struct gpio_reg {
 	u32	gpio1;
 	u32	__pad0;
 	u32	gpio2;
-	u32	__pad1;
+	/* adva_x1: I2C bus ownership register; reserved on other variants */
+	u32	i2c_bus_ctrl;
 };
 
 struct irig_master_reg {
@@ -416,6 +423,23 @@ struct ptp_ocp {
 	dpll_tracker tracker;
 	int signals_nr;
 	int freq_in_nr;
+	/* adva_x1 CPLD I2C (internal use only) */
+	/* serialises CPLD operations */
+	struct mutex		cpld_lock;
+	/* I2C adapter nr; -1 if absent */
+	int			cpld_i2c_adap_nr;
+	/* claimed adapter; valid under cpld_lock */
+	struct i2c_adapter	*cpld_adap;
+	/* scratch, write half then read half; valid under cpld_lock */
+	u8			*cpld_buf;
+	/* Lattice device ID; 0 if unread */
+	u32			cpld_id;
+	/* the one-shot ID read is finished, successfully or not */
+	bool			cpld_id_tried;
+	/* failed ID read attempts so far */
+	unsigned int		cpld_id_attempts;
+	/* x1 TAP CPLD present */
+	bool			has_cpld;
 };
 
 #define OCP_REQ_TIMESTAMP	BIT(0)
@@ -1273,6 +1297,7 @@ static struct ocp_resource ocp_adva_x1_resource[] = {
 			.signals_nr   = 4,
 			.freq_in_nr   = 4,
 			.attr_groups  = adva_timecard_x1_groups,
+			.has_cpld     = true,
 		},
 	},
 	{ }
@@ -2159,6 +2184,7 @@ ptp_ocp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 	const char *fw_image;
 	char buf[32];
 	int err;
+	u32 id;
 
 	fw_image = bp->fw_loader ? "loader" : "fw";
 	sprintf(buf, "%d.%d", bp->fw_tag, bp->fw_version);
@@ -2166,6 +2192,18 @@ ptp_ocp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 	if (err)
 		return err;
 
+	/* Read by the worker, not here: this command is unprivileged and
+	 * reading the ID claims the I2C bus.  0 means unread - skip it
+	 * rather than fail, devlink discards the whole reply on error.
+	 */
+	id = READ_ONCE(bp->cpld_id);
+	if (bp->has_cpld && id) {
+		sprintf(buf, "0x%08x", id);
+		err = devlink_info_version_fixed_put(req, "cpld.id", buf);
+		if (err)
+			return err;
+	}
+
 	if (!bp->has_eeprom_data) {
 		ptp_ocp_read_eeprom(bp);
 		if (!bp->has_eeprom_data)
@@ -3199,6 +3237,8 @@ ptp_ocp_adva_board_init(struct ptp_ocp *bp, struct ocp_resource *r)
 		return err;
 	ptp_ocp_sma_init(bp);
 
+	bp->has_cpld = info->has_cpld;
+
 	return ptp_ocp_init_clock(bp, &info->servo);
 }
 
@@ -4226,6 +4266,367 @@ static const struct ocp_attr_group art_timecard_groups[] = {
 	{ },
 };
 
+/*
+ * adva_x1 TAP CPLD (Lattice LCMXO3LF-2100) helpers.
+ *
+ * The card has two I2C controllers; Linux registers only 0x00150000.
+ * The i2c_bus_ctrl handshake re-routes what each is wired to:
+ *
+ *   grant 0:  0x150000 -> EEPROMs     0x120000 -> TMC or M.2, MB's choice
+ *   grant 1:  0x150000 -> TMC bus     0x120000 -> M.2
+ *
+ * The PCA9548 at 0x74 and the CPLD at 0x40 behind its channel 0 are on the
+ * TMC bus, reachable only while the grant is held.  The EEPROMs are not
+ * behind the adapter for that window, so a CPLD operation holds cpld_lock
+ * and the adapter lock to keep the EEPROM and nvmem paths off it.
+ */
+
+#define ADVA_MUX_ADDR     0x74
+#define ADVA_CPLD_ADDR    0x40
+#define ADVA_MUX_CHANNEL  0
+
+/* Longest command or reply; bounds each half of the per-claim scratch. */
+#define ADVA_CPLD_XFER_MAX 32
+
+#define MBLAZE_REQUEST    0x0000aaaaU
+#define MBLAZE_GRANTED    0x5555aaaaU
+#define MBLAZE_RELEASE    0x55550000U
+#define MBLAZE_RETRIES    200
+#define MBLAZE_RETRY_US   10000
+
+/* Lattice LCMXO3LF ISC command codes */
+#define CPLD_CMD_READ_ID      0xE0000000UL
+#define CPLD_CMD_READ_STATUS  0x3C000000UL
+#define CPLD_ID_MAX_ATTEMPTS  10     /* one per sync_work tick */
+
+/* Status register bit positions (Lattice LCMXO3LF datasheet) */
+#define CPLD_STATUS_DONE   BIT(8)
+#define CPLD_STATUS_BUSY   BIT(12)
+#define CPLD_STATUS_FAILED BIT(13)
+
+/*
+ * Issue one I2C transaction on the TMC bus: @cmd if not negative, then
+ * @wlen bytes of @wdata (NULL sends zeros), then @rlen bytes read back.
+ * The message is built in the scratch buffer from adva_x1_bus_claim(), so
+ * an upload costs one allocation rather than one per page.  I2C_M_DMA_SAFE
+ * is not set: the two halves are not separately aligned, so let the core
+ * bounce.  Caller holds the claim, hence __i2c_transfer().
+ */
+static int adva_x1_i2c_xfer(struct ptp_ocp *bp, u8 addr, int cmd,
+			    const void *wdata, u8 wlen,
+			    void *rdata, u8 rlen)
+{
+	u8 *wbuf, *rbuf;
+	struct i2c_adapter *adap;
+	struct i2c_msg msgs[2];
+	int nmsgs = 0, ret;
+	u16 hdr = cmd >= 0;
+
+	lockdep_assert_held(&bp->cpld_lock);
+
+	adap = bp->cpld_adap;
+	if (!adap || !bp->cpld_buf)
+		return -ENODEV;
+
+	if (hdr + wlen > ADVA_CPLD_XFER_MAX || rlen > ADVA_CPLD_XFER_MAX)
+		return -EINVAL;
+
+	wbuf = bp->cpld_buf;
+	rbuf = bp->cpld_buf + ADVA_CPLD_XFER_MAX;
+
+	if (hdr + wlen) {
+		if (hdr)
+			wbuf[0] = cmd;
+		if (wdata)
+			memcpy(wbuf + hdr, wdata, wlen);
+		else
+			memset(wbuf + hdr, 0, wlen);
+		msgs[nmsgs++] = (struct i2c_msg){
+			.addr  = addr,
+			.flags = 0,
+			.len   = hdr + wlen,
+			.buf   = wbuf,
+		};
+	}
+	if (rlen) {
+		msgs[nmsgs++] = (struct i2c_msg){
+			.addr  = addr,
+			.flags = I2C_M_RD,
+			.len   = rlen,
+			.buf   = rbuf,
+		};
+	}
+
+	ret = __i2c_transfer(adap, msgs, nmsgs);
+	if (ret != nmsgs)
+		return (ret < 0) ? ret : -EIO;
+
+	if (rdata && rlen)
+		memcpy(rdata, rbuf, rlen);
+
+	return 0;
+}
+
+/* Hand the segment back, and wait for the firmware to drop the grant before
+ * the caller unlocks the adapter: a transfer queued behind us would
+ * otherwise be clocked out while the controller is still on the TMC bus,
+ * which is what holding the adapter lock across the grant exists to stop.
+ */
+static void adva_x1_mblaze_release(struct ptp_ocp *bp)
+{
+	int i;
+
+	if (!bp->pps_select)
+		return;
+
+	iowrite32(MBLAZE_RELEASE, &bp->pps_select->i2c_bus_ctrl);
+	/* Flush the posted write, as the acquire path does. */
+	ioread32(&bp->pps_select->i2c_bus_ctrl);
+
+	for (i = 0; i < MBLAZE_RETRIES; i++) {
+		if (ioread32(&bp->pps_select->i2c_bus_ctrl) != MBLAZE_GRANTED)
+			return;
+		usleep_range(MBLAZE_RETRY_US, MBLAZE_RETRY_US + 1000);
+	}
+
+	dev_warn(&bp->pdev->dev, "TMC bus still granted after release\n");
+}
+
+/* Acquire the shared I2C bus from the MicroBlaze firmware.  Returns with no
+ * request outstanding on failure, so the firmware is never left granting a
+ * segment to a host that has given up waiting for it.
+ */
+static int adva_x1_mblaze_acquire(struct ptp_ocp *bp)
+{
+	u32 val;
+	int i;
+
+	if (!bp->pps_select)
+		return -ENODEV;
+
+	/* Drop a request left by a caller that died mid-sequence; cpld_lock
+	 * keeps live ones out.  The read back only flushes the posted write.
+	 */
+	iowrite32(0, &bp->pps_select->i2c_bus_ctrl);
+	ioread32(&bp->pps_select->i2c_bus_ctrl);
+
+	iowrite32(MBLAZE_REQUEST, &bp->pps_select->i2c_bus_ctrl);
+	for (i = 0; i < MBLAZE_RETRIES; i++) {
+		usleep_range(MBLAZE_RETRY_US, MBLAZE_RETRY_US + 1000);
+		val = ioread32(&bp->pps_select->i2c_bus_ctrl);
+		if (val == MBLAZE_GRANTED)
+			return 0;
+	}
+
+	adva_x1_mblaze_release(bp);
+	return -ETIMEDOUT;
+}
+
+/* Route the host controller back to the EEPROMs and release the adapter.
+ * Safe after a failed claim: it also clears a request that was never granted.
+ */
+static void adva_x1_bus_release(struct ptp_ocp *bp)
+{
+	struct i2c_adapter *adap = bp->cpld_adap;
+
+	if (!adap)
+		return;
+
+	adva_x1_mblaze_release(bp);
+	bp->cpld_adap = NULL;
+	kfree(bp->cpld_buf);
+	bp->cpld_buf = NULL;
+	i2c_unlock_bus(adap, I2C_LOCK_ROOT_ADAPTER);
+	i2c_put_adapter(adap);
+}
+
+static int adva_x1_i2c_adap_match(struct device *dev, const void *data)
+{
+	return !!i2c_verify_adapter(dev);
+}
+
+/*
+ * Cache the adapter the CPLD hangs off.  ptp_ocp_i2c_notifier_call() misses
+ * it when the i2c controller driver is already loaded: the adapter is then
+ * added from inside platform_device_register(), before bp->i2c_ctrl is set.
+ * Only direct children are searched, so a mux adapter cannot match.
+ */
+static void adva_x1_cache_i2c_adap(struct ptp_ocp *bp)
+{
+	struct device *child;
+
+	if (!bp->has_cpld || !bp->i2c_ctrl ||
+	    READ_ONCE(bp->cpld_i2c_adap_nr) >= 0)
+		return;
+
+	child = device_find_child(&bp->i2c_ctrl->dev, NULL,
+				  adva_x1_i2c_adap_match);
+	if (!child)
+		return;
+
+	WRITE_ONCE(bp->cpld_i2c_adap_nr, i2c_verify_adapter(child)->nr);
+	put_device(child);
+}
+
+/*
+ * Claim the TMC bus.  The adapter lock is held across the handshake to keep
+ * the EEPROM, nvmem and at24 paths off the controller while it is routed
+ * away from the EEPROMs; an EEPROM read blocks until the claim is dropped.
+ */
+static int adva_x1_bus_claim(struct ptp_ocp *bp)
+{
+	struct i2c_adapter *adap;
+	int ret;
+
+	lockdep_assert_held(&bp->cpld_lock);
+
+	adap = i2c_get_adapter(READ_ONCE(bp->cpld_i2c_adap_nr));
+	if (!adap)
+		return -ENODEV;
+
+	/* The number is freed before the notifier clears it, so it can
+	 * already be another adapter's.  Check this is still ours.
+	 */
+	if (!bp->i2c_ctrl || adap->dev.parent != &bp->i2c_ctrl->dev) {
+		i2c_put_adapter(adap);
+		return -ENODEV;
+	}
+
+	/* One scratch buffer per claim rather than per transfer: an upload
+	 * holds the claim for the whole image.
+	 */
+	bp->cpld_buf = kzalloc(2 * ADVA_CPLD_XFER_MAX, GFP_KERNEL);
+	if (!bp->cpld_buf) {
+		i2c_put_adapter(adap);
+		return -ENOMEM;
+	}
+
+	i2c_lock_bus(adap, I2C_LOCK_ROOT_ADAPTER);
+	bp->cpld_adap = adap;
+
+	ret = adva_x1_mblaze_acquire(bp);
+	if (ret)
+		adva_x1_bus_release(bp);
+
+	return ret;
+}
+
+/* Select a mux channel, or deselect all with ch < 0 - the power-on state.
+ * The mux is on the TMC bus, so what it is left set to never affects the
+ * EEPROM paths.
+ */
+static int adva_x1_mux_select(struct ptp_ocp *bp, int ch)
+{
+	u8 val = (ch >= 0) ? BIT(ch) : 0;
+
+	return adva_x1_i2c_xfer(bp, ADVA_MUX_ADDR, val, NULL, 0, NULL, 0);
+}
+
+/*
+ * Send a 4-byte command and read back without an intermediate STOP: two
+ * messages in one transfer is the Lattice write -> repeated START -> read,
+ * so no protocol-mangling flag is needed.
+ */
+static int adva_x1_cpld_cmd_read(struct ptp_ocp *bp,
+				 u32 cmd_be, u8 *out, u8 out_len)
+{
+	__be32 cmd = cpu_to_be32(cmd_be);
+
+	return adva_x1_i2c_xfer(bp, ADVA_CPLD_ADDR, -1, &cmd, 4, out, out_len);
+}
+
+static int adva_x1_cpld_read_status(struct ptp_ocp *bp, u32 *status)
+{
+	u8 buf[4];
+	int ret;
+
+	ret = adva_x1_cpld_cmd_read(bp, CPLD_CMD_READ_STATUS, buf, 4);
+	if (ret)
+		return ret;
+	*status = get_unaligned_be32(buf);
+	return 0;
+}
+
+/*
+ * Read the Lattice device ID into bp->cpld_id.  Done once, off the
+ * unprivileged devlink path, which reports the cached value.  -EBUSY means
+ * cpld_lock is held, so the caller can retry rather than wait behind a long
+ * CPLD operation.
+ */
+static int adva_x1_cpld_read_id(struct ptp_ocp *bp)
+{
+	u8 data[4];
+	int ret;
+
+	if (!mutex_trylock(&bp->cpld_lock))
+		return -EBUSY;
+
+	ret = adva_x1_bus_claim(bp);
+	if (ret)
+		goto out;
+	ret = adva_x1_mux_select(bp, ADVA_MUX_CHANNEL);
+	if (ret)
+		goto release;
+	ret = adva_x1_cpld_cmd_read(bp, CPLD_CMD_READ_ID, data, 4);
+	if (!ret)
+		WRITE_ONCE(bp->cpld_id, get_unaligned_be32(data));
+	adva_x1_mux_select(bp, -1);
+release:
+	adva_x1_bus_release(bp);
+out:
+	mutex_unlock(&bp->cpld_lock);
+	if (ret)
+		dev_dbg(&bp->pdev->dev,
+			"CPLD device ID unavailable: %d\n", ret);
+
+	return ret;
+}
+
+/* cpld_status - show the flags set in the TAP CPLD status register. */
+static ssize_t
+cpld_status_show(struct device *dev, struct device_attribute *attr,
+		 char *buf)
+{
+	struct ptp_ocp *bp = dev_get_drvdata(dev);
+	int count = 0;
+	u32 st = 0;
+	int ret;
+
+	/* A CPLD operation can hold cpld_lock a long time; stay killable. */
+	ret = mutex_lock_interruptible(&bp->cpld_lock);
+	if (ret)
+		return ret;
+
+	ret = adva_x1_bus_claim(bp);
+	if (ret)
+		goto out;
+	ret = adva_x1_mux_select(bp, ADVA_MUX_CHANNEL);
+	if (ret)
+		goto release;
+	ret = adva_x1_cpld_read_status(bp, &st);
+	adva_x1_mux_select(bp, -1);
+release:
+	adva_x1_bus_release(bp);
+out:
+	mutex_unlock(&bp->cpld_lock);
+	if (ret)
+		return ret;
+
+	/* Flags: the names of the bits that are set, nothing when none is,
+	 * rather than a value per bit in one file.
+	 */
+	if (st & CPLD_STATUS_DONE)
+		count += sysfs_emit_at(buf, count, "done");
+	if (st & CPLD_STATUS_BUSY)
+		count += sysfs_emit_at(buf, count, "%sbusy", count ? " " : "");
+	if (st & CPLD_STATUS_FAILED)
+		count += sysfs_emit_at(buf, count, "%sfailed",
+				       count ? " " : "");
+
+	return count + sysfs_emit_at(buf, count, "\n");
+}
+static DEVICE_ATTR_ADMIN_RO(cpld_status);
+
 static struct attribute *adva_timecard_attrs[] = {
 	&dev_attr_serialnum.attr,
 	&dev_attr_gnss_sync.attr,
@@ -4274,6 +4675,7 @@ static struct attribute *adva_timecard_x1_attrs[] = {
 	&dev_attr_ts_window_adjust.attr,
 	&dev_attr_utc_tai_offset.attr,
 	&dev_attr_tod_correction.attr,
+	&dev_attr_cpld_status.attr,
 	NULL,
 };
 
@@ -4904,6 +5306,7 @@ ptp_ocp_detach(struct ptp_ocp *bp)
 		clk_hw_unregister_fixed_rate(bp->i2c_clk);
 	if (bp->n_irqs)
 		pci_free_irq_vectors(bp->pdev);
+	mutex_destroy(&bp->cpld_lock);
 	device_unregister(&bp->dev);
 }
 
@@ -5052,6 +5455,20 @@ ptp_ocp_sync_work(struct work_struct *work)
 
 	bp->sync = sync;
 
+	/* The adapter can appear after probe, so read the ID from here once
+	 * it has.  A claim can fail transiently - the firmware may not grant
+	 * the segment straight after power-up - so retry a bounded number of
+	 * times.  -EBUSY means cpld_lock is held and does not count.
+	 */
+	if (bp->has_cpld && !READ_ONCE(bp->cpld_id_tried) &&
+	    READ_ONCE(bp->cpld_i2c_adap_nr) >= 0) {
+		int err = adva_x1_cpld_read_id(bp);
+
+		if (err != -EBUSY &&
+		    (!err || ++bp->cpld_id_attempts >= CPLD_ID_MAX_ATTEMPTS))
+			WRITE_ONCE(bp->cpld_id_tried, true);
+	}
+
 	queue_delayed_work(system_power_efficient_wq, &bp->sync_work, HZ);
 }
 
@@ -5080,6 +5497,14 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (err)
 		goto out_disable;
 
+	/* Before the first error path that calls ptp_ocp_detach(), so
+	 * mutex_destroy() runs on an initialised mutex, and before
+	 * ptp_ocp_register_resources(), so this sentinel cannot overwrite an
+	 * adapter number the i2c notifier stores during registration.
+	 */
+	mutex_init(&bp->cpld_lock);
+	bp->cpld_i2c_adap_nr = -1;
+
 	INIT_DELAYED_WORK(&bp->sync_work, ptp_ocp_sync_work);
 
 	/* compat mode.
@@ -5099,6 +5524,8 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (err)
 		goto out;
 
+	adva_x1_cache_i2c_adap(bp);
+
 	bp->ptp = ptp_clock_register(&bp->ptp_info, &pdev->dev);
 	if (IS_ERR(bp->ptp)) {
 		err = PTR_ERR(bp->ptp);
@@ -5158,6 +5585,7 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 out_dpll_put:
 	dpll_device_put(bp->dpll, &bp->tracker);
 out_devlink:
+	/* detach() destroys cpld_lock; no devlink op may still reach it */
 	devlink_unregister(devlink);
 out:
 	ptp_ocp_detach(bp);
@@ -5204,6 +5632,7 @@ ptp_ocp_i2c_notifier_call(struct notifier_block *nb,
 			  unsigned long action, void *data)
 {
 	struct device *dev, *child = data;
+	struct i2c_adapter *adap;
 	struct ptp_ocp *bp;
 	bool add;
 
@@ -5227,11 +5656,21 @@ ptp_ocp_i2c_notifier_call(struct notifier_block *nb,
 
 found:
 	bp = dev_get_drvdata(dev);
-	if (add)
+	adap = i2c_verify_adapter(child);
+	if (add) {
 		ptp_ocp_symlink(bp, child, "i2c");
-	else
+		/* Only the card's own controller: a mux child added on it
+		 * reaches ptp_ocp through the same walk.  Numbers are
+		 * recycled, so the claim re-checks the parent itself.
+		 */
+		if (bp->i2c_ctrl && adap->dev.parent == &bp->i2c_ctrl->dev)
+			WRITE_ONCE(bp->cpld_i2c_adap_nr, adap->nr);
+	} else {
+		/* invalidate before free, but only for the cached adapter */
+		if (READ_ONCE(bp->cpld_i2c_adap_nr) == adap->nr)
+			WRITE_ONCE(bp->cpld_i2c_adap_nr, -1);
 		sysfs_remove_link(&bp->dev.kobj, "i2c");
-
+	}
 	return 0;
 }
 
-- 
2.47.0

[PATCH net-next v14 4/4] ptp: ocp: add TAP CPLD flashing via devlink

[Sagi Maimon]

The Lattice MachXO3 CPLD on the ADVA TimeCard X1 is programmed over I2C
using in-system programming (ISP).  Build on the TMC bus arbitration added
previously and expose the update path as a devlink flash component, next
to the card flash the driver already writes through devlink.

The component is named fw.cpld and reported as a running version, so
->info_get() both publishes the USERCODE of the image currently in the
part and lets the core accept the name:

  devlink dev flash pci/0000:02:00.0 file adva-cpld.bin component fw.cpld

Flashing acquires the bus, erases the configuration flash, programs the
image page by page and activates it with the MachXO3 REFRESH command,
reporting progress with the standard devlink notifications.  The CPLD is
required to come back with DONE set, not busy and no error code before the
update is called successful, as drivers/fpga/machxo2-spi.c does.

The whole sequence runs under cpld_lock and the i2c adapter lock, so an
EEPROM read blocks for as long as programming takes; the alternative is
reading the TMC bus instead.  Flashing is privileged:
DEVLINK_CMD_FLASH_UPDATE carries GENL_ADMIN_PERM.

Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
---
 Documentation/ABI/testing/sysfs-timecard     |   4 +
 Documentation/networking/devlink/ptp_ocp.rst |  37 ++-
 drivers/ptp/ptp_ocp.c                        | 327 ++++++++++++++++++-
 3 files changed, 361 insertions(+), 7 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-timecard b/Documentation/ABI/testing/sysfs-timecard
index c440d8930384..6ddca452bc63 100644
--- a/Documentation/ABI/testing/sysfs-timecard
+++ b/Documentation/ABI/testing/sysfs-timecard
@@ -35,6 +35,10 @@ Description:	(RO, root only) The flags set in the status register of the
 		probe, and reported from that cached value as the fixed
 		"cpld.id" version by devlink dev info.
 
+		New CPLD firmware is programmed with devlink dev flash,
+		selecting the "fw.cpld" component; see
+		Documentation/networking/devlink/ptp_ocp.rst.
+
 What:		/sys/class/timecard/ocpN/available_clock_sources
 Date:		September 2021
 Contact:	Jonathan Lemon <jonathan.lemon@gmail.com>
diff --git a/Documentation/networking/devlink/ptp_ocp.rst b/Documentation/networking/devlink/ptp_ocp.rst
index 3f997cd9f3a3..37e9f1ebd1a8 100644
--- a/Documentation/networking/devlink/ptp_ocp.rst
+++ b/Documentation/networking/devlink/ptp_ocp.rst
@@ -5,7 +5,7 @@ ptp_ocp devlink support
 ========================
 
 This document describes the devlink features implemented by the ``ptp_ocp``
-device driver.
+device driver: the info versions it reports and its flash update support.
 
 Info versions
 =============
@@ -33,3 +33,38 @@ The ``ptp_ocp`` driver reports the following versions
        reprograms the on-card mux, so the driver does that once from its
        own worker and reports the cached value here; the version is
        omitted until that read has succeeded.
+   * - ``fw.cpld``
+     - running
+     - USERCODE of the image programmed into the TAP CPLD, formatted as
+       ``0x%08x``.  Read together with ``cpld.id`` and reported the same
+       way.  This is the component name to pass to ``devlink dev flash``
+       to update the CPLD.
+
+Flash update
+============
+
+The driver implements ``devlink dev flash`` for two separate targets,
+selected with the component name.
+
+.. list-table:: Flash components
+   :widths: 20 80
+
+   * - Component
+     - Description
+   * - (none)
+     - The card's own flash, written through the SPI controller the driver
+       exposes.  The card runs the new image after its next reset.
+   * - ``fw.cpld``
+     - The configuration flash of the TAP CPLD on ADVA TimeCard X1 boards,
+       programmed over I2C with the MachXO3 in-system programming commands
+       and activated with a REFRESH, so the new image runs immediately.
+       The image is the raw configuration bitstream, a whole number of
+       16-byte pages; container formats such as ``.jed`` are rejected.
+
+Programming the CPLD claims the shared I2C bus for the whole cycle, so
+reads of the card's EEPROM block until it completes.  Progress is reported
+with the standard devlink status notifications.
+
+Example::
+
+    $ devlink dev flash pci/0000:02:00.0 file adva-cpld.bin component fw.cpld
diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 98cb26077181..58b90b2b282f 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -434,12 +434,16 @@ struct ptp_ocp {
 	u8			*cpld_buf;
 	/* Lattice device ID; 0 if unread */
 	u32			cpld_id;
+	/* USERCODE of the programmed image; valid once cpld_id is set */
+	u32			cpld_usercode;
 	/* the one-shot ID read is finished, successfully or not */
 	bool			cpld_id_tried;
 	/* failed ID read attempts so far */
 	unsigned int		cpld_id_attempts;
 	/* x1 TAP CPLD present */
 	bool			has_cpld;
+	/* EN_CFG_TP issued but not yet REFRESH'd */
+	bool			cpld_in_config_mode;
 };
 
 #define OCP_REQ_TIMESTAMP	BIT(0)
@@ -476,6 +480,13 @@ static int ptp_ocp_adva_board_init(struct ptp_ocp *bp, struct ocp_resource *r);
 static const struct ocp_sma_op ocp_adva_sma_op;
 static const struct ocp_sma_op ocp_adva_x1_sma_op;
 
+/* Flash component naming the CPLD image, as reported by ->info_get(). */
+#define ADVA_CPLD_COMPONENT	"fw.cpld"
+
+static int adva_x1_cpld_flash(struct ptp_ocp *bp, struct devlink *devlink,
+			      const struct firmware *fw,
+			      struct netlink_ext_ack *extack);
+
 static const struct ocp_attr_group fb_timecard_groups[];
 
 static const struct ocp_attr_group art_timecard_groups[];
@@ -2158,6 +2169,17 @@ ptp_ocp_devlink_flash_update(struct devlink *devlink,
 	const char *msg;
 	int err;
 
+	if (params->component) {
+		if (!bp->has_cpld ||
+		    strcmp(params->component, ADVA_CPLD_COMPONENT)) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "unsupported flash component");
+			return -EOPNOTSUPP;
+		}
+
+		return adva_x1_cpld_flash(bp, devlink, params->fw, extack);
+	}
+
 	dev = ptp_ocp_find_flash(bp);
 	if (!dev) {
 		dev_err(&bp->pdev->dev, "Can't find Flash SPI adapter\n");
@@ -2180,6 +2202,8 @@ static int
 ptp_ocp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 			 struct netlink_ext_ack *extack)
 {
+	enum devlink_info_version_type ver_type =
+					DEVLINK_INFO_VERSION_TYPE_COMPONENT;
 	struct ptp_ocp *bp = devlink_priv(devlink);
 	const char *fw_image;
 	char buf[32];
@@ -2202,6 +2226,16 @@ ptp_ocp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 		err = devlink_info_version_fixed_put(req, "cpld.id", buf);
 		if (err)
 			return err;
+
+		/* The flashable component: naming it here is what lets
+		 * "devlink dev flash ... component fw.cpld" through, as the
+		 * core matches the name against the versions reported here.
+		 */
+		sprintf(buf, "0x%08x", READ_ONCE(bp->cpld_usercode));
+		err = devlink_info_version_running_put_ext(req, "fw.cpld", buf,
+							   ver_type);
+		if (err)
+			return err;
 	}
 
 	if (!bp->has_eeprom_data) {
@@ -4296,13 +4330,25 @@ static const struct ocp_attr_group art_timecard_groups[] = {
 
 /* Lattice LCMXO3LF ISC command codes */
 #define CPLD_CMD_READ_ID      0xE0000000UL
+#define CPLD_CMD_READ_USERCODE 0xC0000000UL
 #define CPLD_CMD_READ_STATUS  0x3C000000UL
+#define CPLD_CMD_EN_CFG_TP    0x74   /* enable config, transparent mode */
+#define CPLD_CMD_DIS_CFG      0x26
+#define CPLD_CMD_ERASE        0x0E
+#define CPLD_CMD_RESET_ADDR   0x46
+#define CPLD_CMD_WRITE_PAGE   0x70
+#define CPLD_CMD_SET_DONE     0x5E
+#define CPLD_CMD_REFRESH      0x79
+#define CPLD_PAGE_SIZE        16
+#define CPLD_POLL_US          10000  /* status poll interval while busy */
+#define CPLD_ERASE_MS         15000  /* config sector erase, datasheet max */
 #define CPLD_ID_MAX_ATTEMPTS  10     /* one per sync_work tick */
 
 /* Status register bit positions (Lattice LCMXO3LF datasheet) */
 #define CPLD_STATUS_DONE   BIT(8)
 #define CPLD_STATUS_BUSY   BIT(12)
 #define CPLD_STATUS_FAILED BIT(13)
+#define CPLD_STATUS_ERR    GENMASK(25, 23)
 
 /*
  * Issue one I2C transaction on the TMC bus: @cmd if not negative, then
@@ -4522,6 +4568,48 @@ static int adva_x1_mux_select(struct ptp_ocp *bp, int ch)
 	return adva_x1_i2c_xfer(bp, ADVA_MUX_ADDR, val, NULL, 0, NULL, 0);
 }
 
+/*
+ * Argument bytes that follow an ISC opcode.  Returns NULL with @nargs set
+ * when the arguments are all zero: adva_x1_i2c_xfer() zeroes the buffer.
+ */
+static const u8 *adva_x1_cpld_args(u8 cmd, u8 *nargs)
+{
+	static const u8 en_cfg_tp[] = { 0x08, 0x00 };
+	/* cfg sector only */
+	static const u8 erase_cfg[] = { 0x04, 0x00, 0x00 };
+
+	switch (cmd) {
+	case CPLD_CMD_EN_CFG_TP:
+		*nargs = sizeof(en_cfg_tp);
+		return en_cfg_tp;
+	case CPLD_CMD_ERASE:
+		*nargs = sizeof(erase_cfg);
+		return erase_cfg;
+	case CPLD_CMD_RESET_ADDR:
+	case CPLD_CMD_SET_DONE:
+		*nargs = 3;
+		return NULL;
+	case CPLD_CMD_DIS_CFG:
+	case CPLD_CMD_REFRESH:
+		*nargs = 2;
+		return NULL;
+	default:
+		*nargs = 0;
+		return NULL;
+	}
+}
+
+/* Send an ISC command with the fixed arguments that belong to it. */
+static int adva_x1_cpld_write(struct ptp_ocp *bp, u8 cmd)
+{
+	const u8 *args;
+	u8 nargs;
+
+	args = adva_x1_cpld_args(cmd, &nargs);
+
+	return adva_x1_i2c_xfer(bp, ADVA_CPLD_ADDR, cmd, args, nargs, NULL, 0);
+}
+
 /*
  * Send a 4-byte command and read back without an intermediate STOP: two
  * messages in one transfer is the Lattice write -> repeated START -> read,
@@ -4547,14 +4635,82 @@ static int adva_x1_cpld_read_status(struct ptp_ocp *bp, u32 *status)
 	return 0;
 }
 
+/* Poll the status register until the CPLD goes idle, or @max_ms elapses.
+ * The deadline is on wall time, so the I2C transactions count against it,
+ * and the status is read once more after it expires before giving up.
+ */
+static int adva_x1_cpld_wait_ready(struct ptp_ocp *bp, unsigned int max_ms)
+{
+	u32 status = 0;
+	int err = 0, ret;
+
+	ret = read_poll_timeout(adva_x1_cpld_read_status, err,
+				err || (status & CPLD_STATUS_FAILED) ||
+				!(status & CPLD_STATUS_BUSY),
+				CPLD_POLL_US, max_ms * USEC_PER_MSEC, false,
+				bp, &status);
+	if (ret)
+		return ret;
+	/* Keep the transport errno so it reaches userspace as-is; -EIO is
+	 * reserved for the CPLD itself reporting FAILED.
+	 */
+	if (err)
+		return err;
+	if (status & CPLD_STATUS_FAILED)
+		return -EIO;
+
+	return 0;
+}
+
+/* Wait for BUSY to clear, ignoring a pending cancel.  An ISC command issued
+ * while the part is still erasing or programming may not latch, so an abort
+ * has to wait the operation out before it can leave configuration mode.
+ */
+static int adva_x1_cpld_wait_idle(struct ptp_ocp *bp, unsigned int max_ms)
+{
+	u32 status = 0;
+	int err = 0, ret;
+
+	ret = read_poll_timeout(adva_x1_cpld_read_status, err,
+				err || !(status & CPLD_STATUS_BUSY),
+				CPLD_POLL_US, max_ms * USEC_PER_MSEC, false,
+				bp, &status);
+	if (ret)
+		return ret;
+
+	return err;
+}
+
+/* Leave transparent configuration mode.  cpld_in_config_mode stays set when
+ * DIS_CFG does not get through, so a later cleanup() tries again rather than
+ * recording an exit that never happened.
+ */
+static void adva_x1_cpld_exit_config(struct ptp_ocp *bp)
+{
+	int err;
+
+	if (!bp->cpld_in_config_mode)
+		return;
+
+	adva_x1_cpld_wait_idle(bp, CPLD_ERASE_MS);
+
+	err = adva_x1_cpld_write(bp, CPLD_CMD_DIS_CFG);
+	if (err)
+		dev_warn(&bp->pdev->dev,
+			 "CPLD left in configuration mode: %d\n", err);
+	else
+		bp->cpld_in_config_mode = false;
+}
+
 /*
- * Read the Lattice device ID into bp->cpld_id.  Done once, off the
- * unprivileged devlink path, which reports the cached value.  -EBUSY means
- * cpld_lock is held, so the caller can retry rather than wait behind a long
- * CPLD operation.
+ * Read the Lattice device ID and the image USERCODE under one bus claim.
+ * Done once, off the unprivileged devlink path, which reports the cached
+ * values.  -EBUSY means cpld_lock is held, so the caller can retry rather
+ * than wait behind a long CPLD operation.
  */
 static int adva_x1_cpld_read_id(struct ptp_ocp *bp)
 {
+	u32 id, usercode;
 	u8 data[4];
 	int ret;
 
@@ -4568,8 +4724,19 @@ static int adva_x1_cpld_read_id(struct ptp_ocp *bp)
 	if (ret)
 		goto release;
 	ret = adva_x1_cpld_cmd_read(bp, CPLD_CMD_READ_ID, data, 4);
-	if (!ret)
-		WRITE_ONCE(bp->cpld_id, get_unaligned_be32(data));
+	if (ret)
+		goto deselect;
+	id = get_unaligned_be32(data);
+
+	ret = adva_x1_cpld_cmd_read(bp, CPLD_CMD_READ_USERCODE, data, 4);
+	if (ret)
+		goto deselect;
+	usercode = get_unaligned_be32(data);
+
+	/* cpld_id is the validity flag for both, so publish it last. */
+	WRITE_ONCE(bp->cpld_usercode, usercode);
+	WRITE_ONCE(bp->cpld_id, id);
+deselect:
 	adva_x1_mux_select(bp, -1);
 release:
 	adva_x1_bus_release(bp);
@@ -4627,6 +4794,154 @@ cpld_status_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_ADMIN_RO(cpld_status);
 
+/*
+ * Program the CPLD configuration flash from @fw and activate it.  Selected
+ * by the "fw.cpld" flash component; the SPI flash keeps the default path.
+ * Runs under cpld_lock with the i2c adapter lock held, so an EEPROM read
+ * blocks for as long as programming takes.
+ */
+static int adva_x1_cpld_flash(struct ptp_ocp *bp, struct devlink *devlink,
+			      const struct firmware *fw,
+			      struct netlink_ext_ack *extack)
+{
+	size_t offset;
+	int err;
+	u32 st;
+
+	if (!fw->size || fw->size % CPLD_PAGE_SIZE) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "image must be a whole number of 16-byte pages");
+		return -EINVAL;
+	}
+
+	mutex_lock(&bp->cpld_lock);
+
+	err = adva_x1_bus_claim(bp);
+	if (err)
+		goto unlock;
+	err = adva_x1_mux_select(bp, ADVA_MUX_CHANNEL);
+	if (err)
+		goto release;
+
+	/* Set before EN_CFG_TP, not after: the CPLD may have entered config
+	 * mode even if the write errors or the wait times out, and only this
+	 * makes the exit path send DIS_CFG.  A stray DIS_CFG is harmless;
+	 * leaving config mode enabled is not.
+	 */
+	bp->cpld_in_config_mode = true;
+
+	err = adva_x1_cpld_write(bp, CPLD_CMD_EN_CFG_TP);
+	if (!err)
+		err = adva_x1_cpld_wait_ready(bp, 5000);
+	if (err)
+		goto exit_config;
+
+	devlink_flash_update_status_notify(devlink, "Erasing",
+					   ADVA_CPLD_COMPONENT, 0, 0);
+	err = adva_x1_cpld_write(bp, CPLD_CMD_ERASE);
+	if (!err)
+		err = adva_x1_cpld_wait_ready(bp, CPLD_ERASE_MS);
+	if (err)
+		goto exit_config;
+
+	err = adva_x1_cpld_write(bp, CPLD_CMD_RESET_ADDR);
+	if (err)
+		goto exit_config;
+
+	for (offset = 0; offset < fw->size; offset += CPLD_PAGE_SIZE) {
+		u8 args[3 + CPLD_PAGE_SIZE] = { 0x00, 0x00, 0x01 };
+
+		memcpy(&args[3], fw->data + offset, CPLD_PAGE_SIZE);
+		err = adva_x1_i2c_xfer(bp, ADVA_CPLD_ADDR, CPLD_CMD_WRITE_PAGE,
+				       args, sizeof(args), NULL, 0);
+		if (!err)
+			err = adva_x1_cpld_wait_ready(bp, 100);
+		if (err)
+			goto exit_config;
+
+		if (!(offset % (CPLD_PAGE_SIZE * 64)))
+			devlink_flash_update_status_notify(devlink,
+							   "Programming",
+							   ADVA_CPLD_COMPONENT,
+							   offset, fw->size);
+	}
+	devlink_flash_update_status_notify(devlink, "Programming",
+					   ADVA_CPLD_COMPONENT,
+					   fw->size, fw->size);
+
+	err = adva_x1_cpld_write(bp, CPLD_CMD_SET_DONE);
+	if (!err)
+		err = adva_x1_cpld_wait_ready(bp, 1000);
+	if (err)
+		goto exit_config;
+
+	err = adva_x1_cpld_read_status(bp, &st);
+	if (err)
+		goto exit_config;
+	if (!(st & CPLD_STATUS_DONE)) {
+		err = -EIO;
+		goto exit_config;
+	}
+
+	devlink_flash_update_status_notify(devlink, "Activating",
+					   ADVA_CPLD_COMPONENT, 0, 0);
+	err = adva_x1_cpld_write(bp, CPLD_CMD_REFRESH);
+	if (err)
+		goto exit_config;
+
+	/* REFRESH reboots the CPLD out of configuration mode, so the exit
+	 * path must not send DIS_CFG afterwards even if a check below fails.
+	 */
+	bp->cpld_in_config_mode = false;
+
+	/* The new image is already running, so a segment that is not back
+	 * yet must not be reported as a failed update: retry the reselect
+	 * rather than sampling the mux once at a fixed delay.
+	 */
+	msleep(1500);
+	err = read_poll_timeout(adva_x1_mux_select, err, !err, CPLD_POLL_US,
+				3000 * USEC_PER_MSEC, false,
+				bp, ADVA_MUX_CHANNEL);
+	if (err)
+		goto deselect;
+
+	err = adva_x1_cpld_wait_ready(bp, 3000);
+	if (err)
+		goto deselect;
+
+	/* 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.
+	 */
+	err = adva_x1_cpld_read_status(bp, &st);
+	if (err)
+		goto deselect;
+	if (!(st & CPLD_STATUS_DONE) || (st & CPLD_STATUS_BUSY) ||
+	    (st & CPLD_STATUS_ERR)) {
+		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");
+		err = -EIO;
+		goto deselect;
+	}
+
+	/* The image on the part changed, so the cached USERCODE has too. */
+	WRITE_ONCE(bp->cpld_id, 0);
+	WRITE_ONCE(bp->cpld_id_tried, false);
+	bp->cpld_id_attempts = 0;
+
+exit_config:
+	adva_x1_cpld_exit_config(bp);
+deselect:
+	adva_x1_mux_select(bp, -1);
+release:
+	adva_x1_bus_release(bp);
+unlock:
+	mutex_unlock(&bp->cpld_lock);
+
+	return err;
+}
+
 static struct attribute *adva_timecard_attrs[] = {
 	&dev_attr_serialnum.attr,
 	&dev_attr_gnss_sync.attr,
-- 
2.47.0

Re: [PATCH net-next v14 3/4] ptp: ocp: add TAP CPLD access for ADVA TimeCard X1

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 5 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 3 · Low: 2

- [Medium] adva_x1_mblaze_release() (drivers/ptp/ptp_ocp.c:4375-4393)
  detects that the MicroBlaze firmware never dropped the grant, but only…
- [Medium] adva_x1_mblaze_release() (drivers/ptp/ptp_ocp.c:4382-4390)
  writes MBLAZE_RELEASE (0x55550000) to pps_select->i2c_bus_ctrl and…
- [Medium] bp->cpld_i2c_adap_nr has two unsynchronized writers -
  ptp_ocp_i2c_notifier_call() (drivers/ptp/ptp_ocp.c:5981-5986) and…
- [Low] This commit adds #include <linux/iopoll.h>
  (drivers/ptp/ptp_ocp.c:23) and #include <linux/firmware.h> (line 30),…
- [Low] In both adva_x1_cpld_read_id() and cpld_status_show(), a failure
  of adva_x1_mux_select(bp, ADVA_MUX_CHANNEL) takes 'goto release',…

--

ptp: ocp: add TAP CPLD access for ADVA TimeCard X1

This adds a request/grant handshake with the on-card MicroBlaze firmware so
the host I2C controller can be re-routed onto the TMC bus, where the PCA9548
and the Lattice TAP CPLD live.  On top of that it adds a root-only
cpld_status sysfs attribute and reports the Lattice device ID as the fixed
devlink version cpld.id, read once from the driver's own worker.

A few questions on the arbitration and on the adapter-number caching below.

> diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> index f4299878c27c..98cb26077181 100644
> --- a/drivers/ptp/ptp_ocp.c
> +++ b/drivers/ptp/ptp_ocp.c
> @@ -20,10 +20,14 @@
>  #include <linux/spi/altera.h>
>  #include <net/devlink.h>
>  #include <linux/i2c.h>
> +#include <linux/iopoll.h>
>  #include <linux/mtd/mtd.h>
>  #include <linux/nvmem-consumer.h>
>  #include <linux/crc16.h>
>  #include <linux/dpll.h>
> +#include <linux/unaligned.h>
> +#include <linux/delay.h>
> +#include <linux/firmware.h>

[Severity: Low]
This isn't a bug, but is linux/iopoll.h used by anything in this commit?
There is no read_poll_timeout() or readx_poll_timeout() caller here; the
first one, adva_x1_cpld_wait_idle(), arrives with the next patch in the
series.

The same applies to the comments in adva_x1_i2c_xfer() and
adva_x1_bus_claim() that justify the per-claim scratch buffer with an image
upload:

	/* One scratch buffer per claim rather than per transfer: an upload
	 * holds the claim for the whole image.
	 */

There is no upload path in this patch either.  Would it be tidier to move
the include (and possibly those sentences) to the patch that introduces the
flashing code?

[ ... ]

> @@ -4226,6 +4266,367 @@ static const struct ocp_attr_group art_timecard_groups[] = {
>  	{ },
>  };
>  
> +/*
> + * adva_x1 TAP CPLD (Lattice LCMXO3LF-2100) helpers.
> + *
> + * The card has two I2C controllers; Linux registers only 0x00150000.
> + * The i2c_bus_ctrl handshake re-routes what each is wired to:
> + *
> + *   grant 0:  0x150000 -> EEPROMs     0x120000 -> TMC or M.2, MB's choice
> + *   grant 1:  0x150000 -> TMC bus     0x120000 -> M.2
> + *
> + * The PCA9548 at 0x74 and the CPLD at 0x40 behind its channel 0 are on the
> + * TMC bus, reachable only while the grant is held.  The EEPROMs are not
> + * behind the adapter for that window, so a CPLD operation holds cpld_lock
> + * and the adapter lock to keep the EEPROM and nvmem paths off it.
> + */
> +
> +#define ADVA_MUX_ADDR     0x74
> +#define ADVA_CPLD_ADDR    0x40
> +#define ADVA_MUX_CHANNEL  0
> +
> +/* Longest command or reply; bounds each half of the per-claim scratch. */
> +#define ADVA_CPLD_XFER_MAX 32
> +
> +#define MBLAZE_REQUEST    0x0000aaaaU
> +#define MBLAZE_GRANTED    0x5555aaaaU
> +#define MBLAZE_RELEASE    0x55550000U
> +#define MBLAZE_RETRIES    200
> +#define MBLAZE_RETRY_US   10000

[ ... ]

> +/* Hand the segment back, and wait for the firmware to drop the grant before
> + * the caller unlocks the adapter: a transfer queued behind us would
> + * otherwise be clocked out while the controller is still on the TMC bus,
> + * which is what holding the adapter lock across the grant exists to stop.
> + */
> +static void adva_x1_mblaze_release(struct ptp_ocp *bp)
> +{
> +	int i;
> +
> +	if (!bp->pps_select)
> +		return;
> +
> +	iowrite32(MBLAZE_RELEASE, &bp->pps_select->i2c_bus_ctrl);
> +	/* Flush the posted write, as the acquire path does. */
> +	ioread32(&bp->pps_select->i2c_bus_ctrl);
> +
> +	for (i = 0; i < MBLAZE_RETRIES; i++) {
> +		if (ioread32(&bp->pps_select->i2c_bus_ctrl) != MBLAZE_GRANTED)
> +			return;
> +		usleep_range(MBLAZE_RETRY_US, MBLAZE_RETRY_US + 1000);
> +	}

[Severity: Medium]
Can this loop ever iterate more than once?

The three magic values look like a host-owned low half (0xaaaa = request) and
a firmware-owned high half (0x5555 = grant):

	MBLAZE_REQUEST    0x0000aaaa
	MBLAZE_GRANTED    0x5555aaaa
	MBLAZE_RELEASE    0x55550000

adva_x1_mblaze_acquire() writes 0x0000aaaa and waits for the composite
0x5555aaaa, so the value read back includes the request bits the host itself
wrote.

If the readback here behaves the same way, then writing 0x55550000 clears the
low half, the very first ioread32() returns something other than 0x5555aaaa,
and adva_x1_mblaze_release() returns straight away - even while the firmware
still has the controller wired to the TMC bus.  The dev_warn() below would
then be unreachable.

Should the predicate look only at the firmware-owned field, for example
waiting for (val & 0xffff0000) == 0, rather than comparing against a value
that contains the host's own request bits?

Or is i2c_bus_ctrl a write-only mailbox whose readback returns firmware state
alone?  If so it would be worth saying that in the comment, since the
guarantee stated in the changelog ("The hand-back waits for the firmware to
drop the grant before the adapter lock is released") rests entirely on this
loop.

> +
> +	dev_warn(&bp->pdev->dev, "TMC bus still granted after release\n");
> +}

[ ... ]

> +/* Route the host controller back to the EEPROMs and release the adapter.
> + * Safe after a failed claim: it also clears a request that was never granted.
> + */
> +static void adva_x1_bus_release(struct ptp_ocp *bp)
> +{
> +	struct i2c_adapter *adap = bp->cpld_adap;
> +
> +	if (!adap)
> +		return;
> +
> +	adva_x1_mblaze_release(bp);
> +	bp->cpld_adap = NULL;
> +	kfree(bp->cpld_buf);
> +	bp->cpld_buf = NULL;
> +	i2c_unlock_bus(adap, I2C_LOCK_ROOT_ADAPTER);
> +	i2c_put_adapter(adap);
> +}

[Severity: Medium]
When the hand-back times out, adva_x1_mblaze_release() only logs "TMC bus
still granted after release" and returns void, so adva_x1_bus_release() has
no way to learn about it and drops i2c_unlock_bus() / i2c_put_adapter()
anyway.

What happens to the next transfer on that adapter if the controller is still
routed to the TMC segment?  ptp_ocp_read_eeprom() is reachable from the
unprivileged DEVLINK_CMD_INFO_GET path:

	if (!bp->has_eeprom_data) {
		ptp_ocp_read_eeprom(bp);

and also from serialnum_show() and the at24 / nvmem sysfs files.  Since an
at24 read starts with an address-byte write to 0x50 or 0x58, this would not
just return TMC data as EEPROM content but also clock writes at whatever sits
at those addresses on the TMC segment.

Would it make sense for adva_x1_mblaze_release() to return an error and for
adva_x1_bus_release() to keep the adapter locked (or otherwise stop using it)
when the grant was never dropped?

[ ... ]

> +/*
> + * Cache the adapter the CPLD hangs off.  ptp_ocp_i2c_notifier_call() misses
> + * it when the i2c controller driver is already loaded: the adapter is then
> + * added from inside platform_device_register(), before bp->i2c_ctrl is set.
> + * Only direct children are searched, so a mux adapter cannot match.
> + */
> +static void adva_x1_cache_i2c_adap(struct ptp_ocp *bp)
> +{
> +	struct device *child;
> +
> +	if (!bp->has_cpld || !bp->i2c_ctrl ||
> +	    READ_ONCE(bp->cpld_i2c_adap_nr) >= 0)
> +		return;
> +
> +	child = device_find_child(&bp->i2c_ctrl->dev, NULL,
> +				  adva_x1_i2c_adap_match);
> +	if (!child)
> +		return;
> +
> +	WRITE_ONCE(bp->cpld_i2c_adap_nr, i2c_verify_adapter(child)->nr);
> +	put_device(child);
> +}

[Severity: Medium]
bp->cpld_i2c_adap_nr now has two unlocked writers doing check-then-act:
this function from probe, and ptp_ocp_i2c_notifier_call().  Can both miss?

device_add() emits the notification before linking the device to its parent:

drivers/base/core.c:device_add() {
	...
	bus_notify(dev, BUS_NOTIFY_ADD_DEVICE);
	...
	if (parent)
		klist_add_tail(&dev->p->knode_parent,
			       &parent->p->klist_children);
	...
}

and bp->i2c_ctrl is only assigned after platform_device_register() returns.
So if the adapter's device_add() is in flight on another thread
(asynchronous probing via driver_async_probe= or the modprobe async_probe
parameter), the notifier skips its store because bp->i2c_ctrl is still NULL,
while device_find_child(&bp->i2c_ctrl->dev, ...) here does not see the
not-yet-linked adapter.

Nothing re-runs adva_x1_cache_i2c_adap(), and ptp_ocp_sync_work() gates on

	READ_ONCE(bp->cpld_i2c_adap_nr) >= 0

so cpld_i2c_adap_nr would stay -1 for the lifetime of the device: cpld.id
never reported, cpld_status returning -ENODEV.

The reverse interleaving looks possible too - this function publishing a
number after a BUS_NOTIFY_DEL_DEVICE has already set it to -1 - in which case
the stale number makes adva_x1_bus_claim() fail its parent re-check ten times
and latch cpld_id_tried.

READ_ONCE()/WRITE_ONCE() stop tearing but do not make the test and the store
atomic here.  Would serializing both writers under cpld_lock, or retrying the
lookup from the worker instead of only once at probe, close this?

> +
> +/*
> + * Claim the TMC bus.  The adapter lock is held across the handshake to keep
> + * the EEPROM, nvmem and at24 paths off the controller while it is routed
> + * away from the EEPROMs; an EEPROM read blocks until the claim is dropped.
> + */
> +static int adva_x1_bus_claim(struct ptp_ocp *bp)

[ ... ]

> +/* Select a mux channel, or deselect all with ch < 0 - the power-on state.
> + * The mux is on the TMC bus, so what it is left set to never affects the
> + * EEPROM paths.
> + */
> +static int adva_x1_mux_select(struct ptp_ocp *bp, int ch)
> +{
> +	u8 val = (ch >= 0) ? BIT(ch) : 0;
> +
> +	return adva_x1_i2c_xfer(bp, ADVA_MUX_ADDR, val, NULL, 0, NULL, 0);
> +}

[ ... ]

> +static int adva_x1_cpld_read_id(struct ptp_ocp *bp)
> +{
> +	u8 data[4];
> +	int ret;
> +
> +	if (!mutex_trylock(&bp->cpld_lock))
> +		return -EBUSY;
> +
> +	ret = adva_x1_bus_claim(bp);
> +	if (ret)
> +		goto out;
> +	ret = adva_x1_mux_select(bp, ADVA_MUX_CHANNEL);
> +	if (ret)
> +		goto release;
> +	ret = adva_x1_cpld_cmd_read(bp, CPLD_CMD_READ_ID, data, 4);
> +	if (!ret)
> +		WRITE_ONCE(bp->cpld_id, get_unaligned_be32(data));
> +	adva_x1_mux_select(bp, -1);
> +release:
> +	adva_x1_bus_release(bp);

[Severity: Low]
When adva_x1_mux_select(bp, ADVA_MUX_CHANNEL) fails, the goto release jumps
past the matching adva_x1_mux_select(bp, -1), so the PCA9548 at 0x74 is
handed back to the MicroBlaze with channel 0 possibly still enabled.  An I2C
transfer can be reported as failed after the mux control byte was already
acknowledged (controller timeout or arbitration handling), so a non-zero
return does not prove the register was not written.

The comment on adva_x1_mux_select() describes deselect-all as the state the
driver restores, so should the select-failure path deselect as well?

On the success path the return value of adva_x1_mux_select(bp, -1) is
discarded, so a failed deselect also leaves channel 0 open with no
indication.  cpld_status_show() has the same two paths.

> +out:
> +	mutex_unlock(&bp->cpld_lock);
> +	if (ret)
> +		dev_dbg(&bp->pdev->dev,
> +			"CPLD device ID unavailable: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +/* cpld_status - show the flags set in the TAP CPLD status register. */
> +static ssize_t
> +cpld_status_show(struct device *dev, struct device_attribute *attr,
> +		 char *buf)
> +{

[ ... ]

> +	ret = adva_x1_bus_claim(bp);
> +	if (ret)
> +		goto out;
> +	ret = adva_x1_mux_select(bp, ADVA_MUX_CHANNEL);
> +	if (ret)
> +		goto release;
> +	ret = adva_x1_cpld_read_status(bp, &st);
> +	adva_x1_mux_select(bp, -1);
> +release:
> +	adva_x1_bus_release(bp);

[ ... ]

> @@ -5052,6 +5455,20 @@ ptp_ocp_sync_work(struct work_struct *work)
>  
>  	bp->sync = sync;
>  
> +	/* The adapter can appear after probe, so read the ID from here once
> +	 * it has.  A claim can fail transiently - the firmware may not grant
> +	 * the segment straight after power-up - so retry a bounded number of
> +	 * times.  -EBUSY means cpld_lock is held and does not count.
> +	 */
> +	if (bp->has_cpld && !READ_ONCE(bp->cpld_id_tried) &&
> +	    READ_ONCE(bp->cpld_i2c_adap_nr) >= 0) {
> +		int err = adva_x1_cpld_read_id(bp);
> +
> +		if (err != -EBUSY &&
> +		    (!err || ++bp->cpld_id_attempts >= CPLD_ID_MAX_ATTEMPTS))
> +			WRITE_ONCE(bp->cpld_id_tried, true);
> +	}
> +
>  	queue_delayed_work(system_power_efficient_wq, &bp->sync_work, HZ);
>  }

[ ... ]

> @@ -5227,11 +5656,21 @@ ptp_ocp_i2c_notifier_call(struct notifier_block *nb,
>  
>  found:
>  	bp = dev_get_drvdata(dev);
> -	if (add)
> +	adap = i2c_verify_adapter(child);
> +	if (add) {
>  		ptp_ocp_symlink(bp, child, "i2c");
> -	else
> +		/* Only the card's own controller: a mux child added on it
> +		 * reaches ptp_ocp through the same walk.  Numbers are
> +		 * recycled, so the claim re-checks the parent itself.
> +		 */
> +		if (bp->i2c_ctrl && adap->dev.parent == &bp->i2c_ctrl->dev)
> +			WRITE_ONCE(bp->cpld_i2c_adap_nr, adap->nr);
> +	} else {
> +		/* invalidate before free, but only for the cached adapter */
> +		if (READ_ONCE(bp->cpld_i2c_adap_nr) == adap->nr)
> +			WRITE_ONCE(bp->cpld_i2c_adap_nr, -1);
>  		sysfs_remove_link(&bp->dev.kobj, "i2c");
> -
> +	}
>  	return 0;
>  }

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914071531.11640-1-maimon.sagi%40gmail.com

Re: [PATCH net-next v14 4/4] ptp: ocp: add TAP CPLD flashing via devlink

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 10 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 8 · Low: 2

- [Medium] The new `fw.cpld` flash component only exists while the driver
  happens to hold a successfully cached CPLD IDCODE.
- [Medium] bp->cpld_id / bp->cpld_usercode are invalidated only on the
  success path of adva_x1_cpld_flash()…
- [Medium] Documentation/networking/devlink/ptp_ocp.rst (line 60) states
  "container formats such as ``.jed`` are rejected", but…
- [Medium] The CPLD "transparent configuration mode" entered by
  CPLD_CMD_EN_CFG_TP is tracked by `bp->cpld_in_config_mode`, but the…
- [Medium] adva_x1_cpld_flash() invalidates the cached CPLD identity
  (`WRITE_ONCE(bp->cpld_id, 0); WRITE_ONCE(bp->cpld_id_tried, false);…
- [Medium] The only validation applied to the user-supplied bitstream in
  adva_x1_cpld_flash() is `if (!fw->size || fw->size % CPLD_PAGE_SIZE)`…
- [Medium] The ISC command frame lengths for CPLD_CMD_EN_CFG_TP (0x74),
  CPLD_CMD_DIS_CFG (0x26) and CPLD_CMD_REFRESH (0x79) disagree with…
- [Medium] The programming sequence in adva_x1_cpld_flash() never issues
  ISC_PROGRAM_USERCODE (0xC2) - the CPLD_CMD_* table added by this patch…
- [Low] Documentation states that the CPLD identification read - which
  claims the shared I2C bus and reprograms the on-card mux - happens…
- [Low] adva_x1_cpld_read_id() publishes bp->cpld_usercode and then
  bp->cpld_id with plain WRITE_ONCE() and an explicit comment that…

--

ptp: ocp: add TAP CPLD flashing via devlink

This adds a devlink flash component named fw.cpld that programs the Lattice
MachXO3 configuration flash on ADVA TimeCard X1 boards over I2C, reusing the
TMC bus arbitration added earlier.  ->info_get() also starts publishing the
image USERCODE as the running version of that component.

A few questions on the new code below.

> diff --git a/Documentation/ABI/testing/sysfs-timecard b/Documentation/ABI/testing/sysfs-timecard
> index c440d8930384..6ddca452bc63 100644
> --- a/Documentation/ABI/testing/sysfs-timecard
> +++ b/Documentation/ABI/testing/sysfs-timecard
> @@ -35,6 +35,10 @@ Description:	(RO, root only) The flags set in the status register of the
>  		probe, and reported from that cached value as the fixed
>  		"cpld.id" version by devlink dev info.
>  
> +		New CPLD firmware is programmed with devlink dev flash,
> +		selecting the "fw.cpld" component; see
> +		Documentation/networking/devlink/ptp_ocp.rst.
> +

[Severity: Low]
This file still describes the identification read as a one-time event ("The
Lattice device ID of the CPLD is read once by the driver, shortly after
probe"), and ptp_ocp.rst says "the driver does that once from its own
worker".

After this patch adva_x1_cpld_flash() re-arms the one-shot gate on success:

	WRITE_ONCE(bp->cpld_id, 0);
	WRITE_ONCE(bp->cpld_id_tried, false);
	bp->cpld_id_attempts = 0;

and ptp_ocp_sync_work() only skips the read while cpld_id_tried is true, so
the bus-claiming, mux-reprogramming ID/USERCODE read runs again after every
successful CPLD update.

Should both documents be reworded now that the read is no longer once per
binding?

> diff --git a/Documentation/networking/devlink/ptp_ocp.rst b/Documentation/networking/devlink/ptp_ocp.rst
> index 3f997cd9f3a3..37e9f1ebd1a8 100644
> --- a/Documentation/networking/devlink/ptp_ocp.rst
> +++ b/Documentation/networking/devlink/ptp_ocp.rst

[ ... ]

> @@ -33,3 +33,38 @@ The ``ptp_ocp`` driver reports the following versions
>         reprograms the on-card mux, so the driver does that once from its
>         own worker and reports the cached value here; the version is
>         omitted until that read has succeeded.
> +   * - ``fw.cpld``
> +     - running
> +     - USERCODE of the image programmed into the TAP CPLD, formatted as
> +       ``0x%08x``.  Read together with ``cpld.id`` and reported the same
> +       way.  This is the component name to pass to ``devlink dev flash``
> +       to update the CPLD.

[ ... ]

> +   * - ``fw.cpld``
> +     - The configuration flash of the TAP CPLD on ADVA TimeCard X1 boards,
> +       programmed over I2C with the MachXO3 in-system programming commands
> +       and activated with a REFRESH, so the new image runs immediately.
> +       The image is the raw configuration bitstream, a whole number of
> +       16-byte pages; container formats such as ``.jed`` are rejected.

[Severity: Medium]
Where are container formats rejected?  The only input check in
adva_x1_cpld_flash() is the length test:

	if (!fw->size || fw->size % CPLD_PAGE_SIZE) {
		NL_SET_ERR_MSG_MOD(extack,
				   "image must be a whole number of 16-byte pages");
		return -EINVAL;
	}

There is no preamble, magic or CRC check, so any file whose size happens to
be a multiple of 16 (a 4096-byte .jed, for instance) is accepted and copied
straight into the LSC_PROG_INCR_NV frames.

Would it be better to either add a real format check or to drop the claim
that .jed files are rejected?

> diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> index 98cb26077181..58b90b2b282f 100644
> --- a/drivers/ptp/ptp_ocp.c
> +++ b/drivers/ptp/ptp_ocp.c

[ ... ]

> @@ -2158,6 +2169,17 @@ ptp_ocp_devlink_flash_update(struct devlink *devlink,
>  	const char *msg;
>  	int err;
>  
> +	if (params->component) {
> +		if (!bp->has_cpld ||
> +		    strcmp(params->component, ADVA_CPLD_COMPONENT)) {
> +			NL_SET_ERR_MSG_MOD(extack,
> +					   "unsupported flash component");
> +			return -EOPNOTSUPP;
> +		}
> +
> +		return adva_x1_cpld_flash(bp, devlink, params->fw, extack);
> +	}
> +

[Severity: Medium]
This gate accepts the component whenever bp->has_cpld is set, but
ptp_ocp_devlink_info_get() only publishes the name inside a narrower
condition:

	id = READ_ONCE(bp->cpld_id);
	if (bp->has_cpld && id) {
		...
		err = devlink_info_version_running_put_ext(req, "fw.cpld", buf,
							   ver_type);

The core resolves the component name only through info_get():

net/devlink/dev.c:devlink_flash_component_get() {
	...
	if (!lookup_ctx.lookup_name_found) {
		NL_SET_ERR_MSG_ATTR(extack, nla_component,
				    "selected component is not supported by this device");
		return -EINVAL;
	}
}

so the check here never runs for a mismatched name, and the stricter
info_get() condition decides whether fw.cpld can be flashed at all.

Can that make the CPLD unflashable in the cases where flashing is the
recovery action?  ptp_ocp_sync_work() latches the one-shot read:

	if (err != -EBUSY &&
	    (!err || ++bp->cpld_id_attempts >= CPLD_ID_MAX_ATTEMPTS))
		WRITE_ONCE(bp->cpld_id_tried, true);

Once ten non-EBUSY failures have happened, cpld_id stays 0 for the lifetime
of the binding, fw.cpld is never reported, and devlink refuses the request
with -EINVAL.

Would deriving the capability from the static has_cpld board flag (or always
emitting the version string, with an unknown value when the read has not
succeeded) be more robust than deriving it from the opportunistic cache?

[ ... ]

> @@ -4522,6 +4568,48 @@ static int adva_x1_mux_select(struct ptp_ocp *bp, int ch)
>  	return adva_x1_i2c_xfer(bp, ADVA_MUX_ADDR, val, NULL, 0, NULL, 0);
>  }
>  
> +/*
> + * Argument bytes that follow an ISC opcode.  Returns NULL with @nargs set
> + * when the arguments are all zero: adva_x1_i2c_xfer() zeroes the buffer.
> + */
> +static const u8 *adva_x1_cpld_args(u8 cmd, u8 *nargs)
> +{
> +	static const u8 en_cfg_tp[] = { 0x08, 0x00 };
> +	/* cfg sector only */
> +	static const u8 erase_cfg[] = { 0x04, 0x00, 0x00 };
> +
> +	switch (cmd) {
> +	case CPLD_CMD_EN_CFG_TP:
> +		*nargs = sizeof(en_cfg_tp);
> +		return en_cfg_tp;
> +	case CPLD_CMD_ERASE:
> +		*nargs = sizeof(erase_cfg);
> +		return erase_cfg;
> +	case CPLD_CMD_RESET_ADDR:
> +	case CPLD_CMD_SET_DONE:
> +		*nargs = 3;
> +		return NULL;
> +	case CPLD_CMD_DIS_CFG:
> +	case CPLD_CMD_REFRESH:
> +		*nargs = 2;
> +		return NULL;
> +	default:
> +		*nargs = 0;
> +		return NULL;
> +	}
> +}

[Severity: Medium]
Are the operand counts for 0x74, 0x26 and 0x79 right?  These three produce
3-byte frames here, while the in-tree Lattice drivers send opcode plus three
operand bytes:

drivers/fpga/machxo2-spi.c:
	#define ISC_ENABLE		{0xc6, 0x08, 0x00, 0x00}
	#define LSC_REFRESH		{0x79, 0x00, 0x00, 0x00}

drivers/fpga/lattice-sysconfig.h:
	#define	SYSCONFIG_ISC_DISABLE		{0x26, 0x00, 0x00, 0x00}
	#define	SYSCONFIG_LSC_REFRESH		{0x79, 0x00, 0x00, 0x00}

Every other frame this patch builds (READ_ID, READ_STATUS, READ_USERCODE,
ERASE {0x0e,0x04,0x00,0x00}, RESET_ADDR, SET_DONE, WRITE_PAGE) matches those
drivers byte for byte, which makes EN_CFG_TP, DIS_CFG and REFRESH the
outliers.  Could the operand counts be quoted against FPGA-TN-02055?

Related: nothing checks that configuration mode was actually entered.
adva_x1_cpld_wait_ready() looks only at BUSY and FAILED, and the ENAB bit
(bit 9, cfgena in machxo2-spi.c) is never read.  If the enable frame is not
latched, an idle part reports neither BUSY nor FAILED, the erase and page
writes are ignored, DONE is still set from the old image, and the final
DONE/!BUSY/ERR check after REFRESH passes - so the update is reported
successful with the configuration flash untouched.  Should the status be
read after EN_CFG_TP and the update fail with -EIO when ENAB is clear?

[ ... ]

> @@ -4547,14 +4635,82 @@ static int adva_x1_cpld_read_status(struct ptp_ocp *bp, u32 *status)

[ ... ]

> +/* Wait for BUSY to clear, ignoring a pending cancel.  An ISC command issued
> + * while the part is still erasing or programming may not latch, so an abort
> + * has to wait the operation out before it can leave configuration mode.
> + */
> +static int adva_x1_cpld_wait_idle(struct ptp_ocp *bp, unsigned int max_ms)
> +{

[ ... ]

> +/* Leave transparent configuration mode.  cpld_in_config_mode stays set when
> + * DIS_CFG does not get through, so a later cleanup() tries again rather than
> + * recording an exit that never happened.
> + */
> +static void adva_x1_cpld_exit_config(struct ptp_ocp *bp)
> +{
> +	int err;
> +
> +	if (!bp->cpld_in_config_mode)
> +		return;
> +
> +	adva_x1_cpld_wait_idle(bp, CPLD_ERASE_MS);
> +
> +	err = adva_x1_cpld_write(bp, CPLD_CMD_DIS_CFG);
> +	if (err)
> +		dev_warn(&bp->pdev->dev,
> +			 "CPLD left in configuration mode: %d\n", err);
> +	else
> +		bp->cpld_in_config_mode = false;
> +}

[Severity: Medium]
Which cleanup() does this comment refer to?  In this tree
cpld_in_config_mode is referenced only in the struct declaration, in
adva_x1_cpld_flash() and in this function, and adva_x1_cpld_flash() sets it
unconditionally on every entry:

	bp->cpld_in_config_mode = true;

so nothing ever observes the "stays set" state and retries the DIS_CFG.
ptp_ocp_detach() destroys cpld_lock and unregisters the device without
claiming the bus or sending DIS_CFG, so if DIS_CFG errors here the part is
left in transparent configuration mode past unbind - the state the comment
in adva_x1_cpld_flash() calls unsafe ("A stray DIS_CFG is harmless; leaving
config mode enabled is not").

Also, the comment above adva_x1_cpld_wait_idle() describes waiting "ignoring
a pending cancel" for "an abort", but there is no cancel or abort path in
this sequence - read_poll_timeout() and msleep() are uninterruptible.  Is
the only real difference from adva_x1_cpld_wait_ready() that FAILED is not
terminal?  If so, could the comments be reworded to match?

>  /*
> - * Read the Lattice device ID into bp->cpld_id.  Done once, off the
> - * unprivileged devlink path, which reports the cached value.  -EBUSY means
> - * cpld_lock is held, so the caller can retry rather than wait behind a long
> - * CPLD operation.
> + * Read the Lattice device ID and the image USERCODE under one bus claim.
> + * Done once, off the unprivileged devlink path, which reports the cached
> + * values.  -EBUSY means cpld_lock is held, so the caller can retry rather
> + * than wait behind a long CPLD operation.
>   */
>  static int adva_x1_cpld_read_id(struct ptp_ocp *bp)
>  {

[ ... ]

> @@ -4568,8 +4724,19 @@ static int adva_x1_cpld_read_id(struct ptp_ocp *bp)
>  	if (ret)
>  		goto release;
>  	ret = adva_x1_cpld_cmd_read(bp, CPLD_CMD_READ_ID, data, 4);
> -	if (!ret)
> -		WRITE_ONCE(bp->cpld_id, get_unaligned_be32(data));
> +	if (ret)
> +		goto deselect;
> +	id = get_unaligned_be32(data);
> +
> +	ret = adva_x1_cpld_cmd_read(bp, CPLD_CMD_READ_USERCODE, data, 4);
> +	if (ret)
> +		goto deselect;
> +	usercode = get_unaligned_be32(data);
> +
> +	/* cpld_id is the validity flag for both, so publish it last. */
> +	WRITE_ONCE(bp->cpld_usercode, usercode);
> +	WRITE_ONCE(bp->cpld_id, id);

[Severity: Low]
Is the publish ordering the comment relies on actually enforced?
WRITE_ONCE() gives no store-store ordering, and the reader side in
ptp_ocp_devlink_info_get() has no matching load-load ordering:

	id = READ_ONCE(bp->cpld_id);
	if (bp->has_cpld && id) {
		...
		sprintf(buf, "0x%08x", READ_ONCE(bp->cpld_usercode));

On arm64 or powerpc a concurrent devlink dev info could see a valid cpld_id
with a stale or zero cpld_usercode and print fw.cpld 0x00000000.  Would
smp_store_release()/smp_load_acquire() (or smp_wmb()/smp_rmb()) be the right
pairing here?

> @@ -4627,6 +4794,154 @@ cpld_status_show(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_ADMIN_RO(cpld_status);
>  
> +/*
> + * Program the CPLD configuration flash from @fw and activate it.  Selected
> + * by the "fw.cpld" flash component; the SPI flash keeps the default path.
> + * Runs under cpld_lock with the i2c adapter lock held, so an EEPROM read
> + * blocks for as long as programming takes.
> + */
> +static int adva_x1_cpld_flash(struct ptp_ocp *bp, struct devlink *devlink,
> +			      const struct firmware *fw,
> +			      struct netlink_ext_ack *extack)
> +{
> +	size_t offset;
> +	int err;
> +	u32 st;
> +
> +	if (!fw->size || fw->size % CPLD_PAGE_SIZE) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "image must be a whole number of 16-byte pages");
> +		return -EINVAL;
> +	}

[Severity: Medium]
Should there also be an upper bound here, and a cross-check against the
cached IDCODE?  Nothing limits fw->size to the configuration-flash capacity
of the part, and CPLD_CMD_RESET_ADDR is issued only once before the loop, so
the auto-incrementing ISP address pointer walks past the sector that ERASE
(operand 0x04) cleared when the image is oversized or of the wrong density.

The same missing bound makes the duration of the operation caller-chosen.
The programming loop runs fw->size / 16 iterations, each an I2C transfer plus
a sleeping status poll, with no fatal_signal_pending() or other cancellation
point, while both bp->cpld_lock and the root adapter lock taken by
adva_x1_bus_claim() are held:

	i2c_lock_bus(adap, I2C_LOCK_ROOT_ADAPTER);

so ptp_ocp_read_eeprom() and cpld_status_show() are blocked for the whole
time.  Would a size cap derived from the IDCODE plus a bail-out on a fatal
signal bound both cases?

[ ... ]

> +	devlink_flash_update_status_notify(devlink, "Erasing",
> +					   ADVA_CPLD_COMPONENT, 0, 0);
> +	err = adva_x1_cpld_write(bp, CPLD_CMD_ERASE);
> +	if (!err)
> +		err = adva_x1_cpld_wait_ready(bp, CPLD_ERASE_MS);
> +	if (err)
> +		goto exit_config;

[Severity: Medium]
Where does the new USERCODE get programmed?  The command table added by this
patch has no ISC_PROGRAM_USERCODE (0xC2), and the sequence goes EN_CFG_TP ->
ERASE {0x0e,0x04,0x00,0x00} -> RESET_ADDR -> page writes -> SET_DONE ->
REFRESH.

On MachXO2/MachXO3 the USERCODE lives in the configuration flash sector that
the ERASE CFG bit wipes, and a raw page bitstream fed to LSC_PROG_INCR_NV
does not carry it (Diamond programs it from the .jed UH field with the
dedicated command).  If that holds for this part, then after a successful
update the worker re-reads an erased USERCODE and

	err = devlink_info_version_running_put_ext(req, "fw.cpld", buf,
						   ver_type);

reports the erased pattern as the running version, which contradicts the
documented meaning of fw.cpld ("USERCODE of the image programmed into the
TAP CPLD").  I could not confirm the MachXO3 USERCODE location from the tree
- note that machxo2-spi.c likewise never programs one - so this may be worth
checking against the datasheet.  Should the flow program the USERCODE before
SET_DONE, or should the documentation stop presenting it as an image
identifier?

[ ... ]

> +	/* The image on the part changed, so the cached USERCODE has too. */
> +	WRITE_ONCE(bp->cpld_id, 0);
> +	WRITE_ONCE(bp->cpld_id_tried, false);
> +	bp->cpld_id_attempts = 0;
> +
> +exit_config:
> +	adva_x1_cpld_exit_config(bp);
> +deselect:
> +	adva_x1_mux_select(bp, -1);
> +release:
> +	adva_x1_bus_release(bp);
> +unlock:
> +	mutex_unlock(&bp->cpld_lock);

[Severity: Medium]
Does this leave stale values published on the failure paths?  The
invalidation sits after the post-REFRESH status check, so every error from
CPLD_CMD_ERASE onwards jumps to exit_config or deselect and skips it, while
the erase has already cleared the configuration sector or REFRESH has
already swapped the running image.

ptp_ocp_devlink_info_get() then keeps reporting the pre-flash IDCODE as the
fixed cpld.id and the pre-flash USERCODE as the running fw.cpld version, and
because cpld_id_tried is still true, ptp_ocp_sync_work() (gated on
!cpld_id_tried) never refreshes them:

	if (bp->has_cpld && !READ_ONCE(bp->cpld_id_tried) &&

so the stale pair is published until unbind/rebind.  Would invalidating
before ERASE, and unconditionally once REFRESH may have been accepted, be
more accurate?

[Severity: Medium]
Separately, these three stores run under cpld_lock, but the matching updates
in ptp_ocp_sync_work() do not - adva_x1_cpld_read_id() drops the mutex
before returning:

adva_x1_cpld_read_id()
out:
	mutex_unlock(&bp->cpld_lock);

ptp_ocp_sync_work()
	if (err != -EBUSY &&
	    (!err || ++bp->cpld_id_attempts >= CPLD_ID_MAX_ATTEMPTS))
		WRITE_ONCE(bp->cpld_id_tried, true);

Can the worker's store then undo this invalidation?  If the worker is
scheduled out between adva_x1_cpld_read_id() returning with a valid cpld_id
and its WRITE_ONCE(cpld_id_tried, true), a flash can complete in between and
reset the state, after which the worker's store leaves cpld_id_tried true
with cpld_id == 0.  The read is gated on !cpld_id_tried, so ID and USERCODE
are never re-read, cpld.id and fw.cpld are permanently omitted from devlink
dev info, and the core then refuses component fw.cpld until module reload.

The unsynchronized pair also makes the non-atomic ++bp->cpld_id_attempts
race with the plain bp->cpld_id_attempts = 0 store here.  Would moving the
attempts/tried bookkeeping into adva_x1_cpld_read_id(), under cpld_lock,
close both?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914071531.11640-1-maimon.sagi%40gmail.com

Re: [PATCH net-next v14 3/4] ptp: ocp: add TAP CPLD access for ADVA TimeCard X1

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 2 · Low: 1

- [Medium] adva_x1_mblaze_release() (drivers/ptp/ptp_ocp.c:4375-4393)
  documents and the changelog promises that the hand-back "waits for the…
- [Medium] The firmware hand-back failure is swallowed.
- [Low] Forward-referencing leftovers in this intermediate patch:
  #include <linux/iopoll.h> is added at drivers/ptp/ptp_ocp.c:23 with no…

--

ptp: ocp: add TAP CPLD access for ADVA TimeCard X1

This adds a request/grant handshake that re-routes the registered i2c
controller of the ADVA TimeCard X1 from the config EEPROMs onto the TMC
bus, so the Lattice CPLD behind the PCA9548 can be reached.  On top of it
the patch exposes a root-only cpld_status sysfs attribute and reports the
cached Lattice device ID as the fixed devlink version cpld.id.

> diff --git a/Documentation/ABI/testing/sysfs-timecard b/Documentation/ABI/testing/sysfs-timecard
> index 3ae41b7634ac..c440d8930384 100644
> --- a/Documentation/ABI/testing/sysfs-timecard
> +++ b/Documentation/ABI/testing/sysfs-timecard
> @@ -11,6 +11,30 @@ Contact:	Jonathan Lemon <jonathan.lemon@gmail.com>
> +		==========  ================================================
> +		done        the configuration flash was programmed
> +			    successfully and is active
> +		busy        an internal operation is in progress
> +		failed      the last ISC operation failed
> +		==========  ================================================
> +
> +		For example "busy" while programming, or "busy failed" if a
> +		failure is latched while another operation runs.

[Severity: Low]
This isn't a bug, but the flag descriptions here are phrased in terms of
programming the part ("the configuration flash was programmed
successfully", "busy while programming"), while this commit only defines
CPLD_CMD_READ_ID and CPLD_CMD_READ_STATUS and cannot program anything.

The same forward reference shows up in the code, see the two comments
below.  The follow-up patch in this series, "ptp: ocp: add TAP CPLD
flashing via devlink", is what adds the page-programming commands, the
read_poll_timeout() users and the fw.cpld component, so everything is
live at the end of the series.  Would it read better to move the
upload-referencing wording and the iopoll.h include into that patch?

[ ... ]

> diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> index f4299878c27c..98cb26077181 100644
> --- a/drivers/ptp/ptp_ocp.c
> +++ b/drivers/ptp/ptp_ocp.c
> @@ -20,10 +20,14 @@
>  #include <linux/spi/altera.h>
>  #include <net/devlink.h>
>  #include <linux/i2c.h>
> +#include <linux/iopoll.h>

[Severity: Low]
Is there a user for this include at this commit?  Grepping the file for
poll_timeout matches only this line; the first *_poll_timeout() caller
(adva_x1_cpld_wait_idle()) arrives with the flashing patch.

>  #include <linux/mtd/mtd.h>
>  #include <linux/nvmem-consumer.h>
>  #include <linux/crc16.h>
>  #include <linux/dpll.h>
> +#include <linux/unaligned.h>
> +#include <linux/delay.h>
> +#include <linux/firmware.h>

[ ... ]

> @@ -4226,6 +4266,367 @@ static const struct ocp_attr_group art_timecard_groups[] = {
>  	{ },
>  };
>
> +#define ADVA_MUX_ADDR     0x74
> +#define ADVA_CPLD_ADDR    0x40
> +#define ADVA_MUX_CHANNEL  0
> +
> +/* Longest command or reply; bounds each half of the per-claim scratch. */
> +#define ADVA_CPLD_XFER_MAX 32
> +
> +#define MBLAZE_REQUEST    0x0000aaaaU
> +#define MBLAZE_GRANTED    0x5555aaaaU
> +#define MBLAZE_RELEASE    0x55550000U
> +#define MBLAZE_RETRIES    200
> +#define MBLAZE_RETRY_US   10000

[ ... ]

> +/*
> + * Issue one I2C transaction on the TMC bus: @cmd if not negative, then
> + * @wlen bytes of @wdata (NULL sends zeros), then @rlen bytes read back.
> + * The message is built in the scratch buffer from adva_x1_bus_claim(), so
> + * an upload costs one allocation rather than one per page.  I2C_M_DMA_SAFE

[Severity: Low]
This isn't a bug, but the "upload costs one allocation rather than one
per page" rationale refers to an image upload path that does not exist
until the next patch.

> + * is not set: the two halves are not separately aligned, so let the core
> + * bounce.  Caller holds the claim, hence __i2c_transfer().
> + */

[ ... ]

> +/* Hand the segment back, and wait for the firmware to drop the grant before
> + * the caller unlocks the adapter: a transfer queued behind us would
> + * otherwise be clocked out while the controller is still on the TMC bus,
> + * which is what holding the adapter lock across the grant exists to stop.
> + */
> +static void adva_x1_mblaze_release(struct ptp_ocp *bp)
> +{
> +	int i;
> +
> +	if (!bp->pps_select)
> +		return;
> +
> +	iowrite32(MBLAZE_RELEASE, &bp->pps_select->i2c_bus_ctrl);
> +	/* Flush the posted write, as the acquire path does. */
> +	ioread32(&bp->pps_select->i2c_bus_ctrl);
> +
> +	for (i = 0; i < MBLAZE_RETRIES; i++) {
> +		if (ioread32(&bp->pps_select->i2c_bus_ctrl) != MBLAZE_GRANTED)
> +			return;
> +		usleep_range(MBLAZE_RETRY_US, MBLAZE_RETRY_US + 1000);
> +	}

[Severity: Medium]
Can this loop ever iterate?

The constants describe a register with a host-written half and a
firmware-written half:

	#define MBLAZE_REQUEST    0x0000aaaaU		/* host half */
	#define MBLAZE_GRANTED    0x5555aaaaU		/* grant | request */
	#define MBLAZE_RELEASE    0x55550000U		/* grant, request cleared */

adva_x1_mblaze_release() writes MBLAZE_RELEASE first, which clears the
host request half.  After that write the register can read back
0x55550000 (grant still asserted) or 0x00000000, but not 0x5555aaaa, so
the first ioread32() in the loop compares unequal to MBLAZE_GRANTED and
returns.

If that is right, then the usleep_range() body, the
MBLAZE_RETRIES/MBLAZE_RETRY_US budget and the "TMC bus still granted
after release" warning below are all unreachable, and the hand-back the
changelog describes:

    "The hand-back waits for the firmware to drop the grant before the
    adapter lock is released, for the same reason."

does not happen.  adva_x1_bus_release() then calls i2c_unlock_bus()
immediately, so a transfer already queued behind the CPLD operation from
ptp_ocp_read_eeprom(), the nvmem attributes or the at24 sysfs files can be
clocked out while the host controller is still routed to the TMC bus.

Should the wait test the firmware-owned grant field on its own, for
instance polling for the register to read 0 or masking off the request
half before the comparison?

The same helper is the timeout cleanup in adva_x1_mblaze_acquire(), so a
grant asserted just after the host gave up waiting is not observed
either.

Both paths are reachable without user action: adva_x1_cpld_read_id() runs
from ptp_ocp_sync_work() after probe, and cpld_status_show() is a root
read of /sys/class/timecard/ocpN/cpld_status.

> +
> +	dev_warn(&bp->pdev->dev, "TMC bus still granted after release\n");
> +}

[Severity: Medium]
If the wait can expire (that is, if the equality test above does work on
real hardware), is losing the failure here intentional?

adva_x1_mblaze_release() returns void, so after the warning the caller
cannot tell that the firmware never dropped the grant.  Nothing records
the condition, so the next claim re-runs the handshake as if the
controller were back on the EEPROMs.

Note this and the question above are two sides of the same code: either
the poll cannot iterate, or it can and its expiry is ignored.

[ ... ]

> +/* Route the host controller back to the EEPROMs and release the adapter.
> + * Safe after a failed claim: it also clears a request that was never granted.
> + */
> +static void adva_x1_bus_release(struct ptp_ocp *bp)
> +{
> +	struct i2c_adapter *adap = bp->cpld_adap;
> +
> +	if (!adap)
> +		return;
> +
> +	adva_x1_mblaze_release(bp);
> +	bp->cpld_adap = NULL;
> +	kfree(bp->cpld_buf);
> +	bp->cpld_buf = NULL;
> +	i2c_unlock_bus(adap, I2C_LOCK_ROOT_ADAPTER);
> +	i2c_put_adapter(adap);
> +}

[Severity: Medium]
Should i2c_unlock_bus()/i2c_put_adapter() run unconditionally when the
hand-back may have failed?

Once the adapter is unlocked, ptp_ocp_read_eeprom(), the nvmem attributes
and the at24 sysfs files address 0x50/0x58 again.  With the controller
still on the TMC segment those transfers either NAK, so has_eeprom_data
stays false, or some TMC device answers and ptp_ocp_read_eeprom() latches
a bogus serial/board_id; an at24 write would be clocked into an unrelated
TMC device.

cpld_status_show() also returns the status word it read as a success in
that case, and adva_x1_cpld_read_id() still publishes bp->cpld_id, so
nothing surfaces the mis-routing to userspace.

[ ... ]

> +/*
> + * Claim the TMC bus.  The adapter lock is held across the handshake to keep
> + * the EEPROM, nvmem and at24 paths off the controller while it is routed
> + * away from the EEPROMs; an EEPROM read blocks until the claim is dropped.
> + */
> +static int adva_x1_bus_claim(struct ptp_ocp *bp)
> +{

[ ... ]

> +	/* One scratch buffer per claim rather than per transfer: an upload
> +	 * holds the claim for the whole image.
> +	 */
> +	bp->cpld_buf = kzalloc(2 * ADVA_CPLD_XFER_MAX, GFP_KERNEL);

[Severity: Low]
This isn't a bug, but same as above: the comment explains the allocation
by an upload that holds the claim for a whole image, which only becomes
true with the flashing patch.

> +	if (!bp->cpld_buf) {
> +		i2c_put_adapter(adap);
> +		return -ENOMEM;
> +	}
> +
> +	i2c_lock_bus(adap, I2C_LOCK_ROOT_ADAPTER);
> +	bp->cpld_adap = adap;
> +
> +	ret = adva_x1_mblaze_acquire(bp);
> +	if (ret)
> +		adva_x1_bus_release(bp);
> +
> +	return ret;
> +}

[ ... ]

All three observations were checked against the last patch of the series
(5c1c35566015); the release wait is unchanged there.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914071555.11684-1-maimon.sagi%40gmail.com

[PATCH net-next v14 3/4] ptp: ocp: add TAP CPLD access for ADVA TimeCard X1

[Sagi Maimon]

The ADVA TimeCard X1 carries a Lattice MachXO3 CPLD on its TMC I2C bus.
Reaching it means taking that bus from the on-card MicroBlaze first.

The card has two I2C controllers.  Linux registers only the one at
0x00150000, normally routed to the config EEPROMs; the one at 0x00120000
belongs to the MicroBlaze and is routed to the TMC or M.2 bus as its
firmware selects.  A request/grant handshake re-routes the host controller
onto the TMC bus, where the PCA9548 and the CPLD behind its channel 0
live.  An operation takes the i2c core adapter lock for the whole grant
window and uses __i2c_transfer() internally; without that, a concurrent
transfer from ptp_ocp_read_eeprom(), from the nvmem attributes or from the
at24 sysfs files would be issued onto the TMC bus instead of to the
EEPROM.  The hand-back waits for the firmware to drop the grant before the
adapter lock is released, for the same reason.

On top of the arbitration this adds two read-only interfaces: a root-only
cpld_status attribute reporting the flags in the CPLD status register, and
the Lattice device ID as the fixed devlink version cpld.id.  Reading the
ID claims the bus, so it must not sit on DEVLINK_CMD_INFO_GET, which
carries no GENL_ADMIN_PERM; the driver reads it once from its own worker,
as soon as the i2c adapter has registered, and info_get() reports the
cached value.

Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
---
 Documentation/ABI/testing/sysfs-timecard     |  24 +
 Documentation/networking/devlink/index.rst   |   1 +
 Documentation/networking/devlink/ptp_ocp.rst |  35 ++
 MAINTAINERS                                  |   1 +
 drivers/ptp/ptp_ocp.c                        | 447 ++++++++++++++++++-
 5 files changed, 504 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/networking/devlink/ptp_ocp.rst

diff --git a/Documentation/ABI/testing/sysfs-timecard b/Documentation/ABI/testing/sysfs-timecard
index 3ae41b7634ac..c440d8930384 100644
--- a/Documentation/ABI/testing/sysfs-timecard
+++ b/Documentation/ABI/testing/sysfs-timecard
@@ -11,6 +11,30 @@ Contact:	Jonathan Lemon <jonathan.lemon@gmail.com>
 Description:	This directory contains the attributes of the Nth timecard
 		registered.
 
+What:		/sys/class/timecard/ocpN/cpld_status
+Date:		July 2026
+Contact:	Sagi Maimon <maimon.sagi@gmail.com>
+Description:	(RO, root only) The flags set in the status register of the
+		TAP CPLD, space separated, or an empty line when none is
+		set.  Only present on ADVA x1 TAP boards (PCI ID
+		0xad5a:0x0410).
+
+		==========  ================================================
+		done        the configuration flash was programmed
+			    successfully and is active
+		busy        an internal operation is in progress
+		failed      the last ISC operation failed
+		==========  ================================================
+
+		For example "busy" while programming, or "busy failed" if a
+		failure is latched while another operation runs.
+
+		A read arbitrates for the shared I2C bus and reprograms the
+		on-card mux, so it is restricted to root.  The Lattice device
+		ID of the CPLD is read once by the driver, shortly after
+		probe, and reported from that cached value as the fixed
+		"cpld.id" version by devlink dev info.
+
 What:		/sys/class/timecard/ocpN/available_clock_sources
 Date:		September 2021
 Contact:	Jonathan Lemon <jonathan.lemon@gmail.com>
diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
index d4a83fdcff7f..1af780c811ee 100644
--- a/Documentation/networking/devlink/index.rst
+++ b/Documentation/networking/devlink/index.rst
@@ -99,6 +99,7 @@ parameters, info versions, and other features it supports.
    nfp
    octeontx2
    prestera
+   ptp_ocp
    qed
    sfc
    stmmac
diff --git a/Documentation/networking/devlink/ptp_ocp.rst b/Documentation/networking/devlink/ptp_ocp.rst
new file mode 100644
index 000000000000..3f997cd9f3a3
--- /dev/null
+++ b/Documentation/networking/devlink/ptp_ocp.rst
@@ -0,0 +1,35 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+========================
+ptp_ocp devlink support
+========================
+
+This document describes the devlink features implemented by the ``ptp_ocp``
+device driver.
+
+Info versions
+=============
+
+The ``ptp_ocp`` driver reports the following versions
+
+.. list-table:: devlink info versions implemented
+   :widths: 5 5 90
+
+   * - Name
+     - Type
+     - Description
+   * - ``fw``
+     - running
+     - Version of the firmware running on the card.  Reported as ``loader``
+       instead when the card is running the firmware loader.
+   * - ``board.id``
+     - fixed
+     - Board identifier, read from the on-card EEPROM.
+   * - ``cpld.id``
+     - fixed
+     - Lattice device ID (IDCODE) of the TAP CPLD, formatted as ``0x%08x``.
+       Only present on ADVA TimeCard X1 boards, which are the only ones
+       carrying that CPLD.  Reading it claims the shared I2C bus and
+       reprograms the on-card mux, so the driver does that once from its
+       own worker and reports the cached value here; the version is
+       omitted until that read has succeeded.
diff --git a/MAINTAINERS b/MAINTAINERS
index 0e04d92d1b09..113f31434617 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20479,6 +20479,7 @@ OPENCOMPUTE PTP CLOCK DRIVER
 M:	Vadim Fedorenko <vadim.fedorenko@linux.dev>
 L:	netdev@vger.kernel.org
 S:	Maintained
+F:	Documentation/networking/devlink/ptp_ocp.rst
 F:	drivers/ptp/ptp_ocp.c
 
 OPENCORES I2C BUS DRIVER
diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index f4299878c27c..98cb26077181 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -20,10 +20,14 @@
 #include <linux/spi/altera.h>
 #include <net/devlink.h>
 #include <linux/i2c.h>
+#include <linux/iopoll.h>
 #include <linux/mtd/mtd.h>
 #include <linux/nvmem-consumer.h>
 #include <linux/crc16.h>
 #include <linux/dpll.h>
+#include <linux/unaligned.h>
+#include <linux/delay.h>
+#include <linux/firmware.h>
 
 #define PCI_DEVICE_ID_META_TIMECARD		0x0400
 
@@ -85,6 +89,8 @@ struct ptp_ocp_adva_info {
 	u8				signals_nr;
 	u8				freq_in_nr;
 	const struct ocp_attr_group	*attr_groups;
+	/* x1: TAP CPLD present */
+	bool				has_cpld;
 };
 
 #define OCP_CTRL_ENABLE		BIT(0)
@@ -163,7 +169,8 @@ struct gpio_reg {
 	u32	gpio1;
 	u32	__pad0;
 	u32	gpio2;
-	u32	__pad1;
+	/* adva_x1: I2C bus ownership register; reserved on other variants */
+	u32	i2c_bus_ctrl;
 };
 
 struct irig_master_reg {
@@ -416,6 +423,23 @@ struct ptp_ocp {
 	dpll_tracker tracker;
 	int signals_nr;
 	int freq_in_nr;
+	/* adva_x1 CPLD I2C (internal use only) */
+	/* serialises CPLD operations */
+	struct mutex		cpld_lock;
+	/* I2C adapter nr; -1 if absent */
+	int			cpld_i2c_adap_nr;
+	/* claimed adapter; valid under cpld_lock */
+	struct i2c_adapter	*cpld_adap;
+	/* scratch, write half then read half; valid under cpld_lock */
+	u8			*cpld_buf;
+	/* Lattice device ID; 0 if unread */
+	u32			cpld_id;
+	/* the one-shot ID read is finished, successfully or not */
+	bool			cpld_id_tried;
+	/* failed ID read attempts so far */
+	unsigned int		cpld_id_attempts;
+	/* x1 TAP CPLD present */
+	bool			has_cpld;
 };
 
 #define OCP_REQ_TIMESTAMP	BIT(0)
@@ -1273,6 +1297,7 @@ static struct ocp_resource ocp_adva_x1_resource[] = {
 			.signals_nr   = 4,
 			.freq_in_nr   = 4,
 			.attr_groups  = adva_timecard_x1_groups,
+			.has_cpld     = true,
 		},
 	},
 	{ }
@@ -2159,6 +2184,7 @@ ptp_ocp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 	const char *fw_image;
 	char buf[32];
 	int err;
+	u32 id;
 
 	fw_image = bp->fw_loader ? "loader" : "fw";
 	sprintf(buf, "%d.%d", bp->fw_tag, bp->fw_version);
@@ -2166,6 +2192,18 @@ ptp_ocp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 	if (err)
 		return err;
 
+	/* Read by the worker, not here: this command is unprivileged and
+	 * reading the ID claims the I2C bus.  0 means unread - skip it
+	 * rather than fail, devlink discards the whole reply on error.
+	 */
+	id = READ_ONCE(bp->cpld_id);
+	if (bp->has_cpld && id) {
+		sprintf(buf, "0x%08x", id);
+		err = devlink_info_version_fixed_put(req, "cpld.id", buf);
+		if (err)
+			return err;
+	}
+
 	if (!bp->has_eeprom_data) {
 		ptp_ocp_read_eeprom(bp);
 		if (!bp->has_eeprom_data)
@@ -3199,6 +3237,8 @@ ptp_ocp_adva_board_init(struct ptp_ocp *bp, struct ocp_resource *r)
 		return err;
 	ptp_ocp_sma_init(bp);
 
+	bp->has_cpld = info->has_cpld;
+
 	return ptp_ocp_init_clock(bp, &info->servo);
 }
 
@@ -4226,6 +4266,367 @@ static const struct ocp_attr_group art_timecard_groups[] = {
 	{ },
 };
 
+/*
+ * adva_x1 TAP CPLD (Lattice LCMXO3LF-2100) helpers.
+ *
+ * The card has two I2C controllers; Linux registers only 0x00150000.
+ * The i2c_bus_ctrl handshake re-routes what each is wired to:
+ *
+ *   grant 0:  0x150000 -> EEPROMs     0x120000 -> TMC or M.2, MB's choice
+ *   grant 1:  0x150000 -> TMC bus     0x120000 -> M.2
+ *
+ * The PCA9548 at 0x74 and the CPLD at 0x40 behind its channel 0 are on the
+ * TMC bus, reachable only while the grant is held.  The EEPROMs are not
+ * behind the adapter for that window, so a CPLD operation holds cpld_lock
+ * and the adapter lock to keep the EEPROM and nvmem paths off it.
+ */
+
+#define ADVA_MUX_ADDR     0x74
+#define ADVA_CPLD_ADDR    0x40
+#define ADVA_MUX_CHANNEL  0
+
+/* Longest command or reply; bounds each half of the per-claim scratch. */
+#define ADVA_CPLD_XFER_MAX 32
+
+#define MBLAZE_REQUEST    0x0000aaaaU
+#define MBLAZE_GRANTED    0x5555aaaaU
+#define MBLAZE_RELEASE    0x55550000U
+#define MBLAZE_RETRIES    200
+#define MBLAZE_RETRY_US   10000
+
+/* Lattice LCMXO3LF ISC command codes */
+#define CPLD_CMD_READ_ID      0xE0000000UL
+#define CPLD_CMD_READ_STATUS  0x3C000000UL
+#define CPLD_ID_MAX_ATTEMPTS  10     /* one per sync_work tick */
+
+/* Status register bit positions (Lattice LCMXO3LF datasheet) */
+#define CPLD_STATUS_DONE   BIT(8)
+#define CPLD_STATUS_BUSY   BIT(12)
+#define CPLD_STATUS_FAILED BIT(13)
+
+/*
+ * Issue one I2C transaction on the TMC bus: @cmd if not negative, then
+ * @wlen bytes of @wdata (NULL sends zeros), then @rlen bytes read back.
+ * The message is built in the scratch buffer from adva_x1_bus_claim(), so
+ * an upload costs one allocation rather than one per page.  I2C_M_DMA_SAFE
+ * is not set: the two halves are not separately aligned, so let the core
+ * bounce.  Caller holds the claim, hence __i2c_transfer().
+ */
+static int adva_x1_i2c_xfer(struct ptp_ocp *bp, u8 addr, int cmd,
+			    const void *wdata, u8 wlen,
+			    void *rdata, u8 rlen)
+{
+	u8 *wbuf, *rbuf;
+	struct i2c_adapter *adap;
+	struct i2c_msg msgs[2];
+	int nmsgs = 0, ret;
+	u16 hdr = cmd >= 0;
+
+	lockdep_assert_held(&bp->cpld_lock);
+
+	adap = bp->cpld_adap;
+	if (!adap || !bp->cpld_buf)
+		return -ENODEV;
+
+	if (hdr + wlen > ADVA_CPLD_XFER_MAX || rlen > ADVA_CPLD_XFER_MAX)
+		return -EINVAL;
+
+	wbuf = bp->cpld_buf;
+	rbuf = bp->cpld_buf + ADVA_CPLD_XFER_MAX;
+
+	if (hdr + wlen) {
+		if (hdr)
+			wbuf[0] = cmd;
+		if (wdata)
+			memcpy(wbuf + hdr, wdata, wlen);
+		else
+			memset(wbuf + hdr, 0, wlen);
+		msgs[nmsgs++] = (struct i2c_msg){
+			.addr  = addr,
+			.flags = 0,
+			.len   = hdr + wlen,
+			.buf   = wbuf,
+		};
+	}
+	if (rlen) {
+		msgs[nmsgs++] = (struct i2c_msg){
+			.addr  = addr,
+			.flags = I2C_M_RD,
+			.len   = rlen,
+			.buf   = rbuf,
+		};
+	}
+
+	ret = __i2c_transfer(adap, msgs, nmsgs);
+	if (ret != nmsgs)
+		return (ret < 0) ? ret : -EIO;
+
+	if (rdata && rlen)
+		memcpy(rdata, rbuf, rlen);
+
+	return 0;
+}
+
+/* Hand the segment back, and wait for the firmware to drop the grant before
+ * the caller unlocks the adapter: a transfer queued behind us would
+ * otherwise be clocked out while the controller is still on the TMC bus,
+ * which is what holding the adapter lock across the grant exists to stop.
+ */
+static void adva_x1_mblaze_release(struct ptp_ocp *bp)
+{
+	int i;
+
+	if (!bp->pps_select)
+		return;
+
+	iowrite32(MBLAZE_RELEASE, &bp->pps_select->i2c_bus_ctrl);
+	/* Flush the posted write, as the acquire path does. */
+	ioread32(&bp->pps_select->i2c_bus_ctrl);
+
+	for (i = 0; i < MBLAZE_RETRIES; i++) {
+		if (ioread32(&bp->pps_select->i2c_bus_ctrl) != MBLAZE_GRANTED)
+			return;
+		usleep_range(MBLAZE_RETRY_US, MBLAZE_RETRY_US + 1000);
+	}
+
+	dev_warn(&bp->pdev->dev, "TMC bus still granted after release\n");
+}
+
+/* Acquire the shared I2C bus from the MicroBlaze firmware.  Returns with no
+ * request outstanding on failure, so the firmware is never left granting a
+ * segment to a host that has given up waiting for it.
+ */
+static int adva_x1_mblaze_acquire(struct ptp_ocp *bp)
+{
+	u32 val;
+	int i;
+
+	if (!bp->pps_select)
+		return -ENODEV;
+
+	/* Drop a request left by a caller that died mid-sequence; cpld_lock
+	 * keeps live ones out.  The read back only flushes the posted write.
+	 */
+	iowrite32(0, &bp->pps_select->i2c_bus_ctrl);
+	ioread32(&bp->pps_select->i2c_bus_ctrl);
+
+	iowrite32(MBLAZE_REQUEST, &bp->pps_select->i2c_bus_ctrl);
+	for (i = 0; i < MBLAZE_RETRIES; i++) {
+		usleep_range(MBLAZE_RETRY_US, MBLAZE_RETRY_US + 1000);
+		val = ioread32(&bp->pps_select->i2c_bus_ctrl);
+		if (val == MBLAZE_GRANTED)
+			return 0;
+	}
+
+	adva_x1_mblaze_release(bp);
+	return -ETIMEDOUT;
+}
+
+/* Route the host controller back to the EEPROMs and release the adapter.
+ * Safe after a failed claim: it also clears a request that was never granted.
+ */
+static void adva_x1_bus_release(struct ptp_ocp *bp)
+{
+	struct i2c_adapter *adap = bp->cpld_adap;
+
+	if (!adap)
+		return;
+
+	adva_x1_mblaze_release(bp);
+	bp->cpld_adap = NULL;
+	kfree(bp->cpld_buf);
+	bp->cpld_buf = NULL;
+	i2c_unlock_bus(adap, I2C_LOCK_ROOT_ADAPTER);
+	i2c_put_adapter(adap);
+}
+
+static int adva_x1_i2c_adap_match(struct device *dev, const void *data)
+{
+	return !!i2c_verify_adapter(dev);
+}
+
+/*
+ * Cache the adapter the CPLD hangs off.  ptp_ocp_i2c_notifier_call() misses
+ * it when the i2c controller driver is already loaded: the adapter is then
+ * added from inside platform_device_register(), before bp->i2c_ctrl is set.
+ * Only direct children are searched, so a mux adapter cannot match.
+ */
+static void adva_x1_cache_i2c_adap(struct ptp_ocp *bp)
+{
+	struct device *child;
+
+	if (!bp->has_cpld || !bp->i2c_ctrl ||
+	    READ_ONCE(bp->cpld_i2c_adap_nr) >= 0)
+		return;
+
+	child = device_find_child(&bp->i2c_ctrl->dev, NULL,
+				  adva_x1_i2c_adap_match);
+	if (!child)
+		return;
+
+	WRITE_ONCE(bp->cpld_i2c_adap_nr, i2c_verify_adapter(child)->nr);
+	put_device(child);
+}
+
+/*
+ * Claim the TMC bus.  The adapter lock is held across the handshake to keep
+ * the EEPROM, nvmem and at24 paths off the controller while it is routed
+ * away from the EEPROMs; an EEPROM read blocks until the claim is dropped.
+ */
+static int adva_x1_bus_claim(struct ptp_ocp *bp)
+{
+	struct i2c_adapter *adap;
+	int ret;
+
+	lockdep_assert_held(&bp->cpld_lock);
+
+	adap = i2c_get_adapter(READ_ONCE(bp->cpld_i2c_adap_nr));
+	if (!adap)
+		return -ENODEV;
+
+	/* The number is freed before the notifier clears it, so it can
+	 * already be another adapter's.  Check this is still ours.
+	 */
+	if (!bp->i2c_ctrl || adap->dev.parent != &bp->i2c_ctrl->dev) {
+		i2c_put_adapter(adap);
+		return -ENODEV;
+	}
+
+	/* One scratch buffer per claim rather than per transfer: an upload
+	 * holds the claim for the whole image.
+	 */
+	bp->cpld_buf = kzalloc(2 * ADVA_CPLD_XFER_MAX, GFP_KERNEL);
+	if (!bp->cpld_buf) {
+		i2c_put_adapter(adap);
+		return -ENOMEM;
+	}
+
+	i2c_lock_bus(adap, I2C_LOCK_ROOT_ADAPTER);
+	bp->cpld_adap = adap;
+
+	ret = adva_x1_mblaze_acquire(bp);
+	if (ret)
+		adva_x1_bus_release(bp);
+
+	return ret;
+}
+
+/* Select a mux channel, or deselect all with ch < 0 - the power-on state.
+ * The mux is on the TMC bus, so what it is left set to never affects the
+ * EEPROM paths.
+ */
+static int adva_x1_mux_select(struct ptp_ocp *bp, int ch)
+{
+	u8 val = (ch >= 0) ? BIT(ch) : 0;
+
+	return adva_x1_i2c_xfer(bp, ADVA_MUX_ADDR, val, NULL, 0, NULL, 0);
+}
+
+/*
+ * Send a 4-byte command and read back without an intermediate STOP: two
+ * messages in one transfer is the Lattice write -> repeated START -> read,
+ * so no protocol-mangling flag is needed.
+ */
+static int adva_x1_cpld_cmd_read(struct ptp_ocp *bp,
+				 u32 cmd_be, u8 *out, u8 out_len)
+{
+	__be32 cmd = cpu_to_be32(cmd_be);
+
+	return adva_x1_i2c_xfer(bp, ADVA_CPLD_ADDR, -1, &cmd, 4, out, out_len);
+}
+
+static int adva_x1_cpld_read_status(struct ptp_ocp *bp, u32 *status)
+{
+	u8 buf[4];
+	int ret;
+
+	ret = adva_x1_cpld_cmd_read(bp, CPLD_CMD_READ_STATUS, buf, 4);
+	if (ret)
+		return ret;
+	*status = get_unaligned_be32(buf);
+	return 0;
+}
+
+/*
+ * Read the Lattice device ID into bp->cpld_id.  Done once, off the
+ * unprivileged devlink path, which reports the cached value.  -EBUSY means
+ * cpld_lock is held, so the caller can retry rather than wait behind a long
+ * CPLD operation.
+ */
+static int adva_x1_cpld_read_id(struct ptp_ocp *bp)
+{
+	u8 data[4];
+	int ret;
+
+	if (!mutex_trylock(&bp->cpld_lock))
+		return -EBUSY;
+
+	ret = adva_x1_bus_claim(bp);
+	if (ret)
+		goto out;
+	ret = adva_x1_mux_select(bp, ADVA_MUX_CHANNEL);
+	if (ret)
+		goto release;
+	ret = adva_x1_cpld_cmd_read(bp, CPLD_CMD_READ_ID, data, 4);
+	if (!ret)
+		WRITE_ONCE(bp->cpld_id, get_unaligned_be32(data));
+	adva_x1_mux_select(bp, -1);
+release:
+	adva_x1_bus_release(bp);
+out:
+	mutex_unlock(&bp->cpld_lock);
+	if (ret)
+		dev_dbg(&bp->pdev->dev,
+			"CPLD device ID unavailable: %d\n", ret);
+
+	return ret;
+}
+
+/* cpld_status - show the flags set in the TAP CPLD status register. */
+static ssize_t
+cpld_status_show(struct device *dev, struct device_attribute *attr,
+		 char *buf)
+{
+	struct ptp_ocp *bp = dev_get_drvdata(dev);
+	int count = 0;
+	u32 st = 0;
+	int ret;
+
+	/* A CPLD operation can hold cpld_lock a long time; stay killable. */
+	ret = mutex_lock_interruptible(&bp->cpld_lock);
+	if (ret)
+		return ret;
+
+	ret = adva_x1_bus_claim(bp);
+	if (ret)
+		goto out;
+	ret = adva_x1_mux_select(bp, ADVA_MUX_CHANNEL);
+	if (ret)
+		goto release;
+	ret = adva_x1_cpld_read_status(bp, &st);
+	adva_x1_mux_select(bp, -1);
+release:
+	adva_x1_bus_release(bp);
+out:
+	mutex_unlock(&bp->cpld_lock);
+	if (ret)
+		return ret;
+
+	/* Flags: the names of the bits that are set, nothing when none is,
+	 * rather than a value per bit in one file.
+	 */
+	if (st & CPLD_STATUS_DONE)
+		count += sysfs_emit_at(buf, count, "done");
+	if (st & CPLD_STATUS_BUSY)
+		count += sysfs_emit_at(buf, count, "%sbusy", count ? " " : "");
+	if (st & CPLD_STATUS_FAILED)
+		count += sysfs_emit_at(buf, count, "%sfailed",
+				       count ? " " : "");
+
+	return count + sysfs_emit_at(buf, count, "\n");
+}
+static DEVICE_ATTR_ADMIN_RO(cpld_status);
+
 static struct attribute *adva_timecard_attrs[] = {
 	&dev_attr_serialnum.attr,
 	&dev_attr_gnss_sync.attr,
@@ -4274,6 +4675,7 @@ static struct attribute *adva_timecard_x1_attrs[] = {
 	&dev_attr_ts_window_adjust.attr,
 	&dev_attr_utc_tai_offset.attr,
 	&dev_attr_tod_correction.attr,
+	&dev_attr_cpld_status.attr,
 	NULL,
 };
 
@@ -4904,6 +5306,7 @@ ptp_ocp_detach(struct ptp_ocp *bp)
 		clk_hw_unregister_fixed_rate(bp->i2c_clk);
 	if (bp->n_irqs)
 		pci_free_irq_vectors(bp->pdev);
+	mutex_destroy(&bp->cpld_lock);
 	device_unregister(&bp->dev);
 }
 
@@ -5052,6 +5455,20 @@ ptp_ocp_sync_work(struct work_struct *work)
 
 	bp->sync = sync;
 
+	/* The adapter can appear after probe, so read the ID from here once
+	 * it has.  A claim can fail transiently - the firmware may not grant
+	 * the segment straight after power-up - so retry a bounded number of
+	 * times.  -EBUSY means cpld_lock is held and does not count.
+	 */
+	if (bp->has_cpld && !READ_ONCE(bp->cpld_id_tried) &&
+	    READ_ONCE(bp->cpld_i2c_adap_nr) >= 0) {
+		int err = adva_x1_cpld_read_id(bp);
+
+		if (err != -EBUSY &&
+		    (!err || ++bp->cpld_id_attempts >= CPLD_ID_MAX_ATTEMPTS))
+			WRITE_ONCE(bp->cpld_id_tried, true);
+	}
+
 	queue_delayed_work(system_power_efficient_wq, &bp->sync_work, HZ);
 }
 
@@ -5080,6 +5497,14 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (err)
 		goto out_disable;
 
+	/* Before the first error path that calls ptp_ocp_detach(), so
+	 * mutex_destroy() runs on an initialised mutex, and before
+	 * ptp_ocp_register_resources(), so this sentinel cannot overwrite an
+	 * adapter number the i2c notifier stores during registration.
+	 */
+	mutex_init(&bp->cpld_lock);
+	bp->cpld_i2c_adap_nr = -1;
+
 	INIT_DELAYED_WORK(&bp->sync_work, ptp_ocp_sync_work);
 
 	/* compat mode.
@@ -5099,6 +5524,8 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (err)
 		goto out;
 
+	adva_x1_cache_i2c_adap(bp);
+
 	bp->ptp = ptp_clock_register(&bp->ptp_info, &pdev->dev);
 	if (IS_ERR(bp->ptp)) {
 		err = PTR_ERR(bp->ptp);
@@ -5158,6 +5585,7 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 out_dpll_put:
 	dpll_device_put(bp->dpll, &bp->tracker);
 out_devlink:
+	/* detach() destroys cpld_lock; no devlink op may still reach it */
 	devlink_unregister(devlink);
 out:
 	ptp_ocp_detach(bp);
@@ -5204,6 +5632,7 @@ ptp_ocp_i2c_notifier_call(struct notifier_block *nb,
 			  unsigned long action, void *data)
 {
 	struct device *dev, *child = data;
+	struct i2c_adapter *adap;
 	struct ptp_ocp *bp;
 	bool add;
 
@@ -5227,11 +5656,21 @@ ptp_ocp_i2c_notifier_call(struct notifier_block *nb,
 
 found:
 	bp = dev_get_drvdata(dev);
-	if (add)
+	adap = i2c_verify_adapter(child);
+	if (add) {
 		ptp_ocp_symlink(bp, child, "i2c");
-	else
+		/* Only the card's own controller: a mux child added on it
+		 * reaches ptp_ocp through the same walk.  Numbers are
+		 * recycled, so the claim re-checks the parent itself.
+		 */
+		if (bp->i2c_ctrl && adap->dev.parent == &bp->i2c_ctrl->dev)
+			WRITE_ONCE(bp->cpld_i2c_adap_nr, adap->nr);
+	} else {
+		/* invalidate before free, but only for the cached adapter */
+		if (READ_ONCE(bp->cpld_i2c_adap_nr) == adap->nr)
+			WRITE_ONCE(bp->cpld_i2c_adap_nr, -1);
 		sysfs_remove_link(&bp->dev.kobj, "i2c");
-
+	}
 	return 0;
 }
 
-- 
2.47.0