[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0673f807-bc3c-e401-ae12-07bfafb55bf7@intel.com>
Date: Wed, 11 Jul 2018 13:50:51 +0300
From: Adrian Hunter <adrian.hunter@...el.com>
To: Asutosh Das <asutoshd@...eaurora.org>, subhashj@...eaurora.org,
cang@...eaurora.org, vivek.gautam@...eaurora.org,
rnayak@...eaurora.org, vinholikatti@...il.com,
jejb@...ux.vnet.ibm.com, martin.petersen@...cle.com,
linux-scsi@...r.kernel.org
Cc: linux-arm-msm@...r.kernel.org,
Venkat Gopalakrishnan <venkatg@...eaurora.org>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Mathieu Malaterre <malat@...ian.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 4/9] scsi: ufs: add option to change default UFS power
management level
On 06/07/18 15:30, Asutosh Das wrote:
> From: Subhash Jadavani <subhashj@...eaurora.org>
>
> UFS device and link can be put in multiple different low power modes hence
> UFS driver supports multiple different low power modes. By default UFS
> driver selects the default (optimal) low power mode (which gives moderate
> power savings and have relatively less enter and exit latencies) but
> we might have to tune this default power mode for different chipset
> platforms to meet the low power requirements/goals. Hence this patch
> adds option to change default UFS low power mode (level).
>
> Signed-off-by: Subhash Jadavani <subhashj@...eaurora.org>
> Signed-off-by: Venkat Gopalakrishnan <venkatg@...eaurora.org>
> Signed-off-by: Can Guo <cang@...eaurora.org>
> Signed-off-by: Asutosh Das <asutoshd@...eaurora.org>
> ---
> .../devicetree/bindings/ufs/ufshcd-pltfrm.txt | 11 ++++++++
> drivers/scsi/ufs/ufshcd-pltfrm.c | 14 +++++++++++
> drivers/scsi/ufs/ufshcd.c | 29 +++++++++++++++-------
> drivers/scsi/ufs/ufshcd.h | 4 +--
> 4 files changed, 47 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> index c39dfef..f564d9a 100644
> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> @@ -38,6 +38,15 @@ Optional properties:
> defined or a value in the array is "0" then it is assumed
> that the frequency is set by the parent clock or a
> fixed rate clock source.
> +- rpm-level : UFS Runtime power management level. Following PM levels are supported:
> + 0 - Both UFS device and Link in active state (Highest power consumption)
> + 1 - UFS device in active state but Link in Hibern8 state
> + 2 - UFS device in Sleep state but Link in active state
> + 3 - UFS device in Sleep state and Link in hibern8 state (default PM level)
> + 4 - UFS device in Power-down state and Link in Hibern8 state
> + 5 - UFS device in Power-down state and Link in OFF state (Lowest power consumption)
> +- spm-level : UFS System power management level. Allowed PM levels are same as rpm-level.
> +
> -lanes-per-direction : number of lanes available per direction - either 1 or 2.
> Note that it is assume same number of lanes is used both
> directions at once. If not specified, default is 2 lanes per direction.
> @@ -66,4 +75,6 @@ Example:
> freq-table-hz = <100000000 200000000>, <0 0>, <0 0>;
> phys = <&ufsphy1>;
> phy-names = "ufsphy";
> + rpm-level = <3>;
> + spm-level = <5>;
> };
> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
> index e82bde0..7dba799 100644
> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c
> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
> @@ -221,6 +221,19 @@ static int ufshcd_parse_regulator_info(struct ufs_hba *hba)
> return err;
> }
>
> +static void ufshcd_parse_pm_levels(struct ufs_hba *hba)
> +{
> + struct device *dev = hba->dev;
> + struct device_node *np = dev->of_node;
> +
> + if (np) {
> + if (of_property_read_u32(np, "rpm-level", &hba->rpm_lvl))
> + hba->rpm_lvl = -1;
> + if (of_property_read_u32(np, "spm-level", &hba->spm_lvl))
> + hba->spm_lvl = -1;
These are generically useful to all UFSHC drivers, so they should be
device_property_read_u32() and they should be read for all drivers not just
pltfrm i.e. move this code into ufshcd.c
> + }
> +}
> +
> #ifdef CONFIG_PM
> /**
> * ufshcd_pltfrm_suspend - suspend power management function
> @@ -340,6 +353,7 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
> goto dealloc_host;
> }
>
> + ufshcd_parse_pm_levels(hba);
> pm_runtime_set_active(&pdev->dev);
> pm_runtime_enable(&pdev->dev);
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index b03f3ea..e950204 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -192,6 +192,14 @@ struct ufs_pm_lvl_states ufs_pm_lvl_states[] = {
> return UFS_PM_LVL_0;
> }
>
> +static inline bool ufshcd_is_valid_pm_lvl(int lvl)
> +{
> + if (lvl >= 0 && lvl < ARRAY_SIZE(ufs_pm_lvl_states))
> + return true;
> + else
> + return false;
> +}
> +
> static struct ufs_dev_fix ufs_fixups[] = {
> /* UFS cards deviations table */
> UFS_FIX(UFS_VENDOR_SAMSUNG, UFS_ANY_MODEL,
> @@ -8051,16 +8059,19 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
> }
>
> /*
> - * Set the default power management level for runtime and system PM.
> - * Default power saving mode is to keep UFS link in Hibern8 state
> - * and UFS device in sleep state.
> + * If rpm_lvl and and spm_lvl are not already set to valid levels,
> + * set the default power management level for UFS runtime and system
> + * suspend. Default power saving mode selected is keeping UFS link in
> + * Hibern8 state and UFS device in sleep state.
> */
> - hba->rpm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
> - UFS_SLEEP_PWR_MODE,
> - UIC_LINK_HIBERN8_STATE);
> - hba->spm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
> - UFS_SLEEP_PWR_MODE,
> - UIC_LINK_HIBERN8_STATE);
> + if (!ufshcd_is_valid_pm_lvl(hba->rpm_lvl))
> + hba->rpm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
> + UFS_SLEEP_PWR_MODE,
> + UIC_LINK_HIBERN8_STATE);
> + if (!ufshcd_is_valid_pm_lvl(hba->spm_lvl))
> + hba->spm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
> + UFS_SLEEP_PWR_MODE,
> + UIC_LINK_HIBERN8_STATE);
>
> /* Set the default auto-hiberate idle timer value to 150 ms */
> if (hba->capabilities & MASK_AUTO_HIBERN8_SUPPORT) {
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index e996a08..a2e1d5c 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -526,9 +526,9 @@ struct ufs_hba {
> enum ufs_dev_pwr_mode curr_dev_pwr_mode;
> enum uic_link_state uic_link_state;
> /* Desired UFS power management level during runtime PM */
> - enum ufs_pm_level rpm_lvl;
> + int rpm_lvl;
> /* Desired UFS power management level during system PM */
> - enum ufs_pm_level spm_lvl;
> + int spm_lvl;
> struct device_attribute rpm_lvl_attr;
> struct device_attribute spm_lvl_attr;
> int pm_op_in_progress;
>
Powered by blists - more mailing lists