Re: [PATCH 2/8] iio: accel: bmc150: Don't make the remove function of the second accelerometer unregister itself

Hi,

On 5/22/21 8:06 PM, Andy Shevchenko wrote:

On Fri, May 21, 2021 at 11:23 PM Hans de Goede [off-list ref] wrote:

quoted

On machines with dual accelerometers described in a single ACPI fwnode,
the bmc150_accel_probe() instantiates a second i2c-client for the second
accelerometer.

A pointer to this manually instantiated second i2c-client is stored
inside the iio_dev's private-data through bmc150_set_second_device(),
so that the i2c-client can be unregistered from bmc150_accel_remove().

Before this commit bmc150_set_second_device() took only 1 argument so it
would store the pointer in private-data of the iio_dev belonging to the
manually instantiated i2c-client, leading to the bmc150_accel_remove()
call for the second_dev trying to unregister *itself* while it was
being removed, leading to a deadlock and rmmod hanging.

Change bmc150_set_second_device() to take 2 arguments: 1. The i2c-client
which is instantiating the second i2c-client for the 2nd accelerometer and
2. The second-device pointer itself (which also is an i2c-client).

This will store the second_device pointer in the private data of the
iio_dev belonging to the (ACPI instantiated) i2c-client for the first
accelerometer and will make bmc150_accel_remove() unregister the
second_device i2c-client when called for the first client,
avoiding the deadlock.

I would rather call it aux device (at least in the code). The
terminology maybe needs more clarification (like the main one in the
block with the display panel and aux in the keyboard).

If you disagree, ignore this comment. I have no strong opinion here
since I don't know the difference between them (accelerometers).

Thank you for your input, but both sensors are identical, so calling
one aux sounds of to me, so lets keep this as is.

Regards,

Hans