| Message ID | 35556cf3af8ce161ab6e178bdf4615ee94407bc6.1739553712.git.gerickson@nuovations.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add QMI Device Service Request Rate-limit Option | expand |
Hi Grant, On 2/14/25 11:22 AM, Grant Erickson wrote: > Determine if we need to rate-limit QMI services requests to a > transport-specific minimum request period. If so, return true so that > the queue can be retried again later. > > Adds the following data members to 'qmi_transport': > > * Minimum request period > - The minimum period, in microseconds, for back-to-back QMI > service requests. > > * Last request time > - Time, in microseconds, when the last QMI service request was > sent. > > to support the implementation. > --- > drivers/qmimodem/qmi.c | 35 +++++++++++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > The CI doesn't like this version either: Alpine (musl) gcc optimized =========================== Configure: PASS Build: FAIL drivers/qmimodem/qmi.c: In function 'can_write_data': drivers/qmimodem/qmi.c:701:50: error: 'now' may be used uninitialized [-Werror=maybe-uninitialized] 701 | transport->last_req_sent_time_us = now; | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~ drivers/qmimodem/qmi.c:669:18: note: 'now' was declared here 669 | uint64_t now; | ^~~ cc1: all warnings being treated as errors make[1]: *** [Makefile:4090: drivers/qmimodem/qmi.o] Error 1 make[1]: Target 'all-am' not remade because of errors. make: *** [Makefile:2405: all] Error 2 Regards, -Denis
On Feb 14, 2025, at 1:08 PM, Denis Kenzior <denkenz@gmail.com> wrote: > > > Hi Grant, > > On 2/14/25 11:22 AM, Grant Erickson wrote: >> Determine if we need to rate-limit QMI services requests to a >> transport-specific minimum request period. If so, return true so that >> the queue can be retried again later. >> Adds the following data members to 'qmi_transport': >> * Minimum request period >> - The minimum period, in microseconds, for back-to-back QMI >> service requests. >> * Last request time >> - Time, in microseconds, when the last QMI service request was >> sent. >> to support the implementation. >> --- >> drivers/qmimodem/qmi.c | 35 +++++++++++++++++++++++++++++++++++ >> 1 file changed, 35 insertions(+) > > The CI doesn't like this version either: > > Alpine (musl) gcc optimized > =========================== > Configure: PASS > Build: FAIL > drivers/qmimodem/qmi.c: In function 'can_write_data': > drivers/qmimodem/qmi.c:701:50: error: 'now' may be used uninitialized [-Werror=maybe-uninitialized] > 701 | transport->last_req_sent_time_us = now; > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~ > drivers/qmimodem/qmi.c:669:18: note: 'now' was declared here > 669 | uint64_t now; > | ^~~ > cc1: all warnings being treated as errors > make[1]: *** [Makefile:4090: drivers/qmimodem/qmi.o] Error 1 > make[1]: Target 'all-am' not remade because of errors. > make: *** [Makefile:2405: all] Error 2 > > Regards, > -Denis Denis, Is there a place, in-tree, where the CI process and configuration are coded / documented? I’d like to be able to produce the same results which: % ./bootstrap % mkdir build-clang % mkdir build-gcc % cd build-clang % CC=clang ../configure -C % make -j`nproc` % cd ../build-gcc % CC=gcc ./configure -C % make -j`nproc` does not seem to replicate. Best, Grant
diff --git a/drivers/qmimodem/qmi.c b/drivers/qmimodem/qmi.c index bbe6440abe0f..7b20dc3f85dd 100644 --- a/drivers/qmimodem/qmi.c +++ b/drivers/qmimodem/qmi.c @@ -92,6 +92,18 @@ struct qmi_transport { const struct qmi_transport_ops *ops; struct debug_data debug; bool writer_active : 1; + + /** + * If specified via qmi_qmux_device_options, the minimum period + * for back-to-back QMI service requests. + */ + unsigned int min_req_period_us; + + /** + * The time, in microseconds, when the last QMI service request + * was sent. + */ + uint64_t last_req_sent_time_us; }; struct qmi_qmux_device { @@ -653,8 +665,28 @@ 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; 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) { + now = l_time_now(); + + if (transport->last_req_sent_time_us != 0) { + const uint64_t 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 +697,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; + if (l_queue_length(transport->req_queue) > 0) return true;
Determine if we need to rate-limit QMI services requests to a transport-specific minimum request period. If so, return true so that the queue can be retried again later. Adds the following data members to 'qmi_transport': * Minimum request period - The minimum period, in microseconds, for back-to-back QMI service requests. * Last request time - Time, in microseconds, when the last QMI service request was sent. to support the implementation. --- drivers/qmimodem/qmi.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)