On Wed, 4 Jul 2018, Guo Ren wrote:

On Tue, Jul 03, 2018 at 11:28:03AM +0200, Thomas Gleixner wrote:
That's Ok. And here is my implementation:

static inline u32 build_intc_ctrl(u32 idx)
{
	/*
	 * One channel is one byte in a word-width register, so
	 * there are four channels in a word-width register.
	 *
	 * Set the same index for each channel and it will make
	 * "irq num = channel num".
	 */
	return (idx             | ((idx + 1) << 8) |
	      ((idx + 2) << 16) | ((idx + 3) << 24));
}
Hmm? (No magic number)

Ok :)

quoted

quoted

+	static void __iomem	*reg_base;
+	irq_hw_number_t		hwirq;
+
+	reg_base = *this_cpu_ptr(&intcl_reg);

Wheee!

	static void __iomem *reg_base = this_cpu_read(intcl_reg);
	irq_hw_number_t	hwirq;

Hmm?

Thx for the tips and I'll use this_cpu_read() without static.
 	void __iomem *reg_base = this_cpu_read(intcl_reg);

Yeah, I surely missed to drop the static ...

quoted

Aside of that the whole thing might share the code with the other one, but
it might not be worth it. At least this wants to be documented in the
changelog why sharing the code is not useful...

Do you mean merge irq-csky-v1.c irq-csky-v2.c irq-nationalchip.c into
one file eg: irq-csky.c? 

Yes, but only if there is enough code to share without creating an ifdef
mess. But that looks doable

Thanks,

	tglx