[U-Boot] [RFC 1/5] CAN interface library

[Mike Frysinger]

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 <wg at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091101/6367ebef/attachment.pgp