Re: DT GPMC SRAM and NOR flash support ?

[Ezequiel Garcia <ezequiel.garcia@free-electrons.com>]

Hi Jon,

I'm working on a similar memory controller for plat-orion.
I have a few questions about your approach.

On Wed, Feb 13, 2013 at 03:07:06PM -0600, Jon Hunter wrote:
> 
> On 02/07/2013 03:51 AM, Mark Jackson wrote:
> > Okay ... I have made some progress, but it's not ideal.
> > 
> > Currently I've hacked the GPMC DT driver (gpmc_probe_dt(), etc) so it now handles setting up the
> > chip selects and timings for NOR devices, e.g.
> > 
> > 		gpmc: gpmc@50000000 {
> > 			status = "okay";
> > 			ranges = <0 0 0x08000000 0x08000000>;	/* CS0: NOR 16M */
> 
> Is that really 16M? Looks more like 128M :-)
> 
> > 			nor@0,0 {
> > 				compatible = "spansion,s29gl064n90t", "cfi-flash";
> > 				reg = <0 0 0>;
> 
> I think that cfi-flash is expecting a length here. Here is
> what I have ...
> 
> +&gpmc {
> +       ranges = <0 0 0x08000000 0x04000000>;
> +
> +       nor@0,0 {
> +               compatible = "cfi-flash";
> +               linux,mtd-name= "intel,ge28f256l18b85";
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               reg = <0 0 0x04000000>;
> +               bank-width = <2>;
> +
> +               partition@0 {
> +                       label = "bootloader";
> +                       reg = <0 0x20000>;
> +               };
> +               partition@0x20000 {
> +                       label = "params";
> +                       reg = <0x20000 0x20000>;
> +               };
> +               partition@0x40000 {
> +                       label = "kernel";
> +                       reg = <0x40000 0x200000>;
> +               };
> +               partition@0x240000 {
> +                       label = "file-system";
> +                       reg = <0x240000 0x3dc0000>;
> +               };
> +       };
> 
> > 	nor-flash@08000000 {
> > 		compatible = "spansion,s29gl064n90t", "cfi-flash";
> > 		reg = <0x08000000 0x00800000>;
> > 		bank-width = <2>;
> > 	};
> 
> You don't need this extra entry if you add "simple-bus" to
> the gpmc node compatible string.
> 

That would be nice. However, I wonder what happens if cfi-flash probing
fails: will the gpmc cs request by undone? See below...

> +               gpmc: gpmc@6800a000 {
> +                       compatible = "ti,omap2420-gpmc", "simple-bus";
> +                       ti,hwmods = "gpmc";
> +                       reg = <0x6800a000 0x1000>;
> +                       interrupts = <20>;
> +
> +                       gpmc,num-cs = <8>;
> +                       gpmc,num-waitpins = <4>;
> +                       #address-cells = <2>;
> +                       #size-cells = <1>;
> +               };
> 
> Currently I have the following and this is working for me. 
> Please note that on my board the bootloader is configuring
> the timings for me and so this is missing from the below
> implementation and still needs to be added.
> 
> Cheers
> Jon
> 
> From c0ede833fad704ab452b116154177e3a59166c7e Mon Sep 17 00:00:00 2001
> From: Jon Hunter <jon-hunter@ti.com>
> Date: Fri, 8 Feb 2013 16:46:13 -0600
> Subject: [PATCH] ARM: OMAP2+: Add device-tree support for NOR flash
> 
> ---
>  arch/arm/mach-omap2/gpmc.c |   69 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index bc90155..421320b 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -26,6 +26,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/platform_device.h>
>  #include <linux/of.h>
> +#include <linux/of_address.h>
>  #include <linux/of_mtd.h>
>  #include <linux/of_device.h>
>  #include <linux/mtd/nand.h>
> @@ -517,6 +518,26 @@ static int gpmc_cs_delete_mem(int cs)
>  	return r;
>  }
>  
> +static int gpmc_cs_remap(int cs, u32 base)
> +{
> +	int r;
> +	u32 old_base, size;
> +
> +	gpmc_cs_get_memconf(cs, &old_base, &size);
> +	if (base == old_base)
> +		return 0;
> +	gpmc_cs_disable_mem(cs);
> +	r = gpmc_cs_delete_mem(cs);
> +	if (IS_ERR_VALUE(r))
> +		return r;
> +	r = gpmc_cs_insert_mem(cs, base, size);
> +	if (IS_ERR_VALUE(r))
> +		return r;
> +	gpmc_cs_enable_mem(cs, base, size);
> +
> +	return 0;
> +}
> +
>  int gpmc_cs_request(int cs, unsigned long size, unsigned long *base)
>  {
>  	struct resource *res = &gpmc_cs_mem[cs];
> @@ -1305,6 +1326,45 @@ static int gpmc_probe_onenand_child(struct platform_device *pdev,
>  }
>  #endif
>  
> +static int gpmc_probe_nor_child(struct platform_device *pdev,
> +				struct device_node *child)
> +{
> +	struct resource res;
> +	unsigned long base;
> +	int rc, cs;
> +
> +	if (of_property_read_u32(child, "reg", &cs) < 0) {
> +		dev_err(&pdev->dev, "%s has no 'reg' property\n",
> +			child->full_name);
> +		return -ENODEV;
> +	}
> +
> +	if (of_address_to_resource(child, 0, &res)) {
> +		dev_err(&pdev->dev, "%s has malformed 'reg' property\n",
> +			child->full_name);
> +		return -ENODEV;
> +	}
> +
> +	rc = gpmc_cs_request(cs, res.end - res.start, &base);
> +	if (rc < 0)
> +		pr_err("%s: cannot request GPMC CS %d\n", __func__, cs);

gpmc_cs_request will request a memory region from the resource allocator
using allocate_resource().
How will this be released in case cfi-flash probe fails?

IMHO, this works fine currently for NAND and OneNAND because we register
the platform device explicitly and, thus, know if that fails.

> +
> +	/*
> +	 * gpmc_cs_request() will map the CS to an arbitary location
> +	 * in the gpmc address space. When booting with device-tree we
> +	 * want the NOR flash to be mapped to the location specified
> +	 * in the device-tree blob. So remap the CS to this location.
> +	 */
> +	rc = gpmc_cs_remap(cs, res.start);
> +	if (rc < 0) {
> +		pr_err("%s: cannot remap GPMC CS %d to 0x%x\n",
> +		       __func__, cs, res.start);
> +		gpmc_cs_free(cs);
> +	}
> +
> +	return rc;
> +}
> +
>  static int gpmc_probe_dt(struct platform_device *pdev)
>  {
>  	int ret;
> @@ -1330,6 +1390,15 @@ static int gpmc_probe_dt(struct platform_device *pdev)
>  			return ret;
>  		}
>  	}
> +
> +	for_each_node_by_name(child, "nor") {
> +		ret = gpmc_probe_nor_child(pdev, child);
> +		if (ret < 0) {
> +			of_node_put(child);
> +			return ret;
> +		}
> +	}
> +
>  	return 0;
>  }
>  #else
> -- 


Thanks,

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html