identify_ppc_sys_by_name_and_id function implementation
RFC PATCH 1/1 identify_ppc_sys_by_name_and_id function implementation
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).
Signature · 2 lines
Sincerely, Vitaly
arch/ppc/syslib/ppc_sys.c +42 -3
@@ -6,6 +6,7 @@* Maintainer: Kumar Gala <[email protected]> * * Copyright 2005 Freescale Semiconductor Inc. +* Copyright 2005 MontaVista, Inc. by Vitaly Bordug <[email protected]> * * 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 ireturn ; } -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 */
include/asm-ppc/ppc_sys.h +1 -1
@@ -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[];
PATCH 1/1 identify_ppc_sys_by_name_and_id function implementation (braces fixed)
The same as above but with correct kernel bracing style.
Signature · 2 lines
Sincerely, Vitaly
arch/ppc/syslib/ppc_sys.c +39 -3
@@ -6,6 +6,7 @@* Maintainer: Kumar Gala <[email protected]> * * Copyright 2005 Freescale Semiconductor Inc. +* Copyright 2005 MontaVista, Inc. by Vitaly Bordug <[email protected]> * * 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 ireturn ; } -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 */
include/asm-ppc/ppc_sys.h +1 -1
@@ -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[];
PATCH 1/1 identify_ppc_sys_by_name_and_id function implementation final
Finally correct indentation style.
Signature · 2 lines
Sincerely, Vitaly
arch/ppc/syslib/ppc_sys.c +40 -3
@@ -6,6 +6,7 @@* Maintainer: Kumar Gala <[email protected]> * * Copyright 2005 Freescale Semiconductor Inc. +* Copyright 2005 MontaVista, Inc. by Vitaly Bordug <[email protected]> * * 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 ireturn ; } -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 */
include/asm-ppc/ppc_sys.h +1 -1
@@ -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[];
Re: [PATCH] identify_ppc_sys_by_name_and_id function implementation final
+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;
+}
Quoted 11 lines · Vitaly Bordug
Finally correct indentation style.
Signed-off-by Vitaly Bordug <[email protected]>Signature · 5 lines
Sincerely, Vitaly <ppc_sys_add.patch> <ATT87954.txt>
Re: [PATCH] identify_ppc_sys_by_name_and_id function implementation final
Quoted 10 lines · Kumar Gala
arch/ppc/syslib/ppc_sys.c +8
-0+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?
Quoted 36 lines
arch/ppc/syslib/ppc_sys.c +18
-0+ +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; +}Quoted 10 lines · Vitaly Bordug
Finally correct indentation style.
Signed-off-by Vitaly Bordug <[email protected]>Quoted 1 line
Quoted 1 line
Sincerely,
Vitaly<ppc_sys_add.patch>
<ATT87954.txt>_______________________________________________
Linuxppc-embedded mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-embedded
Re: [PATCH] identify_ppc_sys_by_name_and_id function implementation final
Quoted 11 lines · Marcelo Tosatti
Quoted 9 lines · Kumar Gala
arch/ppc/syslib/ppc_sys.c +8
-0+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 ()];
Quoted 4 lines
Quoted 1 line
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.
Signature · 2 lines
Sincerely, Vitaly
Re: [PATCH] identify_ppc_sys_by_name_and_id function implementation final
Quoted 16 lines · Vitaly Bordug
Quoted 11 lines · Marcelo Tosatti
Quoted 9 lines · Kumar Gala
arch/ppc/syslib/ppc_sys.c +8
-0+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 ()];Quoted 4 lines
Quoted 1 line
Is is legit in the kernel to use dynamically sized array?
kmalloc()is certainly safer - why not use it?
Quoted 8 lines
Practically , version with kmalloc works, but
setup_archand thus this function is called beforemem_init, so I just wonder if kmalloc can handle this case. On the other hand, I don't like to deal withalloc_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?
Re: [PATCH] identify_ppc_sys_by_name_and_id function implementation final
Quoted 24 lines · Marcelo Tosatti
Quoted 22 lines · Vitaly Bordug
Quoted 12 lines · Marcelo Tosatti
Quoted 9 lines · Kumar Gala
arch/ppc/syslib/ppc_sys.c +8
-0+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 ()];Quoted 4 lines
Quoted 1 line
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_archand thus this function is called beforemem_init, so I just wonder if kmalloc can handle this case. On the other hand, I don't like to deal withalloc_bootmem()if mem_init_done!=1 and kmalloc otherwise (like ocp does) just for the temporary buffer.
Quoted 10 lines
Quoted 1 line
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...
Signature · 2 lines
Sincerely, Vitaly
Re: [PATCH] identify_ppc_sys_by_name_and_id function implementation final
Can you do a sizeof instead?
#define num_ele sizeof(ppc_sys_specs[])/sizeof(struct ppc_sys_spec)
Something like matchted[num_ele] ??
- kumar
Quoted 64 lines · Vitaly Bordug
Quoted 37 lines · Marcelo Tosatti
Quoted 23 lines · Vitaly Bordug
Quoted 13 lines · Marcelo Tosatti
Quoted 9 lines · Kumar Gala
arch/ppc/syslib/ppc_sys.c +8
-0+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 ()];Quoted 6 lines
Quoted 2 lines
Is is legit in the kernel to use dynamically sized array?
kmalloc()is certainly safer - why not use it?Quoted 9 lines
Practically , version with kmalloc works, but
setup_archand thus this function is called beforemem_init, so I just wonder if kmalloc can handle this case. On the other hand, I don't like to deal withalloc_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.
Quoted 8 lines
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_specsarray should always end at the "default match" - element with name field set to "" and value=0 (without it every existingppc_sysidentify 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_bootmemproperly release the memory allocated byalloc_bootmem()? I haven't encountered examples of this so far...Signature · 2 lines
Sincerely, Vitaly
Re: [PATCH] identify_ppc_sys_by_name_and_id function implementation final
Quoted 6 lines · Kumar Gala
Can you do a sizeof instead?
#define
num_elesizeof(ppc_sys_specs[])/sizeof(structppc_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...
Quoted 70 lines
- kumar
Quoted 64 lines · Vitaly Bordug
Quoted 37 lines · Marcelo Tosatti
Quoted 23 lines · Vitaly Bordug
Quoted 13 lines · Marcelo Tosatti
Quoted 9 lines · Kumar Gala
(quoted diff) +8
-0+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()];Quoted 6 lines
Quoted 2 lines
Is is legit in the kernel to use dynamically sized array?
kmalloc()is certainly safer - why not use it?Quoted 9 lines
Practically , version with kmalloc works, but
setup_archand thus this function is called beforemem_init, so I just wonder if kmalloc can handle this case. On the other hand, I don't like to deal withalloc_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.
Quoted 8 lines
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_specsarray should always end at the "default match" - element with name field set to "" and value=0 (without it every existingppc_sysidentify 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_bootmemproperly release the memory allocated byalloc_bootmem()? I haven't encountered examples of this so far...Signature · 2 lines
Sincerely, Vitaly
Signature · 2 lines
Sincerely, Vitaly
Re: [PATCH] identify_ppc_sys_by_name_and_id function implementation final
Quoted 16 lines · Vitaly Bordug
Quoted 7 lines · Kumar Gala
Can you do a sizeof instead?
#define
num_elesizeof(ppc_sys_specs[])/sizeof(structppc_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
Quoted 7 lines
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 ofsizeof' 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
Re: [PATCH] identify_ppc_sys_by_name_and_id function implementation final
Quoted 20 lines · Kumar Gala
Quoted 18 lines · Vitaly Bordug
Quoted 7 lines · Kumar Gala
Can you do a sizeof instead?
#define
num_elesizeof(ppc_sys_specs[])/sizeof(structppc_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 ']' tokenand if I remove [] from the
ppc_sys_specs, it outputs:
Quoted 9 lines
Quoted 5 lines
arch/ppc/syslib/ppc_sys.c: In function `find_chip_by_name_and_id':arch/ppc/syslib/ppc_sys.c:54: error: invalid application ofsizeof' 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.
Signature · 2 lines
Sincerely, Vitaly