[PATCH 0/3] Add phy support for hibern8 enter & exit on gs101

[PATCH 0/3] Add phy support for hibern8 enter & exit on gs101

[Peter Griffin]

Hi Vinod,

This series adds two new phy modes to support UFS HIBERN8_ENTER and
HIBERN8_EXIT.

This is used for UFS phys that need to program specific values when entering
and exiting from ufs hibern8 state. Once such platform that needs to do this is
gs101 SoC found on Pixel 6.

phy-gs101-ufs is also updated to include these hibern8 enter/exit specific tuning
values.

Note: a separate series will be sent for ufs-exynos driver that makes use of
this new UFS phy functionality.

regards,

Peter

Peter Griffin (3):
  phy: Add UFS phy hibernate modes
  phy: samsung-ufs: add support for HIBERN8_ENTER and HIBERN8_EXIT
  phy: samsung: gs101-ufs: Add hibern8 enter and exit specific tuning
    values

 drivers/phy/samsung/phy-gs101-ufs.c   | 31 ++++++++++++++++++++++++---
 drivers/phy/samsung/phy-samsung-ufs.c | 18 ++++++++++++++--
 drivers/phy/samsung/phy-samsung-ufs.h |  2 ++
 include/linux/phy/phy.h               |  4 +++-
 4 files changed, 49 insertions(+), 6 deletions(-)

-- 
2.46.1.824.gd892dcdcdd-goog

[PATCH 1/3] phy: Add UFS phy hibernate modes

[Peter Griffin]

Some UFS phys need to write hibernation specific values
when entering and exiting hibernate state.

Add two new UFS phy modes to the phy framework so that this
is possible. One such platform that requires this is Pixel 6
which uses the gs101 SoC.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 include/linux/phy/phy.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 03cd5bae92d3..1874e55e2bb9 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -42,7 +42,9 @@ enum phy_mode {
 	PHY_MODE_MIPI_DPHY,
 	PHY_MODE_SATA,
 	PHY_MODE_LVDS,
-	PHY_MODE_DP
+	PHY_MODE_DP,
+	PHY_MODE_UFS_HIBERN8_ENTER,
+	PHY_MODE_UFS_HIBERN8_EXIT,
 };
 
 enum phy_media {
-- 
2.46.1.824.gd892dcdcdd-goog

[PATCH 2/3] phy: samsung-ufs: add support for HIBERN8_ENTER and HIBERN8_EXIT

[Peter Griffin]

Add two new states CFG_POST_HIBERN8_ENTER and CFG_PRE_HIBERN8_EXIT to
the phy driver which map to the new PHY_MODE_UFS_HIBERN8_ENTER
and PHY_MODE_UFS_HIBERN8_EXIT modes.

These are used to program phy specific calibration values when entering
and exiting hibern8. When exiting from hibern8 state we also update the
logic to wait for cdr lock.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/phy/samsung/phy-samsung-ufs.c | 18 ++++++++++++++++--
 drivers/phy/samsung/phy-samsung-ufs.h |  2 ++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/samsung/phy-samsung-ufs.c b/drivers/phy/samsung/phy-samsung-ufs.c
index 6c5d41552649..7162ae5c7bc8 100644
--- a/drivers/phy/samsung/phy-samsung-ufs.c
+++ b/drivers/phy/samsung/phy-samsung-ufs.c
@@ -87,6 +87,12 @@ static int samsung_ufs_phy_calibrate(struct phy *phy)
 		return -EINVAL;
 	}
 
+	if (ufs_phy->mode == PHY_MODE_UFS_HIBERN8_ENTER)
+		ufs_phy->ufs_phy_state = CFG_POST_HIBERN8_ENTER;
+
+	if (ufs_phy->mode == PHY_MODE_UFS_HIBERN8_EXIT)
+		ufs_phy->ufs_phy_state = CFG_PRE_HIBERN8_EXIT;
+
 	cfg = cfgs[ufs_phy->ufs_phy_state];
 	if (!cfg)
 		goto out;
@@ -105,8 +111,9 @@ static int samsung_ufs_phy_calibrate(struct phy *phy)
 				goto out;
 		}
 
