| Message ID | 20230326143224.572654-1-u.kleine-koenig@pengutronix.de |
|---|---|
| Headers | show |
| Series | media: Convert to platform remove callback returning void | expand |
On Sun, Mar 26, 2023 at 7:32 AM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > Exiting early in remove without releasing all acquired resources yields > leaks. Note that e.g. memory allocated with devm_zalloc() is freed after > .remove() returns, even if the return code was negative. > > While blocking_notifier_chain_unregister() won't fail and so the > change is somewhat cosmetic, platform driver's .remove callbacks are > about to be converted to return void. To prepare that, keep the error > message but don't return early. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Reviewed-by: Guenter Roeck <groeck@chromium.org> > --- > drivers/media/cec/platform/cros-ec/cros-ec-cec.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c > index 6ebedc71d67d..960432230bbf 100644 > --- a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c > +++ b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c > @@ -332,14 +332,16 @@ static int cros_ec_cec_remove(struct platform_device *pdev) > struct device *dev = &pdev->dev; > int ret; > > + /* > + * blocking_notifier_chain_unregister() only fails if the notifier isn't > + * in the list. We know it was added to it by .probe(), so there should > + * be no need for error checking. Be cautious and still check. > + */ > ret = blocking_notifier_chain_unregister( > &cros_ec_cec->cros_ec->event_notifier, > &cros_ec_cec->notifier); > - > - if (ret) { > + if (ret) > dev_err(dev, "failed to unregister notifier\n"); > - return ret; > - } > > cec_notifier_cec_adap_unregister(cros_ec_cec->notify, > cros_ec_cec->adap); > -- > 2.39.2 >
Hello, On Sun, Mar 26, 2023 at 04:30:28PM +0200, Uwe Kleine-König wrote: > The .remove() callback for a platform driver returns an int which makes > many driver authors wrongly assume it's possible to do error handling by > returning an error code. However the value returned is (mostly) ignored > and this typically results in resource leaks. To improve here there is a > quest to make the remove callback return void. In the first step of this > quest all drivers are converted to .remove_new() which already returns > void. > > Trivially convert this driver from always returning zero in the remove > callback to the void returning variant. I fatfingered sending out this series and this patch is a duplicate, please ignore this patch, 001/117 with the otherwise same subject is the right one. (They only differ in the number, this patch is advertised as 001 in the cover letter.) Sorry, Uwe
Dne nedelja, 26. marec 2023 ob 16:31:49 CEST je Uwe Kleine-König napisal(a): > The .remove() callback for a platform driver returns an int which makes > many driver authors wrongly assume it's possible to do error handling by > returning an error code. However the value returned is (mostly) ignored > and this typically results in resource leaks. To improve here there is a > quest to make the remove callback return void. In the first step of this > quest all drivers are converted to .remove_new() which already returns > void. > > Trivially convert this driver from always returning zero in the remove > callback to the void returning variant. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com> Best regards, Jernej
Dne nedelja, 26. marec 2023 ob 16:31:45 CEST je Uwe Kleine-König napisal(a): > The .remove() callback for a platform driver returns an int which makes > many driver authors wrongly assume it's possible to do error handling by > returning an error code. However the value returned is (mostly) ignored > and this typically results in resource leaks. To improve here there is a > quest to make the remove callback return void. In the first step of this > quest all drivers are converted to .remove_new() which already returns > void. > > Trivially convert this driver from always returning zero in the remove > callback to the void returning variant. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com> Best regards, Jernej
On 3/26/23 16:31, Uwe Kleine-König wrote: > The .remove() callback for a platform driver returns an int which makes > many driver authors wrongly assume it's possible to do error handling by > returning an error code. However the value returned is (mostly) ignored > and this typically results in resource leaks. To improve here there is a > quest to make the remove callback return void. In the first step of this > quest all drivers are converted to .remove_new() which already returns > void. > > Trivially convert this driver from always returning zero in the remove > callback to the void returning variant. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/media/platform/nxp/dw100/dw100.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/nxp/dw100/dw100.c b/drivers/media/platform/nxp/dw100/dw100.c > index 189d60cd5ed1..6eed4bde9611 100644 > --- a/drivers/media/platform/nxp/dw100/dw100.c > +++ b/drivers/media/platform/nxp/dw100/dw100.c > @@ -1633,7 +1633,7 @@ static int dw100_probe(struct platform_device *pdev) > return ret; > } > > -static int dw100_remove(struct platform_device *pdev) > +static void dw100_remove(struct platform_device *pdev) > { > struct dw100_device *dw_dev = platform_get_drvdata(pdev); > > @@ -1649,8 +1649,6 @@ static int dw100_remove(struct platform_device *pdev) > mutex_destroy(dw_dev->vfd.lock); > v4l2_m2m_release(dw_dev->m2m_dev); > v4l2_device_unregister(&dw_dev->v4l2_dev); > - > - return 0; > } > > static int __maybe_unused dw100_runtime_suspend(struct device *dev) > @@ -1692,7 +1690,7 @@ MODULE_DEVICE_TABLE(of, dw100_dt_ids); > > static struct platform_driver dw100_driver = { > .probe = dw100_probe, > - .remove = dw100_remove, > + .remove_new = dw100_remove, > .driver = { > .name = DRV_NAME, > .pm = &dw100_pm, Reviewed-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com>
On Sun, Mar 26, 2023 at 4:34 PM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > The .remove() callback for a platform driver returns an int which makes > many driver authors wrongly assume it's possible to do error handling by > returning an error code. However the value returned is (mostly) ignored > and this typically results in resource leaks. To improve here there is a > quest to make the remove callback return void. In the first step of this > quest all drivers are converted to .remove_new() which already returns > void. > > Trivially convert this driver from always returning zero in the remove > callback to the void returning variant. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
Just ignore this reply: for some reason this patch only ended up in my private mailbox, but not on linux-media (and therefor also not in patchwork). This should hopefully cause patchwork to pick it up. Regards, Hans On 26/03/2023 16:30, Uwe Kleine-König wrote: > Exiting early in remove without releasing all acquired resources yields > leaks. Note that e.g. memory allocated with devm_zalloc() is freed after > .remove() returns, even if the return code was negative. > > While blocking_notifier_chain_unregister() won't fail and so the > change is somewhat cosmetic, platform driver's .remove callbacks are > about to be converted to return void. To prepare that, keep the error > message but don't return early. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/media/cec/platform/cros-ec/cros-ec-cec.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c > index 6ebedc71d67d..960432230bbf 100644 > --- a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c > +++ b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c > @@ -332,14 +332,16 @@ static int cros_ec_cec_remove(struct platform_device *pdev) > struct device *dev = &pdev->dev; > int ret; > > + /* > + * blocking_notifier_chain_unregister() only fails if the notifier isn't > + * in the list. We know it was added to it by .probe(), so there should > + * be no need for error checking. Be cautious and still check. > + */ > ret = blocking_notifier_chain_unregister( > &cros_ec_cec->cros_ec->event_notifier, > &cros_ec_cec->notifier); > - > - if (ret) { > + if (ret) > dev_err(dev, "failed to unregister notifier\n"); > - return ret; > - } > > cec_notifier_cec_adap_unregister(cros_ec_cec->notify, > cros_ec_cec->adap);
On 26.03.2023 16:31, Uwe Kleine-König wrote: > The .remove() callback for a platform driver returns an int which makes > many driver authors wrongly assume it's possible to do error handling by > returning an error code. However the value returned is (mostly) ignored > and this typically results in resource leaks. To improve here there is a > quest to make the remove callback return void. In the first step of this > quest all drivers are converted to .remove_new() which already returns > void. > > Trivially convert this driver from always returning zero in the remove > callback to the void returning variant. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Regards Andrzej > --- > drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c > index 9d2cce124a34..e30e54935d79 100644 > --- a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c > +++ b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c > @@ -1431,7 +1431,7 @@ static int s5p_mfc_probe(struct platform_device *pdev) > } > > /* Remove the driver */ > -static int s5p_mfc_remove(struct platform_device *pdev) > +static void s5p_mfc_remove(struct platform_device *pdev) > { > struct s5p_mfc_dev *dev = platform_get_drvdata(pdev); > struct s5p_mfc_ctx *ctx; > @@ -1463,7 +1463,6 @@ static int s5p_mfc_remove(struct platform_device *pdev) > s5p_mfc_unconfigure_dma_memory(dev); > > s5p_mfc_final_pm(dev); > - return 0; > } > > #ifdef CONFIG_PM_SLEEP > @@ -1690,7 +1689,7 @@ MODULE_DEVICE_TABLE(of, exynos_mfc_match); > > static struct platform_driver s5p_mfc_driver = { > .probe = s5p_mfc_probe, > - .remove = s5p_mfc_remove, > + .remove_new = s5p_mfc_remove, > .driver = { > .name = S5P_MFC_NAME, > .pm = &s5p_mfc_pm_ops,
On 26.03.2023 16:31, Uwe Kleine-König wrote: > The only effect of returning an error code in a remove callback is that > the driver core emits a warning. The device is unbound anyhow. > > As the remove callback already emits a (quite verbose) warning when ret > is non-zero, return zero to suppress the additional warning. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > drivers/media/platform/qcom/venus/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c > index 990a1519f968..403ffb92af60 100644 > --- a/drivers/media/platform/qcom/venus/core.c > +++ b/drivers/media/platform/qcom/venus/core.c > @@ -448,7 +448,7 @@ static int venus_remove(struct platform_device *pdev) > mutex_destroy(&core->lock); > venus_dbgfs_deinit(core); > > - return ret; > + return 0; > } > > static void venus_core_shutdown(struct platform_device *pdev)
On 26.03.2023 16:31, Uwe Kleine-König wrote: > The .remove() callback for a platform driver returns an int which makes > many driver authors wrongly assume it's possible to do error handling by > returning an error code. However the value returned is (mostly) ignored > and this typically results in resource leaks. To improve here there is a > quest to make the remove callback return void. In the first step of this > quest all drivers are converted to .remove_new() which already returns > void. > > Trivially convert this driver from always returning zero in the remove > callback to the void returning variant. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > drivers/media/platform/qcom/venus/vdec.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c > index 4ceaba37e2e5..d47c22015770 100644 > --- a/drivers/media/platform/qcom/venus/vdec.c > +++ b/drivers/media/platform/qcom/venus/vdec.c > @@ -1774,7 +1774,7 @@ static int vdec_probe(struct platform_device *pdev) > return ret; > } > > -static int vdec_remove(struct platform_device *pdev) > +static void vdec_remove(struct platform_device *pdev) > { > struct venus_core *core = dev_get_drvdata(pdev->dev.parent); > > @@ -1783,8 +1783,6 @@ static int vdec_remove(struct platform_device *pdev) > > if (core->pm_ops->vdec_put) > core->pm_ops->vdec_put(core->dev_dec); > - > - return 0; > } > > static __maybe_unused int vdec_runtime_suspend(struct device *dev) > @@ -1825,7 +1823,7 @@ MODULE_DEVICE_TABLE(of, vdec_dt_match); > > static struct platform_driver qcom_venus_dec_driver = { > .probe = vdec_probe, > - .remove = vdec_remove, > + .remove_new = vdec_remove, > .driver = { > .name = "qcom-venus-decoder", > .of_match_table = vdec_dt_match,
Hello Mauro On Sun, Mar 26, 2023 at 04:30:25PM +0200, Uwe Kleine-König wrote: > Hello, > > this series adapts the platform drivers below drivers/pci to use the copy&paste failure here: s/pci/media/ of course. > .remove_new() callback. Compared to the traditional .remove() callback > .remove_new() returns no value. This is a good thing because the driver core > doesn't (and cannot) cope for errors during remove. The only effect of a > non-zero return value in .remove() is that the driver core emits a warning. The > device is removed anyhow and an early return from .remove() usually yields a > resource leak. > > By changing the remove callback to return void driver authors cannot > reasonably assume any more that there is some kind of cleanup later. > > Only three drivers needed some preparation first to make sure they > return 0 unconditionally in their remove callback. Then all drivers > could be trivially converted without side effects to .remove_new(). > > The changes to the individual drivers are all orthogonal. If I need to > resend some patches because of some review feedback, I'd like to only > send the patches that actually needed changes, so please pick up the > remaining patches that don't need changing to reduce the amount of mail. I didn't hear anything back about application of this series. Is there a blocker somewhere? Apart from the three preparatory patches that are a precondition to the conversion of the respective drivers, the patches are all pairwise orthogonal. So from my POV the best would be to apply all patches that still apply (which might be all), I will care for the fallout later then. Best regards Uwe
Hi Uwe, On Mon, Apr 17, 2023 at 08:02:03AM +0200, Uwe Kleine-König wrote: > Hello Mauro > > On Sun, Mar 26, 2023 at 04:30:25PM +0200, Uwe Kleine-König wrote: > > Hello, > > > > this series adapts the platform drivers below drivers/pci to use the > > copy&paste failure here: s/pci/media/ of course. > > > .remove_new() callback. Compared to the traditional .remove() callback > > .remove_new() returns no value. This is a good thing because the driver core > > doesn't (and cannot) cope for errors during remove. The only effect of a > > non-zero return value in .remove() is that the driver core emits a warning. The > > device is removed anyhow and an early return from .remove() usually yields a > > resource leak. > > > > By changing the remove callback to return void driver authors cannot > > reasonably assume any more that there is some kind of cleanup later. > > > > Only three drivers needed some preparation first to make sure they > > return 0 unconditionally in their remove callback. Then all drivers > > could be trivially converted without side effects to .remove_new(). > > > > The changes to the individual drivers are all orthogonal. If I need to > > resend some patches because of some review feedback, I'd like to only > > send the patches that actually needed changes, so please pick up the > > remaining patches that don't need changing to reduce the amount of mail. > > I didn't hear anything back about application of this series. Is there a > blocker somewhere? I think the series got applied to the master branch of git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git. It should thus appear in v6.4. The corresponding series for staging media drivers has also been applied to the same branch as far as I can tell. > Apart from the three preparatory patches that are a precondition to the > conversion of the respective drivers, the patches are all pairwise > orthogonal. So from my POV the best would be to apply all patches that > still apply (which might be all), I will care for the fallout later > then.
Hello Laurent, On Mon, Apr 17, 2023 at 09:19:28AM +0300, Laurent Pinchart wrote: > On Mon, Apr 17, 2023 at 08:02:03AM +0200, Uwe Kleine-König wrote: > > On Sun, Mar 26, 2023 at 04:30:25PM +0200, Uwe Kleine-König wrote: > > > Hello, > > > > > > this series adapts the platform drivers below drivers/pci to use the > > > > copy&paste failure here: s/pci/media/ of course. > > > > > .remove_new() callback. Compared to the traditional .remove() callback > > > .remove_new() returns no value. This is a good thing because the driver core > > > doesn't (and cannot) cope for errors during remove. The only effect of a > > > non-zero return value in .remove() is that the driver core emits a warning. The > > > device is removed anyhow and an early return from .remove() usually yields a > > > resource leak. > > > > > > By changing the remove callback to return void driver authors cannot > > > reasonably assume any more that there is some kind of cleanup later. > > > > > > Only three drivers needed some preparation first to make sure they > > > return 0 unconditionally in their remove callback. Then all drivers > > > could be trivially converted without side effects to .remove_new(). > > > > > > The changes to the individual drivers are all orthogonal. If I need to > > > resend some patches because of some review feedback, I'd like to only > > > send the patches that actually needed changes, so please pick up the > > > remaining patches that don't need changing to reduce the amount of mail. > > > > I didn't hear anything back about application of this series. Is there a > > blocker somewhere? > > I think the series got applied to the master branch of > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git. > It should thus appear in v6.4. I guess that linux-stable.git is a copy&paste failure (and it's not there). I don't see the series in the master branch of git://linuxtv.org/media_tree.git either. .. a bit later ... ah, it's in git://linuxtv.org/mchehab/media-next.git I guess I was just to quick and probably the series will be included in today's next. Thanks Uwe
On Mon, Apr 17, 2023 at 09:30:49AM +0200, Uwe Kleine-König wrote: > Hello Laurent, > > On Mon, Apr 17, 2023 at 09:19:28AM +0300, Laurent Pinchart wrote: > > On Mon, Apr 17, 2023 at 08:02:03AM +0200, Uwe Kleine-König wrote: > > > On Sun, Mar 26, 2023 at 04:30:25PM +0200, Uwe Kleine-König wrote: > > > > Hello, > > > > > > > > this series adapts the platform drivers below drivers/pci to use the > > > > > > copy&paste failure here: s/pci/media/ of course. > > > > > > > .remove_new() callback. Compared to the traditional .remove() callback > > > > .remove_new() returns no value. This is a good thing because the driver core > > > > doesn't (and cannot) cope for errors during remove. The only effect of a > > > > non-zero return value in .remove() is that the driver core emits a warning. The > > > > device is removed anyhow and an early return from .remove() usually yields a > > > > resource leak. > > > > > > > > By changing the remove callback to return void driver authors cannot > > > > reasonably assume any more that there is some kind of cleanup later. > > > > > > > > Only three drivers needed some preparation first to make sure they > > > > return 0 unconditionally in their remove callback. Then all drivers > > > > could be trivially converted without side effects to .remove_new(). > > > > > > > > The changes to the individual drivers are all orthogonal. If I need to > > > > resend some patches because of some review feedback, I'd like to only > > > > send the patches that actually needed changes, so please pick up the > > > > remaining patches that don't need changing to reduce the amount of mail. > > > > > > I didn't hear anything back about application of this series. Is there a > > > blocker somewhere? > > > > I think the series got applied to the master branch of > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git. > > It should thus appear in v6.4. > > I guess that linux-stable.git is a copy&paste failure (and it's not > there). I don't see the series in the master branch of > git://linuxtv.org/media_tree.git either. Oops sorry. It was a copy & paste mistake indeed, I meant git://linuxtv.org/media_stage.git > .. a bit later ... > > ah, it's in git://linuxtv.org/mchehab/media-next.git > > I guess I was just to quick and probably the series will be included in > today's next.
Hi Uwe, > -----Original Message----- > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > Sent: Monday, April 17, 2023 8:31 AM > To: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Heiko Stuebner <heiko@sntech.de>; Eddie James <eajames@linux.ibm.com>; > Hans Verkuil <hverkuil@xs4all.nl>; Alim Akhtar <alim.akhtar@samsung.com>; > Dmitry Osipenko <digetx@gmail.com>; linux-stm32@st-md- > mailman.stormreply.com; Marek Szyprowski <m.szyprowski@samsung.com>; linux- > samsung-soc@vger.kernel.org; Robert Foss <rfoss@kernel.org>; Dafna > Hirschfeld <dafna@fastmail.com>; Samuel Holland <samuel@sholland.org>; Kevin > Hilman <khilman@baylibre.com>; Michal Simek <michal.simek@xilinx.com>; Antti > Palosaari <crope@iki.fi>; NXP Linux Team <linux-imx@nxp.com>; Jerome Brunet > <jbrunet@baylibre.com>; linux-sunxi@lists.linux.dev; ye xingchen > <ye.xingchen@zte.com.cn>; Sascha Hauer <s.hauer@pengutronix.de>; Łukasz > Stelmach <l.stelmach@samsung.com>; Eugen Hristev > <eugen.hristev@collabora.com>; Shuah Khan <skhan@linuxfoundation.org>; Hyun > Kwon <hyun.kwon@xilinx.com>; Andrew Jeffery <andrew@aj.id.au>; Michael > Tretter <m.tretter@pengutronix.de>; Moudy Ho <moudy.ho@mediatek.com>; > kernel@pengutronix.de; Hans Verkuil <hverkuil-cisco@xs4all.nl>; Claudiu > Beznea <claudiu.beznea@microchip.com>; Ming Qian <ming.qian@nxp.com>; > Andrew-CT Chen <andrew-ct.chen@mediatek.com>; Alexandre Belloni > <alexandre.belloni@bootlin.com>; Sylwester Nawrocki > <s.nawrocki@samsung.com>; linux-aspeed@lists.ozlabs.org; Yunfei Dong > <yunfei.dong@mediatek.com>; Lad, Prabhakar <prabhakar.csengg@gmail.com>; > Thierry Reding <thierry.reding@gmail.com>; Guenter Roeck > <groeck@chromium.org>; chrome-platform@lists.linux.dev; Jonathan Hunter > <jonathanh@nvidia.com>; linux-rockchip@lists.infradead.org; Fabien Dessenne > <fabien.dessenne@foss.st.com>; Ezequiel Garcia > <ezequiel@vanguardiasur.com.ar>; Alain Volmat <alain.volmat@foss.st.com>; > Andrzej Pietrasiewicz <andrzejtp2010@gmail.com>; Colin Ian King > <colin.i.king@gmail.com>; linux-media@vger.kernel.org; Jacopo Mondi > <jacopo@jmondi.org>; Rory Liu <hellojacky0226@hotmail.com>; Martin > Blumenstingl <martin.blumenstingl@googlemail.com>; linux-arm- > msm@vger.kernel.org; Sean Wang <sean.wang@mediatek.com>; Maxime Ripard > <mripard@kernel.org>; Fabrizio Castro <fabrizio.castro.jz@renesas.com>; > linux-amlogic@lists.infradead.org; linux-arm-kernel@lists.infradead.org; > Neil Armstrong <neil.armstrong@linaro.org>; Zhou Peng <eagle.zhou@nxp.com>; > Paul Kocialkowski <paul.kocialkowski@bootlin.com>; Maxime Coquelin > <mcoquelin.stm32@gmail.com>; linux-mediatek@lists.infradead.org; Jacek > Anaszewski <jacek.anaszewski@gmail.com>; Dan Carpenter <error27@gmail.com>; > Sean Young <sean@mess.org>; Xavier Roumegue <xavier.roumegue@oss.nxp.com>; > Ettore Chimenti <ek5.chimenti@gmail.com>; Vikash Garodia > <quic_vgarodia@quicinc.com>; linux-tegra@vger.kernel.org; Eduardo Valentin > <edubezval@gmail.com>; Andrzej Hajda <andrzej.hajda@intel.com>; Fabio > Estevam <festevam@gmail.com>; Jean-Christophe Trotin <jean- > christophe.trotin@foss.st.com>; Stanimir Varbanov > <stanimir.k.varbanov@gmail.com>; Kieran Bingham > <kieran.bingham@ideasonboard.com>; Jernej Skrabec > <jernej.skrabec@gmail.com>; Chen-Yu Tsai <wens@csie.org>; Jacob Chen <jacob- > chen@iotwrt.com>; Joel Stanley <joel@jms.id.au>; Yang Yingliang > <yangyingliang@huawei.com>; Patrice Chotard <patrice.chotard@foss.st.com>; > Bin Liu <bin.liu@mediatek.com>; Nathan Chancellor <nathan@kernel.org>; > Sylwester Nawrocki <sylvester.nawrocki@gmail.com>; Mauro Carvalho Chehab > <mchehab@kernel.org>; Benson Leung <bleung@chromium.org>; Daniel W. S. > Almeida <dwlsalmeida@gmail.com>; Qiheng Lin <linqiheng@huawei.com>; Konrad > Dybcio <konrad.dybcio@linaro.org>; Kieran Bingham > <kieran.bingham+renesas@ideasonboard.com>; Yang Li > <yang.lee@linux.alibaba.com>; Sakari Ailus <sakari.ailus@linux.intel.com>; > Ricardo Ribalda <ribalda@chromium.org>; Shawn Guo <shawnguo@kernel.org>; > Minghsiu Tsai <minghsiu.tsai@mediatek.com>; Daniel Almeida > <daniel.almeida@collabora.com>; Alexandre Torgue > <alexandre.torgue@foss.st.com>; Todor Tomov <todor.too@gmail.com>; Mirela > Rabulea <mirela.rabulea@nxp.com>; Ajye Huang <ajye_huang@compal.corp- > partner.google.com>; Scott Chao <scott_chao@wistron.corp- > partner.google.com>; linux-renesas-soc@vger.kernel.org; Hugues Fruchet > <hugues.fruchet@foss.st.com>; openbmc@lists.ozlabs.org; Andy Gross > <agross@kernel.org>; Mikhail Ulyanov <mikhail.ulyanov@cogentembedded.com>; > Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; Benoit > Parrot <bparrot@ti.com>; Rui Miguel Silva <rmfrfs@gmail.com>; Christophe > JAILLET <christophe.jaillet@wanadoo.fr>; Yong Deng <yong.deng@magewell.com>; > Matthias Brugger <matthias.bgg@gmail.com>; Tiffany Lin > <tiffany.lin@mediatek.com>; AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com>; Bjorn Andersson > <andersson@kernel.org>; Nicolas Ferre <nicolas.ferre@microchip.com>; Houlong > Wei <houlong.wei@mediatek.com>; Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org>; Philipp Zabel <p.zabel@pengutronix.de>; > Niklas Söderlund <niklas.soderlund@ragnatech.se> > Subject: Re: [PATCH 000/117] media: Convert to platform remove callback > returning void > > Hello Laurent, > > On Mon, Apr 17, 2023 at 09:19:28AM +0300, Laurent Pinchart wrote: > > On Mon, Apr 17, 2023 at 08:02:03AM +0200, Uwe Kleine-König wrote: > > > On Sun, Mar 26, 2023 at 04:30:25PM +0200, Uwe Kleine-König wrote: > > > > Hello, > > > > > > > > this series adapts the platform drivers below drivers/pci to use > > > > the > > > > > > copy&paste failure here: s/pci/media/ of course. > > > > > > > .remove_new() callback. Compared to the traditional .remove() > > > > callback > > > > .remove_new() returns no value. This is a good thing because the > > > > driver core doesn't (and cannot) cope for errors during remove. > > > > The only effect of a non-zero return value in .remove() is that > > > > the driver core emits a warning. The device is removed anyhow and > > > > an early return from .remove() usually yields a resource leak. > > > > > > > > By changing the remove callback to return void driver authors > > > > cannot reasonably assume any more that there is some kind of cleanup > later. > > > > > > > > Only three drivers needed some preparation first to make sure they > > > > return 0 unconditionally in their remove callback. Then all > > > > drivers could be trivially converted without side effects to > .remove_new(). > > > > > > > > The changes to the individual drivers are all orthogonal. If I > > > > need to resend some patches because of some review feedback, I'd > > > > like to only send the patches that actually needed changes, so > > > > please pick up the remaining patches that don't need changing to > reduce the amount of mail. > > > > > > I didn't hear anything back about application of this series. Is > > > there a blocker somewhere? > > > > I think the series got applied to the master branch of > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git. > > It should thus appear in v6.4. > > I guess that linux-stable.git is a copy&paste failure (and it's not there). > I don't see the series in the master branch of > git://linuxtv.org/media_tree.git either. > > .. a bit later ... > > ah, it's in git://linuxtv.org/mchehab/media-next.git > > I guess I was just to quick and probably the series will be included in > today's next. I believe patchwork <patchwork@linuxtv.org> will send notification to author and along with people who applied tags for that patch. I normally get notification from patchwork <patchwork@linuxtv.org> When the state of patch changes. Cheers, Biju
On Mon, Apr 17, 2023 at 07:57:57AM +0000, Biju Das wrote: > Hi Uwe, > > > I think the series got applied to the master branch of > > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git. > > > It should thus appear in v6.4. > > > > I guess that linux-stable.git is a copy&paste failure (and it's not there). > > I don't see the series in the master branch of > > git://linuxtv.org/media_tree.git either. > > > > .. a bit later ... > > > > ah, it's in git://linuxtv.org/mchehab/media-next.git > > > > I guess I was just to quick and probably the series will be included in > > today's next. > > I believe patchwork <patchwork@linuxtv.org> will send notification to > author and along with people who applied tags for that patch. Indeed, I got such a notification on Apr 11. But even if I had that on my radar when asking and considered such notifications reliable in general, I would have asked, as the patches didn't apprear in next up to now. Thanks Uwe
Hello, this series adapts the platform drivers below drivers/pci to use the .remove_new() callback. Compared to the traditional .remove() callback .remove_new() returns no value. This is a good thing because the driver core doesn't (and cannot) cope for errors during remove. The only effect of a non-zero return value in .remove() is that the driver core emits a warning. The device is removed anyhow and an early return from .remove() usually yields a resource leak. By changing the remove callback to return void driver authors cannot reasonably assume any more that there is some kind of cleanup later. Only three drivers needed some preparation first to make sure they return 0 unconditionally in their remove callback. Then all drivers could be trivially converted without side effects to .remove_new(). The changes to the individual drivers are all orthogonal. If I need to resend some patches because of some review feedback, I'd like to only send the patches that actually needed changes, so please pick up the remaining patches that don't need changing to reduce the amount of mail. Best regards Uwe Uwe Kleine-König (117): media: cec-gpio: Convert to platform remove callback returning void media: cros-ec-cec: Don't exit early in .remove() callback media: cros-ec-cec: Convert to platform remove callback returning void media: ao-cec-g12a: Convert to platform remove callback returning void media: ao-cec: Convert to platform remove callback returning void media: s5p_cec: Convert to platform remove callback returning void media: seco-cec: Convert to platform remove callback returning void media: stih-cec: Convert to platform remove callback returning void media: stm32-cec: Convert to platform remove callback returning void media: tegra_cec: Convert to platform remove callback returning void media: rtl2832_sdr: Convert to platform remove callback returning void media: zd1301_demod: Convert to platform remove callback returning void media: allegro-core: Convert to platform remove callback returning void media: ge2d: Convert to platform remove callback returning void media: vpu_core: Convert to platform remove callback returning void media: vpu_drv: Convert to platform remove callback returning void media: aspeed-video: Convert to platform remove callback returning void media: atmel-isi: Convert to platform remove callback returning void media: cdns-csi2rx: Convert to platform remove callback returning void media: cdns-csi2tx: Convert to platform remove callback returning void media: coda-common: Convert to platform remove callback returning void media: pxa_camera: Convert to platform remove callback returning void media: m2m-deinterlace: Convert to platform remove callback returning void media: marvell: Simplify remove callback media: marvell: Convert to platform remove callback returning void media: mtk_jpeg_core: Convert to platform remove callback returning void media: mtk_mdp_core: Convert to platform remove callback returning void media: mtk-mdp3-core: Convert to platform remove callback returning void media: mtk_vcodec_dec_drv: Convert to platform remove callback returning void media: mtk_vcodec_enc_drv: Convert to platform remove callback returning void media: mtk_vpu: Convert to platform remove callback returning void media: microchip-csi2dc: Convert to platform remove callback returning void media: microchip-sama5d2-isc: Convert to platform remove callback returning void media: microchip-sama7g5-isc: Convert to platform remove callback returning void media: vde: Convert to platform remove callback returning void media: dw100: Convert to platform remove callback returning void media: mxc-jpeg: Convert to platform remove callback returning void media: imx-mipi-csis: Convert to platform remove callback returning void media: imx-pxp: Convert to platform remove callback returning void media: imx7-media-csi: Convert to platform remove callback returning void media: mx2_emmaprp: Convert to platform remove callback returning void media: camss: Convert to platform remove callback returning void media: venus: Warn only once about problems in .remove() media: venus: Convert to platform remove callback returning void media: vdec: Convert to platform remove callback returning void media: venc: Convert to platform remove callback returning void media: rcar-fcp: Convert to platform remove callback returning void media: rcar-isp: Convert to platform remove callback returning void media: rcar-core: Convert to platform remove callback returning void media: rcar-csi2: Convert to platform remove callback returning void media: rcar_drif: Convert to platform remove callback returning void media: rcar_fdp1: Convert to platform remove callback returning void media: rcar_jpu: Convert to platform remove callback returning void media: renesas-ceu: Convert to platform remove callback returning void media: rzg2l-core: Convert to platform remove callback returning void media: rzg2l-csi2: Convert to platform remove callback returning void media: sh_vou: Convert to platform remove callback returning void media: vsp1_drv: Convert to platform remove callback returning void media: rga: Convert to platform remove callback returning void media: rkisp1-dev: Convert to platform remove callback returning void media: gsc-core: Convert to platform remove callback returning void media: fimc-core: Convert to platform remove callback returning void media: fimc-is-i2c: Convert to platform remove callback returning void media: fimc-is: Convert to platform remove callback returning void media: fimc-lite: Convert to platform remove callback returning void media: media-dev: Convert to platform remove callback returning void media: mipi-csis: Convert to platform remove callback returning void media: camif-core: Convert to platform remove callback returning void media: g2d: Convert to platform remove callback returning void media: jpeg-core: Convert to platform remove callback returning void media: s5p_mfc: Convert to platform remove callback returning void media: bdisp-v4l2: Convert to platform remove callback returning void media: c8sectpfe-core: Convert to platform remove callback returning void media: delta-v4l2: Convert to platform remove callback returning void media: hva-v4l2: Convert to platform remove callback returning void media: dma2d: Convert to platform remove callback returning void media: stm32-dcmi: Convert to platform remove callback returning void media: sun4i_csi: Convert to platform remove callback returning void media: sun6i_csi: Convert to platform remove callback returning void media: sun6i_mipi_csi2: Convert to platform remove callback returning void media: sun8i_a83t_mipi_csi2: Convert to platform remove callback returning void media: sun8i-di: Convert to platform remove callback returning void media: sun8i_rotate: Convert to platform remove callback returning void media: am437x-vpfe: Convert to platform remove callback returning void media: cal: Convert to platform remove callback returning void media: vpif: Convert to platform remove callback returning void media: vpif_capture: Convert to platform remove callback returning void media: vpif_display: Convert to platform remove callback returning void media: omap_vout: Convert to platform remove callback returning void media: isp: Convert to platform remove callback returning void media: vpe: Convert to platform remove callback returning void media: hantro_drv: Convert to platform remove callback returning void media: via-camera: Convert to platform remove callback returning void media: video-mux: Convert to platform remove callback returning void media: xilinx-csi2rxss: Convert to platform remove callback returning void media: xilinx-tpg: Convert to platform remove callback returning void media: xilinx-vipp: Convert to platform remove callback returning void media: xilinx-vtc: Convert to platform remove callback returning void media: radio-si476x: Convert to platform remove callback returning void media: radio-timb: Convert to platform remove callback returning void media: radio-wl1273: Convert to platform remove callback returning void media: radio-platform-si4713: Convert to platform remove callback returning void media: gpio-ir-recv: Convert to platform remove callback returning void media: img-ir-core: Convert to platform remove callback returning void media: ir-hix5hd2: Convert to platform remove callback returning void media: meson-ir-tx: Convert to platform remove callback returning void media: meson-ir: Convert to platform remove callback returning void media: mtk-cir: Convert to platform remove callback returning void media: st_rc: Convert to platform remove callback returning void media: sunxi-cir: Convert to platform remove callback returning void media: vicodec-core: Convert to platform remove callback returning void media: vidtv_bridge: Convert to platform remove callback returning void media: vim2m: Convert to platform remove callback returning void media: vimc-core: Convert to platform remove callback returning void media: visl-core: Convert to platform remove callback returning void media: vivid-core: Convert to platform remove callback returning void media: it913x: Convert to platform remove callback returning void drivers/media/cec/platform/cec-gpio/cec-gpio.c | 5 ++--- drivers/media/cec/platform/cros-ec/cros-ec-cec.c | 16 ++++++++-------- drivers/media/cec/platform/meson/ao-cec-g12a.c | 6 ++---- drivers/media/cec/platform/meson/ao-cec.c | 6 ++---- drivers/media/cec/platform/s5p/s5p_cec.c | 5 ++--- drivers/media/cec/platform/seco/seco-cec.c | 6 ++---- drivers/media/cec/platform/sti/stih-cec.c | 6 ++---- drivers/media/cec/platform/stm32/stm32-cec.c | 6 ++---- drivers/media/cec/platform/tegra/tegra_cec.c | 6 ++---- drivers/media/dvb-frontends/rtl2832_sdr.c | 6 ++---- drivers/media/dvb-frontends/zd1301_demod.c | 6 ++---- .../media/platform/allegro-dvt/allegro-core.c | 6 ++---- drivers/media/platform/amlogic/meson-ge2d/ge2d.c | 6 ++---- drivers/media/platform/amphion/vpu_core.c | 6 ++---- drivers/media/platform/amphion/vpu_drv.c | 6 ++---- drivers/media/platform/aspeed/aspeed-video.c | 6 ++---- drivers/media/platform/atmel/atmel-isi.c | 6 ++---- drivers/media/platform/cadence/cdns-csi2rx.c | 6 ++---- drivers/media/platform/cadence/cdns-csi2tx.c | 6 ++---- drivers/media/platform/chips-media/coda-common.c | 5 ++--- drivers/media/platform/intel/pxa_camera.c | 6 ++---- drivers/media/platform/m2m-deinterlace.c | 6 ++---- drivers/media/platform/marvell/mmp-driver.c | 16 +++------------- .../media/platform/mediatek/jpeg/mtk_jpeg_core.c | 6 ++---- .../media/platform/mediatek/mdp/mtk_mdp_core.c | 5 ++--- .../media/platform/mediatek/mdp3/mtk-mdp3-core.c | 5 ++--- .../mediatek/vcodec/mtk_vcodec_dec_drv.c | 5 ++--- .../mediatek/vcodec/mtk_vcodec_enc_drv.c | 5 ++--- drivers/media/platform/mediatek/vpu/mtk_vpu.c | 6 ++---- .../media/platform/microchip/microchip-csi2dc.c | 6 ++---- .../platform/microchip/microchip-sama5d2-isc.c | 6 ++---- .../platform/microchip/microchip-sama7g5-isc.c | 6 ++---- drivers/media/platform/nvidia/tegra-vde/vde.c | 6 ++---- drivers/media/platform/nxp/dw100/dw100.c | 6 ++---- drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 6 ++---- drivers/media/platform/nxp/imx-mipi-csis.c | 6 ++---- drivers/media/platform/nxp/imx-pxp.c | 6 ++---- drivers/media/platform/nxp/imx7-media-csi.c | 6 ++---- drivers/media/platform/nxp/mx2_emmaprp.c | 6 ++---- drivers/media/platform/qcom/camss/camss.c | 6 ++---- drivers/media/platform/qcom/venus/core.c | 6 ++---- drivers/media/platform/qcom/venus/vdec.c | 6 ++---- drivers/media/platform/qcom/venus/venc.c | 6 ++---- drivers/media/platform/renesas/rcar-fcp.c | 6 ++---- drivers/media/platform/renesas/rcar-isp.c | 6 ++---- .../media/platform/renesas/rcar-vin/rcar-core.c | 6 ++---- .../media/platform/renesas/rcar-vin/rcar-csi2.c | 6 ++---- drivers/media/platform/renesas/rcar_drif.c | 8 +++----- drivers/media/platform/renesas/rcar_fdp1.c | 6 ++---- drivers/media/platform/renesas/rcar_jpu.c | 6 ++---- drivers/media/platform/renesas/renesas-ceu.c | 6 ++---- .../platform/renesas/rzg2l-cru/rzg2l-core.c | 6 ++---- .../platform/renesas/rzg2l-cru/rzg2l-csi2.c | 6 ++---- drivers/media/platform/renesas/sh_vou.c | 5 ++--- drivers/media/platform/renesas/vsp1/vsp1_drv.c | 6 ++---- drivers/media/platform/rockchip/rga/rga.c | 6 ++---- .../media/platform/rockchip/rkisp1/rkisp1-dev.c | 6 ++---- .../media/platform/samsung/exynos-gsc/gsc-core.c | 5 ++--- .../platform/samsung/exynos4-is/fimc-core.c | 5 ++--- .../platform/samsung/exynos4-is/fimc-is-i2c.c | 6 ++---- .../media/platform/samsung/exynos4-is/fimc-is.c | 6 ++---- .../platform/samsung/exynos4-is/fimc-lite.c | 5 ++--- .../platform/samsung/exynos4-is/media-dev.c | 8 +++----- .../platform/samsung/exynos4-is/mipi-csis.c | 6 ++---- .../platform/samsung/s3c-camif/camif-core.c | 6 ++---- drivers/media/platform/samsung/s5p-g2d/g2d.c | 5 ++--- .../media/platform/samsung/s5p-jpeg/jpeg-core.c | 6 ++---- drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c | 5 ++--- drivers/media/platform/st/sti/bdisp/bdisp-v4l2.c | 6 ++---- .../platform/st/sti/c8sectpfe/c8sectpfe-core.c | 6 ++---- drivers/media/platform/st/sti/delta/delta-v4l2.c | 6 ++---- drivers/media/platform/st/sti/hva/hva-v4l2.c | 6 ++---- drivers/media/platform/st/stm32/dma2d/dma2d.c | 6 ++---- drivers/media/platform/st/stm32/stm32-dcmi.c | 6 ++---- .../media/platform/sunxi/sun4i-csi/sun4i_csi.c | 6 ++---- .../media/platform/sunxi/sun6i-csi/sun6i_csi.c | 6 ++---- .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c | 6 ++---- .../sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c | 6 ++---- drivers/media/platform/sunxi/sun8i-di/sun8i-di.c | 6 ++---- .../platform/sunxi/sun8i-rotate/sun8i_rotate.c | 6 ++---- drivers/media/platform/ti/am437x/am437x-vpfe.c | 6 ++---- drivers/media/platform/ti/cal/cal.c | 6 ++---- drivers/media/platform/ti/davinci/vpif.c | 6 ++---- drivers/media/platform/ti/davinci/vpif_capture.c | 5 ++--- drivers/media/platform/ti/davinci/vpif_display.c | 6 ++---- drivers/media/platform/ti/omap/omap_vout.c | 5 ++--- drivers/media/platform/ti/omap3isp/isp.c | 6 ++---- drivers/media/platform/ti/vpe/vpe.c | 6 ++---- drivers/media/platform/verisilicon/hantro_drv.c | 5 ++--- drivers/media/platform/via/via-camera.c | 5 ++--- drivers/media/platform/video-mux.c | 6 ++---- drivers/media/platform/xilinx/xilinx-csi2rxss.c | 6 ++---- drivers/media/platform/xilinx/xilinx-tpg.c | 6 ++---- drivers/media/platform/xilinx/xilinx-vipp.c | 6 ++---- drivers/media/platform/xilinx/xilinx-vtc.c | 6 ++---- drivers/media/radio/radio-si476x.c | 6 ++---- drivers/media/radio/radio-timb.c | 5 ++--- drivers/media/radio/radio-wl1273.c | 6 ++---- .../media/radio/si4713/radio-platform-si4713.c | 6 ++---- drivers/media/rc/gpio-ir-recv.c | 6 ++---- drivers/media/rc/img-ir/img-ir-core.c | 5 ++--- drivers/media/rc/ir-hix5hd2.c | 5 ++--- drivers/media/rc/meson-ir-tx.c | 6 ++---- drivers/media/rc/meson-ir.c | 6 ++---- drivers/media/rc/mtk-cir.c | 6 ++---- drivers/media/rc/st_rc.c | 5 ++--- drivers/media/rc/sunxi-cir.c | 6 ++---- .../media/test-drivers/vicodec/vicodec-core.c | 6 ++---- drivers/media/test-drivers/vidtv/vidtv_bridge.c | 6 ++---- drivers/media/test-drivers/vim2m.c | 6 ++---- drivers/media/test-drivers/vimc/vimc-core.c | 6 ++---- drivers/media/test-drivers/visl/visl-core.c | 6 ++---- drivers/media/test-drivers/vivid/vivid-core.c | 5 ++--- drivers/media/tuners/it913x.c | 6 ++---- 114 files changed, 237 insertions(+), 449 deletions(-) base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6