| Message ID | cover.1713858615.git.u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | coresight: Convert to platform remove callback returning void | expand |
Hi Uwe On 23/04/2024 09:06, Uwe Kleine-König wrote: > Hello, > > this series converts a few platform drivers below drivers/hwtracing/coresight > that recently started to implement a .remove() callback to implement > .remove_new() instead. See commit 5c5a7680e67b ("platform: Provide a remove > callback that returns no value") for an extended explanation and the eventual > goal. > > All conversations are trivial, because the driver's .remove() callbacks > returned zero unconditionally already. > > There are no interdependencies between the five patches, so they could be picked > up individually if need be. After the merge window leading to v6.10-rc1 > (assuming Linus has >= 10 fingers this cycle :-) I want to switch the prototype > of struct platform_driver::remove to return void. So please either merge this > series together with the commits introducing .remove() that currently sit in > the next branch of > git://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git, or accept me > sending them together with the patch changing the function's prototype for > inclusion to Greg's driver-core tree. Thanks for catching these. I will apply them to coresight next branch. > > Having said that, the patches adding platform driver support for these could be > improved: > > - dev_get_drvdata() never returns NULL in these .remove() functions because > .probe() called dev_set_drvdata(). For the usage of WARN_ON also see > https://lwn.net/Articles/969923/. (That link's content is behind a paywall > until May 2, the TLDR is: Don't use WARN_ON().) So the respective checks > with the early return could better be dropped IMHO. > > - IS_ERR_OR_NULL(drvdata->pclk) is never true in .remove(). Also note that > IS_ERR_OR_NULL is ugly and gives hardly ever the right condition to check > for. Note further that clk == NULL isn't usually a problem, NULL is used as > dummy clk returned by clk_get_optional() and all clock API functions handle > that just fine. So if at all, better check only for IS_ERR(drvdata->pclk). Thanks for the suggestions, I will let Anshuman address them. Kind regards Suzuki > > Best regards > Uwe > > Uwe Kleine-König (5): > coresight: catu: Convert to platform remove callback returning void > coresight: debug: Convert to platform remove callback returning void > coresight: stm: Convert to platform remove callback returning void > coresight: tmc: Convert to platform remove callback returning void > coresight: tpiu: Convert to platform remove callback returning void > > drivers/hwtracing/coresight/coresight-catu.c | 7 +++---- > drivers/hwtracing/coresight/coresight-cpu-debug.c | 7 +++---- > drivers/hwtracing/coresight/coresight-stm.c | 7 +++---- > drivers/hwtracing/coresight/coresight-tmc-core.c | 7 +++---- > drivers/hwtracing/coresight/coresight-tpiu.c | 7 +++---- > 5 files changed, 15 insertions(+), 20 deletions(-) > > base-commit: a59668a9397e7245b26e9be85d23f242ff757ae8
On Tue, 23 Apr 2024 10:06:57 +0200, Uwe Kleine-König wrote: > this series converts a few platform drivers below drivers/hwtracing/coresight > that recently started to implement a .remove() callback to implement > .remove_new() instead. See commit 5c5a7680e67b ("platform: Provide a remove > callback that returns no value") for an extended explanation and the eventual > goal. > > All conversations are trivial, because the driver's .remove() callbacks > returned zero unconditionally already. > > [...] Applied, thanks! [1/5] coresight: catu: Convert to platform remove callback returning void https://git.kernel.org/coresight/c/c01cb419104c4187f2a148f72a7dcc67e77801a5 [2/5] coresight: debug: Convert to platform remove callback returning void https://git.kernel.org/coresight/c/971c2b107b5742ef1dfa3a405cd5ee7033502d60 [3/5] coresight: stm: Convert to platform remove callback returning void https://git.kernel.org/coresight/c/981d5f92ca2e33d1b0ba3e3563de52dc9661fb92 [4/5] coresight: tmc: Convert to platform remove callback returning void https://git.kernel.org/coresight/c/38a38da447579f683114edb1c8254491b4176853 [5/5] coresight: tpiu: Convert to platform remove callback returning void https://git.kernel.org/coresight/c/ba8c06fe7e16ddfd44ece7fabd9af2dc2e6d49e7 Best regards,
Hello, this series converts a few platform drivers below drivers/hwtracing/coresight that recently started to implement a .remove() callback to implement .remove_new() instead. See commit 5c5a7680e67b ("platform: Provide a remove callback that returns no value") for an extended explanation and the eventual goal. All conversations are trivial, because the driver's .remove() callbacks returned zero unconditionally already. There are no interdependencies between the five patches, so they could be picked up individually if need be. After the merge window leading to v6.10-rc1 (assuming Linus has >= 10 fingers this cycle :-) I want to switch the prototype of struct platform_driver::remove to return void. So please either merge this series together with the commits introducing .remove() that currently sit in the next branch of git://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git, or accept me sending them together with the patch changing the function's prototype for inclusion to Greg's driver-core tree. Having said that, the patches adding platform driver support for these could be improved: - dev_get_drvdata() never returns NULL in these .remove() functions because .probe() called dev_set_drvdata(). For the usage of WARN_ON also see https://lwn.net/Articles/969923/. (That link's content is behind a paywall until May 2, the TLDR is: Don't use WARN_ON().) So the respective checks with the early return could better be dropped IMHO. - IS_ERR_OR_NULL(drvdata->pclk) is never true in .remove(). Also note that IS_ERR_OR_NULL is ugly and gives hardly ever the right condition to check for. Note further that clk == NULL isn't usually a problem, NULL is used as dummy clk returned by clk_get_optional() and all clock API functions handle that just fine. So if at all, better check only for IS_ERR(drvdata->pclk). Best regards Uwe Uwe Kleine-König (5): coresight: catu: Convert to platform remove callback returning void coresight: debug: Convert to platform remove callback returning void coresight: stm: Convert to platform remove callback returning void coresight: tmc: Convert to platform remove callback returning void coresight: tpiu: Convert to platform remove callback returning void drivers/hwtracing/coresight/coresight-catu.c | 7 +++---- drivers/hwtracing/coresight/coresight-cpu-debug.c | 7 +++---- drivers/hwtracing/coresight/coresight-stm.c | 7 +++---- drivers/hwtracing/coresight/coresight-tmc-core.c | 7 +++---- drivers/hwtracing/coresight/coresight-tpiu.c | 7 +++---- 5 files changed, 15 insertions(+), 20 deletions(-) base-commit: a59668a9397e7245b26e9be85d23f242ff757ae8