Received: by 2002:a05:6a10:9848:0:0:0:0 with SMTP id x8csp4223623pxf; Tue, 30 Mar 2021 02:23:48 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwJa5lqMga2iEgSRf4uGABXS9S0029shNcVMCBVE58D2bcuWX5ermuULEmpIP9k04rRvQEy X-Received: by 2002:a17:906:f203:: with SMTP id gt3mr31607450ejb.346.1617096228442; Tue, 30 Mar 2021 02:23:48 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1617096228; cv=none; d=google.com; s=arc-20160816; b=vmnaZjJjwy8b6utJl36PsvXuaYbdtQLPZ0PO0c6q2mDUYFOr+Mp3GbZ/aNrZWNPlZI 72j5o9TtT+dVu9G2tatumL906u7FNPrTcagpbXWxEbsAAtR0qudr/PCXDKVf9UhKbOqT eW9SeQ0AYsBek++rkmFyS3UGcmB3pkq51q4JfG7YwzJvMfdUGJCSLyWECOV0shs/WK1l NaxdnQjMLSX8TgZa3Uo2Stbo1vuK2urWC4dBK8NS1OibNCTA5eqxlg4j/dZm/KArcR7Q IHYHqWl75THLgoxz2SmWa8Gm8zsDuC9VWFWZr7FFV/MwT9U6KiV1hrmSsxAsXUnYsBHL j7BA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=wVl6dx7e0oWpG+IolS4wAAwf1DWhXCYxl3+7BE4Yhzw=; b=SP538+bRC1dkGScRs/qYv/lycMaQdegYkuSWDQxpWOnUDxlLAIFyUwkJsXXJWPMKBC 1JhjbAFv0jfdue1MRQu4u8IsCj734UnuCY3PsCC6id+48jvIV6IRP7EfP4GADdhux9mt TIXnmDPdriOZAlevHboF4Y23+EuFhR+IKsAh57R84bnoymvYekrWHHmoFt1O1TjnXBLF RgDBeqZ9MYeqM+Lo/GRM6rN0KP7tKhcFoPOsfl9P40X344XjptR9jZcayT84Q0jb7hOm A9t6Q4P0cNa1xU48Z5jCLP4yccvaBaSuz1DaoYCpQPwgXNUyt35FVjQ1pzlqKmyp/fpq paSA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=FwbX1h28; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id b5si14160007edq.537.2021.03.30.02.23.25; Tue, 30 Mar 2021 02:23:48 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=FwbX1h28; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231607AbhC3JWR (ORCPT + 99 others); Tue, 30 Mar 2021 05:22:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35224 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231684AbhC3JWQ (ORCPT ); Tue, 30 Mar 2021 05:22:16 -0400 Received: from mail-il1-x12e.google.com (mail-il1-x12e.google.com [IPv6:2607:f8b0:4864:20::12e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9DF78C061762; Tue, 30 Mar 2021 02:22:16 -0700 (PDT) Received: by mail-il1-x12e.google.com with SMTP id u2so13631275ilk.1; Tue, 30 Mar 2021 02:22:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=wVl6dx7e0oWpG+IolS4wAAwf1DWhXCYxl3+7BE4Yhzw=; b=FwbX1h28kR45mojqnBesa55JLZzdeBdBI9HVBxDCBEPHooYF/sv2J7ZM61+yZtTERp 2N9A85IU04ASBmgdWDlHSkXj3ovORSV0wbGjPSYevDRGBmSgySJLvNyd4HJ252KG/ZpH RN1/iXx9yNg9LCj6h+oZCIzRXKWr2aWa6HY5mlRzSQVvtldGBHrdhQc01w/wU/vcLe5H kHbXNS1Zd4ri7LQlZUO7rI8Ar3PvbJ8ATbNLFzbuCdz5miSxnSNao2qhXo/X7apo7Vyg xmwcOq7+xi3ajJyjEkvDHTNBYWaDiiqYEsscuaZKygCKvmKl7DUsZ4dFBxiVWVxU3UBC mbPQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=wVl6dx7e0oWpG+IolS4wAAwf1DWhXCYxl3+7BE4Yhzw=; b=fRfLQiHW/bJD/Kt9yKFX8QubQc6YsQND4se4XJjhbsjWVfNt5LMJpkkhOY/z7Eb+Pu TmnYR61PcfMl3bGlIhQ8kkysmTNiqvDr9XtmHNYb+h7ClSFFp6yJ7LPYrtTGMYZxtQ0X W0i3WdpskLRJUqobxKfNSYzw40c7bp3EdXve9ssE7Gx0sD7tJ306EKJDR7i+sr9ItA20 xrwHjOuJW4f9JneqArfl7wscS3FvQc/dVDAKk7XQx+CZLEU4QFqQqzitME/tEN4RnRr6 uN0O0G5nkDJTDsWEhnRT/vUKI69LvChWBRSRDzcchnXcb/52LqFzWRMR2AoLTjfu7y6x fXLQ== X-Gm-Message-State: AOAM531ryVc4/fgS6l5Lqq3lblV/gkVaE5PnHgOS9a5Hp59z4vwu8KEI nx77Z4eNfCNO7fPn1t0DbrMWc4XxGbPhDmTPBDDAmtWglV0U9Q== X-Received: by 2002:a92:c244:: with SMTP id k4mr6816668ilo.303.1617096135961; Tue, 30 Mar 2021 02:22:15 -0700 (PDT) MIME-Version: 1.0 References: <20210324125548.45983-1-aardelean@deviqon.com> <0fba6355-aaec-b214-cf12-1add08cfca38@redhat.com> In-Reply-To: <0fba6355-aaec-b214-cf12-1add08cfca38@redhat.com> From: Alexandru Ardelean Date: Tue, 30 Mar 2021 12:22:04 +0300 Message-ID: Subject: Re: [PATCH 00/10] platform/x86: toshiba_acpi: move acpi add/remove to device-managed routines To: Hans de Goede Cc: Alexandru Ardelean , platform-driver-x86@vger.kernel.org, LKML , linux-iio , coproscefalo@gmail.com, mgross@linux.intel.com, Jonathan Cameron , linux@deviqon.com Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 30, 2021 at 11:21 AM Hans de Goede wrote: > > Hi Alexadru, Jonathan, > > On 3/24/21 1:55 PM, Alexandru Ardelean wrote: > > This changeset tries to do a conversion of the toshiba_acpi driver to use > > only device-managed routines. The driver registers as a singleton, so no > > more than one device can be registered at a time. > > > > My main intent here is to try to convert the iio_device_alloc() and > > iio_device_register() to their devm_ variants. > > > > Usually, when converting a registration call to device-managed variant, the > > init order must be preserved. And the deregistration order must be a mirror > > of the registration (in reverse order). > > > > This change tries to do that, by using devm_ variants where available and > > devm_add_action_or_reset() where this isn't possible. > > Some deregistration ordering is changed, because it wasn't exactly > > mirroring (in reverse) the init order. > > > > For the IIO subsystem, the toshiba_acpi driver is the only user of > > iio_device_alloc(). If this changeset is accepted (after discussion), I > > will propose to remove the iio_device_alloc() function. > > > > While I admit this may look like an overzealous effort to use devm_ > > everywhere (in IIO at least), for me it's a fun/interesting excercise. > > Alexadru, thank you for the patches. > > Jonathan, thank you for the reviews. > > To be honest I'm currently inclined to not accept / merge these patches, > this is based on 2 assumptions from me, which might be wrong. let me explain. > > If I understand things correctly, the main reason for this rework of > the toshiba_acpi code is to move iio_device_alloc() and iio_device_register() > to their devm_ variants, converting the last users in the tree ? yes well, we still have plenty of users iio_device_alloc() / iio_device_register() inside drivers/iio but the toshipa_acpi driver is the more quirky user of these functions [treewide] i wanted to jump on those simpler IIO cases, but i thought i would leave those to new contributors [for a while]; the complexity of those conversions is good enough to get some people started to contribute changes that are a bit more useful than checkpatch fixes, comment fixes [etc]; [personally] i feel that these devm_ conversions are complex enough to maybe get people wanting to dig more into some kernel design stuff > > This would allow these 2 iio functions to then be e.g. marked as static / > private helpers inside the iio core, so that all new users can only use > the devm_ versions. But if I'm reading Jonathan's reaction correctly then > Jonathan is not planning to do that because they might still be useful > in some cases. > > Jonathan have I correctly understood that you don't plan to make any > changes to the iio_device_alloc() and iio_device_register() functions > even if this gets merged ? > > Which brings me to my next assumption, Alexandru, I don't read anything > about testing anywhere. So I'm currently under the assumption that > you don't have any hardware using the toshiba_acpi driver and that this > is thus untested ? yes, i don't have any hw to test this > > The not being tested state is my main reason for not wanting to merge > this. The toshiba_acpi driver likely does not have a whole lot of users, > so the chances of someone running release candidates or even just the > latest kernels on hardware which uses it are small. This means that if > we accidentally introduce a bug with this series it might not get caught > until say lots of people start upgrading to Ubuntu 22.04 which is > the first Ubuntu kernel with your changes; and then at least one of the > hit users needs to have the skills to find us and get in contact about that. > > TL;DR: we might break stuff and if we do it might be a long time until we > find out we did and then we have been shipping broken code for ages... ack well, i don't insist in pushing this series; another thought was to just send bits of this set, which are simple enough to consider even on their own; maybe i'll look for a toshiba laptop with support for this stuff; i'll see thanks :) Alex > > Regards, > > Hans > > > > > > > > > Alexandru Ardelean (10): > > platform/x86: toshiba_acpi: bind life-time of toshiba_acpi_dev to > > parent > > platform/x86: toshiba_acpi: use devm_add_action_or_reset() for > > singleton clear > > platform/x86: toshiba_acpi: bind registration of miscdev object to > > parent > > platform/x86: toshiba_acpi: use device-managed functions for input > > device > > platform/x86: toshiba_acpi: register backlight with device-managed > > variant > > platform/x86: toshiba_acpi: use devm_led_classdev_register() for LEDs > > platform/x86: toshiba_acpi: use device-managed functions for > > accelerometer > > platform/x86: toshiba_acpi: use device-managed for wwan_rfkill > > management > > platform/x86: toshiba_acpi: use device-managed for sysfs removal > > platform/x86: toshiba_acpi: bind proc entries creation to parent > > > > drivers/platform/x86/toshiba_acpi.c | 249 +++++++++++++++++----------- > > 1 file changed, 150 insertions(+), 99 deletions(-) > > >