Re: [PATCH 1/1] riscv-aplic: manually set pending for the pre-existing interrupts

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, Aug 08 2024 at 18:56, Anup Patel wrote:
> More appropriate patch subject should be:
>
> irqchip: riscv-aplic: retrigger interrupt in MSI mode upon write to
> sourcecfg register

And the correct one would be:

    irqchip/riscv-aplic: Retrigger MSI interrupt on source configuration

1) The prefix is correct

2) Sentence starts with a uppercase letter

3) It uses understandable words. sourcecfg is a implementation detail
   which is irrelevant for the high level overview of a changelog
   subject, which is visible in the short log.

> On Thu, Aug 8, 2024 at 1:44 PM Yong-Xuan Wang <yongxuan.wang@sifive.com> wrote:
>>
>> The section 4.5.2 of the RISC-V AIA specification says that any write
>> to a sourcecfg register of an APLIC might (or might not) cause the
>> corresponding interrupt-pending bit to be set to one if the rectified
>> input value is high (= 1) under the new source mode.
>
> Add quotes around the text from RISC-V AIA specification.
>
>>
>> If an interrupt is asserted before the driver configs its interrupt
>> type to APLIC, it's pending bit will not be set except a relevant
>> write to a setip or setipnum register. When we write the interrupt
>> type to sourcecfg register, if the APLIC device doesn't check and
>> update the pending bit, the interrupt might never becomes pending.
>
> The second sentence above can be re-written as follows:
>
> When interrupt type is changed in sourcecfg register, the APLIC

the interrupt type ... in the source....

> device might not set the corresponding pending bit, the interrupt

bit , so the ...

> might never become pending.

>
> Define APLIC MSI mode specific irq_set_type() like below:
>
> int aplic_msi_irq_set_type(struct irq_data *d, unsigned int type)

static :)

> {
>        int rc;
>
>        rc = aplic_irq_set_type(d, type);

         int rc = aplic...

>        if (rc)
>               return rc;
>
>        /*
>         * Updating sourcecfg register for level-triggered interrupts
>         * requires interrupt retriggering when APLIC in MSI mode.

APLIC is in ....

>         */
>        aplic_msi_irq_retrigger_level(d);

Thanks,

        tglx