Re: [PATCH v2 2/4] clk: qcom: Add SDX55 GCC support

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Fri 06 Nov 03:38 CST 2020, Vinod Koul wrote:

> On 05-11-20, 16:18, Manivannan Sadhasivam wrote:
> > From: Naveen Yadav <naveenky@codeaurora.org>
> > 
> > Add Global Clock Controller (GCC) support for SDX55 SoCs from Qualcomm.
> > 
> > Signed-off-by: Naveen Yadav <naveenky@codeaurora.org>
> > [mani: converted to parent_data, commented critical clocks, cleanups]
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  drivers/clk/qcom/Kconfig     |    7 +
> >  drivers/clk/qcom/Makefile    |    1 +
> >  drivers/clk/qcom/gcc-sdx55.c | 1626 ++++++++++++++++++++++++++++++++++
> >  3 files changed, 1634 insertions(+)
> >  create mode 100644 drivers/clk/qcom/gcc-sdx55.c
> > 
> > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> > index 3a965bd326d5..7897a3947e6d 100644
> > --- a/drivers/clk/qcom/Kconfig
> > +++ b/drivers/clk/qcom/Kconfig
> > @@ -413,6 +413,13 @@ config SDM_LPASSCC_845
> >  	  Say Y if you want to use the LPASS branch clocks of the LPASS clock
> >  	  controller to reset the LPASS subsystem.
> >  
> > +config SDX_GCC_55
> > +	tristate "SDX55 Global Clock Controller"
> > +	help
> > +	  Support for the global clock controller on SDX55 devices.
> > +	  Say Y if you want to use peripheral devices such as UART,
> > +	  SPI, I2C, USB, SD/UFS, PCIe etc.
> > +
> >  config SM_DISPCC_8250
> >  	tristate "SM8150 and SM8250 Display Clock Controller"
> >  	depends on SM_GCC_8150 || SM_GCC_8250
> > diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> > index 11ae86febe87..886b877e70c7 100644
> > --- a/drivers/clk/qcom/Makefile
> > +++ b/drivers/clk/qcom/Makefile
> > @@ -64,6 +64,7 @@ obj-$(CONFIG_SDM_GCC_845) += gcc-sdm845.o
> >  obj-$(CONFIG_SDM_GPUCC_845) += gpucc-sdm845.o
> >  obj-$(CONFIG_SDM_LPASSCC_845) += lpasscc-sdm845.o
> >  obj-$(CONFIG_SDM_VIDEOCC_845) += videocc-sdm845.o
> > +obj-$(CONFIG_SDX_GCC_55) += gcc-sdx55.o
> >  obj-$(CONFIG_SM_DISPCC_8250) += dispcc-sm8250.o
> >  obj-$(CONFIG_SM_GCC_8150) += gcc-sm8150.o
> >  obj-$(CONFIG_SM_GCC_8250) += gcc-sm8250.o
> > diff --git a/drivers/clk/qcom/gcc-sdx55.c b/drivers/clk/qcom/gcc-sdx55.c
> > new file mode 100644
> > index 000000000000..bf114165e24b
> > --- /dev/null
> > +++ b/drivers/clk/qcom/gcc-sdx55.c
> > @@ -0,0 +1,1626 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2018-2019, The Linux Foundation. All rights reserved.
> > + * Copyright (c) 2020, Linaro Ltd.
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +
> > +#include <dt-bindings/clock/qcom,gcc-sdx55.h>
> > +
> > +#include "common.h"
> > +#include "clk-alpha-pll.h"
> > +#include "clk-branch.h"
> > +#include "clk-pll.h"
> > +#include "clk-rcg.h"
> > +#include "clk-regmap.h"
> > +#include "reset.h"
> > +
> > +enum {
> > +	P_BI_TCXO,
> > +	P_CORE_BI_PLL_TEST_SE,
> 
> This is for test and we removed this for upstream, so can you do that as
> well (not parent will decrease for clks below)
> 

We have several other platforms that includes the bi_pll_test clock -
and it's there in the hardware, so I think we should just keep it.

Is it causing any issues?

Regards,
Bjorn

> With that updated:
> 
> Reviewed-by: Vinod Koul <vkoul@kernel.org>
> 
> -- 
> ~Vinod