On 7/2/24 06:47, Vincent MAILHOL wrote:
On Tue. 2 juil. 2024 at 00:50, Jimmy Assarsson <extja@xxxxxxxxxx> wrote:Add helper function kvaser_usb_ticks_to_ktime() that converts from device ticks to ktime. And kvaser_usb_timestamp{48,64}_to_ktime() that converts from device 48-bit or 64-bit timestamp, to ktime. Signed-off-by: Jimmy Assarsson <extja@xxxxxxxxxx> --- Changes in v2: - New in v2. Replaces can: kvaser_usb: Add function kvaser_usb_ticks_to_ktime() - Add two more helper functions, kvaser_usb_timestamp{48,64}_to_ktime() for converting timestamps, suggested by Vincent MAILHOL [2][3] [2] https://lore.kernel.org/linux-can/CAMZ6RqKSa-6KjvgfmN9eL7A=A65gMkYsRrnaF41Azhsc45FA2Q@xxxxxxxxxxxxxx/ [3] https://lore.kernel.org/linux-can/CAMZ6Rq+Xd7+th=dKV+vrqzRtS+GY-xq2UziH1CURcQ3HxEXMqQ@xxxxxxxxxxxxxx/ drivers/net/can/usb/kvaser_usb/kvaser_usb.h | 24 +++++++++++++++++++ .../net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 10 ++++---- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h index ff10b3790d84..4256a0caae20 100644 --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h @@ -22,6 +22,8 @@ */ #include <linux/completion.h> +#include <linux/ktime.h> +#include <linux/math64.h> #include <linux/spinlock.h> #include <linux/types.h> #include <linux/usb.h> @@ -216,4 +218,26 @@ int kvaser_usb_can_rx_over_error(struct net_device *netdev); extern const struct can_bittiming_const kvaser_usb_flexc_bittiming_const; +static inline ktime_t kvaser_usb_ticks_to_ktime(const struct kvaser_usb_dev_cfg *cfg, + u64 ticks) +{ + return ns_to_ktime(div_u64(ticks * 1000, cfg->timestamp_freq)); +} + +static inline ktime_t kvaser_usb_timestamp48_to_ktime(const struct kvaser_usb_dev_cfg *cfg, + const __le16 *timestamp) +{ + u64 ticks = le16_to_cpu(timestamp[0]) | + (u64)(le16_to_cpu(timestamp[1])) << 16 | + (u64)(le16_to_cpu(timestamp[2])) << 32; + + return kvaser_usb_ticks_to_ktime(cfg, ticks); +} + +static inline ktime_t kvaser_usb_timestamp64_to_ktime(const struct kvaser_usb_dev_cfg *cfg, + __le64 timestamp) +{ + return kvaser_usb_ticks_to_ktime(cfg, le64_to_cpu(timestamp)); +} + #endif /* KVASER_USB_H */ diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c index c7ba768dfe17..ad1c6101a0cd 100644 --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c @@ -526,19 +526,17 @@ static ktime_t kvaser_usb_hydra_ktime_from_rx_cmd(const struct kvaser_usb_dev_cfg *cfg, const struct kvaser_cmd *cmd) { - u64 ticks; + ktime_t hwtstamp = 0; if (cmd->header.cmd_no == CMD_EXTENDED) { struct kvaser_cmd_ext *cmd_ext = (struct kvaser_cmd_ext *)cmd; - ticks = le64_to_cpu(cmd_ext->rx_can.timestamp); + hwtstamp = kvaser_usb_timestamp64_to_ktime(cfg, cmd_ext->rx_can.timestamp); } else { - ticks = le16_to_cpu(cmd->rx_can.timestamp[0]); - ticks += (u64)(le16_to_cpu(cmd->rx_can.timestamp[1])) << 16; - ticks += (u64)(le16_to_cpu(cmd->rx_can.timestamp[2])) << 32; + hwtstamp = kvaser_usb_timestamp48_to_ktime(cfg, cmd->rx_can.timestamp); } - return ns_to_ktime(div_u64(ticks * 1000, cfg->timestamp_freq)); + return hwtstamp; }Nitpick: this can slightly be simplified by dropping the hwtstamp local variable: kvaser_usb_hydra_ktime_from_rx_cmd(const struct kvaser_usb_dev_cfg *cfg, const struct kvaser_cmd *cmd) { if (cmd->header.cmd_no == CMD_EXTENDED) { struct kvaser_cmd_ext *cmd_ext = (struct kvaser_cmd_ext *)cmd; return kvaser_usb_timestamp64_to_ktime(cfg, cmd_ext->rx_can.timestamp); } else { return kvaser_usb_timestamp48_to_ktime(cfg, cmd->rx_can.timestamp); } }
I'll keep it as is. I prefer to have a single return at the end of the function. Regards, /jimmy
