* [PATCH v1 1/3] HID: intel-ish-hid: Drop if block with an always false condition @ 2021-02-06 15:13 Uwe Kleine-König 2021-02-06 15:13 ` [PATCH v1 2/3] HID: intel-ish-hid: Simplify logic in ishtp_cl_device_remove() Uwe Kleine-König ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Uwe Kleine-König @ 2021-02-06 15:13 UTC (permalink / raw) To: Srinivas Pandruvada, Jiri Kosina, Benjamin Tissoires, Enric Balletbo i Serra, Guenter Roeck Cc: Greg Kroah-Hartman, linux-input, linux-kernel A remove callback is only ever called for a bound device. So there is no need to check for device or driver being NULL. Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> --- drivers/hid/intel-ish-hid/ishtp/bus.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c index bba29cd36d29..ccd54f244503 100644 --- a/drivers/hid/intel-ish-hid/ishtp/bus.c +++ b/drivers/hid/intel-ish-hid/ishtp/bus.c @@ -257,17 +257,13 @@ static int ishtp_cl_bus_match(struct device *dev, struct device_driver *drv) static int ishtp_cl_device_remove(struct device *dev) { struct ishtp_cl_device *device = to_ishtp_cl_device(dev); - struct ishtp_cl_driver *driver; - - if (!device || !dev->driver) - return 0; + struct ishtp_cl_driver *driver = to_ishtp_cl_driver(dev->driver); if (device->event_cb) { device->event_cb = NULL; cancel_work_sync(&device->event_work); } - driver = to_ishtp_cl_driver(dev->driver); if (!driver->remove) { dev->driver = NULL; base-commit: 5c8fe583cce542aa0b84adc939ce85293de36e5e -- 2.29.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v1 2/3] HID: intel-ish-hid: Simplify logic in ishtp_cl_device_remove() 2021-02-06 15:13 [PATCH v1 1/3] HID: intel-ish-hid: Drop if block with an always false condition Uwe Kleine-König @ 2021-02-06 15:13 ` Uwe Kleine-König 2021-02-06 15:13 ` [PATCH v1 3/3] HID: intel-ish-hid: Make remove callback return void Uwe Kleine-König 2021-03-08 10:07 ` [PATCH v1 1/3] HID: intel-ish-hid: Drop if block with an always false condition Jiri Kosina 2 siblings, 0 replies; 9+ messages in thread From: Uwe Kleine-König @ 2021-02-06 15:13 UTC (permalink / raw) To: Srinivas Pandruvada, Jiri Kosina, Benjamin Tissoires, Enric Balletbo i Serra, Guenter Roeck Cc: Greg Kroah-Hartman, linux-input, linux-kernel There is only a single change in behavior: Now dev->driver isn't modified. Assigning to this variable is in the domain of the driver core only. (And it's done in __device_release_driver shortly after bus->remove() (i.e ishtp_cl_device_remove() here) returns.) Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> --- drivers/hid/intel-ish-hid/ishtp/bus.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c index ccd54f244503..7f36ce6187a1 100644 --- a/drivers/hid/intel-ish-hid/ishtp/bus.c +++ b/drivers/hid/intel-ish-hid/ishtp/bus.c @@ -258,19 +258,17 @@ static int ishtp_cl_device_remove(struct device *dev) { struct ishtp_cl_device *device = to_ishtp_cl_device(dev); struct ishtp_cl_driver *driver = to_ishtp_cl_driver(dev->driver); + int ret = 0; if (device->event_cb) { device->event_cb = NULL; cancel_work_sync(&device->event_work); } - if (!driver->remove) { - dev->driver = NULL; + if (driver->remove) + ret = driver->remove(device); - return 0; - } - - return driver->remove(device); + return ret; } /** -- 2.29.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v1 3/3] HID: intel-ish-hid: Make remove callback return void 2021-02-06 15:13 [PATCH v1 1/3] HID: intel-ish-hid: Drop if block with an always false condition Uwe Kleine-König 2021-02-06 15:13 ` [PATCH v1 2/3] HID: intel-ish-hid: Simplify logic in ishtp_cl_device_remove() Uwe Kleine-König @ 2021-02-06 15:13 ` Uwe Kleine-König 2021-03-08 10:07 ` [PATCH v1 1/3] HID: intel-ish-hid: Drop if block with an always false condition Jiri Kosina 2 siblings, 0 replies; 9+ messages in thread From: Uwe Kleine-König @ 2021-02-06 15:13 UTC (permalink / raw) To: Srinivas Pandruvada, Jiri Kosina, Benjamin Tissoires, Enric Balletbo i Serra, Guenter Roeck Cc: Greg Kroah-Hartman, linux-input, linux-kernel The driver core ignores the return value of struct bus_type::remove() because there is only little that can be done. To simplify the quest to make this function return void, let struct ishtp_cl_driver::remove() return void, too. All users already unconditionally return 0, this commit makes it obvious that returning an error value is a bad idea. Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> --- drivers/hid/intel-ish-hid/ishtp-fw-loader.c | 4 +--- drivers/hid/intel-ish-hid/ishtp-hid-client.c | 4 +--- drivers/hid/intel-ish-hid/ishtp/bus.c | 5 ++--- drivers/platform/chrome/cros_ec_ishtp.c | 4 +--- include/linux/intel-ish-client-if.h | 2 +- 5 files changed, 6 insertions(+), 13 deletions(-) diff --git a/drivers/hid/intel-ish-hid/ishtp-fw-loader.c b/drivers/hid/intel-ish-hid/ishtp-fw-loader.c index 6cf59fd26ad7..edb0bd084c27 100644 --- a/drivers/hid/intel-ish-hid/ishtp-fw-loader.c +++ b/drivers/hid/intel-ish-hid/ishtp-fw-loader.c @@ -1015,7 +1015,7 @@ static int loader_ishtp_cl_probe(struct ishtp_cl_device *cl_device) * * Return: 0 */ -static int loader_ishtp_cl_remove(struct ishtp_cl_device *cl_device) +static void loader_ishtp_cl_remove(struct ishtp_cl_device *cl_device) { struct ishtp_cl_data *client_data; struct ishtp_cl *loader_ishtp_cl = ishtp_get_drvdata(cl_device); @@ -1032,8 +1032,6 @@ static int loader_ishtp_cl_remove(struct ishtp_cl_device *cl_device) cancel_work_sync(&client_data->work_ishtp_reset); loader_deinit(loader_ishtp_cl); ishtp_put_device(cl_device); - - return 0; } /** diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c index 6ba944b40fdb..0f1b5283bab4 100644 --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c @@ -838,7 +838,7 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device *cl_device) * * Return: 0 */ -static int hid_ishtp_cl_remove(struct ishtp_cl_device *cl_device) +static void hid_ishtp_cl_remove(struct ishtp_cl_device *cl_device) { struct ishtp_cl *hid_ishtp_cl = ishtp_get_drvdata(cl_device); struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl); @@ -856,8 +856,6 @@ static int hid_ishtp_cl_remove(struct ishtp_cl_device *cl_device) hid_ishtp_cl = NULL; client_data->num_hid_devices = 0; - - return 0; } /** diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c index 7f36ce6187a1..ffc9ce5c86ee 100644 --- a/drivers/hid/intel-ish-hid/ishtp/bus.c +++ b/drivers/hid/intel-ish-hid/ishtp/bus.c @@ -258,7 +258,6 @@ static int ishtp_cl_device_remove(struct device *dev) { struct ishtp_cl_device *device = to_ishtp_cl_device(dev); struct ishtp_cl_driver *driver = to_ishtp_cl_driver(dev->driver); - int ret = 0; if (device->event_cb) { device->event_cb = NULL; @@ -266,9 +265,9 @@ static int ishtp_cl_device_remove(struct device *dev) } if (driver->remove) - ret = driver->remove(device); + driver->remove(device); - return ret; + return 0; } /** diff --git a/drivers/platform/chrome/cros_ec_ishtp.c b/drivers/platform/chrome/cros_ec_ishtp.c index 81364029af36..bd80173b33a2 100644 --- a/drivers/platform/chrome/cros_ec_ishtp.c +++ b/drivers/platform/chrome/cros_ec_ishtp.c @@ -707,7 +707,7 @@ static int cros_ec_ishtp_probe(struct ishtp_cl_device *cl_device) * * Return: 0 */ -static int cros_ec_ishtp_remove(struct ishtp_cl_device *cl_device) +static void cros_ec_ishtp_remove(struct ishtp_cl_device *cl_device) { struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device); struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl); @@ -716,8 +716,6 @@ static int cros_ec_ishtp_remove(struct ishtp_cl_device *cl_device) cancel_work_sync(&client_data->work_ec_evt); cros_ish_deinit(cros_ish_cl); ishtp_put_device(cl_device); - - return 0; } /** diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h index 0d6b4bc191c5..94669e21dc8b 100644 --- a/include/linux/intel-ish-client-if.h +++ b/include/linux/intel-ish-client-if.h @@ -36,7 +36,7 @@ struct ishtp_cl_driver { const char *name; const guid_t *guid; int (*probe)(struct ishtp_cl_device *dev); - int (*remove)(struct ishtp_cl_device *dev); + void (*remove)(struct ishtp_cl_device *dev); int (*reset)(struct ishtp_cl_device *dev); const struct dev_pm_ops *pm; }; -- 2.29.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/3] HID: intel-ish-hid: Drop if block with an always false condition 2021-02-06 15:13 [PATCH v1 1/3] HID: intel-ish-hid: Drop if block with an always false condition Uwe Kleine-König 2021-02-06 15:13 ` [PATCH v1 2/3] HID: intel-ish-hid: Simplify logic in ishtp_cl_device_remove() Uwe Kleine-König 2021-02-06 15:13 ` [PATCH v1 3/3] HID: intel-ish-hid: Make remove callback return void Uwe Kleine-König @ 2021-03-08 10:07 ` Jiri Kosina 2021-03-08 16:03 ` Srinivas Pandruvada 2 siblings, 1 reply; 9+ messages in thread From: Jiri Kosina @ 2021-03-08 10:07 UTC (permalink / raw) To: Uwe Kleine-König Cc: Srinivas Pandruvada, Benjamin Tissoires, Enric Balletbo i Serra, Guenter Roeck, Greg Kroah-Hartman, linux-input, linux-kernel On Sat, 6 Feb 2021, Uwe Kleine-König wrote: > A remove callback is only ever called for a bound device. So there is no > need to check for device or driver being NULL. Srinivas, any objections to this patchset? The cleanups look good to me. Thanks, -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/3] HID: intel-ish-hid: Drop if block with an always false condition 2021-03-08 10:07 ` [PATCH v1 1/3] HID: intel-ish-hid: Drop if block with an always false condition Jiri Kosina @ 2021-03-08 16:03 ` Srinivas Pandruvada 2021-03-08 16:16 ` Jiri Kosina 0 siblings, 1 reply; 9+ messages in thread From: Srinivas Pandruvada @ 2021-03-08 16:03 UTC (permalink / raw) To: Jiri Kosina, Uwe Kleine-König Cc: Benjamin Tissoires, Enric Balletbo i Serra, Guenter Roeck, Greg Kroah-Hartman, linux-input, linux-kernel On Mon, 2021-03-08 at 11:07 +0100, Jiri Kosina wrote: > On Sat, 6 Feb 2021, Uwe Kleine-König wrote: > > > A remove callback is only ever called for a bound device. So there > > is no > > need to check for device or driver being NULL. > > Srinivas, any objections to this patchset? The cleanups look good to > me. Sorry, I missed this series. No objection for taking these patches. Thanks, Srinivas > Thanks, > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/3] HID: intel-ish-hid: Drop if block with an always false condition 2021-03-08 16:03 ` Srinivas Pandruvada @ 2021-03-08 16:16 ` Jiri Kosina 2021-03-08 16:35 ` Srinivas Pandruvada 2021-05-10 19:43 ` Uwe Kleine-König 0 siblings, 2 replies; 9+ messages in thread From: Jiri Kosina @ 2021-03-08 16:16 UTC (permalink / raw) To: Srinivas Pandruvada Cc: Uwe Kleine-König, Benjamin Tissoires, Enric Balletbo i Serra, Guenter Roeck, Greg Kroah-Hartman, linux-input, linux-kernel On Mon, 8 Mar 2021, Srinivas Pandruvada wrote: > > > A remove callback is only ever called for a bound device. So there > > > is no > > > need to check for device or driver being NULL. > > > > Srinivas, any objections to this patchset? The cleanups look good to > > me. > Sorry, I missed this series. > No objection for taking these patches. Thanks. Applied with your Acked-by: If you disagree with that interpretation of your statement above, please holler :) Thanks, -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/3] HID: intel-ish-hid: Drop if block with an always false condition 2021-03-08 16:16 ` Jiri Kosina @ 2021-03-08 16:35 ` Srinivas Pandruvada 2021-05-10 19:43 ` Uwe Kleine-König 1 sibling, 0 replies; 9+ messages in thread From: Srinivas Pandruvada @ 2021-03-08 16:35 UTC (permalink / raw) To: Jiri Kosina Cc: Uwe Kleine-König, Benjamin Tissoires, Enric Balletbo i Serra, Guenter Roeck, Greg Kroah-Hartman, linux-input, linux-kernel On Mon, 2021-03-08 at 17:16 +0100, Jiri Kosina wrote: > On Mon, 8 Mar 2021, Srinivas Pandruvada wrote: > > > > > A remove callback is only ever called for a bound device. So > > > > there > > > > is no > > > > need to check for device or driver being NULL. > > > > > > Srinivas, any objections to this patchset? The cleanups look good > > > to > > > me. > > Sorry, I missed this series. > > No objection for taking these patches. > > Thanks. Applied with your Acked-by: > If you disagree with that interpretation of your statement above, > please > holler :) I agree. For record: Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> Thanks, Srinivas > > Thanks, > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/3] HID: intel-ish-hid: Drop if block with an always false condition 2021-03-08 16:16 ` Jiri Kosina 2021-03-08 16:35 ` Srinivas Pandruvada @ 2021-05-10 19:43 ` Uwe Kleine-König 2021-05-13 11:20 ` Jiri Kosina 1 sibling, 1 reply; 9+ messages in thread From: Uwe Kleine-König @ 2021-05-10 19:43 UTC (permalink / raw) To: Jiri Kosina, Srinivas Pandruvada Cc: Benjamin Tissoires, Enric Balletbo i Serra, Guenter Roeck, Greg Kroah-Hartman, linux-input, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 1184 bytes --] Hello Jiri, On 3/8/21 5:16 PM, Jiri Kosina wrote: > On Mon, 8 Mar 2021, Srinivas Pandruvada wrote: > >>>> A remove callback is only ever called for a bound device. So there >>>> is no >>>> need to check for device or driver being NULL. >>> >>> Srinivas, any objections to this patchset? The cleanups look good to >>> me. >> Sorry, I missed this series. >> No objection for taking these patches. > > Thanks. Applied with your Acked-by: > If you disagree with that interpretation of your statement above, please > holler :) I expected these patches to go in during the 5.13 merge window, but they didn't. I found your pull request for 5.13 (https://lore.kernel.org/lkml/nycvar.YFH.7.76.2104292151220.18270@cbobk.fhfr.pm/) and they were not included there even though the patches were in next since at least next-20210310. Looking at git log --oneline --cherry v5.13-rc1...dce6a0d56a7719efcad438f5c46a9d192fd36a89 (where dce.. was the tip of your for-next for next-20210506 (i.e. before 5.13-rc1 was cut)) and it seems there are quite a few more commits that didn't make it into your pull request. What am I missing? Best regards Uwe [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/3] HID: intel-ish-hid: Drop if block with an always false condition 2021-05-10 19:43 ` Uwe Kleine-König @ 2021-05-13 11:20 ` Jiri Kosina 0 siblings, 0 replies; 9+ messages in thread From: Jiri Kosina @ 2021-05-13 11:20 UTC (permalink / raw) To: Uwe Kleine-König Cc: Srinivas Pandruvada, Benjamin Tissoires, Enric Balletbo i Serra, Guenter Roeck, Greg Kroah-Hartman, linux-input, linux-kernel On Mon, 10 May 2021, Uwe Kleine-König wrote: > I expected these patches to go in during the 5.13 merge window, but they > didn't. I found your pull request for 5.13 > (https://lore.kernel.org/lkml/nycvar.YFH.7.76.2104292151220.18270@cbobk.fhfr.pm/) > and they were not included there even though the patches were in next since at > least next-20210310. Looking at > > git log --oneline --cherry > v5.13-rc1...dce6a0d56a7719efcad438f5c46a9d192fd36a89 > > (where dce.. was the tip of your for-next for next-20210506 (i.e. before > 5.13-rc1 was cut)) and it seems there are quite a few more commits that didn't > make it into your pull request. > > What am I missing? You are missing the fact that I am a halfwit and I screwed up the merge :) for-5.13/intel-ish branch by mistake didn't make it into final for-linus unfortunately, due to my mistake. Thanks a lot for pointing it out, I will fix that up. -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-05-13 11:20 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-06 15:13 [PATCH v1 1/3] HID: intel-ish-hid: Drop if block with an always false condition Uwe Kleine-König 2021-02-06 15:13 ` [PATCH v1 2/3] HID: intel-ish-hid: Simplify logic in ishtp_cl_device_remove() Uwe Kleine-König 2021-02-06 15:13 ` [PATCH v1 3/3] HID: intel-ish-hid: Make remove callback return void Uwe Kleine-König 2021-03-08 10:07 ` [PATCH v1 1/3] HID: intel-ish-hid: Drop if block with an always false condition Jiri Kosina 2021-03-08 16:03 ` Srinivas Pandruvada 2021-03-08 16:16 ` Jiri Kosina 2021-03-08 16:35 ` Srinivas Pandruvada 2021-05-10 19:43 ` Uwe Kleine-König 2021-05-13 11:20 ` Jiri Kosina
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).