* [PATCH 0/4] mtd: st_spi_fsm: Some fixes and improvements
@ 2022-06-07 15:24 Uwe Kleine-König
2022-06-07 15:24 ` [PATCH 1/4] mtd: st_spi_fsm: Add a clk_disable_unprepare() in .probe()'s error path Uwe Kleine-König
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Uwe Kleine-König @ 2022-06-07 15:24 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Yang Yingliang,
Boris Brezillon, Arvind Yadav, Lee Jones, Brian Norris,
linux-mtd, kernel
Hello,
my intention was to just resend
https://lore.kernel.org/linux-mtd/20220603210758.148493-5-u.kleine-koenig@pengutronix.de
because after a change that was merged for 5.19-rc1 this patch didn't
apply any more. While preparing this update, I looked a bit too deep
into the driver and identified a few more patch opportunities.
I considered swapping the order of patches #2 and #3 because #3 is a fix
which #2 isn't. But doing would require a helper variable added to
.remove() in #2 that then is removed again in #3. If you feel strong
here (e.g. because you think #3 should be backported to stable but #2
shouldn't), I can rework accordingly.
Best regards
Uwe
Uwe Kleine-König (4):
mtd: st_spi_fsm: Add a clk_disable_unprepare() in .probe()'s error path
mtd: st_spi_fsm: Warn about failure to unregister mtd device
mtd: st_spi_fsm: Disable clock only after device was unregistered
mtd: st_spi_fsm: Simplify error checking in .probe() a bit
drivers/mtd/devices/st_spi_fsm.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
base-commit: f2906aa863381afb0015a9eb7fefad885d4e5a56
--
2.36.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/4] mtd: st_spi_fsm: Add a clk_disable_unprepare() in .probe()'s error path
2022-06-07 15:24 [PATCH 0/4] mtd: st_spi_fsm: Some fixes and improvements Uwe Kleine-König
@ 2022-06-07 15:24 ` Uwe Kleine-König
2022-06-09 13:09 ` Miquel Raynal
2022-06-07 15:24 ` [PATCH 2/4] mtd: st_spi_fsm: Warn about failure to unregister mtd device Uwe Kleine-König
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Uwe Kleine-König @ 2022-06-07 15:24 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Yang Yingliang,
Boris Brezillon, Arvind Yadav, Lee Jones, Brian Norris,
linux-mtd, kernel
For all but one error path clk_disable_unprepare() is already there. Add
it to the one location where it's missing.
Fixes: 481815a6193b ("mtd: st_spi_fsm: Handle clk_prepare_enable/clk_disable_unprepare.")
Fixes: 69d5af8d016c ("mtd: st_spi_fsm: Obtain and use EMI clock")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/mtd/devices/st_spi_fsm.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index d3377b10fc0f..52a799cae402 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -2115,10 +2115,12 @@ static int stfsm_probe(struct platform_device *pdev)
(long long)fsm->mtd.size, (long long)(fsm->mtd.size >> 20),
fsm->mtd.erasesize, (fsm->mtd.erasesize >> 10));
- return mtd_device_register(&fsm->mtd, NULL, 0);
-
+ ret = mtd_device_register(&fsm->mtd, NULL, 0);
+ if (ret) {
err_clk_unprepare:
- clk_disable_unprepare(fsm->clk);
+ clk_disable_unprepare(fsm->clk);
+ }
+
return ret;
}
--
2.36.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/4] mtd: st_spi_fsm: Warn about failure to unregister mtd device
2022-06-07 15:24 [PATCH 0/4] mtd: st_spi_fsm: Some fixes and improvements Uwe Kleine-König
2022-06-07 15:24 ` [PATCH 1/4] mtd: st_spi_fsm: Add a clk_disable_unprepare() in .probe()'s error path Uwe Kleine-König
@ 2022-06-07 15:24 ` Uwe Kleine-König
2022-06-09 13:08 ` Miquel Raynal
2022-06-07 15:24 ` [PATCH 3/4] mtd: st_spi_fsm: Disable clock only after device was unregistered Uwe Kleine-König
2022-06-07 15:24 ` [PATCH 4/4] mtd: st_spi_fsm: Simplify error checking in .probe() a bit Uwe Kleine-König
3 siblings, 1 reply; 10+ messages in thread
From: Uwe Kleine-König @ 2022-06-07 15:24 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Yang Yingliang,
Boris Brezillon, Arvind Yadav, Lee Jones, Brian Norris,
linux-mtd, kernel
mtd_device_unregister() shouldn't fail. Wail loudly if it does anyhow.
This matches how other drivers (e.g. nand/raw/nandsim.c) use
mtd_device_unregister().
By returning 0 in the platform remove callback a generic error message
by the device core is suppressed, nothing else changes.
This is a preparation for making platform remove callbacks return void.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/mtd/devices/st_spi_fsm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index 52a799cae402..a5a4b612480c 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -2130,7 +2130,9 @@ static int stfsm_remove(struct platform_device *pdev)
clk_disable_unprepare(fsm->clk);
- return mtd_device_unregister(&fsm->mtd);
+ WARN_ON(mtd_device_unregister(&fsm->mtd));
+
+ return 0;
}
#ifdef CONFIG_PM_SLEEP
--
2.36.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/4] mtd: st_spi_fsm: Disable clock only after device was unregistered
2022-06-07 15:24 [PATCH 0/4] mtd: st_spi_fsm: Some fixes and improvements Uwe Kleine-König
2022-06-07 15:24 ` [PATCH 1/4] mtd: st_spi_fsm: Add a clk_disable_unprepare() in .probe()'s error path Uwe Kleine-König
2022-06-07 15:24 ` [PATCH 2/4] mtd: st_spi_fsm: Warn about failure to unregister mtd device Uwe Kleine-König
@ 2022-06-07 15:24 ` Uwe Kleine-König
2022-06-09 13:08 ` Miquel Raynal
2022-06-07 15:24 ` [PATCH 4/4] mtd: st_spi_fsm: Simplify error checking in .probe() a bit Uwe Kleine-König
3 siblings, 1 reply; 10+ messages in thread
From: Uwe Kleine-König @ 2022-06-07 15:24 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Yang Yingliang,
Boris Brezillon, Arvind Yadav, Lee Jones, Brian Norris,
linux-mtd, kernel
Until mtd_device_unregister() returns the device is expected to be
operational. So only disable the clock after the mtd is unregistered.
Fixes: 1fefc8ecb834 ("mtd: st_spi_fsm: add missing clk_disable_unprepare() in stfsm_remove()")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/mtd/devices/st_spi_fsm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index a5a4b612480c..9f6d4dd8bade 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -2128,10 +2128,10 @@ static int stfsm_remove(struct platform_device *pdev)
{
struct stfsm *fsm = platform_get_drvdata(pdev);
- clk_disable_unprepare(fsm->clk);
-
WARN_ON(mtd_device_unregister(&fsm->mtd));
+ clk_disable_unprepare(fsm->clk);
+
return 0;
}
--
2.36.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/4] mtd: st_spi_fsm: Simplify error checking in .probe() a bit
2022-06-07 15:24 [PATCH 0/4] mtd: st_spi_fsm: Some fixes and improvements Uwe Kleine-König
` (2 preceding siblings ...)
2022-06-07 15:24 ` [PATCH 3/4] mtd: st_spi_fsm: Disable clock only after device was unregistered Uwe Kleine-König
@ 2022-06-07 15:24 ` Uwe Kleine-König
2022-06-07 15:32 ` Miquel Raynal
2022-06-09 13:08 ` Miquel Raynal
3 siblings, 2 replies; 10+ messages in thread
From: Uwe Kleine-König @ 2022-06-07 15:24 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Yang Yingliang,
Boris Brezillon, Arvind Yadav, Lee Jones, Brian Norris,
linux-mtd, kernel
Instead of ending each if branch with the same check, do it once
unconditionally after the if block.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Not entirely sure this is an objective improvement, but I like it better
this way.
It could be shorted one step further by doing
ret = (info->config ?: stfsm_prepare_rwe_seqs_default)(fsm);
if (ret)
goto err_clk_unprepare;
but IMHO readability suffers here.
drivers/mtd/devices/st_spi_fsm.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index 9f6d4dd8bade..54861d889c30 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -2084,15 +2084,12 @@ static int stfsm_probe(struct platform_device *pdev)
* Configure READ/WRITE/ERASE sequences according to platform and
* device flags.
*/
- if (info->config) {
+ if (info->config)
ret = info->config(fsm);
- if (ret)
- goto err_clk_unprepare;
- } else {
+ else
ret = stfsm_prepare_rwe_seqs_default(fsm);
- if (ret)
- goto err_clk_unprepare;
- }
+ if (ret)
+ goto err_clk_unprepare;
fsm->mtd.name = info->name;
fsm->mtd.dev.parent = &pdev->dev;
--
2.36.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] mtd: st_spi_fsm: Simplify error checking in .probe() a bit
2022-06-07 15:24 ` [PATCH 4/4] mtd: st_spi_fsm: Simplify error checking in .probe() a bit Uwe Kleine-König
@ 2022-06-07 15:32 ` Miquel Raynal
2022-06-09 13:08 ` Miquel Raynal
1 sibling, 0 replies; 10+ messages in thread
From: Miquel Raynal @ 2022-06-07 15:32 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Richard Weinberger, Vignesh Raghavendra, Yang Yingliang,
Boris Brezillon, Arvind Yadav, Lee Jones, Brian Norris,
linux-mtd, kernel
Hi Uwe,
u.kleine-koenig@pengutronix.de wrote on Tue, 7 Jun 2022 17:24:58 +0200:
> Instead of ending each if branch with the same check, do it once
> unconditionally after the if block.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Not entirely sure this is an objective improvement, but I like it better
> this way.
>
> It could be shorted one step further by doing
>
> ret = (info->config ?: stfsm_prepare_rwe_seqs_default)(fsm);
> if (ret)
> goto err_clk_unprepare;
>
> but IMHO readability suffers here.
Clearly, yes, I think you opted-out for the right optimization in the
end.
>
> drivers/mtd/devices/st_spi_fsm.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
> index 9f6d4dd8bade..54861d889c30 100644
> --- a/drivers/mtd/devices/st_spi_fsm.c
> +++ b/drivers/mtd/devices/st_spi_fsm.c
> @@ -2084,15 +2084,12 @@ static int stfsm_probe(struct platform_device *pdev)
> * Configure READ/WRITE/ERASE sequences according to platform and
> * device flags.
> */
> - if (info->config) {
> + if (info->config)
> ret = info->config(fsm);
> - if (ret)
> - goto err_clk_unprepare;
> - } else {
> + else
> ret = stfsm_prepare_rwe_seqs_default(fsm);
> - if (ret)
> - goto err_clk_unprepare;
> - }
> + if (ret)
> + goto err_clk_unprepare;
>
> fsm->mtd.name = info->name;
> fsm->mtd.dev.parent = &pdev->dev;
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] mtd: st_spi_fsm: Simplify error checking in .probe() a bit
2022-06-07 15:24 ` [PATCH 4/4] mtd: st_spi_fsm: Simplify error checking in .probe() a bit Uwe Kleine-König
2022-06-07 15:32 ` Miquel Raynal
@ 2022-06-09 13:08 ` Miquel Raynal
1 sibling, 0 replies; 10+ messages in thread
From: Miquel Raynal @ 2022-06-09 13:08 UTC (permalink / raw)
To: Uwe Kleine-König, Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Yang Yingliang,
Boris Brezillon, Arvind Yadav, Lee Jones, Brian Norris,
linux-mtd, kernel
On Tue, 2022-06-07 at 15:24:58 UTC, =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= wrote:
> Instead of ending each if branch with the same check, do it once
> unconditionally after the if block.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.
Miquel
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] mtd: st_spi_fsm: Disable clock only after device was unregistered
2022-06-07 15:24 ` [PATCH 3/4] mtd: st_spi_fsm: Disable clock only after device was unregistered Uwe Kleine-König
@ 2022-06-09 13:08 ` Miquel Raynal
0 siblings, 0 replies; 10+ messages in thread
From: Miquel Raynal @ 2022-06-09 13:08 UTC (permalink / raw)
To: Uwe Kleine-König, Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Yang Yingliang,
Boris Brezillon, Arvind Yadav, Lee Jones, Brian Norris,
linux-mtd, kernel
On Tue, 2022-06-07 at 15:24:57 UTC, =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= wrote:
> Until mtd_device_unregister() returns the device is expected to be
> operational. So only disable the clock after the mtd is unregistered.
>
> Fixes: 1fefc8ecb834 ("mtd: st_spi_fsm: add missing clk_disable_unprepare() in stfsm_remove()")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.
Miquel
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] mtd: st_spi_fsm: Warn about failure to unregister mtd device
2022-06-07 15:24 ` [PATCH 2/4] mtd: st_spi_fsm: Warn about failure to unregister mtd device Uwe Kleine-König
@ 2022-06-09 13:08 ` Miquel Raynal
0 siblings, 0 replies; 10+ messages in thread
From: Miquel Raynal @ 2022-06-09 13:08 UTC (permalink / raw)
To: Uwe Kleine-König, Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Yang Yingliang,
Boris Brezillon, Arvind Yadav, Lee Jones, Brian Norris,
linux-mtd, kernel
On Tue, 2022-06-07 at 15:24:56 UTC, =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= wrote:
> mtd_device_unregister() shouldn't fail. Wail loudly if it does anyhow.
>
> This matches how other drivers (e.g. nand/raw/nandsim.c) use
> mtd_device_unregister().
>
> By returning 0 in the platform remove callback a generic error message
> by the device core is suppressed, nothing else changes.
>
> This is a preparation for making platform remove callbacks return void.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.
Miquel
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] mtd: st_spi_fsm: Add a clk_disable_unprepare() in .probe()'s error path
2022-06-07 15:24 ` [PATCH 1/4] mtd: st_spi_fsm: Add a clk_disable_unprepare() in .probe()'s error path Uwe Kleine-König
@ 2022-06-09 13:09 ` Miquel Raynal
0 siblings, 0 replies; 10+ messages in thread
From: Miquel Raynal @ 2022-06-09 13:09 UTC (permalink / raw)
To: Uwe Kleine-König, Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Yang Yingliang,
Boris Brezillon, Arvind Yadav, Lee Jones, Brian Norris,
linux-mtd, kernel
On Tue, 2022-06-07 at 15:24:55 UTC, =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= wrote:
> For all but one error path clk_disable_unprepare() is already there. Add
> it to the one location where it's missing.
>
> Fixes: 481815a6193b ("mtd: st_spi_fsm: Handle clk_prepare_enable/clk_disable_unprepare.")
> Fixes: 69d5af8d016c ("mtd: st_spi_fsm: Obtain and use EMI clock")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.
Miquel
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-06-09 13:09 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-07 15:24 [PATCH 0/4] mtd: st_spi_fsm: Some fixes and improvements Uwe Kleine-König
2022-06-07 15:24 ` [PATCH 1/4] mtd: st_spi_fsm: Add a clk_disable_unprepare() in .probe()'s error path Uwe Kleine-König
2022-06-09 13:09 ` Miquel Raynal
2022-06-07 15:24 ` [PATCH 2/4] mtd: st_spi_fsm: Warn about failure to unregister mtd device Uwe Kleine-König
2022-06-09 13:08 ` Miquel Raynal
2022-06-07 15:24 ` [PATCH 3/4] mtd: st_spi_fsm: Disable clock only after device was unregistered Uwe Kleine-König
2022-06-09 13:08 ` Miquel Raynal
2022-06-07 15:24 ` [PATCH 4/4] mtd: st_spi_fsm: Simplify error checking in .probe() a bit Uwe Kleine-König
2022-06-07 15:32 ` Miquel Raynal
2022-06-09 13:08 ` Miquel Raynal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.