From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: linux-phy@lists.infradead.org
Cc: Vinod Koul <vkoul@kernel.org>,
Neil Armstrong <neil.armstrong@linaro.org>,
dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org,
linux-arm-kernel@lists.infradead.org,
linux-arm-msm@vger.kernel.org, linux-can@vger.kernel.org,
linux-gpio@vger.kernel.org, linux-ide@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
linux-pci@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
linux-riscv@lists.infradead.org,
linux-rockchip@lists.infradead.org,
linux-samsung-soc@vger.kernel.org, linux-scsi@vger.kernel.org,
linux-sunxi@lists.linux.dev, linux-tegra@vger.kernel.org,
linux-usb@vger.kernel.org, netdev@vger.kernel.org,
spacemit@lists.linux.dev, UNGLinuxDriver@microchip.com,
"James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>,
Manivannan Sadhasivam <mani@kernel.org>,
"Martin K. Petersen" <martin.petersen@oracle.com>,
Nitin Rawat <quic_nitirawa@quicinc.com>
Subject: [PATCH v6 phy-next 10/28] scsi: ufs: qcom: keep parallel track of PHY power state
Date: Fri, 27 Mar 2026 20:46:48 +0200 [thread overview]
Message-ID: <20260327184706.1600329-11-vladimir.oltean@nxp.com> (raw)
In-Reply-To: <20260327184706.1600329-1-vladimir.oltean@nxp.com>
PHY consumer drivers should not look at the phy->power_count (a PHY
internal field), because in the general case there might also be other
consumers who have called phy_power_on() too, so the fact that the
power_count is non-zero does not mean that we did.
Moreover, struct phy will become opaque soon, so the qcom UFS driver
will not be able to apply this pattern anymore.
By all accounts, the need for ufs_qcom_power_up_sequence() to call
phy_power_off() prior to phy_init() denotes a skewed state of affairs.
(1) The Generic PHY API warns if phy_power_on() is called prior to
phy_init() - which ufs-qcom.c does, from ufs_qcom_setup_clocks().
The UFS controller driver hides its tracks by dropping the power
count prior to phy_init(), and that API violation goes undetected.
(2) phy_calibrate(), as implemented by the phy-qcom-qmp-ufs.c provider,
only works once after power on. Next time it will time out. And
since ufs_qcom_hce_enable_notify() -> ufs_qcom_power_up_sequence()
is called in a retry loop by the UFS core, the PHY power would
normally be on, hence the phy_power_off() call to ensure the
consistent state during phy_calibrate().
The above constitute improper Generic PHY API use, *but* fixing that
requires delicate surgery and I'm only here to stop this PHY consumer
from using fields it's not supposed to.
Once this discussion is settled, I will also address the above issues as
follow-ups:
https://lore.kernel.org/linux-phy/20260327112858.r5lpqygtvsane2vf@skbuf/
Until then, we can reimplement the logic in this driver in a
bug-compatible way, by keeping parallel track of just the UFS
controller's calls to phy_power_on() and phy_power_off().
Note that phy_power_off() shouldn't return an error in general and
doesn't return an error in the particular case of the phy-qcom-qmp-ufs.c
provider. So I've removed the one handling of phy_power_off() errors
from ufs_qcom_setup_clocks().
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Nitin Rawat <quic_nitirawa@quicinc.com>
v5->v6:
- rewrite commit message
- drop phy_power_off() error handling from ufs_qcom_setup_clocks()
v4->v5: patch is new
---
drivers/ufs/host/ufs-qcom.c | 15 ++++++++-------
drivers/ufs/host/ufs-qcom.h | 1 +
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 375fd24ba458..99feabc69111 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -508,9 +508,10 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
if (ret)
return ret;
- if (phy->power_count)
+ if (host->phy_powered_on) {
phy_power_off(phy);
-
+ host->phy_powered_on = false;
+ }
/* phy initialization - calibrate the phy */
ret = phy_init(phy);
@@ -531,6 +532,7 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
__func__, ret);
goto out_disable_phy;
}
+ host->phy_powered_on = true;
ret = phy_calibrate(phy);
if (ret) {
@@ -1263,11 +1265,8 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
ufs_qcom_dev_ref_clk_ctrl(host, false);
}
- err = phy_power_off(phy);
- if (err) {
- dev_err(hba->dev, "phy power off failed, ret=%d\n", err);
- return err;
- }
+ phy_power_off(phy);
+ host->phy_powered_on = false;
}
break;
case POST_CHANGE:
@@ -1277,6 +1276,7 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
dev_err(hba->dev, "phy power on failed, ret = %d\n", err);
return err;
}
+ host->phy_powered_on = true;
/* enable the device ref clock for HS mode*/
if (ufshcd_is_hs_mode(&hba->pwr_info))
@@ -1467,6 +1467,7 @@ static void ufs_qcom_exit(struct ufs_hba *hba)
ufs_qcom_disable_lane_clks(host);
phy_power_off(host->generic_phy);
+ host->phy_powered_on = false;
phy_exit(host->generic_phy);
}
diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
index 1111ab34da01..72ce0687fa42 100644
--- a/drivers/ufs/host/ufs-qcom.h
+++ b/drivers/ufs/host/ufs-qcom.h
@@ -282,6 +282,7 @@ struct ufs_qcom_host {
struct clk_bulk_data *clks;
u32 num_clks;
bool is_lane_clks_enabled;
+ bool phy_powered_on;
struct icc_path *icc_ddr;
struct icc_path *icc_cpu;
--
2.43.0
next prev parent reply other threads:[~2026-03-27 18:49 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-27 18:46 [PATCH v6 phy-next 00/28] Split Generic PHY consumer and provider Vladimir Oltean
2026-03-27 18:46 ` [PATCH v6 phy-next 01/28] ata: add <linux/pm_runtime.h> where missing Vladimir Oltean
2026-03-27 18:46 ` [PATCH v6 phy-next 02/28] PCI: Add missing headers transitively included by <linux/phy/phy.h> Vladimir Oltean
2026-03-27 18:46 ` [PATCH v6 phy-next 03/28] usb: add " Vladimir Oltean
2026-03-30 13:19 ` Greg Kroah-Hartman
2026-03-27 18:46 ` [PATCH v6 phy-next 04/28] drm: add <linux/pm_runtime.h> where missing Vladimir Oltean
2026-03-27 18:46 ` [PATCH v6 phy-next 05/28] phy: " Vladimir Oltean
2026-03-27 18:46 ` [PATCH v6 phy-next 06/28] phy: spacemit: include missing <linux/phy/phy.h> Vladimir Oltean
2026-03-27 18:46 ` [PATCH v6 phy-next 07/28] net: lan969x: include missing <linux/of.h> Vladimir Oltean
2026-03-27 18:46 ` [PATCH v6 phy-next 08/28] PCI: Remove device links to PHY Vladimir Oltean
2026-03-27 18:46 ` [PATCH v6 phy-next 09/28] scsi: ufs: exynos: stop poking into struct phy guts Vladimir Oltean
2026-03-27 20:19 ` Martin K. Petersen
2026-03-27 20:23 ` Peter Griffin
2026-03-27 18:46 ` Vladimir Oltean [this message]
2026-03-27 18:46 ` [PATCH v6 phy-next 11/28] scsi: ufs: qcom: include missing <linux/interrupt.h> Vladimir Oltean
2026-03-27 20:20 ` Martin K. Petersen
2026-03-27 18:46 ` [PATCH v6 phy-next 12/28] drm/rockchip: dw_hdmi: avoid direct dereference of phy->dev.of_node Vladimir Oltean
2026-03-27 18:46 ` [PATCH v6 phy-next 13/28] usb: host: tegra: " Vladimir Oltean
2026-03-27 18:46 ` [PATCH v6 phy-next 14/28] usb: gadget: tegra-xudc: " Vladimir Oltean
2026-03-27 18:46 ` [PATCH v6 phy-next 15/28] drm/msm/dp: remove debugging prints with internal struct phy state Vladimir Oltean
2026-03-27 18:46 ` [PATCH v6 phy-next 16/28] phy: move provider API out of public <linux/phy/phy.h> Vladimir Oltean
2026-03-27 18:46 ` [PATCH v6 phy-next 17/28] phy: make phy_get_mode(), phy_(get|set)_bus_width() NULL tolerant Vladimir Oltean
2026-03-27 18:46 ` [PATCH v6 phy-next 18/28] phy: introduce phy_get_max_link_rate() helper for consumers Vladimir Oltean
2026-03-27 18:46 ` [PATCH v6 phy-next 19/28] drm/rockchip: dsi: include PHY provider header Vladimir Oltean
2026-03-27 18:46 ` [PATCH v6 phy-next 20/28] drm: bridge: cdns-mhdp8546: use consumer API for getting PHY bus width Vladimir Oltean
2026-03-27 18:46 ` [PATCH v6 phy-next 21/28] media: sunxi: a83-mips-csi2: include PHY provider header Vladimir Oltean
2026-03-27 18:47 ` [PATCH v6 phy-next 22/28] net: renesas: rswitch: " Vladimir Oltean
2026-03-27 18:47 ` [PATCH v6 phy-next 23/28] pinctrl: tegra-xusb: " Vladimir Oltean
2026-03-27 18:47 ` [PATCH v6 phy-next 24/28] power: supply: cpcap-charger: include missing <linux/property.h> Vladimir Oltean
2026-03-27 18:47 ` [PATCH v6 phy-next 25/28] phy: include PHY provider header (1/2) Vladimir Oltean
2026-03-27 18:47 ` [PATCH v6 phy-next 26/28] phy: include PHY provider header (2/2) Vladimir Oltean
2026-03-27 18:47 ` [PATCH v6 phy-next 27/28] phy: remove temporary provider compatibility from consumer header Vladimir Oltean
2026-03-27 18:47 ` [PATCH v6 phy-next 28/28] MAINTAINERS: add regexes for linux-phy Vladimir Oltean
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=20260327184706.1600329-11-vladimir.oltean@nxp.com \
--to=vladimir.oltean@nxp.com \
--cc=James.Bottomley@HansenPartnership.com \
--cc=UNGLinuxDriver@microchip.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-can@vger.kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-phy@lists.infradead.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=linux-sunxi@lists.linux.dev \
--cc=linux-tegra@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mani@kernel.org \
--cc=martin.petersen@oracle.com \
--cc=neil.armstrong@linaro.org \
--cc=netdev@vger.kernel.org \
--cc=quic_nitirawa@quicinc.com \
--cc=spacemit@lists.linux.dev \
--cc=vkoul@kernel.org \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).