| Message ID | 20220706074630.829607-1-claudiu.beznea@microchip.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] irqchip/atmel-aic: remove #ifdef CONFIG_PM | expand |
Claudiu, If you send more than a single patch, please add a cover letter. On Wed, 06 Jul 2022 08:46:29 +0100, Claudiu Beznea <claudiu.beznea@microchip.com> wrote: > > Remove #ifdef CONFIG_PM around aic_suspend() function. Coding style > recommends (at chapter Conditional Compilation) to avoid using > preprocessor conditional in .c files. > gc->chip_types->chip.irq_suspend()/gc->chip_types->chip.irq_resume() is > called in irq_gc_suspend()/irq_gc_resume() which is NULL in case CONFIG_PM > is not defined. With this gc->chip_types->chip.irq_pm_shutdown is > populated all the time as it should be as irq_gc_shutdown() is not > conditioned by CONFIG_PM. By your very own investigations, aic_suspend() and co are utterly useless when !PM. And yet you want to *preserve* them, despite being dead code? What purpose does it serve (other than some blind compliance to a rule)? M.
Hi, Marc, On 11.07.2022 11:49, Marc Zyngier wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Claudiu, > > If you send more than a single patch, please add a cover letter. OK, I'll kept it in mind. > > On Wed, 06 Jul 2022 08:46:29 +0100, > Claudiu Beznea <claudiu.beznea@microchip.com> wrote: >> >> Remove #ifdef CONFIG_PM around aic_suspend() function. Coding style >> recommends (at chapter Conditional Compilation) to avoid using >> preprocessor conditional in .c files. >> gc->chip_types->chip.irq_suspend()/gc->chip_types->chip.irq_resume() is >> called in irq_gc_suspend()/irq_gc_resume() which is NULL in case CONFIG_PM >> is not defined. With this gc->chip_types->chip.irq_pm_shutdown is >> populated all the time as it should be as irq_gc_shutdown() is not >> conditioned by CONFIG_PM. > > By your very own investigations, aic_suspend() and co are utterly > useless when !PM. And yet you want to *preserve* them, despite being > dead code? What purpose does it serve (other than some blind > compliance to a rule)? Only compliance with the mentioned rule. Thank you, Claudiu Beznea > > M. > > -- > Without deviation from the norm, progress is not possible.
diff --git a/drivers/irqchip/irq-atmel-aic.c b/drivers/irqchip/irq-atmel-aic.c index 4631f6847953..02a9f45a7d2e 100644 --- a/drivers/irqchip/irq-atmel-aic.c +++ b/drivers/irqchip/irq-atmel-aic.c @@ -102,7 +102,6 @@ static int aic_set_type(struct irq_data *d, unsigned type) return 0; } -#ifdef CONFIG_PM static void aic_suspend(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); @@ -132,11 +131,6 @@ static void aic_pm_shutdown(struct irq_data *d) irq_reg_writel(gc, 0xffffffff, AT91_AIC_ICCR); irq_gc_unlock(gc); } -#else -#define aic_suspend NULL -#define aic_resume NULL -#define aic_pm_shutdown NULL -#endif /* CONFIG_PM */ static void __init aic_hw_init(struct irq_domain *domain) {
Remove #ifdef CONFIG_PM around aic_suspend() function. Coding style recommends (at chapter Conditional Compilation) to avoid using preprocessor conditional in .c files. gc->chip_types->chip.irq_suspend()/gc->chip_types->chip.irq_resume() is called in irq_gc_suspend()/irq_gc_resume() which is NULL in case CONFIG_PM is not defined. With this gc->chip_types->chip.irq_pm_shutdown is populated all the time as it should be as irq_gc_shutdown() is not conditioned by CONFIG_PM. Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> --- drivers/irqchip/irq-atmel-aic.c | 6 ------ 1 file changed, 6 deletions(-)