On 18.01.2013 15:24, Bernd Krumboeck wrote: > Add device driver for USB2CAN interface from "8 devices" (http://www.8devices.com). > > Signed-off-by: Bernd Krumboeck <krumboeck@xxxxxxxxxxxxxxx> > Acked-by: Wolfgang Grandegger <wg@xxxxxxxxxxxxxx> You can add my Tested-by: Oliver Hartkopp <socketcan@xxxxxxxxxxxx> in the v11 post :-) I have only one small cosmetic change request: Please remove/split up this #define block. > + > +/* bittiming constants */ > +#define USB_8DEV_ABP_CLOCK 32000000 /* device CAN clock */ #define USB_8DEV_ABP_CLOCK 32000000 > +#define USB_8DEV_BAUD_MANUAL 0x09 -> moved down, see below > +#define USB_8DEV_TSEG1_MIN 1 > +#define USB_8DEV_TSEG1_MAX 16 > +#define USB_8DEV_TSEG2_MIN 1 > +#define USB_8DEV_TSEG2_MAX 8 > +#define USB_8DEV_SJW_MAX 4 > +#define USB_8DEV_BRP_MIN 1 > +#define USB_8DEV_BRP_MAX 1024 > +#define USB_8DEV_BRP_INC 1 It makes no sense to define these values when you can specify them directly in the const struct can_bittiming_const below. This is usual and does not reduce the readability. > + > +/* setup flags */ > +#define USB_8DEV_SILENT 0x01 > +#define USB_8DEV_LOOPBACK 0x02 > +#define USB_8DEV_DISABLE_AUTO_RESTRANS 0x04 > +#define USB_8DEV_STATUS_FRAME 0x08 > + > +/* commands */ > +enum usb_8dev_cmd { > + USB_8DEV_RESET = 1, > + USB_8DEV_OPEN, > + USB_8DEV_CLOSE, > + USB_8DEV_SET_SPEED, > + USB_8DEV_SET_MASK_FILTER, > + USB_8DEV_GET_STATUS, > + USB_8DEV_GET_STATISTICS, > + USB_8DEV_GET_SERIAL, > + USB_8DEV_GET_SOFTW_VER, > + USB_8DEV_GET_HARDW_VER, > + USB_8DEV_RESET_TIMESTAMP, > + USB_8DEV_GET_SOFTW_HARDW_VER > +}; /* command options */ #define USB_8DEV_BAUD_MANUAL 0x09 > + > +#define USB_8DEV_CMD_START 0x11 > +#define USB_8DEV_CMD_END 0x22 > + (..) > + > +static const struct can_bittiming_const usb_8dev_bittiming_const = { > + .name = "usb_8dev", > + .tseg1_min = USB_8DEV_TSEG1_MIN, > + .tseg1_max = USB_8DEV_TSEG1_MAX, > + .tseg2_min = USB_8DEV_TSEG2_MIN, > + .tseg2_max = USB_8DEV_TSEG2_MAX, > + .sjw_max = USB_8DEV_SJW_MAX, > + .brp_min = USB_8DEV_BRP_MIN, > + .brp_max = USB_8DEV_BRP_MAX, > + .brp_inc = USB_8DEV_BRP_INC, > +}; > + insert values directly here. Regards, Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html