| Message ID | 7946a26c6e53a4158f0f5bad5276d8654fd59415.1670673147.git.christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] spi: xtensa-xtfpga: Fix a double put() in xtfpga_spi_remove() | expand |
Hi Christophe, On Sat, Dec 10, 2022 at 3:52 AM Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > > 'master' is allocated with devm_spi_alloc_master(), there is no need to > put it explicitly in the remove function. > > Fixes: 478cc2fc3dd7 ("spi: xtensa-xtfpga: Switch to use devm_spi_alloc_master()") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > drivers/spi/spi-xtensa-xtfpga.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/spi/spi-xtensa-xtfpga.c b/drivers/spi/spi-xtensa-xtfpga.c > index 2fa7608f94cd..271ae98f441c 100644 > --- a/drivers/spi/spi-xtensa-xtfpga.c > +++ b/drivers/spi/spi-xtensa-xtfpga.c > @@ -123,7 +123,6 @@ static int xtfpga_spi_remove(struct platform_device *pdev) > struct xtfpga_spi *xspi = spi_master_get_devdata(master); > > spi_bitbang_stop(&xspi->bitbang); > - spi_master_put(master); This put is matching the get in the spi_bitbang_start. It was discussed here: https://lore.kernel.org/linux-spi/CAMo8BfJaD7pG_iutY8jordysjChyzhTpVSqpxXh3QoZsj2QmaQ@mail.gmail.com/
On Sat, Dec 10, 2022 at 06:48:02AM -0800, Max Filippov wrote: > Hi Christophe, > > On Sat, Dec 10, 2022 at 3:52 AM Christophe JAILLET > <christophe.jaillet@wanadoo.fr> wrote: > > 'master' is allocated with devm_spi_alloc_master(), there is no need to > > put it explicitly in the remove function. > > spi_bitbang_stop(&xspi->bitbang); > > - spi_master_put(master); > This put is matching the get in the spi_bitbang_start. > It was discussed here: > https://lore.kernel.org/linux-spi/CAMo8BfJaD7pG_iutY8jordysjChyzhTpVSqpxXh3QoZsj2QmaQ@mail.gmail.com/ Probably worth a comment though since it is a bit of a gotcha. Ideally we'd improve this in the bitbang code but that's harder.
On 2022/12/12 23:36, Mark Brown wrote: > On Sat, Dec 10, 2022 at 06:48:02AM -0800, Max Filippov wrote: >> Hi Christophe, >> >> On Sat, Dec 10, 2022 at 3:52 AM Christophe JAILLET >> <christophe.jaillet@wanadoo.fr> wrote: >>> 'master' is allocated with devm_spi_alloc_master(), there is no need to >>> put it explicitly in the remove function. >>> spi_bitbang_stop(&xspi->bitbang); >>> - spi_master_put(master); >> This put is matching the get in the spi_bitbang_start. >> It was discussed here: >> https://lore.kernel.org/linux-spi/CAMo8BfJaD7pG_iutY8jordysjChyzhTpVSqpxXh3QoZsj2QmaQ@mail.gmail.com/ > Probably worth a comment though since it is a bit of a gotcha. Ideally > we'd improve this in the bitbang code but that's harder. Ideally, spi_bitbang_stop() should undo spi_bitbang_start(). shall we move spi_master_put() in spi_bitbang_stop() instead of calling it separately in drivers? Thanks, Yang
On Tue, Dec 13, 2022 at 09:30:32AM +0800, Yang Yingliang wrote: > On 2022/12/12 23:36, Mark Brown wrote: > > On Sat, Dec 10, 2022 at 06:48:02AM -0800, Max Filippov wrote: > > Probably worth a comment though since it is a bit of a gotcha. Ideally > > we'd improve this in the bitbang code but that's harder. > Ideally, spi_bitbang_stop() should undo spi_bitbang_start(). shall we move > spi_master_put() in spi_bitbang_stop() instead of > calling it separately in drivers? Ideally like I say, there's issues with devm IIRC which make it more complicated than just adding a put() there.
diff --git a/drivers/spi/spi-xtensa-xtfpga.c b/drivers/spi/spi-xtensa-xtfpga.c index 2fa7608f94cd..271ae98f441c 100644 --- a/drivers/spi/spi-xtensa-xtfpga.c +++ b/drivers/spi/spi-xtensa-xtfpga.c @@ -123,7 +123,6 @@ static int xtfpga_spi_remove(struct platform_device *pdev) struct xtfpga_spi *xspi = spi_master_get_devdata(master); spi_bitbang_stop(&xspi->bitbang); - spi_master_put(master); return 0; }
'master' is allocated with devm_spi_alloc_master(), there is no need to put it explicitly in the remove function. Fixes: 478cc2fc3dd7 ("spi: xtensa-xtfpga: Switch to use devm_spi_alloc_master()") Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- drivers/spi/spi-xtensa-xtfpga.c | 1 - 1 file changed, 1 deletion(-)