| Message ID | 20230419111443.231151-4-raghuhack78@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fixing check patch styling issues | expand |
Raghu H wrote: > The else section here is redundant after return statement, removing it. > Sanity and correctness is not affected due to this fix. > > Signed-off-by: Raghu H <raghuhack78@gmail.com> Ok, per my eyes I would have liked an extra space before the dev_err() but... Reviewed-by: Ira Weiny <ira.weiny@intel.com> > --- > drivers/cxl/core/region.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index f29028148806..1d695107b4a7 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -2666,11 +2666,10 @@ static int cxl_region_invalidate_memregion(struct cxl_region *cxlr) > "Bypassing cpu_cache_invalidate_memregion() for testing!\n"); > clear_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags); > return 0; > - } else { > - dev_err(&cxlr->dev, > - "Failed to synchronize CPU cache state\n"); > - return -ENXIO; > } > + dev_err(&cxlr->dev, > + "Failed to synchronize CPU cache state\n"); > + return -ENXIO; > } > > cpu_cache_invalidate_memregion(IORES_DESC_CXL); > -- > 2.39.2 >
On Thu, Apr 27, 2023 at 12:13:14PM -0700, Ira Weiny wrote: > Raghu H wrote: > > The else section here is redundant after return statement, removing it. > > Sanity and correctness is not affected due to this fix. > > > > Signed-off-by: Raghu H <raghuhack78@gmail.com> > > Ok, per my eyes I would have liked an extra space before the dev_err() > but... Well, I asked Rahgu to give us a v2 with mostly patch formatting fixups, so let's get that extra space too :) > > Reviewed-by: Ira Weiny <ira.weiny@intel.com> > > > --- > > drivers/cxl/core/region.c | 7 +++---- > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > > index f29028148806..1d695107b4a7 100644 > > --- a/drivers/cxl/core/region.c > > +++ b/drivers/cxl/core/region.c > > @@ -2666,11 +2666,10 @@ static int cxl_region_invalidate_memregion(struct cxl_region *cxlr) > > "Bypassing cpu_cache_invalidate_memregion() for testing!\n"); > > clear_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags); > > return 0; > > - } else { > > - dev_err(&cxlr->dev, > > - "Failed to synchronize CPU cache state\n"); > > - return -ENXIO; > > } > > + dev_err(&cxlr->dev, > > + "Failed to synchronize CPU cache state\n"); > > + return -ENXIO; > > } > > > > cpu_cache_invalidate_memregion(IORES_DESC_CXL); > > -- > > 2.39.2 > >
Thanks Alison & Ira for comments I missed your messages due to personal reasons. Will clean and follow it up with v2 very soon. On Fri, Apr 28, 2023 at 1:19 AM Alison Schofield <alison.schofield@intel.com> wrote: > > On Thu, Apr 27, 2023 at 12:13:14PM -0700, Ira Weiny wrote: > > Raghu H wrote: > > > The else section here is redundant after return statement, removing it. > > > Sanity and correctness is not affected due to this fix. > > > > > > Signed-off-by: Raghu H <raghuhack78@gmail.com> > > > > Ok, per my eyes I would have liked an extra space before the dev_err() > > but... > > Well, I asked Rahgu to give us a v2 with mostly patch formatting fixups, > so let's get that extra space too :) > > > > > Reviewed-by: Ira Weiny <ira.weiny@intel.com> > > > > > --- > > > drivers/cxl/core/region.c | 7 +++---- > > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > > > index f29028148806..1d695107b4a7 100644 > > > --- a/drivers/cxl/core/region.c > > > +++ b/drivers/cxl/core/region.c > > > @@ -2666,11 +2666,10 @@ static int cxl_region_invalidate_memregion(struct cxl_region *cxlr) > > > "Bypassing cpu_cache_invalidate_memregion() for testing!\n"); > > > clear_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags); > > > return 0; > > > - } else { > > > - dev_err(&cxlr->dev, > > > - "Failed to synchronize CPU cache state\n"); > > > - return -ENXIO; > > > } > > > + dev_err(&cxlr->dev, > > > + "Failed to synchronize CPU cache state\n"); > > > + return -ENXIO; > > > } > > > > > > cpu_cache_invalidate_memregion(IORES_DESC_CXL); > > > -- > > > 2.39.2 > > >
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index f29028148806..1d695107b4a7 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -2666,11 +2666,10 @@ static int cxl_region_invalidate_memregion(struct cxl_region *cxlr) "Bypassing cpu_cache_invalidate_memregion() for testing!\n"); clear_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags); return 0; - } else { - dev_err(&cxlr->dev, - "Failed to synchronize CPU cache state\n"); - return -ENXIO; } + dev_err(&cxlr->dev, + "Failed to synchronize CPU cache state\n"); + return -ENXIO; } cpu_cache_invalidate_memregion(IORES_DESC_CXL);
The else section here is redundant after return statement, removing it. Sanity and correctness is not affected due to this fix. Signed-off-by: Raghu H <raghuhack78@gmail.com> --- drivers/cxl/core/region.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)