On Thu, Sep 10, 2009 at 02:29:37PM +0200, Wolfgang Grandegger wrote:
[...]
> >>> Index: include/linux/can/dev.h
> >>> ===================================================================
> >>> --- include/linux/can/dev.h (Revision 1048)
> >>> +++ include/linux/can/dev.h (Arbeitskopie)
> >>> @@ -32,7 +32,7 @@
> >>> /*
> >>> * CAN common private data
> >>> */
> >>> -#define CAN_ECHO_SKB_MAX 4
> >>> +#define CAN_ECHO_SKB_MAX 10
> >>>
> >>> struct can_priv {
> >>> #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,23)
> >>> Index: include/socketcan/can/dev.h
> >>> ===================================================================
> >>> --- include/socketcan/can/dev.h (Revision 1048)
> >>> +++ include/socketcan/can/dev.h (Arbeitskopie)
> >>> @@ -32,7 +32,7 @@
> >>> /*
> >>> * CAN common private data
> >>> */
> >>> -#define CAN_ECHO_SKB_MAX 4
> >>> +#define CAN_ECHO_SKB_MAX 10
> >>>
> >>> struct can_priv {
> >>> #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,23)
> >> For the moment I want to keep the old settings. We need to discuss how
> >> to handle this in a generic way. Either by increasing it to 8, let's
> >> say, or by providing a kernel config. Other opinions?
> > with regard to CAN_ECHO_SKB_MAX, I had the same problem in the softing
> > driver.
> > would some interface like this:
> [...]
> > +struct net_device *alloc_candev(int sizeof_priv, int echo_skb_max);
>
> This is a good solution if we want the driver to set the number of echo
> skb's. I would actually put the echo_skbs at the end of the
> netdev_priv(). Would it also be useful that the user decides via kernel
> configuration option?
1) 'behind netdev_priv'
so a layout in memory:
[[can_priv]driver_priv][echo_skb[]]
this is possible, not breaking things. But still, you'd need a pointer
in from within can_priv, as the exact size of the priv is lost after the
constructor. so, it justs saves the heap overhead.
It must then be allocated during alloc_candev(), and requires an extra
argument. I'm not sure how easily such change gets accepted in mainline.
2) 'kernel config option'
I assume that every device will have the same echo_skb_max then?
Before choosing a value, a user needs to be informed very well, but I
can imagine that a user does not even understand the consequences, ie. I
have difficulties understanding the relation between echo_skb_max & URB
traffic in a driver _I didn't write_. How would I choose a good value?
IMO, such things are very driver specific, and thus better put const in
the driver. But this requires a variable echo_skb_max from can_priv
scope. That why I put both echo_skb & echo_skb_max as members.
This addressess the problematic choice of a static echo_skb_max that
would otherwise be left to the end user.
Please mind this does not address how to malloc such things (ie. after
netdev_priv, or seperate block in heap).
Kurt
>
> Wolfgang.
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
