From: "José Expósito" <jose.exposito89@gmail.com>
To: Stefan Berzl <stefanberzl@gmail.com>
Cc: jikos@kernel.org, benjamin.tissoires@redhat.com,
spbnick@gmail.com, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH for-5.19/uclogic] HID: uclogic: Remove useless loop
Date: Mon, 30 May 2022 08:54:52 +0200 [thread overview]
Message-ID: <20220530065452.GA39613@elementary> (raw)
In-Reply-To: <17153eb3-0eb9-cc05-4b65-9c0f4e8d3c90@gmail.com>
Hi Stefan,
Thanks for the patch. You can send it as an standalone patch rather
than as a response to my patches, I don't know if it could be missed by
maintaners this way.
On Sun, May 29, 2022 at 11:49:46PM +0200, Stefan Berzl wrote:
> The while in question does nothing except provide the possibility
> to have an infinite loop in case the subreport id is actually the same
> as the pen id.
>
> Signed-off-by: Stefan Berzl <stefanberzl@gmail.com>
>
> ---
> drivers/hid/hid-uclogic-core.c | 55 ++++++++++++++++------------------
> 1 file changed, 25 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
> index c0fe66e50c58..1a6b941f3964 100644
> --- a/drivers/hid/hid-uclogic-core.c
> +++ b/drivers/hid/hid-uclogic-core.c
> @@ -423,40 +423,35 @@ static int uclogic_raw_event(struct hid_device *hdev,
> if (report->type != HID_INPUT_REPORT)
> return 0;
>
> - while (true) {
> - /* Tweak pen reports, if necessary */
> - if ((report_id == params->pen.id) && (size >= 2)) {
> - subreport_list_end =
> - params->pen.subreport_list +
> - ARRAY_SIZE(params->pen.subreport_list);
> - /* Try to match a subreport */
> - for (subreport = params->pen.subreport_list;
> - subreport < subreport_list_end; subreport++) {
> - if (subreport->value != 0 &&
> - subreport->value == data[1]) {
> - break;
> - }
> - }
> - /* If a subreport matched */
> - if (subreport < subreport_list_end) {
> - /* Change to subreport ID, and restart */
> - report_id = data[0] = subreport->id;
> - continue;
Here, in the previous code, the "report_id" is set to the subreport ID
and the while loop is executed again with the new ID. The loop acts as
a recursive function.
Isn't this behaviour removed by your patch?
Jose
> - } else {
> - return uclogic_raw_event_pen(drvdata, data, size);
> + /* Tweak pen reports, if necessary */
> + if ((report_id == params->pen.id) && (size >= 2)) {
> + subreport_list_end =
> + params->pen.subreport_list +
> + ARRAY_SIZE(params->pen.subreport_list);
> + /* Try to match a subreport */
> + for (subreport = params->pen.subreport_list;
> + subreport < subreport_list_end; subreport++) {
> + if (subreport->value != 0 &&
> + subreport->value == data[1]) {
> + break;
> }
> }
> -
> - /* Tweak frame control reports, if necessary */
> - for (i = 0; i < ARRAY_SIZE(params->frame_list); i++) {
> - if (report_id == params->frame_list[i].id) {
> - return uclogic_raw_event_frame(
> - drvdata, ¶ms->frame_list[i],
> - data, size);
> - }
> + /* If a subreport matched */
> + if (subreport < subreport_list_end) {
> + /* Change to subreport ID, and restart */
> + report_id = data[0] = subreport->id;
> + } else {
> + return uclogic_raw_event_pen(drvdata, data, size);
> }
> + }
>
> - break;
> + /* Tweak frame control reports, if necessary */
> + for (i = 0; i < ARRAY_SIZE(params->frame_list); i++) {
> + if (report_id == params->frame_list[i].id) {
> + return uclogic_raw_event_frame(
> + drvdata, ¶ms->frame_list[i],
> + data, size);
> + }
> }
>
> return 0;
> --
> 2.36.1
>
>
next prev parent reply other threads:[~2022-05-30 6:55 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-18 22:46 [PATCH for-5.19/uclogic v2 0/4] Add support for XP-PEN Deco L José Expósito
2022-05-18 22:46 ` [PATCH for-5.19/uclogic v2 1/4] HID: uclogic: Make template placeholder IDs generic José Expósito
2022-05-18 22:47 ` [PATCH for-5.19/uclogic v2 2/4] HID: uclogic: Add KUnit tests for uclogic_rdesc_template_apply() José Expósito
2022-05-18 22:47 ` [PATCH for-5.19/uclogic v2 3/4] HID: uclogic: Allow to generate frame templates José Expósito
2022-05-29 21:49 ` [PATCH for-5.19/uclogic] HID: uclogic: Remove useless loop Stefan Berzl
2022-05-30 6:54 ` José Expósito [this message]
2022-05-30 16:46 ` Stefan Berzl
2022-05-30 17:33 ` José Expósito
2022-05-18 22:47 ` [PATCH for-5.19/uclogic v2 4/4] HID: uclogic: Add support for XP-PEN Deco L José Expósito
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220530065452.GA39613@elementary \
--to=jose.exposito89@gmail.com \
--cc=benjamin.tissoires@redhat.com \
--cc=jikos@kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=spbnick@gmail.com \
--cc=stefanberzl@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).