From: Asutosh Das <quic_asutoshd@quicinc.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: <martin.petersen@oracle.com>, <jejb@linux.ibm.com>,
<andersson@kernel.org>, <vkoul@kernel.org>,
<quic_cang@quicinc.com>, <linux-arm-msm@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <linux-phy@lists.infradead.org>,
<linux-scsi@vger.kernel.org>, <dmitry.baryshkov@linaro.org>,
<ahalaney@redhat.com>, <abel.vesa@linaro.org>,
<alim.akhtar@samsung.com>, <avri.altman@wdc.com>,
<bvanassche@acm.org>
Subject: Re: [PATCH v4 13/23] scsi: ufs: ufs-qcom: Remove un-necessary goto statements
Date: Mon, 5 Dec 2022 11:53:29 -0800 [thread overview]
Message-ID: <20221205195329.GB15334@asutoshd-linux1.qualcomm.com> (raw)
In-Reply-To: <20221201174328.870152-14-manivannan.sadhasivam@linaro.org>
On Thu, Dec 01 2022 at 09:45 -0800, Manivannan Sadhasivam wrote:
>goto in error path is useful if the function needs to do cleanup other
>than returning the error code. But in this driver, goto statements are
>used for just returning the error code in many places. This really
>makes it hard to read the code.
>
>So let's get rid of those goto statements and just return the error code
>directly.
>
>Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>---
> drivers/ufs/host/ufs-qcom.c | 100 +++++++++++++++---------------------
> 1 file changed, 41 insertions(+), 59 deletions(-)
LGTM.
Reviewed-by: Asutosh Das <quic_asutoshd@quicinc.com>
>
>diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
>index 8ad1415e10b6..7cd996ac180b 100644
>--- a/drivers/ufs/host/ufs-qcom.c
>+++ b/drivers/ufs/host/ufs-qcom.c
>@@ -110,7 +110,7 @@ static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)
>
> static int ufs_qcom_enable_lane_clks(struct ufs_qcom_host *host)
> {
>- int err = 0;
>+ int err;
> struct device *dev = host->hba->dev;
>
> if (host->is_lane_clks_enabled)
>@@ -119,7 +119,7 @@ static int ufs_qcom_enable_lane_clks(struct ufs_qcom_host *host)
> err = ufs_qcom_host_clk_enable(dev, "rx_lane0_sync_clk",
> host->rx_l0_sync_clk);
> if (err)
>- goto out;
>+ return err;
>
> err = ufs_qcom_host_clk_enable(dev, "tx_lane0_sync_clk",
> host->tx_l0_sync_clk);
>@@ -137,7 +137,8 @@ static int ufs_qcom_enable_lane_clks(struct ufs_qcom_host *host)
> goto disable_rx_l1;
>
> host->is_lane_clks_enabled = true;
>- goto out;
>+
>+ return 0;
>
> disable_rx_l1:
> clk_disable_unprepare(host->rx_l1_sync_clk);
>@@ -145,7 +146,7 @@ static int ufs_qcom_enable_lane_clks(struct ufs_qcom_host *host)
> clk_disable_unprepare(host->tx_l0_sync_clk);
> disable_rx_l0:
> clk_disable_unprepare(host->rx_l0_sync_clk);
>-out:
>+
> return err;
> }
>
>@@ -160,25 +161,25 @@ static int ufs_qcom_init_lane_clks(struct ufs_qcom_host *host)
> err = ufs_qcom_host_clk_get(dev, "rx_lane0_sync_clk",
> &host->rx_l0_sync_clk, false);
> if (err)
>- goto out;
>+ return err;
>
> err = ufs_qcom_host_clk_get(dev, "tx_lane0_sync_clk",
> &host->tx_l0_sync_clk, false);
> if (err)
>- goto out;
>+ return err;
>
> /* In case of single lane per direction, don't read lane1 clocks */
> if (host->hba->lanes_per_direction > 1) {
> err = ufs_qcom_host_clk_get(dev, "rx_lane1_sync_clk",
> &host->rx_l1_sync_clk, false);
> if (err)
>- goto out;
>+ return err;
>
> err = ufs_qcom_host_clk_get(dev, "tx_lane1_sync_clk",
> &host->tx_l1_sync_clk, true);
> }
>-out:
>- return err;
>+
>+ return 0;
> }
>
> static int ufs_qcom_check_hibern8(struct ufs_hba *hba)
>@@ -241,7 +242,7 @@ static int ufs_qcom_host_reset(struct ufs_hba *hba)
>
> if (!host->core_reset) {
> dev_warn(hba->dev, "%s: reset control not set\n", __func__);
>- goto out;
>+ return 0;
> }
>
> reenable_intr = hba->is_irq_enabled;
>@@ -252,7 +253,7 @@ static int ufs_qcom_host_reset(struct ufs_hba *hba)
> if (ret) {
> dev_err(hba->dev, "%s: core_reset assert failed, err = %d\n",
> __func__, ret);
>- goto out;
>+ return ret;
> }
>
> /*
>@@ -274,15 +275,14 @@ static int ufs_qcom_host_reset(struct ufs_hba *hba)
> hba->is_irq_enabled = true;
> }
>
>-out:
>- return ret;
>+ return 0;
> }
>
> static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
> {
> struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> struct phy *phy = host->generic_phy;
>- int ret = 0;
>+ int ret;
> bool is_rate_B = UFS_QCOM_LIMIT_HS_RATE == PA_HS_MODE_B;
>
> /* Reset UFS Host Controller and PHY */
>@@ -299,7 +299,7 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
> if (ret) {
> dev_err(hba->dev, "%s: phy init failed, ret = %d\n",
> __func__, ret);
>- goto out;
>+ return ret;
> }
>
> /* power on phy - start serdes and phy's power and clocks */
>@@ -316,7 +316,7 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
>
> out_disable_phy:
> phy_exit(phy);
>-out:
>+
> return ret;
> }
>
>@@ -374,7 +374,6 @@ static int ufs_qcom_hce_enable_notify(struct ufs_hba *hba,
> static int ufs_qcom_cfg_timers(struct ufs_hba *hba, u32 gear,
> u32 hs, u32 rate, bool update_link_startup_timer)
> {
>- int ret = 0;
> struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> struct ufs_clk_info *clki;
> u32 core_clk_period_in_ns;
>@@ -409,11 +408,11 @@ static int ufs_qcom_cfg_timers(struct ufs_hba *hba, u32 gear,
> * Aggregation logic.
> */
> if (ufs_qcom_cap_qunipro(host) && !ufshcd_is_intr_aggr_allowed(hba))
>- goto out;
>+ return 0;
>
> if (gear == 0) {
> dev_err(hba->dev, "%s: invalid gear = %d\n", __func__, gear);
>- goto out_error;
>+ return -EINVAL;
> }
>
> list_for_each_entry(clki, &hba->clk_list_head, list) {
>@@ -436,7 +435,7 @@ static int ufs_qcom_cfg_timers(struct ufs_hba *hba, u32 gear,
> }
>
> if (ufs_qcom_cap_qunipro(host))
>- goto out;
>+ return 0;
>
> core_clk_period_in_ns = NSEC_PER_SEC / core_clk_rate;
> core_clk_period_in_ns <<= OFFSET_CLK_NS_REG;
>@@ -451,7 +450,7 @@ static int ufs_qcom_cfg_timers(struct ufs_hba *hba, u32 gear,
> "%s: index %d exceeds table size %zu\n",
> __func__, gear,
> ARRAY_SIZE(hs_fr_table_rA));
>- goto out_error;
>+ return -EINVAL;
> }
> tx_clk_cycles_per_us = hs_fr_table_rA[gear-1][1];
> } else if (rate == PA_HS_MODE_B) {
>@@ -460,13 +459,13 @@ static int ufs_qcom_cfg_timers(struct ufs_hba *hba, u32 gear,
> "%s: index %d exceeds table size %zu\n",
> __func__, gear,
> ARRAY_SIZE(hs_fr_table_rB));
>- goto out_error;
>+ return -EINVAL;
> }
> tx_clk_cycles_per_us = hs_fr_table_rB[gear-1][1];
> } else {
> dev_err(hba->dev, "%s: invalid rate = %d\n",
> __func__, rate);
>- goto out_error;
>+ return -EINVAL;
> }
> break;
> case SLOWAUTO_MODE:
>@@ -476,14 +475,14 @@ static int ufs_qcom_cfg_timers(struct ufs_hba *hba, u32 gear,
> "%s: index %d exceeds table size %zu\n",
> __func__, gear,
> ARRAY_SIZE(pwm_fr_table));
>- goto out_error;
>+ return -EINVAL;
> }
> tx_clk_cycles_per_us = pwm_fr_table[gear-1][1];
> break;
> case UNCHANGED:
> default:
> dev_err(hba->dev, "%s: invalid mode = %d\n", __func__, hs);
>- goto out_error;
>+ return -EINVAL;
> }
>
> if (ufshcd_readl(hba, REG_UFS_TX_SYMBOL_CLK_NS_US) !=
>@@ -507,12 +506,8 @@ static int ufs_qcom_cfg_timers(struct ufs_hba *hba, u32 gear,
> */
> mb();
> }
>- goto out;
>
>-out_error:
>- ret = -EINVAL;
>-out:
>- return ret;
>+ return 0;
> }
>
> static int ufs_qcom_link_startup_notify(struct ufs_hba *hba,
>@@ -527,8 +522,7 @@ static int ufs_qcom_link_startup_notify(struct ufs_hba *hba,
> 0, true)) {
> dev_err(hba->dev, "%s: ufs_qcom_cfg_timers() failed\n",
> __func__);
>- err = -EINVAL;
>- goto out;
>+ return -EINVAL;
> }
>
> if (ufs_qcom_cap_qunipro(host))
>@@ -554,7 +548,6 @@ static int ufs_qcom_link_startup_notify(struct ufs_hba *hba,
> break;
> }
>
>-out:
> return err;
> }
>
>@@ -691,8 +684,7 @@ static int ufs_qcom_pwr_change_notify(struct ufs_hba *hba,
>
> if (!dev_req_params) {
> pr_err("%s: incoming dev_req_params is NULL\n", __func__);
>- ret = -EINVAL;
>- goto out;
>+ return -EINVAL;
> }
>
> switch (status) {
>@@ -720,7 +712,7 @@ static int ufs_qcom_pwr_change_notify(struct ufs_hba *hba,
> if (ret) {
> pr_err("%s: failed to determine capabilities\n",
> __func__);
>- goto out;
>+ return ret;
> }
>
> /* enable the device ref clock before changing to HS mode */
>@@ -761,7 +753,7 @@ static int ufs_qcom_pwr_change_notify(struct ufs_hba *hba,
> ret = -EINVAL;
> break;
> }
>-out:
>+
> return ret;
> }
>
>@@ -773,14 +765,11 @@ static int ufs_qcom_quirk_host_pa_saveconfigtime(struct ufs_hba *hba)
> err = ufshcd_dme_get(hba, UIC_ARG_MIB(PA_VS_CONFIG_REG1),
> &pa_vs_config_reg1);
> if (err)
>- goto out;
>+ return err;
>
> /* Allow extension of MSB bits of PA_SaveConfigTime attribute */
>- err = ufshcd_dme_set(hba, UIC_ARG_MIB(PA_VS_CONFIG_REG1),
>+ return ufshcd_dme_set(hba, UIC_ARG_MIB(PA_VS_CONFIG_REG1),
> (pa_vs_config_reg1 | (1 << 12)));
>-
>-out:
>- return err;
> }
>
> static int ufs_qcom_apply_dev_quirks(struct ufs_hba *hba)
>@@ -957,9 +946,8 @@ static int ufs_qcom_init(struct ufs_hba *hba)
>
> host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
> if (!host) {
>- err = -ENOMEM;
> dev_err(dev, "%s: no memory for qcom ufs host\n", __func__);
>- goto out;
>+ return -ENOMEM;
> }
>
> /* Make a two way bind between the qcom host and the hba */
>@@ -980,10 +968,8 @@ static int ufs_qcom_init(struct ufs_hba *hba)
> host->rcdev.owner = dev->driver->owner;
> host->rcdev.nr_resets = 1;
> err = devm_reset_controller_register(dev, &host->rcdev);
>- if (err) {
>+ if (err)
> dev_warn(dev, "Failed to register reset controller\n");
>- err = 0;
>- }
>
> if (!has_acpi_companion(dev)) {
> host->generic_phy = devm_phy_get(dev, "ufsphy");
>@@ -1049,17 +1035,16 @@ static int ufs_qcom_init(struct ufs_hba *hba)
> host->dbg_print_en |= UFS_QCOM_DEFAULT_DBG_PRINT_EN;
> ufs_qcom_get_default_testbus_cfg(host);
> err = ufs_qcom_testbus_config(host);
>- if (err) {
>+ if (err)
>+ /* Failure is non-fatal */
> dev_warn(dev, "%s: failed to configure the testbus %d\n",
> __func__, err);
>- err = 0;
>- }
>
>- goto out;
>+ return 0;
>
> out_variant_clear:
> ufshcd_set_variant(hba, NULL);
>-out:
>+
> return err;
> }
>
>@@ -1085,7 +1070,7 @@ static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba,
> UIC_ARG_MIB(DME_VS_CORE_CLK_CTRL),
> &core_clk_ctrl_reg);
> if (err)
>- goto out;
>+ return err;
>
> core_clk_ctrl_reg &= ~DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK;
> core_clk_ctrl_reg |= clk_cycles;
>@@ -1093,11 +1078,9 @@ static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba,
> /* Clear CORE_CLK_DIV_EN */
> core_clk_ctrl_reg &= ~DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT;
>
>- err = ufshcd_dme_set(hba,
>+ return ufshcd_dme_set(hba,
> UIC_ARG_MIB(DME_VS_CORE_CLK_CTRL),
> core_clk_ctrl_reg);
>-out:
>- return err;
> }
>
> static int ufs_qcom_clk_scale_up_pre_change(struct ufs_hba *hba)
>@@ -1180,7 +1163,7 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba,
>
> if (err || !dev_req_params) {
> ufshcd_uic_hibern8_exit(hba);
>- goto out;
>+ return err;
> }
>
> ufs_qcom_cfg_timers(hba,
>@@ -1191,8 +1174,7 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba,
> ufshcd_uic_hibern8_exit(hba);
> }
>
>-out:
>- return err;
>+ return 0;
> }
>
> static void ufs_qcom_print_hw_debug_reg_all(struct ufs_hba *hba,
>--
>2.25.1
>
next prev parent reply other threads:[~2022-12-05 19:55 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-01 17:43 [PATCH v4 00/23] ufs: qcom: Add HS-G4 support Manivannan Sadhasivam
2022-12-01 17:43 ` [PATCH v4 01/23] phy: qcom-qmp-ufs: Remove _tbl suffix from qmp_phy_init_tbl definitions Manivannan Sadhasivam
2022-12-05 21:45 ` Dmitry Baryshkov
2022-12-01 17:43 ` [PATCH v4 02/23] phy: qcom-qmp-ufs: Rename MSM8996 PHY definitions Manivannan Sadhasivam
2022-12-05 21:46 ` Dmitry Baryshkov
2022-12-01 17:43 ` [PATCH v4 03/23] phy: qcom-qmp-ufs: Move register settings to qmp_phy_cfg_tbls struct Manivannan Sadhasivam
2022-12-05 21:48 ` Dmitry Baryshkov
2022-12-01 17:43 ` [PATCH v4 04/23] phy: qcom-qmp-ufs: Add support for configuring PHY in HS Series B mode Manivannan Sadhasivam
2022-12-05 21:51 ` Dmitry Baryshkov
2022-12-06 7:12 ` Manivannan Sadhasivam
2022-12-01 17:43 ` [PATCH v4 05/23] phy: qcom-qmp-ufs: Add support for configuring PHY in HS G4 mode Manivannan Sadhasivam
2022-12-01 17:43 ` [PATCH v4 06/23] phy: qcom-qmp-ufs: Move HS Rate B register setting to tbls_hs_b Manivannan Sadhasivam
2022-12-01 17:43 ` [PATCH v4 07/23] phy: qcom-qmp-ufs: Add HS G4 mode support to SM8150 SoC Manivannan Sadhasivam
2022-12-05 21:52 ` Dmitry Baryshkov
2022-12-01 17:43 ` [PATCH v4 08/23] phy: qcom-qmp-ufs: Add HS G4 mode support to SM8250 SoC Manivannan Sadhasivam
2022-12-05 21:52 ` Dmitry Baryshkov
2022-12-01 17:43 ` [PATCH v4 09/23] phy: qcom-qmp-ufs: Avoid setting HS G3 specific registers Manivannan Sadhasivam
2022-12-05 21:55 ` Dmitry Baryshkov
2022-12-06 7:16 ` Manivannan Sadhasivam
2022-12-01 17:43 ` [PATCH v4 10/23] phy: qcom-qmp-ufs: Add HS G4 mode support to SM8350 SoC Manivannan Sadhasivam
2022-12-05 21:55 ` Dmitry Baryshkov
2022-12-01 17:43 ` [PATCH v4 11/23] phy: qcom-qmp-ufs: Add HS G4 mode support to SM8450 SoC Manivannan Sadhasivam
2022-12-05 21:55 ` Dmitry Baryshkov
2022-12-01 17:43 ` [PATCH v4 12/23] phy: qcom-qmp-ufs: Add HS G4 mode support to SC8280XP SoC Manivannan Sadhasivam
2022-12-05 21:56 ` Dmitry Baryshkov
2022-12-01 17:43 ` [PATCH v4 13/23] scsi: ufs: ufs-qcom: Remove un-necessary goto statements Manivannan Sadhasivam
2022-12-05 19:53 ` Asutosh Das [this message]
2022-12-05 22:26 ` Dmitry Baryshkov
2022-12-01 17:43 ` [PATCH v4 14/23] scsi: ufs: ufs-qcom: Remove un-necessary WARN_ON() Manivannan Sadhasivam
2022-12-05 19:55 ` Asutosh Das
2022-12-01 17:43 ` [PATCH v4 15/23] scsi: ufs: ufs-qcom: Use bitfields where appropriate Manivannan Sadhasivam
2022-12-05 19:56 ` Asutosh Das
2022-12-01 17:43 ` [PATCH v4 16/23] scsi: ufs: ufs-qcom: Use dev_err_probe() for printing probe error Manivannan Sadhasivam
2022-12-05 19:57 ` Asutosh Das
2022-12-01 17:43 ` [PATCH v4 17/23] scsi: ufs: ufs-qcom: Fix the Qcom register name for offset 0xD0 Manivannan Sadhasivam
2022-12-05 19:59 ` Asutosh Das
2022-12-01 17:43 ` [PATCH v4 18/23] scsi: ufs: core: Add reinit_notify() callback Manivannan Sadhasivam
2022-12-01 18:05 ` Bart Van Assche
2022-12-02 7:32 ` Manivannan Sadhasivam
2022-12-01 17:43 ` [PATCH v4 19/23] scsi: ufs: core: Add support for reinitializing the UFS device Manivannan Sadhasivam
2022-12-01 17:43 ` [PATCH v4 20/23] scsi: ufs: ufs-qcom: Factor out the logic finding the HS Gear Manivannan Sadhasivam
2022-12-01 17:43 ` [PATCH v4 21/23] scsi: ufs: ufs-qcom: Add support for reinitializing the UFS device Manivannan Sadhasivam
2022-12-01 17:43 ` [PATCH v4 22/23] scsi: ufs: ufs-qcom: Add support for finding max gear on new platforms Manivannan Sadhasivam
2022-12-01 17:43 ` [PATCH v4 23/23] MAINTAINERS: Add myself as the maintainer for Qcom UFS drivers Manivannan Sadhasivam
2022-12-01 20:15 ` Bjorn Andersson
2022-12-02 20:49 ` [PATCH v4 00/23] ufs: qcom: Add HS-G4 support Andrew Halaney
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221205195329.GB15334@asutoshd-linux1.qualcomm.com \
--to=quic_asutoshd@quicinc.com \
--cc=abel.vesa@linaro.org \
--cc=ahalaney@redhat.com \
--cc=alim.akhtar@samsung.com \
--cc=andersson@kernel.org \
--cc=avri.altman@wdc.com \
--cc=bvanassche@acm.org \
--cc=dmitry.baryshkov@linaro.org \
--cc=jejb@linux.ibm.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-phy@lists.infradead.org \
--cc=linux-scsi@vger.kernel.org \
--cc=manivannan.sadhasivam@linaro.org \
--cc=martin.petersen@oracle.com \
--cc=quic_cang@quicinc.com \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox