[RFC] can: Introducing CANFD for af_can & can-raw

[RFC] can: Introducing CANFD for af_can & can-raw

[Kurt Van Dijck]

Hi,

I thought a bit about the CANFD ideas from last ICC.

The patch below is a working prototype that extends
the frame format to 64 bytes. I explicitely did not
recompile userspace programs to see if current programs
still operate. candump & cansend do still work.
I did not try any other.

This is intended as a proof of concept that it is
possible to extend the ABI towards bigger CAN frames.

Next step could be to operate a vcan bus with bigger CAN frames.

Kurt
---
can: Introducing CANFD for af_can & can-raw

struct can_frame is expanded to contain 64byte.
This patch extends the ABI, while staying compatible
with the existing one.
This patch does not introduce CANFD for chip drivers.

Signed-off-by: Kurt Van Dijck <kurt.van.dijck@eia.be>

diff --git a/include/linux/can.h b/include/linux/can.h
index f4cd17c..ea73a89 100644
--- a/include/linux/can.h
+++ b/include/linux/can.h
@@ -55,7 +55,7 @@ typedef __u32 can_err_mask_t;
 struct can_frame {
 	canid_t can_id;  /* 32 bit CAN_ID + EFF/RTR/ERR flags */
 	__u8    can_dlc; /* data length code: 0 .. 8 */
-	__u8    data[8] __attribute__((aligned(8)));
+	__u8    data[64] __attribute__((aligned(8)));
 };
 
 /* particular protocols of the protocol family PF_CAN */
