On 11/28/2024 12:39 PM, Thomas Gleixner wrote:
> This is just wrong. If the interrupt was torn down, then its state is
> deactivated and it was masked already. So the EOI handling and the
> mask/disable dance are neither required nor make sense.
>
> So this whole thing should be:
>
> chip = irq_desc_get_chip(desc);
> - if (!chip)
> + if (!chip || !irqd_is_started(&desc->irq_data))
> continue;
ACK. Will be done this way in V3.
> But what's worse is that we have 4 almost identical variants of the same code.
>
> So instead of exposing core functionality and "fixing" up four variants, can
> we please have a consolidated version of this function in the core
> code:
> struct irq_chip *chip;
> int check_eoi = 1;
>
> chip = irq_desc_get_chip(desc);
> if (!chip || !irqd_is_started(&desc->irq_data))
> continue;
>
> if (IS_ENABLED(CONFIG_.....)) {
> /*
> * Add a sensible comment which explains this.
> */
> check_eoi = irq_set_irqchip_state(....);
> }
>
> if (check_eoi && ....)
> chip->irq_eoi(&desc->irq_data);
>
> irq_shutdown(desc);
>
> No?
In V3 I will add a preliminary patch that will remove the four variants
and instead add a common implementations to the kexec core.
Thanks, Eliav
