[PATCH v3 0/6] mtd: nand: Fix support for NAND DMA prefetch

[PATCH v3 0/6] mtd: nand: Fix support for NAND DMA prefetch

[Franklin S Cooper Jr]

NAND DMA prefetch has been broken for awhile and seems to have only
worked for SDMA based devices

This patchset fixes DMA prefetch to work on both EDMA and SDMA devices

Test on:
am335x gp evm
am437x gp evm
am37x gp evm

This rev is pretty much the same as v2 which was blocked due to
dependencies to Roger's update GPMC/NAND rework.

This updated rev removes this dependency.

Also it fixes an issue that was introduced when the eDMA driver was
recently updated.

Links to rev 2 patchset:
https://patchwork.kernel.org/patch/7408691/
https://patchwork.kernel.org/patch/7408681/
https://patchwork.kernel.org/patch/7408661/
https://patchwork.kernel.org/patch/7408641/
https://patchwork.kernel.org/patch/7408621/

Franklin S Cooper Jr (6):
  memory: omap-gpmc: Store handle to GPMC dev
  ARM: dts: am437x/am33xx: Fix GPMC dma properties
  mtd: nand: omap2: Support parsing dma channel information from DT
  mtd: nand: omap2: Start dma request before enabling prefetch
  mtd: nand: omap2: Fix high memory dma prefetch transfer
  ARM: OMAP2+: Update GPMC and NAND DT binding documentation

 Documentation/devicetree/bindings/bus/ti-gpmc.txt  |  7 +++++-
 .../devicetree/bindings/mtd/gpmc-nand.txt          |  2 +-
 arch/arm/boot/dts/am33xx.dtsi                      |  2 +-
 arch/arm/boot/dts/am4372.dtsi                      |  2 +-
 drivers/memory/omap-gpmc.c                         |  1 +
 drivers/mtd/nand/omap2.c                           | 27 +++++++++-------------
 include/linux/platform_data/mtd-nand-omap2.h       |  2 ++
 7 files changed, 23 insertions(+), 20 deletions(-)

-- 
2.7.0

[PATCH v3 1/6] memory: omap-gpmc: Store handle to GPMC dev

[Franklin S Cooper Jr]

The dma channel information is located within the GPMC node. The NAND
driver requires a handle to the GPMC's dev to properly parse the DMA
properties. Therefore, store a handle to the dev so it can be referenced
within the NAND driver.

Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
 drivers/memory/omap-gpmc.c                   | 1 +
 include/linux/platform_data/mtd-nand-omap2.h | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index 6515dfc..2932d13 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -1796,6 +1796,7 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
 
 	gpmc_nand_data->cs = val;
 	gpmc_nand_data->of_node = child;
+	gpmc_nand_data->gpmc_dev = &pdev->dev;
 
 	/* Detect availability of ELM module */
 	gpmc_nand_data->elm_of_node = of_parse_phandle(child, "ti,elm-id", 0);
diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
index 090bbab..534b984 100644
--- a/include/linux/platform_data/mtd-nand-omap2.h
+++ b/include/linux/platform_data/mtd-nand-omap2.h
@@ -80,5 +80,7 @@ struct omap_nand_platform_data {
 	/* for passing the partitions */
 	struct device_node	*of_node;
 	struct device_node	*elm_of_node;
+
+	struct device		*gpmc_dev;
 };
 #endif
-- 
2.7.0

[PATCH v3 2/6] ARM: dts: am437x/am33xx: Fix GPMC dma properties

[Franklin S Cooper Jr]

Recent patch series that updated the eDMA driver also updated the eDMA
bindings.

The following patches forgot to update the DMA bindings for the GPMC node.
ARM: DTS: am33xx: Use the new DT bindings for the eDMA3
ARM: DTS: am437x: Use the new DT bindings for the eDMA3

This patch corrects this so NAND with DMA prefetch can work.

Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
 arch/arm/boot/dts/am33xx.dtsi | 2 +-
 arch/arm/boot/dts/am4372.dtsi | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 1fafaad..97471d6 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -860,7 +860,7 @@
 			ti,no-idle-on-init;
 			reg = <0x50000000 0x2000>;
 			interrupts = <100>;
