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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 19/01/2023 10:43, Luca Ceresoli wrote:
Hi Andy,

On Wed, 18 Jan 2023 19:43:23 +0200
Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:

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@xxxxxxxxxxxxxxxx/

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.

Which callbacks do you refer to? i2c_atr_ops? I don't think we can do
without the attach/detach_client ones, it's where the driver-specific
implementation is hooked for the generic ATR infra to call it.

However now I noticed the select/deselect ops are still there. IIRC
they are not used by any driver and in the past the plan was to just
remove them. Tomi, do you think there is a good reason to keep them?

I thought you had a reason to add them, so I didn't remove them =). I can drop them.

 Tomi




[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux