| Message ID | 1662713084-8106-6-git-send-email-quic_krichai@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: qcom: Add system suspend & resume support | expand |
+ Rajendra On Fri, Sep 09, 2022 at 02:14:44PM +0530, Krishna chaitanya chundru wrote: > Make GDSC always on to ensure controller and its dependent clocks > won't go down during system suspend. > You need to mention the SoC name in subject, otherwise one cannot know for which platform this patch applies to. > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> > --- > drivers/clk/qcom/gcc-sc7280.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/qcom/gcc-sc7280.c b/drivers/clk/qcom/gcc-sc7280.c > index 7ff64d4..2f781a2 100644 > --- a/drivers/clk/qcom/gcc-sc7280.c > +++ b/drivers/clk/qcom/gcc-sc7280.c > @@ -3109,7 +3109,7 @@ static struct gdsc gcc_pcie_1_gdsc = { > .name = "gcc_pcie_1_gdsc", > }, > .pwrsts = PWRSTS_OFF_ON, > - .flags = VOTABLE, > + .flags = ALWAYS_ON, Rajendra, should we also put PCIe GDSC into retention state as you have done for USB [1]? Thanks, Mani [1] https://lore.kernel.org/all/20220901101756.28164-2-quic_rjendra@quicinc.com/ > }; > > static struct gdsc gcc_ufs_phy_gdsc = { > -- > 2.7.4 >
On 9/12/2022 10:34 PM, Manivannan Sadhasivam wrote: > + Rajendra > > On Fri, Sep 09, 2022 at 02:14:44PM +0530, Krishna chaitanya chundru wrote: >> Make GDSC always on to ensure controller and its dependent clocks >> won't go down during system suspend. >> > > You need to mention the SoC name in subject, otherwise one cannot know for > which platform this patch applies to. > >> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> >> --- >> drivers/clk/qcom/gcc-sc7280.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/clk/qcom/gcc-sc7280.c b/drivers/clk/qcom/gcc-sc7280.c >> index 7ff64d4..2f781a2 100644 >> --- a/drivers/clk/qcom/gcc-sc7280.c >> +++ b/drivers/clk/qcom/gcc-sc7280.c >> @@ -3109,7 +3109,7 @@ static struct gdsc gcc_pcie_1_gdsc = { >> .name = "gcc_pcie_1_gdsc", >> }, >> .pwrsts = PWRSTS_OFF_ON, >> - .flags = VOTABLE, >> + .flags = ALWAYS_ON, > > Rajendra, should we also put PCIe GDSC into retention state as you have done for > USB [1]? Yes, it looks like we should handle this the same way as we did with usb. Why are we removing the VOTABLE flag anyway? > > Thanks, > Mani > > [1] https://lore.kernel.org/all/20220901101756.28164-2-quic_rjendra@quicinc.com/ > >> }; >> >> static struct gdsc gcc_ufs_phy_gdsc = { >> -- >> 2.7.4 >> >
On Mon, Sep 12, 2022 at 10:34:37PM +0530, Manivannan Sadhasivam wrote: > + Rajendra > > On Fri, Sep 09, 2022 at 02:14:44PM +0530, Krishna chaitanya chundru wrote: > > Make GDSC always on to ensure controller and its dependent clocks > > won't go down during system suspend. > > You need to mention the SoC name in subject, otherwise one cannot know for > which platform this patch applies to. Also: s/Alwaya/Always/ s/pcie/PCIe/ s/gdsc/GDSC/ as you did in commit log I might use "ALWAYS_ON" in the subject to make clear this refers to a specific flag, not a change in the code logic, e.g., clk: qcom: gcc-sc7280: Mark PCIe GDSC clock ALWAYS_ON
On Tue, Sep 13, 2022 at 12:12:32PM +0530, Rajendra Nayak wrote: > > On 9/12/2022 10:34 PM, Manivannan Sadhasivam wrote: > > + Rajendra > > > > On Fri, Sep 09, 2022 at 02:14:44PM +0530, Krishna chaitanya chundru wrote: > > > Make GDSC always on to ensure controller and its dependent clocks > > > won't go down during system suspend. > > > > > > > You need to mention the SoC name in subject, otherwise one cannot know for > > which platform this patch applies to. > > > > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> > > > --- > > > drivers/clk/qcom/gcc-sc7280.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/clk/qcom/gcc-sc7280.c b/drivers/clk/qcom/gcc-sc7280.c > > > index 7ff64d4..2f781a2 100644 > > > --- a/drivers/clk/qcom/gcc-sc7280.c > > > +++ b/drivers/clk/qcom/gcc-sc7280.c > > > @@ -3109,7 +3109,7 @@ static struct gdsc gcc_pcie_1_gdsc = { > > > .name = "gcc_pcie_1_gdsc", > > > }, > > > .pwrsts = PWRSTS_OFF_ON, > > > - .flags = VOTABLE, > > > + .flags = ALWAYS_ON, > > > > Rajendra, should we also put PCIe GDSC into retention state as you have done for > > USB [1]? > > Yes, it looks like we should handle this the same way as we did with usb. Okay, thanks for the confirmation. > Why are we removing the VOTABLE flag anyway? Yeah, I don't see a reason for doing that. Chaitanya, please follow the patch from Rajendra I mentioned above and adapt it for PCIe GDSC. Thanks, Mani > > > > > Thanks, > > Mani > > > > [1] https://lore.kernel.org/all/20220901101756.28164-2-quic_rjendra@quicinc.com/ > > > > > }; > > > static struct gdsc gcc_ufs_phy_gdsc = { > > > -- > > > 2.7.4 > > > > >
On 9/13/2022 10:12 PM, Manivannan Sadhasivam wrote: > On Tue, Sep 13, 2022 at 12:12:32PM +0530, Rajendra Nayak wrote: >> On 9/12/2022 10:34 PM, Manivannan Sadhasivam wrote: >>> + Rajendra >>> >>> On Fri, Sep 09, 2022 at 02:14:44PM +0530, Krishna chaitanya chundru wrote: >>>> Make GDSC always on to ensure controller and its dependent clocks >>>> won't go down during system suspend. >>>> >>> You need to mention the SoC name in subject, otherwise one cannot know for >>> which platform this patch applies to. >>> >>>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> >>>> --- >>>> drivers/clk/qcom/gcc-sc7280.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/clk/qcom/gcc-sc7280.c b/drivers/clk/qcom/gcc-sc7280.c >>>> index 7ff64d4..2f781a2 100644 >>>> --- a/drivers/clk/qcom/gcc-sc7280.c >>>> +++ b/drivers/clk/qcom/gcc-sc7280.c >>>> @@ -3109,7 +3109,7 @@ static struct gdsc gcc_pcie_1_gdsc = { >>>> .name = "gcc_pcie_1_gdsc", >>>> }, >>>> .pwrsts = PWRSTS_OFF_ON, >>>> - .flags = VOTABLE, >>>> + .flags = ALWAYS_ON, >>> Rajendra, should we also put PCIe GDSC into retention state as you have done for >>> USB [1]? >> Yes, it looks like we should handle this the same way as we did with usb. > Okay, thanks for the confirmation. > >> Why are we removing the VOTABLE flag anyway? > Yeah, I don't see a reason for doing that. > > Chaitanya, please follow the patch from Rajendra I mentioned above and adapt it > for PCIe GDSC. > > Thanks, > Mani ok I will try to adapt that. >>> Thanks, >>> Mani >>> >>> [1] https://lore.kernel.org/all/20220901101756.28164-2-quic_rjendra@quicinc.com/ >>> >>>> }; >>>> static struct gdsc gcc_ufs_phy_gdsc = { >>>> -- >>>> 2.7.4 >>>>
On 9/13/2022 10:04 PM, Bjorn Helgaas wrote: > On Mon, Sep 12, 2022 at 10:34:37PM +0530, Manivannan Sadhasivam wrote: >> + Rajendra >> >> On Fri, Sep 09, 2022 at 02:14:44PM +0530, Krishna chaitanya chundru wrote: >>> Make GDSC always on to ensure controller and its dependent clocks >>> won't go down during system suspend. >> You need to mention the SoC name in subject, otherwise one cannot know for >> which platform this patch applies to. > Also: > > s/Alwaya/Always/ > s/pcie/PCIe/ > s/gdsc/GDSC/ as you did in commit log > > I might use "ALWAYS_ON" in the subject to make clear this refers to a > specific flag, not a change in the code logic, e.g., > > clk: qcom: gcc-sc7280: Mark PCIe GDSC clock ALWAYS_ON ok I will update the subject in next patch.
diff --git a/drivers/clk/qcom/gcc-sc7280.c b/drivers/clk/qcom/gcc-sc7280.c index 7ff64d4..2f781a2 100644 --- a/drivers/clk/qcom/gcc-sc7280.c +++ b/drivers/clk/qcom/gcc-sc7280.c @@ -3109,7 +3109,7 @@ static struct gdsc gcc_pcie_1_gdsc = { .name = "gcc_pcie_1_gdsc", }, .pwrsts = PWRSTS_OFF_ON, - .flags = VOTABLE, + .flags = ALWAYS_ON, }; static struct gdsc gcc_ufs_phy_gdsc = {
Make GDSC always on to ensure controller and its dependent clocks won't go down during system suspend. Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> --- drivers/clk/qcom/gcc-sc7280.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)