All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.