| Message ID | 20231010082608.859137-3-jtp.park@samsung.com |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cxl: Fix checkpatch issues | expand |
On Tue, 10 Oct 2023 17:26:04 +0900 Jeongtae Park <jtp.park@samsung.com> wrote: > WARNING: else is not generally useful after a break or return > > Since cpu_cache_invalidate_memregion() already checks for > support of invalidaton operation, it can be removed. > This change would make more efficient or small codes > when 'CONFIG_CXL_REGION_INVALIDATION_TEST' is not set. Oddly short line wrap - aim for 75 chars ish. > > Signed-off-by: Jeongtae Park <jtp.park@samsung.com> > --- > drivers/cxl/core/region.c | 20 +++++++------------- > 1 file changed, 7 insertions(+), 13 deletions(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index e115ba382e04..0eb7a12badb9 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -127,21 +127,15 @@ static struct cxl_region_ref *cxl_rr_load(struct cxl_port *port, > > static int cxl_region_invalidate_memregion(struct cxl_region *cxlr) > { > - if (!cpu_cache_has_invalidate_memregion()) { > - if (IS_ENABLED(CONFIG_CXL_REGION_INVALIDATION_TEST)) { > - dev_warn_once( > - &cxlr->dev, > - "Bypassing cpu_cache_invalidate_memregion() for testing!\n"); > - return 0; > - } else { > - dev_err(&cxlr->dev, > - "Failed to synchronize CPU cache state\n"); > - return -ENXIO; > - } > + if (IS_ENABLED(CONFIG_CXL_REGION_INVALIDATION_TEST) > + && cpu_cache_has_invalidate_memregion()) { > + dev_warn_once( > + &cxlr->dev, > + "Bypassing cpu_cache_invalidate_memregion() for testing!\n"); > + return 0; > } > > - cpu_cache_invalidate_memregion(IORES_DESC_CXL); > - return 0; > + return cpu_cache_invalidate_memregion(IORES_DESC_CXL); This is an arch specific call. Whilst today on x86 (only option) the only way to return an error is if it's not supported, that's not necessarily going to be the case for other architectures. So I'd prefer to keep the dev_err if this fails. Jonathan > } > > static int cxl_region_decode_reset(struct cxl_region *cxlr, int count)
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index e115ba382e04..0eb7a12badb9 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -127,21 +127,15 @@ static struct cxl_region_ref *cxl_rr_load(struct cxl_port *port, static int cxl_region_invalidate_memregion(struct cxl_region *cxlr) { - if (!cpu_cache_has_invalidate_memregion()) { - if (IS_ENABLED(CONFIG_CXL_REGION_INVALIDATION_TEST)) { - dev_warn_once( - &cxlr->dev, - "Bypassing cpu_cache_invalidate_memregion() for testing!\n"); - return 0; - } else { - dev_err(&cxlr->dev, - "Failed to synchronize CPU cache state\n"); - return -ENXIO; - } + if (IS_ENABLED(CONFIG_CXL_REGION_INVALIDATION_TEST) + && cpu_cache_has_invalidate_memregion()) { + dev_warn_once( + &cxlr->dev, + "Bypassing cpu_cache_invalidate_memregion() for testing!\n"); + return 0; } - cpu_cache_invalidate_memregion(IORES_DESC_CXL); - return 0; + return cpu_cache_invalidate_memregion(IORES_DESC_CXL); } static int cxl_region_decode_reset(struct cxl_region *cxlr, int count)
WARNING: else is not generally useful after a break or return Since cpu_cache_invalidate_memregion() already checks for support of invalidaton operation, it can be removed. This change would make more efficient or small codes when 'CONFIG_CXL_REGION_INVALIDATION_TEST' is not set. Signed-off-by: Jeongtae Park <jtp.park@samsung.com> --- drivers/cxl/core/region.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)