| Message ID | 92dd49e261569657414c1e43825cbe8928ace900.1739339173.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/11/25 11:52 PM, Grant Erickson wrote: > Defines options that may alter the default behavior of the QMI driver > for a specific, instantiated device. > --- > drivers/qmimodem/qmi.h | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/drivers/qmimodem/qmi.h b/drivers/qmimodem/qmi.h > index 913035897ea4..3f60e90c015a 100644 > --- a/drivers/qmimodem/qmi.h > +++ b/drivers/qmimodem/qmi.h > @@ -76,6 +76,25 @@ enum qmi_qmux_device_quirk { > QMI_QMUX_DEVICE_QUIRK_REQ_RATE_LIMIT = 0x01 > }; > > +/** > + * Defines options that may alter the default behavior of the QMI > + * driver for a specific, instantiated device. > + */ > +struct qmi_qmux_device_options { > + /** > + * Device-specific "quirk". > + */ > + enum qmi_qmux_device_quirk quirks; > + > + /** > + * If quirks has #QMI_QMUX_DEVICE_QUIRK_REQ_RATE_LIMIT set, this > + * is the minimum period, in microseconds, in which back-to-back > + * QMI service requests may be sent to avoid triggering a > + * firmware lock up and hang. > + */ > + unsigned int min_req_period_us; Why do you need both? Isn't setting min_req_period_us non-zero enough? Also, strictly speaking this may also apply to qrtr devices. Maybe just add bool qmi_qmux_device_set_request_throttle(device, us_time) ...? > +}; > + > typedef void (*qmi_destroy_func_t)(void *user_data); > > struct qmi_service; Regards, -Denis
On Feb 12, 2025, at 11:21 AM, Denis Kenzior <denkenz@gmail.com> wrote: > On 2/11/25 11:52 PM, Grant Erickson wrote: >> Defines options that may alter the default behavior of the QMI driver >> for a specific, instantiated device. >> --- >> drivers/qmimodem/qmi.h | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> diff --git a/drivers/qmimodem/qmi.h b/drivers/qmimodem/qmi.h >> index 913035897ea4..3f60e90c015a 100644 >> --- a/drivers/qmimodem/qmi.h >> +++ b/drivers/qmimodem/qmi.h >> @@ -76,6 +76,25 @@ enum qmi_qmux_device_quirk { >> QMI_QMUX_DEVICE_QUIRK_REQ_RATE_LIMIT = 0x01 >> }; >> +/** >> + * Defines options that may alter the default behavior of the QMI >> + * driver for a specific, instantiated device. >> + */ >> +struct qmi_qmux_device_options { >> + /** >> + * Device-specific "quirk". >> + */ >> + enum qmi_qmux_device_quirk quirks; >> + >> + /** >> + * If quirks has #QMI_QMUX_DEVICE_QUIRK_REQ_RATE_LIMIT set, this >> + * is the minimum period, in microseconds, in which back-to-back >> + * QMI service requests may be sent to avoid triggering a >> + * firmware lock up and hang. >> + */ >> + unsigned int min_req_period_us; > > Why do you need both? Isn't setting min_req_period_us non-zero enough? Also, strictly speaking this may also apply to qrtr devices. Maybe just add > bool qmi_qmux_device_set_request_throttle(device, us_time) ...? Denis, I had originally started with a hard-coded 1,000 us value and the quirk flag just determined whether that value and functionality was used or not (in effect, making it nullable). However, after realizing during further testing that the 1,000 us value didn’t work past 60 back-to-back tests and needed to be bumped to 2,000 us, I added the ability to parameterize the value at the driver level and figured that quirk flags were valuable enough a construct to stay as-is where future quirk flags might be added. More abstractly, I like that the quick flag makes the value explicitly nullable. I tend to not otherwise like “magic” sentinel values (in this case, zero). Best, Grant
diff --git a/drivers/qmimodem/qmi.h b/drivers/qmimodem/qmi.h index 913035897ea4..3f60e90c015a 100644 --- a/drivers/qmimodem/qmi.h +++ b/drivers/qmimodem/qmi.h @@ -76,6 +76,25 @@ enum qmi_qmux_device_quirk { QMI_QMUX_DEVICE_QUIRK_REQ_RATE_LIMIT = 0x01 }; +/** + * Defines options that may alter the default behavior of the QMI + * driver for a specific, instantiated device. + */ +struct qmi_qmux_device_options { + /** + * Device-specific "quirk". + */ + enum qmi_qmux_device_quirk quirks; + + /** + * If quirks has #QMI_QMUX_DEVICE_QUIRK_REQ_RATE_LIMIT set, this + * is the minimum period, in microseconds, in which back-to-back + * QMI service requests may be sent to avoid triggering a + * firmware lock up and hang. + */ + unsigned int min_req_period_us; +}; + typedef void (*qmi_destroy_func_t)(void *user_data); struct qmi_service;