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

[Sagi Maimon]

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.

On Tue, Jul 28, 2026 at 12:03 AM Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
>
> 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.