Re: [PATCH v3 1/1] can: ucan: add driver for Theobroma Systems UCAN devices

2018-03-27 Thread Martin Elshuber
fixed and rechecked again with sparse

Am 24.03.18 um 12:43 schrieb kbuild test robot:
> Hi Jakob,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on net-next/master]
> [also build test WARNING on v4.16-rc6]
> [cannot apply to next-20180323]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Jakob-Unterwurzacher/can-ucan-add-driver-for-Theobroma-Systems-UCAN-devices/20180324-164143
> reproduce:
> # apt-get install sparse
> make ARCH=x86_64 allmodconfig
> make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)
>
>>> drivers/net/can/usb/ucan.c:301:16: sparse: restricted __le32 degrades to 
>>> integer
>drivers/net/can/usb/ucan.c:414:32: sparse: incorrect type in argument 5 
> (different base types) @@expected unsigned short [unsigned] [usertype] 
> value @@got  short [unsigned] [usertype] value @@
>drivers/net/can/usb/ucan.c:414:32:expected unsigned short [unsigned] 
> [usertype] value
>drivers/net/can/usb/ucan.c:414:32:got restricted __le16 [usertype] 
> 
>drivers/net/can/usb/ucan.c:428:32: sparse: incorrect type in argument 5 
> (different base types) @@expected unsigned short [unsigned] [usertype] 
> value @@got  short [unsigned] [usertype] value @@
>drivers/net/can/usb/ucan.c:428:32:expected unsigned short [unsigned] 
> [usertype] value
>drivers/net/can/usb/ucan.c:428:32:got restricted __le16 [usertype] 
> 
>drivers/net/can/usb/ucan.c:683:44: sparse: incorrect type in argument 2 
> (different base types) @@expected unsigned long [unsigned] [usertype] 
> size @@got d long [unsigned] [usertype] size @@
>drivers/net/can/usb/ucan.c:683:44:expected unsigned long [unsigned] 
> [usertype] size
>drivers/net/can/usb/ucan.c:683:44:got restricted __le16 [usertype] 
> wMaxPacketSize
>drivers/net/can/usb/ucan.c:700:44: sparse: incorrect type in argument 2 
> (different base types) @@expected unsigned long [unsigned] [usertype] 
> size @@got d long [unsigned] [usertype] size @@
>drivers/net/can/usb/ucan.c:700:44:expected unsigned long [unsigned] 
> [usertype] size
>drivers/net/can/usb/ucan.c:700:44:got restricted __le16 [usertype] 
> wMaxPacketSize
>drivers/net/can/usb/ucan.c:752:25: sparse: cast to restricted __le16
>drivers/net/can/usb/ucan.c:779:36: sparse: incorrect type in argument 5 
> (different base types) @@expected int [signed] buffer_length @@got 
> restricted __le1int [signed] buffer_length @@
>drivers/net/can/usb/ucan.c:779:36:expected int [signed] buffer_length
>drivers/net/can/usb/ucan.c:779:36:got restricted __le16 [usertype] 
> wMaxPacketSize
>drivers/net/can/usb/ucan.c:793:54: sparse: incorrect type in argument 2 
> (different base types) @@expected unsigned long [unsigned] [usertype] 
> size @@got d long [unsigned] [usertype] size @@
>drivers/net/can/usb/ucan.c:793:54:expected unsigned long [unsigned] 
> [usertype] size
>drivers/net/can/usb/ucan.c:793:54:got restricted __le16 [usertype] 
> wMaxPacketSize
>drivers/net/can/usb/ucan.c:853:62: sparse: incorrect type in argument 2 
> (different base types) @@expected unsigned long [unsigned] [usertype] 
> size @@got d long [unsigned] [usertype] size @@
>drivers/net/can/usb/ucan.c:853:62:expected unsigned long [unsigned] 
> [usertype] size
>drivers/net/can/usb/ucan.c:853:62:got restricted __le16 [usertype] 
> wMaxPacketSize
>drivers/net/can/usb/ucan.c:877:61: sparse: incorrect type in argument 2 
> (different base types) @@expected unsigned long [unsigned] [usertype] 
> size @@got d long [unsigned] [usertype] size @@
>drivers/net/can/usb/ucan.c:877:61:expected unsigned long [unsigned] 
> [usertype] size
>drivers/net/can/usb/ucan.c:877:61:got restricted __le16 [usertype] 
> wMaxPacketSize
>drivers/net/can/usb/ucan.c:891:44: sparse: incorrect type in argument 5 
> (different base types) @@expected int [signed] buffer_length @@got 
> restricted __le1int [signed] buffer_length @@
>drivers/net/can/usb/ucan.c:891:44:expected int [signed] buffer_length
>drivers/net/can/usb/ucan.c:891:44:got restricted __le16 [usertype] 
> wMaxPacketSize
>>> drivers/net/can/usb/ucan.c:1238:41: sparse: incorrect type in assignment 
>>> (different base types) @@expected restricted __le16 [usertype] 
>>> sample_point @@got restricted __le16 [usertype] sample_point @@
>drivers/net/can/usb/ucan.c:1238:41:expected restricted __le16 
> [usertype] sample_point
>drivers/net/can/usb/ucan.c:1238:41:got restricted __le32 [usertype] 
> 
>drivers/net/can/usb/ucan.c:1368:18: sparse: restricted __le16 degrades to 
> integer
>drivers/net/can/usb/ucan.c:1373:19: sparse: restricted __le16 degrades to 
> integer
>

