[PATCH v2 0/3] mtd: rawnand: cadence: improvement and fixes

[PATCH v2 0/3] mtd: rawnand: cadence: improvement and fixes

[niravkumar.l.rabara]

From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>

This patchset introduces improvements and fixes for cadence nand driver.
The changes include:

1. Support deferred prob mechanism when DMA driver is not probed yet.
2. Map the slave DMA address using dma_map_resource. When ARM SMMU
   is enabled, using a direct physical address of SDMA results in
   DMA transaction failure.
3. Fixed the incorrect device context used for dma_unmap_single.

v2 changes:-
- Added the missing Fixes and Cc: stable tags to the patches.

Niravkumar L Rabara (3):
  mtd: rawnand: cadence: support deferred prob when DMA is not ready
  mtd: rawnand: cadence: use dma_map_resource for sdma address
  mtd: rawnand: cadence: fix incorrect dev context in dma_unmap_single

 .../mtd/nand/raw/cadence-nand-controller.c    | 35 +++++++++++++++----
 1 file changed, 28 insertions(+), 7 deletions(-)

-- 
2.25.1


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

[PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob when DMA is not ready

[niravkumar.l.rabara]

From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>

Use deferred driver probe in case the DMA driver is not probed.
When ARM SMMU is enabled, all peripheral device drivers, including NAND,
are probed earlier than the DMA driver.

Fixes: ec4ba01e894d ("mtd: rawnand: Add new Cadence NAND driver to MTD subsystem")
Cc: stable@vger.kernel.org
Signed-off-by: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
---
 drivers/mtd/nand/raw/cadence-nand-controller.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/cadence-nand-controller.c b/drivers/mtd/nand/raw/cadence-nand-controller.c
index 8d1d710e439d..5e27f5546f1b 100644
--- a/drivers/mtd/nand/raw/cadence-nand-controller.c
+++ b/drivers/mtd/nand/raw/cadence-nand-controller.c
@@ -2908,7 +2908,7 @@ static int cadence_nand_init(struct cdns_nand_ctrl *cdns_ctrl)
 		if (!cdns_ctrl->dmac) {
 			dev_err(cdns_ctrl->dev,
 				"Unable to get a DMA channel\n");
-			ret = -EBUSY;
+			ret = -EPROBE_DEFER;
 			goto disable_irq;
 		}
 	}
-- 
2.25.1


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

Re: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob when DMA is not ready

[Miquel Raynal]

Hello,

On 16/01/2025 at 11:21:52 +08, niravkumar.l.rabara@intel.com wrote:

Typo (prob) in the title.

> From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
>
> Use deferred driver probe in case the DMA driver is not probed.

Only devices are probed, not drivers.

