linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH] identify_ppc_sys_by_name_and_id function implementation
@ 2005-08-10 17:15 Vitaly Bordug
  2005-08-10 17:46 ` [PATCH] identify_ppc_sys_by_name_and_id function implementation (braces fixed) Vitaly Bordug
  0 siblings, 1 reply; 12+ messages in thread
From: Vitaly Bordug @ 2005-08-10 17:15 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-embedded list

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

Kumar,

This is preliminary version of the identify_ppc_sys_by_name_and_id(...).
Tested with pq2 devices/sys for 8272ADS and fs_enet. This will BUG_ON if 
nothing found or duplicates exist (guess if we call identify_ppc_sys.. 
hit is expected).

Signed-off-by: Vitaly Bordug <vbordug@ru.mvista.com>

-- 
Sincerely,
Vitaly

[-- Attachment #2: ppc_sys_add.patch --]
[-- Type: text/x-patch, Size: 2183 bytes --]

diff --git a/arch/ppc/syslib/ppc_sys.c b/arch/ppc/syslib/ppc_sys.c
--- a/arch/ppc/syslib/ppc_sys.c
+++ b/arch/ppc/syslib/ppc_sys.c
@@ -6,6 +6,7 @@
  * Maintainer: Kumar Gala <kumar.gala@freescale.com>
  *
  * Copyright 2005 Freescale Semiconductor Inc.
+ * Copyright 2005 MontaVista, Inc. by Vitaly Bordug <vbordug@ru.mvista.com>
  *
  * This program is free software; you can redistribute  it and/or modify it
  * under  the terms of  the GNU General  Public License as published by the
@@ -33,10 +34,48 @@ void __init identify_ppc_sys_by_id(u32 i
 	return;
 }
 
-void __init identify_ppc_sys_by_name(char *name)
+static int __init count_sys_specs(void)
 {
-	/* TODO */
-	return;
+	int i = 0;
+	while (strcmp(ppc_sys_specs[i].ppc_sys_name,""))
+		i++;
+	return i;
+}
+
+static int __init find_chip_by_name_and_id(char *name, u32 id)
+{
+	int ret = -1;
+	unsigned int i = 0;
+	unsigned int j = 0;
+	unsigned int dups = 0;
+	
+	unsigned int matched[count_sys_specs()];
+	
+	while (strcmp(ppc_sys_specs[i].ppc_sys_name,"")) {
+		if (!strcmp(ppc_sys_specs[i].ppc_sys_name, name))
+			matched[j++]=i;
+		i++;
+	}
+	if(j != 0)
+	{    	
+		for(i = 0;i < j;i++)
+		{	
+			if ((ppc_sys_specs[matched[i]].mask & id) == ppc_sys_specs[matched[i]].value)
+			{	
+				ret = matched[i];
+				dups++;
+			}
+		}
+		ret = (dups == 1) ? ret : (-1*dups);  
+	}
+	return ret;
+}
+
+void __init identify_ppc_sys_by_name_and_id(char* name, u32 id)
+{
+    int i = find_chip_by_name_and_id(name,id);
+    BUG_ON(i < 0);
+    cur_ppc_sys_spec = &ppc_sys_specs[i];
 }
 
 /* Update all memory resources by paddr, call before platform_device_register */
diff --git a/include/asm-ppc/ppc_sys.h b/include/asm-ppc/ppc_sys.h
--- a/include/asm-ppc/ppc_sys.h
+++ b/include/asm-ppc/ppc_sys.h
@@ -51,7 +51,7 @@ extern struct ppc_sys_spec *cur_ppc_sys_
 
 /* determine which specific SOC we are */
 extern void identify_ppc_sys_by_id(u32 id) __init;
-extern void identify_ppc_sys_by_name(char *name) __init;
+extern void identify_ppc_sys_by_name_and_id(char *name, u32 id) __init;
 
 /* describes all devices that may exist in a given family of processors */
 extern struct platform_device ppc_sys_platform_devices[];

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

* [PATCH] identify_ppc_sys_by_name_and_id function implementation (braces fixed)
  2005-08-10 17:15 [RFC][PATCH] identify_ppc_sys_by_name_and_id function implementation Vitaly Bordug
@ 2005-08-10 17:46 ` Vitaly Bordug
  2005-08-10 18:01   ` [PATCH] identify_ppc_sys_by_name_and_id function implementation final Vitaly Bordug
  0 siblings, 1 reply; 12+ messages in thread
From: Vitaly Bordug @ 2005-08-10 17:46 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-embedded list

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

The same as above but with correct kernel bracing style.

Signed-off-by: Vitaly Bordug <vbordug@ru.mvista.com>
-- 
Sincerely,
Vitaly

[-- Attachment #2: ppc_sys_add.patch --]
[-- Type: text/x-patch, Size: 2174 bytes --]

diff --git a/arch/ppc/syslib/ppc_sys.c b/arch/ppc/syslib/ppc_sys.c
--- a/arch/ppc/syslib/ppc_sys.c
+++ b/arch/ppc/syslib/ppc_sys.c
@@ -6,6 +6,7 @@
  * Maintainer: Kumar Gala <kumar.gala@freescale.com>
  *
  * Copyright 2005 Freescale Semiconductor Inc.
+ * Copyright 2005 MontaVista, Inc. by Vitaly Bordug <vbordug@ru.mvista.com>
  *
  * This program is free software; you can redistribute  it and/or modify it
  * under  the terms of  the GNU General  Public License as published by the
@@ -33,10 +34,45 @@ void __init identify_ppc_sys_by_id(u32 i
 	return;
 }
 
-void __init identify_ppc_sys_by_name(char *name)
+static int __init count_sys_specs(void)
 {
-	/* TODO */
-	return;
+	int i = 0;
+	while (strcmp(ppc_sys_specs[i].ppc_sys_name,""))
+		i++;
+	return i;
+}
+
+static int __init find_chip_by_name_and_id(char *name, u32 id)
+{
+	int ret = -1;
+	unsigned int i = 0;
+	unsigned int j = 0;
+	unsigned int dups = 0;
+	
+	unsigned int matched[count_sys_specs()];
+	
+	while (strcmp(ppc_sys_specs[i].ppc_sys_name,"")) {
+		if (!strcmp(ppc_sys_specs[i].ppc_sys_name, name))
+			matched[j++]=i;
+		i++;
+	}
+	if(j != 0) {    	
+		for(i = 0;i < j;i++) {	
+			if ((ppc_sys_specs[matched[i]].mask & id) == ppc_sys_specs[matched[i]].value) {	
+				ret = matched[i];
+				dups++;
+			}
+		}
+		ret = (dups == 1) ? ret : (-1*dups);  
+	}
+	return ret;
+}
+
+void __init identify_ppc_sys_by_name_and_id(char* name, u32 id)
+{
+    int i = find_chip_by_name_and_id(name,id);
+    BUG_ON(i < 0);
+    cur_ppc_sys_spec = &ppc_sys_specs[i];
 }
 
 /* Update all memory resources by paddr, call before platform_device_register */
diff --git a/include/asm-ppc/ppc_sys.h b/include/asm-ppc/ppc_sys.h
--- a/include/asm-ppc/ppc_sys.h
+++ b/include/asm-ppc/ppc_sys.h
@@ -51,7 +51,7 @@ extern struct ppc_sys_spec *cur_ppc_sys_
 
 /* determine which specific SOC we are */
 extern void identify_ppc_sys_by_id(u32 id) __init;
-extern void identify_ppc_sys_by_name(char *name) __init;
+extern void identify_ppc_sys_by_name_and_id(char *name, u32 id) __init;
 
 /* describes all devices that may exist in a given family of processors */
 extern struct platform_device ppc_sys_platform_devices[];

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

* [PATCH] identify_ppc_sys_by_name_and_id function implementation final
  2005-08-10 17:46 ` [PATCH] identify_ppc_sys_by_name_and_id function implementation (braces fixed) Vitaly Bordug
@ 2005-08-10 18:01   ` Vitaly Bordug
  2005-08-10 19:16     ` Kumar Gala
  0 siblings, 1 reply; 12+ messages in thread
From: Vitaly Bordug @ 2005-08-10 18:01 UTC (permalink / raw)
  To: linuxppc-embedded list

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

Finally correct indentation style.

Signed-off-by: Vitaly Bordug <vbordug@ru.mvista.com>

-- 
Sincerely,
Vitaly

[-- Attachment #2: ppc_sys_add.patch --]
[-- Type: text/x-patch, Size: 2173 bytes --]

diff --git a/arch/ppc/syslib/ppc_sys.c b/arch/ppc/syslib/ppc_sys.c
--- a/arch/ppc/syslib/ppc_sys.c
+++ b/arch/ppc/syslib/ppc_sys.c
@@ -6,6 +6,7 @@
  * Maintainer: Kumar Gala <kumar.gala@freescale.com>
  *
  * Copyright 2005 Freescale Semiconductor Inc.
+ * Copyright 2005 MontaVista, Inc. by Vitaly Bordug <vbordug@ru.mvista.com>
  *
  * This program is free software; you can redistribute  it and/or modify it
  * under  the terms of  the GNU General  Public License as published by the
@@ -33,10 +34,46 @@ void __init identify_ppc_sys_by_id(u32 i
 	return;
 }
 
-void __init identify_ppc_sys_by_name(char *name)
+static int __init count_sys_specs(void)
 {
-	/* TODO */
-	return;
+	int i = 0;
+	while (strcmp(ppc_sys_specs[i].ppc_sys_name, ""))
+		i++;
+	return i;
+}
+
+static int __init find_chip_by_name_and_id(char *name, u32 id)
+{
+	int ret = -1;
+	unsigned int i = 0;
+	unsigned int j = 0;
+	unsigned int dups = 0;
+
+	unsigned int matched[count_sys_specs()];
+
+	while (strcmp(ppc_sys_specs[i].ppc_sys_name, "")) {
+		if (!strcmp(ppc_sys_specs[i].ppc_sys_name, name))
+			matched[j++] = i;
+		i++;
+	}
+	if (j != 0) {
+		for (i = 0; i < j; i++) {
+			if ((ppc_sys_specs[matched[i]].mask & id) ==
+			    ppc_sys_specs[matched[i]].value) {
+				ret = matched[i];
+				dups++;
+			}
+		}
+		ret = (dups == 1) ? ret : (-1 * dups);
+	}
+	return ret;
+}
+
+void __init identify_ppc_sys_by_name_and_id(char *name, u32 id)
+{
+	int i = find_chip_by_name_and_id(name, id);
+	BUG_ON(i < 0);
+	cur_ppc_sys_spec = &ppc_sys_specs[i];
 }
 
 /* Update all memory resources by paddr, call before platform_device_register */
diff --git a/include/asm-ppc/ppc_sys.h b/include/asm-ppc/ppc_sys.h
--- a/include/asm-ppc/ppc_sys.h
+++ b/include/asm-ppc/ppc_sys.h
@@ -51,7 +51,7 @@ extern struct ppc_sys_spec *cur_ppc_sys_
 
 /* determine which specific SOC we are */
 extern void identify_ppc_sys_by_id(u32 id) __init;
-extern void identify_ppc_sys_by_name(char *name) __init;
+extern void identify_ppc_sys_by_name_and_id(char *name, u32 id) __init;
 
 /* describes all devices that may exist in a given family of processors */
 extern struct platform_device ppc_sys_platform_devices[];

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

* Re: [PATCH] identify_ppc_sys_by_name_and_id function implementation final
  2005-08-10 18:01   ` [PATCH] identify_ppc_sys_by_name_and_id function implementation final Vitaly Bordug
@ 2005-08-10 19:16     ` Kumar Gala
  2005-08-11  5:30       ` Marcelo Tosatti
  0 siblings, 1 reply; 12+ messages in thread
From: Kumar Gala @ 2005-08-10 19:16 UTC (permalink / raw)
  To: Vitaly Bordug; +Cc: linuxppc-embedded list

+static int __init find_chip_by_name_and_id(char *name, u32 id)
+{
+    int ret = -1;
+    unsigned int i = 0;
+    unsigned int j = 0;
+    unsigned int dups = 0;
+
+    unsigned int matched[count_sys_specs()];

Is is legit in the kernel to use dynamically sized array?

+
+    while (strcmp(ppc_sys_specs[i].ppc_sys_name, "")) {
+        if (!strcmp(ppc_sys_specs[i].ppc_sys_name, name))
+            matched[j++] = i;
+        i++;
+    }
+    if (j != 0) {
+        for (i = 0; i < j; i++) {
+            if ((ppc_sys_specs[matched[i]].mask & id) ==
+                ppc_sys_specs[matched[i]].value) {
+                ret = matched[i];
+                dups++;
+            }
+        }
+        ret = (dups == 1) ? ret : (-1 * dups);
+    }
+    return ret;
+}

On Aug 10, 2005, at 1:01 PM, Vitaly Bordug wrote:

> Finally correct indentation style.
>
> Signed-off-by: Vitaly Bordug <vbordug@ru.mvista.com>
>
> -- 
> Sincerely,
> Vitaly
>
> <ppc_sys_add.patch>
> <ATT87954.txt>
>

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

* Re: [PATCH] identify_ppc_sys_by_name_and_id function implementation final
  2005-08-10 19:16     ` Kumar Gala
@ 2005-08-11  5:30       ` Marcelo Tosatti
  2005-08-11 15:25         ` Vitaly Bordug
  0 siblings, 1 reply; 12+ messages in thread
From: Marcelo Tosatti @ 2005-08-11  5:30 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-embedded list

On Wed, Aug 10, 2005 at 02:16:57PM -0500, Kumar Gala wrote:
> +static int __init find_chip_by_name_and_id(char *name, u32 id)
> +{
> +    int ret = -1;
> +    unsigned int i = 0;
> +    unsigned int j = 0;
> +    unsigned int dups = 0;
> +
> +    unsigned int matched[count_sys_specs()];
> 
> Is is legit in the kernel to use dynamically sized array?

kmalloc() is certainly safer - why not use it? 

> +
> +    while (strcmp(ppc_sys_specs[i].ppc_sys_name, "")) {
> +        if (!strcmp(ppc_sys_specs[i].ppc_sys_name, name))
> +            matched[j++] = i;
> +        i++;
> +    }
> +    if (j != 0) {
> +        for (i = 0; i < j; i++) {
> +            if ((ppc_sys_specs[matched[i]].mask & id) ==
> +                ppc_sys_specs[matched[i]].value) {
> +                ret = matched[i];
> +                dups++;
> +            }
> +        }
> +        ret = (dups == 1) ? ret : (-1 * dups);
> +    }
> +    return ret;
> +}
> 
> On Aug 10, 2005, at 1:01 PM, Vitaly Bordug wrote:
> 
> >Finally correct indentation style.
> >
> >Signed-off-by: Vitaly Bordug <vbordug@ru.mvista.com>
> >> >-- 
> >Sincerely,
> >Vitaly
> >
> ><ppc_sys_add.patch>
> ><ATT87954.txt>
> >
> 
> _______________________________________________
> Linuxppc-embedded mailing list
> Linuxppc-embedded@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-embedded

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

* Re: [PATCH] identify_ppc_sys_by_name_and_id function implementation final
  2005-08-11  5:30       ` Marcelo Tosatti
@ 2005-08-11 15:25         ` Vitaly Bordug
  2005-08-11 22:45           ` Marcelo Tosatti
  0 siblings, 1 reply; 12+ messages in thread
From: Vitaly Bordug @ 2005-08-11 15:25 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: linuxppc-embedded list

Marcelo Tosatti wrote:
> On Wed, Aug 10, 2005 at 02:16:57PM -0500, Kumar Gala wrote:
> 
>>+static int __init find_chip_by_name_and_id(char *name, u32 id)
>>+{
>>+    int ret = -1;
>>+    unsigned int i = 0;
>>+    unsigned int j = 0;
>>+    unsigned int dups = 0;
>>+
>>+    unsigned int matched[count_sys_specs()];
>>
>>Is is legit in the kernel to use dynamically sized array?
> 
> 
> kmalloc() is certainly safer - why not use it? 
Practically , version with kmalloc works, but  setup_arch and thus this 
function is called before mem_init, so I just wonder if kmalloc can 
handle this case. On the other hand, I don't like to deal with
alloc_bootmem() if mem_init_done!=1 and kmalloc otherwise (like ocp 
does) just for the temporary buffer.

But it's the only _right_ way (or I 've missed something) - sure I'll 
follow it.

-- 
Sincerely,
Vitaly

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

* Re: [PATCH] identify_ppc_sys_by_name_and_id function implementation final
  2005-08-11 15:25         ` Vitaly Bordug
@ 2005-08-11 22:45           ` Marcelo Tosatti
  2005-08-12 15:37             ` Vitaly Bordug
  0 siblings, 1 reply; 12+ messages in thread
From: Marcelo Tosatti @ 2005-08-11 22:45 UTC (permalink / raw)
  To: Vitaly Bordug; +Cc: linuxppc-embedded list

On Thu, Aug 11, 2005 at 07:25:20PM +0400, Vitaly Bordug wrote:
> Marcelo Tosatti wrote:
> >On Wed, Aug 10, 2005 at 02:16:57PM -0500, Kumar Gala wrote:
> >
> >>+static int __init find_chip_by_name_and_id(char *name, u32 id)
> >>+{
> >>+    int ret = -1;
> >>+    unsigned int i = 0;
> >>+    unsigned int j = 0;
> >>+    unsigned int dups = 0;
> >>+
> >>+    unsigned int matched[count_sys_specs()];
> >>
> >>Is is legit in the kernel to use dynamically sized array?
> >
> >
> >kmalloc() is certainly safer - why not use it? 
> Practically , version with kmalloc works, but  setup_arch and thus this 
> function is called before mem_init, so I just wonder if kmalloc can 
> handle this case. On the other hand, I don't like to deal with
> alloc_bootmem() if mem_init_done!=1 and kmalloc otherwise (like ocp 
> does) just for the temporary buffer.
> 
> But it's the only _right_ way (or I 've missed something) - sure I'll 
> follow it.

I dont see any problem with dynamic array usage on the kernel (maybe someone
else has good argumentation against it).

Just that you have a 4kb stack. Does count_sys_specs() have an appropriate
maximum?

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

* Re: [PATCH] identify_ppc_sys_by_name_and_id function implementation final
  2005-08-11 22:45           ` Marcelo Tosatti
@ 2005-08-12 15:37             ` Vitaly Bordug
  2005-08-12 16:18               ` Kumar Gala
  0 siblings, 1 reply; 12+ messages in thread
From: Vitaly Bordug @ 2005-08-12 15:37 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: linuxppc-embedded list

Marcelo Tosatti wrote:
> On Thu, Aug 11, 2005 at 07:25:20PM +0400, Vitaly Bordug wrote:
> 
>>Marcelo Tosatti wrote:
>>
>>>On Wed, Aug 10, 2005 at 02:16:57PM -0500, Kumar Gala wrote:
>>>
>>>
>>>>+static int __init find_chip_by_name_and_id(char *name, u32 id)
>>>>+{
>>>>+    int ret = -1;
>>>>+    unsigned int i = 0;
>>>>+    unsigned int j = 0;
>>>>+    unsigned int dups = 0;
>>>>+
>>>>+    unsigned int matched[count_sys_specs()];
>>>>
>>>>Is is legit in the kernel to use dynamically sized array?
>>>
>>>
>>>kmalloc() is certainly safer - why not use it? 
>>
>>Practically , version with kmalloc works, but  setup_arch and thus this 
>>function is called before mem_init, so I just wonder if kmalloc can 
>>handle this case. On the other hand, I don't like to deal with
>>alloc_bootmem() if mem_init_done!=1 and kmalloc otherwise (like ocp 
>>does) just for the temporary buffer.
>>
>>But it's the only _right_ way (or I 've missed something) - sure I'll 
>>follow it.
> 
> 
> I dont see any problem with dynamic array usage on the kernel (maybe someone
> else has good argumentation against it).
> 
> Just that you have a 4kb stack. Does count_sys_specs() have an appropriate
> maximum?
> 
> 
Yes it does, but there is no define for it. The ppc_sys_specs array 
should always end at the "default match" - element with name field set 
to "" and value=0 (without it every existing ppc_sys identify will fall 
into infinite loop). This array is defined in syslib/<board>_sys.c end 
is finite of course. Its size depends on amount of supported boards.

So, if the dynamic array is ok with conditions, it will be great.
But now I'm inclined to implement the same using kmalloc/alloc_bootmem 
to prevent flame when it will proceed. BTW, will free_bootmem properly 
release the memory allocated by alloc_bootmem()? I haven't encountered
examples of this so far...


-- 
Sincerely,
Vitaly

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

* Re: [PATCH] identify_ppc_sys_by_name_and_id function implementation final
  2005-08-12 15:37             ` Vitaly Bordug
@ 2005-08-12 16:18               ` Kumar Gala
  2005-08-12 16:30                 ` Vitaly Bordug
  0 siblings, 1 reply; 12+ messages in thread
From: Kumar Gala @ 2005-08-12 16:18 UTC (permalink / raw)
  To: Vitaly Bordug; +Cc: linuxppc-embedded list

Can you do a sizeof instead?

#define num_ele sizeof(ppc_sys_specs[])/sizeof(struct ppc_sys_spec)

Something like matchted[num_ele] ??

- kumar

On Aug 12, 2005, at 10:37 AM, Vitaly Bordug wrote:

> Marcelo Tosatti wrote:
>
>> On Thu, Aug 11, 2005 at 07:25:20PM +0400, Vitaly Bordug wrote:
>>
>>
>>> Marcelo Tosatti wrote:
>>>
>>>
>>>> On Wed, Aug 10, 2005 at 02:16:57PM -0500, Kumar Gala wrote:
>>>>
>>>>
>>>>
>>>>> +static int __init find_chip_by_name_and_id(char *name, u32 id)
>>>>> +{
>>>>> +    int ret = -1;
>>>>> +    unsigned int i = 0;
>>>>> +    unsigned int j = 0;
>>>>> +    unsigned int dups = 0;
>>>>> +
>>>>> +    unsigned int matched[count_sys_specs()];
>>>>>
>>>>> Is is legit in the kernel to use dynamically sized array?
>>>>>
>>>>
>>>>
>>>> kmalloc() is certainly safer - why not use it?
>>>>
>>>
>>> Practically , version with kmalloc works, but  setup_arch and  
>>> thus this
>>> function is called before mem_init, so I just wonder if kmalloc can
>>> handle this case. On the other hand, I don't like to deal with
>>> alloc_bootmem() if mem_init_done!=1 and kmalloc otherwise (like ocp
>>> does) just for the temporary buffer.
>>>
>>> But it's the only _right_ way (or I 've missed something) - sure  
>>> I'll
>>> follow it.
>>>
>>
>>
>> I dont see any problem with dynamic array usage on the kernel  
>> (maybe someone
>> else has good argumentation against it).
>>
>> Just that you have a 4kb stack. Does count_sys_specs() have an  
>> appropriate
>> maximum?
>>
>>
>>
> Yes it does, but there is no define for it. The ppc_sys_specs array
> should always end at the "default match" - element with name field set
> to "" and value=0 (without it every existing ppc_sys identify will  
> fall
> into infinite loop). This array is defined in syslib/<board>_sys.c end
> is finite of course. Its size depends on amount of supported boards.
>
> So, if the dynamic array is ok with conditions, it will be great.
> But now I'm inclined to implement the same using kmalloc/alloc_bootmem
> to prevent flame when it will proceed. BTW, will free_bootmem properly
> release the memory allocated by alloc_bootmem()? I haven't encountered
> examples of this so far...
>
>
> -- 
> Sincerely,
> Vitaly
>

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

* Re: [PATCH] identify_ppc_sys_by_name_and_id function implementation final
  2005-08-12 16:18               ` Kumar Gala
@ 2005-08-12 16:30                 ` Vitaly Bordug
  2005-08-12 19:48                   ` Kumar Gala
  0 siblings, 1 reply; 12+ messages in thread
From: Vitaly Bordug @ 2005-08-12 16:30 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-embedded list

Kumar Gala wrote:
> Can you do a sizeof instead?
> 
> #define num_ele sizeof(ppc_sys_specs[])/sizeof(struct ppc_sys_spec)
> 
> Something like matchted[num_ele] ??
> 

That's what the first I tried actually :)
gcc is not happy with it:

arch/ppc/syslib/ppc_sys.c: In function `find_chip_by_name_and_id':
arch/ppc/syslib/ppc_sys.c:54: error: parse error before ']' token

and if I remove [] from the ppc_sys_specs, it outputs:

arch/ppc/syslib/ppc_sys.c: In function `find_chip_by_name_and_id':
arch/ppc/syslib/ppc_sys.c:54: error: invalid application of `sizeof' to 
incomplete type `({anonymous})'

So I cannot use sizeof this case, I think...
> - kumar
> 
> On Aug 12, 2005, at 10:37 AM, Vitaly Bordug wrote:
> 
>> Marcelo Tosatti wrote:
>>
>>> On Thu, Aug 11, 2005 at 07:25:20PM +0400, Vitaly Bordug wrote:
>>>
>>>
>>>> Marcelo Tosatti wrote:
>>>>
>>>>
>>>>> On Wed, Aug 10, 2005 at 02:16:57PM -0500, Kumar Gala wrote:
>>>>>
>>>>>
>>>>>
>>>>>> +static int __init find_chip_by_name_and_id(char *name, u32 id)
>>>>>> +{
>>>>>> +    int ret = -1;
>>>>>> +    unsigned int i = 0;
>>>>>> +    unsigned int j = 0;
>>>>>> +    unsigned int dups = 0;
>>>>>> +
>>>>>> +    unsigned int matched[count_sys_specs()];
>>>>>>
>>>>>> Is is legit in the kernel to use dynamically sized array?
>>>>>>
>>>>>
>>>>>
>>>>> kmalloc() is certainly safer - why not use it?
>>>>>
>>>>
>>>> Practically , version with kmalloc works, but  setup_arch and  thus 
>>>> this
>>>> function is called before mem_init, so I just wonder if kmalloc can
>>>> handle this case. On the other hand, I don't like to deal with
>>>> alloc_bootmem() if mem_init_done!=1 and kmalloc otherwise (like ocp
>>>> does) just for the temporary buffer.
>>>>
>>>> But it's the only _right_ way (or I 've missed something) - sure  I'll
>>>> follow it.
>>>>
>>>
>>>
>>> I dont see any problem with dynamic array usage on the kernel  (maybe 
>>> someone
>>> else has good argumentation against it).
>>>
>>> Just that you have a 4kb stack. Does count_sys_specs() have an  
>>> appropriate
>>> maximum?
>>>
>>>
>>>
>> Yes it does, but there is no define for it. The ppc_sys_specs array
>> should always end at the "default match" - element with name field set
>> to "" and value=0 (without it every existing ppc_sys identify will  fall
>> into infinite loop). This array is defined in syslib/<board>_sys.c end
>> is finite of course. Its size depends on amount of supported boards.
>>
>> So, if the dynamic array is ok with conditions, it will be great.
>> But now I'm inclined to implement the same using kmalloc/alloc_bootmem
>> to prevent flame when it will proceed. BTW, will free_bootmem properly
>> release the memory allocated by alloc_bootmem()? I haven't encountered
>> examples of this so far...
>>
>>
>> -- 
>> Sincerely,
>> Vitaly
>>
> 
> 


-- 
Sincerely,
Vitaly

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

* Re: [PATCH] identify_ppc_sys_by_name_and_id function implementation final
  2005-08-12 16:30                 ` Vitaly Bordug
@ 2005-08-12 19:48                   ` Kumar Gala
  2005-08-16 13:36                     ` Vitaly Bordug
  0 siblings, 1 reply; 12+ messages in thread
From: Kumar Gala @ 2005-08-12 19:48 UTC (permalink / raw)
  To: Vitaly Bordug; +Cc: linuxppc-embedded list

On Aug 12, 2005, at 11:30 AM, Vitaly Bordug wrote:

> Kumar Gala wrote:
>
>> Can you do a sizeof instead?
>>
>> #define num_ele sizeof(ppc_sys_specs[])/sizeof(struct ppc_sys_spec)
>>
>> Something like matchted[num_ele] ??
>>
>>
>
> That's what the first I tried actually :)
> gcc is not happy with it:
>
> arch/ppc/syslib/ppc_sys.c: In function `find_chip_by_name_and_id':
> arch/ppc/syslib/ppc_sys.c:54: error: parse error before ']' token
>
> and if I remove [] from the ppc_sys_specs, it outputs:
>
> arch/ppc/syslib/ppc_sys.c: In function `find_chip_by_name_and_id':
> arch/ppc/syslib/ppc_sys.c:54: error: invalid application of  
> `sizeof' to
> incomplete type `({anonymous})'
>
> So I cannot use sizeof this case, I think...

Realized the same thing.  I'm thinking that your original method is  
the best solution to just do this on the stack.  In general the  
number or processors (array size) is going to be less than 100.  So  
at most this array is going to end up being 100 bytes on the stack.   
I dont think that's a big deal at the point we are calling this.

- kumar

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

* Re: [PATCH] identify_ppc_sys_by_name_and_id function implementation final
  2005-08-12 19:48                   ` Kumar Gala
@ 2005-08-16 13:36                     ` Vitaly Bordug
  0 siblings, 0 replies; 12+ messages in thread
From: Vitaly Bordug @ 2005-08-16 13:36 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-embedded list

Kumar Gala wrote:
> On Aug 12, 2005, at 11:30 AM, Vitaly Bordug wrote:
> 
>> Kumar Gala wrote:
>>
>>> Can you do a sizeof instead?
>>>
>>> #define num_ele sizeof(ppc_sys_specs[])/sizeof(struct ppc_sys_spec)
>>>
>>> Something like matchted[num_ele] ??
>>>
>>>
>>
>> That's what the first I tried actually :)
>> gcc is not happy with it:
>>
>> arch/ppc/syslib/ppc_sys.c: In function `find_chip_by_name_and_id':
>> arch/ppc/syslib/ppc_sys.c:54: error: parse error before ']' token
>>
>> and if I remove [] from the ppc_sys_specs, it outputs:
>>
>> arch/ppc/syslib/ppc_sys.c: In function `find_chip_by_name_and_id':
>> arch/ppc/syslib/ppc_sys.c:54: error: invalid application of  `sizeof' to
>> incomplete type `({anonymous})'
>>
>> So I cannot use sizeof this case, I think...
> 
> 
> Realized the same thing.  I'm thinking that your original method is  the 
> best solution to just do this on the stack.  In general the  number or 
> processors (array size) is going to be less than 100.  So  at most this 
> array is going to end up being 100 bytes on the stack.   I dont think 
> that's a big deal at the point we are calling this
> 
Thus, are there any modifications needed for the original approach?
Or maybe it's simpler to add say MAX_PPC_SPECS to ppc_sys.h since this 
list will be limited anyway? We could have the simple array with static 
size then, and additional ppc_sys_specs sanity check (whether it has 
"default" element with both 0 id and "" name) could be easily 
implemented as well.


-- 
Sincerely,
Vitaly

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

end of thread, other threads:[~2005-08-16 13:36 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-08-10 17:15 [RFC][PATCH] identify_ppc_sys_by_name_and_id function implementation Vitaly Bordug
2005-08-10 17:46 ` [PATCH] identify_ppc_sys_by_name_and_id function implementation (braces fixed) Vitaly Bordug
2005-08-10 18:01   ` [PATCH] identify_ppc_sys_by_name_and_id function implementation final Vitaly Bordug
2005-08-10 19:16     ` Kumar Gala
2005-08-11  5:30       ` Marcelo Tosatti
2005-08-11 15:25         ` Vitaly Bordug
2005-08-11 22:45           ` Marcelo Tosatti
2005-08-12 15:37             ` Vitaly Bordug
2005-08-12 16:18               ` Kumar Gala
2005-08-12 16:30                 ` Vitaly Bordug
2005-08-12 19:48                   ` Kumar Gala
2005-08-16 13:36                     ` Vitaly Bordug

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).