Re: [PATCH] can: janz-ican3: error handling for CAL/CANopen firmware

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1.1: Type: text/plain, Size: 2900 bytes --]

On 05/08/2016 07:18 PM, Marc Kleine-Budde wrote:
> On 05/06/2016 10:04 AM, Andreas Gröger wrote:
>> Hi,
>>
>> my patch of May 2015 was missing the changed handling of error
>> indications. With CAL/CANopen firmware the NMTS-SlaveEventIndication
>> must be used instead of CAN-EventIndication. An appropriate slave node
>> must be configured to report the errors.
>>
>> In our department (about 15 development systems with Janz ICAN3-
>> modules with firmware 1.48, my system also with firmware ICANOS 1.35)
>> we use the driver with this patch for about one year: no known problems.
>>
>> Signed-off-by: Andreas Gröger <andreas24groeger@gmail.com>
> 
> [...]
> 
>> +/* Handle NMTS Slave Event Indication Messages from the firmware */
>> +static void ican3_handle_nmtsind(struct ican3_dev *mod, struct ican3_msg *msg)
>> +{
>> +	u16 subspec;
>> +
>> +	subspec = msg->data[0] + msg->data[1] * 0x100;
>> +	if (subspec == NMTS_SLAVE_EVENT_IND) {
>> +		switch (msg->data[2]) {
>> +		case NE_LOCAL_OCCURRED:
>> +		case NE_LOCAL_RESOLVED:
>> +			/* now follows the same message as Raw ICANOS CEVTIND
>> +			 * shift the data at the same place and call this method
>> +			 */
>> +			msg->len -= 3;
> 
>> drivers/net/can/janz-ican3.c:1214:34: warning: bad assignment (-=) to restricted __le16
>> drivers/net/can/janz-ican3.c:1215:62: warning: incorrect type in argument 3 (different base types)
>> drivers/net/can/janz-ican3.c:1215:62:    expected unsigned int [unsigned] [usertype] <noident>
>> drivers/net/can/janz-ican3.c:1215:62:    got restricted __le16 [usertype] len
> 
> msg->len is marked as __le16, so you're not allowed to work on it directly.

I've added some endianness handling:

> diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c
> index a75e66e6a5bf..f13bb8d9bb84 100644
> --- a/drivers/net/can/janz-ican3.c
> +++ b/drivers/net/can/janz-ican3.c
> @@ -1211,8 +1211,8 @@ static void ican3_handle_nmtsind(struct ican3_dev *mod, struct ican3_msg *msg)
>                         /* now follows the same message as Raw ICANOS CEVTIND
>                          * shift the data at the same place and call this method
>                          */
> -                       msg->len -= 3;
> -                       memmove(msg->data, msg->data + 3, msg->len);
> +                       le16_add_cpu(&msg->len, -3);
> +                       memmove(msg->data, msg->data + 3, le16_to_cpu(msg->len));
>                         ican3_handle_cevtind(mod, msg);
>                         break;
>                 case NE_REMOTE_OCCURRED:

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]