* [PATCH v6] ptp: ocp: add CPLD ISP support for ADVA TimeCard X1
@ 2026-07-23 14:39 Sagi Maimon
2026-07-27 11:02 ` Vadim Fedorenko
0 siblings, 1 reply; 3+ messages in thread
From: Sagi Maimon @ 2026-07-23 14:39 UTC (permalink / raw)
To: jonathan.lemon, vadim.fedorenko, richardcochran, andrew+netdev,
davem, edumazet, kuba, pabeni
Cc: linux-kernel, netdev, Sagi Maimon
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 taking ownership of this bus and
exposing the required interfaces through sysfs, allowing userspace tools
to perform CPLD programming.
To limit the scope of this functionality, sysfs-based I2C access is
restricted to the ADVA TimeCard X1 variant and only for the two I2C
slave addresses used during ISP (0x40 CPLD, 0x74 mux).
Add two sysfs attributes under /sys/class/timecard/ocpN/ (x1 only):
i2c_bus_ctrl - arbitrate the shared I2C bus from the MicroBlaze via
a three-step read/write/poll handshake
cpld_i2c_xfer - binary passthrough for I2C transactions to the CPLD
and its PCA9548 mux; one atomic request per write()
Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
---
Addressed comments from:
- Jakub Kicinski :https://lore.kernel.org/all/20260722184418.266546-1-kuba@kernel.org/
Changes since v5:
- ptp_ocp.c: move mutex_init(&bp->tap_i2c_lock) and
bp->tap_i2c_adap_nr = -1 to before the first error path that
reaches ptp_ocp_detach(), so mutex_destroy() never operates on an
uninitialised mutex (CONFIG_DEBUG_MUTEXES splat on the
pci_alloc_irq_vectors() failure path).
- ptp_ocp.c: reword the ordering comment above mutex_init /
tap_i2c_adap_nr to correctly describe the notifier race: the -1
sentinel must precede ptp_ocp_register_resources() so that a
ptp_ocp_i2c_notifier_call() firing during adapter registration is
not overwritten by this initialisation line; ptp_ocp_adva_board_init()
does not touch tap_i2c_adap_nr and was wrongly cited in the
previous wording.
- Documentation/ABI/testing/sysfs-timecard: add entries for the two
new attributes cpld_i2c_xfer (binary I2C pass-through, wire protocol,
allowed addresses, response layout) and i2c_bus_ctrl (three-step
handshake, magic values, release requirement, PCIe ordering note).
Documentation/ABI/testing/sysfs-timecard | 56 ++++++
drivers/ptp/ptp_ocp.c | 239 ++++++++++++++++++++++-
2 files changed, 290 insertions(+), 5 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-timecard b/Documentation/ABI/testing/sysfs-timecard
index 3ae41b7634ac..c62766df7a20 100644
--- a/Documentation/ABI/testing/sysfs-timecard
+++ b/Documentation/ABI/testing/sysfs-timecard
@@ -11,6 +11,62 @@ Contact: Jonathan Lemon <jonathan.lemon@gmail.com>
Description: This directory contains the attributes of the Nth timecard
registered.
+What: /sys/class/timecard/ocpN/cpld_i2c_xfer
+Date: July 2026
+Contact: Sagi Maimon <sagi.maimon@adva.com>
+Description: (RW) Binary sysfs attribute providing a raw I2C passthrough to
+ the CPLD and I2C mux on ADVA x1 TAP boards. Only present on
+ that board variant.
+
+ Each write initiates one I2C transaction. The write payload
+ must be exactly four header bytes followed by the write data:
+
+ Byte 0: slave address (only 0x40 and 0x74 are permitted)
+ Byte 1: number of bytes to write (0..67)
+ Byte 2: number of bytes to read back (0..20)
+ Byte 3: flags
+ bit 0 - suppress the repeated START before the
+ read segment (I2C_M_NOSTART); only valid
+ when both write and read lengths are
+ non-zero
+ Bytes 4..: write data (write_len bytes)
+
+ A subsequent read() returns:
+
+ Byte 0: status (0 = success, positive errno on error)
+ Bytes 1..: read data (read_len bytes), present only when
+ status is 0 and read_len > 0
+
+ The write and read portions of the sysfs file share a single
+ per-device response buffer protected by a mutex; a single
+ open() / write() / read() sequence must be used to avoid
+ data races between concurrent users.
+
+ Only slave addresses 0x40 (Lattice CPLD) and 0x74 (PCA9548
+ I2C mux) are accepted; all others return EPERM.
+
+What: /sys/class/timecard/ocpN/i2c_bus_ctrl
+Date: July 2026
+Contact: Sagi Maimon <sagi.maimon@adva.com>
+Description: (RW) Exposes the MicroBlaze I2C bus arbitration register for
+ the shared I2C bus on ADVA x1 and x2 TAP boards. Only
+ present when the board has a pps_select register block.
+
+ Userspace must complete a three-step handshake before
+ driving the bus:
+
+ 1. Read - value must be 0x00000000 (bus is free).
+ 2. Write - 0x0000ffff (request ownership).
+ 3. Poll - read until the value is 0xffffffff (MicroBlaze
+ has acknowledged the handover).
+
+ After all I2C traffic is complete the bus must be released
+ by writing 0x00000000.
+
+ The poll read is a PCIe non-posted read and therefore also
+ flushes the preceding posted write to the FPGA; no
+ additional read-back is required for ordering.
+
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..79fec5161c99 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -163,7 +163,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 +417,11 @@ struct ptp_ocp {
dpll_tracker tracker;
int signals_nr;
int freq_in_nr;
+ /* cpld_i2c_xfer sysfs (adva_x1) */
+ struct mutex tap_i2c_lock;
+ int tap_i2c_adap_nr; /* adapter nr; -1 if absent */
+ u8 tap_i2c_rsp[21]; /* [status, read_data...] */
+ size_t tap_i2c_rsp_len;
};
#define OCP_REQ_TIMESTAMP BIT(0)
@@ -4224,6 +4230,205 @@ static const struct ocp_attr_group art_timecard_groups[] = {
{ },
};
+/*
+ * i2c_bus_ctrl exposes the MicroBlaze I2C bus arbitration register.
+ *
+ * The shared bus requires a three-step handshake before use:
+ * read 0x00000000 (free) -> write 0x0000ffff (request) ->
+ * poll until 0xffffffff (MicroBlaze confirms release).
+ *
+ * The poll is a PCIe non-posted read, which also flushes the preceding
+ * posted write to the FPGA, so no separate kernel read-back is needed.
+ */
+static ssize_t
+i2c_bus_ctrl_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct ptp_ocp *bp = dev_get_drvdata(dev);
+
+ if (!bp->pps_select)
+ return -ENODEV;
+ return sysfs_emit(buf, "0x%08x\n",
+ ioread32(&bp->pps_select->i2c_bus_ctrl));
+}
+
+static ssize_t
+i2c_bus_ctrl_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct ptp_ocp *bp = dev_get_drvdata(dev);
+ u32 val;
+
+ if (!bp->pps_select)
+ return -ENODEV;
+ if (kstrtou32(buf, 0, &val))
+ return -EINVAL;
+ iowrite32(val, &bp->pps_select->i2c_bus_ctrl);
+ return count;
+}
+
+static DEVICE_ATTR_RW(i2c_bus_ctrl);
+
+/*
+ * cpld_i2c_xfer - sysfs binary I2C passthrough for adva_x1 TAP CPLD.
+ *
+ * write: [addr][write_len][read_len][flags][write_data...]
+ * flags bit 0: I2C_M_NOSTART on the read segment
+ * read: [status][read_data...]
+ * status 0 = success, else positive errno
+ *
+ * Only addresses 0x40 (CPLD) and 0x74 (mux) are permitted.
+ */
+#define TAP_I2C_ALLOWED_ADDRS_NUM 2
+static const u8 tap_i2c_allowed_addrs[TAP_I2C_ALLOWED_ADDRS_NUM] = {
+ 0x40, /* CPLD */
+ 0x74, /* mux */
+};
+
+/*
+ * tap_i2c_errno_to_byte - encode a kernel errno as a one-byte status.
+ *
+ * Errnos > 255 (e.g. ENOTSUPP=524) or multiples of 256 would truncate
+ * to a wrong or zero value with a plain (u8) cast. Map those to EIO.
+ */
+static u8 tap_i2c_errno_to_byte(int err)
+{
+ int val = (err < 0) ? -err : EIO;
+
+ return (val > 0 && val <= 0xFF) ? (u8)val : EIO;
+}
+
+#define TAP_I2C_REQ_HDR_LEN 4
+#define TAP_I2C_MAX_WRITE_LEN 67
+#define TAP_I2C_MAX_READ_LEN 20
+#define TAP_I2C_FLAG_NOSTART BIT(0)
+
+static ssize_t
+ptp_ocp_cpld_i2c_write(struct file *file, struct kobject *kobj,
+ const struct bin_attribute *attr,
+ char *buf, loff_t off, size_t count)
+{
+ struct ptp_ocp *bp = dev_get_drvdata(kobj_to_dev(kobj));
+ const u8 *req = (const u8 *)buf;
+ u8 addr, write_len, read_len, flags;
+ struct i2c_adapter *adap;
+ struct i2c_msg msgs[2];
+ u8 *rdbuf = NULL;
+ int nmsgs, ret, i;
+
+ /* Each write is one atomic request; non-zero offset means a
+ * mid-buffer pwrite() which would misparse the header.
+ */
+ if (off != 0)
+ return -EINVAL;
+ if (count < TAP_I2C_REQ_HDR_LEN || count > TAP_I2C_REQ_HDR_LEN + TAP_I2C_MAX_WRITE_LEN)
+ return -EINVAL;
+
+ addr = req[0];
+ write_len = req[1];
+ read_len = req[2];
+ flags = req[3];
+
+ /* Validate */
+ for (i = 0; i < TAP_I2C_ALLOWED_ADDRS_NUM; i++)
+ if (addr == tap_i2c_allowed_addrs[i])
+ break;
+ if (i == TAP_I2C_ALLOWED_ADDRS_NUM)
+ return -EPERM;
+
+ if (write_len > TAP_I2C_MAX_WRITE_LEN)
+ return -EINVAL;
+ if (read_len > TAP_I2C_MAX_READ_LEN)
+ return -EINVAL;
+ if (write_len + TAP_I2C_REQ_HDR_LEN > count)
+ return -EINVAL;
+ if (write_len == 0 && read_len == 0)
+ return -EINVAL;
+ /* I2C_M_NOSTART suppresses the repeated START between write and read
+ * segments; it has no meaning on a first-and-only message.
+ */
+ if ((flags & TAP_I2C_FLAG_NOSTART) && write_len == 0)
+ return -EINVAL;
+
+ /* i2c_get_adapter() takes a reference under core_lock; safe against
+ * concurrent adapter unbind.
+ */
+ adap = i2c_get_adapter(READ_ONCE(bp->tap_i2c_adap_nr));
+ if (!adap)
+ return -ENODEV;
+
+ nmsgs = 0;
+ if (write_len > 0) {
+ msgs[nmsgs].addr = addr;
+ msgs[nmsgs].flags = 0;
+ msgs[nmsgs].len = write_len;
+ msgs[nmsgs].buf = (u8 *)req + TAP_I2C_REQ_HDR_LEN;
+ nmsgs++;
+ }
+ if (read_len > 0) {
+ u16 rd_flags = I2C_M_RD;
+
+ if (flags & TAP_I2C_FLAG_NOSTART)
+ rd_flags |= I2C_M_NOSTART;
+ msgs[nmsgs].addr = addr;
+ msgs[nmsgs].flags = rd_flags | I2C_M_DMA_SAFE;
+ msgs[nmsgs].len = read_len;
+ rdbuf = kzalloc(read_len, GFP_KERNEL);
+ if (!rdbuf) {
+ i2c_put_adapter(adap);
+ return -ENOMEM;
+ }
+ msgs[nmsgs].buf = rdbuf;
+ nmsgs++;
+ }
+
+ /* Serialise transfer+publish so concurrent writers cannot overwrite
+ * each other's response in tap_i2c_rsp.
+ */
+ mutex_lock(&bp->tap_i2c_lock);
+ ret = i2c_transfer(adap, msgs, nmsgs);
+ if (ret == nmsgs) {
+ bp->tap_i2c_rsp[0] = 0;
+ if (read_len > 0)
+ memcpy(&bp->tap_i2c_rsp[1], rdbuf, read_len);
+ bp->tap_i2c_rsp_len = 1 + read_len;
+ ret = count;
+ } else {
+ bp->tap_i2c_rsp[0] = tap_i2c_errno_to_byte(ret);
+ bp->tap_i2c_rsp_len = 1;
+ ret = (ret < 0) ? ret : -EIO;
+ }
+ mutex_unlock(&bp->tap_i2c_lock);
+ kfree(rdbuf);
+ i2c_put_adapter(adap);
+
+ return ret;
+}
+
+static ssize_t
+ptp_ocp_cpld_i2c_read(struct file *file, struct kobject *kobj,
+ const struct bin_attribute *attr,
+ char *buf, loff_t off, size_t count)
+{
+ struct ptp_ocp *bp = dev_get_drvdata(kobj_to_dev(kobj));
+ ssize_t ret;
+
+ mutex_lock(&bp->tap_i2c_lock);
+ if (off >= bp->tap_i2c_rsp_len) {
+ ret = 0;
+ } else {
+ ret = min(count, bp->tap_i2c_rsp_len - (size_t)off);
+ memcpy(buf, bp->tap_i2c_rsp + off, ret);
+ }
+ mutex_unlock(&bp->tap_i2c_lock);
+ return ret;
+}
+
+static const struct bin_attribute tap_i2c_bin_attr = {
+ .attr = { .name = "cpld_i2c_xfer", .mode = 0600 },
+ .write = ptp_ocp_cpld_i2c_write,
+ .read = ptp_ocp_cpld_i2c_read,
+};
+
static struct attribute *adva_timecard_attrs[] = {
&dev_attr_serialnum.attr,
&dev_attr_gnss_sync.attr,
@@ -4272,11 +4477,18 @@ 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_i2c_bus_ctrl.attr,
+ NULL,
+};
+
+static const struct bin_attribute *const bin_adva_x1_timecard_attrs[] = {
+ &tap_i2c_bin_attr,
NULL,
};
static const struct attribute_group adva_timecard_x1_group = {
- .attrs = adva_timecard_x1_attrs,
+ .attrs = adva_timecard_x1_attrs,
+ .bin_attrs = bin_adva_x1_timecard_attrs,
};
static const struct ocp_attr_group adva_timecard_x1_groups[] = {
@@ -4902,6 +5114,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->tap_i2c_lock);
device_unregister(&bp->dev);
}
@@ -5078,6 +5291,17 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (err)
goto out_disable;
+ /* Must be before the first error path that calls ptp_ocp_detach(),
+ * so mutex_destroy() always runs on an initialised mutex.
+ * Must also be before ptp_ocp_register_resources(): the I2C bus
+ * notifier (ptp_ocp_i2c_notifier_call) fires when the adapter
+ * registers and stores the adapter number in tap_i2c_adap_nr; the
+ * -1 sentinel below must already be written so that a notifier
+ * firing during registration is never overwritten by this init.
+ */
+ mutex_init(&bp->tap_i2c_lock);
+ bp->tap_i2c_adap_nr = -1;
+
INIT_DELAYED_WORK(&bp->sync_work, ptp_ocp_sync_work);
/* compat mode.
@@ -5217,11 +5441,16 @@ ptp_ocp_i2c_notifier_call(struct notifier_block *nb,
found:
bp = dev_get_drvdata(dev);
- if (add)
+ if (add) {
ptp_ocp_symlink(bp, child, "i2c");
- else
+ /* Cache adapter number; cpld_i2c_xfer uses i2c_get_adapter()
+ * for a reference-counted, unbind-safe lookup.
+ */
+ WRITE_ONCE(bp->tap_i2c_adap_nr, i2c_verify_adapter(child)->nr);
+ } else {
+ WRITE_ONCE(bp->tap_i2c_adap_nr, -1); /* invalidate before free */
sysfs_remove_link(&bp->dev.kobj, "i2c");
-
+ }
return 0;
}
--
2.47.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v6] ptp: ocp: add CPLD ISP support for ADVA TimeCard X1
2026-07-23 14:39 [PATCH v6] ptp: ocp: add CPLD ISP support for ADVA TimeCard X1 Sagi Maimon
@ 2026-07-27 11:02 ` Vadim Fedorenko
2026-07-27 21:03 ` Jakub Kicinski
0 siblings, 1 reply; 3+ messages in thread
From: Vadim Fedorenko @ 2026-07-27 11:02 UTC (permalink / raw)
To: Sagi Maimon, jonathan.lemon, richardcochran, andrew+netdev, davem,
edumazet, kuba, pabeni
Cc: linux-kernel, netdev
On 23/07/2026 15:39, 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 taking ownership of this bus and
> exposing the required interfaces through sysfs, allowing userspace tools
> to perform CPLD programming.
>
> To limit the scope of this functionality, sysfs-based I2C access is
> restricted to the ADVA TimeCard X1 variant and only for the two I2C
> slave addresses used during ISP (0x40 CPLD, 0x74 mux).
>
> Add two sysfs attributes under /sys/class/timecard/ocpN/ (x1 only):
>
> i2c_bus_ctrl - arbitrate the shared I2C bus from the MicroBlaze via
> a three-step read/write/poll handshake
>
> cpld_i2c_xfer - binary passthrough for I2C transactions to the CPLD
> and its PCA9548 mux; one atomic request per write()
>
> Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
> ---
>
> Addressed comments from:
> - Jakub Kicinski :https://lore.kernel.org/all/20260722184418.266546-1-kuba@kernel.org/
>
> Changes since v5:
> - ptp_ocp.c: move mutex_init(&bp->tap_i2c_lock) and
> bp->tap_i2c_adap_nr = -1 to before the first error path that
> reaches ptp_ocp_detach(), so mutex_destroy() never operates on an
> uninitialised mutex (CONFIG_DEBUG_MUTEXES splat on the
> pci_alloc_irq_vectors() failure path).
> - ptp_ocp.c: reword the ordering comment above mutex_init /
> tap_i2c_adap_nr to correctly describe the notifier race: the -1
> sentinel must precede ptp_ocp_register_resources() so that a
> ptp_ocp_i2c_notifier_call() firing during adapter registration is
> not overwritten by this initialisation line; ptp_ocp_adva_board_init()
> does not touch tap_i2c_adap_nr and was wrongly cited in the
> previous wording.
> - Documentation/ABI/testing/sysfs-timecard: add entries for the two
> new attributes cpld_i2c_xfer (binary I2C pass-through, wire protocol,
> allowed addresses, response layout) and i2c_bus_ctrl (three-step
> handshake, magic values, release requirement, PCIe ordering note).
>
> Documentation/ABI/testing/sysfs-timecard | 56 ++++++
> drivers/ptp/ptp_ocp.c | 239 ++++++++++++++++++++++-
> 2 files changed, 290 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-timecard b/Documentation/ABI/testing/sysfs-timecard
> index 3ae41b7634ac..c62766df7a20 100644
> --- a/Documentation/ABI/testing/sysfs-timecard
> +++ b/Documentation/ABI/testing/sysfs-timecard
> @@ -11,6 +11,62 @@ Contact: Jonathan Lemon <jonathan.lemon@gmail.com>
> Description: This directory contains the attributes of the Nth timecard
> registered.
>
> +What: /sys/class/timecard/ocpN/cpld_i2c_xfer
> +Date: July 2026
> +Contact: Sagi Maimon <sagi.maimon@adva.com>
> +Description: (RW) Binary sysfs attribute providing a raw I2C passthrough to
> + the CPLD and I2C mux on ADVA x1 TAP boards. Only present on
> + that board variant.
> +
> + Each write initiates one I2C transaction. The write payload
> + must be exactly four header bytes followed by the write data:
> +
> + Byte 0: slave address (only 0x40 and 0x74 are permitted)
> + Byte 1: number of bytes to write (0..67)
> + Byte 2: number of bytes to read back (0..20)
> + Byte 3: flags
> + bit 0 - suppress the repeated START before the
> + read segment (I2C_M_NOSTART); only valid
> + when both write and read lengths are
> + non-zero
> + Bytes 4..: write data (write_len bytes)
> +
> + A subsequent read() returns:
> +
> + Byte 0: status (0 = success, positive errno on error)
> + Bytes 1..: read data (read_len bytes), present only when
> + status is 0 and read_len > 0
> +
> + The write and read portions of the sysfs file share a single
> + per-device response buffer protected by a mutex; a single
> + open() / write() / read() sequence must be used to avoid
> + data races between concurrent users.
> +
> + Only slave addresses 0x40 (Lattice CPLD) and 0x74 (PCA9548
> + I2C mux) are accepted; all others return EPERM.
> +
> +What: /sys/class/timecard/ocpN/i2c_bus_ctrl
> +Date: July 2026
> +Contact: Sagi Maimon <sagi.maimon@adva.com>
> +Description: (RW) Exposes the MicroBlaze I2C bus arbitration register for
> + the shared I2C bus on ADVA x1 and x2 TAP boards. Only
> + present when the board has a pps_select register block.
> +
> + Userspace must complete a three-step handshake before
> + driving the bus:
> +
> + 1. Read - value must be 0x00000000 (bus is free).
> + 2. Write - 0x0000ffff (request ownership).
> + 3. Poll - read until the value is 0xffffffff (MicroBlaze
> + has acknowledged the handover).
> +
> + After all I2C traffic is complete the bus must be released
> + by writing 0x00000000.
> +
> + The poll read is a PCIe non-posted read and therefore also
> + flushes the preceding posted write to the FPGA; no
> + additional read-back is required for ordering.
> +
> 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..79fec5161c99 100644
> --- a/drivers/ptp/ptp_ocp.c
> +++ b/drivers/ptp/ptp_ocp.c
> @@ -163,7 +163,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 +417,11 @@ struct ptp_ocp {
> dpll_tracker tracker;
> int signals_nr;
> int freq_in_nr;
> + /* cpld_i2c_xfer sysfs (adva_x1) */
> + struct mutex tap_i2c_lock;
> + int tap_i2c_adap_nr; /* adapter nr; -1 if absent */
> + u8 tap_i2c_rsp[21]; /* [status, read_data...] */
I don't like the idea of having a buffer in a global structure. And it
doesn't look like you can actually serialize access. Imaging 2 processes
manipulating this i2c bus. I can be that mutex will put them like:
1. process1 -> write cmd
2. process2 -> write cmd
3. process1 -> read result
That means the data will be a mess.
That's why it is a really bad thing to allow direct access from the user
space. Is it possible to add a property per command?
> + size_t tap_i2c_rsp_len;
> };
>
[...]
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v6] ptp: ocp: add CPLD ISP support for ADVA TimeCard X1
2026-07-27 11:02 ` Vadim Fedorenko
@ 2026-07-27 21:03 ` Jakub Kicinski
0 siblings, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2026-07-27 21:03 UTC (permalink / raw)
To: Vadim Fedorenko
Cc: Sagi Maimon, jonathan.lemon, richardcochran, andrew+netdev, davem,
edumazet, pabeni, linux-kernel, netdev
On Mon, 27 Jul 2026 12:02:53 +0100 Vadim Fedorenko wrote:
> I don't like the idea of having a buffer in a global structure. And it
> doesn't look like you can actually serialize access. Imaging 2 processes
> manipulating this i2c bus. I can be that mutex will put them like:
>
> 1. process1 -> write cmd
> 2. process2 -> write cmd
> 3. process1 -> read result
>
> That means the data will be a mess.
>
> That's why it is a really bad thing to allow direct access from the user
> space. Is it possible to add a property per command?
At a glance - adding I2C support via 2 magic files in sysfs is rather
odd. Either expose a real i2c bus or make the driver sends the
appropriate commands. We are opposed to raw device write interfaces in
netdev.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-07-27 21:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-23 14:39 [PATCH v6] ptp: ocp: add CPLD ISP support for ADVA TimeCard X1 Sagi Maimon
2026-07-27 11:02 ` Vadim Fedorenko
2026-07-27 21:03 ` Jakub Kicinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox