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