-		if (ufs_phy->ufs_phy_state == CFG_POST_PWR_HS &&
-		    ufs_phy->drvdata->wait_for_cdr) {
+		if ((ufs_phy->ufs_phy_state == CFG_POST_PWR_HS ||
+		     ufs_phy->ufs_phy_state == CFG_PRE_HIBERN8_EXIT) &&
+		     ufs_phy->drvdata->wait_for_cdr) {
 			err = ufs_phy->drvdata->wait_for_cdr(phy, i);
 			if (err)
 				goto out;
@@ -137,6 +144,13 @@ static int samsung_ufs_phy_calibrate(struct phy *phy)
 		/* Change back to INIT state */
 		ufs_phy->ufs_phy_state = CFG_PRE_INIT;
 		break;
+	case CFG_POST_HIBERN8_ENTER:
+		ufs_phy->ufs_phy_state = CFG_PRE_HIBERN8_EXIT;
+		break;
+	case CFG_PRE_HIBERN8_EXIT:
+		/* Change back to INIT state */
+		ufs_phy->ufs_phy_state = CFG_PRE_INIT;
+		break;
 	default:
 		dev_err(ufs_phy->dev, "wrong state for phy calibration\n");
 	}
diff --git a/drivers/phy/samsung/phy-samsung-ufs.h b/drivers/phy/samsung/phy-samsung-ufs.h
index 9b7deef6e10f..b42ed586aba8 100644
--- a/drivers/phy/samsung/phy-samsung-ufs.h
+++ b/drivers/phy/samsung/phy-samsung-ufs.h
@@ -89,6 +89,8 @@ enum {
 	CFG_POST_INIT,
 	CFG_PRE_PWR_HS,
 	CFG_POST_PWR_HS,
+	CFG_POST_HIBERN8_ENTER,
+	CFG_PRE_HIBERN8_EXIT,
 	CFG_TAG_MAX,
 };
 
-- 
2.46.1.824.gd892dcdcdd-goog

[PATCH 3/3] phy: samsung: gs101-ufs: Add hibern8 enter and exit specific tuning values

[Peter Griffin]

Add the gs101 specific phy calibration values that need to be programmed
when entering and exiting hibern8 state.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/phy/samsung/phy-gs101-ufs.c | 31 ++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/samsung/phy-gs101-ufs.c b/drivers/phy/samsung/phy-gs101-ufs.c
index 17b798da5b57..5363d8be6c76 100644
--- a/drivers/phy/samsung/phy-gs101-ufs.c
+++ b/drivers/phy/samsung/phy-gs101-ufs.c
@@ -108,10 +108,35 @@ 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,
+	[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,
+	[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[] = {
-- 
2.46.1.824.gd892dcdcdd-goog

Re: [PATCH 1/3] phy: Add UFS phy hibernate modes

[Vinod Koul]

Hi Peter,

On 02-10-24, 21:15, Peter Griffin wrote:
> Some UFS phys need to write hibernation specific values
> when entering and exiting hibernate state.
> 
> Add two new UFS phy modes to the phy framework so that this
> is possible. One such platform that requires this is Pixel 6
> which uses the gs101 SoC.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  include/linux/phy/phy.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 03cd5bae92d3..1874e55e2bb9 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -42,7 +42,9 @@ enum phy_mode {
>  	PHY_MODE_MIPI_DPHY,
>  	PHY_MODE_SATA,
>  	PHY_MODE_LVDS,
> -	PHY_MODE_DP
> +	PHY_MODE_DP,
> +	PHY_MODE_UFS_HIBERN8_ENTER,
> +	PHY_MODE_UFS_HIBERN8_EXIT,

I am not sure I like this. why should this be the model? Phy drivers
should listen to pm events and handle this in pm_suspend/resume calls,
why do we need this special mode here...
Also, this is not a "mode" for phy

-- 
~Vinod

Re: [PATCH 1/3] phy: Add UFS phy hibernate modes

[Peter Griffin]

Hi Vinod,

Thanks for your review.

On Mon, 7 Oct 2024 at 07:28, Vinod Koul <vkoul@kernel.org> wrote:
>
> Hi Peter,
>
> On 02-10-24, 21:15, Peter Griffin wrote:
> > Some UFS phys need to write hibernation specific values
> > when entering and exiting hibernate state.
> >
> > Add two new UFS phy modes to the phy framework so that this
> > is possible. One such platform that requires this is Pixel 6
> > which uses the gs101 SoC.
> >
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > ---
> >  include/linux/phy/phy.h | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> > index 03cd5bae92d3..1874e55e2bb9 100644
> > --- a/include/linux/phy/phy.h
> > +++ b/include/linux/phy/phy.h
> > @@ -42,7 +42,9 @@ enum phy_mode {
> >       PHY_MODE_MIPI_DPHY,
> >       PHY_MODE_SATA,
> >       PHY_MODE_LVDS,
> > -     PHY_MODE_DP
> > +     PHY_MODE_DP,
> > +     PHY_MODE_UFS_HIBERN8_ENTER,
> > +     PHY_MODE_UFS_HIBERN8_EXIT,
>
> I am not sure I like this. why should this be the model? Phy drivers
> should listen to pm events and handle this in pm_suspend/resume calls,
> why do we need this special mode here...

There are a couple of reasons I added it here:

1) Whilst link hibern8 mode can be used as part of runtime PM and
system PM, it is also used outside of those contexts by ufshcd.c. The
host controller can enable UFSHCD_CAP_HIBERN8_WITH_CLK_GATING (which
will be the case for gs101 / Pixel 6) and the UFS clocks are gated and
link put into hibern8 mode for periods of inactivity. When that
happens the rest of the system isn't entering any sort of sleep state.

2) From looking at the existing code upstream ufs-qcom.c and
phy-qcom-qmp-ufs.c look to have similar requirements in that it needs
to program a set of specific register values depending on the UFS
gear. To achieve that they added PHY_MODE_UFS_HS_B and
PHY_MODE_UFS_HS_A modes here and then use phy_set_mode_ext() API in
ufs_qcom_power_up_sequence() to signal to the phy driver the UFS gear,
which is then used to choose which set of values to program to the
phy.

The two new UFS phy modes added here for hibern8 are for a very
similar purpose (to choose a bunch of register values to program), so
I considered it consistent with what was already being done upstream
to signal between UFS host drivers and UFS phy drivers. Arguably I
guess we could have one "mode" PHY_MODE_UFS_HIBERN8 and use the
submode parameter to indicate whether we are entering (1) or exiting
(0) from it. I wasn't really sure what the rules/guidelines for the
submode parameter were though.

Thanks,

Peter

Re: [PATCH 1/3] phy: Add UFS phy hibernate modes

[Vinod Koul]

On 08-10-24, 11:30, Peter Griffin wrote:

> > > +     PHY_MODE_DP,
> > > +     PHY_MODE_UFS_HIBERN8_ENTER,
> > > +     PHY_MODE_UFS_HIBERN8_EXIT,
> >
> > I am not sure I like this. why should this be the model? Phy drivers
> > should listen to pm events and handle this in pm_suspend/resume calls,
> > why do we need this special mode here...
> 
> There are a couple of reasons I added it here:
> 
> 1) Whilst link hibern8 mode can be used as part of runtime PM and
> system PM, it is also used outside of those contexts by ufshcd.c. The
> host controller can enable UFSHCD_CAP_HIBERN8_WITH_CLK_GATING (which
> will be the case for gs101 / Pixel 6) and the UFS clocks are gated and
> link put into hibern8 mode for periods of inactivity. When that
> happens the rest of the system isn't entering any sort of sleep state.
> 
> 2) From looking at the existing code upstream ufs-qcom.c and
> phy-qcom-qmp-ufs.c look to have similar requirements in that it needs
> to program a set of specific register values depending on the UFS
> gear. To achieve that they added PHY_MODE_UFS_HS_B and
> PHY_MODE_UFS_HS_A modes here and then use phy_set_mode_ext() API in
> ufs_qcom_power_up_sequence() to signal to the phy driver the UFS gear,
> which is then used to choose which set of values to program to the
> phy.
> 
> The two new UFS phy modes added here for hibern8 are for a very
> similar purpose (to choose a bunch of register values to program), so
> I considered it consistent with what was already being done upstream
> to signal between UFS host drivers and UFS phy drivers. Arguably I
> guess we could have one "mode" PHY_MODE_UFS_HIBERN8 and use the
> submode parameter to indicate whether we are entering (1) or exiting
> (0) from it. I wasn't really sure what the rules/guidelines for the
> submode parameter were though.

Yes but not exactly. The HIBERN8_ENTER|EXIT sound like PM events rather
than a PHY mode. If this is resultant from inactivity, then we should
hook this up to runtime pm ?

-- 
~Vinod

Re: [PATCH 1/3] phy: Add UFS phy hibernate modes

[Peter Griffin]

Hi Vinod,

Thanks for your feedback!

On Thu, 17 Oct 2024 at 13:32, Vinod Koul <vkoul@kernel.org> wrote:
>
> On 08-10-24, 11:30, Peter Griffin wrote:
>
> > > > +     PHY_MODE_DP,
> > > > +     PHY_MODE_UFS_HIBERN8_ENTER,
> > > > +     PHY_MODE_UFS_HIBERN8_EXIT,
> > >
> > > I am not sure I like this. why should this be the model? Phy drivers
> > > should listen to pm events and handle this in pm_suspend/resume calls,
> > > why do we need this special mode here...
> >
> > There are a couple of reasons I added it here:
> >
> > 1) Whilst link hibern8 mode can be used as part of runtime PM and
> > system PM, it is also used outside of those contexts by ufshcd.c. The
> > host controller can enable UFSHCD_CAP_HIBERN8_WITH_CLK_GATING (which
> > will be the case for gs101 / Pixel 6) and the UFS clocks are gated and
> > link put into hibern8 mode for periods of inactivity. When that
> > happens the rest of the system isn't entering any sort of sleep state.
> >
> > 2) From looking at the existing code upstream ufs-qcom.c and
> > phy-qcom-qmp-ufs.c look to have similar requirements in that it needs
> > to program a set of specific register values depending on the UFS
> > gear. To achieve that they added PHY_MODE_UFS_HS_B and
> > PHY_MODE_UFS_HS_A modes here and then use phy_set_mode_ext() API in
> > ufs_qcom_power_up_sequence() to signal to the phy driver the UFS gear,
> > which is then used to choose which set of values to program to the
> > phy.
> >
> > The two new UFS phy modes added here for hibern8 are for a very
> > similar purpose (to choose a bunch of register values to program), so
> > I considered it consistent with what was already being done upstream
> > to signal between UFS host drivers and UFS phy drivers. Arguably I
> > guess we could have one "mode" PHY_MODE_UFS_HIBERN8 and use the
> > submode parameter to indicate whether we are entering (1) or exiting
> > (0) from it. I wasn't really sure what the rules/guidelines for the
> > submode parameter were though.
>
> Yes but not exactly. The HIBERN8_ENTER|EXIT sound like PM events rather
> than a PHY mode. If this is resultant from inactivity, then we should
> hook this up to runtime pm ?