Re: [PATCH v3 1/1] can: ucan: add driver for Theobroma Systems UCAN devices

2018-03-27 Thread Martin Elshuber
fixed and rechecked again with sparse

Am 24.03.18 um 12:43 schrieb kbuild test robot:
> Hi Jakob,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on net-next/master]
> [also build test WARNING on v4.16-rc6]
> [cannot apply to next-20180323]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Jakob-Unterwurzacher/can-ucan-add-driver-for-Theobroma-Systems-UCAN-devices/20180324-164143
> reproduce:
> # apt-get install sparse
> make ARCH=x86_64 allmodconfig
> make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)
>
>>> drivers/net/can/usb/ucan.c:301:16: sparse: restricted __le32 degrades to 
>>> integer
>drivers/net/can/usb/ucan.c:414:32: sparse: incorrect type in argument 5 
> (different base types) @@expected unsigned short [unsigned] [usertype] 
> value @@got  short [unsigned] [usertype] value @@
>drivers/net/can/usb/ucan.c:414:32:expected unsigned short [unsigned] 
> [usertype] value
>drivers/net/can/usb/ucan.c:414:32:got restricted __le16 [usertype] 
> 
>drivers/net/can/usb/ucan.c:428:32: sparse: incorrect type in argument 5 
> (different base types) @@expected unsigned short [unsigned] [usertype] 
> value @@got  short [unsigned] [usertype] value @@
>drivers/net/can/usb/ucan.c:428:32:expected unsigned short [unsigned] 
> [usertype] value
>drivers/net/can/usb/ucan.c:428:32:got restricted __le16 [usertype] 
> 
>drivers/net/can/usb/ucan.c:683:44: sparse: incorrect type in argument 2 
> (different base types) @@expected unsigned long [unsigned] [usertype] 
> size @@got d long [unsigned] [usertype] size @@
>drivers/net/can/usb/ucan.c:683:44:expected unsigned long [unsigned] 
> [usertype] size
>drivers/net/can/usb/ucan.c:683:44:got restricted __le16 [usertype] 
> wMaxPacketSize
>drivers/net/can/usb/ucan.c:700:44: sparse: incorrect type in argument 2 
> (different base types) @@expected unsigned long [unsigned] [usertype] 
> size @@got d long [unsigned] [usertype] size @@
>drivers/net/can/usb/ucan.c:700:44:expected unsigned long [unsigned] 
> [usertype] size
>drivers/net/can/usb/ucan.c:700:44:got restricted __le16 [usertype] 
> wMaxPacketSize
>drivers/net/can/usb/ucan.c:752:25: sparse: cast to restricted __le16
>drivers/net/can/usb/ucan.c:779:36: sparse: incorrect type in argument 5 
> (different base types) @@expected int [signed] buffer_length @@got 
> restricted __le1int [signed] buffer_length @@
>drivers/net/can/usb/ucan.c:779:36:expected int [signed] buffer_length
>drivers/net/can/usb/ucan.c:779:36:got restricted __le16 [usertype] 
> wMaxPacketSize
>drivers/net/can/usb/ucan.c:793:54: sparse: incorrect type in argument 2 
> (different base types) @@expected unsigned long [unsigned] [usertype] 
> size @@got d long [unsigned] [usertype] size @@
>drivers/net/can/usb/ucan.c:793:54:expected unsigned long [unsigned] 
> [usertype] size
>drivers/net/can/usb/ucan.c:793:54:got restricted __le16 [usertype] 
> wMaxPacketSize
>drivers/net/can/usb/ucan.c:853:62: sparse: incorrect type in argument 2 
> (different base types) @@expected unsigned long [unsigned] [usertype] 
> size @@got d long [unsigned] [usertype] size @@
>drivers/net/can/usb/ucan.c:853:62:expected unsigned long [unsigned] 
> [usertype] size
>drivers/net/can/usb/ucan.c:853:62:got restricted __le16 [usertype] 
> wMaxPacketSize
>drivers/net/can/usb/ucan.c:877:61: sparse: incorrect type in argument 2 
> (different base types) @@expected unsigned long [unsigned] [usertype] 
> size @@got d long [unsigned] [usertype] size @@
>drivers/net/can/usb/ucan.c:877:61:expected unsigned long [unsigned] 
> [usertype] size
>drivers/net/can/usb/ucan.c:877:61:got restricted __le16 [usertype] 
> wMaxPacketSize
>drivers/net/can/usb/ucan.c:891:44: sparse: incorrect type in argument 5 
> (different base types) @@expected int [signed] buffer_length @@got 
> restricted __le1int [signed] buffer_length @@
>drivers/net/can/usb/ucan.c:891:44:expected int [signed] buffer_length
>drivers/net/can/usb/ucan.c:891:44:got restricted __le16 [usertype] 
> wMaxPacketSize
>>> drivers/net/can/usb/ucan.c:1238:41: sparse: incorrect type in assignment 
>>> (different base types) @@expected restricted __le16 [usertype] 
>>> sample_point @@got restricted __le16 [usertype] sample_point @@
>drivers/net/can/usb/ucan.c:1238:41:expected restricted __le16 
> [usertype] sample_point
>drivers/net/can/usb/ucan.c:1238:41:got restricted __le32 [usertype] 
> 
>drivers/net/can/usb/ucan.c:1368:18: sparse: restricted __le16 degrades to 
> integer
>drivers/net/can/usb/ucan.c:1373:19: sparse: restricted __le16 degrades to 
> integer
>

