Re: [PATCH v3 8/9] i2c: Support dynamic address translation

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

 



Hi Romain,

On 25/11/2024 10:45, Romain Gantois wrote:
The i2c-atr module keeps a list of associations between I2C client aliases
and I2C addresses. This represents the address translation table which is
programmed into an ATR channel at any given time. This list is only updated
when a new client is bound to the channel.

However in some cases, an ATR channel can have more downstream clients than
available aliases. One example of this is an SFP module that is bound to an
FPC202 port. The FPC202 port can only access up to two logical I2C
addresses. However, the SFP module may expose up to three logical I2C
addresses: its EEPROM on 7-bit addresses 0x50 and 0x51, and a PHY
transceiver on address 0x56.

In cases like these, it is necessary to reconfigure the channel's
translation table on the fly, so that all three I2C addresses can be
accessed when needed.

Add an optional "dynamic_c2a" flag to the i2c-atr module which allows it to
provide on-the-fly address translation. This is achieved by modifying an
ATR channel's translation table whenever an I2C transaction with unmapped
clients is requested.

Add a mutex to protect alias_list. This prevents
i2c_atr_dynamic_attach/detach_addr from racing with the bus notifier
handler to modify alias_list.

Signed-off-by: Romain Gantois <romain.gantois@xxxxxxxxxxx>
---
  drivers/i2c/i2c-atr.c         | 244 ++++++++++++++++++++++++++++++++----------
  drivers/media/i2c/ds90ub960.c |   2 +-
  include/linux/i2c-atr.h       |  13 ++-
  3 files changed, 202 insertions(+), 57 deletions(-)

This fails with:

WARNING: CPU: 1 PID: 360 at lib/list_debug.c:35 __list_add_valid_or_report+0xe4/0x100

as the i2c_atr_create_c2a() calls list_add(), but i2c_atr_attach_addr(), which is changed to use i2c_atr_create_c2a(), also calls list_add().

Also, if you add i2c_atr_create_c2a() which hides the allocation and list_add, I think it makes sense to add a i2c_atr_destroy_c2a() to revert that.

There's also a memory error "BUG: KASAN: slab-use-after-free in __lock_acquire+0xc4/0x375c" (see below) when unloading the ub960 or ub953 driver. I haven't looked at that yet.

 Tomi

  316.204193] BUG: KASAN: slab-use-after-free in __lock_acquire+0xc4/0x375c
[  316.211059] Read of size 4 at addr cc44d558 by task rmmod/1505
[  316.216979]
[  316.218475] CPU: 1 UID: 0 PID: 1505 Comm: rmmod Not tainted 6.12.0+ #2
[  316.225097] Hardware name: Generic DRA74X (Flattened Device Tree)
[  316.231231] Call trace:
[  316.231262]  unwind_backtrace from show_stack+0x18/0x1c
[  316.239105]  show_stack from dump_stack_lvl+0x6c/0x8c
[  316.244232]  dump_stack_lvl from print_report+0x130/0x538
[  316.249694]  print_report from kasan_report+0x98/0xd8
[  316.254821]  kasan_report from __lock_acquire+0xc4/0x375c
[  316.260284]  __lock_acquire from lock_acquire.part.0+0x128/0x340
[  316.266357]  lock_acquire.part.0 from _raw_spin_lock+0x4c/0x5c
[ 316.272247] _raw_spin_lock from i2c_atr_release_alias+0x20/0x98 [i2c_atr] [ 316.279235] i2c_atr_release_alias [i2c_atr] from i2c_atr_bus_notifier_call+0x150/0x46c [i2c_atr] [ 316.288238] i2c_atr_bus_notifier_call [i2c_atr] from notifier_call_chain+0x68/0x23c [ 316.296081] notifier_call_chain from blocking_notifier_call_chain+0x5c/0x74
[  316.303192]  blocking_notifier_call_chain from bus_notify+0x3c/0x48
[  316.309539]  bus_notify from device_del+0xf0/0x514
[  316.314392]  device_del from device_unregister+0x28/0x80
[  316.319763]  device_unregister from __unregister_client+0x84/0x90
[  316.325927]  __unregister_client from device_for_each_child+0xc0/0x12c
[  316.332550]  device_for_each_child from i2c_del_adapter+0x28c/0x45c
[ 316.338897] i2c_del_adapter from i2c_atr_del_adapter+0x10c/0x250 [i2c_atr] [ 316.345947] i2c_atr_del_adapter [i2c_atr] from ub953_remove+0x48/0xcc [ds90ub953]
[  316.353637]  ub953_remove [ds90ub953] from i2c_device_remove+0x54/0xf8
[ 316.360260] i2c_device_remove from device_release_driver_internal+0x218/0x2a8 [ 316.367553] device_release_driver_internal from bus_remove_device+0x130/0x1cc
[  316.374847]  bus_remove_device from device_del+0x1f8/0x514
[  316.380401]  device_del from device_unregister+0x28/0x80
[  316.385772]  device_unregister from ub960_remove+0xb0/0x244 [ds90ub960]
[  316.392517]  ub960_remove [ds90ub960] from i2c_device_remove+0x54/0xf8
[ 316.399139] i2c_device_remove from device_release_driver_internal+0x218/0x2a8
[  316.406433]  device_release_driver_internal from driver_detach+0x6c/0xc4
[  316.413238]  driver_detach from bus_remove_driver+0xa0/0x134
[  316.418945]  bus_remove_driver from i2c_del_driver+0x48/0x84
[  316.424682]  i2c_del_driver from sys_delete_module+0x280/0x3cc
[  316.430572]  sys_delete_module from ret_fast_syscall+0x0/0x1c





[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux