Re: [PATCH v2 1/2] mtd: rawnand: qcom: Fix clock sequencing in qcom_nandc_probe()

From: Manivannan Sadhasivam <mani@kernel.org>
To: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: linux-arm-msm@vger.kernel.org, linux-mtd@lists.infradead.org,
	miquel.raynal@bootlin.com, architt@codeaurora.org,
	bbrezillon@kernel.org, absahu@codeaurora.org, baruch@tkos.co.il
Subject: Re: [PATCH v2 1/2] mtd: rawnand: qcom: Fix clock sequencing in qcom_nandc_probe()
Date: Mon, 3 Jan 2022 11:21:52 +0530	[thread overview]
Message-ID: <20220103055152.GA3581@thinkpad> (raw)
In-Reply-To: <20220103030316.58301-2-bryan.odonoghue@linaro.org>

On Mon, Jan 03, 2022 at 03:03:15AM +0000, Bryan O'Donoghue wrote:
> Interacting with a NAND chip on an IPQ6018 I found that the qcomsmem NAND
> partition parser was returning -EPROBE_DEFER waiting for the main smem
> driver to load.
> 
> This caused the board to reset. Playing about with the probe() function
> shows that the problem lies in the core clock being switched off before the
> nandc_unalloc() routine has completed.
> 
> If we look at how qcom_nandc_remove() tears down allocated resources we see
> the expected order is
> 
> qcom_nandc_unalloc(nandc);
> 
> clk_disable_unprepare(nandc->aon_clk);
> clk_disable_unprepare(nandc->core_clk);
> 
> dma_unmap_resource(&pdev->dev, nandc->base_dma, resource_size(res),
> 		   DMA_BIDIRECTIONAL, 0);
> 
> Tweaking probe() to both bring up and tear-down in that order removes the
> reset if we end up deferring elsewhere.
> 
> Fixes: c76b78d8ec05 ("mtd: nand: Qualcomm NAND controller driver")
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>

Can you please CC stable list for backporting?

Thanks,
Mani

> ---
>  drivers/mtd/nand/raw/qcom_nandc.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index 04e6f7b267064..0f41a9a421575 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -2,7 +2,6 @@
>  /*
>   * Copyright (c) 2016, The Linux Foundation. All rights reserved.
>   */
> -
>  #include <linux/clk.h>
>  #include <linux/slab.h>
>  #include <linux/bitops.h>
> @@ -3063,10 +3062,6 @@ static int qcom_nandc_probe(struct platform_device *pdev)
>  	if (dma_mapping_error(dev, nandc->base_dma))
>  		return -ENXIO;
>  
> -	ret = qcom_nandc_alloc(nandc);
> -	if (ret)
> -		goto err_nandc_alloc;
> -
>  	ret = clk_prepare_enable(nandc->core_clk);
>  	if (ret)
>  		goto err_core_clk;
> @@ -3075,6 +3070,10 @@ static int qcom_nandc_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_aon_clk;
>  
> +	ret = qcom_nandc_alloc(nandc);
> +	if (ret)
> +		goto err_nandc_alloc;
> +
>  	ret = qcom_nandc_setup(nandc);
>  	if (ret)
>  		goto err_setup;
> @@ -3086,15 +3085,14 @@ static int qcom_nandc_probe(struct platform_device *pdev)
>  	return 0;
>  
>  err_setup:
> +	qcom_nandc_unalloc(nandc);
> +err_nandc_alloc:
>  	clk_disable_unprepare(nandc->aon_clk);
>  err_aon_clk:
>  	clk_disable_unprepare(nandc->core_clk);
>  err_core_clk:
> -	qcom_nandc_unalloc(nandc);
> -err_nandc_alloc:
>  	dma_unmap_resource(dev, res->start, resource_size(res),
>  			   DMA_BIDIRECTIONAL, 0);
> -
>  	return ret;
>  }
>  
> -- 
> 2.33.0
> 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/