diff --git a/include/linux/can/core.h b/include/linux/can/core.h
index 9716670..01f0923 100644
--- a/include/linux/can/core.h
+++ b/include/linux/can/core.h
@@ -89,4 +89,15 @@ extern void can_rx_unregister(struct net_device *dev, canid_t can_id,
 extern int can_send(struct sk_buff *skb, int loop);
 extern int can_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
 
+/**
+ * LEGACY CAN frame
+ * struct can20b_frame - basic CAN 2.0B frame structure
+ * differs only payload size from struct can_frame
+ */
+struct can20b_frame {
+	canid_t can_id;  /* 32 bit CAN_ID + EFF/RTR/ERR flags */
+	__u8    can_dlc; /* data length code: 0 .. 8 */
+	__u8    data[8] __attribute__((aligned(8)));
+};
+
 #endif /* CAN_CORE_H */
diff --git a/net/can/af_can.c b/net/can/af_can.c
index 8cd9e2e..1ddc8bc 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -221,13 +221,22 @@ static int can_create(struct net *net, struct socket *sock, int protocol,
  *  -EPERM when trying to send on a non-CAN interface
  *  -EINVAL when the skb->data does not contain a valid CAN frame
  */
+#define CANFD_PADSIZE	(sizeof(struct can_frame) - sizeof(struct can20b_frame))
 int can_send(struct sk_buff *skb, int loop)
 {
 	struct sk_buff *newskb = NULL;
 	struct can_frame *cf = (struct can_frame *)skb->data;
 	int err;
 
-	if (skb->len != sizeof(struct can_frame) || cf->can_dlc > 8) {
+	if (skb->len == sizeof(struct can20b_frame))
+		/*
+		 * extend the skb to the new CAN frame format
+		 * fill with zero
+		 */
+		memset(skb_put(skb, CANFD_PADSIZE), 0, CANFD_PADSIZE);
+
+	if (skb->len != sizeof(struct can_frame) ||
+			(cf->can_dlc > sizeof(cf->data))) {
 		kfree_skb(skb);
 		return -EINVAL;
 	}
diff --git a/net/can/raw.c b/net/can/raw.c
index fbdd260..3471d22 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -662,7 +662,8 @@ static int raw_sendmsg(struct kiocb *iocb, struct socket *sock,
 	} else
 		ifindex = ro->ifindex;
 
-	if (size != sizeof(struct can_frame))
+	if ((size != sizeof(struct can_frame)) &&
+			(size != sizeof(struct can20b_frame)))
 		return -EINVAL;
 
 	dev = dev_get_by_index(&init_net, ifindex);

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Kurt Van Dijck]

Hi Sebastian,

On the ICC, Bosch presentend CANFD[1].
I'll share my interpretation:
* CANFD is meant as a successor of CAN.
* it addressess data throughput in 2 ways:
  - higher data bitrate for data using a second
    set of bittimings.
  - longer message payload (up to 64 byte).
* it remains very compatible to CAN, although _NOT_
  bus-compatible (ie. you put it on a seperate bus).

On ICC already, both Oliver & I thought that integrating
this into linux-can would be interesting, especially to
reamain binary compatible.

So I share my effort on the list as RFC :-).

[1]: http://www.semiconductors.bosch.de/media/pdf/canliteratur/can_fd.pdf

Kind regards,

Kurt

On Wed, Mar 21, 2012 at 11:01:20AM +0100, dev@sebastianhaas.info wrote:
> For everyone not included in the discussion at the ICC: What is the background?
>Am 21.03.2012 10:11 schrieb Kurt Van Dijck &lt;kurt.van.dijck@eia.be&gt;: <br></span>Hi,
>> Hi,
>> 
>> I thought a bit about the CANFD ideas from last ICC.
>> 
>> The patch below is a working prototype that extends
>> the frame format to 64 bytes. I explicitely did not
>> recompile userspace programs to see if current programs
>> still operate. candump & cansend do still work.
>> I did not try any other.
>> 
>> This is intended as a proof of concept that it is
>> possible to extend the ABI towards bigger CAN frames.
>> 
>> Next step could be to operate a vcan bus with bigger CAN frames.
>> 
>> Kurt
>> ---
>> can: Introducing CANFD for af_can & can-raw
>> 
>> struct can_frame is expanded to contain 64byte.
>> This patch extends the ABI, while staying compatible
>> with the existing one.
>> This patch does not introduce CANFD for chip drivers.
>> 
>> Signed-off-by: Kurt Van Dijck <kurt.van.dijck@eia.be>
>> 
>> diff --git a/include/linux/can.h b/include/linux/can.h
>> index f4cd17c..ea73a89 100644
>> --- a/include/linux/can.h
>> +++ b/include/linux/can.h
>> @@ -55,7 +55,7 @@ typedef __u32 can_err_mask_t;
>>  struct can_frame {
>>  	canid_t can_id;  /* 32 bit CAN_ID + EFF/RTR/ERR flags */
>>  	__u8    can_dlc; /* data length code: 0 .. 8 */
>> -	__u8    data[8] __attribute__((aligned(8)));
>> +	__u8    data[64] __attribute__((aligned(8)));
>>  };
>>  
>>  /* particular protocols of the protocol family PF_CAN */
>> diff --git a/include/linux/can/core.h b/include/linux/can/core.h
>> index 9716670..01f0923 100644
>> --- a/include/linux/can/core.h
>> +++ b/include/linux/can/core.h
>> @@ -89,4 +89,15 @@ extern void can_rx_unregister(struct net_device *dev, canid_t can_id,
>>  extern int can_send(struct sk_buff *skb, int loop);
>>  extern int can_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
>>  
>> +/**
>> + * LEGACY CAN frame
>> + * struct can20b_frame - basic CAN 2.0B frame structure
>> + * differs only payload size from struct can_frame
>> + */
>> +struct can20b_frame {
>> +	canid_t can_id;  /* 32 bit CAN_ID + EFF/RTR/ERR flags */
>> +	__u8    can_dlc; /* data length code: 0 .. 8 */
>> +	__u8    data[8] __attribute__((aligned(8)));
>> +};
>> +
>>  #endif /* CAN_CORE_H */
>> diff --git a/net/can/af_can.c b/net/can/af_can.c
>> index 8cd9e2e..1ddc8bc 100644
>> --- a/net/can/af_can.c
>> +++ b/net/can/af_can.c
>> @@ -221,13 +221,22 @@ static int can_create(struct net *net, struct socket *sock, int protocol,
>>   *  -EPERM when trying to send on a non-CAN interface
>>   *  -EINVAL when the skb->data does not contain a valid CAN frame
>>   */
>> +#define CANFD_PADSIZE	(sizeof(struct can_frame) - sizeof(struct can20b_frame))
>>  int can_send(struct sk_buff *skb, int loop)
>>  {
>>  	struct sk_buff *newskb = NULL;
>>  	struct can_frame *cf = (struct can_frame *)skb->data;
>>  	int err;
>>  
>> -	if (skb->len != sizeof(struct can_frame) || cf->can_dlc > 8) {
>> +	if (skb->len == sizeof(struct can20b_frame))
>> +		/*
>> +		 * extend the skb to the new CAN frame format
>> +		 * fill with zero
>> +		 */
>> +		memset(skb_put(skb, CANFD_PADSIZE), 0, CANFD_PADSIZE);
>> +
>> +	if (skb->len != sizeof(struct can_frame) ||
>> +			(cf->can_dlc > sizeof(cf->data))) {
>>  		kfree_skb(skb);
>>  		return -EINVAL;
>>  	}
>> diff --git a/net/can/raw.c b/net/can/raw.c
>> index fbdd260..3471d22 100644
>> --- a/net/can/raw.c
>> +++ b/net/can/raw.c
>> @@ -662,7 +662,8 @@ static int raw_sendmsg(struct kiocb *iocb, struct socket *sock,
>>  	} else
>>  		ifindex = ro->ifindex;
>>  
>> -	if (size != sizeof(struct can_frame))
>> +	if ((size != sizeof(struct can_frame)) &&
>> +			(size != sizeof(struct can20b_frame)))
>>  		return -EINVAL;
>>  
>>  	dev = dev_get_by_index(&init_net, ifindex);

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1385 bytes --]

On 03/21/2012 12:05 PM, Kurt Van Dijck wrote:
> Hi Sebastian,
> 
> On the ICC, Bosch presentend CANFD[1].
> I'll share my interpretation:
> * CANFD is meant as a successor of CAN.
> * it addressess data throughput in 2 ways:
>   - higher data bitrate for data using a second
>     set of bittimings.
>   - longer message payload (up to 64 byte).
> * it remains very compatible to CAN, although _NOT_
>   bus-compatible (ie. you put it on a seperate bus).

This means:
You cannot have a Bus using one (or more) of the CAN-FD features with
non CAN-FD compatible nodes.

From my point of view this can be translated into a new property in
"can.ctrlmode_supported". A device adds "CAN_CTRLMODE_CANFD" if it
supports CAN FD mode. Then it's a global setting to put the device into
CAN FD mode.

In the TX path the stack should refuse the new 64 byte frames at all or
frames with dlc > 8 byes if the device is not in CAN FD mode. What about
the RX path? We can just use the new 64 byte CAN frames and have to
tweak the CAN_RAW if the read from the userspace is too small.

regards, 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: 262 bytes --]

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Kurt Van Dijck]

On Wed, Mar 21, 2012 at 12:43:31PM +0100, Marc Kleine-Budde wrote:
> On 03/21/2012 12:05 PM, Kurt Van Dijck wrote:
> > Hi Sebastian,
> > 
> > On the ICC, Bosch presentend CANFD[1].
> > I'll share my interpretation:
> > * CANFD is meant as a successor of CAN.
> > * it addressess data throughput in 2 ways:
> >   - higher data bitrate for data using a second
> >     set of bittimings.
> >   - longer message payload (up to 64 byte).
> > * it remains very compatible to CAN, although _NOT_
> >   bus-compatible (ie. you put it on a seperate bus).
> 
> This means:
> You cannot have a Bus using one (or more) of the CAN-FD features with
> non CAN-FD compatible nodes.
> 
> From my point of view this can be translated into a new property in
> "can.ctrlmode_supported". A device adds "CAN_CTRLMODE_CANFD" if it
> supports CAN FD mode. Then it's a global setting to put the device into
> CAN FD mode.
Yep, good idea.
> 
> In the TX path the stack should refuse the new 64 byte frames at all or
> frames with dlc > 8 byes if the device is not in CAN FD mode.
I did not yet do such think since I did not implement CANFD on device level yet.
I think can_send() is a good place to put such tricks.

> What about
> the RX path? We can just use the new 64 byte CAN frames and have to
> tweak the CAN_RAW if the read from the userspace is too small.
You may have noticed that I did not need to do anything
in the receive path in order to receive longer frames.

look at this block in raw_recvmsg:

	if (size < skb->len)
		msg->msg_flags |= MSG_TRUNC;
	else
		size = skb->len;

Even today already, you can recv. with smaller or bigger buffers than
the actual CAN frame.

I did try to let the kernel work with 64bytes, and still use
8byte frames in userspace. that all works, especially since noone
seems to test for MSG_TRUNC.

An improvement that just crosses my mind is something like:

	int real_len;

	real_len = offsetof(struct can_frame, data) +
		((struct can_frame)skb->data)->dlc;
	if (size < real_len)
		msg->msg_flags |= MSG_TRUNC;
	else
		size = real_len;

Such would modify the ABI a bit more, but prevents copying unused bytes
and avoids unnecessary MSG_TRUNC flags.

Regards,
Kurt

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 3164 bytes --]

On 03/21/2012 01:08 PM, Kurt Van Dijck wrote:
> On Wed, Mar 21, 2012 at 12:43:31PM +0100, Marc Kleine-Budde wrote:
>> On 03/21/2012 12:05 PM, Kurt Van Dijck wrote:
>>> Hi Sebastian,
>>>
>>> On the ICC, Bosch presentend CANFD[1].
>>> I'll share my interpretation:
>>> * CANFD is meant as a successor of CAN.
>>> * it addressess data throughput in 2 ways:
>>>   - higher data bitrate for data using a second
>>>     set of bittimings.
>>>   - longer message payload (up to 64 byte).
>>> * it remains very compatible to CAN, although _NOT_
>>>   bus-compatible (ie. you put it on a seperate bus).
>>
>> This means:
>> You cannot have a Bus using one (or more) of the CAN-FD features with
>> non CAN-FD compatible nodes.
>>
>> From my point of view this can be translated into a new property in
>> "can.ctrlmode_supported". A device adds "CAN_CTRLMODE_CANFD" if it
>> supports CAN FD mode. Then it's a global setting to put the device into
>> CAN FD mode.
> Yep, good idea.
>>
>> In the TX path the stack should refuse the new 64 byte frames at all or
>> frames with dlc > 8 byes if the device is not in CAN FD mode.
> I did not yet do such think since I did not implement CANFD on device level yet.
> I think can_send() is a good place to put such tricks.

Yes, for CAN_RAW, but in the driver we must extend the
"can_dropped_invalid_skb()" function, too: drop frames with dlc >8 if
device is not in CAN FD mode.

Further, there are only certain dlc values allowed for CAN FD. We must
decide if the enforce these values or simply do padding with "0" somewhere.

>> What about
>> the RX path? We can just use the new 64 byte CAN frames and have to
>> tweak the CAN_RAW if the read from the userspace is too small.
> You may have noticed that I did not need to do anything
> in the receive path in order to receive longer frames.
> 
> look at this block in raw_recvmsg:
> 
> 	if (size < skb->len)
> 		msg->msg_flags |= MSG_TRUNC;
> 	else
> 		size = skb->len;
> 
> Even today already, you can recv. with smaller or bigger buffers than
> the actual CAN frame.

...using an unmodified mainline kernel?

> I did try to let the kernel work with 64bytes, and still use
> 8byte frames in userspace. that all works, especially since noone
> seems to test for MSG_TRUNC.

Have you checked _all_ CAN userspace applications? :P
We cannot break the ABI.

> An improvement that just crosses my mind is something like:
> 
> 	int real_len;
> 
> 	real_len = offsetof(struct can_frame, data) +
> 		((struct can_frame)skb->data)->dlc;
> 	if (size < real_len)
> 		msg->msg_flags |= MSG_TRUNC;
> 	else
> 		size = real_len;
> 
> Such would modify the ABI a bit more, but prevents copying unused bytes
> and avoids unnecessary MSG_TRUNC flags.

I suggest to copy the full 8 bytes of data if dlc <= 8, for full ABI
compatibility.

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: 262 bytes --]

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Kurt Van Dijck]

