[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y0yAX1fTCHhKKzeb@iweiny-desk3>
Date: Sun, 16 Oct 2022 15:06:23 -0700
From: Ira Weiny <ira.weiny@...el.com>
To: Davidlohr Bueso <dave@...olabs.net>
CC: <dan.j.williams@...el.com>, <Jonathan.Cameron@...wei.com>,
<dave.jiang@...el.com>, <alison.schofield@...el.com>,
<vishal.l.verma@...el.com>, <bwidawsk@...nel.org>,
<a.manzanares@...sung.com>, <linux-kernel@...r.kernel.org>,
<linux-cxl@...r.kernel.org>
Subject: Re: [PATCH 2/2] cxl/mbox: Wire up basic irq support
On Fri, Oct 14, 2022 at 12:49:30PM -0700, Davidlohr Bueso wrote:
> This adds support for mailbox interrupts, which are needed, for
> example, for background completion handling.
>
> Signed-off-by: Davidlohr Bueso <dave@...olabs.net>
> ---
> Note: We could also handle doorbell irq, but not sure this is
> actually needed.
>
> drivers/cxl/cxl.h | 1 +
> drivers/cxl/pci.c | 27 ++++++++++++++++++++++++++-
> 2 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 879661702054..d15a743bfc9e 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -140,6 +140,7 @@ enum {
> /* CXL 2.0 8.2.8.4 Mailbox Registers */
> #define CXLDEV_MBOX_CAPS_OFFSET 0x00
> #define CXLDEV_MBOX_CAP_PAYLOAD_SIZE_MASK GENMASK(4, 0)
> +#define CXLDEV_MBOX_CAP_IRQ_MSGNUM_MASK GENMASK(10, 7)
> #define CXLDEV_MBOX_CTRL_OFFSET 0x04
> #define CXLDEV_MBOX_CTRL_DOORBELL BIT(0)
> #define CXLDEV_MBOX_CMD_OFFSET 0x08
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 942c4449d30f..6e18ca3e551f 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -51,6 +51,20 @@ static unsigned short mbox_ready_timeout = 60;
> module_param(mbox_ready_timeout, ushort, 0644);
> MODULE_PARM_DESC(mbox_ready_timeout, "seconds to wait for mailbox ready");
>
> +static int cxl_pci_mbox_get_max_msgnum(struct cxl_dev_state *cxlds)
> +{
> + int cap;
> +
> + cap = readl(cxlds->regs.mbox + CXLDEV_MBOX_CAPS_OFFSET);
> + return FIELD_GET(CXLDEV_MBOX_CAP_IRQ_MSGNUM_MASK, cap);
I'm not a fan of the irq_type in cxlds.
Why doesn't this store the msgnum in cxlds and...
> +}
> +
> +static irqreturn_t cxl_pci_mbox_irq(int irq, void *id)
> +{
> + /* TODO: handle completion of background commands */
> + return IRQ_HANDLED;
> +}
> +
> static int cxl_pci_mbox_wait_for_doorbell(struct cxl_dev_state *cxlds)
> {
> const unsigned long start = jiffies;
> @@ -271,6 +285,15 @@ static int cxl_pci_setup_mailbox(struct cxl_dev_state *cxlds)
> dev_dbg(cxlds->dev, "Mailbox payload sized %zu",
> cxlds->payload_size);
>
> + if (cxlds->irq_type == CXL_IRQ_MSI) {
> + struct device *dev = cxlds->dev;
> + int irq = cxl_pci_mbox_get_max_msgnum(cxlds);
... use the stored msgnum in cxlds here? ... and use that as a flag if this
should be set up?
> +
> + if (devm_request_irq(dev, irq, cxl_pci_mbox_irq,
I was using pci_request_irq().
Is devm_request_irq() correct when having allocated the vectors with
pci_alloc_irq_vectors()?
Looking at pci_request_irq() is uses pci_irq_vector() to convert the msgnum to
the irq parameter of request_threaded_irq()?
Ira
> + IRQF_SHARED, "mailbox", cxlds))
> + dev_dbg(dev, "Mailbox irq (%d) supported", irq);
> + }
> +
> return 0;
> }
>
> @@ -441,7 +464,9 @@ struct cxl_irq_cap {
> int (*get_max_msgnum)(struct cxl_dev_state *cxlds);
> };
>
> -static const struct cxl_irq_cap cxl_irq_cap_table[] = { NULL };
> +static const struct cxl_irq_cap cxl_irq_cap_table[] = {
> + { "mailbox", cxl_pci_mbox_get_max_msgnum }
> +};
>
> static void cxl_pci_free_irq_vectors(void *data)
> {
> --
> 2.37.3
>
Powered by blists - more mailing lists