-			dmas = <&edma 52>;
+			dmas = <&edma 52 0>;
 			dma-names = "rxtx";
 			gpmc,num-cs = <7>;
 			gpmc,num-waitpins = <2>;
diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index 92068fb..2878b04 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -884,7 +884,7 @@
 		gpmc: gpmc@50000000 {
 			compatible = "ti,am3352-gpmc";
 			ti,hwmods = "gpmc";
-			dmas = <&edma 52>;
+			dmas = <&edma 52 0>;
 			dma-names = "rxtx";
 			clocks = <&l3s_gclk>;
 			clock-names = "fck";
-- 
2.7.0

[PATCH v3 3/6] mtd: nand: omap2: Support parsing dma channel information from DT

[Franklin S Cooper Jr]

Switch from dma_request_channel to allow passing dma channel
information from DT rather than hardcoding a value.

Also provide a handle to the GPMC's dev so it can be used to parse the DMA
channel information within the GPMC's DT node.

Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
Version 3 changes:
Instead of passing parent dev use the new property

 drivers/mtd/nand/omap2.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index c553f78..1e5c803 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1731,7 +1731,9 @@ static int omap_nand_probe(struct platform_device *pdev)
 		dma_cap_zero(mask);
 		dma_cap_set(DMA_SLAVE, mask);
 		sig = OMAP24XX_DMA_GPMC;
-		info->dma = dma_request_channel(mask, omap_dma_filter_fn, &sig);
+		info->dma = dma_request_slave_channel_compat(mask,
+			omap_dma_filter_fn, &sig, pdata->gpmc_dev, "rxtx");
+
 		if (!info->dma) {
 			dev_err(&pdev->dev, "DMA engine request failed\n");
 			err = -ENXIO;
-- 
2.7.0

[PATCH v3 4/6] mtd: nand: omap2: Start dma request before enabling prefetch

[Franklin S Cooper Jr]

The prefetch engine sends a dma request once a FIFO threshold has
been met. No other requests are received until the previous request
is handled.

Starting an edma transfer (dma_async_issue_pending) results in any
previous event for the dma channel to be cleared. Therefore, starting
the prefetch engine before initiating the dma transfer may result in
the prefetch triggering a dma request but instead of it being handled
it can end up being cleared. This will result in a hang since the code
will continue to wait for the dma request to complete.

By initiating the dma request before enabling the prefetch engine this
race condition is avoided and no dma request are missed/cleared.

Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
 drivers/mtd/nand/omap2.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 1e5c803..f7110d1 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -497,6 +497,11 @@ static inline int omap_nand_dma_transfer(struct mtd_info *mtd, void *addr,
 	tx->callback_param = &info->comp;
 	dmaengine_submit(tx);
 
+	init_completion(&info->comp);
+
+	/* setup and start DMA using dma_addr */
+	dma_async_issue_pending(info->dma);
+
 	/*  configure and start prefetch transfer */
 	ret = omap_prefetch_enable(info->gpmc_cs,
 		PREFETCH_FIFOTHRESHOLD_MAX, 0x1, len, is_write, info);
@@ -504,10 +509,6 @@ static inline int omap_nand_dma_transfer(struct mtd_info *mtd, void *addr,
 		/* PFPW engine is busy, use cpu copy method */
 		goto out_copy_unmap;
 
-	init_completion(&info->comp);
-	dma_async_issue_pending(info->dma);
-
-	/* setup and start DMA using dma_addr */
 	wait_for_completion(&info->comp);
 	tim = 0;
 	limit = (loops_per_jiffy * msecs_to_jiffies(OMAP_NAND_TIMEOUT_MS));
-- 
2.7.0

[PATCH v3 5/6] mtd: nand: omap2: Fix high memory dma prefetch transfer

[Franklin S Cooper Jr]

Based on DMA documentation and testing using high memory buffer when
doing dma transfers can lead to various issues including kernel
panics.

To workaround this simply use cpu copy. The amount of high memory
buffers used are very uncommon so no noticeable performance hit should
be seen.

Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
 drivers/mtd/nand/omap2.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index f7110d1..a174376 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -467,17 +467,8 @@ static inline int omap_nand_dma_transfer(struct mtd_info *mtd, void *addr,
 	int ret;
 	u32 val;
 
-	if (addr >= high_memory) {
-		struct page *p1;
-
-		if (((size_t)addr & PAGE_MASK) !=
-			((size_t)(addr + len - 1) & PAGE_MASK))
-			goto out_copy;
-		p1 = vmalloc_to_page(addr);
-		if (!p1)
-			goto out_copy;
-		addr = page_address(p1) + ((size_t)addr & ~PAGE_MASK);
-	}
+	if (addr >= high_memory)
+		goto out_copy;
 
 	sg_init_one(&sg, addr, len);
 	n = dma_map_sg(info->dma->device->dev, &sg, 1, dir);
@@ -534,6 +525,7 @@ out_copy:
 	else
 		is_write == 0 ? omap_read_buf8(mtd, (u_char *) addr, len)
 			: omap_write_buf8(mtd, (u_char *) addr, len);
+
 	return 0;
 }
 
-- 
2.7.0

[PATCH v3 6/6] ARM: OMAP2+: Update GPMC and NAND DT binding documentation

[Franklin S Cooper Jr]

Add additional details to the GPMC NAND documentation to clarify
what is needed to enable NAND DMA prefetch.

Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
Version 3 changes:
Slight tweaks to the wording

 Documentation/devicetree/bindings/bus/ti-gpmc.txt   | 7 ++++++-
 Documentation/devicetree/bindings/mtd/gpmc-nand.txt | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/bus/ti-gpmc.txt b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
index 704be93..ae6388a 100644
--- a/Documentation/devicetree/bindings/bus/ti-gpmc.txt
+++ b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
@@ -33,6 +33,10 @@ Required properties:
 			As this will change in the future, filling correct
 			values here is a requirement.
 
+Required properties when using NAND prefetch dma:
+ - dmas			GPMC NAND prefetch dma channel
+ - dma-names		Must be set to "rxtx"
+
 Timing properties for child nodes. All are optional and default to 0.
 
  - gpmc,sync-clk-ps:	Minimum clock period for synchronous mode, in picoseconds
@@ -119,7 +123,8 @@ Example for an AM33xx board:
 		ti,hwmods = "gpmc";
 		reg = <0x50000000 0x2000>;
 		interrupts = <100>;
-
+		dmas = <&edma 52 0>;
+		dma-names = "rxtx";
 		gpmc,num-cs = <8>;
 		gpmc,num-waitpins = <2>;
 		#address-cells = <2>;
diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
index fb733c4..7cb9dc56 100644
--- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
@@ -35,7 +35,7 @@ Optional properties:
 
 		"prefetch-polled"	Prefetch polled mode (default)
 		"polled"		Polled mode, without prefetch
-		"prefetch-dma"		Prefetch enabled sDMA mode
+		"prefetch-dma"		Prefetch enabled DMA mode
 		"prefetch-irq"		Prefetch enabled irq mode
 
  - elm_id:	<deprecated> use "ti,elm-id" instead
-- 
2.7.0

Re: [PATCH v3 1/6] memory: omap-gpmc: Store handle to GPMC dev

[Roger Quadros]

Franklin,

On 10/03/16 06:07, Franklin S Cooper Jr wrote:
> The dma channel information is located within the GPMC node. The NAND
> driver requires a handle to the GPMC's dev to properly parse the DMA
> properties. Therefore, store a handle to the dev so it can be referenced
> within the NAND driver.
> 
> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
> ---
>  drivers/memory/omap-gpmc.c                   | 1 +
>  include/linux/platform_data/mtd-nand-omap2.h | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
> index 6515dfc..2932d13 100644
> --- a/drivers/memory/omap-gpmc.c
> +++ b/drivers/memory/omap-gpmc.c
> @@ -1796,6 +1796,7 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
>  
>  	gpmc_nand_data->cs = val;
>  	gpmc_nand_data->of_node = child;
> +	gpmc_nand_data->gpmc_dev = &pdev->dev;
>  
>  	/* Detect availability of ELM module */
>  	gpmc_nand_data->elm_of_node = of_parse_phandle(child, "ti,elm-id", 0);
> diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
> index 090bbab..534b984 100644
> --- a/include/linux/platform_data/mtd-nand-omap2.h
> +++ b/include/linux/platform_data/mtd-nand-omap2.h
> @@ -80,5 +80,7 @@ struct omap_nand_platform_data {
>  	/* for passing the partitions */
>  	struct device_node	*of_node;
>  	struct device_node	*elm_of_node;
> +
> +	struct device		*gpmc_dev;
>  };
>  #endif
> 

Why do you need this? Can't we just use dev->parent in the omap2-nand driver?

cheers,
-roger

Re: [PATCH v3 2/6] ARM: dts: am437x/am33xx: Fix GPMC dma properties

[Roger Quadros]

On 10/03/16 06:07, Franklin S Cooper Jr wrote:
> Recent patch series that updated the eDMA driver also updated the eDMA
> bindings.
> 
> The following patches forgot to update the DMA bindings for the GPMC node.
> ARM: DTS: am33xx: Use the new DT bindings for the eDMA3
> ARM: DTS: am437x: Use the new DT bindings for the eDMA3

Instead of mentioning the patch subject here use the Fixes: tag.

> 
> This patch corrects this so NAND with DMA prefetch can work.
> 
> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>

Please split this patch in to am33xx.dtsi and am4372.dtsi

> ---
>  arch/arm/boot/dts/am33xx.dtsi | 2 +-
>  arch/arm/boot/dts/am4372.dtsi | 2 +-

what about dm816x.dtsi and dm814x.dtsi?

I suppose sdma based SoCs are not affected like this right?

cheers,
-roger

>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index 1fafaad..97471d6 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -860,7 +860,7 @@
>  			ti,no-idle-on-init;
>  			reg = <0x50000000 0x2000>;
>  			interrupts = <100>;
> -			dmas = <&edma 52>;
> +			dmas = <&edma 52 0>;
>  			dma-names = "rxtx";
>  			gpmc,num-cs = <7>;
>  			gpmc,num-waitpins = <2>;
> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
> index 92068fb..2878b04 100644
> --- a/arch/arm/boot/dts/am4372.dtsi
> +++ b/arch/arm/boot/dts/am4372.dtsi
> @@ -884,7 +884,7 @@
>  		gpmc: gpmc@50000000 {
>  			compatible = "ti,am3352-gpmc";
>  			ti,hwmods = "gpmc";
> -			dmas = <&edma 52>;
> +			dmas = <&edma 52 0>;
>  			dma-names = "rxtx";
>  			clocks = <&l3s_gclk>;
>  			clock-names = "fck";
> 

Re: [PATCH v3 1/6] memory: omap-gpmc: Store handle to GPMC dev

[Franklin S Cooper Jr.]

On 03/10/2016 06:40 AM, Roger Quadros wrote:
> Franklin,
>
> On 10/03/16 06:07, Franklin S Cooper Jr wrote:
>> The dma channel information is located within the GPMC node. The NAND
>> driver requires a handle to the GPMC's dev to properly parse the DMA
>> properties. Therefore, store a handle to the dev so it can be referenced
>> within the NAND driver.
>>
>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>> ---
>>  drivers/memory/omap-gpmc.c                   | 1 +
>>  include/linux/platform_data/mtd-nand-omap2.h | 2 ++
>>  2 files changed, 3 insertions(+)
>>
>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
>> index 6515dfc..2932d13 100644
>> --- a/drivers/memory/omap-gpmc.c
>> +++ b/drivers/memory/omap-gpmc.c
>> @@ -1796,6 +1796,7 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
>>  
>>  	gpmc_nand_data->cs = val;
>>  	gpmc_nand_data->of_node = child;
>> +	gpmc_nand_data->gpmc_dev = &pdev->dev;
>>  
>>  	/* Detect availability of ELM module */
>>  	gpmc_nand_data->elm_of_node = of_parse_phandle(child, "ti,elm-id", 0);
>> diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
>> index 090bbab..534b984 100644
>> --- a/include/linux/platform_data/mtd-nand-omap2.h
>> +++ b/include/linux/platform_data/mtd-nand-omap2.h
>> @@ -80,5 +80,7 @@ struct omap_nand_platform_data {
>>  	/* for passing the partitions */
>>  	struct device_node	*of_node;
>>  	struct device_node	*elm_of_node;
>> +
>> +	struct device		*gpmc_dev;
>>  };
>>  #endif
>>
> Why do you need this? Can't we just use dev->parent in the omap2-nand driver?

Dev->parent doesn't point to omap-gpmc's dev. Its because
platform_device_alloc is used to create the platform_device
so a proper parent isn't defined. It should be possible to
force omap2-nand dev's parent property to the GPMC's dev but
that didn't seem right when I thought about doing it.
>
> cheers,
> -roger

Re: [PATCH v3 2/6] ARM: dts: am437x/am33xx: Fix GPMC dma properties

[Franklin S Cooper Jr.]

On 03/10/2016 06:51 AM, Roger Quadros wrote:
> On 10/03/16 06:07, Franklin S Cooper Jr wrote:
>> Recent patch series that updated the eDMA driver also updated the eDMA
>> bindings.
>>
>> The following patches forgot to update the DMA bindings for the GPMC node.
>> ARM: DTS: am33xx: Use the new DT bindings for the eDMA3
>> ARM: DTS: am437x: Use the new DT bindings for the eDMA3
> Instead of mentioning the patch subject here use the Fixes: tag.

Ok.
>
>> This patch corrects this so NAND with DMA prefetch can work.
>>
>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
> Please split this patch in to am33xx.dtsi and am4372.dtsi

Ok
>
>> ---
>>  arch/arm/boot/dts/am33xx.dtsi | 2 +-
>>  arch/arm/boot/dts/am4372.dtsi | 2 +-
> what about dm816x.dtsi and dm814x.dtsi?
>
> I suppose sdma based SoCs are not affected like this right?

Correct. The updated driver was only eDMA. So sDMA bindings
have not been changed.
>
> cheers,
> -roger
>
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
>> index 1fafaad..97471d6 100644
>> --- a/arch/arm/boot/dts/am33xx.dtsi
>> +++ b/arch/arm/boot/dts/am33xx.dtsi
>> @@ -860,7 +860,7 @@
>>  			ti,no-idle-on-init;
>>  			reg = <0x50000000 0x2000>;
>>  			interrupts = <100>;
>> -			dmas = <&edma 52>;
>> +			dmas = <&edma 52 0>;
>>  			dma-names = "rxtx";
>>  			gpmc,num-cs = <7>;
>>  			gpmc,num-waitpins = <2>;
>> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
>> index 92068fb..2878b04 100644
>> --- a/arch/arm/boot/dts/am4372.dtsi
>> +++ b/arch/arm/boot/dts/am4372.dtsi
>> @@ -884,7 +884,7 @@
>>  		gpmc: gpmc@50000000 {
>>  			compatible = "ti,am3352-gpmc";
>>  			ti,hwmods = "gpmc";
>> -			dmas = <&edma 52>;
>> +			dmas = <&edma 52 0>;
>>  			dma-names = "rxtx";
>>  			clocks = <&l3s_gclk>;
>>  			clock-names = "fck";
>>

Re: [PATCH v3 1/6] memory: omap-gpmc: Store handle to GPMC dev

[Franklin S Cooper Jr.]

On 03/10/2016 06:40 AM, Roger Quadros wrote:
> Franklin,
>
> On 10/03/16 06:07, Franklin S Cooper Jr wrote:
>> The dma channel information is located within the GPMC node. The NAND
>> driver requires a handle to the GPMC's dev to properly parse the DMA
>> properties. Therefore, store a handle to the dev so it can be referenced
>> within the NAND driver.
>>
>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>> ---
>>  drivers/memory/omap-gpmc.c                   | 1 +
>>  include/linux/platform_data/mtd-nand-omap2.h | 2 ++
>>  2 files changed, 3 insertions(+)
>>
>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
>> index 6515dfc..2932d13 100644
>> --- a/drivers/memory/omap-gpmc.c
>> +++ b/drivers/memory/omap-gpmc.c
>> @@ -1796,6 +1796,7 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
>>  
>>  	gpmc_nand_data->cs = val;
>>  	gpmc_nand_data->of_node = child;
>> +	gpmc_nand_data->gpmc_dev = &pdev->dev;
>>  
>>  	/* Detect availability of ELM module */
>>  	gpmc_nand_data->elm_of_node = of_parse_phandle(child, "ti,elm-id", 0);
>> diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
>> index 090bbab..534b984 100644
>> --- a/include/linux/platform_data/mtd-nand-omap2.h
>> +++ b/include/linux/platform_data/mtd-nand-omap2.h
>> @@ -80,5 +80,7 @@ struct omap_nand_platform_data {
>>  	/* for passing the partitions */
>>  	struct device_node	*of_node;
>>  	struct device_node	*elm_of_node;
>> +
>> +	struct device		*gpmc_dev;
>>  };
>>  #endif
>>
> Why do you need this? Can't we just use dev->parent in the omap2-nand driver?

The omap2-nand platform_device is created by
platform_device_alloc. The created platform_device dev
parent isn't by default set to the GPMC dev. I didn't think
this was the right approach to manually set the parent
property. However, taking a look at other usages of
platform_device_alloc it seems this is actually pretty
common. If your ok with this then I can go that route.
>
> cheers,
> -roger

Re: [PATCH v3 1/6] memory: omap-gpmc: Store handle to GPMC dev

[Roger Quadros]

Franklin,

On 10/03/16 15:40, Franklin S Cooper Jr. wrote:
> 
> 
> On 03/10/2016 06:40 AM, Roger Quadros wrote:
>> Franklin,
>>
>> On 10/03/16 06:07, Franklin S Cooper Jr wrote:
>>> The dma channel information is located within the GPMC node. The NAND
>>> driver requires a handle to the GPMC's dev to properly parse the DMA
>>> properties. Therefore, store a handle to the dev so it can be referenced
>>> within the NAND driver.
>>>
>>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>>> ---
>>>  drivers/memory/omap-gpmc.c                   | 1 +
>>>  include/linux/platform_data/mtd-nand-omap2.h | 2 ++
>>>  2 files changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
>>> index 6515dfc..2932d13 100644
>>> --- a/drivers/memory/omap-gpmc.c
>>> +++ b/drivers/memory/omap-gpmc.c
>>> @@ -1796,6 +1796,7 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
>>>  
>>>  	gpmc_nand_data->cs = val;
>>>  	gpmc_nand_data->of_node = child;
>>> +	gpmc_nand_data->gpmc_dev = &pdev->dev;
>>>  
>>>  	/* Detect availability of ELM module */
>>>  	gpmc_nand_data->elm_of_node = of_parse_phandle(child, "ti,elm-id", 0);
>>> diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
>>> index 090bbab..534b984 100644
>>> --- a/include/linux/platform_data/mtd-nand-omap2.h
>>> +++ b/include/linux/platform_data/mtd-nand-omap2.h
>>> @@ -80,5 +80,7 @@ struct omap_nand_platform_data {
>>>  	/* for passing the partitions */
>>>  	struct device_node	*of_node;
>>>  	struct device_node	*elm_of_node;
>>> +
>>> +	struct device		*gpmc_dev;
>>>  };
>>>  #endif
>>>
>> Why do you need this? Can't we just use dev->parent in the omap2-nand driver?
> 
> The omap2-nand platform_device is created by
> platform_device_alloc. The created platform_device dev
> parent isn't by default set to the GPMC dev. I didn't think
> this was the right approach to manually set the parent
> property. However, taking a look at other usages of
> platform_device_alloc it seems this is actually pretty
> common. If your ok with this then I can go that route.

After my DT cleanup we no longer use platform_dev_alloc for DT cases.
As Nand node is child of GPMC node, omap2 nand's parent is guaranteed to be the
GPMC device.

But for legacy boot we still use gpmc_nand_init() which calls platform_device_add()
without setting pdev->parent and so it will set the parent to platform bus.

Maybe we could patch gpmc_nand_init() to set NAND's parent to GPMC device?

cheers,
-roger

Re: [PATCH v3 1/6] memory: omap-gpmc: Store handle to GPMC dev

[Franklin S Cooper Jr.]

On 03/10/2016 09:22 AM, Roger Quadros wrote:
> Franklin,
>
> On 10/03/16 15:40, Franklin S Cooper Jr. wrote:
>>
>> On 03/10/2016 06:40 AM, Roger Quadros wrote:
>>> Franklin,
>>>
>>> On 10/03/16 06:07, Franklin S Cooper Jr wrote:
>>>> The dma channel information is located within the GPMC node. The NAND
>>>> driver requires a handle to the GPMC's dev to properly parse the DMA
>>>> properties. Therefore, store a handle to the dev so it can be referenced
>>>> within the NAND driver.
>>>>
>>>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>>>> ---
>>>>  drivers/memory/omap-gpmc.c                   | 1 +
>>>>  include/linux/platform_data/mtd-nand-omap2.h | 2 ++
>>>>  2 files changed, 3 insertions(+)
>>>>
>>>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
>>>> index 6515dfc..2932d13 100644
>>>> --- a/drivers/memory/omap-gpmc.c
>>>> +++ b/drivers/memory/omap-gpmc.c
>>>> @@ -1796,6 +1796,7 @@ static int gpmc_probe_nand_child(struct platform_device *pdev,
>>>>  
>>>>  	gpmc_nand_data->cs = val;
>>>>  	gpmc_nand_data->of_node = child;
>>>> +	gpmc_nand_data->gpmc_dev = &pdev->dev;
>>>>  
>>>>  	/* Detect availability of ELM module */
>>>>  	gpmc_nand_data->elm_of_node = of_parse_phandle(child, "ti,elm-id", 0);
>>>> diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
>>>> index 090bbab..534b984 100644
>>>> --- a/include/linux/platform_data/mtd-nand-omap2.h
>>>> +++ b/include/linux/platform_data/mtd-nand-omap2.h
>>>> @@ -80,5 +80,7 @@ struct omap_nand_platform_data {
>>>>  	/* for passing the partitions */
>>>>  	struct device_node	*of_node;
>>>>  	struct device_node	*elm_of_node;
>>>> +
>>>> +	struct device		*gpmc_dev;
>>>>  };
>>>>  #endif
>>>>
>>> Why do you need this? Can't we just use dev->parent in the omap2-nand driver?
>> The omap2-nand platform_device is created by
>> platform_device_alloc. The created platform_device dev
>> parent isn't by default set to the GPMC dev. I didn't think
>> this was the right approach to manually set the parent
>> property. However, taking a look at other usages of
>> platform_device_alloc it seems this is actually pretty
>> common. If your ok with this then I can go that route.
> After my DT cleanup we no longer use platform_dev_alloc for DT cases.
> As Nand node is child of GPMC node, omap2 nand's parent is guaranteed to be the
> GPMC device.
>
> But for legacy boot we still use gpmc_nand_init() which calls platform_device_add()
> without setting pdev->parent and so it will set the parent to platform bus.
>
> Maybe we could patch gpmc_nand_init() to set NAND's parent to GPMC device?

Yup that was my plan. I'll send a rev 4 with this change and
also your comments on PATCH 2.
>
> cheers,
> -roger