diff mbox series

[v1,1/2] scsi: ufs: Put hba into LPM during clk gating

Message ID ce0a3be9c685506803597fb770e37c099ae27232.1603754932.git.asutoshd@codeaurora.org (mailing list archive)
State Superseded
Headers show
Series [v1,1/2] scsi: ufs: Put hba into LPM during clk gating | expand

Commit Message

Asutosh Das (asd) Oct. 26, 2020, 11:30 p.m. UTC 
From: Can Guo <cang@codeaurora.org>

During clock gating, after clocks are disabled,
put hba into LPM to save more power.

Signed-off-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd.c |  7 +++++--
 drivers/scsi/ufs/ufshcd.h | 13 +++++++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)

Comments

Stanley Chu Oct. 27, 2020, 1:53 a.m. UTC | #1 
On Mon, 2020-10-26 at 16:30 -0700, Asutosh Das wrote:
> From: Can Guo <cang@codeaurora.org>
> 
> During clock gating, after clocks are disabled,
> put hba into LPM to save more power.
> 
> Signed-off-by: Can Guo <cang@codeaurora.org>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>

Acked-by: Stanley Chu <stanley.chu@mediatek.com>
kernel test robot Oct. 27, 2020, 4:42 a.m. UTC | #2 
Hi Asutosh,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on mkp-scsi/for-next]
[also build test ERROR on scsi/for-next v5.10-rc1 next-20201026]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Asutosh-Das/scsi-ufs-Put-hba-into-LPM-during-clk-gating/20201027-073142
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: arm-randconfig-r031-20201026 (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/9e2e3ce8b0bf533614823400cdb43ea92cc6b1c0
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Asutosh-Das/scsi-ufs-Put-hba-into-LPM-during-clk-gating/20201027-073142
        git checkout 9e2e3ce8b0bf533614823400cdb43ea92cc6b1c0
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   drivers/scsi/ufs/ufshcd.c: In function 'ufshcd_ungate_work':
>> drivers/scsi/ufs/ufshcd.c:1551:2: error: implicit declaration of function 'ufshcd_hba_vreg_set_hpm' [-Werror=implicit-function-declaration]
    1551 |  ufshcd_hba_vreg_set_hpm(hba);
         |  ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/scsi/ufs/ufshcd.c: In function 'ufshcd_gate_work':
>> drivers/scsi/ufs/ufshcd.c:1718:2: error: implicit declaration of function 'ufshcd_hba_vreg_set_lpm' [-Werror=implicit-function-declaration]
    1718 |  ufshcd_hba_vreg_set_lpm(hba);
         |  ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/scsi/ufs/ufshcd.c: At top level:
>> drivers/scsi/ufs/ufshcd.c:8409:13: warning: conflicting types for 'ufshcd_hba_vreg_set_lpm'
    8409 | static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba)
         |             ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/scsi/ufs/ufshcd.c:8409:13: error: static declaration of 'ufshcd_hba_vreg_set_lpm' follows non-static declaration
   drivers/scsi/ufs/ufshcd.c:1718:2: note: previous implicit declaration of 'ufshcd_hba_vreg_set_lpm' was here
    1718 |  ufshcd_hba_vreg_set_lpm(hba);
         |  ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/scsi/ufs/ufshcd.c:8415:13: warning: conflicting types for 'ufshcd_hba_vreg_set_hpm'
    8415 | static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba)
         |             ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/scsi/ufs/ufshcd.c:8415:13: error: static declaration of 'ufshcd_hba_vreg_set_hpm' follows non-static declaration
   drivers/scsi/ufs/ufshcd.c:1551:2: note: previous implicit declaration of 'ufshcd_hba_vreg_set_hpm' was here
    1551 |  ufshcd_hba_vreg_set_hpm(hba);
         |  ^~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/ufshcd_hba_vreg_set_hpm +1551 drivers/scsi/ufs/ufshcd.c

  1534	
  1535	static void ufshcd_ungate_work(struct work_struct *work)
  1536	{
  1537		int ret;
  1538		unsigned long flags;
  1539		struct ufs_hba *hba = container_of(work, struct ufs_hba,
  1540				clk_gating.ungate_work);
  1541	
  1542		cancel_delayed_work_sync(&hba->clk_gating.gate_work);
  1543	
  1544		spin_lock_irqsave(hba->host->host_lock, flags);
  1545		if (hba->clk_gating.state == CLKS_ON) {
  1546			spin_unlock_irqrestore(hba->host->host_lock, flags);
  1547			goto unblock_reqs;
  1548		}
  1549	
  1550		spin_unlock_irqrestore(hba->host->host_lock, flags);
> 1551		ufshcd_hba_vreg_set_hpm(hba);
  1552		ufshcd_setup_clocks(hba, true);
  1553	
  1554		ufshcd_enable_irq(hba);
  1555	
  1556		/* Exit from hibern8 */
  1557		if (ufshcd_can_hibern8_during_gating(hba)) {
  1558			/* Prevent gating in this path */
  1559			hba->clk_gating.is_suspended = true;
  1560			if (ufshcd_is_link_hibern8(hba)) {
  1561				ret = ufshcd_uic_hibern8_exit(hba);
  1562				if (ret)
  1563					dev_err(hba->dev, "%s: hibern8 exit failed %d\n",
  1564						__func__, ret);
  1565				else
  1566					ufshcd_set_link_active(hba);
  1567			}
  1568			hba->clk_gating.is_suspended = false;
  1569		}
  1570	unblock_reqs:
  1571		ufshcd_scsi_unblock_requests(hba);
  1572	}
  1573	
  1574	/**
  1575	 * ufshcd_hold - Enable clocks that were gated earlier due to ufshcd_release.
  1576	 * Also, exit from hibern8 mode and set the link as active.
  1577	 * @hba: per adapter instance
  1578	 * @async: This indicates whether caller should ungate clocks asynchronously.
  1579	 */
  1580	int ufshcd_hold(struct ufs_hba *hba, bool async)
  1581	{
  1582		int rc = 0;
  1583		bool flush_result;
  1584		unsigned long flags;
  1585	
  1586		if (!ufshcd_is_clkgating_allowed(hba))
  1587			goto out;
  1588		spin_lock_irqsave(hba->host->host_lock, flags);
  1589		hba->clk_gating.active_reqs++;
  1590	
  1591	start:
  1592		switch (hba->clk_gating.state) {
  1593		case CLKS_ON:
  1594			/*
  1595			 * Wait for the ungate work to complete if in progress.
  1596			 * Though the clocks may be in ON state, the link could
  1597			 * still be in hibner8 state if hibern8 is allowed
  1598			 * during clock gating.
  1599			 * Make sure we exit hibern8 state also in addition to
  1600			 * clocks being ON.
  1601			 */
  1602			if (ufshcd_can_hibern8_during_gating(hba) &&
  1603			    ufshcd_is_link_hibern8(hba)) {
  1604				if (async) {
  1605					rc = -EAGAIN;
  1606					hba->clk_gating.active_reqs--;
  1607					break;
  1608				}
  1609				spin_unlock_irqrestore(hba->host->host_lock, flags);
  1610				flush_result = flush_work(&hba->clk_gating.ungate_work);
  1611				if (hba->clk_gating.is_suspended && !flush_result)
  1612					goto out;
  1613				spin_lock_irqsave(hba->host->host_lock, flags);
  1614				goto start;
  1615			}
  1616			break;
  1617		case REQ_CLKS_OFF:
  1618			if (cancel_delayed_work(&hba->clk_gating.gate_work)) {
  1619				hba->clk_gating.state = CLKS_ON;
  1620				trace_ufshcd_clk_gating(dev_name(hba->dev),
  1621							hba->clk_gating.state);
  1622				break;
  1623			}
  1624			/*
  1625			 * If we are here, it means gating work is either done or
  1626			 * currently running. Hence, fall through to cancel gating
  1627			 * work and to enable clocks.
  1628			 */
  1629			/* fallthrough */
  1630		case CLKS_OFF:
  1631			ufshcd_scsi_block_requests(hba);
  1632			hba->clk_gating.state = REQ_CLKS_ON;
  1633			trace_ufshcd_clk_gating(dev_name(hba->dev),
  1634						hba->clk_gating.state);
  1635			queue_work(hba->clk_gating.clk_gating_workq,
  1636				   &hba->clk_gating.ungate_work);
  1637			/*
  1638			 * fall through to check if we should wait for this
  1639			 * work to be done or not.
  1640			 */
  1641			/* fallthrough */
  1642		case REQ_CLKS_ON:
  1643			if (async) {
  1644				rc = -EAGAIN;
  1645				hba->clk_gating.active_reqs--;
  1646				break;
  1647			}
  1648	
  1649			spin_unlock_irqrestore(hba->host->host_lock, flags);
  1650			flush_work(&hba->clk_gating.ungate_work);
  1651			/* Make sure state is CLKS_ON before returning */
  1652			spin_lock_irqsave(hba->host->host_lock, flags);
  1653			goto start;
  1654		default:
  1655			dev_err(hba->dev, "%s: clk gating is in invalid state %d\n",
  1656					__func__, hba->clk_gating.state);
  1657			break;
  1658		}
  1659		spin_unlock_irqrestore(hba->host->host_lock, flags);
  1660	out:
  1661		return rc;
  1662	}
  1663	EXPORT_SYMBOL_GPL(ufshcd_hold);
  1664	
  1665	static void ufshcd_gate_work(struct work_struct *work)
  1666	{
  1667		struct ufs_hba *hba = container_of(work, struct ufs_hba,
  1668				clk_gating.gate_work.work);
  1669		unsigned long flags;
  1670		int ret;
  1671	
  1672		spin_lock_irqsave(hba->host->host_lock, flags);
  1673		/*
  1674		 * In case you are here to cancel this work the gating state
  1675		 * would be marked as REQ_CLKS_ON. In this case save time by
  1676		 * skipping the gating work and exit after changing the clock
  1677		 * state to CLKS_ON.
  1678		 */
  1679		if (hba->clk_gating.is_suspended ||
  1680			(hba->clk_gating.state != REQ_CLKS_OFF)) {
  1681			hba->clk_gating.state = CLKS_ON;
  1682			trace_ufshcd_clk_gating(dev_name(hba->dev),
  1683						hba->clk_gating.state);
  1684			goto rel_lock;
  1685		}
  1686	
  1687		if (hba->clk_gating.active_reqs
  1688			|| hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL
  1689			|| ufshcd_any_tag_in_use(hba) || hba->outstanding_tasks
  1690			|| hba->active_uic_cmd || hba->uic_async_done)
  1691			goto rel_lock;
  1692	
  1693		spin_unlock_irqrestore(hba->host->host_lock, flags);
  1694	
  1695		/* put the link into hibern8 mode before turning off clocks */
  1696		if (ufshcd_can_hibern8_during_gating(hba)) {
  1697			ret = ufshcd_uic_hibern8_enter(hba);
  1698			if (ret) {
  1699				hba->clk_gating.state = CLKS_ON;
  1700				dev_err(hba->dev, "%s: hibern8 enter failed %d\n",
  1701						__func__, ret);
  1702				trace_ufshcd_clk_gating(dev_name(hba->dev),
  1703							hba->clk_gating.state);
  1704				goto out;
  1705			}
  1706			ufshcd_set_link_hibern8(hba);
  1707		}
  1708	
  1709		ufshcd_disable_irq(hba);
  1710	
  1711		if (!ufshcd_is_link_active(hba))
  1712			ufshcd_setup_clocks(hba, false);
  1713		else
  1714			/* If link is active, device ref_clk can't be switched off */
  1715			__ufshcd_setup_clocks(hba, false, true);
  1716	
  1717		/* Put the host controller in low power mode if possible */
> 1718		ufshcd_hba_vreg_set_lpm(hba);
  1719		/*
  1720		 * In case you are here to cancel this work the gating state
  1721		 * would be marked as REQ_CLKS_ON. In this case keep the state
  1722		 * as REQ_CLKS_ON which would anyway imply that clocks are off
  1723		 * and a request to turn them on is pending. By doing this way,
  1724		 * we keep the state machine in tact and this would ultimately
  1725		 * prevent from doing cancel work multiple times when there are
  1726		 * new requests arriving before the current cancel work is done.
  1727		 */
  1728		spin_lock_irqsave(hba->host->host_lock, flags);
  1729		if (hba->clk_gating.state == REQ_CLKS_OFF) {
  1730			hba->clk_gating.state = CLKS_OFF;
  1731			trace_ufshcd_clk_gating(dev_name(hba->dev),
  1732						hba->clk_gating.state);
  1733		}
  1734	rel_lock:
  1735		spin_unlock_irqrestore(hba->host->host_lock, flags);
  1736	out:
  1737		return;
  1738	}
  1739	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 47c544d..55ca8c6 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1548,6 +1548,7 @@  static void ufshcd_ungate_work(struct work_struct *work)
 	}
 
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
+	ufshcd_hba_vreg_set_hpm(hba);
 	ufshcd_setup_clocks(hba, true);
 
 	ufshcd_enable_irq(hba);