On Wed, Mar 21, 2012 at 01:32:52PM +0100, Marc Kleine-Budde wrote:
> On 03/21/2012 01:08 PM, Kurt Van Dijck wrote:
> > On Wed, Mar 21, 2012 at 12:43:31PM +0100, Marc Kleine-Budde wrote:
> >> On 03/21/2012 12:05 PM, Kurt Van Dijck wrote:
> >>> Hi Sebastian,
> >>
> 
> >> What about
> >> the RX path? We can just use the new 64 byte CAN frames and have to
> >> tweak the CAN_RAW if the read from the userspace is too small.
> > You may have noticed that I did not need to do anything
> > in the receive path in order to receive longer frames.
> > 
> > look at this block in raw_recvmsg:
> > 
> > 	if (size < skb->len)
> > 		msg->msg_flags |= MSG_TRUNC;
> > 	else
> > 		size = skb->len;
> > 
> > Even today already, you can recv. with smaller or bigger buffers than
> > the actual CAN frame.
> 
> ...using an unmodified mainline kernel?

Must be. yes!
I think that's good.
> 
> > I did try to let the kernel work with 64bytes, and still use
> > 8byte frames in userspace. that all works, especially since noone
> > seems to test for MSG_TRUNC.
> 
> Have you checked _all_ CAN userspace applications? :P
> We cannot break the ABI.
I mentioned cansend & candump.
I realize there's a lot of work ahead. I just started, and tried
not to touch too much, and yet prove it can work.

I agree that we cannot break the current ABI.

> 
> > An improvement that just crosses my mind is something like:
> > 
> > 	int real_len;
> > 
> > 	real_len = offsetof(struct can_frame, data) +
> > 		((struct can_frame)skb->data)->dlc;
> > 	if (size < real_len)
> > 		msg->msg_flags |= MSG_TRUNC;
> > 	else
> > 		size = real_len;
> > 
> > Such would modify the ABI a bit more, but prevents copying unused bytes
> > and avoids unnecessary MSG_TRUNC flags.
> 
> I suggest to copy the full 8 bytes of data if dlc <= 8, for full ABI
> compatibility.

Do we then agree that it is desired not to do this beyond the first 8 bytes?
Moreover, we could depend this on the function parameter 'size'. If it
is 16bytes, do as before, else do the new trick. Did I miss something then?

Kurt

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 2289 bytes --]

On 03/21/2012 01:51 PM, Kurt Van Dijck wrote:
>>>> What about
>>>> the RX path? We can just use the new 64 byte CAN frames and have to
>>>> tweak the CAN_RAW if the read from the userspace is too small.
>>> You may have noticed that I did not need to do anything
>>> in the receive path in order to receive longer frames.
>>>
>>> look at this block in raw_recvmsg:
>>>
>>> 	if (size < skb->len)
>>> 		msg->msg_flags |= MSG_TRUNC;
>>> 	else
>>> 		size = skb->len;
>>>
>>> Even today already, you can recv. with smaller or bigger buffers than
>>> the actual CAN frame.
>>
>> ...using an unmodified mainline kernel?
> 
> Must be. yes!
> I think that's good.

Indeed. I wasn't aware of this.

>>> I did try to let the kernel work with 64bytes, and still use
>>> 8byte frames in userspace. that all works, especially since noone
>>> seems to test for MSG_TRUNC.
>>
>> Have you checked _all_ CAN userspace applications? :P
>> We cannot break the ABI.
> I mentioned cansend & candump.
> I realize there's a lot of work ahead. I just started, and tried
> not to touch too much, and yet prove it can work.
> 
> I agree that we cannot break the current ABI.
> 
>>
>>> An improvement that just crosses my mind is something like:
>>>
>>> 	int real_len;
>>>
>>> 	real_len = offsetof(struct can_frame, data) +
>>> 		((struct can_frame)skb->data)->dlc;
>>> 	if (size < real_len)
>>> 		msg->msg_flags |= MSG_TRUNC;
>>> 	else
>>> 		size = real_len;
>>>
>>> Such would modify the ABI a bit more, but prevents copying unused bytes
>>> and avoids unnecessary MSG_TRUNC flags.
>>
>> I suggest to copy the full 8 bytes of data if dlc <= 8, for full ABI
>> compatibility.
> 
> Do we then agree that it is desired not to do this beyond the first 8 bytes?
> Moreover, we could depend this on the function parameter 'size'. If it
> is 16bytes, do as before, else do the new trick. Did I miss something then?

Yes, if 16 bytes is the length of the current struct can_frame.

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: 262 bytes --]

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Oliver Hartkopp]

Hi all,

of course i also thought about some concepts to support CAN FD ;-)

But then it melted down to the question what a potential CAN FD controller
would support:

1. Will it support sending of CAN frames & CAN FD frames with one config?
2. Is a CAN FD frame with DLC=8 different to a CAN2.0 frame with DLC=8 ?
   (as we know CAN ID 0x123 (EFF) and CAN ID 0x123 (SFF) are different)
3. Will these differences be visible in the CAN registers? Is this relevant?

What i got from the iCC was that when you have a partly migrated network and
you want to run e.g. a fast firmware upload between two CAN FD capable nodes,
the other (standard CAN 2.0b) nodes have to be in listen only mode to not jam
the bus with error frames.

After the firmware upload all the CAN nodes switch back to CAN2.0b mode (which
has to be done by root netlink access).

IMO we need to introduce a new struct canfd_frame

struct canfd_frame {
        canid_t can_id;  /* 32 bit CAN_ID + EFF/RTR/ERR flags */
        __u8    can_dlc; /* data length code: 0 .. F */
        __u8    data[64] __attribute__((aligned(8)));
};

which can become the mayor internal data structure.

It should be no problem to move to canfd_frame inside the kernel but if we
come to the point that we get to the userspace ABI there should be no tricky
way to check lengths or flags or return values are are not needed to be
checked with the current ABI now.

When there's a defined socket API that makes use of struct can_frame (like
can_raw, can_bcm, can_gw - but not isotp! \o/ ) we need to

- duplicate the socket API - like CAN_RAWFD (ugly)
- switch the frame size (e.g. via sockopt)
- a third unknown idea ...

E.g. when having a CAN FD aware candump/cansend application, it can try to
switch the socket to CAN FD. If it works -> use CAN FD, if not we're probably
on an older kernel that does not support CAN FD ...

I think it's much more tricky to find a proper solution here.

Regards,
Oliver

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Alexander Stein]

Am Mittwoch, 21. März 2012, 13:32:52 schrieb Marc Kleine-Budde:
> On 03/21/2012 01:08 PM, Kurt Van Dijck wrote:
> > On Wed, Mar 21, 2012 at 12:43:31PM +0100, Marc Kleine-Budde wrote:
> >> On 03/21/2012 12:05 PM, Kurt Van Dijck wrote:
> >>> Hi Sebastian,
> >>> 
> >>> On the ICC, Bosch presentend CANFD[1].
> >>> I'll share my interpretation:
> >>> * CANFD is meant as a successor of CAN.
> >>> 
> >>> * it addressess data throughput in 2 ways:
> >>>   - higher data bitrate for data using a second
> >>>   
> >>>     set of bittimings.
> >>>   
> >>>   - longer message payload (up to 64 byte).
> >>> 
> >>> * it remains very compatible to CAN, although _NOT_
> >>> 
> >>>   bus-compatible (ie. you put it on a seperate bus).
> >> 
> >> This means:
> >> You cannot have a Bus using one (or more) of the CAN-FD features with
> >> non CAN-FD compatible nodes.
> >> 
> >> From my point of view this can be translated into a new property in
> >> "can.ctrlmode_supported". A device adds "CAN_CTRLMODE_CANFD" if it
> >> supports CAN FD mode. Then it's a global setting to put the device
> >> into
> >> CAN FD mode.
> > 
> > Yep, good idea.
> > 
> >> In the TX path the stack should refuse the new 64 byte frames at all
> >> or
> >> frames with dlc > 8 byes if the device is not in CAN FD mode.
> > 
> > I did not yet do such think since I did not implement CANFD on device
> > level yet. I think can_send() is a good place to put such tricks.
> 
> Yes, for CAN_RAW, but in the driver we must extend the
> "can_dropped_invalid_skb()" function, too: drop frames with dlc >8 if
> device is not in CAN FD mode.
> 
> Further, there are only certain dlc values allowed for CAN FD. We must
> decide if the enforce these values or simply do padding with "0" somewhere.

