Re: [PATCH v10] usb_8dev: Add support for USB2CAN interface from 8 devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux