From: Julia Lawall <julia.lawall@inria.fr>
To: Dorine Tipo <dorine.a.tipo@gmail.com>
Cc: outreachy@lists.linux.dev, gregkh@linuxfoundation.org
Subject: Re: [PATCH] Subject: staging: octeon: remove unnecessary parentheses in interface checks
Date: Sun, 29 Oct 2023 17:00:04 +0100 (CET) [thread overview]
Message-ID: <alpine.DEB.2.22.394.2310291657330.3136@hadrien> (raw)
In-Reply-To: <20231029154253.6846-1-dorine.a.tipo@gmail.com>
This is starting to look better, but there is still an extra occurrence of
"Subject:" above.
The subject line also probebly contains too much information. "in
interface checks" seems like a detail. From seeing the subject line only,
probably no one will know what is being referred to. But when looking at
the patch, it is completely obvious.
On Sun, 29 Oct 2023, Dorine Tipo wrote:
> The unnecessary parentheses around 'interface < 2' have been removed.
It would be helpful to explain why the parentheses are unnecessary. This
part of the message should also describe what has been done using the
imperative.
> Signed-off-by: Dorine Tipo <dorine.a.tipo@gmail.com>
The signed off by looks good now.
julia
> ---
> drivers/staging/octeon/ethernet.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
> index 8e1f4b987a25..349308edbc9e 100644
> --- a/drivers/staging/octeon/ethernet.c
> +++ b/drivers/staging/octeon/ethernet.c
> @@ -248,7 +248,7 @@ static int cvm_oct_common_change_mtu(struct net_device *dev, int new_mtu)
>
> dev->mtu = new_mtu;
>
> - if ((interface < 2) &&
> + if (interface < 2 &&
> (cvmx_helper_interface_get_mode(interface) !=
> CVMX_HELPER_INTERFACE_MODE_SPI)) {
> int index = INDEX(priv->port);
> @@ -294,7 +294,7 @@ static void cvm_oct_common_set_multicast_list(struct net_device *dev)
> struct octeon_ethernet *priv = netdev_priv(dev);
> int interface = INTERFACE(priv->port);
>
> - if ((interface < 2) &&
> + if (interface < 2 &&
> (cvmx_helper_interface_get_mode(interface) !=
> CVMX_HELPER_INTERFACE_MODE_SPI)) {
> union cvmx_gmxx_rxx_adr_ctl control;
> @@ -346,7 +346,7 @@ static int cvm_oct_set_mac_filter(struct net_device *dev)
> union cvmx_gmxx_prtx_cfg gmx_cfg;
> int interface = INTERFACE(priv->port);
>
> - if ((interface < 2) &&
> + if (interface < 2 &&
> (cvmx_helper_interface_get_mode(interface) !=
> CVMX_HELPER_INTERFACE_MODE_SPI)) {
> int i;
> --
> 2.25.1
>
>
prev parent reply other threads:[~2023-10-29 16:00 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-29 15:42 [PATCH] Subject: staging: octeon: remove unnecessary parentheses in interface checks Dorine Tipo
2023-10-29 16:00 ` Julia Lawall [this message]
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=alpine.DEB.2.22.394.2310291657330.3136@hadrien \
--to=julia.lawall@inria.fr \
--cc=dorine.a.tipo@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=outreachy@lists.linux.dev \
/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).