From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wolfgang Grandegger <wg@grandegger.com>
Date: Sun, 01 Nov 2009 23:00:17 +0100
Subject: [U-Boot] [RFC 1/5] CAN interface library
In-Reply-To: <200911011305.58099.vapier@gentoo.org>
References: <1257075217-26623-1-git-send-email-wg@grandegger.com>	<200911010936.10570.vapier@gentoo.org>	<4AEDB47B.3010101@grandegger.com>
	<200911011305.58099.vapier@gentoo.org>
Message-ID: <4AEE04F1.3040809@grandegger.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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.