| Message ID | 1545306385-31240-1-git-send-email-tdas@codeaurora.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v1] clk: qcom: lpass: Add CLK_IGNORE_UNUSED for lpass clocks | expand |
Quoting Taniya Das (2018-12-20 03:46:25) > The LPASS clocks has a dependency on the GCC lpass clocks to be enabled > before accessing them and that was the reason to mark the gcc lpass clocks > as critical. But in the case where the lpass subsystem would require a > restart, toggling the lpass reset would from HW clear the SW enable bits > of the GCC lpass clocks. Thus the next time bringing up the lpass subsystem > out of reset would fail. > > Allow the lpass clock driver to enable/disable the gcc lpass clocks and > mark the lpass clocks not be accessed during late_init if no client vote. You need to add more details here. It feels like you wrote the beginning of a paragraph and then stopped abruptly, leaving the reader hanging for the whole story. Why is late_init important? Why do we need to leave them on from the bootloader? What if the bootloader doesn't leave them enabled? This is all rather hacky so I'm deeply confused. Does the lpass driver need to get these gcc clks and enable/prepare them during probe? But then it needs to also allow a reset happen and change the clk state? I suspect this situation is circling a larger problem where a reset is toggled and that changes some clk state without the clk framework knowing. There's not much we can do about that besides having some mechanism for the clks to know that their state is now out of sync. If that can be done on the provider driver side then we should have an easier time not needing to write a bunch of framework code to handle this. OMAP folks are dealing with the same problems from what I recall. > > Signed-off-by: Taniya Das <tdas@codeaurora.org> > @@ -77,6 +81,7 @@ > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "lpass_qdsp6ss_sleep_clk", > + .flags = CLK_IGNORE_UNUSED, All uses of CLK_IGNORE_UNUSED and CLK_IS_CRITICAL need comments about why they're there. It's never obvious why these things are being done.
Hello Stephen, On 12/21/2018 2:34 AM, Stephen Boyd wrote: > Quoting Taniya Das (2018-12-20 03:46:25) >> The LPASS clocks has a dependency on the GCC lpass clocks to be enabled >> before accessing them and that was the reason to mark the gcc lpass clocks >> as critical. But in the case where the lpass subsystem would require a >> restart, toggling the lpass reset would from HW clear the SW enable bits >> of the GCC lpass clocks. Thus the next time bringing up the lpass subsystem >> out of reset would fail. >> >> Allow the lpass clock driver to enable/disable the gcc lpass clocks and >> mark the lpass clocks not be accessed during late_init if no client vote. > > You need to add more details here. It feels like you wrote the beginning > of a paragraph and then stopped abruptly, leaving the reader hanging for > the whole story. Why is late_init important? Why do we need to leave > them on from the bootloader? What if the bootloader doesn't leave them > enabled? This is all rather hacky so I'm deeply confused. Does the lpass > driver need to get these gcc clks and enable/prepare them during probe? > But then it needs to also allow a reset happen and change the clk state? > > I suspect this situation is circling a larger problem where a reset is > toggled and that changes some clk state without the clk framework > knowing. There's not much we can do about that besides having some > mechanism for the clks to know that their state is now out of sync. If > that can be done on the provider driver side then we should have an > easier time not needing to write a bunch of framework code to handle > this. OMAP folks are dealing with the same problems from what I recall. > Hmm, if I mark the CLK_IS_CRITICAL, I don't see a way to handle it in provider. Please suggest if you think provider could handle it. >> >> Signed-off-by: Taniya Das <tdas@codeaurora.org> > >> @@ -77,6 +81,7 @@ >> .enable_mask = BIT(0), >> .hw.init = &(struct clk_init_data){ >> .name = "lpass_qdsp6ss_sleep_clk", >> + .flags = CLK_IGNORE_UNUSED, > > All uses of CLK_IGNORE_UNUSED and CLK_IS_CRITICAL need comments about > why they're there. It's never obvious why these things are being done. > Sure, would add comments.
Quoting Taniya Das (2019-01-06 22:26:00) > Hello Stephen, > > On 12/21/2018 2:34 AM, Stephen Boyd wrote: > > Quoting Taniya Das (2018-12-20 03:46:25) > >> The LPASS clocks has a dependency on the GCC lpass clocks to be enabled > >> before accessing them and that was the reason to mark the gcc lpass clocks > >> as critical. But in the case where the lpass subsystem would require a > >> restart, toggling the lpass reset would from HW clear the SW enable bits > >> of the GCC lpass clocks. Thus the next time bringing up the lpass subsystem > >> out of reset would fail. > >> > >> Allow the lpass clock driver to enable/disable the gcc lpass clocks and > >> mark the lpass clocks not be accessed during late_init if no client vote. > > > > You need to add more details here. It feels like you wrote the beginning > > of a paragraph and then stopped abruptly, leaving the reader hanging for > > the whole story. Why is late_init important? Why do we need to leave > > them on from the bootloader? What if the bootloader doesn't leave them > > enabled? This is all rather hacky so I'm deeply confused. Does the lpass > > driver need to get these gcc clks and enable/prepare them during probe? > > But then it needs to also allow a reset happen and change the clk state? > > > > I suspect this situation is circling a larger problem where a reset is > > toggled and that changes some clk state without the clk framework > > knowing. There's not much we can do about that besides having some > > mechanism for the clks to know that their state is now out of sync. If > > that can be done on the provider driver side then we should have an > > easier time not needing to write a bunch of framework code to handle > > this. OMAP folks are dealing with the same problems from what I recall. > > > > Hmm, if I mark the CLK_IS_CRITICAL, I don't see a way to handle it in > provider. Please suggest if you think provider could handle it. As far as I know, I'm not suggesting the use of CLK_IS_CRITICAL here. But removing CLK_IS_CRITICAL and relying on some random bootloader behavior also looks wrong. Can you clarify what's going on?
On 1/8/2019 2:34 AM, Stephen Boyd wrote: > Quoting Taniya Das (2019-01-06 22:26:00) >> Hello Stephen, >> >> On 12/21/2018 2:34 AM, Stephen Boyd wrote: >>> Quoting Taniya Das (2018-12-20 03:46:25) >>>> The LPASS clocks has a dependency on the GCC lpass clocks to be enabled >>>> before accessing them and that was the reason to mark the gcc lpass clocks >>>> as critical. But in the case where the lpass subsystem would require a >>>> restart, toggling the lpass reset would from HW clear the SW enable bits >>>> of the GCC lpass clocks. Thus the next time bringing up the lpass subsystem >>>> out of reset would fail. >>>> >>>> Allow the lpass clock driver to enable/disable the gcc lpass clocks and >>>> mark the lpass clocks not be accessed during late_init if no client vote. >>> >>> You need to add more details here. It feels like you wrote the beginning >>> of a paragraph and then stopped abruptly, leaving the reader hanging for >>> the whole story. Why is late_init important? Why do we need to leave >>> them on from the bootloader? What if the bootloader doesn't leave them >>> enabled? This is all rather hacky so I'm deeply confused. Does the lpass >>> driver need to get these gcc clks and enable/prepare them during probe? >>> But then it needs to also allow a reset happen and change the clk state? >>> >>> I suspect this situation is circling a larger problem where a reset is >>> toggled and that changes some clk state without the clk framework >>> knowing. There's not much we can do about that besides having some >>> mechanism for the clks to know that their state is now out of sync. If >>> that can be done on the provider driver side then we should have an >>> easier time not needing to write a bunch of framework code to handle >>> this. OMAP folks are dealing with the same problems from what I recall. >>> >> >> Hmm, if I mark the CLK_IS_CRITICAL, I don't see a way to handle it in >> provider. Please suggest if you think provider could handle it. > > As far as I know, I'm not suggesting the use of CLK_IS_CRITICAL here. > But removing CLK_IS_CRITICAL and relying on some random bootloader > behavior also looks wrong. Can you clarify what's going on? > To enable LPASS clocks the requirement is to enable the GCC_LPASS_SWAY clock. 1) If the LPASS drivers are enabled/probed before the clock late init the client would take care to maintain the dependency to enable the GCC_LPASS_SWAY clock before enabling the LPASS clocks. 2) There could be a condition where the LPASS drivers would probe/init later the clock late_init. When the clock_late_init would try to access the LPASS clocks, since we cannot maintain the dependency this access would fail. To avoid this the earlier patch has made the GCC_LPASS_SWAY clock as CRITICAL. 3) Marking the GCC_LPASS_SWAY clock as CRITICAL has a issue, in the case where the LPASS subsystem would be restarted due to some critical failure on LPASS. Toggling the restart register of LPASS would clear the hardware state of this clock and thus the next access of the LPASS clocks would result in failure of the system. 4) To avoid issues happening in (2) and (3) all the LPASS clocks chould be safely marked as CLK_IGNORE_UNUSED. And lpass drivers would take care of the dependency to enable the required clocks. Hope the above helps.
Quoting Taniya Das (2019-01-13 22:12:39) > > > On 1/8/2019 2:34 AM, Stephen Boyd wrote: > > > > As far as I know, I'm not suggesting the use of CLK_IS_CRITICAL here. > > But removing CLK_IS_CRITICAL and relying on some random bootloader > > behavior also looks wrong. Can you clarify what's going on? > > > > To enable LPASS clocks the requirement is to enable the GCC_LPASS_SWAY > clock. > 1) If the LPASS drivers are enabled/probed before the clock late init > the client would take care to maintain the dependency to enable the > GCC_LPASS_SWAY clock before enabling the LPASS clocks. > > 2) There could be a condition where the LPASS drivers would probe/init > later the clock late_init. When the clock_late_init would try to access > the LPASS clocks, since we cannot maintain the dependency this access > would fail. To avoid this the earlier patch has made the GCC_LPASS_SWAY > clock as CRITICAL. > > 3) Marking the GCC_LPASS_SWAY clock as CRITICAL has a issue, in the case > where the LPASS subsystem would be restarted due to some critical > failure on LPASS. Toggling the restart register of LPASS would clear the > hardware state of this clock and thus the next access of the LPASS > clocks would result in failure of the system. > > 4) To avoid issues happening in (2) and (3) all the LPASS clocks chould > be safely marked as CLK_IGNORE_UNUSED. And lpass drivers would take care > of the dependency to enable the required clocks. > Ok, so why can't we enable/disable the lpass sway clk in the prepare/unprepare phase of the lpass clk driver paths? Or why can't we forcibly enable this lpass sway clk after the reset is deasserted? Which clk controller is the reset part of? GCC or LPASS? It still sounds like the LPASS clk driver isn't handling dependencies it has on accessing registers, but maybe we can get away with not handling the dependency still if we make the reset "do the right thing" and turn the clk back on so it stays "critical".
On 1/15/2019 3:55 AM, Stephen Boyd wrote: > Quoting Taniya Das (2019-01-13 22:12:39) >> >> >> On 1/8/2019 2:34 AM, Stephen Boyd wrote: >>> >>> As far as I know, I'm not suggesting the use of CLK_IS_CRITICAL here. >>> But removing CLK_IS_CRITICAL and relying on some random bootloader >>> behavior also looks wrong. Can you clarify what's going on? >>> >> >> To enable LPASS clocks the requirement is to enable the GCC_LPASS_SWAY >> clock. >> 1) If the LPASS drivers are enabled/probed before the clock late init >> the client would take care to maintain the dependency to enable the >> GCC_LPASS_SWAY clock before enabling the LPASS clocks. >> >> 2) There could be a condition where the LPASS drivers would probe/init >> later the clock late_init. When the clock_late_init would try to access >> the LPASS clocks, since we cannot maintain the dependency this access >> would fail. To avoid this the earlier patch has made the GCC_LPASS_SWAY >> clock as CRITICAL. >> >> 3) Marking the GCC_LPASS_SWAY clock as CRITICAL has a issue, in the case >> where the LPASS subsystem would be restarted due to some critical >> failure on LPASS. Toggling the restart register of LPASS would clear the >> hardware state of this clock and thus the next access of the LPASS >> clocks would result in failure of the system. >> >> 4) To avoid issues happening in (2) and (3) all the LPASS clocks chould >> be safely marked as CLK_IGNORE_UNUSED. And lpass drivers would take care >> of the dependency to enable the required clocks. >> > > Ok, so why can't we enable/disable the lpass sway clk in the > prepare/unprepare phase of the lpass clk driver paths? Or why can't we > forcibly enable this lpass sway clk after the reset is deasserted? Which > clk controller is the reset part of? GCC or LPASS? It is part of Always On Subsystem. > > It still sounds like the LPASS clk driver isn't handling dependencies it > has on accessing registers, but maybe we can get away with not handling > the dependency still if we make the reset "do the right thing" and turn > the clk back on so it stays "critical". > This is a reset from hardware and it does not bring back the clock to the previous state and so we can not mark it "critical". I would submit the next series with comments updated. Please let me know in case you have any comments.
Quoting Taniya Das (2019-01-17 03:19:22) > > > On 1/15/2019 3:55 AM, Stephen Boyd wrote: > > Quoting Taniya Das (2019-01-13 22:12:39) > >> > >> > >> On 1/8/2019 2:34 AM, Stephen Boyd wrote: > >>> > >>> As far as I know, I'm not suggesting the use of CLK_IS_CRITICAL here. > >>> But removing CLK_IS_CRITICAL and relying on some random bootloader > >>> behavior also looks wrong. Can you clarify what's going on? > >>> > >> > >> To enable LPASS clocks the requirement is to enable the GCC_LPASS_SWAY > >> clock. > >> 1) If the LPASS drivers are enabled/probed before the clock late init > >> the client would take care to maintain the dependency to enable the > >> GCC_LPASS_SWAY clock before enabling the LPASS clocks. > >> > >> 2) There could be a condition where the LPASS drivers would probe/init > >> later the clock late_init. When the clock_late_init would try to access > >> the LPASS clocks, since we cannot maintain the dependency this access > >> would fail. To avoid this the earlier patch has made the GCC_LPASS_SWAY > >> clock as CRITICAL. > >> > >> 3) Marking the GCC_LPASS_SWAY clock as CRITICAL has a issue, in the case > >> where the LPASS subsystem would be restarted due to some critical > >> failure on LPASS. Toggling the restart register of LPASS would clear the > >> hardware state of this clock and thus the next access of the LPASS > >> clocks would result in failure of the system. > >> > >> 4) To avoid issues happening in (2) and (3) all the LPASS clocks chould > >> be safely marked as CLK_IGNORE_UNUSED. And lpass drivers would take care > >> of the dependency to enable the required clocks. > >> > > > > Ok, so why can't we enable/disable the lpass sway clk in the > > prepare/unprepare phase of the lpass clk driver paths? Or why can't we > > forcibly enable this lpass sway clk after the reset is deasserted? Which > > clk controller is the reset part of? GCC or LPASS? > > It is part of Always On Subsystem. Ok. Is this merged upstream? > > > > > It still sounds like the LPASS clk driver isn't handling dependencies it > > has on accessing registers, but maybe we can get away with not handling > > the dependency still if we make the reset "do the right thing" and turn > > the clk back on so it stays "critical". > > > > This is a reset from hardware and it does not bring back the clock to > the previous state and so we can not mark it "critical". I would submit > the next series with comments updated. Please let me know in case you > have any comments. > > Can we have the always on subsystem reset code go hit this clk enable bit back on? And also have the LPASS clk driver get this GCC sway clk and enable it during probe? Maybe we need to get some sort of API in the clk provider layer that can tell us that the clk state has changed now and it needs to be restored. I haven't thought about it deeply but that may be the best solution here.
Hello Stephen, On 1/19/2019 12:31 AM, Stephen Boyd wrote: > Quoting Taniya Das (2019-01-17 03:19:22) >> >> >> On 1/15/2019 3:55 AM, Stephen Boyd wrote: >>> Quoting Taniya Das (2019-01-13 22:12:39) >>>> >>>> >>>> On 1/8/2019 2:34 AM, Stephen Boyd wrote: >>>>> >>>>> As far as I know, I'm not suggesting the use of CLK_IS_CRITICAL here. >>>>> But removing CLK_IS_CRITICAL and relying on some random bootloader >>>>> behavior also looks wrong. Can you clarify what's going on? >>>>> >>>> >>>> To enable LPASS clocks the requirement is to enable the GCC_LPASS_SWAY >>>> clock. >>>> 1) If the LPASS drivers are enabled/probed before the clock late init >>>> the client would take care to maintain the dependency to enable the >>>> GCC_LPASS_SWAY clock before enabling the LPASS clocks. >>>> >>>> 2) There could be a condition where the LPASS drivers would probe/init >>>> later the clock late_init. When the clock_late_init would try to access >>>> the LPASS clocks, since we cannot maintain the dependency this access >>>> would fail. To avoid this the earlier patch has made the GCC_LPASS_SWAY >>>> clock as CRITICAL. >>>> >>>> 3) Marking the GCC_LPASS_SWAY clock as CRITICAL has a issue, in the case >>>> where the LPASS subsystem would be restarted due to some critical >>>> failure on LPASS. Toggling the restart register of LPASS would clear the >>>> hardware state of this clock and thus the next access of the LPASS >>>> clocks would result in failure of the system. >>>> >>>> 4) To avoid issues happening in (2) and (3) all the LPASS clocks chould >>>> be safely marked as CLK_IGNORE_UNUSED. And lpass drivers would take care >>>> of the dependency to enable the required clocks. >>>> >>> >>> Ok, so why can't we enable/disable the lpass sway clk in the >>> prepare/unprepare phase of the lpass clk driver paths? Or why can't we >>> forcibly enable this lpass sway clk after the reset is deasserted? Which >>> clk controller is the reset part of? GCC or LPASS? >> >> It is part of Always On Subsystem. > > Ok. Is this merged upstream? > >> >>> >>> It still sounds like the LPASS clk driver isn't handling dependencies it >>> has on accessing registers, but maybe we can get away with not handling >>> the dependency still if we make the reset "do the right thing" and turn >>> the clk back on so it stays "critical". >>> >> >> This is a reset from hardware and it does not bring back the clock to >> the previous state and so we can not mark it "critical". I would submit >> the next series with comments updated. Please let me know in case you >> have any comments. >> >> > > Can we have the always on subsystem reset code go hit this clk enable > bit back on? There is no code, it is a reset from hardware. And also have the LPASS clk driver get this GCC sway clk > and enable it during probe? Maybe we need to get some sort of API in the > clk provider layer that can tell us that the clk state has changed now > and it needs to be restored. I haven't thought about it deeply but that > may be the best solution here. > Would it be possible to go about with the current patch and then have it updated with the possible solutions.
On Thu 20 Dec 03:46 PST 2018, Taniya Das wrote: > The LPASS clocks has a dependency on the GCC lpass clocks to be enabled > before accessing them and that was the reason to mark the gcc lpass clocks > as critical. I have the same problem with the TuringCC in QCS404, that in order to disable the unused clocks GCC_CDSP_CFG_AHB_CLK must be on. But rather than marking either side as critical or ignore-unused I think a better solution is to describe this dependency by using the fact that the clock framework wraps any accesses in pm_runtime_get()/put() calls. So we can use this to link the clock controller to some resources that needs to be turned on whenever we communicate with it. > But in the case where the lpass subsystem would require a > restart, toggling the lpass reset would from HW clear the SW enable bits > of the GCC lpass clocks. Thus the next time bringing up the lpass subsystem > out of reset would fail. Are you saying that when we toggle AOSS_CC_LPASS_RESTART the enable bit is cleared on all these clocks and we might get out of sync between the clock framework and the hardware? Can you please elaborate a little bit on this? Afaict in the remoteproc driver at least, we first disable the clock then we issue the reset. Regards, Bjorn > > Allow the lpass clock driver to enable/disable the gcc lpass clocks and > mark the lpass clocks not be accessed during late_init if no client vote. > > Signed-off-by: Taniya Das <tdas@codeaurora.org> > --- > drivers/clk/qcom/gcc-sdm845.c | 2 -- > drivers/clk/qcom/lpasscc-sdm845.c | 5 +++++ > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/qcom/gcc-sdm845.c b/drivers/clk/qcom/gcc-sdm845.c > index c782e62..8365c97 100644 > --- a/drivers/clk/qcom/gcc-sdm845.c > +++ b/drivers/clk/qcom/gcc-sdm845.c > @@ -3163,7 +3163,6 @@ enum { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_lpass_q6_axi_clk", > - .flags = CLK_IS_CRITICAL, > .ops = &clk_branch2_ops, > }, > }, > @@ -3177,7 +3176,6 @@ enum { > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "gcc_lpass_sway_clk", > - .flags = CLK_IS_CRITICAL, > .ops = &clk_branch2_ops, > }, > }, > diff --git a/drivers/clk/qcom/lpasscc-sdm845.c b/drivers/clk/qcom/lpasscc-sdm845.c > index e246b99..1acc741 100644 > --- a/drivers/clk/qcom/lpasscc-sdm845.c > +++ b/drivers/clk/qcom/lpasscc-sdm845.c > @@ -22,6 +22,7 @@ > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "lpass_q6ss_ahbm_aon_clk", > + .flags = CLK_IGNORE_UNUSED, > .ops = &clk_branch2_ops, > }, > }, > @@ -35,6 +36,7 @@ > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "lpass_q6ss_ahbs_aon_clk", > + .flags = CLK_IGNORE_UNUSED, > .ops = &clk_branch2_ops, > }, > }, > @@ -49,6 +51,7 @@ > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "lpass_qdsp6ss_core_clk", > + .flags = CLK_IGNORE_UNUSED, > .ops = &clk_branch2_ops, > }, > }, > @@ -63,6 +66,7 @@ > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "lpass_qdsp6ss_xo_clk", > + .flags = CLK_IGNORE_UNUSED, > .ops = &clk_branch2_ops, > }, > }, > @@ -77,6 +81,7 @@ > .enable_mask = BIT(0), > .hw.init = &(struct clk_init_data){ > .name = "lpass_qdsp6ss_sleep_clk", > + .flags = CLK_IGNORE_UNUSED, > .ops = &clk_branch2_ops, > }, > }, > -- > Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member > of the Code Aurora Forum, hosted by the Linux Foundation. >
diff --git a/drivers/clk/qcom/gcc-sdm845.c b/drivers/clk/qcom/gcc-sdm845.c index c782e62..8365c97 100644 --- a/drivers/clk/qcom/gcc-sdm845.c +++ b/drivers/clk/qcom/gcc-sdm845.c @@ -3163,7 +3163,6 @@ enum { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_lpass_q6_axi_clk", - .flags = CLK_IS_CRITICAL, .ops = &clk_branch2_ops, }, }, @@ -3177,7 +3176,6 @@ enum { .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "gcc_lpass_sway_clk", - .flags = CLK_IS_CRITICAL, .ops = &clk_branch2_ops, }, }, diff --git a/drivers/clk/qcom/lpasscc-sdm845.c b/drivers/clk/qcom/lpasscc-sdm845.c index e246b99..1acc741 100644 --- a/drivers/clk/qcom/lpasscc-sdm845.c +++ b/drivers/clk/qcom/lpasscc-sdm845.c @@ -22,6 +22,7 @@ .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "lpass_q6ss_ahbm_aon_clk", + .flags = CLK_IGNORE_UNUSED, .ops = &clk_branch2_ops, }, }, @@ -35,6 +36,7 @@ .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "lpass_q6ss_ahbs_aon_clk", + .flags = CLK_IGNORE_UNUSED, .ops = &clk_branch2_ops, }, }, @@ -49,6 +51,7 @@ .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "lpass_qdsp6ss_core_clk", + .flags = CLK_IGNORE_UNUSED, .ops = &clk_branch2_ops, }, }, @@ -63,6 +66,7 @@ .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "lpass_qdsp6ss_xo_clk", + .flags = CLK_IGNORE_UNUSED, .ops = &clk_branch2_ops, }, }, @@ -77,6 +81,7 @@ .enable_mask = BIT(0), .hw.init = &(struct clk_init_data){ .name = "lpass_qdsp6ss_sleep_clk", + .flags = CLK_IGNORE_UNUSED, .ops = &clk_branch2_ops, }, },
The LPASS clocks has a dependency on the GCC lpass clocks to be enabled before accessing them and that was the reason to mark the gcc lpass clocks as critical. But in the case where the lpass subsystem would require a restart, toggling the lpass reset would from HW clear the SW enable bits of the GCC lpass clocks. Thus the next time bringing up the lpass subsystem out of reset would fail. Allow the lpass clock driver to enable/disable the gcc lpass clocks and mark the lpass clocks not be accessed during late_init if no client vote. Signed-off-by: Taniya Das <tdas@codeaurora.org> --- drivers/clk/qcom/gcc-sdm845.c | 2 -- drivers/clk/qcom/lpasscc-sdm845.c | 5 +++++ 2 files changed, 5 insertions(+), 2 deletions(-) -- Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.