On 4/27/20 10:10 PM, Marek Vasut wrote:
On 4/27/20 10:08 PM, Miquel Raynal wrote:
Hi Marek,
Marek Vasut <marex@xxxxxxx> wrote on Mon, 27 Apr 2020 21:46:44 +0200:
On 4/27/20 8:08 PM, Miquel Raynal wrote:
[...]
/* FMC2 init routine */
stm32_fmc2_init(fmc2);
@@ -1997,7 +2001,7 @@ static int stm32_fmc2_probe(struct platform_device *pdev)
/* Scan to find existence of the device */
ret = nand_scan(chip, nand->ncs);
if (ret)
- goto err_scan;
+ goto err_dma_setup;
ret = mtd_device_register(mtd, NULL, 0);
if (ret)
@@ -2010,7 +2014,7 @@ static int stm32_fmc2_probe(struct platform_device *pdev)
err_device_register:
nand_cleanup(chip);
-err_scan:
+err_dma_setup:
if (fmc2->dma_ecc_ch)
dma_release_channel(fmc2->dma_ecc_ch);
if (fmc2->dma_tx_ch)
@@ -2021,6 +2025,7 @@ static int stm32_fmc2_probe(struct platform_device *pdev)
sg_free_table(&fmc2->dma_data_sg);
sg_free_table(&fmc2->dma_ecc_sg);
+err_clk_disable:
clk_disable_unprepare(fmc2->clk);
return ret;
I didn't spot it during my earlier reviews but I really prefer using
labels explaining what you do than having the same name of the function
which failed. This way you don't have to rework the error path when
you handle an additional error.
So, would you mind doing this in two steps:
1/
Replace
err_scan:
with, eg.
release_dma_objs:
The ^err_ prefix in failpath labels is useful, since it's easily
possible to match on it with regexes ; not so much on arbitrary label name.
I guess so, but is it actually useful to catch labels in a regex? (real
question)
I find it useful to have a unified way to find those labels, e.g.
err_because_foo:
err_because_bar:
err_last_one:
is much nicer than:
foo_failed:
bar_also_failed:
its_total_randomness:
My point being, Christophe, you can use err_ as a prefix but I think
it's better to use:
err_do_this_cleanup
than
err_this_failed
That's fine either way.
Hi Miquel,
I will rename the label in v3:
- err_device_register => err_nand_cleanup
- err_dma_setup => err_release_dma
- err_clk_disable => will keep this one
Regards,
Christophe Kerello.
Any way I suppose catching ":\n" is already a good approximation to
find labels?
Not very practical with git grep (^err.*: works nicely though)
I suppose ^.*:$ would work the same ;)
Try and see how much other irrelevant stuff that sucks in ;-)
