From: "Asutosh Das (asd)" <asutoshd@codeaurora.org>
To: Bean Huo <huobean@gmail.com>,
alim.akhtar@samsung.com, avri.altman@wdc.com, jejb@linux.ibm.com,
martin.petersen@oracle.com, stanley.chu@mediatek.com,
beanhuo@micron.com, bvanassche@acm.org, tomas.winkler@intel.com,
cang@codeaurora.org
Cc: linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 3/8] scsi: ufs: Split ufshcd_probe_hba() based on its called flow
Date: Mon, 20 Jan 2020 12:35:17 -0800 [thread overview]
Message-ID: <d4b28bfb-9d60-5a27-2997-73f7a11d30ed@codeaurora.org> (raw)
In-Reply-To: <20200120130820.1737-4-huobean@gmail.com>
On 1/20/2020 5:08 AM, Bean Huo wrote:
> From: Bean Huo <beanhuo@micron.com>
>
> This patch has two major non-functionality changes:
>
> 1. Take scanning host if-statement out from ufshcd_probe_hba(), and
> move into a new added function ufshcd_add_lus().
> In this new function ufshcd_add_lus(), the main functionalitis include:
> ICC initialization, add well-known LUs, devfreq initialization, UFS
> bsg probe and scsi host scan. The reason for this change is that these
> functionalities only being called during booting stage flow
> ufshcd_init()->ufshcd_async_scan(). In the processes of error handling
> and power management ufshcd_suspend(), ufshcd_resume(), ufshcd_probe_hba()
> being called, but these functionalitis above metioned are not hit.
>
> 2. Move context of initialization of parameters associated with the UFS
> device to a new added function ufshcd_device_params_init().
> The reason of this change is that all of these parameters are used by
> driver, but only need to be initialized once when booting. Combine them
> into an integral function, make them easier maintain.
>
> Signed-off-by: Bean Huo <beanhuo@micron.com>
> ---
LGTM.
Reviewed-by: Asutosh Das <asutoshd@codeaurora.org>
> drivers/scsi/ufs/ufshcd.c | 167 +++++++++++++++++++++++---------------
> 1 file changed, 101 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 58ef45b80cb0..935b50861864 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -246,7 +246,7 @@ static int ufshcd_reset_and_restore(struct ufs_hba *hba);
> static int ufshcd_eh_host_reset_handler(struct scsi_cmnd *cmd);
> static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag);
> static void ufshcd_hba_exit(struct ufs_hba *hba);
> -static int ufshcd_probe_hba(struct ufs_hba *hba);
> +static int ufshcd_probe_hba(struct ufs_hba *hba, bool async);
> static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
> bool skip_ref_clk);
> static int ufshcd_setup_clocks(struct ufs_hba *hba, bool on);
> @@ -6307,7 +6307,7 @@ static int ufshcd_host_reset_and_restore(struct ufs_hba *hba)
> goto out;
>
> /* Establish the link again and restore the device */
> - err = ufshcd_probe_hba(hba);
> + err = ufshcd_probe_hba(hba, false);
>
> if (!err && (hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL))
> err = -EIO;
> @@ -6935,13 +6935,83 @@ static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba)
> return err;
> }
>
> +static int ufshcd_device_params_init(struct ufs_hba *hba)
> +{
> + bool flag;
> + int ret;
> +
> + /* Init check for device descriptor sizes */
> + ufshcd_init_desc_sizes(hba);
> +
> + /* Check and apply UFS device quirks */
> + ret = ufs_get_device_desc(hba);
> + if (ret) {
> + dev_err(hba->dev, "%s: Failed getting device info. err = %d\n",
> + __func__, ret);
> + goto out;
> + }
> +
> + ufs_fixup_device_setup(hba);
> +
> + /* Clear any previous UFS device information */
> + memset(&hba->dev_info, 0, sizeof(hba->dev_info));
> + if (!ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_READ_FLAG,
> + QUERY_FLAG_IDN_PWR_ON_WPE, &flag))
> + hba->dev_info.f_power_on_wp_en = flag;
> +
> +out:
> + return ret;
> +}
> +
> +/**
> + * ufshcd_add_lus - probe and add UFS logical units
> + * @hba: per-adapter instance
> + */
> +static int ufshcd_add_lus(struct ufs_hba *hba)
> +{
> + int ret;
> +
> + if (!hba->is_init_prefetch)
> + ufshcd_init_icc_levels(hba);
> +
> + /* Add required well known logical units to scsi mid layer */
> + ret = ufshcd_scsi_add_wlus(hba);
> + if (ret)
> + goto out;
> +
> + /* Initialize devfreq after UFS device is detected */
> + if (ufshcd_is_clkscaling_supported(hba)) {
> + memcpy(&hba->clk_scaling.saved_pwr_info.info,
> + &hba->pwr_info,
> + sizeof(struct ufs_pa_layer_attr));
> + hba->clk_scaling.saved_pwr_info.is_valid = true;
> + if (!hba->devfreq) {
> + ret = ufshcd_devfreq_init(hba);
> + if (ret)
> + goto out;
> + }
> +
> + hba->clk_scaling.is_allowed = true;
> + }
> +
> + ufs_bsg_probe(hba);
> + scsi_scan_host(hba->host);
> + pm_runtime_put_sync(hba->dev);
> +
> + if (!hba->is_init_prefetch)
> + hba->is_init_prefetch = true;
> +out:
> + return ret;
> +}
> +
> /**
> * ufshcd_probe_hba - probe hba to detect device and initialize
> * @hba: per-adapter instance
> + * @async: asynchronous execution or not
> *
> * Execute link-startup and verify device initialization
> */
> -static int ufshcd_probe_hba(struct ufs_hba *hba)
> +static int ufshcd_probe_hba(struct ufs_hba *hba, bool async)
> {
> int ret;
> ktime_t start = ktime_get();
> @@ -6960,25 +7030,26 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
> /* UniPro link is active now */
> ufshcd_set_link_active(hba);
>
> + /* Verify device initialization by sending NOP OUT UPIU */
> ret = ufshcd_verify_dev_init(hba);
> if (ret)
> goto out;
>
> + /* Initiate UFS initialization, and waiting until completion */
> ret = ufshcd_complete_dev_init(hba);
> if (ret)
> goto out;
>
> - /* Init check for device descriptor sizes */
> - ufshcd_init_desc_sizes(hba);
> -
> - ret = ufs_get_device_desc(hba);
> - if (ret) {
> - dev_err(hba->dev, "%s: Failed getting device info. err = %d\n",
> - __func__, ret);
> - goto out;
> + /*
> + * Initialize UFS device parameters used by driver, these
> + * parameters are associated with UFS descriptors.
> + */
> + if (async) {
> + ret = ufshcd_device_params_init(hba);
> + if (ret)
> + goto out;
> }
>
> - ufs_fixup_device_setup(hba);
> ufshcd_tune_unipro_params(hba);
>
> /* UFS device is also active now */
> @@ -7011,60 +7082,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
> /* Enable Auto-Hibernate if configured */
> ufshcd_auto_hibern8_enable(hba);
>
> - /*
> - * If we are in error handling context or in power management callbacks
> - * context, no need to scan the host
> - */
> - if (!ufshcd_eh_in_progress(hba) && !hba->pm_op_in_progress) {
> - bool flag;
> -
> - /* clear any previous UFS device information */
> - memset(&hba->dev_info, 0, sizeof(hba->dev_info));
> - if (!ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_READ_FLAG,
> - QUERY_FLAG_IDN_PWR_ON_WPE, &flag))
> - hba->dev_info.f_power_on_wp_en = flag;
> -
> - if (!hba->is_init_prefetch)
> - ufshcd_init_icc_levels(hba);
> -
> - /* Add required well known logical units to scsi mid layer */
> - ret = ufshcd_scsi_add_wlus(hba);
> - if (ret)
> - goto out;
> -
> - /* Initialize devfreq after UFS device is detected */
> - if (ufshcd_is_clkscaling_supported(hba)) {
> - memcpy(&hba->clk_scaling.saved_pwr_info.info,
> - &hba->pwr_info,
> - sizeof(struct ufs_pa_layer_attr));
> - hba->clk_scaling.saved_pwr_info.is_valid = true;
> - if (!hba->devfreq) {
> - ret = ufshcd_devfreq_init(hba);
> - if (ret)
> - goto out;
> - }
> - hba->clk_scaling.is_allowed = true;
> - }
> -
> - ufs_bsg_probe(hba);
> -
> - scsi_scan_host(hba->host);
> - pm_runtime_put_sync(hba->dev);
> - }
> -
> - if (!hba->is_init_prefetch)
> - hba->is_init_prefetch = true;
> -
> out:
> - /*
> - * If we failed to initialize the device or the device is not
> - * present, turn off the power/clocks etc.
> - */
> - if (ret && !ufshcd_eh_in_progress(hba) && !hba->pm_op_in_progress) {
> - pm_runtime_put_sync(hba->dev);
> - ufshcd_exit_clk_scaling(hba);
> - ufshcd_hba_exit(hba);
> - }
>
> trace_ufshcd_init(dev_name(hba->dev), ret,
> ktime_to_us(ktime_sub(ktime_get(), start)),
> @@ -7080,8 +7098,25 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
> static void ufshcd_async_scan(void *data, async_cookie_t cookie)
> {
> struct ufs_hba *hba = (struct ufs_hba *)data;
> + int ret;
>
> - ufshcd_probe_hba(hba);
> + /* Initialize hba, detect and initialize UFS device */
> + ret = ufshcd_probe_hba(hba, true);
> + if (ret)
> + goto out;
> +
> + /* Probe and add UFS logical units */
> + ret = ufshcd_add_lus(hba);
> +out:
> + /*
> + * If we failed to initialize the device or the device is not
> + * present, turn off the power/clocks etc.
> + */
> + if (ret) {
> + pm_runtime_put_sync(hba->dev);
> + ufshcd_exit_clk_scaling(hba);
> + ufshcd_hba_exit(hba);
> + }
> }
>
> static const struct attribute_group *ufshcd_driver_groups[] = {
>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project
next prev parent reply other threads:[~2020-01-20 20:35 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-20 13:08 [PATCH v4 0/8] Use UFS device indicated maximum LU number Bean Huo
2020-01-20 13:08 ` [PATCH v4 1/8] scsi: ufs: Fix ufshcd_probe_hba() reture value in case ufshcd_scsi_add_wlus() fails Bean Huo
2020-01-20 13:08 ` [PATCH v4 2/8] scsi: ufs: Delete struct ufs_dev_desc Bean Huo
2020-01-20 13:08 ` [PATCH v4 3/8] scsi: ufs: Split ufshcd_probe_hba() based on its called flow Bean Huo
2020-01-20 20:35 ` Asutosh Das (asd) [this message]
2020-01-20 13:08 ` [PATCH v4 4/8] scsi: ufs: Move ufshcd_get_max_pwr_mode() to ufshcd_device_params_init() Bean Huo
2020-01-20 20:39 ` Asutosh Das (asd)
2020-01-20 13:08 ` [PATCH v4 5/8] scsi: ufs: Inline two functions into their callers Bean Huo
2020-01-20 20:41 ` Asutosh Das (asd)
2020-01-22 5:08 ` Bart Van Assche
2020-01-20 13:08 ` [PATCH v4 6/8] scsi: ufs: Delete is_init_prefetch from struct ufs_hba Bean Huo
2020-01-20 20:43 ` Asutosh Das (asd)
2020-01-22 5:13 ` Bart Van Assche
2020-01-22 10:13 ` [EXT] " Bean Huo (beanhuo)
2020-01-20 13:08 ` [PATCH v4 7/8] scsi: ufs: Add max_lu_supported in struct ufs_dev_info Bean Huo
2020-01-20 13:08 ` [PATCH v4 8/8] scsi: ufs: Use UFS device indicated maximum LU number Bean Huo
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=d4b28bfb-9d60-5a27-2997-73f7a11d30ed@codeaurora.org \
--to=asutoshd@codeaurora.org \
--cc=alim.akhtar@samsung.com \
--cc=avri.altman@wdc.com \
--cc=beanhuo@micron.com \
--cc=bvanassche@acm.org \
--cc=cang@codeaurora.org \
--cc=huobean@gmail.com \
--cc=jejb@linux.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=stanley.chu@mediatek.com \
--cc=tomas.winkler@intel.com \
/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