[PATCH v2 0/2] Add new phy_notify_pmstate() api

[PATCH v2 0/2] Add new phy_notify_pmstate() api

[Peter Griffin]

This series adds a new phy_notify_pmstate() API to the phy subsystem. It is
designed to be used when some specific runtime configuration parameters
need to be changed when transitioning to the desired pm state which can't
be handled by phy_calibrate()or phy_power_{on|off}().

The first user of the new API is phy-samsung-ufs and phy-gs101-ufs which
needs to issue some register writes when entering and exiting the hibern8
link state.

A separate patch will be sent for ufs-exynos driver to make use of this new
API in the hibern8 callbacks.

To: Vinod Koul <vkoul@kernel.org>
To: Kishon Vijay Abraham I <kishon@kernel.org>
To: André Draszik <andre.draszik@linaro.org>
To: Tudor Ambarus <tudor.ambarus@linaro.org>
To: Alim Akhtar <alim.akhtar@samsung.com>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: linux-phy@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: kernel-team@android.com
Cc: William Mcvicker <willmcvicker@google.com>
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>

Changes in v2:
- Add new phy_notify_pmstate API() instead of using phy_set_mode() (Vinod)
- Link to v1: https://lore.kernel.org/r/20241002201555.3332138-1-peter.griffin@linaro.org

---
Peter Griffin (2):
      phy: add new phy_notify_pmstate() api
      phy: samsung: gs101-ufs: Add .notify_pmstate() and hibern8 enter/exit values

 drivers/phy/phy-core.c                | 25 +++++++++++++++++++++++
 drivers/phy/samsung/phy-gs101-ufs.c   | 28 ++++++++++++++++++++++++++
 drivers/phy/samsung/phy-samsung-ufs.c | 38 +++++++++++++++++++++++++++++++++++
 drivers/phy/samsung/phy-samsung-ufs.h |  7 +++++++
 include/linux/phy/phy.h               | 25 +++++++++++++++++++++++
 5 files changed, 123 insertions(+)
---
base-commit: 97bdc30f39b63758868f67841cebb8c50869e16d
change-id: 20250703-phy-notify-pmstate-f02ba5582f65

Best regards,
-- 
Peter Griffin <peter.griffin@linaro.org>

[PATCH v2 1/2] phy: add new phy_notify_pmstate() api

[Peter Griffin]

Add a new phy_notify_pmstate() api that notifies and configures a phy for a
given PM link state transition.

This is intended to be by phy drivers which need to do some runtime
configuration of parameters during the transition that can't be handled by
phy_calibrate() or phy_power_{on|off}().

The first usage of this API is in the Samsung UFS phy that needs to issue
some register writes when entering and exiting the hibernate link state.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/phy/phy-core.c  | 25 +++++++++++++++++++++++++
 include/linux/phy/phy.h | 25 +++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 04a5a34e7a950ae94fae915673c25d476fc071c1..0b29bc2c709890d7fc27d1480a35cda6a826fd30 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -520,6 +520,31 @@ int phy_notify_disconnect(struct phy *phy, int port)
 }
 EXPORT_SYMBOL_GPL(phy_notify_disconnect);
 
+/**
+ * phy_notify_pmstate() - phy link state notification
+ * @phy: the phy returned by phy_get()
+ * @state: the link state
+ *
+ * Notify the phy of some PM link state transition. Used to notify and
+ * configure the phy accordingly.
+ *
+ * Returns: %0 if successful, a negative error code otherwise
+ */
+int phy_notify_pmstate(struct phy *phy, enum phy_linkstate state)
+{
+	int ret;
+
+	if (!phy || !phy->ops->notify_pmstate)
+		return 0;
+
+	mutex_lock(&phy->mutex);
+	ret = phy->ops->notify_pmstate(phy, state);
+	mutex_unlock(&phy->mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(phy_notify_pmstate);
+
 /**
  * phy_configure() - Changes the phy parameters
  * @phy: the phy returned by phy_get()
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 13add0c2c40721fe9ca3f0350d13c035cd25af45..d904ec4edb7e2be41fcf6ab780d3148c2ee8a950 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -53,6 +53,11 @@ enum phy_media {
 	PHY_MEDIA_DAC,
 };
 
+enum phy_linkstate {
+	PHY_UFS_HIBERN8_ENTER,
+	PHY_UFS_HIBERN8_EXIT,
+};
+
 /**
  * union phy_configure_opts - Opaque generic phy configuration
  *
@@ -132,6 +137,18 @@ struct phy_ops {
 	int	(*connect)(struct phy *phy, int port);
 	int	(*disconnect)(struct phy *phy, int port);
 
+	/**
+	 * @notify_pmstate:
+	 *
+	 * Optional.
+	 *
+	 * Used to notify and configure the phy for a PM link state
+	 * transition.
+	 *
+	 * Returns: 0 if successful, an negative error code otherwise
+	 */
+	int	(*notify_pmstate)(struct phy *phy, enum phy_linkstate state);
+
 	void	(*release)(struct phy *phy);
 	struct module *owner;
 };
@@ -255,6 +272,7 @@ int phy_reset(struct phy *phy);
 int phy_calibrate(struct phy *phy);
 int phy_notify_connect(struct phy *phy, int port);
 int phy_notify_disconnect(struct phy *phy, int port);
+int phy_notify_pmstate(struct phy *phy, enum phy_linkstate state);
 static inline int phy_get_bus_width(struct phy *phy)
 {
 	return phy->attrs.bus_width;
@@ -412,6 +430,13 @@ static inline int phy_notify_disconnect(struct phy *phy, int index)
 	return -ENOSYS;
 }
 
+static inline int phy_notify_pmstate(struct phy *phy, enum phy_linkstate state)
+{
+	if (!phy)
+		return 0;
+	return -ENOSYS;
+}
+
 static inline int phy_configure(struct phy *phy,
 				union phy_configure_opts *opts)
 {

-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v2 2/2] phy: samsung: gs101-ufs: Add .notify_pmstate() and hibern8 enter/exit values

[Peter Griffin]

Implement the .notify_pmstate() callback and provide the gs101 specific phy
values that need to be programmed when entering and exiting the hibern8 state.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/phy/samsung/phy-gs101-ufs.c   | 28 ++++++++++++++++++++++++++
 drivers/phy/samsung/phy-samsung-ufs.c | 38 +++++++++++++++++++++++++++++++++++
 drivers/phy/samsung/phy-samsung-ufs.h |  7 +++++++
 3 files changed, 73 insertions(+)

diff --git a/drivers/phy/samsung/phy-gs101-ufs.c b/drivers/phy/samsung/phy-gs101-ufs.c
index 17b798da5b5761f8e367599517d2d97bf0bb6b74..a15e1f453f7f3cecd6d3aa75217633ac4b6085d0 100644
--- a/drivers/phy/samsung/phy-gs101-ufs.c
+++ b/drivers/phy/samsung/phy-gs101-ufs.c
@@ -108,12 +108,39 @@ static const struct samsung_ufs_phy_cfg tensor_gs101_post_pwr_hs_config[] = {
 	END_UFS_PHY_CFG,
 };
 
+static const struct samsung_ufs_phy_cfg tensor_gs101_post_h8_enter[] = {
+	PHY_TRSV_REG_CFG_GS101(0x262, 0x08, PWR_MODE_ANY),
+	PHY_TRSV_REG_CFG_GS101(0x265, 0x0A, PWR_MODE_ANY),
+	PHY_COMN_REG_CFG(0x1, 0x8,  PWR_MODE_ANY),
+	PHY_COMN_REG_CFG(0x0, 0x86,  PWR_MODE_ANY),
+	PHY_COMN_REG_CFG(0x8, 0x60,  PWR_MODE_HS_ANY),
+	PHY_TRSV_REG_CFG_GS101(0x222, 0x08, PWR_MODE_HS_ANY),
+	PHY_TRSV_REG_CFG_GS101(0x246, 0x01, PWR_MODE_HS_ANY),
+	END_UFS_PHY_CFG,
+};
+
+static const struct samsung_ufs_phy_cfg tensor_gs101_pre_h8_exit[] = {
+	PHY_COMN_REG_CFG(0x0, 0xC6,  PWR_MODE_ANY),
+	PHY_COMN_REG_CFG(0x1, 0x0C,  PWR_MODE_ANY),
+	PHY_TRSV_REG_CFG_GS101(0x262, 0x00, PWR_MODE_ANY),
+	PHY_TRSV_REG_CFG_GS101(0x265, 0x00, PWR_MODE_ANY),
+	PHY_COMN_REG_CFG(0x8, 0xE0,  PWR_MODE_HS_ANY),
+	PHY_TRSV_REG_CFG_GS101(0x246, 0x03, PWR_MODE_HS_ANY),
+	PHY_TRSV_REG_CFG_GS101(0x222, 0x18, PWR_MODE_HS_ANY),
+	END_UFS_PHY_CFG,
+};
+
 static const struct samsung_ufs_phy_cfg *tensor_gs101_ufs_phy_cfgs[CFG_TAG_MAX] = {
 	[CFG_PRE_INIT]		= tensor_gs101_pre_init_cfg,
 	[CFG_PRE_PWR_HS]	= tensor_gs101_pre_pwr_hs_config,
 	[CFG_POST_PWR_HS]	= tensor_gs101_post_pwr_hs_config,
 };
 
+static const struct samsung_ufs_phy_cfg *tensor_gs101_hibern8_cfgs[] = {
+	[CFG_POST_HIBERN8_ENTER]	= tensor_gs101_post_h8_enter,
+	[CFG_PRE_HIBERN8_EXIT]		= tensor_gs101_pre_h8_exit,
+};
+
 static const char * const tensor_gs101_ufs_phy_clks[] = {
 	"ref_clk",
 };
@@ -170,6 +197,7 @@ static int gs101_phy_wait_for_cdr_lock(struct phy *phy, u8 lane)
 
 const struct samsung_ufs_phy_drvdata tensor_gs101_ufs_phy = {
 	.cfgs = tensor_gs101_ufs_phy_cfgs,
+	.cfgs_hibern8 = tensor_gs101_hibern8_cfgs,
 	.isol = {
 		.offset = TENSOR_GS101_PHY_CTRL,
 		.mask = TENSOR_GS101_PHY_CTRL_MASK,
diff --git a/drivers/phy/samsung/phy-samsung-ufs.c b/drivers/phy/samsung/phy-samsung-ufs.c
index f3cbe6b17b235bb181b3fae628d75822f0c9183a..b0dc40c19d2ed85dbdb74aff768a24f03b4b3f49 100644
--- a/drivers/phy/samsung/phy-samsung-ufs.c
+++ b/drivers/phy/samsung/phy-samsung-ufs.c
@@ -217,6 +217,42 @@ static int samsung_ufs_phy_set_mode(struct phy *generic_phy,
 	return 0;
 }
 
+static int samsung_ufs_phy_notify_pmstate(struct phy *phy,
+					  enum phy_linkstate pmstate)
+{
+	struct samsung_ufs_phy *ufs_phy = get_samsung_ufs_phy(phy);
+	const struct samsung_ufs_phy_cfg *cfg;
+	int i, err;
+
+	if (!ufs_phy->cfgs_hibern8)
+		return 0;
+
+	if (pmstate == PHY_UFS_HIBERN8_ENTER)
+		cfg = ufs_phy->cfgs_hibern8[CFG_POST_HIBERN8_ENTER];
+	else if (pmstate == PHY_UFS_HIBERN8_EXIT)
+		cfg = ufs_phy->cfgs_hibern8[CFG_PRE_HIBERN8_EXIT];
+
+	for_each_phy_cfg(cfg) {
+		for_each_phy_lane(ufs_phy, i) {
+			samsung_ufs_phy_config(ufs_phy, cfg, i);
+		}
+	}
+
+	if (pmstate == PHY_UFS_HIBERN8_EXIT) {
+		for_each_phy_lane(ufs_phy, i) {
+			if (ufs_phy->drvdata->wait_for_cdr) {
+				err = ufs_phy->drvdata->wait_for_cdr(phy, i);
+				if (err)
+					goto err_out;
+			}
+		}
+	}
+
+	return 0;
+err_out:
+	return err;
+}
+
 static int samsung_ufs_phy_exit(struct phy *phy)
 {
 	struct samsung_ufs_phy *ss_phy = get_samsung_ufs_phy(phy);
@@ -233,6 +269,7 @@ static const struct phy_ops samsung_ufs_phy_ops = {
 	.power_off	= samsung_ufs_phy_power_off,
 	.calibrate	= samsung_ufs_phy_calibrate,
 	.set_mode	= samsung_ufs_phy_set_mode,
+	.notify_pmstate = samsung_ufs_phy_notify_pmstate,
 	.owner          = THIS_MODULE,
 };
 
@@ -287,6 +324,7 @@ static int samsung_ufs_phy_probe(struct platform_device *pdev)
 	phy->dev = dev;
 	phy->drvdata = drvdata;
 	phy->cfgs = drvdata->cfgs;
+	phy->cfgs_hibern8 = drvdata->cfgs_hibern8;
 	memcpy(&phy->isol, &drvdata->isol, sizeof(phy->isol));
 
 	if (!of_property_read_u32_index(dev->of_node, "samsung,pmu-syscon", 1,
diff --git a/drivers/phy/samsung/phy-samsung-ufs.h b/drivers/phy/samsung/phy-samsung-ufs.h
index a28f148081d168344b47f2798b00cb098f0a8574..f2c2e744e5bae87c9cfcaa17f4a09456f134966a 100644
--- a/drivers/phy/samsung/phy-samsung-ufs.h
+++ b/drivers/phy/samsung/phy-samsung-ufs.h
@@ -92,6 +92,11 @@ enum {
 	CFG_TAG_MAX,
 };
 
+enum {
+	CFG_POST_HIBERN8_ENTER,
+	CFG_PRE_HIBERN8_EXIT,
+};
+
 struct samsung_ufs_phy_cfg {
 	u32 off_0;
 	u32 off_1;
@@ -108,6 +113,7 @@ struct samsung_ufs_phy_pmu_isol {
 
 struct samsung_ufs_phy_drvdata {
 	const struct samsung_ufs_phy_cfg **cfgs;
+	const struct samsung_ufs_phy_cfg **cfgs_hibern8;
 	struct samsung_ufs_phy_pmu_isol isol;
 	const char * const *clk_list;
 	int num_clks;
@@ -124,6 +130,7 @@ struct samsung_ufs_phy {
 	struct clk_bulk_data *clks;
 	const struct samsung_ufs_phy_drvdata *drvdata;
 	const struct samsung_ufs_phy_cfg * const *cfgs;
+	const struct samsung_ufs_phy_cfg * const *cfgs_hibern8;
 	struct samsung_ufs_phy_pmu_isol isol;
 	u8 lane_cnt;
 	int ufs_phy_state;

-- 
2.50.0.727.gbf7dc18ff4-goog

Re: [PATCH v2 1/2] phy: add new phy_notify_pmstate() api

[Vinod Koul]

On 22-07-25, 22:04, Manivannan Sadhasivam wrote:
> On Thu, Jul 03, 2025 at 02:03:22PM GMT, Peter Griffin wrote:
> > Add a new phy_notify_pmstate() api that notifies and configures a phy for a
> > given PM link state transition.
> > 
> > This is intended to be by phy drivers which need to do some runtime
> > configuration of parameters during the transition that can't be handled by
> > phy_calibrate() or phy_power_{on|off}().
> > 
> > The first usage of this API is in the Samsung UFS phy that needs to issue
> > some register writes when entering and exiting the hibernate link state.
> > 
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > ---
> >  drivers/phy/phy-core.c  | 25 +++++++++++++++++++++++++
> >  include/linux/phy/phy.h | 25 +++++++++++++++++++++++++
> >  2 files changed, 50 insertions(+)
> > 
> > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> > index 04a5a34e7a950ae94fae915673c25d476fc071c1..0b29bc2c709890d7fc27d1480a35cda6a826fd30 100644
> > --- a/drivers/phy/phy-core.c
> > +++ b/drivers/phy/phy-core.c
> > @@ -520,6 +520,31 @@ int phy_notify_disconnect(struct phy *phy, int port)
> >  }
> >  EXPORT_SYMBOL_GPL(phy_notify_disconnect);
> >  
> > +/**
> > + * phy_notify_pmstate() - phy link state notification
> 
> 'pmstate' doesn't correspond to 'link state'. So how about,
> phy_notify_link_state()?
> 
> s/phy/PHY (here and below)
> 
> > + * @phy: the phy returned by phy_get()
> > + * @state: the link state
> > + *
> > + * Notify the phy of some PM link state transition. Used to notify and
> 
> Link state change is common for the PHY. So remove 'PM'.

Is it really link or phy state?

> 
> > + * configure the phy accordingly.
> > + *
> > + * Returns: %0 if successful, a negative error code otherwise
> > + */
> > +int phy_notify_pmstate(struct phy *phy, enum phy_linkstate state)
> 
> I think you need to use 'int state' and let drivers pass their own link state
> values. You cannot have generic link states across all peripherals.

I would avoid that, people start overloading this if we let it keep
open! I would like to avoid the api -(ab)use

-- 
~Vinod

Re: [PATCH v2 1/2] phy: add new phy_notify_pmstate() api

[neil.armstrong]

On 23/07/2025 09:07, Vinod Koul wrote:
> On 22-07-25, 22:04, Manivannan Sadhasivam wrote:
>> On Thu, Jul 03, 2025 at 02:03:22PM GMT, Peter Griffin wrote:
>>> Add a new phy_notify_pmstate() api that notifies and configures a phy for a
>>> given PM link state transition.
>>>
>>> This is intended to be by phy drivers which need to do some runtime
>>> configuration of parameters during the transition that can't be handled by
>>> phy_calibrate() or phy_power_{on|off}().
>>>
>>> The first usage of this API is in the Samsung UFS phy that needs to issue
>>> some register writes when entering and exiting the hibernate link state.
>>>
>>> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
>>> ---
>>>   drivers/phy/phy-core.c  | 25 +++++++++++++++++++++++++
>>>   include/linux/phy/phy.h | 25 +++++++++++++++++++++++++
>>>   2 files changed, 50 insertions(+)
>>>
>>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>>> index 04a5a34e7a950ae94fae915673c25d476fc071c1..0b29bc2c709890d7fc27d1480a35cda6a826fd30 100644
>>> --- a/drivers/phy/phy-core.c
>>> +++ b/drivers/phy/phy-core.c
>>> @@ -520,6 +520,31 @@ int phy_notify_disconnect(struct phy *phy, int port)
>>>   }
>>>   EXPORT_SYMBOL_GPL(phy_notify_disconnect);
>>>   
>>> +/**
>>> + * phy_notify_pmstate() - phy link state notification
>>
>> 'pmstate' doesn't correspond to 'link state'. So how about,
>> phy_notify_link_state()?
>>
>> s/phy/PHY (here and below)
>>
>>> + * @phy: the phy returned by phy_get()
>>> + * @state: the link state
>>> + *
>>> + * Notify the phy of some PM link state transition. Used to notify and
>>
>> Link state change is common for the PHY. So remove 'PM'.
> 
> Is it really link or phy state?

It seems to be a link state, and I think adding a way to force
a link state to a phy, which is basically the role of a phy,
more coherent.

Neil

> 
>>
>>> + * configure the phy accordingly.
>>> + *
>>> + * Returns: %0 if successful, a negative error code otherwise
>>> + */
>>> +int phy_notify_pmstate(struct phy *phy, enum phy_linkstate state)
>>
>> I think you need to use 'int state' and let drivers pass their own link state
>> values. You cannot have generic link states across all peripherals.
> 
> I would avoid that, people start overloading this if we let it keep
> open! I would like to avoid the api -(ab)use
> 

Re: [PATCH v2 1/2] phy: add new phy_notify_pmstate() api

[Manivannan Sadhasivam]

On Wed, Jul 23, 2025 at 12:37:31PM GMT, Vinod Koul wrote:
> On 22-07-25, 22:04, Manivannan Sadhasivam wrote:
> > On Thu, Jul 03, 2025 at 02:03:22PM GMT, Peter Griffin wrote:
> > > Add a new phy_notify_pmstate() api that notifies and configures a phy for a
> > > given PM link state transition.
> > > 
> > > This is intended to be by phy drivers which need to do some runtime
> > > configuration of parameters during the transition that can't be handled by
> > > phy_calibrate() or phy_power_{on|off}().
> > > 
> > > The first usage of this API is in the Samsung UFS phy that needs to issue
> > > some register writes when entering and exiting the hibernate link state.
> > > 
> > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > > ---
> > >  drivers/phy/phy-core.c  | 25 +++++++++++++++++++++++++
> > >  include/linux/phy/phy.h | 25 +++++++++++++++++++++++++
> > >  2 files changed, 50 insertions(+)
> > > 
> > > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> > > index 04a5a34e7a950ae94fae915673c25d476fc071c1..0b29bc2c709890d7fc27d1480a35cda6a826fd30 100644
> > > --- a/drivers/phy/phy-core.c
> > > +++ b/drivers/phy/phy-core.c
> > > @@ -520,6 +520,31 @@ int phy_notify_disconnect(struct phy *phy, int port)
> > >  }
> > >  EXPORT_SYMBOL_GPL(phy_notify_disconnect);
> > >  
> > > +/**
> > > + * phy_notify_pmstate() - phy link state notification
> > 
> > 'pmstate' doesn't correspond to 'link state'. So how about,
> > phy_notify_link_state()?
> > 
> > s/phy/PHY (here and below)
> > 
> > > + * @phy: the phy returned by phy_get()
> > > + * @state: the link state
> > > + *
> > > + * Notify the phy of some PM link state transition. Used to notify and
> > 
> > Link state change is common for the PHY. So remove 'PM'.
> 
> Is it really link or phy state?
> 

This is a bit of ambiguity. But as per the spec, Hibern8 is the low power state
of the M-PHY and Unipro controller.

Maybe, phy_notify_state()?

> > 
> > > + * configure the phy accordingly.
> > > + *
> > > + * Returns: %0 if successful, a negative error code otherwise
> > > + */
> > > +int phy_notify_pmstate(struct phy *phy, enum phy_linkstate state)
> > 
> > I think you need to use 'int state' and let drivers pass their own link state
> > values. You cannot have generic link states across all peripherals.
> 
> I would avoid that, people start overloading this if we let it keep
> open! I would like to avoid the api -(ab)use
> 

Then we will end up with peripheral specific enums in include/linux/phy/phy.h.
If you are OK with that, fine with me.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 1/2] phy: add new phy_notify_pmstate() api

[Peter Griffin]

Hi Vinod, Mani & Neil,

Thanks a lot for the valuable review feedback.

On Wed, 23 Jul 2025 at 09:04, Manivannan Sadhasivam <mani@kernel.org> wrote:
>
> On Wed, Jul 23, 2025 at 12:37:31PM GMT, Vinod Koul wrote:
> > On 22-07-25, 22:04, Manivannan Sadhasivam wrote:
> > > On Thu, Jul 03, 2025 at 02:03:22PM GMT, Peter Griffin wrote:
> > > > Add a new phy_notify_pmstate() api that notifies and configures a phy for a
> > > > given PM link state transition.
> > > >
> > > > This is intended to be by phy drivers which need to do some runtime
> > > > configuration of parameters during the transition that can't be handled by
> > > > phy_calibrate() or phy_power_{on|off}().
> > > >
> > > > The first usage of this API is in the Samsung UFS phy that needs to issue
> > > > some register writes when entering and exiting the hibernate link state.
> > > >
> > > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > > > ---
> > > >  drivers/phy/phy-core.c  | 25 +++++++++++++++++++++++++
> > > >  include/linux/phy/phy.h | 25 +++++++++++++++++++++++++
> > > >  2 files changed, 50 insertions(+)
> > > >
> > > > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> > > > index 04a5a34e7a950ae94fae915673c25d476fc071c1..0b29bc2c709890d7fc27d1480a35cda6a826fd30 100644
> > > > --- a/drivers/phy/phy-core.c
> > > > +++ b/drivers/phy/phy-core.c
> > > > @@ -520,6 +520,31 @@ int phy_notify_disconnect(struct phy *phy, int port)
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(phy_notify_disconnect);
> > > >
> > > > +/**
> > > > + * phy_notify_pmstate() - phy link state notification
> > >
> > > 'pmstate' doesn't correspond to 'link state'. So how about,
> > > phy_notify_link_state()?
> > >
> > > s/phy/PHY (here and below)

will fix

> > >
> > > > + * @phy: the phy returned by phy_get()
> > > > + * @state: the link state
> > > > + *
> > > > + * Notify the phy of some PM link state transition. Used to notify and
> > >
> > > Link state change is common for the PHY. So remove 'PM'.
> >
> > Is it really link or phy state?

I think it is likely both link and phy state.

Looking at the wording in section '9.5 Hibernate' in mipi unipro 1.8
spec we have phrases such as

9.5 Hibernate "Hibernate is a UniPro state in which the PHY is in the
HIBERNATE_STATE, and the UniPro stack keeps only a minimal set of
features active."

9.5 Figure 99 describes Link hibernation where one Device, typically a
Host Device, initiates Link hibernation with a peer Device.

9.5.1.2 The local PA Layer receives this request and places the M-PHY
Link into hibernate using the PACP protocol (detailed description of
PACP protocol can be found in Section 5.7.7). Once the PA Layer has
successfully hibernated the M-PHY Link, subsequent layers of the local
and peer
UniPro stack (L4 to L2) shall be hibernated by the DME by sending a
<layer-identifer>_LM_HIBERNATE_ENTER.req SAP primitive to the
respective layers.

> >
>
> This is a bit of ambiguity. But as per the spec, Hibern8 is the low power state
> of the M-PHY and Unipro controller.
>
> Maybe, phy_notify_state()?
>

phy_notify_state() seems like a good name. It might be better suited
for other peripherals as well rather than narrowing it with link_state
or pmstate.

Vinod, any thoughts on your preferred name?

> > >

> > > > + * configure the phy accordingly.
> > > > + *
> > > > + * Returns: %0 if successful, a negative error code otherwise
> > > > + */
> > > > +int phy_notify_pmstate(struct phy *phy, enum phy_linkstate state)
> > >
> > > I think you need to use 'int state' and let drivers pass their own link state
> > > values. You cannot have generic link states across all peripherals.
> >
> > I would avoid that, people start overloading this if we let it keep
> > open! I would like to avoid the api -(ab)use
> >
>
> Then we will end up with peripheral specific enums in include/linux/phy/phy.h.
> If you are OK with that, fine with me.

Ok I'll add peripheral specific enums to include/linux/phy/phy.h in
the next version.

Thanks,

Peter

Re: [PATCH v2 1/2] phy: add new phy_notify_pmstate() api

[Neil Armstrong]

Hi,

On 25/07/2025 12:21, Peter Griffin wrote:
> Hi Vinod, Mani & Neil,
> 
> Thanks a lot for the valuable review feedback.
> 
> On Wed, 23 Jul 2025 at 09:04, Manivannan Sadhasivam <mani@kernel.org> wrote:
>>
>> On Wed, Jul 23, 2025 at 12:37:31PM GMT, Vinod Koul wrote:
>>> On 22-07-25, 22:04, Manivannan Sadhasivam wrote:
>>>> On Thu, Jul 03, 2025 at 02:03:22PM GMT, Peter Griffin wrote:
>>>>> Add a new phy_notify_pmstate() api that notifies and configures a phy for a
>>>>> given PM link state transition.
>>>>>
>>>>> This is intended to be by phy drivers which need to do some runtime
>>>>> configuration of parameters during the transition that can't be handled by
>>>>> phy_calibrate() or phy_power_{on|off}().
>>>>>
>>>>> The first usage of this API is in the Samsung UFS phy that needs to issue
>>>>> some register writes when entering and exiting the hibernate link state.
>>>>>
>>>>> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
>>>>> ---
>>>>>   drivers/phy/phy-core.c  | 25 +++++++++++++++++++++++++
>>>>>   include/linux/phy/phy.h | 25 +++++++++++++++++++++++++
>>>>>   2 files changed, 50 insertions(+)
>>>>>
>>>>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>>>>> index 04a5a34e7a950ae94fae915673c25d476fc071c1..0b29bc2c709890d7fc27d1480a35cda6a826fd30 100644
>>>>> --- a/drivers/phy/phy-core.c
>>>>> +++ b/drivers/phy/phy-core.c
>>>>> @@ -520,6 +520,31 @@ int phy_notify_disconnect(struct phy *phy, int port)
>>>>>   }
>>>>>   EXPORT_SYMBOL_GPL(phy_notify_disconnect);
>>>>>
>>>>> +/**
>>>>> + * phy_notify_pmstate() - phy link state notification
>>>>
>>>> 'pmstate' doesn't correspond to 'link state'. So how about,
>>>> phy_notify_link_state()?
>>>>
>>>> s/phy/PHY (here and below)
> 
> will fix
> 
>>>>
>>>>> + * @phy: the phy returned by phy_get()
>>>>> + * @state: the link state
>>>>> + *
>>>>> + * Notify the phy of some PM link state transition. Used to notify and
>>>>
>>>> Link state change is common for the PHY. So remove 'PM'.
>>>
>>> Is it really link or phy state?
> 
> I think it is likely both link and phy state.
> 
> Looking at the wording in section '9.5 Hibernate' in mipi unipro 1.8
> spec we have phrases such as
> 
> 9.5 Hibernate "Hibernate is a UniPro state in which the PHY is in the
> HIBERNATE_STATE, and the UniPro stack keeps only a minimal set of
> features active."
> 
> 9.5 Figure 99 describes Link hibernation where one Device, typically a
> Host Device, initiates Link hibernation with a peer Device.

So this is part handled by the PHY

> 
> 9.5.1.2 The local PA Layer receives this request and places the M-PHY
> Link into hibernate using the PACP protocol (detailed description of
> PACP protocol can be found in Section 5.7.7). Once the PA Layer has
> successfully hibernated the M-PHY Link, subsequent layers of the local
> and peer
> UniPro stack (L4 to L2) shall be hibernated by the DME by sending a
> <layer-identifer>_LM_HIBERNATE_ENTER.req SAP primitive to the
> respective layers.
> 

And this by the controller, so yeah we set the PHY state, and the PHY will
set the link state accordingly.

>>>
>>
>> This is a bit of ambiguity. But as per the spec, Hibern8 is the low power state
>> of the M-PHY and Unipro controller.
>>
>> Maybe, phy_notify_state()?
>>
> 
> phy_notify_state() seems like a good name. It might be better suited
> for other peripherals as well rather than narrowing it with link_state
> or pmstate.

Ack

> 
> Vinod, any thoughts on your preferred name?
> 
>>>>
> 
>>>>> + * configure the phy accordingly.
>>>>> + *
>>>>> + * Returns: %0 if successful, a negative error code otherwise
>>>>> + */
>>>>> +int phy_notify_pmstate(struct phy *phy, enum phy_linkstate state)
>>>>
>>>> I think you need to use 'int state' and let drivers pass their own link state
>>>> values. You cannot have generic link states across all peripherals.
>>>
>>> I would avoid that, people start overloading this if we let it keep
>>> open! I would like to avoid the api -(ab)use
>>>
>>
>> Then we will end up with peripheral specific enums in include/linux/phy/phy.h.
>> If you are OK with that, fine with me.
> 
> Ok I'll add peripheral specific enums to include/linux/phy/phy.h in
> the next version.
> 
> Thanks,
> 
> Peter
> 

Thanks,
Neil