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) ? > >> +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 > >> +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 > >> --- /dev/null > >> +++ b/include/can.h > >> @@ -0,0 +1,70 @@ > >> +/* > >> + * (C) Copyright 2007-2009, Wolfgang Grandegger <w...@denx.de> > > > > have you really been working on this stuff since 2007 ? > > The code was written in 2007. "2007, 2009" is more appropriate. the intro made it sound like this was a recent development (i.e. last few days/weeks). if code is actually from 2007, then the range is fine. -mike
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot