On Mon, Jan 15, 2018 at 07:42:09PM -0800, Darren Hart wrote:
> To follow this email, are two patches which are minor incremental
> cleanups, which I'd appreciate your review of before I push them to
> testing.
>From 2c5d26ee913e0f5f7d13c559d85e8da7e4ddcdab Mon Sep 17 00:00:00 2001
Message-Id: <2c5d26ee913e0f5f7d13c559d85e8da7e4ddcdab.1516073719.git.dvhart@xxxxxxxxxxxxx>
From: "Darren Hart (VMware)" <dvhart@xxxxxxxxxxxxx>
Date: Mon, 15 Jan 2018 19:16:36 -0800
Subject: [PATCH 1/2] platform/mellanox: mlxreg-hotplug: Simplify exit path
Several functions use a "goto access_error" pattern result in a
duplicated exit path at the end of the function. Address this by using a
single exit path, and checking for (ret) prior to printing the error
message. This adds a conditional, but only in the failure case, and
simplifies the exit path.
Signed-off-by: Darren Hart (VMware) <dvhart@xxxxxxxxxxxxx>
Cc: Vadim Pasternak <vadimp@xxxxxxxxxxxx>
---
drivers/platform/mellanox/mlxreg-hotplug.c | 62 ++++++++++++------------------
1 file changed, 25 insertions(+), 37 deletions(-)
diff --git a/drivers/platform/mellanox/mlxreg-hotplug.c b/drivers/platform/mellanox/mlxreg-hotplug.c
index 9bbaa8c..78056f7 100644
--- a/drivers/platform/mellanox/mlxreg-hotplug.c
+++ b/drivers/platform/mellanox/mlxreg-hotplug.c
@@ -236,12 +236,12 @@ mlxreg_hotplug_work_helper(struct mlxreg_hotplug_priv_data *priv,
ret = regmap_write(priv->regmap, item->reg + MLXREG_HOTPLUG_MASK_OFF,
0);
if (ret)
- goto access_error;
+ goto out;
/* Read status. */
ret = regmap_read(priv->regmap, item->reg, ®val);
if (ret)
- goto access_error;
+ goto out;
/* Set asserted bits and save last status. */
regval &= item->mask;
@@ -267,18 +267,15 @@ mlxreg_hotplug_work_helper(struct mlxreg_hotplug_priv_data *priv,
ret = regmap_write(priv->regmap, item->reg + MLXREG_HOTPLUG_EVENT_OFF,
0);
if (ret)
- goto access_error;
+ goto out:
/* Unmask event. */
ret = regmap_write(priv->regmap, item->reg + MLXREG_HOTPLUG_MASK_OFF,
item->mask);
- if (ret)
- goto access_error;
-
- return;
-access_error:
- dev_err(priv->dev, "Failed to complete workqueue.\n");
+ out:
+ if (ret)
+ dev_err(priv->dev, "Failed to complete workqueue.\n");
}
static void
@@ -294,12 +291,12 @@ mlxreg_hotplug_health_work_helper(struct mlxreg_hotplug_priv_data *priv,
ret = regmap_write(priv->regmap, data->reg +
MLXREG_HOTPLUG_MASK_OFF, 0);
if (ret)
- goto access_error;
+ goto out;
/* Read status. */
ret = regmap_read(priv->regmap, data->reg, ®val);
if (ret)
- goto access_error;
+ goto out;
regval &= data->mask;
item->cache = regval;
@@ -321,19 +318,18 @@ mlxreg_hotplug_health_work_helper(struct mlxreg_hotplug_priv_data *priv,
ret = regmap_write(priv->regmap, data->reg +
MLXREG_HOTPLUG_EVENT_OFF, 0);
if (ret)
- goto access_error;
+ goto out;
/* Unmask event. */
ret = regmap_write(priv->regmap, data->reg +
MLXREG_HOTPLUG_MASK_OFF, data->mask);
if (ret)
- goto access_error;
+ goto out;
}
- return;
-
-access_error:
- dev_err(priv->dev, "Failed to complete workqueue.\n");
+ out:
+ if (ret)
+ dev_err(priv->dev, "Failed to complete workqueue.\n");
}
/*
@@ -380,12 +376,12 @@ static void mlxreg_hotplug_work_handler(struct work_struct *work)
ret = regmap_write(priv->regmap, pdata->cell +
MLXREG_HOTPLUG_AGGR_MASK_OFF, 0);
if (ret < 0)
- goto access_error;
+ goto out;
/* Read aggregation status. */
ret = regmap_read(priv->regmap, pdata->cell, ®val);
if (ret)
- goto access_error;
+ goto out;
regval &= pdata->mask;
aggr_asserted = priv->aggr_cache ^ regval;
@@ -425,13 +421,10 @@ static void mlxreg_hotplug_work_handler(struct work_struct *work)
/* Unmask aggregation event (no need acknowledge). */
ret = regmap_write(priv->regmap, pdata->cell +
MLXREG_HOTPLUG_AGGR_MASK_OFF, pdata->mask);
- if (ret)
- goto access_error;
- return;
-
-access_error:
- dev_err(priv->dev, "Failed to complete workqueue.\n");
+ out:
+ if (ret)
+ dev_err(priv->dev, "Failed to complete workqueue.\n");
}
static int mlxreg_hotplug_set_irq(struct mlxreg_hotplug_priv_data *priv)
@@ -449,7 +442,7 @@ static int mlxreg_hotplug_set_irq(struct mlxreg_hotplug_priv_data *priv)
ret = regmap_write(priv->regmap, item->reg +
MLXREG_HOTPLUG_EVENT_OFF, 0);
if (ret)
- goto access_error;
+ goto out;
/* Set group initial status as mask and unmask group event. */
if (item->inversed) {
@@ -458,7 +451,7 @@ static int mlxreg_hotplug_set_irq(struct mlxreg_hotplug_priv_data *priv)
MLXREG_HOTPLUG_MASK_OFF,
item->mask);
if (ret)
- goto access_error;
+ goto out;
}
}
@@ -466,7 +459,7 @@ static int mlxreg_hotplug_set_irq(struct mlxreg_hotplug_priv_data *priv)
ret = regmap_write(priv->regmap, pdata->cell +
MLXREG_HOTPLUG_AGGR_MASK_OFF, pdata->mask);
if (ret)
- goto access_error;
+ goto out;
/* Keep low aggregation initial status as zero and unmask events. */
if (pdata->cell_low) {
@@ -474,21 +467,16 @@ static int mlxreg_hotplug_set_irq(struct mlxreg_hotplug_priv_data *priv)
MLXREG_HOTPLUG_AGGR_MASK_OFF,
pdata->mask_low);
if (ret)
- goto access_error;
+ goto out;
}
/* Invoke work handler for initializing hot plug devices setting. */
mlxreg_hotplug_work_handler(&priv->dwork_irq.work);
+ out:
+ if (ret)
+ dev_err(priv->dev, "Failed to set interrupts.\n");
enable_irq(priv->irq);
-
- return 0;
-
-access_error:
- dev_err(priv->dev, "Failed to set interrupts.\n");
-
- enable_irq(priv->irq);
-
return ret;
}
--
2.9.4
