[PATCH] mtd: nand: qcom: Fix the node for nand unmap resource

[PATCH] mtd: nand: qcom: Fix the node for nand unmap resource

[Bibek Kumar Patro]

While unmapping the nand resource in case of err_core_clk
the dev node being passed is res_start instead of nand->dma_base
(where the iova returned from map operation is stored) causing
failure in unmap operation. Hence modifying the unmap operation
to pass the nand->base_dma instead of res_start.

Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com>
---
 drivers/mtd/nand/raw/qcom_nandc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index f583022755a2..e085a0f588eb 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -3322,7 +3322,7 @@ static int qcom_nandc_probe(struct platform_device *pdev)
 err_aon_clk:
 	clk_disable_unprepare(nandc->core_clk);
 err_core_clk:
-	dma_unmap_resource(dev, res->start, resource_size(res),
+	dma_unmap_resource(dev, nandc->base_dma, resource_size(res),
 			   DMA_BIDIRECTIONAL, 0);
 	dev_err(&pdev->dev, "DEBUG: probe failed for nandc module\n");
 	return ret;
-- 
2.17.1


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

Re: [PATCH] mtd: nand: qcom: Fix the node for nand unmap resource

[Pavan Kondeti]

On Thu, Sep 07, 2023 at 02:58:54PM +0530, Bibek Kumar Patro wrote:
> While unmapping the nand resource in case of err_core_clk
> the dev node being passed is res_start instead of nand->dma_base

It is not not dev not but addr argument.

> (where the iova returned from map operation is stored) causing
> failure in unmap operation. Hence modifying the unmap operation
> to pass the nand->base_dma instead of res_start.
> 

Pls simplify this commit description. I think, it was a simple copy/paste
mistake. I would write

"Fix addr argument to dma_unmap_resource() in the error path of probe.
The addr argument should be dma address not physical address."

> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com>
> ---
>  drivers/mtd/nand/raw/qcom_nandc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index f583022755a2..e085a0f588eb 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -3322,7 +3322,7 @@ static int qcom_nandc_probe(struct platform_device *pdev)
>  err_aon_clk:
>  	clk_disable_unprepare(nandc->core_clk);
>  err_core_clk:
> -	dma_unmap_resource(dev, res->start, resource_size(res),
> +	dma_unmap_resource(dev, nandc->base_dma, resource_size(res),
>  			   DMA_BIDIRECTIONAL, 0);
>  	dev_err(&pdev->dev, "DEBUG: probe failed for nandc module\n");
>  	return ret;

Since you are fixing a bug introduced by a previous commit, you should
add Fixes tag like below. Refer to Documentation [1].

Fixes: 7330fc505af4 ("mtd: rawnand: qcom: stop using phys_to_dma()")

[1] https://docs.kernel.org/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes

Thanks,
Pavan

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

Re: [PATCH] mtd: nand: qcom: Fix the node for nand unmap resource

[Bibek Kumar Patro]

On 9/8/2023 3:52 PM, Pavan Kondeti wrote:
> On Thu, Sep 07, 2023 at 02:58:54PM +0530, Bibek Kumar Patro wrote:
>> While unmapping the nand resource in case of err_core_clk
>> the dev node being passed is res_start instead of nand->dma_base
> 
> It is not not dev not but addr argument.
> 
>> (where the iova returned from map operation is stored) causing
>> failure in unmap operation. Hence modifying the unmap operation
>> to pass the nand->base_dma instead of res_start.
>>
> 
> Pls simplify this commit description. I think, it was a simple copy/paste
> mistake. I would write
> 
> "Fix addr argument to dma_unmap_resource() in the error path of probe.
> The addr argument should be dma address not physical address."
> 

Thanks for this suggestion Pavan, this looks simplified and easier to
understand the bug. Incorporated this in next revision.

>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com>
>> ---
>>   drivers/mtd/nand/raw/qcom_nandc.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
>> index f583022755a2..e085a0f588eb 100644
>> --- a/drivers/mtd/nand/raw/qcom_nandc.c
>> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
>> @@ -3322,7 +3322,7 @@ static int qcom_nandc_probe(struct platform_device *pdev)
>>   err_aon_clk:
>>   	clk_disable_unprepare(nandc->core_clk);
>>   err_core_clk:
>> -	dma_unmap_resource(dev, res->start, resource_size(res),
>> +	dma_unmap_resource(dev, nandc->base_dma, resource_size(res),
>>   			   DMA_BIDIRECTIONAL, 0);
>>   	dev_err(&pdev->dev, "DEBUG: probe failed for nandc module\n");
>>   	return ret;
> 
> Since you are fixing a bug introduced by a previous commit, you should
> add Fixes tag like below. Refer to Documentation [1].
> 
> Fixes: 7330fc505af4 ("mtd: rawnand: qcom: stop using phys_to_dma()")
> 
> [1] https://docs.kernel.org/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes

Thanks very much for the inputs, addressed this in next revision.

Thanks & regards,
Bibek

> 
> Thanks,
> Pavan

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

Re: [PATCH] mtd: nand: qcom: Fix the node for nand unmap resource

[Manivannan Sadhasivam]

On Thu, Sep 07, 2023 at 02:58:54PM +0530, Bibek Kumar Patro wrote:
> While unmapping the nand resource in case of err_core_clk
> the dev node being passed is res_start instead of nand->dma_base
> (where the iova returned from map operation is stored) causing
> failure in unmap operation. Hence modifying the unmap operation
> to pass the nand->base_dma instead of res_start.
> 
> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com>
> ---
>  drivers/mtd/nand/raw/qcom_nandc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index f583022755a2..e085a0f588eb 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -3322,7 +3322,7 @@ static int qcom_nandc_probe(struct platform_device *pdev)
>  err_aon_clk:
>  	clk_disable_unprepare(nandc->core_clk);
>  err_core_clk:
> -	dma_unmap_resource(dev, res->start, resource_size(res),
> +	dma_unmap_resource(dev, nandc->base_dma, resource_size(res),
>  			   DMA_BIDIRECTIONAL, 0);
>  	dev_err(&pdev->dev, "DEBUG: probe failed for nandc module\n");

This error indicates that you are sending the patch against downstream tree.
That's not appropriate. Please send your patches against mainline/mtd-next
instead and also validate properly.

- Mani

>  	return ret;
> -- 
> 2.17.1
> 

-- 
மணிவண்ணன் சதாசிவம்

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

Re: [PATCH] mtd: nand: qcom: Fix the node for nand unmap resource

[Bibek Kumar Patro]

On 9/9/2023 11:33 AM, Manivannan Sadhasivam wrote:
> On Thu, Sep 07, 2023 at 02:58:54PM +0530, Bibek Kumar Patro wrote:
>> While unmapping the nand resource in case of err_core_clk
>> the dev node being passed is res_start instead of nand->dma_base
>> (where the iova returned from map operation is stored) causing
>> failure in unmap operation. Hence modifying the unmap operation
>> to pass the nand->base_dma instead of res_start.
>>
>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com>
>> ---
>>   drivers/mtd/nand/raw/qcom_nandc.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
>> index f583022755a2..e085a0f588eb 100644
>> --- a/drivers/mtd/nand/raw/qcom_nandc.c
>> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
>> @@ -3322,7 +3322,7 @@ static int qcom_nandc_probe(struct platform_device *pdev)
>>   err_aon_clk:
>>   	clk_disable_unprepare(nandc->core_clk);
>>   err_core_clk:
>> -	dma_unmap_resource(dev, res->start, resource_size(res),
>> +	dma_unmap_resource(dev, nandc->base_dma, resource_size(res),
>>   			   DMA_BIDIRECTIONAL, 0);
>>   	dev_err(&pdev->dev, "DEBUG: probe failed for nandc module\n");
> 
> This error indicates that you are sending the patch against downstream tree.
> That's not appropriate. Please send your patches against mainline/mtd-next
> instead and also validate properly.
> 
> - Mani

This patch is created against upstream tree only,
These debug prints got added from a conflicting patch
created for debugging.
apologies for this mistake. Will revise this and send
a new patch set.

regards,
Bibek


> 
>>   	return ret;
>> -- 
>> 2.17.1
>>
> 

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