* Re: [PATCH 2/7] scsi: ufs: find out sense data over scsi status values
2013-07-26 13:46 ` [PATCH 2/7] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
@ 2013-07-29 6:35 ` Subhash Jadavani
2013-07-30 13:00 ` Seungwon Jeon
2013-07-29 10:51 ` Sujit Reddy Thumma
` (14 subsequent siblings)
15 siblings, 1 reply; 72+ messages in thread
From: Subhash Jadavani @ 2013-07-29 6:35 UTC (permalink / raw)
To: Seungwon Jeon
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
On 7/26/2013 7:16 PM, Seungwon Jeon wrote:
> Except for 'GOOD' and 'CHECK CONDITION', other status value
This is what UFS device spec says: "A GOOD status indicates successful
SCSI completion and therefore no Sense Data will be returned." So please
change the commit text accordingly.
> in Response UPIU may or may contain sense data. If a non-zero
s/"may or may contain ..."/"may or may not contain"
> value is in the Data Segment Length field, it means that UPIU
> has Sense Data in the Data Segment area.
>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> ---
> drivers/scsi/ufs/ufs.h | 1 +
> drivers/scsi/ufs/ufshcd.c | 27 +++++++++++++++++++--------
> 2 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index 139bc06..737c31b 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -114,6 +114,7 @@ enum {
> MASK_SCSI_STATUS = 0xFF,
> MASK_TASK_RESPONSE = 0xFF00,
> MASK_RSP_UPIU_RESULT = 0xFFFF,
> + MASK_RSP_UPIU_DATA_SEG_LEN = 0xFFFF,
> };
>
> /* Task management service response */
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 4cf3a2d..688ae0e 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -218,6 +218,20 @@ ufshcd_get_rsp_upiu_result(struct utp_upiu_rsp *ucd_rsp_ptr)
> return be32_to_cpu(ucd_rsp_ptr->header.dword_1) & MASK_RSP_UPIU_RESULT;
> }
>
> +/*
> + * ufshcd_get_rsp_upiu_data_seg_len - Get the data segment length
> + * from response UPIU
> + * @ucd_rsp_ptr: pointer to response UPIU
> + *
> + * Return the data segment length.
> + */
> +static inline int
> +ufshcd_get_rsp_upiu_data_seg_len(struct utp_upiu_rsp *ucd_rsp_ptr)
> +{
> + return be32_to_cpu(ucd_rsp_ptr->header.dword_2) &
> + MASK_RSP_UPIU_DATA_SEG_LEN;
> +}
> +
> /**
> * ufshcd_config_int_aggr - Configure interrupt aggregation values.
> * Currently there is no use case where we want to configure
> @@ -298,7 +312,8 @@ void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag)
> static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
> {
> int len;
> - if (lrbp->sense_buffer) {
> + if (lrbp->sense_buffer &&
> + !ufshcd_get_rsp_upiu_data_seg_len(lrbp->ucd_rsp_ptr)) {
why if(!data_seg_len)? I guess you want to copy the sense data tot sense
buffer only if the data segment length is non zero so in that case it
should be "if(data_seg_len)".
> len = be16_to_cpu(lrbp->ucd_rsp_ptr->sense_data_len);
> memcpy(lrbp->sense_buffer,
> lrbp->ucd_rsp_ptr->sense_data,
> @@ -1156,21 +1171,17 @@ ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp, int scsi_status)
> SAM_STAT_CHECK_CONDITION;
> ufshcd_copy_sense_data(lrbp);
> break;
> - case SAM_STAT_BUSY:
> - result |= SAM_STAT_BUSY;
> - break;
> case SAM_STAT_TASK_SET_FULL:
> -
> /*
> * If a LUN reports SAM_STAT_TASK_SET_FULL, then the LUN queue
> * depth needs to be adjusted to the exact number of
> * outstanding commands the LUN can handle at any given time.
> */
> ufshcd_adjust_lun_qdepth(lrbp->cmd);
> - result |= SAM_STAT_TASK_SET_FULL;
> - break;
> + case SAM_STAT_BUSY:
> case SAM_STAT_TASK_ABORTED:
> - result |= SAM_STAT_TASK_ABORTED;
> + result |= scsi_status;
> + ufshcd_copy_sense_data(lrbp);
> break;
> default:
> result |= DID_ERROR << 16;
^ permalink raw reply [flat|nested] 72+ messages in thread* RE: [PATCH 2/7] scsi: ufs: find out sense data over scsi status values
2013-07-29 6:35 ` Subhash Jadavani
@ 2013-07-30 13:00 ` Seungwon Jeon
0 siblings, 0 replies; 72+ messages in thread
From: Seungwon Jeon @ 2013-07-30 13:00 UTC (permalink / raw)
To: 'Subhash Jadavani'
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
On Mon, July 29, 2013, Subhash Jadavani wrote:
> On 7/26/2013 7:16 PM, Seungwon Jeon wrote:
> > Except for 'GOOD' and 'CHECK CONDITION', other status value
> This is what UFS device spec says: "A GOOD status indicates successful
> SCSI completion and therefore no Sense Data will be returned." So please
> change the commit text accordingly.
Ok. It may seem vague.
> > in Response UPIU may or may contain sense data. If a non-zero
>
> s/"may or may contain ..."/"may or may not contain"
Yes.
> > value is in the Data Segment Length field, it means that UPIU
> > has Sense Data in the Data Segment area.
> >
> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> > ---
> > drivers/scsi/ufs/ufs.h | 1 +
> > drivers/scsi/ufs/ufshcd.c | 27 +++++++++++++++++++--------
> > 2 files changed, 20 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> > index 139bc06..737c31b 100644
> > --- a/drivers/scsi/ufs/ufs.h
> > +++ b/drivers/scsi/ufs/ufs.h
> > @@ -114,6 +114,7 @@ enum {
> > MASK_SCSI_STATUS = 0xFF,
> > MASK_TASK_RESPONSE = 0xFF00,
> > MASK_RSP_UPIU_RESULT = 0xFFFF,
> > + MASK_RSP_UPIU_DATA_SEG_LEN = 0xFFFF,
> > };
> >
> > /* Task management service response */
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 4cf3a2d..688ae0e 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -218,6 +218,20 @@ ufshcd_get_rsp_upiu_result(struct utp_upiu_rsp *ucd_rsp_ptr)
> > return be32_to_cpu(ucd_rsp_ptr->header.dword_1) & MASK_RSP_UPIU_RESULT;
> > }
> >
> > +/*
> > + * ufshcd_get_rsp_upiu_data_seg_len - Get the data segment length
> > + * from response UPIU
> > + * @ucd_rsp_ptr: pointer to response UPIU
> > + *
> > + * Return the data segment length.
> > + */
> > +static inline int
> > +ufshcd_get_rsp_upiu_data_seg_len(struct utp_upiu_rsp *ucd_rsp_ptr)
> > +{
> > + return be32_to_cpu(ucd_rsp_ptr->header.dword_2) &
> > + MASK_RSP_UPIU_DATA_SEG_LEN;
> > +}
> > +
> > /**
> > * ufshcd_config_int_aggr - Configure interrupt aggregation values.
> > * Currently there is no use case where we want to configure
> > @@ -298,7 +312,8 @@ void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag)
> > static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
> > {
> > int len;
> > - if (lrbp->sense_buffer) {
> > + if (lrbp->sense_buffer &&
> > + !ufshcd_get_rsp_upiu_data_seg_len(lrbp->ucd_rsp_ptr)) {
>
> why if(!data_seg_len)? I guess you want to copy the sense data tot sense
> buffer only if the data segment length is non zero so in that case it
> should be "if(data_seg_len)".
Correct.
Thanks,
Seungwon Jeon
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 2/7] scsi: ufs: find out sense data over scsi status values
2013-07-26 13:46 ` [PATCH 2/7] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
2013-07-29 6:35 ` Subhash Jadavani
@ 2013-07-29 10:51 ` Sujit Reddy Thumma
2013-07-30 13:03 ` Seungwon Jeon
2013-07-30 3:53 ` Santosh Y
` (13 subsequent siblings)
15 siblings, 1 reply; 72+ messages in thread
From: Sujit Reddy Thumma @ 2013-07-29 10:51 UTC (permalink / raw)
To: Seungwon Jeon
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
On 7/26/2013 7:16 PM, Seungwon Jeon wrote:
> Except for 'GOOD' and 'CHECK CONDITION', other status value
> in Response UPIU may or may contain sense data. If a non-zero
> value is in the Data Segment Length field, it means that UPIU
> has Sense Data in the Data Segment area.
>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> ---
> drivers/scsi/ufs/ufs.h | 1 +
> drivers/scsi/ufs/ufshcd.c | 27 +++++++++++++++++++--------
> 2 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index 139bc06..737c31b 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -114,6 +114,7 @@ enum {
> MASK_SCSI_STATUS = 0xFF,
> MASK_TASK_RESPONSE = 0xFF00,
> MASK_RSP_UPIU_RESULT = 0xFFFF,
> + MASK_RSP_UPIU_DATA_SEG_LEN = 0xFFFF,
> };
>
> /* Task management service response */
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 4cf3a2d..688ae0e 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -218,6 +218,20 @@ ufshcd_get_rsp_upiu_result(struct utp_upiu_rsp *ucd_rsp_ptr)
> return be32_to_cpu(ucd_rsp_ptr->header.dword_1) & MASK_RSP_UPIU_RESULT;
> }
>
> +/*
> + * ufshcd_get_rsp_upiu_data_seg_len - Get the data segment length
> + * from response UPIU
> + * @ucd_rsp_ptr: pointer to response UPIU
> + *
> + * Return the data segment length.
> + */
> +static inline int
> +ufshcd_get_rsp_upiu_data_seg_len(struct utp_upiu_rsp *ucd_rsp_ptr)
> +{
> + return be32_to_cpu(ucd_rsp_ptr->header.dword_2) &
> + MASK_RSP_UPIU_DATA_SEG_LEN;
> +}
> +
> /**
> * ufshcd_config_int_aggr - Configure interrupt aggregation values.
> * Currently there is no use case where we want to configure
> @@ -298,7 +312,8 @@ void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag)
> static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
> {
> int len;
> - if (lrbp->sense_buffer) {
> + if (lrbp->sense_buffer &&
> + !ufshcd_get_rsp_upiu_data_seg_len(lrbp->ucd_rsp_ptr)) {
> len = be16_to_cpu(lrbp->ucd_rsp_ptr->sense_data_len);
> memcpy(lrbp->sense_buffer,
> lrbp->ucd_rsp_ptr->sense_data,
> @@ -1156,21 +1171,17 @@ ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp, int scsi_status)
> SAM_STAT_CHECK_CONDITION;
The whole function is doing things wrong. It is redundant to interpret
the scsi_status at the LLD layer and make decisions while otherwise the
SCSI layer anyway knows how handle them properly.
Please see scsi_decide_disposition().
Ideally, this is what we should do -
if (response_upiu_sense_data_len > 0 && lrbp->sense_buffer) {
ufshcd_copy_sense_data();
}
result = set_host_byte(DID_OK) |
set_msg_byte(COMMAND_COMPLETE) |
(scsi_status & 0xff);
> ufshcd_copy_sense_data(lrbp);
> break;
> - case SAM_STAT_BUSY:
> - result |= SAM_STAT_BUSY;
> - break;
> case SAM_STAT_TASK_SET_FULL:
> -
> /*
> * If a LUN reports SAM_STAT_TASK_SET_FULL, then the LUN queue
> * depth needs to be adjusted to the exact number of
> * outstanding commands the LUN can handle at any given time.
> */
> ufshcd_adjust_lun_qdepth(lrbp->cmd);
> - result |= SAM_STAT_TASK_SET_FULL;
> - break;
> + case SAM_STAT_BUSY:
> case SAM_STAT_TASK_ABORTED:
> - result |= SAM_STAT_TASK_ABORTED;
> + result |= scsi_status;
> + ufshcd_copy_sense_data(lrbp);
> break;
> default:
> result |= DID_ERROR << 16;
>
--
Regards,
Sujit
^ permalink raw reply [flat|nested] 72+ messages in thread* RE: [PATCH 2/7] scsi: ufs: find out sense data over scsi status values
2013-07-29 10:51 ` Sujit Reddy Thumma
@ 2013-07-30 13:03 ` Seungwon Jeon
0 siblings, 0 replies; 72+ messages in thread
From: Seungwon Jeon @ 2013-07-30 13:03 UTC (permalink / raw)
To: 'Sujit Reddy Thumma'
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
On Mon, July 29, 2013, Sujit Reddy Thumma wrote:
> On 7/26/2013 7:16 PM, Seungwon Jeon wrote:
> > Except for 'GOOD' and 'CHECK CONDITION', other status value
> > in Response UPIU may or may contain sense data. If a non-zero
> > value is in the Data Segment Length field, it means that UPIU
> > has Sense Data in the Data Segment area.
> >
> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> > ---
> > drivers/scsi/ufs/ufs.h | 1 +
> > drivers/scsi/ufs/ufshcd.c | 27 +++++++++++++++++++--------
> > 2 files changed, 20 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> > index 139bc06..737c31b 100644
> > --- a/drivers/scsi/ufs/ufs.h
> > +++ b/drivers/scsi/ufs/ufs.h
> > @@ -114,6 +114,7 @@ enum {
> > MASK_SCSI_STATUS = 0xFF,
> > MASK_TASK_RESPONSE = 0xFF00,
> > MASK_RSP_UPIU_RESULT = 0xFFFF,
> > + MASK_RSP_UPIU_DATA_SEG_LEN = 0xFFFF,
> > };
> >
> > /* Task management service response */
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 4cf3a2d..688ae0e 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -218,6 +218,20 @@ ufshcd_get_rsp_upiu_result(struct utp_upiu_rsp *ucd_rsp_ptr)
> > return be32_to_cpu(ucd_rsp_ptr->header.dword_1) & MASK_RSP_UPIU_RESULT;
> > }
> >
> > +/*
> > + * ufshcd_get_rsp_upiu_data_seg_len - Get the data segment length
> > + * from response UPIU
> > + * @ucd_rsp_ptr: pointer to response UPIU
> > + *
> > + * Return the data segment length.
> > + */
> > +static inline int
> > +ufshcd_get_rsp_upiu_data_seg_len(struct utp_upiu_rsp *ucd_rsp_ptr)
> > +{
> > + return be32_to_cpu(ucd_rsp_ptr->header.dword_2) &
> > + MASK_RSP_UPIU_DATA_SEG_LEN;
> > +}
> > +
> > /**
> > * ufshcd_config_int_aggr - Configure interrupt aggregation values.
> > * Currently there is no use case where we want to configure
> > @@ -298,7 +312,8 @@ void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag)
> > static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
> > {
> > int len;
> > - if (lrbp->sense_buffer) {
> > + if (lrbp->sense_buffer &&
> > + !ufshcd_get_rsp_upiu_data_seg_len(lrbp->ucd_rsp_ptr)) {
> > len = be16_to_cpu(lrbp->ucd_rsp_ptr->sense_data_len);
> > memcpy(lrbp->sense_buffer,
> > lrbp->ucd_rsp_ptr->sense_data,
> > @@ -1156,21 +1171,17 @@ ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp, int scsi_status)
> > SAM_STAT_CHECK_CONDITION;
>
> The whole function is doing things wrong. It is redundant to interpret
> the scsi_status at the LLD layer and make decisions while otherwise the
> SCSI layer anyway knows how handle them properly.
Yes, scsi status can be set simply here.
>
> Please see scsi_decide_disposition().
>
> Ideally, this is what we should do -
> if (response_upiu_sense_data_len > 0 && lrbp->sense_buffer) {
> ufshcd_copy_sense_data();
> }
> result = set_host_byte(DID_OK) |
How can we determine DID_OK in ufshcd_scsi_cmd_status?
I think we can add this only in case of OCS_SUCCESS in ufshcd_transfer_rsp_status.
> set_msg_byte(COMMAND_COMPLETE) |
I'm not sure that we need to set COMMAND_COMPLETE?
Message is related to SCSI-2.
I didn't get this information on SAM, SBC and SPC.
Do you have any idea?
Thanks,
Seungwon Jeon
> (scsi_status & 0xff);
>
> > ufshcd_copy_sense_data(lrbp);
> > break;
> > - case SAM_STAT_BUSY:
> > - result |= SAM_STAT_BUSY;
> > - break;
> > case SAM_STAT_TASK_SET_FULL:
> > -
> > /*
> > * If a LUN reports SAM_STAT_TASK_SET_FULL, then the LUN queue
> > * depth needs to be adjusted to the exact number of
> > * outstanding commands the LUN can handle at any given time.
> > */
> > ufshcd_adjust_lun_qdepth(lrbp->cmd);
> > - result |= SAM_STAT_TASK_SET_FULL;
> > - break;
> > + case SAM_STAT_BUSY:
> > case SAM_STAT_TASK_ABORTED:
> > - result |= SAM_STAT_TASK_ABORTED;
> > + result |= scsi_status;
> > + ufshcd_copy_sense_data(lrbp);
> > break;
> > default:
> > result |= DID_ERROR << 16;
> >
>
> --
> Regards,
> Sujit
> --
> 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
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH 2/7] scsi: ufs: find out sense data over scsi status values
2013-07-26 13:46 ` [PATCH 2/7] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
2013-07-29 6:35 ` Subhash Jadavani
2013-07-29 10:51 ` Sujit Reddy Thumma
@ 2013-07-30 3:53 ` Santosh Y
2013-07-30 13:03 ` Seungwon Jeon
2013-08-23 13:00 ` [PATCH v2 1/6] " Seungwon Jeon
` (12 subsequent siblings)
15 siblings, 1 reply; 72+ messages in thread
From: Santosh Y @ 2013-07-30 3:53 UTC (permalink / raw)
To: Seungwon Jeon; +Cc: linux-scsi, Vinayak Holikatti, James E.J. Bottomley
On Fri, Jul 26, 2013 at 7:16 PM, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> Except for 'GOOD' and 'CHECK CONDITION', other status value
> in Response UPIU may or may contain sense data. If a non-zero
> value is in the Data Segment Length field, it means that UPIU
> has Sense Data in the Data Segment area.
>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> ---
> drivers/scsi/ufs/ufs.h | 1 +
> drivers/scsi/ufs/ufshcd.c | 27 +++++++++++++++++++--------
> 2 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index 139bc06..737c31b 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -114,6 +114,7 @@ enum {
> MASK_SCSI_STATUS = 0xFF,
> MASK_TASK_RESPONSE = 0xFF00,
> MASK_RSP_UPIU_RESULT = 0xFFFF,
> + MASK_RSP_UPIU_DATA_SEG_LEN = 0xFFFF,
> };
>
> /* Task management service response */
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 4cf3a2d..688ae0e 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -218,6 +218,20 @@ ufshcd_get_rsp_upiu_result(struct utp_upiu_rsp *ucd_rsp_ptr)
> return be32_to_cpu(ucd_rsp_ptr->header.dword_1) & MASK_RSP_UPIU_RESULT;
> }
>
> +/*
> + * ufshcd_get_rsp_upiu_data_seg_len - Get the data segment length
> + * from response UPIU
> + * @ucd_rsp_ptr: pointer to response UPIU
> + *
> + * Return the data segment length.
> + */
> +static inline int
> +ufshcd_get_rsp_upiu_data_seg_len(struct utp_upiu_rsp *ucd_rsp_ptr)
> +{
> + return be32_to_cpu(ucd_rsp_ptr->header.dword_2) &
> + MASK_RSP_UPIU_DATA_SEG_LEN;
> +}
> +
> /**
> * ufshcd_config_int_aggr - Configure interrupt aggregation values.
> * Currently there is no use case where we want to configure
> @@ -298,7 +312,8 @@ void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag)
> static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
> {
> int len;
> - if (lrbp->sense_buffer) {
> + if (lrbp->sense_buffer &&
> + !ufshcd_get_rsp_upiu_data_seg_len(lrbp->ucd_rsp_ptr)) {
> len = be16_to_cpu(lrbp->ucd_rsp_ptr->sense_data_len);
> memcpy(lrbp->sense_buffer,
> lrbp->ucd_rsp_ptr->sense_data,
Also please change the SCSI_SENSE_BUFFERSIZE to 18-bytes as per the
spec, *in case* if wrong 'data segment length' is returned, there
won't be a need to memcpy extra bytes ;-).
#define UFS_SENSE_BUFFERSIZE 18
memcpy(lrbp->sense_buffer,
lrbp->ucd_rsp_ptr->sense_data,
min_t(int, len, UFS_SENSE_BUFFERSIZE));
> @@ -1156,21 +1171,17 @@ ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp, int scsi_status)
> SAM_STAT_CHECK_CONDITION;
> ufshcd_copy_sense_data(lrbp);
> break;
> - case SAM_STAT_BUSY:
> - result |= SAM_STAT_BUSY;
> - break;
> case SAM_STAT_TASK_SET_FULL:
> -
> /*
> * If a LUN reports SAM_STAT_TASK_SET_FULL, then the LUN queue
> * depth needs to be adjusted to the exact number of
> * outstanding commands the LUN can handle at any given time.
> */
> ufshcd_adjust_lun_qdepth(lrbp->cmd);
> - result |= SAM_STAT_TASK_SET_FULL;
> - break;
> + case SAM_STAT_BUSY:
> case SAM_STAT_TASK_ABORTED:
> - result |= SAM_STAT_TASK_ABORTED;
> + result |= scsi_status;
> + ufshcd_copy_sense_data(lrbp);
> break;
> default:
> result |= DID_ERROR << 16;
> --
> 1.7.0.4
>
>
--
~Santosh
^ permalink raw reply [flat|nested] 72+ messages in thread* RE: [PATCH 2/7] scsi: ufs: find out sense data over scsi status values
2013-07-30 3:53 ` Santosh Y
@ 2013-07-30 13:03 ` Seungwon Jeon
2013-07-31 0:15 ` Elliott, Robert (Server Storage)
0 siblings, 1 reply; 72+ messages in thread
From: Seungwon Jeon @ 2013-07-30 13:03 UTC (permalink / raw)
To: 'Santosh Y'
Cc: linux-scsi, 'Vinayak Holikatti', 'James E.J. Bottomley'
On Tue, July 30, 2013, Santosh Y wrote:
> On Fri, Jul 26, 2013 at 7:16 PM, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> > Except for 'GOOD' and 'CHECK CONDITION', other status value
> > in Response UPIU may or may contain sense data. If a non-zero
> > value is in the Data Segment Length field, it means that UPIU
> > has Sense Data in the Data Segment area.
> >
> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> > ---
> > drivers/scsi/ufs/ufs.h | 1 +
> > drivers/scsi/ufs/ufshcd.c | 27 +++++++++++++++++++--------
> > 2 files changed, 20 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> > index 139bc06..737c31b 100644
> > --- a/drivers/scsi/ufs/ufs.h
> > +++ b/drivers/scsi/ufs/ufs.h
> > @@ -114,6 +114,7 @@ enum {
> > MASK_SCSI_STATUS = 0xFF,
> > MASK_TASK_RESPONSE = 0xFF00,
> > MASK_RSP_UPIU_RESULT = 0xFFFF,
> > + MASK_RSP_UPIU_DATA_SEG_LEN = 0xFFFF,
> > };
> >
> > /* Task management service response */
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 4cf3a2d..688ae0e 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -218,6 +218,20 @@ ufshcd_get_rsp_upiu_result(struct utp_upiu_rsp *ucd_rsp_ptr)
> > return be32_to_cpu(ucd_rsp_ptr->header.dword_1) & MASK_RSP_UPIU_RESULT;
> > }
> >
> > +/*
> > + * ufshcd_get_rsp_upiu_data_seg_len - Get the data segment length
> > + * from response UPIU
> > + * @ucd_rsp_ptr: pointer to response UPIU
> > + *
> > + * Return the data segment length.
> > + */
> > +static inline int
> > +ufshcd_get_rsp_upiu_data_seg_len(struct utp_upiu_rsp *ucd_rsp_ptr)
> > +{
> > + return be32_to_cpu(ucd_rsp_ptr->header.dword_2) &
> > + MASK_RSP_UPIU_DATA_SEG_LEN;
> > +}
> > +
> > /**
> > * ufshcd_config_int_aggr - Configure interrupt aggregation values.
> > * Currently there is no use case where we want to configure
> > @@ -298,7 +312,8 @@ void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag)
> > static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
> > {
> > int len;
> > - if (lrbp->sense_buffer) {
> > + if (lrbp->sense_buffer &&
> > + !ufshcd_get_rsp_upiu_data_seg_len(lrbp->ucd_rsp_ptr)) {
> > len = be16_to_cpu(lrbp->ucd_rsp_ptr->sense_data_len);
> > memcpy(lrbp->sense_buffer,
> > lrbp->ucd_rsp_ptr->sense_data,
>
> Also please change the SCSI_SENSE_BUFFERSIZE to 18-bytes as per the
> spec, *in case* if wrong 'data segment length' is returned, there
> won't be a need to memcpy extra bytes ;-).
>
> #define UFS_SENSE_BUFFERSIZE 18
> memcpy(lrbp->sense_buffer,
> lrbp->ucd_rsp_ptr->sense_data,
> min_t(int, len, UFS_SENSE_BUFFERSIZE));
Ok. I'll check.
But considering the extension with spec. revision, it's not bad to be kept.
Current: [#define SCSI_SENSE_BUFFERSIZE 96]
Thanks,
Seungwon Jeon
>
>
> > @@ -1156,21 +1171,17 @@ ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp, int scsi_status)
> > SAM_STAT_CHECK_CONDITION;
> > ufshcd_copy_sense_data(lrbp);
> > break;
> > - case SAM_STAT_BUSY:
> > - result |= SAM_STAT_BUSY;
> > - break;
> > case SAM_STAT_TASK_SET_FULL:
> > -
> > /*
> > * If a LUN reports SAM_STAT_TASK_SET_FULL, then the LUN queue
> > * depth needs to be adjusted to the exact number of
> > * outstanding commands the LUN can handle at any given time.
> > */
> > ufshcd_adjust_lun_qdepth(lrbp->cmd);
> > - result |= SAM_STAT_TASK_SET_FULL;
> > - break;
> > + case SAM_STAT_BUSY:
> > case SAM_STAT_TASK_ABORTED:
> > - result |= SAM_STAT_TASK_ABORTED;
> > + result |= scsi_status;
> > + ufshcd_copy_sense_data(lrbp);
> > break;
> > default:
> > result |= DID_ERROR << 16;
> > --
> > 1.7.0.4
> >
> >
>
>
>
> --
> ~Santosh
> --
> 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
^ permalink raw reply [flat|nested] 72+ messages in thread* RE: [PATCH 2/7] scsi: ufs: find out sense data over scsi status values
2013-07-30 13:03 ` Seungwon Jeon
@ 2013-07-31 0:15 ` Elliott, Robert (Server Storage)
2013-08-06 12:08 ` Seungwon Jeon
0 siblings, 1 reply; 72+ messages in thread
From: Elliott, Robert (Server Storage) @ 2013-07-31 0:15 UTC (permalink / raw)
To: Seungwon Jeon, 'Santosh Y'
Cc: linux-scsi, 'Vinayak Holikatti', 'James E.J. Bottomley'
I don't know how well UFS keeps up with SCSI architecture, but SCSI
has expanded from the old 18-byte fixed format sense data to a
variable-length descriptor format sense data, in which an 8 byte
header precedes a variable number of descriptors. This was devised
to support 8-byte LBAs in the INFORMATION field and
COMMAND-SPECIFIC INFORMATION field for >2 TiB drives.
A few other new features:
1. Extended INQUIRY Data VPD page
MAXIMUM SUPPORTED SENSE DATA LENGTH field:
Report how much sense data the device server is capable of returning
2. Control Extensions mode page
MAXIMUM SENSE DATA LENGTH field:
Specify how much sense data the device server is allowed to return
3. Descriptor format sense data header
SDAT_OVFL bit:
Reports that the device server had to drop some descriptors
to fit in the specified length
4. "Direct-access block device sense data descriptor"
Devised for the use by Base feature set compliant designs.
Combined with the 8 byte header, results in 32 bytes of sense data.
Always carries an INFORMATION field, COMMAND-SPECIFIC
INFORMATION field, sense-key specific information, field
replaceable unit code, and ILI bit.
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> owner@vger.kernel.org] On Behalf Of Seungwon Jeon
> Sent: Tuesday, 30 July, 2013 8:03 AM
> To: 'Santosh Y'
> Cc: linux-scsi@vger.kernel.org; 'Vinayak Holikatti'; 'James E.J. Bottomley'
> Subject: RE: [PATCH 2/7] scsi: ufs: find out sense data over scsi status values
>
> On Tue, July 30, 2013, Santosh Y wrote:
> > On Fri, Jul 26, 2013 at 7:16 PM, Seungwon Jeon <tgih.jun@samsung.com>
> wrote:
> > > Except for 'GOOD' and 'CHECK CONDITION', other status value
> > > in Response UPIU may or may contain sense data. If a non-zero
> > > value is in the Data Segment Length field, it means that UPIU
> > > has Sense Data in the Data Segment area.
> > >
> > > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> > > ---
> > > drivers/scsi/ufs/ufs.h | 1 +
> > > drivers/scsi/ufs/ufshcd.c | 27 +++++++++++++++++++--------
> > > 2 files changed, 20 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> > > index 139bc06..737c31b 100644
> > > --- a/drivers/scsi/ufs/ufs.h
> > > +++ b/drivers/scsi/ufs/ufs.h
> > > @@ -114,6 +114,7 @@ enum {
> > > MASK_SCSI_STATUS = 0xFF,
> > > MASK_TASK_RESPONSE = 0xFF00,
> > > MASK_RSP_UPIU_RESULT = 0xFFFF,
> > > + MASK_RSP_UPIU_DATA_SEG_LEN = 0xFFFF,
> > > };
> > >
> > > /* Task management service response */
> > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > > index 4cf3a2d..688ae0e 100644
> > > --- a/drivers/scsi/ufs/ufshcd.c
> > > +++ b/drivers/scsi/ufs/ufshcd.c
> > > @@ -218,6 +218,20 @@ ufshcd_get_rsp_upiu_result(struct utp_upiu_rsp
> *ucd_rsp_ptr)
> > > return be32_to_cpu(ucd_rsp_ptr->header.dword_1) &
> MASK_RSP_UPIU_RESULT;
> > > }
> > >
> > > +/*
> > > + * ufshcd_get_rsp_upiu_data_seg_len - Get the data segment length
> > > + * from response UPIU
> > > + * @ucd_rsp_ptr: pointer to response UPIU
> > > + *
> > > + * Return the data segment length.
> > > + */
> > > +static inline int
> > > +ufshcd_get_rsp_upiu_data_seg_len(struct utp_upiu_rsp *ucd_rsp_ptr)
> > > +{
> > > + return be32_to_cpu(ucd_rsp_ptr->header.dword_2) &
> > > + MASK_RSP_UPIU_DATA_SEG_LEN;
> > > +}
> > > +
> > > /**
> > > * ufshcd_config_int_aggr - Configure interrupt aggregation values.
> > > * Currently there is no use case where we want to configure
> > > @@ -298,7 +312,8 @@ void ufshcd_send_command(struct ufs_hba
> *hba, unsigned int task_tag)
> > > static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
> > > {
> > > int len;
> > > - if (lrbp->sense_buffer) {
> > > + if (lrbp->sense_buffer &&
> > > + !ufshcd_get_rsp_upiu_data_seg_len(lrbp->ucd_rsp_ptr)) {
> > > len = be16_to_cpu(lrbp->ucd_rsp_ptr->sense_data_len);
> > > memcpy(lrbp->sense_buffer,
> > > lrbp->ucd_rsp_ptr->sense_data,
> >
> > Also please change the SCSI_SENSE_BUFFERSIZE to 18-bytes as per the
> > spec, *in case* if wrong 'data segment length' is returned, there
> > won't be a need to memcpy extra bytes ;-).
> >
> > #define UFS_SENSE_BUFFERSIZE 18
> > memcpy(lrbp->sense_buffer,
> > lrbp->ucd_rsp_ptr->sense_data,
> > min_t(int, len, UFS_SENSE_BUFFERSIZE));
> Ok. I'll check.
> But considering the extension with spec. revision, it's not bad to be kept.
> Current: [#define SCSI_SENSE_BUFFERSIZE 96]
>
> Thanks,
> Seungwon Jeon
>
> >
> >
> > > @@ -1156,21 +1171,17 @@ ufshcd_scsi_cmd_status(struct ufshcd_lrb
> *lrbp, int scsi_status)
> > > SAM_STAT_CHECK_CONDITION;
> > > ufshcd_copy_sense_data(lrbp);
> > > break;
> > > - case SAM_STAT_BUSY:
> > > - result |= SAM_STAT_BUSY;
> > > - break;
> > > case SAM_STAT_TASK_SET_FULL:
> > > -
> > > /*
> > > * If a LUN reports SAM_STAT_TASK_SET_FULL, then the LUN
> queue
> > > * depth needs to be adjusted to the exact number of
> > > * outstanding commands the LUN can handle at any given time.
> > > */
> > > ufshcd_adjust_lun_qdepth(lrbp->cmd);
> > > - result |= SAM_STAT_TASK_SET_FULL;
> > > - break;
> > > + case SAM_STAT_BUSY:
> > > case SAM_STAT_TASK_ABORTED:
> > > - result |= SAM_STAT_TASK_ABORTED;
> > > + result |= scsi_status;
> > > + ufshcd_copy_sense_data(lrbp);
> > > break;
> > > default:
> > > result |= DID_ERROR << 16;
> > > --
> > > 1.7.0.4
> > >
> > >
> >
> >
> >
> > --
> > ~Santosh
> > --
> > 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-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 72+ messages in thread* RE: [PATCH 2/7] scsi: ufs: find out sense data over scsi status values
2013-07-31 0:15 ` Elliott, Robert (Server Storage)
@ 2013-08-06 12:08 ` Seungwon Jeon
0 siblings, 0 replies; 72+ messages in thread
From: Seungwon Jeon @ 2013-08-06 12:08 UTC (permalink / raw)
To: 'Elliott, Robert (Server Storage)', 'Santosh Y'
Cc: linux-scsi, 'Vinayak Holikatti', 'James E.J. Bottomley'
Hi Elliott,
Thank you for information.
It might be considered, if UFS has the extension someday.
Thanks,
Seungwon Jeon
On Wed, July 31, 2013, Elliott, Robert wrote:
> I don't know how well UFS keeps up with SCSI architecture, but SCSI
> has expanded from the old 18-byte fixed format sense data to a
> variable-length descriptor format sense data, in which an 8 byte
> header precedes a variable number of descriptors. This was devised
> to support 8-byte LBAs in the INFORMATION field and
> COMMAND-SPECIFIC INFORMATION field for >2 TiB drives.
>
> A few other new features:
>
> 1. Extended INQUIRY Data VPD page
> MAXIMUM SUPPORTED SENSE DATA LENGTH field:
> Report how much sense data the device server is capable of returning
> 2. Control Extensions mode page
> MAXIMUM SENSE DATA LENGTH field:
> Specify how much sense data the device server is allowed to return
> 3. Descriptor format sense data header
> SDAT_OVFL bit:
> Reports that the device server had to drop some descriptors
> to fit in the specified length
> 4. "Direct-access block device sense data descriptor"
> Devised for the use by Base feature set compliant designs.
> Combined with the 8 byte header, results in 32 bytes of sense data.
> Always carries an INFORMATION field, COMMAND-SPECIFIC
> INFORMATION field, sense-key specific information, field
> replaceable unit code, and ILI bit.
>
>
>
>
> > -----Original Message-----
> > From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> > owner@vger.kernel.org] On Behalf Of Seungwon Jeon
> > Sent: Tuesday, 30 July, 2013 8:03 AM
> > To: 'Santosh Y'
> > Cc: linux-scsi@vger.kernel.org; 'Vinayak Holikatti'; 'James E.J. Bottomley'
> > Subject: RE: [PATCH 2/7] scsi: ufs: find out sense data over scsi status values
> >
> > On Tue, July 30, 2013, Santosh Y wrote:
> > > On Fri, Jul 26, 2013 at 7:16 PM, Seungwon Jeon <tgih.jun@samsung.com>
> > wrote:
> > > > Except for 'GOOD' and 'CHECK CONDITION', other status value
> > > > in Response UPIU may or may contain sense data. If a non-zero
> > > > value is in the Data Segment Length field, it means that UPIU
> > > > has Sense Data in the Data Segment area.
> > > >
> > > > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> > > > ---
> > > > drivers/scsi/ufs/ufs.h | 1 +
> > > > drivers/scsi/ufs/ufshcd.c | 27 +++++++++++++++++++--------
> > > > 2 files changed, 20 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> > > > index 139bc06..737c31b 100644
> > > > --- a/drivers/scsi/ufs/ufs.h
> > > > +++ b/drivers/scsi/ufs/ufs.h
> > > > @@ -114,6 +114,7 @@ enum {
> > > > MASK_SCSI_STATUS = 0xFF,
> > > > MASK_TASK_RESPONSE = 0xFF00,
> > > > MASK_RSP_UPIU_RESULT = 0xFFFF,
> > > > + MASK_RSP_UPIU_DATA_SEG_LEN = 0xFFFF,
> > > > };
> > > >
> > > > /* Task management service response */
> > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > > > index 4cf3a2d..688ae0e 100644
> > > > --- a/drivers/scsi/ufs/ufshcd.c
> > > > +++ b/drivers/scsi/ufs/ufshcd.c
> > > > @@ -218,6 +218,20 @@ ufshcd_get_rsp_upiu_result(struct utp_upiu_rsp
> > *ucd_rsp_ptr)
> > > > return be32_to_cpu(ucd_rsp_ptr->header.dword_1) &
> > MASK_RSP_UPIU_RESULT;
> > > > }
> > > >
> > > > +/*
> > > > + * ufshcd_get_rsp_upiu_data_seg_len - Get the data segment length
> > > > + * from response UPIU
> > > > + * @ucd_rsp_ptr: pointer to response UPIU
> > > > + *
> > > > + * Return the data segment length.
> > > > + */
> > > > +static inline int
> > > > +ufshcd_get_rsp_upiu_data_seg_len(struct utp_upiu_rsp *ucd_rsp_ptr)
> > > > +{
> > > > + return be32_to_cpu(ucd_rsp_ptr->header.dword_2) &
> > > > + MASK_RSP_UPIU_DATA_SEG_LEN;
> > > > +}
> > > > +
> > > > /**
> > > > * ufshcd_config_int_aggr - Configure interrupt aggregation values.
> > > > * Currently there is no use case where we want to configure
> > > > @@ -298,7 +312,8 @@ void ufshcd_send_command(struct ufs_hba
> > *hba, unsigned int task_tag)
> > > > static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
> > > > {
> > > > int len;
> > > > - if (lrbp->sense_buffer) {
> > > > + if (lrbp->sense_buffer &&
> > > > + !ufshcd_get_rsp_upiu_data_seg_len(lrbp->ucd_rsp_ptr)) {
> > > > len = be16_to_cpu(lrbp->ucd_rsp_ptr->sense_data_len);
> > > > memcpy(lrbp->sense_buffer,
> > > > lrbp->ucd_rsp_ptr->sense_data,
> > >
> > > Also please change the SCSI_SENSE_BUFFERSIZE to 18-bytes as per the
> > > spec, *in case* if wrong 'data segment length' is returned, there
> > > won't be a need to memcpy extra bytes ;-).
> > >
> > > #define UFS_SENSE_BUFFERSIZE 18
> > > memcpy(lrbp->sense_buffer,
> > > lrbp->ucd_rsp_ptr->sense_data,
> > > min_t(int, len, UFS_SENSE_BUFFERSIZE));
> > Ok. I'll check.
> > But considering the extension with spec. revision, it's not bad to be kept.
> > Current: [#define SCSI_SENSE_BUFFERSIZE 96]
> >
> > Thanks,
> > Seungwon Jeon
> >
> > >
> > >
> > > > @@ -1156,21 +1171,17 @@ ufshcd_scsi_cmd_status(struct ufshcd_lrb
> > *lrbp, int scsi_status)
> > > > SAM_STAT_CHECK_CONDITION;
> > > > ufshcd_copy_sense_data(lrbp);
> > > > break;
> > > > - case SAM_STAT_BUSY:
> > > > - result |= SAM_STAT_BUSY;
> > > > - break;
> > > > case SAM_STAT_TASK_SET_FULL:
> > > > -
> > > > /*
> > > > * If a LUN reports SAM_STAT_TASK_SET_FULL, then the LUN
> > queue
> > > > * depth needs to be adjusted to the exact number of
> > > > * outstanding commands the LUN can handle at any given time.
> > > > */
> > > > ufshcd_adjust_lun_qdepth(lrbp->cmd);
> > > > - result |= SAM_STAT_TASK_SET_FULL;
> > > > - break;
> > > > + case SAM_STAT_BUSY:
> > > > case SAM_STAT_TASK_ABORTED:
> > > > - result |= SAM_STAT_TASK_ABORTED;
> > > > + result |= scsi_status;
> > > > + ufshcd_copy_sense_data(lrbp);
> > > > break;
> > > > default:
> > > > result |= DID_ERROR << 16;
> > > > --
> > > > 1.7.0.4
> > > >
> > > >
> > >
> > >
> > >
> > > --
> > > ~Santosh
> > > --
> > > 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-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-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v2 1/6] scsi: ufs: find out sense data over scsi status values
2013-07-26 13:46 ` [PATCH 2/7] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
` (2 preceding siblings ...)
2013-07-30 3:53 ` Santosh Y
@ 2013-08-23 13:00 ` Seungwon Jeon
2013-08-23 13:00 ` [PATCH v2 2/6] scsi: ufs: fix the setting interrupt aggregation counter Seungwon Jeon
` (11 subsequent siblings)
15 siblings, 0 replies; 72+ messages in thread
From: Seungwon Jeon @ 2013-08-23 13:00 UTC (permalink / raw)
To: linux-scsi
Cc: 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
Unlike 'GOOD' and 'CHECK CONDITION', other status values in
Response UPIU may or may not contain sense data. That is returning
sense data isn't obvious. So, in this case the Data Segment Length
field should be checked. If a non-zero value, it means that UPIU
has Sense Data in the Data Segment area.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
---
drivers/scsi/ufs/ufs.h | 1 +
drivers/scsi/ufs/ufshcd.c | 37 ++++++++++++++++++++++---------------
2 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 139bc06..737c31b 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -114,6 +114,7 @@ enum {
MASK_SCSI_STATUS = 0xFF,
MASK_TASK_RESPONSE = 0xFF00,
MASK_RSP_UPIU_RESULT = 0xFFFF,
+ MASK_RSP_UPIU_DATA_SEG_LEN = 0xFFFF,
};
/* Task management service response */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b743bd6..529a0fc 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -218,6 +218,20 @@ ufshcd_get_rsp_upiu_result(struct utp_upiu_rsp *ucd_rsp_ptr)
return be32_to_cpu(ucd_rsp_ptr->header.dword_1) & MASK_RSP_UPIU_RESULT;
}
+/*
+ * ufshcd_get_rsp_upiu_data_seg_len - Get the data segment length
+ * from response UPIU
+ * @ucd_rsp_ptr: pointer to response UPIU
+ *
+ * Return the data segment length.
+ */
+static inline unsigned int
+ufshcd_get_rsp_upiu_data_seg_len(struct utp_upiu_rsp *ucd_rsp_ptr)
+{
+ return be32_to_cpu(ucd_rsp_ptr->header.dword_2) &
+ MASK_RSP_UPIU_DATA_SEG_LEN;
+}
+
/**
* ufshcd_config_int_aggr - Configure interrupt aggregation values.
* Currently there is no use case where we want to configure
@@ -298,7 +312,8 @@ void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag)
static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
{
int len;
- if (lrbp->sense_buffer) {
+ if (lrbp->sense_buffer &&
+ ufshcd_get_rsp_upiu_data_seg_len(lrbp->ucd_rsp_ptr)) {
len = be16_to_cpu(lrbp->ucd_rsp_ptr->sense_data_len);
memcpy(lrbp->sense_buffer,
lrbp->ucd_rsp_ptr->sense_data,
@@ -1145,32 +1160,24 @@ ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp, int scsi_status)
int result = 0;
switch (scsi_status) {
- case SAM_STAT_GOOD:
- result |= DID_OK << 16 |
- COMMAND_COMPLETE << 8 |
- SAM_STAT_GOOD;
- break;
case SAM_STAT_CHECK_CONDITION:
+ ufshcd_copy_sense_data(lrbp);
+ case SAM_STAT_GOOD:
result |= DID_OK << 16 |
COMMAND_COMPLETE << 8 |
- SAM_STAT_CHECK_CONDITION;
- ufshcd_copy_sense_data(lrbp);
- break;
- case SAM_STAT_BUSY:
- result |= SAM_STAT_BUSY;
+ scsi_status;
break;
case SAM_STAT_TASK_SET_FULL:
-
/*
* If a LUN reports SAM_STAT_TASK_SET_FULL, then the LUN queue
* depth needs to be adjusted to the exact number of
* outstanding commands the LUN can handle at any given time.
*/
ufshcd_adjust_lun_qdepth(lrbp->cmd);
- result |= SAM_STAT_TASK_SET_FULL;
- break;
+ case SAM_STAT_BUSY:
case SAM_STAT_TASK_ABORTED:
- result |= SAM_STAT_TASK_ABORTED;
+ ufshcd_copy_sense_data(lrbp);
+ result |= scsi_status;
break;
default:
result |= DID_ERROR << 16;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH v2 2/6] scsi: ufs: fix the setting interrupt aggregation counter
2013-07-26 13:46 ` [PATCH 2/7] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
` (3 preceding siblings ...)
2013-08-23 13:00 ` [PATCH v2 1/6] " Seungwon Jeon
@ 2013-08-23 13:00 ` Seungwon Jeon
2013-08-23 13:00 ` [PATCH v2 3/6] scsi: ufs: add dme configuration primitives Seungwon Jeon
` (10 subsequent siblings)
15 siblings, 0 replies; 72+ messages in thread
From: Seungwon Jeon @ 2013-08-23 13:00 UTC (permalink / raw)
To: linux-scsi
Cc: 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
IACTH(Interrupt aggregation counter threshold) value is allowed
up to 0x1F and current setting value is the maximum.
This value is related with NUTRS(max:0x20) of HCI's capability.
Considering HCI controller doesn't support the maximum, IACTH
setting should be adjusted with possible value.
For that, existing 'ufshcd_config_int_aggr' is split into two part
[reset, configure].
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
---
drivers/scsi/ufs/ufshcd.c | 53 +++++++++++++++++++++-----------------------
drivers/scsi/ufs/ufshci.h | 4 +-
2 files changed, 27 insertions(+), 30 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 529a0fc..2263795 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -43,6 +43,9 @@
/* UIC command timeout, unit: ms */
#define UIC_CMD_TIMEOUT 500
+/* Interrupt aggregation default timeout, unit: 40us */
+#define INT_AGGR_DEF_TO 0x02
+
enum {
UFSHCD_MAX_CHANNEL = 0,
UFSHCD_MAX_ID = 1,
@@ -65,12 +68,6 @@ enum {
UFSHCD_INT_CLEAR,
};
-/* Interrupt aggregation options */
-enum {
- INT_AGGR_RESET,
- INT_AGGR_CONFIG,
-};
-
/**
* ufshcd_get_intr_mask - Get the interrupt bit mask
* @hba - Pointer to adapter instance
@@ -233,30 +230,30 @@ ufshcd_get_rsp_upiu_data_seg_len(struct utp_upiu_rsp *ucd_rsp_ptr)
}
/**
- * ufshcd_config_int_aggr - Configure interrupt aggregation values.
- * Currently there is no use case where we want to configure
- * interrupt aggregation dynamically. So to configure interrupt
- * aggregation, #define INT_AGGR_COUNTER_THRESHOLD_VALUE and
- * INT_AGGR_TIMEOUT_VALUE are used.
+ * ufshcd_reset_intr_aggr - Reset interrupt aggregation values.
* @hba: per adapter instance
- * @option: Interrupt aggregation option
*/
static inline void
-ufshcd_config_int_aggr(struct ufs_hba *hba, int option)
+ufshcd_reset_intr_aggr(struct ufs_hba *hba)
{
- switch (option) {
- case INT_AGGR_RESET:
- ufshcd_writel(hba, INT_AGGR_ENABLE |
- INT_AGGR_COUNTER_AND_TIMER_RESET,
- REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
- break;
- case INT_AGGR_CONFIG:
- ufshcd_writel(hba, INT_AGGR_ENABLE | INT_AGGR_PARAM_WRITE |
- INT_AGGR_COUNTER_THRESHOLD_VALUE |
- INT_AGGR_TIMEOUT_VALUE,
- REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
- break;
- }
+ ufshcd_writel(hba, INT_AGGR_ENABLE |
+ INT_AGGR_COUNTER_AND_TIMER_RESET,
+ REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
+}
+
+/**
+ * ufshcd_config_intr_aggr - Configure interrupt aggregation values.
+ * @hba: per adapter instance
+ * @cntr_thld: Interrupt aggregation counter threshold
+ * @timeout: Interrupt aggregation timeout value
+ */
+static inline void
+ufshcd_config_intr_aggr(struct ufs_hba *hba, u8 cnt, u8 timeout)
+{
+ ufshcd_writel(hba, INT_AGGR_ENABLE | INT_AGGR_PARAM_WRITE |
+ INT_AGGR_COUNTER_THLD_VAL(cnt) |
+ INT_AGGR_TIMEOUT_VAL(timeout),
+ REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
}
/**
@@ -853,7 +850,7 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba)
ufshcd_enable_intr(hba, UFSHCD_ENABLE_INTRS);
/* Configure interrupt aggregation */
- ufshcd_config_int_aggr(hba, INT_AGGR_CONFIG);
+ ufshcd_config_intr_aggr(hba, hba->nutrs - 1, INT_AGGR_DEF_TO);
/* Configure UTRL and UTMRL base address registers */
ufshcd_writel(hba, lower_32_bits(hba->utrdl_dma_addr),
@@ -1296,7 +1293,7 @@ static void ufshcd_transfer_req_compl(struct ufs_hba *hba)
hba->outstanding_reqs ^= completed_reqs;
/* Reset interrupt aggregation counters */
- ufshcd_config_int_aggr(hba, INT_AGGR_RESET);
+ ufshcd_reset_intr_aggr(hba);
}
/**
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index d5c5f14..a8f69cc 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -226,8 +226,8 @@ enum {
#define MASK_UIC_COMMAND_RESULT 0xFF
-#define INT_AGGR_COUNTER_THRESHOLD_VALUE (0x1F << 8)
-#define INT_AGGR_TIMEOUT_VALUE (0x02)
+#define INT_AGGR_COUNTER_THLD_VAL(c) (((c) & 0x1F) << 8)
+#define INT_AGGR_TIMEOUT_VAL(t) (((t) & 0xFF) << 0)
/* Interrupt disable masks */
enum {
--
1.7.0.4
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH v2 3/6] scsi: ufs: add dme configuration primitives
2013-07-26 13:46 ` [PATCH 2/7] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
` (4 preceding siblings ...)
2013-08-23 13:00 ` [PATCH v2 2/6] scsi: ufs: fix the setting interrupt aggregation counter Seungwon Jeon
@ 2013-08-23 13:00 ` Seungwon Jeon
2013-08-23 13:00 ` [PATCH v2 4/6] scsi: ufs: add unipro attribute IDs Seungwon Jeon
` (9 subsequent siblings)
15 siblings, 0 replies; 72+ messages in thread
From: Seungwon Jeon @ 2013-08-23 13:00 UTC (permalink / raw)
To: linux-scsi
Cc: 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
Implements to support GET and SET operations of the DME.
These operations are used to configure the behavior of
the UNIPRO. Along with basic operation, {Peer/AttrSetType}
can be mixed.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
---
drivers/scsi/ufs/ufshcd.c | 88 +++++++++++++++++++++++++++++++++++++++++++++
drivers/scsi/ufs/ufshcd.h | 51 ++++++++++++++++++++++++++
drivers/scsi/ufs/ufshci.h | 6 +++
3 files changed, 145 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 2263795..7fc262e 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -188,6 +188,18 @@ static inline int ufshcd_get_uic_cmd_result(struct ufs_hba *hba)
}
/**
+ * ufshcd_get_dme_attr_val - Get the value of attribute returned by UIC command
+ * @hba: Pointer to adapter instance
+ *
+ * This function gets UIC command argument3
+ * Returns 0 on success, non zero value on error
+ */
+static inline u32 ufshcd_get_dme_attr_val(struct ufs_hba *hba)
+{
+ return ufshcd_readl(hba, REG_UIC_COMMAND_ARG_3);
+}
+
+/**
* ufshcd_is_valid_req_rsp - checks if controller TR response is valid
* @ucd_rsp_ptr: pointer to response UPIU
*
@@ -821,6 +833,80 @@ static int ufshcd_dme_link_startup(struct ufs_hba *hba)
}
/**
+ * ufshcd_dme_set_attr - UIC command for DME_SET, DME_PEER_SET
+ * @hba: per adapter instance
+ * @attr_sel: uic command argument1
+ * @attr_set: attribute set type as uic command argument2
+ * @mib_val: setting value as uic command argument3
+ * @peer: indicate whether peer or local
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel,
+ u8 attr_set, u32 mib_val, u8 peer)
+{
+ struct uic_command uic_cmd = {0};
+ static const char *const action[] = {
+ "dme-set",
+ "dme-peer-set"
+ };
+ const char *set = action[!!peer];
+ int ret;
+
+ uic_cmd.command = peer ?
+ UIC_CMD_DME_PEER_SET : UIC_CMD_DME_SET;
+ uic_cmd.argument1 = attr_sel;
+ uic_cmd.argument2 = UIC_ARG_ATTR_TYPE(attr_set);
+ uic_cmd.argument3 = mib_val;
+
+ ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
+ if (ret)
+ dev_err(hba->dev, "%s: attr-id 0x%x val 0x%x error code %d\n",
+ set, UIC_GET_ATTR_ID(attr_sel), mib_val, ret);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(ufshcd_dme_set_attr);
+
+/**
+ * ufshcd_dme_get_attr - UIC command for DME_GET, DME_PEER_GET
+ * @hba: per adapter instance
+ * @attr_sel: uic command argument1
+ * @mib_val: the value of the attribute as returned by the UIC command
+ * @peer: indicate whether peer or local
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
+ u32 *mib_val, u8 peer)
+{
+ struct uic_command uic_cmd = {0};
+ static const char *const action[] = {
+ "dme-get",
+ "dme-peer-get"
+ };
+ const char *get = action[!!peer];
+ int ret;
+
+ uic_cmd.command = peer ?
+ UIC_CMD_DME_PEER_GET : UIC_CMD_DME_GET;
+ uic_cmd.argument1 = attr_sel;
+
+ ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
+ if (ret) {
+ dev_err(hba->dev, "%s: attr-id 0x%x error code %d\n",
+ get, UIC_GET_ATTR_ID(attr_sel), ret);
+ goto out;
+ }
+
+ if (mib_val)
+ *mib_val = uic_cmd.argument3;
+out:
+ return ret;
+}
+EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
+
+/**
* ufshcd_make_hba_operational - Make UFS controller operational
* @hba: per adapter instance
*
@@ -1253,6 +1339,8 @@ static void ufshcd_uic_cmd_compl(struct ufs_hba *hba)
if (hba->active_uic_cmd) {
hba->active_uic_cmd->argument2 |=
ufshcd_get_uic_cmd_result(hba);
+ hba->active_uic_cmd->argument3 =
+ ufshcd_get_dme_attr_val(hba);
complete(&hba->active_uic_cmd->done);
}
}
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 49590ee..50bcd29 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -199,6 +199,11 @@ int ufshcd_init(struct device *, struct ufs_hba ** , void __iomem * ,
unsigned int);
void ufshcd_remove(struct ufs_hba *);
+extern int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel,
+ u8 attr_set, u32 mib_val, u8 peer);
+extern int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
+ u32 *mib_val, u8 peer);
+
/**
* ufshcd_hba_stop - Send controller to reset state
* @hba: per adapter instance
@@ -208,4 +213,50 @@ static inline void ufshcd_hba_stop(struct ufs_hba *hba)
ufshcd_writel(hba, CONTROLLER_DISABLE, REG_CONTROLLER_ENABLE);
}
+/* UIC command interfaces for DME primitives */
+#define DME_LOCAL 0
+#define DME_PEER 1
+#define ATTR_SET_NOR 0 /* NORMAL */
+#define ATTR_SET_ST 1 /* STATIC */
+
+static inline int ufshcd_dme_set(struct ufs_hba *hba, u32 attr_sel,
+ u32 mib_val)
+{
+ return ufshcd_dme_set_attr(hba, attr_sel, ATTR_SET_NOR,
+ mib_val, DME_LOCAL);
+}
+
+static inline int ufshcd_dme_st_set(struct ufs_hba *hba, u32 attr_sel,
+ u32 mib_val)
+{
+ return ufshcd_dme_set_attr(hba, attr_sel, ATTR_SET_ST,
+ mib_val, DME_LOCAL);
+}
+
+static inline int ufshcd_dme_peer_set(struct ufs_hba *hba, u32 attr_sel,
+ u32 mib_val)
+{
+ return ufshcd_dme_set_attr(hba, attr_sel, ATTR_SET_NOR,
+ mib_val, DME_PEER);
+}
+
+static inline int ufshcd_dme_peer_st_set(struct ufs_hba *hba, u32 attr_sel,
+ u32 mib_val)
+{
+ return ufshcd_dme_set_attr(hba, attr_sel, ATTR_SET_ST,
+ mib_val, DME_PEER);
+}
+
+static inline int ufshcd_dme_get(struct ufs_hba *hba,
+ u32 attr_sel, u32 *mib_val)
+{
+ return ufshcd_dme_get_attr(hba, attr_sel, mib_val, DME_LOCAL);
+}
+
+static inline int ufshcd_dme_peer_get(struct ufs_hba *hba,
+ u32 attr_sel, u32 *mib_val)
+{
+ return ufshcd_dme_get_attr(hba, attr_sel, mib_val, DME_PEER);
+}
+
#endif /* End of Header */
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index a8f69cc..df4901e 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -191,6 +191,12 @@ enum {
#define CONFIG_RESULT_CODE_MASK 0xFF
#define GENERIC_ERROR_CODE_MASK 0xFF
+#define UIC_ARG_MIB_SEL(attr, sel) ((((attr) & 0xFFFF) << 16) |\
+ ((sel) & 0xFFFF))
+#define UIC_ARG_MIB(attr) UIC_ARG_MIB_SEL(attr, 0)
+#define UIC_ARG_ATTR_TYPE(t) (((t) & 0xFF) << 16)
+#define UIC_GET_ATTR_ID(v) (((v) >> 16) & 0xFFFF)
+
/* UIC Commands */
enum {
UIC_CMD_DME_GET = 0x01,
--
1.7.0.4
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH v2 4/6] scsi: ufs: add unipro attribute IDs
2013-07-26 13:46 ` [PATCH 2/7] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
` (5 preceding siblings ...)
2013-08-23 13:00 ` [PATCH v2 3/6] scsi: ufs: add dme configuration primitives Seungwon Jeon
@ 2013-08-23 13:00 ` Seungwon Jeon
2013-08-23 13:00 ` [PATCH v2 5/6] scsi: ufs: add operation for the uic power mode change Seungwon Jeon
` (8 subsequent siblings)
15 siblings, 0 replies; 72+ messages in thread
From: Seungwon Jeon @ 2013-08-23 13:00 UTC (permalink / raw)
To: linux-scsi
Cc: 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
'drivers/scsi/ufs/unipro.h' is added.
Attributes in the layers of the UNIPRO stack can be
read and written via the DME.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
---
drivers/scsi/ufs/unipro.h | 130 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 130 insertions(+), 0 deletions(-)
create mode 100644 drivers/scsi/ufs/unipro.h
diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h
new file mode 100644
index 0000000..3a710eb
--- /dev/null
+++ b/drivers/scsi/ufs/unipro.h
@@ -0,0 +1,130 @@
+/*
+ * drivers/scsi/ufs/unipro.h
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef _UNIPRO_H_
+#define _UNIPRO_H_
+
+/*
+ * PHY Adpater attributes
+ */
+#define PA_ACTIVETXDATALANES 0x1560
+#define PA_ACTIVERXDATALANES 0x1580
+#define PA_TXTRAILINGCLOCKS 0x1564
+#define PA_PHY_TYPE 0x1500
+#define PA_AVAILTXDATALANES 0x1520
+#define PA_AVAILRXDATALANES 0x1540
+#define PA_MINRXTRAILINGCLOCKS 0x1543
+#define PA_TXPWRSTATUS 0x1567
+#define PA_RXPWRSTATUS 0x1582
+#define PA_TXFORCECLOCK 0x1562
+#define PA_TXPWRMODE 0x1563
+#define PA_LEGACYDPHYESCDL 0x1570
+#define PA_MAXTXSPEEDFAST 0x1521
+#define PA_MAXTXSPEEDSLOW 0x1522
+#define PA_MAXRXSPEEDFAST 0x1541
+#define PA_MAXRXSPEEDSLOW 0x1542
+#define PA_TXLINKSTARTUPHS 0x1544
+#define PA_TXSPEEDFAST 0x1565
+#define PA_TXSPEEDSLOW 0x1566
+#define PA_REMOTEVERINFO 0x15A0
+#define PA_TXGEAR 0x1568
+#define PA_TXTERMINATION 0x1569
+#define PA_HSSERIES 0x156A
+#define PA_PWRMODE 0x1571
+#define PA_RXGEAR 0x1583
+#define PA_RXTERMINATION 0x1584
+#define PA_MAXRXPWMGEAR 0x1586
+#define PA_MAXRXHSGEAR 0x1587
+#define PA_RXHSUNTERMCAP 0x15A5
+#define PA_RXLSTERMCAP 0x15A6
+#define PA_PACPREQTIMEOUT 0x1590
+#define PA_PACPREQEOBTIMEOUT 0x1591
+#define PA_HIBERN8TIME 0x15A7
+#define PA_LOCALVERINFO 0x15A9
+#define PA_TACTIVATE 0x15A8
+#define PA_PACPFRAMECOUNT 0x15C0
+#define PA_PACPERRORCOUNT 0x15C1
+#define PA_PHYTESTCONTROL 0x15C2
+#define PA_PWRMODEUSERDATA0 0x15B0
+#define PA_PWRMODEUSERDATA1 0x15B1
+#define PA_PWRMODEUSERDATA2 0x15B2
+#define PA_PWRMODEUSERDATA3 0x15B3
+#define PA_PWRMODEUSERDATA4 0x15B4
+#define PA_PWRMODEUSERDATA5 0x15B5
+#define PA_PWRMODEUSERDATA6 0x15B6
+#define PA_PWRMODEUSERDATA7 0x15B7
+#define PA_PWRMODEUSERDATA8 0x15B8
+#define PA_PWRMODEUSERDATA9 0x15B9
+#define PA_PWRMODEUSERDATA10 0x15BA
+#define PA_PWRMODEUSERDATA11 0x15BB
+#define PA_CONNECTEDTXDATALANES 0x1561
+#define PA_CONNECTEDRXDATALANES 0x1581
+#define PA_LOGICALLANEMAP 0x15A1
+#define PA_SLEEPNOCONFIGTIME 0x15A2
+#define PA_STALLNOCONFIGTIME 0x15A3
+#define PA_SAVECONFIGTIME 0x15A4
+
+/*
+ * Data Link Layer Attributes
+ */
+#define DL_TC0TXFCTHRESHOLD 0x2040
+#define DL_FC0PROTTIMEOUTVAL 0x2041
+#define DL_TC0REPLAYTIMEOUTVAL 0x2042
+#define DL_AFC0REQTIMEOUTVAL 0x2043
+#define DL_AFC0CREDITTHRESHOLD 0x2044
+#define DL_TC0OUTACKTHRESHOLD 0x2045
+#define DL_TC1TXFCTHRESHOLD 0x2060
+#define DL_FC1PROTTIMEOUTVAL 0x2061
+#define DL_TC1REPLAYTIMEOUTVAL 0x2062
+#define DL_AFC1REQTIMEOUTVAL 0x2063
+#define DL_AFC1CREDITTHRESHOLD 0x2064
+#define DL_TC1OUTACKTHRESHOLD 0x2065
+#define DL_TXPREEMPTIONCAP 0x2000
+#define DL_TC0TXMAXSDUSIZE 0x2001
+#define DL_TC0RXINITCREDITVAL 0x2002
+#define DL_TC0TXBUFFERSIZE 0x2005
+#define DL_PEERTC0PRESENT 0x2046
+#define DL_PEERTC0RXINITCREVAL 0x2047
+#define DL_TC1TXMAXSDUSIZE 0x2003
+#define DL_TC1RXINITCREDITVAL 0x2004
+#define DL_TC1TXBUFFERSIZE 0x2006
+#define DL_PEERTC1PRESENT 0x2066
+#define DL_PEERTC1RXINITCREVAL 0x2067
+
+/*
+ * Network Layer Attributes
+ */
+#define N_DEVICEID 0x3000
+#define N_DEVICEID_VALID 0x3001
+#define N_TC0TXMAXSDUSIZE 0x3020
+#define N_TC1TXMAXSDUSIZE 0x3021
+
+/*
+ * Transport Layer Attributes
+ */
+#define T_NUMCPORTS 0x4000
+#define T_NUMTESTFEATURES 0x4001
+#define T_CONNECTIONSTATE 0x4020
+#define T_PEERDEVICEID 0x4021
+#define T_PEERCPORTID 0x4022
+#define T_TRAFFICCLASS 0x4023
+#define T_PROTOCOLID 0x4024
+#define T_CPORTFLAGS 0x4025
+#define T_TXTOKENVALUE 0x4026
+#define T_RXTOKENVALUE 0x4027
+#define T_LOCALBUFFERSPACE 0x4028
+#define T_PEERBUFFERSPACE 0x4029
+#define T_CREDITSTOSEND 0x402A
+#define T_CPORTMODE 0x402B
+#define T_TC0TXMAXSDUSIZE 0x4060
+#define T_TC1TXMAXSDUSIZE 0x4061
+
+#endif /* _UNIPRO_H_ */
--
1.7.0.4
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH v2 5/6] scsi: ufs: add operation for the uic power mode change
2013-07-26 13:46 ` [PATCH 2/7] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
` (6 preceding siblings ...)
2013-08-23 13:00 ` [PATCH v2 4/6] scsi: ufs: add unipro attribute IDs Seungwon Jeon
@ 2013-08-23 13:00 ` Seungwon Jeon
2013-08-23 13:00 ` [PATCH v2 6/6] scsi: ufs: configure the attribute for power mode Seungwon Jeon
` (7 subsequent siblings)
15 siblings, 0 replies; 72+ messages in thread
From: Seungwon Jeon @ 2013-08-23 13:00 UTC (permalink / raw)
To: linux-scsi
Cc: 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
Setting PA_PWRMode using DME_SET triggers the power mode
change. And then the result will be given by the HCS.UPMCRS.
This operation should be done atomically.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
---
drivers/scsi/ufs/ufshcd.c | 84 ++++++++++++++++++++++++++++++++++++++++++--
drivers/scsi/ufs/ufshcd.h | 3 ++
drivers/scsi/ufs/ufshci.h | 12 ++++++
3 files changed, 95 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 7fc262e..8f25df4 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -36,9 +36,11 @@
#include <linux/async.h>
#include "ufshcd.h"
+#include "unipro.h"
#define UFSHCD_ENABLE_INTRS (UTP_TRANSFER_REQ_COMPL |\
UTP_TASK_REQ_COMPL |\
+ UIC_POWER_MODE |\
UFSHCD_ERROR_MASK)
/* UIC command timeout, unit: ms */
#define UIC_CMD_TIMEOUT 500
@@ -359,6 +361,18 @@ static inline bool ufshcd_ready_for_uic_cmd(struct ufs_hba *hba)
}
/**
+ * ufshcd_get_upmcrs - Get the power mode change request status
+ * @hba: Pointer to adapter instance
+ *
+ * This function gets the UPMCRS field of HCS register
+ * Returns value of UPMCRS field
+ */
+static inline u8 ufshcd_get_upmcrs(struct ufs_hba *hba)
+{
+ return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) & 0x7;
+}
+
+/**
* ufshcd_dispatch_uic_cmd - Dispatch UIC commands to unipro layers
* @hba: per adapter instance
* @uic_cmd: UIC command
@@ -907,6 +921,64 @@ out:
EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
/**
+ * ufshcd_uic_change_pwr_mode - Perform the UIC power mode chage
+ * using DME_SET primitives.
+ * @hba: per adapter instance
+ * @mode: powr mode value
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode)
+{
+ struct uic_command uic_cmd = {0};
+ struct completion pwr_done;
+ unsigned long flags;
+ u8 status;
+ int ret;
+
+ uic_cmd.command = UIC_CMD_DME_SET;
+ uic_cmd.argument1 = UIC_ARG_MIB(PA_PWRMODE);
+ uic_cmd.argument3 = mode;
+ init_completion(&pwr_done);
+
+ mutex_lock(&hba->uic_cmd_mutex);
+
+ spin_lock_irqsave(hba->host->host_lock, flags);
+ hba->pwr_done = &pwr_done;
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+ ret = __ufshcd_send_uic_cmd(hba, &uic_cmd);
+ if (ret) {
+ dev_err(hba->dev,
+ "pwr mode change with mode 0x%x uic error %d\n",
+ mode, ret);
+ goto out;
+ }
+
+ if (!wait_for_completion_timeout(hba->pwr_done,
+ msecs_to_jiffies(UIC_CMD_TIMEOUT))) {
+ dev_err(hba->dev,
+ "pwr mode change with mode 0x%x completion timeout\n",
+ mode);
+ ret = -ETIMEDOUT;
+ goto out;
+ }
+
+ status = ufshcd_get_upmcrs(hba);
+ if (status != PWR_LOCAL) {
+ dev_err(hba->dev,
+ "pwr mode change failed, host umpcrs:0x%x\n",
+ status);
+ ret = (status != PWR_OK) ? status : -1;
+ }
+out:
+ spin_lock_irqsave(hba->host->host_lock, flags);
+ hba->pwr_done = NULL;
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+ mutex_unlock(&hba->uic_cmd_mutex);
+ return ret;
+}
+
+/**
* ufshcd_make_hba_operational - Make UFS controller operational
* @hba: per adapter instance
*
@@ -1333,16 +1405,20 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
/**
* ufshcd_uic_cmd_compl - handle completion of uic command
* @hba: per adapter instance
+ * @intr_status: interrupt status generated by the controller
*/
-static void ufshcd_uic_cmd_compl(struct ufs_hba *hba)
+static void ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
{
- if (hba->active_uic_cmd) {
+ if ((intr_status & UIC_COMMAND_COMPL) && hba->active_uic_cmd) {
hba->active_uic_cmd->argument2 |=
ufshcd_get_uic_cmd_result(hba);
hba->active_uic_cmd->argument3 =
ufshcd_get_dme_attr_val(hba);
complete(&hba->active_uic_cmd->done);
}
+
+ if ((intr_status & UIC_POWER_MODE) && hba->pwr_done)
+ complete(hba->pwr_done);
}
/**
@@ -1444,8 +1520,8 @@ static void ufshcd_sl_intr(struct ufs_hba *hba, u32 intr_status)
if (hba->errors)
ufshcd_err_handler(hba);
- if (intr_status & UIC_COMMAND_COMPL)
- ufshcd_uic_cmd_compl(hba);
+ if (intr_status & UFSHCD_UIC_MASK)
+ ufshcd_uic_cmd_compl(hba, intr_status);
if (intr_status & UTP_TASK_REQ_COMPL)
ufshcd_tmc_handler(hba);
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 50bcd29..3956579 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -142,6 +142,7 @@ struct ufshcd_lrb {
* @uic_cmd_mutex: mutex for uic command
* @ufshcd_tm_wait_queue: wait queue for task management
* @tm_condition: condition variable for task management
+ * @pwr_done: completion for power mode change
* @ufshcd_state: UFSHCD states
* @intr_mask: Interrupt Mask Bits
* @feh_workq: Work queue for fatal controller error handling
@@ -180,6 +181,8 @@ struct ufs_hba {
wait_queue_head_t ufshcd_tm_wait_queue;
unsigned long tm_condition;
+ struct completion *pwr_done;
+
u32 ufshcd_state;
u32 intr_mask;
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index df4901e..d92fd9c 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -124,6 +124,9 @@ enum {
#define CONTROLLER_FATAL_ERROR UFS_BIT(16)
#define SYSTEM_BUS_FATAL_ERROR UFS_BIT(17)
+#define UFSHCD_UIC_MASK (UIC_COMMAND_COMPL |\
+ UIC_POWER_MODE)
+
#define UFSHCD_ERROR_MASK (UIC_ERROR |\
DEVICE_FATAL_ERROR |\
CONTROLLER_FATAL_ERROR |\
@@ -142,6 +145,15 @@ enum {
#define DEVICE_ERROR_INDICATOR UFS_BIT(5)
#define UIC_POWER_MODE_CHANGE_REQ_STATUS_MASK UFS_MASK(0x7, 8)
+enum {
+ PWR_OK = 0x0,
+ PWR_LOCAL = 0x01,
+ PWR_REMOTE = 0x02,
+ PWR_BUSY = 0x03,
+ PWR_ERROR_CAP = 0x04,
+ PWR_FATAL_ERROR = 0x05,
+};
+
/* HCE - Host Controller Enable 34h */
#define CONTROLLER_ENABLE UFS_BIT(0)
#define CONTROLLER_DISABLE 0x0
--
1.7.0.4
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH v2 6/6] scsi: ufs: configure the attribute for power mode
2013-07-26 13:46 ` [PATCH 2/7] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
` (7 preceding siblings ...)
2013-08-23 13:00 ` [PATCH v2 5/6] scsi: ufs: add operation for the uic power mode change Seungwon Jeon
@ 2013-08-23 13:00 ` Seungwon Jeon
2013-08-26 14:40 ` [PATCH v3 1/6] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
` (6 subsequent siblings)
15 siblings, 0 replies; 72+ messages in thread
From: Seungwon Jeon @ 2013-08-23 13:00 UTC (permalink / raw)
To: linux-scsi
Cc: 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
UIC attributes can be set with using DME_SET command for
power mode change. For configuration the link capability
attributes are used, which is updated after successful
link startup.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
---
drivers/scsi/ufs/ufshcd.c | 74 +++++++++++++++++++++++++++++++++++++++++++-
drivers/scsi/ufs/unipro.h | 21 +++++++++++++
2 files changed, 93 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 8f25df4..54fdbab 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -979,6 +979,70 @@ out:
}
/**
+ * ufshcd_config_max_pwr_mode - Set & Change power mode with
+ * maximum capability attribute information.
+ * @hba: per adapter instance
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+static int ufshcd_config_max_pwr_mode(struct ufs_hba *hba)
+{
+ enum {RX = 0, TX = 1};
+ u32 lanes[] = {1, 1};
+ u32 gear[] = {1, 1};
+ u8 pwr[] = {FASTAUTO_MODE, FASTAUTO_MODE};
+ int i, ret;
+
+ /* Get the connected lane count */
+ ufshcd_dme_get(hba, UIC_ARG_MIB(PA_CONNECTEDRXDATALANES), &lanes[RX]);
+ ufshcd_dme_get(hba, UIC_ARG_MIB(PA_CONNECTEDTXDATALANES), &lanes[TX]);
+
+ /*
+ * First, get the maximum gears of HS speed.
+ * If a zero value, it means there is no HSGEAR capability.
+ * Then, get the maximum gears of PWM speed.
+ */
+ ufshcd_dme_get(hba, UIC_ARG_MIB(PA_MAXRXHSGEAR), &gear[RX]);
+ if (!gear[RX]) {
+ ufshcd_dme_get(hba, UIC_ARG_MIB(PA_MAXRXPWMGEAR), &gear[RX]);
+ pwr[RX] = SLOWAUTO_MODE;
+ }
+
+ ufshcd_dme_peer_get(hba, UIC_ARG_MIB(PA_MAXRXHSGEAR), &gear[TX]);
+ if (!gear[TX]) {
+ ufshcd_dme_peer_get(hba, UIC_ARG_MIB(PA_MAXRXPWMGEAR),
+ &gear[TX]);
+ pwr[TX] = SLOWAUTO_MODE;
+ }
+
+ /*
+ * Configure attributes for power mode change with below.
+ * - PA_RXGEAR, PA_ACTIVERXDATALANES, PA_RXTERMINATION,
+ * - PA_TXGEAR, PA_ACTIVETXDATALANES, PA_TXTERMINATION,
+ * - PA_HSSERIES
+ */
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_RXGEAR), gear[RX]);
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_ACTIVERXDATALANES), lanes[RX]);
+ if (pwr[RX] == FASTAUTO_MODE)
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_RXTERMINATION), TRUE);
+
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXGEAR), gear[TX]);
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_ACTIVETXDATALANES), lanes[TX]);
+ if (pwr[TX] == FASTAUTO_MODE)
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXTERMINATION), TRUE);
+
+ if (pwr[RX] == FASTAUTO_MODE || pwr[TX] == FASTAUTO_MODE)
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_HSSERIES), PA_HS_MODE_B);
+
+ ret = ufshcd_uic_change_pwr_mode(hba, pwr[RX] << 4 | pwr[TX]);
+ if (ret)
+ dev_err(hba->dev,
+ "pwr_mode: power mode change failed %d\n", ret);
+
+ return ret;
+}
+
+/**
* ufshcd_make_hba_operational - Make UFS controller operational
* @hba: per adapter instance
*
@@ -1755,8 +1819,14 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
int ret;
ret = ufshcd_link_startup(hba);
- if (!ret)
- scsi_scan_host(hba->host);
+ if (ret)
+ goto out;
+
+ ufshcd_config_max_pwr_mode(hba);
+
+ scsi_scan_host(hba->host);
+out:
+ return;
}
static struct scsi_host_template ufshcd_driver_template = {
diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h
index 3a710eb..0bb8041 100644
--- a/drivers/scsi/ufs/unipro.h
+++ b/drivers/scsi/ufs/unipro.h
@@ -72,6 +72,21 @@
#define PA_STALLNOCONFIGTIME 0x15A3
#define PA_SAVECONFIGTIME 0x15A4
+/* PA power modes */
+enum {
+ FAST_MODE = 1,
+ SLOW_MODE = 2,
+ FASTAUTO_MODE = 4,
+ SLOWAUTO_MODE = 5,
+ UNCHANGED = 7,
+};
+
+/* PA TX/RX Frequency Series */
+enum {
+ PA_HS_MODE_A = 1,
+ PA_HS_MODE_B = 2,
+};
+
/*
* Data Link Layer Attributes
*/
@@ -127,4 +142,10 @@
#define T_TC0TXMAXSDUSIZE 0x4060
#define T_TC1TXMAXSDUSIZE 0x4061
+/* Boolean attribute values */
+enum {
+ FALSE = 0,
+ TRUE,
+};
+
#endif /* _UNIPRO_H_ */
--
1.7.0.4
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH v3 1/6] scsi: ufs: find out sense data over scsi status values
2013-07-26 13:46 ` [PATCH 2/7] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
` (8 preceding siblings ...)
2013-08-23 13:00 ` [PATCH v2 6/6] scsi: ufs: configure the attribute for power mode Seungwon Jeon
@ 2013-08-26 14:40 ` Seungwon Jeon
2013-08-27 8:53 ` Subhash Jadavani
2013-08-26 14:40 ` [PATCH v3 2/6] scsi: ufs: fix the setting interrupt aggregation counter Seungwon Jeon
` (5 subsequent siblings)
15 siblings, 1 reply; 72+ messages in thread
From: Seungwon Jeon @ 2013-08-26 14:40 UTC (permalink / raw)
To: linux-scsi
Cc: 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
Unlike 'GOOD' and 'CHECK CONDITION', other status values in
Response UPIU may or may not contain sense data. That is returning
sense data isn't obvious. So, in this case the Data Segment Length
field should be checked. If a non-zero value, it means that UPIU
has Sense Data in the Data Segment area.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
---
drivers/scsi/ufs/ufs.h | 1 +
drivers/scsi/ufs/ufshcd.c | 37 ++++++++++++++++++++++---------------
2 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index bce09a6..7210500 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -177,6 +177,7 @@ enum {
MASK_TASK_RESPONSE = 0xFF00,
MASK_RSP_UPIU_RESULT = 0xFFFF,
MASK_QUERY_DATA_SEG_LEN = 0xFFFF,
+ MASK_RSP_UPIU_DATA_SEG_LEN = 0xFFFF,
MASK_RSP_EXCEPTION_EVENT = 0x10000,
};
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 1b99c0a..6ff16c9 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -310,6 +310,20 @@ ufshcd_get_rsp_upiu_result(struct utp_upiu_rsp *ucd_rsp_ptr)
return be32_to_cpu(ucd_rsp_ptr->header.dword_1) & MASK_RSP_UPIU_RESULT;
}
+/*
+ * ufshcd_get_rsp_upiu_data_seg_len - Get the data segment length
+ * from response UPIU
+ * @ucd_rsp_ptr: pointer to response UPIU
+ *
+ * Return the data segment length.
+ */
+static inline unsigned int
+ufshcd_get_rsp_upiu_data_seg_len(struct utp_upiu_rsp *ucd_rsp_ptr)
+{
+ return be32_to_cpu(ucd_rsp_ptr->header.dword_2) &
+ MASK_RSP_UPIU_DATA_SEG_LEN;
+}
+
/**
* ufshcd_is_exception_event - Check if the device raised an exception event
* @ucd_rsp_ptr: pointer to response UPIU
@@ -405,7 +419,8 @@ void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag)
static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
{
int len;
- if (lrbp->sense_buffer) {
+ if (lrbp->sense_buffer &&
+ ufshcd_get_rsp_upiu_data_seg_len(lrbp->ucd_rsp_ptr)) {
len = be16_to_cpu(lrbp->ucd_rsp_ptr->sr.sense_data_len);
memcpy(lrbp->sense_buffer,
lrbp->ucd_rsp_ptr->sr.sense_data,
@@ -1789,32 +1804,24 @@ ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp, int scsi_status)
int result = 0;
switch (scsi_status) {
- case SAM_STAT_GOOD:
- result |= DID_OK << 16 |
- COMMAND_COMPLETE << 8 |
- SAM_STAT_GOOD;
- break;
case SAM_STAT_CHECK_CONDITION:
+ ufshcd_copy_sense_data(lrbp);
+ case SAM_STAT_GOOD:
result |= DID_OK << 16 |
COMMAND_COMPLETE << 8 |
- SAM_STAT_CHECK_CONDITION;
- ufshcd_copy_sense_data(lrbp);
- break;
- case SAM_STAT_BUSY:
- result |= SAM_STAT_BUSY;
+ scsi_status;
break;
case SAM_STAT_TASK_SET_FULL:
-
/*
* If a LUN reports SAM_STAT_TASK_SET_FULL, then the LUN queue
* depth needs to be adjusted to the exact number of
* outstanding commands the LUN can handle at any given time.
*/
ufshcd_adjust_lun_qdepth(lrbp->cmd);
- result |= SAM_STAT_TASK_SET_FULL;
- break;
+ case SAM_STAT_BUSY:
case SAM_STAT_TASK_ABORTED:
- result |= SAM_STAT_TASK_ABORTED;
+ ufshcd_copy_sense_data(lrbp);
+ result |= scsi_status;
break;
default:
result |= DID_ERROR << 16;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH v3 1/6] scsi: ufs: find out sense data over scsi status values
2013-08-26 14:40 ` [PATCH v3 1/6] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
@ 2013-08-27 8:53 ` Subhash Jadavani
2013-08-28 12:43 ` Yaniv Gardi
0 siblings, 1 reply; 72+ messages in thread
From: Subhash Jadavani @ 2013-08-27 8:53 UTC (permalink / raw)
To: Seungwon Jeon
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
Looks good to me.
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
On 8/26/2013 8:10 PM, Seungwon Jeon wrote:
> Unlike 'GOOD' and 'CHECK CONDITION', other status values in
> Response UPIU may or may not contain sense data. That is returning
> sense data isn't obvious. So, in this case the Data Segment Length
> field should be checked. If a non-zero value, it means that UPIU
> has Sense Data in the Data Segment area.
>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
> ---
> drivers/scsi/ufs/ufs.h | 1 +
> drivers/scsi/ufs/ufshcd.c | 37 ++++++++++++++++++++++---------------
> 2 files changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index bce09a6..7210500 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -177,6 +177,7 @@ enum {
> MASK_TASK_RESPONSE = 0xFF00,
> MASK_RSP_UPIU_RESULT = 0xFFFF,
> MASK_QUERY_DATA_SEG_LEN = 0xFFFF,
> + MASK_RSP_UPIU_DATA_SEG_LEN = 0xFFFF,
> MASK_RSP_EXCEPTION_EVENT = 0x10000,
> };
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 1b99c0a..6ff16c9 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -310,6 +310,20 @@ ufshcd_get_rsp_upiu_result(struct utp_upiu_rsp *ucd_rsp_ptr)
> return be32_to_cpu(ucd_rsp_ptr->header.dword_1) & MASK_RSP_UPIU_RESULT;
> }
>
> +/*
> + * ufshcd_get_rsp_upiu_data_seg_len - Get the data segment length
> + * from response UPIU
> + * @ucd_rsp_ptr: pointer to response UPIU
> + *
> + * Return the data segment length.
> + */
> +static inline unsigned int
> +ufshcd_get_rsp_upiu_data_seg_len(struct utp_upiu_rsp *ucd_rsp_ptr)
> +{
> + return be32_to_cpu(ucd_rsp_ptr->header.dword_2) &
> + MASK_RSP_UPIU_DATA_SEG_LEN;
> +}
> +
> /**
> * ufshcd_is_exception_event - Check if the device raised an exception event
> * @ucd_rsp_ptr: pointer to response UPIU
> @@ -405,7 +419,8 @@ void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag)
> static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
> {
> int len;
> - if (lrbp->sense_buffer) {
> + if (lrbp->sense_buffer &&
> + ufshcd_get_rsp_upiu_data_seg_len(lrbp->ucd_rsp_ptr)) {
> len = be16_to_cpu(lrbp->ucd_rsp_ptr->sr.sense_data_len);
> memcpy(lrbp->sense_buffer,
> lrbp->ucd_rsp_ptr->sr.sense_data,
> @@ -1789,32 +1804,24 @@ ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp, int scsi_status)
> int result = 0;
>
> switch (scsi_status) {
> - case SAM_STAT_GOOD:
> - result |= DID_OK << 16 |
> - COMMAND_COMPLETE << 8 |
> - SAM_STAT_GOOD;
> - break;
> case SAM_STAT_CHECK_CONDITION:
> + ufshcd_copy_sense_data(lrbp);
> + case SAM_STAT_GOOD:
> result |= DID_OK << 16 |
> COMMAND_COMPLETE << 8 |
> - SAM_STAT_CHECK_CONDITION;
> - ufshcd_copy_sense_data(lrbp);
> - break;
> - case SAM_STAT_BUSY:
> - result |= SAM_STAT_BUSY;
> + scsi_status;
> break;
> case SAM_STAT_TASK_SET_FULL:
> -
> /*
> * If a LUN reports SAM_STAT_TASK_SET_FULL, then the LUN queue
> * depth needs to be adjusted to the exact number of
> * outstanding commands the LUN can handle at any given time.
> */
> ufshcd_adjust_lun_qdepth(lrbp->cmd);
> - result |= SAM_STAT_TASK_SET_FULL;
> - break;
> + case SAM_STAT_BUSY:
> case SAM_STAT_TASK_ABORTED:
> - result |= SAM_STAT_TASK_ABORTED;
> + ufshcd_copy_sense_data(lrbp);
> + result |= scsi_status;
> break;
> default:
> result |= DID_ERROR << 16;
^ permalink raw reply [flat|nested] 72+ messages in thread* RE: [PATCH v3 1/6] scsi: ufs: find out sense data over scsi status values
2013-08-27 8:53 ` Subhash Jadavani
@ 2013-08-28 12:43 ` Yaniv Gardi
0 siblings, 0 replies; 72+ messages in thread
From: Yaniv Gardi @ 2013-08-28 12:43 UTC (permalink / raw)
To: 'Subhash Jadavani', 'Seungwon Jeon'
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
Tested-by: Yaniv Gardi <ygardi@codeaurora.org>
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
= > -----Original Message-----
= > From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
= > owner@vger.kernel.org] On Behalf Of Subhash Jadavani
= > Sent: Tuesday, August 27, 2013 11:53 AM
= > To: Seungwon Jeon
= > Cc: linux-scsi@vger.kernel.org; 'Vinayak Holikatti'; 'Santosh Y'; 'James E.J.
= > Bottomley'
= > Subject: Re: [PATCH v3 1/6] scsi: ufs: find out sense data over scsi status
= > values
= >
= >
= > Looks good to me.
= > Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
= >
= > On 8/26/2013 8:10 PM, Seungwon Jeon wrote:
= > > Unlike 'GOOD' and 'CHECK CONDITION', other status values in Response
= > > UPIU may or may not contain sense data. That is returning sense data
= > > isn't obvious. So, in this case the Data Segment Length field should
= > > be checked. If a non-zero value, it means that UPIU has Sense Data in
= > > the Data Segment area.
= > >
= > > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
= > > Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
= > > ---
= > > drivers/scsi/ufs/ufs.h | 1 +
= > > drivers/scsi/ufs/ufshcd.c | 37 ++++++++++++++++++++++---------------
= > > 2 files changed, 23 insertions(+), 15 deletions(-)
= > >
= > > diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h index
= > > bce09a6..7210500 100644
= > > --- a/drivers/scsi/ufs/ufs.h
= > > +++ b/drivers/scsi/ufs/ufs.h
= > > @@ -177,6 +177,7 @@ enum {
= > > MASK_TASK_RESPONSE = 0xFF00,
= > > MASK_RSP_UPIU_RESULT = 0xFFFF,
= > > MASK_QUERY_DATA_SEG_LEN = 0xFFFF,
= > > + MASK_RSP_UPIU_DATA_SEG_LEN = 0xFFFF,
= > > MASK_RSP_EXCEPTION_EVENT = 0x10000,
= > > };
= > >
= > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
= > > index 1b99c0a..6ff16c9 100644
= > > --- a/drivers/scsi/ufs/ufshcd.c
= > > +++ b/drivers/scsi/ufs/ufshcd.c
= > > @@ -310,6 +310,20 @@ ufshcd_get_rsp_upiu_result(struct utp_upiu_rsp
= > *ucd_rsp_ptr)
= > > return be32_to_cpu(ucd_rsp_ptr->header.dword_1) &
= > MASK_RSP_UPIU_RESULT;
= > > }
= > >
= > > +/*
= > > + * ufshcd_get_rsp_upiu_data_seg_len - Get the data segment length
= > > + * from response UPIU
= > > + * @ucd_rsp_ptr: pointer to response UPIU
= > > + *
= > > + * Return the data segment length.
= > > + */
= > > +static inline unsigned int
= > > +ufshcd_get_rsp_upiu_data_seg_len(struct utp_upiu_rsp *ucd_rsp_ptr) {
= > > + return be32_to_cpu(ucd_rsp_ptr->header.dword_2) &
= > > + MASK_RSP_UPIU_DATA_SEG_LEN;
= > > +}
= > > +
= > > /**
= > > * ufshcd_is_exception_event - Check if the device raised an exception
= > event
= > > * @ucd_rsp_ptr: pointer to response UPIU @@ -405,7 +419,8 @@ void
= > > ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag)
= > > static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
= > > {
= > > int len;
= > > - if (lrbp->sense_buffer) {
= > > + if (lrbp->sense_buffer &&
= > > + ufshcd_get_rsp_upiu_data_seg_len(lrbp->ucd_rsp_ptr)) {
= > > len = be16_to_cpu(lrbp->ucd_rsp_ptr->sr.sense_data_len);
= > > memcpy(lrbp->sense_buffer,
= > > lrbp->ucd_rsp_ptr->sr.sense_data, @@ -1789,32
= > +1804,24 @@
= > > ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp, int scsi_status)
= > > int result = 0;
= > >
= > > switch (scsi_status) {
= > > - case SAM_STAT_GOOD:
= > > - result |= DID_OK << 16 |
= > > - COMMAND_COMPLETE << 8 |
= > > - SAM_STAT_GOOD;
= > > - break;
= > > case SAM_STAT_CHECK_CONDITION:
= > > + ufshcd_copy_sense_data(lrbp);
= > > + case SAM_STAT_GOOD:
= > > result |= DID_OK << 16 |
= > > COMMAND_COMPLETE << 8 |
= > > - SAM_STAT_CHECK_CONDITION;
= > > - ufshcd_copy_sense_data(lrbp);
= > > - break;
= > > - case SAM_STAT_BUSY:
= > > - result |= SAM_STAT_BUSY;
= > > + scsi_status;
= > > break;
= > > case SAM_STAT_TASK_SET_FULL:
= > > -
= > > /*
= > > * If a LUN reports SAM_STAT_TASK_SET_FULL, then the
= > LUN queue
= > > * depth needs to be adjusted to the exact number of
= > > * outstanding commands the LUN can handle at any given
= > time.
= > > */
= > > ufshcd_adjust_lun_qdepth(lrbp->cmd);
= > > - result |= SAM_STAT_TASK_SET_FULL;
= > > - break;
= > > + case SAM_STAT_BUSY:
= > > case SAM_STAT_TASK_ABORTED:
= > > - result |= SAM_STAT_TASK_ABORTED;
= > > + ufshcd_copy_sense_data(lrbp);
= > > + result |= scsi_status;
= > > break;
= > > default:
= > > result |= DID_ERROR << 16;
= >
= > --
= > 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
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v3 2/6] scsi: ufs: fix the setting interrupt aggregation counter
2013-07-26 13:46 ` [PATCH 2/7] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
` (9 preceding siblings ...)
2013-08-26 14:40 ` [PATCH v3 1/6] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
@ 2013-08-26 14:40 ` Seungwon Jeon
2013-08-27 9:01 ` Subhash Jadavani
2013-08-26 14:40 ` [PATCH v3 3/6] scsi: ufs: add dme configuration primitives Seungwon Jeon
` (4 subsequent siblings)
15 siblings, 1 reply; 72+ messages in thread
From: Seungwon Jeon @ 2013-08-26 14:40 UTC (permalink / raw)
To: linux-scsi
Cc: 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
IACTH(Interrupt aggregation counter threshold) value is allowed
up to 0x1F and current setting value is the maximum.
This value is related with NUTRS(max:0x20) of HCI's capability.
Considering HCI controller doesn't support the maximum, IACTH
setting should be adjusted with possible value.
For that, existing 'ufshcd_config_int_aggr' is split into two part
[reset, configure].
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
---
drivers/scsi/ufs/ufshcd.c | 53 +++++++++++++++++++++-----------------------
drivers/scsi/ufs/ufshci.h | 4 +-
2 files changed, 27 insertions(+), 30 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 6ff16c9..c90b88a 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -59,6 +59,9 @@
/* Expose the flag value from utp_upiu_query.value */
#define MASK_QUERY_UPIU_FLAG_LOC 0xFF
+/* Interrupt aggregation default timeout, unit: 40us */
+#define INT_AGGR_DEF_TO 0x02
+
enum {
UFSHCD_MAX_CHANNEL = 0,
UFSHCD_MAX_ID = 1,
@@ -94,12 +97,6 @@ enum {
UFSHCD_INT_CLEAR,
};
-/* Interrupt aggregation options */
-enum {
- INT_AGGR_RESET,
- INT_AGGR_CONFIG,
-};
-
#define ufshcd_set_eh_in_progress(h) \
(h->eh_flags |= UFSHCD_EH_IN_PROGRESS)
#define ufshcd_eh_in_progress(h) \
@@ -340,30 +337,30 @@ static inline bool ufshcd_is_exception_event(struct utp_upiu_rsp *ucd_rsp_ptr)
}
/**
- * ufshcd_config_int_aggr - Configure interrupt aggregation values.
- * Currently there is no use case where we want to configure
- * interrupt aggregation dynamically. So to configure interrupt
- * aggregation, #define INT_AGGR_COUNTER_THRESHOLD_VALUE and
- * INT_AGGR_TIMEOUT_VALUE are used.
+ * ufshcd_reset_intr_aggr - Reset interrupt aggregation values.
* @hba: per adapter instance
- * @option: Interrupt aggregation option
*/
static inline void
-ufshcd_config_int_aggr(struct ufs_hba *hba, int option)
+ufshcd_reset_intr_aggr(struct ufs_hba *hba)
{
- switch (option) {
- case INT_AGGR_RESET:
- ufshcd_writel(hba, INT_AGGR_ENABLE |
- INT_AGGR_COUNTER_AND_TIMER_RESET,
- REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
- break;
- case INT_AGGR_CONFIG:
- ufshcd_writel(hba, INT_AGGR_ENABLE | INT_AGGR_PARAM_WRITE |
- INT_AGGR_COUNTER_THRESHOLD_VALUE |
- INT_AGGR_TIMEOUT_VALUE,
- REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
- break;
- }
+ ufshcd_writel(hba, INT_AGGR_ENABLE |
+ INT_AGGR_COUNTER_AND_TIMER_RESET,
+ REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
+}
+
+/**
+ * ufshcd_config_intr_aggr - Configure interrupt aggregation values.
+ * @hba: per adapter instance
+ * @cnt: Interrupt aggregation counter threshold
+ * @tmout: Interrupt aggregation timeout value
+ */
+static inline void
+ufshcd_config_intr_aggr(struct ufs_hba *hba, u8 cnt, u8 tmout)
+{
+ ufshcd_writel(hba, INT_AGGR_ENABLE | INT_AGGR_PARAM_WRITE |
+ INT_AGGR_COUNTER_THLD_VAL(cnt) |
+ INT_AGGR_TIMEOUT_VAL(tmout),
+ REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
}
/**
@@ -1523,7 +1520,7 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba)
ufshcd_enable_intr(hba, UFSHCD_ENABLE_INTRS);
/* Configure interrupt aggregation */
- ufshcd_config_int_aggr(hba, INT_AGGR_CONFIG);
+ ufshcd_config_intr_aggr(hba, hba->nutrs - 1, INT_AGGR_DEF_TO);
/* Configure UTRL and UTMRL base address registers */
ufshcd_writel(hba, lower_32_bits(hba->utrdl_dma_addr),
@@ -1971,7 +1968,7 @@ static void ufshcd_transfer_req_compl(struct ufs_hba *hba)
/* Reset interrupt aggregation counters */
if (int_aggr_reset)
- ufshcd_config_int_aggr(hba, INT_AGGR_RESET);
+ ufshcd_reset_intr_aggr(hba);
}
/**
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index f1e1b74..739ae3a 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -226,8 +226,8 @@ enum {
#define MASK_UIC_COMMAND_RESULT 0xFF
-#define INT_AGGR_COUNTER_THRESHOLD_VALUE (0x1F << 8)
-#define INT_AGGR_TIMEOUT_VALUE (0x02)
+#define INT_AGGR_COUNTER_THLD_VAL(c) (((c) & 0x1F) << 8)
+#define INT_AGGR_TIMEOUT_VAL(t) (((t) & 0xFF) << 0)
/* Interrupt disable masks */
enum {
--
1.7.0.4
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH v3 2/6] scsi: ufs: fix the setting interrupt aggregation counter
2013-08-26 14:40 ` [PATCH v3 2/6] scsi: ufs: fix the setting interrupt aggregation counter Seungwon Jeon
@ 2013-08-27 9:01 ` Subhash Jadavani
2013-08-28 12:43 ` Yaniv Gardi
0 siblings, 1 reply; 72+ messages in thread
From: Subhash Jadavani @ 2013-08-27 9:01 UTC (permalink / raw)
To: Seungwon Jeon
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
Looks good to me.
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
On 8/26/2013 8:10 PM, Seungwon Jeon wrote:
> IACTH(Interrupt aggregation counter threshold) value is allowed
> up to 0x1F and current setting value is the maximum.
> This value is related with NUTRS(max:0x20) of HCI's capability.
> Considering HCI controller doesn't support the maximum, IACTH
> setting should be adjusted with possible value.
> For that, existing 'ufshcd_config_int_aggr' is split into two part
> [reset, configure].
>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
> ---
> drivers/scsi/ufs/ufshcd.c | 53 +++++++++++++++++++++-----------------------
> drivers/scsi/ufs/ufshci.h | 4 +-
> 2 files changed, 27 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 6ff16c9..c90b88a 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -59,6 +59,9 @@
> /* Expose the flag value from utp_upiu_query.value */
> #define MASK_QUERY_UPIU_FLAG_LOC 0xFF
>
> +/* Interrupt aggregation default timeout, unit: 40us */
> +#define INT_AGGR_DEF_TO 0x02
> +
> enum {
> UFSHCD_MAX_CHANNEL = 0,
> UFSHCD_MAX_ID = 1,
> @@ -94,12 +97,6 @@ enum {
> UFSHCD_INT_CLEAR,
> };
>
> -/* Interrupt aggregation options */
> -enum {
> - INT_AGGR_RESET,
> - INT_AGGR_CONFIG,
> -};
> -
> #define ufshcd_set_eh_in_progress(h) \
> (h->eh_flags |= UFSHCD_EH_IN_PROGRESS)
> #define ufshcd_eh_in_progress(h) \
> @@ -340,30 +337,30 @@ static inline bool ufshcd_is_exception_event(struct utp_upiu_rsp *ucd_rsp_ptr)
> }
>
> /**
> - * ufshcd_config_int_aggr - Configure interrupt aggregation values.
> - * Currently there is no use case where we want to configure
> - * interrupt aggregation dynamically. So to configure interrupt
> - * aggregation, #define INT_AGGR_COUNTER_THRESHOLD_VALUE and
> - * INT_AGGR_TIMEOUT_VALUE are used.
> + * ufshcd_reset_intr_aggr - Reset interrupt aggregation values.
> * @hba: per adapter instance
> - * @option: Interrupt aggregation option
> */
> static inline void
> -ufshcd_config_int_aggr(struct ufs_hba *hba, int option)
> +ufshcd_reset_intr_aggr(struct ufs_hba *hba)
> {
> - switch (option) {
> - case INT_AGGR_RESET:
> - ufshcd_writel(hba, INT_AGGR_ENABLE |
> - INT_AGGR_COUNTER_AND_TIMER_RESET,
> - REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
> - break;
> - case INT_AGGR_CONFIG:
> - ufshcd_writel(hba, INT_AGGR_ENABLE | INT_AGGR_PARAM_WRITE |
> - INT_AGGR_COUNTER_THRESHOLD_VALUE |
> - INT_AGGR_TIMEOUT_VALUE,
> - REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
> - break;
> - }
> + ufshcd_writel(hba, INT_AGGR_ENABLE |
> + INT_AGGR_COUNTER_AND_TIMER_RESET,
> + REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
> +}
> +
> +/**
> + * ufshcd_config_intr_aggr - Configure interrupt aggregation values.
> + * @hba: per adapter instance
> + * @cnt: Interrupt aggregation counter threshold
> + * @tmout: Interrupt aggregation timeout value
> + */
> +static inline void
> +ufshcd_config_intr_aggr(struct ufs_hba *hba, u8 cnt, u8 tmout)
> +{
> + ufshcd_writel(hba, INT_AGGR_ENABLE | INT_AGGR_PARAM_WRITE |
> + INT_AGGR_COUNTER_THLD_VAL(cnt) |
> + INT_AGGR_TIMEOUT_VAL(tmout),
> + REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
> }
>
> /**
> @@ -1523,7 +1520,7 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba)
> ufshcd_enable_intr(hba, UFSHCD_ENABLE_INTRS);
>
> /* Configure interrupt aggregation */
> - ufshcd_config_int_aggr(hba, INT_AGGR_CONFIG);
> + ufshcd_config_intr_aggr(hba, hba->nutrs - 1, INT_AGGR_DEF_TO);
>
> /* Configure UTRL and UTMRL base address registers */
> ufshcd_writel(hba, lower_32_bits(hba->utrdl_dma_addr),
> @@ -1971,7 +1968,7 @@ static void ufshcd_transfer_req_compl(struct ufs_hba *hba)
>
> /* Reset interrupt aggregation counters */
> if (int_aggr_reset)
> - ufshcd_config_int_aggr(hba, INT_AGGR_RESET);
> + ufshcd_reset_intr_aggr(hba);
> }
>
> /**
> diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
> index f1e1b74..739ae3a 100644
> --- a/drivers/scsi/ufs/ufshci.h
> +++ b/drivers/scsi/ufs/ufshci.h
> @@ -226,8 +226,8 @@ enum {
>
> #define MASK_UIC_COMMAND_RESULT 0xFF
>
> -#define INT_AGGR_COUNTER_THRESHOLD_VALUE (0x1F << 8)
> -#define INT_AGGR_TIMEOUT_VALUE (0x02)
> +#define INT_AGGR_COUNTER_THLD_VAL(c) (((c) & 0x1F) << 8)
> +#define INT_AGGR_TIMEOUT_VAL(t) (((t) & 0xFF) << 0)
>
> /* Interrupt disable masks */
> enum {
^ permalink raw reply [flat|nested] 72+ messages in thread* RE: [PATCH v3 2/6] scsi: ufs: fix the setting interrupt aggregation counter
2013-08-27 9:01 ` Subhash Jadavani
@ 2013-08-28 12:43 ` Yaniv Gardi
0 siblings, 0 replies; 72+ messages in thread
From: Yaniv Gardi @ 2013-08-28 12:43 UTC (permalink / raw)
To: 'Subhash Jadavani', 'Seungwon Jeon'
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
Tested-by: Yaniv Gardi <ygardi@codeaurora.org>
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
= > -----Original Message-----
= > From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
= > owner@vger.kernel.org] On Behalf Of Subhash Jadavani
= > Sent: Tuesday, August 27, 2013 12:02 PM
= > To: Seungwon Jeon
= > Cc: linux-scsi@vger.kernel.org; 'Vinayak Holikatti'; 'Santosh Y'; 'James E.J.
= > Bottomley'
= > Subject: Re: [PATCH v3 2/6] scsi: ufs: fix the setting interrupt aggregation
= > counter
= >
= >
= > Looks good to me.
= > Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
= >
= > On 8/26/2013 8:10 PM, Seungwon Jeon wrote:
= > > IACTH(Interrupt aggregation counter threshold) value is allowed up to
= > > 0x1F and current setting value is the maximum.
= > > This value is related with NUTRS(max:0x20) of HCI's capability.
= > > Considering HCI controller doesn't support the maximum, IACTH setting
= > > should be adjusted with possible value.
= > > For that, existing 'ufshcd_config_int_aggr' is split into two part
= > > [reset, configure].
= > >
= > > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
= > > Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
= > > ---
= > > drivers/scsi/ufs/ufshcd.c | 53 +++++++++++++++++++++------------------
= > -----
= > > drivers/scsi/ufs/ufshci.h | 4 +-
= > > 2 files changed, 27 insertions(+), 30 deletions(-)
= > >
= > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
= > > index 6ff16c9..c90b88a 100644
= > > --- a/drivers/scsi/ufs/ufshcd.c
= > > +++ b/drivers/scsi/ufs/ufshcd.c
= > > @@ -59,6 +59,9 @@
= > > /* Expose the flag value from utp_upiu_query.value */
= > > #define MASK_QUERY_UPIU_FLAG_LOC 0xFF
= > >
= > > +/* Interrupt aggregation default timeout, unit: 40us */
= > > +#define INT_AGGR_DEF_TO 0x02
= > > +
= > > enum {
= > > UFSHCD_MAX_CHANNEL = 0,
= > > UFSHCD_MAX_ID = 1,
= > > @@ -94,12 +97,6 @@ enum {
= > > UFSHCD_INT_CLEAR,
= > > };
= > >
= > > -/* Interrupt aggregation options */
= > > -enum {
= > > - INT_AGGR_RESET,
= > > - INT_AGGR_CONFIG,
= > > -};
= > > -
= > > #define ufshcd_set_eh_in_progress(h) \
= > > (h->eh_flags |= UFSHCD_EH_IN_PROGRESS)
= > > #define ufshcd_eh_in_progress(h) \
= > > @@ -340,30 +337,30 @@ static inline bool
= > ufshcd_is_exception_event(struct utp_upiu_rsp *ucd_rsp_ptr)
= > > }
= > >
= > > /**
= > > - * ufshcd_config_int_aggr - Configure interrupt aggregation values.
= > > - * Currently there is no use case where we want to configure
= > > - * interrupt aggregation dynamically. So to configure interrupt
= > > - * aggregation, #define
= > INT_AGGR_COUNTER_THRESHOLD_VALUE and
= > > - * INT_AGGR_TIMEOUT_VALUE are used.
= > > + * ufshcd_reset_intr_aggr - Reset interrupt aggregation values.
= > > * @hba: per adapter instance
= > > - * @option: Interrupt aggregation option
= > > */
= > > static inline void
= > > -ufshcd_config_int_aggr(struct ufs_hba *hba, int option)
= > > +ufshcd_reset_intr_aggr(struct ufs_hba *hba)
= > > {
= > > - switch (option) {
= > > - case INT_AGGR_RESET:
= > > - ufshcd_writel(hba, INT_AGGR_ENABLE |
= > > - INT_AGGR_COUNTER_AND_TIMER_RESET,
= > > -
= > REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
= > > - break;
= > > - case INT_AGGR_CONFIG:
= > > - ufshcd_writel(hba, INT_AGGR_ENABLE |
= > INT_AGGR_PARAM_WRITE |
= > > - INT_AGGR_COUNTER_THRESHOLD_VALUE |
= > > - INT_AGGR_TIMEOUT_VALUE,
= > > -
= > REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
= > > - break;
= > > - }
= > > + ufshcd_writel(hba, INT_AGGR_ENABLE |
= > > + INT_AGGR_COUNTER_AND_TIMER_RESET,
= > > + REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
= > > +}
= > > +
= > > +/**
= > > + * ufshcd_config_intr_aggr - Configure interrupt aggregation values.
= > > + * @hba: per adapter instance
= > > + * @cnt: Interrupt aggregation counter threshold
= > > + * @tmout: Interrupt aggregation timeout value */ static inline void
= > > +ufshcd_config_intr_aggr(struct ufs_hba *hba, u8 cnt, u8 tmout) {
= > > + ufshcd_writel(hba, INT_AGGR_ENABLE |
= > INT_AGGR_PARAM_WRITE |
= > > + INT_AGGR_COUNTER_THLD_VAL(cnt) |
= > > + INT_AGGR_TIMEOUT_VAL(tmout),
= > > + REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
= > > }
= > >
= > > /**
= > > @@ -1523,7 +1520,7 @@ static int ufshcd_make_hba_operational(struct
= > ufs_hba *hba)
= > > ufshcd_enable_intr(hba, UFSHCD_ENABLE_INTRS);
= > >
= > > /* Configure interrupt aggregation */
= > > - ufshcd_config_int_aggr(hba, INT_AGGR_CONFIG);
= > > + ufshcd_config_intr_aggr(hba, hba->nutrs - 1, INT_AGGR_DEF_TO);
= > >
= > > /* Configure UTRL and UTMRL base address registers */
= > > ufshcd_writel(hba, lower_32_bits(hba->utrdl_dma_addr),
= > > @@ -1971,7 +1968,7 @@ static void ufshcd_transfer_req_compl(struct
= > > ufs_hba *hba)
= > >
= > > /* Reset interrupt aggregation counters */
= > > if (int_aggr_reset)
= > > - ufshcd_config_int_aggr(hba, INT_AGGR_RESET);
= > > + ufshcd_reset_intr_aggr(hba);
= > > }
= > >
= > > /**
= > > diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
= > > index f1e1b74..739ae3a 100644
= > > --- a/drivers/scsi/ufs/ufshci.h
= > > +++ b/drivers/scsi/ufs/ufshci.h
= > > @@ -226,8 +226,8 @@ enum {
= > >
= > > #define MASK_UIC_COMMAND_RESULT 0xFF
= > >
= > > -#define INT_AGGR_COUNTER_THRESHOLD_VALUE (0x1F << 8)
= > > -#define INT_AGGR_TIMEOUT_VALUE (0x02)
= > > +#define INT_AGGR_COUNTER_THLD_VAL(c) (((c) & 0x1F) << 8)
= > > +#define INT_AGGR_TIMEOUT_VAL(t) (((t) & 0xFF) << 0)
= > >
= > > /* Interrupt disable masks */
= > > enum {
= >
= > --
= > 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
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v3 3/6] scsi: ufs: add dme configuration primitives
2013-07-26 13:46 ` [PATCH 2/7] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
` (10 preceding siblings ...)
2013-08-26 14:40 ` [PATCH v3 2/6] scsi: ufs: fix the setting interrupt aggregation counter Seungwon Jeon
@ 2013-08-26 14:40 ` Seungwon Jeon
2013-08-27 9:15 ` Subhash Jadavani
2013-08-26 14:40 ` [PATCH v3 4/6] scsi: ufs: add unipro attribute IDs Seungwon Jeon
` (3 subsequent siblings)
15 siblings, 1 reply; 72+ messages in thread
From: Seungwon Jeon @ 2013-08-26 14:40 UTC (permalink / raw)
To: linux-scsi
Cc: 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
Implements to support GET and SET operations of the DME.
These operations are used to configure the behavior of
the UNIPRO. Along with basic operation, {Peer/AttrSetType}
can be mixed.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
---
drivers/scsi/ufs/ufshcd.c | 88 +++++++++++++++++++++++++++++++++++++++++++++
drivers/scsi/ufs/ufshcd.h | 51 ++++++++++++++++++++++++++
drivers/scsi/ufs/ufshci.h | 6 +++
3 files changed, 145 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c90b88a..00bfc1a 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -285,6 +285,18 @@ static inline int ufshcd_get_uic_cmd_result(struct ufs_hba *hba)
}
/**
+ * ufshcd_get_dme_attr_val - Get the value of attribute returned by UIC command
+ * @hba: Pointer to adapter instance
+ *
+ * This function gets UIC command argument3
+ * Returns 0 on success, non zero value on error
+ */
+static inline u32 ufshcd_get_dme_attr_val(struct ufs_hba *hba)
+{
+ return ufshcd_readl(hba, REG_UIC_COMMAND_ARG_3);
+}
+
+/**
* ufshcd_get_req_rsp - returns the TR response transaction type
* @ucd_rsp_ptr: pointer to response UPIU
*/
@@ -1440,6 +1452,80 @@ static int ufshcd_dme_link_startup(struct ufs_hba *hba)
}
/**
+ * ufshcd_dme_set_attr - UIC command for DME_SET, DME_PEER_SET
+ * @hba: per adapter instance
+ * @attr_sel: uic command argument1
+ * @attr_set: attribute set type as uic command argument2
+ * @mib_val: setting value as uic command argument3
+ * @peer: indicate whether peer or local
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel,
+ u8 attr_set, u32 mib_val, u8 peer)
+{
+ struct uic_command uic_cmd = {0};
+ static const char *const action[] = {
+ "dme-set",
+ "dme-peer-set"
+ };
+ const char *set = action[!!peer];
+ int ret;
+
+ uic_cmd.command = peer ?
+ UIC_CMD_DME_PEER_SET : UIC_CMD_DME_SET;
+ uic_cmd.argument1 = attr_sel;
+ uic_cmd.argument2 = UIC_ARG_ATTR_TYPE(attr_set);
+ uic_cmd.argument3 = mib_val;
+
+ ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
+ if (ret)
+ dev_err(hba->dev, "%s: attr-id 0x%x val 0x%x error code %d\n",
+ set, UIC_GET_ATTR_ID(attr_sel), mib_val, ret);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(ufshcd_dme_set_attr);
+
+/**
+ * ufshcd_dme_get_attr - UIC command for DME_GET, DME_PEER_GET
+ * @hba: per adapter instance
+ * @attr_sel: uic command argument1
+ * @mib_val: the value of the attribute as returned by the UIC command
+ * @peer: indicate whether peer or local
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
+ u32 *mib_val, u8 peer)
+{
+ struct uic_command uic_cmd = {0};
+ static const char *const action[] = {
+ "dme-get",
+ "dme-peer-get"
+ };
+ const char *get = action[!!peer];
+ int ret;
+
+ uic_cmd.command = peer ?
+ UIC_CMD_DME_PEER_GET : UIC_CMD_DME_GET;
+ uic_cmd.argument1 = attr_sel;
+
+ ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
+ if (ret) {
+ dev_err(hba->dev, "%s: attr-id 0x%x error code %d\n",
+ get, UIC_GET_ATTR_ID(attr_sel), ret);
+ goto out;
+ }
+
+ if (mib_val)
+ *mib_val = uic_cmd.argument3;
+out:
+ return ret;
+}
+EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
+
+/**
* ufshcd_complete_dev_init() - checks device readiness
* hba: per-adapter instance
*
@@ -1912,6 +1998,8 @@ static void ufshcd_uic_cmd_compl(struct ufs_hba *hba)
if (hba->active_uic_cmd) {
hba->active_uic_cmd->argument2 |=
ufshcd_get_uic_cmd_result(hba);
+ hba->active_uic_cmd->argument3 =
+ ufshcd_get_dme_attr_val(hba);
complete(&hba->active_uic_cmd->done);
}
}
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 8f5624e..ab1518d 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -275,4 +275,55 @@ static inline void check_upiu_size(void)
extern int ufshcd_runtime_suspend(struct ufs_hba *hba);
extern int ufshcd_runtime_resume(struct ufs_hba *hba);
extern int ufshcd_runtime_idle(struct ufs_hba *hba);
+extern int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel,
+ u8 attr_set, u32 mib_val, u8 peer);
+extern int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
+ u32 *mib_val, u8 peer);
+
+/* UIC command interfaces for DME primitives */
+#define DME_LOCAL 0
+#define DME_PEER 1
+#define ATTR_SET_NOR 0 /* NORMAL */
+#define ATTR_SET_ST 1 /* STATIC */
+
+static inline int ufshcd_dme_set(struct ufs_hba *hba, u32 attr_sel,
+ u32 mib_val)
+{
+ return ufshcd_dme_set_attr(hba, attr_sel, ATTR_SET_NOR,
+ mib_val, DME_LOCAL);
+}
+
+static inline int ufshcd_dme_st_set(struct ufs_hba *hba, u32 attr_sel,
+ u32 mib_val)
+{
+ return ufshcd_dme_set_attr(hba, attr_sel, ATTR_SET_ST,
+ mib_val, DME_LOCAL);
+}
+
+static inline int ufshcd_dme_peer_set(struct ufs_hba *hba, u32 attr_sel,
+ u32 mib_val)
+{
+ return ufshcd_dme_set_attr(hba, attr_sel, ATTR_SET_NOR,
+ mib_val, DME_PEER);
+}
+
+static inline int ufshcd_dme_peer_st_set(struct ufs_hba *hba, u32 attr_sel,
+ u32 mib_val)
+{
+ return ufshcd_dme_set_attr(hba, attr_sel, ATTR_SET_ST,
+ mib_val, DME_PEER);
+}
+
+static inline int ufshcd_dme_get(struct ufs_hba *hba,
+ u32 attr_sel, u32 *mib_val)
+{
+ return ufshcd_dme_get_attr(hba, attr_sel, mib_val, DME_LOCAL);
+}
+
+static inline int ufshcd_dme_peer_get(struct ufs_hba *hba,
+ u32 attr_sel, u32 *mib_val)
+{
+ return ufshcd_dme_get_attr(hba, attr_sel, mib_val, DME_PEER);
+}
+
#endif /* End of Header */
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index 739ae3a..1e1fe26 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -191,6 +191,12 @@ enum {
#define CONFIG_RESULT_CODE_MASK 0xFF
#define GENERIC_ERROR_CODE_MASK 0xFF
+#define UIC_ARG_MIB_SEL(attr, sel) ((((attr) & 0xFFFF) << 16) |\
+ ((sel) & 0xFFFF))
+#define UIC_ARG_MIB(attr) UIC_ARG_MIB_SEL(attr, 0)
+#define UIC_ARG_ATTR_TYPE(t) (((t) & 0xFF) << 16)
+#define UIC_GET_ATTR_ID(v) (((v) >> 16) & 0xFFFF)
+
/* UIC Commands */
enum {
UIC_CMD_DME_GET = 0x01,
--
1.7.0.4
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH v3 3/6] scsi: ufs: add dme configuration primitives
2013-08-26 14:40 ` [PATCH v3 3/6] scsi: ufs: add dme configuration primitives Seungwon Jeon
@ 2013-08-27 9:15 ` Subhash Jadavani
2013-08-28 12:44 ` Yaniv Gardi
0 siblings, 1 reply; 72+ messages in thread
From: Subhash Jadavani @ 2013-08-27 9:15 UTC (permalink / raw)
To: Seungwon Jeon
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
Looks good to me.
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
On 8/26/2013 8:10 PM, Seungwon Jeon wrote:
> Implements to support GET and SET operations of the DME.
> These operations are used to configure the behavior of
> the UNIPRO. Along with basic operation, {Peer/AttrSetType}
> can be mixed.
>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
> ---
> drivers/scsi/ufs/ufshcd.c | 88 +++++++++++++++++++++++++++++++++++++++++++++
> drivers/scsi/ufs/ufshcd.h | 51 ++++++++++++++++++++++++++
> drivers/scsi/ufs/ufshci.h | 6 +++
> 3 files changed, 145 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index c90b88a..00bfc1a 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -285,6 +285,18 @@ static inline int ufshcd_get_uic_cmd_result(struct ufs_hba *hba)
> }
>
> /**
> + * ufshcd_get_dme_attr_val - Get the value of attribute returned by UIC command
> + * @hba: Pointer to adapter instance
> + *
> + * This function gets UIC command argument3
> + * Returns 0 on success, non zero value on error
> + */
> +static inline u32 ufshcd_get_dme_attr_val(struct ufs_hba *hba)
> +{
> + return ufshcd_readl(hba, REG_UIC_COMMAND_ARG_3);
> +}
> +
> +/**
> * ufshcd_get_req_rsp - returns the TR response transaction type
> * @ucd_rsp_ptr: pointer to response UPIU
> */
> @@ -1440,6 +1452,80 @@ static int ufshcd_dme_link_startup(struct ufs_hba *hba)
> }
>
> /**
> + * ufshcd_dme_set_attr - UIC command for DME_SET, DME_PEER_SET
> + * @hba: per adapter instance
> + * @attr_sel: uic command argument1
> + * @attr_set: attribute set type as uic command argument2
> + * @mib_val: setting value as uic command argument3
> + * @peer: indicate whether peer or local
> + *
> + * Returns 0 on success, non-zero value on failure
> + */
> +int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel,
> + u8 attr_set, u32 mib_val, u8 peer)
> +{
> + struct uic_command uic_cmd = {0};
> + static const char *const action[] = {
> + "dme-set",
> + "dme-peer-set"
> + };
> + const char *set = action[!!peer];
> + int ret;
> +
> + uic_cmd.command = peer ?
> + UIC_CMD_DME_PEER_SET : UIC_CMD_DME_SET;
> + uic_cmd.argument1 = attr_sel;
> + uic_cmd.argument2 = UIC_ARG_ATTR_TYPE(attr_set);
> + uic_cmd.argument3 = mib_val;
> +
> + ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
> + if (ret)
> + dev_err(hba->dev, "%s: attr-id 0x%x val 0x%x error code %d\n",
> + set, UIC_GET_ATTR_ID(attr_sel), mib_val, ret);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(ufshcd_dme_set_attr);
> +
> +/**
> + * ufshcd_dme_get_attr - UIC command for DME_GET, DME_PEER_GET
> + * @hba: per adapter instance
> + * @attr_sel: uic command argument1
> + * @mib_val: the value of the attribute as returned by the UIC command
> + * @peer: indicate whether peer or local
> + *
> + * Returns 0 on success, non-zero value on failure
> + */
> +int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
> + u32 *mib_val, u8 peer)
> +{
> + struct uic_command uic_cmd = {0};
> + static const char *const action[] = {
> + "dme-get",
> + "dme-peer-get"
> + };
> + const char *get = action[!!peer];
> + int ret;
> +
> + uic_cmd.command = peer ?
> + UIC_CMD_DME_PEER_GET : UIC_CMD_DME_GET;
> + uic_cmd.argument1 = attr_sel;
> +
> + ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
> + if (ret) {
> + dev_err(hba->dev, "%s: attr-id 0x%x error code %d\n",
> + get, UIC_GET_ATTR_ID(attr_sel), ret);
> + goto out;
> + }
> +
> + if (mib_val)
> + *mib_val = uic_cmd.argument3;
> +out:
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
> +
> +/**
> * ufshcd_complete_dev_init() - checks device readiness
> * hba: per-adapter instance
> *
> @@ -1912,6 +1998,8 @@ static void ufshcd_uic_cmd_compl(struct ufs_hba *hba)
> if (hba->active_uic_cmd) {
> hba->active_uic_cmd->argument2 |=
> ufshcd_get_uic_cmd_result(hba);
> + hba->active_uic_cmd->argument3 =
> + ufshcd_get_dme_attr_val(hba);
> complete(&hba->active_uic_cmd->done);
> }
> }
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 8f5624e..ab1518d 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -275,4 +275,55 @@ static inline void check_upiu_size(void)
> extern int ufshcd_runtime_suspend(struct ufs_hba *hba);
> extern int ufshcd_runtime_resume(struct ufs_hba *hba);
> extern int ufshcd_runtime_idle(struct ufs_hba *hba);
> +extern int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel,
> + u8 attr_set, u32 mib_val, u8 peer);
> +extern int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
> + u32 *mib_val, u8 peer);
> +
> +/* UIC command interfaces for DME primitives */
> +#define DME_LOCAL 0
> +#define DME_PEER 1
> +#define ATTR_SET_NOR 0 /* NORMAL */
> +#define ATTR_SET_ST 1 /* STATIC */
> +
> +static inline int ufshcd_dme_set(struct ufs_hba *hba, u32 attr_sel,
> + u32 mib_val)
> +{
> + return ufshcd_dme_set_attr(hba, attr_sel, ATTR_SET_NOR,
> + mib_val, DME_LOCAL);
> +}
> +
> +static inline int ufshcd_dme_st_set(struct ufs_hba *hba, u32 attr_sel,
> + u32 mib_val)
> +{
> + return ufshcd_dme_set_attr(hba, attr_sel, ATTR_SET_ST,
> + mib_val, DME_LOCAL);
> +}
> +
> +static inline int ufshcd_dme_peer_set(struct ufs_hba *hba, u32 attr_sel,
> + u32 mib_val)
> +{
> + return ufshcd_dme_set_attr(hba, attr_sel, ATTR_SET_NOR,
> + mib_val, DME_PEER);
> +}
> +
> +static inline int ufshcd_dme_peer_st_set(struct ufs_hba *hba, u32 attr_sel,
> + u32 mib_val)
> +{
> + return ufshcd_dme_set_attr(hba, attr_sel, ATTR_SET_ST,
> + mib_val, DME_PEER);
> +}
> +
> +static inline int ufshcd_dme_get(struct ufs_hba *hba,
> + u32 attr_sel, u32 *mib_val)
> +{
> + return ufshcd_dme_get_attr(hba, attr_sel, mib_val, DME_LOCAL);
> +}
> +
> +static inline int ufshcd_dme_peer_get(struct ufs_hba *hba,
> + u32 attr_sel, u32 *mib_val)
> +{
> + return ufshcd_dme_get_attr(hba, attr_sel, mib_val, DME_PEER);
> +}
> +
> #endif /* End of Header */
> diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
> index 739ae3a..1e1fe26 100644
> --- a/drivers/scsi/ufs/ufshci.h
> +++ b/drivers/scsi/ufs/ufshci.h
> @@ -191,6 +191,12 @@ enum {
> #define CONFIG_RESULT_CODE_MASK 0xFF
> #define GENERIC_ERROR_CODE_MASK 0xFF
>
> +#define UIC_ARG_MIB_SEL(attr, sel) ((((attr) & 0xFFFF) << 16) |\
> + ((sel) & 0xFFFF))
> +#define UIC_ARG_MIB(attr) UIC_ARG_MIB_SEL(attr, 0)
> +#define UIC_ARG_ATTR_TYPE(t) (((t) & 0xFF) << 16)
> +#define UIC_GET_ATTR_ID(v) (((v) >> 16) & 0xFFFF)
> +
> /* UIC Commands */
> enum {
> UIC_CMD_DME_GET = 0x01,
^ permalink raw reply [flat|nested] 72+ messages in thread* RE: [PATCH v3 3/6] scsi: ufs: add dme configuration primitives
2013-08-27 9:15 ` Subhash Jadavani
@ 2013-08-28 12:44 ` Yaniv Gardi
0 siblings, 0 replies; 72+ messages in thread
From: Yaniv Gardi @ 2013-08-28 12:44 UTC (permalink / raw)
To: 'Subhash Jadavani', 'Seungwon Jeon'
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
Tested-by: Yaniv Gardi <ygardi@codeaurora.org>
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
= > -----Original Message-----
= > From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
= > owner@vger.kernel.org] On Behalf Of Subhash Jadavani
= > Sent: Tuesday, August 27, 2013 12:16 PM
= > To: Seungwon Jeon
= > Cc: linux-scsi@vger.kernel.org; 'Vinayak Holikatti'; 'Santosh Y'; 'James E.J.
= > Bottomley'
= > Subject: Re: [PATCH v3 3/6] scsi: ufs: add dme configuration primitives
= >
= >
= > Looks good to me.
= > Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
= >
= > On 8/26/2013 8:10 PM, Seungwon Jeon wrote:
= > > Implements to support GET and SET operations of the DME.
= > > These operations are used to configure the behavior of the UNIPRO.
= > > Along with basic operation, {Peer/AttrSetType} can be mixed.
= > >
= > > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
= > > Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
= > > ---
= > > drivers/scsi/ufs/ufshcd.c | 88
= > +++++++++++++++++++++++++++++++++++++++++++++
= > > drivers/scsi/ufs/ufshcd.h | 51 ++++++++++++++++++++++++++
= > > drivers/scsi/ufs/ufshci.h | 6 +++
= > > 3 files changed, 145 insertions(+), 0 deletions(-)
= > >
= > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
= > > index c90b88a..00bfc1a 100644
= > > --- a/drivers/scsi/ufs/ufshcd.c
= > > +++ b/drivers/scsi/ufs/ufshcd.c
= > > @@ -285,6 +285,18 @@ static inline int
= > ufshcd_get_uic_cmd_result(struct ufs_hba *hba)
= > > }
= > >
= > > /**
= > > + * ufshcd_get_dme_attr_val - Get the value of attribute returned by
= > > +UIC command
= > > + * @hba: Pointer to adapter instance
= > > + *
= > > + * This function gets UIC command argument3
= > > + * Returns 0 on success, non zero value on error */ static inline
= > > +u32 ufshcd_get_dme_attr_val(struct ufs_hba *hba) {
= > > + return ufshcd_readl(hba, REG_UIC_COMMAND_ARG_3); }
= > > +
= > > +/**
= > > * ufshcd_get_req_rsp - returns the TR response transaction type
= > > * @ucd_rsp_ptr: pointer to response UPIU
= > > */
= > > @@ -1440,6 +1452,80 @@ static int ufshcd_dme_link_startup(struct
= > ufs_hba *hba)
= > > }
= > >
= > > /**
= > > + * ufshcd_dme_set_attr - UIC command for DME_SET, DME_PEER_SET
= > > + * @hba: per adapter instance
= > > + * @attr_sel: uic command argument1
= > > + * @attr_set: attribute set type as uic command argument2
= > > + * @mib_val: setting value as uic command argument3
= > > + * @peer: indicate whether peer or local
= > > + *
= > > + * Returns 0 on success, non-zero value on failure */ int
= > > +ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel,
= > > + u8 attr_set, u32 mib_val, u8 peer) {
= > > + struct uic_command uic_cmd = {0};
= > > + static const char *const action[] = {
= > > + "dme-set",
= > > + "dme-peer-set"
= > > + };
= > > + const char *set = action[!!peer];
= > > + int ret;
= > > +
= > > + uic_cmd.command = peer ?
= > > + UIC_CMD_DME_PEER_SET : UIC_CMD_DME_SET;
= > > + uic_cmd.argument1 = attr_sel;
= > > + uic_cmd.argument2 = UIC_ARG_ATTR_TYPE(attr_set);
= > > + uic_cmd.argument3 = mib_val;
= > > +
= > > + ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
= > > + if (ret)
= > > + dev_err(hba->dev, "%s: attr-id 0x%x val 0x%x error code
= > %d\n",
= > > + set, UIC_GET_ATTR_ID(attr_sel), mib_val, ret);
= > > +
= > > + return ret;
= > > +}
= > > +EXPORT_SYMBOL_GPL(ufshcd_dme_set_attr);
= > > +
= > > +/**
= > > + * ufshcd_dme_get_attr - UIC command for DME_GET, DME_PEER_GET
= > > + * @hba: per adapter instance
= > > + * @attr_sel: uic command argument1
= > > + * @mib_val: the value of the attribute as returned by the UIC
= > > +command
= > > + * @peer: indicate whether peer or local
= > > + *
= > > + * Returns 0 on success, non-zero value on failure */ int
= > > +ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
= > > + u32 *mib_val, u8 peer)
= > > +{
= > > + struct uic_command uic_cmd = {0};
= > > + static const char *const action[] = {
= > > + "dme-get",
= > > + "dme-peer-get"
= > > + };
= > > + const char *get = action[!!peer];
= > > + int ret;
= > > +
= > > + uic_cmd.command = peer ?
= > > + UIC_CMD_DME_PEER_GET : UIC_CMD_DME_GET;
= > > + uic_cmd.argument1 = attr_sel;
= > > +
= > > + ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
= > > + if (ret) {
= > > + dev_err(hba->dev, "%s: attr-id 0x%x error code %d\n",
= > > + get, UIC_GET_ATTR_ID(attr_sel), ret);
= > > + goto out;
= > > + }
= > > +
= > > + if (mib_val)
= > > + *mib_val = uic_cmd.argument3;
= > > +out:
= > > + return ret;
= > > +}
= > > +EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
= > > +
= > > +/**
= > > * ufshcd_complete_dev_init() - checks device readiness
= > > * hba: per-adapter instance
= > > *
= > > @@ -1912,6 +1998,8 @@ static void ufshcd_uic_cmd_compl(struct
= > ufs_hba *hba)
= > > if (hba->active_uic_cmd) {
= > > hba->active_uic_cmd->argument2 |=
= > > ufshcd_get_uic_cmd_result(hba);
= > > + hba->active_uic_cmd->argument3 =
= > > + ufshcd_get_dme_attr_val(hba);
= > > complete(&hba->active_uic_cmd->done);
= > > }
= > > }
= > > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
= > > index 8f5624e..ab1518d 100644
= > > --- a/drivers/scsi/ufs/ufshcd.h
= > > +++ b/drivers/scsi/ufs/ufshcd.h
= > > @@ -275,4 +275,55 @@ static inline void check_upiu_size(void)
= > > extern int ufshcd_runtime_suspend(struct ufs_hba *hba);
= > > extern int ufshcd_runtime_resume(struct ufs_hba *hba);
= > > extern int ufshcd_runtime_idle(struct ufs_hba *hba);
= > > +extern int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel,
= > > + u8 attr_set, u32 mib_val, u8 peer); extern int
= > > +ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
= > > + u32 *mib_val, u8 peer);
= > > +
= > > +/* UIC command interfaces for DME primitives */
= > > +#define DME_LOCAL 0
= > > +#define DME_PEER 1
= > > +#define ATTR_SET_NOR 0 /* NORMAL */
= > > +#define ATTR_SET_ST 1 /* STATIC */
= > > +
= > > +static inline int ufshcd_dme_set(struct ufs_hba *hba, u32 attr_sel,
= > > + u32 mib_val)
= > > +{
= > > + return ufshcd_dme_set_attr(hba, attr_sel, ATTR_SET_NOR,
= > > + mib_val, DME_LOCAL);
= > > +}
= > > +
= > > +static inline int ufshcd_dme_st_set(struct ufs_hba *hba, u32 attr_sel,
= > > + u32 mib_val)
= > > +{
= > > + return ufshcd_dme_set_attr(hba, attr_sel, ATTR_SET_ST,
= > > + mib_val, DME_LOCAL);
= > > +}
= > > +
= > > +static inline int ufshcd_dme_peer_set(struct ufs_hba *hba, u32 attr_sel,
= > > + u32 mib_val)
= > > +{
= > > + return ufshcd_dme_set_attr(hba, attr_sel, ATTR_SET_NOR,
= > > + mib_val, DME_PEER);
= > > +}
= > > +
= > > +static inline int ufshcd_dme_peer_st_set(struct ufs_hba *hba, u32
= > attr_sel,
= > > + u32 mib_val)
= > > +{
= > > + return ufshcd_dme_set_attr(hba, attr_sel, ATTR_SET_ST,
= > > + mib_val, DME_PEER);
= > > +}
= > > +
= > > +static inline int ufshcd_dme_get(struct ufs_hba *hba,
= > > + u32 attr_sel, u32 *mib_val)
= > > +{
= > > + return ufshcd_dme_get_attr(hba, attr_sel, mib_val, DME_LOCAL); }
= > > +
= > > +static inline int ufshcd_dme_peer_get(struct ufs_hba *hba,
= > > + u32 attr_sel, u32 *mib_val) {
= > > + return ufshcd_dme_get_attr(hba, attr_sel, mib_val, DME_PEER); }
= > > +
= > > #endif /* End of Header */
= > > diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
= > > index 739ae3a..1e1fe26 100644
= > > --- a/drivers/scsi/ufs/ufshci.h
= > > +++ b/drivers/scsi/ufs/ufshci.h
= > > @@ -191,6 +191,12 @@ enum {
= > > #define CONFIG_RESULT_CODE_MASK 0xFF
= > > #define GENERIC_ERROR_CODE_MASK 0xFF
= > >
= > > +#define UIC_ARG_MIB_SEL(attr, sel) ((((attr) & 0xFFFF) << 16) |\
= > > + ((sel) & 0xFFFF))
= > > +#define UIC_ARG_MIB(attr) UIC_ARG_MIB_SEL(attr, 0)
= > > +#define UIC_ARG_ATTR_TYPE(t) (((t) & 0xFF) << 16)
= > > +#define UIC_GET_ATTR_ID(v) (((v) >> 16) & 0xFFFF)
= > > +
= > > /* UIC Commands */
= > > enum {
= > > UIC_CMD_DME_GET = 0x01,
= >
= > --
= > 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
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v3 4/6] scsi: ufs: add unipro attribute IDs
2013-07-26 13:46 ` [PATCH 2/7] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
` (11 preceding siblings ...)
2013-08-26 14:40 ` [PATCH v3 3/6] scsi: ufs: add dme configuration primitives Seungwon Jeon
@ 2013-08-26 14:40 ` Seungwon Jeon
2013-08-27 9:14 ` Subhash Jadavani
2013-08-26 14:40 ` [PATCH v3 5/6] scsi: ufs: add operation for the uic power mode change Seungwon Jeon
` (2 subsequent siblings)
15 siblings, 1 reply; 72+ messages in thread
From: Seungwon Jeon @ 2013-08-26 14:40 UTC (permalink / raw)
To: linux-scsi
Cc: 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
'drivers/scsi/ufs/unipro.h' is added.
Attributes in the layers of the UNIPRO stack can be
read and written via the DME.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
---
drivers/scsi/ufs/unipro.h | 130 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 130 insertions(+), 0 deletions(-)
create mode 100644 drivers/scsi/ufs/unipro.h
diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h
new file mode 100644
index 0000000..3a710eb
--- /dev/null
+++ b/drivers/scsi/ufs/unipro.h
@@ -0,0 +1,130 @@
+/*
+ * drivers/scsi/ufs/unipro.h
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef _UNIPRO_H_
+#define _UNIPRO_H_
+
+/*
+ * PHY Adpater attributes
+ */
+#define PA_ACTIVETXDATALANES 0x1560
+#define PA_ACTIVERXDATALANES 0x1580
+#define PA_TXTRAILINGCLOCKS 0x1564
+#define PA_PHY_TYPE 0x1500
+#define PA_AVAILTXDATALANES 0x1520
+#define PA_AVAILRXDATALANES 0x1540
+#define PA_MINRXTRAILINGCLOCKS 0x1543
+#define PA_TXPWRSTATUS 0x1567
+#define PA_RXPWRSTATUS 0x1582
+#define PA_TXFORCECLOCK 0x1562
+#define PA_TXPWRMODE 0x1563
+#define PA_LEGACYDPHYESCDL 0x1570
+#define PA_MAXTXSPEEDFAST 0x1521
+#define PA_MAXTXSPEEDSLOW 0x1522
+#define PA_MAXRXSPEEDFAST 0x1541
+#define PA_MAXRXSPEEDSLOW 0x1542
+#define PA_TXLINKSTARTUPHS 0x1544
+#define PA_TXSPEEDFAST 0x1565
+#define PA_TXSPEEDSLOW 0x1566
+#define PA_REMOTEVERINFO 0x15A0
+#define PA_TXGEAR 0x1568
+#define PA_TXTERMINATION 0x1569
+#define PA_HSSERIES 0x156A
+#define PA_PWRMODE 0x1571
+#define PA_RXGEAR 0x1583
+#define PA_RXTERMINATION 0x1584
+#define PA_MAXRXPWMGEAR 0x1586
+#define PA_MAXRXHSGEAR 0x1587
+#define PA_RXHSUNTERMCAP 0x15A5
+#define PA_RXLSTERMCAP 0x15A6
+#define PA_PACPREQTIMEOUT 0x1590
+#define PA_PACPREQEOBTIMEOUT 0x1591
+#define PA_HIBERN8TIME 0x15A7
+#define PA_LOCALVERINFO 0x15A9
+#define PA_TACTIVATE 0x15A8
+#define PA_PACPFRAMECOUNT 0x15C0
+#define PA_PACPERRORCOUNT 0x15C1
+#define PA_PHYTESTCONTROL 0x15C2
+#define PA_PWRMODEUSERDATA0 0x15B0
+#define PA_PWRMODEUSERDATA1 0x15B1
+#define PA_PWRMODEUSERDATA2 0x15B2
+#define PA_PWRMODEUSERDATA3 0x15B3
+#define PA_PWRMODEUSERDATA4 0x15B4
+#define PA_PWRMODEUSERDATA5 0x15B5
+#define PA_PWRMODEUSERDATA6 0x15B6
+#define PA_PWRMODEUSERDATA7 0x15B7
+#define PA_PWRMODEUSERDATA8 0x15B8
+#define PA_PWRMODEUSERDATA9 0x15B9
+#define PA_PWRMODEUSERDATA10 0x15BA
+#define PA_PWRMODEUSERDATA11 0x15BB
+#define PA_CONNECTEDTXDATALANES 0x1561
+#define PA_CONNECTEDRXDATALANES 0x1581
+#define PA_LOGICALLANEMAP 0x15A1
+#define PA_SLEEPNOCONFIGTIME 0x15A2
+#define PA_STALLNOCONFIGTIME 0x15A3
+#define PA_SAVECONFIGTIME 0x15A4
+
+/*
+ * Data Link Layer Attributes
+ */
+#define DL_TC0TXFCTHRESHOLD 0x2040
+#define DL_FC0PROTTIMEOUTVAL 0x2041
+#define DL_TC0REPLAYTIMEOUTVAL 0x2042
+#define DL_AFC0REQTIMEOUTVAL 0x2043
+#define DL_AFC0CREDITTHRESHOLD 0x2044
+#define DL_TC0OUTACKTHRESHOLD 0x2045
+#define DL_TC1TXFCTHRESHOLD 0x2060
+#define DL_FC1PROTTIMEOUTVAL 0x2061
+#define DL_TC1REPLAYTIMEOUTVAL 0x2062
+#define DL_AFC1REQTIMEOUTVAL 0x2063
+#define DL_AFC1CREDITTHRESHOLD 0x2064
+#define DL_TC1OUTACKTHRESHOLD 0x2065
+#define DL_TXPREEMPTIONCAP 0x2000
+#define DL_TC0TXMAXSDUSIZE 0x2001
+#define DL_TC0RXINITCREDITVAL 0x2002
+#define DL_TC0TXBUFFERSIZE 0x2005
+#define DL_PEERTC0PRESENT 0x2046
+#define DL_PEERTC0RXINITCREVAL 0x2047
+#define DL_TC1TXMAXSDUSIZE 0x2003
+#define DL_TC1RXINITCREDITVAL 0x2004
+#define DL_TC1TXBUFFERSIZE 0x2006
+#define DL_PEERTC1PRESENT 0x2066
+#define DL_PEERTC1RXINITCREVAL 0x2067
+
+/*
+ * Network Layer Attributes
+ */
+#define N_DEVICEID 0x3000
+#define N_DEVICEID_VALID 0x3001
+#define N_TC0TXMAXSDUSIZE 0x3020
+#define N_TC1TXMAXSDUSIZE 0x3021
+
+/*
+ * Transport Layer Attributes
+ */
+#define T_NUMCPORTS 0x4000
+#define T_NUMTESTFEATURES 0x4001
+#define T_CONNECTIONSTATE 0x4020
+#define T_PEERDEVICEID 0x4021
+#define T_PEERCPORTID 0x4022
+#define T_TRAFFICCLASS 0x4023
+#define T_PROTOCOLID 0x4024
+#define T_CPORTFLAGS 0x4025
+#define T_TXTOKENVALUE 0x4026
+#define T_RXTOKENVALUE 0x4027
+#define T_LOCALBUFFERSPACE 0x4028
+#define T_PEERBUFFERSPACE 0x4029
+#define T_CREDITSTOSEND 0x402A
+#define T_CPORTMODE 0x402B
+#define T_TC0TXMAXSDUSIZE 0x4060
+#define T_TC1TXMAXSDUSIZE 0x4061
+
+#endif /* _UNIPRO_H_ */
--
1.7.0.4
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH v3 4/6] scsi: ufs: add unipro attribute IDs
2013-08-26 14:40 ` [PATCH v3 4/6] scsi: ufs: add unipro attribute IDs Seungwon Jeon
@ 2013-08-27 9:14 ` Subhash Jadavani
2013-08-28 12:46 ` Yaniv Gardi
0 siblings, 1 reply; 72+ messages in thread
From: Subhash Jadavani @ 2013-08-27 9:14 UTC (permalink / raw)
To: Seungwon Jeon
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
Looks good to me.
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
On 8/26/2013 8:10 PM, Seungwon Jeon wrote:
> 'drivers/scsi/ufs/unipro.h' is added.
> Attributes in the layers of the UNIPRO stack can be
> read and written via the DME.
>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
> ---
> drivers/scsi/ufs/unipro.h | 130 +++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 130 insertions(+), 0 deletions(-)
> create mode 100644 drivers/scsi/ufs/unipro.h
>
> diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h
> new file mode 100644
> index 0000000..3a710eb
> --- /dev/null
> +++ b/drivers/scsi/ufs/unipro.h
> @@ -0,0 +1,130 @@
> +/*
> + * drivers/scsi/ufs/unipro.h
> + *
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#ifndef _UNIPRO_H_
> +#define _UNIPRO_H_
> +
> +/*
> + * PHY Adpater attributes
> + */
> +#define PA_ACTIVETXDATALANES 0x1560
> +#define PA_ACTIVERXDATALANES 0x1580
> +#define PA_TXTRAILINGCLOCKS 0x1564
> +#define PA_PHY_TYPE 0x1500
> +#define PA_AVAILTXDATALANES 0x1520
> +#define PA_AVAILRXDATALANES 0x1540
> +#define PA_MINRXTRAILINGCLOCKS 0x1543
> +#define PA_TXPWRSTATUS 0x1567
> +#define PA_RXPWRSTATUS 0x1582
> +#define PA_TXFORCECLOCK 0x1562
> +#define PA_TXPWRMODE 0x1563
> +#define PA_LEGACYDPHYESCDL 0x1570
> +#define PA_MAXTXSPEEDFAST 0x1521
> +#define PA_MAXTXSPEEDSLOW 0x1522
> +#define PA_MAXRXSPEEDFAST 0x1541
> +#define PA_MAXRXSPEEDSLOW 0x1542
> +#define PA_TXLINKSTARTUPHS 0x1544
> +#define PA_TXSPEEDFAST 0x1565
> +#define PA_TXSPEEDSLOW 0x1566
> +#define PA_REMOTEVERINFO 0x15A0
> +#define PA_TXGEAR 0x1568
> +#define PA_TXTERMINATION 0x1569
> +#define PA_HSSERIES 0x156A
> +#define PA_PWRMODE 0x1571
> +#define PA_RXGEAR 0x1583
> +#define PA_RXTERMINATION 0x1584
> +#define PA_MAXRXPWMGEAR 0x1586
> +#define PA_MAXRXHSGEAR 0x1587
> +#define PA_RXHSUNTERMCAP 0x15A5
> +#define PA_RXLSTERMCAP 0x15A6
> +#define PA_PACPREQTIMEOUT 0x1590
> +#define PA_PACPREQEOBTIMEOUT 0x1591
> +#define PA_HIBERN8TIME 0x15A7
> +#define PA_LOCALVERINFO 0x15A9
> +#define PA_TACTIVATE 0x15A8
> +#define PA_PACPFRAMECOUNT 0x15C0
> +#define PA_PACPERRORCOUNT 0x15C1
> +#define PA_PHYTESTCONTROL 0x15C2
> +#define PA_PWRMODEUSERDATA0 0x15B0
> +#define PA_PWRMODEUSERDATA1 0x15B1
> +#define PA_PWRMODEUSERDATA2 0x15B2
> +#define PA_PWRMODEUSERDATA3 0x15B3
> +#define PA_PWRMODEUSERDATA4 0x15B4
> +#define PA_PWRMODEUSERDATA5 0x15B5
> +#define PA_PWRMODEUSERDATA6 0x15B6
> +#define PA_PWRMODEUSERDATA7 0x15B7
> +#define PA_PWRMODEUSERDATA8 0x15B8
> +#define PA_PWRMODEUSERDATA9 0x15B9
> +#define PA_PWRMODEUSERDATA10 0x15BA
> +#define PA_PWRMODEUSERDATA11 0x15BB
> +#define PA_CONNECTEDTXDATALANES 0x1561
> +#define PA_CONNECTEDRXDATALANES 0x1581
> +#define PA_LOGICALLANEMAP 0x15A1
> +#define PA_SLEEPNOCONFIGTIME 0x15A2
> +#define PA_STALLNOCONFIGTIME 0x15A3
> +#define PA_SAVECONFIGTIME 0x15A4
> +
> +/*
> + * Data Link Layer Attributes
> + */
> +#define DL_TC0TXFCTHRESHOLD 0x2040
> +#define DL_FC0PROTTIMEOUTVAL 0x2041
> +#define DL_TC0REPLAYTIMEOUTVAL 0x2042
> +#define DL_AFC0REQTIMEOUTVAL 0x2043
> +#define DL_AFC0CREDITTHRESHOLD 0x2044
> +#define DL_TC0OUTACKTHRESHOLD 0x2045
> +#define DL_TC1TXFCTHRESHOLD 0x2060
> +#define DL_FC1PROTTIMEOUTVAL 0x2061
> +#define DL_TC1REPLAYTIMEOUTVAL 0x2062
> +#define DL_AFC1REQTIMEOUTVAL 0x2063
> +#define DL_AFC1CREDITTHRESHOLD 0x2064
> +#define DL_TC1OUTACKTHRESHOLD 0x2065
> +#define DL_TXPREEMPTIONCAP 0x2000
> +#define DL_TC0TXMAXSDUSIZE 0x2001
> +#define DL_TC0RXINITCREDITVAL 0x2002
> +#define DL_TC0TXBUFFERSIZE 0x2005
> +#define DL_PEERTC0PRESENT 0x2046
> +#define DL_PEERTC0RXINITCREVAL 0x2047
> +#define DL_TC1TXMAXSDUSIZE 0x2003
> +#define DL_TC1RXINITCREDITVAL 0x2004
> +#define DL_TC1TXBUFFERSIZE 0x2006
> +#define DL_PEERTC1PRESENT 0x2066
> +#define DL_PEERTC1RXINITCREVAL 0x2067
> +
> +/*
> + * Network Layer Attributes
> + */
> +#define N_DEVICEID 0x3000
> +#define N_DEVICEID_VALID 0x3001
> +#define N_TC0TXMAXSDUSIZE 0x3020
> +#define N_TC1TXMAXSDUSIZE 0x3021
> +
> +/*
> + * Transport Layer Attributes
> + */
> +#define T_NUMCPORTS 0x4000
> +#define T_NUMTESTFEATURES 0x4001
> +#define T_CONNECTIONSTATE 0x4020
> +#define T_PEERDEVICEID 0x4021
> +#define T_PEERCPORTID 0x4022
> +#define T_TRAFFICCLASS 0x4023
> +#define T_PROTOCOLID 0x4024
> +#define T_CPORTFLAGS 0x4025
> +#define T_TXTOKENVALUE 0x4026
> +#define T_RXTOKENVALUE 0x4027
> +#define T_LOCALBUFFERSPACE 0x4028
> +#define T_PEERBUFFERSPACE 0x4029
> +#define T_CREDITSTOSEND 0x402A
> +#define T_CPORTMODE 0x402B
> +#define T_TC0TXMAXSDUSIZE 0x4060
> +#define T_TC1TXMAXSDUSIZE 0x4061
> +
> +#endif /* _UNIPRO_H_ */
^ permalink raw reply [flat|nested] 72+ messages in thread
* RE: [PATCH v3 4/6] scsi: ufs: add unipro attribute IDs
2013-08-27 9:14 ` Subhash Jadavani
@ 2013-08-28 12:46 ` Yaniv Gardi
0 siblings, 0 replies; 72+ messages in thread
From: Yaniv Gardi @ 2013-08-28 12:46 UTC (permalink / raw)
To: 'Subhash Jadavani', 'Seungwon Jeon'
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
Tested-by: Yaniv Gardi <ygardi@codeaurora.org>
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
= > -----Original Message-----
= > From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
= > owner@vger.kernel.org] On Behalf Of Subhash Jadavani
= > Sent: Tuesday, August 27, 2013 12:15 PM
= > To: Seungwon Jeon
= > Cc: linux-scsi@vger.kernel.org; 'Vinayak Holikatti'; 'Santosh Y'; 'James E.J.
= > Bottomley'
= > Subject: Re: [PATCH v3 4/6] scsi: ufs: add unipro attribute IDs
= >
= >
= > Looks good to me.
= > Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
= >
= > On 8/26/2013 8:10 PM, Seungwon Jeon wrote:
= > > 'drivers/scsi/ufs/unipro.h' is added.
= > > Attributes in the layers of the UNIPRO stack can be read and written
= > > via the DME.
= > >
= > > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
= > > Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
= > > ---
= > > drivers/scsi/ufs/unipro.h | 130
= > +++++++++++++++++++++++++++++++++++++++++++++
= > > 1 files changed, 130 insertions(+), 0 deletions(-)
= > > create mode 100644 drivers/scsi/ufs/unipro.h
= > >
= > > diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h new
= > > file mode 100644 index 0000000..3a710eb
= > > --- /dev/null
= > > +++ b/drivers/scsi/ufs/unipro.h
= > > @@ -0,0 +1,130 @@
= > > +/*
= > > + * drivers/scsi/ufs/unipro.h
= > > + *
= > > + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
= > > + *
= > > + * This program is free software; you can redistribute it and/or
= > > +modify
= > > + * it under the terms of the GNU General Public License as published
= > > +by
= > > + * the Free Software Foundation; either version 2 of the License, or
= > > + * (at your option) any later version.
= > > + */
= > > +
= > > +#ifndef _UNIPRO_H_
= > > +#define _UNIPRO_H_
= > > +
= > > +/*
= > > + * PHY Adpater attributes
= > > + */
= > > +#define PA_ACTIVETXDATALANES 0x1560
= > > +#define PA_ACTIVERXDATALANES 0x1580
= > > +#define PA_TXTRAILINGCLOCKS 0x1564
= > > +#define PA_PHY_TYPE 0x1500
= > > +#define PA_AVAILTXDATALANES 0x1520
= > > +#define PA_AVAILRXDATALANES 0x1540
= > > +#define PA_MINRXTRAILINGCLOCKS 0x1543
= > > +#define PA_TXPWRSTATUS 0x1567
= > > +#define PA_RXPWRSTATUS 0x1582
= > > +#define PA_TXFORCECLOCK 0x1562
= > > +#define PA_TXPWRMODE 0x1563
= > > +#define PA_LEGACYDPHYESCDL 0x1570
= > > +#define PA_MAXTXSPEEDFAST 0x1521
= > > +#define PA_MAXTXSPEEDSLOW 0x1522
= > > +#define PA_MAXRXSPEEDFAST 0x1541
= > > +#define PA_MAXRXSPEEDSLOW 0x1542
= > > +#define PA_TXLINKSTARTUPHS 0x1544
= > > +#define PA_TXSPEEDFAST 0x1565
= > > +#define PA_TXSPEEDSLOW 0x1566
= > > +#define PA_REMOTEVERINFO 0x15A0
= > > +#define PA_TXGEAR 0x1568
= > > +#define PA_TXTERMINATION 0x1569
= > > +#define PA_HSSERIES 0x156A
= > > +#define PA_PWRMODE 0x1571
= > > +#define PA_RXGEAR 0x1583
= > > +#define PA_RXTERMINATION 0x1584
= > > +#define PA_MAXRXPWMGEAR 0x1586
= > > +#define PA_MAXRXHSGEAR 0x1587
= > > +#define PA_RXHSUNTERMCAP 0x15A5
= > > +#define PA_RXLSTERMCAP 0x15A6
= > > +#define PA_PACPREQTIMEOUT 0x1590
= > > +#define PA_PACPREQEOBTIMEOUT 0x1591
= > > +#define PA_HIBERN8TIME 0x15A7
= > > +#define PA_LOCALVERINFO 0x15A9
= > > +#define PA_TACTIVATE 0x15A8
= > > +#define PA_PACPFRAMECOUNT 0x15C0
= > > +#define PA_PACPERRORCOUNT 0x15C1
= > > +#define PA_PHYTESTCONTROL 0x15C2
= > > +#define PA_PWRMODEUSERDATA0 0x15B0
= > > +#define PA_PWRMODEUSERDATA1 0x15B1
= > > +#define PA_PWRMODEUSERDATA2 0x15B2
= > > +#define PA_PWRMODEUSERDATA3 0x15B3
= > > +#define PA_PWRMODEUSERDATA4 0x15B4
= > > +#define PA_PWRMODEUSERDATA5 0x15B5
= > > +#define PA_PWRMODEUSERDATA6 0x15B6
= > > +#define PA_PWRMODEUSERDATA7 0x15B7
= > > +#define PA_PWRMODEUSERDATA8 0x15B8
= > > +#define PA_PWRMODEUSERDATA9 0x15B9
= > > +#define PA_PWRMODEUSERDATA10 0x15BA
= > > +#define PA_PWRMODEUSERDATA11 0x15BB
= > > +#define PA_CONNECTEDTXDATALANES 0x1561
= > > +#define PA_CONNECTEDRXDATALANES 0x1581
= > > +#define PA_LOGICALLANEMAP 0x15A1
= > > +#define PA_SLEEPNOCONFIGTIME 0x15A2
= > > +#define PA_STALLNOCONFIGTIME 0x15A3
= > > +#define PA_SAVECONFIGTIME 0x15A4
= > > +
= > > +/*
= > > + * Data Link Layer Attributes
= > > + */
= > > +#define DL_TC0TXFCTHRESHOLD 0x2040
= > > +#define DL_FC0PROTTIMEOUTVAL 0x2041
= > > +#define DL_TC0REPLAYTIMEOUTVAL 0x2042
= > > +#define DL_AFC0REQTIMEOUTVAL 0x2043
= > > +#define DL_AFC0CREDITTHRESHOLD 0x2044
= > > +#define DL_TC0OUTACKTHRESHOLD 0x2045
= > > +#define DL_TC1TXFCTHRESHOLD 0x2060
= > > +#define DL_FC1PROTTIMEOUTVAL 0x2061
= > > +#define DL_TC1REPLAYTIMEOUTVAL 0x2062
= > > +#define DL_AFC1REQTIMEOUTVAL 0x2063
= > > +#define DL_AFC1CREDITTHRESHOLD 0x2064
= > > +#define DL_TC1OUTACKTHRESHOLD 0x2065
= > > +#define DL_TXPREEMPTIONCAP 0x2000
= > > +#define DL_TC0TXMAXSDUSIZE 0x2001
= > > +#define DL_TC0RXINITCREDITVAL 0x2002
= > > +#define DL_TC0TXBUFFERSIZE 0x2005
= > > +#define DL_PEERTC0PRESENT 0x2046
= > > +#define DL_PEERTC0RXINITCREVAL 0x2047
= > > +#define DL_TC1TXMAXSDUSIZE 0x2003
= > > +#define DL_TC1RXINITCREDITVAL 0x2004
= > > +#define DL_TC1TXBUFFERSIZE 0x2006
= > > +#define DL_PEERTC1PRESENT 0x2066
= > > +#define DL_PEERTC1RXINITCREVAL 0x2067
= > > +
= > > +/*
= > > + * Network Layer Attributes
= > > + */
= > > +#define N_DEVICEID 0x3000
= > > +#define N_DEVICEID_VALID 0x3001
= > > +#define N_TC0TXMAXSDUSIZE 0x3020
= > > +#define N_TC1TXMAXSDUSIZE 0x3021
= > > +
= > > +/*
= > > + * Transport Layer Attributes
= > > + */
= > > +#define T_NUMCPORTS 0x4000
= > > +#define T_NUMTESTFEATURES 0x4001
= > > +#define T_CONNECTIONSTATE 0x4020
= > > +#define T_PEERDEVICEID 0x4021
= > > +#define T_PEERCPORTID 0x4022
= > > +#define T_TRAFFICCLASS 0x4023
= > > +#define T_PROTOCOLID 0x4024
= > > +#define T_CPORTFLAGS 0x4025
= > > +#define T_TXTOKENVALUE 0x4026
= > > +#define T_RXTOKENVALUE 0x4027
= > > +#define T_LOCALBUFFERSPACE 0x4028
= > > +#define T_PEERBUFFERSPACE 0x4029
= > > +#define T_CREDITSTOSEND 0x402A
= > > +#define T_CPORTMODE 0x402B
= > > +#define T_TC0TXMAXSDUSIZE 0x4060
= > > +#define T_TC1TXMAXSDUSIZE 0x4061
= > > +
= > > +#endif /* _UNIPRO_H_ */
= >
= > --
= > 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
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v3 5/6] scsi: ufs: add operation for the uic power mode change
2013-07-26 13:46 ` [PATCH 2/7] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
` (12 preceding siblings ...)
2013-08-26 14:40 ` [PATCH v3 4/6] scsi: ufs: add unipro attribute IDs Seungwon Jeon
@ 2013-08-26 14:40 ` Seungwon Jeon
2013-08-27 9:53 ` Subhash Jadavani
2013-08-26 14:41 ` [PATCH v3 6/6] scsi: ufs: configure the attribute for power mode Seungwon Jeon
2013-09-09 11:51 ` [PATCH] scsi: ufs: export the helper functions for vender probe/remove Seungwon Jeon
15 siblings, 1 reply; 72+ messages in thread
From: Seungwon Jeon @ 2013-08-26 14:40 UTC (permalink / raw)
To: linux-scsi
Cc: 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
Setting PA_PWRMode using DME_SET triggers the power mode
change. And then the result will be given by the HCS.UPMCRS.
This operation should be done atomically.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
---
drivers/scsi/ufs/ufshcd.c | 84 ++++++++++++++++++++++++++++++++++++++++++--
drivers/scsi/ufs/ufshcd.h | 3 ++
drivers/scsi/ufs/ufshci.h | 12 ++++++
3 files changed, 95 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 00bfc1a..c67118d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -36,9 +36,11 @@
#include <linux/async.h>
#include "ufshcd.h"
+#include "unipro.h"
#define UFSHCD_ENABLE_INTRS (UTP_TRANSFER_REQ_COMPL |\
UTP_TASK_REQ_COMPL |\
+ UIC_POWER_MODE |\
UFSHCD_ERROR_MASK)
/* UIC command timeout, unit: ms */
#define UIC_CMD_TIMEOUT 500
@@ -520,6 +522,18 @@ static inline bool ufshcd_ready_for_uic_cmd(struct ufs_hba *hba)
}
/**
+ * ufshcd_get_upmcrs - Get the power mode change request status
+ * @hba: Pointer to adapter instance
+ *
+ * This function gets the UPMCRS field of HCS register
+ * Returns value of UPMCRS field
+ */
+static inline u8 ufshcd_get_upmcrs(struct ufs_hba *hba)
+{
+ return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) & 0x7;
+}
+
+/**
* ufshcd_dispatch_uic_cmd - Dispatch UIC commands to unipro layers
* @hba: per adapter instance
* @uic_cmd: UIC command
@@ -1526,6 +1540,64 @@ out:
EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
/**
+ * ufshcd_uic_change_pwr_mode - Perform the UIC power mode chage
+ * using DME_SET primitives.
+ * @hba: per adapter instance
+ * @mode: powr mode value
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode)
+{
+ struct uic_command uic_cmd = {0};
+ struct completion pwr_done;
+ unsigned long flags;
+ u8 status;
+ int ret;
+
+ uic_cmd.command = UIC_CMD_DME_SET;
+ uic_cmd.argument1 = UIC_ARG_MIB(PA_PWRMODE);
+ uic_cmd.argument3 = mode;
+ init_completion(&pwr_done);
+
+ mutex_lock(&hba->uic_cmd_mutex);
+
+ spin_lock_irqsave(hba->host->host_lock, flags);
+ hba->pwr_done = &pwr_done;
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+ ret = __ufshcd_send_uic_cmd(hba, &uic_cmd);
+ if (ret) {
+ dev_err(hba->dev,
+ "pwr mode change with mode 0x%x uic error %d\n",
+ mode, ret);
+ goto out;
+ }
+
+ if (!wait_for_completion_timeout(hba->pwr_done,
+ msecs_to_jiffies(UIC_CMD_TIMEOUT))) {
+ dev_err(hba->dev,
+ "pwr mode change with mode 0x%x completion timeout\n",
+ mode);
+ ret = -ETIMEDOUT;
+ goto out;
+ }
+
+ status = ufshcd_get_upmcrs(hba);
+ if (status != PWR_LOCAL) {
+ dev_err(hba->dev,
+ "pwr mode change failed, host umpcrs:0x%x\n",
+ status);
+ ret = (status != PWR_OK) ? status : -1;
+ }
+out:
+ spin_lock_irqsave(hba->host->host_lock, flags);
+ hba->pwr_done = NULL;
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+ mutex_unlock(&hba->uic_cmd_mutex);
+ return ret;
+}
+
+/**
* ufshcd_complete_dev_init() - checks device readiness
* hba: per-adapter instance
*
@@ -1992,16 +2064,20 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
/**
* ufshcd_uic_cmd_compl - handle completion of uic command
* @hba: per adapter instance
+ * @intr_status: interrupt status generated by the controller
*/
-static void ufshcd_uic_cmd_compl(struct ufs_hba *hba)
+static void ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
{
- if (hba->active_uic_cmd) {
+ if ((intr_status & UIC_COMMAND_COMPL) && hba->active_uic_cmd) {
hba->active_uic_cmd->argument2 |=
ufshcd_get_uic_cmd_result(hba);
hba->active_uic_cmd->argument3 =
ufshcd_get_dme_attr_val(hba);
complete(&hba->active_uic_cmd->done);
}
+
+ if ((intr_status & UIC_POWER_MODE) && hba->pwr_done)
+ complete(hba->pwr_done);
}
/**
@@ -2451,8 +2527,8 @@ static void ufshcd_sl_intr(struct ufs_hba *hba, u32 intr_status)
if (hba->errors)
ufshcd_check_errors(hba);
- if (intr_status & UIC_COMMAND_COMPL)
- ufshcd_uic_cmd_compl(hba);
+ if (intr_status & UFSHCD_UIC_MASK)
+ ufshcd_uic_cmd_compl(hba, intr_status);
if (intr_status & UTP_TASK_REQ_COMPL)
ufshcd_tmc_handler(hba);
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index ab1518d..ecff83d 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -178,6 +178,7 @@ struct ufs_dev_cmd {
* @tm_tag_wq: wait queue for free task management slots
* @tm_condition: condition variable for task management
* @tm_slots_in_use: bit map of task management request slots in use
+ * @pwr_done: completion for power mode change
* @ufshcd_state: UFSHCD states
* @eh_flags: Error handling flags
* @intr_mask: Interrupt Mask Bits
@@ -227,6 +228,8 @@ struct ufs_hba {
unsigned long tm_condition;
unsigned long tm_slots_in_use;
+ struct completion *pwr_done;
+
u32 ufshcd_state;
u32 eh_flags;
u32 intr_mask;
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index 1e1fe26..0475c66 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -124,6 +124,9 @@ enum {
#define CONTROLLER_FATAL_ERROR UFS_BIT(16)
#define SYSTEM_BUS_FATAL_ERROR UFS_BIT(17)
+#define UFSHCD_UIC_MASK (UIC_COMMAND_COMPL |\
+ UIC_POWER_MODE)
+
#define UFSHCD_ERROR_MASK (UIC_ERROR |\
DEVICE_FATAL_ERROR |\
CONTROLLER_FATAL_ERROR |\
@@ -142,6 +145,15 @@ enum {
#define DEVICE_ERROR_INDICATOR UFS_BIT(5)
#define UIC_POWER_MODE_CHANGE_REQ_STATUS_MASK UFS_MASK(0x7, 8)
+enum {
+ PWR_OK = 0x0,
+ PWR_LOCAL = 0x01,
+ PWR_REMOTE = 0x02,
+ PWR_BUSY = 0x03,
+ PWR_ERROR_CAP = 0x04,
+ PWR_FATAL_ERROR = 0x05,
+};
+
/* HCE - Host Controller Enable 34h */
#define CONTROLLER_ENABLE UFS_BIT(0)
#define CONTROLLER_DISABLE 0x0
--
1.7.0.4
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH v3 5/6] scsi: ufs: add operation for the uic power mode change
2013-08-26 14:40 ` [PATCH v3 5/6] scsi: ufs: add operation for the uic power mode change Seungwon Jeon
@ 2013-08-27 9:53 ` Subhash Jadavani
2013-08-27 11:28 ` Seungwon Jeon
0 siblings, 1 reply; 72+ messages in thread
From: Subhash Jadavani @ 2013-08-27 9:53 UTC (permalink / raw)
To: Seungwon Jeon
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
On 8/26/2013 8:10 PM, Seungwon Jeon wrote:
> Setting PA_PWRMode using DME_SET triggers the power mode
> change. And then the result will be given by the HCS.UPMCRS.
> This operation should be done atomically.
>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> ---
> drivers/scsi/ufs/ufshcd.c | 84 ++++++++++++++++++++++++++++++++++++++++++--
> drivers/scsi/ufs/ufshcd.h | 3 ++
> drivers/scsi/ufs/ufshci.h | 12 ++++++
> 3 files changed, 95 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 00bfc1a..c67118d 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -36,9 +36,11 @@
> #include <linux/async.h>
>
> #include "ufshcd.h"
> +#include "unipro.h"
>
> #define UFSHCD_ENABLE_INTRS (UTP_TRANSFER_REQ_COMPL |\
> UTP_TASK_REQ_COMPL |\
> + UIC_POWER_MODE |\
> UFSHCD_ERROR_MASK)
> /* UIC command timeout, unit: ms */
> #define UIC_CMD_TIMEOUT 500
> @@ -520,6 +522,18 @@ static inline bool ufshcd_ready_for_uic_cmd(struct ufs_hba *hba)
> }
>
> /**
> + * ufshcd_get_upmcrs - Get the power mode change request status
> + * @hba: Pointer to adapter instance
> + *
> + * This function gets the UPMCRS field of HCS register
> + * Returns value of UPMCRS field
> + */
> +static inline u8 ufshcd_get_upmcrs(struct ufs_hba *hba)
> +{
> + return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) & 0x7;
> +}
> +
> +/**
> * ufshcd_dispatch_uic_cmd - Dispatch UIC commands to unipro layers
> * @hba: per adapter instance
> * @uic_cmd: UIC command
> @@ -1526,6 +1540,64 @@ out:
> EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
>
> /**
> + * ufshcd_uic_change_pwr_mode - Perform the UIC power mode chage
> + * using DME_SET primitives.
> + * @hba: per adapter instance
> + * @mode: powr mode value
> + *
> + * Returns 0 on success, non-zero value on failure
> + */
> +int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode)
> +{
> + struct uic_command uic_cmd = {0};
> + struct completion pwr_done;
> + unsigned long flags;
> + u8 status;
> + int ret;
> +
> + uic_cmd.command = UIC_CMD_DME_SET;
> + uic_cmd.argument1 = UIC_ARG_MIB(PA_PWRMODE);
> + uic_cmd.argument3 = mode;
> + init_completion(&pwr_done);
> +
> + mutex_lock(&hba->uic_cmd_mutex);
> +
> + spin_lock_irqsave(hba->host->host_lock, flags);
> + hba->pwr_done = &pwr_done;
> + spin_unlock_irqrestore(hba->host->host_lock, flags);
> + ret = __ufshcd_send_uic_cmd(hba, &uic_cmd);
> + if (ret) {
> + dev_err(hba->dev,
> + "pwr mode change with mode 0x%x uic error %d\n",
> + mode, ret);
> + goto out;
> + }
> +
> + if (!wait_for_completion_timeout(hba->pwr_done,
> + msecs_to_jiffies(UIC_CMD_TIMEOUT))) {
> + dev_err(hba->dev,
> + "pwr mode change with mode 0x%x completion timeout\n",
> + mode);
> + ret = -ETIMEDOUT;
> + goto out;
> + }
> +
> + status = ufshcd_get_upmcrs(hba);
I have a doubt on the relation between UIC Power Mode Status (UPMS) bit
& UIC Power Mode Change Request Status (UPMCRS) field.
Once we send the DME_SET(PA_PWRMODE) command, we will get the UIC
command completion interrupt (with IS.UCCS bit set) which just indicate
that the attribute is written but power mode change procedure will
complete only after some delay which is indicated via IS.UPMS status bit
and then we would read the HCS.UPMCRS field to know the status of power
mode change.
This is my exact doubt: Currently UFSHCD driver first clears the IS.UPMS
bit (ufshcd_intr() first clears the current interrupts in
REG_INTERRUPT_STATUS and then calls the ufshcd_sl_intr() handler, and in
your current patch we are anyway not reading the UPMCRS field from
interrupt contex ) and then read the HCS.UPMCRS field. Will HCS.UPMCRS
field retain its status value even after IS.UPMS bit is cleared? UFSHCI
specification doesn't clearly mention this so either we have to get the
clarity from JEDEC or from your ufs host controller vendor on the
behaviour. I am checking with our ufs host controller vendor as well and
will let you know what is the expected behaviour. BTW, i guess you might
have already tested this patch to be working on your UFS host controller?
> + if (status != PWR_LOCAL) {
> + dev_err(hba->dev,
> + "pwr mode change failed, host umpcrs:0x%x\n",
> + status);
> + ret = (status != PWR_OK) ? status : -1;
> + }
> +out:
> + spin_lock_irqsave(hba->host->host_lock, flags);
> + hba->pwr_done = NULL;
> + spin_unlock_irqrestore(hba->host->host_lock, flags);
> + mutex_unlock(&hba->uic_cmd_mutex);
> + return ret;
> +}
> +
> +/**
> * ufshcd_complete_dev_init() - checks device readiness
> * hba: per-adapter instance
> *
> @@ -1992,16 +2064,20 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
> /**
> * ufshcd_uic_cmd_compl - handle completion of uic command
> * @hba: per adapter instance
> + * @intr_status: interrupt status generated by the controller
> */
> -static void ufshcd_uic_cmd_compl(struct ufs_hba *hba)
> +static void ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
> {
> - if (hba->active_uic_cmd) {
> + if ((intr_status & UIC_COMMAND_COMPL) && hba->active_uic_cmd) {
> hba->active_uic_cmd->argument2 |=
> ufshcd_get_uic_cmd_result(hba);
> hba->active_uic_cmd->argument3 =
> ufshcd_get_dme_attr_val(hba);
> complete(&hba->active_uic_cmd->done);
> }
> +
> + if ((intr_status & UIC_POWER_MODE) && hba->pwr_done)
> + complete(hba->pwr_done);
> }
>
> /**
> @@ -2451,8 +2527,8 @@ static void ufshcd_sl_intr(struct ufs_hba *hba, u32 intr_status)
> if (hba->errors)
> ufshcd_check_errors(hba);
>
> - if (intr_status & UIC_COMMAND_COMPL)
> - ufshcd_uic_cmd_compl(hba);
> + if (intr_status & UFSHCD_UIC_MASK)
> + ufshcd_uic_cmd_compl(hba, intr_status);
>
> if (intr_status & UTP_TASK_REQ_COMPL)
> ufshcd_tmc_handler(hba);
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index ab1518d..ecff83d 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -178,6 +178,7 @@ struct ufs_dev_cmd {
> * @tm_tag_wq: wait queue for free task management slots
> * @tm_condition: condition variable for task management
> * @tm_slots_in_use: bit map of task management request slots in use
> + * @pwr_done: completion for power mode change
> * @ufshcd_state: UFSHCD states
> * @eh_flags: Error handling flags
> * @intr_mask: Interrupt Mask Bits
> @@ -227,6 +228,8 @@ struct ufs_hba {
> unsigned long tm_condition;
> unsigned long tm_slots_in_use;
>
> + struct completion *pwr_done;
> +
> u32 ufshcd_state;
> u32 eh_flags;
> u32 intr_mask;
> diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
> index 1e1fe26..0475c66 100644
> --- a/drivers/scsi/ufs/ufshci.h
> +++ b/drivers/scsi/ufs/ufshci.h
> @@ -124,6 +124,9 @@ enum {
> #define CONTROLLER_FATAL_ERROR UFS_BIT(16)
> #define SYSTEM_BUS_FATAL_ERROR UFS_BIT(17)
>
> +#define UFSHCD_UIC_MASK (UIC_COMMAND_COMPL |\
> + UIC_POWER_MODE)
> +
> #define UFSHCD_ERROR_MASK (UIC_ERROR |\
> DEVICE_FATAL_ERROR |\
> CONTROLLER_FATAL_ERROR |\
> @@ -142,6 +145,15 @@ enum {
> #define DEVICE_ERROR_INDICATOR UFS_BIT(5)
> #define UIC_POWER_MODE_CHANGE_REQ_STATUS_MASK UFS_MASK(0x7, 8)
>
> +enum {
> + PWR_OK = 0x0,
> + PWR_LOCAL = 0x01,
> + PWR_REMOTE = 0x02,
> + PWR_BUSY = 0x03,
> + PWR_ERROR_CAP = 0x04,
> + PWR_FATAL_ERROR = 0x05,
> +};
> +
> /* HCE - Host Controller Enable 34h */
> #define CONTROLLER_ENABLE UFS_BIT(0)
> #define CONTROLLER_DISABLE 0x0
^ permalink raw reply [flat|nested] 72+ messages in thread* RE: [PATCH v3 5/6] scsi: ufs: add operation for the uic power mode change
2013-08-27 9:53 ` Subhash Jadavani
@ 2013-08-27 11:28 ` Seungwon Jeon
2013-08-27 11:47 ` Subhash Jadavani
0 siblings, 1 reply; 72+ messages in thread
From: Seungwon Jeon @ 2013-08-27 11:28 UTC (permalink / raw)
To: 'Subhash Jadavani'
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
On Tue, August 27, 2013, Subhash Jadavani wrote:
> On 8/26/2013 8:10 PM, Seungwon Jeon wrote:
> > Setting PA_PWRMode using DME_SET triggers the power mode
> > change. And then the result will be given by the HCS.UPMCRS.
> > This operation should be done atomically.
> >
> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> > ---
> > drivers/scsi/ufs/ufshcd.c | 84 ++++++++++++++++++++++++++++++++++++++++++--
> > drivers/scsi/ufs/ufshcd.h | 3 ++
> > drivers/scsi/ufs/ufshci.h | 12 ++++++
> > 3 files changed, 95 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 00bfc1a..c67118d 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -36,9 +36,11 @@
> > #include <linux/async.h>
> >
> > #include "ufshcd.h"
> > +#include "unipro.h"
> >
> > #define UFSHCD_ENABLE_INTRS (UTP_TRANSFER_REQ_COMPL |\
> > UTP_TASK_REQ_COMPL |\
> > + UIC_POWER_MODE |\
> > UFSHCD_ERROR_MASK)
> > /* UIC command timeout, unit: ms */
> > #define UIC_CMD_TIMEOUT 500
> > @@ -520,6 +522,18 @@ static inline bool ufshcd_ready_for_uic_cmd(struct ufs_hba *hba)
> > }
> >
> > /**
> > + * ufshcd_get_upmcrs - Get the power mode change request status
> > + * @hba: Pointer to adapter instance
> > + *
> > + * This function gets the UPMCRS field of HCS register
> > + * Returns value of UPMCRS field
> > + */
> > +static inline u8 ufshcd_get_upmcrs(struct ufs_hba *hba)
> > +{
> > + return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) & 0x7;
> > +}
> > +
> > +/**
> > * ufshcd_dispatch_uic_cmd - Dispatch UIC commands to unipro layers
> > * @hba: per adapter instance
> > * @uic_cmd: UIC command
> > @@ -1526,6 +1540,64 @@ out:
> > EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
> >
> > /**
> > + * ufshcd_uic_change_pwr_mode - Perform the UIC power mode chage
> > + * using DME_SET primitives.
> > + * @hba: per adapter instance
> > + * @mode: powr mode value
> > + *
> > + * Returns 0 on success, non-zero value on failure
> > + */
> > +int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode)
> > +{
> > + struct uic_command uic_cmd = {0};
> > + struct completion pwr_done;
> > + unsigned long flags;
> > + u8 status;
> > + int ret;
> > +
> > + uic_cmd.command = UIC_CMD_DME_SET;
> > + uic_cmd.argument1 = UIC_ARG_MIB(PA_PWRMODE);
> > + uic_cmd.argument3 = mode;
> > + init_completion(&pwr_done);
> > +
> > + mutex_lock(&hba->uic_cmd_mutex);
> > +
> > + spin_lock_irqsave(hba->host->host_lock, flags);
> > + hba->pwr_done = &pwr_done;
> > + spin_unlock_irqrestore(hba->host->host_lock, flags);
> > + ret = __ufshcd_send_uic_cmd(hba, &uic_cmd);
> > + if (ret) {
> > + dev_err(hba->dev,
> > + "pwr mode change with mode 0x%x uic error %d\n",
> > + mode, ret);
> > + goto out;
> > + }
> > +
> > + if (!wait_for_completion_timeout(hba->pwr_done,
> > + msecs_to_jiffies(UIC_CMD_TIMEOUT))) {
> > + dev_err(hba->dev,
> > + "pwr mode change with mode 0x%x completion timeout\n",
> > + mode);
> > + ret = -ETIMEDOUT;
> > + goto out;
> > + }
> > +
> > + status = ufshcd_get_upmcrs(hba);
>
> I have a doubt on the relation between UIC Power Mode Status (UPMS) bit
> & UIC Power Mode Change Request Status (UPMCRS) field.
>
> Once we send the DME_SET(PA_PWRMODE) command, we will get the UIC
> command completion interrupt (with IS.UCCS bit set) which just indicate
> that the attribute is written but power mode change procedure will
> complete only after some delay which is indicated via IS.UPMS status bit
> and then we would read the HCS.UPMCRS field to know the status of power
> mode change.
>
> This is my exact doubt: Currently UFSHCD driver first clears the IS.UPMS
> bit (ufshcd_intr() first clears the current interrupts in
> REG_INTERRUPT_STATUS and then calls the ufshcd_sl_intr() handler, and in
> your current patch we are anyway not reading the UPMCRS field from
> interrupt contex ) and then read the HCS.UPMCRS field. Will HCS.UPMCRS
> field retain its status value even after IS.UPMS bit is cleared? UFSHCI
> specification doesn't clearly mention this so either we have to get the
> clarity from JEDEC or from your ufs host controller vendor on the
> behaviour. I am checking with our ufs host controller vendor as well and
> will let you know what is the expected behaviour. BTW, i guess you might
> have already tested this patch to be working on your UFS host controller?
Hi Subhash,
Yes, It's already tested and works fine.
IS.UPMS is a indication to inform the result of 'power mode change'(UPMCRS).
So clearing the interrupt would not affect this result.
Generally it seems not common idea.
Anyway, if your host has different behavior, please let me know.
Thanks,
Seungwon Jeon
>
> > + if (status != PWR_LOCAL) {
> > + dev_err(hba->dev,
> > + "pwr mode change failed, host umpcrs:0x%x\n",
> > + status);
> > + ret = (status != PWR_OK) ? status : -1;
> > + }
> > +out:
> > + spin_lock_irqsave(hba->host->host_lock, flags);
> > + hba->pwr_done = NULL;
> > + spin_unlock_irqrestore(hba->host->host_lock, flags);
> > + mutex_unlock(&hba->uic_cmd_mutex);
> > + return ret;
> > +}
> > +
> > +/**
> > * ufshcd_complete_dev_init() - checks device readiness
> > * hba: per-adapter instance
> > *
> > @@ -1992,16 +2064,20 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
> > /**
> > * ufshcd_uic_cmd_compl - handle completion of uic command
> > * @hba: per adapter instance
> > + * @intr_status: interrupt status generated by the controller
> > */
> > -static void ufshcd_uic_cmd_compl(struct ufs_hba *hba)
> > +static void ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
> > {
> > - if (hba->active_uic_cmd) {
> > + if ((intr_status & UIC_COMMAND_COMPL) && hba->active_uic_cmd) {
> > hba->active_uic_cmd->argument2 |=
> > ufshcd_get_uic_cmd_result(hba);
> > hba->active_uic_cmd->argument3 =
> > ufshcd_get_dme_attr_val(hba);
> > complete(&hba->active_uic_cmd->done);
> > }
> > +
> > + if ((intr_status & UIC_POWER_MODE) && hba->pwr_done)
> > + complete(hba->pwr_done);
> > }
> >
> > /**
> > @@ -2451,8 +2527,8 @@ static void ufshcd_sl_intr(struct ufs_hba *hba, u32 intr_status)
> > if (hba->errors)
> > ufshcd_check_errors(hba);
> >
> > - if (intr_status & UIC_COMMAND_COMPL)
> > - ufshcd_uic_cmd_compl(hba);
> > + if (intr_status & UFSHCD_UIC_MASK)
> > + ufshcd_uic_cmd_compl(hba, intr_status);
> >
> > if (intr_status & UTP_TASK_REQ_COMPL)
> > ufshcd_tmc_handler(hba);
> > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> > index ab1518d..ecff83d 100644
> > --- a/drivers/scsi/ufs/ufshcd.h
> > +++ b/drivers/scsi/ufs/ufshcd.h
> > @@ -178,6 +178,7 @@ struct ufs_dev_cmd {
> > * @tm_tag_wq: wait queue for free task management slots
> > * @tm_condition: condition variable for task management
> > * @tm_slots_in_use: bit map of task management request slots in use
> > + * @pwr_done: completion for power mode change
> > * @ufshcd_state: UFSHCD states
> > * @eh_flags: Error handling flags
> > * @intr_mask: Interrupt Mask Bits
> > @@ -227,6 +228,8 @@ struct ufs_hba {
> > unsigned long tm_condition;
> > unsigned long tm_slots_in_use;
> >
> > + struct completion *pwr_done;
> > +
> > u32 ufshcd_state;
> > u32 eh_flags;
> > u32 intr_mask;
> > diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
> > index 1e1fe26..0475c66 100644
> > --- a/drivers/scsi/ufs/ufshci.h
> > +++ b/drivers/scsi/ufs/ufshci.h
> > @@ -124,6 +124,9 @@ enum {
> > #define CONTROLLER_FATAL_ERROR UFS_BIT(16)
> > #define SYSTEM_BUS_FATAL_ERROR UFS_BIT(17)
> >
> > +#define UFSHCD_UIC_MASK (UIC_COMMAND_COMPL |\
> > + UIC_POWER_MODE)
> > +
> > #define UFSHCD_ERROR_MASK (UIC_ERROR |\
> > DEVICE_FATAL_ERROR |\
> > CONTROLLER_FATAL_ERROR |\
> > @@ -142,6 +145,15 @@ enum {
> > #define DEVICE_ERROR_INDICATOR UFS_BIT(5)
> > #define UIC_POWER_MODE_CHANGE_REQ_STATUS_MASK UFS_MASK(0x7, 8)
> >
> > +enum {
> > + PWR_OK = 0x0,
> > + PWR_LOCAL = 0x01,
> > + PWR_REMOTE = 0x02,
> > + PWR_BUSY = 0x03,
> > + PWR_ERROR_CAP = 0x04,
> > + PWR_FATAL_ERROR = 0x05,
> > +};
> > +
> > /* HCE - Host Controller Enable 34h */
> > #define CONTROLLER_ENABLE UFS_BIT(0)
> > #define CONTROLLER_DISABLE 0x0
>
> --
> 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
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v3 5/6] scsi: ufs: add operation for the uic power mode change
2013-08-27 11:28 ` Seungwon Jeon
@ 2013-08-27 11:47 ` Subhash Jadavani
2013-08-27 11:58 ` Seungwon Jeon
0 siblings, 1 reply; 72+ messages in thread
From: Subhash Jadavani @ 2013-08-27 11:47 UTC (permalink / raw)
To: Seungwon Jeon
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
On 8/27/2013 4:58 PM, Seungwon Jeon wrote:
> On Tue, August 27, 2013, Subhash Jadavani wrote:
>> On 8/26/2013 8:10 PM, Seungwon Jeon wrote:
>>> Setting PA_PWRMode using DME_SET triggers the power mode
>>> change. And then the result will be given by the HCS.UPMCRS.
>>> This operation should be done atomically.
>>>
>>> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
>>> ---
>>> drivers/scsi/ufs/ufshcd.c | 84 ++++++++++++++++++++++++++++++++++++++++++--
>>> drivers/scsi/ufs/ufshcd.h | 3 ++
>>> drivers/scsi/ufs/ufshci.h | 12 ++++++
>>> 3 files changed, 95 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>>> index 00bfc1a..c67118d 100644
>>> --- a/drivers/scsi/ufs/ufshcd.c
>>> +++ b/drivers/scsi/ufs/ufshcd.c
>>> @@ -36,9 +36,11 @@
>>> #include <linux/async.h>
>>>
>>> #include "ufshcd.h"
>>> +#include "unipro.h"
>>>
>>> #define UFSHCD_ENABLE_INTRS (UTP_TRANSFER_REQ_COMPL |\
>>> UTP_TASK_REQ_COMPL |\
>>> + UIC_POWER_MODE |\
>>> UFSHCD_ERROR_MASK)
>>> /* UIC command timeout, unit: ms */
>>> #define UIC_CMD_TIMEOUT 500
>>> @@ -520,6 +522,18 @@ static inline bool ufshcd_ready_for_uic_cmd(struct ufs_hba *hba)
>>> }
>>>
>>> /**
>>> + * ufshcd_get_upmcrs - Get the power mode change request status
>>> + * @hba: Pointer to adapter instance
>>> + *
>>> + * This function gets the UPMCRS field of HCS register
>>> + * Returns value of UPMCRS field
>>> + */
>>> +static inline u8 ufshcd_get_upmcrs(struct ufs_hba *hba)
>>> +{
>>> + return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) & 0x7;
>>> +}
>>> +
>>> +/**
>>> * ufshcd_dispatch_uic_cmd - Dispatch UIC commands to unipro layers
>>> * @hba: per adapter instance
>>> * @uic_cmd: UIC command
>>> @@ -1526,6 +1540,64 @@ out:
>>> EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
>>>
>>> /**
>>> + * ufshcd_uic_change_pwr_mode - Perform the UIC power mode chage
>>> + * using DME_SET primitives.
>>> + * @hba: per adapter instance
>>> + * @mode: powr mode value
>>> + *
>>> + * Returns 0 on success, non-zero value on failure
>>> + */
>>> +int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode)
>>> +{
>>> + struct uic_command uic_cmd = {0};
>>> + struct completion pwr_done;
>>> + unsigned long flags;
>>> + u8 status;
>>> + int ret;
>>> +
>>> + uic_cmd.command = UIC_CMD_DME_SET;
>>> + uic_cmd.argument1 = UIC_ARG_MIB(PA_PWRMODE);
>>> + uic_cmd.argument3 = mode;
>>> + init_completion(&pwr_done);
>>> +
>>> + mutex_lock(&hba->uic_cmd_mutex);
>>> +
>>> + spin_lock_irqsave(hba->host->host_lock, flags);
>>> + hba->pwr_done = &pwr_done;
>>> + spin_unlock_irqrestore(hba->host->host_lock, flags);
>>> + ret = __ufshcd_send_uic_cmd(hba, &uic_cmd);
>>> + if (ret) {
>>> + dev_err(hba->dev,
>>> + "pwr mode change with mode 0x%x uic error %d\n",
>>> + mode, ret);
>>> + goto out;
>>> + }
>>> +
>>> + if (!wait_for_completion_timeout(hba->pwr_done,
>>> + msecs_to_jiffies(UIC_CMD_TIMEOUT))) {
>>> + dev_err(hba->dev,
>>> + "pwr mode change with mode 0x%x completion timeout\n",
>>> + mode);
>>> + ret = -ETIMEDOUT;
>>> + goto out;
>>> + }
>>> +
>>> + status = ufshcd_get_upmcrs(hba);
>> I have a doubt on the relation between UIC Power Mode Status (UPMS) bit
>> & UIC Power Mode Change Request Status (UPMCRS) field.
>>
>> Once we send the DME_SET(PA_PWRMODE) command, we will get the UIC
>> command completion interrupt (with IS.UCCS bit set) which just indicate
>> that the attribute is written but power mode change procedure will
>> complete only after some delay which is indicated via IS.UPMS status bit
>> and then we would read the HCS.UPMCRS field to know the status of power
>> mode change.
>>
>> This is my exact doubt: Currently UFSHCD driver first clears the IS.UPMS
>> bit (ufshcd_intr() first clears the current interrupts in
>> REG_INTERRUPT_STATUS and then calls the ufshcd_sl_intr() handler, and in
>> your current patch we are anyway not reading the UPMCRS field from
>> interrupt contex ) and then read the HCS.UPMCRS field. Will HCS.UPMCRS
>> field retain its status value even after IS.UPMS bit is cleared? UFSHCI
>> specification doesn't clearly mention this so either we have to get the
>> clarity from JEDEC or from your ufs host controller vendor on the
>> behaviour. I am checking with our ufs host controller vendor as well and
>> will let you know what is the expected behaviour. BTW, i guess you might
>> have already tested this patch to be working on your UFS host controller?
> Hi Subhash,
>
> Yes, It's already tested and works fine.
> IS.UPMS is a indication to inform the result of 'power mode change'(UPMCRS).
> So clearing the interrupt would not affect this result.
Thanks for clarifying.
> Generally it seems not common idea.
Infact on SDHCi controller, there is a case where "auto-cmd error
status" register fields are only valid when the "auto-cmd error"
interrupt bit is set in interrupt status register. Clearing status bit
also clears the "auto-cmd error status" register fields automatically.
So i was in doubt whether similar could be the case here or not.
> Anyway, if your host has different behavior, please let me know.
Sure, checking on this. Would let you know soon. BTW, UFSHCI
specification doesn't mention it clearly so it could be implementation
dependent as well and if it is the case, we might have to change it
later to accomodate the different implementation. So for now, you have
my Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
>
> Thanks,
> Seungwon Jeon
>
>>> + if (status != PWR_LOCAL) {
>>> + dev_err(hba->dev,
>>> + "pwr mode change failed, host umpcrs:0x%x\n",
>>> + status);
>>> + ret = (status != PWR_OK) ? status : -1;
>>> + }
>>> +out:
>>> + spin_lock_irqsave(hba->host->host_lock, flags);
>>> + hba->pwr_done = NULL;
>>> + spin_unlock_irqrestore(hba->host->host_lock, flags);
>>> + mutex_unlock(&hba->uic_cmd_mutex);
>>> + return ret;
>>> +}
>>> +
>>> +/**
>>> * ufshcd_complete_dev_init() - checks device readiness
>>> * hba: per-adapter instance
>>> *
>>> @@ -1992,16 +2064,20 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
>>> /**
>>> * ufshcd_uic_cmd_compl - handle completion of uic command
>>> * @hba: per adapter instance
>>> + * @intr_status: interrupt status generated by the controller
>>> */
>>> -static void ufshcd_uic_cmd_compl(struct ufs_hba *hba)
>>> +static void ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
>>> {
>>> - if (hba->active_uic_cmd) {
>>> + if ((intr_status & UIC_COMMAND_COMPL) && hba->active_uic_cmd) {
>>> hba->active_uic_cmd->argument2 |=
>>> ufshcd_get_uic_cmd_result(hba);
>>> hba->active_uic_cmd->argument3 =
>>> ufshcd_get_dme_attr_val(hba);
>>> complete(&hba->active_uic_cmd->done);
>>> }
>>> +
>>> + if ((intr_status & UIC_POWER_MODE) && hba->pwr_done)
>>> + complete(hba->pwr_done);
>>> }
>>>
>>> /**
>>> @@ -2451,8 +2527,8 @@ static void ufshcd_sl_intr(struct ufs_hba *hba, u32 intr_status)
>>> if (hba->errors)
>>> ufshcd_check_errors(hba);
>>>
>>> - if (intr_status & UIC_COMMAND_COMPL)
>>> - ufshcd_uic_cmd_compl(hba);
>>> + if (intr_status & UFSHCD_UIC_MASK)
>>> + ufshcd_uic_cmd_compl(hba, intr_status);
>>>
>>> if (intr_status & UTP_TASK_REQ_COMPL)
>>> ufshcd_tmc_handler(hba);
>>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
>>> index ab1518d..ecff83d 100644
>>> --- a/drivers/scsi/ufs/ufshcd.h
>>> +++ b/drivers/scsi/ufs/ufshcd.h
>>> @@ -178,6 +178,7 @@ struct ufs_dev_cmd {
>>> * @tm_tag_wq: wait queue for free task management slots
>>> * @tm_condition: condition variable for task management
>>> * @tm_slots_in_use: bit map of task management request slots in use
>>> + * @pwr_done: completion for power mode change
>>> * @ufshcd_state: UFSHCD states
>>> * @eh_flags: Error handling flags
>>> * @intr_mask: Interrupt Mask Bits
>>> @@ -227,6 +228,8 @@ struct ufs_hba {
>>> unsigned long tm_condition;
>>> unsigned long tm_slots_in_use;
>>>
>>> + struct completion *pwr_done;
>>> +
>>> u32 ufshcd_state;
>>> u32 eh_flags;
>>> u32 intr_mask;
>>> diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
>>> index 1e1fe26..0475c66 100644
>>> --- a/drivers/scsi/ufs/ufshci.h
>>> +++ b/drivers/scsi/ufs/ufshci.h
>>> @@ -124,6 +124,9 @@ enum {
>>> #define CONTROLLER_FATAL_ERROR UFS_BIT(16)
>>> #define SYSTEM_BUS_FATAL_ERROR UFS_BIT(17)
>>>
>>> +#define UFSHCD_UIC_MASK (UIC_COMMAND_COMPL |\
>>> + UIC_POWER_MODE)
>>> +
>>> #define UFSHCD_ERROR_MASK (UIC_ERROR |\
>>> DEVICE_FATAL_ERROR |\
>>> CONTROLLER_FATAL_ERROR |\
>>> @@ -142,6 +145,15 @@ enum {
>>> #define DEVICE_ERROR_INDICATOR UFS_BIT(5)
>>> #define UIC_POWER_MODE_CHANGE_REQ_STATUS_MASK UFS_MASK(0x7, 8)
>>>
>>> +enum {
>>> + PWR_OK = 0x0,
>>> + PWR_LOCAL = 0x01,
>>> + PWR_REMOTE = 0x02,
>>> + PWR_BUSY = 0x03,
>>> + PWR_ERROR_CAP = 0x04,
>>> + PWR_FATAL_ERROR = 0x05,
>>> +};
>>> +
>>> /* HCE - Host Controller Enable 34h */
>>> #define CONTROLLER_ENABLE UFS_BIT(0)
>>> #define CONTROLLER_DISABLE 0x0
>> --
>> 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-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 72+ messages in thread* RE: [PATCH v3 5/6] scsi: ufs: add operation for the uic power mode change
2013-08-27 11:47 ` Subhash Jadavani
@ 2013-08-27 11:58 ` Seungwon Jeon
2013-08-28 12:45 ` Yaniv Gardi
0 siblings, 1 reply; 72+ messages in thread
From: Seungwon Jeon @ 2013-08-27 11:58 UTC (permalink / raw)
To: 'Subhash Jadavani'
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
On Tue, August 27, 2013, Subhash Jadavani wrote:
> On 8/27/2013 4:58 PM, Seungwon Jeon wrote:
> > On Tue, August 27, 2013, Subhash Jadavani wrote:
> >> On 8/26/2013 8:10 PM, Seungwon Jeon wrote:
> >>> Setting PA_PWRMode using DME_SET triggers the power mode
> >>> change. And then the result will be given by the HCS.UPMCRS.
> >>> This operation should be done atomically.
> >>>
> >>> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> >>> ---
> >>> drivers/scsi/ufs/ufshcd.c | 84 ++++++++++++++++++++++++++++++++++++++++++--
> >>> drivers/scsi/ufs/ufshcd.h | 3 ++
> >>> drivers/scsi/ufs/ufshci.h | 12 ++++++
> >>> 3 files changed, 95 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> >>> index 00bfc1a..c67118d 100644
> >>> --- a/drivers/scsi/ufs/ufshcd.c
> >>> +++ b/drivers/scsi/ufs/ufshcd.c
> >>> @@ -36,9 +36,11 @@
> >>> #include <linux/async.h>
> >>>
> >>> #include "ufshcd.h"
> >>> +#include "unipro.h"
> >>>
> >>> #define UFSHCD_ENABLE_INTRS (UTP_TRANSFER_REQ_COMPL |\
> >>> UTP_TASK_REQ_COMPL |\
> >>> + UIC_POWER_MODE |\
> >>> UFSHCD_ERROR_MASK)
> >>> /* UIC command timeout, unit: ms */
> >>> #define UIC_CMD_TIMEOUT 500
> >>> @@ -520,6 +522,18 @@ static inline bool ufshcd_ready_for_uic_cmd(struct ufs_hba *hba)
> >>> }
> >>>
> >>> /**
> >>> + * ufshcd_get_upmcrs - Get the power mode change request status
> >>> + * @hba: Pointer to adapter instance
> >>> + *
> >>> + * This function gets the UPMCRS field of HCS register
> >>> + * Returns value of UPMCRS field
> >>> + */
> >>> +static inline u8 ufshcd_get_upmcrs(struct ufs_hba *hba)
> >>> +{
> >>> + return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) & 0x7;
> >>> +}
> >>> +
> >>> +/**
> >>> * ufshcd_dispatch_uic_cmd - Dispatch UIC commands to unipro layers
> >>> * @hba: per adapter instance
> >>> * @uic_cmd: UIC command
> >>> @@ -1526,6 +1540,64 @@ out:
> >>> EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
> >>>
> >>> /**
> >>> + * ufshcd_uic_change_pwr_mode - Perform the UIC power mode chage
> >>> + * using DME_SET primitives.
> >>> + * @hba: per adapter instance
> >>> + * @mode: powr mode value
> >>> + *
> >>> + * Returns 0 on success, non-zero value on failure
> >>> + */
> >>> +int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode)
> >>> +{
> >>> + struct uic_command uic_cmd = {0};
> >>> + struct completion pwr_done;
> >>> + unsigned long flags;
> >>> + u8 status;
> >>> + int ret;
> >>> +
> >>> + uic_cmd.command = UIC_CMD_DME_SET;
> >>> + uic_cmd.argument1 = UIC_ARG_MIB(PA_PWRMODE);
> >>> + uic_cmd.argument3 = mode;
> >>> + init_completion(&pwr_done);
> >>> +
> >>> + mutex_lock(&hba->uic_cmd_mutex);
> >>> +
> >>> + spin_lock_irqsave(hba->host->host_lock, flags);
> >>> + hba->pwr_done = &pwr_done;
> >>> + spin_unlock_irqrestore(hba->host->host_lock, flags);
> >>> + ret = __ufshcd_send_uic_cmd(hba, &uic_cmd);
> >>> + if (ret) {
> >>> + dev_err(hba->dev,
> >>> + "pwr mode change with mode 0x%x uic error %d\n",
> >>> + mode, ret);
> >>> + goto out;
> >>> + }
> >>> +
> >>> + if (!wait_for_completion_timeout(hba->pwr_done,
> >>> + msecs_to_jiffies(UIC_CMD_TIMEOUT))) {
> >>> + dev_err(hba->dev,
> >>> + "pwr mode change with mode 0x%x completion timeout\n",
> >>> + mode);
> >>> + ret = -ETIMEDOUT;
> >>> + goto out;
> >>> + }
> >>> +
> >>> + status = ufshcd_get_upmcrs(hba);
> >> I have a doubt on the relation between UIC Power Mode Status (UPMS) bit
> >> & UIC Power Mode Change Request Status (UPMCRS) field.
> >>
> >> Once we send the DME_SET(PA_PWRMODE) command, we will get the UIC
> >> command completion interrupt (with IS.UCCS bit set) which just indicate
> >> that the attribute is written but power mode change procedure will
> >> complete only after some delay which is indicated via IS.UPMS status bit
> >> and then we would read the HCS.UPMCRS field to know the status of power
> >> mode change.
> >>
> >> This is my exact doubt: Currently UFSHCD driver first clears the IS.UPMS
> >> bit (ufshcd_intr() first clears the current interrupts in
> >> REG_INTERRUPT_STATUS and then calls the ufshcd_sl_intr() handler, and in
> >> your current patch we are anyway not reading the UPMCRS field from
> >> interrupt contex ) and then read the HCS.UPMCRS field. Will HCS.UPMCRS
> >> field retain its status value even after IS.UPMS bit is cleared? UFSHCI
> >> specification doesn't clearly mention this so either we have to get the
> >> clarity from JEDEC or from your ufs host controller vendor on the
> >> behaviour. I am checking with our ufs host controller vendor as well and
> >> will let you know what is the expected behaviour. BTW, i guess you might
> >> have already tested this patch to be working on your UFS host controller?
> > Hi Subhash,
> >
> > Yes, It's already tested and works fine.
> > IS.UPMS is a indication to inform the result of 'power mode change'(UPMCRS).
> > So clearing the interrupt would not affect this result.
> Thanks for clarifying.
>
> > Generally it seems not common idea.
>
> Infact on SDHCi controller, there is a case where "auto-cmd error
> status" register fields are only valid when the "auto-cmd error"
> interrupt bit is set in interrupt status register. Clearing status bit
> also clears the "auto-cmd error status" register fields automatically.
> So i was in doubt whether similar could be the case here or not.
It's interesting thing to me.
>
> > Anyway, if your host has different behavior, please let me know.
>
> Sure, checking on this. Would let you know soon. BTW, UFSHCI
> specification doesn't mention it clearly so it could be implementation
> dependent as well and if it is the case, we might have to change it
> later to accomodate the different implementation. So for now, you have
> my Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
Thank you for review.
Seungwon Jeon
>
> >
> > Thanks,
> > Seungwon Jeon
> >
> >>> + if (status != PWR_LOCAL) {
> >>> + dev_err(hba->dev,
> >>> + "pwr mode change failed, host umpcrs:0x%x\n",
> >>> + status);
> >>> + ret = (status != PWR_OK) ? status : -1;
> >>> + }
> >>> +out:
> >>> + spin_lock_irqsave(hba->host->host_lock, flags);
> >>> + hba->pwr_done = NULL;
> >>> + spin_unlock_irqrestore(hba->host->host_lock, flags);
> >>> + mutex_unlock(&hba->uic_cmd_mutex);
> >>> + return ret;
> >>> +}
> >>> +
> >>> +/**
> >>> * ufshcd_complete_dev_init() - checks device readiness
> >>> * hba: per-adapter instance
> >>> *
> >>> @@ -1992,16 +2064,20 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
> >>> /**
> >>> * ufshcd_uic_cmd_compl - handle completion of uic command
> >>> * @hba: per adapter instance
> >>> + * @intr_status: interrupt status generated by the controller
> >>> */
> >>> -static void ufshcd_uic_cmd_compl(struct ufs_hba *hba)
> >>> +static void ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
> >>> {
> >>> - if (hba->active_uic_cmd) {
> >>> + if ((intr_status & UIC_COMMAND_COMPL) && hba->active_uic_cmd) {
> >>> hba->active_uic_cmd->argument2 |=
> >>> ufshcd_get_uic_cmd_result(hba);
> >>> hba->active_uic_cmd->argument3 =
> >>> ufshcd_get_dme_attr_val(hba);
> >>> complete(&hba->active_uic_cmd->done);
> >>> }
> >>> +
> >>> + if ((intr_status & UIC_POWER_MODE) && hba->pwr_done)
> >>> + complete(hba->pwr_done);
> >>> }
> >>>
> >>> /**
> >>> @@ -2451,8 +2527,8 @@ static void ufshcd_sl_intr(struct ufs_hba *hba, u32 intr_status)
> >>> if (hba->errors)
> >>> ufshcd_check_errors(hba);
> >>>
> >>> - if (intr_status & UIC_COMMAND_COMPL)
> >>> - ufshcd_uic_cmd_compl(hba);
> >>> + if (intr_status & UFSHCD_UIC_MASK)
> >>> + ufshcd_uic_cmd_compl(hba, intr_status);
> >>>
> >>> if (intr_status & UTP_TASK_REQ_COMPL)
> >>> ufshcd_tmc_handler(hba);
> >>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> >>> index ab1518d..ecff83d 100644
> >>> --- a/drivers/scsi/ufs/ufshcd.h
> >>> +++ b/drivers/scsi/ufs/ufshcd.h
> >>> @@ -178,6 +178,7 @@ struct ufs_dev_cmd {
> >>> * @tm_tag_wq: wait queue for free task management slots
> >>> * @tm_condition: condition variable for task management
> >>> * @tm_slots_in_use: bit map of task management request slots in use
> >>> + * @pwr_done: completion for power mode change
> >>> * @ufshcd_state: UFSHCD states
> >>> * @eh_flags: Error handling flags
> >>> * @intr_mask: Interrupt Mask Bits
> >>> @@ -227,6 +228,8 @@ struct ufs_hba {
> >>> unsigned long tm_condition;
> >>> unsigned long tm_slots_in_use;
> >>>
> >>> + struct completion *pwr_done;
> >>> +
> >>> u32 ufshcd_state;
> >>> u32 eh_flags;
> >>> u32 intr_mask;
> >>> diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
> >>> index 1e1fe26..0475c66 100644
> >>> --- a/drivers/scsi/ufs/ufshci.h
> >>> +++ b/drivers/scsi/ufs/ufshci.h
> >>> @@ -124,6 +124,9 @@ enum {
> >>> #define CONTROLLER_FATAL_ERROR UFS_BIT(16)
> >>> #define SYSTEM_BUS_FATAL_ERROR UFS_BIT(17)
> >>>
> >>> +#define UFSHCD_UIC_MASK (UIC_COMMAND_COMPL |\
> >>> + UIC_POWER_MODE)
> >>> +
> >>> #define UFSHCD_ERROR_MASK (UIC_ERROR |\
> >>> DEVICE_FATAL_ERROR |\
> >>> CONTROLLER_FATAL_ERROR |\
> >>> @@ -142,6 +145,15 @@ enum {
> >>> #define DEVICE_ERROR_INDICATOR UFS_BIT(5)
> >>> #define UIC_POWER_MODE_CHANGE_REQ_STATUS_MASK UFS_MASK(0x7, 8)
> >>>
> >>> +enum {
> >>> + PWR_OK = 0x0,
> >>> + PWR_LOCAL = 0x01,
> >>> + PWR_REMOTE = 0x02,
> >>> + PWR_BUSY = 0x03,
> >>> + PWR_ERROR_CAP = 0x04,
> >>> + PWR_FATAL_ERROR = 0x05,
> >>> +};
> >>> +
> >>> /* HCE - Host Controller Enable 34h */
> >>> #define CONTROLLER_ENABLE UFS_BIT(0)
> >>> #define CONTROLLER_DISABLE 0x0
> >> --
> >> 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-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-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 72+ messages in thread* RE: [PATCH v3 5/6] scsi: ufs: add operation for the uic power mode change
2013-08-27 11:58 ` Seungwon Jeon
@ 2013-08-28 12:45 ` Yaniv Gardi
0 siblings, 0 replies; 72+ messages in thread
From: Yaniv Gardi @ 2013-08-28 12:45 UTC (permalink / raw)
To: 'Seungwon Jeon', 'Subhash Jadavani'
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
Tested-by: Yaniv Gardi <ygardi@codeaurora.org>
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
= > -----Original Message-----
= > From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
= > owner@vger.kernel.org] On Behalf Of Seungwon Jeon
= > Sent: Tuesday, August 27, 2013 2:58 PM
= > To: 'Subhash Jadavani'
= > Cc: linux-scsi@vger.kernel.org; 'Vinayak Holikatti'; 'Santosh Y'; 'James E.J.
= > Bottomley'
= > Subject: RE: [PATCH v3 5/6] scsi: ufs: add operation for the uic power
= > mode change
= >
= > On Tue, August 27, 2013, Subhash Jadavani wrote:
= > > On 8/27/2013 4:58 PM, Seungwon Jeon wrote:
= > > > On Tue, August 27, 2013, Subhash Jadavani wrote:
= > > >> On 8/26/2013 8:10 PM, Seungwon Jeon wrote:
= > > >>> Setting PA_PWRMode using DME_SET triggers the power mode
= > change.
= > > >>> And then the result will be given by the HCS.UPMCRS.
= > > >>> This operation should be done atomically.
= > > >>>
= > > >>> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
= > > >>> ---
= > > >>> drivers/scsi/ufs/ufshcd.c | 84
= > ++++++++++++++++++++++++++++++++++++++++++--
= > > >>> drivers/scsi/ufs/ufshcd.h | 3 ++
= > > >>> drivers/scsi/ufs/ufshci.h | 12 ++++++
= > > >>> 3 files changed, 95 insertions(+), 4 deletions(-)
= > > >>>
= > > >>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
= > > >>> index 00bfc1a..c67118d 100644
= > > >>> --- a/drivers/scsi/ufs/ufshcd.c
= > > >>> +++ b/drivers/scsi/ufs/ufshcd.c
= > > >>> @@ -36,9 +36,11 @@
= > > >>> #include <linux/async.h>
= > > >>>
= > > >>> #include "ufshcd.h"
= > > >>> +#include "unipro.h"
= > > >>>
= > > >>> #define UFSHCD_ENABLE_INTRS
= > (UTP_TRANSFER_REQ_COMPL |\
= > > >>> UTP_TASK_REQ_COMPL |\
= > > >>> + UIC_POWER_MODE |\
= > > >>> UFSHCD_ERROR_MASK)
= > > >>> /* UIC command timeout, unit: ms */
= > > >>> #define UIC_CMD_TIMEOUT 500
= > > >>> @@ -520,6 +522,18 @@ static inline bool
= > ufshcd_ready_for_uic_cmd(struct ufs_hba *hba)
= > > >>> }
= > > >>>
= > > >>> /**
= > > >>> + * ufshcd_get_upmcrs - Get the power mode change request status
= > > >>> + * @hba: Pointer to adapter instance
= > > >>> + *
= > > >>> + * This function gets the UPMCRS field of HCS register
= > > >>> + * Returns value of UPMCRS field
= > > >>> + */
= > > >>> +static inline u8 ufshcd_get_upmcrs(struct ufs_hba *hba) {
= > > >>> + return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) & 0x7;
= > }
= > > >>> +
= > > >>> +/**
= > > >>> * ufshcd_dispatch_uic_cmd - Dispatch UIC commands to unipro
= > layers
= > > >>> * @hba: per adapter instance
= > > >>> * @uic_cmd: UIC command
= > > >>> @@ -1526,6 +1540,64 @@ out:
= > > >>> EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
= > > >>>
= > > >>> /**
= > > >>> + * ufshcd_uic_change_pwr_mode - Perform the UIC power mode
= > chage
= > > >>> + * using DME_SET primitives.
= > > >>> + * @hba: per adapter instance
= > > >>> + * @mode: powr mode value
= > > >>> + *
= > > >>> + * Returns 0 on success, non-zero value on failure */ int
= > > >>> +ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode) {
= > > >>> + struct uic_command uic_cmd = {0};
= > > >>> + struct completion pwr_done;
= > > >>> + unsigned long flags;
= > > >>> + u8 status;
= > > >>> + int ret;
= > > >>> +
= > > >>> + uic_cmd.command = UIC_CMD_DME_SET;
= > > >>> + uic_cmd.argument1 = UIC_ARG_MIB(PA_PWRMODE);
= > > >>> + uic_cmd.argument3 = mode;
= > > >>> + init_completion(&pwr_done);
= > > >>> +
= > > >>> + mutex_lock(&hba->uic_cmd_mutex);
= > > >>> +
= > > >>> + spin_lock_irqsave(hba->host->host_lock, flags);
= > > >>> + hba->pwr_done = &pwr_done;
= > > >>> + spin_unlock_irqrestore(hba->host->host_lock, flags);
= > > >>> + ret = __ufshcd_send_uic_cmd(hba, &uic_cmd);
= > > >>> + if (ret) {
= > > >>> + dev_err(hba->dev,
= > > >>> + "pwr mode change with mode 0x%x uic error
= > %d\n",
= > > >>> + mode, ret);
= > > >>> + goto out;
= > > >>> + }
= > > >>> +
= > > >>> + if (!wait_for_completion_timeout(hba->pwr_done,
= > > >>> +
= > msecs_to_jiffies(UIC_CMD_TIMEOUT))) {
= > > >>> + dev_err(hba->dev,
= > > >>> + "pwr mode change with mode 0x%x completion
= > timeout\n",
= > > >>> + mode);
= > > >>> + ret = -ETIMEDOUT;
= > > >>> + goto out;
= > > >>> + }
= > > >>> +
= > > >>> + status = ufshcd_get_upmcrs(hba);
= > > >> I have a doubt on the relation between UIC Power Mode Status
= > (UPMS)
= > > >> bit & UIC Power Mode Change Request Status (UPMCRS) field.
= > > >>
= > > >> Once we send the DME_SET(PA_PWRMODE) command, we will get
= > the UIC
= > > >> command completion interrupt (with IS.UCCS bit set) which just
= > > >> indicate that the attribute is written but power mode change
= > > >> procedure will complete only after some delay which is indicated
= > > >> via IS.UPMS status bit and then we would read the HCS.UPMCRS field
= > > >> to know the status of power mode change.
= > > >>
= > > >> This is my exact doubt: Currently UFSHCD driver first clears the
= > > >> IS.UPMS bit (ufshcd_intr() first clears the current interrupts in
= > > >> REG_INTERRUPT_STATUS and then calls the ufshcd_sl_intr() handler,
= > > >> and in your current patch we are anyway not reading the UPMCRS
= > > >> field from interrupt contex ) and then read the HCS.UPMCRS field.
= > > >> Will HCS.UPMCRS field retain its status value even after IS.UPMS
= > > >> bit is cleared? UFSHCI specification doesn't clearly mention this
= > > >> so either we have to get the clarity from JEDEC or from your ufs
= > > >> host controller vendor on the behaviour. I am checking with our ufs
= > > >> host controller vendor as well and will let you know what is the
= > > >> expected behaviour. BTW, i guess you might have already tested this
= > patch to be working on your UFS host controller?
= > > > Hi Subhash,
= > > >
= > > > Yes, It's already tested and works fine.
= > > > IS.UPMS is a indication to inform the result of 'power mode
= > change'(UPMCRS).
= > > > So clearing the interrupt would not affect this result.
= > > Thanks for clarifying.
= > >
= > > > Generally it seems not common idea.
= > >
= > > Infact on SDHCi controller, there is a case where "auto-cmd error
= > > status" register fields are only valid when the "auto-cmd error"
= > > interrupt bit is set in interrupt status register. Clearing status bit
= > > also clears the "auto-cmd error status" register fields automatically.
= > > So i was in doubt whether similar could be the case here or not.
= >
= > It's interesting thing to me.
= >
= > >
= > > > Anyway, if your host has different behavior, please let me know.
= > >
= > > Sure, checking on this. Would let you know soon. BTW, UFSHCI
= > > specification doesn't mention it clearly so it could be implementation
= > > dependent as well and if it is the case, we might have to change it
= > > later to accomodate the different implementation. So for now, you have
= > > my Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
= >
= > Thank you for review.
= >
= > Seungwon Jeon
= >
= > >
= > > >
= > > > Thanks,
= > > > Seungwon Jeon
= > > >
= > > >>> + if (status != PWR_LOCAL) {
= > > >>> + dev_err(hba->dev,
= > > >>> + "pwr mode change failed, host umpcrs:0x%x\n",
= > > >>> + status);
= > > >>> + ret = (status != PWR_OK) ? status : -1;
= > > >>> + }
= > > >>> +out:
= > > >>> + spin_lock_irqsave(hba->host->host_lock, flags);
= > > >>> + hba->pwr_done = NULL;
= > > >>> + spin_unlock_irqrestore(hba->host->host_lock, flags);
= > > >>> + mutex_unlock(&hba->uic_cmd_mutex);
= > > >>> + return ret;
= > > >>> +}
= > > >>> +
= > > >>> +/**
= > > >>> * ufshcd_complete_dev_init() - checks device readiness
= > > >>> * hba: per-adapter instance
= > > >>> *
= > > >>> @@ -1992,16 +2064,20 @@ ufshcd_transfer_rsp_status(struct
= > ufs_hba *hba, struct ufshcd_lrb *lrbp)
= > > >>> /**
= > > >>> * ufshcd_uic_cmd_compl - handle completion of uic command
= > > >>> * @hba: per adapter instance
= > > >>> + * @intr_status: interrupt status generated by the controller
= > > >>> */
= > > >>> -static void ufshcd_uic_cmd_compl(struct ufs_hba *hba)
= > > >>> +static void ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32
= > > >>> +intr_status)
= > > >>> {
= > > >>> - if (hba->active_uic_cmd) {
= > > >>> + if ((intr_status & UIC_COMMAND_COMPL) && hba-
= > >active_uic_cmd) {
= > > >>> hba->active_uic_cmd->argument2 |=
= > > >>> ufshcd_get_uic_cmd_result(hba);
= > > >>> hba->active_uic_cmd->argument3 =
= > > >>> ufshcd_get_dme_attr_val(hba);
= > > >>> complete(&hba->active_uic_cmd->done);
= > > >>> }
= > > >>> +
= > > >>> + if ((intr_status & UIC_POWER_MODE) && hba->pwr_done)
= > > >>> + complete(hba->pwr_done);
= > > >>> }
= > > >>>
= > > >>> /**
= > > >>> @@ -2451,8 +2527,8 @@ static void ufshcd_sl_intr(struct ufs_hba
= > *hba, u32 intr_status)
= > > >>> if (hba->errors)
= > > >>> ufshcd_check_errors(hba);
= > > >>>
= > > >>> - if (intr_status & UIC_COMMAND_COMPL)
= > > >>> - ufshcd_uic_cmd_compl(hba);
= > > >>> + if (intr_status & UFSHCD_UIC_MASK)
= > > >>> + ufshcd_uic_cmd_compl(hba, intr_status);
= > > >>>
= > > >>> if (intr_status & UTP_TASK_REQ_COMPL)
= > > >>> ufshcd_tmc_handler(hba);
= > > >>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
= > > >>> index ab1518d..ecff83d 100644
= > > >>> --- a/drivers/scsi/ufs/ufshcd.h
= > > >>> +++ b/drivers/scsi/ufs/ufshcd.h
= > > >>> @@ -178,6 +178,7 @@ struct ufs_dev_cmd {
= > > >>> * @tm_tag_wq: wait queue for free task management slots
= > > >>> * @tm_condition: condition variable for task management
= > > >>> * @tm_slots_in_use: bit map of task management request slots
= > > >>> in use
= > > >>> + * @pwr_done: completion for power mode change
= > > >>> * @ufshcd_state: UFSHCD states
= > > >>> * @eh_flags: Error handling flags
= > > >>> * @intr_mask: Interrupt Mask Bits @@ -227,6 +228,8 @@ struct
= > > >>> ufs_hba {
= > > >>> unsigned long tm_condition;
= > > >>> unsigned long tm_slots_in_use;
= > > >>>
= > > >>> + struct completion *pwr_done;
= > > >>> +
= > > >>> u32 ufshcd_state;
= > > >>> u32 eh_flags;
= > > >>> u32 intr_mask;
= > > >>> diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
= > > >>> index 1e1fe26..0475c66 100644
= > > >>> --- a/drivers/scsi/ufs/ufshci.h
= > > >>> +++ b/drivers/scsi/ufs/ufshci.h
= > > >>> @@ -124,6 +124,9 @@ enum {
= > > >>> #define CONTROLLER_FATAL_ERROR
= > UFS_BIT(16)
= > > >>> #define SYSTEM_BUS_FATAL_ERROR
= > UFS_BIT(17)
= > > >>>
= > > >>> +#define UFSHCD_UIC_MASK (UIC_COMMAND_COMPL
= > |\
= > > >>> + UIC_POWER_MODE)
= > > >>> +
= > > >>> #define UFSHCD_ERROR_MASK (UIC_ERROR |\
= > > >>> DEVICE_FATAL_ERROR |\
= > > >>> CONTROLLER_FATAL_ERROR |\
= > > >>> @@ -142,6 +145,15 @@ enum {
= > > >>> #define DEVICE_ERROR_INDICATOR UFS_BIT(5)
= > > >>> #define UIC_POWER_MODE_CHANGE_REQ_STATUS_MASK
= > UFS_MASK(0x7, 8)
= > > >>>
= > > >>> +enum {
= > > >>> + PWR_OK = 0x0,
= > > >>> + PWR_LOCAL = 0x01,
= > > >>> + PWR_REMOTE = 0x02,
= > > >>> + PWR_BUSY = 0x03,
= > > >>> + PWR_ERROR_CAP = 0x04,
= > > >>> + PWR_FATAL_ERROR = 0x05,
= > > >>> +};
= > > >>> +
= > > >>> /* HCE - Host Controller Enable 34h */
= > > >>> #define CONTROLLER_ENABLE UFS_BIT(0)
= > > >>> #define CONTROLLER_DISABLE 0x0
= > > >> --
= > > >> 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-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-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-scsi" in the
= > body of a message to majordomo@vger.kernel.org More majordomo info
= > at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v3 6/6] scsi: ufs: configure the attribute for power mode
2013-07-26 13:46 ` [PATCH 2/7] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
` (13 preceding siblings ...)
2013-08-26 14:40 ` [PATCH v3 5/6] scsi: ufs: add operation for the uic power mode change Seungwon Jeon
@ 2013-08-26 14:41 ` Seungwon Jeon
2013-08-27 10:21 ` Subhash Jadavani
2013-09-09 11:51 ` [PATCH] scsi: ufs: export the helper functions for vender probe/remove Seungwon Jeon
15 siblings, 1 reply; 72+ messages in thread
From: Seungwon Jeon @ 2013-08-26 14:41 UTC (permalink / raw)
To: linux-scsi
Cc: 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
UIC attributes can be set with using DME_SET command for
power mode change. For configuration the link capability
attributes are used, which is updated after successful
link startup.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
---
drivers/scsi/ufs/ufshcd.c | 66 +++++++++++++++++++++++++++++++++++++++++++++
drivers/scsi/ufs/unipro.h | 21 ++++++++++++++
2 files changed, 87 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c67118d..2ae845c 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1598,6 +1598,70 @@ out:
}
/**
+ * ufshcd_config_max_pwr_mode - Set & Change power mode with
+ * maximum capability attribute information.
+ * @hba: per adapter instance
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+static int ufshcd_config_max_pwr_mode(struct ufs_hba *hba)
+{
+ enum {RX = 0, TX = 1};
+ u32 lanes[] = {1, 1};
+ u32 gear[] = {1, 1};
+ u8 pwr[] = {FASTAUTO_MODE, FASTAUTO_MODE};
+ int ret;
+
+ /* Get the connected lane count */
+ ufshcd_dme_get(hba, UIC_ARG_MIB(PA_CONNECTEDRXDATALANES), &lanes[RX]);
+ ufshcd_dme_get(hba, UIC_ARG_MIB(PA_CONNECTEDTXDATALANES), &lanes[TX]);
+
+ /*
+ * First, get the maximum gears of HS speed.
+ * If a zero value, it means there is no HSGEAR capability.
+ * Then, get the maximum gears of PWM speed.
+ */
+ ufshcd_dme_get(hba, UIC_ARG_MIB(PA_MAXRXHSGEAR), &gear[RX]);
+ if (!gear[RX]) {
+ ufshcd_dme_get(hba, UIC_ARG_MIB(PA_MAXRXPWMGEAR), &gear[RX]);
+ pwr[RX] = SLOWAUTO_MODE;
+ }
+
+ ufshcd_dme_peer_get(hba, UIC_ARG_MIB(PA_MAXRXHSGEAR), &gear[TX]);
+ if (!gear[TX]) {
+ ufshcd_dme_peer_get(hba, UIC_ARG_MIB(PA_MAXRXPWMGEAR),
+ &gear[TX]);
+ pwr[TX] = SLOWAUTO_MODE;
+ }
+
+ /*
+ * Configure attributes for power mode change with below.
+ * - PA_RXGEAR, PA_ACTIVERXDATALANES, PA_RXTERMINATION,
+ * - PA_TXGEAR, PA_ACTIVETXDATALANES, PA_TXTERMINATION,
+ * - PA_HSSERIES
+ */
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_RXGEAR), gear[RX]);
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_ACTIVERXDATALANES), lanes[RX]);
+ if (pwr[RX] == FASTAUTO_MODE)
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_RXTERMINATION), TRUE);
+
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXGEAR), gear[TX]);
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_ACTIVETXDATALANES), lanes[TX]);
+ if (pwr[TX] == FASTAUTO_MODE)
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXTERMINATION), TRUE);
+
+ if (pwr[RX] == FASTAUTO_MODE || pwr[TX] == FASTAUTO_MODE)
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_HSSERIES), PA_HS_MODE_B);
+
+ ret = ufshcd_uic_change_pwr_mode(hba, pwr[RX] << 4 | pwr[TX]);
+ if (ret)
+ dev_err(hba->dev,
+ "pwr_mode: power mode change failed %d\n", ret);
+
+ return ret;
+}
+
+/**
* ufshcd_complete_dev_init() - checks device readiness
* hba: per-adapter instance
*
@@ -2942,6 +3006,8 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
if (ret)
goto out;
+ ufshcd_config_max_pwr_mode(hba);
+
ret = ufshcd_verify_dev_init(hba);
if (ret)
goto out;
diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h
index 3a710eb..0bb8041 100644
--- a/drivers/scsi/ufs/unipro.h
+++ b/drivers/scsi/ufs/unipro.h
@@ -72,6 +72,21 @@
#define PA_STALLNOCONFIGTIME 0x15A3
#define PA_SAVECONFIGTIME 0x15A4
+/* PA power modes */
+enum {
+ FAST_MODE = 1,
+ SLOW_MODE = 2,
+ FASTAUTO_MODE = 4,
+ SLOWAUTO_MODE = 5,
+ UNCHANGED = 7,
+};
+
+/* PA TX/RX Frequency Series */
+enum {
+ PA_HS_MODE_A = 1,
+ PA_HS_MODE_B = 2,
+};
+
/*
* Data Link Layer Attributes
*/
@@ -127,4 +142,10 @@
#define T_TC0TXMAXSDUSIZE 0x4060
#define T_TC1TXMAXSDUSIZE 0x4061
+/* Boolean attribute values */
+enum {
+ FALSE = 0,
+ TRUE,
+};
+
#endif /* _UNIPRO_H_ */
--
1.7.0.4
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH v3 6/6] scsi: ufs: configure the attribute for power mode
2013-08-26 14:41 ` [PATCH v3 6/6] scsi: ufs: configure the attribute for power mode Seungwon Jeon
@ 2013-08-27 10:21 ` Subhash Jadavani
2013-08-27 10:27 ` Subhash Jadavani
0 siblings, 1 reply; 72+ messages in thread
From: Subhash Jadavani @ 2013-08-27 10:21 UTC (permalink / raw)
To: Seungwon Jeon
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
On 8/26/2013 8:11 PM, Seungwon Jeon wrote:
> UIC attributes can be set with using DME_SET command for
> power mode change. For configuration the link capability
> attributes are used, which is updated after successful
> link startup.
>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
> ---
> drivers/scsi/ufs/ufshcd.c | 66 +++++++++++++++++++++++++++++++++++++++++++++
> drivers/scsi/ufs/unipro.h | 21 ++++++++++++++
> 2 files changed, 87 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index c67118d..2ae845c 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -1598,6 +1598,70 @@ out:
> }
>
> /**
> + * ufshcd_config_max_pwr_mode - Set & Change power mode with
> + * maximum capability attribute information.
> + * @hba: per adapter instance
> + *
> + * Returns 0 on success, non-zero value on failure
> + */
> +static int ufshcd_config_max_pwr_mode(struct ufs_hba *hba)
> +{
> + enum {RX = 0, TX = 1};
> + u32 lanes[] = {1, 1};
> + u32 gear[] = {1, 1};
> + u8 pwr[] = {FASTAUTO_MODE, FASTAUTO_MODE};
> + int ret;
> +
> + /* Get the connected lane count */
> + ufshcd_dme_get(hba, UIC_ARG_MIB(PA_CONNECTEDRXDATALANES), &lanes[RX]);
> + ufshcd_dme_get(hba, UIC_ARG_MIB(PA_CONNECTEDTXDATALANES), &lanes[TX]);
> +
> + /*
> + * First, get the maximum gears of HS speed.
> + * If a zero value, it means there is no HSGEAR capability.
> + * Then, get the maximum gears of PWM speed.
> + */
> + ufshcd_dme_get(hba, UIC_ARG_MIB(PA_MAXRXHSGEAR), &gear[RX]);
> + if (!gear[RX]) {
> + ufshcd_dme_get(hba, UIC_ARG_MIB(PA_MAXRXPWMGEAR), &gear[RX]);
> + pwr[RX] = SLOWAUTO_MODE;
> + }
As per UniPro specification, PA_MaxRxHsGear valid values are 0 (NO, HS),
1 (HS_Gear1), 2 (HS_Gear2) & 3 (HS_Gear3). But UFS device specification
(v1.1) only mandates to support Gear-1 and Gear-2 is optional. Gear-3 is
no where mentioned in the specifcation so it means unsupported gear. So
i guess after reading PA_MaxRxHsGear attribute, we have to ensure that
we only set the Gear-1 or Gear-2 as mandated by UFS 1.1 spec.
Also, for PA_MaxRxPWMGear both UniPro and UFS device specification
support the gear-1 to gear-7. so i guess we should have some safely
check to ensure that gear is not 0 or greater than 7.
> +
> + ufshcd_dme_peer_get(hba, UIC_ARG_MIB(PA_MAXRXHSGEAR), &gear[TX]);
> + if (!gear[TX]) {
> + ufshcd_dme_peer_get(hba, UIC_ARG_MIB(PA_MAXRXPWMGEAR),
> + &gear[TX]);
> + pwr[TX] = SLOWAUTO_MODE;
> + }
> +
> + /*
> + * Configure attributes for power mode change with below.
> + * - PA_RXGEAR, PA_ACTIVERXDATALANES, PA_RXTERMINATION,
> + * - PA_TXGEAR, PA_ACTIVETXDATALANES, PA_TXTERMINATION,
> + * - PA_HSSERIES
> + */
> + ufshcd_dme_set(hba, UIC_ARG_MIB(PA_RXGEAR), gear[RX]);
> + ufshcd_dme_set(hba, UIC_ARG_MIB(PA_ACTIVERXDATALANES), lanes[RX]);
> + if (pwr[RX] == FASTAUTO_MODE)
> + ufshcd_dme_set(hba, UIC_ARG_MIB(PA_RXTERMINATION), TRUE);
> +
> + ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXGEAR), gear[TX]);
> + ufshcd_dme_set(hba, UIC_ARG_MIB(PA_ACTIVETXDATALANES), lanes[TX]);
> + if (pwr[TX] == FASTAUTO_MODE)
> + ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXTERMINATION), TRUE);
> +
> + if (pwr[RX] == FASTAUTO_MODE || pwr[TX] == FASTAUTO_MODE)
> + ufshcd_dme_set(hba, UIC_ARG_MIB(PA_HSSERIES), PA_HS_MODE_B);
> +
> + ret = ufshcd_uic_change_pwr_mode(hba, pwr[RX] << 4 | pwr[TX]);
> + if (ret)
> + dev_err(hba->dev,
> + "pwr_mode: power mode change failed %d\n", ret);
I guess it would be helpful to print the powermode, Gear, Lanes & Rate
information after error. Also, i guess the gear change is one time
operation, won't it be useful to print this information so we know what
is the host-device configuration?
> +
> + return ret;
> +}
> +
> +/**
> * ufshcd_complete_dev_init() - checks device readiness
> * hba: per-adapter instance
> *
> @@ -2942,6 +3006,8 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
> if (ret)
> goto out;
>
> + ufshcd_config_max_pwr_mode(hba);
> +
> ret = ufshcd_verify_dev_init(hba);
> if (ret)
> goto out;
> diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h
> index 3a710eb..0bb8041 100644
> --- a/drivers/scsi/ufs/unipro.h
> +++ b/drivers/scsi/ufs/unipro.h
> @@ -72,6 +72,21 @@
> #define PA_STALLNOCONFIGTIME 0x15A3
> #define PA_SAVECONFIGTIME 0x15A4
>
> +/* PA power modes */
> +enum {
> + FAST_MODE = 1,
> + SLOW_MODE = 2,
> + FASTAUTO_MODE = 4,
> + SLOWAUTO_MODE = 5,
> + UNCHANGED = 7,
> +};
> +
> +/* PA TX/RX Frequency Series */
> +enum {
> + PA_HS_MODE_A = 1,
> + PA_HS_MODE_B = 2,
> +};
> +
> /*
> * Data Link Layer Attributes
> */
> @@ -127,4 +142,10 @@
> #define T_TC0TXMAXSDUSIZE 0x4060
> #define T_TC1TXMAXSDUSIZE 0x4061
>
> +/* Boolean attribute values */
> +enum {
> + FALSE = 0,
> + TRUE,
> +};
> +
> #endif /* _UNIPRO_H_ */
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v3 6/6] scsi: ufs: configure the attribute for power mode
2013-08-27 10:21 ` Subhash Jadavani
@ 2013-08-27 10:27 ` Subhash Jadavani
0 siblings, 0 replies; 72+ messages in thread
From: Subhash Jadavani @ 2013-08-27 10:27 UTC (permalink / raw)
To: Seungwon Jeon
Cc: linux-scsi, 'Vinayak Holikatti', 'Santosh Y',
'James E.J. Bottomley'
On 8/27/2013 3:51 PM, Subhash Jadavani wrote:
> On 8/26/2013 8:11 PM, Seungwon Jeon wrote:
>> UIC attributes can be set with using DME_SET command for
>> power mode change. For configuration the link capability
>> attributes are used, which is updated after successful
>> link startup.
>>
>> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
>> Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
>> ---
>> drivers/scsi/ufs/ufshcd.c | 66
>> +++++++++++++++++++++++++++++++++++++++++++++
>> drivers/scsi/ufs/unipro.h | 21 ++++++++++++++
>> 2 files changed, 87 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index c67118d..2ae845c 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -1598,6 +1598,70 @@ out:
>> }
>> /**
>> + * ufshcd_config_max_pwr_mode - Set & Change power mode with
>> + * maximum capability attribute information.
>> + * @hba: per adapter instance
>> + *
>> + * Returns 0 on success, non-zero value on failure
>> + */
>> +static int ufshcd_config_max_pwr_mode(struct ufs_hba *hba)
>> +{
>> + enum {RX = 0, TX = 1};
>> + u32 lanes[] = {1, 1};
>> + u32 gear[] = {1, 1};
>> + u8 pwr[] = {FASTAUTO_MODE, FASTAUTO_MODE};
>> + int ret;
>> +
>> + /* Get the connected lane count */
>> + ufshcd_dme_get(hba, UIC_ARG_MIB(PA_CONNECTEDRXDATALANES),
>> &lanes[RX]);
>> + ufshcd_dme_get(hba, UIC_ARG_MIB(PA_CONNECTEDTXDATALANES),
>> &lanes[TX]);
>> +
>> + /*
>> + * First, get the maximum gears of HS speed.
>> + * If a zero value, it means there is no HSGEAR capability.
>> + * Then, get the maximum gears of PWM speed.
>> + */
>> + ufshcd_dme_get(hba, UIC_ARG_MIB(PA_MAXRXHSGEAR), &gear[RX]);
>> + if (!gear[RX]) {
>> + ufshcd_dme_get(hba, UIC_ARG_MIB(PA_MAXRXPWMGEAR), &gear[RX]);
>> + pwr[RX] = SLOWAUTO_MODE;
>> + }
> As per UniPro specification, PA_MaxRxHsGear valid values are 0 (NO,
> HS), 1 (HS_Gear1), 2 (HS_Gear2) & 3 (HS_Gear3). But UFS device
> specification (v1.1) only mandates to support Gear-1 and Gear-2 is
> optional. Gear-3 is no where mentioned in the specifcation so it means
> unsupported gear. So i guess after reading PA_MaxRxHsGear attribute,
> we have to ensure that we only set the Gear-1 or Gear-2 as mandated by
> UFS 1.1 spec.
>
> Also, for PA_MaxRxPWMGear both UniPro and UFS device specification
> support the gear-1 to gear-7. so i guess we should have some safely
> check to ensure that gear is not 0 or greater than 7.
Sorry, please ignore this comment. PA_MaxRxHsGear and PA_MaxRxPWMGear
are automatically updated after the capability negotiation between host
and device during link start up so they must be supporting the mutually
agreed gears.
>
>> +
>> + ufshcd_dme_peer_get(hba, UIC_ARG_MIB(PA_MAXRXHSGEAR), &gear[TX]);
>> + if (!gear[TX]) {
>> + ufshcd_dme_peer_get(hba, UIC_ARG_MIB(PA_MAXRXPWMGEAR),
>> + &gear[TX]);
>> + pwr[TX] = SLOWAUTO_MODE;
>> + }
>> +
>> + /*
>> + * Configure attributes for power mode change with below.
>> + * - PA_RXGEAR, PA_ACTIVERXDATALANES, PA_RXTERMINATION,
>> + * - PA_TXGEAR, PA_ACTIVETXDATALANES, PA_TXTERMINATION,
>> + * - PA_HSSERIES
>> + */
>> + ufshcd_dme_set(hba, UIC_ARG_MIB(PA_RXGEAR), gear[RX]);
>> + ufshcd_dme_set(hba, UIC_ARG_MIB(PA_ACTIVERXDATALANES), lanes[RX]);
>> + if (pwr[RX] == FASTAUTO_MODE)
>> + ufshcd_dme_set(hba, UIC_ARG_MIB(PA_RXTERMINATION), TRUE);
>> +
>> + ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXGEAR), gear[TX]);
>> + ufshcd_dme_set(hba, UIC_ARG_MIB(PA_ACTIVETXDATALANES), lanes[TX]);
>> + if (pwr[TX] == FASTAUTO_MODE)
>> + ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXTERMINATION), TRUE);
>> +
>> + if (pwr[RX] == FASTAUTO_MODE || pwr[TX] == FASTAUTO_MODE)
>> + ufshcd_dme_set(hba, UIC_ARG_MIB(PA_HSSERIES), PA_HS_MODE_B);
>> +
>> + ret = ufshcd_uic_change_pwr_mode(hba, pwr[RX] << 4 | pwr[TX]);
>> + if (ret)
>> + dev_err(hba->dev,
>> + "pwr_mode: power mode change failed %d\n", ret);
> I guess it would be helpful to print the powermode, Gear, Lanes & Rate
> information after error. Also, i guess the gear change is one time
> operation, won't it be useful to print this information so we know
> what is the host-device configuration?
This is just a debug enhancement so its upto you whether you want to fix
it now or not. In any case, you will still have my Reviewed-by: Subhash
Jadavani <subhashj@codeauora.org>
>
>> +
>> + return ret;
>> +}
>> +
>> +/**
>> * ufshcd_complete_dev_init() - checks device readiness
>> * hba: per-adapter instance
>> *
>> @@ -2942,6 +3006,8 @@ static void ufshcd_async_scan(void *data,
>> async_cookie_t cookie)
>> if (ret)
>> goto out;
>> + ufshcd_config_max_pwr_mode(hba);
>> +
>> ret = ufshcd_verify_dev_init(hba);
>> if (ret)
>> goto out;
>> diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h
>> index 3a710eb..0bb8041 100644
>> --- a/drivers/scsi/ufs/unipro.h
>> +++ b/drivers/scsi/ufs/unipro.h
>> @@ -72,6 +72,21 @@
>> #define PA_STALLNOCONFIGTIME 0x15A3
>> #define PA_SAVECONFIGTIME 0x15A4
>> +/* PA power modes */
>> +enum {
>> + FAST_MODE = 1,
>> + SLOW_MODE = 2,
>> + FASTAUTO_MODE = 4,
>> + SLOWAUTO_MODE = 5,
>> + UNCHANGED = 7,
>> +};
>> +
>> +/* PA TX/RX Frequency Series */
>> +enum {
>> + PA_HS_MODE_A = 1,
>> + PA_HS_MODE_B = 2,
>> +};
>> +
>> /*
>> * Data Link Layer Attributes
>> */
>> @@ -127,4 +142,10 @@
>> #define T_TC0TXMAXSDUSIZE 0x4060
>> #define T_TC1TXMAXSDUSIZE 0x4061
>> +/* Boolean attribute values */
>> +enum {
>> + FALSE = 0,
>> + TRUE,
>> +};
>> +
>> #endif /* _UNIPRO_H_ */
>
> --
> 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
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH] scsi: ufs: export the helper functions for vender probe/remove
2013-07-26 13:46 ` [PATCH 2/7] scsi: ufs: find out sense data over scsi status values Seungwon Jeon
` (14 preceding siblings ...)
2013-08-26 14:41 ` [PATCH v3 6/6] scsi: ufs: configure the attribute for power mode Seungwon Jeon
@ 2013-09-09 11:51 ` Seungwon Jeon
15 siblings, 0 replies; 72+ messages in thread
From: Seungwon Jeon @ 2013-09-09 11:51 UTC (permalink / raw)
To: linux-scsi
Cc: 'Santosh Y', 'Vinayak Holikatti',
'James E.J. Bottomley'
This change provides the common routines for driver's probe/remove.
Especially host driver including specific operations can invoke the
initial routine at the own probing phase and pass its operations to
ufshcd's common part.
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
---
drivers/scsi/ufs/ufshcd-pltfrm.c | 49 +++++++++++++++++++++++++++----------
drivers/scsi/ufs/ufshcd-pltfrm.h | 19 ++++++++++++++
2 files changed, 55 insertions(+), 13 deletions(-)
create mode 100644 drivers/scsi/ufs/ufshcd-pltfrm.h
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 9c94052..4900597 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -38,6 +38,7 @@
#include <linux/of.h>
#include "ufshcd.h"
+#include "ufshcd-pltfrm.h"
static const struct of_device_id ufs_of_match[];
static struct ufs_hba_variant_ops *get_variant_ops(struct device *dev)
@@ -137,12 +138,13 @@ static int ufshcd_pltfrm_runtime_idle(struct device *dev)
#endif /* CONFIG_PM_RUNTIME */
/**
- * ufshcd_pltfrm_probe - probe routine of the driver
+ * ufshcd_pltfrm_init - common routine for the driver's initialization
* @pdev: pointer to Platform device handle
*
* Returns 0 on success, non-zero value on failure
*/
-static int ufshcd_pltfrm_probe(struct platform_device *pdev)
+int ufshcd_pltfrm_init(struct platform_device *pdev,
+ const struct ufs_hba_variant_ops *vops)
{
struct ufs_hba *hba;
void __iomem *mmio_base;
@@ -166,14 +168,13 @@ static int ufshcd_pltfrm_probe(struct platform_device *pdev)
err = ufshcd_alloc_host(dev, &hba);
if (err) {
- dev_err(&pdev->dev, "Allocation failed\n");
+ dev_err(dev, "Allocation failed\n");
goto out;
}
- hba->vops = get_variant_ops(&pdev->dev);
-
- pm_runtime_set_active(&pdev->dev);
- pm_runtime_enable(&pdev->dev);
+ hba->vops = vops;
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
err = ufshcd_init(hba, mmio_base, irq);
if (err) {
@@ -186,24 +187,46 @@ static int ufshcd_pltfrm_probe(struct platform_device *pdev)
return 0;
out_disable_rpm:
- pm_runtime_disable(&pdev->dev);
- pm_runtime_set_suspended(&pdev->dev);
+ pm_runtime_disable(dev);
+ pm_runtime_set_suspended(dev);
out:
return err;
}
+EXPORT_SYMBOL_GPL(ufshcd_pltfrm_init);
/**
- * ufshcd_pltfrm_remove - remove platform driver routine
+ * ufshcd_pltfrm_exit - common routine for the driver's exit
* @pdev: pointer to platform device handle
- *
- * Returns 0 on success, non-zero value on failure
*/
-static int ufshcd_pltfrm_remove(struct platform_device *pdev)
+void ufshcd_pltfrm_exit(struct platform_device *pdev)
{
struct ufs_hba *hba = platform_get_drvdata(pdev);
pm_runtime_get_sync(&(pdev)->dev);
ufshcd_remove(hba);
+}
+EXPORT_SYMBOL_GPL(ufshcd_pltfrm_exit);
+
+/**
+ * ufshcd_pltfrm_probe - probe the platform driver
+ * @pdev: pointer to Platform device handle
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+static int ufshcd_pltfrm_probe(struct platform_device *pdev)
+{
+ return ufshcd_pltfrm_init(pdev, get_variant_ops(&pdev->dev));
+}
+
+/**
+ * ufshcd_pltfrm_remove - remove the platform driver
+ * @pdev: pointer to platform device handle
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+static int ufshcd_pltfrm_remove(struct platform_device *pdev)
+{
+ ufshcd_pltfrm_exit(pdev);
return 0;
}
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.h b/drivers/scsi/ufs/ufshcd-pltfrm.h
new file mode 100644
index 0000000..6adab05
--- /dev/null
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.h
@@ -0,0 +1,19 @@
+/*
+ * Universal Flash Storage Host controller Platform bus based glue driver
+ *
+ * Copyright (C) 2013, Samsung Electronics Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * at your option) any later version.
+ */
+
+#ifndef _UFSHCD_PLTFRM_H_
+#define _UFSHCD_PLTFRM_H_
+
+extern int ufshcd_pltfrm_init(struct platform_device *pdev,
+ const struct ufs_hba_variant_ops *vops);
+extern void ufshcd_pltfrm_exit(struct platform_device *pdev);
+
+#endif /* _UFSHCD_PLTFRM_H_ */
--
1.7.0.4
^ permalink raw reply related [flat|nested] 72+ messages in thread