Re: [PATCH v3 1/1] can: ucan: add driver for Theobroma Systems UCAN devices

2018-03-24 Thread kbuild test robot
Hi Jakob,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on v4.16-rc6]
[cannot apply to next-20180323]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Jakob-Unterwurzacher/can-ucan-add-driver-for-Theobroma-Systems-UCAN-devices/20180324-164143
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/net/can/usb/ucan.c:301:16: sparse: restricted __le32 degrades to 
>> integer
   drivers/net/can/usb/ucan.c:414:32: sparse: incorrect type in argument 5 
(different base types) @@expected unsigned short [unsigned] [usertype] 
value @@got  short [unsigned] [usertype] value @@
   drivers/net/can/usb/ucan.c:414:32:expected unsigned short [unsigned] 
[usertype] value
   drivers/net/can/usb/ucan.c:414:32:got restricted __le16 [usertype] 

   drivers/net/can/usb/ucan.c:428:32: sparse: incorrect type in argument 5 
(different base types) @@expected unsigned short [unsigned] [usertype] 
value @@got  short [unsigned] [usertype] value @@
   drivers/net/can/usb/ucan.c:428:32:expected unsigned short [unsigned] 
[usertype] value
   drivers/net/can/usb/ucan.c:428:32:got restricted __le16 [usertype] 

   drivers/net/can/usb/ucan.c:683:44: sparse: incorrect type in argument 2 
(different base types) @@expected unsigned long [unsigned] [usertype] size 
@@got d long [unsigned] [usertype] size @@
   drivers/net/can/usb/ucan.c:683:44:expected unsigned long [unsigned] 
[usertype] size
   drivers/net/can/usb/ucan.c:683:44:got restricted __le16 [usertype] 
wMaxPacketSize
   drivers/net/can/usb/ucan.c:700:44: sparse: incorrect type in argument 2 
(different base types) @@expected unsigned long [unsigned] [usertype] size 
@@got d long [unsigned] [usertype] size @@
   drivers/net/can/usb/ucan.c:700:44:expected unsigned long [unsigned] 
[usertype] size
   drivers/net/can/usb/ucan.c:700:44:got restricted __le16 [usertype] 
wMaxPacketSize
   drivers/net/can/usb/ucan.c:752:25: sparse: cast to restricted __le16
   drivers/net/can/usb/ucan.c:779:36: sparse: incorrect type in argument 5 
(different base types) @@expected int [signed] buffer_length @@got 
restricted __le1int [signed] buffer_length @@
   drivers/net/can/usb/ucan.c:779:36:expected int [signed] buffer_length
   drivers/net/can/usb/ucan.c:779:36:got restricted __le16 [usertype] 
wMaxPacketSize
   drivers/net/can/usb/ucan.c:793:54: sparse: incorrect type in argument 2 
(different base types) @@expected unsigned long [unsigned] [usertype] size 
@@got d long [unsigned] [usertype] size @@
   drivers/net/can/usb/ucan.c:793:54:expected unsigned long [unsigned] 
[usertype] size
   drivers/net/can/usb/ucan.c:793:54:got restricted __le16 [usertype] 
wMaxPacketSize
   drivers/net/can/usb/ucan.c:853:62: sparse: incorrect type in argument 2 
(different base types) @@expected unsigned long [unsigned] [usertype] size 
@@got d long [unsigned] [usertype] size @@
   drivers/net/can/usb/ucan.c:853:62:expected unsigned long [unsigned] 
[usertype] size
   drivers/net/can/usb/ucan.c:853:62:got restricted __le16 [usertype] 
wMaxPacketSize
   drivers/net/can/usb/ucan.c:877:61: sparse: incorrect type in argument 2 
(different base types) @@expected unsigned long [unsigned] [usertype] size 
@@got d long [unsigned] [usertype] size @@
   drivers/net/can/usb/ucan.c:877:61:expected unsigned long [unsigned] 
[usertype] size
   drivers/net/can/usb/ucan.c:877:61:got restricted __le16 [usertype] 
wMaxPacketSize
   drivers/net/can/usb/ucan.c:891:44: sparse: incorrect type in argument 5 
(different base types) @@expected int [signed] buffer_length @@got 
restricted __le1int [signed] buffer_length @@
   drivers/net/can/usb/ucan.c:891:44:expected int [signed] buffer_length
   drivers/net/can/usb/ucan.c:891:44:got restricted __le16 [usertype] 
wMaxPacketSize
>> drivers/net/can/usb/ucan.c:1238:41: sparse: incorrect type in assignment 
>> (different base types) @@expected restricted __le16 [usertype] 
>> sample_point @@got restricted __le16 [usertype] sample_point @@
   drivers/net/can/usb/ucan.c:1238:41:expected restricted __le16 [usertype] 
sample_point
   drivers/net/can/usb/ucan.c:1238:41:got restricted __le32 [usertype] 

   drivers/net/can/usb/ucan.c:1368:18: sparse: restricted __le16 degrades to 
integer
   drivers/net/can/usb/ucan.c:1373:19: sparse: restricted __le16 degrades to 