What I understand from the proposal is that DLCs >8 are optional. So you might 
get hardware which still supports only 8 bytes. How can/should this be 
handled?

Alexnder
-- 
Dipl.-Inf. Alexander Stein

SYS TEC electronic GmbH
August-Bebel-Str. 29
D-07973 Greiz

Tel: +49-3661-6279-0, Fax: +49-3661-6279-99
eMail:    Alexander.Stein@systec-electronic.com
Internet: http://www.systec-electronic.com

Managing Director: Dipl.-Phys. Siegmar Schmidt
Commercial registry: Amtsgericht Jena, HRB 205563

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Kurt Van Dijck]

On Wed, Mar 21, 2012 at 02:29:05PM +0100, Alexander Stein wrote:
> Am Mittwoch, 21. März 2012, 13:32:52 schrieb Marc Kleine-Budde:
> > On 03/21/2012 01:08 PM, Kurt Van Dijck wrote:
> > 
> > Further, there are only certain dlc values allowed for CAN FD. We must
> > decide if the enforce these values or simply do padding with "0" somewhere.
> 
> What I understand from the proposal is that DLCs >8 are optional. So you might 
> get hardware which still supports only 8 bytes. How can/should this be 
> handled?

I assume that if the hardware does not support a thing, things get complicated
anyway.
But there is no hardware yet :-)

Kurt

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 800 bytes --]

On 03/21/2012 02:29 PM, Alexander Stein wrote:
>> Further, there are only certain dlc values allowed for CAN FD. We must
>> decide if the enforce these values or simply do padding with "0" somewhere.
> 
> What I understand from the proposal is that DLCs >8 are optional. So you might 
> get hardware which still supports only 8 bytes. How can/should this be 
> handled?

I haven't read the pdf that Kurt linked in the original mail. Is the
usage of dlc > 8 optional or the support of dlc > 8 in hardware?

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: 262 bytes --]

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Kurt Van Dijck]

On Wed, Mar 21, 2012 at 02:21:22PM +0100, Oliver Hartkopp wrote:
> Hi all,
> 
> of course i also thought about some concepts to support CAN FD ;-)
:-)
> 
> But then it melted down to the question what a potential CAN FD controller
> would support:
> 
> 1. Will it support sending of CAN frames & CAN FD frames with one config?
At least, you'd select if it operates in CAN or CANFD mode, since the bus
cannot switch dynamically.

> 2. Is a CAN FD frame with DLC=8 different to a CAN2.0 frame with DLC=8 ?
>    (as we know CAN ID 0x123 (EFF) and CAN ID 0x123 (SFF) are different)
a CANFD frame with DLC=8 is different to a CAN2.0 frame with DLC=8 on the wire,
but they are logically equal. Since we're into software, IMO we must treat them
equal.
I think of some differences:
* no RTR allowed.
* ESI bit.

> 3. Will these differences be visible in the CAN registers? Is this relevant?
Without hardware, it's a bit early to predict. I guess it will be visible, but
not relevant since that's driver stuff.

I did not get into real drivers yet...
> 
> What i got from the iCC was that when you have a partly migrated network and
> you want to run e.g. a fast firmware upload between two CAN FD capable nodes,
> the other (standard CAN 2.0b) nodes have to be in listen only mode to not jam
> the bus with error frames.
> 
> After the firmware upload all the CAN nodes switch back to CAN2.0b mode (which
> has to be done by root netlink access).
Since CANFD is not bus compatible, the chip must go down & up. It really depends
on the chips & drivers to combine CAN2.0 & CANFD.
As Marc already suggested, CANFD could be a ctrlmode.
> 
> IMO we need to introduce a new struct canfd_frame
IMO we need not to introduce a new struct since they are logically equivalent.

I created an old can20b_frame just to compute the sizeof.
I'd go for a combined can_frame that could hold both.
> 
> It should be no problem to move to canfd_frame inside the kernel but if we
> come to the point that we get to the userspace ABI there should be no tricky
> way to check lengths or flags or return values are are not needed to be
> checked with the current ABI now.
> 
> When there's a defined socket API that makes use of struct can_frame (like
> can_raw, can_bcm, can_gw - but not isotp! \o/ ) we need to
> 
> - duplicate the socket API - like CAN_RAWFD (ugly)
> - switch the frame size (e.g. via sockopt)
> - a third unknown idea ...
When you look at my patch, you can see that only a recompile of candump with
the bigger can_frame is all what is between me and a virtual CANFD bus.
Candump does not really need to care about the CANFD mode.
I think it can do both types of busses as in

$ candump any
> 
> E.g. when having a CAN FD aware candump/cansend application, it can try to
> switch the socket to CAN FD. 
Since CANFD is a different bus, selecting a bus (like 'can0') implicetely selects
the CANFD mode. IMHO, from that point on, there's no difference for candump with
a legacy CAN2.0 bus. So why make things complicated.

> on an older kernel that does not support CAN FD ...
> 
> I think it's much more tricky to find a proper solution here.
As I mentioned before, I just use CAN_RAW with this. Actually, I just
ran a real CAN program that I wrote years ago. It still operates!

Regards,
Kurt

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Oliver Hartkopp]

On 21.03.2012 14:53, Kurt Van Dijck wrote:

> On Wed, Mar 21, 2012 at 02:21:22PM +0100, Oliver Hartkopp wrote:

>> 2. Is a CAN FD frame with DLC=8 different to a CAN2.0 frame with DLC=8 ?
>>    (as we know CAN ID 0x123 (EFF) and CAN ID 0x123 (SFF) are different)
> a CANFD frame with DLC=8 is different to a CAN2.0 frame with DLC=8 on the wire,
> but they are logically equal. Since we're into software, IMO we must treat them
> equal.


Yes - that would fit the idea to have internally only canfd_frames ...

> I think of some differences:
> * no RTR allowed.

Yes, that's important when sending CAN FD frames to be checked.

> * ESI bit.


Is something that IMO needs to go into the error message when set active.

>>
>> What i got from the iCC was that when you have a partly migrated network and
>> you want to run e.g. a fast firmware upload between two CAN FD capable nodes,
>> the other (standard CAN 2.0b) nodes have to be in listen only mode to not jam
>> the bus with error frames.
>>
>> After the firmware upload all the CAN nodes switch back to CAN2.0b mode (which
>> has to be done by root netlink access).
> Since CANFD is not bus compatible, the chip must go down & up.


Really? We don't know so far.

> It really depends
> on the chips & drivers to combine CAN2.0 & CANFD.
> As Marc already suggested, CANFD could be a ctrlmode.


At least this capability has to be very fast to be checked, as we would need
to check it every time when we pass a CAN frame to the CAN netdevice - to give
a proper feedback in can_send() when the given frame does not fit into the
destination device ...

>>
>> IMO we need to introduce a new struct canfd_frame
> IMO we need not to introduce a new struct since they are logically equivalent.
> 
> I created an old can20b_frame just to compute the sizeof.
> I'd go for a combined can_frame that could hold both.


No. You only think of CAN_RAW. There might be a way to do it with CAN_RAW but
changing an exported visible userspace structure breaks the ABI.

Expect that we can never change struct can_frame as this is written into stone.

How do you want to adapt the struct can_frames in the ABI for CAN_BCM and the
netlink API for CAN_GW?

If we want to support CAN FD the only way is to introduce a struct canfd_frame
or whatever we would like to call it ...

>>
>> It should be no problem to move to canfd_frame inside the kernel but if we
>> come to the point that we get to the userspace ABI there should be no tricky
>> way to check lengths or flags or return values are are not needed to be
>> checked with the current ABI now.
>>
>> When there's a defined socket API that makes use of struct can_frame (like
>> can_raw, can_bcm, can_gw - but not isotp! \o/ ) we need to
>>
>> - duplicate the socket API - like CAN_RAWFD (ugly)
>> - switch the frame size (e.g. via sockopt)
>> - a third unknown idea ...
> When you look at my patch, you can see that only a recompile of candump with
> the bigger can_frame is all what is between me and a virtual CANFD bus.
> Candump does not really need to care about the CANFD mode.


Yes - if you recompile it. But assume some binaries being out there where you
don't know how they are implemented ... all our apps are just *examples* how
you can use the socket API. I've seen really bad SocketCAN application code i
was asked why it doesn't work as expected the last years, believe me %-)

