* [PATCH 1/6] canfd: add new data structures and constants
@ 2012-06-18 17:39 Oliver Hartkopp
2012-06-18 17:51 ` Oliver Hartkopp
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Oliver Hartkopp @ 2012-06-18 17:39 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can
- add new struct canfd_frame
- check identical element offsets in struct can_frame and struct canfd_frame
- new ETH_P_CANFD definition to tag CAN FD skbs correctly
- add CAN_MTU and CANFD_MTU definitions for easy frame and mode detection
- add CAN[FD]_MAX_[DLC|DLEN] helper constants to remove hard coded values
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
include/linux/can.h | 55 ++++++++++++++++++++++++++++++++++++++++++----
include/linux/if_ether.h | 3 ++-
net/can/af_can.c | 7 ++++++
3 files changed, 60 insertions(+), 5 deletions(-)
diff --git a/include/linux/can.h b/include/linux/can.h
index 17334c0..25265e7 100644
--- a/include/linux/can.h
+++ b/include/linux/can.h
@@ -46,18 +46,65 @@ typedef __u32 canid_t;
*/
typedef __u32 can_err_mask_t;
+#define CAN_MAX_DLC 8
+#define CAN_MAX_DLEN 8
+
+#define CANFD_MAX_DLC 15
+#define CANFD_MAX_DLEN 64
+
/**
* struct can_frame - basic CAN frame structure
- * @can_id: the CAN ID of the frame and CAN_*_FLAG flags, see above.
- * @can_dlc: the data length field of the CAN frame
- * @data: the CAN frame payload.
+ * @can_id: CAN ID of the frame and CAN_*_FLAG flags, see canid_t definition
+ * @can_dlc: frame payload length in byte (0 .. 8) aka data length code
+ * N.B. the DLC field from ISO 11898-1 Chapter 8.4.2.3 has a 1:1
+ * mapping of the 'data length code' to the real payload length
+ * @data: CAN frame payload (up to 8 byte)
*/
struct can_frame {
canid_t can_id; /* 32 bit CAN_ID + EFF/RTR/ERR flags */
- __u8 can_dlc; /* data length code: 0 .. 8 */
+ __u8 can_dlc; /* frame payload length in byte (0 .. 8) */
__u8 data[8] __attribute__((aligned(8)));
};
+/*
+ * defined bits for canfd_frame.flags
+ *
+ * As the default for CAN FD should be to support the high data rate in the
+ * payload section of the frame (HDR) and to support up to 64 byte in the
+ * data section (EDL) the bits are only set in the non-default case.
+ * Btw. as long as there's no real implementation for CAN FD network driver
+ * these bits are only preliminary.
+ *
+ * RX: NOHDR/NOEDL - info about received CAN FD frame
+ * ESI - bit from originating CAN controller
+ * TX: NOHDR/NOEDL - control per-frame settings if supported by CAN controller
+ * ESI - bit is set by local CAN controller
+ */
+#define CANFD_NOHDR 0x01 /* frame without high data rate */
+#define CANFD_NOEDL 0x02 /* frame without extended data length */
+#define CANFD_ESI 0x04 /* error state indicator */
+
+/**
+ * struct canfd_frame - CAN flexible data rate frame structure
+ * @can_id: CAN ID of the frame and CAN_*_FLAG flags, see canid_t definition
+ * @len: frame payload length in byte (0 .. 64)
+ * @flags: additional flags for CAN FD
+ * @__res0: reserved / padding
+ * @__res1: reserved / padding
+ * @data: CAN FD frame payload (up to 64 byte)
+ */
+struct canfd_frame {
+ canid_t can_id; /* 32 bit CAN_ID + EFF/RTR/ERR flags */
+ __u8 len; /* frame payload length in byte (0 .. 64) */
+ __u8 flags; /* additional flags for CAN FD */
+ __u8 __res0; /* reserved / padding */
+ __u8 __res1; /* reserved / padding */
+ __u8 data[64] __attribute__((aligned(8)));
+};
+
+#define CAN_MTU (sizeof(struct can_frame))
+#define CANFD_MTU (sizeof(struct canfd_frame))
+
/* particular protocols of the protocol family PF_CAN */
#define CAN_RAW 1 /* RAW sockets */
#define CAN_BCM 2 /* Broadcast Manager */
diff --git a/include/linux/if_ether.h b/include/linux/if_ether.h
index 56d907a..260138b 100644
--- a/include/linux/if_ether.h
+++ b/include/linux/if_ether.h
@@ -105,7 +105,8 @@
#define ETH_P_WAN_PPP 0x0007 /* Dummy type for WAN PPP frames*/
#define ETH_P_PPP_MP 0x0008 /* Dummy type for PPP MP frames */
#define ETH_P_LOCALTALK 0x0009 /* Localtalk pseudo type */
-#define ETH_P_CAN 0x000C /* Controller Area Network */
+#define ETH_P_CAN 0x000C /* Controller Area Network (CAN)*/
+#define ETH_P_CANFD 0x000D /* CAN FD 64 byte payload frames*/
#define ETH_P_PPPTALK 0x0010 /* Dummy type for Atalk over PPP*/
#define ETH_P_TR_802_2 0x0011 /* 802.2 frames */
#define ETH_P_MOBITEX 0x0015 /* Mobitex (kaz@cafe.net) */
diff --git a/net/can/af_can.c b/net/can/af_can.c
index 6efcd37..c96140a 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -41,6 +41,7 @@
*/
#include <linux/module.h>
+#include <linux/stddef.h>
#include <linux/init.h>
#include <linux/kmod.h>
#include <linux/slab.h>
@@ -824,6 +825,12 @@ static struct notifier_block can_netdev_notifier __read_mostly = {
static __init int can_init(void)
{
+ /* check for correct padding to be able to use the structs similarly */
+ BUILD_BUG_ON(offsetof(struct can_frame, can_dlc) !=
+ offsetof(struct canfd_frame, len) ||
+ offsetof(struct can_frame, data) !=
+ offsetof(struct canfd_frame, data));
+
printk(banner);
memset(&can_rx_alldev_list, 0, sizeof(can_rx_alldev_list));
--
1.7.10.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] canfd: add new data structures and constants
2012-06-18 17:39 [PATCH 1/6] canfd: add new data structures and constants Oliver Hartkopp
@ 2012-06-18 17:51 ` Oliver Hartkopp
2012-06-18 19:12 ` Marc Kleine-Budde
2012-06-18 19:13 ` Marc Kleine-Budde
2012-06-19 6:45 ` Wolfgang Grandegger
2 siblings, 1 reply; 14+ messages in thread
From: Oliver Hartkopp @ 2012-06-18 17:51 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can
On 18.06.2012 19:39, Oliver Hartkopp wrote:
> --- a/include/linux/can.h
> +++ b/include/linux/can.h
> @@ -46,18 +46,65 @@ typedef __u32 canid_t;
> */
> typedef __u32 can_err_mask_t;
> +#define CAN_MAX_DLC 8
> +#define CAN_MAX_DLEN 8
> +
Hm looks broken - but the patch and the repo content is ok.
Oh, well.
Anyone an idea how to set up imap with strato.de for git send-imap with a
debian system ??
I tried this imapsslappend.pl script from
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=572521
which obviously mangles the mails a bit :-(
Regards,
Oliver
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] canfd: add new data structures and constants
2012-06-18 17:51 ` Oliver Hartkopp
@ 2012-06-18 19:12 ` Marc Kleine-Budde
0 siblings, 0 replies; 14+ messages in thread
From: Marc Kleine-Budde @ 2012-06-18 19:12 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can
[-- Attachment #1: Type: text/plain, Size: 968 bytes --]
On 06/18/2012 07:51 PM, Oliver Hartkopp wrote:
> On 18.06.2012 19:39, Oliver Hartkopp wrote:
>
>
>> --- a/include/linux/can.h
>> +++ b/include/linux/can.h
>> @@ -46,18 +46,65 @@ typedef __u32 canid_t;
>> */
>> typedef __u32 can_err_mask_t;
>> +#define CAN_MAX_DLC 8
>> +#define CAN_MAX_DLEN 8
>> +
>
>
> Hm looks broken - but the patch and the repo content is ok.
I suggest to use git send-email. It can talk smtp with authentication
and all bells and whistles you need. Yes, the repo is all-right.
>
> Oh, well.
>
> Anyone an idea how to set up imap with strato.de for git send-imap with a
> debian system ??
sudo aptitude install dovecot-imapd
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 --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] canfd: add new data structures and constants
2012-06-18 17:39 [PATCH 1/6] canfd: add new data structures and constants Oliver Hartkopp
2012-06-18 17:51 ` Oliver Hartkopp
@ 2012-06-18 19:13 ` Marc Kleine-Budde
2012-06-18 19:19 ` Oliver Hartkopp
2012-06-19 6:45 ` Wolfgang Grandegger
2 siblings, 1 reply; 14+ messages in thread
From: Marc Kleine-Budde @ 2012-06-18 19:13 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can
[-- Attachment #1: Type: text/plain, Size: 2414 bytes --]
On 06/18/2012 07:39 PM, Oliver Hartkopp wrote:
> - add new struct canfd_frame
> - check identical element offsets in struct can_frame and struct canfd_frame
> - new ETH_P_CANFD definition to tag CAN FD skbs correctly
> - add CAN_MTU and CANFD_MTU definitions for easy frame and mode detection
> - add CAN[FD]_MAX_[DLC|DLEN] helper constants to remove hard coded values
>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
> include/linux/can.h | 55 ++++++++++++++++++++++++++++++++++++++++++----
> include/linux/if_ether.h | 3 ++-
> net/can/af_can.c | 7 ++++++
> 3 files changed, 60 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/can.h b/include/linux/can.h
> index 17334c0..25265e7 100644
> --- a/include/linux/can.h
> +++ b/include/linux/can.h
> @@ -46,18 +46,65 @@ typedef __u32 canid_t;
> */
> typedef __u32 can_err_mask_t;
> +#define CAN_MAX_DLC 8
> +#define CAN_MAX_DLEN 8
> +
> +#define CANFD_MAX_DLC 15
> +#define CANFD_MAX_DLEN 64
> +
> /**
> * struct can_frame - basic CAN frame structure
> - * @can_id: the CAN ID of the frame and CAN_*_FLAG flags, see above.
> - * @can_dlc: the data length field of the CAN frame
> - * @data: the CAN frame payload.
> + * @can_id: CAN ID of the frame and CAN_*_FLAG flags, see canid_t definition
> + * @can_dlc: frame payload length in byte (0 .. 8) aka data length code
> + * N.B. the DLC field from ISO 11898-1 Chapter 8.4.2.3 has a 1:1
> + * mapping of the 'data length code' to the real payload length
> + * @data: CAN frame payload (up to 8 byte)
> */
> struct can_frame {
> canid_t can_id; /* 32 bit CAN_ID + EFF/RTR/ERR flags */
> - __u8 can_dlc; /* data length code: 0 .. 8 */
> + __u8 can_dlc; /* frame payload length in byte (0 .. 8) */
> __u8 data[8] __attribute__((aligned(8)));
checkpatch complains:
WARNING: __aligned(size) is preferred over __attribute__((aligned(size)))
#84: FILE: include/linux/can.h:102:
+ __u8 data[64] __attribute__((aligned(8)));
total: 0 errors, 1 warnings, 97 lines checked
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 --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] canfd: add new data structures and constants
2012-06-18 19:13 ` Marc Kleine-Budde
@ 2012-06-18 19:19 ` Oliver Hartkopp
2012-06-18 19:48 ` Marc Kleine-Budde
0 siblings, 1 reply; 14+ messages in thread
From: Oliver Hartkopp @ 2012-06-18 19:19 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can
On 18.06.2012 21:13, Marc Kleine-Budde wrote:
> On 06/18/2012 07:39 PM, Oliver Hartkopp wrote:
>> - add new struct canfd_frame
>> struct can_frame {
>> canid_t can_id; /* 32 bit CAN_ID + EFF/RTR/ERR flags */
>> - __u8 can_dlc; /* data length code: 0 .. 8 */
>> + __u8 can_dlc; /* frame payload length in byte (0 .. 8) */
>> __u8 data[8] __attribute__((aligned(8)));
>
> checkpatch complains:
>
> WARNING: __aligned(size) is preferred over __attribute__((aligned(size)))
> #84: FILE: include/linux/can.h:102:
> + __u8 data[64] __attribute__((aligned(8)));
>
> total: 0 errors, 1 warnings, 97 lines checked
>
Hm, yes. I wanted to have it the same way as in struct can_frame.
I wonder if "__aligened(size)" is also compatible for backports - or
alternative if we should also modify the original can_frame:
>> - __u8 data[8] __attribute__((aligned(8)));
>> + __u8 data[8] __aligned(8);
???
Regards,
Oliver
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] canfd: add new data structures and constants
2012-06-18 19:19 ` Oliver Hartkopp
@ 2012-06-18 19:48 ` Marc Kleine-Budde
2012-06-18 19:55 ` Marc Kleine-Budde
0 siblings, 1 reply; 14+ messages in thread
From: Marc Kleine-Budde @ 2012-06-18 19:48 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can
[-- Attachment #1: Type: text/plain, Size: 1988 bytes --]
On 06/18/2012 09:19 PM, Oliver Hartkopp wrote:
> On 18.06.2012 21:13, Marc Kleine-Budde wrote:
>
>> On 06/18/2012 07:39 PM, Oliver Hartkopp wrote:
>>> - add new struct canfd_frame
>
>>> struct can_frame {
>>> canid_t can_id; /* 32 bit CAN_ID + EFF/RTR/ERR flags */
>>> - __u8 can_dlc; /* data length code: 0 .. 8 */
>>> + __u8 can_dlc; /* frame payload length in byte (0 .. 8) */
>>> __u8 data[8] __attribute__((aligned(8)));
>>
>> checkpatch complains:
>>
>> WARNING: __aligned(size) is preferred over __attribute__((aligned(size)))
>> #84: FILE: include/linux/can.h:102:
>> + __u8 data[64] __attribute__((aligned(8)));
>>
>> total: 0 errors, 1 warnings, 97 lines checked
> Hm, yes. I wanted to have it the same way as in struct can_frame.
> I wonder if "__aligened(size)" is also compatible for backports - or
> alternative if we should also modify the original can_frame:
__aligened(size) has been introduced in v2.6.21-rc1~91^2~274.
(How to figure out:
$ git blame include/linux/compiler-gcc.h
$ git describe 82ddcb04
v2.6.20-1507-g82ddcb0
$ git describe --contains 82ddcb04
v2.6.21-rc1~91^2~274
)
>>> - __u8 data[8] __attribute__((aligned(8)));
>>> + __u8 data[8] __aligned(8);
If you want to be super clean you can first make a patch that changes
the existing __attribute__((aligned(8))) to __aligned(8), and then add
your canfd series. If not, just mention it in the patch description
(While being here change can_fd to __aligned(8)).
Err..this is a userspace header, I just tested it, headers_install
doesn't remove it. And userspace fails to compile with this. For now
stick to __attribute__((aligned(8))).
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 --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] canfd: add new data structures and constants
2012-06-18 19:48 ` Marc Kleine-Budde
@ 2012-06-18 19:55 ` Marc Kleine-Budde
2012-06-18 20:03 ` Oliver Hartkopp
0 siblings, 1 reply; 14+ messages in thread
From: Marc Kleine-Budde @ 2012-06-18 19:55 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can
[-- Attachment #1: Type: text/plain, Size: 911 bytes --]
On 06/18/2012 09:48 PM, Marc Kleine-Budde wrote:
> If you want to be super clean you can first make a patch that changes
> the existing __attribute__((aligned(8))) to __aligned(8), and then add
> your canfd series. If not, just mention it in the patch description
> (While being here change can_fd to __aligned(8)).
>
> Err..this is a userspace header, I just tested it, headers_install
> doesn't remove it. And userspace fails to compile with this. For now
> stick to __attribute__((aligned(8))).
BTW: and it fails to compile due to a __kernel_sa_family_t which isn't
defined on my ubuntu with 2.6.32er headers
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 --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] canfd: add new data structures and constants
2012-06-18 19:55 ` Marc Kleine-Budde
@ 2012-06-18 20:03 ` Oliver Hartkopp
2012-06-18 20:20 ` Marc Kleine-Budde
0 siblings, 1 reply; 14+ messages in thread
From: Oliver Hartkopp @ 2012-06-18 20:03 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can
On 18.06.2012 21:55, Marc Kleine-Budde wrote:
> On 06/18/2012 09:48 PM, Marc Kleine-Budde wrote:
>> If you want to be super clean you can first make a patch that changes
>> the existing __attribute__((aligned(8))) to __aligned(8), and then add
>> your canfd series. If not, just mention it in the patch description
>> (While being here change can_fd to __aligned(8)).
>>
>> Err..this is a userspace header, I just tested it, headers_install
>> doesn't remove it. And userspace fails to compile with this. For now
>> stick to __attribute__((aligned(8))).
>
> BTW: and it fails to compile due to a __kernel_sa_family_t which isn't
> defined on my ubuntu with 2.6.32er headers
__kernel_sa_family_t is defined in include/linux/socket.h but with
#ifdef __KERNEL__
around it.
typedef __kernel_sa_family_t sa_family_t;
What do you compile?
If you compile a kernel -> it is defined.
If you compile an application -> it is not needed.
Regards,
Oliver
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] canfd: add new data structures and constants
2012-06-18 20:03 ` Oliver Hartkopp
@ 2012-06-18 20:20 ` Marc Kleine-Budde
0 siblings, 0 replies; 14+ messages in thread
From: Marc Kleine-Budde @ 2012-06-18 20:20 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can
[-- Attachment #1: Type: text/plain, Size: 1302 bytes --]
On 06/18/2012 10:03 PM, Oliver Hartkopp wrote:
>>> Err..this is a userspace header, I just tested it, headers_install
>>> doesn't remove it. And userspace fails to compile with this. For now
>>> stick to __attribute__((aligned(8))).
>>
>> BTW: and it fails to compile due to a __kernel_sa_family_t which isn't
>> defined on my ubuntu with 2.6.32er headers
>
>
> __kernel_sa_family_t is defined in include/linux/socket.h but with
>
> #ifdef __KERNEL__
>
> around it.
>
> typedef __kernel_sa_family_t sa_family_t;
>
> What do you compile?
canutils, I used linux/can.h generated by install_headers.
> If you compile a kernel -> it is defined.
> If you compile an application -> it is not needed.
You need recent kernel headers on your linux box to work. On the
compiler machine in the office we have:
$ grep __kernel_sa_family_t /usr/include/linux/socket.h
typedef unsigned short __kernel_sa_family_t;
__kernel_sa_family_t ss_family; /* address family */
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 --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] canfd: add new data structures and constants
2012-06-18 17:39 [PATCH 1/6] canfd: add new data structures and constants Oliver Hartkopp
2012-06-18 17:51 ` Oliver Hartkopp
2012-06-18 19:13 ` Marc Kleine-Budde
@ 2012-06-19 6:45 ` Wolfgang Grandegger
2012-06-19 7:11 ` Oliver Hartkopp
2 siblings, 1 reply; 14+ messages in thread
From: Wolfgang Grandegger @ 2012-06-19 6:45 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: Marc Kleine-Budde, linux-can
Hi Oliver,
On 06/18/2012 07:39 PM, Oliver Hartkopp wrote:
> - add new struct canfd_frame
> - check identical element offsets in struct can_frame and struct canfd_frame
> - new ETH_P_CANFD definition to tag CAN FD skbs correctly
> - add CAN_MTU and CANFD_MTU definitions for easy frame and mode detection
> - add CAN[FD]_MAX_[DLC|DLEN] helper constants to remove hard coded values
>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
> include/linux/can.h | 55 ++++++++++++++++++++++++++++++++++++++++++----
> include/linux/if_ether.h | 3 ++-
> net/can/af_can.c | 7 ++++++
> 3 files changed, 60 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/can.h b/include/linux/can.h
> index 17334c0..25265e7 100644
> --- a/include/linux/can.h
> +++ b/include/linux/can.h
> @@ -46,18 +46,65 @@ typedef __u32 canid_t;
> */
> typedef __u32 can_err_mask_t;
> +#define CAN_MAX_DLC 8
> +#define CAN_MAX_DLEN 8
> +
> +#define CANFD_MAX_DLC 15
> +#define CANFD_MAX_DLEN 64
> +
> /**
> * struct can_frame - basic CAN frame structure
> - * @can_id: the CAN ID of the frame and CAN_*_FLAG flags, see above.
> - * @can_dlc: the data length field of the CAN frame
> - * @data: the CAN frame payload.
> + * @can_id: CAN ID of the frame and CAN_*_FLAG flags, see canid_t definition
> + * @can_dlc: frame payload length in byte (0 .. 8) aka data length code
> + * N.B. the DLC field from ISO 11898-1 Chapter 8.4.2.3 has a 1:1
> + * mapping of the 'data length code' to the real payload length
> + * @data: CAN frame payload (up to 8 byte)
> */
> struct can_frame {
> canid_t can_id; /* 32 bit CAN_ID + EFF/RTR/ERR flags */
> - __u8 can_dlc; /* data length code: 0 .. 8 */
> + __u8 can_dlc; /* frame payload length in byte (0 .. 8) */
> __u8 data[8] __attribute__((aligned(8)));
> };
> +/*
> + * defined bits for canfd_frame.flags
> + *
> + * As the default for CAN FD should be to support the high data rate in the
> + * payload section of the frame (HDR) and to support up to 64 byte in the
> + * data section (EDL) the bits are only set in the non-default case.
> + * Btw. as long as there's no real implementation for CAN FD network driver
> + * these bits are only preliminary.
> + *
> + * RX: NOHDR/NOEDL - info about received CAN FD frame
> + * ESI - bit from originating CAN controller
> + * TX: NOHDR/NOEDL - control per-frame settings if supported by CAN controller
> + * ESI - bit is set by local CAN controller
> + */
> +#define CANFD_NOHDR 0x01 /* frame without high data rate */
> +#define CANFD_NOEDL 0x02 /* frame without extended data length */
> +#define CANFD_ESI 0x04 /* error state indicator */
> +
> +/**
> + * struct canfd_frame - CAN flexible data rate frame structure
> + * @can_id: CAN ID of the frame and CAN_*_FLAG flags, see canid_t definition
> + * @len: frame payload length in byte (0 .. 64)
> + * @flags: additional flags for CAN FD
> + * @__res0: reserved / padding
> + * @__res1: reserved / padding
> + * @data: CAN FD frame payload (up to 64 byte)
> + */
> +struct canfd_frame {
> + canid_t can_id; /* 32 bit CAN_ID + EFF/RTR/ERR flags */
> + __u8 len; /* frame payload length in byte (0 .. 64) */
> + __u8 flags; /* additional flags for CAN FD */
> + __u8 __res0; /* reserved / padding */
> + __u8 __res1; /* reserved / padding */
> + __u8 data[64] __attribute__((aligned(8)));
Shouldn't we use "CANFD_MAX_DLEN" here (... and maybe above as well)?.
IIRC, CANFD_MAX_DLEN might be even 128 in the finalized standard.
> +};
> +
> +#define CAN_MTU (sizeof(struct can_frame))
> +#define CANFD_MTU (sizeof(struct canfd_frame))
> +
> /* particular protocols of the protocol family PF_CAN */
> #define CAN_RAW 1 /* RAW sockets */
> #define CAN_BCM 2 /* Broadcast Manager */
> diff --git a/include/linux/if_ether.h b/include/linux/if_ether.h
> index 56d907a..260138b 100644
> --- a/include/linux/if_ether.h
> +++ b/include/linux/if_ether.h
> @@ -105,7 +105,8 @@
> #define ETH_P_WAN_PPP 0x0007 /* Dummy type for WAN PPP frames*/
> #define ETH_P_PPP_MP 0x0008 /* Dummy type for PPP MP frames */
> #define ETH_P_LOCALTALK 0x0009 /* Localtalk pseudo type */
> -#define ETH_P_CAN 0x000C /* Controller Area Network */
> +#define ETH_P_CAN 0x000C /* Controller Area Network (CAN)*/
> +#define ETH_P_CANFD 0x000D /* CAN FD 64 byte payload frames*/
Then hardcoding 64 here is also not be nice.
> #define ETH_P_PPPTALK 0x0010 /* Dummy type for Atalk over PPP*/
> #define ETH_P_TR_802_2 0x0011 /* 802.2 frames */
> #define ETH_P_MOBITEX 0x0015 /* Mobitex (kaz@cafe.net) */
> diff --git a/net/can/af_can.c b/net/can/af_can.c
> index 6efcd37..c96140a 100644
> --- a/net/can/af_can.c
> +++ b/net/can/af_can.c
> @@ -41,6 +41,7 @@
> */
> #include <linux/module.h>
> +#include <linux/stddef.h>
> #include <linux/init.h>
> #include <linux/kmod.h>
> #include <linux/slab.h>
> @@ -824,6 +825,12 @@ static struct notifier_block can_netdev_notifier __read_mostly = {
> static __init int can_init(void)
> {
> + /* check for correct padding to be able to use the structs similarly */
> + BUILD_BUG_ON(offsetof(struct can_frame, can_dlc) !=
> + offsetof(struct canfd_frame, len) ||
> + offsetof(struct can_frame, data) !=
> + offsetof(struct canfd_frame, data));
> +
> printk(banner);
> memset(&can_rx_alldev_list, 0, sizeof(can_rx_alldev_list));
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] canfd: add new data structures and constants
2012-06-19 6:45 ` Wolfgang Grandegger
@ 2012-06-19 7:11 ` Oliver Hartkopp
2012-06-19 7:28 ` Wolfgang Grandegger
2012-06-19 9:30 ` Oliver Hartkopp
0 siblings, 2 replies; 14+ messages in thread
From: Oliver Hartkopp @ 2012-06-19 7:11 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Marc Kleine-Budde, linux-can
On 19.06.2012 08:45, Wolfgang Grandegger wrote:
> Hi Oliver,
>
> On 06/18/2012 07:39 PM, Oliver Hartkopp wrote:
>> - add new struct canfd_frame
>> - check identical element offsets in struct can_frame and struct canfd_frame
>> - new ETH_P_CANFD definition to tag CAN FD skbs correctly
>> - add CAN_MTU and CANFD_MTU definitions for easy frame and mode detection
>> - add CAN[FD]_MAX_[DLC|DLEN] helper constants to remove hard coded values
>>
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
>> ---
>> include/linux/can.h | 55 ++++++++++++++++++++++++++++++++++++++++++----
>> include/linux/if_ether.h | 3 ++-
>> net/can/af_can.c | 7 ++++++
>> 3 files changed, 60 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/can.h b/include/linux/can.h
>> index 17334c0..25265e7 100644
>> --- a/include/linux/can.h
>> +++ b/include/linux/can.h
>> @@ -46,18 +46,65 @@ typedef __u32 canid_t;
>> */
>> typedef __u32 can_err_mask_t;
>> +#define CAN_MAX_DLC 8
>> +#define CAN_MAX_DLEN 8
>> +
>> +#define CANFD_MAX_DLC 15
>> +#define CANFD_MAX_DLEN 64
>> +
(..)
>> struct can_frame {
>> canid_t can_id; /* 32 bit CAN_ID + EFF/RTR/ERR flags */
>> - __u8 can_dlc; /* data length code: 0 .. 8 */
>> + __u8 can_dlc; /* frame payload length in byte (0 .. 8) */
>> __u8 data[8] __attribute__((aligned(8)));
>> };
(..)
>> + * @data: CAN FD frame payload (up to 64 byte)
>> + */
>> +struct canfd_frame {
>> + canid_t can_id; /* 32 bit CAN_ID + EFF/RTR/ERR flags */
>> + __u8 len; /* frame payload length in byte (0 .. 64) */
>> + __u8 flags; /* additional flags for CAN FD */
>> + __u8 __res0; /* reserved / padding */
>> + __u8 __res1; /* reserved / padding */
>> + __u8 data[64] __attribute__((aligned(8)));
>
> Shouldn't we use "CANFD_MAX_DLEN" here (... and maybe above as well)?.
> IIRC, CANFD_MAX_DLEN might be even 128 in the finalized standard.
>
Yes. Good point.
(..)
>> -#define ETH_P_CAN 0x000C /* Controller Area Network */
>> +#define ETH_P_CAN 0x000C /* Controller Area Network (CAN)*/
>> +#define ETH_P_CANFD 0x000D /* CAN FD 64 byte payload frames*/
>
> Then hardcoding 64 here is also not be nice.
Yes. Will change that too.
Even if there's a little chance to have more than 64 bytes it's not the right
place for this kind of information anyway.
I would wait until this evening if there are more of these remarks and send a
v2 then. A review of the documentation would be nice - if everything seems
clear to you.
Thanks,
Oliver
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] canfd: add new data structures and constants
2012-06-19 7:11 ` Oliver Hartkopp
@ 2012-06-19 7:28 ` Wolfgang Grandegger
2012-06-19 7:39 ` Oliver Hartkopp
2012-06-19 9:30 ` Oliver Hartkopp
1 sibling, 1 reply; 14+ messages in thread
From: Wolfgang Grandegger @ 2012-06-19 7:28 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: Marc Kleine-Budde, linux-can
On 06/19/2012 09:11 AM, Oliver Hartkopp wrote:
> On 19.06.2012 08:45, Wolfgang Grandegger wrote:
>
>> Hi Oliver,
>>
>> On 06/18/2012 07:39 PM, Oliver Hartkopp wrote:
>>> - add new struct canfd_frame
>>> - check identical element offsets in struct can_frame and struct canfd_frame
>>> - new ETH_P_CANFD definition to tag CAN FD skbs correctly
>>> - add CAN_MTU and CANFD_MTU definitions for easy frame and mode detection
>>> - add CAN[FD]_MAX_[DLC|DLEN] helper constants to remove hard coded values
>>>
>>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
>>> ---
>>> include/linux/can.h | 55 ++++++++++++++++++++++++++++++++++++++++++----
>>> include/linux/if_ether.h | 3 ++-
>>> net/can/af_can.c | 7 ++++++
>>> 3 files changed, 60 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/include/linux/can.h b/include/linux/can.h
>>> index 17334c0..25265e7 100644
>>> --- a/include/linux/can.h
>>> +++ b/include/linux/can.h
>>> @@ -46,18 +46,65 @@ typedef __u32 canid_t;
>>> */
>>> typedef __u32 can_err_mask_t;
>>> +#define CAN_MAX_DLC 8
>>> +#define CAN_MAX_DLEN 8
>>> +
>>> +#define CANFD_MAX_DLC 15
>>> +#define CANFD_MAX_DLEN 64
>>> +
>
> (..)
>
>>> struct can_frame {
>>> canid_t can_id; /* 32 bit CAN_ID + EFF/RTR/ERR flags */
>>> - __u8 can_dlc; /* data length code: 0 .. 8 */
>>> + __u8 can_dlc; /* frame payload length in byte (0 .. 8) */
>>> __u8 data[8] __attribute__((aligned(8)));
>>> };
>
>
> (..)
>
>>> + * @data: CAN FD frame payload (up to 64 byte)
>>> + */
>>> +struct canfd_frame {
>>> + canid_t can_id; /* 32 bit CAN_ID + EFF/RTR/ERR flags */
>>> + __u8 len; /* frame payload length in byte (0 .. 64) */
>>> + __u8 flags; /* additional flags for CAN FD */
>>> + __u8 __res0; /* reserved / padding */
>>> + __u8 __res1; /* reserved / padding */
>>> + __u8 data[64] __attribute__((aligned(8)));
>>
>> Shouldn't we use "CANFD_MAX_DLEN" here (... and maybe above as well)?.
>> IIRC, CANFD_MAX_DLEN might be even 128 in the finalized standard.
>>
>
>
> Yes. Good point.
>
> (..)
>
>>> -#define ETH_P_CAN 0x000C /* Controller Area Network */
>>> +#define ETH_P_CAN 0x000C /* Controller Area Network (CAN)*/
>>> +#define ETH_P_CANFD 0x000D /* CAN FD 64 byte payload frames*/
>>
>> Then hardcoding 64 here is also not be nice.
>
>
> Yes. Will change that too.
> Even if there's a little chance to have more than 64 bytes it's not the right
> place for this kind of information anyway.
>
> I would wait until this evening if there are more of these remarks and send a
> v2 then. A review of the documentation would be nice - if everything seems
> clear to you.
Where can I pull/fetch the patches from? IIUC, the latest patch series
you sent is broken.
Wolfgang.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] canfd: add new data structures and constants
2012-06-19 7:28 ` Wolfgang Grandegger
@ 2012-06-19 7:39 ` Oliver Hartkopp
0 siblings, 0 replies; 14+ messages in thread
From: Oliver Hartkopp @ 2012-06-19 7:39 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Marc Kleine-Budde, linux-can
>> I would wait until this evening if there are more of these remarks and send a
>> v2 then. A review of the documentation would be nice - if everything seems
>> clear to you.
>
> Where can I pull/fetch the patches from? IIUC, the latest patch series
> you sent is broken.
No. It was only the mail representation which was broken.
The following changes since commit 4670fd819e7f47392c7c6fc6168ea2857c66d163:
tcp: Get rid of inetpeer special cases. (2012-06-09 01:25:47 -0700)
are available in the git repository at:
git://gitorious.org/~hartkopp/linux-can/ollis-can-next.git canfd
for you to fetch changes up to d55c45913f93ccbd4378195e2d6ca96b2d14573b:
canfd: update documentation according to CAN FD extensions (2012-06-17 21:23:55 +0200)
----------------------------------------------------------------
Oliver Hartkopp (6):
canfd: add new data structures and constants
canfd: add support for CAN FD in PF_CAN core
canfd: add support for CAN FD in CAN_RAW sockets
candev: add/update helpers for CAN FD
vcan: add CAN FD support
canfd: update documentation according to CAN FD extensions
(..)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] canfd: add new data structures and constants
2012-06-19 7:11 ` Oliver Hartkopp
2012-06-19 7:28 ` Wolfgang Grandegger
@ 2012-06-19 9:30 ` Oliver Hartkopp
1 sibling, 0 replies; 14+ messages in thread
From: Oliver Hartkopp @ 2012-06-19 9:30 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Marc Kleine-Budde, linux-can
On 19.06.2012 09:11, Oliver Hartkopp wrote:
> On 19.06.2012 08:45, Wolfgang Grandegger wrote:
>>> + * @data: CAN FD frame payload (up to 64 byte)
>>> + */
>>> +struct canfd_frame {
>>> + canid_t can_id; /* 32 bit CAN_ID + EFF/RTR/ERR flags */
>>> + __u8 len; /* frame payload length in byte (0 .. 64) */
>>> + __u8 flags; /* additional flags for CAN FD */
>>> + __u8 __res0; /* reserved / padding */
>>> + __u8 __res1; /* reserved / padding */
>>> + __u8 data[64] __attribute__((aligned(8)));
>>
>> Shouldn't we use "CANFD_MAX_DLEN" here (... and maybe above as well)?.
>> IIRC, CANFD_MAX_DLEN might be even 128 in the finalized standard.
>>
>
>
> Yes. Good point.
>
> (..)
>
>>> -#define ETH_P_CAN 0x000C /* Controller Area Network */
>>> +#define ETH_P_CAN 0x000C /* Controller Area Network (CAN)*/
>>> +#define ETH_P_CANFD 0x000D /* CAN FD 64 byte payload frames*/
>>
>> Then hardcoding 64 here is also not be nice.
>
>
> Yes. Will change that too.
> Even if there's a little chance to have more than 64 bytes it's not the right
> place for this kind of information anyway.
>
Here is the updated patch.
I also changed the structure definition of struct can_frame to use the
defined CAN_MAX_DLEN (==8) but i preserved the struct documentation to remain
readable.
Regards,
Oliver
From 561a5c55627bb54a50784aca5c9de6352c1303ee Mon Sep 17 00:00:00 2001
From: Oliver Hartkopp <socketcan@hartkopp.net>
Date: Wed, 13 Jun 2012 20:04:33 +0200
Subject: [PATCH 1/6] canfd: add new data structures and constants
- add new struct canfd_frame
- check identical element offsets in struct can_frame and struct canfd_frame
- new ETH_P_CANFD definition to tag CAN FD skbs correctly
- add CAN_MTU and CANFD_MTU definitions for easy frame and mode detection
- add CAN[FD]_MAX_[DLC|DLEN] helper constants to remove hard coded values
- update existing struct can_frame with helper constants and comments
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
include/linux/can.h | 59 ++++++++++++++++++++++++++++++++++++++++++----
include/linux/if_ether.h | 3 ++-
net/can/af_can.c | 7 ++++++
3 files changed, 63 insertions(+), 6 deletions(-)
diff --git a/include/linux/can.h b/include/linux/can.h
index 17334c0..1a66cf61 100644
--- a/include/linux/can.h
+++ b/include/linux/can.h
@@ -46,18 +46,67 @@ typedef __u32 canid_t;
*/
typedef __u32 can_err_mask_t;
+/* CAN payload length and DLC definitions according to ISO 11898-1 */
+#define CAN_MAX_DLC 8
+#define CAN_MAX_DLEN 8
+
+/* CAN FD payload length and DLC definitions according to ISO 11898-7 */
+#define CANFD_MAX_DLC 15
+#define CANFD_MAX_DLEN 64
+
/**
* struct can_frame - basic CAN frame structure
- * @can_id: the CAN ID of the frame and CAN_*_FLAG flags, see above.
- * @can_dlc: the data length field of the CAN frame
- * @data: the CAN frame payload.
+ * @can_id: CAN ID of the frame and CAN_*_FLAG flags, see canid_t definition
+ * @can_dlc: frame payload length in byte (0 .. 8) aka data length code
+ * N.B. the DLC field from ISO 11898-1 Chapter 8.4.2.3 has a 1:1
+ * mapping of the 'data length code' to the real payload length
+ * @data: CAN frame payload (up to 8 byte)
*/
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 can_dlc; /* frame payload length in byte (0 .. CAN_MAX_DLEN) */
+ __u8 data[CAN_MAX_DLEN] __attribute__((aligned(8)));
};
+/*
+ * defined bits for canfd_frame.flags
+ *
+ * As the default for CAN FD should be to support the high data rate in the
+ * payload section of the frame (HDR) and to support up to 64 byte in the
+ * data section (EDL) the bits are only set in the non-default case.
+ * Btw. as long as there's no real implementation for CAN FD network driver
+ * these bits are only preliminary.
+ *
+ * RX: NOHDR/NOEDL - info about received CAN FD frame
+ * ESI - bit from originating CAN controller
+ * TX: NOHDR/NOEDL - control per-frame settings if supported by CAN controller
+ * ESI - bit is set by local CAN controller
+ */
+#define CANFD_NOHDR 0x01 /* frame without high data rate */
+#define CANFD_NOEDL 0x02 /* frame without extended data length */
+#define CANFD_ESI 0x04 /* error state indicator */
+
+/**
+ * struct canfd_frame - CAN flexible data rate frame structure
+ * @can_id: CAN ID of the frame and CAN_*_FLAG flags, see canid_t definition
+ * @len: frame payload length in byte (0 .. CANFD_MAX_DLEN)
+ * @flags: additional flags for CAN FD
+ * @__res0: reserved / padding
+ * @__res1: reserved / padding
+ * @data: CAN FD frame payload (up to CANFD_MAX_DLEN byte)
+ */
+struct canfd_frame {
+ canid_t can_id; /* 32 bit CAN_ID + EFF/RTR/ERR flags */
+ __u8 len; /* frame payload length in byte */
+ __u8 flags; /* additional flags for CAN FD */
+ __u8 __res0; /* reserved / padding */
+ __u8 __res1; /* reserved / padding */
+ __u8 data[CANFD_MAX_DLEN] __attribute__((aligned(8)));
+};
+
+#define CAN_MTU (sizeof(struct can_frame))
+#define CANFD_MTU (sizeof(struct canfd_frame))
+
/* particular protocols of the protocol family PF_CAN */
#define CAN_RAW 1 /* RAW sockets */
#define CAN_BCM 2 /* Broadcast Manager */
diff --git a/include/linux/if_ether.h b/include/linux/if_ether.h
index 56d907a..ff423d7 100644
--- a/include/linux/if_ether.h
+++ b/include/linux/if_ether.h
@@ -105,7 +105,8 @@
#define ETH_P_WAN_PPP 0x0007 /* Dummy type for WAN PPP frames*/
#define ETH_P_PPP_MP 0x0008 /* Dummy type for PPP MP frames */
#define ETH_P_LOCALTALK 0x0009 /* Localtalk pseudo type */
-#define ETH_P_CAN 0x000C /* Controller Area Network */
+#define ETH_P_CAN 0x000C /* CAN: Controller Area Network */
+#define ETH_P_CANFD 0x000D /* CAN FD: CAN w flexi datarate */
#define ETH_P_PPPTALK 0x0010 /* Dummy type for Atalk over PPP*/
#define ETH_P_TR_802_2 0x0011 /* 802.2 frames */
#define ETH_P_MOBITEX 0x0015 /* Mobitex (kaz@cafe.net) */
diff --git a/net/can/af_can.c b/net/can/af_can.c
index 6efcd37..c96140a 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -41,6 +41,7 @@
*/
#include <linux/module.h>
+#include <linux/stddef.h>
#include <linux/init.h>
#include <linux/kmod.h>
#include <linux/slab.h>
@@ -824,6 +825,12 @@ static struct notifier_block can_netdev_notifier __read_mostly = {
static __init int can_init(void)
{
+ /* check for correct padding to be able to use the structs similarly */
+ BUILD_BUG_ON(offsetof(struct can_frame, can_dlc) !=
+ offsetof(struct canfd_frame, len) ||
+ offsetof(struct can_frame, data) !=
+ offsetof(struct canfd_frame, data));
+
printk(banner);
memset(&can_rx_alldev_list, 0, sizeof(can_rx_alldev_list));
--
1.7.10.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2012-06-19 9:30 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-18 17:39 [PATCH 1/6] canfd: add new data structures and constants Oliver Hartkopp
2012-06-18 17:51 ` Oliver Hartkopp
2012-06-18 19:12 ` Marc Kleine-Budde
2012-06-18 19:13 ` Marc Kleine-Budde
2012-06-18 19:19 ` Oliver Hartkopp
2012-06-18 19:48 ` Marc Kleine-Budde
2012-06-18 19:55 ` Marc Kleine-Budde
2012-06-18 20:03 ` Oliver Hartkopp
2012-06-18 20:20 ` Marc Kleine-Budde
2012-06-19 6:45 ` Wolfgang Grandegger
2012-06-19 7:11 ` Oliver Hartkopp
2012-06-19 7:28 ` Wolfgang Grandegger
2012-06-19 7:39 ` Oliver Hartkopp
2012-06-19 9:30 ` Oliver Hartkopp
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.