OpenCores

OpenCores

[or1ksim #13] Modularise config file parsing

no use

no use

1/1

no use

no use

[or1ksim #13] Modularise config file parsing by Unknown on Feb 3, 2005			Not available!
Hi, (I like to modularise everyting, don't I? ;) This patch is basically a brainless no-op. It just moves quite some code from sim-config.c to the peripherals that the code belongs to. This patch is the first in a series that will eventually lead to the removal of all the _clock functions from the main simulator loop. I know I could just do a ADD_SCHED(_clock, runtime.sim.cycles + 1); at the end of the functions but I'm just not happy with that approach, for performance reasons. In the end I want to remove all the peripheral (I define peripherals as the stuff under the peripherals/ directory) states from the config structure. I'm going to look at each peripheral seporatly, and submit a patch for each one, optimising them one at a time. The config file structure is likely to change a very small amout aswell. ChangeLog: * Modularise config file parseing. Paving the way for further modularisation. nog. -------------- next part -------------- A non-text attachment was scrubbed... Name: patch13.gz Type: application/x-gunzip Size: 14145 bytes Desc: not available Url : <A HREF="http://www.opencores.org/forums/openrisc/attachments/20050203/106ba874/patch13.bin">http://www.opencores.org/forums/openrisc/attachments/20050203/106ba874/patch13.bin</A> Hi, (I like to modularise everyting, don't I? ;) This patch is basically a brainless no-op. It just moves quite some code from sim-config.c to the peripherals that the code belongs to. This patch is the first in a series that will eventually lead to the removal of all the _clock functions from the main simulator loop. I know I could just do a ADD_SCHED(_clock, runtime.sim.cycles + 1); at the end of the functions but I'm just not happy with that approach, for performance reasons. In the end I want to remove all the peripheral (I define peripherals as the stuff under the peripherals/ directory) states from the config structure. I'm going to look at each peripheral seporatly, and submit a patch for each one, optimising them one at a time. The config file structure is likely to change a very small amout aswell. ChangeLog: * Modularise config file parseing. Paving the way for further modularisation. nog. -------------- next part -------------- A non-text attachment was scrubbed... Name: patch13.gz Type: application/x-gunzip Size: 14145 bytes Desc: not available Url : http://www.opencores.org/forums/openrisc/attachments/20050203/106ba874/patch13.bin

[or1ksim #13] Modularise config file parsing by Unknown on Feb 3, 2005			Not available!
[q] ChangeLog: * Modularise config file parseing. Paving the way for further modularisation.[/q] Sorry, that was the wrong patch: I forgot to add the extra arguments to the dma section parsing functions. I have corrected it in the attached patch nog. -------------- next part -------------- A non-text attachment was scrubbed... Name: patch13.gz Type: application/x-gunzip Size: 14189 bytes Desc: not available Url : <A HREF="http://www.opencores.org/forums/openrisc/attachments/20050203/587c37d6/patch13-0001.bin">http://www.opencores.org/forums/openrisc/attachments/20050203/587c37d6/patch13-0001.bin</A> ChangeLog: * Modularise config file parseing. Paving the way for further modularisation. Sorry, that was the wrong patch: I forgot to add the extra arguments to the dma section parsing functions. I have corrected it in the attached patch nog. -------------- next part -------------- A non-text attachment was scrubbed... Name: patch13.gz Type: application/x-gunzip Size: 14189 bytes Desc: not available Url : http://www.opencores.org/forums/openrisc/attachments/20050203/587c37d6/patch13-0001.bin

[or1ksim #13] Modularise config file parsing by Unknown on Feb 3, 2005			Not available!
On 0, GyÃ¶rgy 'nog' Jeney <<A HREF="mailto:nog@sdf.lonestar.org">nog@sdf.lonestar.org</A>> wrote: [q]> ChangeLog: > * Modularise config file parseing. Paving the way for further modularisation.[/q] Sorry, that was the wrong patch: I forgot to add the extra arguments to the dma section parsing functions. I have corrected it in the attached patch [/q] 13 Isn't my lucky number. This time I forgot to remove some fixmes... nog. -------------- next part -------------- A non-text attachment was scrubbed... Name: patch13.gz Type: application/x-gunzip Size: 13947 bytes Desc: not available Url : <A HREF="http://www.opencores.org/forums/openrisc/attachments/20050203/3263d168/patch13.bin">http://www.opencores.org/forums/openrisc/attachments/20050203/3263d168/patch13.bin</A> On 0, GyÃ¶rgy 'nog' Jeney nog@sdf.lonestar.org> wrote: > ChangeLog: > * Modularise config file parseing. Paving the way for further modularisation. Sorry, that was the wrong patch: I forgot to add the extra arguments to the dma section parsing functions. I have corrected it in the attached patch 13 Isn't my lucky number. This time I forgot to remove some fixmes... nog. -------------- next part -------------- A non-text attachment was scrubbed... Name: patch13.gz Type: application/x-gunzip Size: 13947 bytes Desc: not available Url : http://www.opencores.org/forums/openrisc/attachments/20050203/3263d168/patch13.bin

[or1ksim #13] Modularise config file parsing by Unknown on Feb 4, 2005			Not available!
* GyÃ¶rgy 'nog' Jeney (<A HREF="mailto:nog@sdf.lonestar.org">nog@sdf.lonestar.org</A>) wrote: [q] On 0, GyÃ¶rgy 'nog' Jeney <<A HREF="mailto:nog@sdf.lonestar.org">nog@sdf.lonestar.org</A>> wrote: [q] > ChangeLog: > * Modularise config file parseing. Paving the way for further modularisation.[/q] > > Sorry, that was the wrong patch: I forgot to add the extra arguments to the dma > section parsing functions. I have corrected it in the attached patch > [/q] 13 Isn't my lucky number. This time I forgot to remove some fixmes...[/q] some more fixme's are still there, but i guess they are ok. i think your patch is a step in the right direction. but i'd just make a sim-config.h: /* registers a new config section * / section_t register_config_section(char name, char keyword...); typedef enum { INTEGER, STRING, ... } parameter_type_t; /* registers new parameter for a given section * * / bool_t register_config_section_parameter(section_t section, char keyword, parameter_type_t type, (bool_t )(store_parameter)(type_t data)) (data passed would always be of type defined by the parameter type) this two functions would allow every module to get the right info from the config file, and allow it to store parameters it needs in it's own data structures. basicaly the first function would generate an eqvivalent of (in sim-config.c): struct section sections[] = { {"base", 0}, / 0 / {"mc", 0}, {"uart", 0}, {"dma", 0}, {"memory", 0}, {"cpu", 0}, / 5 / {"sim", 0}, {"debug", 0}, {"VAPI", 0}, {"ethernet",0}, {"", 0}, / 10 / and the second an eqvivalent of: } config_params[] = { {0, "include", "\"%s\"", base_include, (void )(&tempS), 0}, {1, "enabled", "=%i", NULL, (void)(&config.mc.en abled), 0}, now the sim-config would be just a parser which would pass the parameter values in the expected format to some (boot_t )(store_parameter)() function module would provide... every module that would like to have some config option would have to include sim-config.h function and register the the section and parameter it wanted... best regards, p. PS. I know, i'm an idealist. ;) * GyÃ¶rgy 'nog' Jeney (nog@sdf.lonestar.org) wrote: On 0, GyÃ¶rgy 'nog' Jeney nog@sdf.lonestar.org> wrote: > ChangeLog: > * Modularise config file parseing. Paving the way for further modularisation. > > Sorry, that was the wrong patch: I forgot to add the extra arguments to the dma > section parsing functions. I have corrected it in the attached patch > 13 Isn't my lucky number. This time I forgot to remove some fixmes... some more fixme's are still there, but i guess they are ok. i think your patch is a step in the right direction. but i'd just make a sim-config.h: /* registers a new config section * / section_t register_config_section(char name, char keyword...); typedef enum { INTEGER, STRING, ... } parameter_type_t; /* registers new parameter for a given section * * / bool_t register_config_section_parameter(section_t section, char keyword, parameter_type_t type, (bool_t )(store_parameter)(type_t data)) (data passed would always be of type defined by the parameter type) this two functions would allow every module to get the right info from the config file, and allow it to store parameters it needs in it's own data structures. basicaly the first function would generate an eqvivalent of (in sim-config.c): struct section sections[] = { {"base", 0}, / 0 / {"mc", 0}, {"uart", 0}, {"dma", 0}, {"memory", 0}, {"cpu", 0}, / 5 / {"sim", 0}, {"debug", 0}, {"VAPI", 0}, {"ethernet",0}, {"", 0}, / 10 / and the second an eqvivalent of: } config_params[] = { {0, "include", "\"%s\"", base_include, (void )(&tempS), 0}, {1, "enabled", "=%i", NULL, (void)(&config.mc.en abled), 0}, now the sim-config would be just a parser which would pass the parameter values in the expected format to some (boot_t )(store_parameter)() function module would provide... every module that would like to have some config option would have to include sim-config.h function and register the the section and parameter it wanted... best regards, p. PS. I know, i'm an idealist. ;)

[or1ksim #13] Modularise config file parsing by Unknown on Feb 4, 2005			Not available!
[q] some more fixme's are still there, but i guess they are ok.[/q] Those fixmes denote things that will still happen. I could ofcourse removed them in one patch, but such a patch would be BIG. [q] i think your patch is a step in the right direction. but i'd just make a sim-config.h: /* registers a new config section * / section_t register_config_section(char name, char keyword...);[/q] What does `name' refer to? I guess it's the name of the config section (ie. `cpu', `sim', `immu', etc.). What information do you want passed by keyword? [q] typedef enum { INTEGER, STRING, ... } parameter_type_t;[/q] This is what I called `enum param_t' in my patch. I prefix the types with `paramt_' to avoid polluting the namespace. I also used lowercase because they are not macros. Should I change it? [q] /* registers new parameter for a given section * * / bool_t register_config_section_parameter(section_t section, char keyword, parameter_type_t type, (bool_t )(store_parameter)(type_t data))[/q] This looks very similar to this function I introduced: void reg_config_param(struct config_section sec, const char param, enum param_t type, void (param_cb)(union param_val, void )) where the value of the parsed parameter is in the `union param_val'. void is whatever data was returned from the sec_start function (as passed to reg_config_sec). The union could be removed and the `store_parameter' could cast the pointer to the reqired type, though that would have been more typeing and since writting this patch was exceptionally boring, I took all the shortcuts I could... [q] this two functions would allow every module to get the right info from the config file, and allow it to store parameters it needs in it's own data structures.[/q] `in it's own data structures', yes the way you defined register_config_section_parameter that would be possible, but we couldn't remove loops that itterate through MAX_SOME_PERIPHERAL (ok, we could but it won't be so easy). [q] basicaly the first function would generate an eqvivalent of (in sim-config.c): struct section sections[] = { {"base", 0}, /* 0 / {"mc", 0}, {"uart", 0}, {"dma", 0}, {"memory", 0}, {"cpu", 0}, / 5 / {"sim", 0}, {"debug", 0}, {"VAPI", 0}, {"ethernet",0}, {"", 0}, / 10 /[/q] Didn't I do that? [q] and the second an eqvivalent of:[/q] I believe I did this. I just linked this structure into the previous one. [q] now the sim-config would be just a parser which would pass the parameter values in the expected format to some (boot_t )(store_parameter)() function module would provide...[/q] Doesn't my patch do this? [q] every module that would like to have some config option would have to include sim-config.h function and register the the section and parameter it wanted...[/q] Didn't I do this with the `reg__sec' functions? The bottom line is, I don't mind changeing this patch with any suggestions you have but unfortionatly you're just gona have to spoon feed me ;) (Maybe being awake till 4am took it's toll on you ;) nog. some more fixme's are still there, but i guess they are ok. Those fixmes denote things that will still happen. I could ofcourse removed them in one patch, but such a patch would be BIG. i think your patch is a step in the right direction. but i'd just make a sim-config.h: / registers a new config section * / section_t register_config_section(char name, char keyword...); What does `name' refer to? I guess it's the name of the config section (ie. `cpu', `sim', `immu', etc.). What information do you want passed by keyword? typedef enum { INTEGER, STRING, ... } parameter_type_t; This is what I called `enum param_t' in my patch. I prefix the types with `paramt_' to avoid polluting the namespace. I also used lowercase because they are not macros. Should I change it? /* registers new parameter for a given section * * / bool_t register_config_section_parameter(section_t section, char keyword, parameter_type_t type, (bool_t )(store_parameter)(type_t data)) This looks very similar to this function I introduced: void reg_config_param(struct config_section sec, const char param, enum param_t type, void (param_cb)(union param_val, void )) where the value of the parsed parameter is in the `union param_val'. void is whatever data was returned from the sec_start function (as passed to reg_config_sec). The union could be removed and the `store_parameter' could cast the pointer to the reqired type, though that would have been more typeing and since writting this patch was exceptionally boring, I took all the shortcuts I could... this two functions would allow every module to get the right info from the config file, and allow it to store parameters it needs in it's own data structures. `in it's own data structures', yes the way you defined register_config_section_parameter that would be possible, but we couldn't remove loops that itterate through MAX_SOME_PERIPHERAL (ok, we could but it won't be so easy). basicaly the first function would generate an eqvivalent of (in sim-config.c): struct section sections[] = { {"base", 0}, /* 0 / {"mc", 0}, {"uart", 0}, {"dma", 0}, {"memory", 0}, {"cpu", 0}, / 5 / {"sim", 0}, {"debug", 0}, {"VAPI", 0}, {"ethernet",0}, {"", 0}, / 10 / Didn't I do that? and the second an eqvivalent of: I believe I did this. I just linked this structure into the previous one. now the sim-config would be just a parser which would pass the parameter values in the expected format to some (boot_t )(store_parameter)() function module would provide... Doesn't my patch do this? every module that would like to have some config option would have to include sim-config.h function and register the the section and parameter it wanted... Didn't I do this with the `reg_*_sec' functions? The bottom line is, I don't mind changeing this patch with any suggestions you have but unfortionatly you're just gona have to spoon feed me ;) (Maybe being awake till 4am took it's toll on you ;) nog.

[or1ksim #13] Modularise config file parsing by Unknown on Feb 5, 2005			Not available!
* GyÃ¶rgy 'nog' Jeney (<A HREF="mailto:nog@sdf.lonestar.org">nog@sdf.lonestar.org</A>) wrote: [q] [q]> i think your patch is a step in the right direction. but i'd just make a > > sim-config.h: > > /* registers a new config section > * > / > section_t register_config_section(char name, char keyword...);[/q] What does `name' refer to? I guess it's the name of the config section (ie. `cpu', `sim', `immu', etc.). What information do you want passed by keyword?[/q] yes one is redundand (i remember i added it in the end because i thought i was missing something ... oh well) [q]> typedef enum { > INTEGER, > STRING, > ... > } parameter_type_t;[/q] This is what I called `enum param_t' in my patch. I prefix the types with `paramt_' to avoid polluting the namespace. I also used lowercase because they are not macros. Should I change it?[/q] your names are ok. [q]> /* registers new parameter for a given section > * > * > / > bool_t register_config_section_parameter(section_t section, > char keyword, > parameter_type_t type, > (bool_t )(store_parameter)(type_t data))[/q] This looks very similar to this function I introduced: void reg_config_param(struct config_section sec, const char param, enum param_t type, void (param_cb)(union param_val, void )) where the value of the parsed parameter is in the `union param_val'. void is whatever data was returned from the sec_start function (as passed to reg_config_sec). The union could be removed and the `store_parameter' could cast the pointer to the reqired type, though that would have been more typeing and since writting this patch was exceptionally boring, I took all the shortcuts I could... [q]> this two functions would allow every module to get the right info from the > config file, and allow it to store parameters it needs in it's own > data structures.[/q] `in it's own data structures', yes the way you defined register_config_section_parameter that would be possible, but we couldn't remove loops that itterate through MAX_SOME_PERIPHERAL (ok, we could but it won't be so easy). [q]> and the second an eqvivalent of:[/q] I believe I did this. I just linked this structure into the previous one. [q]> now the sim-config would be just a parser which would pass the parameter > values in the expected format to some (boot_t )(store_parameter)() function > module would provide...[/q] Doesn't my patch do this? [q]> every module that would like to have some config option would have to > include sim-config.h function and register the the section and parameter > it wanted...[/q] Didn't I do this with the `reg__sec' functions? The bottom line is, I don't mind changeing this patch with any suggestions you have but unfortionatly you're just gona have to spoon feed me ;) (Maybe being awake till 4am took it's toll on you ;)[/q] Yes i really just skimmed through this diff and kind of assumed (don't know why, maybe it's the 4am effect) that you just moved some parts of sim-config to other files (i scrolled down really quickly). But two things make me extreamly happy. It seams your patch is exactly what i wanted it to be (even if i overlooked it). The other thing is that it's only 3:20 am now, so there is still a chance that i got it right today ;)... So after you commit the pathes we were talking about the other day and i test & tag or1ksim, go ahead with this one... best regards, p. * GyÃ¶rgy 'nog' Jeney (nog@sdf.lonestar.org) wrote: > i think your patch is a step in the right direction. but i'd just make a > > sim-config.h: > > /* registers a new config section > * > / > section_t register_config_section(char name, char keyword...); What does `name' refer to? I guess it's the name of the config section (ie. `cpu', `sim', `immu', etc.). What information do you want passed by keyword? yes one is redundand (i remember i added it in the end because i thought i was missing something ... oh well) > typedef enum { > INTEGER, > STRING, > ... > } parameter_type_t; This is what I called `enum param_t' in my patch. I prefix the types with `paramt_' to avoid polluting the namespace. I also used lowercase because they are not macros. Should I change it? your names are ok. > /* registers new parameter for a given section > * > * > / > bool_t register_config_section_parameter(section_t section, > char keyword, > parameter_type_t type, > (bool_t )(store_parameter)(type_t data)) This looks very similar to this function I introduced: void reg_config_param(struct config_section sec, const char param, enum param_t type, void (param_cb)(union param_val, void )) where the value of the parsed parameter is in the `union param_val'. void is whatever data was returned from the sec_start function (as passed to reg_config_sec). The union could be removed and the `store_parameter' could cast the pointer to the reqired type, though that would have been more typeing and since writting this patch was exceptionally boring, I took all the shortcuts I could... > this two functions would allow every module to get the right info from the > config file, and allow it to store parameters it needs in it's own > data structures. `in it's own data structures', yes the way you defined register_config_section_parameter that would be possible, but we couldn't remove loops that itterate through MAX_SOME_PERIPHERAL (ok, we could but it won't be so easy). > and the second an eqvivalent of: I believe I did this. I just linked this structure into the previous one. > now the sim-config would be just a parser which would pass the parameter > values in the expected format to some (boot_t )(store_parameter)() function > module would provide... Doesn't my patch do this? > every module that would like to have some config option would have to > include sim-config.h function and register the the section and parameter > it wanted... Didn't I do this with the `reg__sec' functions? The bottom line is, I don't mind changeing this patch with any suggestions you have but unfortionatly you're just gona have to spoon feed me ;) (Maybe being awake till 4am took it's toll on you ;) Yes i really just skimmed through this diff and kind of assumed (don't know why, maybe it's the 4am effect) that you just moved some parts of sim-config to other files (i scrolled down really quickly). But two things make me extreamly happy. It seams your patch is exactly what i wanted it to be (even if i overlooked it). The other thing is that it's only 3:20 am now, so there is still a chance that i got it right today ;)... So after you commit the pathes we were talking about the other day and i test & tag or1ksim, go ahead with this one... best regards, p.

no use

no use

1/1

no use

no use

© copyright 1999-2026 OpenCores.org, equivalent to Oliscience, all rights reserved. OpenCores®, registered trademark.