From: Dan Carpenter <dan.carpenter@oracle.com>
To: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Nicolas Saenz Julienne <nsaenz@kernel.org>,
linux-staging@lists.linux.dev
Subject: Re: [PATCH 18/20] staging: vchiq_core: introduce parse_message
Date: Mon, 17 May 2021 14:49:48 +0300 [thread overview]
Message-ID: <20210517114948.GH1955@kadam> (raw)
In-Reply-To: <1621105859-30215-19-git-send-email-stefan.wahren@i2se.com>
On Sat, May 15, 2021 at 09:10:57PM +0200, Stefan Wahren wrote:
> +bail_not_ready:
> + if (service)
> + unlock_service(service);
Not related to this patch, but the unlock_service() function doesn't
having anything to do with locking so it's confusing.
> +
> + return ret;
> +}
> +
> +/* Called by the slot handler thread */
> +static void
> +parse_rx_slots(struct vchiq_state *state)
> +{
> + struct vchiq_shared_state *remote = state->remote;
> + int tx_pos;
> +
> + DEBUG_INITIALISE(state->local)
> +
> + tx_pos = remote->tx_pos;
> +
> + while (state->rx_pos != tx_pos) {
> + struct vchiq_header *header;
> + int size;
> +
> + DEBUG_TRACE(PARSE_LINE);
> + if (!state->rx_data) {
> + int rx_index;
> +
> + WARN_ON(!((state->rx_pos & VCHIQ_SLOT_MASK) == 0));
Also not related but this WARN_ON() has a confusing double negative. In
future patches someone could change this to:
WARN_ON(state->rx_pos & VCHIQ_SLOT_MASK);
Back in the day, I sort of hired someone to create a TODO website so
that if you're reviewing comments you could put a note:
TODO: staging: vchiq_core: tidy up naming
Then the website would make a list of all the TODOs for that driver.
But then I never went live with the website and we introduces
lore.kernel.org and it's almost searchable so hopefully some day it will
be able to do searches like this...
regards,
dan carpenter
next prev parent reply other threads:[~2021-05-17 11:50 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-15 19:10 [PATCH 00/20] staging: vchiq_arm: more code clean-up Stefan Wahren
2021-05-15 19:10 ` [PATCH 01/20] staging: vchiq_core: fix return type of vchiq_init_state Stefan Wahren
2021-05-16 7:27 ` Fabio Aiuto
2021-05-16 9:38 ` Stefan Wahren
2021-05-16 16:54 ` Fabio Aiuto
2021-05-17 11:25 ` Dan Carpenter
2021-05-15 19:10 ` [PATCH 02/20] staging: vchiq_core: drop unnecessary release_count Stefan Wahren
2021-05-15 19:10 ` [PATCH 03/20] staging: vchiq_core: separate postfix increment Stefan Wahren
2021-05-16 7:31 ` Fabio Aiuto
2021-05-15 19:10 ` [PATCH 04/20] staging: vc04_services: remove __VCCOREVER__ Stefan Wahren
2021-05-15 19:10 ` [PATCH 05/20] staging: vchiq_arm: balance braces for if-else statements Stefan Wahren
2021-05-15 19:10 ` [PATCH 06/20] staging: vchiq_core: introduce poll_services_of_group Stefan Wahren
2021-05-15 19:10 ` [PATCH 07/20] staging: vchiq_core: avoid indention in poll_services_of_group Stefan Wahren
2021-05-15 19:10 ` [PATCH 08/20] staging: vchiq_arm: Use define for doorbell irq Stefan Wahren
2021-05-15 19:10 ` [PATCH 09/20] staging: vchiq_arm: drop ftrace-like logging Stefan Wahren
2021-05-15 19:10 ` [PATCH 10/20] staging: vchiq_arm: Prefer kzalloc(sizeof(*waiter)...) Stefan Wahren
2021-05-15 19:10 ` [PATCH 11/20] staging: vchiq_arm: drop non-beneficial comments Stefan Wahren
2021-05-15 19:10 ` [PATCH 12/20] staging: vchiq_arm: add blank line after declarations Stefan Wahren
2021-05-15 19:10 ` [PATCH 13/20] staging: vchiq_arm: re-arrange function header Stefan Wahren
2021-05-15 19:10 ` [PATCH 14/20] staging: vchiq_core: reduce indention in release_service_messages Stefan Wahren
2021-05-15 19:10 ` [PATCH 15/20] staging: vchiq_core: fix comment in vchiq_shutdown_internal Stefan Wahren
2021-05-15 19:10 ` [PATCH 16/20] staging: vchiq_arm: make vchiq_shutdown_internal return void Stefan Wahren
2021-05-15 19:10 ` [PATCH 17/20] staging: vchiq_arm: Avoid unnecessary line breaks Stefan Wahren
2021-05-15 19:10 ` [PATCH 18/20] staging: vchiq_core: introduce parse_message Stefan Wahren
2021-05-17 11:49 ` Dan Carpenter [this message]
2021-05-17 17:38 ` Stefan Wahren
2021-05-18 7:36 ` Dan Carpenter
2021-05-15 19:10 ` [PATCH 19/20] staging: vchiq_core: introduce defines for close_recvd Stefan Wahren
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=20210517114948.GH1955@kadam \
--to=dan.carpenter@oracle.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-staging@lists.linux.dev \
--cc=nsaenz@kernel.org \
--cc=stefan.wahren@i2se.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 read-only IMAP folder(s) and NNTP newsgroup(s).