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

[PATCH 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

Roger Quadros (4):
  dt-bindings: memory-controllers: ti,gpmc: Add compatible for AM64
  memory: omap-gpmc: Fix menuconfig visibility
  memory: omap-gpmc: Add support for GPMC on AM64 SoC
  memory: omap-gpmc: check for nand node name instead of just
    compatibility

 .../bindings/memory-controllers/ti,gpmc.yaml  | 12 +++++-
 drivers/memory/Kconfig                        |  4 +-
 drivers/memory/omap-gpmc.c                    | 43 ++++++++++++++-----
 3 files changed, 45 insertions(+), 14 deletions(-)

-- 
2.17.1

[PATCH 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.

Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
 .../bindings/memory-controllers/ti,gpmc.yaml         | 12 +++++++++++-
 1 file changed, 11 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..1869cc6f949b 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
-- 
2.17.1

[PATCH 2/4] memory: gpmc: Fix menuconfig visibility

[Roger Quadros]

GPMC was not being visible if COMPILE_TEST is not enabled.

Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
 drivers/memory/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index 30bff6cb1b8d..da2af9c38fe3 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -103,8 +103,8 @@ config TI_EMIF
 	  temperature changes
 
 config OMAP_GPMC
-	bool "Texas Instruments OMAP SoC GPMC driver" if COMPILE_TEST
-	depends on OF_ADDRESS
+	bool "Texas Instruments OMAP SoC GPMC driver"
+	depends on OF_ADDRESS || COMPILE_TEST
 	select GPIOLIB
 	help
 	  This driver is for the General Purpose Memory Controller (GPMC)
-- 
2.17.1

[PATCH 2/4] memory: omap-gpmc: Fix menuconfig visibility

[Roger Quadros]

GPMC was not being visible if COMPILE_TEST is not enabled.

Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
 drivers/memory/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index 30bff6cb1b8d..da2af9c38fe3 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -103,8 +103,8 @@ config TI_EMIF
 	  temperature changes
 
 config OMAP_GPMC
-	bool "Texas Instruments OMAP SoC GPMC driver" if COMPILE_TEST
-	depends on OF_ADDRESS
+	bool "Texas Instruments OMAP SoC GPMC driver"
+	depends on OF_ADDRESS || COMPILE_TEST
 	select GPIOLIB
 	help
 	  This driver is for the General Purpose Memory Controller (GPMC)
-- 
2.17.1

[PATCH 3/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 3/4] memory: omap-gpmc: 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 4/4] memory: omap-gpmc: check for nand node name instead of just compatibility

[Roger Quadros]

We have added new compatibles for controller so just checking for
"ti,omap2-nand" compatible is not enough. Check for "nand" node name
as well.

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

diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index 624153048182..9f0062a262db 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -2183,7 +2183,8 @@ static int gpmc_probe_generic_child(struct platform_device *pdev,
 		}
 	}
 
