Mike Frysinger wrote:
On Sunday 01 November 2009 11:16:59 Wolfgang Grandegger wrote:
Mike Frysinger wrote:
On Sunday 01 November 2009 06:33:33 Wolfgang Grandegger wrote:
--- /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.
this makes sense if the device supports a limited number of baud rates. but what if the baud rate is arbitrary (between two limits) ?
Board specific code can define what ever table it likes, including non-standard bit-rate and bit-timings. Nevertheless, for most CAN controllers the default bit-timing parameters are just fine.
+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.
it's (thankfully) been changing to Linux kernel style
I really appreciate that.
+struct can_dev *can_init (int dev_num, int ibaud)
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?
i dont think so
Nor do I.
Wolfgang.