Re: [PATCH v2 3/5] HID: intel_ish-hid: convert timespec to ktime_t

[Srinivas Pandruvada]

On Thu, 2017-05-18 at 22:21 +0200, Arnd Bergmann wrote:
> The internal accounting uses 'timespec' based time stamps, which is
> slightly inefficient and also problematic once we get to the time_t
> overflow in 2038.
> 
> When communicating to the firmware, we even get an open-coded 64-bit
> division that prevents the code from being build-tested on 32-bit
> architectures and is inefficient due to the double conversion from
> 64-bit nanoseconds to seconds+nanoseconds and then microseconds.
> 
> This changes the code to use ktime_t instead.
> 
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
ÂAcked-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>

> ---
> Âdrivers/hid/intel-ish-hid/ipc/ipc.cÂÂÂÂÂÂ| 15 ++++-----------
> Âdrivers/hid/intel-ish-hid/ishtp/client.c |ÂÂ4 ++--
> Âdrivers/hid/intel-ish-hid/ishtp/client.h |ÂÂ6 +++---
> Âdrivers/hid/intel-ish-hid/ishtp/hbm.cÂÂÂÂ| 11 ++++-------
> Â4 files changed, 13 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-
> ish-hid/ipc/ipc.c
> index 842d8416a7a6..9a60ec13cb10 100644
> --- a/drivers/hid/intel-ish-hid/ipc/ipc.c
> +++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
> @@ -296,17 +296,12 @@ static int write_ipc_from_queue(struct
> ishtp_device *dev)
> Â	/* If sending MNG_SYNC_FW_CLOCK, update clock again */
> Â	if (IPC_HEADER_GET_PROTOCOL(doorbell_val) ==
> IPC_PROTOCOL_MNG &&
> Â		IPC_HEADER_GET_MNG_CMD(doorbell_val) ==
> MNG_SYNC_FW_CLOCK) {
> -		struct timespec ts_system;
> -		struct timeval tv_utc;
> -		uint64_tÂÂÂÂÂÂÂÂusec_system, usec_utc;
> +		uint64_t usec_system, usec_utc;
> Â		struct ipc_time_update_msg time_update;
> Â		struct time_sync_format ts_format;
> Â
> -		get_monotonic_boottime(&ts_system);
> -		do_gettimeofday(&tv_utc);
> -		usec_system = (timespec_to_ns(&ts_system)) /
> NSEC_PER_USEC;
> -		usec_utc = (uint64_t)tv_utc.tv_sec * 1000000 +
> -						((uint32_t)tv_utc.tv
> _usec);
> +		usec_system = ktime_to_us(ktime_get_boottime());
> +		usec_utc = ktime_to_us(ktime_get_real());
> Â		ts_format.ts1_source = HOST_SYSTEM_TIME_USEC;
> Â		ts_format.ts2_source = HOST_UTC_TIME_USEC;
> Â		ts_format.reserved = 0;
> @@ -575,15 +570,13 @@ static void fw_reset_work_fn(struct work_struct
> *unused)
> Âstatic void _ish_sync_fw_clock(struct ishtp_device *dev)
> Â{
> Â	static unsigned long	prev_sync;
> -	struct timespec	ts;
> Â	uint64_t	usec;
> Â
> Â	if (prev_sync && jiffies - prev_sync < 20 * HZ)
> Â		return;
> Â
> Â	prev_sync = jiffies;
> -	get_monotonic_boottime(&ts);
> -	usec = (timespec_to_ns(&ts)) / NSEC_PER_USEC;
> +	usec = ktime_to_us(ktime_get_boottime());
> Â	ipc_send_mng_msg(dev, MNG_SYNC_FW_CLOCK, &usec,
> sizeof(uint64_t));
> Â}
> Â
> diff --git a/drivers/hid/intel-ish-hid/ishtp/client.c
> b/drivers/hid/intel-ish-hid/ishtp/client.c
> index f54689ee67e1..007443ef5fca 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/client.c
> +++ b/drivers/hid/intel-ish-hid/ishtp/client.c
> @@ -921,7 +921,7 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev,
> Â
> Â	if (complete_rb) {
> Â		cl = complete_rb->cl;
> -		getnstimeofday(&cl->ts_rx);
> +		cl->ts_rx = ktime_get();
> Â		++cl->recv_msg_cnt_ipc;
> Â		ishtp_cl_read_complete(complete_rb);
> Â	}
> @@ -1038,7 +1038,7 @@ void recv_ishtp_cl_msg_dma(struct ishtp_device
> *dev, void *msg,
> Â
> Â	if (complete_rb) {
> Â		cl = complete_rb->cl;
> -		getnstimeofday(&cl->ts_rx);
> +		cl->ts_rx = ktime_get();
> Â		++cl->recv_msg_cnt_dma;
> Â		ishtp_cl_read_complete(complete_rb);
> Â	}
> diff --git a/drivers/hid/intel-ish-hid/ishtp/client.h
> b/drivers/hid/intel-ish-hid/ishtp/client.h
> index 444d069c2ed4..79eade547f5d 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/client.h
> +++ b/drivers/hid/intel-ish-hid/ishtp/client.h
> @@ -118,9 +118,9 @@ struct ishtp_cl {
> Â	unsigned int	out_flow_ctrl_cnt;
> Â
> Â	/* Rx msg ... out FC timing */
> -	struct timespec ts_rx;
> -	struct timespec ts_out_fc;
> -	struct timespec ts_max_fc_delay;
> +	ktime_t ts_rx;
> +	ktime_t ts_out_fc;
> +	ktime_t ts_max_fc_delay;
> Â	void *client_data;
> Â};
> Â
> diff --git a/drivers/hid/intel-ish-hid/ishtp/hbm.c
> b/drivers/hid/intel-ish-hid/ishtp/hbm.c
> index b7213608ce43..ae4a69f7f2f4 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/hbm.c
> +++ b/drivers/hid/intel-ish-hid/ishtp/hbm.c
> @@ -321,13 +321,10 @@ int ishtp_hbm_cl_flow_control_req(struct
> ishtp_device *dev,
> Â	if (!rv) {
> Â		++cl->out_flow_ctrl_creds;
> Â		++cl->out_flow_ctrl_cnt;
> -		getnstimeofday(&cl->ts_out_fc);
> -		if (cl->ts_rx.tv_sec && cl->ts_rx.tv_nsec) {
> -			struct timespec ts_diff;
> -
> -			ts_diff = timespec_sub(cl->ts_out_fc, cl-
> >ts_rx);
> -			if (timespec_compare(&ts_diff, &cl-
> >ts_max_fc_delay)
> -					> 0)
> +		cl->ts_out_fc = ktime_get();
> +		if (cl->ts_rx) {
> +			ktime_t ts_diff = ktime_sub(cl->ts_out_fc,
> cl->ts_rx);
> +			if (ktime_after(ts_diff, cl-
> >ts_max_fc_delay))
> Â				cl->ts_max_fc_delay = ts_diff;
> Â		}
> Â	} else {