[PATCH v3 0/4] memory: omap-gpmc: Add AM64 SoC support

[PATCH v3 0/4] memory: omap-gpmc: Add AM64 SoC support

[Roger Quadros]

Hi,

TI's AM64 SoC contains one GPMC module. Add driver support for it.

cheers,
-roger

Changelog:
v3
- use compatible match table for checking for NAND controller node in
GPMC driver.

v2
- update DT binding doc to make reg-names and power-domains property
required only for specific SoC.

Roger Quadros (4):
  dt-bindings: memory-controllers: ti,gpmc: Add compatible for AM64
  memory: omap-gpmc: Add support for GPMC on AM64 SoC
  memory: omap-gpmc: Use a compatible match table when checking for NAND
    controller
  arm64: arch_k3: Select GPMC device driver

 .../bindings/memory-controllers/ti,gpmc.yaml  | 23 ++++++++-
 arch/arm64/Kconfig.platforms                  |  1 +
 drivers/memory/omap-gpmc.c                    | 48 ++++++++++++++-----
 drivers/mtd/nand/raw/omap2.c                  |  2 +-
 include/linux/platform_data/mtd-nand-omap2.h  |  5 ++
 5 files changed, 66 insertions(+), 13 deletions(-)

-- 
2.17.1

[PATCH v3 1/4] dt-bindings: memory-controllers: ti,gpmc: Add compatible for AM64

[Roger Quadros]

AM64 SoC contains the GPMC module. Add compatible for it.

Newer SoCs don't necessarily map GPMC data region at the same place
as legacy SoCs. Add reg-names "data", to provide this information to
the device driver.

Signed-off-by: Roger Quadros <rogerq@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/memory-controllers/ti,gpmc.yaml  | 23 ++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
index 25b42d68f9b3..64dc9d398d9a 100644
--- a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
@@ -23,13 +23,20 @@ properties:
     items:
       - enum:
           - ti,am3352-gpmc
+          - ti,am64-gpmc
           - ti,omap2420-gpmc
           - ti,omap2430-gpmc
           - ti,omap3430-gpmc
           - ti,omap4430-gpmc
 
   reg:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
+
+  reg-names:
+    items:
+      - const: cfg
+      - const: data
 
   interrupts:
     maxItems: 1
@@ -44,6 +51,9 @@ properties:
     items:
       - const: fck
 
+  power-domains:
+    maxItems: 1
+
   dmas:
     items:
       - description: DMA channel for GPMC NAND prefetch
@@ -133,6 +143,17 @@ required:
   - "#address-cells"
   - "#size-cells"
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: ti,am64-gpmc
+    then:
+      required:
+        - reg-names
+        - power-domains
+
 additionalProperties: false
 
 examples:
-- 
2.17.1

[PATCH v3 2/4] memory: omap-gpmc: Add support for GPMC on AM64 SoC

[Roger Quadros]

The TI's AM64 SoC has the GPMC module. Add compatible for it.

Traditionally GPMC external addresses have always been mapped to first
1GB physical address. However newer platforms, can have it mapped
at different locations. Support this address provision via device tree.

Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
 drivers/memory/omap-gpmc.c | 40 ++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index be0858bff4d3..624153048182 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -237,6 +237,7 @@ struct gpmc_device {
 	struct omap3_gpmc_regs context;
 	int nirqs;
 	unsigned int is_suspended:1;
+	struct resource *data;
 };
 
 static struct irq_domain *gpmc_irq_domain;
@@ -1456,12 +1457,18 @@ static void gpmc_mem_exit(void)
 	}
 }
 