integer
   drivers/net/can/usb/ucan.c:1407:31: sparse: incorrect type in argument 5 
(different base types) @@expected unsigned short [unsigned] [usertype] 
value @@got  short [unsigned] [usertype] value @@
   drivers/net/can/usb/ucan.c:1407:31:expected unsigned short 

Re: [PATCH v3 1/1] can: ucan: add driver for Theobroma Systems UCAN devices

2018-03-24 Thread kbuild test robot
Hi Jakob,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on v4.16-rc6]
[cannot apply to next-20180323]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Jakob-Unterwurzacher/can-ucan-add-driver-for-Theobroma-Systems-UCAN-devices/20180324-164143
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/net/can/usb/ucan.c:301:16: sparse: restricted __le32 degrades to 
>> integer
   drivers/net/can/usb/ucan.c:414:32: sparse: incorrect type in argument 5 
(different base types) @@expected unsigned short [unsigned] [usertype] 
value @@got  short [unsigned] [usertype] value @@
   drivers/net/can/usb/ucan.c:414:32:expected unsigned short [unsigned] 
[usertype] value
   drivers/net/can/usb/ucan.c:414:32:got restricted __le16 [usertype] 

   drivers/net/can/usb/ucan.c:428:32: sparse: incorrect type in argument 5 
(different base types) @@expected unsigned short [unsigned] [usertype] 
value @@got  short [unsigned] [usertype] value @@
   drivers/net/can/usb/ucan.c:428:32:expected unsigned short [unsigned] 
[usertype] value
   drivers/net/can/usb/ucan.c:428:32:got restricted __le16 [usertype] 

   drivers/net/can/usb/ucan.c:683:44: sparse: incorrect type in argument 2 
(different base types) @@expected unsigned long [unsigned] [usertype] size 
@@got d long [unsigned] [usertype] size @@
   drivers/net/can/usb/ucan.c:683:44:expected unsigned long [unsigned] 
[usertype] size
   drivers/net/can/usb/ucan.c:683:44:got restricted __le16 [usertype] 
wMaxPacketSize
   drivers/net/can/usb/ucan.c:700:44: sparse: incorrect type in argument 2 
(different base types) @@expected unsigned long [unsigned] [usertype] size 
@@got d long [unsigned] [usertype] size @@
   drivers/net/can/usb/ucan.c:700:44:expected unsigned long [unsigned] 
[usertype] size
   drivers/net/can/usb/ucan.c:700:44:got restricted __le16 [usertype] 
wMaxPacketSize
   drivers/net/can/usb/ucan.c:752:25: sparse: cast to restricted __le16
   drivers/net/can/usb/ucan.c:779:36: sparse: incorrect type in argument 5 
(different base types) @@expected int [signed] buffer_length @@got 
restricted __le1int [signed] buffer_length @@
   drivers/net/can/usb/ucan.c:779:36:expected int [signed] buffer_length
   drivers/net/can/usb/ucan.c:779:36:got restricted __le16 [usertype] 
wMaxPacketSize
   drivers/net/can/usb/ucan.c:793:54: sparse: incorrect type in argument 2 
(different base types) @@expected unsigned long [unsigned] [usertype] size 
@@got d long [unsigned] [usertype] size @@
   drivers/net/can/usb/ucan.c:793:54:expected unsigned long [unsigned] 
[usertype] size
   drivers/net/can/usb/ucan.c:793:54:got restricted __le16 [usertype] 
wMaxPacketSize
   drivers/net/can/usb/ucan.c:853:62: sparse: incorrect type in argument 2 
(different base types) @@expected unsigned long [unsigned] [usertype] size 
@@got d long [unsigned] [usertype] size @@
   drivers/net/can/usb/ucan.c:853:62:expected unsigned long [unsigned] 
[usertype] size
   drivers/net/can/usb/ucan.c:853:62:got restricted __le16 [usertype] 
wMaxPacketSize
   drivers/net/can/usb/ucan.c:877:61: sparse: incorrect type in argument 2 
(different base types) @@expected unsigned long [unsigned] [usertype] size 
@@got d long [unsigned] [usertype] size @@
   drivers/net/can/usb/ucan.c:877:61:expected unsigned long [unsigned] 
[usertype] size
   drivers/net/can/usb/ucan.c:877:61:got restricted __le16 [usertype] 
wMaxPacketSize
   drivers/net/can/usb/ucan.c:891:44: sparse: incorrect type in argument 5 
(different base types) @@expected int [signed] buffer_length @@got 
restricted __le1int [signed] buffer_length @@
   drivers/net/can/usb/ucan.c:891:44:expected int [signed] buffer_length
   drivers/net/can/usb/ucan.c:891:44:got restricted __le16 [usertype] 
wMaxPacketSize
>> drivers/net/can/usb/ucan.c:1238:41: sparse: incorrect type in assignment 
>> (different base types) @@expected restricted __le16 [usertype] 
>> sample_point @@got restricted __le16 [usertype] sample_point @@
   drivers/net/can/usb/ucan.c:1238:41:expected restricted __le16 [usertype] 
sample_point
   drivers/net/can/usb/ucan.c:1238:41:got restricted __le32 [usertype] 

   drivers/net/can/usb/ucan.c:1368:18: sparse: restricted __le16 degrades to 
