2016-03-01 07:35:40

by Hannes Reinecke

[permalink] [raw]
Subject: Re: [PATCH v5 05/15] scsi: ufs: add support to read device and string descriptors

On 02/28/2016 09:32 PM, Yaniv Gardi wrote:
> This change adds support to read device descriptor and string descriptor
> from a UFS device
>
> Reviewed-by: Gilad Broner <[email protected]>
> Signed-off-by: Raviv Shvili <[email protected]>
> Signed-off-by: Yaniv Gardi <[email protected]>
>
> ---
> drivers/scsi/ufs/ufs.h | 1 +
> drivers/scsi/ufs/ufshcd.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++-
> drivers/scsi/ufs/ufshcd.h | 7 ++++
> 3 files changed, 95 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index 54a16ce..aacb235 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -43,6 +43,7 @@
> #define GENERAL_UPIU_REQUEST_SIZE 32
> #define QUERY_DESC_MAX_SIZE 255
> #define QUERY_DESC_MIN_SIZE 2
> +#define QUERY_DESC_HDR_SIZE 2
> #define QUERY_OSF_SIZE (GENERAL_UPIU_REQUEST_SIZE - \
> (sizeof(struct utp_upiu_header)))
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 80031e6..e2ed415 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -39,7 +39,7 @@
>
> #include <linux/async.h>
> #include <linux/devfreq.h>
> -
> +#include <linux/nls.h>
> #include <linux/of.h>
> #include "ufshcd.h"
> #include "unipro.h"
> @@ -232,6 +232,16 @@ static inline void ufshcd_disable_irq(struct ufs_hba *hba)
> }
> }
>
> +/* replace non-printable or non-ASCII characters with spaces */
> +static inline void ufshcd_remove_non_printable(char *val)
> +{
> + if (!val)
> + return;
> +
> + if (*val < 0x20 || *val > 0x7e)
> + *val = ' ';
> +}
> +
> /*
> * ufshcd_wait_for_register - wait for register value to change
> * @hba - per-adapter interface
> @@ -2021,6 +2031,82 @@ static inline int ufshcd_read_power_desc(struct ufs_hba *hba,
> return ufshcd_read_desc(hba, QUERY_DESC_IDN_POWER, 0, buf, size);
> }
>
> +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);
> +}
> +EXPORT_SYMBOL(ufshcd_read_device_desc);
> +
> +/**
> + * 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
> + * @ascii: if true convert from unicode to ascii characters
> + *
> + * Return 0 in case of success, non-zero otherwise
> + */
> +int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index, u8 *buf,
> + u32 size, bool ascii)
> +{
> + int err = 0;
> +
> + err = ufshcd_read_desc(hba,
> + QUERY_DESC_IDN_STRING, desc_index, buf, size);
> +
> + if (err) {
> + dev_err(hba->dev, "%s: reading String Desc failed after %d retries. err = %d\n",
> + __func__, QUERY_REQ_RETRIES, err);
> + goto out;
> + }
> +
> + if (ascii) {
> + int desc_len;
> + int ascii_len;
> + int i;
> + char *buff_ascii;
> +
> + desc_len = buf[0];
> + /* remove header and divide by 2 to move from UTF16 to UTF8 */
> + ascii_len = (desc_len - QUERY_DESC_HDR_SIZE) / 2 + 1;
> + if (size < ascii_len + QUERY_DESC_HDR_SIZE) {
> + dev_err(hba->dev, "%s: buffer allocated size is too small\n",
> + __func__);
> + err = -ENOMEM;
> + goto out;
> + }
> +
> + buff_ascii = kmalloc(ascii_len, GFP_KERNEL);
> + if (!buff_ascii) {
> + err = -ENOMEM;
> + goto out_free_buff;
> + }
> +
> + /*
> + * the descriptor contains string in UTF16 format
> + * we need to convert to utf-8 so it can be displayed
> + */
> + utf16s_to_utf8s((wchar_t *)&buf[QUERY_DESC_HDR_SIZE],
> + desc_len - QUERY_DESC_HDR_SIZE,
> + UTF16_BIG_ENDIAN, buff_ascii, ascii_len);
> +
> + /* replace non-printable or non-ASCII characters with spaces */
> + for (i = 0; i < ascii_len; i++)
> + ufshcd_remove_non_printable(&buff_ascii[i]);
> +
> + memset(buf + QUERY_DESC_HDR_SIZE, 0,
> + size - QUERY_DESC_HDR_SIZE);
> + memcpy(buf + QUERY_DESC_HDR_SIZE, buff_ascii, ascii_len);
> + buf[QUERY_DESC_LENGTH_OFFSET] = ascii_len + QUERY_DESC_HDR_SIZE;
> +out_free_buff:
> + kfree(buff_ascii);
> + }
> +out:
> + return err;
> +}
> +EXPORT_SYMBOL(ufshcd_read_string_desc);
> +
> /**
> * ufshcd_read_unit_desc_param - read the specified unit descriptor parameter
> * @hba: Pointer to adapter instance
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index a6d3572..54e13cc 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -678,6 +678,13 @@ static inline int ufshcd_dme_peer_get(struct ufs_hba *hba,
> return ufshcd_dme_get_attr(hba, attr_sel, mib_val, DME_PEER);
> }
>
> +int ufshcd_read_device_desc(struct ufs_hba *hba, u8 *buf, u32 size);
> +
> +#define ASCII_STD true
> +
> +int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index, u8 *buf,
> + u32 size, bool ascii);
> +
> /* Expose Query-Request API */
> int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
> enum flag_idn idn, bool *flag_res);
>

