Hi Wolfgang,
2010/8/12 Wolfgang Denk wd@denx.de:
Dear Remy Bohmer,
In message AANLkTi=z0SX9YrXgH-_ogcPdUXhyrHADNErHVVBoNW7L@mail.gmail.com you wrote:
Use struct access for SoC registers. Offset adressing is depreciated. See the struct in my at91_dbgu.h. Use readl/writel to access the hardware.
Most of the code is copied from Linux. Do you really think we should rewrite it all?
Indeed. Using I/O accessors is mandatory in U-Boot.
Ok, but here ends the goal for easy synching with the kernel. In that case we can write our own USB gadget layer
Indeed, but again, it is copied from Linux...
Maybe we can do it better?
As mentioned: we can, but I did not want to at the time of writing that code.
This file is copied from Linux. That was the goal of this port; to be able to integrate new devices and fixes easier in the future. It was not our goal to make it nicer compared to Linux. If the comments are wrong, I would suggest to fix it in Linux first after which we integrate it in U-boot.
Come on. Here a bit statistics:
-> diff -u linux/drivers/usb/gadget/at91_udc.c u-boot/drivers/usb/gadget/at91_udc.c | wc -l 1057 -> wc -l linux/drivers/usb/gadget/at91_udc.c u_boot/drivers/usb/gadget/at91_udc.c 1908 linux/drivers/usb/gadget/at91_udc.c 1553 u-boot/drivers/usb/gadget/at91_udc.c
There are so many lines changed already in the code that claoming "This file is copied from Linux." does not make much sense any more.
Well, it is derived from 2.6.27, not latest git. True, there are differences, but mostly it is a strip down of the original code( /proc support is removed for example) And I also noticed that the file can be synced to Linux more than it currently is...
And many of the code changes are to the worse actually.
For example:
- ep->is_in = usb_endpoint_dir_in(desc); + ep->is_in = (desc->bEndpointAddress & USB_DIR_IN) != 0;
This was NOT the case in 2.6.27. So, it is not worse compared to 2.6.27...
Does this mean the system is dead and would need a watchdog or reset button to come alive again?
Yep, if that happens things are really broken...
So broken that you cannot get back to the command line interpreter? Really?
If malloc is broken, I consider that reason enough for hang/reboot...
- // terminer chaque requete dans la queue
C++ comment in unknown language ;)?
Linux code...
And that justifies it all.
No, but please look at the overall goal here. It was the goal to use the Linux code as much as-is, and to allow easy merging new driver code in the future. Everything can be made better, but in that case I would prefer to first make the origin better, and port that back to U-boot.
Further, I am not planning to rewrite the code to the struct based register access _right now_, since the struct based access mechanism is somehow work in progress that is not finalised yet (CONFIG_AT91_LEGACY is still defined for at91sam9261ek). I am not going to shoot on a moving duck. Also because you mention that at91_dbgu.h patches float around that are not in mainline yet. I will probably clean it up later when things are stabilised.
So, if you prefer I can drop patch 2/3 and 3/3 from the series, since those are only needed to prove that patch 1/3 works and to have at least 1 board in U-boot that make use of the USB-CDC-ECM gadget layer... I expect that there will be other boards soon that will use it, since others are working on it right now.
Kind regards,
Remy