[PATCH] nand: ranw: qcom_nand: stop using phys_to_dma()

[PATCH] nand: ranw: qcom_nand: stop using phys_to_dma()

[Arnd Bergmann]

Compile-testing this driver on x86 caused a link error:

ERROR: "__phys_to_dma" [drivers/mtd/nand/raw/qcom_nandc.ko] undefined!

The problem here is that the driver attempts to convert the physical
address into the DMA controller as a dma_addr_t and calls phys_to_dma()
to do the conversion.

However, there is no generic way to convert a phys_addr_t into a dma_addr_t
for anything other than RAM (which should use the dma-mapping API instead).
The only correct use of phys_to_dma() instead is inside of the dma-mapping
implementation.

In all other drivers that deal with DMA FIFO addresses, we just pass the
physical address directly and have the DMA controller deal with that
if necessary, so let's do the same thing here.

Fixes: c76b78d8ec05 ("mtd: nand: Qualcomm NAND controller driver")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/mtd/nand/raw/qcom_nandc.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 994f980c6d86..f047e2819041 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -338,7 +338,6 @@ struct nandc_regs {
  * @dev:			parent device
  * @base:			MMIO base
  * @base_phys:			physical base address of controller registers
- * @base_dma:			dma base address of controller registers
  * @core_clk:			controller clock
  * @aon_clk:			another controller clock
  *
@@ -372,7 +371,6 @@ struct qcom_nand_controller {
 
 	void __iomem *base;
 	phys_addr_t base_phys;
-	dma_addr_t base_dma;
 
 	struct clk *core_clk;
 	struct clk *aon_clk;
@@ -935,11 +933,11 @@ static int prep_adm_dma_desc(struct qcom_nand_controller *nandc, bool read,
 	slave_conf.device_fc = flow_control;
 	if (read) {
 		slave_conf.src_maxburst = 16;
-		slave_conf.src_addr = nandc->base_dma + reg_off;
+		slave_conf.src_addr = (dma_addr_t)nandc->base_phys + reg_off;
 		slave_conf.slave_id = nandc->data_crci;
 	} else {
 		slave_conf.dst_maxburst = 16;
-		slave_conf.dst_addr = nandc->base_dma + reg_off;
+		slave_conf.dst_addr = (dma_addr_t)nandc->base_phys + reg_off;
 		slave_conf.slave_id = nandc->cmd_crci;
 	}
 
@@ -2963,7 +2961,6 @@ static int qcom_nandc_probe(struct platform_device *pdev)
 		return PTR_ERR(nandc->base);
 
 	nandc->base_phys = res->start;
-	nandc->base_dma = phys_to_dma(dev, (phys_addr_t)res->start);
 
 	nandc->core_clk = devm_clk_get(dev, "core");
 	if (IS_ERR(nandc->core_clk))
-- 
2.9.0

Re: [PATCH] nand: ranw: qcom_nand: stop using phys_to_dma()

[Boris Brezillon]

On Wed, 11 Jul 2018 14:26:58 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> Compile-testing this driver on x86 caused a link error:
> 
> ERROR: "__phys_to_dma" [drivers/mtd/nand/raw/qcom_nandc.ko] undefined!
> 
> The problem here is that the driver attempts to convert the physical
> address into the DMA controller as a dma_addr_t and calls phys_to_dma()
> to do the conversion.
> 
> However, there is no generic way to convert a phys_addr_t into a dma_addr_t
> for anything other than RAM (which should use the dma-mapping API instead).
> The only correct use of phys_to_dma() instead is inside of the dma-mapping
> implementation.

Should we use dma_map_resource() to do the phys_addr_t to dma_addr_t
conversion?

> 
> In all other drivers that deal with DMA FIFO addresses, we just pass the
> physical address directly and have the DMA controller deal with that
> if necessary, so let's do the same thing here.
> 
> Fixes: c76b78d8ec05 ("mtd: nand: Qualcomm NAND controller driver")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/mtd/nand/raw/qcom_nandc.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index 994f980c6d86..f047e2819041 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -338,7 +338,6 @@ struct nandc_regs {
>   * @dev:			parent device
>   * @base:			MMIO base
>   * @base_phys:			physical base address of controller registers
> - * @base_dma:			dma base address of controller registers
>   * @core_clk:			controller clock
>   * @aon_clk:			another controller clock
>   *
> @@ -372,7 +371,6 @@ struct qcom_nand_controller {
>  
>  	void __iomem *base;
>  	phys_addr_t base_phys;
> -	dma_addr_t base_dma;
>  
>  	struct clk *core_clk;
>  	struct clk *aon_clk;
> @@ -935,11 +933,11 @@ static int prep_adm_dma_desc(struct qcom_nand_controller *nandc, bool read,
>  	slave_conf.device_fc = flow_control;
>  	if (read) {
>  		slave_conf.src_maxburst = 16;
> -		slave_conf.src_addr = nandc->base_dma + reg_off;
> +		slave_conf.src_addr = (dma_addr_t)nandc->base_phys + reg_off;
>  		slave_conf.slave_id = nandc->data_crci;
>  	} else {
>  		slave_conf.dst_maxburst = 16;
> -		slave_conf.dst_addr = nandc->base_dma + reg_off;
> +		slave_conf.dst_addr = (dma_addr_t)nandc->base_phys + reg_off;
>  		slave_conf.slave_id = nandc->cmd_crci;
>  	}
>  
> @@ -2963,7 +2961,6 @@ static int qcom_nandc_probe(struct platform_device *pdev)
>  		return PTR_ERR(nandc->base);
>  
>  	nandc->base_phys = res->start;
> -	nandc->base_dma = phys_to_dma(dev, (phys_addr_t)res->start);
>  
>  	nandc->core_clk = devm_clk_get(dev, "core");
>  	if (IS_ERR(nandc->core_clk))

Re: [PATCH] nand: ranw: qcom_nand: stop using phys_to_dma()

[Arnd Bergmann]

On Wed, Jul 11, 2018 at 3:04 PM, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> On Wed, 11 Jul 2018 14:26:58 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
>
>> Compile-testing this driver on x86 caused a link error:
>>
>> ERROR: "__phys_to_dma" [drivers/mtd/nand/raw/qcom_nandc.ko] undefined!
>>
>> The problem here is that the driver attempts to convert the physical
>> address into the DMA controller as a dma_addr_t and calls phys_to_dma()
>> to do the conversion.
>>
>> However, there is no generic way to convert a phys_addr_t into a dma_addr_t
>> for anything other than RAM (which should use the dma-mapping API instead).
>> The only correct use of phys_to_dma() instead is inside of the dma-mapping
>> implementation.
>
> Should we use dma_map_resource() to do the phys_addr_t to dma_addr_t
> conversion?

I had not seen that interface before, but yes, that does seem like the best
way to do it here.

       Arnd

Re: [PATCH] nand: ranw: qcom_nand: stop using phys_to_dma()

[Boris Brezillon]

Hi Arnd,

On Wed, 11 Jul 2018 16:26:31 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Wed, Jul 11, 2018 at 3:04 PM, Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> > On Wed, 11 Jul 2018 14:26:58 +0200
> > Arnd Bergmann <arnd@arndb.de> wrote:
> >  
> >> Compile-testing this driver on x86 caused a link error:
> >>
> >> ERROR: "__phys_to_dma" [drivers/mtd/nand/raw/qcom_nandc.ko] undefined!
> >>
> >> The problem here is that the driver attempts to convert the physical
> >> address into the DMA controller as a dma_addr_t and calls phys_to_dma()
> >> to do the conversion.
> >>
> >> However, there is no generic way to convert a phys_addr_t into a dma_addr_t
> >> for anything other than RAM (which should use the dma-mapping API instead).
> >> The only correct use of phys_to_dma() instead is inside of the dma-mapping
> >> implementation.  
> >
> > Should we use dma_map_resource() to do the phys_addr_t to dma_addr_t
> > conversion?  
> 
> I had not seen that interface before, but yes, that does seem like the best
> way to do it here.

Do you plan to send a new version using dma_map_resource()? BTW,
there's typo in the subject prefix (s/ranw/rawnand/).

Regards,

Boris

Re: [PATCH] nand: ranw: qcom_nand: stop using phys_to_dma()

[Arnd Bergmann]

On Tue, Jul 17, 2018 at 10:01 PM, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> Hi Arnd,
>
> On Wed, 11 Jul 2018 16:26:31 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
>
>> On Wed, Jul 11, 2018 at 3:04 PM, Boris Brezillon
>> <boris.brezillon@bootlin.com> wrote:
>> > On Wed, 11 Jul 2018 14:26:58 +0200
>> > Arnd Bergmann <arnd@arndb.de> wrote:
>> >
>> >> Compile-testing this driver on x86 caused a link error:
>> >>
>> >> ERROR: "__phys_to_dma" [drivers/mtd/nand/raw/qcom_nandc.ko] undefined!
>> >>
>> >> The problem here is that the driver attempts to convert the physical
>> >> address into the DMA controller as a dma_addr_t and calls phys_to_dma()
>> >> to do the conversion.
>> >>
>> >> However, there is no generic way to convert a phys_addr_t into a dma_addr_t
>> >> for anything other than RAM (which should use the dma-mapping API instead).
>> >> The only correct use of phys_to_dma() instead is inside of the dma-mapping
>> >> implementation.
>> >
>> > Should we use dma_map_resource() to do the phys_addr_t to dma_addr_t
>> > conversion?
>>
>> I had not seen that interface before, but yes, that does seem like the best
>> way to do it here.
>
> Do you plan to send a new version using dma_map_resource()?

I somehow thought you were going to do that yourself, but have now
posted a new version of the patch the way you suggested.

> BTW, there's typo in the subject prefix (s/ranw/rawnand/).

Hmm, missed that again in v2. Can you fix it up when applying?

     Arnd

Re: [PATCH] nand: ranw: qcom_nand: stop using phys_to_dma()

[Boris Brezillon]

On Tue, 17 Jul 2018 22:29:58 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Tue, Jul 17, 2018 at 10:01 PM, Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> > Hi Arnd,
> >
> > On Wed, 11 Jul 2018 16:26:31 +0200
> > Arnd Bergmann <arnd@arndb.de> wrote:
> >  
> >> On Wed, Jul 11, 2018 at 3:04 PM, Boris Brezillon
> >> <boris.brezillon@bootlin.com> wrote:  
> >> > On Wed, 11 Jul 2018 14:26:58 +0200
> >> > Arnd Bergmann <arnd@arndb.de> wrote:
> >> >  
> >> >> Compile-testing this driver on x86 caused a link error:
> >> >>
> >> >> ERROR: "__phys_to_dma" [drivers/mtd/nand/raw/qcom_nandc.ko] undefined!
> >> >>
> >> >> The problem here is that the driver attempts to convert the physical
> >> >> address into the DMA controller as a dma_addr_t and calls phys_to_dma()
> >> >> to do the conversion.
> >> >>
> >> >> However, there is no generic way to convert a phys_addr_t into a dma_addr_t
> >> >> for anything other than RAM (which should use the dma-mapping API instead).
> >> >> The only correct use of phys_to_dma() instead is inside of the dma-mapping
> >> >> implementation.  
> >> >
> >> > Should we use dma_map_resource() to do the phys_addr_t to dma_addr_t
> >> > conversion?  
> >>
> >> I had not seen that interface before, but yes, that does seem like the best
> >> way to do it here.  
> >
> > Do you plan to send a new version using dma_map_resource()?  
> 
> I somehow thought you were going to do that yourself, but have now
> posted a new version of the patch the way you suggested.

I tend not to do that, unless I'm explicitly asked to :-).

> 
> > BTW, there's typo in the subject prefix (s/ranw/rawnand/).  
> 
> Hmm, missed that again in v2. Can you fix it up when applying?

Sure.