On Sat, Jul 13, 2024 at 07:21:29PM +0300, Dmitry Baryshkov wrote:
> On Thu, Jul 11, 2024 at 05:02:38PM GMT, Varadarajan Narayanan wrote:
> > Use the icc-clk framework to enable few clocks to be able to
> > create paths and use the peripherals connected on those NoCs.
> >
> > Signed-off-by: Varadarajan Narayanan <quic_varada@xxxxxxxxxxx>
> > ---
> > drivers/clk/qcom/gcc-ipq5332.c | 36 +++++++++++++++++++++++++++++-----
> > 1 file changed, 31 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/clk/qcom/gcc-ipq5332.c b/drivers/clk/qcom/gcc-ipq5332.c
> > index f98591148a97..6d7672cae0f7 100644
> > --- a/drivers/clk/qcom/gcc-ipq5332.c
> > +++ b/drivers/clk/qcom/gcc-ipq5332.c
> > @@ -4,12 +4,14 @@
> > */
> >
> > #include <linux/clk-provider.h>
> > +#include <linux/interconnect-provider.h>
> > #include <linux/mod_devicetable.h>
> > #include <linux/module.h>
> > #include <linux/platform_device.h>
> > #include <linux/regmap.h>
> >
> > #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> > +#include <dt-bindings/interconnect/qcom,ipq5332.h>
> >
> > #include "clk-alpha-pll.h"
> > #include "clk-branch.h"
> > @@ -131,12 +133,14 @@ static struct clk_alpha_pll gpll4_main = {
> > * (will be added soon), so the clock framework
> > * disables this source. But some of the clocks
> > * initialized by boot loaders uses this source. So we
> > - * need to keep this clock ON. Add the
> > - * CLK_IGNORE_UNUSED flag so the clock will not be
> > - * disabled. Once the consumer in kernel is added, we
> > - * can get rid of this flag.
> > + * need to keep this clock ON.
> > + *
> > + * After initial bootup, when the ICC framework turns
> > + * off unused paths, as part of the icc-clk dependencies
> > + * this clock gets disabled resulting in a hang. Marking
> > + * it as critical to ensure it is not turned off.
>
> Previous comment was pretty clear: there are missing consumers, the flag
> will be removed once they are added. Current comment doesn't make sense.
> What is the reason for the device hang if we have all the consumers in
> place?
Earlier, since there were no consumers for this clock, it got
disabled via clk_disable_unused() and CLK_IGNORE_UNUSED helped
prevent that.
Now, since this clk is getting used indirectly via icc-clk
framework, it doesn't qualify for being disabled by
clk_disable_unused(). However, when icc_sync_state is called, if
it sees there are no consumers for a path and that path gets
disabled, the relevant clocks get disabled and eventually this
clock also gets disabled. To avoid this have changed 'flags' from
CLK_IGNORE_UNUSED -> CLK_IS_CRITICAL.
Thanks
Varada
> > */
> > - .flags = CLK_IGNORE_UNUSED,
> > + .flags = CLK_IS_CRITICAL,
> > },
> > },
> > };
>
>
> --
> With best wishes
> Dmitry
