Re: [PATCH v13 14/21] PCI: microchip: Add get_events() callback and add PLDA get_event()

From: Lorenzo Pieralisi <lpieralisi@kernel.org>
To: Minda Chen <minda.chen@starfivetech.com>
Cc: "Conor Dooley" <conor@kernel.org>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Daire McNamara" <daire.mcnamara@microchip.com>,
	"Emil Renner Berthing" <emil.renner.berthing@canonical.com>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-riscv@lists.infradead.org, linux-pci@vger.kernel.org,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Philipp Zabel" <p.zabel@pengutronix.de>,
	"Mason Huo" <mason.huo@starfivetech.com>,
	"Leyfoon Tan" <leyfoon.tan@starfivetech.com>,
	"Kevin Xie" <kevin.xie@starfivetech.com>
Subject: Re: [PATCH v13 14/21] PCI: microchip: Add get_events() callback and add PLDA get_event()
Date: Wed, 27 Dec 2023 17:31:59 +0100	[thread overview]
Message-ID: <ZYxRf8d0+gSoqeRs@lpieralisi> (raw)
In-Reply-To: <20231214072839.2367-15-minda.chen@starfivetech.com>

On Thu, Dec 14, 2023 at 03:28:32PM +0800, Minda Chen wrote:
> PolarFire implements its own PCIe interrupts, additional to the regular
> PCIe interrupts, due to lack of an MSI controller, so the interrupt to
> event number mapping is different to the PLDA local interrupts,

I am sorry I don't understand what you mean here.

"its own PCIe interrupts" ?

"regular PCIe interrupts" ?

"PLDA local interrupts" ?

Can you spell out what those are referring to please ?

Thanks,
Lorenzo

> necessitating a custom get_events() implementation.
> 
> plda_get_events() adds interrupt register to PLDA local event num mapping
> codes except DMA engine interrupt events. The DMA engine interrupt events
> are implemented by vendors.
> 
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  .../pci/controller/plda/pcie-microchip-host.c | 35 ++++++++++++++++++-
>  drivers/pci/controller/plda/pcie-plda.h       | 33 +++++++++++++++++
>  2 files changed, 67 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/plda/pcie-microchip-host.c b/drivers/pci/controller/plda/pcie-microchip-host.c
> index 00250781b11c..fd0d92c3d03f 100644
> --- a/drivers/pci/controller/plda/pcie-microchip-host.c
> +++ b/drivers/pci/controller/plda/pcie-microchip-host.c
> @@ -627,6 +627,26 @@ static u32 mc_get_events(struct plda_pcie_rp *port)
>  	return events;
>  }
>  
> +static u32 plda_get_events(struct plda_pcie_rp *port)
> +{
> +	u32 events, val, origin;
> +
> +	origin = readl_relaxed(port->bridge_addr + ISTATUS_LOCAL);
> +
> +	/* Error events and doorbell events */
> +	events = (origin & ERROR_AND_DOORBELL_MASK) >> A_ATR_EVT_POST_ERR_SHIFT;
> +
> +	/* INTx events */
> +	if (origin & PM_MSI_INT_INTX_MASK)
> +		events |= BIT(EVENT_PM_MSI_INT_INTX);
> +
> +	/* MSI event and sys events */
> +	val = (origin & SYS_AND_MSI_MASK) >> PM_MSI_INT_MSI_SHIFT;
> +	events |= val << EVENT_PM_MSI_INT_MSI;
> +
> +	return events;
> +}
> +
>  static irqreturn_t mc_event_handler(int irq, void *dev_id)
>  {
>  	struct plda_pcie_rp *port = dev_id;
> @@ -657,7 +677,7 @@ static void plda_handle_event(struct irq_desc *desc)
>  
>  	chained_irq_enter(chip, desc);
>  
> -	events = mc_get_events(port);
> +	events = port->event_ops->get_events(port);
>  
>  	for_each_set_bit(bit, &events, port->num_events)
>  		generic_handle_domain_irq(port->event_domain, bit);
> @@ -751,6 +771,10 @@ static struct irq_chip mc_event_irq_chip = {
>  	.irq_unmask = mc_unmask_event_irq,
>  };
>  
> +static const struct plda_event_ops plda_event_ops = {
> +	.get_events = plda_get_events,
> +};
> +
>  static int plda_pcie_event_map(struct irq_domain *domain, unsigned int irq,
>  			       irq_hw_number_t hwirq)
>  {
> @@ -816,6 +840,10 @@ static int mc_request_event_irq(struct plda_pcie_rp *plda, int event_irq,
>  				0, event_cause[event].sym, plda);
>  }
>  
> +static const struct plda_event_ops mc_event_ops = {
> +	.get_events = mc_get_events,
> +};
> +
>  static const struct plda_event mc_event = {
>  	.request_event_irq = mc_request_event_irq,
>  	.intx_event        = EVENT_LOCAL_PM_MSI_INT_INTX,
> @@ -932,6 +960,9 @@ static int plda_init_interrupts(struct platform_device *pdev,
>  	int i, intx_irq, msi_irq, event_irq;
>  	int ret;
>  
> +	if (!port->event_ops)
> +		port->event_ops = &plda_event_ops;
> +
>  	ret = plda_pcie_init_irq_domains(port);
>  	if (ret) {
>  		dev_err(dev, "failed creating IRQ domains\n");
> @@ -1008,6 +1039,8 @@ static int mc_platform_init(struct pci_config_window *cfg)
>  	if (ret)
>  		return ret;
>  
> +	port->plda.event_ops = &mc_event_ops;
> +
>  	/* Address translation is up; safe to enable interrupts */
>  	ret = plda_init_interrupts(pdev, &port->plda, &mc_event);
>  	if (ret)
> diff --git a/drivers/pci/controller/plda/pcie-plda.h b/drivers/pci/controller/plda/pcie-plda.h
> index fba7343f9a96..dd8bc2750bfc 100644
> --- a/drivers/pci/controller/plda/pcie-plda.h
> +++ b/drivers/pci/controller/plda/pcie-plda.h
> @@ -40,6 +40,7 @@
>  #define  P_ATR_EVT_DISCARD_ERR_SHIFT		22
>  #define  P_ATR_EVT_DOORBELL_MASK		0x00000000u
>  #define  P_ATR_EVT_DOORBELL_SHIFT		23
> +#define  ERROR_AND_DOORBELL_MASK		GENMASK(23, 16)
>  #define  PM_MSI_INT_INTA_MASK			0x01000000u
>  #define  PM_MSI_INT_INTA_SHIFT			24
>  #define  PM_MSI_INT_INTB_MASK			0x02000000u
> @@ -58,6 +59,7 @@
>  #define  PM_MSI_INT_EVENTS_SHIFT		30
>  #define  PM_MSI_INT_SYS_ERR_MASK		0x80000000u
>  #define  PM_MSI_INT_SYS_ERR_SHIFT		31
> +#define  SYS_AND_MSI_MASK			GENMASK(31, 28)
>  #define  NUM_LOCAL_EVENTS			15
>  #define ISTATUS_LOCAL				0x184
>  #define IMASK_HOST				0x188
> @@ -102,6 +104,36 @@
>  #define EVENT_PM_MSI_INT_SYS_ERR		12
>  #define NUM_PLDA_EVENTS				13
>  
> +/*
> + * PLDA local interrupt register
> + *
> + * 31         27     23              15           7          0
> + * +--+--+--+-+------+-+-+-+-+-+-+-+-+-----------+-----------+
> + * |12|11|10|9| intx |7|6|5|4|3|2|1|0| DMA error | DMA end   |
> + * +--+--+--+-+------+-+-+-+-+-+-+-+-+-----------+-----------+
> + * 0:  AXI post error
> + * 1:  AXI fetch error
> + * 2:  AXI discard error
> + * 3:  AXI doorbell
> + * 4:  PCIe post error
> + * 5:  PCIe fetch error
> + * 6:  PCIe discard error
> + * 7:  PCIe doorbell
> + * 8:  4 INTx interruts
> + * 9:  MSI interrupt
> + * 10: AER event
> + * 11: PM/LTR/Hotplug
> + * 12: System error
> + * DMA error : reserved for vendor implement
> + * DMA end : reserved for vendor implement
> + */
> +
> +struct plda_pcie_rp;
> +
> +struct plda_event_ops {
> +	u32 (*get_events)(struct plda_pcie_rp *pcie);
> +};
> +
>  struct plda_msi {
>  	struct mutex lock;		/* Protect used bitmap */
>  	struct irq_domain *msi_domain;
> @@ -117,6 +149,7 @@ struct plda_pcie_rp {
>  	struct irq_domain *event_domain;
>  	raw_spinlock_t lock;
>  	struct plda_msi msi;
> +	const struct plda_event_ops *event_ops;
>  	void __iomem *bridge_addr;
>  	int num_events;
>  };
> -- 
> 2.17.1
> 