* [PATCH 1/2] scsi: ufs: use MASK_EE_STATUS
@ 2017-04-20 12:01 kusumi.tomohiro
2017-04-20 12:01 ` [PATCH 2/2] scsi: ufs: make ufshcd_get_lists_status() register operation obvious kusumi.tomohiro
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: kusumi.tomohiro @ 2017-04-20 12:01 UTC (permalink / raw)
To: subhashj, vinholikatti, linux-scsi; +Cc: Tomohiro Kusumi
From: Tomohiro Kusumi <tkusumi@tuxera.com>
MASK_EE_STATUS added by 66ec6d59 was unused, but it seems to have been
defined to do this.
Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
drivers/scsi/ufs/ufshcd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 096e95b..9278666 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -4717,7 +4717,7 @@ static int ufshcd_disable_ee(struct ufs_hba *hba, u16 mask)
goto out;
val = hba->ee_ctrl_mask & ~mask;
- val &= 0xFFFF; /* 2 bytes */
+ val &= MASK_EE_STATUS;
err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
QUERY_ATTR_IDN_EE_CONTROL, 0, 0, &val);
if (!err)
@@ -4745,7 +4745,7 @@ static int ufshcd_enable_ee(struct ufs_hba *hba, u16 mask)
goto out;
val = hba->ee_ctrl_mask | mask;
- val &= 0xFFFF; /* 2 bytes */
+ val &= MASK_EE_STATUS;
err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
QUERY_ATTR_IDN_EE_CONTROL, 0, 0, &val);
if (!err)
--
2.9.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/2] scsi: ufs: make ufshcd_get_lists_status() register operation obvious
2017-04-20 12:01 [PATCH 1/2] scsi: ufs: use MASK_EE_STATUS kusumi.tomohiro
@ 2017-04-20 12:01 ` kusumi.tomohiro
2017-04-26 16:49 ` Subhash Jadavani
2017-04-24 22:34 ` [PATCH 1/2] scsi: ufs: use MASK_EE_STATUS Martin K. Petersen
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: kusumi.tomohiro @ 2017-04-20 12:01 UTC (permalink / raw)
To: subhashj, vinholikatti, linux-scsi; +Cc: Tomohiro Kusumi
From: Tomohiro Kusumi <tkusumi@tuxera.com>
It could be just cmp 0xe instead of >>1 and cmp 0x7, with readable code.
Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
drivers/scsi/ufs/ufshcd.c | 11 +----------
drivers/scsi/ufs/ufshci.h | 4 ++++
2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 9278666..ad9532b 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -668,16 +668,7 @@ static inline void ufshcd_outstanding_req_clear(struct ufs_hba *hba, int tag)
*/
static inline int ufshcd_get_lists_status(u32 reg)
{
- /*
- * The mask 0xFF is for the following HCS register bits
- * Bit Description
- * 0 Device Present
- * 1 UTRLRDY
- * 2 UTMRLRDY
- * 3 UCRDY
- * 4-7 reserved
- */
- return ((reg & 0xFF) >> 1) ^ 0x07;
+ return !((reg & UFSHCD_STATUS_READY) == (u32)UFSHCD_STATUS_READY);
}
/**
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index d14e9b9..8cd4f0e 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -159,6 +159,10 @@ enum {
#define DEVICE_ERROR_INDICATOR UFS_BIT(5)
#define UIC_POWER_MODE_CHANGE_REQ_STATUS_MASK UFS_MASK(0x7, 8)
+#define UFSHCD_STATUS_READY (UTP_TRANSFER_REQ_LIST_READY |\
+ UTP_TASK_REQ_LIST_READY |\
+ UIC_COMMAND_READY)
+
enum {
PWR_OK = 0x0,
PWR_LOCAL = 0x01,
--
2.9.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 2/2] scsi: ufs: make ufshcd_get_lists_status() register operation obvious
2017-04-20 12:01 ` [PATCH 2/2] scsi: ufs: make ufshcd_get_lists_status() register operation obvious kusumi.tomohiro
@ 2017-04-26 16:49 ` Subhash Jadavani
2017-04-26 17:04 ` Tomohiro Kusumi
0 siblings, 1 reply; 7+ messages in thread
From: Subhash Jadavani @ 2017-04-26 16:49 UTC (permalink / raw)
To: kusumi.tomohiro; +Cc: vinholikatti, linux-scsi, Tomohiro Kusumi
On 2017-04-20 05:01, kusumi.tomohiro@gmail.com wrote:
> From: Tomohiro Kusumi <tkusumi@tuxera.com>
>
> It could be just cmp 0xe instead of >>1 and cmp 0x7, with readable
> code.
>
> Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
> ---
> drivers/scsi/ufs/ufshcd.c | 11 +----------
> drivers/scsi/ufs/ufshci.h | 4 ++++
> 2 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 9278666..ad9532b 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -668,16 +668,7 @@ static inline void
> ufshcd_outstanding_req_clear(struct ufs_hba *hba, int tag)
> */
> static inline int ufshcd_get_lists_status(u32 reg)
> {
> - /*
> - * The mask 0xFF is for the following HCS register bits
> - * Bit Description
> - * 0 Device Present
> - * 1 UTRLRDY
> - * 2 UTMRLRDY
> - * 3 UCRDY
> - * 4-7 reserved
> - */
> - return ((reg & 0xFF) >> 1) ^ 0x07;
> + return !((reg & UFSHCD_STATUS_READY) == (u32)UFSHCD_STATUS_READY);
yes, this makes it more readable, but i am not sure why we will have to
type cast UFSHCD_STATUS_READY macro to u32 type? i think it is
redundant, let me know if you think otherwise.
> }
>
> /**
> diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
> index d14e9b9..8cd4f0e 100644
> --- a/drivers/scsi/ufs/ufshci.h
> +++ b/drivers/scsi/ufs/ufshci.h
> @@ -159,6 +159,10 @@ enum {
> #define DEVICE_ERROR_INDICATOR UFS_BIT(5)
> #define UIC_POWER_MODE_CHANGE_REQ_STATUS_MASK UFS_MASK(0x7, 8)
>
> +#define UFSHCD_STATUS_READY (UTP_TRANSFER_REQ_LIST_READY |\
> + UTP_TASK_REQ_LIST_READY |\
> + UIC_COMMAND_READY)
> +
> enum {
> PWR_OK = 0x0,
> PWR_LOCAL = 0x01,
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 2/2] scsi: ufs: make ufshcd_get_lists_status() register operation obvious
2017-04-26 16:49 ` Subhash Jadavani
@ 2017-04-26 17:04 ` Tomohiro Kusumi
0 siblings, 0 replies; 7+ messages in thread
From: Tomohiro Kusumi @ 2017-04-26 17:04 UTC (permalink / raw)
To: Subhash Jadavani; +Cc: Vinayak Holikatti, linux-scsi, Tomohiro Kusumi
Hi,
Thanks for your review.
> yes, this makes it more readable, but i am not sure why we will have to type cast UFSHCD_STATUS_READY macro to u32 type? i think it is redundant, let me know if you think otherwise.
Sorry, nothing really.
I'll resend without u32 cast in short (i.e. change to below).
return !((reg & UFSHCD_STATUS_READY) == UFSHCD_STATUS_READY);
2017-04-26 19:49 GMT+03:00 Subhash Jadavani <subhashj@codeaurora.org>:
> On 2017-04-20 05:01, kusumi.tomohiro@gmail.com wrote:
>>
>> From: Tomohiro Kusumi <tkusumi@tuxera.com>
>>
>> It could be just cmp 0xe instead of >>1 and cmp 0x7, with readable code.
>>
>> Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
>> ---
>> drivers/scsi/ufs/ufshcd.c | 11 +----------
>> drivers/scsi/ufs/ufshci.h | 4 ++++
>> 2 files changed, 5 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index 9278666..ad9532b 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -668,16 +668,7 @@ static inline void
>> ufshcd_outstanding_req_clear(struct ufs_hba *hba, int tag)
>> */
>> static inline int ufshcd_get_lists_status(u32 reg)
>> {
>> - /*
>> - * The mask 0xFF is for the following HCS register bits
>> - * Bit Description
>> - * 0 Device Present
>> - * 1 UTRLRDY
>> - * 2 UTMRLRDY
>> - * 3 UCRDY
>> - * 4-7 reserved
>> - */
>> - return ((reg & 0xFF) >> 1) ^ 0x07;
>> + return !((reg & UFSHCD_STATUS_READY) == (u32)UFSHCD_STATUS_READY);
>
>
> yes, this makes it more readable, but i am not sure why we will have to type
> cast UFSHCD_STATUS_READY macro to u32 type? i think it is redundant, let me
> know if you think otherwise.
>
>> }
>>
>> /**
>> diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
>> index d14e9b9..8cd4f0e 100644
>> --- a/drivers/scsi/ufs/ufshci.h
>> +++ b/drivers/scsi/ufs/ufshci.h
>> @@ -159,6 +159,10 @@ enum {
>> #define DEVICE_ERROR_INDICATOR UFS_BIT(5)
>> #define UIC_POWER_MODE_CHANGE_REQ_STATUS_MASK UFS_MASK(0x7, 8)
>>
>> +#define UFSHCD_STATUS_READY (UTP_TRANSFER_REQ_LIST_READY |\
>> + UTP_TASK_REQ_LIST_READY |\
>> + UIC_COMMAND_READY)
>> +
>> enum {
>> PWR_OK = 0x0,
>> PWR_LOCAL = 0x01,
>
>
> --
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] scsi: ufs: use MASK_EE_STATUS
2017-04-20 12:01 [PATCH 1/2] scsi: ufs: use MASK_EE_STATUS kusumi.tomohiro
2017-04-20 12:01 ` [PATCH 2/2] scsi: ufs: make ufshcd_get_lists_status() register operation obvious kusumi.tomohiro
@ 2017-04-24 22:34 ` Martin K. Petersen
2017-04-26 16:39 ` Subhash Jadavani
2017-04-26 22:29 ` Martin K. Petersen
3 siblings, 0 replies; 7+ messages in thread
From: Martin K. Petersen @ 2017-04-24 22:34 UTC (permalink / raw)
To: kusumi.tomohiro; +Cc: subhashj, vinholikatti, linux-scsi, Tomohiro Kusumi
> From: Tomohiro Kusumi <tkusumi@tuxera.com>
>
> MASK_EE_STATUS added by 66ec6d59 was unused, but it seems to have been
> defined to do this.
>
> Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
Subhash: Please review these two. Thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] scsi: ufs: use MASK_EE_STATUS
2017-04-20 12:01 [PATCH 1/2] scsi: ufs: use MASK_EE_STATUS kusumi.tomohiro
2017-04-20 12:01 ` [PATCH 2/2] scsi: ufs: make ufshcd_get_lists_status() register operation obvious kusumi.tomohiro
2017-04-24 22:34 ` [PATCH 1/2] scsi: ufs: use MASK_EE_STATUS Martin K. Petersen
@ 2017-04-26 16:39 ` Subhash Jadavani
2017-04-26 22:29 ` Martin K. Petersen
3 siblings, 0 replies; 7+ messages in thread
From: Subhash Jadavani @ 2017-04-26 16:39 UTC (permalink / raw)
To: kusumi.tomohiro; +Cc: vinholikatti, linux-scsi, Tomohiro Kusumi
On 2017-04-20 05:01, kusumi.tomohiro@gmail.com wrote:
> From: Tomohiro Kusumi <tkusumi@tuxera.com>
>
> MASK_EE_STATUS added by 66ec6d59 was unused, but it seems to have been
> defined to do this.
>
> Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
> ---
> drivers/scsi/ufs/ufshcd.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 096e95b..9278666 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -4717,7 +4717,7 @@ static int ufshcd_disable_ee(struct ufs_hba
> *hba, u16 mask)
> goto out;
>
> val = hba->ee_ctrl_mask & ~mask;
> - val &= 0xFFFF; /* 2 bytes */
> + val &= MASK_EE_STATUS;
> err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
> QUERY_ATTR_IDN_EE_CONTROL, 0, 0, &val);
> if (!err)
> @@ -4745,7 +4745,7 @@ static int ufshcd_enable_ee(struct ufs_hba *hba,
> u16 mask)
> goto out;
>
> val = hba->ee_ctrl_mask | mask;
> - val &= 0xFFFF; /* 2 bytes */
> + val &= MASK_EE_STATUS;
> err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
> QUERY_ATTR_IDN_EE_CONTROL, 0, 0, &val);
> if (!err)
Looks good to me.
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] scsi: ufs: use MASK_EE_STATUS
2017-04-20 12:01 [PATCH 1/2] scsi: ufs: use MASK_EE_STATUS kusumi.tomohiro
` (2 preceding siblings ...)
2017-04-26 16:39 ` Subhash Jadavani
@ 2017-04-26 22:29 ` Martin K. Petersen
3 siblings, 0 replies; 7+ messages in thread
From: Martin K. Petersen @ 2017-04-26 22:29 UTC (permalink / raw)
To: kusumi.tomohiro; +Cc: subhashj, vinholikatti, linux-scsi, Tomohiro Kusumi
> MASK_EE_STATUS added by 66ec6d59 was unused, but it seems to have been
> defined to do this.
Applied to 4.12/scsi-queue. Thank you!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-04-26 22:30 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-20 12:01 [PATCH 1/2] scsi: ufs: use MASK_EE_STATUS kusumi.tomohiro
2017-04-20 12:01 ` [PATCH 2/2] scsi: ufs: make ufshcd_get_lists_status() register operation obvious kusumi.tomohiro
2017-04-26 16:49 ` Subhash Jadavani
2017-04-26 17:04 ` Tomohiro Kusumi
2017-04-24 22:34 ` [PATCH 1/2] scsi: ufs: use MASK_EE_STATUS Martin K. Petersen
2017-04-26 16:39 ` Subhash Jadavani
2017-04-26 22:29 ` Martin K. Petersen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox