Re: [PATCH v8] ptp: ocp: add CPLD ISP support for ADVA TimeCard X1
Sagi Maimon <[email protected]>
| Newsgroups | org.kernel.vger.netdev,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAMuE1bEaRBJVSJYONJ2o3adOOzayv0ayNZ2qCB1zxuodpbiR=Q@mail.gmail.com> |
On Tue, Aug 4, 2026 at 2:35 PM Vadim Fedorenko <[email protected]> wrote: > > 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 <[email protected]> > > --- > > > > Addressed comments from: > > - Jakub Kicinski :https://lore.kernel.org/all/[email protected]/ > > - Vadim Fedorenko :https://lore.kernel.org/all/[email protected]/ > > > > 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 <[email protected]> > > 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 <[email protected]> > > +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 <[email protected]> > > +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 <[email protected]> > > 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? > > Yes, cpld_cancel can change asynchronously. The first check is after entering configuration mode, while the second one is after the erase operation, which can take up to 15s (adva_x1_cpld_wait_ready(bp, 15000)). A cancellation request may arrive during that wait, so the second check is intentional rather than a duplicate.