Re: [PATCH v4 1/4] genirq: define an empty function set_handle_irq() if !GENERIC_IRQ_MULTI_HANDLER

2020-09-17 Thread Leizhen (ThunderTown) 



On 2020/9/17 17:32, Marc Zyngier wrote:
> On 2020-09-17 04:46, Leizhen (ThunderTown) wrote:
>> On 2020/9/15 16:43, Zhen Lei wrote:
>>> To avoid compilation error if an irqchip driver references the function
>>> set_handle_irq() but may not select GENERIC_IRQ_MULTI_HANDLER on some
>>> systems.
>>
>> Hi, Marc:
>>   Do you agree with this method?
>>
>>   Otherwise, I should use "#ifdef CONFIG_GENERIC_IRQ_MULTI_HANDLER ... 
>> #endif"
>> to perform the compilation isolation. This may make the code less beautiful.
>>
>>>
>>> For example, the Synopsys DesignWare APB interrupt controller
>>> (dw_apb_ictl) is used as the secondary interrupt controller on arc, csky,
>>> arm64, and most arm32 SoCs, and it's also used as the primary interrupt
>>> controller on Hisilicon SD5203 (an arm32 SoC). The latter need to use
>>> set_handle_irq() to register the top-level IRQ handler, but this multi
>>> irq handler registration mechanism is not implemented on arc system.
>>>
>>> The input parameter "handle_irq" maybe defined as static and only
>>> set_handle_irq() references it. This will trigger "defined but not used"
>>> warning. So add "(void)handle_irq" to suppress it.
>>>
>>> Signed-off-by: Zhen Lei 
>>> ---
>>>  include/linux/irq.h | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>>> index 1b7f4dfee35b397..0848a2aaa9b40b1 100644
>>> --- a/include/linux/irq.h
>>> +++ b/include/linux/irq.h
>>> @@ -1252,6 +1252,8 @@ void irq_matrix_free(struct irq_matrix *m, unsigned 
>>> int cpu,
>>>   * top-level IRQ handler.
>>>   */
>>>  extern void (*handle_arch_irq)(struct pt_regs *) __ro_after_init;
>>> +#else
>>> +#define set_handle_irq(handle_irq)    do { (void)handle_irq; } while (0)
>>>  #endif
>>>
>>>  #endif /* _LINUX_IRQ_H */
>>>
> 
> You shouldn't just make it a NOP. Consider adding a WARN_ON(1), so that
> people can realize this cannot work without the required architecture support.

Oh, right. I will add it.

> 
>     M.

Re: [PATCH v4 1/4] genirq: define an empty function set_handle_irq() if !GENERIC_IRQ_MULTI_HANDLER

2020-09-17 Thread Marc Zyngier 

On 2020-09-17 04:46, Leizhen (ThunderTown) wrote:

On 2020/9/15 16:43, Zhen Lei wrote:
To avoid compilation error if an irqchip driver references the 
function

set_handle_irq() but may not select GENERIC_IRQ_MULTI_HANDLER on some
systems.


Hi, Marc:
  Do you agree with this method?

  Otherwise, I should use "#ifdef CONFIG_GENERIC_IRQ_MULTI_HANDLER ... 
#endif"
to perform the compilation isolation. This may make the code less 
beautiful.




For example, the Synopsys DesignWare APB interrupt controller
(dw_apb_ictl) is used as the secondary interrupt controller on arc, 
csky,
arm64, and most arm32 SoCs, and it's also used as the primary 
interrupt

controller on Hisilicon SD5203 (an arm32 SoC). The latter need to use
set_handle_irq() to register the top-level IRQ handler, but this multi
irq handler registration mechanism is not implemented on arc system.

The input parameter "handle_irq" maybe defined as static and only
set_handle_irq() references it. This will trigger "defined but not 
used"

warning. So add "(void)handle_irq" to suppress it.

Signed-off-by: Zhen Lei 
---
 include/linux/irq.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 1b7f4dfee35b397..0848a2aaa9b40b1 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -1252,6 +1252,8 @@ void irq_matrix_free(struct irq_matrix *m, 
unsigned int cpu,

  * top-level IRQ handler.
  */
 extern void (*handle_arch_irq)(struct pt_regs *) __ro_after_init;
+#else
+#define set_handle_irq(handle_irq) do { (void)handle_irq; } while (0)
 #endif

 #endif /* _LINUX_IRQ_H */



You shouldn't just make it a NOP. Consider adding a WARN_ON(1), so that
people can realize this cannot work without the required architecture 
support.


M.
--
Jazz is not dead. It just smells funny...

Re: [PATCH v4 1/4] genirq: define an empty function set_handle_irq() if !GENERIC_IRQ_MULTI_HANDLER

2020-09-16 Thread Leizhen (ThunderTown) 



On 2020/9/15 16:43, Zhen Lei wrote:
> To avoid compilation error if an irqchip driver references the function
> set_handle_irq() but may not select GENERIC_IRQ_MULTI_HANDLER on some
> systems.

Hi, Marc:
  Do you agree with this method?

  Otherwise, I should use "#ifdef CONFIG_GENERIC_IRQ_MULTI_HANDLER ... #endif"
to perform the compilation isolation. This may make the code less beautiful.

> 
> For example, the Synopsys DesignWare APB interrupt controller
> (dw_apb_ictl) is used as the secondary interrupt controller on arc, csky,
> arm64, and most arm32 SoCs, and it's also used as the primary interrupt
> controller on Hisilicon SD5203 (an arm32 SoC). The latter need to use
> set_handle_irq() to register the top-level IRQ handler, but this multi
> irq handler registration mechanism is not implemented on arc system.
> 
> The input parameter "handle_irq" maybe defined as static and only
> set_handle_irq() references it. This will trigger "defined but not used"
> warning. So add "(void)handle_irq" to suppress it.
> 
> Signed-off-by: Zhen Lei 
> ---
>  include/linux/irq.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 1b7f4dfee35b397..0848a2aaa9b40b1 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -1252,6 +1252,8 @@ void irq_matrix_free(struct irq_matrix *m, unsigned int 
> cpu,
>   * top-level IRQ handler.
>   */
>  extern void (*handle_arch_irq)(struct pt_regs *) __ro_after_init;
> +#else
> +#define set_handle_irq(handle_irq)   do { (void)handle_irq; } while (0)
>  #endif
>  
>  #endif /* _LINUX_IRQ_H */
>

[PATCH v4 1/4] genirq: define an empty function set_handle_irq() if !GENERIC_IRQ_MULTI_HANDLER

2020-09-15 Thread Zhen Lei 
To avoid compilation error if an irqchip driver references the function
set_handle_irq() but may not select GENERIC_IRQ_MULTI_HANDLER on some
systems.

For example, the Synopsys DesignWare APB interrupt controller
(dw_apb_ictl) is used as the secondary interrupt controller on arc, csky,
arm64, and most arm32 SoCs, and it's also used as the primary interrupt
controller on Hisilicon SD5203 (an arm32 SoC). The latter need to use
set_handle_irq() to register the top-level IRQ handler, but this multi
irq handler registration mechanism is not implemented on arc system.

The input parameter "handle_irq" maybe defined as static and only
set_handle_irq() references it. This will trigger "defined but not used"
warning. So add "(void)handle_irq" to suppress it.

Signed-off-by: Zhen Lei 
---
 include/linux/irq.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 1b7f4dfee35b397..0848a2aaa9b40b1 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -1252,6 +1252,8 @@ void irq_matrix_free(struct irq_matrix *m, unsigned int 
cpu,
  * top-level IRQ handler.
  */
 extern void (*handle_arch_irq)(struct pt_regs *) __ro_after_init;
+#else
+#define set_handle_irq(handle_irq) do { (void)handle_irq; } while (0)
 #endif
 
 #endif /* _LINUX_IRQ_H */
-- 
1.8.3