> I think it can do both types of busses as in
> 
> $ candump any


Yes, when candump expects canfd_frames, everything can be put into that structure.


>>
>> E.g. when having a CAN FD aware candump/cansend application, it can try to
>> switch the socket to CAN FD. 
> Since CANFD is a different bus, selecting a bus (like 'can0') implicetely selects
> the CANFD mode.


Which probably could be changed at runtime with a netlink command. So far
there's no reason why the switch of CAN <-> CAN FD should not be possible at
runtime, when the interface is up and both bitrates (slow & high) have been
configured before.

> IMHO, from that point on, there's no difference for candump with
> a legacy CAN2.0 bus. So why make things complicated.


When the API is all CAN FD capable and using always canfd_frames, nothing is
complicated indeed. But this is a different ABI then the current one, where
everyone relies on a struct can_frame being 16 bytes long.

> 
>> on an older kernel that does not support CAN FD ...
>>
>> I think it's much more tricky to find a proper solution here.
> As I mentioned before, I just use CAN_RAW with this. Actually, I just
> ran a real CAN program that I wrote years ago. It still operates!


Yes - CAN_RAW is an example where your approach works. But unfortunately it
does not cover the entire problem ...

Regards,
Oliver

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Wolfgang Grandegger]

Hi,

On 03/21/2012 02:53 PM, Kurt Van Dijck wrote:
> On Wed, Mar 21, 2012 at 02:21:22PM +0100, Oliver Hartkopp wrote:
>> Hi all,
>>
>> of course i also thought about some concepts to support CAN FD ;-)
> :-)
>>
>> But then it melted down to the question what a potential CAN FD controller
>> would support:
>>
>> 1. Will it support sending of CAN frames & CAN FD frames with one config?
> At least, you'd select if it operates in CAN or CANFD mode, since the bus
> cannot switch dynamically.
> 
>> 2. Is a CAN FD frame with DLC=8 different to a CAN2.0 frame with DLC=8 ?
>>    (as we know CAN ID 0x123 (EFF) and CAN ID 0x123 (SFF) are different)
> a CANFD frame with DLC=8 is different to a CAN2.0 frame with DLC=8 on the wire,
> but they are logically equal. Since we're into software, IMO we must treat them
> equal.

As I see it, the main difference to standard CAN is the bit-rate
switching for transferring just the data bytes using alternate
bit-timing parameters.

> I think of some differences:
> * no RTR allowed.
> * ESI bit.

Also DLC=9 means 12 bytes, DLC=10 means 16 bytes, DLC=15 means 64 bytes.
This may even change in the final spec.

>> 3. Will these differences be visible in the CAN registers? Is this relevant?
> Without hardware, it's a bit early to predict. I guess it will be visible, but
> not relevant since that's driver stuff.

As CANFD controllers also supports CAN2.0 frames, they must provide the
the relevant information somehow, similar to EFF and SFF.

> I did not get into real drivers yet...
>>
>> What i got from the iCC was that when you have a partly migrated network and
>> you want to run e.g. a fast firmware upload between two CAN FD capable nodes,
>> the other (standard CAN 2.0b) nodes have to be in listen only mode to not jam
>> the bus with error frames.

Due to the bit-rate switching, a assume.

>> After the firmware upload all the CAN nodes switch back to CAN2.0b mode (which
>> has to be done by root netlink access).
> Since CANFD is not bus compatible, the chip must go down & up. It really depends
> on the chips & drivers to combine CAN2.0 & CANFD.
> As Marc already suggested, CANFD could be a ctrlmode.

I also see this problem.

>> IMO we need to introduce a new struct canfd_frame
> IMO we need not to introduce a new struct since they are logically equivalent.
> 
> I created an old can20b_frame just to compute the sizeof.
> I'd go for a combined can_frame that could hold both.

Well, I see it as an extension of the standard CAN and therefore I was
also thinking about an extra "struct canfd_frame". The size is more than
3 times larger and we may only allocate the extra space if it's really
required. I'm also thinking about the impact on queues, etc.

>> It should be no problem to move to canfd_frame inside the kernel but if we
>> come to the point that we get to the userspace ABI there should be no tricky
>> way to check lengths or flags or return values are are not needed to be
>> checked with the current ABI now.
>>
>> When there's a defined socket API that makes use of struct can_frame (like
>> can_raw, can_bcm, can_gw - but not isotp! \o/ ) we need to
>>
>> - duplicate the socket API - like CAN_RAWFD (ugly)
>> - switch the frame size (e.g. via sockopt)
>> - a third unknown idea ...
> When you look at my patch, you can see that only a recompile of candump with
> the bigger can_frame is all what is between me and a virtual CANFD bus.
> Candump does not really need to care about the CANFD mode.
> I think it can do both types of busses as in
> 
> $ candump any

Well, candump is a simple application and it does not really care about
the payload. This is different for real legacy application, I think,
which assumes a payload of 8 bytes only.

>> E.g. when having a CAN FD aware candump/cansend application, it can try to
>> switch the socket to CAN FD. 
> Since CANFD is a different bus, selecting a bus (like 'can0') implicetely selects
> the CANFD mode. IMHO, from that point on, there's no difference for candump with
> a legacy CAN2.0 bus. So why make things complicated.

See above. Also, I think CANFD has been invented to support a fast
transfer mode for special purposes which will be selected from time to
time making the bus incompatible with legacy CAN controllers.

>> on an older kernel that does not support CAN FD ...
>>
>> I think it's much more tricky to find a proper solution here.
> As I mentioned before, I just use CAN_RAW with this. Actually, I just
> ran a real CAN program that I wrote years ago. It still operates!

Would you program still run if the payload is bigger than 8 bytes?
I agree with Oliver.

Wolfgang.

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Oliver Hartkopp]

On 21.03.2012 15:56, Wolfgang Grandegger wrote:

> Hi,
> 

> 
> As I see it, the main difference to standard CAN is the bit-rate
> switching for transferring just the data bytes using alternate
> bit-timing parameters.


Yes. You have two sets of bittiming registers one for the normal operation and
for arbitration and the second for the data &crc section in the case of
enabled FD mode.

And in the FD mode you have up to 64 byte payload.

>> I think of some differences:
>> * no RTR allowed.
>> * ESI bit.
> 
> Also DLC=9 means 12 bytes, DLC=10 means 16 bytes, DLC=15 means 64 bytes.
> This may even change in the final spec.


Yep!

> 
>>> 3. Will these differences be visible in the CAN registers? Is this relevant?
>> Without hardware, it's a bit early to predict. I guess it will be visible, but
>> not relevant since that's driver stuff.
> 
> As CANFD controllers also supports CAN2.0 frames, they must provide the
> the relevant information somehow, similar to EFF and SFF.
> 
>> I did not get into real drivers yet...
>>>
>>> What i got from the iCC was that when you have a partly migrated network and
>>> you want to run e.g. a fast firmware upload between two CAN FD capable nodes,
>>> the other (standard CAN 2.0b) nodes have to be in listen only mode to not jam
>>> the bus with error frames.
> 
> Due to the bit-rate switching, a assume.


Yes - the 'old' controllers would put error frames on the fast payload data.


> 
>>> After the firmware upload all the CAN nodes switch back to CAN2.0b mode (which
>>> has to be done by root netlink access).
>> Since CANFD is not bus compatible, the chip must go down & up. It really depends
>> on the chips & drivers to combine CAN2.0 & CANFD.
>> As Marc already suggested, CANFD could be a ctrlmode.
> 
> I also see this problem.
> 
>>> IMO we need to introduce a new struct canfd_frame
>> IMO we need not to introduce a new struct since they are logically equivalent.
>>
>> I created an old can20b_frame just to compute the sizeof.
>> I'd go for a combined can_frame that could hold both.
> 
> Well, I see it as an extension of the standard CAN and therefore I was
> also thinking about an extra "struct canfd_frame". The size is more than
> 3 times larger and we may only allocate the extra space if it's really
> required. I'm also thinking about the impact on queues, etc.


Yes. Additionally we need to have a proper distinction between can_frame and
canfd_frame when we play with recvmmsg in the future.


