Re: [PATCH v6] ptp: ocp: add CPLD ISP support for ADVA TimeCard X1
From: Jakub Kicinski
Date: Tue Jul 28 2026 - 18:26:25 EST
On Tue, 28 Jul 2026 09:09:33 +0300 Sagi Maimon wrote:
> Thanks for the feedback.
>
> My initial implementation actually exposed a real I2C bus to user
> space. However, following Vadim Fedorenko's review comments:
>
> https://lore.kernel.org/all/c6aff5f7-e087-4bd9-b159-7adeb82e19f4@xxxxxxxxx/
>
> I changed the design and switched to a sysfs-based interface for the
> ADVA device only.
>
> From your comment, it sounds like neither of these approaches is
> acceptable in its current form. Could you please clarify what
> implementation you would consider appropriate for this use case?
>
> Specifically, would you prefer:
>
> exposing a proper I2C adapter/bus to user space, or
> implementing the required device operations directly in the driver and
> exposing only higher-level controls?
>
> I'd like to understand the expected direction before respinning the patch.
Reminder: please don't top post.
I'm not sure what hangs on that bus. The commit message seemed
to go into the topology but not into the "why" of the patch.
Assuming this is a fixed function bus within the device sysfs
attributes dedicated to what the user wants to do are likely
best. Again, hard to say for sure since I don't know what sits
on that bus.
Exposing what is effectively raw write over a custom attribute
is the worst possible choice. Neither does it use the standard i2c
infra, nor does it avoid raw access and per-vendor tooling.
Ideally you'd add dedicated attributes that are human-readable/writable
and have the driver translate the user intent into transactions on
the bus. I'd assume this is what Vadim expected in the email you quoted.