Re: [PATCH v7 0/7] i2c-atr and FPDLink

Andy Shevchenko
2023-01-18T17:45:17Z

On Wed, Jan 18, 2023 at 07:28:20PM +0200, Tomi Valkeinen wrote:
> On 18/01/2023 18:01, Andy Shevchenko wrote:
> > On Wed, Jan 18, 2023 at 02:40:24PM +0200, Tomi Valkeinen wrote:
> > > Hi,
> > >
> > > You can find the v6 from:
> > >
> > > https://lore.kernel.org/all/20230105140307.272052-1-tomi.valkeinen@ideasonboard.com/
> > >
> > > Main changes:
> > >
> > > * i2c-atr: Use bus notifier. This allows us to drop the patch that adds
> > > the attach_client/detach_client callbacks. On the downside, it removes
> > > the option for error handling if the translation setup fails, and also
> > > doesn't provide us the pointer to the i2c_board_info. I belive both
> > > are acceptable downsides.
> > >
> > > * Use fwnode in the fpdlink drivers instead of OF
> > >
> > > * Addressed all the review comments (I hope)
> > >
> > > * Lots of cosmetic or minor fixes which I came up while doing the fwnode
> > > change
> >
> > I believe my comments to the first driver applies to the next two, so please
> > address them whenever you are agree / it's possible / it makes sense.
> >
> > About ATR implementation. We have the i2c bus (Linux representation of
> > the driver model) and i2c_adapter and i2c_client objects there. Can't we
> > have an i2c_client_aliased in similar way and be transparent with users?

> Can you clarify what you mean here?
>
> The i2c_clients are not aware of the i2c-atr. They are normal i2c clients.
> The FPD-Link drivers are aware of the ATR, as the FPD-Link hardware contains
> the ATR support.

Can't that hardware be represented as I2C adapter? In such case the ATR specifics
can be hidden from the client (drivers).

I'm worrying about code duplication and other things that leak into drivers as
ATR callbacks. It might be that I didn't get how hw exactly functioning on this
level and why we need those callbacks.

--
With Best Regards,
Andy Shevchenko