| Message ID | 20201110101852.1973-8-socketcan@hartkopp.net (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Introduce optional DLC element for Classic CAN | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | success | Link |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 1 this patch: 1 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | CHECK: Unnecessary parentheses around 'cf->len == CAN_MAX_DLEN' CHECK: Unnecessary parentheses around 'dlc > CAN_MAX_DLEN' |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 1 this patch: 1 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On 11/10/20 11:18 AM, Oliver Hartkopp wrote: > can_get_cc_dlc: get the data length code for Classical CAN raw DLC access > can_get_cc_len: get len and len8_dlc value for Classical CAN raw DLC access The final patch needs a bit more prose, like: This patch adds the following helper to functions to access Classical CAN DLC values. > Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net> > --- > include/linux/can/dev.h | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h > index e767a96ae075..9a787f73399e 100644 > --- a/include/linux/can/dev.h > +++ b/include/linux/can/dev.h > @@ -168,10 +168,38 @@ static inline bool can_is_canfd_skb(const struct sk_buff *skb) > { > /* the CAN specific type of skb is identified by its data length */ > return skb->len == CANFD_MTU; > } > > +/* helper to get the data length code (DLC) for Classical CAN raw DLC access */ > +static inline u8 can_get_cc_dlc(const u32 ctrlmode, const struct can_frame *cf) Please make the cf the first argument. > +{ > + /* return len8_dlc as dlc value only if all conditions apply */ > + if ((ctrlmode & CAN_CTRLMODE_CC_LEN8_DLC) && > + (cf->len == CAN_MAX_DLEN) && > + (cf->len8_dlc > CAN_MAX_DLEN && cf->len8_dlc <= CAN_MAX_RAW_DLC)) > + return cf->len8_dlc; > + > + /* return the payload length as dlc value */ > + return cf->len; > +} > + > +/* helper to get len and len8_dlc value for Classical CAN raw DLC access */ > +static inline u8 can_get_cc_len(const u32 ctrlmode, struct can_frame *cf, > + u8 dlc) Please make the cf the first argument. > +{ > + /* the caller already ensured that dlc is a value from 0 .. 15 */ > + if ((ctrlmode & CAN_CTRLMODE_CC_LEN8_DLC) && (dlc > CAN_MAX_DLEN)) > + cf->len8_dlc = dlc; > + > + /* limit the payload length 'len' to CAN_MAX_DLEN */ > + if (dlc > CAN_MAX_DLEN) > + return CAN_MAX_DLEN; can_cc_dlc2len() I still think, that can_frame_set_cc_len() makes more sense. See my just posted patches for illustration. > + > + return dlc; > +} > + > /* helper to define static CAN controller features at device creation time */ > static inline void can_set_static_ctrlmode(struct net_device *dev, > u32 static_mode) > { > struct can_priv *priv = netdev_priv(dev); > Marc
On 10.11.20 16:50, Marc Kleine-Budde wrote: > On 11/10/20 11:18 AM, Oliver Hartkopp wrote: > I still think, that can_frame_set_cc_len() makes more sense. See my just posted > patches for illustration. > Yep. Your patches look fine! Just remove my patches 7 & 8 and apply your suggestions instead. Many thanks, Oliver
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h index e767a96ae075..9a787f73399e 100644 --- a/include/linux/can/dev.h +++ b/include/linux/can/dev.h @@ -168,10 +168,38 @@ static inline bool can_is_canfd_skb(const struct sk_buff *skb) { /* the CAN specific type of skb is identified by its data length */ return skb->len == CANFD_MTU; } +/* helper to get the data length code (DLC) for Classical CAN raw DLC access */ +static inline u8 can_get_cc_dlc(const u32 ctrlmode, const struct can_frame *cf) +{ + /* return len8_dlc as dlc value only if all conditions apply */ + if ((ctrlmode & CAN_CTRLMODE_CC_LEN8_DLC) && + (cf->len == CAN_MAX_DLEN) && + (cf->len8_dlc > CAN_MAX_DLEN && cf->len8_dlc <= CAN_MAX_RAW_DLC)) + return cf->len8_dlc; + + /* return the payload length as dlc value */ + return cf->len; +} + +/* helper to get len and len8_dlc value for Classical CAN raw DLC access */ +static inline u8 can_get_cc_len(const u32 ctrlmode, struct can_frame *cf, + u8 dlc) +{ + /* the caller already ensured that dlc is a value from 0 .. 15 */ + if ((ctrlmode & CAN_CTRLMODE_CC_LEN8_DLC) && (dlc > CAN_MAX_DLEN)) + cf->len8_dlc = dlc; + + /* limit the payload length 'len' to CAN_MAX_DLEN */ + if (dlc > CAN_MAX_DLEN) + return CAN_MAX_DLEN; + + return dlc; +} + /* helper to define static CAN controller features at device creation time */ static inline void can_set_static_ctrlmode(struct net_device *dev, u32 static_mode) { struct can_priv *priv = netdev_priv(dev);
can_get_cc_dlc: get the data length code for Classical CAN raw DLC access can_get_cc_len: get len and len8_dlc value for Classical CAN raw DLC access Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net> --- include/linux/can/dev.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)