> 
>>> It should be no problem to move to canfd_frame inside the kernel but if we
>>> come to the point that we get to the userspace ABI there should be no tricky
>>> way to check lengths or flags or return values are are not needed to be
>>> checked with the current ABI now.
>>>
>>> When there's a defined socket API that makes use of struct can_frame (like
>>> can_raw, can_bcm, can_gw - but not isotp! \o/ ) we need to
>>>
>>> - duplicate the socket API - like CAN_RAWFD (ugly)
>>> - switch the frame size (e.g. via sockopt)
>>> - a third unknown idea ...
>> When you look at my patch, you can see that only a recompile of candump with
>> the bigger can_frame is all what is between me and a virtual CANFD bus.
>> Candump does not really need to care about the CANFD mode.
>> I think it can do both types of busses as in
>>
>> $ candump any
> 
> Well, candump is a simple application and it does not really care about
> the payload. This is different for real legacy application, I think,
> which assumes a payload of 8 bytes only.


My words :-)

> 
>>> E.g. when having a CAN FD aware candump/cansend application, it can try to
>>> switch the socket to CAN FD. 
>> Since CANFD is a different bus, selecting a bus (like 'can0') implicetely selects
>> the CANFD mode. IMHO, from that point on, there's no difference for candump with
>> a legacy CAN2.0 bus. So why make things complicated.
> 
> See above. Also, I think CANFD has been invented to support a fast
> transfer mode for special purposes which will be selected from time to
> time making the bus incompatible with legacy CAN controllers.
> 


When you have an entire CAN bus where all nodes support CAN FD this is nice
also. But the current implementation will remain the standard for years too.

>>> on an older kernel that does not support CAN FD ...
>>>
>>> I think it's much more tricky to find a proper solution here.
>> As I mentioned before, I just use CAN_RAW with this. Actually, I just
>> ran a real CAN program that I wrote years ago. It still operates!
> 
> Would you program still run if the payload is bigger than 8 bytes?
> I agree with Oliver.


puh. Time for a coffee :-)

Regards,
Oliver

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Oliver Hartkopp]

On 21.03.2012 15:49, Oliver Hartkopp wrote:

> On 21.03.2012 14:53, Kurt Van Dijck wrote:

>>> IMO we need to introduce a new struct canfd_frame
>> IMO we need not to introduce a new struct since they are logically equivalent.
>>
>> I created an old can20b_frame just to compute the sizeof.
>> I'd go for a combined can_frame that could hold both.
> 
> 
> No. You only think of CAN_RAW. There might be a way to do it with CAN_RAW but
> changing an exported visible userspace structure breaks the ABI.
> 
> Expect that we can never change struct can_frame as this is written into stone.


Answering myself, but here's a very recent statement from Linus focussing
binary compatibility:

http://thread.gmane.org/gmane.linux.kernel/1245999/focus%3D1264170

Indeed we once broke the ABI when fixing the EFF/SFF filters reported by
yourself. But this was a very simple thing in one bit that has not been
handled correctly in opposite to change struct can_frame ...

Regards,
Oliver

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Alexander Stein]

Am Mittwoch, 21. März 2012, 14:51:43 schrieb Marc Kleine-Budde:
> On 03/21/2012 02:29 PM, Alexander Stein wrote:
> >> Further, there are only certain dlc values allowed for CAN FD. We must
> >> decide if the enforce these values or simply do padding with "0"
> >> somewhere.> 
> > What I understand from the proposal is that DLCs >8 are optional. So you
> > might get hardware which still supports only 8 bytes. How can/should
> > this be handled?
> 
> I haven't read the pdf that Kurt linked in the original mail. Is the
> usage of dlc > 8 optional or the support of dlc > 8 in hardware?

Quoted from section 6:
> The CAN FD protocol allows frames with more than eight data bytes. It is
> not required that all CAN FD implementations support longer frames, CAN
> FD implementations may be limited to a subset of DATA FIELD length. A CAN FD
> implementation that supports only up to e.g. eight data bytes in a frame
> shall not treat longer received frames as an error, fault-free longer
> frames shall be acknowledged and shall take part in acceptance filtering.
> Received data bytes that exceed the CAN FD’s data handling capacity shall
> be discarded. A such limited CAN FD implementation that is requested to
> transmit a longer frame shall fill up the data bytes in the frame that
> exceed the data handling capacity with a constant byte pattern. This
> pattern shall be chosen so that it does not cause the insertion of STUFF
> BITS, e.g. 0xCC.

Sounds to me like hardware support of dlc > 8 is optional. Even in the fact 
that you want the controller to send more than e.g. 8 bytes the controller 
shall insert stuff bits (0xCC) for unsupported data bytes.

Alexander
-- 
Dipl.-Inf. Alexander Stein

SYS TEC electronic GmbH
August-Bebel-Str. 29
D-07973 Greiz

Tel: +49-3661-6279-0, Fax: +49-3661-6279-99
eMail:    Alexander.Stein@systec-electronic.com
Internet: http://www.systec-electronic.com

Managing Director: Dipl.-Phys. Siegmar Schmidt
Commercial registry: Amtsgericht Jena, HRB 205563

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Kurt Van Dijck]

On Wed, Mar 21, 2012 at 03:49:25PM +0100, Oliver Hartkopp wrote:
> On 21.03.2012 14:53, Kurt Van Dijck wrote:
> 
> > On Wed, Mar 21, 2012 at 02:21:22PM +0100, Oliver Hartkopp wrote:
> > * ESI bit.
> 
> 
> Is something that IMO needs to go into the error message when set active.
nice idea!

[...]
> 
> >>
> >> What i got from the iCC was that when you have a partly migrated network and
> >> you want to run e.g. a fast firmware upload between two CAN FD capable nodes,
> >> the other (standard CAN 2.0b) nodes have to be in listen only mode to not jam
> >> the bus with error frames.
> >>
> >> After the firmware upload all the CAN nodes switch back to CAN2.0b mode (which
> >> has to be done by root netlink access).
> > Since CANFD is not bus compatible, the chip must go down & up.
> 
> 
> Really? We don't know so far.
You're right, this theory can never be proved to be right.
At least, I'll be supprised to see some form of dynamic switch.
> 

Regards,
Kurt

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Kurt Van Dijck]

On Wed, Mar 21, 2012 at 04:05:28PM +0100, Oliver Hartkopp wrote:
> On 21.03.2012 15:56, Wolfgang Grandegger wrote:
> 
> > Also DLC=9 means 12 bytes, DLC=10 means 16 bytes, DLC=15 means 64 bytes.
> > This may even change in the final spec.
> 
> Yep!

Although the precise coding is not final yet, I'd propose to not use that coding
in the ABI for these reasons:
* The reason of fitting a DLC in 4 bits makes sense on the wire
  but not on the ABI. We still use an u8!
* Decoding & encoding between real length & DLC IMO is best done next to the
  chips register access.

> 
> 
> > 
> >>> 3. Will these differences be visible in the CAN registers? Is this relevant?
> >> Without hardware, it's a bit early to predict. I guess it will be visible, but
> >> not relevant since that's driver stuff.
> > 
> > As CANFD controllers also supports CAN2.0 frames, they must provide the
> > the relevant information somehow, similar to EFF and SFF.
I doubt this.
EFF & SFF share the same bus. CANFD vs. CAN2.0 is not a per-frame thing. You
have configured it yourself at chip initialization time...
> > 
> >> I did not get into real drivers yet...
> >>>
> >>> What i got from the iCC was that when you have a partly migrated network and
> >>> you want to run e.g. a fast firmware upload between two CAN FD capable nodes,
> >>> the other (standard CAN 2.0b) nodes have to be in listen only mode to not jam
> >>> the bus with error frames.
> > 
> > Due to the bit-rate switching, a assume.
> 
> 
> Yes - the 'old' controllers would put error frames on the fast payload data.
> 
FYI: The bitrate switch is not the only cause.
The bitstream on the wire for 'equal' CANFD & CAN2.0 frames is different.
A regular CAN2.0 chip will signal protocol violations.
> 

Kind regards,
Kurt

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 2497 bytes --]

