| Message ID | 20210126165835.687514-1-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | amba: minor fix and various cleanups | expand |
Hi On 1/26/21 4:58 PM, Uwe Kleine-König wrote: > All amba drivers return 0 in their remove callback. Together with the > driver core ignoring the return value anyhow, it doesn't make sense to > return a value here. > > Change the remove prototype to return void, which makes it explicit that > returning an error value doesn't work as expected. This simplifies changing > the core remove callback to return void, too. > > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > Reviewed-by: Arnd Bergmann <arnd@arndb.de> > Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > Acked-by: Krzysztof Kozlowski <krzk@kernel.org> # for drivers/memory > Acked-by: Mark Brown <broonie@kernel.org> > Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > Acked-by: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 +--- You are most likely to have a conflict for the above file, with what is in coresight/next. It should be easy to resolve. Otherwise, the changes look good for the drivers/hwtracing/coresight/* Acked-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Hello, On Tue, Jan 26, 2021 at 05:08:40PM +0000, Suzuki K Poulose wrote: > On 1/26/21 4:58 PM, Uwe Kleine-König wrote: > > All amba drivers return 0 in their remove callback. Together with the > > driver core ignoring the return value anyhow, it doesn't make sense to > > return a value here. > > > > Change the remove prototype to return void, which makes it explicit that > > returning an error value doesn't work as expected. This simplifies changing > > the core remove callback to return void, too. > > > > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > > Reviewed-by: Arnd Bergmann <arnd@arndb.de> > > Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > > Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > Acked-by: Krzysztof Kozlowski <krzk@kernel.org> # for drivers/memory > > Acked-by: Mark Brown <broonie@kernel.org> > > Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > Acked-by: Linus Walleij <linus.walleij@linaro.org> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > > > drivers/hwtracing/coresight/coresight-etm4x-core.c | 4 +--- > > You are most likely to have a conflict for the above file, with what is > in coresight/next. It should be easy to resolve. I'm surprised to see that the remove callback introduced in 2952ecf5df33 ("coresight: etm4x: Refactor probing routine") has an __exit annotation. With .suppress_bind_attrs = true you don't need a remove callback at all. (And without .suppress_bind_attrs = true the remove callback must have no __exit annotation.) This make me looking at commit 45fe7befe0db ("coresight: remove broken __exit annotations") by Arnd. Unless I miss something the better change would have been to remove the unused remove callbacks instead of dropping their __exit annotation?! Anyhow, my conflict resolution looks as follows: diff --cc drivers/hwtracing/coresight/coresight-etm4x-core.c index 82787cba537d,473ab7480a36..000000000000 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@@ -1703,6 -1903,28 +1903,27 @@@ static int __exit etm4_remove_dev(struc cpus_read_unlock(); coresight_unregister(drvdata->csdev); + + return 0; + } + -static int __exit etm4_remove_amba(struct amba_device *adev) ++static void __exit etm4_remove_amba(struct amba_device *adev) + { + struct etmv4_drvdata *drvdata = dev_get_drvdata(&adev->dev); + + if (drvdata) - return etm4_remove_dev(drvdata); - return 0; ++ etm4_remove_dev(drvdata); + } + + static int __exit etm4_remove_platform_dev(struct platform_device *pdev) + { + int ret = 0; + struct etmv4_drvdata *drvdata = dev_get_drvdata(&pdev->dev); + + if (drvdata) + ret = etm4_remove_dev(drvdata); + pm_runtime_disable(&pdev->dev); + return ret; } static const struct amba_id etm4_ids[] = { If this series should make it in for 5.12 we probably need an immutable branch between hwtracing and amba. > Otherwise, the changes look good for the drivers/hwtracing/coresight/* > > Acked-by: Suzuki K Poulose <suzuki.poulose@arm.com> Thanks Uwe
On Tue, Jan 26, 2021 at 06:56:52PM +0100, Uwe Kleine-König wrote: > I'm surprised to see that the remove callback introduced in 2952ecf5df33 > ("coresight: etm4x: Refactor probing routine") has an __exit annotation. In general, remove callbacks should not have an __exit annotation. __exit _can_ be discarded at link time for built-in stuff.
On Tue, Jan 26, 2021 at 05:58:30PM +0100, Uwe Kleine-König wrote: > From: Uwe Kleine-König <u.kleine-koenig.org@pengutronix.de > > Hello, > > Changes since v2 sent with Message-Id: > 20201124133139.3072124-1-uwe@kleine-koenig.org: > > - Rebase to v5.11-rc1 (which resulted in a few conflicts in > drivers/hwtracing). > - Add various Acks. > - Send to more maintainers directly (which I think is one of the > reasons why there are so few Acks). > > For my taste patch 4 needs some more acks (drivers/char/hw_random, > drivers/dma, drivers/gpu/drm/pl111, drivers/i2c, drivers/mmc, > drivers/vfio, drivers/watchdog and sound/arm have no maintainer feedback > yet). > > My suggestion is to let this series go in via Russell King (who cares > for amba). Once enough Acks are there I can also provide a tag for > merging into different trees. Just tell me if you prefer this solution. > > Would be great if this could make it for v5.12, but I'm aware it's > already late in the v5.11 cycle so it might have to wait for v5.13. I think you need to have a 6th patch which moves the probe/remove/shutdown methods into the bus_type - if you're setting them for every struct device_driver, then there's no point doing that and they may as well be in the bus_type. Apart from that, it looks good.
Hello, we already talked about this via irc, but for the record and the benefit of others: On Tue, Feb 02, 2021 at 10:49:15AM +0000, Russell King - ARM Linux admin wrote: > I think you need to have a 6th patch which moves the > probe/remove/shutdown methods into the bus_type - if you're setting > them for every struct device_driver, then there's no point doing that > and they may as well be in the bus_type. This is implemented in patch 5 already. Best regards Uwe
From: Uwe Kleine-König <u.kleine-koenig.org@pengutronix.de Hello, Changes since v2 sent with Message-Id: 20201124133139.3072124-1-uwe@kleine-koenig.org: - Rebase to v5.11-rc1 (which resulted in a few conflicts in drivers/hwtracing). - Add various Acks. - Send to more maintainers directly (which I think is one of the reasons why there are so few Acks). For my taste patch 4 needs some more acks (drivers/char/hw_random, drivers/dma, drivers/gpu/drm/pl111, drivers/i2c, drivers/mmc, drivers/vfio, drivers/watchdog and sound/arm have no maintainer feedback yet). My suggestion is to let this series go in via Russell King (who cares for amba). Once enough Acks are there I can also provide a tag for merging into different trees. Just tell me if you prefer this solution. Would be great if this could make it for v5.12, but I'm aware it's already late in the v5.11 cycle so it might have to wait for v5.13. Best regards Uwe Uwe Kleine-König (5): amba: Fix resource leak for drivers without .remove amba: reorder functions vfio: platform: simplify device removal amba: Make the remove callback return void amba: Make use of bus_type functions drivers/amba/bus.c | 234 +++++++++--------- drivers/char/hw_random/nomadik-rng.c | 3 +- drivers/dma/pl330.c | 3 +- drivers/gpu/drm/pl111/pl111_drv.c | 4 +- drivers/hwtracing/coresight/coresight-catu.c | 3 +- .../hwtracing/coresight/coresight-cpu-debug.c | 4 +- .../hwtracing/coresight/coresight-cti-core.c | 4 +- drivers/hwtracing/coresight/coresight-etb10.c | 4 +- .../coresight/coresight-etm3x-core.c | 4 +- .../coresight/coresight-etm4x-core.c | 4 +- .../hwtracing/coresight/coresight-funnel.c | 4 +- .../coresight/coresight-replicator.c | 4 +- drivers/hwtracing/coresight/coresight-stm.c | 4 +- .../hwtracing/coresight/coresight-tmc-core.c | 4 +- drivers/hwtracing/coresight/coresight-tpiu.c | 4 +- drivers/i2c/busses/i2c-nomadik.c | 4 +- drivers/input/serio/ambakmi.c | 3 +- drivers/memory/pl172.c | 4 +- drivers/memory/pl353-smc.c | 4 +- drivers/mmc/host/mmci.c | 4 +- drivers/rtc/rtc-pl030.c | 4 +- drivers/rtc/rtc-pl031.c | 4 +- drivers/spi/spi-pl022.c | 5 +- drivers/tty/serial/amba-pl010.c | 4 +- drivers/tty/serial/amba-pl011.c | 3 +- drivers/vfio/platform/vfio_amba.c | 15 +- drivers/video/fbdev/amba-clcd.c | 4 +- drivers/watchdog/sp805_wdt.c | 4 +- include/linux/amba/bus.h | 2 +- sound/arm/aaci.c | 4 +- 30 files changed, 157 insertions(+), 198 deletions(-) base-commit: 5c8fe583cce542aa0b84adc939ce85293de36e5e