All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH] add FDT address translation
@ 2010-12-29 23:38 Walter Goossens
       [not found] ` <4D1BC658.4040608-CmkmPbn3yAE@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Walter Goossens @ 2010-12-29 23:38 UTC (permalink / raw)
  To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

[-- Attachment #1: Type: text/plain, Size: 361 bytes --]

This patch adds address translations to fdt. Needed when the early 
console is connected to a simple-bus that has address translation enabled.

To be honest, I'm not too happy about this solution but it works and 
most code is stolen from other parts of the fdt code, so I'm probably 
not doing too weird stuff :)
What do you guys think of this?

Greetz Walter

[-- Attachment #2: fdt_address_translation.patch --]
[-- Type: text/plain, Size: 4750 bytes --]

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index c1360e0..d586efe 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -538,6 +538,161 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
 	/* break now */
 	return 1;
 }
+/**
+ * flat_dt_translate_address - Translate an address using the ranges property
+ *
+ * This function converts address from "node address-space" to "parent address-
+ * space"
+ */
+static int __init flat_dt_translate_address(unsigned long node, unsigned long parent, u64 *address)
+{
+	unsigned long size = 0;
+	__be32 *prop;
+	__be32 *ranges;
+	int size_cells = 0;
+	int addr_cells = 0;
+	int paddr_cells = OF_ROOT_NODE_ADDR_CELLS_DEFAULT;
+
+	ranges = (u32 *)of_get_flat_dt_prop(node,"ranges",&size);
+
+	if(size==0) {
+		pr_debug("No translation possible/necessary\n");
+		return 0;
+	}
+
+	prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
+	if (prop)
+		size_cells = be32_to_cpup(prop);
+	pr_debug("size_cells = %x\n", size_cells);
+
+	prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
+	if (prop)
+		addr_cells = be32_to_cpup(prop);
+	pr_debug("addr_cells = %x\n", addr_cells);
+
+	if(parent) {
+		prop = of_get_flat_dt_prop(parent, "#address-cells", NULL);
+		if (prop)
+			paddr_cells = be32_to_cpup(prop);
+		pr_debug("paddr_cells = %x\n", paddr_cells);
+	}
+	if((addr_cells<=0)||(size_cells<=0)||
+		(addr_cells>2)||(size_cells>2)||(paddr_cells>2)) {
+		pr_warn("Translation not possible in fdt. Invalid address.\n");
+		*address = 0;
+		return -1;
+	}
+	pr_debug("FDT-Ranges:\n");
+	while(size>0) {
+		u64 from,to,tsize;
+		from = be32_to_cpup(ranges++);
+		size-=4;
+		if(addr_cells==2) {
+			from += (((u64)be32_to_cpup(ranges++))<<32);
+			size-=4;
+		}
+		to = be32_to_cpup(ranges++);
+		size-=4;
+		if(paddr_cells==2) {
+			to += (((u64)be32_to_cpup(ranges++))<<32);
+			size-=4;
+		}
+		tsize = be32_to_cpup(ranges++);
+		size-=4;
+		if(size_cells==2) {
+			tsize += (((u64)be32_to_cpup(ranges++))<<32);
+			size-=4;
+		}
+		pr_debug("  From %llX To %llX Size %llX\n", from, to, tsize);
+		if((*address>=from)&&(*address<(from+tsize)))
+			*address += (to - from);
+	}
+	return 1;
+}
+
+static int __init of_scan_flat_dt_ranges(unsigned long *pnode, 
+				unsigned long parent, unsigned long target, 
+				u64 *address, int ignore)
+{
+	int rc = 0;
+	int depth = -1;
+	char *pathp;
+	unsigned long p = *pnode;
+	do {
+		u32 tag = be32_to_cpup((__be32 *)p);
+
+		p += 4;
+		if (tag == OF_DT_END_NODE) {
+			if(depth--) {
+				break;
+			} else {
+				continue;
+			}
+		}
+		if (tag == OF_DT_NOP)
+			continue;
+		if (tag == OF_DT_END)
+			break;
+		if (tag == OF_DT_PROP) {
+			u32 sz = be32_to_cpup((__be32 *)p);
+			p += 8;
+			if (be32_to_cpu(initial_boot_params->version) < 0x10)
+				p = ALIGN(p, sz >= 8 ? 8 : 4);
+			p += sz;
+			p = ALIGN(p, 4);
+			continue;
+		}
+		if (tag != OF_DT_BEGIN_NODE) {
+			pr_err("Invalid tag %x in flat device tree!\n", tag);
+			return -EINVAL;
+		}
+		pathp = (char *)p;
+		p = ALIGN(p + strlen(pathp) + 1, 4);
+		if ((*pathp) == '/') {
+			char *lp, *np;
+			for (lp = NULL, np = pathp; *np; np++)
+				if ((*np) == '/')
+					lp = np+1;
+			if (lp != NULL)
+				pathp = lp;
+		}
+		if((ignore==0) && (p == target)) {
+			rc = 1;
+			ignore++;
+			pr_debug("Found target. Start address translation\n");
+		}
+		if(depth) {
+			int res;
+			*pnode=p;
+			res = of_scan_flat_dt_ranges(pnode, p, target, 
+							address,ignore);
+			if(res<0) {
+				/* Something bad happened */
+				return -1;
+			} else if(res>0) {
+				/* Something gooed happened. Translate. */
+				rc++;
+				pr_debug("translate %08lX %s\n", p, pathp);
+				if(flat_dt_translate_address(p, parent, 
+						address)<0) {
+					return -1;
+				}
+			}
+			p=*pnode;
+		} else {
+			depth++;
+		}
+	} while (1);
+	*pnode=p;
+	return rc;
+}
+int __init early_init_dt_translate_address(unsigned long node, u64 *address)
+{
+	unsigned long p = ((unsigned long)initial_boot_params) +
+			be32_to_cpu(initial_boot_params->off_dt_struct);
+	return of_scan_flat_dt_ranges(&p, 0, node, address,0);
+}
+
 
 /**
  * unflatten_device_tree - create tree of device_nodes from flat blob
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index 7bbf5b3..92e8694 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -82,6 +82,8 @@ extern void early_init_dt_add_memory_arch(u64 base, u64 size);
 extern u64 early_init_dt_alloc_memory_arch(u64 size, u64 align);
 extern u64 dt_mem_next_cell(int s, __be32 **cellp);
 
+extern int early_init_dt_translate_address(unsigned long node, u64 *address);
+
 /*
  * If BLK_DEV_INITRD, the fdt early init code will call this function,
  * to be provided by the arch code. start and end are specified as

[-- Attachment #3: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RFC][PATCH] add FDT address translation
       [not found] ` <4D1BC658.4040608-CmkmPbn3yAE@public.gmane.org>
