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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZxED5/HdlBLWqesE@vaman>
Date: Thu, 17 Oct 2024 18:02:39 +0530
From: Vinod Koul <vkoul@...nel.org>
To: Peter Griffin <peter.griffin@...aro.org>
Cc: kishon@...nel.org, krzysztof.kozlowski@...aro.org,
	alim.akhtar@...sung.com, tudor.ambarus@...aro.org,
	andre.draszik@...aro.org, kernel-team@...roid.com,
	willmcvicker@...gle.com, linux-phy@...ts.infradead.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-samsung-soc@...r.kernel.org
Subject: Re: [PATCH 1/3] phy: Add UFS phy hibernate modes

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