Message ID | 20200116215914.16015-3-huobean@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Use UFS device indicated maximum LU number | expand |
On 2020-01-16 13:59, Bean Huo wrote: > struct ufs_dev_fix { > - struct ufs_dev_desc card; > + u16 wmanufacturerid; > + u8 *model; > unsigned int quirk; > }; > > -#define END_FIX { { 0 }, 0 } > +#define END_FIX { 0 } A minor comment: please use { } instead of { 0 }. > /* add specific device quirk */ > #define UFS_FIX(_vendor, _model, _quirk) { \ > - .card.wmanufacturerid = (_vendor),\ > - .card.model = (_model), \ > + .wmanufacturerid = (_vendor),\ > + .model = (_model), \ > .quirk = (_quirk), \ > } Is this macro useful? Does it improve readability of the code? If not, how about removing it (maybe later)? Anyway: Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Hi, Bart > On 2020-01-16 13:59, Bean Huo wrote: > > struct ufs_dev_fix { > > - struct ufs_dev_desc card; > > + u16 wmanufacturerid; > > + u8 *model; > > unsigned int quirk; > > }; > > > > -#define END_FIX { { 0 }, 0 } > > +#define END_FIX { 0 } > > A minor comment: please use { } instead of { 0 }. > Will be changed in the next version. > > /* add specific device quirk */ > > #define UFS_FIX(_vendor, _model, _quirk) { \ > > - .card.wmanufacturerid = (_vendor),\ > > - .card.model = (_model), \ > > + .wmanufacturerid = (_vendor),\ > > + .model = (_model), \ > > .quirk = (_quirk), \ > > } > > Is this macro useful? Does it improve readability of the code? If not, how about > removing it (maybe later)? > Before no better solution, I think now we just keep it. > Anyway: > > Reviewed-by: Bart Van Assche <bvanassche@acm.org> Thanks for reviewing. //Bean
On 1/16/2020 1:59 PM, Bean Huo wrote: > From: Bean Huo <beanhuo@micron.com> > > In consideration of Host driver uses some certain UFS device properties, > move parameters of struct ufs_dev_desc to struct ufs_dev_info, and delete > struct ufs_dev_desc. > > Signed-off-by: Bean Huo <beanhuo@micron.com> > --- > 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..16c324c61b6e 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 { 0 } > > /* 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; > } > > Reviewed-by: Asutosh Das <asutoshd@codeaurora.org>
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..16c324c61b6e 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 { 0 } /* 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; }