Re: [PATCH v8] ptp: ocp: add CPLD ISP support for ADVA TimeCard X1

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 02/08/2026 08:11, Sagi Maimon wrote:
> The ADVA TimeCard X1 (PCI device 0x0410) uses a Lattice MachXO3 CPLD
> that is programmed over I2C using in-system programming (ISP).
> 
> The CPLD is connected to a secondary I2C bus shared with the onboard
> MicroBlaze soft CPU.
> 
> Add support for CPLD access and firmware updates on the ADVA TimeCard X1
> board by arbitration of the shared I2C bus, CPLD ISP command handling,
> status polling, and firmware upload operations using the firmware-upload
> subsystem.
> 
> Add the following X1-only user-visible interfaces:
> 
>    /sys/class/timecard/ocpN/cpld_device_id
>          report the 32-bit Lattice MachXO3 CPLD device ID
> 
>    /sys/class/timecard/ocpN/cpld_status
>          report the CPLD status register, including the DONE,
>          BUSY, and FAILED indicators
> 
> Firmware updates are performed through the firmware-upload framework,
> which acquires ownership of the shared I2C bus, erases the CPLD
> configuration flash, programs the image page-by-page, and activates
> the new image using the MachXO3 REFRESH command.
> 
> All CPLD operations are serialized and coordinated with the MicroBlaze
> firmware to ensure exclusive access to the shared I2C bus. The added
> interfaces are available only on ADVA TimeCard X1 boards.
> 
> Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
> ---
> 
>   Addressed comments from:
>    - Jakub Kicinski :https://lore.kernel.org/all/20260730130424.77a90fa3@kernel.org/
>    - Vadim Fedorenko :https://lore.kernel.org/all/7075bcdb-da4d-4d2c-bd09-516d9f244277@linux.dev/
>   
>   Changes since v7:
>   - Use %pe instead of %ld + PTR_ERR() (coccicheck)
>   - Add #include <linux/unaligned.h>
>   - Widen adva_x1_i2c_xfer() buffer pointers to void *
>   - Use cpu_to_be32() in adva_x1_cpld_cmd_read()
>   - Use get_unaligned_be32() in adva_x1_cpld_read_status()
>   
>   Documentation/ABI/testing/sysfs-timecard |  34 ++
>   drivers/ptp/ptp_ocp.c                    | 497 ++++++++++++++++++++++-
>   2 files changed, 527 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-timecard b/Documentation/ABI/testing/sysfs-timecard
> index 3ae41b7634ac..f7c9955acb0a 100644
> --- a/Documentation/ABI/testing/sysfs-timecard
> +++ b/Documentation/ABI/testing/sysfs-timecard
> @@ -11,6 +11,40 @@ Contact:	Jonathan Lemon <jonathan.lemon@gmail.com>
>   Description:	This directory contains the attributes of the Nth timecard
>   		registered.
>   
> +What:		/sys/class/timecard/ocpN/cpld_device_id
> +Date:		July 2026
> +Contact:	Sagi Maimon <maimon.sagi@gmail.com>
> +Description:	(RO) The 32-bit Lattice device ID of the TAP CPLD, reported as
> +		a hex string, e.g. "0xe12bc043".
> +
> +		Only present on ADVA x1 TAP boards (PCI ID 0xad5a:0x0410).
> +		The Lattice LCMXO3LF-210 reports 0xe12bc043.
> +
> +		The driver acquires the MicroBlaze I2C bus internally before
> +		issuing the READ_IDCODE command; no bus arbitration is required
> +		from userspace.
> +
> +What:		/sys/class/timecard/ocpN/cpld_status
> +Date:		July 2026
> +Contact:	Sagi Maimon <maimon.sagi@gmail.com>
> +Description:	(RO) The status register of the TAP CPLD, in human-readable
> +		form:
> +
> +		  done=<0|1> busy=<0|1> failed=<0|1>
> +
> +		Only present on ADVA x1 TAP boards (PCI ID 0xad5a:0x0410).
> +
> +		done=1 indicates the configuration flash was successfully
> +		programmed and is active.  busy=1 means an internal operation
> +		is in progress.  failed=1 means the last ISC operation failed.
> +
> +		The driver acquires the MicroBlaze I2C bus internally; no bus
> +		arbitration is required from userspace.
> +
> +		To program new CPLD firmware use the standard kernel
> +		firmware-upload interface registered at:
> +		/sys/class/firmware/adva-cpld/
> +
>   What:		/sys/class/timecard/ocpN/available_clock_sources
>   Date:		September 2021
>   Contact:	Jonathan Lemon <jonathan.lemon@gmail.com>
> diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> index 35e911f1ad78..a4a8cab5ed7d 100644
> --- a/drivers/ptp/ptp_ocp.c
> +++ b/drivers/ptp/ptp_ocp.c
> @@ -24,6 +24,11 @@
>   #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>
> +#include <linux/delay.h>
> +#include <linux/firmware.h>

why do you double include headers?

>   
>   #define PCI_DEVICE_ID_META_TIMECARD		0x0400
>   
> @@ -85,6 +90,7 @@ struct ptp_ocp_adva_info {
>   	u8				signals_nr;
>   	u8				freq_in_nr;
>   	const struct ocp_attr_group	*attr_groups;
> +	bool				has_cpld; /* x1: supports CPLD firmware upload */
>   };
>   
>   #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,12 @@ struct ptp_ocp {
>   	dpll_tracker tracker;
>   	int signals_nr;
>   	int freq_in_nr;
> +	/* adva_x1 CPLD I2C (internal use only) */
> +	struct mutex		cpld_lock;            /* serialises CPLD operations */
> +	int			cpld_i2c_adap_nr;     /* I2C adapter nr; -1 if absent */
> +	struct fw_upload	*cpld_fw_upload;      /* firmware upload handle; NULL if absent */
> +	bool			cpld_cancel;          /* cancellation requested */
> +	bool			cpld_in_config_mode;  /* EN_CFG_TP issued but not yet REFRESH'd */
>   };
>   
>   #define OCP_REQ_TIMESTAMP	BIT(0)
> @@ -449,6 +462,8 @@ static int ptp_ocp_art_board_init(struct ptp_ocp *bp, struct ocp_resource *r);
>   
>   static int ptp_ocp_adva_board_init(struct ptp_ocp *bp, struct ocp_resource *r);
>   
> +static const struct fw_upload_ops adva_cpld_upload_ops;
> +
>   static const struct ocp_sma_op ocp_adva_sma_op;
>   static const struct ocp_sma_op ocp_adva_x1_sma_op;
>   
> @@ -1273,6 +1288,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,
>   		},
>   	},
>   	{ }
> @@ -3197,6 +3213,20 @@ ptp_ocp_adva_board_init(struct ptp_ocp *bp, struct ocp_resource *r)
>   		return err;
>   	ptp_ocp_sma_init(bp);
>   
> +	if (info->has_cpld) {
> +		struct fw_upload *fwl;
> +
> +		fwl = firmware_upload_register(THIS_MODULE, &bp->pdev->dev,
> +					       "adva-cpld",
> +					       &adva_cpld_upload_ops, bp);
> +		if (IS_ERR(fwl))
> +			dev_warn(&bp->pdev->dev,
> +				 "CPLD firmware upload unavailable: %pe\n",
> +				 fwl);
> +		else
> +			bp->cpld_fw_upload = fwl;
> +	}
> +
>   	return ptp_ocp_init_clock(bp, &info->servo);
>   }
>   
> @@ -4224,6 +4254,442 @@ static const struct ocp_attr_group art_timecard_groups[] = {
>   	{ },
>   };
>   
> +/*
> + * Internal helpers for the adva_x1 TAP CPLD (Lattice LCMXO3LF-210).
> + *
> + * The CPLD sits at I2C address 0x40 behind a PCA9548 mux (0x74) on
> + * channel 0.  The I2C bus is shared with the MicroBlaze firmware;
> + * cpld_lock + mblaze acquire/release provide mutual exclusion for the
> + * full duration of any CPLD operation.  No raw I2C access is exposed
> + * to userspace; only the high-level attributes below are.
> + */
> +
> +#define ADVA_MUX_ADDR     0x74
> +#define ADVA_CPLD_ADDR    0x40
> +#define ADVA_MUX_CHANNEL  0
> +
> +#define MBLAZE_REQUEST    0x0000ffffU
> +#define MBLAZE_GRANTED    0xffffffffU
> +#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_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
> +
> +/* 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)
> +
> +/*
> + * adva_x1_i2c_xfer() - issue a single I2C transaction on the CPLD bus.
> + *
> + * All buffers are heap-allocated internally to guarantee DMA safety for
> + * the Xilinx I2C controller.  Caller must hold bp->cpld_lock.
> + */
> +static int adva_x1_i2c_xfer(struct ptp_ocp *bp,
> +			    u8 addr, const void *wdata, u8 wlen,
> +			    void *rdata, u8 rlen, bool nostart)
> +{
> +	struct i2c_adapter *adap;
> +	struct i2c_msg msgs[2];
> +	u8 *wbuf = NULL, *rbuf = NULL;
> +	int nmsgs = 0, ret;

as this patch goes through netdev, please respect reverse xmass tree
ordering of variables declaration here and futher in the patch

> +
> +	adap = i2c_get_adapter(READ_ONCE(bp->cpld_i2c_adap_nr));
> +	if (!adap)
> +		return -ENODEV;
> +
> +	if (wlen) {
> +		wbuf = kmemdup(wdata, wlen, GFP_KERNEL);
> +		if (!wbuf) {
> +			ret = -ENOMEM;
> +			goto put;
> +		}
> +		msgs[nmsgs++] = (struct i2c_msg){
> +			.addr  = addr,
> +			.flags = I2C_M_DMA_SAFE,
> +			.len   = wlen,
> +			.buf   = wbuf,
> +		};
> +	}
> +	if (rlen) {
> +		rbuf = kzalloc(rlen, GFP_KERNEL);
> +		if (!rbuf) {
> +			ret = -ENOMEM;
> +			goto put;
> +		}
> +		msgs[nmsgs++] = (struct i2c_msg){
> +			.addr  = addr,
> +			.flags = I2C_M_RD | I2C_M_DMA_SAFE |
> +				 (nostart ? I2C_M_NOSTART : 0),
> +			.len   = rlen,
> +			.buf   = rbuf,
> +		};
> +	}
> +
> +	ret = i2c_transfer(adap, msgs, nmsgs);
> +	if (ret == nmsgs) {
> +		if (rdata && rlen)
> +			memcpy(rdata, rbuf, rlen);
> +		ret = 0;
> +	} else {
> +		ret = (ret < 0) ? ret : -EIO;
> +	}
> +put:
> +	kfree(wbuf);
> +	kfree(rbuf);
> +	i2c_put_adapter(adap);
> +	return ret;
> +}
> +
> +/* Acquire the shared I2C bus from the MicroBlaze firmware. */
> +static int adva_x1_mblaze_acquire(struct ptp_ocp *bp)
> +{
> +	u32 val;
> +	int i;
> +
> +	if (!bp->pps_select)
> +		return -ENODEV;
> +
> +	/* Release any stale grant left by a previous crashed caller. */
> +	iowrite32(0, &bp->pps_select->i2c_bus_ctrl);
> +	val = ioread32(&bp->pps_select->i2c_bus_ctrl);
> +	if (val != 0)
> +		return -EBUSY;
> +
> +	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;
> +	}
> +	return -ETIMEDOUT;
> +}
> +
> +static void adva_x1_mblaze_release(struct ptp_ocp *bp)
> +{
> +	if (bp->pps_select)
> +		iowrite32(0, &bp->pps_select->i2c_bus_ctrl);
> +}
> +
> +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, 1, NULL, 0, false);
> +}
> +
> +/* Send 1-byte ISC command + optional arguments. */
> +static int adva_x1_cpld_write(struct ptp_ocp *bp,
> +			      u8 cmd, const u8 *args, u8 nargs)
> +{
> +	u8 buf[1 + 64];
> +
> +	if (nargs > 64)
> +		return -EINVAL;
> +	buf[0] = cmd;
> +	if (nargs)
> +		memcpy(&buf[1], args, nargs);
> +	return adva_x1_i2c_xfer(bp, ADVA_CPLD_ADDR, buf, 1 + nargs,
> +				 NULL, 0, false);
> +}
> +
> +/*
> + * Send a 4-byte command then read data back without an intermediate STOP
> + * (Lattice combined write→repeated-START→read).
> + */
> +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, &cmd, 4, out, out_len, true);
> +}
> +
> +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;
> +}
> +
> +static int adva_x1_cpld_wait_ready(struct ptp_ocp *bp, unsigned int max_ms)
> +{
> +	u32 status;
> +	unsigned int elapsed = 0;
> +
> +	while (elapsed < max_ms) {
> +		if (adva_x1_cpld_read_status(bp, &status))
> +			return -EIO;
> +		if (status & CPLD_STATUS_FAILED)
> +			return -EIO;
> +		if (!(status & CPLD_STATUS_BUSY))
> +			return 0;
> +		usleep_range(100000, 101000);
> +		elapsed += 100;
> +	}
> +	return -ETIMEDOUT;
> +}
> +
> +/*
> + * cpld_device_id - show the Lattice device ID of the TAP CPLD.
> + *
> + * Returns the 32-bit ID as a hex string, e.g. "0x612bc043\n".
> + * Lattice LCMXO3LF-210 reports 0x612BC043.
> + */
> +static ssize_t
> +cpld_device_id_show(struct device *dev, struct device_attribute *attr,
> +		    char *buf)
> +{
> +	struct ptp_ocp *bp = dev_get_drvdata(dev);
> +	u8 data[4];
> +	u32 id = 0;
> +	int ret;
> +
> +	mutex_lock(&bp->cpld_lock);
> +	ret = adva_x1_mblaze_acquire(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)
> +		id = ((u32)data[0] << 24) | ((u32)data[1] << 16) |
> +		     ((u32)data[2] <<  8) |  (u32)data[3];

why is this code not changed to be32_to_cpu()?

> +	adva_x1_mux_select(bp, -1);
> +release:
> +	adva_x1_mblaze_release(bp);
> +out:
> +	mutex_unlock(&bp->cpld_lock);
> +	return ret ? ret : sysfs_emit(buf, "0x%08x\n", id);
> +}
> +static DEVICE_ATTR_RO(cpld_device_id);
> +
> +/*
> + * cpld_status - show the status register of the TAP CPLD.
> + *
> + * Returns a human-readable string: "done=<0|1> busy=<0|1> failed=<0|1>\n"
> + */
> +static ssize_t
> +cpld_status_show(struct device *dev, struct device_attribute *attr,
> +		 char *buf)
> +{
> +	struct ptp_ocp *bp = dev_get_drvdata(dev);
> +	u32 st = 0;
> +	int ret;
> +
> +	mutex_lock(&bp->cpld_lock);
> +	ret = adva_x1_mblaze_acquire(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_mblaze_release(bp);
> +out:
> +	mutex_unlock(&bp->cpld_lock);
> +	return ret ? ret : sysfs_emit(buf, "done=%u busy=%u failed=%u\n",
> +				      !!(st & CPLD_STATUS_DONE),
> +				      !!(st & CPLD_STATUS_BUSY),
> +				      !!(st & CPLD_STATUS_FAILED));
> +}
> +static DEVICE_ATTR_RO(cpld_status);
> +
> +/*
> + * adva_x1 CPLD firmware-upload callbacks.
> + *
> + * The kernel firmware-upload subsystem (CONFIG_FW_UPLOAD) exposes:
> + *   /sys/class/firmware/adva-cpld/{data,loading,status,error,...}
> + * Userspace writes the raw binary page data directly — no /lib/firmware/
> + * staging file is needed.
> + *
> + * Callback sequence driven by the framework:
> + *   prepare()      - validate size, acquire bus, enable config, erase flash
> + *   write()        - program one 16-byte page per call
> + *   poll_complete()- set DONE, REFRESH, wait for CPLD to reboot
> + *   cancel()       - set flag; checked at the start of each callback
> + *   cleanup()      - release bus resources (called on success or failure)
> + */
> +static enum fw_upload_err
> +adva_cpld_prepare(struct fw_upload *fwl, const u8 *data, u32 size)
> +{
> +	struct ptp_ocp *bp = fwl->dd_handle;
> +	const u8 en_args[2]  = { 0x08, 0x00 };
> +	const u8 era_args[3] = { 0x04, 0x00, 0x00 }; /* cfg sector only */
> +	const u8 zero3[3]    = { 0 };
> +	const u8 dis_args[2] = { 0x00, 0x00 };

this constants can be made static, no need to re-init stack on every
call

> +	enum fw_upload_err ret = FW_UPLOAD_ERR_NONE;
> +
> +	if (!size || size % CPLD_PAGE_SIZE)
> +		return FW_UPLOAD_ERR_INVALID_SIZE;
> +
> +	bp->cpld_cancel		= false;
> +	bp->cpld_in_config_mode	= false;
> +
> +	mutex_lock(&bp->cpld_lock);
> +
> +	if (adva_x1_mblaze_acquire(bp)) {
> +		ret = FW_UPLOAD_ERR_TIMEOUT;
> +		goto err_unlock;
> +	}
> +
> +	if (adva_x1_mux_select(bp, ADVA_MUX_CHANNEL)) {
> +		ret = FW_UPLOAD_ERR_HW_ERROR;
> +		goto err_release;
> +	}
> +
> +	if (adva_x1_cpld_write(bp, CPLD_CMD_EN_CFG_TP, en_args, 2) ||
> +	    adva_x1_cpld_wait_ready(bp, 5000)) {
> +		ret = FW_UPLOAD_ERR_HW_ERROR;
> +		goto err_deselect;
> +	}
> +	bp->cpld_in_config_mode = true;
> +
> +	if (bp->cpld_cancel) {
> +		ret = FW_UPLOAD_ERR_CANCELED;
> +		goto err_deselect;
> +	}
> +
> +	if (adva_x1_cpld_write(bp, CPLD_CMD_ERASE, era_args, 3) ||
> +	    adva_x1_cpld_wait_ready(bp, 15000)) {
> +		ret = FW_UPLOAD_ERR_HW_ERROR;
> +		goto err_deselect;
> +	}
> +
> +	if (bp->cpld_cancel) {
> +		ret = FW_UPLOAD_ERR_CANCELED;
> +		goto err_deselect;

it looks like a duplicate of check, or do you expect something to change
cpld_cancel during writes?