Representation of external memory-mapped devices in DT (gpmc)

Representation of external memory-mapped devices in DT (gpmc)

[Daniel Mack]

Hi,

we're currently working on a DT binding for the GPMC bus that is found
on SoCs by TI. The implementation is based on CS lines and an 8, 16 or
32 bit parallel interface. That IP is quite flexible, and it can for
example be used for physmap flash, external peripherals or even NAND.

Depending on which CS is used to control the device, different memory
regions are reserved, and there's code to calculate the location and
size of them, given a CS number (see arch/arm/mach-omap2/gpmc.c).

The binding will use one top-level node to describe the GPMC controller
itself and register the actual devices as sub-nodes to it. The NAND type
is the only one that is currently supported. This is how it currently looks:

	gpmc: gpmc at 50000000 {
		compatible = "ti,gpmc";
		ti,hwmods = "gpmc";
		reg = <0x50000000 0x2000>;
		interrupt-parent = <&intc>;
		interrupts = <100>;
		#address-cells = <1>;
		#size-cells = <0>;

		nand at 0 {
			reg = <0>; /* CS0 */
			nand-bus-width = <16>;
			nand-ecc-mode = "soft";

			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>;

			partition at 0 {
				label = "1st";
				reg = <0x0 0x20000>;
			};
			/* more partitions ... */
		};
	};

The question is where the resource location and sizes should be
described so that the code that does the magic run-time calculations can
be removed eventually. I would clearly prefer not to have them in the
child, as the only thing these nodes really care about is the chip
select index the hardware is wired to.

Should the "reg" property in the parent be augmented to hold such details?

Once we got that sorted out, I'll do a re-spin of the series and copy
devicetree-discuss on the patch that adds the bindings.


Thanks,
Daniel

Representation of external memory-mapped devices in DT (gpmc)

[Rob Herring]

On 10/29/2012 09:39 AM, Daniel Mack wrote:
> Hi,
> 
> we're currently working on a DT binding for the GPMC bus that is found
> on SoCs by TI. The implementation is based on CS lines and an 8, 16 or
> 32 bit parallel interface. That IP is quite flexible, and it can for
> example be used for physmap flash, external peripherals or even NAND.
> 
> Depending on which CS is used to control the device, different memory
> regions are reserved, and there's code to calculate the location and
> size of them, given a CS number (see arch/arm/mach-omap2/gpmc.c).

I don't know the details of the h/w, but I would think the DT core code
should be able work out the addresses. This can be done using ranges
property which defines the mapping of a child bus into the parent bus
addresses.

> The binding will use one top-level node to describe the GPMC controller
> itself and register the actual devices as sub-nodes to it. The NAND type
> is the only one that is currently supported. This is how it currently looks:
> 
> 	gpmc: gpmc at 50000000 {
> 		compatible = "ti,gpmc";
> 		ti,hwmods = "gpmc";
> 		reg = <0x50000000 0x2000>;
> 		interrupt-parent = <&intc>;
> 		interrupts = <100>;
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		nand at 0 {

You may want a CS0 node with nand as a child node of that.

Rob

> 			reg = <0>; /* CS0 */
> 			nand-bus-width = <16>;
> 			nand-ecc-mode = "soft";
> 
> 			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>;
> 
> 			partition at 0 {
> 				label = "1st";
> 				reg = <0x0 0x20000>;
> 			};
> 			/* more partitions ... */
> 		};
> 	};
> 
> The question is where the resource location and sizes should be
> described so that the code that does the magic run-time calculations can
> be removed eventually. I would clearly prefer not to have them in the
> child, as the only thing these nodes really care about is the chip
> select index the hardware is wired to.
> 
> Should the "reg" property in the parent be augmented to hold such details?
> 
> Once we got that sorted out, I'll do a re-spin of the series and copy
> devicetree-discuss on the patch that adds the bindings.
> 
> 
> Thanks,
> Daniel
> 

Representation of external memory-mapped devices in DT (gpmc)

[Daniel Mack]

On 29.10.2012 16:09, Rob Herring wrote:
> On 10/29/2012 09:39 AM, Daniel Mack wrote:
>> Hi,
>>
>> we're currently working on a DT binding for the GPMC bus that is found
>> on SoCs by TI. The implementation is based on CS lines and an 8, 16 or
>> 32 bit parallel interface. That IP is quite flexible, and it can for
>> example be used for physmap flash, external peripherals or even NAND.
>>
>> Depending on which CS is used to control the device, different memory
>> regions are reserved, and there's code to calculate the location and
>> size of them, given a CS number (see arch/arm/mach-omap2/gpmc.c).
> 
> I don't know the details of the h/w, but I would think the DT core code
> should be able work out the addresses. This can be done using ranges
> property which defines the mapping of a child bus into the parent bus
> addresses.