On 03/22/2012 10:24 AM, Kurt Van Dijck wrote:
> On Wed, Mar 21, 2012 at 04:05:28PM +0100, Oliver Hartkopp wrote:
>> On 21.03.2012 15:56, Wolfgang Grandegger wrote:
>>
>>> Also DLC=9 means 12 bytes, DLC=10 means 16 bytes, DLC=15 means 64 bytes.
>>> This may even change in the final spec.
>>
>> Yep!
> 
> Although the precise coding is not final yet, I'd propose to not use that coding
> in the ABI for these reasons:
> * The reason of fitting a DLC in 4 bits makes sense on the wire
>   but not on the ABI. We still use an u8!
> * Decoding & encoding between real length & DLC IMO is best done next to the
>   chips register access.

Yes - probably with a helper function.

>>>>> 3. Will these differences be visible in the CAN registers? Is this relevant?
>>>> Without hardware, it's a bit early to predict. I guess it will be visible, but
>>>> not relevant since that's driver stuff.
>>>
>>> As CANFD controllers also supports CAN2.0 frames, they must provide the
>>> the relevant information somehow, similar to EFF and SFF.
> I doubt this.
> EFF & SFF share the same bus. CANFD vs. CAN2.0 is not a per-frame thing. You
> have configured it yourself at chip initialization time...

Although I haven't seen any data sheet nor hardware I suppose you have
to configure a chip for CANFD during initialisation.

Oliver can you ask at Bosch if we can get a manual for the upcoming CAN
FD chips?

>>>> I did not get into real drivers yet...
>>>>>
>>>>> What i got from the iCC was that when you have a partly migrated network and
>>>>> you want to run e.g. a fast firmware upload between two CAN FD capable nodes,
>>>>> the other (standard CAN 2.0b) nodes have to be in listen only mode to not jam
>>>>> the bus with error frames.
>>>
>>> Due to the bit-rate switching, a assume.
>>
>>
>> Yes - the 'old' controllers would put error frames on the fast payload data.
>>
> FYI: The bitrate switch is not the only cause.
> The bitstream on the wire for 'equal' CANFD & CAN2.0 frames is different.
> A regular CAN2.0 chip will signal protocol violations.

IIRC in the data phase CANFD doesn't use the prop seg anymore and some
reserved bits in the frame are now used.

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: 262 bytes --]

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Wolfgang Grandegger]

On 03/22/2012 10:24 AM, Kurt Van Dijck wrote:
> On Wed, Mar 21, 2012 at 04:05:28PM +0100, Oliver Hartkopp wrote:
>> On 21.03.2012 15:56, Wolfgang Grandegger wrote:
>>
>>> Also DLC=9 means 12 bytes, DLC=10 means 16 bytes, DLC=15 means 64 bytes.
>>> This may even change in the final spec.
>>
>> Yep!
> 
> Although the precise coding is not final yet, I'd propose to not use that coding
> in the ABI for these reasons:
> * The reason of fitting a DLC in 4 bits makes sense on the wire
>   but not on the ABI. We still use an u8!
> * Decoding & encoding between real length & DLC IMO is best done next to the
>   chips register access.

I was speaking of the frame's DLC field. The app/abi should support any
integer length, of course.

>>>>> 3. Will these differences be visible in the CAN registers? Is this relevant?
>>>> Without hardware, it's a bit early to predict. I guess it will be visible, but
>>>> not relevant since that's driver stuff.
>>>
>>> As CANFD controllers also supports CAN2.0 frames, they must provide the
>>> the relevant information somehow, similar to EFF and SFF.
> I doubt this.
> EFF & SFF share the same bus. CANFD vs. CAN2.0 is not a per-frame thing. You
> have configured it yourself at chip initialization time...

Why not? A CANFD capable controller should be able to distinguish
between a legacy and a CANFD frame by looking to the BRS bit in the frame.

Wolfgang.

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Kurt Van Dijck]

On Wed, Mar 21, 2012 at 04:05:28PM +0100, Oliver Hartkopp wrote:
> On 21.03.2012 15:56, Wolfgang Grandegger wrote:
> 
> >>>
> >>> I think it's much more tricky to find a proper solution here.
> >> As I mentioned before, I just use CAN_RAW with this. Actually, I just
> >> ran a real CAN program that I wrote years ago. It still operates!
> > 
> > Would you program still run if the payload is bigger than 8 bytes?
> > I agree with Oliver.
> 
The other side would have noticed at design time that it cannot
send >8 bytes :-)

Did I miss something here? Is there a reason why existing programs
may or should not work on a CANFD bus? I wanted to incorporate
CANFD functionality _without_ putting an alternative API next to the
current one, so to be able to use existing binaries on a future
CANFD bus _without_ recompile (and thus not using the extras
possibilities for that program).
> 
> puh. Time for a coffee :-)
Ack
> 
regards,
Kurt

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Wolfgang Grandegger]

Hi Kurt,

On 03/22/2012 10:57 AM, Kurt Van Dijck wrote:
> On Wed, Mar 21, 2012 at 04:05:28PM +0100, Oliver Hartkopp wrote:
>> On 21.03.2012 15:56, Wolfgang Grandegger wrote:
>>
>>>>>
>>>>> I think it's much more tricky to find a proper solution here.
>>>> As I mentioned before, I just use CAN_RAW with this. Actually, I just
>>>> ran a real CAN program that I wrote years ago. It still operates!
>>>
>>> Would you program still run if the payload is bigger than 8 bytes?
>>> I agree with Oliver.
>>
> The other side would have noticed at design time that it cannot
> send >8 bytes :-)
> 
> Did I miss something here? Is there a reason why existing programs
> may or should not work on a CANFD bus? I wanted to incorporate
> CANFD functionality _without_ putting an alternative API next to the
> current one, so to be able to use existing binaries on a future
> CANFD bus _without_ recompile (and thus not using the extras
> possibilities for that program).

I believe that 99% of the existing apps, including candump, do work with
a (hard-coded) maximum length of 8 bytes instead of a variable length.

Wolfgang.

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Kurt Van Dijck]

On Thu, Mar 22, 2012 at 10:38:40AM +0100, Wolfgang Grandegger wrote:
> On 03/22/2012 10:24 AM, Kurt Van Dijck wrote:
> > On Wed, Mar 21, 2012 at 04:05:28PM +0100, Oliver Hartkopp wrote:
> >> On 21.03.2012 15:56, Wolfgang Grandegger wrote:
> >>
> 
> >>>>> 3. Will these differences be visible in the CAN registers? Is this relevant?
> >>>> Without hardware, it's a bit early to predict. I guess it will be visible, but
> >>>> not relevant since that's driver stuff.
> >>>
> >>> As CANFD controllers also supports CAN2.0 frames, they must provide the
> >>> the relevant information somehow, similar to EFF and SFF.
> > I doubt this.
> > EFF & SFF share the same bus. CANFD vs. CAN2.0 is not a per-frame thing. You
> > have configured it yourself at chip initialization time...
> 
> Why not? A CANFD capable controller should be able to distinguish
> between a legacy and a CANFD frame by looking to the BRS bit in the frame.
Good point!
Yet, it remains a bus property, not a per-frame property.
From the point that you know that CANFD frames are allowed,
putting single CAN2.0 frames on the bus looks useless.

Regards,
Kurt

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Kurt Van Dijck]

Hey Wolfgang,

On Thu, Mar 22, 2012 at 11:06:44AM +0100, Wolfgang Grandegger wrote:
> Hi Kurt,
> 
> On 03/22/2012 10:57 AM, Kurt Van Dijck wrote:
> > On Wed, Mar 21, 2012 at 04:05:28PM +0100, Oliver Hartkopp wrote:
> >> On 21.03.2012 15:56, Wolfgang Grandegger wrote:
> >>
> >>>>>
> >>>>> I think it's much more tricky to find a proper solution here.
> >>>> As I mentioned before, I just use CAN_RAW with this. Actually, I just
> >>>> ran a real CAN program that I wrote years ago. It still operates!
> >>>
> >>> Would you program still run if the payload is bigger than 8 bytes?
> >>> I agree with Oliver.
> >>
> > The other side would have noticed at design time that it cannot
> > send >8 bytes :-)
> > 
> > Did I miss something here? Is there a reason why existing programs
> > may or should not work on a CANFD bus? I wanted to incorporate
> > CANFD functionality _without_ putting an alternative API next to the
> > current one, so to be able to use existing binaries on a future
> > CANFD bus _without_ recompile (and thus not using the extras
> > possibilities for that program).
> 
> I believe that 99% of the existing apps, including candump, do work with
> a (hard-coded) maximum length of 8 bytes instead of a variable length.
I agree with that (99% may even be a bit low :-) )

