Re: [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support

[Jianmin Lv]

On 2022/6/7 下午2:56, Marc Zyngier wrote:
> On Tue, 07 Jun 2022 04:41:22 +0100,
> Jianmin Lv <lvjianmin@xxxxxxxxxxx> wrote:
> > On 2022/6/6 下午6:02, Marc Zyngier wrote:
> > > + Lorenzo and Hanjun who maintain the ACPI irq code
> > >
> > > On Thu, 02 Jun 2022 04:16:30 +0100,
> > > Jianmin Lv <lvjianmin@xxxxxxxxxxx> wrote:
>
> [...]
>
> > > > I'm so sorry, I really don't find a way to reuse driver/acpi/irq.c
> > > > after my humble work.
> > > I don't think reimplementing ACPI is the solution. What could be a
> > > reasonable approach is a way to overload the retrieval of the
> > > acpi_gsi_domain_id fwnode with a GSI parameter.
> > >
> > > I hacked the following patch, which will give you an idea of what I
> > > have in mind (only compile-tested).
> >
> >
> > Hi, Marc, thanks so much for your patch. I have verified it on my
> > LoongArch machine and it works well.
> >
> >
> > BTW, in acpi_get_irq_source_fwhandle(), maybe
> > acpi_get_gsi_domain_id(ctx->index) is needed to changed to
> > acpi_get_gsi_domain_id(irq->interrupts[ctx->index])?
>
> Yes, absolutely. Thanks for spotting it.
>
> > I have another question, for LoongArch, acpi_isa_irq_to_gsi is
> > required to implemente, but no common version, do we need to
> > implemente an weak version in driver/acpi/irq.c as following?
> >
> >
> > int __weak acpi_isa_irq_to_gsi(unsigned int isa_irq, u32 *gsi)
> > {
> >          if (gsi)
> >                  *gsi = isa_irq;
> >          return 0;
> > }
>
> Do you actually have CONFIG_ISA? In 2022? For a brand new architecture?
>
> If you really have to, then this needs to be a bit more involved:
>
> #ifdef CONFIG_ISA
> int __weak acpi_isa_irq_to_gsi(unsigned int isa_irq, u32 *gsi)
> {
> 	if (irq < nr_legacy_irqs()) {
> 		*gsi = isa_irq;
> 		return 0;
> 	}
>
> 	return -1;
> }
> #endif
>
> But I'd rather you get rid of any such legacy if this can be avoided.

Thanks for your suggestion, I have confirmed that we don't need 
CONFIT_ISA for LoongArch, so acpi_isa_irq_to_gsi is not needed either.

> > I'll use the way you provided here to reuse driver/acpi/irq.c in next
> > version. How should I do next? Should I integrate your patch into my
> > next version or wait for you to merge it first?
>
> Please pick up the patch (with the above fix), and use it as a prefix
> to your series. It needs to be reviewed by the relevant maintainers
> anyway.

Ok, thanks, I'll add relevant maintainers to review for next version.

> Thanks,
>
> 	M.