Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1354838 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v3 1/6] memory: omap-gpmc: Store handle to GPMC dev Franklin S Cooper Jr <fcooper@ti.com> - 2016-03-10 05:10 +0100
    Re: [PATCH v3 1/6] memory: omap-gpmc: Store handle to GPMC dev Roger Quadros <rogerq@ti.com> - 2016-03-10 13:50 +0100
      Re: [PATCH v3 1/6] memory: omap-gpmc: Store handle to GPMC dev "Franklin S Cooper Jr." <fcooper@ti.com> - 2016-03-10 14:30 +0100
      Re: [PATCH v3 1/6] memory: omap-gpmc: Store handle to GPMC dev "Franklin S Cooper Jr." <fcooper@ti.com> - 2016-03-10 14:50 +0100
        Re: [PATCH v3 1/6] memory: omap-gpmc: Store handle to GPMC dev "Franklin S Cooper Jr." <fcooper@ti.com> - 2016-03-10 16:30 +0100
        Re: [PATCH v3 1/6] memory: omap-gpmc: Store handle to GPMC dev Roger Quadros <rogerq@ti.com> - 2016-03-10 16:30 +0100

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

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

[toc] | [next] | [standalone]

#1355092

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

[toc] | [prev] | [next] | [standalone]

#1355115

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

[toc] | [prev] | [next] | [standalone]

#1355127

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

[toc] | [prev] | [next] | [standalone]

#1355200

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

[toc] | [prev] | [next] | [standalone]

#1355201

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web