Probably shows my ignorance, but _where_ is this function actually used?

Cheers,

Hannes
--
Dr. Hannes Reinecke zSeries & Storage
[email protected] +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: J. Hawn, J. Guild, F. Imend?rffer, HRB 16746 (AG N?rnberg)


2016-03-01 10:01:29

by Yaniv Gardi

[permalink] [raw]
Subject: Re: [PATCH v5 05/15] scsi: ufs: add support to read device and string descriptors

> On 02/28/2016 09:32 PM, Yaniv Gardi wrote:
>> This change adds support to read device descriptor and string descriptor
>> from a UFS device
>>
>> Reviewed-by: Gilad Broner <[email protected]>
>> Signed-off-by: Raviv Shvili <[email protected]>
>> Signed-off-by: Yaniv Gardi <[email protected]>
>>
>> ---
>> drivers/scsi/ufs/ufs.h | 1 +
>> drivers/scsi/ufs/ufshcd.c | 88
>> ++++++++++++++++++++++++++++++++++++++++++++++-
>> drivers/scsi/ufs/ufshcd.h | 7 ++++
>> 3 files changed, 95 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
>> index 54a16ce..aacb235 100644
>> --- a/drivers/scsi/ufs/ufs.h
>> +++ b/drivers/scsi/ufs/ufs.h
>> @@ -43,6 +43,7 @@
>> #define GENERAL_UPIU_REQUEST_SIZE 32
>> #define QUERY_DESC_MAX_SIZE 255
>> #define QUERY_DESC_MIN_SIZE 2
>> +#define QUERY_DESC_HDR_SIZE 2
>> #define QUERY_OSF_SIZE (GENERAL_UPIU_REQUEST_SIZE - \
>> (sizeof(struct utp_upiu_header)))
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index 80031e6..e2ed415 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -39,7 +39,7 @@
>>
>> #include <linux/async.h>
>> #include <linux/devfreq.h>
>> -
>> +#include <linux/nls.h>
>> #include <linux/of.h>
>> #include "ufshcd.h"
>> #include "unipro.h"
>> @@ -232,6 +232,16 @@ static inline void ufshcd_disable_irq(struct
>> ufs_hba *hba)
>> }
>> }
>>
>> +/* replace non-printable or non-ASCII characters with spaces */
>> +static inline void ufshcd_remove_non_printable(char *val)
>> +{
>> + if (!val)
>> + return;
>> +
>> + if (*val < 0x20 || *val > 0x7e)
>> + *val = ' ';
>> +}
>> +
>> /*
>> * ufshcd_wait_for_register - wait for register value to change
>> * @hba - per-adapter interface
>> @@ -2021,6 +2031,82 @@ static inline int ufshcd_read_power_desc(struct
>> ufs_hba *hba,
>> return ufshcd_read_desc(hba, QUERY_DESC_IDN_POWER, 0, buf, size);
>> }
>>
>> +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);
>> +}
>> +EXPORT_SYMBOL(ufshcd_read_device_desc);
>> +
>> +/**
>> + * 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
>> + * @ascii: if true convert from unicode to ascii characters
>> + *
>> + * Return 0 in case of success, non-zero otherwise
>> + */
>> +int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index, u8
>> *buf,
>> + u32 size, bool ascii)
>> +{
>> + int err = 0;
>> +
>> + err = ufshcd_read_desc(hba,
>> + QUERY_DESC_IDN_STRING, desc_index, buf, size);
>> +
>> + if (err) {
>> + dev_err(hba->dev, "%s: reading String Desc failed after %d retries.
>> err = %d\n",
>> + __func__, QUERY_REQ_RETRIES, err);
>> + goto out;
>> + }
>> +
>> + if (ascii) {
>> + int desc_len;
>> + int ascii_len;
>> + int i;
>> + char *buff_ascii;
>> +
>> + desc_len = buf[0];
>> + /* remove header and divide by 2 to move from UTF16 to UTF8 */
>> + ascii_len = (desc_len - QUERY_DESC_HDR_SIZE) / 2 + 1;
>> + if (size < ascii_len + QUERY_DESC_HDR_SIZE) {
>> + dev_err(hba->dev, "%s: buffer allocated size is too small\n",
>> + __func__);
>> + err = -ENOMEM;
>> + goto out;
>> + }
>> +
>> + buff_ascii = kmalloc(ascii_len, GFP_KERNEL);
>> + if (!buff_ascii) {
>> + err = -ENOMEM;
>> + goto out_free_buff;
>> + }
>> +
>> + /*
>> + * the descriptor contains string in UTF16 format
>> + * we need to convert to utf-8 so it can be displayed
>> + */
>> + utf16s_to_utf8s((wchar_t *)&buf[QUERY_DESC_HDR_SIZE],
>> + desc_len - QUERY_DESC_HDR_SIZE,
>> + UTF16_BIG_ENDIAN, buff_ascii, ascii_len);
>> +
>> + /* replace non-printable or non-ASCII characters with spaces */
>> + for (i = 0; i < ascii_len; i++)
>> + ufshcd_remove_non_printable(&buff_ascii[i]);
>> +
>> + memset(buf + QUERY_DESC_HDR_SIZE, 0,
>> + size - QUERY_DESC_HDR_SIZE);
>> + memcpy(buf + QUERY_DESC_HDR_SIZE, buff_ascii, ascii_len);
>> + buf[QUERY_DESC_LENGTH_OFFSET] = ascii_len + QUERY_DESC_HDR_SIZE;
>> +out_free_buff:
>> + kfree(buff_ascii);
>> + }
>> +out:
>> + return err;
>> +}
>> +EXPORT_SYMBOL(ufshcd_read_string_desc);
>> +
>> /**
>> * ufshcd_read_unit_desc_param - read the specified unit descriptor
>> parameter
>> * @hba: Pointer to adapter instance
>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
>> index a6d3572..54e13cc 100644
>> --- a/drivers/scsi/ufs/ufshcd.h
>> +++ b/drivers/scsi/ufs/ufshcd.h
>> @@ -678,6 +678,13 @@ static inline int ufshcd_dme_peer_get(struct
>> ufs_hba *hba,
>> return ufshcd_dme_get_attr(hba, attr_sel, mib_val, DME_PEER);
>> }
>>
>> +int ufshcd_read_device_desc(struct ufs_hba *hba, u8 *buf, u32 size);
>> +
>> +#define ASCII_STD true
>> +
>> +int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index, u8
>> *buf,
>> + u32 size, bool ascii);
>> +
>> /* Expose Query-Request API */
>> int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
>> enum flag_idn idn, bool *flag_res);
>>
>
> Probably shows my ignorance, but _where_ is this function actually used?
>
> Cheers,
> Hannes


Hello Hannes,
I was asked by reviewer Tomas Winkler, to seperate a patch (v4) into 2
patches in v5.
so, this is part I of the seperation, and the functions here, are being
called by the next patch v5 06/15 - in the quirk.c file in
ufs_get_device_info() routine.
hope that helps.

regards,
Yaniv


> --
> Dr. Hannes Reinecke zSeries & Storage
> [email protected] +49 911 74053 688
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N?rnberg
> GF: J. Hawn, J. Guild, F. Imend?rffer, HRB 16746 (AG N?rnberg)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>


2016-03-01 10:04:06

by Hannes Reinecke

[permalink] [raw]
Subject: Re: [PATCH v5 05/15] scsi: ufs: add support to read device and string descriptors

On 03/01/2016 06:01 PM, [email protected] wrote:
>> On 02/28/2016 09:32 PM, Yaniv Gardi wrote:
>>> This change adds support to read device descriptor and string descriptor
>>> from a UFS device
>>>
>>> Reviewed-by: Gilad Broner <[email protected]>
>>> Signed-off-by: Raviv Shvili <[email protected]>
>>> Signed-off-by: Yaniv Gardi <[email protected]>
>>>
>>> ---
>>> drivers/scsi/ufs/ufs.h | 1 +
>>> drivers/scsi/ufs/ufshcd.c | 88
>>> ++++++++++++++++++++++++++++++++++++++++++++++-
>>> drivers/scsi/ufs/ufshcd.h | 7 ++++
>>> 3 files changed, 95 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
>>> index 54a16ce..aacb235 100644
>>> --- a/drivers/scsi/ufs/ufs.h
>>> +++ b/drivers/scsi/ufs/ufs.h
>>> @@ -43,6 +43,7 @@
>>> #define GENERAL_UPIU_REQUEST_SIZE 32
>>> #define QUERY_DESC_MAX_SIZE 255
>>> #define QUERY_DESC_MIN_SIZE 2
>>> +#define QUERY_DESC_HDR_SIZE 2
>>> #define QUERY_OSF_SIZE (GENERAL_UPIU_REQUEST_SIZE - \
>>> (sizeof(struct utp_upiu_header)))
>>>
>>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>>> index 80031e6..e2ed415 100644
>>> --- a/drivers/scsi/ufs/ufshcd.c
>>> +++ b/drivers/scsi/ufs/ufshcd.c
>>> @@ -39,7 +39,7 @@
>>>
>>> #include <linux/async.h>
>>> #include <linux/devfreq.h>
>>> -
>>> +#include <linux/nls.h>
>>> #include <linux/of.h>
>>> #include "ufshcd.h"
>>> #include "unipro.h"
>>> @@ -232,6 +232,16 @@ static inline void ufshcd_disable_irq(struct
>>> ufs_hba *hba)
>>> }
>>> }
>>>
>>> +/* replace non-printable or non-ASCII characters with spaces */
>>> +static inline void ufshcd_remove_non_printable(char *val)
>>> +{
>>> + if (!val)
>>> + return;
>>> +
>>> + if (*val < 0x20 || *val > 0x7e)
>>> + *val = ' ';
>>> +}
>>> +
>>> /*
>>> * ufshcd_wait_for_register - wait for register value to change
>>> * @hba - per-adapter interface
>>> @@ -2021,6 +2031,82 @@ static inline int ufshcd_read_power_desc(struct
>>> ufs_hba *hba,
>>> return ufshcd_read_desc(hba, QUERY_DESC_IDN_POWER, 0, buf, size);
>>> }
>>>
>>> +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);
>>> +}
>>> +EXPORT_SYMBOL(ufshcd_read_device_desc);
>>> +
>>> +/**
>>> + * 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
>>> + * @ascii: if true convert from unicode to ascii characters
>>> + *
>>> + * Return 0 in case of success, non-zero otherwise
>>> + */
>>> +int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index, u8
>>> *buf,
>>> + u32 size, bool ascii)
>>> +{
>>> + int err = 0;
>>> +
>>> + err = ufshcd_read_desc(hba,
>>> + QUERY_DESC_IDN_STRING, desc_index, buf, size);
>>> +
>>> + if (err) {
>>> + dev_err(hba->dev, "%s: reading String Desc failed after %d retries.
>>> err = %d\n",
>>> + __func__, QUERY_REQ_RETRIES, err);
>>> + goto out;
>>> + }
>>> +
>>> + if (ascii) {
>>> + int desc_len;
>>> + int ascii_len;
>>> + int i;
>>> + char *buff_ascii;
>>> +
>>> + desc_len = buf[0];
>>> + /* remove header and divide by 2 to move from UTF16 to UTF8 */
>>> + ascii_len = (desc_len - QUERY_DESC_HDR_SIZE) / 2 + 1;
>>> + if (size < ascii_len + QUERY_DESC_HDR_SIZE) {
>>> + dev_err(hba->dev, "%s: buffer allocated size is too small\n",
>>> + __func__);
>>> + err = -ENOMEM;
>>> + goto out;
>>> + }
>>> +
>>> + buff_ascii = kmalloc(ascii_len, GFP_KERNEL);
>>> + if (!buff_ascii) {
>>> + err = -ENOMEM;
>>> + goto out_free_buff;
>>> + }
>>> +
>>> + /*
>>> + * the descriptor contains string in UTF16 format
>>> + * we need to convert to utf-8 so it can be displayed
>>> + */
>>> + utf16s_to_utf8s((wchar_t *)&buf[QUERY_DESC_HDR_SIZE],
>>> + desc_len - QUERY_DESC_HDR_SIZE,
>>> + UTF16_BIG_ENDIAN, buff_ascii, ascii_len);
>>> +
>>> + /* replace non-printable or non-ASCII characters with spaces */
>>> + for (i = 0; i < ascii_len; i++)
>>> + ufshcd_remove_non_printable(&buff_ascii[i]);
>>> +
>>> + memset(buf + QUERY_DESC_HDR_SIZE, 0,
>>> + size - QUERY_DESC_HDR_SIZE);
>>> + memcpy(buf + QUERY_DESC_HDR_SIZE, buff_ascii, ascii_len);
>>> + buf[QUERY_DESC_LENGTH_OFFSET] = ascii_len + QUERY_DESC_HDR_SIZE;
>>> +out_free_buff:
>>> + kfree(buff_ascii);
>>> + }
>>> +out:
>>> + return err;
>>> +}
>>> +EXPORT_SYMBOL(ufshcd_read_string_desc);
>>> +
>>> /**
>>> * ufshcd_read_unit_desc_param - read the specified unit descriptor
>>> parameter
>>> * @hba: Pointer to adapter instance
>>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
>>> index a6d3572..54e13cc 100644
>>> --- a/drivers/scsi/ufs/ufshcd.h
>>> +++ b/drivers/scsi/ufs/ufshcd.h
>>> @@ -678,6 +678,13 @@ static inline int ufshcd_dme_peer_get(struct
>>> ufs_hba *hba,
>>> return ufshcd_dme_get_attr(hba, attr_sel, mib_val, DME_PEER);
>>> }
>>>
>>> +int ufshcd_read_device_desc(struct ufs_hba *hba, u8 *buf, u32 size);
>>> +
>>> +#define ASCII_STD true
>>> +
>>> +int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index, u8
>>> *buf,
>>> + u32 size, bool ascii);
>>> +
>>> /* Expose Query-Request API */
>>> int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
>>> enum flag_idn idn, bool *flag_res);
>>>
>>
>> Probably shows my ignorance, but _where_ is this function actually used?
>>
>> Cheers,
>> Hannes
>
>
> Hello Hannes,
> I was asked by reviewer Tomas Winkler, to seperate a patch (v4) into 2
> patches in v5.
> so, this is part I of the seperation, and the functions here, are being
> called by the next patch v5 06/15 - in the quirk.c file in
> ufs_get_device_info() routine.
> hope that helps.
>
Okay; but you should've put that into the patch description.

Reviewed-by: Hannes Reinecke <[email protected]>

Cheers,

Hannes
--
Dr. Hannes Reinecke zSeries & Storage
[email protected] +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: J. Hawn, J. Guild, F. Imend?rffer, HRB 16746 (AG N?rnberg)