Re: [PATCH 05/13] cxl/pci: Disambiguate cxl_pci further from cxl_mem

From: Dan Williams <dan.j.williams@intel.com>
To: Ben Widawsky <ben.widawsky@intel.com>
Cc: linux-cxl@vger.kernel.org,
	Alison Schofield <alison.schofield@intel.com>,
	Ira Weiny <ira.weiny@intel.com>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Vishal Verma <vishal.l.verma@intel.com>
Subject: Re: [PATCH 05/13] cxl/pci: Disambiguate cxl_pci further from cxl_mem
Date: Fri, 10 Sep 2021 12:27:11 -0700	[thread overview]
Message-ID: <CAPcyv4ivjUqTwd9QiGQ1hHxPwu4Czw0M1Ht5po+cGEiQbvU4ew@mail.gmail.com> (raw)
In-Reply-To: <20210902195017.2516472-6-ben.widawsky@intel.com>

On Thu, Sep 2, 2021 at 12:50 PM Ben Widawsky <ben.widawsky@intel.com> wrote:
>
> Commit 21e9f76733a8 ("cxl: Rename mem to pci") introduced the cxl_pci
> driver which had formerly been named cxl_mem. At the time, the goal was
> to be as light touch as possible because there were other patches in
> flight. Since things have settled now, and a new cxl_mem driver will be
> introduced shortly, spend the LOC now to clean up the existing names.
>
> While here, fix the kernel docs to explain the situation better after
> the core rework that has already landed.
>
> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
> ---
>  drivers/cxl/pci.c | 70 +++++++++++++++++++++++------------------------
>  1 file changed, 35 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index b13884275d96..6931885c83ce 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -16,14 +16,14 @@
>   *
>   * This implements the PCI exclusive functionality for a CXL device as it is
>   * defined by the Compute Express Link specification. CXL devices may surface
> - * certain functionality even if it isn't CXL enabled.
> + * certain functionality even if it isn't CXL enabled. While this driver is
> + * focused around the PCI specific aspects of a CXL device, it binds to the
> + * specific CXL memory device class code, and therefore the implementation of
> + * cxl_pci is focused around CXL memory devices.
>   *
> - * The driver has several responsibilities, mainly:
> + * The driver has two responsibilities:

Perhaps:

s/driver has two responsibilities:/driver's responsibilities include:/

>   *  - Create the memX device and register on the CXL bus.
>   *  - Enumerate device's register interface and map them.

..since there are other things to add here:

It also registers an nvdimm bridge device, and you might mention it
registers the mbox_send mechanism to mailbox core. Later it will also
be responsible for coordinating the retrieval of the CDAT.

> - *  - Probe the device attributes to establish sysfs interface.
> - *  - Provide an IOCTL interface to userspace to communicate with the device for
> - *    things like firmware update.
>   */
>
>  #define cxl_doorbell_busy(cxlm)                                                \
> @@ -33,7 +33,7 @@
>  /* CXL 2.0 - 8.2.8.4 */
>  #define CXL_MAILBOX_TIMEOUT_MS (2 * HZ)
>
> -static int cxl_mem_wait_for_doorbell(struct cxl_mem *cxlm)
> +static int cxl_pci_wait_for_doorbell(struct cxl_mem *cxlm)

Should this be cxl_pci_mbox_wait_for_doobell() to match the other helpers?

Other than that, rename looks good to me.