I did manage to let such applications still function as expected.

The remaining 1% may or may not work. In case of proper coded programs,
I'm sure they work. In other cases, I'm not sure.
Thus the question is:
Does the remaining 1% that we don't know, justify an alternate approach?

My initial patch mainly illustrates that this may be a reasonable question.

As ethernet user, on a socket level, I also do not care about low-level
technology (100Mbit, 1GBit, UTP, ...).

Kind regards,
Kurt

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Wolfgang Grandegger]

On 03/22/2012 11:35 AM, Kurt Van Dijck wrote:
> Hey Wolfgang,
> 
> On Thu, Mar 22, 2012 at 11:06:44AM +0100, Wolfgang Grandegger wrote:
>> Hi Kurt,
>>
>> On 03/22/2012 10:57 AM, Kurt Van Dijck wrote:
>>> On Wed, Mar 21, 2012 at 04:05:28PM +0100, Oliver Hartkopp wrote:
>>>> On 21.03.2012 15:56, Wolfgang Grandegger wrote:
>>>>
>>>>>>>
>>>>>>> I think it's much more tricky to find a proper solution here.
>>>>>> As I mentioned before, I just use CAN_RAW with this. Actually, I just
>>>>>> ran a real CAN program that I wrote years ago. It still operates!
>>>>>
>>>>> Would you program still run if the payload is bigger than 8 bytes?
>>>>> I agree with Oliver.
>>>>
>>> The other side would have noticed at design time that it cannot
>>> send >8 bytes :-)
>>>
>>> Did I miss something here? Is there a reason why existing programs
>>> may or should not work on a CANFD bus? I wanted to incorporate
>>> CANFD functionality _without_ putting an alternative API next to the
>>> current one, so to be able to use existing binaries on a future
>>> CANFD bus _without_ recompile (and thus not using the extras
>>> possibilities for that program).
>>
>> I believe that 99% of the existing apps, including candump, do work with
>> a (hard-coded) maximum length of 8 bytes instead of a variable length.
> I agree with that (99% may even be a bit low :-) )
> 
> I did manage to let such applications still function as expected.
> 
> The remaining 1% may or may not work. In case of proper coded programs,
> I'm sure they work. In other cases, I'm not sure.
> Thus the question is:
> Does the remaining 1% that we don't know, justify an alternate approach?

Well, I do not yet know. I first would like to see the data sheet of a
CANFD controller to understand better the possible use cases. And
without real use case, we will not add any CANFD support to the mainline
kernel anyway. There is still some time to think and discuss.

> My initial patch mainly illustrates that this may be a reasonable question.
> 
> As ethernet user, on a socket level, I also do not care about low-level
> technology (100Mbit, 1GBit, UTP, ...).

But that's because we use higher-level protocols like UDP and TCP which
do not care about the hardware frame size.

Wolfgang.

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Oliver Hartkopp]

On 22.03.2012 12:00, Wolfgang Grandegger wrote:

> On 03/22/2012 11:35 AM, Kurt Van Dijck wrote:

>> The remaining 1% may or may not work. In case of proper coded programs,
>> I'm sure they work. In other cases, I'm not sure.
>> Thus the question is:
>> Does the remaining 1% that we don't know, justify an alternate approach?


Yes it does. As pointed out yesterday breaking ABI is a no go.
I won't like to come into focus with Linus discussing a pointless ABI item.

> Well, I do not yet know. I first would like to see the data sheet of a
> CANFD controller to understand better the possible use cases. And
> without real use case, we will not add any CANFD support to the mainline
> kernel anyway. There is still some time to think and discuss.


That's also my impression.

> 
>> My initial patch mainly illustrates that this may be a reasonable question.
>>
>> As ethernet user, on a socket level, I also do not care about low-level
>> technology (100Mbit, 1GBit, UTP, ...).
> 
> But that's because we use higher-level protocols like UDP and TCP which
> do not care about the hardware frame size.


Indeed this is what i wanted to express, when writing this:

When there's a defined socket API that makes use of struct can_frame (like
can_raw, can_bcm, can_gw - but not isotp! \o/ ) we need to

- duplicate the socket API - like CAN_RAWFD (ugly)
- switch the frame size (e.g. via sockopt)
- a third unknown idea ...

ISOTP is off this discussion as it does not deal with struct can_frame.

Any other of the CAN protocol definitions does.

Regards,
Oliver

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Kurt Van Dijck]

On Thu, Mar 22, 2012 at 01:25:47PM +0100, Oliver Hartkopp wrote:
> On 22.03.2012 12:00, Wolfgang Grandegger wrote:
> 
> > On 03/22/2012 11:35 AM, Kurt Van Dijck wrote:
> 
> > Well, I do not yet know. I first would like to see the data sheet of a
> > CANFD controller to understand better the possible use cases. And
> > without real use case, we will not add any CANFD support to the mainline
> > kernel anyway. There is still some time to think and discuss.
> 
> 
> That's also my impression.
ack.

Kurt

Re: [RFC] can: Introducing CANFD for af_can & can-raw

[Wolfgang Grandegger]

On 03/22/2012 11:13 AM, Kurt Van Dijck wrote:
> On Thu, Mar 22, 2012 at 10:38:40AM +0100, Wolfgang Grandegger wrote:
>> On 03/22/2012 10:24 AM, Kurt Van Dijck wrote:
>>> On Wed, Mar 21, 2012 at 04:05:28PM +0100, Oliver Hartkopp wrote:
>>>> On 21.03.2012 15:56, Wolfgang Grandegger wrote:
>>>>
>>
>>>>>>> 3. Will these differences be visible in the CAN registers? Is this relevant?
>>>>>> Without hardware, it's a bit early to predict. I guess it will be visible, but
>>>>>> not relevant since that's driver stuff.
>>>>>
>>>>> As CANFD controllers also supports CAN2.0 frames, they must provide the
>>>>> the relevant information somehow, similar to EFF and SFF.
>>> I doubt this.
>>> EFF & SFF share the same bus. CANFD vs. CAN2.0 is not a per-frame thing. You
>>> have configured it yourself at chip initialization time...
>>
>> Why not? A CANFD capable controller should be able to distinguish
>> between a legacy and a CANFD frame by looking to the BRS bit in the frame.
> Good point!

I got some more infos about the up-coming CAN FD controllers:

- There will be two sets of bit-timing configuration registers.

- They will have configuration bits to disable the FD functions. Then
  they will behave just like legacy CAN controllers. This can only be
  configured before going "bus on" ("clear init").

- When the FD function is enabled, *both* legacy CAN and FD frames
  will be accepted. The *TX* FD function must be enabled separately,
  otherwise legacy CAN frames will be sent. This can be changed while
  running (bus-on).

- This means, on a bus with only CAN FD controllers, both, legacy and FD
  frames can be sent and received. Not sure if that makes sense, though.

- The CAN FD format (DLC up to 64 bytes) and the bit-rate switching can
  be enabled *individually*.

- Per message, there will be three additional bits in the control field:

  - EDL bit: shows if the message is in the CAN FD format.
  - BRS bit: shows if the bit-rate was switched.
  - ESI bit: shows if the sender of the CAN FD-Frames was in error-
             active or error-passive state.

- The bits above need not be set for sending CAN FD frames as they are
  already defined by the selected TX FD modes.

- If EDL=1, DLC has a different meaning.

- RTR frames will always be send in the legacy format.

- Furthermore, there might also be cost-efficient CAN FD controller
  chips which only support 8 bytes per frame (instead of 64). The
  software needs to be aware of that.

See also:

- http://www.can-cia.org/fileadmin/cia/files/icc/13/hartwich.pdf
- http://www.can-cia.org/fileadmin/cia/files/icc/13/oertel.pdf

I think this makes a bit more clear what needs to be done to support CAN
FD on the protocol and driver level. My feeling is still that we should
add a new "struct canfd_frame", which the apps then can specify with
send and recv calls maintaining full backward with "struct can_frame".
For the RX path, skbs for either "canfd_frame" or "can_frame" shall be
allocated depending on the EDL bit (or DLC). We may also want to have an
interface for dynamic bit-rate switching.

Wolfgang.