integer
   drivers/net/can/usb/ucan.c:1373:19: sparse: restricted __le16 degrades to 
integer
   drivers/net/can/usb/ucan.c:1407:31: sparse: incorrect type in argument 5 
(different base types) @@expected unsigned short [unsigned] [usertype] 
value @@got  short [unsigned] [usertype] value @@
   drivers/net/can/usb/ucan.c:1407:31:expected unsigned short 

Re: [PATCH v3 1/1] can: ucan: add driver for Theobroma Systems UCAN devices

2018-03-23 Thread Wolfgang Grandegger

Am 22.03.2018 um 14:53 schrieb Jakob Unterwurzacher:
> The UCAN driver supports the microcontroller-based USB/CAN
> adapters from Theobroma Systems. There are two form-factors
> that run essentially the same firmware:
> 
> * Seal: standalone USB stick ( https://www.theobroma-systems.com/seal )
> 
> * Mule: integrated on the PCB of various System-on-Modules from
>   Theobroma Systems like the A31-µQ7 and the RK3399-Q7
>   ( https://www.theobroma-systems.com/rk3399-q7 )
> 
> The USB wire protocol has been designed to be as generic and
> hardware-indendent as possible in the hope of being useful for
> implementation on other microcontrollers.
> 
> Signed-off-by: Martin Elshuber 
> Signed-off-by: Jakob Unterwurzacher 
> 
> Signed-off-by: Philipp Tomsich 
> ---
>  Documentation/networking/can_ucan_protocol.rst |  315 +
>  Documentation/networking/index.rst |1 +
>  drivers/net/can/usb/Kconfig|   10 +
>  drivers/net/can/usb/Makefile   |1 +
>  drivers/net/can/usb/ucan.c | 1628 
> 
>  5 files changed, 1955 insertions(+)
>  create mode 100644 Documentation/networking/can_ucan_protocol.rst
>  create mode 100644 drivers/net/can/usb/ucan.c

> 

[...snip...]

> diff --git a/drivers/net/can/usb/ucan.c b/drivers/net/can/usb/ucan.c
> new file mode 100644
> index ..5a98ef5d95da
> --- /dev/null
> +++ b/drivers/net/can/usb/ucan.c

[...snip...]

> +static void ucan_rx_can_msg(struct ucan_priv *up, struct ucan_message_in *m)
> +{
> + int len;
> + canid_t canid, canid_mask;
> + struct can_frame *cf;
> + struct sk_buff *skb;
> + struct net_device_stats *stats = >netdev->stats;
> +
> + /* get the contents of the length field */
> + len = le16_to_cpu(m->len);
> +
> + /* check sanity */
> + if (len < UCAN_IN_HDR_SIZE + sizeof(m->msg.can_msg.id)) {
> + dev_warn(>udev->dev,
> +  "%s: invalid input message len: %d\n", __func__, len);
> + return;
> + }
> +
> + /* handle error frames */
> + canid = le32_to_cpu(m->msg.can_msg.id);
> + if (canid & CAN_ERR_FLAG) {
> + bool busstate_changed = ucan_handle_error_frame(up, m, canid);
> + /* if berr-reporting is off only state changes get through */
> + if (!(up->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) &&
> + !busstate_changed)
> + return;
> + }
> +
> + /* allocate skb */
> + skb = alloc_can_skb(up->netdev, );
> + if (!skb)
> + return;
> +
> + /* compute the mask for canid */
> + canid_mask = CAN_RTR_FLAG | CAN_ERR_FLAG;
> + if (canid & CAN_EFF_FLAG)
> + canid_mask |= CAN_EFF_MASK | CAN_EFF_FLAG;
> + else
> + canid_mask |= CAN_SFF_MASK;

You do not want that for error frames... or have I missed something?

> + /* fill the can frame */
> + cf->can_id = canid & canid_mask;
> +
> + /* compute DLC taking RTR_FLAG into account */
> + cf->can_dlc = ucan_get_can_dlc(>msg.can_msg, len);
> +
> + /* copy the payload of non RTR frames */
> + if (!(cf->can_id & CAN_RTR_FLAG))
> + memcpy(cf->data, m->msg.can_msg.data, cf->can_dlc);

Ditto.

> + /* don't count error frames as real packets */
> + stats->rx_packets++;
> + stats->rx_bytes += cf->can_dlc;
> +
> + /* pass it to Linux */
> + netif_rx(skb);
> +}
> +
> +/* callback indicating completed transmission */
> +static void ucan_tx_complete_msg(struct ucan_priv *up,
> +  struct ucan_message_in *m)
> +{
> + u16 count, i;
> + u8 echo_index, dlc;
> + u16 len = le16_to_cpu(m->len);
> +
> + struct ucan_urb_context *context;
> +
> + if (len < UCAN_IN_HDR_SIZE || (len % 2 != 0)) {
> + dev_err(>udev->dev,
> + "%s: invalid tx complete length\n",
> + __func__);
> + return;
> + }
> +
> + count = (len - UCAN_IN_HDR_SIZE) / 2;
> + for (i = 0; i < count; i++) {
> + /* we did not submit such echo ids */
> + echo_index = m->msg.can_tx_complete_msg[i].echo_index;
> + if (echo_index >= up->device_info.tx_fifo) {
> + up->netdev->stats.tx_errors++;
> + dev_err(>udev->dev,
> + "%s: device answered with invalid echo_index\n",
> + __func__);
> + continue;
> + }
> +
> + /* gather information from the context */
> + context = >context_array[echo_index];
> + dlc = READ_ONCE(context->dlc);
> +
> + /* Release context and restart queue if necessary.
> +  * Also check if the context was allocated
> +  */
> +  

Re: [PATCH v3 1/1] can: ucan: add driver for Theobroma Systems UCAN devices

2018-03-23 Thread Wolfgang Grandegger

Am 22.03.2018 um 14:53 schrieb Jakob Unterwurzacher:
> The UCAN driver supports the microcontroller-based USB/CAN
> adapters from Theobroma Systems. There are two form-factors
> that run essentially the same firmware:
> 
> * Seal: standalone USB stick ( https://www.theobroma-systems.com/seal )
> 
> * Mule: integrated on the PCB of various System-on-Modules from
>   Theobroma Systems like the A31-µQ7 and the RK3399-Q7
>   ( https://www.theobroma-systems.com/rk3399-q7 )
> 
> The USB wire protocol has been designed to be as generic and
> hardware-indendent as possible in the hope of being useful for
> implementation on other microcontrollers.
> 
> Signed-off-by: Martin Elshuber 
> Signed-off-by: Jakob Unterwurzacher 
> 
> Signed-off-by: Philipp Tomsich 
> ---
>  Documentation/networking/can_ucan_protocol.rst |  315 +
>  Documentation/networking/index.rst |1 +
>  drivers/net/can/usb/Kconfig|   10 +
>  drivers/net/can/usb/Makefile   |1 +
>  drivers/net/can/usb/ucan.c | 1628 
> 
>  5 files changed, 1955 insertions(+)
>  create mode 100644 Documentation/networking/can_ucan_protocol.rst
>  create mode 100644 drivers/net/can/usb/ucan.c

> 

[...snip...]

> diff --git a/drivers/net/can/usb/ucan.c b/drivers/net/can/usb/ucan.c
> new file mode 100644
> index ..5a98ef5d95da
> --- /dev/null
> +++ b/drivers/net/can/usb/ucan.c

[...snip...]

> +static void ucan_rx_can_msg(struct ucan_priv *up, struct ucan_message_in *m)
> +{
> + int len;
> + canid_t canid, canid_mask;
> + struct can_frame *cf;
> + struct sk_buff *skb;
> + struct net_device_stats *stats = >netdev->stats;
> +
> + /* get the contents of the length field */
> + len = le16_to_cpu(m->len);
> +
> + /* check sanity */
> + if (len < UCAN_IN_HDR_SIZE + sizeof(m->msg.can_msg.id)) {
> + dev_warn(>udev->dev,
> +  "%s: invalid input message len: %d\n", __func__, len);
> + return;
> + }
> +
> + /* handle error frames */
> + canid = le32_to_cpu(m->msg.can_msg.id);
> + if (canid & CAN_ERR_FLAG) {
> + bool busstate_changed = ucan_handle_error_frame(up, m, canid);
> + /* if berr-reporting is off only state changes get through */
> + if (!(up->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) &&
> + !busstate_changed)
> + return;
> + }
> +
> + /* allocate skb */
> + skb = alloc_can_skb(up->netdev, );
> + if (!skb)
> + return;
> +
> + /* compute the mask for canid */
> + canid_mask = CAN_RTR_FLAG | CAN_ERR_FLAG;
> + if (canid & CAN_EFF_FLAG)
> + canid_mask |= CAN_EFF_MASK | CAN_EFF_FLAG;
> + else
> + canid_mask |= CAN_SFF_MASK;

You do not want that for error frames... or have I missed something?

> + /* fill the can frame */
> + cf->can_id = canid & canid_mask;
> +
> + /* compute DLC taking RTR_FLAG into account */
> + cf->can_dlc = ucan_get_can_dlc(>msg.can_msg, len);
> +
> + /* copy the payload of non RTR frames */
> + if (!(cf->can_id & CAN_RTR_FLAG))
> + memcpy(cf->data, m->msg.can_msg.data, cf->can_dlc);

Ditto.

> + /* don't count error frames as real packets */
> + stats->rx_packets++;
> + stats->rx_bytes += cf->can_dlc;
> +
> + /* pass it to Linux */
> + netif_rx(skb);
> +}
> +
> +/* callback indicating completed transmission */
> +static void ucan_tx_complete_msg(struct ucan_priv *up,
> +  struct ucan_message_in *m)
> +{
> + u16 count, i;
> + u8 echo_index, dlc;
> + u16 len = le16_to_cpu(m->len);
> +
> + struct ucan_urb_context *context;
> +
> + if (len < UCAN_IN_HDR_SIZE || (len % 2 != 0)) {
> + dev_err(>udev->dev,
> + "%s: invalid tx complete length\n",
> + __func__);
> + return;
> + }
> +
> + count = (len - UCAN_IN_HDR_SIZE) / 2;
> + for (i = 0; i < count; i++) {
> + /* we did not submit such echo ids */
> + echo_index = m->msg.can_tx_complete_msg[i].echo_index;
> + if (echo_index >= up->device_info.tx_fifo) {
> + up->netdev->stats.tx_errors++;
> + dev_err(>udev->dev,
> + "%s: device answered with invalid echo_index\n",
> + __func__);
> + continue;
> + }
> +
> + /* gather information from the context */
> + context = >context_array[echo_index];
> + dlc = READ_ONCE(context->dlc);
> +
> + /* Release context and restart queue if necessary.
> +  * Also check if the context was allocated
> +  */
> + if (ucan_release_context(up, context))
> + continue;
> +
> + if 

[PATCH v3 1/1] can: ucan: add driver for Theobroma Systems UCAN devices

2018-03-22 Thread Jakob Unterwurzacher
The UCAN driver supports the microcontroller-based USB/CAN
adapters from Theobroma Systems. There are two form-factors
that run essentially the same firmware:

* Seal: standalone USB stick ( https://www.theobroma-systems.com/seal )

* Mule: integrated on the PCB of various System-on-Modules from
  Theobroma Systems like the A31-µQ7 and the RK3399-Q7
  ( https://www.theobroma-systems.com/rk3399-q7 )

The USB wire protocol has been designed to be as generic and
hardware-indendent as possible in the hope of being useful for
implementation on other microcontrollers.

Signed-off-by: Martin Elshuber 
Signed-off-by: Jakob Unterwurzacher 
Signed-off-by: Philipp Tomsich 
---
 Documentation/networking/can_ucan_protocol.rst |  315 +
 Documentation/networking/index.rst |1 +
 drivers/net/can/usb/Kconfig|   10 +
 drivers/net/can/usb/Makefile   |1 +
 drivers/net/can/usb/ucan.c | 1628 
 5 files changed, 1955 insertions(+)
 create mode 100644 Documentation/networking/can_ucan_protocol.rst
 create mode 100644 drivers/net/can/usb/ucan.c

diff --git a/Documentation/networking/can_ucan_protocol.rst 
b/Documentation/networking/can_ucan_protocol.rst
new file mode 100644
index ..d859b36200b4
--- /dev/null
+++ b/Documentation/networking/can_ucan_protocol.rst
@@ -0,0 +1,315 @@
+=
+The UCAN Protocol
+=
+
+UCAN is the protocol used by the microcontroller-based USB-CAN
+adapter that is integrated on System-on-Modules from Theobroma Systems
+and that is also available as a standalone USB stick.
+
+The UCAN protocol has been designed to be hardware-independent.
+It is modeled closely after how Linux represents CAN devices
+internally. All multi-byte integers are encoded as Little Endian.
+
+All structures mentioned in this document are defined in
+``drivers/net/can/usb/ucan.c``.
+
+USB Endpoints
+=
+
+UCAN devices use three USB endpoints:
+
+CONTROL endpoint
+  The driver sends device management commands on this endpoint
+
+IN endpoint
+  The device sends CAN data frames and CAN error frames
+
+OUT endpoint
+  The driver sends CAN data frames on the out endpoint
+
+
+CONTROL Messages
+
+
+UCAN devices are configured using vendor requests on the control pipe.
+
+To support multiple CAN interfaces in a single USB device all
+configuration commands target the corresponding interface in the USB
+descriptor.
+
+The driver uses ``ucan_ctrl_command_in/out`` and
+``ucan_device_request_in`` to deliver commands to the device.
+
+Setup Packet
+
+
+=  =
+``bmRequestType``  Direction | Vendor | (Interface or Device)
+``bRequest``   Command Number
+``wValue`` Subcommand Number (16 Bit) or 0 if not used
+``wIndex`` USB Interface Index (0 for device commands)
+``wLength``* Host to Device - Number of bytes to transmit
+   * Device to Host - Maximum Number of bytes to
+ receive. If the device send less. Commom ZLP
+ semantics are used.
+=  =
+
+Error Handling
+--
+
+The device indicates failed control commands by stalling the
+pipe.
+
+Device Commands
+---
+
+UCAN_DEVICE_GET_FW_STRING
+~
+
+*Dev2Host; optional*
+
+Request the device firmware string.
+
+
+Interface Commands
+--
+
+UCAN_COMMAND_START
+~~
+
+*Host2Dev; mandatory*
+
+Bring the CAN interface up.
+
+Payload Format
+  ``ucan_ctl_payload_t.cmd_start``
+
+  
+mode  or mask of ``UCAN_MODE_*``
+  
+
+UCAN_COMMAND_STOP
+~~
+
+*Host2Dev; mandatory*
+
+Stop the CAN interface
+
+Payload Format
+  *empty*
+
+UCAN_COMMAND_RESET
+~~
+
+*Host2Dev; mandatory*
+
+Reset the CAN controller (including error counters)
+
+Payload Format
+  *empty*
+
+UCAN_COMMAND_GET
+
+
+*Host2Dev; mandatory*
+
+Get Information from the Device
+
+Subcommands
+^^^
+
+UCAN_COMMAND_GET_INFO
+  Request the device information structure ``ucan_ctl_payload_t.device_info``.
+
+  See the ``device_info`` field for details, and
+  ``uapi/linux/can/netlink.h`` for an explanation of the
+  ``can_bittiming fields``.
+
+  Payload Format
+``ucan_ctl_payload_t.device_info``
+
+UCAN_COMMAND_GET_PROTOCOL_VERSION
+
+  Request the device protocol version
+  ``ucan_ctl_payload_t.protocol_version``. The current protocol version is 3.
+
+  Payload Format
+``ucan_ctl_payload_t.protocol_version``
+
+.. note:: Devices that do not implement this command use the old
+  protocol version 1
+
+UCAN_COMMAND_SET_BITTIMING

[PATCH v3 1/1] can: ucan: add driver for Theobroma Systems UCAN devices

2018-03-22 Thread Jakob Unterwurzacher
The UCAN driver supports the microcontroller-based USB/CAN
adapters from Theobroma Systems. There are two form-factors
that run essentially the same firmware:

* Seal: standalone USB stick ( https://www.theobroma-systems.com/seal )

* Mule: integrated on the PCB of various System-on-Modules from
  Theobroma Systems like the A31-µQ7 and the RK3399-Q7
  ( https://www.theobroma-systems.com/rk3399-q7 )

The USB wire protocol has been designed to be as generic and
hardware-indendent as possible in the hope of being useful for
implementation on other microcontrollers.

Signed-off-by: Martin Elshuber 
Signed-off-by: Jakob Unterwurzacher 
Signed-off-by: Philipp Tomsich 
---
 Documentation/networking/can_ucan_protocol.rst |  315 +
 Documentation/networking/index.rst |1 +
 drivers/net/can/usb/Kconfig|   10 +
 drivers/net/can/usb/Makefile   |1 +
 drivers/net/can/usb/ucan.c | 1628 
 5 files changed, 1955 insertions(+)
 create mode 100644 Documentation/networking/can_ucan_protocol.rst
 create mode 100644 drivers/net/can/usb/ucan.c

diff --git a/Documentation/networking/can_ucan_protocol.rst 
b/Documentation/networking/can_ucan_protocol.rst
new file mode 100644
index ..d859b36200b4
--- /dev/null
+++ b/Documentation/networking/can_ucan_protocol.rst
@@ -0,0 +1,315 @@
+=
+The UCAN Protocol
+=
+
+UCAN is the protocol used by the microcontroller-based USB-CAN
+adapter that is integrated on System-on-Modules from Theobroma Systems
+and that is also available as a standalone USB stick.
+
+The UCAN protocol has been designed to be hardware-independent.
+It is modeled closely after how Linux represents CAN devices
+internally. All multi-byte integers are encoded as Little Endian.
+
+All structures mentioned in this document are defined in
+``drivers/net/can/usb/ucan.c``.
+
+USB Endpoints
+=
+
+UCAN devices use three USB endpoints:
+
+CONTROL endpoint
+  The driver sends device management commands on this endpoint
+
+IN endpoint
+  The device sends CAN data frames and CAN error frames
+
+OUT endpoint
+  The driver sends CAN data frames on the out endpoint
+
+
+CONTROL Messages
+
+
+UCAN devices are configured using vendor requests on the control pipe.
+
+To support multiple CAN interfaces in a single USB device all
+configuration commands target the corresponding interface in the USB
+descriptor.
+
+The driver uses ``ucan_ctrl_command_in/out`` and
+``ucan_device_request_in`` to deliver commands to the device.
+
+Setup Packet
+
+
+=  =
+``bmRequestType``  Direction | Vendor | (Interface or Device)
+``bRequest``   Command Number
+``wValue`` Subcommand Number (16 Bit) or 0 if not used
+``wIndex`` USB Interface Index (0 for device commands)
+``wLength``* Host to Device - Number of bytes to transmit
+   * Device to Host - Maximum Number of bytes to
+ receive. If the device send less. Commom ZLP
+ semantics are used.
+=  =
+
+Error Handling
+--
+
+The device indicates failed control commands by stalling the
+pipe.
+
+Device Commands
+---
+
+UCAN_DEVICE_GET_FW_STRING
+~
+
+*Dev2Host; optional*
+
+Request the device firmware string.
+
+
+Interface Commands
+--
+
+UCAN_COMMAND_START
+~~
+
+*Host2Dev; mandatory*
+
+Bring the CAN interface up.
+
+Payload Format
+  ``ucan_ctl_payload_t.cmd_start``
+
+  
+mode  or mask of ``UCAN_MODE_*``
+  
+
+UCAN_COMMAND_STOP
+~~
+
+*Host2Dev; mandatory*
+
+Stop the CAN interface
+
+Payload Format
+  *empty*
+
+UCAN_COMMAND_RESET
+~~
+
+*Host2Dev; mandatory*
+
+Reset the CAN controller (including error counters)
+
+Payload Format
+  *empty*
+
+UCAN_COMMAND_GET
+
+
+*Host2Dev; mandatory*
+
+Get Information from the Device
+
+Subcommands
+^^^
+
+UCAN_COMMAND_GET_INFO
+  Request the device information structure ``ucan_ctl_payload_t.device_info``.
+
+  See the ``device_info`` field for details, and
+  ``uapi/linux/can/netlink.h`` for an explanation of the
+  ``can_bittiming fields``.
+
+  Payload Format
+``ucan_ctl_payload_t.device_info``
+
+UCAN_COMMAND_GET_PROTOCOL_VERSION
+
+  Request the device protocol version
+  ``ucan_ctl_payload_t.protocol_version``. The current protocol version is 3.
+
+  Payload Format
+``ucan_ctl_payload_t.protocol_version``
+
+.. note:: Devices that do not implement this command use the old
+  protocol version 1
+
+UCAN_COMMAND_SET_BITTIMING
+~~
+
+*Host2Dev; mandatory*
+
+Setup bittiming by sending the the structure