Re: [PATCH v3 4/7] qmi: Implement QMI service request rate limiting in 'can_write_data'.

[Denis Kenzior <denkenz@gmail.com>]

Hi Grant,

 >   struct qmi_qmux_device {
> @@ -653,8 +665,25 @@ static bool can_write_data(struct l_io *io, void *user_data)
>   {
>   	struct qmi_transport *transport = user_data;
>   	struct qmi_request *req;
> +	const bool throttle_enabled = transport->min_req_period_us > 0;
> +	uint64_t now = 0;

I think this is still wrong?  There's a reason why we have doc/coding-style.txt 
item M7.

> +	uint64_t delta;
>   	int r;
>   
> +	/*
> +	 * Determine if we need to rate-limit commands to a
> +	 * transport-specific minimum request period. If so,
> +	 * return true so that the queue can be retried again
> +	 * later.
> +	 */
> +	if (throttle_enabled && transport->last_req_sent_time_us != 0) {

Consider the starting condition:

- last_req_sent_time_us -> zero
- throttle_enabled -> true

We don't enter into this if(), 'now' is still zero.

> +		now = l_time_now();
> +		delta = l_time_diff(now, transport->last_req_sent_time_us);
> +
> +		if (delta < transport->min_req_period_us)
> +			return true;
> +	}
> +
>   	req = l_queue_pop_head(transport->req_queue);
>   	if (!req)
>   		return false;
> @@ -665,6 +694,9 @@ static bool can_write_data(struct l_io *io, void *user_data)
>   		return false;
>   	}
>   
> +	if (throttle_enabled)
> +		transport->last_req_sent_time_us = now;
> +

throttle_enabled is true, we set last_req_sent_time_us to 0.  In effect your 
throttling doesn't work...?

>   	if (l_queue_length(transport->req_queue) > 0)
>   		return true;
>   

Regards,
-Denis