On Thu, Sep 23, 2010 at 11:39:58PM +0200, Peter Stuge wrote:
Uwe Hermann wrote:
I also cleaned up the Debug Port page in the wiki a bit.
Thanks for that! A link to #55 in Trac could be added, that's a libusb program to read the device, instead of the usb_debug module.
Will do, thanks.
+++ src/southbridge/amd/sb600/sb600_enable_usbdebug.c (Arbeitskopie)
..
+/* Required for successful build, but currently empty. */ +void set_debug_port(unsigned int port) +{ +}
Maybe make that a weak function in common code instead? Is there actually any instance of the function which is not empty?
Maybe, or maybe eliminate it all-together, I haven't looked at the code in detail.
There is one implementation where it's non-empty, the MCP55 (which I cannot check for correctness against, of course, as there's no public datasheet). But I assume the MCP55 code to be correct (and I guess it might also work for CK804, will try that and report back).
On MCP55 it seems that it's configurable on which physical USB port the Debug Port is located, thus a set_debug_port() is required. On ICH7 (probably all ICH*) this is not the case, Debug Port is always on physical USB port 1 (hardcoded as per datasheet). I _think_ the same might be true for SB600/SB700, the datasheet says the default is "1" (port 1), but they don't clearly say whether a board designer or firmware can change that register bit.
+++ src/mainboard/asrock/939a785gmh/romstage.c (Arbeitskopie)
..
+#if CONFIG_USBDEBUG +#include "southbridge/amd/sb700/sb700_enable_usbdebug.c" +#include "pc80/usbdebug_serial.c" +#endif
Can this go somewhere outside the mainboard directory?
Maybe, yes, but that's probably a follow-up patch.
If anything does need to go in the mainboard dir now, then would it be enough to add pc80/usbdebug_serial.c, and without #if? The #if could easily be moved into that file to save repetitive lines. And I'm thinking that maybe sb700_enable_usbdebug.c could be pulled in when both SOUTHBRIDGE_AMD_SB700 and CONFIG_USBDEBUG are selected?
I agree, that would be nice (if it can be done).
+++ src/mainboard/gigabyte/ma78gm/romstage.c (Arbeitskopie)
..
@@ -139,6 +145,12 @@ it8718f_disable_reboot(); uart_init(); console_init();
+#if CONFIG_USBDEBUG
- sb700_enable_usbdebug(0);
- early_usbdebug_init();
+#endif
- printk(BIOS_DEBUG, "\n");
Here console_init() comes before early_usbdebug_init(), that's different from above. Does it matter?
Don't know, that was an error on my side, I intended all such blocks to be in the same place. I don't really know how early or late the functions need to or should be called, It just sounded like it'd make sense to do it before console_init(), but maybe there's a better place.
I know the Debug Port stuff can use more work, I'll try to fix further issues in follow-up patches, but IMHO this one can go in as it improves the situation (multiple boards with Debug Port debug in coreboot vs. none).
Uwe.