Re: [PATCH] mtd: rawnand: mxic: Enable and prepare clocks in probe

From: Evgeny Novikov
Date: Tue Aug 17 2021 - 06:36:20 EST

Hi Miquel,

On 16.08.2021 11:01, Miquel Raynal wrote:
Hi Andy,

Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote on Thu, 12 Aug 2021
15:13:10 +0300:

On Thursday, August 12, 2021, Evgeny Novikov <novikov@xxxxxxxxx> wrote:

It seems that mxic_nfc_probe() missed invocation of
mxic_nfc_clk_enable(). The patch fixed that. In addition, error handling
was refined appropriately.

NAK. Until you provide a deeper analysis, like how this works before your

Please, don’t blindly generate patches, this can even your bot do, just
think about each change and preferable test on the real hardware.

The above is to all your lovely contributions.

Found by Linux Driver Verification project (

Signed-off-by: Evgeny Novikov <novikov@xxxxxxxxx>
Co-developed-by: Kirill Shilimanov <kirill.shilimanov@xxxxxxxxxx>
Signed-off-by: Kirill Shilimanov <kirill.shilimanov@xxxxxxxxxx>
drivers/mtd/nand/raw/mxic_nand.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/mxic_nand.c b/drivers/mtd/nand/raw/mxic_
index da1070993994..37e75bf60ee5 100644
--- a/drivers/mtd/nand/raw/mxic_nand.c
+++ b/drivers/mtd/nand/raw/mxic_nand.c
@@ -509,9 +509,15 @@ static int mxic_nfc_probe(struct platform_device
if (IS_ERR(nfc->send_dly_clk))
return PTR_ERR(nfc->send_dly_clk);

+ err = mxic_nfc_clk_enable(nfc);
+ if (err)
+ return err;
As Andy said, this is not needed.

nfc->regs = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(nfc->regs))
- return PTR_ERR(nfc->regs);
+ if (IS_ERR(nfc->regs)) {
+ err = PTR_ERR(nfc->regs);
+ goto fail;
+ }

nand_chip = &nfc->chip;
mtd = nand_to_mtd(nand_chip);
@@ -527,8 +533,10 @@ static int mxic_nfc_probe(struct platform_device
nand_chip->controller = &nfc->controller;

irq = platform_get_irq(pdev, 0);
- if (irq < 0)
- return irq;
+ if (irq < 0) {
+ err = irq;
+ goto fail;
However some reworking is needed in the error path.

That goto statement should be renamed and devm_request_irq() should not
jump to it.

We still need some help and clarification from those who are very familiar with this sort of drivers or/and can test this particular driver. mxic_nfc_clk_enable() is the complementary function for mxic_nfc_clk_disable(). No other functions invoke clk_prepare_enable()/clk_disable_unprepare() in the driver. Unlikely somebody in its environment does that since driver specific clocks are dealt with. At the moment the driver invokes mxic_nfc_clk_disable() on error handling paths in probe, in remove and in mxic_nfc_set_freq(). mxic_nfc_clk_enable() is called just by mxic_nfc_set_freq() that moreover does this after calling mxic_nfc_clk_disable() first. So, we did not find any place in the driver that invokes mxic_nfc_clk_enable() prior to mxic_nfc_clk_disable(). Basing on this we added mxic_nfc_clk_enable() just after getting clocks. As I explained in the previous large e-mail, we may be wrong in our understanding of the driver environment or/and at specification of requirements being checked. It would be great if you will point out on our mistakes.

Best regards,
Evgeny Novikov
+ }