| Message ID | 20240816-dcd-type2-upstream-v3-12-7c9b96cba6d7@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | DCD: Add support for Dynamic Capacity Devices (DCD) | expand |
On 8/16/24 7:44 AM, Ira Weiny wrote: > create_pmem_region_store() and create_ram_region_store() are identical > with the exception of the region mode. With the addition of DC region > mode this would end up being 3 copies of the same code. > > Refactor create_pmem_region_store() and create_ram_region_store() to use > a single common function to be used in subsequent DC code. > > Suggested-by: Fan Ni <fan.ni@samsung.com> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> > --- > drivers/cxl/core/region.c | 28 +++++++++++----------------- > 1 file changed, 11 insertions(+), 17 deletions(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 650fe33f2ed4..f85b26b39b2f 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -2553,9 +2553,8 @@ static struct cxl_region *__create_region(struct cxl_root_decoder *cxlrd, > return devm_cxl_add_region(cxlrd, id, mode, CXL_DECODER_HOSTONLYMEM); > } > > -static ssize_t create_pmem_region_store(struct device *dev, > - struct device_attribute *attr, > - const char *buf, size_t len) > +static ssize_t create_region_store(struct device *dev, const char *buf, > + size_t len, enum cxl_region_mode mode) > { > struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev); > struct cxl_region *cxlr; > @@ -2565,31 +2564,26 @@ static ssize_t create_pmem_region_store(struct device *dev, > if (rc != 1) > return -EINVAL; > > - cxlr = __create_region(cxlrd, CXL_REGION_PMEM, id); > + cxlr = __create_region(cxlrd, mode, id); > if (IS_ERR(cxlr)) > return PTR_ERR(cxlr); > > return len; > } > + > +static ssize_t create_pmem_region_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t len) > +{ > + return create_region_store(dev, buf, len, CXL_REGION_PMEM); > +} > DEVICE_ATTR_RW(create_pmem_region); > > static ssize_t create_ram_region_store(struct device *dev, > struct device_attribute *attr, > const char *buf, size_t len) > { > - struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev); > - struct cxl_region *cxlr; > - int rc, id; > - > - rc = sscanf(buf, "region%d\n", &id); > - if (rc != 1) > - return -EINVAL; > - > - cxlr = __create_region(cxlrd, CXL_REGION_RAM, id); > - if (IS_ERR(cxlr)) > - return PTR_ERR(cxlr); > - > - return len; > + return create_region_store(dev, buf, len, CXL_REGION_RAM); > } > DEVICE_ATTR_RW(create_ram_region); > >
On Fri, Aug 16, 2024 at 09:44:20AM -0500, Ira Weiny wrote: > create_pmem_region_store() and create_ram_region_store() are identical > with the exception of the region mode. With the addition of DC region > mode this would end up being 3 copies of the same code. > > Refactor create_pmem_region_store() and create_ram_region_store() to use > a single common function to be used in subsequent DC code. > > Suggested-by: Fan Ni <fan.ni@samsung.com> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > --- Reviewed-by: Fan Ni <fan.ni@samsung.com> > drivers/cxl/core/region.c | 28 +++++++++++----------------- > 1 file changed, 11 insertions(+), 17 deletions(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 650fe33f2ed4..f85b26b39b2f 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -2553,9 +2553,8 @@ static struct cxl_region *__create_region(struct cxl_root_decoder *cxlrd, > return devm_cxl_add_region(cxlrd, id, mode, CXL_DECODER_HOSTONLYMEM); > } > > -static ssize_t create_pmem_region_store(struct device *dev, > - struct device_attribute *attr, > - const char *buf, size_t len) > +static ssize_t create_region_store(struct device *dev, const char *buf, > + size_t len, enum cxl_region_mode mode) > { > struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev); > struct cxl_region *cxlr; > @@ -2565,31 +2564,26 @@ static ssize_t create_pmem_region_store(struct device *dev, > if (rc != 1) > return -EINVAL; > > - cxlr = __create_region(cxlrd, CXL_REGION_PMEM, id); > + cxlr = __create_region(cxlrd, mode, id); > if (IS_ERR(cxlr)) > return PTR_ERR(cxlr); > > return len; > } > + > +static ssize_t create_pmem_region_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t len) > +{ > + return create_region_store(dev, buf, len, CXL_REGION_PMEM); > +} > DEVICE_ATTR_RW(create_pmem_region); > > static ssize_t create_ram_region_store(struct device *dev, > struct device_attribute *attr, > const char *buf, size_t len) > { > - struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev); > - struct cxl_region *cxlr; > - int rc, id; > - > - rc = sscanf(buf, "region%d\n", &id); > - if (rc != 1) > - return -EINVAL; > - > - cxlr = __create_region(cxlrd, CXL_REGION_RAM, id); > - if (IS_ERR(cxlr)) > - return PTR_ERR(cxlr); > - > - return len; > + return create_region_store(dev, buf, len, CXL_REGION_RAM); > } > DEVICE_ATTR_RW(create_ram_region); > > > -- > 2.45.2 >
On Fri, 16 Aug 2024 09:44:20 -0500 Ira Weiny <ira.weiny@intel.com> wrote: > create_pmem_region_store() and create_ram_region_store() are identical > with the exception of the region mode. With the addition of DC region > mode this would end up being 3 copies of the same code. > > Refactor create_pmem_region_store() and create_ram_region_store() to use > a single common function to be used in subsequent DC code. > > Suggested-by: Fan Ni <fan.ni@samsung.com> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> Make sense Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/cxl/core/region.c | 28 +++++++++++----------------- > 1 file changed, 11 insertions(+), 17 deletions(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 650fe33f2ed4..f85b26b39b2f 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -2553,9 +2553,8 @@ static struct cxl_region *__create_region(struct cxl_root_decoder *cxlrd, > return devm_cxl_add_region(cxlrd, id, mode, CXL_DECODER_HOSTONLYMEM); > } > > -static ssize_t create_pmem_region_store(struct device *dev, > - struct device_attribute *attr, > - const char *buf, size_t len) > +static ssize_t create_region_store(struct device *dev, const char *buf, > + size_t len, enum cxl_region_mode mode) > { > struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev); > struct cxl_region *cxlr; > @@ -2565,31 +2564,26 @@ static ssize_t create_pmem_region_store(struct device *dev, > if (rc != 1) > return -EINVAL; > > - cxlr = __create_region(cxlrd, CXL_REGION_PMEM, id); > + cxlr = __create_region(cxlrd, mode, id); > if (IS_ERR(cxlr)) > return PTR_ERR(cxlr); > > return len; > } > + > +static ssize_t create_pmem_region_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t len) > +{ > + return create_region_store(dev, buf, len, CXL_REGION_PMEM); > +} > DEVICE_ATTR_RW(create_pmem_region); > > static ssize_t create_ram_region_store(struct device *dev, > struct device_attribute *attr, > const char *buf, size_t len) > { > - struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev); > - struct cxl_region *cxlr; > - int rc, id; > - > - rc = sscanf(buf, "region%d\n", &id); > - if (rc != 1) > - return -EINVAL; > - > - cxlr = __create_region(cxlrd, CXL_REGION_RAM, id); > - if (IS_ERR(cxlr)) > - return PTR_ERR(cxlr); > - > - return len; > + return create_region_store(dev, buf, len, CXL_REGION_RAM); > } > DEVICE_ATTR_RW(create_ram_region); > >
On 8/16/2024 10:44 PM, Ira Weiny wrote: > create_pmem_region_store() and create_ram_region_store() are identical > with the exception of the region mode. With the addition of DC region > mode this would end up being 3 copies of the same code. > > Refactor create_pmem_region_store() and create_ram_region_store() to use > a single common function to be used in subsequent DC code. > > Suggested-by: Fan Ni <fan.ni@samsung.com> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> Reviewed-by: Li Ming <ming4.li@intel.com>
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index 650fe33f2ed4..f85b26b39b2f 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -2553,9 +2553,8 @@ static struct cxl_region *__create_region(struct cxl_root_decoder *cxlrd, return devm_cxl_add_region(cxlrd, id, mode, CXL_DECODER_HOSTONLYMEM); } -static ssize_t create_pmem_region_store(struct device *dev, - struct device_attribute *attr, - const char *buf, size_t len) +static ssize_t create_region_store(struct device *dev, const char *buf, + size_t len, enum cxl_region_mode mode) { struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev); struct cxl_region *cxlr; @@ -2565,31 +2564,26 @@ static ssize_t create_pmem_region_store(struct device *dev, if (rc != 1) return -EINVAL; - cxlr = __create_region(cxlrd, CXL_REGION_PMEM, id); + cxlr = __create_region(cxlrd, mode, id); if (IS_ERR(cxlr)) return PTR_ERR(cxlr); return len; } + +static ssize_t create_pmem_region_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t len) +{ + return create_region_store(dev, buf, len, CXL_REGION_PMEM); +} DEVICE_ATTR_RW(create_pmem_region); static ssize_t create_ram_region_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t len) { - struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev); - struct cxl_region *cxlr; - int rc, id; - - rc = sscanf(buf, "region%d\n", &id); - if (rc != 1) - return -EINVAL; - - cxlr = __create_region(cxlrd, CXL_REGION_RAM, id); - if (IS_ERR(cxlr)) - return PTR_ERR(cxlr); - - return len; + return create_region_store(dev, buf, len, CXL_REGION_RAM); } DEVICE_ATTR_RW(create_ram_region);
create_pmem_region_store() and create_ram_region_store() are identical with the exception of the region mode. With the addition of DC region mode this would end up being 3 copies of the same code. Refactor create_pmem_region_store() and create_ram_region_store() to use a single common function to be used in subsequent DC code. Suggested-by: Fan Ni <fan.ni@samsung.com> Signed-off-by: Ira Weiny <ira.weiny@intel.com> --- drivers/cxl/core/region.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-)