Re: [PATCH V3] can: usb: f81604: add Fintek F81604 support
From: Peter Hong
Date: Thu Mar 30 2023 - 01:41:45 EST
Hi Marc,
Marc Kleine-Budde 於 2023/3/27 下午 11:09 寫道:
On 27.03.2023 13:10:48, Ji-Ze Hong (Peter Hong) wrote:
This patch add support for Fintek USB to 2CAN controller support.
In addition to Vincent's comment, please describe your change
declarative, e.g.:
| Add support for the Fintek USB to 2CAN controller.
Please add an entry in the MAINTAINERS file.
We'll add description to MAINTAINERS file as following:
FINTEK F81604 USB to 2CAN DEVICE DRIVERS
M: Ji-Ze Hong (Peter Hong) <peter_hong@xxxxxxxxxxxxx>
L: linux-can@xxxxxxxxxxxxxxx
S: Maintained
Is this OK?
+ u8 bulk_read_buffer[F81604_MAX_RX_URBS][F81604_BULK_SIZE];
allocate the URB dynamic with kmalloc() and use urb->transfer_flags |=
URB_FREE_BUFFER for automatic free()ing.
Could I use kmalloc on device probe() and kfree on disconnect() ?
It may reduce a lot malloc/free times.
+
+ struct urb *write_urb;
+ u8 bulk_write_buffer[F81604_DATA_SIZE];
With just 1 TX URB the TX would be quite slow. Does your hardware
support more than 1 TX URB at a time? USB devices usually answer with
NAK it they are busy, automatic and transparent retry is handled by the
USB host controller.
This device only NAK when TX EP buf full. It maybe malfunction when sending
TX URB while CAN transmitting.
+};
+
+/* Interrupt endpoint data format: SR/IR/IER/ALC/ECC/EWLR/RXERR/TXERR/VAL */
+struct f81604_int_data {
+ u8 sr;
+ u8 isrc;
+ u8 ier;
+ u8 alc;
+ u8 ecc;
+ u8 ewlr;
+ u8 rxerr;
+ u8 txerr;
+ u8 val;
+} __packed;
I think this struct is always aligned to 4 bytes, so add a __aligned(4)
behind __packed.
+
+struct f81604_sff {
+ __be16 id;
+ u8 data[CAN_MAX_DLEN];
+} __packed;
__aligned(2)
+
+struct f81604_eff {
+ __be32 id;
+ u8 data[CAN_MAX_DLEN];
+} __packed;
__aligned(2)
+
+struct f81604_bulk_data {
+ u8 cmd;
+
+ /* According for F81604 DLC define:
+ * #define F81604_DLC_LEN_MASK 0x0f
+ * #define F81604_DLC_EFF_BIT BIT(7)
+ * #define F81604_DLC_RTR_BIT BIT(6)
no need to duplicate code
+ */
+ u8 dlc;
+
+ union {
+ struct f81604_sff sff;
+ struct f81604_eff eff;
+ };
+} __packed;
__aligned(4)
I had add following aligned is OK.
struct f81604_int_data; __aligned(4)
struct f81604_sff; __aligned(2)
struct f81604_eff; __aligned(2)
But when I add __aligned(4) to struct f81604_bulk_data, It'll generate
error about size != 14.
./include/linux/build_bug.h:78:41: error: static assertion failed:
"sizeof(struct f81604_bulk_data) == F81604_DATA_SIZE"
So I'll add align to these structs without f81604_bulk_data. Is it OK?
+ usb_fill_bulk_urb(priv->read_urb[i], priv->dev,
+ usb_rcvbulkpipe(priv->dev, bulk_in_addr[id]),
+ priv->bulk_read_buffer[i], F81604_BULK_SIZE,
+ f81604_read_bulk_callback, netdev);
+ }
+
+ priv->write_urb = usb_alloc_urb(0, GFP_KERNEL);
+ if (!priv->write_urb)
+ goto error;
+
+ usb_fill_bulk_urb(priv->write_urb, priv->dev,
+ usb_sndbulkpipe(priv->dev, bulk_out_addr[id]),
+ priv->bulk_write_buffer, F81604_DATA_SIZE,
+ f81604_write_bulk_callback, netdev);
+
+ priv->int_urb = usb_alloc_urb(0, GFP_KERNEL);
+ if (!priv->int_urb)
+ goto error;
+
+ usb_fill_int_urb(priv->int_urb, priv->dev,
+ usb_rcvintpipe(priv->dev, int_in_addr[id]),
+ priv->int_read_buffer, F81604_INT_SIZE,
+ f81604_read_int_callback, netdev, 1);
Does the endpoint descriptor specify a proper interval?
The interval of interrupt endpoint is 1 (1ms).
Bus 001 Device 010: ID 2c42:1709 FINTEK USB TO CANBUS BRIDGE
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 1.10
bDeviceClass 0
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 64
idVendor 0x2c42
idProduct 0x1709
bcdDevice 0.01
iManufacturer 1 FINTEK
iProduct 2 USB TO CANBUS BRIDGE
iSerial 3 88635600168801
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 0x003c
bNumInterfaces 1
bConfigurationValue 1
iConfiguration 0
bmAttributes 0xa0
(Bus Powered)
Remote Wakeup
MaxPower 100mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 6
bInterfaceClass 0
bInterfaceSubClass 0
bInterfaceProtocol 0
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0010 1x 16 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x82 EP 2 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x01 EP 1 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0010 1x 16 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x84 EP 4 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 0
Device Status: 0xa780
(Bus Powered)
Thanks