linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Subhash Jadavani <subhashj@codeaurora.org>
To: Seungwon Jeon <tgih.jun@samsung.com>
Cc: linux-scsi@vger.kernel.org,
	'Vinayak Holikatti' <vinholikatti@gmail.com>,
	'Santosh Y' <santoshsy@gmail.com>,
	"'James E.J. Bottomley'" <James.Bottomley@HansenPartnership.com>
Subject: Re: [PATCH v3 6/6] scsi: ufs: configure the attribute for power mode
Date: Tue, 27 Aug 2013 15:51:29 +0530	[thread overview]
Message-ID: <521C7DA9.1080602@codeaurora.org> (raw)
In-Reply-To: <003b01cea26a$48ffa670$dafef350$%jun@samsung.com>

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_ */


  reply	other threads:[~2013-08-27 10:21 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-20  0:41 [PATCH v2 0/3] ufs: fix bugs in probing and removing driver paths Akinobu Mita
2013-07-20  0:41 ` [PATCH v2 1/3] ufshcd-pci: release ioremapped region during removing driver Akinobu Mita
2013-07-26 13:45   ` [PATCH 1/7] scsi: ufs: amend the ocs handling with fatal error Seungwon Jeon
2013-07-29  6:17     ` Subhash Jadavani
2013-07-29 10:05       ` Seungwon Jeon
2013-07-29 10:27         ` Subhash Jadavani
2013-07-29 10:51       ` Sujit Reddy Thumma
2013-07-30 13:02         ` Seungwon Jeon
2013-08-12  7:17           ` Subhash Jadavani
2013-08-13 11:50             ` Seungwon Jeon
2013-08-13 13:39               ` Subhash Jadavani
2013-07-29 18:03     ` Santosh Y
2013-07-20  0:41 ` [PATCH v2 2/3] ufs: don't disable_irq() if the IRQ can be shared among devices Akinobu Mita
2013-07-26 13:44   ` [PATCH 0/7] scsi: ufs: some fixes and updates Seungwon Jeon
2013-08-23 13:00     ` [PATCH v2 0/6] " Seungwon Jeon
2013-08-25 11:23       ` Dolev Raviv
2013-08-26 14:40     ` [PATCH v3 " Seungwon Jeon
2013-08-28 10:46       ` Subhash Jadavani
2013-07-20  0:41 ` [PATCH v2 3/3] ufs: don't stop controller before scsi_remove_host() Akinobu Mita
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
2013-07-30 13:03     ` Seungwon Jeon
2013-07-30  3:53   ` Santosh Y
2013-07-30 13:03     ` Seungwon Jeon
2013-07-31  0:15       ` Elliott, Robert (Server Storage)
2013-08-06 12:08         ` Seungwon Jeon
2013-08-23 13:00   ` [PATCH v2 1/6] " Seungwon Jeon
2013-08-23 13:00   ` [PATCH v2 2/6] scsi: ufs: fix the setting interrupt aggregation counter Seungwon Jeon
2013-08-23 13:00   ` [PATCH v2 3/6] scsi: ufs: add dme configuration primitives Seungwon Jeon
2013-08-23 13:00   ` [PATCH v2 4/6] scsi: ufs: add unipro attribute IDs Seungwon Jeon
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   ` [PATCH v2 6/6] scsi: ufs: configure the attribute for power mode Seungwon Jeon
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
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
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
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
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
2013-08-27 11:47         ` Subhash Jadavani
2013-08-27 11:58           ` Seungwon Jeon
2013-08-28 12:45             ` Yaniv Gardi
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 [this message]
2013-08-27 10:27       ` Subhash Jadavani
2013-09-09 11:51   ` [PATCH] scsi: ufs: export the helper functions for vender probe/remove Seungwon Jeon
2013-07-26 13:46 ` [PATCH 3/7] scsi: ufs: fix the setting interrupt aggregation counter Seungwon Jeon
2013-07-29  7:03   ` Subhash Jadavani
2013-07-30 13:01     ` Seungwon Jeon
2013-07-26 13:47 ` [PATCH 4/7] scsi: ufs: add dme configuration primitives Seungwon Jeon
2013-07-29  9:24   ` Subhash Jadavani
2013-07-30 13:02     ` Seungwon Jeon
2013-08-13  6:56       ` Subhash Jadavani
2013-07-26 13:48 ` [PATCH 5/7] scsi: ufs: add unipro attribute IDs Seungwon Jeon
2013-07-29  9:26   ` Subhash Jadavani
2013-07-26 13:48 ` [PATCH 6/7] scsi: ufs: add operation for the uic power mode change Seungwon Jeon
2013-07-29  9:53   ` Subhash Jadavani
2013-07-30 13:02     ` Seungwon Jeon
2013-07-26 13:49 ` [PATCH 7/7] scsi: ufs: configure the attribute for power mode Seungwon Jeon
2013-07-31 13:28   ` Subhash Jadavani
2013-08-06 12:08     ` Seungwon Jeon
2013-08-13  7:00       ` Subhash Jadavani

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=521C7DA9.1080602@codeaurora.org \
    --to=subhashj@codeaurora.org \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=santoshsy@gmail.com \
    --cc=tgih.jun@samsung.com \
    --cc=vinholikatti@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).