Message ID | 20190111230129.127037-1-evgreen@chromium.org |
---|---|
Headers | show |
Series | phy: qcom-ufs: Enable regulators to be off in suspend | expand |
Quoting Evan Green (2019-01-11 15:01:21) > > Because the UFS PHY reset bit is now toggled in the PHY, rather > than in ufs-qcom, this also percolated to all other PHYs using > ufs-qcom, which from what I can see is just 8996. > > There are a couple of tradeoffs in this series that I'd welcome feedback > on. First, it breaks compatibility with device trees that don't expose > this new reset controller. Making this work with older device trees > would be pretty ugly in the code, and given that the SDM845 UFS DT nodes > aren't accepted upstream yet, the breakage seemed worth it. I'm not as > sure about 8996. It may take some minor surgery to the reset framework, but it should be possible to register a reset lookup that can be used if nothing matches in DT. Right now the reset code looks to only want to use DT if it's there for the device requesting the reset. If there isn't a DT node for the device it will look in the global lookup list. That could be changed to fallback to the global lookup that uses device names (similar to clk framework) when the DT lookup fails. Then it's just a matter of registering the lookup for this reset with the handful of device names that need the non-DT way of finding the reset. Of course, that's another change and if breaking DT is simpler and acceptable then I would say go for the path of least resistance and don't try to modify reset framework for this.
Quoting Evan Green (2019-01-11 15:01:24) > Wire up the reset controller in the Qcom UFS controller for the PHY. > This will be used to toggle PHY reset during initialization of the PHY. > > Signed-off-by: Evan Green <evgreen@chromium.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Quoting Evan Green (2019-01-11 15:01:25) > Add the reset controller for the UFS controller, and wire it up > so that the UFS PHY can initialize itself without relying on > implicit sequencing between the two drivers. > > Signed-off-by: Evan Green <evgreen@chromium.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org>
On Fri, Jan 11, 2019 at 03:01:24PM -0800, Evan Green wrote: > Wire up the reset controller in the Qcom UFS controller for the PHY. > This will be used to toggle PHY reset during initialization of the PHY. > > Signed-off-by: Evan Green <evgreen@chromium.org> > --- > This commit is based atop the series at [1]. Patches 1 and 2 of that > series have landed, but 3, 4, and 5 are still outstanding. > > [1] https://lore.kernel.org/lkml/20181210192826.241350-1-evgreen@chromium.org/ > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index b29332b265d9e..029ab66405cf4 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -990,6 +990,7 @@ > phy-names = "ufsphy"; > lanes-per-direction = <2>; > power-domains = <&gcc UFS_PHY_GDSC>; > + #reset-cells = <1>; Is this documented? > > clock-names = > "core_clk", > @@ -1033,6 +1034,8 @@ > clocks = <&gcc GCC_UFS_MEM_CLKREF_CLK>, > <&gcc GCC_UFS_PHY_PHY_AUX_CLK>; > > + resets = <&ufs_mem_hc 0>; > + reset-names = "ufsphy"; > status = "disabled"; > > ufs_mem_phy_lanes: lanes@1d87400 { > -- > 2.18.1 >
On Mon, Jan 21, 2019 at 4:25 PM Rob Herring <robh@kernel.org> wrote: > > On Fri, Jan 11, 2019 at 03:01:24PM -0800, Evan Green wrote: > > Wire up the reset controller in the Qcom UFS controller for the PHY. > > This will be used to toggle PHY reset during initialization of the PHY. > > > > Signed-off-by: Evan Green <evgreen@chromium.org> > > --- > > This commit is based atop the series at [1]. Patches 1 and 2 of that > > series have landed, but 3, 4, and 5 are still outstanding. > > > > [1] https://lore.kernel.org/lkml/20181210192826.241350-1-evgreen@chromium.org/ > > > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > index b29332b265d9e..029ab66405cf4 100644 > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > @@ -990,6 +990,7 @@ > > phy-names = "ufsphy"; > > lanes-per-direction = <2>; > > power-domains = <&gcc UFS_PHY_GDSC>; > > + #reset-cells = <1>; > > Is this documented? > No, I'll add this in my next spin. Thanks. -Evan