Sebastian Haas wrote:
> Index: include/linux/cpc.h
> ===================================================================
> --- include/linux/cpc.h (Revision 0)
> +++ include/linux/cpc.h (Revision 0)
> @@ -0,0 +1,440 @@
> +/*
> + * CPC CAN Interface Definitions
Please check, if the needed definitions could become part of ems_usb.c
> +
> +// check the operating system used
> +#ifdef _WIN32 // running a Windows OS
e.g. this could probably be removed ;-)
> + // Kernel headers already define this types
> + #ifndef __KERNEL__
> + // define basic types
> + typedef unsigned char u8;
> + typedef unsigned short u16;
> + typedef unsigned int u32;
> + #endif
and this also
> +
> +// representation of the CAN parameters for the PCA82C200 controller
> +typedef struct CPC_PCA82C200_PARAMS {
> + u8 acc_code; // Acceptance-code for receive, Standard: 0
> + u8 acc_mask; // Acceptance-mask for receive, Standard: 0xff
> + u8 btr0; // Bus-timing register 0
> + u8 btr1; // Bus-timing register 1
> + u8 outp_contr; // Output-control register
> +} CPC_PCA82C200_PARAMS_T;
> +
> +// representation of the CAN parameters for the SJA1000 controller
> +typedef struct CPC_SJA1000_PARAMS {
> + u8 mode; // enables single or dual acceptance filtering
> + u8 acc_code0; // Acceptance-code for receive, Standard: 0
> + u8 acc_code1;
> + u8 acc_code2;
> + u8 acc_code3;
> + u8 acc_mask0; // Acceptance-mask for receive, Standard: 0xff
> + u8 acc_mask1;
> + u8 acc_mask2;
> + u8 acc_mask3;
> + u8 btr0; // Bus-timing register 0
> + u8 btr1; // Bus-timing register 1
> + u8 outp_contr; // Output-control register
> +} CPC_SJA1000_PARAMS_T;
> +
> +// representation of the CAN parameters for the M16C controller
> +// in basic CAN mode (means no full CAN)
> +typedef struct CPC_M16C_BASIC_PARAMS {
> + u8 con0;
> + u8 con1;
> + u8 ctlr0;
> + u8 ctlr1;
> + u8 clk;
> + u8 acc_std_code0;
> + u8 acc_std_code1;
> + u8 acc_ext_code0;
> + u8 acc_ext_code1;
> + u8 acc_ext_code2;
> + u8 acc_ext_code3;
> + u8 acc_std_mask0;
> + u8 acc_std_mask1;
> + u8 acc_ext_mask0;
> + u8 acc_ext_mask1;
> + u8 acc_ext_mask2;
> + u8 acc_ext_mask3;
> +} CPC_M16C_BASIC_PARAMS_T;
typedefs are bad.
This will bounce on netdev-ML ...
> +#ifdef _WIN32
WIN what? :-))
> +// OVERRUN ///////////////////////////////////////
> +// In general two types of overrun may occur.
> +// A hardware overrun, where the CAN controller
> +// lost a message, because the interrupt was
> +// not handled before the next messgae comes in.
> +// Or a software overrun, where i.e. a received
> +// message could not be stored in the CPC_MSG
> +// buffer.
> +
> +// After a software overrun has occurred
> +// we wait until we have CPC_OVR_GAP slots
> +// free in the CPC_MSG buffer.
(..)
Nice documentation but the '//' comments need to be converted to /* */
> Index: drivers/net/can/ems_usb.c
> ===================================================================
> --- drivers/net/can/ems_usb.c (Revision 0)
> +++ drivers/net/can/ems_usb.c (Revision 0)
(..)
> +struct ems_usb {
> + struct can_priv can; /* must be the first member */
> +
> + int open_time;
> +
> + struct usb_device *udev;
> + struct net_device *netdev;
> +
> + struct urb *rx_urb, *tx_urb, *intr_urb;
> +
> + __u8 *rx_buffer;
> +
> + __u8 *tx_msg_buffer; /* Buffer for synchronous commands */
> +
Are additional buffers really needed or can you probably better process the
data on the fly?
> +static void init_params(CPC_MSG_T *msg, __u8 btr0, __u8 btr1,
> + __u8 outp, __u8 mod)
> +{
> + msg->type = CPC_CMD_T_CAN_PRMS;
> + msg->length = CPC_MSG_HEADER_LEN + sizeof(CPC_CAN_PARAMS_T);
> + msg->msgid = 0;
> +
> + msg->msg.canparams.cc_type = SJA1000;
> +
> + msg->msg.canparams.cc_params.sja1000.acc_code0 = 0;
> + msg->msg.canparams.cc_params.sja1000.acc_code1 = 0;
> + msg->msg.canparams.cc_params.sja1000.acc_code2 = 0;
> + msg->msg.canparams.cc_params.sja1000.acc_code3 = 0;
> +
> + msg->msg.canparams.cc_params.sja1000.acc_mask0 = 0xFF;
> + msg->msg.canparams.cc_params.sja1000.acc_mask1 = 0xFF;
> + msg->msg.canparams.cc_params.sja1000.acc_mask2 = 0xFF;
> + msg->msg.canparams.cc_params.sja1000.acc_mask3 = 0xFF;
> +
> + msg->msg.canparams.cc_params.sja1000.btr0 = btr0;
> + msg->msg.canparams.cc_params.sja1000.btr1 = btr1;
> +
> + msg->msg.canparams.cc_params.sja1000.outp_contr = outp;
> + msg->msg.canparams.cc_params.sja1000.mode = mod;
> +}
init_params_sja1000() ?
> +static void ems_usb_rx_canmsg(struct ems_usb *dev, CPC_MSG_T *msg)
> +{
(..)
> +
> + 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 */
> + for (i = 0; i < cf->can_dlc; i++)
> + cf->data[i] = msg->msg.canmsg.msg[i];
> +
> + /* Clear remaining bytes */
> + while (i < 8)
> + cf->data[i++] = 0;
is obsoltete then.
> +
> +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?
>
> 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.
Thanks,
Oliver
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
