| Message ID | 162042788015.1202325.6094533661799093199.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | CXL Port Enumeration and Plans for v5.14 | expand |
On Fri, 7 May 2021 15:51:20 -0700 Dan Williams <dan.j.williams@intel.com> wrote: > In preparation for sharing cxl.h with other generic CXL consumers, > move / consolidate some of the memory device specifics to mem.h. > > The motivation for moving out of cxl.h is to maintain least privilege > access to memory-device details since cxl.h is used in multiple files. > The motivation for moving definitions into a new mem.h header is for > code readability and organization. I.e. minimize implementation details > when reading data structures and other definitions. > > Reviewed-by: Ben Widawsky <ben.widawsky@intel.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> I'll do these one by one as not sure if you use automated scripts to pick up (which get confused by any time people do "for patches 1-4 + 7") Basically I'm fine with all but patch 5... Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/cxl/cxl.h | 57 ------------------------------------ > drivers/cxl/mem.c | 25 +--------------- > drivers/cxl/mem.h | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 86 insertions(+), 81 deletions(-) > create mode 100644 drivers/cxl/mem.h > > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index 6f14838c2d25..2e3bdacb32e7 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > @@ -34,62 +34,5 @@ > #define CXLDEV_MBOX_BG_CMD_STATUS_OFFSET 0x18 > #define CXLDEV_MBOX_PAYLOAD_OFFSET 0x20 > > -/* CXL 2.0 8.2.8.5.1.1 Memory Device Status Register */ > -#define CXLMDEV_STATUS_OFFSET 0x0 > -#define CXLMDEV_DEV_FATAL BIT(0) > -#define CXLMDEV_FW_HALT BIT(1) > -#define CXLMDEV_STATUS_MEDIA_STATUS_MASK GENMASK(3, 2) > -#define CXLMDEV_MS_NOT_READY 0 > -#define CXLMDEV_MS_READY 1 > -#define CXLMDEV_MS_ERROR 2 > -#define CXLMDEV_MS_DISABLED 3 > -#define CXLMDEV_READY(status) \ > - (FIELD_GET(CXLMDEV_STATUS_MEDIA_STATUS_MASK, status) == \ > - CXLMDEV_MS_READY) > -#define CXLMDEV_MBOX_IF_READY BIT(4) > -#define CXLMDEV_RESET_NEEDED_MASK GENMASK(7, 5) > -#define CXLMDEV_RESET_NEEDED_NOT 0 > -#define CXLMDEV_RESET_NEEDED_COLD 1 > -#define CXLMDEV_RESET_NEEDED_WARM 2 > -#define CXLMDEV_RESET_NEEDED_HOT 3 > -#define CXLMDEV_RESET_NEEDED_CXL 4 > -#define CXLMDEV_RESET_NEEDED(status) \ > - (FIELD_GET(CXLMDEV_RESET_NEEDED_MASK, status) != \ > - CXLMDEV_RESET_NEEDED_NOT) > - > -struct cxl_memdev; > -/** > - * struct cxl_mem - A CXL memory device > - * @pdev: The PCI device associated with this CXL device. > - * @regs: IO mappings to the device's MMIO > - * @status_regs: CXL 2.0 8.2.8.3 Device Status Registers > - * @mbox_regs: CXL 2.0 8.2.8.4 Mailbox Registers > - * @memdev_regs: CXL 2.0 8.2.8.5 Memory Device Registers > - * @payload_size: Size of space for payload > - * (CXL 2.0 8.2.8.4.3 Mailbox Capabilities Register) > - * @mbox_mutex: Mutex to synchronize mailbox access. > - * @firmware_version: Firmware version for the memory device. > - * @enabled_commands: Hardware commands found enabled in CEL. > - * @pmem_range: Persistent memory capacity information. > - * @ram_range: Volatile memory capacity information. > - */ > -struct cxl_mem { > - struct pci_dev *pdev; > - void __iomem *regs; > - struct cxl_memdev *cxlmd; > - > - void __iomem *status_regs; > - void __iomem *mbox_regs; > - void __iomem *memdev_regs; > - > - size_t payload_size; > - struct mutex mbox_mutex; /* Protects device mailbox and firmware */ > - char firmware_version[0x10]; > - unsigned long *enabled_cmds; > - > - struct range pmem_range; > - struct range ram_range; > -}; > - > extern struct bus_type cxl_bus_type; > #endif /* __CXL_H__ */ > diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c > index 244cb7d89678..45871ef65152 100644 > --- a/drivers/cxl/mem.c > +++ b/drivers/cxl/mem.c > @@ -12,6 +12,7 @@ > #include <linux/io-64-nonatomic-lo-hi.h> > #include "pci.h" > #include "cxl.h" > +#include "mem.h" > > /** > * DOC: cxl mem > @@ -29,12 +30,6 @@ > * - Handle and manage error conditions. > */ > > -/* > - * An entire PCI topology full of devices should be enough for any > - * config > - */ > -#define CXL_MEM_MAX_DEVS 65536 > - > #define cxl_doorbell_busy(cxlm) \ > (readl((cxlm)->mbox_regs + CXLDEV_MBOX_CTRL_OFFSET) & \ > CXLDEV_MBOX_CTRL_DOORBELL) > @@ -91,24 +86,6 @@ struct mbox_cmd { > #define CXL_MBOX_SUCCESS 0 > }; > > -/** > - * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device > - * @dev: driver core device object > - * @cdev: char dev core object for ioctl operations > - * @cxlm: pointer to the parent device driver data > - * @ops_active: active user of @cxlm in ops handlers > - * @ops_dead: completion when all @cxlm ops users have exited > - * @id: id number of this memdev instance. > - */ > -struct cxl_memdev { > - struct device dev; > - struct cdev cdev; > - struct cxl_mem *cxlm; > - struct percpu_ref ops_active; > - struct completion ops_dead; > - int id; > -}; > - > static int cxl_mem_major; > static DEFINE_IDA(cxl_memdev_ida); > static struct dentry *cxl_debugfs; > diff --git a/drivers/cxl/mem.h b/drivers/cxl/mem.h > new file mode 100644 > index 000000000000..616bb6fd721f > --- /dev/null > +++ b/drivers/cxl/mem.h > @@ -0,0 +1,85 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* Copyright(c) 2020-2021 Intel Corporation. */ > +#ifndef __CXL_MEM_H__ > +#define __CXL_MEM_H__ > + > +/* CXL 2.0 8.2.8.5.1.1 Memory Device Status Register */ > +#define CXLMDEV_STATUS_OFFSET 0x0 > +#define CXLMDEV_DEV_FATAL BIT(0) > +#define CXLMDEV_FW_HALT BIT(1) > +#define CXLMDEV_STATUS_MEDIA_STATUS_MASK GENMASK(3, 2) > +#define CXLMDEV_MS_NOT_READY 0 > +#define CXLMDEV_MS_READY 1 > +#define CXLMDEV_MS_ERROR 2 > +#define CXLMDEV_MS_DISABLED 3 > +#define CXLMDEV_READY(status) \ > + (FIELD_GET(CXLMDEV_STATUS_MEDIA_STATUS_MASK, status) == \ > + CXLMDEV_MS_READY) > +#define CXLMDEV_MBOX_IF_READY BIT(4) > +#define CXLMDEV_RESET_NEEDED_MASK GENMASK(7, 5) > +#define CXLMDEV_RESET_NEEDED_NOT 0 > +#define CXLMDEV_RESET_NEEDED_COLD 1 > +#define CXLMDEV_RESET_NEEDED_WARM 2 > +#define CXLMDEV_RESET_NEEDED_HOT 3 > +#define CXLMDEV_RESET_NEEDED_CXL 4 > +#define CXLMDEV_RESET_NEEDED(status) \ > + (FIELD_GET(CXLMDEV_RESET_NEEDED_MASK, status) != \ > + CXLMDEV_RESET_NEEDED_NOT) > + > +/* > + * An entire PCI topology full of devices should be enough for any > + * config > + */ > +#define CXL_MEM_MAX_DEVS 65536 > + > +/** > + * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device > + * @dev: driver core device object > + * @cdev: char dev core object for ioctl operations > + * @cxlm: pointer to the parent device driver data > + * @ops_active: active user of @cxlm in ops handlers > + * @ops_dead: completion when all @cxlm ops users have exited > + * @id: id number of this memdev instance. > + */ > +struct cxl_memdev { > + struct device dev; > + struct cdev cdev; > + struct cxl_mem *cxlm; > + struct percpu_ref ops_active; > + struct completion ops_dead; > + int id; > +}; > + > +/** > + * struct cxl_mem - A CXL memory device > + * @pdev: The PCI device associated with this CXL device. > + * @regs: IO mappings to the device's MMIO > + * @status_regs: CXL 2.0 8.2.8.3 Device Status Registers > + * @mbox_regs: CXL 2.0 8.2.8.4 Mailbox Registers > + * @memdev_regs: CXL 2.0 8.2.8.5 Memory Device Registers > + * @payload_size: Size of space for payload > + * (CXL 2.0 8.2.8.4.3 Mailbox Capabilities Register) > + * @mbox_mutex: Mutex to synchronize mailbox access. > + * @firmware_version: Firmware version for the memory device. > + * @enabled_cmds: Hardware commands found enabled in CEL. > + * @pmem_range: Persistent memory capacity information. > + * @ram_range: Volatile memory capacity information. > + */ > +struct cxl_mem { > + struct pci_dev *pdev; > + void __iomem *regs; > + struct cxl_memdev *cxlmd; > + > + void __iomem *status_regs; > + void __iomem *mbox_regs; > + void __iomem *memdev_regs; > + > + size_t payload_size; > + struct mutex mbox_mutex; /* Protects device mailbox and firmware */ > + char firmware_version[0x10]; > + unsigned long *enabled_cmds; > + > + struct range pmem_range; > + struct range ram_range; > +}; > +#endif /* __CXL_MEM_H__ */ >
On Mon, May 10, 2021 at 8:17 AM Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote: > > On Fri, 7 May 2021 15:51:20 -0700 > Dan Williams <dan.j.williams@intel.com> wrote: > > > In preparation for sharing cxl.h with other generic CXL consumers, > > move / consolidate some of the memory device specifics to mem.h. > > > > The motivation for moving out of cxl.h is to maintain least privilege > > access to memory-device details since cxl.h is used in multiple files. > > The motivation for moving definitions into a new mem.h header is for > > code readability and organization. I.e. minimize implementation details > > when reading data structures and other definitions. > > > > Reviewed-by: Ben Widawsky <ben.widawsky@intel.com> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > I'll do these one by one as not sure if you use automated scripts to pick up > (which get confused by any time people do "for patches 1-4 + 7") > Basically I'm fine with all but patch 5... Sounds good. I do use b4 which has the "--apply-cover-trailers" option. So in the future if you do want to apply reviewed-by to the whole series it's sufficient to just reply to the cover letter. > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Thanks, Jonathan.
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index 6f14838c2d25..2e3bdacb32e7 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -34,62 +34,5 @@ #define CXLDEV_MBOX_BG_CMD_STATUS_OFFSET 0x18 #define CXLDEV_MBOX_PAYLOAD_OFFSET 0x20 -/* CXL 2.0 8.2.8.5.1.1 Memory Device Status Register */ -#define CXLMDEV_STATUS_OFFSET 0x0 -#define CXLMDEV_DEV_FATAL BIT(0) -#define CXLMDEV_FW_HALT BIT(1) -#define CXLMDEV_STATUS_MEDIA_STATUS_MASK GENMASK(3, 2) -#define CXLMDEV_MS_NOT_READY 0 -#define CXLMDEV_MS_READY 1 -#define CXLMDEV_MS_ERROR 2 -#define CXLMDEV_MS_DISABLED 3 -#define CXLMDEV_READY(status) \ - (FIELD_GET(CXLMDEV_STATUS_MEDIA_STATUS_MASK, status) == \ - CXLMDEV_MS_READY) -#define CXLMDEV_MBOX_IF_READY BIT(4) -#define CXLMDEV_RESET_NEEDED_MASK GENMASK(7, 5) -#define CXLMDEV_RESET_NEEDED_NOT 0 -#define CXLMDEV_RESET_NEEDED_COLD 1 -#define CXLMDEV_RESET_NEEDED_WARM 2 -#define CXLMDEV_RESET_NEEDED_HOT 3 -#define CXLMDEV_RESET_NEEDED_CXL 4 -#define CXLMDEV_RESET_NEEDED(status) \ - (FIELD_GET(CXLMDEV_RESET_NEEDED_MASK, status) != \ - CXLMDEV_RESET_NEEDED_NOT) - -struct cxl_memdev; -/** - * struct cxl_mem - A CXL memory device - * @pdev: The PCI device associated with this CXL device. - * @regs: IO mappings to the device's MMIO - * @status_regs: CXL 2.0 8.2.8.3 Device Status Registers - * @mbox_regs: CXL 2.0 8.2.8.4 Mailbox Registers - * @memdev_regs: CXL 2.0 8.2.8.5 Memory Device Registers - * @payload_size: Size of space for payload - * (CXL 2.0 8.2.8.4.3 Mailbox Capabilities Register) - * @mbox_mutex: Mutex to synchronize mailbox access. - * @firmware_version: Firmware version for the memory device. - * @enabled_commands: Hardware commands found enabled in CEL. - * @pmem_range: Persistent memory capacity information. - * @ram_range: Volatile memory capacity information. - */ -struct cxl_mem { - struct pci_dev *pdev; - void __iomem *regs; - struct cxl_memdev *cxlmd; - - void __iomem *status_regs; - void __iomem *mbox_regs; - void __iomem *memdev_regs; - - size_t payload_size; - struct mutex mbox_mutex; /* Protects device mailbox and firmware */ - char firmware_version[0x10]; - unsigned long *enabled_cmds; - - struct range pmem_range; - struct range ram_range; -}; - extern struct bus_type cxl_bus_type; #endif /* __CXL_H__ */ diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c index 244cb7d89678..45871ef65152 100644 --- a/drivers/cxl/mem.c +++ b/drivers/cxl/mem.c @@ -12,6 +12,7 @@ #include <linux/io-64-nonatomic-lo-hi.h> #include "pci.h" #include "cxl.h" +#include "mem.h" /** * DOC: cxl mem @@ -29,12 +30,6 @@ * - Handle and manage error conditions. */ -/* - * An entire PCI topology full of devices should be enough for any - * config - */ -#define CXL_MEM_MAX_DEVS 65536 - #define cxl_doorbell_busy(cxlm) \ (readl((cxlm)->mbox_regs + CXLDEV_MBOX_CTRL_OFFSET) & \ CXLDEV_MBOX_CTRL_DOORBELL) @@ -91,24 +86,6 @@ struct mbox_cmd { #define CXL_MBOX_SUCCESS 0 }; -/** - * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device - * @dev: driver core device object - * @cdev: char dev core object for ioctl operations - * @cxlm: pointer to the parent device driver data - * @ops_active: active user of @cxlm in ops handlers - * @ops_dead: completion when all @cxlm ops users have exited - * @id: id number of this memdev instance. - */ -struct cxl_memdev { - struct device dev; - struct cdev cdev; - struct cxl_mem *cxlm; - struct percpu_ref ops_active; - struct completion ops_dead; - int id; -}; - static int cxl_mem_major; static DEFINE_IDA(cxl_memdev_ida); static struct dentry *cxl_debugfs; diff --git a/drivers/cxl/mem.h b/drivers/cxl/mem.h new file mode 100644 index 000000000000..616bb6fd721f --- /dev/null +++ b/drivers/cxl/mem.h @@ -0,0 +1,85 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* Copyright(c) 2020-2021 Intel Corporation. */ +#ifndef __CXL_MEM_H__ +#define __CXL_MEM_H__ + +/* CXL 2.0 8.2.8.5.1.1 Memory Device Status Register */ +#define CXLMDEV_STATUS_OFFSET 0x0 +#define CXLMDEV_DEV_FATAL BIT(0) +#define CXLMDEV_FW_HALT BIT(1) +#define CXLMDEV_STATUS_MEDIA_STATUS_MASK GENMASK(3, 2) +#define CXLMDEV_MS_NOT_READY 0 +#define CXLMDEV_MS_READY 1 +#define CXLMDEV_MS_ERROR 2 +#define CXLMDEV_MS_DISABLED 3 +#define CXLMDEV_READY(status) \ + (FIELD_GET(CXLMDEV_STATUS_MEDIA_STATUS_MASK, status) == \ + CXLMDEV_MS_READY) +#define CXLMDEV_MBOX_IF_READY BIT(4) +#define CXLMDEV_RESET_NEEDED_MASK GENMASK(7, 5) +#define CXLMDEV_RESET_NEEDED_NOT 0 +#define CXLMDEV_RESET_NEEDED_COLD 1 +#define CXLMDEV_RESET_NEEDED_WARM 2 +#define CXLMDEV_RESET_NEEDED_HOT 3 +#define CXLMDEV_RESET_NEEDED_CXL 4 +#define CXLMDEV_RESET_NEEDED(status) \ + (FIELD_GET(CXLMDEV_RESET_NEEDED_MASK, status) != \ + CXLMDEV_RESET_NEEDED_NOT) + +/* + * An entire PCI topology full of devices should be enough for any + * config + */ +#define CXL_MEM_MAX_DEVS 65536 + +/** + * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device + * @dev: driver core device object + * @cdev: char dev core object for ioctl operations + * @cxlm: pointer to the parent device driver data + * @ops_active: active user of @cxlm in ops handlers + * @ops_dead: completion when all @cxlm ops users have exited + * @id: id number of this memdev instance. + */ +struct cxl_memdev { + struct device dev; + struct cdev cdev; + struct cxl_mem *cxlm; + struct percpu_ref ops_active; + struct completion ops_dead; + int id; +}; + +/** + * struct cxl_mem - A CXL memory device + * @pdev: The PCI device associated with this CXL device. + * @regs: IO mappings to the device's MMIO + * @status_regs: CXL 2.0 8.2.8.3 Device Status Registers + * @mbox_regs: CXL 2.0 8.2.8.4 Mailbox Registers + * @memdev_regs: CXL 2.0 8.2.8.5 Memory Device Registers + * @payload_size: Size of space for payload + * (CXL 2.0 8.2.8.4.3 Mailbox Capabilities Register) + * @mbox_mutex: Mutex to synchronize mailbox access. + * @firmware_version: Firmware version for the memory device. + * @enabled_cmds: Hardware commands found enabled in CEL. + * @pmem_range: Persistent memory capacity information. + * @ram_range: Volatile memory capacity information. + */ +struct cxl_mem { + struct pci_dev *pdev; + void __iomem *regs; + struct cxl_memdev *cxlmd; + + void __iomem *status_regs; + void __iomem *mbox_regs; + void __iomem *memdev_regs; + + size_t payload_size; + struct mutex mbox_mutex; /* Protects device mailbox and firmware */ + char firmware_version[0x10]; + unsigned long *enabled_cmds; + + struct range pmem_range; + struct range ram_range; +}; +#endif /* __CXL_MEM_H__ */