This series of patches is to simplify UFS driver initialization flow
and add a new parameter max_lu_supported used to specify how many LUs
supported by the UFS device.
This series of patches being tested on my two platforms, Qualcomm SOC
based and Hisilicon SOC based platforms.
v1-v2:
1. Split ufshcd_probe_hba() based on its called flow
2. Delete two unnecessary functions
3. Add a fixup patch
v2-v3:
1. Combine patches 7/9 and 8/9 of v2 to patch 7/8 of v3
2. Change patches 1/8 and 5/8 subject
3. Change the name of two functions in patch 7/8
Bean Huo (8):
scsi: ufs: Fix ufshcd_probe_hba() reture value in case
ufshcd_scsi_add_wlus() fails
scsi: ufs: Delete struct ufs_dev_desc
scsi: ufs: Split ufshcd_probe_hba() based on its called flow
scsi: ufs: move ufshcd_get_max_pwr_mode() to ufs_init_params()
scsi: ufs: Inline two functions into their callers
scsi: ufs: Delete is_init_prefetch from struct ufs_hba
scsi: ufs: Add max_lu_supported in struct ufs_dev_info
scsi: ufs: Use UFS device indicated maximum LU number
drivers/scsi/ufs/ufs-mediatek.c | 7 +-
drivers/scsi/ufs/ufs-qcom.c | 6 +-
drivers/scsi/ufs/ufs-sysfs.c | 2 +-
drivers/scsi/ufs/ufs.h | 25 ++-
drivers/scsi/ufs/ufs_quirks.h | 9 +-
drivers/scsi/ufs/ufshcd.c | 276 +++++++++++++++++++-------------
drivers/scsi/ufs/ufshcd.h | 9 +-
7 files changed, 196 insertions(+), 138 deletions(-)
--
2.17.1
From: Bean Huo <[email protected]>
A non-zero error value likely being returned by ufshcd_scsi_add_wlus()
in case of failure of adding the WLs, but ufshcd_probe_hba() doesn't
use this value, and doesn't report this failure to upper caller.
This patch is to fix this issue.
Fixes: 2a8fa600445c ("ufs: manually add well known logical units")
Reviewed-by: Asutosh Das <[email protected]>
Signed-off-by: Bean Huo <[email protected]>
---
drivers/scsi/ufs/ufshcd.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index bea036ab189a..9a9085a7bcc5 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7032,7 +7032,8 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
ufshcd_init_icc_levels(hba);
/* Add required well known logical units to scsi mid layer */
- if (ufshcd_scsi_add_wlus(hba))
+ ret = ufshcd_scsi_add_wlus(hba);
+ if (ret)
goto out;
/* Initialize devfreq after UFS device is detected */
--
2.17.1
From: Bean Huo <[email protected]>
In consideration of UFS host driver uses parameters of struct
ufs_dev_desc, move its parameters to struct ufs_dev_info,
delete struct ufs_dev_desc.
Reviewed-by: Bart Van Assche <[email protected]>
Reviewed-by: Asutosh Das <[email protected]>
Signed-off-by: Bean Huo <[email protected]>
---
drivers/scsi/ufs/ufs-mediatek.c | 7 ++---
drivers/scsi/ufs/ufs-qcom.c | 6 ++---
drivers/scsi/ufs/ufs.h | 11 +-------
drivers/scsi/ufs/ufs_quirks.h | 9 ++++---
drivers/scsi/ufs/ufshcd.c | 47 +++++++++++++++------------------
drivers/scsi/ufs/ufshcd.h | 7 +++--
6 files changed, 38 insertions(+), 49 deletions(-)
diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index 8d999c0e60fe..f8dd992b6f3a 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -406,10 +406,11 @@ static int ufs_mtk_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
return 0;
}
-static int ufs_mtk_apply_dev_quirks(struct ufs_hba *hba,
- struct ufs_dev_desc *card)
+static int ufs_mtk_apply_dev_quirks(struct ufs_hba *hba)
{
- if (card->wmanufacturerid == UFS_VENDOR_SAMSUNG)
+ struct ufs_dev_info *dev_info = hba->dev_info;
+
+ if (dev_info->wmanufacturerid == UFS_VENDOR_SAMSUNG)
ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TACTIVATE), 6);
return 0;
diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index ebb5c66e069f..9c6a182b3ed9 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -949,12 +949,12 @@ static int ufs_qcom_quirk_host_pa_saveconfigtime(struct ufs_hba *hba)
return err;
}
-static int ufs_qcom_apply_dev_quirks(struct ufs_hba *hba,
- struct ufs_dev_desc *card)
+static int ufs_qcom_apply_dev_quirks(struct ufs_hba *hba)
{
int err = 0;
+ struct ufs_dev_info *dev_info = hba->dev_info;
- if (hba->dev_quirks & UFS_DEVICE_QUIRK_HOST_PA_SAVECONFIGTIME)
+ if (dev_info->dev_quirks & UFS_DEVICE_QUIRK_HOST_PA_SAVECONFIGTIME)
err = ufs_qcom_quirk_host_pa_saveconfigtime(hba);
return err;
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index c89f21698629..fcc9b4d4e56f 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -530,17 +530,8 @@ struct ufs_dev_info {
bool f_power_on_wp_en;
/* Keeps information if any of the LU is power on write protected */
bool is_lu_power_on_wp;
-};
-
-#define MAX_MODEL_LEN 16
-/**
- * ufs_dev_desc - ufs device details from the device descriptor
- *
- * @wmanufacturerid: card details
- * @model: card model
- */
-struct ufs_dev_desc {
u16 wmanufacturerid;
+ /*UFS device Product Name */
u8 *model;
};
diff --git a/drivers/scsi/ufs/ufs_quirks.h b/drivers/scsi/ufs/ufs_quirks.h
index fe6cad9b2a0d..d0ab147f98d3 100644
--- a/drivers/scsi/ufs/ufs_quirks.h
+++ b/drivers/scsi/ufs/ufs_quirks.h
@@ -22,16 +22,17 @@
* @quirk: device quirk
*/
struct ufs_dev_fix {
- struct ufs_dev_desc card;
+ u16 wmanufacturerid;
+ u8 *model;
unsigned int quirk;
};
-#define END_FIX { { 0 }, 0 }
+#define END_FIX { }
/* add specific device quirk */
#define UFS_FIX(_vendor, _model, _quirk) { \
- .card.wmanufacturerid = (_vendor),\
- .card.model = (_model), \
+ .wmanufacturerid = (_vendor),\
+ .model = (_model), \
.quirk = (_quirk), \
}
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 9a9085a7bcc5..58ef45b80cb0 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6583,16 +6583,13 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
return ret;
}
-static int ufs_get_device_desc(struct ufs_hba *hba,
- struct ufs_dev_desc *dev_desc)
+static int ufs_get_device_desc(struct ufs_hba *hba)
{
int err;
size_t buff_len;
u8 model_index;
u8 *desc_buf;
-
- if (!dev_desc)
- return -EINVAL;
+ struct ufs_dev_info *dev_info = &hba->dev_info;
buff_len = max_t(size_t, hba->desc_size.dev_desc,
QUERY_DESC_MAX_SIZE + 1);
@@ -6613,12 +6610,12 @@ static int ufs_get_device_desc(struct ufs_hba *hba,
* getting vendor (manufacturerID) and Bank Index in big endian
* format
*/
- dev_desc->wmanufacturerid = desc_buf[DEVICE_DESC_PARAM_MANF_ID] << 8 |
+ dev_info->wmanufacturerid = desc_buf[DEVICE_DESC_PARAM_MANF_ID] << 8 |
desc_buf[DEVICE_DESC_PARAM_MANF_ID + 1];
model_index = desc_buf[DEVICE_DESC_PARAM_PRDCT_NAME];
err = ufshcd_read_string_desc(hba, model_index,
- &dev_desc->model, SD_ASCII_STD);
+ &dev_info->model, SD_ASCII_STD);
if (err < 0) {
dev_err(hba->dev, "%s: Failed reading Product Name. err = %d\n",
__func__, err);
@@ -6636,23 +6633,25 @@ static int ufs_get_device_desc(struct ufs_hba *hba,
return err;
}
-static void ufs_put_device_desc(struct ufs_dev_desc *dev_desc)
+static void ufs_put_device_desc(struct ufs_hba *hba)
{
- kfree(dev_desc->model);
- dev_desc->model = NULL;
+ struct ufs_dev_info *dev_info = &hba->dev_info;
+
+ kfree(dev_info->model);
+ dev_info->model = NULL;
}
-static void ufs_fixup_device_setup(struct ufs_hba *hba,
- struct ufs_dev_desc *dev_desc)
+static void ufs_fixup_device_setup(struct ufs_hba *hba)
{
struct ufs_dev_fix *f;
+ struct ufs_dev_info *dev_info = &hba->dev_info;
for (f = ufs_fixups; f->quirk; f++) {
- if ((f->card.wmanufacturerid == dev_desc->wmanufacturerid ||
- f->card.wmanufacturerid == UFS_ANY_VENDOR) &&
- ((dev_desc->model &&
- STR_PRFX_EQUAL(f->card.model, dev_desc->model)) ||
- !strcmp(f->card.model, UFS_ANY_MODEL)))
+ if ((f->wmanufacturerid == dev_info->wmanufacturerid ||
+ f->wmanufacturerid == UFS_ANY_VENDOR) &&
+ ((dev_info->model &&
+ STR_PRFX_EQUAL(f->model, dev_info->model)) ||
+ !strcmp(f->model, UFS_ANY_MODEL)))
hba->dev_quirks |= f->quirk;
}
}
@@ -6804,8 +6803,7 @@ static int ufshcd_quirk_tune_host_pa_tactivate(struct ufs_hba *hba)
return ret;
}
-static void ufshcd_tune_unipro_params(struct ufs_hba *hba,
- struct ufs_dev_desc *card)
+static void ufshcd_tune_unipro_params(struct ufs_hba *hba)
{
if (ufshcd_is_unipro_pa_params_tuning_req(hba)) {
ufshcd_tune_pa_tactivate(hba);
@@ -6819,7 +6817,7 @@ static void ufshcd_tune_unipro_params(struct ufs_hba *hba,
if (hba->dev_quirks & UFS_DEVICE_QUIRK_HOST_PA_TACTIVATE)
ufshcd_quirk_tune_host_pa_tactivate(hba);
- ufshcd_vops_apply_dev_quirks(hba, card);
+ ufshcd_vops_apply_dev_quirks(hba);
}
static void ufshcd_clear_dbg_ufs_stats(struct ufs_hba *hba)
@@ -6945,7 +6943,6 @@ static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba)
*/
static int ufshcd_probe_hba(struct ufs_hba *hba)
{
- struct ufs_dev_desc card = {0};
int ret;
ktime_t start = ktime_get();
@@ -6974,16 +6971,15 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
/* Init check for device descriptor sizes */
ufshcd_init_desc_sizes(hba);
- ret = ufs_get_device_desc(hba, &card);
+ 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, &card);
- ufshcd_tune_unipro_params(hba, &card);
- ufs_put_device_desc(&card);
+ ufs_fixup_device_setup(hba);
+ ufshcd_tune_unipro_params(hba);
/* UFS device is also active now */
ufshcd_set_ufs_dev_active(hba);
@@ -7544,6 +7540,7 @@ static void ufshcd_hba_exit(struct ufs_hba *hba)
ufshcd_setup_clocks(hba, false);
ufshcd_setup_hba_vreg(hba, false);
hba->is_powered = false;
+ ufs_put_device_desc(hba);
}
}
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index b1a1c65be8b1..32b6714f25a5 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -320,7 +320,7 @@ struct ufs_hba_variant_ops {
void (*setup_task_mgmt)(struct ufs_hba *, int, u8);
void (*hibern8_notify)(struct ufs_hba *, enum uic_cmd_dme,
enum ufs_notify_change_status);
- int (*apply_dev_quirks)(struct ufs_hba *, struct ufs_dev_desc *);
+ int (*apply_dev_quirks)(struct ufs_hba *hba);
int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
int (*resume)(struct ufs_hba *, enum ufs_pm_op);
void (*dbg_register_dump)(struct ufs_hba *hba);
@@ -1054,11 +1054,10 @@ static inline void ufshcd_vops_hibern8_notify(struct ufs_hba *hba,
return hba->vops->hibern8_notify(hba, cmd, status);
}
-static inline int ufshcd_vops_apply_dev_quirks(struct ufs_hba *hba,
- struct ufs_dev_desc *card)
+static inline int ufshcd_vops_apply_dev_quirks(struct ufs_hba *hba)
{
if (hba->vops && hba->vops->apply_dev_quirks)
- return hba->vops->apply_dev_quirks(hba, card);
+ return hba->vops->apply_dev_quirks(hba);
return 0;
}
--
2.17.1
From: Bean Huo <[email protected]>
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_init_params().
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 <[email protected]>
---
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..54c127ef360b 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_init_params(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_init_params(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[] = {
--
2.17.1
From: Bean Huo <[email protected]>
ufshcd_get_max_pwr_mode() only need to be called once while booting,
take it out from ufshcd_probe_hba() and inline into ufshcd_init_params().
Signed-off-by: Bean Huo <[email protected]>
---
drivers/scsi/ufs/ufshcd.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 54c127ef360b..925b31dc3110 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6959,6 +6959,11 @@ static int ufshcd_init_params(struct ufs_hba *hba)
QUERY_FLAG_IDN_PWR_ON_WPE, &flag))
hba->dev_info.f_power_on_wp_en = flag;
+ /* Probe maximum power mode co-supported by both UFS host and device */
+ if (ufshcd_get_max_pwr_mode(hba))
+ dev_err(hba->dev,
+ "%s: Failed getting max supported power mode\n",
+ __func__);
out:
return ret;
}
@@ -7057,11 +7062,8 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool async)
ufshcd_force_reset_auto_bkops(hba);
hba->wlun_dev_clr_ua = true;
- if (ufshcd_get_max_pwr_mode(hba)) {
- dev_err(hba->dev,
- "%s: Failed getting max supported power mode\n",
- __func__);
- } else {
+ /* Gear up to HS gear if supported */
+ if (hba->max_pwr_info.is_valid) {
/*
* Set the right value to bRefClkFreq before attempting to
* switch to HS gears.
--
2.17.1
From: Bean Huo <[email protected]>
Delete ufshcd_read_power_desc() and ufshcd_read_device_desc(), directly
inline ufshcd_read_desc() into its callers.
Signed-off-by: Bean Huo <[email protected]>
---
drivers/scsi/ufs/ufshcd.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 925b31dc3110..5f3b0ad5135a 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3146,17 +3146,6 @@ static inline int ufshcd_read_desc(struct ufs_hba *hba,
return ufshcd_read_desc_param(hba, desc_id, desc_index, 0, buf, size);
}
-static inline int ufshcd_read_power_desc(struct ufs_hba *hba,
- u8 *buf,
- u32 size)
-{
- return ufshcd_read_desc(hba, QUERY_DESC_IDN_POWER, 0, buf, size);
-}
-
-static int ufshcd_read_device_desc(struct ufs_hba *hba, u8 *buf, u32 size)
-{
- return ufshcd_read_desc(hba, QUERY_DESC_IDN_DEVICE, 0, buf, size);
-}
/**
* struct uc_string_id - unicode string
@@ -6493,7 +6482,8 @@ static void ufshcd_init_icc_levels(struct ufs_hba *hba)
if (!desc_buf)
return;
- ret = ufshcd_read_power_desc(hba, desc_buf, buff_len);
+ ret = ufshcd_read_desc(hba, QUERY_DESC_IDN_POWER, 0,
+ desc_buf, buff_len);
if (ret) {
dev_err(hba->dev,
"%s: Failed reading power descriptor.len = %d ret = %d",
@@ -6599,7 +6589,8 @@ static int ufs_get_device_desc(struct ufs_hba *hba)
goto out;
}
- err = ufshcd_read_device_desc(hba, desc_buf, hba->desc_size.dev_desc);
+ err = ufshcd_read_desc(hba, QUERY_DESC_IDN_DEVICE, 0, desc_buf,
+ hba->desc_size.dev_desc);
if (err) {
dev_err(hba->dev, "%s: Failed reading Device Desc. err = %d\n",
__func__, err);
--
2.17.1
From: Bean Huo <[email protected]>
Without variable is_init_prefetch, the current logic can guarantee
ufshcd_init_icc_levels() will execute only once, delete it now.
Signed-off-by: Bean Huo <[email protected]>
---
drivers/scsi/ufs/ufshcd.c | 5 +----
drivers/scsi/ufs/ufshcd.h | 2 --
2 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 5f3b0ad5135a..4f8fcbb5f92e 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6967,8 +6967,7 @@ static int ufshcd_add_lus(struct ufs_hba *hba)
{
int ret;
- if (!hba->is_init_prefetch)
- ufshcd_init_icc_levels(hba);
+ ufshcd_init_icc_levels(hba);
/* Add required well known logical units to scsi mid layer */
ret = ufshcd_scsi_add_wlus(hba);
@@ -6994,8 +6993,6 @@ static int ufshcd_add_lus(struct ufs_hba *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;
}
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 32b6714f25a5..5c65d9fdeb14 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -501,7 +501,6 @@ struct ufs_stats {
* @intr_mask: Interrupt Mask Bits
* @ee_ctrl_mask: Exception event control mask
* @is_powered: flag to check if HBA is powered
- * @is_init_prefetch: flag to check if data was pre-fetched in initialization
* @init_prefetch_data: data pre-fetched during initialization
* @eh_work: Worker to handle UFS errors that require s/w attention
* @eeh_work: Worker to handle exception events
@@ -652,7 +651,6 @@ struct ufs_hba {
u32 intr_mask;
u16 ee_ctrl_mask;
bool is_powered;
- bool is_init_prefetch;
struct ufs_init_prefetch init_prefetch_data;
/* Work Queues */
--
2.17.1
From: Bean Huo <[email protected]>
Add one new parameter max_lu_supported in struct ufs_dev_info,
which will be used to express exactly how many general LUs being
supported by UFS device, and initialize it during booting stage.
This patch also adds a new function ufshcd_init_device_geo_params()
for initialization of UFS device geometry descriptor related parameters.
Reviewed-by: Asutosh Das <[email protected]>
Signed-off-by: Bean Huo <[email protected]>
---
drivers/scsi/ufs/ufs.h | 2 ++
drivers/scsi/ufs/ufshcd.c | 41 +++++++++++++++++++++++++++++++++++++--
2 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index fcc9b4d4e56f..c982bcc94662 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -530,6 +530,8 @@ struct ufs_dev_info {
bool f_power_on_wp_en;
/* Keeps information if any of the LU is power on write protected */
bool is_lu_power_on_wp;
+ /* Maximum number of general LU supported by the UFS device */
+ u8 max_lu_supported;
u16 wmanufacturerid;
/*UFS device Product Name */
u8 *model;
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 4f8fcbb5f92e..dd10558f4d01 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6858,6 +6858,37 @@ static void ufshcd_init_desc_sizes(struct ufs_hba *hba)
hba->desc_size.hlth_desc = QUERY_DESC_HEALTH_DEF_SIZE;
}
+static int ufshcd_init_device_geo_params(struct ufs_hba *hba)
+{
+ int err;
+ size_t buff_len;
+ u8 *desc_buf;
+
+ buff_len = hba->desc_size.geom_desc;
+ desc_buf = kmalloc(buff_len, GFP_KERNEL);
+ if (!desc_buf) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ err = ufshcd_read_desc(hba, QUERY_DESC_IDN_GEOMETRY, 0,
+ desc_buf, buff_len);
+ if (err) {
+ dev_err(hba->dev, "%s: Failed reading Geometry Desc. err = %d\n",
+ __func__, err);
+ goto out;
+ }
+
+ if (desc_buf[GEOMETRY_DESC_PARAM_MAX_NUM_LUN] == 1)
+ hba->dev_info.max_lu_supported = 32;
+ else if (desc_buf[GEOMETRY_DESC_PARAM_MAX_NUM_LUN] == 0)
+ hba->dev_info.max_lu_supported = 8;
+
+out:
+ kfree(desc_buf);
+ return err;
+}
+
static struct ufs_ref_clk ufs_ref_clk_freqs[] = {
{19200000, REF_CLK_FREQ_19_2_MHZ},
{26000000, REF_CLK_FREQ_26_MHZ},
@@ -6931,9 +6962,17 @@ static int ufshcd_init_params(struct ufs_hba *hba)
bool flag;
int ret;
+ /* Clear any previous UFS device information */
+ memset(&hba->dev_info, 0, sizeof(hba->dev_info));
+
/* Init check for device descriptor sizes */
ufshcd_init_desc_sizes(hba);
+ /* Init UFS geometry descriptor related parameters */
+ ret = ufshcd_init_device_geo_params(hba);
+ if (ret)
+ goto out;
+
/* Check and apply UFS device quirks */
ret = ufs_get_device_desc(hba);
if (ret) {
@@ -6944,8 +6983,6 @@ static int ufshcd_init_params(struct ufs_hba *hba)
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;
--
2.17.1
From: Bean Huo <[email protected]>
According to Jedec standard UFS 3.0 and UFS 2.1 Spec, Maximum number
of logical units supported by the UFS device is indicated by parameter
bMaxNumberLU in Geometry Descriptor. This patch is to delete current
hard code macro definition of UFS_UPIU_MAX_GENERAL_LUN, and switch to
use device indicated number instead.
Reviewed-by: Asutosh Das <[email protected]>
Signed-off-by: Bean Huo <[email protected]>
---
drivers/scsi/ufs/ufs-sysfs.c | 2 +-
drivers/scsi/ufs/ufs.h | 12 +++++++++---
drivers/scsi/ufs/ufshcd.c | 4 ++--
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
index 720be3f64be7..dbdf8b01abed 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -713,7 +713,7 @@ static ssize_t _pname##_show(struct device *dev, \
struct scsi_device *sdev = to_scsi_device(dev); \
struct ufs_hba *hba = shost_priv(sdev->host); \
u8 lun = ufshcd_scsi_to_upiu_lun(sdev->lun); \
- if (!ufs_is_valid_unit_desc_lun(lun)) \
+ if (!ufs_is_valid_unit_desc_lun(&hba->dev_info, lun)) \
return -EINVAL; \
return ufs_sysfs_read_desc_param(hba, QUERY_DESC_IDN_##_duname, \
lun, _duname##_DESC_PARAM##_puname, buf, _size); \
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index c982bcc94662..dde2eb02f76f 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -63,7 +63,6 @@
#define UFS_UPIU_MAX_UNIT_NUM_ID 0x7F
#define UFS_MAX_LUNS (SCSI_W_LUN_BASE + UFS_UPIU_MAX_UNIT_NUM_ID)
#define UFS_UPIU_WLUN_ID (1 << 7)
-#define UFS_UPIU_MAX_GENERAL_LUN 8
/* Well known logical unit id in LUN field of UPIU */
enum {
@@ -539,12 +538,19 @@ struct ufs_dev_info {
/**
* ufs_is_valid_unit_desc_lun - checks if the given LUN has a unit descriptor
+ * @dev_info: pointer of instance of struct ufs_dev_info
* @lun: LU number to check
* @return: true if the lun has a matching unit descriptor, false otherwise
*/
-static inline bool ufs_is_valid_unit_desc_lun(u8 lun)
+static inline bool ufs_is_valid_unit_desc_lun(struct ufs_dev_info *dev_info,
+ u8 lun)
{
- return lun == UFS_UPIU_RPMB_WLUN || (lun < UFS_UPIU_MAX_GENERAL_LUN);
+ if (!dev_info || !dev_info->max_lu_supported) {
+ pr_err("Max General LU supported by UFS isn't initilized\n");
+ return false;
+ }
+
+ return lun == UFS_UPIU_RPMB_WLUN || (lun < dev_info->max_lu_supported);
}
#endif /* End of Header */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index dd10558f4d01..bf714221455e 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3270,7 +3270,7 @@ static inline int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
* Unit descriptors are only available for general purpose LUs (LUN id
* from 0 to 7) and RPMB Well known LU.
*/
- if (!ufs_is_valid_unit_desc_lun(lun))
+ if (!ufs_is_valid_unit_desc_lun(&hba->dev_info, lun))
return -EOPNOTSUPP;
return ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, lun,
@@ -4525,7 +4525,7 @@ static int ufshcd_get_lu_wp(struct ufs_hba *hba,
* protected so skip reading bLUWriteProtect parameter for
* it. For other W-LUs, UNIT DESCRIPTOR is not available.
*/
- else if (lun >= UFS_UPIU_MAX_GENERAL_LUN)
+ else if (lun >= hba->dev_info.max_lu_supported)
ret = -ENOTSUPP;
else
ret = ufshcd_read_unit_desc_param(hba,
--
2.17.1
Hi Bean
On Sun, Jan 19, 2020 at 5:44 AM Bean Huo <[email protected]> wrote:
>
> From: Bean Huo <[email protected]>
>
> A non-zero error value likely being returned by ufshcd_scsi_add_wlus()
> in case of failure of adding the WLs, but ufshcd_probe_hba() doesn't
> use this value, and doesn't report this failure to upper caller.
> This patch is to fix this issue.
>
> Fixes: 2a8fa600445c ("ufs: manually add well known logical units")
> Reviewed-by: Asutosh Das <[email protected]>
> Signed-off-by: Bean Huo <[email protected]>
> ---
Signed-off-by: Alim Akhtar <[email protected]>
> drivers/scsi/ufs/ufshcd.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index bea036ab189a..9a9085a7bcc5 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -7032,7 +7032,8 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
> ufshcd_init_icc_levels(hba);
>
> /* Add required well known logical units to scsi mid layer */
> - if (ufshcd_scsi_add_wlus(hba))
> + ret = ufshcd_scsi_add_wlus(hba);
> + if (ret)
> goto out;
>
> /* Initialize devfreq after UFS device is detected */
> --
> 2.17.1
>
--
Regards,
Alim
Hi Bean
On Sun, Jan 19, 2020 at 7:00 AM Alim Akhtar <[email protected]> wrote:
>
> Hi Bean
>
> On Sun, Jan 19, 2020 at 5:44 AM Bean Huo <[email protected]> wrote:
> >
> > From: Bean Huo <[email protected]>
> >
> > A non-zero error value likely being returned by ufshcd_scsi_add_wlus()
> > in case of failure of adding the WLs, but ufshcd_probe_hba() doesn't
> > use this value, and doesn't report this failure to upper caller.
> > This patch is to fix this issue.
> >
> > Fixes: 2a8fa600445c ("ufs: manually add well known logical units")
> > Reviewed-by: Asutosh Das <[email protected]>
> > Signed-off-by: Bean Huo <[email protected]>
> > ---
> Signed-off-by: Alim Akhtar <[email protected]>
>
Sorry for noise, its not signed-off, please have my
Reviewed-by: Alim Akhtar <[email protected]>
> > drivers/scsi/ufs/ufshcd.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index bea036ab189a..9a9085a7bcc5 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -7032,7 +7032,8 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
> > ufshcd_init_icc_levels(hba);
> >
> > /* Add required well known logical units to scsi mid layer */
> > - if (ufshcd_scsi_add_wlus(hba))
> > + ret = ufshcd_scsi_add_wlus(hba);
> > + if (ret)
> > goto out;
> >
> > /* Initialize devfreq after UFS device is detected */
> > --
> > 2.17.1
> >
>
>
> --
> Regards,
> Alim
--
Regards,
Alim
Hi Bean
On Sun, Jan 19, 2020 at 5:44 AM Bean Huo <[email protected]> wrote:
>
> From: Bean Huo <[email protected]>
>
> In consideration of UFS host driver uses parameters of struct
> ufs_dev_desc, move its parameters to struct ufs_dev_info,
> delete struct ufs_dev_desc.
>
> Reviewed-by: Bart Van Assche <[email protected]>
> Reviewed-by: Asutosh Das <[email protected]>
> Signed-off-by: Bean Huo <[email protected]>
> ---
Reviewed-by: Alim Akhtar <[email protected]>
> drivers/scsi/ufs/ufs-mediatek.c | 7 ++---
> drivers/scsi/ufs/ufs-qcom.c | 6 ++---
> drivers/scsi/ufs/ufs.h | 11 +-------
> drivers/scsi/ufs/ufs_quirks.h | 9 ++++---
> drivers/scsi/ufs/ufshcd.c | 47 +++++++++++++++------------------
> drivers/scsi/ufs/ufshcd.h | 7 +++--
> 6 files changed, 38 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
> index 8d999c0e60fe..f8dd992b6f3a 100644
> --- a/drivers/scsi/ufs/ufs-mediatek.c
> +++ b/drivers/scsi/ufs/ufs-mediatek.c
> @@ -406,10 +406,11 @@ static int ufs_mtk_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
> return 0;
> }
>
> -static int ufs_mtk_apply_dev_quirks(struct ufs_hba *hba,
> - struct ufs_dev_desc *card)
> +static int ufs_mtk_apply_dev_quirks(struct ufs_hba *hba)
> {
> - if (card->wmanufacturerid == UFS_VENDOR_SAMSUNG)
> + struct ufs_dev_info *dev_info = hba->dev_info;
> +
> + if (dev_info->wmanufacturerid == UFS_VENDOR_SAMSUNG)
> ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TACTIVATE), 6);
>
> return 0;
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index ebb5c66e069f..9c6a182b3ed9 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -949,12 +949,12 @@ static int ufs_qcom_quirk_host_pa_saveconfigtime(struct ufs_hba *hba)
> return err;
> }
>
> -static int ufs_qcom_apply_dev_quirks(struct ufs_hba *hba,
> - struct ufs_dev_desc *card)
> +static int ufs_qcom_apply_dev_quirks(struct ufs_hba *hba)
> {
> int err = 0;
> + struct ufs_dev_info *dev_info = hba->dev_info;
>
> - if (hba->dev_quirks & UFS_DEVICE_QUIRK_HOST_PA_SAVECONFIGTIME)
> + if (dev_info->dev_quirks & UFS_DEVICE_QUIRK_HOST_PA_SAVECONFIGTIME)
> err = ufs_qcom_quirk_host_pa_saveconfigtime(hba);
>
> return err;
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index c89f21698629..fcc9b4d4e56f 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -530,17 +530,8 @@ struct ufs_dev_info {
> bool f_power_on_wp_en;
> /* Keeps information if any of the LU is power on write protected */
> bool is_lu_power_on_wp;
> -};
> -
> -#define MAX_MODEL_LEN 16
> -/**
> - * ufs_dev_desc - ufs device details from the device descriptor
> - *
> - * @wmanufacturerid: card details
> - * @model: card model
> - */
> -struct ufs_dev_desc {
> u16 wmanufacturerid;
> + /*UFS device Product Name */
> u8 *model;
> };
>
> diff --git a/drivers/scsi/ufs/ufs_quirks.h b/drivers/scsi/ufs/ufs_quirks.h
> index fe6cad9b2a0d..d0ab147f98d3 100644
> --- a/drivers/scsi/ufs/ufs_quirks.h
> +++ b/drivers/scsi/ufs/ufs_quirks.h
> @@ -22,16 +22,17 @@
> * @quirk: device quirk
> */
> struct ufs_dev_fix {
> - struct ufs_dev_desc card;
> + u16 wmanufacturerid;
> + u8 *model;
> unsigned int quirk;
> };
>
> -#define END_FIX { { 0 }, 0 }
> +#define END_FIX { }
>
> /* add specific device quirk */
> #define UFS_FIX(_vendor, _model, _quirk) { \
> - .card.wmanufacturerid = (_vendor),\
> - .card.model = (_model), \
> + .wmanufacturerid = (_vendor),\
> + .model = (_model), \
> .quirk = (_quirk), \
> }
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 9a9085a7bcc5..58ef45b80cb0 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -6583,16 +6583,13 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
> return ret;
> }
>
> -static int ufs_get_device_desc(struct ufs_hba *hba,
> - struct ufs_dev_desc *dev_desc)
> +static int ufs_get_device_desc(struct ufs_hba *hba)
> {
> int err;
> size_t buff_len;
> u8 model_index;
> u8 *desc_buf;
> -
> - if (!dev_desc)
> - return -EINVAL;
> + struct ufs_dev_info *dev_info = &hba->dev_info;
>
> buff_len = max_t(size_t, hba->desc_size.dev_desc,
> QUERY_DESC_MAX_SIZE + 1);
> @@ -6613,12 +6610,12 @@ static int ufs_get_device_desc(struct ufs_hba *hba,
> * getting vendor (manufacturerID) and Bank Index in big endian
> * format
> */
> - dev_desc->wmanufacturerid = desc_buf[DEVICE_DESC_PARAM_MANF_ID] << 8 |
> + dev_info->wmanufacturerid = desc_buf[DEVICE_DESC_PARAM_MANF_ID] << 8 |
> desc_buf[DEVICE_DESC_PARAM_MANF_ID + 1];
>
> model_index = desc_buf[DEVICE_DESC_PARAM_PRDCT_NAME];
> err = ufshcd_read_string_desc(hba, model_index,
> - &dev_desc->model, SD_ASCII_STD);
> + &dev_info->model, SD_ASCII_STD);
> if (err < 0) {
> dev_err(hba->dev, "%s: Failed reading Product Name. err = %d\n",
> __func__, err);
> @@ -6636,23 +6633,25 @@ static int ufs_get_device_desc(struct ufs_hba *hba,
> return err;
> }
>
> -static void ufs_put_device_desc(struct ufs_dev_desc *dev_desc)
> +static void ufs_put_device_desc(struct ufs_hba *hba)
> {
> - kfree(dev_desc->model);
> - dev_desc->model = NULL;
> + struct ufs_dev_info *dev_info = &hba->dev_info;
> +
> + kfree(dev_info->model);
> + dev_info->model = NULL;
> }
>
> -static void ufs_fixup_device_setup(struct ufs_hba *hba,
> - struct ufs_dev_desc *dev_desc)
> +static void ufs_fixup_device_setup(struct ufs_hba *hba)
> {
> struct ufs_dev_fix *f;
> + struct ufs_dev_info *dev_info = &hba->dev_info;
>
> for (f = ufs_fixups; f->quirk; f++) {
> - if ((f->card.wmanufacturerid == dev_desc->wmanufacturerid ||
> - f->card.wmanufacturerid == UFS_ANY_VENDOR) &&
> - ((dev_desc->model &&
> - STR_PRFX_EQUAL(f->card.model, dev_desc->model)) ||
> - !strcmp(f->card.model, UFS_ANY_MODEL)))
> + if ((f->wmanufacturerid == dev_info->wmanufacturerid ||
> + f->wmanufacturerid == UFS_ANY_VENDOR) &&
> + ((dev_info->model &&
> + STR_PRFX_EQUAL(f->model, dev_info->model)) ||
> + !strcmp(f->model, UFS_ANY_MODEL)))
> hba->dev_quirks |= f->quirk;
> }
> }
> @@ -6804,8 +6803,7 @@ static int ufshcd_quirk_tune_host_pa_tactivate(struct ufs_hba *hba)
> return ret;
> }
>
> -static void ufshcd_tune_unipro_params(struct ufs_hba *hba,
> - struct ufs_dev_desc *card)
> +static void ufshcd_tune_unipro_params(struct ufs_hba *hba)
> {
> if (ufshcd_is_unipro_pa_params_tuning_req(hba)) {
> ufshcd_tune_pa_tactivate(hba);
> @@ -6819,7 +6817,7 @@ static void ufshcd_tune_unipro_params(struct ufs_hba *hba,
> if (hba->dev_quirks & UFS_DEVICE_QUIRK_HOST_PA_TACTIVATE)
> ufshcd_quirk_tune_host_pa_tactivate(hba);
>
> - ufshcd_vops_apply_dev_quirks(hba, card);
> + ufshcd_vops_apply_dev_quirks(hba);
> }
>
> static void ufshcd_clear_dbg_ufs_stats(struct ufs_hba *hba)
> @@ -6945,7 +6943,6 @@ static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba)
> */
> static int ufshcd_probe_hba(struct ufs_hba *hba)
> {
> - struct ufs_dev_desc card = {0};
> int ret;
> ktime_t start = ktime_get();
>
> @@ -6974,16 +6971,15 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
> /* Init check for device descriptor sizes */
> ufshcd_init_desc_sizes(hba);
>
> - ret = ufs_get_device_desc(hba, &card);
> + 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, &card);
> - ufshcd_tune_unipro_params(hba, &card);
> - ufs_put_device_desc(&card);
> + ufs_fixup_device_setup(hba);
> + ufshcd_tune_unipro_params(hba);
>
> /* UFS device is also active now */
> ufshcd_set_ufs_dev_active(hba);
> @@ -7544,6 +7540,7 @@ static void ufshcd_hba_exit(struct ufs_hba *hba)
> ufshcd_setup_clocks(hba, false);
> ufshcd_setup_hba_vreg(hba, false);
> hba->is_powered = false;
> + ufs_put_device_desc(hba);
> }
> }
>
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index b1a1c65be8b1..32b6714f25a5 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -320,7 +320,7 @@ struct ufs_hba_variant_ops {
> void (*setup_task_mgmt)(struct ufs_hba *, int, u8);
> void (*hibern8_notify)(struct ufs_hba *, enum uic_cmd_dme,
> enum ufs_notify_change_status);
> - int (*apply_dev_quirks)(struct ufs_hba *, struct ufs_dev_desc *);
> + int (*apply_dev_quirks)(struct ufs_hba *hba);
> int (*suspend)(struct ufs_hba *, enum ufs_pm_op);
> int (*resume)(struct ufs_hba *, enum ufs_pm_op);
> void (*dbg_register_dump)(struct ufs_hba *hba);
> @@ -1054,11 +1054,10 @@ static inline void ufshcd_vops_hibern8_notify(struct ufs_hba *hba,
> return hba->vops->hibern8_notify(hba, cmd, status);
> }
>
> -static inline int ufshcd_vops_apply_dev_quirks(struct ufs_hba *hba,
> - struct ufs_dev_desc *card)
> +static inline int ufshcd_vops_apply_dev_quirks(struct ufs_hba *hba)
> {
> if (hba->vops && hba->vops->apply_dev_quirks)
> - return hba->vops->apply_dev_quirks(hba, card);
> + return hba->vops->apply_dev_quirks(hba);
> return 0;
> }
>
> --
> 2.17.1
>
--
Regards,
Alim
On Sun, Jan 19, 2020 at 5:45 AM Bean Huo <[email protected]> wrote:
>
> From: Bean Huo <[email protected]>
>
> 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_init_params().
Since you are moving UFS device initialization related code, this
function can be renamed as
ufshcd_device_params_init() to make it more obvious about the function.
whats your thought on this?
> 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 <[email protected]>
> ---
> 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..54c127ef360b 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_init_params(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_init_params(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[] = {
> --
> 2.17.1
>
--
Regards,
Alim
On Sun, Jan 19, 2020 at 5:44 AM Bean Huo <[email protected]> wrote:
>
> From: Bean Huo <[email protected]>
>
> ufshcd_get_max_pwr_mode() only need to be called once while booting,
> take it out from ufshcd_probe_hba() and inline into ufshcd_init_params().
>
This can be safely squash with the previous patch which introduce
ufshcd_init_params.
> Signed-off-by: Bean Huo <[email protected]>
> ---
> drivers/scsi/ufs/ufshcd.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 54c127ef360b..925b31dc3110 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -6959,6 +6959,11 @@ static int ufshcd_init_params(struct ufs_hba *hba)
> QUERY_FLAG_IDN_PWR_ON_WPE, &flag))
> hba->dev_info.f_power_on_wp_en = flag;
>
> + /* Probe maximum power mode co-supported by both UFS host and device */
> + if (ufshcd_get_max_pwr_mode(hba))
> + dev_err(hba->dev,
> + "%s: Failed getting max supported power mode\n",
> + __func__);
> out:
> return ret;
> }
> @@ -7057,11 +7062,8 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool async)
> ufshcd_force_reset_auto_bkops(hba);
> hba->wlun_dev_clr_ua = true;
>
> - if (ufshcd_get_max_pwr_mode(hba)) {
> - dev_err(hba->dev,
> - "%s: Failed getting max supported power mode\n",
> - __func__);
> - } else {
> + /* Gear up to HS gear if supported */
> + if (hba->max_pwr_info.is_valid) {
> /*
> * Set the right value to bRefClkFreq before attempting to
> * switch to HS gears.
> --
> 2.17.1
>
--
Regards,
Alim
On Sun, Jan 19, 2020 at 5:45 AM Bean Huo <[email protected]> wrote:
>
> From: Bean Huo <[email protected]>
>
> Delete ufshcd_read_power_desc() and ufshcd_read_device_desc(), directly
> inline ufshcd_read_desc() into its callers.
>
> Signed-off-by: Bean Huo <[email protected]>
> ---
Reviewed-by: Alim Akhtar <[email protected]>
> drivers/scsi/ufs/ufshcd.c | 17 ++++-------------
> 1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 925b31dc3110..5f3b0ad5135a 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -3146,17 +3146,6 @@ static inline int ufshcd_read_desc(struct ufs_hba *hba,
> return ufshcd_read_desc_param(hba, desc_id, desc_index, 0, buf, size);
> }
>
> -static inline int ufshcd_read_power_desc(struct ufs_hba *hba,
> - u8 *buf,
> - u32 size)
> -{
> - return ufshcd_read_desc(hba, QUERY_DESC_IDN_POWER, 0, buf, size);
> -}
> -
> -static int ufshcd_read_device_desc(struct ufs_hba *hba, u8 *buf, u32 size)
> -{
> - return ufshcd_read_desc(hba, QUERY_DESC_IDN_DEVICE, 0, buf, size);
> -}
>
> /**
> * struct uc_string_id - unicode string
> @@ -6493,7 +6482,8 @@ static void ufshcd_init_icc_levels(struct ufs_hba *hba)
> if (!desc_buf)
> return;
>
> - ret = ufshcd_read_power_desc(hba, desc_buf, buff_len);
> + ret = ufshcd_read_desc(hba, QUERY_DESC_IDN_POWER, 0,
> + desc_buf, buff_len);
> if (ret) {
> dev_err(hba->dev,
> "%s: Failed reading power descriptor.len = %d ret = %d",
> @@ -6599,7 +6589,8 @@ static int ufs_get_device_desc(struct ufs_hba *hba)
> goto out;
> }
>
> - err = ufshcd_read_device_desc(hba, desc_buf, hba->desc_size.dev_desc);
> + err = ufshcd_read_desc(hba, QUERY_DESC_IDN_DEVICE, 0, desc_buf,
> + hba->desc_size.dev_desc);
> if (err) {
> dev_err(hba->dev, "%s: Failed reading Device Desc. err = %d\n",
> __func__, err);
> --
> 2.17.1
>
--
Regards,
Alim
On Sun, Jan 19, 2020 at 5:45 AM Bean Huo <[email protected]> wrote:
>
> From: Bean Huo <[email protected]>
>
> Add one new parameter max_lu_supported in struct ufs_dev_info,
> which will be used to express exactly how many general LUs being
> supported by UFS device, and initialize it during booting stage.
> This patch also adds a new function ufshcd_init_device_geo_params()
> for initialization of UFS device geometry descriptor related parameters.
>
> Reviewed-by: Asutosh Das <[email protected]>
> Signed-off-by: Bean Huo <[email protected]>
> ---
Reviewed-by: Alim Akhtar <[email protected]>
> drivers/scsi/ufs/ufs.h | 2 ++
> drivers/scsi/ufs/ufshcd.c | 41 +++++++++++++++++++++++++++++++++++++--
> 2 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index fcc9b4d4e56f..c982bcc94662 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -530,6 +530,8 @@ struct ufs_dev_info {
> bool f_power_on_wp_en;
> /* Keeps information if any of the LU is power on write protected */
> bool is_lu_power_on_wp;
> + /* Maximum number of general LU supported by the UFS device */
> + u8 max_lu_supported;
> u16 wmanufacturerid;
> /*UFS device Product Name */
> u8 *model;
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 4f8fcbb5f92e..dd10558f4d01 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -6858,6 +6858,37 @@ static void ufshcd_init_desc_sizes(struct ufs_hba *hba)
> hba->desc_size.hlth_desc = QUERY_DESC_HEALTH_DEF_SIZE;
> }
>
> +static int ufshcd_init_device_geo_params(struct ufs_hba *hba)
> +{
> + int err;
> + size_t buff_len;
> + u8 *desc_buf;
> +
> + buff_len = hba->desc_size.geom_desc;
> + desc_buf = kmalloc(buff_len, GFP_KERNEL);
> + if (!desc_buf) {
> + err = -ENOMEM;
> + goto out;
> + }
> +
> + err = ufshcd_read_desc(hba, QUERY_DESC_IDN_GEOMETRY, 0,
> + desc_buf, buff_len);
> + if (err) {
> + dev_err(hba->dev, "%s: Failed reading Geometry Desc. err = %d\n",
> + __func__, err);
> + goto out;
> + }
> +
> + if (desc_buf[GEOMETRY_DESC_PARAM_MAX_NUM_LUN] == 1)
> + hba->dev_info.max_lu_supported = 32;
> + else if (desc_buf[GEOMETRY_DESC_PARAM_MAX_NUM_LUN] == 0)
> + hba->dev_info.max_lu_supported = 8;
> +
> +out:
> + kfree(desc_buf);
> + return err;
> +}
> +
> static struct ufs_ref_clk ufs_ref_clk_freqs[] = {
> {19200000, REF_CLK_FREQ_19_2_MHZ},
> {26000000, REF_CLK_FREQ_26_MHZ},
> @@ -6931,9 +6962,17 @@ static int ufshcd_init_params(struct ufs_hba *hba)
> bool flag;
> int ret;
>
> + /* Clear any previous UFS device information */
> + memset(&hba->dev_info, 0, sizeof(hba->dev_info));
> +
> /* Init check for device descriptor sizes */
> ufshcd_init_desc_sizes(hba);
>
> + /* Init UFS geometry descriptor related parameters */
> + ret = ufshcd_init_device_geo_params(hba);
> + if (ret)
> + goto out;
> +
> /* Check and apply UFS device quirks */
> ret = ufs_get_device_desc(hba);
> if (ret) {
> @@ -6944,8 +6983,6 @@ static int ufshcd_init_params(struct ufs_hba *hba)
>
> 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;
> --
> 2.17.1
>
--
Regards,
Alim
On Sun, Jan 19, 2020 at 5:45 AM Bean Huo <[email protected]> wrote:
>
> From: Bean Huo <[email protected]>
>
> According to Jedec standard UFS 3.0 and UFS 2.1 Spec, Maximum number
> of logical units supported by the UFS device is indicated by parameter
> bMaxNumberLU in Geometry Descriptor. This patch is to delete current
> hard code macro definition of UFS_UPIU_MAX_GENERAL_LUN, and switch to
> use device indicated number instead.
>
> Reviewed-by: Asutosh Das <[email protected]>
> Signed-off-by: Bean Huo <[email protected]>
> ---
Reviewed-by: Alim Akhtar <[email protected]>
> drivers/scsi/ufs/ufs-sysfs.c | 2 +-
> drivers/scsi/ufs/ufs.h | 12 +++++++++---
> drivers/scsi/ufs/ufshcd.c | 4 ++--
> 3 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
> index 720be3f64be7..dbdf8b01abed 100644
> --- a/drivers/scsi/ufs/ufs-sysfs.c
> +++ b/drivers/scsi/ufs/ufs-sysfs.c
> @@ -713,7 +713,7 @@ static ssize_t _pname##_show(struct device *dev, \
> struct scsi_device *sdev = to_scsi_device(dev); \
> struct ufs_hba *hba = shost_priv(sdev->host); \
> u8 lun = ufshcd_scsi_to_upiu_lun(sdev->lun); \
> - if (!ufs_is_valid_unit_desc_lun(lun)) \
> + if (!ufs_is_valid_unit_desc_lun(&hba->dev_info, lun)) \
> return -EINVAL; \
> return ufs_sysfs_read_desc_param(hba, QUERY_DESC_IDN_##_duname, \
> lun, _duname##_DESC_PARAM##_puname, buf, _size); \
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index c982bcc94662..dde2eb02f76f 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -63,7 +63,6 @@
> #define UFS_UPIU_MAX_UNIT_NUM_ID 0x7F
> #define UFS_MAX_LUNS (SCSI_W_LUN_BASE + UFS_UPIU_MAX_UNIT_NUM_ID)
> #define UFS_UPIU_WLUN_ID (1 << 7)
> -#define UFS_UPIU_MAX_GENERAL_LUN 8
>
> /* Well known logical unit id in LUN field of UPIU */
> enum {
> @@ -539,12 +538,19 @@ struct ufs_dev_info {
>
> /**
> * ufs_is_valid_unit_desc_lun - checks if the given LUN has a unit descriptor
> + * @dev_info: pointer of instance of struct ufs_dev_info
> * @lun: LU number to check
> * @return: true if the lun has a matching unit descriptor, false otherwise
> */
> -static inline bool ufs_is_valid_unit_desc_lun(u8 lun)
> +static inline bool ufs_is_valid_unit_desc_lun(struct ufs_dev_info *dev_info,
> + u8 lun)
> {
> - return lun == UFS_UPIU_RPMB_WLUN || (lun < UFS_UPIU_MAX_GENERAL_LUN);
> + if (!dev_info || !dev_info->max_lu_supported) {
> + pr_err("Max General LU supported by UFS isn't initilized\n");
> + return false;
> + }
> +
> + return lun == UFS_UPIU_RPMB_WLUN || (lun < dev_info->max_lu_supported);
> }
>
> #endif /* End of Header */
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index dd10558f4d01..bf714221455e 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -3270,7 +3270,7 @@ static inline int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
> * Unit descriptors are only available for general purpose LUs (LUN id
> * from 0 to 7) and RPMB Well known LU.
> */
> - if (!ufs_is_valid_unit_desc_lun(lun))
> + if (!ufs_is_valid_unit_desc_lun(&hba->dev_info, lun))
> return -EOPNOTSUPP;
>
> return ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, lun,
> @@ -4525,7 +4525,7 @@ static int ufshcd_get_lu_wp(struct ufs_hba *hba,
> * protected so skip reading bLUWriteProtect parameter for
> * it. For other W-LUs, UNIT DESCRIPTOR is not available.
> */
> - else if (lun >= UFS_UPIU_MAX_GENERAL_LUN)
> + else if (lun >= hba->dev_info.max_lu_supported)
> ret = -ENOTSUPP;
> else
> ret = ufshcd_read_unit_desc_param(hba,
> --
> 2.17.1
>
--
Regards,
Alim
Hi Bean
Your patches based on which tree? At least on Jame's for-next, it give
compilation errors.
(git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git)
Looks like patch-2 introduces below error:
========
drivers/scsi/ufs/ufs-qcom.c: In function ‘ufs_qcom_apply_dev_quirks’:
drivers/scsi/ufs/ufs-qcom.c:955:34: error: incompatible types when
initializing type ‘struct ufs_dev_info *’ using type ‘struct
ufs_dev_info’
struct ufs_dev_info *dev_info = hba->dev_info;
^~~
drivers/scsi/ufs/ufs-qcom.c:957:14: error: ‘struct ufs_dev_info’ has
no member named ‘dev_quirks’
if (dev_info->dev_quirks & UFS_DEVICE_QUIRK_HOST_PA_SAVECONFIGTIME)
^~
scripts/Makefile.build:265: recipe for target
'drivers/scsi/ufs/ufs-qcom.o' failed
make[3]: *** [drivers/scsi/ufs/ufs-qcom.o] Error 1
=========
On Sun, Jan 19, 2020 at 5:44 AM Bean Huo <[email protected]> wrote:
>
> This series of patches is to simplify UFS driver initialization flow
> and add a new parameter max_lu_supported used to specify how many LUs
> supported by the UFS device.
> This series of patches being tested on my two platforms, Qualcomm SOC
> based and Hisilicon SOC based platforms.
>
> v1-v2:
> 1. Split ufshcd_probe_hba() based on its called flow
> 2. Delete two unnecessary functions
> 3. Add a fixup patch
> v2-v3:
> 1. Combine patches 7/9 and 8/9 of v2 to patch 7/8 of v3
> 2. Change patches 1/8 and 5/8 subject
> 3. Change the name of two functions in patch 7/8
>
> Bean Huo (8):
> scsi: ufs: Fix ufshcd_probe_hba() reture value in case
> ufshcd_scsi_add_wlus() fails
> scsi: ufs: Delete struct ufs_dev_desc
> scsi: ufs: Split ufshcd_probe_hba() based on its called flow
> scsi: ufs: move ufshcd_get_max_pwr_mode() to ufs_init_params()
> scsi: ufs: Inline two functions into their callers
> scsi: ufs: Delete is_init_prefetch from struct ufs_hba
> scsi: ufs: Add max_lu_supported in struct ufs_dev_info
> scsi: ufs: Use UFS device indicated maximum LU number
>
> drivers/scsi/ufs/ufs-mediatek.c | 7 +-
> drivers/scsi/ufs/ufs-qcom.c | 6 +-
> drivers/scsi/ufs/ufs-sysfs.c | 2 +-
> drivers/scsi/ufs/ufs.h | 25 ++-
> drivers/scsi/ufs/ufs_quirks.h | 9 +-
> drivers/scsi/ufs/ufshcd.c | 276 +++++++++++++++++++-------------
> drivers/scsi/ufs/ufshcd.h | 9 +-
> 7 files changed, 196 insertions(+), 138 deletions(-)
>
> --
> 2.17.1
>
--
Regards,
Alim
On Sun, 2020-01-19 at 01:13 +0100, Bean Huo wrote:
> From: Bean Huo <[email protected]>
>
> A non-zero error value likely being returned by ufshcd_scsi_add_wlus()
> in case of failure of adding the WLs, but ufshcd_probe_hba() doesn't
> use this value, and doesn't report this failure to upper caller.
> This patch is to fix this issue.
>
> Fixes: 2a8fa600445c ("ufs: manually add well known logical units")
> Reviewed-by: Asutosh Das <[email protected]>
> Signed-off-by: Bean Huo <[email protected]>
Reviewed-by: Stanley Chu <[email protected]>
On Sun, 2020-01-19 at 01:13 +0100, Bean Huo wrote:
> From: Bean Huo <[email protected]>
>
> In consideration of UFS host driver uses parameters of struct
> ufs_dev_desc, move its parameters to struct ufs_dev_info,
> delete struct ufs_dev_desc.
>
> Reviewed-by: Bart Van Assche <[email protected]>
> Reviewed-by: Asutosh Das <[email protected]>
> Signed-off-by: Bean Huo <[email protected]>
Reviewed-by: Stanley Chu <[email protected]>
On Sun, 2020-01-19 at 01:13 +0100, Bean Huo wrote:
> From: Bean Huo <[email protected]>
>
> ufshcd_get_max_pwr_mode() only need to be called once while booting,
> take it out from ufshcd_probe_hba() and inline into ufshcd_init_params().
>
> Signed-off-by: Bean Huo <[email protected]>
Reviewed-by: Stanley Chu <[email protected]>
Hi, Alim
>
> Hi Bean
>
> Your patches based on which tree? At least on Jame's for-next, it give
> compilation errors.
> (git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git)
> Looks like patch-2 introduces below error:
My patches are based on the Martin's tree 5.6/scsi-queue. I tested my patches on James' tree.
Didn't find the compilation error as you mentioned. You can check your source code if it is pretty new,
The last UFS driver updated commit id should be :
commit ea92c32bd336efba89c5b09cf609e6e26e963796
Author: Stanley Chu <[email protected]>
Date: Sat Jan 11 15:11:47 2020 +0800
scsi: ufs-mediatek: add apply_dev_quirks variant operation
Add vendor-specific variant callback "apply_dev_quirks" to MediaTek UFS driver.
Thanks,
//Bean
Hi, Alim
> > From: Bean Huo <[email protected]>
> >
> > ufshcd_get_max_pwr_mode() only need to be called once while booting,
> > take it out from ufshcd_probe_hba() and inline into ufshcd_init_params().
> >
> This can be safely squash with the previous patch which introduce
> ufshcd_init_params.
>
I kept this patch because I want you to review the patch easily. The previous patch is exactly to
split ufshcd_probe_hba(), doesn't want to do any initialization path flow changing. If I merge this
patch to the previous one, that will disorder the original calling process, and some reviewers will
have more concern about previous patch.
kept them independently, for the previous patch, you only need to check its split-up validity.
Thanks,
//Bean
On Mon, Jan 20, 2020 at 3:29 AM Bean Huo (beanhuo) <[email protected]> wrote:
>
> Hi, Alim
>
> >
> > Hi Bean
> >
> > Your patches based on which tree? At least on Jame's for-next, it give
> > compilation errors.
> > (git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git)
> > Looks like patch-2 introduces below error:
>
> My patches are based on the Martin's tree 5.6/scsi-queue. I tested my patches on James' tree.
> Didn't find the compilation error as you mentioned. You can check your source code if it is pretty new,
> The last UFS driver updated commit id should be :
>
>
> commit ea92c32bd336efba89c5b09cf609e6e26e963796
> Author: Stanley Chu <[email protected]>
> Date: Sat Jan 11 15:11:47 2020 +0800
>
> scsi: ufs-mediatek: add apply_dev_quirks variant operation
>
> Add vendor-specific variant callback "apply_dev_quirks" to MediaTek UFS driver.
>
Thanks Bean for letting me know about this.
Unfortunately, I still see the error (with enabled MKT and QCOM driver
in defconfig)
===================
drivers/scsi/ufs/ufs-mediatek.c: In function ‘ufs_mtk_apply_dev_quirks’:
drivers/scsi/ufs/ufs-mediatek.c:411:34: error: incompatible types when
initializing type ‘struct ufs_dev_info *’ using type ‘struct
ufs_dev_info’
struct ufs_dev_info *dev_info = hba->dev_info;
^~~
scripts/Makefile.build:265: recipe for target
'drivers/scsi/ufs/ufs-mediatek.o' failed
make[3]: *** [drivers/scsi/ufs/ufs-mediatek.o] Error 1
make[3]: *** Waiting for unfinished jobs....
drivers/scsi/ufs/ufs-qcom.c: In function ‘ufs_qcom_apply_dev_quirks’:
drivers/scsi/ufs/ufs-qcom.c:955:34: error: incompatible types when
initializing type ‘struct ufs_dev_info *’ using type ‘struct
ufs_dev_info’
struct ufs_dev_info *dev_info = hba->dev_info;
^~~
drivers/scsi/ufs/ufs-qcom.c:957:14: error: ‘struct ufs_dev_info’ has
no member named ‘dev_quirks’
if (dev_info->dev_quirks & UFS_DEVICE_QUIRK_HOST_PA_SAVECONFIGTIME)
^~
scripts/Makefile.build:265: recipe for target
'drivers/scsi/ufs/ufs-qcom.o' failed
======
git log --oneline on the tree looks like below:
7b700f8ec213 (HEAD -> local-martin-5.6-scsi-queue) scsi: ufs: Use UFS
device indicated maximum LU number
be7a1644f3af scsi: ufs: Add max_lu_supported in struct ufs_dev_info
13cd5c519941 scsi: ufs: Delete is_init_prefetch from struct ufs_hba
27539cb1d81d scsi: ufs: Inline two functions into their callers
cbe20395fe01 scsi: ufs: move ufshcd_get_max_pwr_mode() to ufs_init_params()
9fb0c168ef38 scsi: ufs: Split ufshcd_probe_hba() based on its called flow
a5d0e0fbd4d0 scsi: ufs: Delete struct ufs_dev_desc
c2aa6b365857 scsi: ufs: Fix ufshcd_probe_hba() reture value in case
ufshcd_scsi_add_wlus() fails
824b72db5086 (tag: mkp-scsi-queue, scsi/misc, martin-scsi/queue,
martin-scsi/5.6/scsi-queue) scsi: megaraid_sas: Update driver version
to 07.713.01.00-rc1
4d1634b8d12e scsi: megaraid_sas: Use Block layer API to check SCSI
device in-flight IO requests
56ee0c585602 scsi: megaraid_sas: Limit the number of retries for the
IOCTLs causing firmware fault
6d7537270e32 scsi: megaraid_sas: Do not initiate OCR if controller is
not in ready state
=====
~/work/linux (local-martin-5.6-scsi-queue) $ git log --oneline
drivers/scsi/ufs/ufs-mediatek.c
a5d0e0fbd4d0 scsi: ufs: Delete struct ufs_dev_desc
ea92c32bd336 scsi: ufs-mediatek: add apply_dev_quirks variant operation
5d74e18edd7b scsi: ufs-mediatek: configure and enable clk-gating
========
~/work/linux (local-martin-5.6-scsi-queue) $ git log --oneline
drivers/scsi/ufs/ufs-mediatek.c
a5d0e0fbd4d0 scsi: ufs: Delete struct ufs_dev_desc
ea92c32bd336 scsi: ufs-mediatek: add apply_dev_quirks variant operation
5d74e18edd7b scsi: ufs-mediatek: configure and enable clk-gating
Which has the mentioned commit by you.
=======
I am on gcc: aarch64-linux-gnu-gcc (Ubuntu/Linaro 7.4.0-1ubuntu1~18.04.1) 7.4.0
Am I missed any patches which is needed?
>
> Thanks,
>
> //Bean
--
Regards,
Alim
Hi, Alim
> I am on gcc: aarch64-linux-gnu-gcc (Ubuntu/Linaro 7.4.0-1ubuntu1~18.04.1)
> 7.4.0
>
> Am I missed any patches which is needed?
You didn't miss any patch, It is my fault, I have fixed it in my new version. You can check now.
Thanks,
//Bean