Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1252981 > unrolled thread

Re: [PATCH v1 07/17] scsi: ufs: separate device and host quirks

Started byAkinobu Mita <akinobu.mita@gmail.com>
First post2015-10-21 18:00 +0200
Last post2015-10-26 14:40 +0100
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v1 07/17] scsi: ufs: separate device and host quirks Akinobu Mita <akinobu.mita@gmail.com> - 2015-10-21 18:00 +0200
    Re: [PATCH v1 07/17] scsi: ufs: separate device and host quirks ygardi@codeaurora.org - 2015-10-26 14:40 +0100

#1252981 — Re: [PATCH v1 07/17] scsi: ufs: separate device and host quirks

FromAkinobu Mita <akinobu.mita@gmail.com>
Date2015-10-21 18:00 +0200
SubjectRe: [PATCH v1 07/17] scsi: ufs: separate device and host quirks
Message-ID<qm3XX-7C9-11@gated-at.bofh.it>
2015-09-13 23:52 GMT+09:00 Yaniv Gardi <ygardi@codeaurora.org>:

> diff --git a/drivers/scsi/ufs/ufs_quirks.c b/drivers/scsi/ufs/ufs_quirks.c
> new file mode 100644
> index 0000000..b649bbf
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs_quirks.c
> @@ -0,0 +1,101 @@
> +/*
> + * Copyright (c) 2013-2015, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include "ufshcd.h"
> +#include "ufs_quirks.h"
> +
> +

Single blank line is enough.

> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index e5a876c..0803a89 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -39,9 +39,10 @@
>
>  #include <linux/async.h>
>  #include <linux/devfreq.h>
> -
> +#include <linux/nls.h>
>  #include <linux/of.h>
>  #include "ufshcd.h"
> +#include "ufs_quirks.h"
>  #include "unipro.h"
>  #define UFSHCD_REQ_SENSE_SIZE  18
>
> @@ -259,6 +260,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
> @@ -2052,6 +2063,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;
> +       }

I actually tried this patch and ufshcd_read_desc() always returns
-EINVAL due to the following check in the end of ufshcd_read_desc().

        if (ret || (buff_len < ufs_query_desc_max_size[desc_id]) ||
            (desc_buf[QUERY_DESC_LENGTH_OFFSET] !=
             ufs_query_desc_max_size[desc_id])
            || (desc_buf[QUERY_DESC_DESC_TYPE_OFFSET] != desc_id)) {
                ...
                if (!ret)
                        ret = -EINVAL;

Could you also include a fix fir ufshcd_read_desc()?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1255908

Fromygardi@codeaurora.org
Date2015-10-26 14:40 +0100
Message-ID<qnQae-mF-17@gated-at.bofh.it>
In reply to#1252981
> 2015-09-13 23:52 GMT+09:00 Yaniv Gardi <ygardi@codeaurora.org>:
>
>> diff --git a/drivers/scsi/ufs/ufs_quirks.c
>> b/drivers/scsi/ufs/ufs_quirks.c
>> new file mode 100644
>> index 0000000..b649bbf
>> --- /dev/null
>> +++ b/drivers/scsi/ufs/ufs_quirks.c
>> @@ -0,0 +1,101 @@
>> +/*
>> + * Copyright (c) 2013-2015, The Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include "ufshcd.h"
>> +#include "ufs_quirks.h"
>> +
>> +
>
> Single blank line is enough.

done

>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index e5a876c..0803a89 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -39,9 +39,10 @@
>>
>>  #include <linux/async.h>
>>  #include <linux/devfreq.h>
>> -
>> +#include <linux/nls.h>
>>  #include <linux/of.h>
>>  #include "ufshcd.h"
>> +#include "ufs_quirks.h"
>>  #include "unipro.h"
>>  #define UFSHCD_REQ_SENSE_SIZE  18
>>
>> @@ -259,6 +260,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
>> @@ -2052,6 +2063,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;
>> +       }
>
> I actually tried this patch and ufshcd_read_desc() always returns
> -EINVAL due to the following check in the end of ufshcd_read_desc().
>
>         if (ret || (buff_len < ufs_query_desc_max_size[desc_id]) ||
>             (desc_buf[QUERY_DESC_LENGTH_OFFSET] !=
>              ufs_query_desc_max_size[desc_id])
>             || (desc_buf[QUERY_DESC_DESC_TYPE_OFFSET] != desc_id)) {
>                 ...
>                 if (!ret)
>                         ret = -EINVAL;
>
> Could you also include a fix fir ufshcd_read_desc()?

i'm not sure i understand what should be fixed.
if the check you mentioned implies that something is wrong,
we shouldn't ignore the error. otherwise we might exceed the buf
boundaries for example.
can you elaborate what is that you think we should fix ?





> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web