-static void gpmc_mem_init(void)
+static void gpmc_mem_init(struct gpmc_device *gpmc)
 {
 	int cs;
 
-	gpmc_mem_root.start = GPMC_MEM_START;
-	gpmc_mem_root.end = GPMC_MEM_END;
+	if (!gpmc->data) {
+		/* All legacy devices have same data IO window */
+		gpmc_mem_root.start = GPMC_MEM_START;
+		gpmc_mem_root.end = GPMC_MEM_END;
+	} else {
+		gpmc_mem_root.start = gpmc->data->start;
+		gpmc_mem_root.end = gpmc->data->end;
+	}
 
 	/* Reserve all regions that has been set up by bootloader */
 	for (cs = 0; cs < gpmc_cs_num; cs++) {
@@ -1888,6 +1895,7 @@ static const struct of_device_id gpmc_dt_ids[] = {
 	{ .compatible = "ti,omap3430-gpmc" },	/* omap3430 & omap3630 */
 	{ .compatible = "ti,omap4430-gpmc" },	/* omap4430 & omap4460 & omap543x */
 	{ .compatible = "ti,am3352-gpmc" },	/* am335x devices */
+	{ .compatible = "ti,am64-gpmc" },
 	{ }
 };
 
@@ -2502,13 +2510,25 @@ static int gpmc_probe(struct platform_device *pdev)
 	gpmc->dev = &pdev->dev;
 	platform_set_drvdata(pdev, gpmc);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
-		return -ENOENT;
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
+	if (!res) {
+		/* legacy DT */
+		gpmc_base = devm_platform_ioremap_resource(pdev, 0);
+		if (IS_ERR(gpmc_base))
+			return PTR_ERR(gpmc_base);
+	} else {
+		gpmc_base = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(gpmc_base))
+			return PTR_ERR(gpmc_base);
+
+		res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "data");
+		if (!res) {
+			dev_err(&pdev->dev, "couldn't get data reg resource\n");
+			return -ENOENT;
+		}
 
-	gpmc_base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(gpmc_base))
-		return PTR_ERR(gpmc_base);
+		gpmc->data = res;
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (!res) {
@@ -2562,7 +2582,7 @@ static int gpmc_probe(struct platform_device *pdev)
 	dev_info(gpmc->dev, "GPMC revision %d.%d\n", GPMC_REVISION_MAJOR(l),
 		 GPMC_REVISION_MINOR(l));
 
-	gpmc_mem_init();
+	gpmc_mem_init(gpmc);
 	rc = gpmc_gpio_init(gpmc);
 	if (rc)
 		goto gpio_init_failed;
-- 
2.17.1

[PATCH v3 3/4] memory: omap-gpmc: Use a compatible match table when checking for NAND controller

[Roger Quadros]

As more compatibles can be added to the GPMC NAND controller driver
use a compatible match table.

Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
 drivers/memory/omap-gpmc.c                   | 8 +++++++-
 drivers/mtd/nand/raw/omap2.c                 | 2 +-
 include/linux/platform_data/mtd-nand-omap2.h | 5 +++++
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index 624153048182..814ddb45c13d 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -2091,6 +2091,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
 	u32 val;
 	struct gpio_desc *waitpin_desc = NULL;
 	struct gpmc_device *gpmc = platform_get_drvdata(pdev);
+	bool is_nand = false;
 
 	if (of_property_read_u32(child, "reg", &cs) < 0) {
 		dev_err(&pdev->dev, "%pOF has no 'reg' property\n",
@@ -2183,7 +2184,12 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
 		}
 	}
 
-	if (of_device_is_compatible(child, "ti,omap2-nand")) {
+#if defined(CONFIG_MTD_NAND_OMAP2)
+	if (of_match_node(omap_nand_ids, child))
+		is_nand = true;
+#endif
+
+	if (is_nand) {
 		/* NAND specific setup */
 		val = 8;
 		of_property_read_u32(child, "nand-bus-width", &val);
diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c
index b26d4947af02..fff834ee726f 100644
--- a/drivers/mtd/nand/raw/omap2.c
+++ b/drivers/mtd/nand/raw/omap2.c
@@ -2352,7 +2352,7 @@ static int omap_nand_remove(struct platform_device *pdev)
 	return ret;
 }
 
-static const struct of_device_id omap_nand_ids[] = {
+const struct of_device_id omap_nand_ids[] = {
 	{ .compatible = "ti,omap2-nand", },
 	{},
 };
diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
index de6ada739121..e1bb90a8db03 100644
--- a/include/linux/platform_data/mtd-nand-omap2.h
+++ b/include/linux/platform_data/mtd-nand-omap2.h
@@ -61,4 +61,9 @@ struct gpmc_nand_regs {
 	void __iomem	*gpmc_bch_result5[GPMC_BCH_NUM_REMAINDER];
 	void __iomem	*gpmc_bch_result6[GPMC_BCH_NUM_REMAINDER];
 };
+
+#if defined(CONFIG_MTD_NAND_OMAP2)
+extern const struct of_device_id omap_nand_ids[];
+#endif
+
 #endif
-- 
2.17.1

Re: [PATCH v3 3/4] memory: omap-gpmc: Use a compatible match table when checking for NAND controller

[Krzysztof Kozlowski]

On 17/12/2021 11:29, Roger Quadros wrote:
> As more compatibles can be added to the GPMC NAND controller driver
> use a compatible match table.
> 
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
>  drivers/memory/omap-gpmc.c                   | 8 +++++++-
>  drivers/mtd/nand/raw/omap2.c                 | 2 +-
>  include/linux/platform_data/mtd-nand-omap2.h | 5 +++++
>  3 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
> index 624153048182..814ddb45c13d 100644
> --- a/drivers/memory/omap-gpmc.c
> +++ b/drivers/memory/omap-gpmc.c
> @@ -2091,6 +2091,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>  	u32 val;
>  	struct gpio_desc *waitpin_desc = NULL;
>  	struct gpmc_device *gpmc = platform_get_drvdata(pdev);
> +	bool is_nand = false;
>  
>  	if (of_property_read_u32(child, "reg", &cs) < 0) {
>  		dev_err(&pdev->dev, "%pOF has no 'reg' property\n",
> @@ -2183,7 +2184,12 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>  		}
>  	}
>  
> -	if (of_device_is_compatible(child, "ti,omap2-nand")) {
> +#if defined(CONFIG_MTD_NAND_OMAP2)

if (IS_ENABLED()) is preferred. If needed, you could make omap_nand_ids
symbol visible always (so without ifdef around it), because extern
structure should not have impact when not defined (if I recall
correctly...).

> +	if (of_match_node(omap_nand_ids, child))
> +		is_nand = true;
> +#endif
> +
> +	if (is_nand) {
>  		/* NAND specific setup */
>  		val = 8;
>  		of_property_read_u32(child, "nand-bus-width", &val);
> diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c
> index b26d4947af02..fff834ee726f 100644
> --- a/drivers/mtd/nand/raw/omap2.c
> +++ b/drivers/mtd/nand/raw/omap2.c
> @@ -2352,7 +2352,7 @@ static int omap_nand_remove(struct platform_device *pdev)
>  	return ret;
>  }
>  
> -static const struct of_device_id omap_nand_ids[] = {
> +const struct of_device_id omap_nand_ids[] = {
>  	{ .compatible = "ti,omap2-nand", },
>  	{},
>  };

I think OMAP2 NAND driver can be a module, so this should have
EXPORT_SYMBOL.

> diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
> index de6ada739121..e1bb90a8db03 100644
> --- a/include/linux/platform_data/mtd-nand-omap2.h
> +++ b/include/linux/platform_data/mtd-nand-omap2.h
> @@ -61,4 +61,9 @@ struct gpmc_nand_regs {
>  	void __iomem	*gpmc_bch_result5[GPMC_BCH_NUM_REMAINDER];
>  	void __iomem	*gpmc_bch_result6[GPMC_BCH_NUM_REMAINDER];
>  };
> +
> +#if defined(CONFIG_MTD_NAND_OMAP2)
> +extern const struct of_device_id omap_nand_ids[];
> +#endif
> +
>  #endif
> 


Best regards,
Krzysztof

Re: [PATCH v3 3/4] memory: omap-gpmc: Use a compatible match table when checking for NAND controller

[Roger Quadros]

On 17/12/2021 17:21, Krzysztof Kozlowski wrote:
> On 17/12/2021 11:29, Roger Quadros wrote:
>> As more compatibles can be added to the GPMC NAND controller driver
>> use a compatible match table.
>>
>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>> ---
>>  drivers/memory/omap-gpmc.c                   | 8 +++++++-
>>  drivers/mtd/nand/raw/omap2.c                 | 2 +-
>>  include/linux/platform_data/mtd-nand-omap2.h | 5 +++++
>>  3 files changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
>> index 624153048182..814ddb45c13d 100644
>> --- a/drivers/memory/omap-gpmc.c
>> +++ b/drivers/memory/omap-gpmc.c
>> @@ -2091,6 +2091,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>>  	u32 val;
>>  	struct gpio_desc *waitpin_desc = NULL;
>>  	struct gpmc_device *gpmc = platform_get_drvdata(pdev);
>> +	bool is_nand = false;
>>  
>>  	if (of_property_read_u32(child, "reg", &cs) < 0) {
>>  		dev_err(&pdev->dev, "%pOF has no 'reg' property\n",
>> @@ -2183,7 +2184,12 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>>  		}
>>  	}
>>  
>> -	if (of_device_is_compatible(child, "ti,omap2-nand")) {
>> +#if defined(CONFIG_MTD_NAND_OMAP2)
> 
> if (IS_ENABLED()) is preferred. If needed, you could make omap_nand_ids
> symbol visible always (so without ifdef around it), because extern
> structure should not have impact when not defined (if I recall
> correctly...).

OK.

> 
>> +	if (of_match_node(omap_nand_ids, child))
>> +		is_nand = true;
>> +#endif
>> +
>> +	if (is_nand) {
>>  		/* NAND specific setup */
>>  		val = 8;
>>  		of_property_read_u32(child, "nand-bus-width", &val);
>> diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c
>> index b26d4947af02..fff834ee726f 100644
>> --- a/drivers/mtd/nand/raw/omap2.c
>> +++ b/drivers/mtd/nand/raw/omap2.c
>> @@ -2352,7 +2352,7 @@ static int omap_nand_remove(struct platform_device *pdev)
>>  	return ret;
>>  }
>>  
>> -static const struct of_device_id omap_nand_ids[] = {
>> +const struct of_device_id omap_nand_ids[] = {
>>  	{ .compatible = "ti,omap2-nand", },
>>  	{},
>>  };
> 
> I think OMAP2 NAND driver can be a module, so this should have
> EXPORT_SYMBOL.

Indeed. Good catch!
> 
>> diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
>> index de6ada739121..e1bb90a8db03 100644
>> --- a/include/linux/platform_data/mtd-nand-omap2.h
>> +++ b/include/linux/platform_data/mtd-nand-omap2.h
>> @@ -61,4 +61,9 @@ struct gpmc_nand_regs {
>>  	void __iomem	*gpmc_bch_result5[GPMC_BCH_NUM_REMAINDER];
>>  	void __iomem	*gpmc_bch_result6[GPMC_BCH_NUM_REMAINDER];
>>  };
>> +
>> +#if defined(CONFIG_MTD_NAND_OMAP2)
>> +extern const struct of_device_id omap_nand_ids[];
>> +#endif
>> +
>>  #endif
>>
> 
> 
> Best regards,
> Krzysztof
> 

cheers,
-roger

Re: [PATCH v3 3/4] memory: omap-gpmc: Use a compatible match table when checking for NAND controller

[Roger Quadros]

On 17/12/2021 17:21, Krzysztof Kozlowski wrote:
> On 17/12/2021 11:29, Roger Quadros wrote:
>> As more compatibles can be added to the GPMC NAND controller driver
>> use a compatible match table.
>>
>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>> ---
>>  drivers/memory/omap-gpmc.c                   | 8 +++++++-
>>  drivers/mtd/nand/raw/omap2.c                 | 2 +-
>>  include/linux/platform_data/mtd-nand-omap2.h | 5 +++++
>>  3 files changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
>> index 624153048182..814ddb45c13d 100644
>> --- a/drivers/memory/omap-gpmc.c
>> +++ b/drivers/memory/omap-gpmc.c
>> @@ -2091,6 +2091,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>>  	u32 val;
>>  	struct gpio_desc *waitpin_desc = NULL;
>>  	struct gpmc_device *gpmc = platform_get_drvdata(pdev);
>> +	bool is_nand = false;
>>  
>>  	if (of_property_read_u32(child, "reg", &cs) < 0) {
>>  		dev_err(&pdev->dev, "%pOF has no 'reg' property\n",
>> @@ -2183,7 +2184,12 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>>  		}
>>  	}
>>  
>> -	if (of_device_is_compatible(child, "ti,omap2-nand")) {
>> +#if defined(CONFIG_MTD_NAND_OMAP2)
> 
> if (IS_ENABLED()) is preferred. If needed, you could make omap_nand_ids
> symbol visible always (so without ifdef around it), because extern
> structure should not have impact when not defined (if I recall
> correctly...).
> 
>> +	if (of_match_node(omap_nand_ids, child))
>> +		is_nand = true;
>> +#endif
>> +
>> +	if (is_nand) {
>>  		/* NAND specific setup */
>>  		val = 8;
>>  		of_property_read_u32(child, "nand-bus-width", &val);
>> diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c
>> index b26d4947af02..fff834ee726f 100644
>> --- a/drivers/mtd/nand/raw/omap2.c
>> +++ b/drivers/mtd/nand/raw/omap2.c
>> @@ -2352,7 +2352,7 @@ static int omap_nand_remove(struct platform_device *pdev)
>>  	return ret;
>>  }
>>  
>> -static const struct of_device_id omap_nand_ids[] = {
>> +const struct of_device_id omap_nand_ids[] = {
>>  	{ .compatible = "ti,omap2-nand", },
>>  	{},
>>  };
> 
> I think OMAP2 NAND driver can be a module, so this should have
> EXPORT_SYMBOL.

To make it work in all combinations (e.g. omap_gpmc built in and
nand/raw/omap2.c as module) I had to define omap_nand_ids table as static
in the linux/platform_data/mtd-nand-omap2.h header.

EXPORT_SYMBOL will of course be not required there. ;)

> 
>> diff --git a/include/linux/platform_data/mtd-nand-omap2.h b/include/linux/platform_data/mtd-nand-omap2.h
>> index de6ada739121..e1bb90a8db03 100644
>> --- a/include/linux/platform_data/mtd-nand-omap2.h
>> +++ b/include/linux/platform_data/mtd-nand-omap2.h
>> @@ -61,4 +61,9 @@ struct gpmc_nand_regs {
>>  	void __iomem	*gpmc_bch_result5[GPMC_BCH_NUM_REMAINDER];
>>  	void __iomem	*gpmc_bch_result6[GPMC_BCH_NUM_REMAINDER];
>>  };
>> +
>> +#if defined(CONFIG_MTD_NAND_OMAP2)
>> +extern const struct of_device_id omap_nand_ids[];
>> +#endif
>> +
>>  #endif
>>
> 
> 
> Best regards,
> Krzysztof
> 

cheers,
-roger

Re: [PATCH v3 3/4] memory: omap-gpmc: Use a compatible match table when checking for NAND controller

[Krzysztof Kozlowski]

On 20/12/2021 11:53, Roger Quadros wrote:
> 
> 
> On 17/12/2021 17:21, Krzysztof Kozlowski wrote:
>> On 17/12/2021 11:29, Roger Quadros wrote:
>>> As more compatibles can be added to the GPMC NAND controller driver
>>> use a compatible match table.
>>>
>>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>>> ---
>>>  drivers/memory/omap-gpmc.c                   | 8 +++++++-
>>>  drivers/mtd/nand/raw/omap2.c                 | 2 +-
>>>  include/linux/platform_data/mtd-nand-omap2.h | 5 +++++
>>>  3 files changed, 13 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
>>> index 624153048182..814ddb45c13d 100644
>>> --- a/drivers/memory/omap-gpmc.c
>>> +++ b/drivers/memory/omap-gpmc.c
>>> @@ -2091,6 +2091,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>>>  	u32 val;
>>>  	struct gpio_desc *waitpin_desc = NULL;
>>>  	struct gpmc_device *gpmc = platform_get_drvdata(pdev);
>>> +	bool is_nand = false;
>>>  
>>>  	if (of_property_read_u32(child, "reg", &cs) < 0) {
>>>  		dev_err(&pdev->dev, "%pOF has no 'reg' property\n",
>>> @@ -2183,7 +2184,12 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>>>  		}
>>>  	}
>>>  
>>> -	if (of_device_is_compatible(child, "ti,omap2-nand")) {
>>> +#if defined(CONFIG_MTD_NAND_OMAP2)
>>
>> if (IS_ENABLED()) is preferred. If needed, you could make omap_nand_ids
>> symbol visible always (so without ifdef around it), because extern
>> structure should not have impact when not defined (if I recall
>> correctly...).
>>
>>> +	if (of_match_node(omap_nand_ids, child))
>>> +		is_nand = true;
>>> +#endif
>>> +
>>> +	if (is_nand) {
>>>  		/* NAND specific setup */
>>>  		val = 8;
>>>  		of_property_read_u32(child, "nand-bus-width", &val);
>>> diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c
>>> index b26d4947af02..fff834ee726f 100644
>>> --- a/drivers/mtd/nand/raw/omap2.c
>>> +++ b/drivers/mtd/nand/raw/omap2.c
>>> @@ -2352,7 +2352,7 @@ static int omap_nand_remove(struct platform_device *pdev)
>>>  	return ret;
>>>  }
>>>  
>>> -static const struct of_device_id omap_nand_ids[] = {
>>> +const struct of_device_id omap_nand_ids[] = {
>>>  	{ .compatible = "ti,omap2-nand", },
>>>  	{},
>>>  };
>>
>> I think OMAP2 NAND driver can be a module, so this should have
>> EXPORT_SYMBOL.
> 
> To make it work in all combinations (e.g. omap_gpmc built in and
> nand/raw/omap2.c as module) I had to define omap_nand_ids table as static
> in the linux/platform_data/mtd-nand-omap2.h header.
> 
> EXPORT_SYMBOL will of course be not required there. ;)
> 
Which case exactly does it require to be static in the header?

Best regards,
Krzysztof

Re: [PATCH v3 3/4] memory: omap-gpmc: Use a compatible match table when checking for NAND controller

[Roger Quadros]

On 20/12/2021 13:05, Krzysztof Kozlowski wrote:
> On 20/12/2021 11:53, Roger Quadros wrote:
>>
>>
>> On 17/12/2021 17:21, Krzysztof Kozlowski wrote:
>>> On 17/12/2021 11:29, Roger Quadros wrote:
>>>> As more compatibles can be added to the GPMC NAND controller driver
>>>> use a compatible match table.
>>>>
>>>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>>>> ---
>>>>  drivers/memory/omap-gpmc.c                   | 8 +++++++-
>>>>  drivers/mtd/nand/raw/omap2.c                 | 2 +-
>>>>  include/linux/platform_data/mtd-nand-omap2.h | 5 +++++
>>>>  3 files changed, 13 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
>>>> index 624153048182..814ddb45c13d 100644
>>>> --- a/drivers/memory/omap-gpmc.c
>>>> +++ b/drivers/memory/omap-gpmc.c
>>>> @@ -2091,6 +2091,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>>>>  	u32 val;
>>>>  	struct gpio_desc *waitpin_desc = NULL;
>>>>  	struct gpmc_device *gpmc = platform_get_drvdata(pdev);
>>>> +	bool is_nand = false;
>>>>  
>>>>  	if (of_property_read_u32(child, "reg", &cs) < 0) {
>>>>  		dev_err(&pdev->dev, "%pOF has no 'reg' property\n",
>>>> @@ -2183,7 +2184,12 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>>>>  		}
>>>>  	}
>>>>  
>>>> -	if (of_device_is_compatible(child, "ti,omap2-nand")) {
>>>> +#if defined(CONFIG_MTD_NAND_OMAP2)
>>>
>>> if (IS_ENABLED()) is preferred. If needed, you could make omap_nand_ids
>>> symbol visible always (so without ifdef around it), because extern
>>> structure should not have impact when not defined (if I recall
>>> correctly...).
>>>
>>>> +	if (of_match_node(omap_nand_ids, child))
>>>> +		is_nand = true;
>>>> +#endif
>>>> +
>>>> +	if (is_nand) {
>>>>  		/* NAND specific setup */
>>>>  		val = 8;
>>>>  		of_property_read_u32(child, "nand-bus-width", &val);
>>>> diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c
>>>> index b26d4947af02..fff834ee726f 100644
>>>> --- a/drivers/mtd/nand/raw/omap2.c
>>>> +++ b/drivers/mtd/nand/raw/omap2.c
>>>> @@ -2352,7 +2352,7 @@ static int omap_nand_remove(struct platform_device *pdev)
>>>>  	return ret;
>>>>  }
>>>>  
>>>> -static const struct of_device_id omap_nand_ids[] = {
>>>> +const struct of_device_id omap_nand_ids[] = {
>>>>  	{ .compatible = "ti,omap2-nand", },
>>>>  	{},
>>>>  };
>>>
>>> I think OMAP2 NAND driver can be a module, so this should have
>>> EXPORT_SYMBOL.
>>
>> To make it work in all combinations (e.g. omap_gpmc built in and
>> nand/raw/omap2.c as module) I had to define omap_nand_ids table as static
>> in the linux/platform_data/mtd-nand-omap2.h header.
>>
>> EXPORT_SYMBOL will of course be not required there. ;)
>>
> Which case exactly does it require to be static in the header?

Maybe there is a better way to do it.
e.g. If omap_gpmc.c is built-in and nand/raw/omap2.c is not built or built as
a module, what better way we can use?

--
cheers,
-roger

Re: [PATCH v3 3/4] memory: omap-gpmc: Use a compatible match table when checking for NAND controller

[Krzysztof Kozlowski]

On 20/12/2021 12:51, Roger Quadros wrote:
> 
> 
> On 20/12/2021 13:05, Krzysztof Kozlowski wrote:
>> On 20/12/2021 11:53, Roger Quadros wrote:
>>>
>>>
>>> On 17/12/2021 17:21, Krzysztof Kozlowski wrote:
>>>> On 17/12/2021 11:29, Roger Quadros wrote:
>>>>> As more compatibles can be added to the GPMC NAND controller driver
>>>>> use a compatible match table.
>>>>>
>>>>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>>>>> ---
>>>>>  drivers/memory/omap-gpmc.c                   | 8 +++++++-
>>>>>  drivers/mtd/nand/raw/omap2.c                 | 2 +-
>>>>>  include/linux/platform_data/mtd-nand-omap2.h | 5 +++++
>>>>>  3 files changed, 13 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
>>>>> index 624153048182..814ddb45c13d 100644
>>>>> --- a/drivers/memory/omap-gpmc.c
>>>>> +++ b/drivers/memory/omap-gpmc.c
>>>>> @@ -2091,6 +2091,7 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>>>>>  	u32 val;
>>>>>  	struct gpio_desc *waitpin_desc = NULL;
>>>>>  	struct gpmc_device *gpmc = platform_get_drvdata(pdev);
>>>>> +	bool is_nand = false;
>>>>>  
>>>>>  	if (of_property_read_u32(child, "reg", &cs) < 0) {
>>>>>  		dev_err(&pdev->dev, "%pOF has no 'reg' property\n",
>>>>> @@ -2183,7 +2184,12 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
>>>>>  		}
>>>>>  	}
>>>>>  
>>>>> -	if (of_device_is_compatible(child, "ti,omap2-nand")) {
>>>>> +#if defined(CONFIG_MTD_NAND_OMAP2)
>>>>
>>>> if (IS_ENABLED()) is preferred. If needed, you could make omap_nand_ids
>>>> symbol visible always (so without ifdef around it), because extern
>>>> structure should not have impact when not defined (if I recall
>>>> correctly...).
>>>>
>>>>> +	if (of_match_node(omap_nand_ids, child))
>>>>> +		is_nand = true;
>>>>> +#endif
>>>>> +
>>>>> +	if (is_nand) {
>>>>>  		/* NAND specific setup */
>>>>>  		val = 8;
>>>>>  		of_property_read_u32(child, "nand-bus-width", &val);
>>>>> diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c
>>>>> index b26d4947af02..fff834ee726f 100644
>>>>> --- a/drivers/mtd/nand/raw/omap2.c
>>>>> +++ b/drivers/mtd/nand/raw/omap2.c
>>>>> @@ -2352,7 +2352,7 @@ static int omap_nand_remove(struct platform_device *pdev)
>>>>>  	return ret;
>>>>>  }
>>>>>  
>>>>> -static const struct of_device_id omap_nand_ids[] = {
>>>>> +const struct of_device_id omap_nand_ids[] = {
>>>>>  	{ .compatible = "ti,omap2-nand", },
>>>>>  	{},
>>>>>  };
>>>>
>>>> I think OMAP2 NAND driver can be a module, so this should have
>>>> EXPORT_SYMBOL.
>>>
>>> To make it work in all combinations (e.g. omap_gpmc built in and
>>> nand/raw/omap2.c as module) I had to define omap_nand_ids table as static
>>> in the linux/platform_data/mtd-nand-omap2.h header.
>>>
>>> EXPORT_SYMBOL will of course be not required there. ;)
>>>
>> Which case exactly does it require to be static in the header?
> 
> Maybe there is a better way to do it.
> e.g. If omap_gpmc.c is built-in and nand/raw/omap2.c is not built or built as
> a module, what better way we can use?

Ah, you are right, that is the tricky configuration. It could be a
separate built-in source file which is being selected by OMAP_GPMC and
MTD_NAND_OMAP2, but it would be also an overkill for one-item array.

I guess your solution with header is the easiest.


Best regards,
Krzysztof

[PATCH v3 4/4] arm64: arch_k3: Select GPMC device driver

[Roger Quadros]

The GPMC controller is present on some K3 SoCs.
It provides access to NOR/NAND flashes and asynchronous
SRAM-like memories and ASICs.

Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
 arch/arm64/Kconfig.platforms | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 1aa8b7073218..f447b120f863 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -122,6 +122,7 @@ config ARCH_K3
 	select TI_SCI_INTR_IRQCHIP
 	select TI_SCI_INTA_IRQCHIP
 	select TI_K3_SOCINFO
+	select OMAP_GPMC
 	help
 	  This enables support for Texas Instruments' K3 multicore SoC
 	  architecture.
-- 
2.17.1

Re: [PATCH v3 4/4] arm64: arch_k3: Select GPMC device driver

[Krzysztof Kozlowski]

On 17/12/2021 11:29, Roger Quadros wrote:
> The GPMC controller is present on some K3 SoCs.
> It provides access to NOR/NAND flashes and asynchronous
> SRAM-like memories and ASICs.
> 
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
>  arch/arm64/Kconfig.platforms | 1 +
>  1 file changed, 1 insertion(+)
> 

FWIW:
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

Other option would be to make it "default ARCH_K3" in OMAP_GPMC, but
this is fine for me.


Best regards,
Krzysztof

Re: [PATCH v3 4/4] arm64: arch_k3: Select GPMC device driver

[Nishanth Menon]

On 12:29-20211217, Roger Quadros wrote:
> The GPMC controller is present on some K3 SoCs.
> It provides access to NOR/NAND flashes and asynchronous
> SRAM-like memories and ASICs.
> 
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
>  arch/arm64/Kconfig.platforms | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index 1aa8b7073218..f447b120f863 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -122,6 +122,7 @@ config ARCH_K3
>  	select TI_SCI_INTR_IRQCHIP
>  	select TI_SCI_INTA_IRQCHIP
>  	select TI_K3_SOCINFO
> +	select OMAP_GPMC
I dont think GPMC should be part of the select list here. instead
defconfig patch is prefered. the existance and usage of the same are
limited and it is NOT a core functional component required to boot up a
K3 platform.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D)/Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH v3 4/4] arm64: arch_k3: Select GPMC device driver

[Roger Quadros]

Nishanth,

On 17/12/2021 18:14, Nishanth Menon wrote:
> On 12:29-20211217, Roger Quadros wrote:
>> The GPMC controller is present on some K3 SoCs.
>> It provides access to NOR/NAND flashes and asynchronous
>> SRAM-like memories and ASICs.
>>
>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>> ---
>>  arch/arm64/Kconfig.platforms | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
>> index 1aa8b7073218..f447b120f863 100644
>> --- a/arch/arm64/Kconfig.platforms
>> +++ b/arch/arm64/Kconfig.platforms
>> @@ -122,6 +122,7 @@ config ARCH_K3
>>  	select TI_SCI_INTR_IRQCHIP
>>  	select TI_SCI_INTA_IRQCHIP
>>  	select TI_K3_SOCINFO
>> +	select OMAP_GPMC
> I dont think GPMC should be part of the select list here. instead
> defconfig patch is prefered. the existance and usage of the same are
> limited and it is NOT a core functional component required to boot up a
> K3 platform.
> 

Since OMAP_GPMC is not visible it cannot be enabled via defconfig file.
I tried to make it visible in earlier revision of this patchset but it looks
like OMAP_GPMC config was meant to be not a visible option form the beginning.

All legacy platforms have been selecting it in some way or the other but I agree
with you that selecting it at SOC level may not be the best option.

If not here, any suggestions where should I select it from? Maybe from
mtd/nand/raw/Kconfig if GPMC NAND driver is enabled?

cheers,
-roger