Hi Wolfgang,

On Fri, Nov 27, 2009 at 10:24:46AM +0100, Wolfgang Grandegger wrote:
> Hi Fu,
> 
> Luotao Fu wrote:
> > Hi,
> > 
> > On Thu, Nov 26, 2009 at 01:09:55PM +0100, Wolfgang Grandegger wrote:
> >> Wolfgang Grandegger wrote:
> >>> Hi Fu,
> >>>
> >>> Luotao Fu wrote:
> > <snip>
> >> Just looked to the code. scan_do_restart() just does the bus-off
> >> recovery if appropriate. But I realized that the scan_set_* functions do
> >>  stop the device before setting the property. That's dangerous and error
> >> prune and therefore we did not allow it in the kernel. It's up to the
> >> user/application to handle up/down properly. Furthermore, these
> >> functions seem to start the device even if it was not up before.
> >>
> > 
> > I updated the GIT tree. Now we have
> > scan_do_start/stop/restart
> > callbacks to start/stop/restart the device. Also elimanated duplicated
> > code and remove the internal starting/stoping the device in _set calls.
> > canconfig is updated too. I will rename the library to simply
> > libsocketcan and the API prefix to can_. All good with these names?
> 
> At a first glance, it looks good now. My favorite is libcan. 

let's stick to libsocketcan, libcan sounds somehow like libobama ;-) I'll
change the API namespace prefix to can_, so hopefull every one is
satisfied.

> Just one
> final comment about:
> 
>   int scan_set_bitrate(const char *name, __u32 bitrate, __u32 sample_point)
>   {
>         struct can_bittiming bt;
> 
>         memset(&bt, 0, sizeof(bt));
>         bt.bitrate = bitrate;
>         bt.sample_point = sample_point;
> 
>         return scan_set_bittiming(name, &bt);
>   }
> 
> I would prefer two functions here:
> 
>   int scan_set_bitrate(const char *name, __u32 bitrate)
>   int scan_set_bitrate_sample_point(const char *name, __u32 bitrate, __u32 
> sample_point)
> 
> Not sure if we need the latter, though. This would avoid non-expert users
> using non-standard sample-points.
> 

I'd prefer not to split it, since they're parameters, which are closelsy
related to another. sample_point will anyway be ignored, if it's set to
zero. To avoid misuse we can add some extra hint on this to the (still
non-existent) documentation.

cheers
Fu
-- 
Pengutronix e.K.                           | Dipl.-Ing. Luotao Fu        |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Attachment: signature.asc
Description: Digital signature

_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core

Reply via email to