@ 2011-01-04 21:47   ` Walter Goossens
       [not found]     ` <4D23958D.6000206-CmkmPbn3yAE@public.gmane.org>
  2011-02-12  9:11   ` Grant Likely
  1 sibling, 1 reply; 6+ messages in thread
From: Walter Goossens @ 2011-01-04 21:47 UTC (permalink / raw)
  To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ


[-- Attachment #1.1: Type: text/plain, Size: 949 bytes --]

Nobody interested in this stuff?

I'm working on devicetree support for the NiosII processor and there 
bridges are quite commonly used so this is a must to get early printk 
going without dirty hacks. I thought at least Microblaze would need this 
as well as that's also a synthesizable core but maybe I'm mistaking.

On 12/30/10 12:38 AM, Walter Goossens wrote:
> This patch adds address translations to fdt. Needed when the early 
> console is connected to a simple-bus that has address translation 
> enabled.
>
> To be honest, I'm not too happy about this solution but it works and 
> most code is stolen from other parts of the fdt code, so I'm probably 
> not doing too weird stuff :)
> What do you guys think of this?
>
> Greetz Walter
>
>
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss


[-- Attachment #1.2: Type: text/html, Size: 1708 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC][PATCH] add FDT address translation
       [not found]     ` <4D23958D.6000206-CmkmPbn3yAE@public.gmane.org>
@ 2011-01-04 22:38       ` Grant Likely
       [not found]         ` <20110104223847.GA13014-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Grant Likely @ 2011-01-04 22:38 UTC (permalink / raw)
  To: Walter Goossens; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

