Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 16 Oct 2022 15:06:23 -0700
From:   Ira Weiny <ira.weiny@intel.com>
To:     Davidlohr Bueso <dave@stgolabs.net>
CC:     <dan.j.williams@intel.com>, <Jonathan.Cameron@huawei.com>,
        <dave.jiang@intel.com>, <alison.schofield@intel.com>,
        <vishal.l.verma@intel.com>, <bwidawsk@kernel.org>,
        <a.manzanares@samsung.com>, <linux-kernel@vger.kernel.org>,
        <linux-cxl@vger.kernel.org>
Subject: Re: [PATCH 2/2] cxl/mbox: Wire up basic irq support
Message-ID: <Y0yAX1fTCHhKKzeb@iweiny-desk3>
References: <20221014194930.2630416-1-dave@stgolabs.net>
 <20221014194930.2630416-3-dave@stgolabs.net>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20221014194930.2630416-3-dave@stgolabs.net>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?GyHMc7RSaCqTZfRLrkqKokdVer7aldsHcv6Uz9UFwhHtFoPUtb73sxqXusxy?=
 =?us-ascii?Q?ghTcZcbY3AE7OAPGW9ztUIEQDfn7FvhGCl7OvxMH2v7HSHpVJ/NEWD1VlXIW?=
 =?us-ascii?Q?CGFZOVz20G2mJw8WB6a8gtMeE8hSyUis4DhRrt3r0A7h3rw4TY9k/jzE/Xr6?=
 =?us-ascii?Q?RQFGuV0aYnqWpGfDxM10jhxERudEwZpjyLCe76f+oC0q+M2/vSz5MWtvYxHP?=
 =?us-ascii?Q?Ids/+285WE2QLKpripJ4uPr81o+DMa31ME463ikZAEkhmhU5SNvPe7U/vMD0?=
 =?us-ascii?Q?QCcU/pHSOPaevlDj5jTqKmmhcCxP46AbW2bRW5cbmoz3tvCD6cdHO1E3WJlq?=
 =?us-ascii?Q?YAp+JeJV5sCoVO30vg9aJ/4KYt07lUhjJ8qeu0lmDibgzRm84f0AChKnjXnr?=
 =?us-ascii?Q?47i0IYhl9rgp0O8u2yuuV1nThAT41Rzs5gE1xucURoZuLk3WkctuXACypiE3?=
 =?us-ascii?Q?CC3K2vus2zJSUCO3IhEg4c0GcsQDOIBdn7H2McVVtt+bm20yAd6eFNcPIiCg?=
 =?us-ascii?Q?GMWL1IRVW4bXbHX7XBRkyvqB4VaGkodvnzxdBJjfhtK+d65a5x0E1iTinCCp?=
 =?us-ascii?Q?v6agy9xCQkamgSOp7ZpbUME0tvSRfvpVMnmRgVdl0WqJ+Bze6ob42UrmW8Lt?=
 =?us-ascii?Q?S+rnQvqswuhqZguwtu07g73yR/ulf+msT0+jFUpSxtJZgMA0Xcck9f0hVNDJ?=
 =?us-ascii?Q?Ex5p1geJarNnF2e8Brk7LxUb7h8FUw5OfWyfT2GUn0m58VIGMHHFUyRZZlbI?=
 =?us-ascii?Q?IfFLayaXbinNvtKu1itTZ/wl2jEh6ihVac9IRgNuhimLay+Nm2lf12uz62A2?=
 =?us-ascii?Q?xgAkxES99oin+FZY5C3lFQp3MrvLlrGCwwAI2aDVgYc4W9RIdsUCvaDro65u?=
 =?us-ascii?Q?XSQ1pQ74WaipbGjFfSGTvzT3D2ECOCpe4tC4UlJF6SWK6J4H/kVRdYJHYEJ9?=
 =?us-ascii?Q?3uYP2wJ7KkZf1C5X4KXL87FOEaBmYUtop+UXzI2ijmovRem+G+r4qarwyTpZ?=
 =?us-ascii?Q?rC00zRoJYu/U9IxnLjHY/AY0bQu1wFQPP+t1Mvqxa/+D2wPEu9C62AmWD14N?=
 =?us-ascii?Q?zQ0tKaQeC/YsCT5yN7XwOL5LKCUs8CxF57PC/8SQR/Y6HH0LN9RXaPmUiq2a?=
 =?us-ascii?Q?6eyBQ3zbTFT04Oo1e15m6Gb3R41XR/83v4Actfm2s/niytXwomSAJvnVeyPM?=
 =?us-ascii?Q?UPAKOncCrDiDbzEf7Y9G4mgLGjMG57nDf2S0Mu6Py9T6v0BjZdnOpM3K9GaD?=
 =?us-ascii?Q?sMXe2SLsaxB0CwI1sxZX0gyhn5p5YdXFWpcGXF9j7fMpilT5vnn1IqEiOmY1?=
 =?us-ascii?Q?yefQBaK5CAiLeOHf832mA9bIMpg1hrFVfGuiHK08rgqObBHHWq8bMbQKKgw8?=
 =?us-ascii?Q?0v7muK/AyjahSoBtqDYzzzSpEn2UwHXlzQ5XGNtiJYL1C3UvbGlbi+fxEpln?=
 =?us-ascii?Q?Q0djoZgFXADZNTInBFNwK1T0sRBdY+CqVhmOcEjhtPG6IeyfT/UuUc5CkUne?=
 =?us-ascii?Q?bkM+FG5CEDJKo9igxv8vR9zmu2MVCKCzb8Z8EZbHsmYqDryN/uUy6EorwKiR?=
 =?us-ascii?Q?FMY+Jg8ExMY7Uug9nVZF2msiFT4I4PDeb54SPTAv?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 2754f605-e514-416b-ddf8-08daafc2ac39
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Oct 2022 22:06:27.6523
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: eSTK7pDKMT1LmAYxJ3k3q5SOGL3C6+zrz0bo+tfunZ33OfBzfdOljV6Xo1tzwjKPu/fU8rDkULdEwll9EKrvqA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB4966
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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
>