As I mentioned above HIBERN8 is implemented and runs independently of
runtime pm in the upstream UFS stack. To be clear that isn't a
ufs-exynos thing, but rather the core ufs stack in
drivers/ufs/core/ufshcd.c.

I've added Bart, Alim, Avri to this thread, as they are listed as
reviewers of the core ufshcd file and also Sahitya who originally
introduced this UFSHCD_CAP_HIBERN8_WITH_CLK_GATING functionality.
Hopefully they can provide more details as to why hibern8 has been
implemented like this outside of runtime pm. The original commit where
this was introduced is

commit 1ab27c9cf8b63dd8dec9e17b5c17721c7f3b6cc7
Author: Sahitya Tummala <quic_stummala@quicinc.com>
Date:   Thu Sep 25 15:32:32 2014 +0300

    ufs: Add support for clock gating

which mentions in the description that the timeout is typically less
than the runtime suspend timeout.

I think the main difference here versus other SoC UFS controllers and
UFS phys is that the others don't appear to need any additional phy
register writes when entering/leaving hibern8, which the ufs-exynos
phy does. It looks like all the hibern8 callbacks upstream in the core
UFS stack were added by Samsung for ufs-exynos. In downstream they
don't split the ufs controller and ufs phy parts into separate
drivers, it is all done from the controller glue driver so it isn't an
issue.

With regards to whether it is a "mode" or not, HIBERN8 is a M-Phy
power state, but then PHY_MODE_UFS_HS_A and  PHY_MODE_UFS_HS_B are
also m-phy power states (HS_BURST with rate A or HS_BURST with rate b)
so it seemed in keeping with what you are already doing here.

regards,

Peter.