diff mbox series

[v1,3/6] net/can: Add can_dlc2len and can_len2dlc for CAN FD.

Message ID 30758547c49f254b3965fc12500735bea8265c97.1594725647.git.pisa@cmp.felk.cvut.cz (mailing list archive)
State New, archived
Headers show
Series CTU CAN FD core support | expand

Commit Message

Pavel Pisa July 14, 2020, 12:20 p.m. UTC
From: Jan Charvat <charvj10@fel.cvut.cz>

Signed-off-by: Jan Charvat <charvj10@fel.cvut.cz>
Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
---
 include/net/can_emu.h |  4 ++++
 net/can/can_core.c    | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

Comments

Vikram Garhwal Sept. 3, 2020, 5:43 a.m. UTC | #1
On Tue, Jul 14, 2020 at 02:20:16PM +0200, pisa@cmp.felk.cvut.cz wrote:
Hi Pavel,
> From: Jan Charvat <charvj10@fel.cvut.cz>
>
> Signed-off-by: Jan Charvat <charvj10@fel.cvut.cz>
> Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
> ---
>  include/net/can_emu.h |  4 ++++
>  net/can/can_core.c    | 36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
>
> diff --git a/include/net/can_emu.h b/include/net/can_emu.h
> index c6164dcfb4..7d395fbb9b 100644
> --- a/include/net/can_emu.h
> +++ b/include/net/can_emu.h
> @@ -127,4 +127,8 @@ int can_bus_client_set_filters(CanBusClientState *,
>                                 const struct qemu_can_filter *filters,
>                                 size_t filters_cnt);
>
> +uint8_t can_dlc2len(uint8_t can_dlc);
> +
> +uint8_t can_len2dlc(uint8_t len);
> +
These function are aimed for canfd. Perhaps rename these to canfd_dlc2len and
canfd_len2dlc for better distinction?
Rest of the patch looks good to me.
>  #endif
> diff --git a/net/can/can_core.c b/net/can/can_core.c
> index 90f4d8576a..0115d78794 100644
> --- a/net/can/can_core.c
> +++ b/net/can/can_core.c
> @@ -33,6 +33,42 @@
>  #include "net/can_emu.h"
>  #include "qom/object_interfaces.h"
>
> +/* CAN DLC to real data length conversion helpers */
> +
> +static const uint8_t dlc2len[] = {
> +    0, 1, 2, 3, 4, 5, 6, 7,
> +    8, 12, 16, 20, 24, 32, 48, 64
> +};
> +
> +/* get data length from can_dlc with sanitized can_dlc */
> +uint8_t can_dlc2len(uint8_t can_dlc)
> +{
> +    return dlc2len[can_dlc & 0x0F];
> +}
> +
> +static const uint8_t len2dlc[] = {
> +    0, 1, 2, 3, 4, 5, 6, 7, 8,                              /* 0 - 8 */
> +    9, 9, 9, 9,                                             /* 9 - 12 */
> +    10, 10, 10, 10,                                         /* 13 - 16 */
> +    11, 11, 11, 11,                                         /* 17 - 20 */
> +    12, 12, 12, 12,                                         /* 21 - 24 */
> +    13, 13, 13, 13, 13, 13, 13, 13,                         /* 25 - 32 */
> +    14, 14, 14, 14, 14, 14, 14, 14,                         /* 33 - 40 */
> +    14, 14, 14, 14, 14, 14, 14, 14,                         /* 41 - 48 */
> +    15, 15, 15, 15, 15, 15, 15, 15,                         /* 49 - 56 */
> +    15, 15, 15, 15, 15, 15, 15, 15                          /* 57 - 64 */
> +};
> +
> +/* map the sanitized data length to an appropriate data length code */
> +uint8_t can_len2dlc(uint8_t len)
> +{
> +    if (unlikely(len > 64)) {
> +        return 0xF;
> +    }
> +
> +    return len2dlc[len];
> +}
> +
>  struct CanBusState {
>      Object object;
>
> --
> 2.20.1
>
>
Pavel Pisa Sept. 3, 2020, 6:12 a.m. UTC | #2
Hello Vikram,

On Thursday 03 of September 2020 07:43:34 Vikram Garhwal wrote:
> On Tue, Jul 14, 2020 at 02:20:16PM +0200, pisa@cmp.felk.cvut.cz wrote:
> Hi Pavel,
>
> > From: Jan Charvat <charvj10@fel.cvut.cz>
> >
> > Signed-off-by: Jan Charvat <charvj10@fel.cvut.cz>
> > Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
> > ---
> >  include/net/can_emu.h |  4 ++++
> >  net/can/can_core.c    | 36 ++++++++++++++++++++++++++++++++++++
> >  2 files changed, 40 insertions(+)
> >
> > diff --git a/include/net/can_emu.h b/include/net/can_emu.h
> > index c6164dcfb4..7d395fbb9b 100644
> > --- a/include/net/can_emu.h
> > +++ b/include/net/can_emu.h
> > @@ -127,4 +127,8 @@ int can_bus_client_set_filters(CanBusClientState *,
> >                                 const struct qemu_can_filter *filters,
> >                                 size_t filters_cnt);
> >
> > +uint8_t can_dlc2len(uint8_t can_dlc);
> > +
> > +uint8_t can_len2dlc(uint8_t len);
> > +
>
> These function are aimed for canfd. Perhaps rename these to canfd_dlc2len
> and canfd_len2dlc for better distinction?
> Rest of the patch looks good to me.

I do not insits on name. But function correspond 1:1 to the Linux
kernel ones

https://elixir.bootlin.com/linux/latest/source/drivers/net/can/dev.c#L34

These functions/tables are very short, but may it be, we should add comment
about their origin. This part of original file is Oliver Hartkopp
contribution

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/can/dev.c?h=v5.9-rc3&id=1e0625facab2e871472472b7df87d8fbe6caf75a

some other helpers are from Wolfgang Grandegger and me.

Best wishes,

Pavel
Vikram Garhwal Sept. 3, 2020, 6:38 a.m. UTC | #3
On Thu, Sep 03, 2020 at 08:12:42AM +0200, Pavel Pisa wrote:
Hey Pavel,
Thanks for clarifying this and sharing the relevant links.

Regards,
Vikram
> Hello Vikram,
>
> On Thursday 03 of September 2020 07:43:34 Vikram Garhwal wrote:
> > On Tue, Jul 14, 2020 at 02:20:16PM +0200, pisa@cmp.felk.cvut.cz wrote:
> > Hi Pavel,
> >
> > > From: Jan Charvat <charvj10@fel.cvut.cz>
> > >
> > > Signed-off-by: Jan Charvat <charvj10@fel.cvut.cz>
> > > Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Reviewed-by: Vikram Garhwal <fnu.vikram@xilinx.com>
> > > ---
> > >  include/net/can_emu.h |  4 ++++
> > >  net/can/can_core.c    | 36 ++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 40 insertions(+)
> > >
> > > diff --git a/include/net/can_emu.h b/include/net/can_emu.h
> > > index c6164dcfb4..7d395fbb9b 100644
> > > --- a/include/net/can_emu.h
> > > +++ b/include/net/can_emu.h
> > > @@ -127,4 +127,8 @@ int can_bus_client_set_filters(CanBusClientState *,
> > >                                 const struct qemu_can_filter *filters,
> > >                                 size_t filters_cnt);
> > >
> > > +uint8_t can_dlc2len(uint8_t can_dlc);
> > > +
> > > +uint8_t can_len2dlc(uint8_t len);
> > > +
> >
> > These function are aimed for canfd. Perhaps rename these to canfd_dlc2len
> > and canfd_len2dlc for better distinction?
> > Rest of the patch looks good to me.
>
> I do not insits on name. But function correspond 1:1 to the Linux
> kernel ones
>
> https://elixir.bootlin.com/linux/latest/source/drivers/net/can/dev.c#L34
>
> These functions/tables are very short, but may it be, we should add comment
> about their origin. This part of original file is Oliver Hartkopp
> contribution
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/can/dev.c?h=v5.9-rc3&id=1e0625facab2e871472472b7df87d8fbe6caf75a
>
> some other helpers are from Wolfgang Grandegger and me.
>
> Best wishes,
>
> Pavel
diff mbox series

Patch

diff --git a/include/net/can_emu.h b/include/net/can_emu.h
index c6164dcfb4..7d395fbb9b 100644
--- a/include/net/can_emu.h
+++ b/include/net/can_emu.h
@@ -127,4 +127,8 @@  int can_bus_client_set_filters(CanBusClientState *,
                                const struct qemu_can_filter *filters,
                                size_t filters_cnt);
 
+uint8_t can_dlc2len(uint8_t can_dlc);
+
+uint8_t can_len2dlc(uint8_t len);
+
 #endif
diff --git a/net/can/can_core.c b/net/can/can_core.c
index 90f4d8576a..0115d78794 100644
--- a/net/can/can_core.c
+++ b/net/can/can_core.c
@@ -33,6 +33,42 @@ 
 #include "net/can_emu.h"
 #include "qom/object_interfaces.h"
 
+/* CAN DLC to real data length conversion helpers */
+
+static const uint8_t dlc2len[] = {
+    0, 1, 2, 3, 4, 5, 6, 7,
+    8, 12, 16, 20, 24, 32, 48, 64
+};
+
+/* get data length from can_dlc with sanitized can_dlc */
+uint8_t can_dlc2len(uint8_t can_dlc)
+{
+    return dlc2len[can_dlc & 0x0F];
+}
+
+static const uint8_t len2dlc[] = {
+    0, 1, 2, 3, 4, 5, 6, 7, 8,                              /* 0 - 8 */
+    9, 9, 9, 9,                                             /* 9 - 12 */
+    10, 10, 10, 10,                                         /* 13 - 16 */
+    11, 11, 11, 11,                                         /* 17 - 20 */
+    12, 12, 12, 12,                                         /* 21 - 24 */
+    13, 13, 13, 13, 13, 13, 13, 13,                         /* 25 - 32 */
+    14, 14, 14, 14, 14, 14, 14, 14,                         /* 33 - 40 */
+    14, 14, 14, 14, 14, 14, 14, 14,                         /* 41 - 48 */
+    15, 15, 15, 15, 15, 15, 15, 15,                         /* 49 - 56 */
+    15, 15, 15, 15, 15, 15, 15, 15                          /* 57 - 64 */
+};
+
+/* map the sanitized data length to an appropriate data length code */
+uint8_t can_len2dlc(uint8_t len)
+{
+    if (unlikely(len > 64)) {
+        return 0xF;
+    }
+
+    return len2dlc[len];
+}
+
 struct CanBusState {
     Object object;