linux-kernel - Re: [tip: irq/core] irqchip/armada-370-xp: Change symbol prefixes to mpic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <z3vgu5vimj5wkcvlxs27y5s7q3epvmtgrtb7zovdfojpregcye@pqdxzesipb36>
Date: Tue, 30 Jul 2024 09:56:04 +0200
From: Marek Behún <kabel@...nel.org>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: linux-kernel@...r.kernel.org, linux-tip-commits@...r.kernel.org, 
	kabel@...nel.org
Subject: Re: [tip: irq/core] irqchip/armada-370-xp: Change symbol prefixes to
 mpic

Hi Thomas,

so I am going through the differences of the commits that you applied
in contrast to the originals that I sent, as you asked.

This commit is rather large, so understandably there was a larger
probability for a mistake. It seems ok, but I have the notes:

>  	if (WARN_ON(!base_ipi))
>  		return;
> -
>  	set_smp_ipi_range(base_ipi, IPI_DOORBELL_END);
>  }

^^ I did not remove this line in my original patch, I always try to
leave an empty line after conditional return:
  if (cond)
    return;
  // empty line
  code;

I guess this is not an issue, though.

> @@ -704,62 +687,60 @@ armada_370_xp_handle_irq(struct pt_regs *regs)
>  			int ipi;
>  
>  			ipimask = readl_relaxed(per_cpu_int_base + MPIC_IN_DRBEL_CAUSE) &
> -						IPI_DOORBELL_MASK;
> +				  IPI_DOORBELL_MASK;
>  

^^ This change also was not in my original patch. I would have put this
into the "improve indentation" commit, instead of the one that "changes
symbol prefixes to mpic_", since semantically it belongs into the first
one.

IMO it would have been better to leave these patches as they are, and
do the change to 100 columns in a subsequent patch, all in one, since
there are many other parts of the code that would have benefited from
it. And you would not need to spend time going through the patches line
by line :-)

Anyway, this patch seems ok.

Marek