On 2023/12/13 17:50, Lorenzo Pieralisi wrote: > On Wed, Dec 13, 2023 at 04:15:39PM +0800, Minda Chen wrote: >> >> >> On 2023/12/12 19:19, Lorenzo Pieralisi wrote: >> > On Wed, Dec 06, 2023 at 06:58:33PM +0800, Minda Chen wrote: >> >> For lack of an MSI controller, The new added PCIe interrupts have to be >> >> added to global interrupt event field. PolarFire event domain ops can not >> >> be re-used. >> > >> > I don't understand what this means, please explain and I will >> > add it to the commit log. >> > >> Sorry. >> Microchip Polarfire PCIe adds 11 PCIe interrupts to PCIe global event domain.(Total 28 PCIe interrupts) >> The microchip event domain and event irqchip will handle these interrupts. >> But PLDA host contain 13 fixed PCIe interrupts. PLDA codes just process these >> 13 interrupts. Microchip the event irq codes are quite different and can't be used by PLDA codes. >> So add an event domain field support microchip and other vendor who just using the PLDA interrupts. >> >> PLDA event domain ops instances will be implemented in later patch. >> > >> > Future patches don't exist, each commit log is a logical change >> > that must make sense on its own, I will remove this sentence. >> > >> > Lorenzo >> >> OK, Thanks. >> >> >> Signed-off-by: Minda Chen <minda.chen@xxxxxxxxxxxxxxxx> >> >> Acked-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> > > I am sorry folks but I still don't get what this patch is supposed > to do. To start with it looks like a preparation patch (it does > not have any net effect), that's not OK since it has to be merged > with the patches that actually apply significant changes on top > of this. > > Then I need an explanation of what the problem is and what is the > code actually changing in irqdomain/irqchip specific terms. > > I can't merge it as-is because I don't understand what it changes > and it is not clear by reading the commit log. > > Thanks, > Lorenzo > Actually I am not explain this clear. Sorry. I will changed this patch in next version and add more commit messages. The added parameter in struct plda_event is event IRQ chip instead of event IRQ domain ops. Actually the different configuration is irqchip. And I will add PLDA event irqchip and its functions to this patch, which can be compared with microchip's event irqchip functions. >> >> --- >> >> drivers/pci/controller/plda/pcie-microchip-host.c | 6 ++++-- >> >> drivers/pci/controller/plda/pcie-plda.h | 1 + >> >> 2 files changed, 5 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/drivers/pci/controller/plda/pcie-microchip-host.c b/drivers/pci/controller/plda/pcie-microchip-host.c >> >> index f5e7da242aec..e6dcc572b65b 100644 >> >> --- a/drivers/pci/controller/plda/pcie-microchip-host.c >> >> +++ b/drivers/pci/controller/plda/pcie-microchip-host.c >> >> @@ -821,13 +821,15 @@ static const struct plda_event_ops mc_event_ops = { >> >> }; >> >> >> >> static const struct plda_event mc_event = { >> >> + .domain_ops = &mc_event_domain_ops, >> >> .event_ops = &mc_event_ops, >> >> .request_event_irq = mc_request_event_irq, >> >> .intx_event = EVENT_LOCAL_PM_MSI_INT_INTX, >> >> .msi_event = EVENT_LOCAL_PM_MSI_INT_MSI, >> >> }; >> >> >> >> -static int plda_pcie_init_irq_domains(struct plda_pcie_rp *port) >> >> +static int plda_pcie_init_irq_domains(struct plda_pcie_rp *port, >> >> + const struct irq_domain_ops *ops) >> >> { >> >> struct device *dev = port->dev; >> >> struct device_node *node = dev->of_node; >> >> @@ -941,7 +943,7 @@ static int plda_init_interrupts(struct platform_device *pdev, >> >> return -EINVAL; >> >> } >> >> >> >> - ret = plda_pcie_init_irq_domains(port); >> >> + ret = plda_pcie_init_irq_domains(port, event->domain_ops); >> >> if (ret) { >> >> dev_err(dev, "failed creating IRQ domains\n"); >> >> return ret; >> >> diff --git a/drivers/pci/controller/plda/pcie-plda.h b/drivers/pci/controller/plda/pcie-plda.h >> >> index df1729095952..820ea16855b5 100644 >> >> --- a/drivers/pci/controller/plda/pcie-plda.h >> >> +++ b/drivers/pci/controller/plda/pcie-plda.h >> >> @@ -129,6 +129,7 @@ struct plda_pcie_rp { >> >> }; >> >> >> >> struct plda_event { >> >> + const struct irq_domain_ops *domain_ops; >> >> const struct plda_event_ops *event_ops; >> >> int (*request_event_irq)(struct plda_pcie_rp *pcie, >> >> int event_irq, int event); >> >> -- >> >> 2.17.1 >> >>