[PATCH 0/5] coresight: Convert to platform remove callback returning void

[Suzuki K Poulose]

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