Mike Frysinger wrote:
On Sunday 01 November 2009 06:33:33 Wolfgang Grandegger wrote:
[snip]
--- /dev/null +++ b/drivers/can/can.c
+static char *baudrates[] = { "125K", "250K", "500K" };
so we're restricting ourselves to these three speeds ? i have passing familiarity with CAN, but i didnt think the protocol was restricted to specific speeds.
Well, this is an RFC and as I wrote in the introduction some features need to be added or extended, especially for CAN device configuration. My idea is to have a more complete default bit-timing table, which board specific code may overwrite using, for example:
sja1000_register(&my_sja1000, &my_config_opts);
This would also allow to set the CAN clock, cdr and ocr registers.
+int can_register (struct can_dev* can_dev)
no space before the paren, and the * is cuddled on the wrong side of the space. seems like a lot of this code suffers from these two issues.
U-Boot coding style requires a space after the function name and before "(". But the "*" is misplaced, of course.
+{
- struct can_dev* dev;
- can_dev->next = NULL;
- if (!can_devs)
can_devs = can_dev;- else {
for (dev = can_devs; dev->next; dev = dev->next);dev->next = can_dev;- }
invert the if logic and i think the code would look "nicer" -- use braces on the first branch instead of the second.
OK.
+struct can_dev *can_init (int dev_num, int ibaud) +{
- struct can_dev *dev;
- int i;
- if (!can_devs) {
puts ("No CAN devices registered\n");return NULL;- }
- /* Advance to selected device */
- for (i = 0, dev = can_devs; dev; i++, dev = dev->next) {
if (i == dev_num)break;- }
- if (!dev) {
printf ("CAN device %d does not exist\n", dev_num);return NULL;- }
- printf ("Initializing CAN%d at 0x%08x with baudrate %s\n",
i, dev->base, baudrates[ibaud]);- dev->init (dev, ibaud);
- return dev;
+}
wonder if we should have a generic device list code base since this looks similar to a lot of other u-boot device lists ...
Do we already have a generic interface?
--- /dev/null +++ b/include/can.h @@ -0,0 +1,70 @@ +/*
- (C) Copyright 2007-2009, Wolfgang Grandegger wg@denx.de
have you really been working on this stuff since 2007 ?
The code was written in 2007. "2007, 2009" is more appropriate.
+struct can_dev {
- char *name;
const ?
OK.
Wolfgang.