In this case, the controller is @0x50000000 while the external device is
mapped to address 0x0.

>> The binding will use one top-level node to describe the GPMC controller
>> itself and register the actual devices as sub-nodes to it. The NAND type
>> is the only one that is currently supported. This is how it currently looks:
>>
>> 	gpmc: gpmc at 50000000 {
>> 		compatible = "ti,gpmc";
>> 		ti,hwmods = "gpmc";
>> 		reg = <0x50000000 0x2000>;
>> 		interrupt-parent = <&intc>;
>> 		interrupts = <100>;
>> 		#address-cells = <1>;
>> 		#size-cells = <0>;
>>
>> 		nand at 0 {
> 
> You may want a CS0 node with nand as a child node of that.

Hmm, I don't see what that would buy us. The question is which way is
feasible for storing both the memory region and the cs number in the
device tree. The CS number should certainly go to the child node, no?

IOW, would it be a good idea to have something like the following layout?

	gpmc: gpmc at 50000000 {
		compatible = "ti,gpmc";
		ti,hwmods = "gpmc";
		reg = <0x50000000 0x2000>;

		/* cs-reg stores the setup of the controller's
		   memory map */

			/* offset	size */
		cs-reg = <0x0		0x1000000
			  ....		.....
			  ....		.....>;

		nand: child at 0 {
			/* timings */
			/* peripheral specifics */
		};
	};

I would actually much prefer that approach.

Afzal, because because that way, we can leave the code as-is for now and
add the "cs-reg" property once the code is switched to dynamic handling.
What do you think?


Thanks,
Daniel

Representation of external memory-mapped devices in DT (gpmc)

[Rob Herring]

On 10/29/2012 12:22 PM, Daniel Mack wrote:
> On 29.10.2012 16:09, Rob Herring wrote:
>> On 10/29/2012 09:39 AM, Daniel Mack wrote:
>>> Hi,
>>>
>>> we're currently working on a DT binding for the GPMC bus that is found
>>> on SoCs by TI. The implementation is based on CS lines and an 8, 16 or
>>> 32 bit parallel interface. That IP is quite flexible, and it can for
>>> example be used for physmap flash, external peripherals or even NAND.
>>>
>>> Depending on which CS is used to control the device, different memory
>>> regions are reserved, and there's code to calculate the location and
>>> size of them, given a CS number (see arch/arm/mach-omap2/gpmc.c).
>>
>> I don't know the details of the h/w, but I would think the DT core code
>> should be able work out the addresses. This can be done using ranges
>> property which defines the mapping of a child bus into the parent bus
>> addresses.
> 
> In this case, the controller is @0x50000000 while the external device is
> mapped to address 0x0.

Okay, so it's not fixed or some sub-range of the memory map.

>>> The binding will use one top-level node to describe the GPMC controller
>>> itself and register the actual devices as sub-nodes to it. The NAND type
>>> is the only one that is currently supported. This is how it currently looks:
>>>
>>> 	gpmc: gpmc at 50000000 {
>>> 		compatible = "ti,gpmc";
>>> 		ti,hwmods = "gpmc";
>>> 		reg = <0x50000000 0x2000>;
>>> 		interrupt-parent = <&intc>;
>>> 		interrupts = <100>;
>>> 		#address-cells = <1>;
>>> 		#size-cells = <0>;
>>>
>>> 		nand at 0 {
>>
>> You may want a CS0 node with nand as a child node of that.
> 
> Hmm, I don't see what that would buy us. The question is which way is
> feasible for storing both the memory region and the cs number in the
> device tree. The CS number should certainly go to the child node, no?

I was thinking of if you had per CS properties you needed to setup each
CS. So something like this (using non-zero address to show the
translation better):

gpmc {
	compatible = "ti,gpmc", "simple-bus";
	ti,hwmods = "gpmc";
	reg = <0x50000000 0x2000>;

	CS0 {
		// map 1MB @ gpmc offset 0 to host address 0x10000000
		ranges = <0x10000000 0x0 <0x100000>;
		// other gpmc specific per CS properties.
		nand {
			reg = <0x0 0x100000>;
		}
	};


The core code will look at parents' ranges properties to translate the
local offset of 0 into host address of 0x10000000 Then the nand node's
reg address will get translated to 0x10000000.

The gpmc code can then use the ranges properties to setup the mapping of
each chip select to the host address. You don't need a new property to
describe the cs mapping.

Rob

> 
> IOW, would it be a good idea to have something like the following layout?
> 
> 	gpmc: gpmc at 50000000 {
> 		compatible = "ti,gpmc";
> 		ti,hwmods = "gpmc";
> 		reg = <0x50000000 0x2000>;
> 
> 		/* cs-reg stores the setup of the controller's
> 		   memory map */
> 
> 			/* offset	size */
> 		cs-reg = <0x0		0x1000000
> 			  ....		.....
> 			  ....		.....>;
> 
> 		nand: child at 0 {
> 			/* timings */
> 			/* peripheral specifics */
> 		};
> 	};
> 
> I would actually much prefer that approach.
> 
> Afzal, because because that way, we can leave the code as-is for now and
> add the "cs-reg" property once the code is switched to dynamic handling.
> What do you think?
> 
> 
> Thanks,
> Daniel
> 

Representation of external memory-mapped devices in DT (gpmc)

[Daniel Mack]

Hi Rob,

On Oct 29, 2012 10:49 PM, "Rob Herring" <robherring2@gmail.com> wrote:
>
> On 10/29/2012 12:22 PM, Daniel Mack wrote:
> > On 29.10.2012 16:09, Rob Herring wrote:
> >> On 10/29/2012 09:39 AM, Daniel Mack wrote:
> >>> Hi,
> >>>
> >>> we're currently working on a DT binding for the GPMC bus that is found
> >>> on SoCs by TI. The implementation is based on CS lines and an 8, 16 or
> >>> 32 bit parallel interface. That IP is quite flexible, and it can for
> >>> example be used for physmap flash, external peripherals or even NAND.
> >>>
> >>> Depending on which CS is used to control the device, different memory
> >>> regions are reserved, and there's code to calculate the location and
> >>> size of them, given a CS number (see arch/arm/mach-omap2/gpmc.c).
> >>
> >> I don't know the details of the h/w, but I would think the DT core code
> >> should be able work out the addresses. This can be done using ranges
> >> property which defines the mapping of a child bus into the parent bus
> >> addresses.
> >
> > In this case, the controller is @0x50000000 while the external device is
> > mapped to address 0x0.
>
> Okay, so it's not fixed or some sub-range of the memory map.

It still haz a fixed position i  the memory map (0 - 0x1fffffff), but how
that space is divided for peripherals depends on the configuration.

>
> >>> The binding will use one top-level node to describe the GPMC
controller
> >>> itself and register the actual devices as sub-nodes to it. The NAND
type
> >>> is the only one that is currently supported. This is how it currently
looks:
> >>>
> >>>     gpmc: gpmc at 50000000 {
> >>>             compatible = "ti,gpmc";
> >>>             ti,hwmods = "gpmc";
> >>>             reg = <0x50000000 0x2000>;
> >>>             interrupt-parent = <&intc>;
> >>>             interrupts = <100>;
> >>>             #address-cells = <1>;
> >>>             #size-cells = <0>;
> >>>
> >>>             nand at 0 {
> >>
> >> You may want a CS0 node with nand as a child node of that.
> >
> > Hmm, I don't see what that would buy us. The question is which way is
> > feasible for storing both the memory region and the cs number in the
> > device tree. The CS number should certainly go to the child node, no?
>
> I was thinking of if you had per CS properties you needed to setup each
> CS. So something like this (using non-zero address to show the
> translation better):
>
> gpmc {
>         compatible = "ti,gpmc", "simple-bus";
>         ti,hwmods = "gpmc";
>         reg = <0x50000000 0x2000>;
>
>         CS0 {
>                 // map 1MB @ gpmc offset 0 to host address 0x10000000
>                 ranges = <0x10000000 0x0 <0x100000>;

The most important bit has a syntax error ;-) Was that property meant to
carry 3 values?

>                 // other gpmc specific per CS properties.
>                 nand {
>                         reg = <0x0 0x100000>;
>                 }
>         };
>

so that way, the child has the awareness about the gpmc controller config
(the offset in the memory map), and is agnostic to the cs number in use,
right? I think this is not really the most convenient approach from a
user's perspective.

> The core code will look at parents' ranges properties to translate the
> local offset of 0 into host address of 0x10000000 Then the nand node's
> reg address will get translated to 0x10000000.

That part is understood, I'm just not sure whether this is the most useful
way of describing the h/w capabilities.

Thanks,
Daniel

>
> The gpmc code can then use the ranges properties to setup the mapping of
> each chip select to the host address. You don't need a new property to
> describe the cs mapping.
>
> Rob
>
> >
> > IOW, would it be a good idea to have something like the following
layout?
> >
> >       gpmc: gpmc at 50000000 {
> >               compatible = "ti,gpmc";
> >               ti,hwmods = "gpmc";
> >               reg = <0x50000000 0x2000>;
> >
> >               /* cs-reg stores the setup of the controller's
> >                  memory map */
> >
> >                       /* offset       size */
> >               cs-reg = <0x0           0x1000000
> >                         ....          .....
> >                         ....          .....>;
> >
> >               nand: child at 0 {
> >                       /* timings */
> >                       /* peripheral specifics */
> >               };
> >       };
> >
> > I would actually much prefer that approach.
> >
> > Afzal, because because that way, we can leave the code as-is for now and
> > add the "cs-reg" property once the code is switched to dynamic handling.
> > What do you think?
> >
> >
> > Thanks,
> > Daniel
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121029/0729e0f3/attachment-0001.html>

Representation of external memory-mapped devices in DT (gpmc)

[Afzal Mohammed]

Hi Daniel,

On Monday 29 October 2012 10:52 PM, Daniel Mack wrote:
> On 29.10.2012 16:09, Rob Herring wrote:

>> You may want a CS0 node with nand as a child node of that.

> Hmm, I don't see what that would buy us. The question is which way is
> feasible for storing both the memory region and the cs number in the
> device tree. The CS number should certainly go to the child node, no?
>
> IOW, would it be a good idea to have something like the following layout?
>
> 	gpmc: gpmc at 50000000 {
> 		compatible = "ti,gpmc";
> 		ti,hwmods = "gpmc";
> 		reg =<0x50000000 0x2000>;
>
> 		/* cs-reg stores the setup of the controller's
> 		   memory map */
>
> 			/* offset	size */
> 		cs-reg =<0x0		0x1000000
> 			  ....		.....
> 			  ....		.....>;
>
> 		nand: child at 0 {
> 			/* timings */
> 			/* peripheral specifics */
> 		};
> 	};
>
> I would actually much prefer that approach.
>
> Afzal, because because that way, we can leave the code as-is for now and
> add the "cs-reg" property once the code is switched to dynamic handling.
> What do you think?

I don't know what to say, don't have a good grasp on DT to give
right suggestion.

It seems offset field may not be necessary. memory for connected
peripherals is not fixed, only CS is fixed (as CS pin is hard-wired).
Physical memory can be anywhere between 0-512MB (with
alignment constraints depending on size, refer GPMC_CONFIG7
register), even though right now memory region for peripheral
seems to be fixed (for boards supported in mainline it will be
what bootloader configures), it is possible to have it in a different
region for those peripherals.

And this memory should be presented to child device as resource,
say for smsc911x ethernet driver.

I see some similarity with PCI - BAR's, where address space
is configured (here a difference would be that size needs to
be known while configuring). Size here would be based on
connected peripheral (but no auto detection)

Regards
Afzal

Representation of external memory-mapped devices in DT (gpmc)

[Daniel Mack]

On 30.10.2012 11:50, Afzal Mohammed wrote:
> Hi Daniel,
> 
> On Monday 29 October 2012 10:52 PM, Daniel Mack wrote:
>> On 29.10.2012 16:09, Rob Herring wrote:
> 
>>> You may want a CS0 node with nand as a child node of that.
> 
>> Hmm, I don't see what that would buy us. The question is which way is
>> feasible for storing both the memory region and the cs number in the
>> device tree. The CS number should certainly go to the child node, no?
>>
>> IOW, would it be a good idea to have something like the following layout?
>>
>> 	gpmc: gpmc at 50000000 {
>> 		compatible = "ti,gpmc";
>> 		ti,hwmods = "gpmc";
>> 		reg =<0x50000000 0x2000>;
>>
>> 		/* cs-reg stores the setup of the controller's
>> 		   memory map */
>>
>> 			/* offset	size */
>> 		cs-reg =<0x0		0x1000000
>> 			  ....		.....
>> 			  ....		.....>;
>>
>> 		nand: child at 0 {
>> 			/* timings */
>> 			/* peripheral specifics */
>> 		};
>> 	};
>>
>> I would actually much prefer that approach.
>>
>> Afzal, because because that way, we can leave the code as-is for now and
>> add the "cs-reg" property once the code is switched to dynamic handling.
>> What do you think?
> 
> I don't know what to say, don't have a good grasp on DT to give
> right suggestion.
> 
> It seems offset field may not be necessary. memory for connected
> peripherals is not fixed, only CS is fixed (as CS pin is hard-wired).
> Physical memory can be anywhere between 0-512MB (with
> alignment constraints depending on size, refer GPMC_CONFIG7
> register), even though right now memory region for peripheral
> seems to be fixed (for boards supported in mainline it will be
> what bootloader configures), it is possible to have it in a different
> region for those peripherals.

The question is whether this is transparent to the client driver at the
end. If the driver needs to know about the address of the external
memory (that's the case for the smsx911x for example), that value should
be in the device tree.

Actually, there's an example here that matches our case quite well:

http://devicetree.org/Device_Tree_Usage#Ranges_.28Address_Translation.29

I think the important part is to get the bindings straight so we don't
have to change them anymore later on; we don't really need to parse the
values from the generic driver and set up the mappings accrodingly -
that can be added easily later on. For a first shot, we can just write
default values to the DT that are computed anyway, right?



Daniel

Representation of external memory-mapped devices in DT (gpmc)

[Rob Herring]

On 10/31/2012 07:08 PM, Daniel Mack wrote:
> On 30.10.2012 11:50, Afzal Mohammed wrote:
>> Hi Daniel,
>>
>> On Monday 29 October 2012 10:52 PM, Daniel Mack wrote:
>>> On 29.10.2012 16:09, Rob Herring wrote:
>>
>>>> You may want a CS0 node with nand as a child node of that.
>>
>>> Hmm, I don't see what that would buy us. The question is which way is
>>> feasible for storing both the memory region and the cs number in the
>>> device tree. The CS number should certainly go to the child node, no?
>>>
>>> IOW, would it be a good idea to have something like the following layout?
>>>
>>> 	gpmc: gpmc at 50000000 {
>>> 		compatible = "ti,gpmc";
>>> 		ti,hwmods = "gpmc";
>>> 		reg =<0x50000000 0x2000>;
>>>
>>> 		/* cs-reg stores the setup of the controller's
>>> 		   memory map */
>>>
>>> 			/* offset	size */
>>> 		cs-reg =<0x0		0x1000000
>>> 			  ....		.....
>>> 			  ....		.....>;
>>>
>>> 		nand: child at 0 {
>>> 			/* timings */
>>> 			/* peripheral specifics */
>>> 		};
>>> 	};
>>>
>>> I would actually much prefer that approach.
>>>
>>> Afzal, because because that way, we can leave the code as-is for now and
>>> add the "cs-reg" property once the code is switched to dynamic handling.
>>> What do you think?
>>
>> I don't know what to say, don't have a good grasp on DT to give
>> right suggestion.
>>
>> It seems offset field may not be necessary. memory for connected
>> peripherals is not fixed, only CS is fixed (as CS pin is hard-wired).
>> Physical memory can be anywhere between 0-512MB (with
>> alignment constraints depending on size, refer GPMC_CONFIG7
>> register), even though right now memory region for peripheral
>> seems to be fixed (for boards supported in mainline it will be
>> what bootloader configures), it is possible to have it in a different
>> region for those peripherals.
> 
> The question is whether this is transparent to the client driver at the
> end. If the driver needs to know about the address of the external
> memory (that's the case for the smsx911x for example), that value should
> be in the device tree.
> 
> Actually, there's an example here that matches our case quite well:
> 
> http://devicetree.org/Device_Tree_Usage#Ranges_.28Address_Translation.29
> 

I had tried to find an example in PPC dts files, but didn't. This
example is what you should follow.

> I think the important part is to get the bindings straight so we don't
> have to change them anymore later on; we don't really need to parse the
> values from the generic driver and set up the mappings accrodingly -
> that can be added easily later on. For a first shot, we can just write
> default values to the DT that are computed anyway, right?

For whatever parts of a CS are programmable, make sure they are in the
DT or you can calculate them from the DT data.

Rob