> When ARM SMMU is enabled, all peripheral device drivers, including NAND,
> are probed earlier than the DMA driver.
>
> Fixes: ec4ba01e894d ("mtd: rawnand: Add new Cadence NAND driver to MTD subsystem")
> Cc: stable@vger.kernel.org
> Signed-off-by: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
> ---
>  drivers/mtd/nand/raw/cadence-nand-controller.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/cadence-nand-controller.c b/drivers/mtd/nand/raw/cadence-nand-controller.c
> index 8d1d710e439d..5e27f5546f1b 100644
> --- a/drivers/mtd/nand/raw/cadence-nand-controller.c
> +++ b/drivers/mtd/nand/raw/cadence-nand-controller.c
> @@ -2908,7 +2908,7 @@ static int cadence_nand_init(struct cdns_nand_ctrl *cdns_ctrl)
>  		if (!cdns_ctrl->dmac) {
>  			dev_err(cdns_ctrl->dev,
>  				"Unable to get a DMA channel\n");
> -			ret = -EBUSY;
> +			ret = -EPROBE_DEFER;

Does it work if there is no DMA channel provided? The bindings do not
mention DMA channels as mandatory.

Also, wouldn't it be more pleasant to use another helper from the DMA
core that returns a proper return code? So we now which one among
-EBUSY, -ENODEV or -EPROBE_DEFER we get?

Thanks,
Miquèl

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

RE: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob when DMA is not ready

[Rabara, Niravkumar L]

Hi Miquèl,


> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: Tuesday, 21 January, 2025 5:52 PM
> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>
> Cc: Richard Weinberger <richard@nod.at>; Vignesh Raghavendra
> <vigneshr@ti.com>; linux@treblig.org; Shen Lichuan <shenlichuan@vivo.com>;
> Jinjie Ruan <ruanjinjie@huawei.com>; u.kleine-koenig@baylibre.com; linux-
> mtd@lists.infradead.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org
> Subject: Re: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob when
> DMA is not ready
> 
> 
> Typo (prob) in the title.
> 
> > From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
> >
> > Use deferred driver probe in case the DMA driver is not probed.
> 
> Only devices are probed, not drivers.

I will fix the title and commit message in v3.
> 
> > --- a/drivers/mtd/nand/raw/cadence-nand-controller.c
> > +++ b/drivers/mtd/nand/raw/cadence-nand-controller.c
> > @@ -2908,7 +2908,7 @@ static int cadence_nand_init(struct cdns_nand_ctrl
> *cdns_ctrl)
> >  		if (!cdns_ctrl->dmac) {
> >  			dev_err(cdns_ctrl->dev,
> >  				"Unable to get a DMA channel\n");
> > -			ret = -EBUSY;
> > +			ret = -EPROBE_DEFER;
> 
> Does it work if there is no DMA channel provided? The bindings do not mention
> DMA channels as mandatory.
> 

The way Cadence NAND controller driver is written in such a way that it uses 
has_dma=1 as hardcoded value, indicating that slave DMA interface is connected
to DMA engine. However, it does not utilize the dedicated DMA channel information
from the device tree.

Driver works without external DMA interface i.e. has_dma=0. 
However current driver does not have a mechanism to configure it from device tree. 

> Also, wouldn't it be more pleasant to use another helper from the DMA core
> that returns a proper return code? So we now which one among -EBUSY, -
> ENODEV or -EPROBE_DEFER we get?
> 

Agree.
I will change to "dma_request_chan_by_mask" instead of "dma_request_channel "
so it can return a proper error code. 
  
		cdns_ctrl->dmac = dma_request_chan_by_mask(&mask);
		if (IS_ERR(cdns_ctrl->dmac)) {
			ret = PTR_ERR(cdns_ctrl->dmac);
			if (ret != -EPROBE_DEFER)
				dev_err(cdns_ctrl->dev,
					"Failed to get a DMA channel:%d\n",ret);
			goto disable_irq;
		}

Is this reasonable? 

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

Re: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob when DMA is not ready

[Miquel Raynal]

Hello,

>> > --- a/drivers/mtd/nand/raw/cadence-nand-controller.c
>> > +++ b/drivers/mtd/nand/raw/cadence-nand-controller.c
>> > @@ -2908,7 +2908,7 @@ static int cadence_nand_init(struct cdns_nand_ctrl
>> *cdns_ctrl)
>> >  		if (!cdns_ctrl->dmac) {
>> >  			dev_err(cdns_ctrl->dev,
>> >  				"Unable to get a DMA channel\n");
>> > -			ret = -EBUSY;
>> > +			ret = -EPROBE_DEFER;
>> 
>> Does it work if there is no DMA channel provided? The bindings do not mention
>> DMA channels as mandatory.
>> 
>
> The way Cadence NAND controller driver is written in such a way that it uses 
> has_dma=1 as hardcoded value, indicating that slave DMA interface is connected
> to DMA engine. However, it does not utilize the dedicated DMA channel information
> from the device tree.

This is not ok.

> Driver works without external DMA interface i.e. has_dma=0. 
> However current driver does not have a mechanism to configure it from
> device tree.

What? Why are you requesting a DMA channel from a dmaengine in this case?

Please make the distinction between the OS implementation (the driver)
and the DT binding which describe the HW and only the HW.

>> Also, wouldn't it be more pleasant to use another helper from the DMA core
>> that returns a proper return code? So we now which one among -EBUSY, -
>> ENODEV or -EPROBE_DEFER we get?
>> 
>
> Agree.
> I will change to "dma_request_chan_by_mask" instead of "dma_request_channel "
> so it can return a proper error code. 
>   
> 		cdns_ctrl->dmac = dma_request_chan_by_mask(&mask);
> 		if (IS_ERR(cdns_ctrl->dmac)) {
> 			ret = PTR_ERR(cdns_ctrl->dmac);
> 			if (ret != -EPROBE_DEFER)
> 				dev_err(cdns_ctrl->dev,
> 					"Failed to get a DMA channel:%d\n",ret);
> 			goto disable_irq;
> 		}
>
> Is this reasonable?

It is better, but maybe you can use dev_err_probe() instead to include
the EPROBE_DEFER error handling.

Thanks,
Miquèl

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

RE: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob when DMA is not ready

[Rabara, Niravkumar L]

Hi Miquel,

> >> Does it work if there is no DMA channel provided? The bindings do not
> >> mention DMA channels as mandatory.
> >>
> >
> > The way Cadence NAND controller driver is written in such a way that
> > it uses
> > has_dma=1 as hardcoded value, indicating that slave DMA interface is
> > connected to DMA engine. However, it does not utilize the dedicated
> > DMA channel information from the device tree.
> 
> This is not ok.
> 
> > Driver works without external DMA interface i.e. has_dma=0.
> > However current driver does not have a mechanism to configure it from
> > device tree.
> 
> What? Why are you requesting a DMA channel from a dmaengine in this case?
> 
> Please make the distinction between the OS implementation (the driver) and
> the DT binding which describe the HW and only the HW.
> 

Let me clarify from bindings(hw) and driver prospective. 

Bindings :-
Cadence NAND controller HW has MMIO registers, so called slave DMA interface
for page programming or page read. 
        reg = <0x10b80000 0x10000>,
              <0x10840000 0x10000>;
        reg-names = "reg", "sdma"; // sdma =  Slave DMA data port register set

It appears that dt bindings has captured sdma interface correctly.   

Linux Driver:-
Driver can read these sdma registers directly or it can use the DMA.
Existing driver code has hardcoded has_dma with an assumption that
an external DMA is always used and relies on DMA API for data transfer. 
Thant is why it requires to use DMA channel from dmaengine. 

In my previous reply, I tried to describe this driver scenario but maybe I mixed up. 
has_dma=0, i.e. accessing sdma register without using dmaengine is also working.
However, currently there is no option in driver to choose between using dmaengine and
direct register access.



> > 		cdns_ctrl->dmac = dma_request_chan_by_mask(&mask);
> > 		if (IS_ERR(cdns_ctrl->dmac)) {
> > 			ret = PTR_ERR(cdns_ctrl->dmac);
> > 			if (ret != -EPROBE_DEFER)
> > 				dev_err(cdns_ctrl->dev,
> > 					"Failed to get a DMA
> channel:%d\n",ret);
> > 			goto disable_irq;
> > 		}
> >
> > Is this reasonable?
> 
> It is better, but maybe you can use dev_err_probe() instead to include the
> EPROBE_DEFER error handling.
> 

Got it. I will update the code as below. 

		cdns_ctrl->dmac = dma_request_chan_by_mask(&mask);
		if (IS_ERR(cdns_ctrl->dmac)) {
			ret = dev_err_probe(cdns_ctrl->dev, PTR_ERR(cdns_ctrl->dmac),
				    "%d: Failed to get a DMA channel\n",ret);		
			goto disable_irq;
		}

Thanks,
Nirav

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

Re: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob when DMA is not ready

[Miquel Raynal]

Hello,

>> > Driver works without external DMA interface i.e. has_dma=0.
>> > However current driver does not have a mechanism to configure it from
>> > device tree.
>> 
>> What? Why are you requesting a DMA channel from a dmaengine in this case?
>> 
>> Please make the distinction between the OS implementation (the driver) and
>> the DT binding which describe the HW and only the HW.
>> 
>
> Let me clarify from bindings(hw) and driver prospective. 
>
> Bindings :-
> Cadence NAND controller HW has MMIO registers, so called slave DMA interface
> for page programming or page read. 
>         reg = <0x10b80000 0x10000>,
>               <0x10840000 0x10000>;
>         reg-names = "reg", "sdma"; // sdma =  Slave DMA data port register set
>
> It appears that dt bindings has captured sdma interface correctly.

Slave DMA is very confusing because in Linux we make the distinction
between:
1- external DMA (generic DMA controller) driven
   through the dmaengine API, through which we interact using the so
   called slave API
2- peripheral DMA (DMA controller embedded in the NAND IP) when there is
   no "external/generic" engine. In this case we control DMA transfers
   using the registers of the NAND controller (or a nearby range, in
   this case), the same driver handles both the NAND and the DMA part.

You used the wording Slave DMA (#1), but it feels like you are talking
about the other (#2). Can you please confirm in which case we are?

> Linux Driver:-
> Driver can read these sdma registers directly or it can use the DMA.
> Existing driver code has hardcoded has_dma with an assumption that
> an external DMA is always used and relies on DMA API for data
> transfer.

I am sorry but DMA API does not mean much. There are 3 APIs:
- dma-mapping, for the buffers and the coherency
- dmaengine, used in case #1 only, to drive the external DMA controllers
- dma-buf to share buffers between areas in the kernel (out of scope)

> Thant is why it requires to use DMA channel from dmaengine.

If I understand it right, no :-)

Either you have an external DMA controller (#2) or an internal one (#1)
but in this second case there is no DMA channel request nor any
engine-related API. Of course you need to use the dma-mapping API for
the buffers.

> In my previous reply, I tried to describe this driver scenario but maybe I mixed up. 
> has_dma=0, i.e. accessing sdma register without using dmaengine is
> also working.

But do you have an external DMA engine in the end? Or is it specific to
the NAND controller?

> However, currently there is no option in driver to choose between using dmaengine and
> direct register access.
>

Thanks,
Miquèl

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

RE: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob when DMA is not ready

[Rabara, Niravkumar L]

Hi Miquel,

> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: Thursday, 30 January, 2025 11:20 PM
> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>
> Cc: Richard Weinberger <richard@nod.at>; Vignesh Raghavendra
> <vigneshr@ti.com>; linux@treblig.org; Shen Lichuan
> <shenlichuan@vivo.com>; Jinjie Ruan <ruanjinjie@huawei.com>; u.kleine-
> koenig@baylibre.com; linux-mtd@lists.infradead.org; linux-
> kernel@vger.kernel.org; stable@vger.kernel.org
> Subject: Re: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob
> when DMA is not ready
> 
> Hello,
> 
> >> > Driver works without external DMA interface i.e. has_dma=0.
> >> > However current driver does not have a mechanism to configure it
> >> > from device tree.
> >>
> >> What? Why are you requesting a DMA channel from a dmaengine in this
> case?
> >>
> >> Please make the distinction between the OS implementation (the
> >> driver) and the DT binding which describe the HW and only the HW.
> >>
> >
> > Let me clarify from bindings(hw) and driver prospective.
> >
> > Bindings :-
> > Cadence NAND controller HW has MMIO registers, so called slave DMA
> > interface for page programming or page read.
> >         reg = <0x10b80000 0x10000>,
> >               <0x10840000 0x10000>;
> >         reg-names = "reg", "sdma"; // sdma =  Slave DMA data port
> > register set
> >
> > It appears that dt bindings has captured sdma interface correctly.
> 
> Slave DMA is very confusing because in Linux we make the distinction
> between:
> 1- external DMA (generic DMA controller) driven
>    through the dmaengine API, through which we interact using the so
>    called slave API
> 2- peripheral DMA (DMA controller embedded in the NAND IP) when there is
>    no "external/generic" engine. In this case we control DMA transfers
>    using the registers of the NAND controller (or a nearby range, in
>    this case), the same driver handles both the NAND and the DMA part.
> 
> You used the wording Slave DMA (#1), but it feels like you are talking about
> the other (#2). Can you please confirm in which case we are?
>

My apologies for the confusion.
Slave DMA terminology used in cadence nand controller bindings and
driver is indeed confusing.  

To answer your question it is, 
1 - External DMA (Generic DMA controller).

Nand controller IP do not have embedded DMA controller (2 - peripheral DMA). 

FYR, how external DMA is used. 
https://elixir.bootlin.com/linux/v6.13.1/source/drivers/mtd/nand/raw/cadence-nand-controller.c#L1962

 
> > Linux Driver:-
> > Driver can read these sdma registers directly or it can use the DMA.
> > Existing driver code has hardcoded has_dma with an assumption that an
> > external DMA is always used and relies on DMA API for data transfer.
> 
> I am sorry but DMA API does not mean much. There are 3 APIs:
> - dma-mapping, for the buffers and the coherency
> - dmaengine, used in case #1 only, to drive the external DMA controllers
> - dma-buf to share buffers between areas in the kernel (out of scope)
> 
> > Thant is why it requires to use DMA channel from dmaengine.
> 
> If I understand it right, no :-)
> 
> Either you have an external DMA controller (#2) or an internal one (#1) but in
> this second case there is no DMA channel request nor any engine-related
> API. Of course you need to use the dma-mapping API for the buffers.
> 
> > In my previous reply, I tried to describe this driver scenario but maybe I
> mixed up.
> > has_dma=0, i.e. accessing sdma register without using dmaengine is
> > also working.
> 
> But do you have an external DMA engine in the end? Or is it specific to the
> NAND controller?
> 

Yes I am using external DMA engine. 

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

Re: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob when DMA is not ready

[Miquel Raynal]

Hello,

> My apologies for the confusion.
> Slave DMA terminology used in cadence nand controller bindings and
> driver is indeed confusing.  
>
> To answer your question it is, 
> 1 - External DMA (Generic DMA controller).
>
> Nand controller IP do not have embedded DMA controller (2 - peripheral DMA). 
>
> FYR, how external DMA is used. 
> https://elixir.bootlin.com/linux/v6.13.1/source/drivers/mtd/nand/raw/cadence-nand-controller.c#L1962

In this case we should have a dmas property (and perhaps dma-names), no?

Miquèl

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

RE: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob when DMA is not ready

[Rabara, Niravkumar L]

Hi Miquel,

> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: Tuesday, 4 February, 2025 5:20 PM
> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>
> Cc: Richard Weinberger <richard@nod.at>; Vignesh Raghavendra
> <vigneshr@ti.com>; linux@treblig.org; Shen Lichuan <shenlichuan@vivo.com>;
> Jinjie Ruan <ruanjinjie@huawei.com>; u.kleine-koenig@baylibre.com; linux-
> mtd@lists.infradead.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org
> Subject: Re: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob when
> DMA is not ready
> 
> Hello,
> 
> > My apologies for the confusion.
> > Slave DMA terminology used in cadence nand controller bindings and
> > driver is indeed confusing.
> >
> > To answer your question it is,
> > 1 - External DMA (Generic DMA controller).
> >
> > Nand controller IP do not have embedded DMA controller (2 - peripheral
> DMA).
> >
> > FYR, how external DMA is used.
> > https://elixir.bootlin.com/linux/v6.13.1/source/drivers/mtd/nand/raw/c
> > adence-nand-controller.c#L1962
> 
> In this case we should have a dmas property (and perhaps dma-names), no?
> 
No, I believe.
Cadence NAND controller IP do not have dedicated handshake interface to connect
with DMA controller.
My understanding is dmas (and dma-names) are only used for the dedicated handshake
interface between peripheral and the DMA controller.

Thanks,
Nirav

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

Re: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob when DMA is not ready

[Miquel Raynal]

On 04/02/2025 at 10:43:20 GMT, "Rabara, Niravkumar L" <niravkumar.l.rabara@intel.com> wrote:

> Hi Miquel,
>
>> -----Original Message-----
>> From: Miquel Raynal <miquel.raynal@bootlin.com>
>> Sent: Tuesday, 4 February, 2025 5:20 PM
>> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>
>> Cc: Richard Weinberger <richard@nod.at>; Vignesh Raghavendra
>> <vigneshr@ti.com>; linux@treblig.org; Shen Lichuan <shenlichuan@vivo.com>;
>> Jinjie Ruan <ruanjinjie@huawei.com>; u.kleine-koenig@baylibre.com; linux-
>> mtd@lists.infradead.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org
>> Subject: Re: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob when
>> DMA is not ready
>> 
>> Hello,
>> 
>> > My apologies for the confusion.
>> > Slave DMA terminology used in cadence nand controller bindings and
>> > driver is indeed confusing.
>> >
>> > To answer your question it is,
>> > 1 - External DMA (Generic DMA controller).
>> >
>> > Nand controller IP do not have embedded DMA controller (2 - peripheral
>> DMA).
>> >
>> > FYR, how external DMA is used.
>> > https://elixir.bootlin.com/linux/v6.13.1/source/drivers/mtd/nand/raw/c
>> > adence-nand-controller.c#L1962
>> 
>> In this case we should have a dmas property (and perhaps dma-names), no?
>> 
> No, I believe.
> Cadence NAND controller IP do not have dedicated handshake interface to connect
> with DMA controller.
> My understanding is dmas (and dma-names) are only used for the dedicated handshake
> interface between peripheral and the DMA controller.

I don't see well how you can defer if there is no resource to grab. And
if there is a resource to grab, why is it not described anywhere?

Thanks,
Miquèl

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

RE: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob when DMA is not ready

[Rabara, Niravkumar L]

Hi Miquel,

> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: Tuesday, 4 February, 2025 9:33 PM
> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>
> Cc: Richard Weinberger <richard@nod.at>; Vignesh Raghavendra
> <vigneshr@ti.com>; linux@treblig.org; Shen Lichuan <shenlichuan@vivo.com>;
> Jinjie Ruan <ruanjinjie@huawei.com>; u.kleine-koenig@baylibre.com; linux-
> mtd@lists.infradead.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org
> Subject: Re: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob when
> DMA is not ready
> 
> On 04/02/2025 at 10:43:20 GMT, "Rabara, Niravkumar L"
> <niravkumar.l.rabara@intel.com> wrote:
> >> Hello,
> >>
> >> > My apologies for the confusion.
> >> > Slave DMA terminology used in cadence nand controller bindings and
> >> > driver is indeed confusing.
> >> >
> >> > To answer your question it is,
> >> > 1 - External DMA (Generic DMA controller).
> >> >
> >> > Nand controller IP do not have embedded DMA controller (2 -
> >> > peripheral
> >> DMA).
> >> >
> >> > FYR, how external DMA is used.
> >> > https://elixir.bootlin.com/linux/v6.13.1/source/drivers/mtd/nand/ra
> >> > w/c
> >> > adence-nand-controller.c#L1962
> >>
> >> In this case we should have a dmas property (and perhaps dma-names), no?
> >>
> > No, I believe.
> > Cadence NAND controller IP do not have dedicated handshake interface
> > to connect with DMA controller.
> > My understanding is dmas (and dma-names) are only used for the
> > dedicated handshake interface between peripheral and the DMA controller.
> 
> I don't see well how you can defer if there is no resource to grab. And if there is
> a resource to grab, why is it not described anywhere?
> 

Since NAND controller do not have handshake interface with DMA controller.
Driver is using external DMA for memory-to-memory copy. 

Your point is since the driver is using external DMA and it should be described in bindings? 

Thanks,
Nirav





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

Re: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob when DMA is not ready

[Miquel Raynal]

Hello,

>> >> > My apologies for the confusion.
>> >> > Slave DMA terminology used in cadence nand controller bindings and
>> >> > driver is indeed confusing.
>> >> >
>> >> > To answer your question it is,
>> >> > 1 - External DMA (Generic DMA controller).
>> >> >
>> >> > Nand controller IP do not have embedded DMA controller (2 -
>> >> > peripheral
>> >> DMA).
>> >> >
>> >> > FYR, how external DMA is used.
>> >> > https://elixir.bootlin.com/linux/v6.13.1/source/drivers/mtd/nand/ra
>> >> > w/c
>> >> > adence-nand-controller.c#L1962
>> >>
>> >> In this case we should have a dmas property (and perhaps dma-names), no?
>> >>
>> > No, I believe.
>> > Cadence NAND controller IP do not have dedicated handshake interface
>> > to connect with DMA controller.
>> > My understanding is dmas (and dma-names) are only used for the
>> > dedicated handshake interface between peripheral and the DMA controller.
>> 
>> I don't see well how you can defer if there is no resource to grab. And if there is
>> a resource to grab, why is it not described anywhere?
>> 
>
> Since NAND controller do not have handshake interface with DMA controller.
> Driver is using external DMA for memory-to-memory copy.

I'm sorry you lost me again. What do you mean handshake? There is no
request line? There is no way the NAND controller can trigger DMA
transfers?

What do you mean mem-to-mem, how is this useful to the controller?

> Your point is since the driver is using external DMA and it should be
> described in bindings?

Yes. But maybe I still don't get it correctly.

Thanks,
Miquèl

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

RE: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob when DMA is not ready

[Rabara, Niravkumar L]

Hi Miquel,

> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: Friday, 7 February, 2025 1:02 AM
> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>
> Cc: Richard Weinberger <richard@nod.at>; Vignesh Raghavendra
> <vigneshr@ti.com>; linux@treblig.org; Shen Lichuan
> <shenlichuan@vivo.com>; Jinjie Ruan <ruanjinjie@huawei.com>; u.kleine-
> koenig@baylibre.com; linux-mtd@lists.infradead.org; linux-
> kernel@vger.kernel.org; stable@vger.kernel.org
> Subject: Re: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob
> when DMA is not ready
> 
> Hello,
> 
> >> >> > My apologies for the confusion.
> >> >> > Slave DMA terminology used in cadence nand controller bindings
> >> >> > and driver is indeed confusing.
> >> >> >
> >> >> > To answer your question it is,
> >> >> > 1 - External DMA (Generic DMA controller).
> >> >> >
> >> >> > Nand controller IP do not have embedded DMA controller (2 -
> >> >> > peripheral
> >> >> DMA).
> >> >> >
> >> >> > FYR, how external DMA is used.
> >> >> > https://elixir.bootlin.com/linux/v6.13.1/source/drivers/mtd/nand
> >> >> > /ra
> >> >> > w/c
> >> >> > adence-nand-controller.c#L1962
> >> >>
> >> >> In this case we should have a dmas property (and perhaps dma-names),
> no?
> >> >>
> >> > No, I believe.
> >> > Cadence NAND controller IP do not have dedicated handshake
> >> > interface to connect with DMA controller.
> >> > My understanding is dmas (and dma-names) are only used for the
> >> > dedicated handshake interface between peripheral and the DMA
> controller.
> >>
> >> I don't see well how you can defer if there is no resource to grab.
> >> And if there is a resource to grab, why is it not described anywhere?
> >>
> >
> > Since NAND controller do not have handshake interface with DMA
> controller.
> > Driver is using external DMA for memory-to-memory copy.
> 
> I'm sorry you lost me again. What do you mean handshake? There is no
> request line? There is no way the NAND controller can trigger DMA transfers?
> 
Yes, I mean there is no request line, so there is no way the NAND controller can
trigger DMA transfer.

Sorry I used the terminology based on Synopsys DesignWare AXI DMA Controller
that is used with Agilex5 SoCFPGA platform.  
https://github.com/torvalds/linux/blob/v6.14-rc1/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c#L1372

> What do you mean mem-to-mem, how is this useful to the controller?
> 
I mean system memory to/from NAND MMIO register address for page
 read/write data transfer. 

	reg = <0x10b80000 0x10000>, 
		<0x10840000 0x1000>; <--- This MMIO address block
	reg-names = "reg", "sdma";

> > Your point is since the driver is using external DMA and it should be
> > described in bindings?
> 
> Yes. But maybe I still don't get it correctly.
> 
dmas is an optional property in cadence nand controller bindings. 
https://github.com/torvalds/linux/blob/v6.14-rc1/Documentation/devicetree/bindings/mtd/cdns%2Chp-nfc.yaml#L36
Does it need to change to required property in bindings? 
 
Please let me know if you have any suggestion/advise.

Thanks,
Nirav


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

Re: [PATCH v2 1/3] mtd: rawnand: cadence: support deferred prob when DMA is not ready

[Miquel Raynal]

>> >> >> > My apologies for the confusion.
>> >> >> > Slave DMA terminology used in cadence nand controller bindings
>> >> >> > and driver is indeed confusing.
>> >> >> >
>> >> >> > To answer your question it is,
>> >> >> > 1 - External DMA (Generic DMA controller).
>> >> >> >
>> >> >> > Nand controller IP do not have embedded DMA controller (2 -
>> >> >> > peripheral
>> >> >> DMA).
>> >> >> >
>> >> >> > FYR, how external DMA is used.
>> >> >> > https://elixir.bootlin.com/linux/v6.13.1/source/drivers/mtd/nand
>> >> >> > /ra
>> >> >> > w/c
>> >> >> > adence-nand-controller.c#L1962
>> >> >>
>> >> >> In this case we should have a dmas property (and perhaps dma-names),
>> no?
>> >> >>
>> >> > No, I believe.
>> >> > Cadence NAND controller IP do not have dedicated handshake
>> >> > interface to connect with DMA controller.
>> >> > My understanding is dmas (and dma-names) are only used for the
>> >> > dedicated handshake interface between peripheral and the DMA
>> controller.
>> >>
>> >> I don't see well how you can defer if there is no resource to grab.
>> >> And if there is a resource to grab, why is it not described anywhere?
>> >>
>> >
>> > Since NAND controller do not have handshake interface with DMA
>> controller.
>> > Driver is using external DMA for memory-to-memory copy.
>> 
>> I'm sorry you lost me again. What do you mean handshake? There is no
>> request line? There is no way the NAND controller can trigger DMA transfers?
>> 
> Yes, I mean there is no request line, so there is no way the NAND controller can
> trigger DMA transfer.
>
> Sorry I used the terminology based on Synopsys DesignWare AXI DMA Controller
> that is used with Agilex5 SoCFPGA platform.  
> https://github.com/torvalds/linux/blob/v6.14-rc1/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c#L1372
>
>> What do you mean mem-to-mem, how is this useful to the controller?
>> 
> I mean system memory to/from NAND MMIO register address for page
>  read/write data transfer. 
>
> 	reg = <0x10b80000 0x10000>, 
> 		<0x10840000 0x1000>; <--- This MMIO address block
> 	reg-names = "reg", "sdma";
>
>> > Your point is since the driver is using external DMA and it should be
>> > described in bindings?
>> 
>> Yes. But maybe I still don't get it correctly.
>> 
> dmas is an optional property in cadence nand controller bindings. 
> https://github.com/torvalds/linux/blob/v6.14-rc1/Documentation/devicetree/bindings/mtd/cdns%2Chp-nfc.yaml#L36
> Does it need to change to required property in bindings?

On one side you have a dedicated MMIO region, which imply we should have
an external DMA engine that is probably generic. On the other side it
feels like only the NAND controller uses it and it should be pictured as
a peripheral DMA controller and in this case we should not use the DMA
engine API at all. Your case is something in between, I don't like it
much. Anyway, we cannot break bindings, so please respin the series
because I totally lost your initial target.

Miquèl

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

[PATCH v2 2/3] mtd: rawnand: cadence: use dma_map_resource for sdma address

[niravkumar.l.rabara]

From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>

Map the slave DMA I/O address using dma_map_resource.
When ARM SMMU is enabled, using a direct physical address of SDMA results
in DMA transaction failure.

Fixes: ec4ba01e894d ("mtd: rawnand: Add new Cadence NAND driver to MTD subsystem")
Cc: stable@vger.kernel.org
Signed-off-by: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
---
 .../mtd/nand/raw/cadence-nand-controller.c    | 29 ++++++++++++++++---
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/cadence-nand-controller.c b/drivers/mtd/nand/raw/cadence-nand-controller.c
index 5e27f5546f1b..8281151cf869 100644
--- a/drivers/mtd/nand/raw/cadence-nand-controller.c
+++ b/drivers/mtd/nand/raw/cadence-nand-controller.c
@@ -471,6 +471,8 @@ struct cdns_nand_ctrl {
 	struct {
 		void __iomem *virt;
 		dma_addr_t dma;
+		dma_addr_t iova_dma;
+		u32 size;
 	} io;
 
 	int irq;
@@ -1835,11 +1837,11 @@ static int cadence_nand_slave_dma_transfer(struct cdns_nand_ctrl *cdns_ctrl,
 	}
 
 	if (dir == DMA_FROM_DEVICE) {
-		src_dma = cdns_ctrl->io.dma;
+		src_dma = cdns_ctrl->io.iova_dma;
 		dst_dma = buf_dma;
 	} else {
 		src_dma = buf_dma;
-		dst_dma = cdns_ctrl->io.dma;
+		dst_dma = cdns_ctrl->io.iova_dma;
 	}
 
 	tx = dmaengine_prep_dma_memcpy(cdns_ctrl->dmac, dst_dma, src_dma, len,
@@ -2869,6 +2871,7 @@ cadence_nand_irq_cleanup(int irqnum, struct cdns_nand_ctrl *cdns_ctrl)
 static int cadence_nand_init(struct cdns_nand_ctrl *cdns_ctrl)
 {
 	dma_cap_mask_t mask;
+	struct dma_device *dma_dev = cdns_ctrl->dmac->device;
 	int ret;
 
 	cdns_ctrl->cdma_desc = dma_alloc_coherent(cdns_ctrl->dev,
@@ -2913,6 +2916,16 @@ static int cadence_nand_init(struct cdns_nand_ctrl *cdns_ctrl)
 		}
 	}
 
+	cdns_ctrl->io.iova_dma = dma_map_resource(dma_dev->dev, cdns_ctrl->io.dma,
+						  cdns_ctrl->io.size,
+						  DMA_BIDIRECTIONAL, 0);
+
+	ret = dma_mapping_error(dma_dev->dev, cdns_ctrl->io.iova_dma);
+	if (ret) {
+		dev_err(cdns_ctrl->dev, "Failed to map I/O resource to DMA\n");
+		goto dma_release_chnl;
+	}
+
 	nand_controller_init(&cdns_ctrl->controller);
 	INIT_LIST_HEAD(&cdns_ctrl->chips);
 
@@ -2923,18 +2936,22 @@ static int cadence_nand_init(struct cdns_nand_ctrl *cdns_ctrl)
 	if (ret) {
 		dev_err(cdns_ctrl->dev, "Failed to register MTD: %d\n",
 			ret);
-		goto dma_release_chnl;
+		goto unmap_dma_resource;
 	}
 
 	kfree(cdns_ctrl->buf);
 	cdns_ctrl->buf = kzalloc(cdns_ctrl->buf_size, GFP_KERNEL);
 	if (!cdns_ctrl->buf) {
 		ret = -ENOMEM;
-		goto dma_release_chnl;
+		goto unmap_dma_resource;
 	}
 
 	return 0;
 
+unmap_dma_resource:
+	dma_unmap_resource(dma_dev->dev, cdns_ctrl->io.iova_dma,
+			   cdns_ctrl->io.size, DMA_BIDIRECTIONAL, 0);
+
 dma_release_chnl:
 	if (cdns_ctrl->dmac)
 		dma_release_channel(cdns_ctrl->dmac);
@@ -2956,6 +2973,8 @@ static int cadence_nand_init(struct cdns_nand_ctrl *cdns_ctrl)
 static void cadence_nand_remove(struct cdns_nand_ctrl *cdns_ctrl)
 {
 	cadence_nand_chips_cleanup(cdns_ctrl);
+	dma_unmap_resource(cdns_ctrl->dmac->device->dev, cdns_ctrl->io.iova_dma,
+			   cdns_ctrl->io.size, DMA_BIDIRECTIONAL, 0);
 	cadence_nand_irq_cleanup(cdns_ctrl->irq, cdns_ctrl);
 	kfree(cdns_ctrl->buf);
 	dma_free_coherent(cdns_ctrl->dev, sizeof(struct cadence_nand_cdma_desc),
@@ -3020,7 +3039,9 @@ static int cadence_nand_dt_probe(struct platform_device *ofdev)
 	cdns_ctrl->io.virt = devm_platform_get_and_ioremap_resource(ofdev, 1, &res);
 	if (IS_ERR(cdns_ctrl->io.virt))
 		return PTR_ERR(cdns_ctrl->io.virt);
+
 	cdns_ctrl->io.dma = res->start;
+	cdns_ctrl->io.size = resource_size(res);
 
 	dt->clk = devm_clk_get(cdns_ctrl->dev, "nf_clk");
 	if (IS_ERR(dt->clk))
-- 
2.25.1


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

Re: [PATCH v2 2/3] mtd: rawnand: cadence: use dma_map_resource for sdma address

[Miquel Raynal]

Hello,

On 16/01/2025 at 11:21:53 +08, niravkumar.l.rabara@intel.com wrote:

> From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
>
> Map the slave DMA I/O address using dma_map_resource.
> When ARM SMMU is enabled, using a direct physical address of SDMA results
> in DMA transaction failure.

It is in general a better practice anyway. Drivers should be portable
and always remap resources.

> Fixes: ec4ba01e894d ("mtd: rawnand: Add new Cadence NAND driver to MTD subsystem")
> Cc: stable@vger.kernel.org
> Signed-off-by: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>

Thanks,
Miquèl

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

RE: [PATCH v2 2/3] mtd: rawnand: cadence: use dma_map_resource for sdma address

[Rabara, Niravkumar L]

Hi Miquel,

> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: Tuesday, 21 January, 2025 5:53 PM
> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>
> Cc: Richard Weinberger <richard@nod.at>; Vignesh Raghavendra
> <vigneshr@ti.com>; linux@treblig.org; Shen Lichuan
> <shenlichuan@vivo.com>; Jinjie Ruan <ruanjinjie@huawei.com>; u.kleine-
> koenig@baylibre.com; linux-mtd@lists.infradead.org; linux-
> kernel@vger.kernel.org; stable@vger.kernel.org
> Subject: Re: [PATCH v2 2/3] mtd: rawnand: cadence: use dma_map_resource
> for sdma address
> 
> Hello,
> 
> On 16/01/2025 at 11:21:53 +08, niravkumar.l.rabara@intel.com wrote:
> 
> > From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
> >
> > Map the slave DMA I/O address using dma_map_resource.
> > When ARM SMMU is enabled, using a direct physical address of SDMA
> > results in DMA transaction failure.
> 
> It is in general a better practice anyway. Drivers should be portable and
> always remap resources.
> 

Do you think the commit message below would be better, or 
stick with the existing one?

Remap the slave DMA I/O resources to enhance driver portability.
Using a physical address causes DMA translation failure when the
ARM SMMU is enabled.

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

Re: [PATCH v2 2/3] mtd: rawnand: cadence: use dma_map_resource for sdma address

[Miquel Raynal]

Hello,

>> > Map the slave DMA I/O address using dma_map_resource.
>> > When ARM SMMU is enabled, using a direct physical address of SDMA
>> > results in DMA transaction failure.
>> 
>> It is in general a better practice anyway. Drivers should be portable and
>> always remap resources.

I actually had a look at the kernel sources again regarding the use of
the map_resource() helper, and it is very strangely used. Sometimes the
DMA controller does the remapping, sometimes it is the slave device. The
core and headers are totally unclear about who should take the action.
Anyway, your diff is fine I believe.

> Do you think the commit message below would be better, or 
> stick with the existing one?
>
> Remap the slave DMA I/O resources to enhance driver portability.
> Using a physical address causes DMA translation failure when the
> ARM SMMU is enabled.

Fine by me!

Thanks,
Miquèl

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

[PATCH v2 3/3] mtd: rawnand: cadence: fix incorrect dev context in dma_unmap_single

[niravkumar.l.rabara]

From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>

dma_map_single is using dma_dev->dev, however dma_unmap_single
is using cdns_ctrl->dev, which is incorrect.
Used the correct device context dma_dev->dev for dma_unmap_single.

Fixes: ec4ba01e894d ("mtd: rawnand: Add new Cadence NAND driver to MTD subsystem")
Cc: stable@vger.kernel.org
Signed-off-by: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
---
 drivers/mtd/nand/raw/cadence-nand-controller.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/cadence-nand-controller.c b/drivers/mtd/nand/raw/cadence-nand-controller.c
index 8281151cf869..2d50eeb902ac 100644
--- a/drivers/mtd/nand/raw/cadence-nand-controller.c
+++ b/drivers/mtd/nand/raw/cadence-nand-controller.c
@@ -1863,12 +1863,12 @@ static int cadence_nand_slave_dma_transfer(struct cdns_nand_ctrl *cdns_ctrl,
 	dma_async_issue_pending(cdns_ctrl->dmac);
 	wait_for_completion(&finished);
 
-	dma_unmap_single(cdns_ctrl->dev, buf_dma, len, dir);
+	dma_unmap_single(dma_dev->dev, buf_dma, len, dir);
 
 	return 0;
 
 err_unmap:
-	dma_unmap_single(cdns_ctrl->dev, buf_dma, len, dir);
+	dma_unmap_single(dma_dev->dev, buf_dma, len, dir);
 
 err:
 	dev_dbg(cdns_ctrl->dev, "Fall back to CPU I/O\n");
-- 
2.25.1


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

Re: [PATCH v2 3/3] mtd: rawnand: cadence: fix incorrect dev context in dma_unmap_single

[Miquel Raynal]

Hello,

On 16/01/2025 at 11:21:54 +08, niravkumar.l.rabara@intel.com wrote:

> From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
>
> dma_map_single is using dma_dev->dev, however dma_unmap_single
> is using cdns_ctrl->dev, which is incorrect.
> Used the correct device context dma_dev->dev for dma_unmap_single.

I guess on is the physical/bus device and the other the framework
device? It would be nice to clarify this in the commit log.

> Fixes: ec4ba01e894d ("mtd: rawnand: Add new Cadence NAND driver to MTD subsystem")
> Cc: stable@vger.kernel.org
> Signed-off-by: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>

Thanks,
Miquèl

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

RE: [PATCH v2 3/3] mtd: rawnand: cadence: fix incorrect dev context in dma_unmap_single

[Rabara, Niravkumar L]

Hi Miquel,

> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: Tuesday, 21 January, 2025 5:55 PM
> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>
> Cc: Richard Weinberger <richard@nod.at>; Vignesh Raghavendra
> <vigneshr@ti.com>; linux@treblig.org; Shen Lichuan
> <shenlichuan@vivo.com>; Jinjie Ruan <ruanjinjie@huawei.com>; u.kleine-
> koenig@baylibre.com; linux-mtd@lists.infradead.org; linux-
> kernel@vger.kernel.org; stable@vger.kernel.org
> Subject: Re: [PATCH v2 3/3] mtd: rawnand: cadence: fix incorrect dev context
> in dma_unmap_single
> 
> Hello,
> 
> On 16/01/2025 at 11:21:54 +08, niravkumar.l.rabara@intel.com wrote:
> 
> > From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
> >
> > dma_map_single is using dma_dev->dev, however dma_unmap_single is
> > using cdns_ctrl->dev, which is incorrect.
> > Used the correct device context dma_dev->dev for dma_unmap_single.
> 
> I guess on is the physical/bus device and the other the framework device? It
> would be nice to clarify this in the commit log.
> 

Noted. Is the commit message below acceptable? 

dma_map_single is using physical/bus device (DMA) but dma_unmap_single
is using framework device(NAND controller), which is incorrect.
Fixed dma_unmap_single to use correct physical/bus device.

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

Re: [PATCH v2 3/3] mtd: rawnand: cadence: fix incorrect dev context in dma_unmap_single

[Miquel Raynal]

Hello,

>> > dma_map_single is using dma_dev->dev, however dma_unmap_single is
>> > using cdns_ctrl->dev, which is incorrect.
>> > Used the correct device context dma_dev->dev for dma_unmap_single.
>> 
>> I guess on is the physical/bus device and the other the framework device? It
>> would be nice to clarify this in the commit log.
>> 
>
> Noted. Is the commit message below acceptable? 
>
> dma_map_single is using physical/bus device (DMA) but dma_unmap_single
> is using framework device(NAND controller), which is incorrect.
> Fixed dma_unmap_single to use correct physical/bus device.

Ok for me.

Thanks,
Miquèl

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