Re: [PATCH] <driver/net/ethernet/amd/nmclan_cs.c> Remove unnecessary line

[Bagas Sanjaya <bagasdotme@gmail.com>]

[-- 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 --]