Hi Tomas,
>
> Define new a type: uc_string_id for easier string
> handling and less casting. Reduce number or string
> copies in price of a dynamic allocation.
>
> Signed-off-by: Tomas Winkler <tomas.wink...@intel.com>
> Tested-by: Avri Altman <avri.alt...@wdc.com>
> ---
>
> Resend: It was reviewed by not merged.
>
> drivers/scsi/ufs/ufs-sysfs.c | 20 ++---
> drivers/scsi/ufs/ufs.h | 2 +-
> drivers/scsi/ufs/ufshcd.c | 164 +++++++++++++++++++++--------------
> drivers/scsi/ufs/ufshcd.h | 9 +-
> 4 files changed, 115 insertions(+), 80 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
> index f478685122ff..13e357f01025 100644
> --- a/drivers/scsi/ufs/ufs-sysfs.c
> +++ b/drivers/scsi/ufs/ufs-sysfs.c
> @@ -570,10 +570,11 @@ static ssize_t _name##_show(struct device *dev,
> \
> struct ufs_hba *hba = dev_get_drvdata(dev); \
> int ret; \
> int desc_len = QUERY_DESC_MAX_SIZE; \
> - u8 *desc_buf; \
> + char *desc_buf; \
> + \
Leaving it a u8 * here, will assure it would be utf-8,
And save you making param_read_buf opaque,
in ufshcd_read_desc and ufshcd_read_desc_param.
A fare tradeoff, don't you think?
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index 99a9c4d16f6b..b3e1b2a0f463 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -541,7 +541,7 @@ struct ufs_dev_info {
> */
> struct ufs_dev_desc {
> u16 wmanufacturerid;
> - char model[MAX_MODEL_LEN + 1];
> + char *model;
> };
Belongs to a different patch?
> /**
> * ufshcd_read_string_desc - read string descriptor
> * @hba: pointer to adapter instance
> * @desc_index: descriptor index
> - * @buf: pointer to buffer where descriptor would be read
> - * @size: size of buf
> + * @buf: pointer to buffer where descriptor would be read,
> + * the caller should free the memory.
> * @ascii: if true convert from unicode to ascii characters
> + * null terminated string.
Since ascii is always true, maybe omit this argument altogether,
and group the if (asci) clause in some handler?
> /**
> @@ -6452,6 +6478,9 @@ static int ufs_get_device_desc(struct ufs_hba
> *hba,
> u8 model_index;
> u8 *desc_buf;
>
> + if (!dev_desc)
> + return -EINVAL;
> +
A different patch?
>
> +static void ufs_put_device_desc(struct ufs_dev_desc *dev_desc)
> +{
> + kfree(dev_desc->model);
> + dev_desc->model = NULL;
> +}
A different patch?
While it's true that dev_desc->model conclude its part after
ufs_fixup_device_setup,
Why are you releasing specifically this part of card?
>
> ufs_fixup_device_setup(hba, &card);
> + ufs_put_device_desc(&card);
A different patch?
> +
> ufshcd_tune_unipro_params(hba);
>
> /* UFS device is also active now */
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 994d73d03207..10935548d1fc 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -885,14 +885,17 @@ int ufshcd_read_desc_param(struct ufs_hba
> *hba,
> enum desc_idn desc_id,
> int desc_index,
> u8 param_offset,
> - u8 *param_read_buf,
> + void *param_read_buf,
> u8 param_size);
> int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
> enum attr_idn idn, u8 index, u8 selector, u32
> *attr_val);
> int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
> enum flag_idn idn, bool *flag_res);
> -int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index,
> - u8 *buf, u32 size, bool ascii);
> +
> +#define SD_ASCII_STD true
> +#define SD_RAW false
Not really needed?
Thanks,
Avri
