Re: [PATCH v2] clk: qcom: Enable clocks which needs to be always on for SDM845

[Amit Nischal]

On 2018-06-01 22:06, Stephen Boyd wrote:
> Quoting Amit Nischal (2018-05-28 23:07:46)
> > There are certain clocks which needs to be always enabled for system
> > operation. Remove support for such clocks from the clock driver and
> > enable them explicitly gcc driver probe. Also fix the target name in
> > kernel configuration description for SDM845 gcc driver.
>
> This is why we have CLK_IS_CRITICAL.
>
> > Signed-off-by: Amit Nischal <anischal@xxxxxxxxxxxxxx>
> > ---
> > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> > index fc3edd1..79e9144 100644
> > --- a/drivers/clk/qcom/Kconfig
> > +++ b/drivers/clk/qcom/Kconfig
> > @@ -231,7 +231,7 @@ config SDM_GCC_845
> >         select QCOM_GDSC
> >         depends on COMMON_CLK_QCOM
> >         help
> > -         Support for the global clock controller on msm8998 devices.
> > +         Support for the global clock controller on SDM845 devices.
> >           Say Y if you want to use peripheral devices such as UART, 
> > SPI,
> >           I2C, USB, UFS, SDDC, PCIe, etc.
>
> This can be its own patch.

Thanks for fixing above as this is already in the clk-next branch
under commit "77122d".

> > diff --git a/drivers/clk/qcom/gcc-sdm845.c 
> > b/drivers/clk/qcom/gcc-sdm845.c
> > index e78e6f5..b40aafc 100644
> > --- a/drivers/clk/qcom/gcc-sdm845.c
> > +++ b/drivers/clk/qcom/gcc-sdm845.c
> > @@ -3433,9 +3327,16 @@ static int gcc_sdm845_probe(struct 
> > platform_device *pdev)
> >         regmap_update_bits(regmap, 0x09ffc, 0x3, 0x3);
> >         regmap_update_bits(regmap, 0x71028, 0x3, 0x3);
> >
> > -       /* Enable CPUSS clocks */
> > -       regmap_update_bits(regmap, 0x48190, BIT(0), 0x1);
> > -       regmap_update_bits(regmap, 0x52004, BIT(22), 0x1);
> > +       /* Enable clocks which are required to be always ON */
> > +       regmap_update_bits(regmap, 0xb004, BIT(0), BIT(0));
> > +       regmap_update_bits(regmap, 0xb008, BIT(0), BIT(0));
> > +       regmap_update_bits(regmap, 0xb00c, BIT(0), BIT(0));
> > +       regmap_update_bits(regmap, 0xb028, BIT(0), BIT(0));
> > +       regmap_update_bits(regmap, 0xb02c, BIT(0), BIT(0));
> > +       regmap_update_bits(regmap, 0xb030, BIT(0), BIT(0));
> > +       regmap_update_bits(regmap, 0x48190, BIT(0), BIT(0));
> > +       regmap_update_bits(regmap, 0x52004, BIT(22), BIT(22));
> > +       regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0));
>
> Let's do CLK_IS_CRITICAL flag instead. That has the added benefit of
> checking to make sure the halt bit toggles correctly so the clks are
> verified to be enabled.

I will submit the next patch for all above-listed clocks with
CLK_IS_CRITICAL flag.

> >         return qcom_cc_really_probe(pdev, &gcc_sdm845_desc, regmap);
> >  }
> > diff --git a/include/dt-bindings/clock/qcom,gcc-sdm845.h 
> > b/include/dt-bindings/clock/qcom,gcc-sdm845.h
> > index aca6126..6330c3f 100644
> > --- a/include/dt-bindings/clock/qcom,gcc-sdm845.h
> > +++ b/include/dt-bindings/clock/qcom,gcc-sdm845.h
> > @@ -13,185 +13,178 @@
> >  #define GCC_AGGRE_USB3_PRIM_AXI_CLK                            3
> >  #define GCC_AGGRE_USB3_SEC_AXI_CLK                             4
> >  #define GCC_BOOT_ROM_AHB_CLK                                   5
> > -#define GCC_CAMERA_AHB_CLK                                     6
>
> Don't do this. Things should only be added to here, not removed or
> reshuffled.
>
> > -#define GCC_CAMERA_AXI_CLK                                     7
> > -#define GCC_CAMERA_XO_CLK                                      8
> > -#define GCC_CE1_AHB_CLK                                               
> >  9
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html