Re: [PATCH v12 15/21] PCI: microchip: Add event IRQ domain ops to struct plda_event

[Minda Chen]

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
>> >>