-	if (of_device_is_compatible(child, "ti,omap2-nand")) {
+	if (of_device_is_compatible(child, "ti,omap2-nand") ||
+	    of_node_name_eq(child, "nand")) {
 		/* NAND specific setup */
 		val = 8;
 		of_property_read_u32(child, "nand-bus-width", &val);
-- 
2.17.1

Re: [PATCH 2/4] memory: gpmc: Fix menuconfig visibility

[Roger Quadros]

please ignore this. Went out by mistake.

On 23/11/2021 12:26, Roger Quadros wrote:
> GPMC was not being visible if COMPILE_TEST is not enabled.
> 
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
>  drivers/memory/Kconfig | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
> index 30bff6cb1b8d..da2af9c38fe3 100644
> --- a/drivers/memory/Kconfig
> +++ b/drivers/memory/Kconfig
> @@ -103,8 +103,8 @@ config TI_EMIF
>  	  temperature changes
>  
>  config OMAP_GPMC
> -	bool "Texas Instruments OMAP SoC GPMC driver" if COMPILE_TEST
> -	depends on OF_ADDRESS
> +	bool "Texas Instruments OMAP SoC GPMC driver"
> +	depends on OF_ADDRESS || COMPILE_TEST
>  	select GPIOLIB
>  	help
>  	  This driver is for the General Purpose Memory Controller (GPMC)
> 

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

[Roger Quadros]

Please ignore this one as well.

On 23/11/2021 12:26, Roger Quadros wrote:
> 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;
> 

Re: [PATCH 2/4] memory: omap-gpmc: Fix menuconfig visibility

[Krzysztof Kozlowski]

On 23/11/2021 11:26, Roger Quadros wrote:
> GPMC was not being visible if COMPILE_TEST is not enabled.
> 
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
>  drivers/memory/Kconfig | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

... which was probably on purpose, similarly to many other SoC drivers
which are selected by platform. Therefore there is no bug to fix here -
lack of visibility is not a problem. Please document instead why you
want to change it, e.g. why this is a problem or unwanted behavior.

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 23/11/2021 11:26, Roger Quadros wrote:
> 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.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
>  .../bindings/memory-controllers/ti,gpmc.yaml         | 12 +++++++++++-
>  1 file changed, 11 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..1869cc6f949b 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

I see your driver handles cases with only one reg item, but I have other
question - is it correct to have older (ARMv7) platform with two reg
items? Or can am64-gpmc come with only one reg?
IOW, I am surprised there is no if-else case precising this minItems
requirement for different SocS.

>  
>    interrupts:
>      maxItems: 1
> @@ -44,6 +51,9 @@ properties:
>      items:
>        - const: fck
>  
> +  power-domains:
> +    maxItems: 1

Similar, but looks like a weaker requirement - could an older SoC define
power-domain?

> +
>    dmas:
>      items:
>        - description: DMA channel for GPMC NAND prefetch
> 


Best regards,
Krzysztof

Re: [PATCH 2/4] memory: omap-gpmc: Fix menuconfig visibility

[Roger Quadros]

On 23/11/2021 21:43, Krzysztof Kozlowski wrote:
> On 23/11/2021 11:26, Roger Quadros wrote:
>> GPMC was not being visible if COMPILE_TEST is not enabled.
>>
>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>> ---
>>  drivers/memory/Kconfig | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
> 
> ... which was probably on purpose, similarly to many other SoC drivers
> which are selected by platform. Therefore there is no bug to fix here -
> lack of visibility is not a problem. Please document instead why you
> want to change it, e.g. why this is a problem or unwanted behavior.
> 

No strong enough reason. I'll drop this patch.

cheers,
-roger

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

[Roger Quadros]

On 23/11/2021 21:47, Krzysztof Kozlowski wrote:
> On 23/11/2021 11:26, Roger Quadros wrote:
>> 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.
>>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>> ---
>>  .../bindings/memory-controllers/ti,gpmc.yaml         | 12 +++++++++++-
>>  1 file changed, 11 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..1869cc6f949b 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
> 
> I see your driver handles cases with only one reg item, but I have other

The support for these two items is added in patch 3 of this series "memory: omap-gpmc: Add support for GPMC on AM64 SoC"

> question - is it correct to have older (ARMv7) platform with two reg
> items? Or can am64-gpmc come with only one reg?

Older platforms currently have only one reg, but they can be updated to come with two without breaking functionality.
am64-gpmc cannot come with one reg as the defaults for data window are not suitable for AM64.

All legacy platforms were using a fixed Data IO window (first 1 GB) but from AM64 this was moved elsewhere, so the need for this change.

> IOW, I am surprised there is no if-else case precising this minItems
> requirement for different SocS.
> 

OK. I will add this.

>>  
>>    interrupts:
>>      maxItems: 1
>> @@ -44,6 +51,9 @@ properties:
>>      items:
>>        - const: fck
>>  
>> +  power-domains:
>> +    maxItems: 1
> 
> Similar, but looks like a weaker requirement - could an older SoC define
> power-domain?

No. Will add SoC specific constraint for this as well.

> 
>> +
>>    dmas:
>>      items:
>>        - description: DMA channel for GPMC NAND prefetch
>>
> 
> 
> Best regards,
> Krzysztof
> 

cheers,
-roger

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

[Rob Herring]

On Tue, Nov 23, 2021 at 08:47:57PM +0100, Krzysztof Kozlowski wrote:
> On 23/11/2021 11:26, Roger Quadros wrote:
> > 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.
> > 
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Signed-off-by: Roger Quadros <rogerq@kernel.org>
> > ---
> >  .../bindings/memory-controllers/ti,gpmc.yaml         | 12 +++++++++++-
> >  1 file changed, 11 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..1869cc6f949b 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
> 
> I see your driver handles cases with only one reg item, but I have other
> question - is it correct to have older (ARMv7) platform with two reg
> items? Or can am64-gpmc come with only one reg?
> IOW, I am surprised there is no if-else case precising this minItems
> requirement for different SocS.

I don't think that is needed here. If the assumption is 'reg-names' is 
only present when there are 2 entries, then it is fine. Maybe 
'reg-names' should be required for ti,am64-gpmc though.

Rob

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

[Roger Quadros]

On 01/12/2021 00:02, Rob Herring wrote:
> On Tue, Nov 23, 2021 at 08:47:57PM +0100, Krzysztof Kozlowski wrote:
>> On 23/11/2021 11:26, Roger Quadros wrote:
>>> 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.
>>>
>>> Cc: Rob Herring <robh+dt@kernel.org>
>>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>>> ---
>>>  .../bindings/memory-controllers/ti,gpmc.yaml         | 12 +++++++++++-
>>>  1 file changed, 11 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..1869cc6f949b 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
>>
>> I see your driver handles cases with only one reg item, but I have other
>> question - is it correct to have older (ARMv7) platform with two reg
>> items? Or can am64-gpmc come with only one reg?
>> IOW, I am surprised there is no if-else case precising this minItems
>> requirement for different SocS.
> 
> I don't think that is needed here. If the assumption is 'reg-names' is 
> only present when there are 2 entries, then it is fine. Maybe 
> 'reg-names' should be required for ti,am64-gpmc though.

Yes, I'll make 'reg-names' property required for ti,am64-gpmc.

cheers,
-roger