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

[Vinod Koul <vkoul@kernel.org>]

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