On 09/08/2009 03:10 PM, Oliver Hartkopp wrote:
> Sebastian Haas wrote:
[snip]
>> + if(msg->type == CPC_MSG_T_RTR || msg->type == CPC_MSG_T_XRTR) {
>> + cf->can_id |= CAN_RTR_FLAG;
>> + } else {
>
> *(u64 *) (&cf.data) = 0; /* clear payload */
Hm, do we need to clear the payload at all?
[snip]
>> +
>> +static struct can_bittiming_const ems_usb_bittiming_const = {
>> + .name = "ems_usb",
>> + .tseg1_min = 1,
>> + .tseg1_max = 16,
>> + .tseg2_min = 1,
>> + .tseg2_max = 8,
>> + .sjw_max = 4,
>> + .brp_min = 1,
>> + .brp_max = 64,
>> + .brp_inc = 1,
>> +};
>
>
> maybe:
>
>
> static struct can_bittiming_const ems_usb_sja1000_bittiming_const = {
> .name = "ems_usb_sja1000",
> .tseg1_min = 1,
> (..)
> };
>
> static struct can_bittiming_const ems_usb_m16_bittiming_const = {
> .name = "ems_usb_m16",
> .tseg1_min = 1,
> (..)
> };
>
> @Wolfgang: would this be a correct naming scheme as the ems_usb driver has
> different hardware types?
The name should be "sja1000" in case there is no difference to the other
SJA1000 CAN controllers. What is "m16"?
>> Any ideas? I hope I'm using the candev interface correctly.
>>
>
> This was a really good start to merge the stuff!
>
> The missing parts should be connectable by the time.
There are more issues, e.g. "__u8" -> "u8", no typedefs, lowercase names
for structs, ...
Also I don't like that we introduce another set of SJA1000 register
definitions and offsets. Furthermore, there are many UBS based SJA1000
devices on the market and we should think if we could provide a common
interface for these, if feasible. E.g. sja1000_usb.c similar to sja1000.c.
Wolfgang.
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
