Make ATNGW100 serial ports configurable
PATCH 1/1 Make ATNGW100 serial ports configurable
Make configuration of ATNGW100 serial ports selectable in kernel configuration.
arch/avr32/boards/atngw100/Kconfig +18 -0
@@ -0,0 +1,18 @@ +config SERIAL_ATMEL_USART0 + bool "Enable USART0" + depends on SERIAL_ATMEL + help + Enable USART0 (on expansion header J5, pins 12, 11) + +config SERIAL_ATMEL_USART2 + bool "Enable USART2" + depends on SERIAL_ATMEL + help + Enable USART2 (on expansion header J6, pins 27, 26) + +config SERIAL_ATMEL_USART3 + bool "Enable USART3" + depends on SERIAL_ATMEL + help + Enable USART3 (on expansion header J6, pins 17, 18) +
arch/avr32/boards/atngw100/setup.c +24 -2
@@ -114,8 +114,20 @@void __init setup_board (void ) { -at32_map_usart (1 ,0 );/* USART 1: /dev/ttyS0, DB9 */ +int i =0 ; + +at32_map_usart (1 , i++);/* USART 1: /dev/ttyS0, DB9 */ at32_setup_serial_console (0 ); + +#ifdef CONFIG_SERIAL_ATMEL_USART0 +at32_map_usart (0 , i++);/* USART 0: /dev/ttySX, J5, pins 12, 11 */ +#endif +#ifdef CONFIG_SERIAL_ATMEL_USART2 +at32_map_usart (2 , i++);/* USART 2: /dev/ttySX, J6, pins 27, 26 */ +#endif +#ifdef CONFIG_SERIAL_ATMEL_USART3 +at32_map_usart (3 , i++);/* USART 3: /dev/ttySX, J6, pins 17, 18 */ +#endif }static const struct gpio_led ngw_leds[] = {
@@ -170,7 +182,17 @@at32_add_system_devices (); -at32_add_device_usart (0 ); + i =0 ; +at32_add_device_usart (i++); +#ifdef CONFIG_SERIAL_ATMEL_USART0 +at32_add_device_usart (i++); +#endif +#ifdef CONFIG_SERIAL_ATMEL_USART2 +at32_add_device_usart (i++); +#endif +#ifdef CONFIG_SERIAL_ATMEL_USART3 +at32_add_device_usart (i++); +#endif set_hw_addr (at32_add_device_eth (0 , ð_data[0 ]));set_hw_addr (at32_add_device_eth (1 , ð_data[1 ]));
arch/avr32/Kconfig +4 -0
@@ -125,6 +125,10 @@ source "arch/avr32/boards/atstk1000/Kconfig" endif +if BOARD_ATNGW100 +source "arch/avr32/boards/atngw100/Kconfig" +endif + choice prompt "Boot loader type" default LOADER_U_BOOT
Quoted 3 lines · Anders Blomdell
Make configuration of ATNGW100 serial ports selectable in kernel configuration.
Signed-off-by Anders Blomdell <[email protected]>
Gak. If we're going down this path, why not add #ifdefs for every other conceivable hardware mod as well and turn the ATNGW100 board code into an even worse mess than the ATSTK1000 board code?
The ATNGW100 has one serial port on board, so IMO the standard board code should only initialize that one port.
However, it might be sensible to add some sort of interface for expansion board code. For example something like this:
#ifdef CONFIG_ATNGW100_EXPANSION atngw100_setup_expansion_board();
#endif
This will allow people with hardware mods to add all the extra devices they need, including serial ports, by simply adding another file with expansion board code. People who aren't afraid to solder stuff on their boards shouldn't be afraid of writing some board code too, right?
What do you think?
Haavard
Quoted 8 lines · Haavard Skinnemoen
Quoted 3 lines · Anders Blomdell
Make configuration of ATNGW100 serial ports selectable in kernel configuration.
Signed-off-by Anders Blomdell <[email protected]>Gak. If we're going down this path, why not add #ifdefs for every other conceivable hardware mod as well and turn the ATNGW100 board code into an even worse mess than the ATSTK1000 board code?
OK, good point. The reason I sent in the patch is that people on AVRFreaks had asked how to do it, and putting it in the configuration would (theoretically) make it possible to catch pin conflicts.
Quoted 2 lines
The ATNGW100 has one serial port on board, so IMO the standard board code should only initialize that one port.
Which was my intention, unless SERIAL_ATMEL_USARTn was selected.
Quoted 6 lines
However, it might be sensible to add some sort of interface for expansion board code. For example something like this:
#ifdef CONFIG_ATNGW100_EXPANSION atngw100_setup_expansion_board();#endif
Since that file has to reside inside the kernel tree (?), one could almost as easily replace the entire setup.c.
Shouldn't this be something like:
void __init setup_board(void)
{
...
#ifdef CONFIG_ATNGW100_EXPANSION
atngw100_setup_expansion_board();
#endif
}
static int __init atngw100_init(void)
{
...
#ifdef CONFIG_ATNGW100_EXPANSION
atngw100_init_expansion_board();
#endif
}
Quoted 6 lines
This will allow people with hardware mods to add all the extra devices they need, including serial ports, by simply adding another file with expansion board code. People who aren't afraid to solder stuff on their boards shouldn't be afraid of writing some board code too, right?
What do you think?
An even nicer way of handling it (provided that initialization does not need to take place during boot), might be to do EXPORT_SYMBOL() on at32_add_device_usart, at32_map_usart, etc and then write a loadable module that handles the initialization.
Regards
Anders Blomdell
Signature · 5 lines
Anders Blomdell Email: [email protected] Department of Automatic Control Lund University Phone: +46 46 222 4625 P.O. Box 118 Fax: +46 46 138118 SE-221 00 Lund, Sweden
Quoted 4 lines · Anders Blomdell
Quoted 3 lines · Haavard Skinnemoen
Quoted 2 lines · Anders Blomdell
Make configuration of ATNGW100 serial ports selectable in kernel configuration.
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Signed-off-by Anders Blomdell <[email protected]>Gak. If we're going down this path, why not add #ifdefs for every other conceivable hardware mod as well and turn the ATNGW100 board code into an even worse mess than the ATSTK1000 board code?
OK, good point. The reason I sent in the patch is that people on AVRFreaks had asked how to do it, and putting it in the configuration would (theoretically) make it possible to catch pin conflicts.
Yes, I do realize people want this kind of thing. But I also think there's more than one way to do it, and while a Kconfig option might make things easier while trying it out, I think it will cause maintenance pains in the long run.
Quoted 3 lines
Quoted 2 lines
The ATNGW100 has one serial port on board, so IMO the standard board code should only initialize that one port.
Which was my intention, unless
SERIAL_ATMEL_USARTnwas selected.
Yes, but it's not sensible to even offer the option of other ports when the board just doesn't have them.
Quoted 8 lines
Quoted 6 lines
However, it might be sensible to add some sort of interface for expansion board code. For example something like this:
#ifdef CONFIG_ATNGW100_EXPANSION atngw100_setup_expansion_board();#endif
Since that file has to reside inside the kernel tree (?), one could almost as easily replace the entire
setup.c.
True...but it would take quite a bit of duplication, and you'll have to sync up whenever the "main" setup.c changes.
Quoted 10 lines
Shouldn't this be something like:
void
__initsetup_board(void)
{
...
#ifdefCONFIG_ATNGW100_EXPANSION
atngw100_setup_expansion_board();
#endif
}
Quoted 7 lines
static int
__initatngw100_init(void)
{
...
#ifdefCONFIG_ATNGW100_EXPANSION
atngw100_init_expansion_board();
#endif
}
Well, you could do it like that, but you could also just add another postcore_initcall. The only hook which is really needed is the "setup" part, and only if you have additional serial ports.
In fact, it's probably better to just add a weak definition for it since not all expansion boards will need to override it.
Quoted 10 lines
Quoted 6 lines
This will allow people with hardware mods to add all the extra devices they need, including serial ports, by simply adding another file with expansion board code. People who aren't afraid to solder stuff on their boards shouldn't be afraid of writing some board code too, right?
What do you think?
An even nicer way of handling it (provided that initialization does not need to take place during boot), might be to do
EXPORT_SYMBOL()onat32_add_device_usart,at32_map_usart, etc and then write a loadable module that handles the initialization.
Hmm...why would that be nicer exactly?
What _I_ think would be a nicer way to do it is to implement support for flattened device trees and get rid of the board code entirely. Or almost entirely; it depends on how complete we can make the device tree.
Haavard
Quoted 55 lines · Haavard Skinnemoen
Quoted 11 lines · Anders Blomdell
Quoted 7 lines · Haavard Skinnemoen
Quoted 3 lines · Anders Blomdell
Make configuration of ATNGW100 serial ports selectable in kernel configuration.
Signed-off-by Anders Blomdell <[email protected]>Gak. If we're going down this path, why not add #ifdefs for every other conceivable hardware mod as well and turn the ATNGW100 board code into an even worse mess than the ATSTK1000 board code?
OK, good point. The reason I sent in the patch is that people on AVRFreaks had asked how to do it, and putting it in the configuration would (theoretically) make it possible to catch pin conflicts.
Yes, I do realize people want this kind of thing. But I also think there's more than one way to do it, and while a
Kconfigoption might make things easier while trying it out, I think it will cause maintenance pains in the long run.Quoted 3 lines
Quoted 2 lines
The ATNGW100 has one serial port on board, so IMO the standard board code should only initialize that one port.
Which was my intention, unless
SERIAL_ATMEL_USARTnwas selected.Yes, but it's not sensible to even offer the option of other ports when the board just doesn't have them.
Quoted 8 lines
Quoted 6 lines
However, it might be sensible to add some sort of interface for expansion board code. For example something like this:
#ifdef CONFIG_ATNGW100_EXPANSION atngw100_setup_expansion_board();#endif
Since that file has to reside inside the kernel tree (?), one could almost as easily replace the entire
setup.c.True...but it would take quite a bit of duplication, and you'll have to sync up whenever the "main"
setup.cchanges.Quoted 10 lines
Shouldn't this be something like:
void
__initsetup_board(void)
{
...
#ifdefCONFIG_ATNGW100_EXPANSION
atngw100_setup_expansion_board();
#endif
}Quoted 7 lines
static int
__initatngw100_init(void)
{
...
#ifdefCONFIG_ATNGW100_EXPANSION
atngw100_init_expansion_board();
#endif
}
Quoted 3 lines
Well, you could do it like that, but you could also just add another
postcore_initcall. The only hook which is really needed is the "setup" part, and only if you have additional serial ports.
Does that mean that 'at32_add_device_usart(...);' is not needed?
Quoted 8 lines
In fact, it's probably better to just add a weak definition for it since not all expansion boards will need to override it.
Quoted 5 lines
Quoted 5 lines
This will allow people with hardware mods to add all the extra devices they need, including serial ports, by simply adding another file with expansion board code. People who aren't afraid to solder stuff on their boards shouldn't be afraid of writing some board code too, right?
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
What do you think?
An even nicer way of handling it (provided that initialization does not need to take place during boot), might be to do
EXPORT_SYMBOL()onat32_add_device_usart,at32_map_usart, etc and then write a loadable module that handles the initialization.Hmm...why would that be nicer exactly?
You could compile your board specific inits outside the kernel tree, making it much easier to follow kernel versions (not everyone gets their board specific code into the kernel tree :-).
Quoted 3 lines
What _I_ think would be a nicer way to do it is to implement support for flattened device trees and get rid of the board code entirely. Or almost entirely; it depends on how complete we can make the device tree.
I don't understand the above paragraph, could you please elaborate?
Signature · 5 lines
Anders Blomdell Email: [email protected] Department of Automatic Control Lund University Phone: +46 46 222 4625 P.O. Box 118 Fax: +46 46 138118 SE-221 00 Lund, Sweden
Quoted 5 lines · Anders Blomdell
Quoted 3 lines · Haavard Skinnemoen
Well, you could do it like that, but you could also just add another
postcore_initcall. The only hook which is really needed is the "setup" part, and only if you have additional serial ports.Does that mean that '
at32_add_device_usart(...);' is not needed?
It's needed, but you don't need a hook in the main board code to do it. You can simply do
static int __init my_expansion_board_init(void)
{
at32_add_device_usart(...);
/* more stuff */
}postcore_initcall(my_expansion_board_init);
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
An even nicer way of handling it (provided that initialization does not need to take place during boot), might be to do
EXPORT_SYMBOL()onat32_add_device_usart,at32_map_usart, etc and then write a loadable module that handles the initialization.Hmm...why would that be nicer exactly?
You could compile your board specific inits outside the kernel tree, making it much easier to follow kernel versions (not everyone gets their board specific code into the kernel tree :-).
Adding EXPORT_SYMBOLs purely for use by out-of-tree modules is generally frowned upon. And patching in an additional file to the build is just about the easiest thing you can do.
Quoted 4 lines
Quoted 3 lines
What _I_ think would be a nicer way to do it is to implement support for flattened device trees and get rid of the board code entirely. Or almost entirely; it depends on how complete we can make the device tree.
I don't understand the above paragraph, could you please elaborate?
Haavard
Quoted 21 lines · Haavard Skinnemoen
Quoted 5 lines · Anders Blomdell
Quoted 3 lines · Haavard Skinnemoen
Well, you could do it like that, but you could also just add another
postcore_initcall. The only hook which is really needed is the "setup" part, and only if you have additional serial ports.Does that mean that '
at32_add_device_usart(...);' is not needed?It's needed, but you don't need a hook in the main board code to do it. You can simply do
static int
__initmy_expansion_board_init(void){ at32_add_device_usart(...); /* more stuff */ }
postcore_initcall(my_expansion_board_init);Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
An even nicer way of handling it (provided that initialization does not need to take place during boot), might be to do
EXPORT_SYMBOL()onat32_add_device_usart,at32_map_usart, etc and then write a loadable module that handles the initialization.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Hmm...why would that be nicer exactly?
You could compile your board specific inits outside the kernel tree, making it much easier to follow kernel versions (not everyone gets their board specific code into the kernel tree :-).
Adding
EXPORT_SYMBOLspurely for use by out-of-tree modules is generally frowned upon.
If I'm not mistaken, you need EXPORT_SYMBOL for any loadable module, be it in- or out-of-tree.
Quoted 2 lines
And patching in an additional file to the build is just about the easiest thing you can do.
Next to config options and loadable modules out-of-tree :-)
Quoted 6 lines
Quoted 4 lines
Quoted 3 lines
What _I_ think would be a nicer way to do it is to implement support for flattened device trees and get rid of the board code entirely. Or almost entirely; it depends on how complete we can make the device tree.
I don't understand the above paragraph, could you please elaborate?
OK, essentially compile all drivers into the kernel and let the boot-loader tell us what to activate. Gives a small kernel bloat (which I can live with), but what if two conflicting drivers (e.g. same I/O pin) are selected by the flattened tree?
But I take your replies that you are adamantly against kernel options for the atngw100, so I'll drop the subject for now (letting everyone figure out their favorite way to configure modified boards).
Regards
Anders
Signature · 5 lines
Anders Blomdell Email: [email protected] Department of Automatic Control Lund University Phone: +46 46 222 4625 P.O. Box 118 Fax: +46 46 138118 SE-221 00 Lund, Sweden
Quoted 5 lines · Anders Blomdell
Quoted 2 lines · Haavard Skinnemoen
Adding
EXPORT_SYMBOLspurely for use by out-of-tree modules is generally frowned upon.If I'm not mistaken, you need
EXPORT_SYMBOLfor any loadable module, be it in- or out-of-tree.
Yes, but no in-kernel modules need those symbols exported.
Quoted 3 lines
Quoted 2 lines
And patching in an additional file to the build is just about the easiest thing you can do.
Next to config options and loadable modules out-of-tree :-)
Really? I personally think out-of-tree modules is a real hassle causing extra work each time I need to update the kernel, whether it's on my embedded target or my PC.
On the other hand, a trivial patch adding support for an expansion board can just stay in your tree while you do a 'git pull' to grab the most up-to-date kernel.
Even better, if you send your trivial patch upstream, it will just be there the next time you update your kernel. No need to do anything extra at all.
A Kconfig option is admittedly just as easy, but with the added downside that it makes the board code less readable, and there will never be enough of them to support every board mod that people make.
Quoted 10 lines
Quoted 6 lines
Quoted 4 lines
Quoted 3 lines
What _I_ think would be a nicer way to do it is to implement support for flattened device trees and get rid of the board code entirely. Or almost entirely; it depends on how complete we can make the device tree.
I don't understand the above paragraph, could you please elaborate?
OK, essentially compile all drivers into the kernel and let the boot-loader tell us what to activate. Gives a small kernel bloat (which I can live with), but what if two conflicting drivers (e.g. same I/O pin) are selected by the flattened tree?
That's how it works today, except that it's the board code which specifies what to activate instead of the boot loader (or a device tree blob in flash.) So there won't be any additional bloat unless you go and select lots of drivers you don't need.
On the other hand, these functions are currently __init, so if we export them for use by modules, it will definitely bloat the kernel.
Quoted 3 lines
But I take your replies that you are adamantly against kernel options for the atngw100, so I'll drop the subject for now (letting everyone figure out their favorite way to configure modified boards).
I'm not fundamentally against kernel options for the atngw100. I just don't like cluttering up the board code.
If you add support for an expansion board, you'll definitely need a config option to select it as well. But that option is only used to select the implementation file for that board; it won't add any additional mess to the core NGW100 code.
Have a look at the EVKLCD10x code that was just applied for an example of how to add support for an expansion board:
Most of it is LCD-related, so if you only need to add a USART, you'll only need a fraction of the code.
Also note that it shows a huge problem with the Kconfig approach: Most devices need additional board-specific data. And even though the USART doesn't currently need any of that, it may in the future when we add support for RS485, hardware flow control, etc. How will you deal with that through Kconfig?
Haavard
Quoted 8 lines · Haavard Skinnemoen
Quoted 5 lines · Anders Blomdell
Quoted 2 lines · Haavard Skinnemoen
Adding
EXPORT_SYMBOLspurely for use by out-of-tree modules is generally frowned upon.If I'm not mistaken, you need
EXPORT_SYMBOLfor any loadable module, be it in- or out-of-tree.Yes, but no in-kernel modules need those symbols exported.
No, that's right.
Quoted 7 lines
Quoted 3 lines
Quoted 2 lines
And patching in an additional file to the build is just about the easiest thing you can do.
Next to config options and loadable modules out-of-tree :-)
Really? I personally think out-of-tree modules is a real hassle causing extra work each time I need to update the kernel, whether it's on my embedded target or my PC.
Depends on if it's you or somebody else (who doesn't care about your code) that rebuilds the kernel. Since I plan to use some tens of atngw100's eventually, with at least 3 different expansions boards, a solution that doesn't need 3 different kernels (i.e. loadable modules) is much preferred to a Kconfig (I know I started out with this, but one does change opinion :-) occasionally) or in-tree init code.
Quoted 42 lines
On the other hand, a trivial patch adding support for an expansion board can just stay in your tree while you do a 'git pull' to grab the most up-to-date kernel.
Even better, if you send your trivial patch upstream, it will just be there the next time you update your kernel. No need to do anything extra at all.
A
Kconfigoption is admittedly just as easy, but with the added downside that it makes the board code less readable, and there will never be enough of them to support every board mod that people make.Quoted 9 lines
Quoted 5 lines
Quoted 4 lines
Quoted 3 lines
What _I_ think would be a nicer way to do it is to implement support for flattened device trees and get rid of the board code entirely. Or almost entirely; it depends on how complete we can make the device tree.
I don't understand the above paragraph, could you please elaborate?
OK, essentially compile all drivers into the kernel and let the boot-loader tell us what to activate. Gives a small kernel bloat (which I can live with), but what if two conflicting drivers (e.g. same I/O pin) are selected by the flattened tree?
That's how it works today, except that it's the board code which specifies what to activate instead of the boot loader (or a device tree blob in flash.) So there won't be any additional bloat unless you go and select lots of drivers you don't need.
On the other hand, these functions are currently
__init, so if we export them for use by modules, it will definitely bloat the kernel.Quoted 3 lines
But I take your replies that you are adamantly against kernel options for the atngw100, so I'll drop the subject for now (letting everyone figure out their favorite way to configure modified boards).
I'm not fundamentally against kernel options for the atngw100. I just don't like cluttering up the board code.
If you add support for an expansion board, you'll definitely need a config option to select it as well. But that option is only used to select the implementation file for that board; it won't add any additional mess to the core NGW100 code.
Quoted 7 lines
Have a look at the EVKLCD10x code that was just applied for an example of how to add support for an expansion board:
Most of it is LCD-related, so if you only need to add a USART, you'll only need a fraction of the code.
I see a distinct clutter in setup.c (#ifndef CONFIG_BOARD_ATNGW100_EVKLCD10X) :-) (it clearly shows the need for conflict resolution, though)
Quoted 5 lines
Also note that it shows a huge problem with the
Kconfigapproach: Most devices need additional board-specific data. And even though the USART doesn't currently need any of that, it may in the future when we add support for RS485, hardware flow control, etc. How will you deal with that throughKconfig?
Honestly, I don't know :-(, but it would be nice to be able to select all these options wihout having to write the code myself :-)
Question is if there is any way to come up with a solution that makes it possible to select software modules for what is present on a specific expansion board, my naive thought was that it should be selected by Kconfig (and in that vein, my 5 cents is that video for ngw100/stk100x should be selected as TCG057QVLAD/PH320240T/LTV350QV, and not as a by-product of selecting a specific expansion board, this way it would be easier to add video to a custom expansion board withou dragging in unrelated functions like USARTs). Perhaps it is possible to solve my problems as well as the stk100x clutter if it is done right?
Regards
Anders
Signature · 5 lines
Anders Blomdell Email: [email protected] Department of Automatic Control Lund University Phone: +46 46 222 4625 P.O. Box 118 Fax: +46 46 138118 SE-221 00 Lund, Sweden
Quoted 10 lines · Anders Blomdell
Quoted 3 lines · Haavard Skinnemoen
Really? I personally think out-of-tree modules is a real hassle causing extra work each time I need to update the kernel, whether it's on my embedded target or my PC.
Depends on if it's you or somebody else (who doesn't care about your code) that rebuilds the kernel. Since I plan to use some tens of atngw100's eventually, with at least 3 different expansions boards, a solution that doesn't need 3 different kernels (i.e. loadable modules) is much preferred to a
Kconfig(I know I started out with this, but one does change opinion :-) occasionally) or in-tree init code.
So what you really need is device tree support.
I suppose modules would do the trick in your case, but it sounds like a very specialized setup, and I don't feel too good about bloating the kernel for everyone else just to support this kind of thing.
Device tree support would probably make the kernel bigger too, but I think that would be mostly __init code, so it doesn't matter as much.
Quoted 9 lines
Quoted 7 lines
Have a look at the EVKLCD10x code that was just applied for an example of how to add support for an expansion board:
Most of it is LCD-related, so if you only need to add a USART, you'll only need a fraction of the code.
I see a distinct clutter in
setup.c(#ifndefCONFIG_BOARD_ATNGW100_EVKLCD10X) :-) (it clearly shows the need for conflict resolution, though)
Yes, but it's hard to avoid that, it's confined to the support code for that particular expansion board, and the clutter is there to support actual variations of the board. It could have been moved into separate files, but that would have led to more duplication.
It's all about finding the right balance, really.
I didn't quite understand what you mean by "conflict resolution".
Quoted 7 lines
Quoted 5 lines
Also note that it shows a huge problem with the
Kconfigapproach: Most devices need additional board-specific data. And even though the USART doesn't currently need any of that, it may in the future when we add support for RS485, hardware flow control, etc. How will you deal with that throughKconfig?Honestly, I don't know :-(, but it would be nice to be able to select all these options wihout having to write the code myself :-)
But is wading through several dozen config options really that much easier than writing ten lines of code?
Quoted 8 lines
Question is if there is any way to come up with a solution that makes it possible to select software modules for what is present on a specific expansion board, my naive thought was that it should be selected by
Kconfig(and in that vein, my 5 cents is that video for ngw100/stk100x should be selected as TCG057QVLAD/PH320240T/LTV350QV, and not as a by-product of selecting a specific expansion board, this way it would be easier to add video to a custom expansion board withou dragging in unrelated functions like USARTs). Perhaps it is possible to solve my problems as well as the stk100x clutter if it is done right?
Possibly. But adding options for the USARTs is only solving a tiny part of the problem, and I'm worried that solving the rest in the same manner is going to end up as a spectacular mess. If you can prove me wrong, I'm all for it.
Haavard
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
Dropped the kernel list CC on previous post, sorry.
Quoted 9 lines · Haavard Skinnemoen
Quoted 8 lines · Anders Blomdell
Quoted 6 lines
Quoted 2 lines
I see a distinct clutter in
setup.c(#ifndefCONFIG_BOARD_ATNGW100_EVKLCD10X) :-) (it clearly shows the need for conflict resolution, though)Yes, but it's hard to avoid that, it's confined to the support code for that particular expansion board, and the clutter is there to support actual variations of the board. It could have been moved into separate files, but that would have led to more duplication.
Nope, it's right in the
setup.cfile.
Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
It's all about finding the right balance, really.
I didn't quite understand what you mean by "conflict resolution".
That .detect_pin and .wp_pin conflicts with the LCD (so the #ifdef disables them)
Oh, that stuff. Yeah, when an expansion board needs to disable features on the motherboard, it gets a bit messy.
How should this be handled with device tree?
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
But is wading through several dozen config options really that much easier than writing ten lines of code?
Nope, but if there are changes in some kernel code, local changes can easily get out of sync.
That's why you should send it upstream so that it can be kept in sync whenever something changes. Also note that the same applies to your out-of-tree module.
Which is what I'm trying to do, but the maintainer is complaining :-) But we probably don't want to add everyones prototype boards to the kernel tree!
Quoted 21 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Question is if there is any way to come up with a solution that makes it possible to select software modules for what is present on a specific expansion board, my naive thought was that it should be selected by
Kconfig(and in that vein, my 5 cents is that video for ngw100/stk100x should be selected as TCG057QVLAD/PH320240T/LTV350QV, and not as a by-product of selecting a specific expansion board, this way it would be easier to add video to a custom expansion board withou dragging in unrelated functions like USARTs). Perhaps it is possible to solve my problems as well as the stk100x clutter if it is done right?Quoted 7 lines
Quoted 4 lines
Possibly. But adding options for the USARTs is only solving a tiny part of the problem, and I'm worried that solving the rest in the same manner is going to end up as a spectacular mess. If you can prove me wrong, I'm all for it.
It's not about proving either me or you wrong, it's about making this easy to do for everybody (not only me, I can live with modifying the kernel tree), if it is not possible, so be it.
I'm asserting that doing this kind of board customization through
Kconfigis going to get messy once we support enough features to make everyone happy. If you can show that it can be done in a clean way, i.e. prove me wrong, nothing would make me happier.
Quoted 4 lines
But I do think device trees along with a nice graphical editor, or a few u-boot commands, would go a long way towards this goal. If we manage to get something like that working, you won't even have to recompile anything.
As far as I can see, the device trees will push the conflict resolution to run-time instead of compile-time, which I belive is bad for both memory footprint as well as performance (as well as predictability, this kernel worked yesterday; who added which device which makes it crash today...)
Quoted 7 lines
Quoted 4 lines
But personally I would be happy with a generic ap7000
board, where I could pick all the options I like and the ngw100 and stk100x would just be an instance of this board with all/most options preselected. That #ifdefs are messy to read is something we agree about...Right, I also want more generic board support. But I don't think
Kconfigis the way to go. There are just too many variables.
Wouldn't there be as many variables with a device tree?
A graphical board-configurator against the Kconfig should certainly be possible?
I'm also not (yet) convinced that your approach makes the configuration any simpler.
How about putting each needed extension in a separate file (with a specified format), and use some kind of preprocessor to generate Kconfig's, Makefiles, setup.c, etc from those files (and generating the appropriate at32_reserve_*, at32_select* as well). I.e. something like:
USART2.def:
%PINS {
PA08, PA09
}
%GLOBAL {
platform device *usart2;
}
%INIT {
usart2 = at32_add_device_usart(2);
}
%SETUP {
new_at32_map_usart(usart2, at_32_last_mapped_usart++);
}
/Anders
Signature · 5 lines
Anders Blomdell Email: [email protected] Department of Automatic Control Lund University Phone: +46 46 222 4625 P.O. Box 118 Fax: +46 46 138118 SE-221 00 Lund, Sweden
Quoted 6 lines · On Mon, 13 Oct 2008 20:08:21 +0200 Anders Blomdell…
Dropped the kernel list CC on previous post, sorry.
Quoted 2 lines · Haavard Skinnemoen
Oh, that stuff. Yeah, when an expansion board needs to disable features on the motherboard, it gets a bit messy.
How should this be handled with device tree?
By simply leaving those pins out. Or perhaps there's some mechanism for disabling them.
Quoted 4 lines
Quoted 3 lines
That's why you should send it upstream so that it can be kept in sync whenever something changes. Also note that the same applies to your out-of-tree module.
Which is what I'm trying to do, but the maintainer is complaining :-)
No, you're trying to push a mechanism which will allow you to avoid doing that, at the expense of less readable board code for everyone else.
Quoted 1 line
But we probably don't want to add everyones prototype boards to the kernel tree!
Well, why not? It's not like it takes up much space. And it can always be removed later when the prototype gets reassigned to collecting dust at the top of some shelf.
It might even be a nice experience for later when you want support for your production board upstream...
Quoted 8 lines
Quoted 4 lines
But I do think device trees along with a nice graphical editor, or a few u-boot commands, would go a long way towards this goal. If we manage to get something like that working, you won't even have to recompile anything.
As far as I can see, the device trees will push the conflict resolution to run-time instead of compile-time, which I belive is bad for both memory footprint as well as performance (as well as predictability, this kernel worked yesterday; who added which device which makes it crash today...)
Any conflict resolution _today_ happens at run time in the form of a friendly error message and a stack dump if you try to assign the same pin to two different devices.
And with your patch, some device might suddenly stop working because you enabled a USART which conflicts with some other device that gets initialized later (the USARTs are initialized very early). How is that any better?
As for performance and memory footprint, I think the impact will be minimal. This is init code that we're talking about -- it gets run once and then discarded.
Oh, and the device tree stuff comes with some nice infrastructure which would make it quite easy to write a static validator for pin assignments and such. That would actually move the conflict resolution from run-time to compile-time.
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines
But personally I would be happy with a generic ap7000
board, where I could pick all the options I like and the ngw100 and stk100x would just be an instance of this board with all/most options preselected. That #ifdefs are messy to read is something we agree about...Right, I also want more generic board support. But I don't think
Kconfigis the way to go. There are just too many variables.Wouldn't there be as many variables with a device tree?
Yes, but you're exposing them to the right audience: The board vendors, not the users. In the case of hobbyists creating their own boards, they're of course the same people, but they are definitely power users.
Also, I believe that, if the device tree layout is carefully designed, it might be possible to turn bits of it on and off at run-time. That would be great for configurable boards like the STK1000 -- with a bit of extra support from u-boot, you should be able to do something like
switch 5 on
to select between two different mux settings (e.g. second ethernet controller vs. LCD).
Quoted 1 line
A graphical board-configurator against the
Kconfigshould certainly be possible?
It will probably be just as easy to have the board configurator generate C code, or a device tree.
Quoted 1 line
I'm also not (yet) convinced that your approach makes the configuration any simpler.
I guess we'll just have to implement it and find out. Until then, there's really no way to tell -- you may of course be right.
Quoted 4 lines
How about putting each needed extension in a separate file (with a specified format), and use some kind of preprocessor to generate Kconfig's, Makefiles,
setup.c, etc from those files (and generating the appropriate at32_reserve_*, at32_select* as well). I.e. something like:
If you're generating code, why bother with Kconfig at all?
Quoted 10 lines
USART2.def:
%PINS {
PA08, PA09
}%GLOBAL {
platform device *usart2;
}
Quoted 7 lines
%INIT {
usart2 =at32_add_device_usart(2);
}%SETUP {
new_at32_map_usart(usart2, at_32_last_mapped_usart++);
}
You've already written most of the board code, and added quite a bit of additional noise. Why not write it in C instead of some yet-another-weird-configuration-language?
Hey, why not use XML? I'm sure lots of LKML subscribers would be absolutely thrilled! ;-)
Haavard
Quoted 82 lines · Haavard Skinnemoen
Quoted 6 lines · On Mon, 13 Oct 2008 20:08:21 +0200 Anders Blomdell…
Dropped the kernel list CC on previous post, sorry.
Quoted 2 lines · Haavard Skinnemoen
Oh, that stuff. Yeah, when an expansion board needs to disable features on the motherboard, it gets a bit messy.
How should this be handled with device tree?
By simply leaving those pins out. Or perhaps there's some mechanism for disabling them.
Quoted 4 lines
Quoted 3 lines
That's why you should send it upstream so that it can be kept in sync whenever something changes. Also note that the same applies to your out-of-tree module.
Which is what I'm trying to do, but the maintainer is complaining :-)
No, you're trying to push a mechanism which will allow you to avoid doing that, at the expense of less readable board code for everyone else.
Quoted 1 line
But we probably don't want to add everyones prototype boards to the kernel tree!
Well, why not? It's not like it takes up much space. And it can always be removed later when the prototype gets reassigned to collecting dust at the top of some shelf.
It might even be a nice experience for later when you want support for your production board upstream...
Quoted 8 lines
Quoted 4 lines
But I do think device trees along with a nice graphical editor, or a few u-boot commands, would go a long way towards this goal. If we manage to get something like that working, you won't even have to recompile anything.
As far as I can see, the device trees will push the conflict resolution to run-time instead of compile-time, which I belive is bad for both memory footprint as well as performance (as well as predictability, this kernel worked yesterday; who added which device which makes it crash today...)
Any conflict resolution _today_ happens at run time in the form of a friendly error message and a stack dump if you try to assign the same pin to two different devices.
And with your patch, some device might suddenly stop working because you enabled a USART which conflicts with some other device that gets initialized later (the USARTs are initialized very early). How is that any better?
As for performance and memory footprint, I think the impact will be minimal. This is init code that we're talking about -- it gets run once and then discarded.
Oh, and the device tree stuff comes with some nice infrastructure which would make it quite easy to write a static validator for pin assignments and such. That would actually move the conflict resolution from run-time to compile-time.
Quoted 7 lines
Quoted 6 lines
Quoted 4 lines
But personally I would be happy with a generic ap7000
board, where I could pick all the options I like and the ngw100 and stk100x would just be an instance of this board with all/most options preselected. That #ifdefs are messy to read is something we agree about...Right, I also want more generic board support. But I don't think
Kconfigis the way to go. There are just too many variables.Wouldn't there be as many variables with a device tree?
Yes, but you're exposing them to the right audience: The board vendors, not the users. In the case of hobbyists creating their own boards, they're of course the same people, but they are definitely power users.
Also, I believe that, if the device tree layout is carefully designed, it might be possible to turn bits of it on and off at run-time. That would be great for configurable boards like the STK1000 -- with a bit of extra support from u-boot, you should be able to do something like
switch 5 on
to select between two different mux settings (e.g. second ethernet controller vs. LCD).
Quoted 1 line
A graphical board-configurator against the
Kconfigshould certainly be possible?
Quoted 7 lines
It will probably be just as easy to have the board configurator generate C code, or a device tree.
Quoted 1 line
I'm also not (yet) convinced that your approach makes the configuration any simpler.
I guess we'll just have to implement it and find out. Until then, there's really no way to tell -- you may of course be right.
OK, I'll wait for the device tree to appear...
Quoted 6 lines
Quoted 4 lines
How about putting each needed extension in a separate file (with a specified format), and use some kind of preprocessor to generate Kconfig's, Makefiles,
setup.c, etc from those files (and generating the appropriate at32_reserve_*, at32_select* as well). I.e. something like:If you're generating code, why bother with
Kconfigat all?
Because that is the way I'm used to select drivers when I configure a kernel (call me old fashioned if you will :-).
Quoted 14 lines
Quoted 14 lines
USART2.def:
%PINS {
PA08, PA09
}%GLOBAL {
platform device *usart2;
}%INIT {
usart2 =at32_add_device_usart(2);
}
Quoted 7 lines
Quoted 3 lines
%SETUP {
new_at32_map_usart(usart2, at_32_last_mapped_usart++);
}You've already written most of the board code, and added quite a bit of additional noise. Why not write it in C instead of some yet-another-weird-configuration-language?
Oh, that leads to lots of noise^H^H^H^H^H^H #ifdefs :-)
Quoted 2 lines
Hey, why not use XML? I'm sure lots of LKML subscribers would be absolutely thrilled! ;-)
XML is OK with me...
Signature · 5 lines
Anders Blomdell Email: [email protected] Department of Automatic Control Lund University Phone: +46 46 222 4625 P.O. Box 118 Fax: +46 46 138118 SE-221 00 Lund, Sweden