* Re: [PATCH] <driver/net/ethernet/amd/nmclan_cs.c> Remove unnecessary line
2022-12-10 8:47 [PATCH] <driver/net/ethernet/amd/nmclan_cs.c> Remove unnecessary line Takahashi Akari
@ 2022-12-10 13:18 ` Bagas Sanjaya
0 siblings, 0 replies; 2+ messages in thread
From: Bagas Sanjaya @ 2022-12-10 13:18 UTC (permalink / raw)
To: Takahashi Akari, linux-kernel, Networking
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Haoyue Xu, Guofeng Yue, Geoff Levand, Krzysztof Hałasa,
Wolfram Sang
[-- Attachment #1: Type: text/plain, Size: 2801 bytes --]
On Sat, Dec 10, 2022 at 05:47:13PM +0900, Takahashi Akari wrote:
> Hello:
>
> I sent a patch. Please review and merge.
>
> Reason:
> Remove unnecessaty line. (if statement is always true)
>
> Git repository URL:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>
> File:
> drivers/net/ethernet/amd/nmclan_cs.c : 931
>
Hi and welcome to LKML!
Based on your patch, here are my notes:
* Write the patch description in imperative mood (e.g. it should have
been "Remove redundant inner tx_irq_disabled conditional in
mace_interrupt() since it has already been handled". Wrap it in about
72-75 column wide (to account indentation in git-log(1) so that the
total line length is at maximum 80).
* Write also short but concise patch subject (one-line summary), while also
paying attention to the subject prefix. In this
case, it should have been "net: amd: remove inner tx_irq_disabled
conditional".
* Your patch looks like corrupted (tabs converted to spaces, linewrapped,
etc.). Please configure your email-client not to do such things, or better
yet, use git-send-email(1) to send patches.
* You need to Cc: relevant maintainers and lists, which can be obtained
by `scripts/get_maintainer.pl -norolestats -separator , --
/path/to/your/patch`. Personally I put lists in To: header and individual
addresses in Cc:. I have added them for you.
* Last but not least, build the kernel with your patch applied (preferably
enable W=1 and CONFIG_WERROR and also cross-compile).
See also tips from one of kernel subsystem maintainer at [1].
> Signed-off-by: Takahashi Akari <akaritakahashioss@gmail.com>
> ---
> drivers/net/ethernet/amd/nmclan_cs.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/amd/nmclan_cs.c b/drivers/net/ethernet/amd/nmclan_cs.c
> index 823a329a921f..a90e0c5b603d 100644
> --- a/drivers/net/ethernet/amd/nmclan_cs.c
> +++ b/drivers/net/ethernet/amd/nmclan_cs.c
> @@ -928,10 +928,7 @@ static irqreturn_t mace_interrupt(int irq, void *dev_id)
>
> if (lp->tx_irq_disabled) {
> const char *msg;
> - if (lp->tx_irq_disabled)
> - msg = "Interrupt with tx_irq_disabled";
> - else
> - msg = "Re-entering the interrupt handler";
> + msg = "Interrupt with tx_irq_disabled";
> netdev_notice(dev, "%s [isr=%02X, imr=%02X]\n",
> msg,
> inb(ioaddr + AM2150_MACE_BASE + MACE_IR),
Dunno if you need to also handle the else case (that gives "Re-entering" msg)
in the outer conditional.
Thanks.
[1]: https://lore.kernel.org/all/20171026223701.GA25649@bhelgaas-glaptop.roam.corp.google.com/
--
An old man doll... just what I always wanted! - Clara
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread