diff mbox

[v7,5/5] ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

Message ID 1354734571-10774-6-git-send-email-zonque@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Mack Dec. 5, 2012, 7:09 p.m. UTC
This patch adds basic DT bindings for OMAP GPMC.

The actual peripherals are instantiated from child nodes within the GPMC
node, and the only type of device that is currently supported is NAND.

Code was added to parse the generic GPMC timing parameters and some
documentation with examples on how to use them.

Successfully tested on an AM33xx board.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 Documentation/devicetree/bindings/bus/ti-gpmc.txt  |  77 ++++++++++
 .../devicetree/bindings/mtd/gpmc-nand.txt          |  76 +++++++++
 arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
 3 files changed, 323 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt

Comments

Grant Likely Dec. 5, 2012, 10:22 p.m. UTC | #1
On Wed,  5 Dec 2012 20:09:31 +0100, Daniel Mack <zonque@gmail.com> wrote:
> This patch adds basic DT bindings for OMAP GPMC.
> 
> The actual peripherals are instantiated from child nodes within the GPMC
> node, and the only type of device that is currently supported is NAND.
> 
> Code was added to parse the generic GPMC timing parameters and some
> documentation with examples on how to use them.
> 
> Successfully tested on an AM33xx board.
> 
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> ---
>  Documentation/devicetree/bindings/bus/ti-gpmc.txt  |  77 ++++++++++
>  .../devicetree/bindings/mtd/gpmc-nand.txt          |  76 +++++++++
>  arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
>  3 files changed, 323 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt
>  create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> 
> diff --git a/Documentation/devicetree/bindings/bus/ti-gpmc.txt b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
> new file mode 100644
> index 0000000..7d2a645
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
> @@ -0,0 +1,77 @@
> +Device tree bindings for OMAP general purpose memory controllers (GPMC)
> +
> +The actual devices are instantiated from the child nodes of a GPMC node.
> +
> +Required properties:
> +
> + - compatible:		Should be set to "ti,gpmc"

Please, be specific. Use something like "ti,am3340-gpmc" or
"ti,omap3430-gpmc". The compatible property is a list so that new
devices can claim compatibility with old. Compatible strings that are
overly generic are a pet-peave of mine.

> + - reg:			A resource specifier for the register space
> +			(see the example below)
> + - ti,hwmods:		Should be set to "ti,gpmc" until the DT transition is
> +			completed.
> + - #address-cells:	Must be set to 2 to allow memory address translation
> + - #size-cells:		Must be set to 1 to allow CS address passing
> + - num-cs:		The maximum number of chip-select lines that controller
> +			can support.

gpmc,num-cs

> + - num-waitpins:	The maximum number of wait pins that controller can
> +			support.

gpmc,num-waitpins

> + - ranges:		Must be set up to reflect the memory layout with four
> +			integer values for each chip-select line in use:
> +
> +			   <cs-number> 0 <physical address of mapping> <size>
> +
> +			Note that this property is not currently parsed.
> +			Calculated values derived from the contents of the
> +			per-CS register GPMC_CONFIG7 (as set up by the
> +			bootloader) are used. That will change in the future,
> +			so be sure to fill the correct values here.

The core linux code will use ranges to decode the reg property of child
devices, even if the gpmc driver doesn't use it, so the comment here is
misleading.

Otherwise the binding looks fine to me.

> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> new file mode 100644
> index 0000000..b3fafb1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> @@ -0,0 +1,76 @@
> +Device tree bindings for GPMC connected NANDs
> +
> +GPMC connected NAND (found on OMAP boards) are represented as child nodes of
> +the GPMC controller with a name of "nand".
> +
> +All timing relevant properties as well as generic gpmc child properties are
> +explained in a separate documents - please refer to
> +Documentation/devicetree/bindings/bus/ti-gpmc.txt
> +
> +For NAND specific properties such as ECC modes or bus width, please refer to
> +Documentation/devicetree/bindings/mtd/nand.txt
> +
> +
> +Required properties:
> +
> + - reg:		The CS line the peripheral is connected to
> +
> +Optional properties:
> +
> + - nand-bus-width: 		Set this numeric value to 16 if the hardware
> +				is wired that way. If not specified, a bus
> +				width of 8 is assumed.
> +
> + - ti,nand-ecc-opt:		A string setting the ECC layout to use. One of:
> +
> +		"sw"		Software method (default)
> +		"hw"		Hardware method
> +		"hw-romcode"	gpmc hamming mode method & romcode layout
> +		"bch4"		4-bit BCH ecc code
> +		"bch8"		8-bit BCH ecc code
> +
> +For inline partiton table parsing (optional):
> +
> + - #address-cells: should be set to 1
> + - #size-cells: should be set to 1
> +
> +Example for an AM33xx board:
> +
> +	gpmc: gpmc@50000000 {
> +		compatible = "ti,gpmc";
> +		ti,hwmods = "gpmc";
> +		reg = <0x50000000 0x1000000>;
> +		interrupts = <100>;
> +		num-cs = <8>;
> +		num-waitpins = <2>;
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +		ranges = <0 0 0x08000000 0x2000>;	/* CS0: NAND */
> +
> +		nand@0,0 {
> +			reg = <0 0 0>; /* CS0, offset 0 */
> +			nand-bus-width = <16>;
> +			ti,nand-ecc-opt = "bch8";
> +
> +			gpmc,sync-clk = <0>;
> +			gpmc,cs-on = <0>;
> +			gpmc,cs-rd-off = <44>;
> +			gpmc,cs-wr-off = <44>;
> +			gpmc,adv-on = <6>;
> +			gpmc,adv-rd-off = <34>;
> +			gpmc,adv-wr-off = <44>;
> +			gpmc,we-off = <40>;
> +			gpmc,oe-off = <54>;
> +			gpmc,access = <64>;
> +			gpmc,rd-cycle = <82>;
> +			gpmc,wr-cycle = <82>;
> +			gpmc,wr-access = <40>;
> +			gpmc,wr-data-mux-bus = <0>;
> +
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			/* partitions go here */
> +		};
> +	};
> +
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index d5cbd29..0baf9df 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -25,6 +25,10 @@
>  #include <linux/module.h>
>  #include <linux/interrupt.h>
>  #include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/of_mtd.h>
> +#include <linux/of_device.h>
> +#include <linux/mtd/nand.h>
>  
>  #include <linux/platform_data/mtd-nand-omap2.h>
>  
> @@ -34,6 +38,7 @@
>  #include "common.h"
>  #include "omap_device.h"
>  #include "gpmc.h"
> +#include "gpmc-nand.h"
>  
>  #define	DEVICE_NAME		"omap-gpmc"
>  
> @@ -1121,7 +1126,162 @@ int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
>  	return 0;
>  }
>  
> -static __devinit int gpmc_probe(struct platform_device *pdev)
> +#ifdef CONFIG_OF
> +static struct of_device_id gpmc_dt_ids[] = {
> +	{ .compatible = "ti,gpmc" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, gpmc_dt_ids);
> +
> +static void __maybe_unused gpmc_read_timings_dt(struct device_node *np,
> +						struct gpmc_timings *gpmc_t)
> +{
> +	u32 val;
> +
> +	memset(gpmc_t, 0, sizeof(*gpmc_t));
> +
> +	/* minimum clock period for syncronous mode */
> +	if (!of_property_read_u32(np, "gpmc,sync-clk", &val))
> +		gpmc_t->sync_clk = val;
> +
> +	/* chip select timtings */
> +	if (!of_property_read_u32(np, "gpmc,cs-on", &val))
> +		gpmc_t->cs_on = val;
> +
> +	if (!of_property_read_u32(np, "gpmc,cs-rd-off", &val))
> +		gpmc_t->cs_rd_off = val;
> +
> +	if (!of_property_read_u32(np, "gpmc,cs-wr-off", &val))
> +		gpmc_t->cs_wr_off = val;
> +
> +	/* ADV signal timings */
> +	if (!of_property_read_u32(np, "gpmc,adv-on", &val))
> +		gpmc_t->adv_on = val;
> +
> +	if (!of_property_read_u32(np, "gpmc,adv-rd-off", &val))
> +		gpmc_t->adv_rd_off = val;
> +
> +	if (!of_property_read_u32(np, "gpmc,adv-wr-off", &val))
> +		gpmc_t->adv_wr_off = val;
> +
> +	/* WE signal timings */
> +	if (!of_property_read_u32(np, "gpmc,we-on", &val))
> +		gpmc_t->we_on = val;
> +
> +	if (!of_property_read_u32(np, "gpmc,we-off", &val))
> +		gpmc_t->we_off = val;
> +
> +	/* OE signal timings */
> +	if (!of_property_read_u32(np, "gpmc,oe-on", &val))
> +		gpmc_t->oe_on = val;
> +
> +	if (!of_property_read_u32(np, "gpmc,oe-off", &val))
> +		gpmc_t->oe_off = val;
> +
> +	/* access and cycle timings */
> +	if (!of_property_read_u32(np, "gpmc,page-burst-access", &val))
> +		gpmc_t->page_burst_access = val;
> +
> +	if (!of_property_read_u32(np, "gpmc,access", &val))
> +		gpmc_t->access = val;
> +
> +	if (!of_property_read_u32(np, "gpmc,rd-cycle", &val))
> +		gpmc_t->rd_cycle = val;
> +
> +	if (!of_property_read_u32(np, "gpmc,wr-cycle", &val))
> +		gpmc_t->wr_cycle = val;
> +
> +	/* only for OMAP3430 */
> +	if (!of_property_read_u32(np, "gpmc,wr-access", &val))
> +		gpmc_t->wr_access = val;
> +
> +	if (!of_property_read_u32(np, "gpmc,wr-data-mux-bus", &val))
> +		gpmc_t->wr_data_mux_bus = val;
> +}
> +
> +#ifdef CONFIG_MTD_NAND
> +
> +static const char * const nand_ecc_opts[] = {
> +	[OMAP_ECC_HAMMING_CODE_DEFAULT]		= "sw",
> +	[OMAP_ECC_HAMMING_CODE_HW]		= "hw",
> +	[OMAP_ECC_HAMMING_CODE_HW_ROMCODE]	= "hw-romcode",
> +	[OMAP_ECC_BCH4_CODE_HW]			= "bch4",
> +	[OMAP_ECC_BCH8_CODE_HW]			= "bch8",
> +};
> +
> +static int gpmc_probe_nand_child(struct platform_device *pdev,
> +				 struct device_node *child)
> +{
> +	u32 val;
> +	const char *s;
> +	struct gpmc_timings gpmc_t;
> +	struct omap_nand_platform_data *gpmc_nand_data;
> +
> +	if (of_property_read_u32(child, "reg", &val) < 0) {
> +		dev_err(&pdev->dev, "%s has no 'reg' property\n",
> +			child->full_name);
> +		return -ENODEV;
> +	}
> +
> +	gpmc_nand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_nand_data),
> +				      GFP_KERNEL);
> +	if (!gpmc_nand_data)
> +		return -ENOMEM;
> +
> +	gpmc_nand_data->cs = val;
> +	gpmc_nand_data->of_node = child;
> +
> +	if (!of_property_read_string(child, "ti,nand-ecc-opt", &s))
> +		for (val = 0; val < ARRAY_SIZE(nand_ecc_opts); val++)
> +			if (!strcasecmp(s, nand_ecc_opts[val])) {
> +				gpmc_nand_data->ecc_opt = val;
> +				break;
> +			}
> +
> +	val = of_get_nand_bus_width(child);
> +	if (val == 16)
> +		gpmc_nand_data->devsize = NAND_BUSWIDTH_16;
> +
> +	gpmc_read_timings_dt(child, &gpmc_t);
> +	gpmc_nand_init(gpmc_nand_data, &gpmc_t);
> +
> +	return 0;
> +}
> +#else
> +static int gpmc_probe_nand_child(struct platform_device *pdev,
> +				 struct device_node *child)
> +{
> +	return 0;
> +}
> +#endif
> +
> +static int gpmc_probe_dt(struct platform_device *pdev)
> +{
> +	int ret;
> +	struct device_node *child;
> +	const struct of_device_id *of_id =
> +		of_match_device(gpmc_dt_ids, &pdev->dev);
> +
> +	if (!of_id)
> +		return 0;
> +
> +	for_each_node_by_name(child, "nand") {
> +		ret = gpmc_probe_nand_child(pdev, child);
> +		of_node_put(child);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +#else
> +static int gpmc_probe_dt(struct platform_device *pdev)
> +{
> +	return 0;
> +}
> +#endif
> +
> +static int __devinit gpmc_probe(struct platform_device *pdev)
>  {
>  	int rc;
>  	u32 l;
> @@ -1174,6 +1334,14 @@ static __devinit int gpmc_probe(struct platform_device *pdev)
>  	if (IS_ERR_VALUE(gpmc_setup_irq()))
>  		dev_warn(gpmc_dev, "gpmc_setup_irq failed\n");
>  
> +	rc = gpmc_probe_dt(pdev);
> +	if (rc < 0) {
> +		clk_disable_unprepare(gpmc_l3_clk);
> +		clk_put(gpmc_l3_clk);
> +		dev_err(gpmc_dev, "failed to probe DT parameters\n");
> +		return rc;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -1191,6 +1359,7 @@ static struct platform_driver gpmc_driver = {
>  	.driver		= {
>  		.name	= DEVICE_NAME,
>  		.owner	= THIS_MODULE,
> +		.of_match_table = of_match_ptr(gpmc_dt_ids),
>  	},
>  };
>  
> -- 
> 1.7.11.7
>
Hunter, Jon Dec. 5, 2012, 10:33 p.m. UTC | #2
Hi Grant,

On 12/05/2012 04:22 PM, Grant Likely wrote:
> On Wed,  5 Dec 2012 20:09:31 +0100, Daniel Mack <zonque@gmail.com> wrote:
>> This patch adds basic DT bindings for OMAP GPMC.
>>
>> The actual peripherals are instantiated from child nodes within the GPMC
>> node, and the only type of device that is currently supported is NAND.
>>
>> Code was added to parse the generic GPMC timing parameters and some
>> documentation with examples on how to use them.
>>
>> Successfully tested on an AM33xx board.
>>
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>> ---
>>  Documentation/devicetree/bindings/bus/ti-gpmc.txt  |  77 ++++++++++
>>  .../devicetree/bindings/mtd/gpmc-nand.txt          |  76 +++++++++
>>  arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
>>  3 files changed, 323 insertions(+), 1 deletion(-)
>>  create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt
>>  create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
>>
>> diff --git a/Documentation/devicetree/bindings/bus/ti-gpmc.txt b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
>> new file mode 100644
>> index 0000000..7d2a645
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
>> @@ -0,0 +1,77 @@
>> +Device tree bindings for OMAP general purpose memory controllers (GPMC)
>> +
>> +The actual devices are instantiated from the child nodes of a GPMC node.
>> +
>> +Required properties:
>> +
>> + - compatible:		Should be set to "ti,gpmc"
> 
> Please, be specific. Use something like "ti,am3340-gpmc" or
> "ti,omap3430-gpmc". The compatible property is a list so that new
> devices can claim compatibility with old. Compatible strings that are
> overly generic are a pet-peave of mine.

We aim to use the binding for omap2,3,4,5 as well as the am33xx devices
(which are omap based). Would it be sufficient to have "ti,omap2-gpmc"
implying all omap2+ based devices or should we have a compatible string
for each device supported?

Thanks
Jon
--
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
Grant Likely Dec. 5, 2012, 11:24 p.m. UTC | #3
On Wed, 5 Dec 2012 16:33:48 -0600, Jon Hunter <jon-hunter@ti.com> wrote:
> Hi Grant,
> 
> On 12/05/2012 04:22 PM, Grant Likely wrote:
> > On Wed,  5 Dec 2012 20:09:31 +0100, Daniel Mack <zonque@gmail.com> wrote:
> >> This patch adds basic DT bindings for OMAP GPMC.
> >>
> >> The actual peripherals are instantiated from child nodes within the GPMC
> >> node, and the only type of device that is currently supported is NAND.
> >>
> >> Code was added to parse the generic GPMC timing parameters and some
> >> documentation with examples on how to use them.
> >>
> >> Successfully tested on an AM33xx board.
> >>
> >> Signed-off-by: Daniel Mack <zonque@gmail.com>
> >> ---
> >>  Documentation/devicetree/bindings/bus/ti-gpmc.txt  |  77 ++++++++++
> >>  .../devicetree/bindings/mtd/gpmc-nand.txt          |  76 +++++++++
> >>  arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
> >>  3 files changed, 323 insertions(+), 1 deletion(-)
> >>  create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt
> >>  create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/bus/ti-gpmc.txt b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
> >> new file mode 100644
> >> index 0000000..7d2a645
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
> >> @@ -0,0 +1,77 @@
> >> +Device tree bindings for OMAP general purpose memory controllers (GPMC)
> >> +
> >> +The actual devices are instantiated from the child nodes of a GPMC node.
> >> +
> >> +Required properties:
> >> +
> >> + - compatible:		Should be set to "ti,gpmc"
> > 
> > Please, be specific. Use something like "ti,am3340-gpmc" or
> > "ti,omap3430-gpmc". The compatible property is a list so that new
> > devices can claim compatibility with old. Compatible strings that are
> > overly generic are a pet-peave of mine.
> 
> We aim to use the binding for omap2,3,4,5 as well as the am33xx devices
> (which are omap based). Would it be sufficient to have "ti,omap2-gpmc"
> implying all omap2+ based devices or should we have a compatible string
> for each device supported?

Are they each register-level compatible with one another?

The general recommended approach here is to make subsequent silicon
claim compatibility with the first compatible implementation.

So, for an am3358 board:
	compatible = "ti,am3358-gpmc", "ti,omap2420-gpmc";

Essentially, what this means is that "ti,omap2420-gpmc" is the generic
value instead of "omap2-gpmc". The reason for this is so that the value
is anchored against a specific implementation, and not against something
completely imaginary or idealized. If a newer version isn't quite
compatible with the omap2420-gpmc, then it can drop the compatible claim
and the driver really should be told about the new device.

g.



> 
> Thanks
> Jon
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Tony Lindgren Dec. 6, 2012, 12:03 a.m. UTC | #4
* Grant Likely <grant.likely@secretlab.ca> [121205 15:26]:
> On Wed, 5 Dec 2012 16:33:48 -0600, Jon Hunter <jon-hunter@ti.com> wrote:
> > On 12/05/2012 04:22 PM, Grant Likely wrote:
> > > 
> > > Please, be specific. Use something like "ti,am3340-gpmc" or
> > > "ti,omap3430-gpmc". The compatible property is a list so that new
> > > devices can claim compatibility with old. Compatible strings that are
> > > overly generic are a pet-peave of mine.
> > 
> > We aim to use the binding for omap2,3,4,5 as well as the am33xx devices
> > (which are omap based). Would it be sufficient to have "ti,omap2-gpmc"
> > implying all omap2+ based devices or should we have a compatible string
> > for each device supported?
> 
> Are they each register-level compatible with one another?
> 
> The general recommended approach here is to make subsequent silicon
> claim compatibility with the first compatible implementation.
> 
> So, for an am3358 board:
> 	compatible = "ti,am3358-gpmc", "ti,omap2420-gpmc";
> 
> Essentially, what this means is that "ti,omap2420-gpmc" is the generic
> value instead of "omap2-gpmc". The reason for this is so that the value
> is anchored against a specific implementation, and not against something
> completely imaginary or idealized. If a newer version isn't quite
> compatible with the omap2420-gpmc, then it can drop the compatible claim
> and the driver really should be told about the new device.

The compatible property can also be used to figure out which ones
need the workarounds in patch #4 of this series for the DT case.
So we should be specific with the compatible.

Regards,

Tony
--
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
Hunter, Jon Dec. 6, 2012, 4:19 p.m. UTC | #5
On 12/05/2012 05:24 PM, Grant Likely wrote:
> On Wed, 5 Dec 2012 16:33:48 -0600, Jon Hunter <jon-hunter@ti.com> wrote:
>> Hi Grant,
>>
>> On 12/05/2012 04:22 PM, Grant Likely wrote:
>>> On Wed,  5 Dec 2012 20:09:31 +0100, Daniel Mack <zonque@gmail.com> wrote:
>>>> This patch adds basic DT bindings for OMAP GPMC.
>>>>
>>>> The actual peripherals are instantiated from child nodes within the GPMC
>>>> node, and the only type of device that is currently supported is NAND.
>>>>
>>>> Code was added to parse the generic GPMC timing parameters and some
>>>> documentation with examples on how to use them.
>>>>
>>>> Successfully tested on an AM33xx board.
>>>>
>>>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/bus/ti-gpmc.txt  |  77 ++++++++++
>>>>  .../devicetree/bindings/mtd/gpmc-nand.txt          |  76 +++++++++
>>>>  arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
>>>>  3 files changed, 323 insertions(+), 1 deletion(-)
>>>>  create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt
>>>>  create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/bus/ti-gpmc.txt b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
>>>> new file mode 100644
>>>> index 0000000..7d2a645
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
>>>> @@ -0,0 +1,77 @@
>>>> +Device tree bindings for OMAP general purpose memory controllers (GPMC)
>>>> +
>>>> +The actual devices are instantiated from the child nodes of a GPMC node.
>>>> +
>>>> +Required properties:
>>>> +
>>>> + - compatible:		Should be set to "ti,gpmc"
>>>
>>> Please, be specific. Use something like "ti,am3340-gpmc" or
>>> "ti,omap3430-gpmc". The compatible property is a list so that new
>>> devices can claim compatibility with old. Compatible strings that are
>>> overly generic are a pet-peave of mine.
>>
>> We aim to use the binding for omap2,3,4,5 as well as the am33xx devices
>> (which are omap based). Would it be sufficient to have "ti,omap2-gpmc"
>> implying all omap2+ based devices or should we have a compatible string
>> for each device supported?
> 
> Are they each register-level compatible with one another?

They are not 100% register compatible. There are a couple fields in the
binding that are only applicable to OMAP3+ devices.

> The general recommended approach here is to make subsequent silicon
> claim compatibility with the first compatible implementation.
> 
> So, for an am3358 board:
> 	compatible = "ti,am3358-gpmc", "ti,omap2420-gpmc";
> 
> Essentially, what this means is that "ti,omap2420-gpmc" is the generic
> value instead of "omap2-gpmc". The reason for this is so that the value
> is anchored against a specific implementation, and not against something
> completely imaginary or idealized. If a newer version isn't quite
> compatible with the omap2420-gpmc, then it can drop the compatible claim
> and the driver really should be told about the new device.

Ok, gotcha! I will do a register comparison and may be recommend to
Daniel which compatible strings we will need.

Thanks!
Jon
--
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
Hunter, Jon Dec. 6, 2012, 4:22 p.m. UTC | #6
On 12/05/2012 06:03 PM, Tony Lindgren wrote:
> * Grant Likely <grant.likely@secretlab.ca> [121205 15:26]:
>> On Wed, 5 Dec 2012 16:33:48 -0600, Jon Hunter <jon-hunter@ti.com> wrote:
>>> On 12/05/2012 04:22 PM, Grant Likely wrote:
>>>>
>>>> Please, be specific. Use something like "ti,am3340-gpmc" or
>>>> "ti,omap3430-gpmc". The compatible property is a list so that new
>>>> devices can claim compatibility with old. Compatible strings that are
>>>> overly generic are a pet-peave of mine.
>>>
>>> We aim to use the binding for omap2,3,4,5 as well as the am33xx devices
>>> (which are omap based). Would it be sufficient to have "ti,omap2-gpmc"
>>> implying all omap2+ based devices or should we have a compatible string
>>> for each device supported?
>>
>> Are they each register-level compatible with one another?
>>
>> The general recommended approach here is to make subsequent silicon
>> claim compatibility with the first compatible implementation.
>>
>> So, for an am3358 board:
>> 	compatible = "ti,am3358-gpmc", "ti,omap2420-gpmc";
>>
>> Essentially, what this means is that "ti,omap2420-gpmc" is the generic
>> value instead of "omap2-gpmc". The reason for this is so that the value
>> is anchored against a specific implementation, and not against something
>> completely imaginary or idealized. If a newer version isn't quite
>> compatible with the omap2420-gpmc, then it can drop the compatible claim
>> and the driver really should be told about the new device.
> 
> The compatible property can also be used to figure out which ones
> need the workarounds in patch #4 of this series for the DT case.
> So we should be specific with the compatible.

We should not merged patch #4. Daniel included this here because he is
using this on the current mainline, however, this is not needed for
linux-next and so we should drop it.

Cheers
Jon
--
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
Daniel Mack Dec. 6, 2012, 4:54 p.m. UTC | #7
On 06.12.2012 17:22, Jon Hunter wrote:
> 
> On 12/05/2012 06:03 PM, Tony Lindgren wrote:
>> * Grant Likely <grant.likely@secretlab.ca> [121205 15:26]:
>>> On Wed, 5 Dec 2012 16:33:48 -0600, Jon Hunter <jon-hunter@ti.com> wrote:
>>>> On 12/05/2012 04:22 PM, Grant Likely wrote:
>>>>>
>>>>> Please, be specific. Use something like "ti,am3340-gpmc" or
>>>>> "ti,omap3430-gpmc". The compatible property is a list so that new
>>>>> devices can claim compatibility with old. Compatible strings that are
>>>>> overly generic are a pet-peave of mine.
>>>>
>>>> We aim to use the binding for omap2,3,4,5 as well as the am33xx devices
>>>> (which are omap based). Would it be sufficient to have "ti,omap2-gpmc"
>>>> implying all omap2+ based devices or should we have a compatible string
>>>> for each device supported?
>>>
>>> Are they each register-level compatible with one another?
>>>
>>> The general recommended approach here is to make subsequent silicon
>>> claim compatibility with the first compatible implementation.
>>>
>>> So, for an am3358 board:
>>> 	compatible = "ti,am3358-gpmc", "ti,omap2420-gpmc";
>>>
>>> Essentially, what this means is that "ti,omap2420-gpmc" is the generic
>>> value instead of "omap2-gpmc". The reason for this is so that the value
>>> is anchored against a specific implementation, and not against something
>>> completely imaginary or idealized. If a newer version isn't quite
>>> compatible with the omap2420-gpmc, then it can drop the compatible claim
>>> and the driver really should be told about the new device.
>>
>> The compatible property can also be used to figure out which ones
>> need the workarounds in patch #4 of this series for the DT case.
>> So we should be specific with the compatible.
> 
> We should not merged patch #4. Daniel included this here because he is
> using this on the current mainline, however, this is not needed for
> linux-next and so we should drop it.

I think we're talking about different things here since awhile.

The patch I pointed you which is in mainline and which removes the
reference to <plat/gpmc.h> from drivers/mtd/nand/omap2.c has nothing to
do with my patch #4. It just solves Tony's concern that regarding the
multi-arch zImages.

My code in gpmc.c calls gpmc_nand_init() which in turn calls
gpmc_hwecc_bch_capable(). Without path #4, gpmc_hwecc_bch_capable() will
return 0, and the nand init will fail consequently, in mainline as well
as in linux-next.

I understood Tony that he wanted to remove the entiry function and do
the check based on DT properties, which will then solve the problem on a
different level. However, that change is planned for *after* the merge
window.


Daniel

--
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
Daniel Mack Dec. 6, 2012, 4:59 p.m. UTC | #8
Hi Jon,

On 06.12.2012 17:19, Jon Hunter wrote:
> 
> On 12/05/2012 05:24 PM, Grant Likely wrote:
>> On Wed, 5 Dec 2012 16:33:48 -0600, Jon Hunter <jon-hunter@ti.com> wrote:
>>> Hi Grant,
>>>
>>> On 12/05/2012 04:22 PM, Grant Likely wrote:
>>>> On Wed,  5 Dec 2012 20:09:31 +0100, Daniel Mack <zonque@gmail.com> wrote:
>>>>> This patch adds basic DT bindings for OMAP GPMC.
>>>>>
>>>>> The actual peripherals are instantiated from child nodes within the GPMC
>>>>> node, and the only type of device that is currently supported is NAND.
>>>>>
>>>>> Code was added to parse the generic GPMC timing parameters and some
>>>>> documentation with examples on how to use them.
>>>>>
>>>>> Successfully tested on an AM33xx board.
>>>>>
>>>>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>>>>> ---
>>>>>  Documentation/devicetree/bindings/bus/ti-gpmc.txt  |  77 ++++++++++
>>>>>  .../devicetree/bindings/mtd/gpmc-nand.txt          |  76 +++++++++
>>>>>  arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
>>>>>  3 files changed, 323 insertions(+), 1 deletion(-)
>>>>>  create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt
>>>>>  create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/bus/ti-gpmc.txt b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
>>>>> new file mode 100644
>>>>> index 0000000..7d2a645
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
>>>>> @@ -0,0 +1,77 @@
>>>>> +Device tree bindings for OMAP general purpose memory controllers (GPMC)
>>>>> +
>>>>> +The actual devices are instantiated from the child nodes of a GPMC node.
>>>>> +
>>>>> +Required properties:
>>>>> +
>>>>> + - compatible:		Should be set to "ti,gpmc"
>>>>
>>>> Please, be specific. Use something like "ti,am3340-gpmc" or
>>>> "ti,omap3430-gpmc". The compatible property is a list so that new
>>>> devices can claim compatibility with old. Compatible strings that are
>>>> overly generic are a pet-peave of mine.
>>>
>>> We aim to use the binding for omap2,3,4,5 as well as the am33xx devices
>>> (which are omap based). Would it be sufficient to have "ti,omap2-gpmc"
>>> implying all omap2+ based devices or should we have a compatible string
>>> for each device supported?
>>
>> Are they each register-level compatible with one another?
> 
> They are not 100% register compatible. There are a couple fields in the
> binding that are only applicable to OMAP3+ devices.
> 
>> The general recommended approach here is to make subsequent silicon
>> claim compatibility with the first compatible implementation.
>>
>> So, for an am3358 board:
>> 	compatible = "ti,am3358-gpmc", "ti,omap2420-gpmc";
>>
>> Essentially, what this means is that "ti,omap2420-gpmc" is the generic
>> value instead of "omap2-gpmc". The reason for this is so that the value
>> is anchored against a specific implementation, and not against something
>> completely imaginary or idealized. If a newer version isn't quite
>> compatible with the omap2420-gpmc, then it can drop the compatible claim
>> and the driver really should be told about the new device.
> 
> Ok, gotcha! I will do a register comparison and may be recommend to
> Daniel which compatible strings we will need.

Ok, thanks a lot!

However, I don't know whether we should treat different models
differently in the code yet, distiguished by their 'compatible' string.

We should register the driver correctly so we can use the power of that
feature once we get rid of the runtime-hacks (cpu_is_xxx() macros), but
I think until we're there, we can well live with multiple
compatible-entries that all map to the same driver, right?

OTOH, we could also think about tieing num-waitpins and num-cs to a
specific 'compatible' entry (by passing a struct along in .data members
of of_device_id), but I'm not sure whether that's really better here.


Daniel

--
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
Hunter, Jon Dec. 6, 2012, 6:11 p.m. UTC | #9
On 12/06/2012 10:54 AM, Daniel Mack wrote:
> On 06.12.2012 17:22, Jon Hunter wrote:
>>
>> On 12/05/2012 06:03 PM, Tony Lindgren wrote:
>>> * Grant Likely <grant.likely@secretlab.ca> [121205 15:26]:
>>>> On Wed, 5 Dec 2012 16:33:48 -0600, Jon Hunter <jon-hunter@ti.com> wrote:
>>>>> On 12/05/2012 04:22 PM, Grant Likely wrote:
>>>>>>
>>>>>> Please, be specific. Use something like "ti,am3340-gpmc" or
>>>>>> "ti,omap3430-gpmc". The compatible property is a list so that new
>>>>>> devices can claim compatibility with old. Compatible strings that are
>>>>>> overly generic are a pet-peave of mine.
>>>>>
>>>>> We aim to use the binding for omap2,3,4,5 as well as the am33xx devices
>>>>> (which are omap based). Would it be sufficient to have "ti,omap2-gpmc"
>>>>> implying all omap2+ based devices or should we have a compatible string
>>>>> for each device supported?
>>>>
>>>> Are they each register-level compatible with one another?
>>>>
>>>> The general recommended approach here is to make subsequent silicon
>>>> claim compatibility with the first compatible implementation.
>>>>
>>>> So, for an am3358 board:
>>>> 	compatible = "ti,am3358-gpmc", "ti,omap2420-gpmc";
>>>>
>>>> Essentially, what this means is that "ti,omap2420-gpmc" is the generic
>>>> value instead of "omap2-gpmc". The reason for this is so that the value
>>>> is anchored against a specific implementation, and not against something
>>>> completely imaginary or idealized. If a newer version isn't quite
>>>> compatible with the omap2420-gpmc, then it can drop the compatible claim
>>>> and the driver really should be told about the new device.
>>>
>>> The compatible property can also be used to figure out which ones
>>> need the workarounds in patch #4 of this series for the DT case.
>>> So we should be specific with the compatible.
>>
>> We should not merged patch #4. Daniel included this here because he is
>> using this on the current mainline, however, this is not needed for
>> linux-next and so we should drop it.
> 
> I think we're talking about different things here since awhile.
> 
> The patch I pointed you which is in mainline and which removes the
> reference to <plat/gpmc.h> from drivers/mtd/nand/omap2.c has nothing to
> do with my patch #4. It just solves Tony's concern that regarding the
> multi-arch zImages.
>
> My code in gpmc.c calls gpmc_nand_init() which in turn calls
> gpmc_hwecc_bch_capable(). Without path #4, gpmc_hwecc_bch_capable() will
> return 0, and the nand init will fail consequently, in mainline as well
> as in linux-next.

Ok, yes I see that now. I should have looked more closely at linux-next.

> I understood Tony that he wanted to remove the entiry function and do
> the check based on DT properties, which will then solve the problem on a
> different level. However, that change is planned for *after* the merge
> window.

Well now that it is only being called from within the platform code and
not from drivers, it is ok.

Cheers
Jon
--
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
Daniel Mack Dec. 12, 2012, 9:13 a.m. UTC | #10
On 06.12.2012 17:19, Jon Hunter wrote:
> 
> On 12/05/2012 05:24 PM, Grant Likely wrote:
>> On Wed, 5 Dec 2012 16:33:48 -0600, Jon Hunter <jon-hunter@ti.com> wrote:
>>> Hi Grant,
>>>
>>> On 12/05/2012 04:22 PM, Grant Likely wrote:
>>>> On Wed,  5 Dec 2012 20:09:31 +0100, Daniel Mack <zonque@gmail.com> wrote:
>>>>> This patch adds basic DT bindings for OMAP GPMC.
>>>>>
>>>>> The actual peripherals are instantiated from child nodes within the GPMC
>>>>> node, and the only type of device that is currently supported is NAND.
>>>>>
>>>>> Code was added to parse the generic GPMC timing parameters and some
>>>>> documentation with examples on how to use them.
>>>>>
>>>>> Successfully tested on an AM33xx board.
>>>>>
>>>>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>>>>> ---
>>>>>  Documentation/devicetree/bindings/bus/ti-gpmc.txt  |  77 ++++++++++
>>>>>  .../devicetree/bindings/mtd/gpmc-nand.txt          |  76 +++++++++
>>>>>  arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
>>>>>  3 files changed, 323 insertions(+), 1 deletion(-)
>>>>>  create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt
>>>>>  create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/bus/ti-gpmc.txt b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
>>>>> new file mode 100644
>>>>> index 0000000..7d2a645
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
>>>>> @@ -0,0 +1,77 @@
>>>>> +Device tree bindings for OMAP general purpose memory controllers (GPMC)
>>>>> +
>>>>> +The actual devices are instantiated from the child nodes of a GPMC node.
>>>>> +
>>>>> +Required properties:
>>>>> +
>>>>> + - compatible:		Should be set to "ti,gpmc"
>>>>
>>>> Please, be specific. Use something like "ti,am3340-gpmc" or
>>>> "ti,omap3430-gpmc". The compatible property is a list so that new
>>>> devices can claim compatibility with old. Compatible strings that are
>>>> overly generic are a pet-peave of mine.
>>>
>>> We aim to use the binding for omap2,3,4,5 as well as the am33xx devices
>>> (which are omap based). Would it be sufficient to have "ti,omap2-gpmc"
>>> implying all omap2+ based devices or should we have a compatible string
>>> for each device supported?
>>
>> Are they each register-level compatible with one another?
> 
> They are not 100% register compatible. There are a couple fields in the
> binding that are only applicable to OMAP3+ devices.
> 
>> The general recommended approach here is to make subsequent silicon
>> claim compatibility with the first compatible implementation.
>>
>> So, for an am3358 board:
>> 	compatible = "ti,am3358-gpmc", "ti,omap2420-gpmc";
>>
>> Essentially, what this means is that "ti,omap2420-gpmc" is the generic
>> value instead of "omap2-gpmc". The reason for this is so that the value
>> is anchored against a specific implementation, and not against something
>> completely imaginary or idealized. If a newer version isn't quite
>> compatible with the omap2420-gpmc, then it can drop the compatible claim
>> and the driver really should be told about the new device.
> 
> Ok, gotcha! I will do a register comparison and may be recommend to
> Daniel which compatible strings we will need.

Any idea yet how we want to continue on this? I'm asking because I'm
leaving for a longer trip by the end of this week, and so anything I
haven't finished until then will have to be postponed until February or
be taken over by someone else :)

Thanks,
Daniel

--
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
Hunter, Jon Dec. 12, 2012, 11:02 p.m. UTC | #11
On 12/12/2012 03:13 AM, Daniel Mack wrote:
> On 06.12.2012 17:19, Jon Hunter wrote:
>>
>> On 12/05/2012 05:24 PM, Grant Likely wrote:
>>> On Wed, 5 Dec 2012 16:33:48 -0600, Jon Hunter <jon-hunter@ti.com> wrote:
>>>> Hi Grant,
>>>>
>>>> On 12/05/2012 04:22 PM, Grant Likely wrote:
>>>>> On Wed,  5 Dec 2012 20:09:31 +0100, Daniel Mack <zonque@gmail.com> wrote:
>>>>>> This patch adds basic DT bindings for OMAP GPMC.
>>>>>>
>>>>>> The actual peripherals are instantiated from child nodes within the GPMC
>>>>>> node, and the only type of device that is currently supported is NAND.
>>>>>>
>>>>>> Code was added to parse the generic GPMC timing parameters and some
>>>>>> documentation with examples on how to use them.
>>>>>>
>>>>>> Successfully tested on an AM33xx board.
>>>>>>
>>>>>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>>>>>> ---
>>>>>>  Documentation/devicetree/bindings/bus/ti-gpmc.txt  |  77 ++++++++++
>>>>>>  .../devicetree/bindings/mtd/gpmc-nand.txt          |  76 +++++++++
>>>>>>  arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
>>>>>>  3 files changed, 323 insertions(+), 1 deletion(-)
>>>>>>  create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt
>>>>>>  create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/bus/ti-gpmc.txt b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
>>>>>> new file mode 100644
>>>>>> index 0000000..7d2a645
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
>>>>>> @@ -0,0 +1,77 @@
>>>>>> +Device tree bindings for OMAP general purpose memory controllers (GPMC)
>>>>>> +
>>>>>> +The actual devices are instantiated from the child nodes of a GPMC node.
>>>>>> +
>>>>>> +Required properties:
>>>>>> +
>>>>>> + - compatible:		Should be set to "ti,gpmc"
>>>>>
>>>>> Please, be specific. Use something like "ti,am3340-gpmc" or
>>>>> "ti,omap3430-gpmc". The compatible property is a list so that new
>>>>> devices can claim compatibility with old. Compatible strings that are
>>>>> overly generic are a pet-peave of mine.
>>>>
>>>> We aim to use the binding for omap2,3,4,5 as well as the am33xx devices
>>>> (which are omap based). Would it be sufficient to have "ti,omap2-gpmc"
>>>> implying all omap2+ based devices or should we have a compatible string
>>>> for each device supported?
>>>
>>> Are they each register-level compatible with one another?
>>
>> They are not 100% register compatible. There are a couple fields in the
>> binding that are only applicable to OMAP3+ devices.
>>
>>> The general recommended approach here is to make subsequent silicon
>>> claim compatibility with the first compatible implementation.
>>>
>>> So, for an am3358 board:
>>> 	compatible = "ti,am3358-gpmc", "ti,omap2420-gpmc";
>>>
>>> Essentially, what this means is that "ti,omap2420-gpmc" is the generic
>>> value instead of "omap2-gpmc". The reason for this is so that the value
>>> is anchored against a specific implementation, and not against something
>>> completely imaginary or idealized. If a newer version isn't quite
>>> compatible with the omap2420-gpmc, then it can drop the compatible claim
>>> and the driver really should be told about the new device.
>>
>> Ok, gotcha! I will do a register comparison and may be recommend to
>> Daniel which compatible strings we will need.
> 
> Any idea yet how we want to continue on this? I'm asking because I'm
> leaving for a longer trip by the end of this week, and so anything I
> haven't finished until then will have to be postponed until February or
> be taken over by someone else :)

Thanks for the reminder!

So looking at this today, here is what I see when comparing the
registers ...

omap2430 != omap2420
omap3430 != omap2430
omap3630 == omap3430
omap4430 != omap3430
omap4460 == omap4430
omap543x == omap4430
am335x != omap4430

Therefore, I believe that we need to have the following compatible
strings ...

ti,omap2420-gpmc
ti,omap2430-gpmc
ti,omap3430-gpmc (omap3430 & omap3630)
ti,omap4430-gpmc (omap4430 & omap4460 & omap543x)
ti,am3352-gpmc (am335x devices)

Probably worth adding a comment to the Documentation what should be used
for which device.

Cheers
Jon



--
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
Grant Likely Dec. 15, 2012, 12:37 a.m. UTC | #12
On Wed, 12 Dec 2012 17:02:10 -0600, Jon Hunter <jon-hunter@ti.com> wrote:
> 
> So looking at this today, here is what I see when comparing the
> registers ...
> 
> omap2430 != omap2420
> omap3430 != omap2430
> omap3630 == omap3430
> omap4430 != omap3430
> omap4460 == omap4430
> omap543x == omap4430
> am335x != omap4430
> 
> Therefore, I believe that we need to have the following compatible
> strings ...
> 
> ti,omap2420-gpmc
> ti,omap2430-gpmc
> ti,omap3430-gpmc (omap3430 & omap3630)
> ti,omap4430-gpmc (omap4430 & omap4460 & omap543x)
> ti,am3352-gpmc (am335x devices)
> 
> Probably worth adding a comment to the Documentation what should be used
> for which device.

Yes, it is completely appropriate to add a comment to the documentation
here so this doesn't get lost.

g.

--
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
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/bus/ti-gpmc.txt b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
new file mode 100644
index 0000000..7d2a645
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/ti-gpmc.txt
@@ -0,0 +1,77 @@ 
+Device tree bindings for OMAP general purpose memory controllers (GPMC)
+
+The actual devices are instantiated from the child nodes of a GPMC node.
+
+Required properties:
+
+ - compatible:		Should be set to "ti,gpmc"
+ - reg:			A resource specifier for the register space
+			(see the example below)
+ - ti,hwmods:		Should be set to "ti,gpmc" until the DT transition is
+			completed.
+ - #address-cells:	Must be set to 2 to allow memory address translation
+ - #size-cells:		Must be set to 1 to allow CS address passing
+ - num-cs:		The maximum number of chip-select lines that controller
+			can support.
+ - num-waitpins:	The maximum number of wait pins that controller can
+			support.
+ - ranges:		Must be set up to reflect the memory layout with four
+			integer values for each chip-select line in use:
+
+			   <cs-number> 0 <physical address of mapping> <size>
+
+			Note that this property is not currently parsed.
+			Calculated values derived from the contents of the
+			per-CS register GPMC_CONFIG7 (as set up by the
+			bootloader) are used. That will change in the future,
+			so be sure to fill the correct values here.
+
+Timing properties for child nodes. All are optional and default to 0.
+
+ - gpmc,sync-clk:	Minimum clock period for synchronous mode, in picoseconds
+
+ Chip-select signal timings corresponding to GPMC_CONFIG2:
+ - gpmc,cs-on:		Assertion time
+ - gpmc,cs-rd-off:	Read deassertion time
+ - gpmc,cs-wr-off:	Write deassertion time
+
+ ADV signal timings corresponding to GPMC_CONFIG3:
+ - gpmc,adv-on:		Assertion time
+ - gpmc,adv-rd-off:	Read deassertion time
+ - gpmc,adv-wr-off:	Write deassertion time
+
+ WE signals timings corresponding to GPMC_CONFIG4:
+ - gpmc,we-on:		Assertion time
+ - gpmc,we-off:		Deassertion time
+
+ OE signals timings corresponding to GPMC_CONFIG4:
+ - gpmc,oe-on:		Assertion time
+ - gpmc,oe-off:		Deassertion time
+
+ Access time and cycle time timings corresponding to GPMC_CONFIG5:
+ - gpmc,page-burst-access: Multiple access word delay
+ - gpmc,access:		Start-cycle to first data valid delay
+ - gpmc,rd-cycle:	Total read cycle time
+ - gpmc,wr-cycle:	Total write cycle time
+
+The following are only applicable to OMAP3+ and AM335x:
+ - gpmc,wr-access
+ - gpmc,wr-data-mux-bus
+
+
+Example for an AM33xx board:
+
+	gpmc: gpmc@50000000 {
+		compatible = "ti,gpmc";
+		ti,hwmods = "gpmc";
+		reg = <0x50000000 0x2000>;
+		interrupts = <100>;
+
+		num-cs = <8>;
+		num-waitpins = <2>;
+		#address-cells = <2>;
+		#size-cells = <1>;
+		ranges = <0 0 0x08000000 0x10000000>; /* CS0 @addr 0x8000000, size 0x10000000 */
+
+		/* child nodes go here */
+	};
diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
new file mode 100644
index 0000000..b3fafb1
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
@@ -0,0 +1,76 @@ 
+Device tree bindings for GPMC connected NANDs
+
+GPMC connected NAND (found on OMAP boards) are represented as child nodes of
+the GPMC controller with a name of "nand".
+
+All timing relevant properties as well as generic gpmc child properties are
+explained in a separate documents - please refer to
+Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+For NAND specific properties such as ECC modes or bus width, please refer to
+Documentation/devicetree/bindings/mtd/nand.txt
+
+
+Required properties:
+
+ - reg:		The CS line the peripheral is connected to
+
+Optional properties:
+
+ - nand-bus-width: 		Set this numeric value to 16 if the hardware
+				is wired that way. If not specified, a bus
+				width of 8 is assumed.
+
+ - ti,nand-ecc-opt:		A string setting the ECC layout to use. One of:
+
+		"sw"		Software method (default)
+		"hw"		Hardware method
+		"hw-romcode"	gpmc hamming mode method & romcode layout
+		"bch4"		4-bit BCH ecc code
+		"bch8"		8-bit BCH ecc code
+
+For inline partiton table parsing (optional):
+
+ - #address-cells: should be set to 1
+ - #size-cells: should be set to 1
+
+Example for an AM33xx board:
+
+	gpmc: gpmc@50000000 {
+		compatible = "ti,gpmc";
+		ti,hwmods = "gpmc";
+		reg = <0x50000000 0x1000000>;
+		interrupts = <100>;
+		num-cs = <8>;
+		num-waitpins = <2>;
+		#address-cells = <2>;
+		#size-cells = <1>;
+		ranges = <0 0 0x08000000 0x2000>;	/* CS0: NAND */
+
+		nand@0,0 {
+			reg = <0 0 0>; /* CS0, offset 0 */
+			nand-bus-width = <16>;
+			ti,nand-ecc-opt = "bch8";
+
+			gpmc,sync-clk = <0>;
+			gpmc,cs-on = <0>;
+			gpmc,cs-rd-off = <44>;
+			gpmc,cs-wr-off = <44>;
+			gpmc,adv-on = <6>;
+			gpmc,adv-rd-off = <34>;
+			gpmc,adv-wr-off = <44>;
+			gpmc,we-off = <40>;
+			gpmc,oe-off = <54>;
+			gpmc,access = <64>;
+			gpmc,rd-cycle = <82>;
+			gpmc,wr-cycle = <82>;
+			gpmc,wr-access = <40>;
+			gpmc,wr-data-mux-bus = <0>;
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			/* partitions go here */
+		};
+	};
+
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index d5cbd29..0baf9df 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -25,6 +25,10 @@ 
 #include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_mtd.h>
+#include <linux/of_device.h>
+#include <linux/mtd/nand.h>
 
 #include <linux/platform_data/mtd-nand-omap2.h>
 
@@ -34,6 +38,7 @@ 
 #include "common.h"
 #include "omap_device.h"
 #include "gpmc.h"
+#include "gpmc-nand.h"
 
 #define	DEVICE_NAME		"omap-gpmc"
 
@@ -1121,7 +1126,162 @@  int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
 	return 0;
 }
 
-static __devinit int gpmc_probe(struct platform_device *pdev)
+#ifdef CONFIG_OF
+static struct of_device_id gpmc_dt_ids[] = {
+	{ .compatible = "ti,gpmc" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, gpmc_dt_ids);
+
+static void __maybe_unused gpmc_read_timings_dt(struct device_node *np,
+						struct gpmc_timings *gpmc_t)
+{
+	u32 val;
+
+	memset(gpmc_t, 0, sizeof(*gpmc_t));
+
+	/* minimum clock period for syncronous mode */
+	if (!of_property_read_u32(np, "gpmc,sync-clk", &val))
+		gpmc_t->sync_clk = val;
+
+	/* chip select timtings */
+	if (!of_property_read_u32(np, "gpmc,cs-on", &val))
+		gpmc_t->cs_on = val;
+
+	if (!of_property_read_u32(np, "gpmc,cs-rd-off", &val))
+		gpmc_t->cs_rd_off = val;
+
+	if (!of_property_read_u32(np, "gpmc,cs-wr-off", &val))
+		gpmc_t->cs_wr_off = val;
+
+	/* ADV signal timings */
+	if (!of_property_read_u32(np, "gpmc,adv-on", &val))
+		gpmc_t->adv_on = val;
+
+	if (!of_property_read_u32(np, "gpmc,adv-rd-off", &val))
+		gpmc_t->adv_rd_off = val;
+
+	if (!of_property_read_u32(np, "gpmc,adv-wr-off", &val))
+		gpmc_t->adv_wr_off = val;
+
+	/* WE signal timings */
+	if (!of_property_read_u32(np, "gpmc,we-on", &val))
+		gpmc_t->we_on = val;
+
+	if (!of_property_read_u32(np, "gpmc,we-off", &val))
+		gpmc_t->we_off = val;
+
+	/* OE signal timings */
+	if (!of_property_read_u32(np, "gpmc,oe-on", &val))
+		gpmc_t->oe_on = val;
+
+	if (!of_property_read_u32(np, "gpmc,oe-off", &val))
+		gpmc_t->oe_off = val;
+
+	/* access and cycle timings */
+	if (!of_property_read_u32(np, "gpmc,page-burst-access", &val))
+		gpmc_t->page_burst_access = val;
+
+	if (!of_property_read_u32(np, "gpmc,access", &val))
+		gpmc_t->access = val;
+
+	if (!of_property_read_u32(np, "gpmc,rd-cycle", &val))
+		gpmc_t->rd_cycle = val;
+
+	if (!of_property_read_u32(np, "gpmc,wr-cycle", &val))
+		gpmc_t->wr_cycle = val;
+
+	/* only for OMAP3430 */
+	if (!of_property_read_u32(np, "gpmc,wr-access", &val))
+		gpmc_t->wr_access = val;
+
+	if (!of_property_read_u32(np, "gpmc,wr-data-mux-bus", &val))
+		gpmc_t->wr_data_mux_bus = val;
+}
+
+#ifdef CONFIG_MTD_NAND
+
+static const char * const nand_ecc_opts[] = {
+	[OMAP_ECC_HAMMING_CODE_DEFAULT]		= "sw",
+	[OMAP_ECC_HAMMING_CODE_HW]		= "hw",
+	[OMAP_ECC_HAMMING_CODE_HW_ROMCODE]	= "hw-romcode",
+	[OMAP_ECC_BCH4_CODE_HW]			= "bch4",
+	[OMAP_ECC_BCH8_CODE_HW]			= "bch8",
+};
+
+static int gpmc_probe_nand_child(struct platform_device *pdev,
+				 struct device_node *child)
+{
+	u32 val;
+	const char *s;
+	struct gpmc_timings gpmc_t;
+	struct omap_nand_platform_data *gpmc_nand_data;
+
+	if (of_property_read_u32(child, "reg", &val) < 0) {
+		dev_err(&pdev->dev, "%s has no 'reg' property\n",
+			child->full_name);
+		return -ENODEV;
+	}
+
+	gpmc_nand_data = devm_kzalloc(&pdev->dev, sizeof(*gpmc_nand_data),
+				      GFP_KERNEL);
+	if (!gpmc_nand_data)
+		return -ENOMEM;
+
+	gpmc_nand_data->cs = val;
+	gpmc_nand_data->of_node = child;
+
+	if (!of_property_read_string(child, "ti,nand-ecc-opt", &s))
+		for (val = 0; val < ARRAY_SIZE(nand_ecc_opts); val++)
+			if (!strcasecmp(s, nand_ecc_opts[val])) {
+				gpmc_nand_data->ecc_opt = val;
+				break;
+			}
+
+	val = of_get_nand_bus_width(child);
+	if (val == 16)
+		gpmc_nand_data->devsize = NAND_BUSWIDTH_16;
+
+	gpmc_read_timings_dt(child, &gpmc_t);
+	gpmc_nand_init(gpmc_nand_data, &gpmc_t);
+
+	return 0;
+}
+#else
+static int gpmc_probe_nand_child(struct platform_device *pdev,
+				 struct device_node *child)
+{
+	return 0;
+}
+#endif
+
+static int gpmc_probe_dt(struct platform_device *pdev)
+{
+	int ret;
+	struct device_node *child;
+	const struct of_device_id *of_id =
+		of_match_device(gpmc_dt_ids, &pdev->dev);
+
+	if (!of_id)
+		return 0;
+
+	for_each_node_by_name(child, "nand") {
+		ret = gpmc_probe_nand_child(pdev, child);
+		of_node_put(child);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+#else
+static int gpmc_probe_dt(struct platform_device *pdev)
+{
+	return 0;
+}
+#endif
+
+static int __devinit gpmc_probe(struct platform_device *pdev)
 {
 	int rc;
 	u32 l;
@@ -1174,6 +1334,14 @@  static __devinit int gpmc_probe(struct platform_device *pdev)
 	if (IS_ERR_VALUE(gpmc_setup_irq()))
 		dev_warn(gpmc_dev, "gpmc_setup_irq failed\n");
 
+	rc = gpmc_probe_dt(pdev);
+	if (rc < 0) {
+		clk_disable_unprepare(gpmc_l3_clk);
+		clk_put(gpmc_l3_clk);
+		dev_err(gpmc_dev, "failed to probe DT parameters\n");
+		return rc;
+	}
+
 	return 0;
 }
 
@@ -1191,6 +1359,7 @@  static struct platform_driver gpmc_driver = {
 	.driver		= {
 		.name	= DEVICE_NAME,
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(gpmc_dt_ids),
 	},
 };