Re: [PATCH] soundwire: intel_auxdevice: Don't disable IRQs before removing children

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



On 16/12/2024 5:35 pm, Pierre-Louis Bossart wrote:
On 12/12/24 5:02 AM, Charles Keepax wrote:
Currently the auxiliary device for the link disables IRQs before it
calls sdw_bus_master_delete(). This has the side effect that
none of the devices on the link can access their own registers whilst
their remove functions run, because the IRQs are required for bus
transactions to function. Obviously, devices should be able to access
their own registers during disable to park the device suitably.

What does 'park the device suitably' mean really? That sounds scary. What happens if the manager abruptly ceases to operate and yanks the power? Does the device catch on fire? On a related note, the manager should *never* expect to find the device in a 'suitable' state but always proceed with complete re-initialization.

It would be good to expand on the issue, introducing new locks is one of those "no good deed goes unpunished" situations.

It would appear the reason for the disabling of the IRQs is that the IRQ
handler iterates through a linked list of all the links, once a link is
removed the memory pointed at by this linked list is freed, but not
removed from the linked_list itself. Add a list_del() for the linked
list item, note whilst the list itself is contained in the intel_init
portion of the code, the list remove needs to be attached to the
auxiliary device for the link, since that owns the memory that the list
points at. Locking is also required to ensure the IRQ handler runs
before or after any additions/removals from the list.

that sounds very detailed but the initial reason for all this is still unclear.

For example, if the bus driver module is unloaded, the kernel will call
remove() on all the child drivers. The bus should remain functional
while the child drivers deal with this unexpected unload. This could
for example be writing some registers to put the peripheral into a
low-power state. On ACPI systems the drivers don't have control of
regulators so can't just pull power from the peripheral.





[Index of Archives]     [Pulseaudio]     [Linux Audio Users]     [ALSA Devel]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux