[2/2] cxl/mbox: Wire up basic irq support

Message ID 20221014194930.2630416-3-dave@stgolabs.net
State New
Headers
Series cxl: Add basic MSI/MSI-X support |

Commit Message

Davidlohr Bueso Oct. 14, 2022, 7:49 p.m. UTC
  This adds support for mailbox interrupts, which are needed, for
example, for background completion handling.

Signed-off-by: Davidlohr Bueso <dave@stgolabs.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(-)
  

Comments

Ira Weiny Oct. 16, 2022, 10:06 p.m. UTC | #1
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@stgolabs.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
>
  
Jonathan Cameron Oct. 17, 2022, 10:08 a.m. UTC | #2
On Sun, 16 Oct 2022 15:06:23 -0700
Ira Weiny <ira.weiny@intel.com> wrote:

> 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@stgolabs.net>
> > ---
...

> > +				     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[] = {

Trivial patch mangling point but better to have the NULL on separate line
in previous patch to reduce the diff here.

> > +	{ "mailbox", cxl_pci_mbox_get_max_msgnum }

trailing comma needed as we definitely expect to add more entries!

> > +};
> >  
> >  static void cxl_pci_free_irq_vectors(void *data)
> >  {
> > -- 
> > 2.37.3
> >
  

Patch

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);
+}
+
+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);
+
+		if (devm_request_irq(dev, irq, cxl_pci_mbox_irq,
+				     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)
 {