Re: Usb to can driver

["Max S." <max@schneidersoft.net>]

On Wed, 2013-04-24 at 23:24 +0200, Marc Kleine-Budde wrote:
> On 04/24/2013 07:40 PM, Oliver Hartkopp wrote:
> >>> assumptions that are made:
> >>> * The sizes of the struct can_frame members are id:u32 dlc:u8 data:u64 .
> >>> * It is also assumed that existing defines like CAN_EFF_FLAG in can.h
> >>> and can/error.h don't change, as they are used by the device to
> >>> construct the can_id field.
> >>
> >> You cannot rely in your firmware, that the struct can_frame and
> >> CAN_*_FLAG doesn't change. Please define your own struct.
> >>
> > 
> > 
> > Hm - i really appreciate the memcopy-only approach which can cope with the
> > host byte order directly. This is a real improvement on the host side.
> > 
> > The struct can_frame and the error message content is official Kernel API and
> > therefore can be assumed to be fix.
> 
> Yes but no. The struct can_frame to the _userspace_ is official Kernel
> API/ABI, but within the kernel the is no stable API (see
> stable_api_nonsense.txt). But if we go this way we should make compile
> time checks, so that the compilation breaks if the struct can_frame changes.

ok. tell me if i understood correctly.
A user-space program can safely assume:
sizeof((struct can_frame *)NULL)->can_id) == 4
and:
CAN_EFF_FLAG == 0x80000000U 

but a kernel-space driver can not?

If I cannot make sufficient assumptions about the struct can_frame and
CAN_*_FLAGs It will make the code harder to maintain in the long run.
the speed gains derived from a direct memcpy/assignment will later be
eaten up by the code designed to convert the old struct can_frame
received from the device to the new struct can_frame found in the
kernel.

The question is in the end, where should i draw the line.

One option would be to define a new struct ss_frame to standardize
communication between device & host, but still use the device_config to
allow the changing of byte order.
The driver would then need to do three assignments to rearrange padding,
and adjust any difference in the id flags:

* arrange for correct byte order beforehand...

can_frame->can_id  = CONVERT_ME_FLAGS(ss_frame->can_id);
can_frame->can_dlc = ss_frame->can_dlc;
can_frame->data    = ss_frame->data;

In this case the 'line' would be explicitly between the struct ss_frame
and the struct can_frame not floating somewhere vague between the kernel
and firmware...
Naturally you will no longer be able to simply:
*can_frame = frame_wrapper->can_frame;
As i am doing now.

What do you think?
Max Schneider.