@@ -1713,6 +1714,8 @@  static void ufshcd_gate_work(struct work_struct *work)
 		/* If link is active, device ref_clk can't be switched off */
 		__ufshcd_setup_clocks(hba, false, true);
 
+	/* Put the host controller in low power mode if possible */
+	ufshcd_hba_vreg_set_lpm(hba);
 	/*
 	 * In case you are here to cancel this work the gating state
 	 * would be marked as REQ_CLKS_ON. In this case keep the state
@@ -8405,13 +8408,13 @@  static int ufshcd_vreg_set_hpm(struct ufs_hba *hba)
 
 static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba)
 {
-	if (ufshcd_is_link_off(hba))
+	if (ufshcd_is_link_off(hba) || ufshcd_can_aggressive_pc(hba))
 		ufshcd_setup_hba_vreg(hba, false);
 }
 
 static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba)
 {
-	if (ufshcd_is_link_off(hba))
+	if (ufshcd_is_link_off(hba) || ufshcd_can_aggressive_pc(hba))
 		ufshcd_setup_hba_vreg(hba, true);
 }
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 47eb143..0fbb735 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -592,6 +592,13 @@  enum ufshcd_caps {
 	 * inline crypto engine, if it is present
 	 */
 	UFSHCD_CAP_CRYPTO				= 1 << 8,
+
+	/*
+	 * This capability allows the controller regulators to be put into
+	 * lpm mode aggressively during clock gating.
+	 * This would increase power savings.
+	 */
+	UFSHCD_CAP_AGGR_POWER_COLLAPSE			= 1 << 9,
 };
 
 struct ufs_hba_variant_params {
@@ -829,6 +836,12 @@  return true;
 #endif
 }
 
+static inline bool ufshcd_can_aggressive_pc(struct ufs_hba *hba)
+{
+	return !!(ufshcd_is_link_hibern8(hba) &&
+		  (hba->caps & UFSHCD_CAP_AGGR_POWER_COLLAPSE));
+}
+
 static inline bool ufshcd_is_auto_hibern8_supported(struct ufs_hba *hba)
 {
 	return (hba->capabilities & MASK_AUTO_HIBERN8_SUPPORT) &&