On Wed, Jan 05, 2022 at 09:26:58PM +0900, William Breathitt Gray wrote: > On Wed, Dec 29, 2021 at 04:44:18PM +0100, Uwe Kleine-König wrote: > > - I think intel-qep.c makes the counter unfunctional in > > intel_qep_remove before the counter is unregistered. > > Hello Uwe, > > Would you elaborate some more on this? I think intel_qep_remove() is > only called after the counter is unregistered because the struct > counter_device parent is set to &pci->dev in intel_qep_probe(). Am I > misunderstanding the removal path? If the counter device is unbound (e.g. via sysfs), the following calls are made: intel_qep_remove() (stopping the hardware?) devm_counter_release (devm callback of devm_counter_register or ..._add) then the release callbacks of the earlier devm functions My concern is, that in the timeslot between intel_qep_remove() and devm_counter_release() the device looks like a functional device and might be queried/reconfigured/... while the hardware is already dead. It's probably not a big issue (unless for example reading the counter this race window makes the hardware hang?), but it's at least ugly. Maybe the worst effect is that a counter value is missed (which is OK at unregister time). Still it would be nicer to first take down the counter device and only then stop the hardware. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature