| Message ID | 20220607152458.232847-5-u.kleine-koenig@pengutronix.de |
|---|---|
| State | Accepted |
| Headers | show |
| Series | mtd: st_spi_fsm: Some fixes and improvements | expand |
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
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
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;
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(-)