Hi All,
I've added patch 1/3 and 2/3 of this series to my review-hans
(soon to be for-next) branch now.
I'll send out a v2 of patch 3/3 to address the review comments.
Regards,
Hans
On 4/6/24 2:50 PM, Hans de Goede wrote:
> Not all subsystems support a device getting removed while there are
> still consumers of the device with a reference to the device.
>
> One example of this is the regulator subsystem. If a regulator gets
> unregistered while there are still drivers holding a reference
> a WARN() at drivers/regulator/core.c:5829 triggers, e.g.:
>
> WARNING: CPU: 1 PID: 1587 at drivers/regulator/core.c:5829 regulator_unregister
> Hardware name: Intel Corp. VALLEYVIEW C0 PLATFORM/BYT-T FFD8, BIOS BLADE_21.X64.0005.R00.1504101516 FFD8_X64_R_2015_04_10_1516 04/10/2015
> RIP: 0010:regulator_unregister
> Call Trace:
> <TASK>
> regulator_unregister
> devres_release_group
> i2c_device_remove
> device_release_driver_internal
> bus_remove_device
> device_del
> device_unregister
> x86_android_tablet_remove
>
> On the Lenovo Yoga Tablet 2 series the bq24190 charger chip also provides
> a 5V boost converter output for powering USB devices connected to the micro
> USB port, the bq24190-charger driver exports this as a Vbus regulator.
>
> On the 830 (8") and 1050 ("10") models this regulator is controlled by
> a platform_device and x86_android_tablet_remove() removes platform_device-s
> before i2c_clients so the consumer gets removed first.
>
> But on the 1380 (13") model there is a lc824206xa micro-USB switch
> connected over I2C and the extcon driver for that controls the regulator.
> The bq24190 i2c-client *must* be registered first, because that creates
> the regulator with the lc824206xa listed as its consumer. If the regulator
> has not been registered yet the lc824206xa driver will end up getting
> a dummy regulator.
>
> Since in this case both the regulator provider and consumer are I2C
> devices, the only way to ensure that the consumer is unregistered first
> is to unregister the I2C devices in reverse order of in which they were
> created.
>
> For consistency and to avoid similar problems in the future change
> x86_android_tablet_remove() to unregister all device types in reverse
> order.
>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> drivers/platform/x86/x86-android-tablets/core.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
> index a3415f1c0b5f..6559bb4ea730 100644
> --- a/drivers/platform/x86/x86-android-tablets/core.c
> +++ b/drivers/platform/x86/x86-android-tablets/core.c
> @@ -278,25 +278,25 @@ static void x86_android_tablet_remove(struct platform_device *pdev)
> {
> int i;
>
> - for (i = 0; i < serdev_count; i++) {
> + for (i = serdev_count - 1; i >= 0; i--) {
> if (serdevs[i])
> serdev_device_remove(serdevs[i]);
> }
>
> kfree(serdevs);
>
> - for (i = 0; i < pdev_count; i++)
> + for (i = pdev_count - 1; i >= 0; i--)
> platform_device_unregister(pdevs[i]);
>
> kfree(pdevs);
> kfree(buttons);
>
> - for (i = 0; i < spi_dev_count; i++)
> + for (i = spi_dev_count - 1; i >= 0; i--)
> spi_unregister_device(spi_devs[i]);
>
> kfree(spi_devs);
>
> - for (i = 0; i < i2c_client_count; i++)
> + for (i = i2c_client_count - 1; i >= 0; i--)
> i2c_unregister_device(i2c_clients[i]);
>
> kfree(i2c_clients);