On Tue, Jan 04, 2011 at 10:47:57PM +0100, Walter Goossens wrote:
> Nobody interested in this stuff?

Heh, it's a little hard to get people's attention on patches posted
right before the new year and right before the merge window.  I
personally won't be able to take a proper look at it before -rc1.

g.

> 
> I'm working on devicetree support for the NiosII processor and there
> bridges are quite commonly used so this is a must to get early
> printk going without dirty hacks. I thought at least Microblaze
> would need this as well as that's also a synthesizable core but
> maybe I'm mistaking.
> 
> On 12/30/10 12:38 AM, Walter Goossens wrote:
> >This patch adds address translations to fdt. Needed when the early
> >console is connected to a simple-bus that has address translation
> >enabled.
> >
> >To be honest, I'm not too happy about this solution but it works
> >and most code is stolen from other parts of the fdt code, so I'm
> >probably not doing too weird stuff :)
> >What do you guys think of this?
> >
> >Greetz Walter
> >
> >
> >_______________________________________________
> >devicetree-discuss mailing list
> >devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> >https://lists.ozlabs.org/listinfo/devicetree-discuss
> 

> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC][PATCH] add FDT address translation
       [not found]         ` <20110104223847.GA13014-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org>
@ 2011-01-04 23:20           ` Walter Goossens
  0 siblings, 0 replies; 6+ messages in thread
From: Walter Goossens @ 2011-01-04 23:20 UTC (permalink / raw)
  To: Grant Likely; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

On 1/4/11 11:38 PM, Grant Likely wrote:
> On Tue, Jan 04, 2011 at 10:47:57PM +0100, Walter Goossens wrote:
>> Nobody interested in this stuff?
> Heh, it's a little hard to get people's attention on patches posted
> right before the new year and right before the merge window.  I
> personally won't be able to take a proper look at it before -rc1.
>
> g.
That's no problem, I'm in no hurry, just wanted to know if there was 
still interest. Otherwise I'd just push it in the nios2 tree, but since 
this is not at all nios2 specific I'd rather wait a bit longer and get 
it in the devicetree git...

Thanks for the reply!

Walter
>> I'm working on devicetree support for the NiosII processor and there
>> bridges are quite commonly used so this is a must to get early
>> printk going without dirty hacks. I thought at least Microblaze
>> would need this as well as that's also a synthesizable core but
>> maybe I'm mistaking.
>>
>> On 12/30/10 12:38 AM, Walter Goossens wrote:
>>> This patch adds address translations to fdt. Needed when the early
>>> console is connected to a simple-bus that has address translation
>>> enabled.
>>>
>>> To be honest, I'm not too happy about this solution but it works
>>> and most code is stolen from other parts of the fdt code, so I'm
>>> probably not doing too weird stuff :)
>>> What do you guys think of this?
>>>
>>> Greetz Walter
>>>
>>>
>>> _______________________________________________
>>> devicetree-discuss mailing list
>>> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>>> https://lists.ozlabs.org/listinfo/devicetree-discuss
>> _______________________________________________
>> devicetree-discuss mailing list
>> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>> https://lists.ozlabs.org/listinfo/devicetree-discuss
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC][PATCH] add FDT address translation
       [not found] ` <4D1BC658.4040608-CmkmPbn3yAE@public.gmane.org>
  2011-01-04 21:47   ` Walter Goossens
@ 2011-02-12  9:11   ` Grant Likely
       [not found]     ` <20110212091124.GC17755-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org>
  1 sibling, 1 reply; 6+ messages in thread
From: Grant Likely @ 2011-02-12  9:11 UTC (permalink / raw)
  To: Walter Goossens; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

On Thu, Dec 30, 2010 at 12:38:00AM +0100, Walter Goossens wrote:
> This patch adds address translations to fdt. Needed when the early
> console is connected to a simple-bus that has address translation
> enabled.
> 
> To be honest, I'm not too happy about this solution but it works and
> most code is stolen from other parts of the fdt code, so I'm
> probably not doing too weird stuff :)
> What do you guys think of this?

Hi Walter,

Thanks for the patch, comments below.

g.

> 
> Greetz Walter

> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index c1360e0..d586efe 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -538,6 +538,161 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>  	/* break now */
>  	return 1;
>  }
> +/**
> + * flat_dt_translate_address - Translate an address using the ranges property
> + *
> + * This function converts address from "node address-space" to "parent address-
> + * space"
> + */
> +static int __init flat_dt_translate_address(unsigned long node, unsigned long parent, u64 *address)
> +{
> +	unsigned long size = 0;
> +	__be32 *prop;
> +	__be32 *ranges;
> +	int size_cells = 0;
> +	int addr_cells = 0;
> +	int paddr_cells = OF_ROOT_NODE_ADDR_CELLS_DEFAULT;
> +
> +	ranges = (u32 *)of_get_flat_dt_prop(node,"ranges",&size);

Shouldn't need the cast here.  Also coding style.  Need spaced after each ','

Lots of other style issues through this patch.  You should run this
patch through scripts/checkpatch.pl before resubmitting.

> +
> +	if(size==0) {

Coding style:
	if (size == 0) {

> +		pr_debug("No translation possible/necessary\n");

These are two different cases.  If ranges is *not* present, then the
address cannot be translated and it looks like -EINVAL should be
returned.  If ranges is empty, then the node address is 1:1 mapped to
the parent address and success should be returned.

> +		return 0;
> +	}
> +
> +	prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> +	if (prop)
> +		size_cells = be32_to_cpup(prop);

I'd reverse the logic:

	prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
	if (!prop)
		return -EINVAL;
	size_cells = be32_to_cpup(prop);

> +	pr_debug("size_cells = %x\n", size_cells);
> +
> +	prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> +	if (prop)
> +		addr_cells = be32_to_cpup(prop);

Ditto here.

Also, before I dig too deep into the implementation, have you looked
at dt_xlate() in arch/powerpc/boot/devtree.c?  It would need a bit of
rework, but it is essentially the algorithm you want to implement.

> +	pr_debug("addr_cells = %x\n", addr_cells);
> +
> +	if(parent) {
> +		prop = of_get_flat_dt_prop(parent, "#address-cells", NULL);
> +		if (prop)
> +			paddr_cells = be32_to_cpup(prop);
> +		pr_debug("paddr_cells = %x\n", paddr_cells);
> +	}
> +	if((addr_cells<=0)||(size_cells<=0)||
> +		(addr_cells>2)||(size_cells>2)||(paddr_cells>2)) {
> +		pr_warn("Translation not possible in fdt. Invalid address.\n");
> +		*address = 0;
> +		return -1;
> +	}
> +	pr_debug("FDT-Ranges:\n");
> +	while(size>0) {
> +		u64 from,to,tsize;
> +		from = be32_to_cpup(ranges++);
> +		size-=4;
> +		if(addr_cells==2) {
> +			from += (((u64)be32_to_cpup(ranges++))<<32);
> +			size-=4;
> +		}
> +		to = be32_to_cpup(ranges++);
> +		size-=4;
> +		if(paddr_cells==2) {
> +			to += (((u64)be32_to_cpup(ranges++))<<32);
> +			size-=4;
> +		}
> +		tsize = be32_to_cpup(ranges++);
> +		size-=4;
> +		if(size_cells==2) {
> +			tsize += (((u64)be32_to_cpup(ranges++))<<32);
> +			size-=4;
> +		}
> +		pr_debug("  From %llX To %llX Size %llX\n", from, to, tsize);
> +		if((*address>=from)&&(*address<(from+tsize)))
> +			*address += (to - from);
> +	}
> +	return 1;
> +}
> +
> +static int __init of_scan_flat_dt_ranges(unsigned long *pnode, 
> +				unsigned long parent, unsigned long target, 
> +				u64 *address, int ignore)
> +{
> +	int rc = 0;
> +	int depth = -1;
> +	char *pathp;
> +	unsigned long p = *pnode;
> +	do {
> +		u32 tag = be32_to_cpup((__be32 *)p);
> +
> +		p += 4;
> +		if (tag == OF_DT_END_NODE) {
> +			if(depth--) {
> +				break;
> +			} else {
> +				continue;
> +			}
> +		}
> +		if (tag == OF_DT_NOP)
> +			continue;
> +		if (tag == OF_DT_END)
> +			break;
> +		if (tag == OF_DT_PROP) {
> +			u32 sz = be32_to_cpup((__be32 *)p);
> +			p += 8;
> +			if (be32_to_cpu(initial_boot_params->version) < 0x10)
> +				p = ALIGN(p, sz >= 8 ? 8 : 4);
> +			p += sz;
> +			p = ALIGN(p, 4);
> +			continue;
> +		}
> +		if (tag != OF_DT_BEGIN_NODE) {
> +			pr_err("Invalid tag %x in flat device tree!\n", tag);
> +			return -EINVAL;
> +		}
> +		pathp = (char *)p;
> +		p = ALIGN(p + strlen(pathp) + 1, 4);
> +		if ((*pathp) == '/') {
> +			char *lp, *np;
> +			for (lp = NULL, np = pathp; *np; np++)
> +				if ((*np) == '/')
> +					lp = np+1;
> +			if (lp != NULL)
> +				pathp = lp;
> +		}
> +		if((ignore==0) && (p == target)) {
> +			rc = 1;
> +			ignore++;
> +			pr_debug("Found target. Start address translation\n");
> +		}
> +		if(depth) {
> +			int res;
> +			*pnode=p;
> +			res = of_scan_flat_dt_ranges(pnode, p, target, 
> +							address,ignore);
> +			if(res<0) {
> +				/* Something bad happened */
> +				return -1;
> +			} else if(res>0) {
> +				/* Something gooed happened. Translate. */
> +				rc++;
> +				pr_debug("translate %08lX %s\n", p, pathp);
> +				if(flat_dt_translate_address(p, parent, 
> +						address)<0) {
> +					return -1;
> +				}
> +			}
> +			p=*pnode;
> +		} else {
> +			depth++;
> +		}
> +	} while (1);
> +	*pnode=p;
> +	return rc;
> +}

You don't need to implement this.  drivers/of/fdt.c has functions that
already parse the flattened tree.

> +int __init early_init_dt_translate_address(unsigned long node, u64 *address)
> +{
> +	unsigned long p = ((unsigned long)initial_boot_params) +
> +			be32_to_cpu(initial_boot_params->off_dt_struct);

of_get_flat_dt_root()

> +	return of_scan_flat_dt_ranges(&p, 0, node, address,0);
> +}
> +
>  
>  /**
>   * unflatten_device_tree - create tree of device_nodes from flat blob
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index 7bbf5b3..92e8694 100644
> --- a/include/linux/of_fdt.h
> +++ b/include/linux/of_fdt.h
> @@ -82,6 +82,8 @@ extern void early_init_dt_add_memory_arch(u64 base, u64 size);
>  extern u64 early_init_dt_alloc_memory_arch(u64 size, u64 align);
>  extern u64 dt_mem_next_cell(int s, __be32 **cellp);
>  
> +extern int early_init_dt_translate_address(unsigned long node, u64 *address);
> +
>  /*
>   * If BLK_DEV_INITRD, the fdt early init code will call this function,
>   * to be provided by the arch code. start and end are specified as

> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC][PATCH] add FDT address translation
       [not found]     ` <20110212091124.GC17755-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org>
@ 2011-02-13 21:51       ` Walter Goossens
  0 siblings, 0 replies; 6+ messages in thread
From: Walter Goossens @ 2011-02-13 21:51 UTC (permalink / raw)
  To: Grant Likely; +Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ

Thanks for reviewing, I'll create a new version.

Walter

On 2/12/11 10:11 AM, Grant Likely wrote:
> On Thu, Dec 30, 2010 at 12:38:00AM +0100, Walter Goossens wrote:
>> This patch adds address translations to fdt. Needed when the early
>> console is connected to a simple-bus that has address translation
>> enabled.
>>
>> To be honest, I'm not too happy about this solution but it works and
>> most code is stolen from other parts of the fdt code, so I'm
>> probably not doing too weird stuff :)
>> What do you guys think of this?
> Hi Walter,
>
> Thanks for the patch, comments below.
>
> g.
>
>> Greetz Walter
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index c1360e0..d586efe 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -538,6 +538,161 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>>   	/* break now */
>>   	return 1;
>>   }
>> +/**
>> + * flat_dt_translate_address - Translate an address using the ranges property
>> + *
>> + * This function converts address from "node address-space" to "parent address-
>> + * space"
>> + */
>> +static int __init flat_dt_translate_address(unsigned long node, unsigned long parent, u64 *address)
>> +{
>> +	unsigned long size = 0;
>> +	__be32 *prop;
>> +	__be32 *ranges;
>> +	int size_cells = 0;
>> +	int addr_cells = 0;
>> +	int paddr_cells = OF_ROOT_NODE_ADDR_CELLS_DEFAULT;
>> +
>> +	ranges = (u32 *)of_get_flat_dt_prop(node,"ranges",&size);
> Shouldn't need the cast here.  Also coding style.  Need spaced after each ','
>
> Lots of other style issues through this patch.  You should run this
> patch through scripts/checkpatch.pl before resubmitting.
>
>> +
>> +	if(size==0) {
> Coding style:
> 	if (size == 0) {
>
>> +		pr_debug("No translation possible/necessary\n");
> These are two different cases.  If ranges is *not* present, then the
> address cannot be translated and it looks like -EINVAL should be
> returned.  If ranges is empty, then the node address is 1:1 mapped to
> the parent address and success should be returned.
>
>> +		return 0;
>> +	}
>> +
>> +	prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
>> +	if (prop)
>> +		size_cells = be32_to_cpup(prop);
> I'd reverse the logic:
>
> 	prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> 	if (!prop)
> 		return -EINVAL;
> 	size_cells = be32_to_cpup(prop);
>
>> +	pr_debug("size_cells = %x\n", size_cells);
>> +
>> +	prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
>> +	if (prop)
>> +		addr_cells = be32_to_cpup(prop);
> Ditto here.
>
> Also, before I dig too deep into the implementation, have you looked
> at dt_xlate() in arch/powerpc/boot/devtree.c?  It would need a bit of
> rework, but it is essentially the algorithm you want to implement.
>
>> +	pr_debug("addr_cells = %x\n", addr_cells);
>> +
>> +	if(parent) {
>> +		prop = of_get_flat_dt_prop(parent, "#address-cells", NULL);
>> +		if (prop)
>> +			paddr_cells = be32_to_cpup(prop);
>> +		pr_debug("paddr_cells = %x\n", paddr_cells);
>> +	}
>> +	if((addr_cells<=0)||(size_cells<=0)||
>> +		(addr_cells>2)||(size_cells>2)||(paddr_cells>2)) {
>> +		pr_warn("Translation not possible in fdt. Invalid address.\n");
>> +		*address = 0;
>> +		return -1;
>> +	}
>> +	pr_debug("FDT-Ranges:\n");
>> +	while(size>0) {
>> +		u64 from,to,tsize;
>> +		from = be32_to_cpup(ranges++);
>> +		size-=4;
>> +		if(addr_cells==2) {
>> +			from += (((u64)be32_to_cpup(ranges++))<<32);
>> +			size-=4;
>> +		}
>> +		to = be32_to_cpup(ranges++);
>> +		size-=4;
>> +		if(paddr_cells==2) {
>> +			to += (((u64)be32_to_cpup(ranges++))<<32);
>> +			size-=4;
>> +		}
>> +		tsize = be32_to_cpup(ranges++);
>> +		size-=4;
>> +		if(size_cells==2) {
>> +			tsize += (((u64)be32_to_cpup(ranges++))<<32);
>> +			size-=4;
>> +		}
>> +		pr_debug("  From %llX To %llX Size %llX\n", from, to, tsize);
>> +		if((*address>=from)&&(*address<(from+tsize)))
>> +			*address += (to - from);
>> +	}
>> +	return 1;
>> +}
>> +
>> +static int __init of_scan_flat_dt_ranges(unsigned long *pnode,
>> +				unsigned long parent, unsigned long target,
>> +				u64 *address, int ignore)
>> +{
>> +	int rc = 0;
>> +	int depth = -1;
>> +	char *pathp;
>> +	unsigned long p = *pnode;
>> +	do {
>> +		u32 tag = be32_to_cpup((__be32 *)p);
>> +
>> +		p += 4;
>> +		if (tag == OF_DT_END_NODE) {
>> +			if(depth--) {
>> +				break;
>> +			} else {
>> +				continue;
>> +			}
>> +		}
>> +		if (tag == OF_DT_NOP)
>> +			continue;
>> +		if (tag == OF_DT_END)
>> +			break;
>> +		if (tag == OF_DT_PROP) {
>> +			u32 sz = be32_to_cpup((__be32 *)p);
>> +			p += 8;
>> +			if (be32_to_cpu(initial_boot_params->version)<  0x10)
>> +				p = ALIGN(p, sz>= 8 ? 8 : 4);
>> +			p += sz;
>> +			p = ALIGN(p, 4);
>> +			continue;
>> +		}
>> +		if (tag != OF_DT_BEGIN_NODE) {
>> +			pr_err("Invalid tag %x in flat device tree!\n", tag);
>> +			return -EINVAL;
>> +		}
>> +		pathp = (char *)p;
>> +		p = ALIGN(p + strlen(pathp) + 1, 4);
>> +		if ((*pathp) == '/') {
>> +			char *lp, *np;
>> +			for (lp = NULL, np = pathp; *np; np++)
>> +				if ((*np) == '/')
>> +					lp = np+1;
>> +			if (lp != NULL)
>> +				pathp = lp;
>> +		}
>> +		if((ignore==0)&&  (p == target)) {
>> +			rc = 1;
>> +			ignore++;
>> +			pr_debug("Found target. Start address translation\n");
>> +		}
>> +		if(depth) {
>> +			int res;
>> +			*pnode=p;
>> +			res = of_scan_flat_dt_ranges(pnode, p, target,
>> +							address,ignore);
>> +			if(res<0) {
>> +				/* Something bad happened */
>> +				return -1;
>> +			} else if(res>0) {
>> +				/* Something gooed happened. Translate. */
>> +				rc++;
>> +				pr_debug("translate %08lX %s\n", p, pathp);
>> +				if(flat_dt_translate_address(p, parent,
>> +						address)<0) {
>> +					return -1;
>> +				}
>> +			}
>> +			p=*pnode;
>> +		} else {
>> +			depth++;
>> +		}
>> +	} while (1);
>> +	*pnode=p;
>> +	return rc;
>> +}
> You don't need to implement this.  drivers/of/fdt.c has functions that
> already parse the flattened tree.
>
>> +int __init early_init_dt_translate_address(unsigned long node, u64 *address)
>> +{
>> +	unsigned long p = ((unsigned long)initial_boot_params) +
>> +			be32_to_cpu(initial_boot_params->off_dt_struct);
> of_get_flat_dt_root()
>
>> +	return of_scan_flat_dt_ranges(&p, 0, node, address,0);
>> +}
>> +
>>
>>   /**
>>    * unflatten_device_tree - create tree of device_nodes from flat blob
>> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
>> index 7bbf5b3..92e8694 100644
>> --- a/include/linux/of_fdt.h
>> +++ b/include/linux/of_fdt.h
>> @@ -82,6 +82,8 @@ extern void early_init_dt_add_memory_arch(u64 base, u64 size);
>>   extern u64 early_init_dt_alloc_memory_arch(u64 size, u64 align);
>>   extern u64 dt_mem_next_cell(int s, __be32 **cellp);
>>
>> +extern int early_init_dt_translate_address(unsigned long node, u64 *address);
>> +
>>   /*
>>    * If BLK_DEV_INITRD, the fdt early init code will call this function,
>>    * to be provided by the arch code. start and end are specified as
>> _______________________________________________
>> devicetree-discuss mailing list
>> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>> https://lists.ozlabs.org/listinfo/devicetree-discuss
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-02-13 21:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-29 23:38 [RFC][PATCH] add FDT address translation Walter Goossens
     [not found] ` <4D1BC658.4040608-CmkmPbn3yAE@public.gmane.org>
2011-01-04 21:47   ` Walter Goossens
     [not found]     ` <4D23958D.6000206-CmkmPbn3yAE@public.gmane.org>
2011-01-04 22:38       ` Grant Likely
     [not found]         ` <20110104223847.GA13014-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org>
2011-01-04 23:20           ` Walter Goossens
2011-02-12  9:11   ` Grant Likely
     [not found]     ` <20110212091124.GC17755-MrY2KI0G/OVr83L8+7iqerDks+cytr/Z@public.gmane.org>
2011-02-13 21:51       ` Walter Goossens

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.