On Sun, 2024-09-08 at 23:07 +0200, Heiko Stuebner wrote: > The hid-sensor-hub creates the individual device structs and > transfers them > to the created mfd platform-devices via the platform_data in the > mfd_cell. > > Before commit e651a1da442a ("HID: hid-sensor-hub: Allow parallel > synchronous reads") > the sensor-hub was managing access centrally, with one "completion" > in the > hub's data structure, which needed to be finished on removal at the > latest. > > The mentioned commit then moved this central management to each hid > sensor > device, resulting on a completion in each struct > hid_sensor_hub_device. > The remove procedure was adapted to go through all sensor devices and > finish any pending "completion". > > What this didn't take into account was, platform_device_add_data() > that is > used by mfd_add{_hotplug}_devices() does a kmemdup on the submitted > platform-data. So the data the platform-device gets is a copy of the > original data, meaning that the device worked on a different > completion > than what sensor_hub_remove() currently wants to access. > > To fix that, use device_for_each_child() to go through each child- > device > similar to how mfd_remove_devices() unregisters the devices later and > with that get the live platform_data to finalize the correct > completion. > > Fixes: e651a1da442a ("HID: hid-sensor-hub: Allow parallel synchronous > reads") > Signed-off-by: Heiko Stuebner <heiko@xxxxxxxxx> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> > --- > drivers/hid/hid-sensor-hub.c | 21 ++++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor- > hub.c > index 26e93a331a51..3cd00afa453a 100644 > --- a/drivers/hid/hid-sensor-hub.c > +++ b/drivers/hid/hid-sensor-hub.c > @@ -730,23 +730,30 @@ static int sensor_hub_probe(struct hid_device > *hdev, > return ret; > } > > +static int sensor_hub_finalize_pending_fn(struct device *dev, void > *data) > +{ > + struct hid_sensor_hub_device *hsdev = dev->platform_data; > + > + if (hsdev->pending.status) > + complete(&hsdev->pending.ready); > + > + return 0; > +} > + > static void sensor_hub_remove(struct hid_device *hdev) > { > struct sensor_hub_data *data = hid_get_drvdata(hdev); > unsigned long flags; > - int i; > > hid_dbg(hdev, " hardware removed\n"); > hid_hw_close(hdev); > hid_hw_stop(hdev); > + > spin_lock_irqsave(&data->lock, flags); > - for (i = 0; i < data->hid_sensor_client_cnt; ++i) { > - struct hid_sensor_hub_device *hsdev = > - data- > >hid_sensor_hub_client_devs[i].platform_data; > - if (hsdev->pending.status) > - complete(&hsdev->pending.ready); > - } > + device_for_each_child(&hdev->dev, NULL, > + sensor_hub_finalize_pending_fn); > spin_unlock_irqrestore(&data->lock, flags); > + > mfd_remove_devices(&hdev->dev); > mutex_destroy(&data->mutex); > }