[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <hjummqbdurgohpv2kvrnr23abirami642vrpvs4j272yhw5hna@k3rqwvzax5no>
Date: Fri, 8 Aug 2025 07:18:48 +0800
From: Inochi Amaoto <inochiama@...il.com>
To: Bjorn Helgaas <helgaas@...nel.org>,
Inochi Amaoto <inochiama@...il.com>
Cc: Thomas Gleixner <tglx@...utronix.de>,
Bjorn Helgaas <bhelgaas@...gle.com>, Marc Zyngier <maz@...nel.org>,
Lorenzo Pieralisi <lpieralisi@...nel.org>, Saurabh Sengar <ssengar@...ux.microsoft.com>,
Shradha Gupta <shradhagupta@...ux.microsoft.com>, Jonathan Cameron <Jonathan.Cameron@...ei.com>,
Nicolin Chen <nicolinc@...dia.com>, Jason Gunthorpe <jgg@...pe.ca>,
Chen Wang <unicorn_wang@...look.com>, linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
Yixun Lan <dlan@...too.org>, Longbin Li <looong.bin@...il.com>
Subject: Re: [PATCH 2/4] PCI/MSI: Add startup/shutdown support for per device
MSI[X] domains
On Thu, Aug 07, 2025 at 11:25:21AM -0500, Bjorn Helgaas wrote:
> In subject, s/MSI[X]// and s/support for/for/
>
> The "MSI[X]" notation really isn't used anywhere else, and we already
> include "PCI/MSI" in the prefix, so I don't think we need it again.
>
> On Thu, Aug 07, 2025 at 07:23:23PM +0800, Inochi Amaoto wrote:
> > As The RISC-V PLIC can not apply affinity setting without calling
> > irq_enable(), it will make the interrupt unavaible when using as
> > an underlying irq chip for MSI controller.
>
> s/As The/As the/
> s/unavaible/unavailable/
> s/irq chip/IRQ chip/
>
These are good for me. I will take it.
> > Introduce the irq_startup/irq_shutdown for PCI domain template with
> > new MSI domain flag. This allow the PLIC can be properly configurated
> > when calling irq_startup().
>
> Maybe something like:
>
> Implement .irq_startup() and .irq_shutdown() for the PCI MSI and
> MSI-X templates. For chips that specify MSI_FLAG_PCI_MSI_STARTUP_PARENT,
> these startup and shutdown the parent as well, which allows ...
>
> s/This allow/This allows/
> s/can be properly configurated/to be configured/
>
Thanks, I will update my commit log.
> Evidently PLIC depends on this "parent" connection, but that isn't
> explained at all in the commit log.
>
When call irq_startup, the PLIC is called irq_enable() instead of
irq_unmask(), the the irq on PLIC can be enabled.
I will add this to the commit log.
> > Suggested-by: Thomas Gleixner <tglx@...utronix.de>
> > Signed-off-by: Inochi Amaoto <inochiama@...il.com>
> > ---
> > drivers/pci/msi/irqdomain.c | 52 +++++++++++++++++++++++++++++++++++++
> > include/linux/msi.h | 2 ++
> > 2 files changed, 54 insertions(+)
> >
> > diff --git a/drivers/pci/msi/irqdomain.c b/drivers/pci/msi/irqdomain.c
> > index 0938ef7ebabf..f0d18cadbe20 100644
> > --- a/drivers/pci/msi/irqdomain.c
> > +++ b/drivers/pci/msi/irqdomain.c
> > @@ -148,6 +148,23 @@ static void pci_device_domain_set_desc(msi_alloc_info_t *arg, struct msi_desc *d
> > arg->hwirq = desc->msi_index;
> > }
> >
> > +static __always_inline void cond_shutdown_parent(struct irq_data *data)
>
> Is there a functional reason why we need __always_inline?
>
I am not sure for this. As I found other cond_[mask/unmask]_parent()
also have this attribute, I added this as well.
> If not, it seems like this annotation is just clutter, and the compiler
> will probably inline it all by itself.
>
I will see if someone know the reason. If there is no other objection,
I will remove this in the next version
Regards,
Inochi
Powered by blists - more mailing lists